Skip to content
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

fix: command to fix active subs w/ missing next_payment dates #3484

Merged
merged 11 commits into from
Nov 4, 2024

Conversation

dkoo
Copy link
Contributor

@dkoo dkoo commented Oct 17, 2024

All Submissions:

Changes proposed in this Pull Request:

Adds a CLI command to report and and optionally fix active or pending subscriptions that have a next_payment date in the past, or are missing a next_payment date entirely. This could happen if Action Scheduler jobs failed to update or renew subscriptions as scheduled, or as a result of missing data on migrated subscriptions.

How to test the changes in this Pull Request:

  1. Create some subscriptions with missing next_payment dates. This must be done using wp shell (replace 888 with a subscription ID on your test site) as the UI has safeguards to prevent the deletion of next payment dates. Make sure to do this for at least a few subscriptions that meet the following criteria:
  • A subscription started within the past 90 days
  • A subscription started more than 90 days ago
  • A subscription with no end date
  • A subscription with an end date BEFORE when the next payment date should be
  • A subscription with an end date AFTER when the next payment date
  • A subscription with at least one completed order
  • A subscription with no successful orders (you may need to create this one manually in the admin dashboard)
> $sub = wcs_get_subscription( 888 )
= WC_Subscription {#15359
    +refunds: null,
    +order_type: "shop_subscription",
  }

> $sub->delete_date( 'next_payment' ) // OR, update the `next_payment` date to a date in the past with $sub->update_dates( [ 'next_payment' => '2024-09-01 21:13:36' ] )
= null

> $sub->save()
= 888
  1. Run the new CLI command in dry run mode and confirm it reports the subscriptions and missed total revenue, but without actually updating the subscriptions' next_payment dates:
$ wp newspack-woocommerce fix_missing_next_payment_dates --dry-run

===================
=     DRY RUN     =
===================


Fetching active subscriptions with missing or missed next_payment dates...

+-----+--------+---------------------+---------------------+---------------------+----------------+----------------+--------------+
| ID  | status | start_date          | next_payment_date   | end_date            | billing_period | missed_periods | missed_total |
+-----+--------+---------------------+---------------------+---------------------+----------------+----------------+--------------+
| 969 | active | 2023-11-04 20:25:12 | 0                   | 2024-11-04 22:25:44 | month          | 12             | 2400         |
| 888 | active | 2024-06-01 21:13:36 | 2024-12-01 21:13:36 | 0                   | month          | 5              | 250          |
| 475 | active | 2024-08-05 20:36:11 | 2024-11-11 19:24:45 | 0                   | month          | 0              | 0            |
+-----+--------+---------------------+---------------------+---------------------+----------------+----------------+--------------+
Success: Finished processing 3 subscriptions. Total missed revenue: $2,650.00
  1. In the reported results, ensure the following are true:
  • Only subscriptions started in the past 90 days are handled
  • Subscriptions with no end date get a newly calculated next_payment date (reported in the next_payment_date column)
  • Subscriptions with a far-future end date should get a newly calculated next_payment date before the end date
  • Subscriptions with an imminent end date don't get a new next_payment date (they should show 0 in the next_payment_date column)
  • Missed periods and total values accurately reflect the number of renewals that should have occurred since the most recent completed renewal order (if any) or the subscription start (if no orders), and based on the subscription product's billing interval and period (e.g. every month/year, every 6 months, etc.).
  1. Re-run the command with a --start-date arg in YYYY-MM-DD format and confirm that the command only reports subscriptions started after the given date.
  2. Re-run the command without --dry-run and confirm that the command provides the same report data as in step 2, but also updates the subscriptions with the reported next_payment dates.

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?

@dkoo dkoo added the [Status] Needs Review The issue or pull request needs to be reviewed label Oct 17, 2024
@dkoo dkoo self-assigned this Oct 17, 2024
@dkoo dkoo requested a review from a team as a code owner October 17, 2024 21:41
@dkoo dkoo requested a review from adekbadek October 23, 2024 22:25
Co-authored-by: Adam Cassis <adam@adamcassis.com>
@dkoo dkoo force-pushed the fix/cli-fix-missing-next-payment-dates branch from f8e829a to ba88700 Compare October 25, 2024 19:54
@dkoo dkoo requested a review from adekbadek October 25, 2024 19:59
@github-actions 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 Oct 29, 2024
@dkoo
Copy link
Contributor Author

dkoo commented Oct 29, 2024

@adekbadek just thought of a case that we need to handle with this. The script should check subscriptions for an end date, and if the end date is before the calculated next payment date, it should bail. Otherwise those subscriptions may get renewed instead of properly expiring.

@adekbadek
Copy link
Member

Ah, good catch! Makes sense. Since this is "business-sensitive", would mind adding unit tests here?

@dkoo dkoo added Do Not Merge! and removed [Status] Approved The pull request has been reviewed and is ready to merge labels Oct 30, 2024
@dkoo
Copy link
Contributor Author

dkoo commented Oct 30, 2024

Great idea to add unit tests! I'll keep this PR alive until we add those and handle the "end dates" scenario above.

@dkoo
Copy link
Contributor Author

dkoo commented Nov 1, 2024

@adekbadek 9686c86 adds handling for end dates (if a subscription has an end date that will occur before the calculated next payment date, a next payment date will not be set). It also calculates "missed" billing periods by starting from the last successful order, if one exists.

ff7c665 adds a new suite of unit tests and fixes a couple of minor issues exposed by the test suite.

@dkoo dkoo requested a review from adekbadek November 1, 2024 22:10
@dkoo dkoo removed the Do Not Merge! label Nov 1, 2024
tests/unit-tests/woocommerce-cli.php Outdated Show resolved Hide resolved
*
* @return array|false The result array or false if the subscription is broken.
*/
public static function calculate_next_payment_date( $subscription, $dry_run = false ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the function name suggests the the return value is a date. Could be something along the lines of validate_subscription_dates.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to validate_subscription_dates in 879ba1a

@github-actions github-actions bot added the [Status] Approved The pull request has been reviewed and is ready to merge label Nov 4, 2024
@dkoo
Copy link
Contributor Author

dkoo commented Nov 4, 2024

@adekbadek Merging this one since you preapproved it!

@dkoo dkoo merged commit 2e05fd4 into trunk Nov 4, 2024
8 checks passed
@dkoo dkoo deleted the fix/cli-fix-missing-next-payment-dates branch November 4, 2024 20:37
Copy link

github-actions bot commented Nov 4, 2024

Hey @dkoo, 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 it to our shared document, with screenshots and testing instructions if applicable, then remove the label.

Thank you! ❤️

matticbot pushed a commit that referenced this pull request Nov 6, 2024
# [5.7.0-alpha.1](v5.6.0...v5.7.0-alpha.1) (2024-11-06)

### Bug Fixes

* avoid duplicate info notices in email editors ([#3512](#3512)) ([d38fc1a](d38fc1a))
* **co-authors-plus:** CLI for migrating from CAP GA ([9a81584](9a81584))
* command to fix active subs w/ missing next_payment dates ([#3484](#3484)) ([2e05fd4](2e05fd4))
* php fatal and warning ([#3502](#3502)) ([e089172](e089172))
* **site-kit:** update logger cron to hourly interval ([#3485](#3485)) ([e3823e7](e3823e7))
* **webhooks:** deprecate global endpoint ([#3492](#3492)) ([63e8ab2](63e8ab2))
* **wp-6.7:** update radio control styles ([#3518](#3518)) ([831756e](831756e))

### Features

* add user name to woocommerce data events ([#3473](#3473)) ([5312d30](5312d30))
* automatically disable guest authors ([#3345](#3345)) ([d0db6ba](d0db6ba))
* **connections:** jetpack sso ([#3486](#3486)) ([123408e](123408e))
* display list remote name on newsletter wizard ([#3478](#3478)) ([cd0b859](cd0b859))
* **site-kit:** add logging when site kit disconnects ([#3472](#3472)) ([62bf98c](62bf98c))
@matticbot
Copy link
Contributor

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

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Nov 11, 2024
# [5.7.0](v5.6.1...v5.7.0) (2024-11-11)

### Bug Fixes

* avoid duplicate info notices in email editors ([#3512](#3512)) ([d38fc1a](d38fc1a))
* **co-authors-plus:** CLI for migrating from CAP GA ([9a81584](9a81584))
* command to fix active subs w/ missing next_payment dates ([#3484](#3484)) ([2e05fd4](2e05fd4))
* php fatal and warning ([#3502](#3502)) ([e089172](e089172))
* **site-kit:** update logger cron to hourly interval ([#3485](#3485)) ([e3823e7](e3823e7))
* **webhooks:** deprecate global endpoint ([#3492](#3492)) ([63e8ab2](63e8ab2))
* **wp-6.7:** update radio control styles ([#3518](#3518)) ([831756e](831756e))

### Features

* add user name to woocommerce data events ([#3473](#3473)) ([5312d30](5312d30))
* automatically disable guest authors ([#3345](#3345)) ([d0db6ba](d0db6ba))
* **connections:** jetpack sso ([#3486](#3486)) ([123408e](123408e))
* display list remote name on newsletter wizard ([#3478](#3478)) ([cd0b859](cd0b859))
* **site-kit:** add logging when site kit disconnects ([#3472](#3472)) ([62bf98c](62bf98c))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 5.7.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released on @alpha released [Status] Approved The pull request has been reviewed and is ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants