-
Notifications
You must be signed in to change notification settings - Fork 146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
enhance: support for additional CPT's #1483
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -293,6 +293,7 @@ public static function get_subscription_meta( $subscription_id, $pack_post = nul | |||||||||
$meta['trial_duration'] = get_post_meta( $subscription_id, '_trial_duration', true ); | ||||||||||
$meta['trial_duration_type'] = get_post_meta( $subscription_id, '_trial_duration_type', true ); | ||||||||||
$meta['post_type_name'] = get_post_meta( $subscription_id, '_post_type_name', true ); | ||||||||||
$meta['additional_cpt_options'] = get_post_meta( $subscription_id, 'additional_cpt_options', true ); | ||||||||||
$meta['_enable_post_expiration'] = get_post_meta( $subscription_id, '_enable_post_expiration', true ); | ||||||||||
$meta['_post_expiration_time'] = get_post_meta( $subscription_id, '_post_expiration_time', true ); | ||||||||||
$meta['_expired_post_status'] = get_post_meta( $subscription_id, '_expired_post_status', true ); | ||||||||||
|
@@ -418,6 +419,7 @@ public function save_form_meta( $subscription_id, $post ) { | |||||||||
update_post_meta( $subscription_id, '_trial_duration', $trial_duration ); | ||||||||||
update_post_meta( $subscription_id, '_trial_duration_type', $trial_duration_type ); | ||||||||||
update_post_meta( $subscription_id, '_post_type_name', array_map( 'sanitize_text_field', $post_data['post_type_name'] ) ); | ||||||||||
update_post_meta( $subscription_id, 'additional_cpt_options', array_map( 'sanitize_text_field', $post_data['additional_cpt_options'] ) ); | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure To prevent potential undefined index notices, please check if Apply this diff to add the necessary check: +if ( isset( $post_data['additional_cpt_options'] ) ) {
update_post_meta( $subscription_id, 'additional_cpt_options', array_map( 'sanitize_text_field', $post_data['additional_cpt_options'] ) );
+} 📝 Committable suggestion
Suggested change
|
||||||||||
update_post_meta( $subscription_id, '_enable_post_expiration', $enable_post_expir ); | ||||||||||
update_post_meta( $subscription_id, '_post_expiration_time', $expiration_time ); | ||||||||||
update_post_meta( $subscription_id, '_expired_post_status', $expire_post_status ); | ||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -66,17 +66,18 @@ public function third_party_cpt_options( $additional_options ) { | |||||||||||||||||||||
|
||||||||||||||||||||||
if ( $post_type_object ) { | ||||||||||||||||||||||
$additional_options['additional'][ $key ] = [ | ||||||||||||||||||||||
'id' => $key, | ||||||||||||||||||||||
'name' => $key, | ||||||||||||||||||||||
'db_key' => $key, | ||||||||||||||||||||||
'db_type' => 'meta', | ||||||||||||||||||||||
'type' => 'input-number', | ||||||||||||||||||||||
'label' => sprintf( 'Number of %s', esc_html( $post_type_object->label ) ), | ||||||||||||||||||||||
'tooltip' => sprintf( | ||||||||||||||||||||||
'id' => $key, | ||||||||||||||||||||||
'name' => $key, | ||||||||||||||||||||||
'db_key' => 'additional_cpt_options', | ||||||||||||||||||||||
'db_type' => 'meta_serialized', | ||||||||||||||||||||||
'serialize_key' => $key, | ||||||||||||||||||||||
Comment on lines
+69
to
+73
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Improper Use of Meta Keys and Serialization The array keys Consider refactoring to ensure that each custom post type option has a unique and coherent meta key. For example: - 'db_key' => 'additional_cpt_options',
- 'db_type' => 'meta_serialized',
- 'serialize_key' => $key,
+ 'db_key' => 'additional_cpt_options_' . $key,
+ 'db_type' => 'meta',
+ 'serialize_key' => null, This change assigns a unique meta key for each post type option and avoids complex serialization. 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||
'type' => 'input-number', | ||||||||||||||||||||||
'label' => sprintf( 'Number of %s', esc_html( $post_type_object->label ) ), | ||||||||||||||||||||||
'tooltip' => sprintf( | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Internationalization (i18n) Issue in String Formatting The usage of Update the code to wrap the strings with translation functions: - 'label' => sprintf( 'Number of %s', esc_html( $post_type_object->label ) ),
- 'tooltip' => sprintf(
- 'Set the maximum number of %s users can create within their subscription period. Enter -1 for unlimited',
+ 'label' => sprintf( __( 'Number of %s', 'wp-user-frontend' ), esc_html( $post_type_object->label ) ),
+ 'tooltip' => sprintf(
+ __( 'Set the maximum number of %s users can create within their subscription period. Enter -1 for unlimited', 'wp-user-frontend' ),
|
||||||||||||||||||||||
'Set the maximum number of %s users can create within their subscription period. Enter -1 for unlimited', | ||||||||||||||||||||||
esc_html( $key ) | ||||||||||||||||||||||
), | ||||||||||||||||||||||
'default' => '-1', | ||||||||||||||||||||||
'default' => '-1', | ||||||||||||||||||||||
]; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
@@ -809,9 +810,6 @@ public function profile_subscription_details( $profileuser ) { | |||||||||||||||||||||
$_post_expiration_time = explode( ' ', isset( $user_sub['_post_expiration_time'] ) ? $user_sub['_post_expiration_time'] : '' ); | ||||||||||||||||||||||
$time_value = isset( $_post_expiration_time[0] ) && ! empty( $_post_expiration_time[0] ) ? $_post_expiration_time[0] : '1'; | ||||||||||||||||||||||
$time_type = isset( $_post_expiration_time[1] ) && ! empty( $_post_expiration_time[1] ) ? $_post_expiration_time[1] : 'day'; | ||||||||||||||||||||||
|
||||||||||||||||||||||
error_log( print_r( $_post_expiration_time, true ) ); | ||||||||||||||||||||||
error_log( print_r( $time_type, true ) ); | ||||||||||||||||||||||
?> | ||||||||||||||||||||||
<tr> | ||||||||||||||||||||||
<th><label><?php esc_html_e( 'Post Expiration Enabled', 'wp-user-frontend' ); ?></label></th> | ||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -304,6 +304,7 @@ public static function get_subscription_meta( $subscription_id, $pack_post = nul | |||||||||||
$meta['_trial_duration_type'] = $meta['trial_duration_type']; | ||||||||||||
$meta['post_type_name'] = get_post_meta( $subscription_id, '_post_type_name', true ); | ||||||||||||
$meta['_post_type_name'] = $meta['post_type_name']; | ||||||||||||
$meta['additional_cpt_options'] = get_post_meta( $subscription_id, 'additional_cpt_options', true ); | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Use consistent meta key naming with underscore prefix In the existing code, meta keys are stored with an underscore prefix (e.g., Apply this diff to adjust the meta key: -$meta['additional_cpt_options'] = get_post_meta( $subscription_id, 'additional_cpt_options', true );
+$meta['_additional_cpt_options'] = get_post_meta( $subscription_id, '_additional_cpt_options', true );
+$meta['additional_cpt_options'] = $meta['_additional_cpt_options']; 📝 Committable suggestion
Suggested change
|
||||||||||||
$meta['_enable_post_expiration'] = get_post_meta( $subscription_id, '_enable_post_expiration', true ); | ||||||||||||
$meta['_post_expiration_time'] = get_post_meta( $subscription_id, '_post_expiration_time', true ); | ||||||||||||
$meta['_post_expiration_number'] = get_post_meta( $subscription_id, '_post_expiration_number', true ); | ||||||||||||
|
@@ -431,6 +432,7 @@ public function save_form_meta( $subscription_id, $post ) { | |||||||||||
update_post_meta( $subscription_id, '_trial_duration', $trial_duration ); | ||||||||||||
update_post_meta( $subscription_id, '_trial_duration_type', $trial_duration_type ); | ||||||||||||
update_post_meta( $subscription_id, '_post_type_name', array_map( 'sanitize_text_field', $post_data['post_type_name'] ) ); | ||||||||||||
update_post_meta( $subscription_id, 'additional_cpt_options', array_map( 'sanitize_text_field', $post_data['additional_cpt_options'] ) ); | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add check for 'additional_cpt_options' existence before updating meta To prevent undefined index notices, ensure that Apply this diff to add the necessary check and adjust the meta key: +if ( ! empty( $post_data['additional_cpt_options'] ) && is_array( $post_data['additional_cpt_options'] ) ) {
+ $sanitized_options = array_map( 'sanitize_text_field', $post_data['additional_cpt_options'] );
+ update_post_meta( $subscription_id, '_additional_cpt_options', $sanitized_options );
+} 📝 Committable suggestion
Suggested change
|
||||||||||||
update_post_meta( $subscription_id, '_enable_post_expiration', $enable_post_expir ); | ||||||||||||
update_post_meta( $subscription_id, '_post_expiration_time', $expiration_time ); | ||||||||||||
update_post_meta( $subscription_id, '_expired_post_status', $expire_post_status ); | ||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -19,6 +19,10 @@ class Frontend_Form_Ajax { | |||||||||||||||||||||
private $expired_post_status = 'wpuf-expired_post_status'; | ||||||||||||||||||||||
|
||||||||||||||||||||||
private $post_expiration_message = 'wpuf-post_expiration_message'; | ||||||||||||||||||||||
/** | ||||||||||||||||||||||
* @var array | ||||||||||||||||||||||
*/ | ||||||||||||||||||||||
private $form_fields; | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Proper Documentation for New Property The new private property Apply this diff to add documentation: /**
- * @var array
+ * An array of form fields retrieved from the form configuration.
+ *
+ * @var array
*/
private $form_fields; 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||
|
||||||||||||||||||||||
/** | ||||||||||||||||||||||
* New/Edit post submit handler | ||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -384,7 +384,7 @@ function wpuf_tax_rate_country_state( $country, $state ) { | |||||
continue; | ||||||
} | ||||||
if ( $rate['state'] === $state && $rate['country'] === $country ) { | ||||||
$tax_amount = $rate['rate']; | ||||||
$tax_amount = ! empty( $rate['rate'] ) ? $rate['rate'] : 0; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct the check for 'rate' to accurately handle zero values Using Apply this diff to fix the condition: - $tax_amount = ! empty( $rate['rate'] ) ? $rate['rate'] : 0;
+ $tax_amount = isset( $rate['rate'] ) ? $rate['rate'] : 0; 📝 Committable suggestion
Suggested change
|
||||||
} | ||||||
|
||||||
if ( intval( $tax_amount ) === 0 && $rate['country'] === $country && 'country_wide' === $rate['state'] ) { | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -56,7 +56,7 @@ | |||||
if ( ! isset( $this->pack['pack_id'] ) ) { | ||||||
$pack_page = get_permalink( wpuf_get_option( 'subscription_page', 'wpuf_payment' ) ); | ||||||
|
||||||
return new WP_Error( 'no-pack', sprintf( __( 'You must <a href="%s">purchase a subscription package</a> before posting', 'wp-user-frontend' ), $pack_page ) ); | ||||||
Check failure on line 59 in includes/User_Subscription.php GitHub Actions / Run PHPCS inspection
|
||||||
} | ||||||
|
||||||
// seems like the user has a pack, now check expiration | ||||||
|
@@ -175,10 +175,12 @@ | |||||
$result = ''; | ||||||
$subscription = wpuf()->subscription->get_subscription( $pack_id ); | ||||||
|
||||||
$additional_cpt_options = ! empty( $subscription->meta_value['additional_cpt_options'] ) ? $subscription->meta_value['additional_cpt_options'] : []; | ||||||
|
||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Use Null Coalescing Operator for Default Value Assignment For improved readability and cleaner code, consider using the null coalescing operator Apply this diff: -$additional_cpt_options = ! empty( $subscription->meta_value['additional_cpt_options'] ) ? $subscription->meta_value['additional_cpt_options'] : [];
+$additional_cpt_options = $subscription->meta_value['additional_cpt_options'] ?? []; 📝 Committable suggestion
Suggested change
|
||||||
if ( $this->user->id && $subscription ) { | ||||||
$user_meta = [ | ||||||
'pack_id' => $pack_id, | ||||||
'posts' => $subscription->meta_value['post_type_name'], | ||||||
'posts' => array_merge( $subscription->meta_value['post_type_name'], $additional_cpt_options ), | ||||||
sapayth marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
'total_feature_item' => $subscription->meta_value['_total_feature_item'], | ||||||
'remove_feature_item' => $subscription->meta_value['_remove_feature_item'], | ||||||
'status' => $status, | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Conversion of
meta_value[key]
from String to Object Affects Multiple ComponentsThe verification indicates that
meta_value
is utilized in various parts of the codebase. Changingmeta_value[key]
from a string to an object in themodifyCurrentSubscription
method may lead to inconsistencies and unexpected behaviors in other components that expect it to be a string.Potential Issues:
meta_value[key]
is a string, leading to type errors or unexpected behavior.Recommendations:
meta_value
is used to ensure they handle the new object structure appropriately.meta_value[key]
is a string in the first place and address any underlying data handling inconsistencies.🔗 Analysis chain
Ensure data integrity when converting
meta_value[key]
to an objectThe added type check and conversion for
meta_value[key]
prevents errors when assigning properties to a string. However, there are some potential issues to consider:meta_value[key]
to be a string.Consider the following improvements:
meta_value[key]
might be a string and address the root cause if possible.To ensure this change doesn't introduce regressions, please run the following script to check for other occurrences of
meta_value
usage:This will help identify other parts of the code that might be affected by this change.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 11996