fix(wcs): expire manual subscriptions after on-hold duration

[chickenn00dle]

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:

Part of https://app.asana.com/0/1200550061930446/1209066781737999/f

This PR adds a new scheduled action to expire manual subscriptions after the specified on-hold duration:

How to test the changes in this Pull Request:

1. Set at least two active subscriptions to manual, either as the reader via my-account or as admin via the edit subscriptions page in the edit billing menu.
2. Manually process renewals for both subscriptions via the edit subscriptions page.
3. In WPAdmin, WooCommerce > Status > Scheduled Actions, look for two pending newspack_expire_manual_subscription events that are scheduled to run X days after the next payment date, where X is what you have set as your site's on-hold duration (WooCommerce > Settings > Subscriptions > On-hold Duration) plus 7 days
4. As a reader, renew one of the on-hold subscriptions via my-account
5. Verify the subscription is now active as admin, and the matching scheduled action is no longer pending.
6. As admin, manually trigger the other newspack_expire_manual_subscription action
7. Verify the matching subscription is now expired

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?

---

• To see the specific tasks where the Asana app for GitHub is being used, see below:
  • https://app.asana.com/0/0/1209066782957092

[leogermani]

Works as described.

I tested one other thing I'm not sure if there's an issue there or not.

While the subscription is on hold, and there's a scheduled action to expire it, if an admin manually sets the subscription as Active again it won't delete the scheduled action.

However, when that action runs, it won't expire the subscription because it's active, so no idea if it's a problem or not. thoughts? But if it goes to on hold again, then it will be expired...

Also, the expiration date is set to 7 days + on hold duration + the next payment date. So if I process a renewal but they still have 15 paid days, there will be an extra 15 days there... All good, just highlighting because it was different from the testing instructions

[chickenn00dle]

Thanks for the review on this one as well @leogermani!

While the subscription is on hold, and there's a scheduled action to expire it, if an admin manually sets the subscription as Active again it won't delete the scheduled action.

However, when that action runs, it won't expire the subscription because it's active, so no idea if it's a problem or not. thoughts? But if it goes to on hold again, then it will be expired...

Yeah. I purposefully made the scheduled action do nothing if the subscription is not on-hold to account for any scenarios where the admin wants to handle a subscription manually. I also am only removing the scheduled action only when a successful payment is made to account for any strange situations where an admin changes a subscriptions status multiple times with the final change being on-hold. My assumption here is, if a payment isn't made and the subscription is on-hold by the time we reach the end of the total grace period, we should expire. WDYT?

Also, the expiration date is set to 7 days + on hold duration + the next payment date. So if I process a renewal but they still have 15 paid days, there will be an extra 15 days there... All good, just highlighting because it was different from the testing instructions

Ah. I think what you've written here is the expected behavior. My bad. The reader should not lose any time on their subscription when it is renewed early.

[leogermani]

LGTM

[github-actions]

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! ❤️

[matticbot]

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

The release is available on GitHub release

Your semantic-release bot 📦🚀