-
Notifications
You must be signed in to change notification settings - Fork 2
Fix inconsistent subscription status after cancellation with centralized cancellation logic #88
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
Conversation
…tatus synchronization - Introduced a new method `handle_subscription_cancellation` to manage subscription cancellations consistently across orders. - Added `sync_parent_order_status` method to update the parent order status when all associated subscriptions are cancelled. - Updated existing cancellation logic to utilize the new centralized handling for improved maintainability.
|
Confirmed in unit testing, does anyone have access to trying on local machine with the paid subscription plugin? @diegocurbelo @peterwilsoncc @joemcgill @dkotter |
iamdharmesh
left a 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.
Thanks a lot for the PR, @lmcrean. While reviewing the PR, I noticed that the issue report(#78) didn’t include the background details and all the necessary information (apologies for that). I’ve added additional context and details about the issue here.
While testing this PR, I also noticed that it doesn’t fully resolve the issue. Could you please review the additional information I’ve shared and confirm?
Thanks again for your contribution!
Co-authored-by: Darin Kotter <[email protected]>
Co-authored-by: Darin Kotter <[email protected]>
…e presence in order
…h unconfirmed payments - Introduced a new method `maybe_skip_pending_cancel_status` to handle subscription cancellations more effectively. - This method ensures that subscriptions are cancelled immediately if the associated GoCardless payment is unconfirmed, aligning behavior with user expectations. - Added logging for cases where pending-cancel is skipped due to unconfirmed payment status.
|
Thanks @iamdharmesh for the context! I can see that the initial PR handled webhook-based cancellations but missed user-initiated cancellations with unconfirmed payments. So I've added a fix for the missing piece. This completes the behavior described in your summary:
Let me know if you'd like me to adjust anything |
iamdharmesh
left a 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.
Thanks for the changes @lmcrean. I have added some more comments could you please check it once?
…sitions - Updated `sync_parent_order_status` method to immediately cancel subscriptions with unconfirmed payments during pending-cancel transitions. - Removed the `maybe_skip_pending_cancel_status` method as its functionality is now integrated into the cancellation logic. - Improved logging for better tracking of subscription cancellations related to payment confirmation status.
|
thanks @iamdharmesh could i get your review please? Addressed your suggestions |
iamdharmesh
left a 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.
Thanks a lot for the changes @lmcrean and sorry for the delayed review. I was on vacation, just came back this week. Changes now looks good. However, added some feedback to check. Could you please check it once. Thanks.
…ion checks - Enhanced the `sync_parent_order_status` method to check the most recent order's payment status (parent or renewal) during pending-cancel transitions. - Removed the centralized `handle_subscription_cancellation` method to streamline cancellation handling directly within the order processing logic. - Improved logging messages for clarity regarding subscription cancellations due to unconfirmed payments.
- Updated the order check condition to ensure that the variable is an instance of `WC_Abstract_Order` before proceeding with payment status retrieval. - This change improves the robustness of payment confirmation checks during subscription processing.
- Updated the logic to fetch the payment status directly from the GoCardless API during pending-cancel transitions, improving reliability by addressing potential webhook delays. - Added error handling and logging for cases where payment retrieval fails, ensuring safe defaults are maintained.
|
@iamdharmesh thanks as always for the codebase insights. I have addressed all points made; see resolved comments. Ready for next review. |
|
@iamdharmesh this is back for your re-review when you get a moment. |
iamdharmesh
left a 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.
Thanks a lot for the PR @lmcrean. I have made some minor changes and this is good to go now.
Regression / Smoke Test Report ✅Tested with Archive File created via "php woorelease.phar build repo_URL" (Composer version 2.8.4, npm version 10.8.2, node version 20.18.1) Status- Working expected with Plugin Archive/Zip file same as fix specific branch. Screen.Recording.2025-12-17.at.2.42.57.AM.movTesting Environment Details
Next Step- Ready to Merge 🚀 |
Problem Statement
When users cancel subscriptions before GoCardless payment is confirmed (2-7 day window), the status incorrectly shows "Pending Cancellation" instead of "Cancelled". This creates confusion as the payment hasn't been confirmed yet.
Root Cause
Two issues:
Solution
1. Centralized Cancellation Handling
handle_subscription_cancellation()method inclass-wc-gocardless-gateway.php2. Parent Order Status Sync
sync_parent_order_status()inclass-wc-gocardless-gateway-addons.php3. User-Initiated Cancellation with Payment Check
maybe_skip_pending_cancel_status()filter callback_gocardless_payment_statusmetaExpected Behavior
confirmed/paid_outpending_submission/submitted/pending_customer_approvalTesting
Manual Test:
pending_submission)After Payment Confirmed:
confirmedFiles Changed
includes/class-wc-gocardless-gateway.php- Centralized cancellation handlerincludes/class-wc-gocardless-gateway-addons.php- Parent sync + payment check filterRelated
PR #50 (webhook-based cancellation handling)
Fixes #78
Changelog entry