feat(subscriptions): add cancellation reason metadata #3568

chickenn00dle · 2024-11-22T17:41:23Z

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/1208702687865961

This PR adds meta data to cancelled woo subscriptions and uses this new meta to sync cancellation reason to ESPs for cancelled subscriptions.

How to test the changes in this Pull Request:

Before testing be sure to set the feature flag in wp-config: define( 'NEWSPACK_SUBSCRIPTIONS_EXPIRATION', true );. Also ensure Newspack logging is active on the site: define( 'WP_NEWSPACK_DEBUG', true )

As admin ensure an ESP is connected and sync is working
Go to Newspack > Engagement > Show Advanced Settings > Sync contacts to ESP and ensure Subscription Cancellation Reason is checked:
As a reader purchase a new subscription. This should be the only active subscription for the reader since our ESP sync will only sync cancelled subscription data if there are no other active subscriptions
As admin, go to the edit subscription page for the above subscription and cancel (WooCommerce > Subscriptions)
After page reload scroll down to the custom fields section of the edit subscription page and verify a new cancellation reason field exists with a value of manually-cancelled:
As a reader again, purchase another subscription
Still as reader go to my account and cancel the subscription (My Account > Subscriptions > View then click the cancel button)
Via wp cli, update the pending cancellation subscription end date to one minute from now:

$subscription = wcs_get_subscription( SUBSCRIPTION_ID );
$subscription->update_dates( array( 'end_date' => "2024-11-22 19:35:01" ) );

Wait for the end date time to lapse, then visit the site as the reader to ensure the transition from pending cancel to cancel happens
As admin, visit the edit subscriptions page for the subscription and verify the cancellation reason meta field exists with a value of user-cancelled
Check the logs and ensure the Subscription Cancellation Reason field with the expected value is present in the normalized ESP sync data:

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-11-26T20:22:49Z

includes/plugins/woocommerce-subscriptions/class-subscriptions-meta.php

+			return;
+		}
+		if ( 'cancelled' === $to_status && ! in_array( $from_status, [ 'cancelled', 'expired' ], true ) ) {
+			$meta_value = is_admin() ? self::CANCELLATION_REASON_ADMIN_CANCELLED : self::CANCELLATION_REASON_USER_CANCELLED;


We want a third expired option here

As we discussed, let's leave this for another PR, but I left another comment because it didn't work for me

leogermani · 2024-11-26T21:23:49Z

includes/reader-activation/sync/class-woocommerce.php

@@ -285,6 +285,12 @@ private static function get_order_metadata( $order, $payment_page_url = false )
 					$metadata['product_name'] = reset( $subscription_order_items )->get_name();
 				}
 			}
+
+			// Record the cancellation reason if the subscription was cancelled.
+			$cancellation_reason = $current_subscription->get_meta( WooCommerce_Subscriptions::CANCELLATION_REASON_META_KEY, '' );


I'm getting an error that I believe is coming from this line

Class "Newspack\Reader_Activation\Sync\WooCommerce_Subscriptions" not found

The meta was not added neither to the subscription nor to the ESP

Ah. Good catch! Fixed in 2a51cf4

The meta was not added neither to the subscription nor to the ESP

Strange that the meta was not being added to the subscription though. I just updated the testing instructions, but make sure you've defined the ff constant to see the changes at work.

Also, I've run into this problem in the past so thought I'd mention it here as well, but if you have a contact with too many meta fields in MC, the newer fields will simply not be added. You can still check the logs for what will be synced with Newspack logging enabled in these cases:

make sure you've defined the ff constant to see the changes at work.

I'd never make such a mistake........ 😬

Also, I've run into this problem in the past so thought I'd mention it here as well, but if you have a contact with too many meta fields in MC, the newer fields will simply not be added.

Yes. This is a known issue. It's a limitation we have to deal with Mailchimp, that's why we can choose which fields we want. If you scroll the logs up a little you will see that there was an attempt to create the merge field and it failed because it reached the limit.

leogermani

@chickenn00dle I'm not getting an infinite loop when I cancel the subscription.

I added some logging and I can see the maybe_record_cancelled_subscription_meta method being called over and over again...

the woo_meta was added successfully though

leogermani · 2024-11-27T17:07:50Z

includes/plugins/woocommerce-subscriptions/class-subscriptions-meta.php

+class Subscriptions_Meta {
+	const CANCELLATION_REASON_META_KEY        = 'newspack_subscriptions_cancellation_reason';
+	const CANCELLATION_REASON_USER_CANCELLED  = 'user-cancelled';
+	const CANCELLATION_REASON_ADMIN_CANCELLED = 'manually-cancelled';


Every time I see this value I have to ask myself if manually means by the admin or by the user. Why not call this admin-cancelled?

I used the names @kmwilkerson decided on here. But am happy to change this if there is a strong preference otherwise.

I will say that from the perspective of a non-technical publisher, I think manual probably makes more sense than admin.

chickenn00dle · 2024-11-27T18:00:06Z

I added some logging and I can see the maybe_record_cancelled_subscription_meta method being called over and over again...

That's odd. I can't reproduce this on my end when I cancel as admin or as the user. This hook should only trigger when a subscription status is actually updated and once a subscription is cancelled it should no longer be possible to update the status again?

That said, I went ahead and added a preliminary check for cancellation reason meta before updating in the method in 7c582fb. Maybe this can help resolve things on your end? If not, would you mind sharing your steps to reproduce.

leogermani · 2024-11-27T19:40:50Z

Wait for the end date time to lapse, then visit the site as the reader to ensure the transition from pending cancel to cancel happens

This will be triggered by any request. If I close the reader window and navigate as an admin, it will trigger the cancellation and store manually-cancelled as the meta. We need a better strategy to make sure we capture the original intent at the moment the reader clicks the cancel button

chickenn00dle · 2024-11-27T22:11:38Z

This will be triggered by any request. If I close the reader window and navigate as an admin, it will trigger the cancellation and store manually-cancelled as the meta. We need a better strategy to make sure we capture the original intent at the moment the reader clicks the cancel button

Hmm, the scheduled cancellation uses action scheduler which uses WP_Cron. I thought that is_admin only returned true if the request comes from an admin screen? If so, wouldn't is_admin resolve to false for scheduled actions? (I could be misunderstanding this though)

I tested this and when I am on an admin screen at the time a subscription is cancelled by a user reaches its end date, the meta still shows user-cancelled:

Are you seeing otherwise?

leogermani · 2024-11-28T13:57:26Z

Are you seeing otherwise?

Yes. As I user, I cancelled the subscription and then closed the window... did everything else having only the admin tab active

chickenn00dle · 2024-12-02T23:52:50Z

tests/unit-tests/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php

+	 */
+	public function test_is_enabled() {
+		$is_enabled = WooCommerce_Subscriptions::is_enabled();
+		$this->assertTrue( $is_enabled, 'WooCommerce Subscriptions integration should be enabled when Feature Flag is present.' );


I wanted to test the case where FF is not defined, as well as when Reader Activation is not enabled, but could not find a way to modify globally defined constants between test runs. I tried @preserveGlobalState paired with @runInSeparateProcess, but this ended up removing other globals the tested method relied on.

Would love any advice on how to approach this if possible

there's no good way to test constants... The best we could do would be to change the approach we use for Feature Flags and use something else to handle these things

chickenn00dle · 2024-12-02T23:55:49Z

This is now ready for another round of review @leogermani.

I updated the strategy for determining admin vs. reader cancellations by adding an additional set of meta values for the pending-cancel state (manually-pending-cancel and user-pending-cancel, both ignored by ESP sync logic). These are checked when the subscription reaches the cancelled state to determine which cancelled meta value to use.

This means that if a reader "cancels" to pending-cancel and then the admin fully cancels prior to the end date, the meta value will still show user-cancelled. I think this is alright though since the users decision to cancel is what I think we want to capture here.

leogermani

I've pushed a commit to avoid that infinite loop again. Everything looks good now :)

github-actions · 2024-12-06T18:29:00Z

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 meta data to cancelled woo subscriptions and uses this new meta to sync cancellation reason to ESPs for cancelled subscriptions.

# [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:28Z

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

The release is available on GitHub release

Your semantic-release bot 📦🚀

chickenn00dle force-pushed the feat/add-cancellation-reason-metadata branch from a4963d1 to 79f9590 Compare November 22, 2024 20:47

chickenn00dle marked this pull request as ready for review November 22, 2024 21:22

chickenn00dle requested a review from a team as a code owner November 22, 2024 21:22

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

leogermani mentioned this pull request Nov 25, 2024

feat(subscriptions): add setting to reattempt payment after final retry #3560

Merged

6 tasks

chickenn00dle force-pushed the feat/add-cancellation-reason-metadata branch from c09ba1b to 6deebfd Compare November 25, 2024 19:09

leogermani requested changes Nov 26, 2024

View reviewed changes

leogermani reviewed Nov 26, 2024

View reviewed changes

chickenn00dle requested a review from leogermani November 26, 2024 22:04

leogermani requested changes Nov 27, 2024

View reviewed changes

chickenn00dle requested a review from leogermani November 27, 2024 18:00

chickenn00dle added 15 commits December 2, 2024 16:51

feat: add cancellation reason metadata

cf11b4f

feat: add cancellation reason to esp sync

06b1135

fix: save subscription after meta update

51cf666

fix: conditionally add esp cancellation reason

0d33c90

fix: remove uneeded comment

a80e024

fix: add phpunit tests

0701a6d

fix: move cancellation_reason in fields array

8d0ec4d

fix: add feature flag

99dd7da

fix: remove ff constant

eda0e35

fix: correctly sync esp data

b3b4024

fix: update unit test

ba22e30

fix: add check for existing cancellation reason

e5689aa

fix: init on plugin_loaded

9bff2cd

fix: move active/enabled checks

f321f5f

fix: handle pending-cancel to cancel transition

ffca1d4

chickenn00dle force-pushed the feat/add-cancellation-reason-metadata branch 2 times, most recently from b13e91f to c531e69 Compare December 2, 2024 23:28

fix: update unit tests

558d515

chickenn00dle force-pushed the feat/add-cancellation-reason-metadata branch from c531e69 to 558d515 Compare December 2, 2024 23:45

chickenn00dle commented Dec 2, 2024

View reviewed changes

fix: avoid infinite loop

f1c44e9

leogermani approved these changes Dec 6, 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 6, 2024

chickenn00dle merged commit de83e02 into trunk Dec 6, 2024
7 checks passed

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

leogermani deleted the feat/add-cancellation-reason-metadata branch December 6, 2024 18:41

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

feat(subscriptions): add cancellation reason metadata (#3568)

086a5ca

This PR adds meta data to cancelled woo subscriptions and uses this new meta to sync cancellation reason to ESPs for cancelled subscriptions.

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

leogermani removed the needs-changelog label Dec 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(subscriptions): add cancellation reason metadata #3568

feat(subscriptions): add cancellation reason metadata #3568

chickenn00dle commented Nov 22, 2024 •

edited

Loading

leogermani Nov 26, 2024

leogermani Nov 26, 2024

leogermani Nov 26, 2024

chickenn00dle Nov 26, 2024

leogermani Nov 27, 2024

leogermani Nov 27, 2024

leogermani left a comment

leogermani Nov 27, 2024

chickenn00dle Nov 27, 2024

chickenn00dle commented Nov 27, 2024

leogermani commented Nov 27, 2024

chickenn00dle commented Nov 27, 2024

leogermani commented Nov 28, 2024

chickenn00dle Dec 2, 2024 •

edited

Loading

leogermani Dec 6, 2024

chickenn00dle commented Dec 2, 2024

leogermani left a comment

github-actions bot commented Dec 6, 2024

matticbot commented Dec 12, 2024

feat(subscriptions): add cancellation reason metadata #3568

feat(subscriptions): add cancellation reason metadata #3568

Conversation

chickenn00dle commented Nov 22, 2024 • edited Loading

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Other information:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leogermani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chickenn00dle commented Nov 27, 2024

leogermani commented Nov 27, 2024

chickenn00dle commented Nov 27, 2024

leogermani commented Nov 28, 2024

chickenn00dle Dec 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chickenn00dle commented Dec 2, 2024

leogermani left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 6, 2024

matticbot commented Dec 12, 2024

chickenn00dle commented Nov 22, 2024 •

edited

Loading

chickenn00dle Dec 2, 2024 •

edited

Loading