feat(wcs): add expired subscription cli tool #3593

chickenn00dle · 2024-12-03T16:36:20Z

All Submissions:

Have you followed the Newspack Contributing guideline?
Does your code follow the WordPress' coding standards and VIP Go coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

Closes https://app.asana.com/0/1208702687864922/1208702687865965/f

This PR adds a cli command to migrate the status of all on-hold subscriptions that have failed all retries to expired:

NAME

  wp newspack migrate-expired-subscriptions

DESCRIPTION

  Migrate status of on-hold WooCommerce subscriptions that have failed all payment retries to expired.

SYNOPSIS

  wp newspack migrate-expired-subscriptions [--live] [--verbose] [--ids]

OPTIONS

  [--live]
    Run the command in live mode, updating the subscriptions.

  [--verbose]
    Produce more output.

  [--ids]
    Comma-separated list of subscription IDs. If provided, only subscriptions with these IDs will be processed.

How to test the changes in this Pull Request:

Make sure you DO NOT have the NEWSPACK_SUBSCRIPTIONS_EXPIRATION constant defined
If you don't already have any active subscriptions, as a reader purchase a handful of subscriptions
Follow the steps here to trigger scheduled retry for a handful of subscriptions: https://woocommerce.com/document/subscriptions/develop/failed-payment-retry/#section-7
Ensure you have at least one subscription of each:

An on-hold subscription with all payment retries completed
An on-hold subscription that is awaiting another payment retry
An on-hold subscription that has no retries scheduled (manually move this to on-hold)
A non-on-hold subscription (active, expired, etc.)

Via terminal, run the wp newspack migrate-expired-subscriptions --verbose command. Confirm an error appears letting you know subscription integration is not enabled.
Define the NEWSPACK_SUBSCRIPTIONS_EXPIRATION constant: define( 'NEWSPACK_SUBSCRIPTIONS_EXPIRATION', true );
Via terminal, run the command again. Confirm the output shows you are in dry run mode, 1 subscription has been updated, and the other subscriptions are skipped with the expected reasons. Also confirm the non-on-hold subscription is not included.
Now include some ids to confirm the command will work on only the provided ids: wp newspack migrate-expired-subscriptions --verbose --ids=1,2,3,4,5
Finally, run the command with the live flag: wp newspack migrate-expired-subscriptions --live. Confirm the status of the on-hold subscription has updated to expired.
Smoke test the command with some more subscriptions!

Other information:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes, as applicable?
Have you successfully ran tests with your changes locally?

leogermani · 2024-12-05T18:56:29Z

includes/cli/class-woocommerce-subscriptions.php

+						WP_CLI::line( 'Updating subscription status to expired...' );
+					}
+					if ( self::$live ) {
+						$subscription->update_status( 'expired', __( 'Subscription status updated by Newspack CLI command.', 'newspack-plugin' ) );


Let's also add one of those Subscriptions Notes that show up in the subscription page?

In discussion with Tracy and Katie, it would be best if the subscription had the expiration date of the last failed payment rather than the expiration date of when the script was run.

Let's also add one of those Subscriptions Notes that show up in the subscription page?

The second argument to $subscription->update_status is added as a subscription note. Is this what you were referring to?

In discussion with Tracy and Katie, it would be best if the subscription had the expiration date of the last failed payment rather than the expiration date of when the script was run.

Sure thing @claudiulodro! I updated to script to use the date of the last retry object. This should be the same as the last failed payment: b29bdd1

When testing though, since we are manually triggering the scheduled actions, this end date will be set as though the scheduled action was not triggered manually, using the expected end date for the retry. I think this should be fine since our publishers shouldn't be doing this, however let me know if we need to be more specific here.

claudiulodro · 2024-12-05T21:42:11Z

includes/cli/class-woocommerce-subscriptions.php

+						WP_CLI::line( 'Updating subscription status to expired...' );
+					}
+					if ( self::$live ) {
+						$subscription->update_status( 'expired', __( 'Subscription status updated by Newspack CLI command.', 'newspack-plugin' ) );


In discussion with Tracy and Katie, it would be best if the subscription had the expiration date of the last failed payment rather than the expiration date of when the script was run.

leogermani

The detection and the update seems to be working fine.

But the script runs in an infinite loop because get_subscriptions always return the same subs.

I dont see a paged param in wcs_get_subscriptions -> https://github.com/riclain/woocommerce-subscriptions/blob/b0dd2efd869b0639ad54131fba9c13b4ed5a09a2/wcs-functions.php#L332

Also, the End date didn't work for me. It was set to the current date. I did have some old expired subs in my DB so tit's easy to spot it

chickenn00dle · 2024-12-06T19:54:08Z

I dont see a paged param in wcs_get_subscriptions -> https://github.com/riclain/woocommerce-subscriptions/blob/b0dd2efd869b0639ad54131fba9c13b4ed5a09a2/wcs-functions.php#L332

@leogermani, paged is here in the official WCS core repo: https://github.com/Automattic/woocommerce-subscriptions-core/blob/1692c41cf43e2c3c5ef0816a48196ffb241b9e72/wcs-functions.php#L402

Looks like I forgot to add a call to $subscription->save(), although its strange that on my end I don't see the infinite loop without this. Does this fix that issue for you?

Looking into the end date issue now.

leogermani · 2024-12-06T20:12:11Z

Looks like I forgot to add a call to $subscription->save(), although its strange that on my end I don't see the infinite loop without this. Does this fix that issue for you?

I don't think it will, because it happens on dry mode as well.

In fact, I tried on a shell to call wcs_get_subscriptions passing paged as big number, just to see if I got an empty array as a result.. and I didn't

Looks like I was runing a pretty old version of woo subscriptions :) upgrading it fixed the infinite loop. I'try to test the data again in a bit

leogermani · 2024-12-06T22:45:04Z

Tested again and the "End date" is still set to the date I ran the script.

chickenn00dle · 2024-12-06T23:40:21Z

Tested again and the "End date" is still set to the date I ran the script.

Oof. This was because the call to $subscription->set_end_date needs to happen after $subscription->update_status since the status update will also set an end date.

When I initially made the changes and tested locally, this was the case, but before pushing the commit, I thought I'd "tidy up the changes" before pushing and reversed the order.

Sorry about that! Should be fixed now.

leogermani

When I initially made the changes and tested locally, this was the case, but before pushing the commit, I thought I'd "tidy up the changes" before pushing and reversed the order.

been there. done that! :)

All good now!

changes applied

github-actions · 2024-12-09T16:43:10Z

Hey @chickenn00dle, good job getting this PR merged! 🎉

Now, the needs-changelog label has been added to it.

Please check if this PR needs to be included in the "Upcoming Changes" and "Release Notes" doc. If it doesn't, simply remove the label.

If it does, please add an entry to our shared document, with screenshots and testing instructions if applicable, then remove the label.

Thank you! ❤️

This PR adds a cli command to migrate the status of all on-hold subscriptions that have failed all retries to expired

# [5.10.0-alpha.1](v5.9.1...v5.10.0-alpha.1) (2024-12-12) ### Bug Fixes * duplicate orders save on cron ([#3604](#3604)) ([ec69167](ec69167)) * **ras-acc:** re-add recaptcha to the WooCommerce checkout ([#3605](#3605)) ([f42a75b](f42a75b)) * **ras:** do not require Woo plugins if using NRH ([#3614](#3614)) ([363a834](363a834)) * **wcs:** remove subscriptions expiration feature flag ([#3618](#3618)) ([7c175d9](7c175d9)) * **wcs:** update subscription expiration feature ([#3613](#3613)) ([ebf6e6d](ebf6e6d)) * **wcs:** update subscriptions expiration cli behavior ([#3617](#3617)) ([07e768c](07e768c)) ### Features * **subscriptions:** add cancellation reason metadata ([#3568](#3568)) ([de83e02](de83e02)) * **wc:** duplicate orders admin notice ([#3555](#3555)) ([cb764e3](cb764e3)) * **wcs:** add expired subscription cli tool ([#3593](#3593)) ([5d39398](5d39398)) * **webhooks:** filter request priority ([#3587](#3587)) ([1928a6a](1928a6a)) * **woocommerce-subscriptions:** add url redirect for wc subscription renewals ([#3525](#3525)) ([5b14aeb](5b14aeb)) ### Reverts * Revert "feat: command to initialize cron job to slowly backfill CAP term data (#3425)" (#3620) ([c9a9d45](c9a9d45)), closes [#3425](#3425) [#3620](#3620)

matticbot · 2024-12-12T20:15:37Z

🎉 This PR is included in version 5.10.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

# [5.10.0](v5.9.2...v5.10.0) (2024-12-16) ### Bug Fixes * dont load textdomain too early ([#3629](#3629)) ([76c1f97](76c1f97)) * duplicate orders save on cron ([#3604](#3604)) ([ec69167](ec69167)) * hide duplicate notices if all was dismissed ([#3630](#3630)) ([cf48188](cf48188)) * **ras-acc:** re-add recaptcha to the WooCommerce checkout ([#3605](#3605)) ([f42a75b](f42a75b)) * **ras:** do not require Woo plugins if using NRH ([#3614](#3614)) ([363a834](363a834)) * **wcs:** remove subscriptions expiration feature flag ([#3618](#3618)) ([7c175d9](7c175d9)) * **wcs:** update subscription expiration feature ([#3613](#3613)) ([ebf6e6d](ebf6e6d)) * **wcs:** update subscriptions expiration cli behavior ([#3617](#3617)) ([07e768c](07e768c)) ### Features * **subscriptions:** add cancellation reason metadata ([#3568](#3568)) ([de83e02](de83e02)) * **wc:** duplicate orders admin notice ([#3555](#3555)) ([cb764e3](cb764e3)) * **wcs:** add expired subscription cli tool ([#3593](#3593)) ([5d39398](5d39398)) * **webhooks:** filter request priority ([#3587](#3587)) ([1928a6a](1928a6a)) * **woocommerce-subscriptions:** add url redirect for wc subscription renewals ([#3525](#3525)) ([5b14aeb](5b14aeb)) ### Reverts * Revert "feat: command to initialize cron job to slowly backfill CAP term data (#3425)" (#3620) ([c9a9d45](c9a9d45)), closes [#3425](#3425) [#3620](#3620)

matticbot · 2024-12-16T18:41:56Z

🎉 This PR is included in version 5.10.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

chickenn00dle force-pushed the feat/add-subscriptions-expiration-cli-tool branch 5 times, most recently from 7e3606a to c29f210 Compare December 4, 2024 23:10

chickenn00dle marked this pull request as ready for review December 4, 2024 23:14

chickenn00dle requested a review from a team as a code owner December 4, 2024 23:14

chickenn00dle force-pushed the feat/add-subscriptions-expiration-cli-tool branch from c29f210 to ebdb377 Compare December 4, 2024 23:16

chickenn00dle added the [Status] Needs Review The issue or pull request needs to be reviewed label Dec 4, 2024

leogermani reviewed Dec 5, 2024

View reviewed changes

claudiulodro previously requested changes Dec 5, 2024

View reviewed changes

chickenn00dle requested review from claudiulodro and leogermani December 6, 2024 16:47

leogermani requested changes Dec 6, 2024

View reviewed changes

chickenn00dle added 5 commits December 6, 2024 14:52

feat(wcs): add expired subscription cli tool

558b8f5

feat: add subscription ids handling

60f5810

fix: only expire failed retries

11c951f

fix: update subscription end date

247838a

fix: save subscription update

e84ebeb

chickenn00dle force-pushed the feat/add-subscriptions-expiration-cli-tool branch from b29bdd1 to e84ebeb Compare December 6, 2024 19:53

fix: fix subscription update order

efdf5a7

chickenn00dle requested a review from leogermani December 6, 2024 23:40

leogermani approved these changes Dec 7, 2024

View reviewed changes

github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Dec 7, 2024

chickenn00dle merged commit 5d39398 into trunk Dec 9, 2024
9 checks passed

github-actions bot added the needs-changelog label Dec 9, 2024

chickenn00dle added a commit that referenced this pull request Dec 12, 2024

feat(wcs): add expired subscription cli tool (#3593)

463c063

This PR adds a cli command to migrate the status of all on-hold subscriptions that have failed all retries to expired

matticbot added the released on @alpha label Dec 12, 2024

leogermani removed the needs-changelog label Dec 13, 2024

matticbot added the released label Dec 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(wcs): add expired subscription cli tool #3593

feat(wcs): add expired subscription cli tool #3593

chickenn00dle commented Dec 3, 2024 •

edited

Loading

leogermani Dec 5, 2024

claudiulodro Dec 5, 2024

chickenn00dle Dec 6, 2024

claudiulodro Dec 5, 2024

leogermani left a comment

chickenn00dle commented Dec 6, 2024 •

edited

Loading

leogermani commented Dec 6, 2024

leogermani commented Dec 6, 2024

chickenn00dle commented Dec 6, 2024

leogermani left a comment

github-actions bot commented Dec 9, 2024

matticbot commented Dec 12, 2024

matticbot commented Dec 16, 2024

feat(wcs): add expired subscription cli tool #3593

feat(wcs): add expired subscription cli tool #3593

Conversation

chickenn00dle commented Dec 3, 2024 • edited Loading

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Other information:

leogermani Dec 5, 2024

Choose a reason for hiding this comment

claudiulodro Dec 5, 2024

Choose a reason for hiding this comment

chickenn00dle Dec 6, 2024

Choose a reason for hiding this comment

claudiulodro Dec 5, 2024

Choose a reason for hiding this comment

leogermani left a comment

Choose a reason for hiding this comment

chickenn00dle commented Dec 6, 2024 • edited Loading

leogermani commented Dec 6, 2024

leogermani commented Dec 6, 2024

chickenn00dle commented Dec 6, 2024

leogermani left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 9, 2024

matticbot commented Dec 12, 2024

matticbot commented Dec 16, 2024

chickenn00dle commented Dec 3, 2024 •

edited

Loading

chickenn00dle commented Dec 6, 2024 •

edited

Loading