-
Notifications
You must be signed in to change notification settings - Fork 154
fix: resolve subscription post number rollback toggle not saving #1777
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
base: develop
Are you sure you want to change the base?
fix: resolve subscription post number rollback toggle not saving #1777
Conversation
WalkthroughAdds handling and persistence for a new subscription meta Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
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: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
includes/Api/Subscription.php
🧰 Additional context used
🧬 Code graph analysis (1)
includes/Api/Subscription.php (1)
includes/Traits/FieldableTrait.php (1)
update_post_meta(367-477)
🪛 GitHub Actions: Inspections
includes/Api/Subscription.php
[error] 1-1: Whitespace found at end of line.
🔇 Additional comments (1)
includes/Api/Subscription.php (1)
540-540: Implementation looks correct.The persistence logic correctly follows the established pattern for saving subscription meta fields. The sanitized value from line 463 is properly saved using
update_post_meta().
Updated logic in Paypal gateway and Subscription API to ensure 'postnum_rollback_on_delete' is set to 'yes' only if explicitly set, otherwise defaults to an empty string. This improves consistency in how the value is handled across the codebase.
Refactored the logic for 'postnum_rollback_on_delete' to directly use the value from metadata if set, and sanitized it in the API. Also marked the 'postnum_rollback_on_delete' option as a pro feature in the admin subscription settings.
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: 0
🧹 Nitpick comments (1)
includes/Api/Subscription.php (1)
463-465: Consider normalizing toggle value to enforce 'yes' or '' only.The extraction logic is consistent with other toggle fields in the codebase. However, it accepts any string value after sanitization rather than explicitly normalizing to the expected toggle values (
'yes'or''). This could allow inconsistent values like'no','false', or'off'to be stored if sent from the frontend.🔎 Proposed normalization logic
- $postnum_rollback_on_delete = ! empty( $subscription['meta_value']['postnum_rollback_on_delete'] ) ? sanitize_text_field( - $subscription['meta_value']['postnum_rollback_on_delete'] - ) : ''; + $postnum_rollback_on_delete = ''; + if ( ! empty( $subscription['meta_value']['postnum_rollback_on_delete'] ) ) { + $value = sanitize_text_field( $subscription['meta_value']['postnum_rollback_on_delete'] ); + $postnum_rollback_on_delete = ( 'yes' === $value ) ? 'yes' : ''; + }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Lib/Gateway/Paypal.phpincludes/Admin/Admin_Subscription.phpincludes/Api/Subscription.php
🚧 Files skipped from review as they are similar to previous changes (1)
- Lib/Gateway/Paypal.php
🔇 Additional comments (2)
includes/Admin/Admin_Subscription.php (1)
1342-1342: LGTM! Pro feature flag correctly added.The addition of the
is_proflag follows the established pattern (consistent with line 1405) and correctly marks the post number rollback feature as Pro-only.includes/Api/Subscription.php (1)
540-540: LGTM!The persistence logic correctly saves the
postnum_rollback_on_deletevalue and follows the established pattern of other meta field updates in this method.
fix: resolve subscription post number rollback toggle not saving
Close 1342
The postnum_rollback_on_delete meta field was missing from the REST API
save handler in includes/Api/Subscription.php. Added extraction and
persistence of the field value when creating or updating subscriptions
via the Vue.js-based subscription editor.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.