-
Notifications
You must be signed in to change notification settings - Fork 151
Feat/paypal library upgrade #1578
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
Feat/paypal library upgrade #1578
Conversation
WalkthroughThe exclusion of the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
includes/Admin/Subscription.php (2)
62-72: 🛠️ Refactor suggestion
⚠️ Potential issueGood SQL fix with unused variable.
The SQL query fix correctly separates
prepare()fromget_row(). However, the$sqlvariable defined on line 62 is never used and should be removed.Remove the unused variable:
-$sql = $wpdb->prepare( - 'SELECT transaction_id FROM ' . $wpdb->prefix . 'wpuf_transaction - WHERE user_id = %d AND pack_id = %d LIMIT 1', $user_id, $pack_id -); $result = $wpdb->get_row( $wpdb->prepare( 'SELECT transaction_id FROM ' . $wpdb->prefix . 'wpuf_transaction WHERE user_id = %d AND pack_id = %d LIMIT 1', $user_id, $pack_id ) );
759-772: 🛠️ Refactor suggestion
⚠️ Potential issueConsistent SQL fix needed.
Good fix for the nested prepare issue, but like the previous SQL fix, the unused
$sqlvariable should be removed.Apply the same cleanup:
-$sql = $wpdb->prepare( - "SELECT p.ID, p.post_status - FROM $wpdb->posts p, $wpdb->postmeta m - WHERE p.ID = m.post_id AND p.post_status <> 'publish' AND m.meta_key = '_wpuf_order_id' AND m.meta_value = %s", $order_id -); - return $wpdb->get_row( $wpdb->prepare( "SELECT p.ID, p.post_status FROM $wpdb->posts p, $wpdb->postmeta m WHERE p.ID = m.post_id AND p.post_status <> 'publish' AND m.meta_key = '_wpuf_order_id' AND m.meta_value = %s", $order_id ) );
🧹 Nitpick comments (7)
admin/form-builder/assets/js/components/field-multiselect/index.js (2)
27-61: Excellent dynamic options implementation with room for modernization.The new
dynamic_optionscomputed property effectively generates taxonomy-specific options. However, the code can be modernized for better safety and readability.Apply these improvements for modern JavaScript best practices:
// Dynamic options for taxonomy fields dynamic_options: function () { // Check if this is a Selection Terms field for a taxonomy - if (this.option_field.name === 'exclude' && - this.editing_form_field && - this.editing_form_field.input_type === 'taxonomy' && - this.editing_form_field.name) { + if (this.option_field.name === 'exclude' && + this.editing_form_field?.input_type === 'taxonomy' && + this.editing_form_field?.name) { var taxonomy_name = this.editing_form_field.name; // Look for terms in the wp_post_types data - if (wpuf_form_builder && wpuf_form_builder.wp_post_types) { + if (wpuf_form_builder?.wp_post_types) { for (var post_type in wpuf_form_builder.wp_post_types) { var taxonomies = wpuf_form_builder.wp_post_types[post_type]; - if (taxonomies && taxonomies.hasOwnProperty(taxonomy_name)) { + if (taxonomies && Object.hasOwn(taxonomies, taxonomy_name)) { var tax_field = taxonomies[taxonomy_name]; - if (tax_field && tax_field.terms && tax_field.terms.length > 0) { + if (tax_field?.terms?.length > 0) { var options = {}; tax_field.terms.forEach(function(term) { - if (term && term.term_id && term.name) { + if (term?.term_id && term?.name) { options[term.term_id] = term.name; } }); return options; } } } } } // Return original options if not a taxonomy field or no dynamic options found return this.option_field.options || {}; }🧰 Tools
🪛 Biome (1.9.4)
[error] 38-38: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 42-42: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 42-42: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
[error] 45-45: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 48-48: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
81-89: Excellent selectize management with minor improvement opportunity.The logic to destroy existing selectize instances prevents memory leaks and duplicate bindings. The implementation can be slightly modernized.
Apply this minor improvement using optional chaining:
// Destroy existing selectize if it exists var $select = $(this.$el).find('.term-list-selector'); - if ($select[0] && $select[0].selectize) { + if ($select[0]?.selectize) { $select[0].selectize.destroy(); }🧰 Tools
🪛 Biome (1.9.4)
[error] 83-83: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
assets/js/wpuf-form-builder-components.js (1)
446-480: Approve the dynamic options implementation with minor suggestions.The implementation correctly handles dynamic population of taxonomy terms. However, consider these improvements:
- Use optional chaining as suggested by static analysis to make the code more concise and safer
- Use
Object.hasOwn()instead ofhasOwnPropertyfor better securityApply this diff to improve the code:
- if (this.option_field.name === 'exclude' && - this.editing_form_field && - this.editing_form_field.input_type === 'taxonomy' && - this.editing_form_field.name) { + if (this.option_field.name === 'exclude' && + this.editing_form_field?.input_type === 'taxonomy' && + this.editing_form_field?.name) { var taxonomy_name = this.editing_form_field.name; // Look for terms in the wp_post_types data - if (wpuf_form_builder && wpuf_form_builder.wp_post_types) { + if (wpuf_form_builder?.wp_post_types) { for (var post_type in wpuf_form_builder.wp_post_types) { var taxonomies = wpuf_form_builder.wp_post_types[post_type]; - if (taxonomies && taxonomies.hasOwnProperty(taxonomy_name)) { + if (taxonomies && Object.hasOwn(taxonomies, taxonomy_name)) { var tax_field = taxonomies[taxonomy_name]; - if (tax_field && tax_field.terms && tax_field.terms.length > 0) { + if (tax_field?.terms?.length > 0) { var options = {}; tax_field.terms.forEach(function(term) { - if (term && term.term_id && term.name) { + if (term?.term_id && term?.name) { options[term.term_id] = term.name; } });🧰 Tools
🪛 Biome (1.9.4)
[error] 457-457: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 461-461: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 461-461: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
[error] 464-464: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 467-467: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
templates/dashboard/subscription.php (2)
14-70: Well-structured subscription data retrieval with proper security measures.The function correctly uses
$wpdb->prepare()for SQL queries, preventing SQL injection. The queries are well-formatted and the return structure is clear. However, consider adding type declarations for better code documentation.Consider adding return type declaration for PHP 7+:
- function wpuf_dashboard_get_subscription_data( $user_sub ) { + function wpuf_dashboard_get_subscription_data( array $user_sub ) : array {
241-258: Consider extracting inline JavaScript to improve maintainability.While the show more/less functionality works correctly, inline JavaScript can be harder to maintain and test. Consider moving this to an external JavaScript file or at least wrapping it in a more structured approach.
Consider creating a reusable JavaScript component:
// In an external JS file wpuf.subscriptionToggle = { init: function(toggleId, hiddenClass) { var btn = document.getElementById(toggleId); if (!btn) return; var hidden = document.querySelectorAll('.' + hiddenClass); var expanded = false; btn.addEventListener('click', function() { expanded = !expanded; hidden.forEach(function(el) { el.style.display = expanded ? 'block' : 'none'; }); btn.textContent = expanded ? btn.dataset.showLess : btn.dataset.showMore; }); // Initially hide elements hidden.forEach(function(el) { el.style.display = 'none'; }); } };Then in the template:
<button type="button" id="wpuf-remaining-posts-toggle" class="btn btn-link" style="padding:0;" data-show-more="<?php echo esc_attr__( 'Show More', 'wp-user-frontend' ); ?>" data-show-less="<?php echo esc_attr__( 'Show Less', 'wp-user-frontend' ); ?>"> <?php esc_html_e( 'Show More', 'wp-user-frontend' ); ?> </button> <script> wpuf.subscriptionToggle.init('wpuf-remaining-posts-toggle', 'wpuf-remaining-post-hidden'); </script>admin/form-builder/assets/js/form-builder.js (1)
1128-1174: Excellent refactoring with one minor issue.The refactoring significantly improves the robustness of the taxonomy field handling:
- ✅ Guard clause prevents unnecessary AJAX calls
- ✅ More reliable DOM manipulation
- ✅ Better error handling
- ✅ Proper selectize initialization
However, the
isInitialLoadparameter is never used within the function. Consider either utilizing it for different behavior between initial load and updates, or removing it if not needed.If the
isInitialLoadparameter is truly not needed, consider simplifying:-function populate_default_categories(obj, isInitialLoad) { +function populate_default_categories(obj) {And update the calling code accordingly:
-populate_default_categories($postTypeSelect[0], true); +populate_default_categories($postTypeSelect[0]);-populate_default_categories(this, false); +populate_default_categories(this);includes/Admin/Subscription.php (1)
401-403: Remove or document commented code.The commented validation check should either be removed if no longer needed, or include a comment explaining why it's disabled.
Either remove the commented code or add explanation:
-// if ( ! isset( $post_data['billing_amount'] ) ) { -// return; -// } +// TODO: Billing amount validation removed as it's now optional - see issue #XXX
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
assets/js/forms-list.min.jsis excluded by!**/*.min.jsassets/js/forms-list.min.js.mapis excluded by!**/*.map,!**/*.min.js.mapassets/js/subscriptions.min.jsis excluded by!**/*.min.jsassets/js/subscriptions.min.js.mapis excluded by!**/*.map,!**/*.min.js.map
📒 Files selected for processing (23)
admin/form-builder/assets/js/components/field-multiselect/index.js(1 hunks)admin/form-builder/assets/js/components/field-multiselect/template.php(1 hunks)admin/form-builder/assets/js/form-builder.js(1 hunks)assets/css/admin/form-builder.css(4 hunks)assets/js-templates/form-components.php(1 hunks)assets/js/components/FormsList.vue(3 hunks)assets/js/components/subscriptions/SectionInputField.vue(3 hunks)assets/js/components/subscriptions/SubscriptionBox.vue(1 hunks)assets/js/stores/subscription.js(5 hunks)assets/js/subscriptions-old.js(1 hunks)assets/js/wpuf-form-builder-components.js(1 hunks)assets/js/wpuf-form-builder.js(1 hunks)class/subscription.php(9 hunks)includes/Admin/Admin_Subscription.php(1 hunks)includes/Admin/Menu.php(1 hunks)includes/Admin/Subscription.php(10 hunks)includes/Ajax/Admin_Form_Builder_Ajax.php(2 hunks)includes/Frontend/Payment.php(0 hunks)includes/User_Subscription.php(6 hunks)languages/wp-user-frontend.pot(12 hunks)readme.md(1 hunks)readme.txt(1 hunks)templates/dashboard/subscription.php(2 hunks)
💤 Files with no reviewable changes (1)
- includes/Frontend/Payment.php
✅ Files skipped from review due to trivial changes (3)
- readme.md
- languages/wp-user-frontend.pot
- readme.txt
🧰 Additional context used
🧬 Code Graph Analysis (1)
includes/Admin/Subscription.php (4)
class/subscription.php (1)
subs_redirect_pram(134-169)includes/User_Subscription.php (1)
current_pack(53-73)wpuf-functions.php (2)
wpuf_is_checkbox_or_toggle_on(5457-5459)wpuf_get_user(3361-3367)includes/WPUF_User.php (1)
post_locked(52-54)
🪛 Biome (1.9.4)
admin/form-builder/assets/js/components/field-multiselect/index.js
[error] 38-38: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 42-42: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 42-42: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 45-45: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 48-48: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 83-83: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
assets/js/wpuf-form-builder-components.js
[error] 457-457: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 461-461: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 461-461: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 464-464: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 467-467: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 502-502: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (44)
assets/css/admin/form-builder.css (4)
3691-3699: New yellow border utilities added
The.wpuf-border-yellow-200and.wpuf-border-yellow-300classes align with existing yellow border utilities. Ensure these are documented in your style guide for consistency.
3727-3730: Amber background utility introduced
The.wpuf-bg-amber-500class adds the intended amber tone. Confirm this matches the design specs and update any related component documentation.
5386-5389: Subtle yellow hover background utility
The.hover\:wpuf-bg-yellow-100:hoverutility provides a light yellow hover effect. Verify it respects accessibility contrast guidelines and doesn’t clash with existing hover styles.
5585-5588: Yellow focus ring and offset utilities
Adding.focus\:wpuf-ring-yellow-500:focuswith.focus\:wpuf-ring-offset-2:focusenhances focus indication on interactive elements. Check that this doesn’t override other focus styles and works across components.Also applies to: 5590-5592
assets/js/subscriptions-old.js (1)
300-300: LGTM! Syntax error fixed.The missing comma after the
changeExpirationFieldVisibilityfunction has been correctly added. This fixes the JavaScript object literal syntax error, ensuring valid structure within thesubscriptionobject.admin/form-builder/assets/js/components/field-multiselect/template.php (1)
16-16:❓ Verification inconclusive
Template updated to use dynamic options.
The change from
option_field.optionstodynamic_optionsaligns the template with the computed property for dynamic option generation in taxonomy-related fields. This enables reactive updates for taxonomy exclusion fields.Please verify that the corresponding Vue component has the
dynamic_optionscomputed property implemented:
🏁 Script executed:
#!/bin/bash # Description: Verify dynamic_options computed property exists in multiselect component # Expected: Find computed property definition for dynamic_options ast-grep --pattern $'computed: { $$$ dynamic_options() { $$$ } $$$ }'Length of output: 82
🏁 Script executed:
#!/bin/bash # Search for dynamic_options or dynamicOptions in JS/Vue files to verify computed property rg --color=never -n "dynamic_options" -tjs -tvue rg --color=never -n "dynamicOptions" -tjs -tvueLength of output: 152
🏁 Script executed:
#!/bin/bash # List all files in the field-multiselect component directory echo "Directory contents:" find admin/form-builder/assets/js/components/field-multiselect -maxdepth 1 -type f # Search for props definitions in JS/Vue files echo "Looking for props:" rg --color=never -n "props" -C3 admin/form-builder/assets/js/components/field-multiselect/* || true # Search for computed definitions in JS/Vue files echo "Looking for computed properties:" rg --color=never -n "computed" -C3 admin/form-builder/assets/js/components/field-multiselect/* || trueLength of output: 1411
#!/bin/bash # Locate the option_field_mixin definition and inspect its computed properties echo "Searching for option_field_mixin definition..." mixin_files=$(rg -l "option_field_mixin" -tjs) if [[ -z "$mixin_files" ]]; then echo "No mixin file found for option_field_mixin." exit 0 fi echo "Found mixin files:" echo "$mixin_files" for file in $mixin_files; do echo -e "\n--- $file: computed properties ---" rg --color=never -n "computed" -C3 "$file" || echo "No computed block in $file." echo -e "\n--- $file: dynamic_options occurrences ---" rg --color=never -n "dynamic_options" -C3 "$file" || echo "No dynamic_options in $file." done
Confirm dynamic_options computed property exists
The switch from
option_field.optionstodynamic_optionscorrectly reflects the computed property approach for taxonomy-related fields, enabling reactive updates. Please verify that thedynamic_optionscomputed property is defined in the multiselect component (e.g., inindex.jsor its imported mixin).assets/js-templates/form-components.php (1)
295-295: Template consistency maintained.The change from
option_field.optionstodynamic_optionsin the multiselect template maintains consistency with the corresponding component template in the admin form builder.includes/Admin/Menu.php (1)
129-133: Enhanced permalink data for client-side logic.The addition of
is_plain_permalinkandpermalink_settings_urlto the localized script data provides valuable information for client-side permalink handling. Theis_plain_permalinkcorrectly checks for empty permalink structure, andpermalink_settings_urlgives easy access to the settings page.This enhancement supports dynamic REST API URL construction and user guidance when plain permalinks are detected.
assets/js/components/subscriptions/SubscriptionBox.vue (1)
75-81: LGTM! Proper REST API URL construction for subdirectory support.The dynamic REST API URL construction correctly addresses WordPress installations in subdirectories by using the localized
wpufSubscriptions.rest_urlinstead of hardcoded paths. The trailing slash removal ensures proper URL formatting.assets/js/stores/subscription.js (4)
150-156: Consistent REST API URL construction in updateSubscription method.The implementation correctly uses dynamic REST API root construction, maintaining consistency with other updated methods in the store.
323-325: Proper URL construction in deleteSubscription method.The REST API URL construction follows the established pattern and correctly handles the DELETE request endpoint.
Also applies to: 334-334
350-355: Correct implementation in setSubscriptionsByStatus method.The dynamic URL construction properly integrates with the existing
addQueryArgsusage for parameter handling.
378-380: Consistent pattern in getSubscriptionCount method.The REST API URL construction maintains consistency with other methods while properly handling the count endpoint path construction.
assets/js/components/subscriptions/SectionInputField.vue (3)
99-99: Minor code cleanup - blank line removal.Good cleanup removing the extraneous blank line in the switch statement.
243-243: Enhanced button styling with proper interactive states.The CSS class updates improve the "Upgrade to Pro" button by:
- Changing default background from
wpuf-bg-amber-600towpuf-bg-amber-500- Adding proper hover and focus states with
hover:wpuf-bg-amber-600andfocus:wpuf-bg-amber-600This provides consistent interactive feedback to users.
271-273: Dynamic numeric input attributes for better flexibility.The change from hardcoded
min="-1"to dynamic bindings usingfield.min,field.step, andfield.defaultmakes the component more configurable and reusable. This allows different numeric inputs to have appropriate constraints based on their field configuration.includes/Admin/Admin_Subscription.php (1)
123-123: LGTM! REST URL localization supports subdirectory installations.The addition of
'rest_url'to the localized script data is correctly implemented usingesc_url_raw( rest_url() )for proper escaping. This aligns with the dynamic REST API URL construction pattern seen in other components and supports WordPress installations in subdirectories.assets/js/components/FormsList.vue (3)
11-12: LGTM! Clean extraction of permalink configuration.The constants are properly extracted from the localized
wpuf_forms_listscript data, providing a clean way to access permalink configuration in the component.
39-41: LGTM! Robust REST API URL construction.The dynamic REST API URL construction correctly handles subdirectory installations by:
- Removing trailing slashes from the base URL
- Dynamically constructing the full endpoint URL
- Ensuring compatibility across different WordPress configurations
This is a significant improvement over hardcoded API URLs.
317-346: LGTM! Excellent user experience for permalink issues.The conditional warning notice is well-implemented with:
- Clear visual hierarchy using Tailwind classes
- Informative warning icon and messaging
- Direct link to permalink settings for resolution
- Professional styling consistent with the interface
This proactively helps users resolve potential REST API issues.
includes/Ajax/Admin_Form_Builder_Ajax.php (2)
155-164: LGTM! Proper form settings preservation.The code correctly preserves existing form settings by:
- Safely retrieving the form ID with
absint()- Fetching current settings from post meta
- Ensuring
$current_settingsis always an array- Handling cases where no form ID is provided
This prevents loss of existing configuration when post types are changed.
174-205: LGTM! Enhanced taxonomy handling with security improvements.The enhanced taxonomy handling provides significant improvements:
Security: Proper escaping with
esc_attr()andesc_html()for all output
Functionality: Preserves existing selections using the current settings
Structure: Better HTML with proper classes, IDs, and data attributes
Error handling: Checks foris_wp_error()and empty categoriesThe generated HTML is more robust and maintains user selections during post type changes.
includes/User_Subscription.php (4)
1-1: Excellent addition of strict type declaration!Adding
declare(strict_types=1)improves type safety and helps catch type-related errors early. This is a best practice for modern PHP development.
91-91: Good enhancement to expiration logic.Adding
-1 === intval( $expire_date )to the non-expired conditions is consistent with treating -1 as "unlimited" throughout the codebase. This improves the robustness of subscription expiration checks.
197-197: Improved recurring payment detection logic.The enhanced logic using
wpuf_is_checkbox_or_toggle_on( $subscription->meta_value['recurring_pay'] )provides more robust checking of the recurring payment meta value instead of relying on a simple boolean parameter.
206-206: Consistent unlimited value handling.Treating both
0and'-1'as unlimited in expiration calculations maintains consistency with the expiration logic updates throughout the class.class/subscription.php (6)
62-67: Excellent database query refactoring.Separating
$wpdb->prepare()from$wpdb->get_row()follows WordPress coding standards and improves code clarity. This is the recommended approach for prepared statements.
134-134: Method signature properly updated.The
subs_redirect_prammethod signature correctly matches the updated filter hook parameters, maintaining consistency.
867-867: Good defensive programming practice.Adding a conditional check before calling
strtolower()prevents potential errors when$payment_gatewayis null or falsy. This is a solid improvement to error handling.
872-872: Cleaner output formatting.Using
printf()instead ofecho sprintf()is more direct and efficient for formatted output. This is a good optimization.
1131-1131: Improved control flow clarity.Changing from
elsetoelseifwith an explicit condition makes the logic more readable and maintainable.
38-38:❓ Verification inconclusive
Verify filter hook parameter reduction impact.
The filter hook
wpuf_user_register_redirectwas reduced from 5 to 2 parameters. Ensure this change doesn't break existing implementations.
🏁 Script executed:
#!/bin/bash # Description: Check for usages of the wpuf_user_register_redirect filter to verify parameter compatibility # Search for filter hook usages rg -A 5 -B 2 "wpuf_user_register_redirect" --type phpLength of output: 1346
🏁 Script executed:
#!/bin/bash # Search for the wpuf_user_register_redirect hook invocation to inspect its parameter count rg -n "apply_filters.*wpuf_user_register_redirect" -R . rg -n "apply_filters_ref_array.*wpuf_user_register_redirect" -R .Length of output: 935
#!/bin/bash # Correct search for filter invocation of wpuf_user_register_redirect rg -n "apply_filters.*wpuf_user_register_redirect" .
Verify filter hook parameter reduction impact
I wasn’t able to locate any
apply_filters('wpuf_user_register_redirect', …)calls in the codebase, so please confirm that dropping from 5 to 2 arguments won’t break existing extensions or custom code.• Search for where the redirect URL is filtered (look for
apply_filters('wpuf_user_register_redirect') and ensure it only ever passes two parameters
• If third–fifth parameters are used elsewhere, update those calls or restore the full signature
• Consider bumping the plugin’s major version or documenting this as a breaking change for downstream consumersadmin/form-builder/assets/js/components/field-multiselect/index.js (1)
68-75: Good reactive design with watcher.The watcher on
dynamic_optionsensures the selectize component updates when taxonomy data changes. Using$nextTickis the correct approach to wait for DOM updates.assets/js/wpuf-form-builder-components.js (1)
500-508: Good implementation of selectize lifecycle management.The code properly destroys existing selectize instances before creating new ones, preventing memory leaks and stale state issues. The use of
$select[0].selectizeto check for existing instances is the correct approach.🧰 Tools
🪛 Biome (1.9.4)
[error] 502-502: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
assets/js/wpuf-form-builder.js (3)
1114-1126: Well-structured initialization logic.The implementation correctly handles both initial page load and change events. The use of a boolean parameter to differentiate between initial load and subsequent changes is a clean approach.
1128-1135: Good defensive programming with the guard clause.The early return when no post type is selected prevents unnecessary AJAX calls and potential errors. This is a good practice for performance and error prevention.
1157-1167: Proper selectize initialization with duplication prevention.The code correctly checks for already initialized selectize instances using
:not(.selectized)selector, preventing duplicate initialization. The comment about server-rendered HTML handling selected options is helpful for understanding the logic.templates/dashboard/subscription.php (2)
184-196: Good UI improvement for handling unlimited values.The code properly differentiates between limited and unlimited featured items and reusable blocks, displaying "Unlimited" text where appropriate. The use of strict comparison (
-1 == intval()) ensures proper type handling.
94-98:✅ Verification successful
Security: Add nonce verification in the form handler.
While the form includes a nonce field, ensure that the form submission handler properly verifies this nonce using
wp_verify_nonce()to prevent CSRF attacks.
🏁 Script executed:
#!/bin/bash # Search for the subscription cancellation handler to verify nonce checking rg -A 10 "wpuf_cancel_subscription.*POST|POST.*wpuf_cancel_subscription" --type phpLength of output: 3277
🏁 Script executed:
#!/bin/bash # Also search for the specific nonce verification rg -A 5 "wp_verify_nonce.*wpuf-sub-cancel|verify_nonce.*wpuf-sub-cancel" --type phpLength of output: 800
Nonce Verification Confirmed
The subscription cancellation handlers in bothclass/subscription.phpandincludes/Admin/Subscription.phpproperly sanitize the_wpnoncefield and verify it withwp_verify_nonce( $nonce, 'wpuf-sub-cancel' )returning early on failure. No further changes are needed.
admin/form-builder/assets/js/form-builder.js (1)
1113-1127: Good implementation of taxonomy initialization!The changes properly handle both initial page load and dynamic updates when the post type changes. The use of the
isInitialLoadflag is a clean way to differentiate between the two scenarios.includes/Admin/Subscription.php (4)
886-892: Good null safety improvement!The null check prevents potential PHP errors when
$payment_gatewayis null.
897-897: Correct usage of printf.Good fix -
printfis the appropriate function here since it outputs directly, makingecho sprintfredundant.
1190-1206: Improved code clarity with explicit condition.The change from
elsetoelseif ( ! wpuf_get_user()->post_locked() )makes the logic more explicit and easier to understand. Good improvement for maintainability.
42-42:✅ Verification successful
Verify the breaking change in filter hook parameters.
The
wpuf_user_register_redirectfilter has been reduced from 5 parameters to 2. This is a breaking change that could affect other code hooking into this filter.Please verify that no other code depends on the removed parameters:
Also applies to: 132-132
🏁 Script executed:
#!/bin/bash # Description: Search for other implementations using the wpuf_user_register_redirect filter # Search for add_filter calls with this hook rg "add_filter\s*\(\s*['\"]wpuf_user_register_redirect" -A 5 # Search for apply_filters calls to understand the actual usage rg "apply_filters\s*\(\s*['\"]wpuf_user_register_redirect" -A 5Length of output: 1029
No internal dependencies on
wpuf_user_register_redirecthook
Search across the repository only finds twoadd_filtercalls for this hook—inincludes/Admin/Subscription.phpandclass/subscription.php—both now correctly using 2 parameters. There are noapply_filtersinvocations forwpuf_user_register_redirect. Reducing the parameter count therefore does not break any existing internal code.
| continue; | ||
| } | ||
| $value = ( $value == '-1' ) ? __( 'Unlimited', 'wp-user-frontend' ) : $value; | ||
| $value = ( '-1' === $value ) ? __( 'Unlimited', 'wp-user-frontend' ) : $value; |
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.
Fix type inconsistency in comparison.
The comparison uses string '-1' but should be consistent with the integer -1 used elsewhere in the class. The intval() conversion earlier suggests integer handling is preferred.
Apply this fix to maintain type consistency:
- $value = ( '-1' === $value ) ? __( 'Unlimited', 'wp-user-frontend' ) : $value;
+ $value = ( -1 === $value ) ? __( 'Unlimited', 'wp-user-frontend' ) : $value;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $value = ( '-1' === $value ) ? __( 'Unlimited', 'wp-user-frontend' ) : $value; | |
| $value = ( -1 === $value ) ? __( 'Unlimited', 'wp-user-frontend' ) : $value; |
🤖 Prompt for AI Agents
In includes/User_Subscription.php at line 361, the comparison uses the string
'-1' which is inconsistent with the integer -1 used elsewhere. Change the
comparison to use the integer -1 instead of the string '-1' to maintain type
consistency with the earlier intval() conversion.
| // Check if the nonce is valid | ||
| if ( ! isset( $_GET['_wpnonce'] ) || ! wp_verify_nonce( sanitize_key( wp_unslash( $_GET['_wpnonce'] ) ), 'wpuf_register_form' ) ) { | ||
| return; | ||
| } | ||
|
|
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.
💡 Verification agent
🧩 Analysis chain
Excellent security improvement!
Adding nonce validation prevents CSRF attacks on the registration form.
Verify that the nonce is being generated with the correct action:
🏁 Script executed:
#!/bin/bash
# Description: Search for where the wpuf_register_form nonce is created
# Search for wp_nonce_url or wp_create_nonce with wpuf_register_form
rg "wp_(nonce_url|create_nonce)\s*\(\s*['\"]wpuf_register_form" -A 2
# Search for registration form generation to ensure nonce is included
rg "wpuf_register_form" --type php -A 5 -B 5Length of output: 946
🏁 Script executed:
#!/bin/bash
# Search for where the nonce field or nonce is generated
rg "wp_nonce_field\s*\(" -A 2
rg "wp_create_nonce\s*\(" -A 2
rg "wp_nonce_url\s*\(" -A 2Length of output: 34134
Nonce action mismatch – fix required
The verification in includes/Admin/Subscription.php is checking for _wpnonce with action "wpuf_register_form", but the registration template generates the nonce with action "wpuf_registration_action". These must match.
• In includes/Admin/Subscription.php (lines 177–181), either update the verify call to use "wpuf_registration_action":
- wp_verify_nonce( sanitize_key( wp_unslash( $_GET['_wpnonce'] ) ), 'wpuf_register_form' )
+ wp_verify_nonce( sanitize_key( wp_unslash( $_GET['_wpnonce'] ) ), 'wpuf_registration_action' )• Or, in templates/registration-form.php, change the field to:
<?php wp_nonce_field( 'wpuf_register_form' ); ?>Please align the action names so the nonce check passes as intended.
🤖 Prompt for AI Agents
In includes/Admin/Subscription.php around lines 177 to 181, the nonce
verification uses the action "wpuf_register_form" which does not match the nonce
action "wpuf_registration_action" used in the registration template. To fix
this, update the wp_verify_nonce call to use "wpuf_registration_action" as the
action parameter so that the nonce verification aligns with the nonce generation
and passes correctly.
f9e8a23 to
b35a1a7
Compare
Description :
-Settings updated for paypal (user experience update)
Summary by CodeRabbit