From cf11b4f46df00b1c190da51ebd2cc3fad04f2587 Mon Sep 17 00:00:00 2001 From: Rasmy Nguyen Date: Fri, 22 Nov 2024 12:39:57 -0500 Subject: [PATCH 01/17] feat: add cancellation reason metadata --- .../class-woocommerce-subscriptions.php | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 includes/plugins/class-woocommerce-subscriptions.php diff --git a/includes/plugins/class-woocommerce-subscriptions.php b/includes/plugins/class-woocommerce-subscriptions.php new file mode 100644 index 0000000000..5bba44c363 --- /dev/null +++ b/includes/plugins/class-woocommerce-subscriptions.php @@ -0,0 +1,56 @@ +update_meta_data( self::CANCELLATION_REASON_META_KEY, $meta_value ); + } + } +} +WooCommerce_Subscriptions::init(); From 06b1135d4506e4fad971fe6bd70ccf7537f2c3c1 Mon Sep 17 00:00:00 2001 From: Rasmy Nguyen Date: Fri, 22 Nov 2024 12:54:43 -0500 Subject: [PATCH 02/17] feat: add cancellation reason to esp sync --- includes/plugins/class-woocommerce-subscriptions.php | 3 +-- includes/reader-activation/sync/class-metadata.php | 1 + includes/reader-activation/sync/class-woocommerce.php | 9 +++++---- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/includes/plugins/class-woocommerce-subscriptions.php b/includes/plugins/class-woocommerce-subscriptions.php index 5bba44c363..cf9c45a009 100644 --- a/includes/plugins/class-woocommerce-subscriptions.php +++ b/includes/plugins/class-woocommerce-subscriptions.php @@ -15,7 +15,6 @@ */ class WooCommerce_Subscriptions { const CANCELLATION_REASON_META_KEY = 'newspack_subscriptions_cancellation_reason'; - const CANCELLATION_REASON_EXPIRED = 'expired'; const CANCELLATION_REASON_USER_CANCELLED = 'user-cancelled'; const CANCELLATION_REASON_ADMIN_CANCELLED = 'manually-cancelled'; @@ -32,7 +31,7 @@ public static function init() { * @return bool */ public static function is_active() { - return class_exists( 'WC_Subscriptions' ) && Reader_Activation::is_active(); + return class_exists( 'WC_Subscriptions' ) && Reader_Activation::is_enabled(); } /** diff --git a/includes/reader-activation/sync/class-metadata.php b/includes/reader-activation/sync/class-metadata.php index 7747d6f574..74c4665ab0 100644 --- a/includes/reader-activation/sync/class-metadata.php +++ b/includes/reader-activation/sync/class-metadata.php @@ -242,6 +242,7 @@ public static function get_payment_fields() { 'next_payment_date' => 'Next Payment Date', // Total value spent by this customer on the site. 'total_paid' => 'Total Paid', + 'cancellation_reason' => 'Cancellation Reason', ]; } diff --git a/includes/reader-activation/sync/class-woocommerce.php b/includes/reader-activation/sync/class-woocommerce.php index cb14002b21..30b094d98a 100644 --- a/includes/reader-activation/sync/class-woocommerce.php +++ b/includes/reader-activation/sync/class-woocommerce.php @@ -264,12 +264,13 @@ private static function get_order_metadata( $order, $payment_page_url = false ) $metadata['membership_status'] = $current_subscription->get_status(); } - $metadata['sub_start_date'] = $current_subscription->get_date( 'start' ); - $metadata['sub_end_date'] = $current_subscription->get_date( 'end' ) ? $current_subscription->get_date( 'end' ) : ''; - $metadata['billing_cycle'] = $current_subscription->get_billing_period(); - $metadata['recurring_payment'] = $current_subscription->get_total(); + $metadata['sub_start_date'] = $current_subscription->get_date( 'start' ); + $metadata['sub_end_date'] = $current_subscription->get_date( 'end' ) ? $current_subscription->get_date( 'end' ) : ''; + $metadata['billing_cycle'] = $current_subscription->get_billing_period(); + $metadata['recurring_payment'] = $current_subscription->get_total(); $metadata['last_payment_amount'] = $current_subscription->get_total(); $metadata['last_payment_date'] = $current_subscription->get_date( 'last_order_date_paid' ) ? $current_subscription->get_date( 'last_order_date_paid' ) : gmdate( Metadata::DATE_FORMAT ); + $metadata['cancellation_reason'] = $current_subscription->get_meta( WooCommerce_Subscriptions::CANCELLATION_REASON_META_KEY ); // When a WC Subscription is terminated, the next payment date is set to 0. We don't want to sync that – the next payment date should remain as it was // in the event of cancellation. From 51cf6660fc9830bc309ac0b206c4c11157e76393 Mon Sep 17 00:00:00 2001 From: Rasmy Nguyen Date: Fri, 22 Nov 2024 14:36:33 -0500 Subject: [PATCH 03/17] fix: save subscription after meta update --- includes/plugins/class-woocommerce-subscriptions.php | 1 + 1 file changed, 1 insertion(+) diff --git a/includes/plugins/class-woocommerce-subscriptions.php b/includes/plugins/class-woocommerce-subscriptions.php index cf9c45a009..a331f2eb89 100644 --- a/includes/plugins/class-woocommerce-subscriptions.php +++ b/includes/plugins/class-woocommerce-subscriptions.php @@ -49,6 +49,7 @@ public static function maybe_record_cancelled_subscription( $id, $from_status, $ 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; $subscription->update_meta_data( self::CANCELLATION_REASON_META_KEY, $meta_value ); + $subscription->save(); } } } From 0d33c906cd3811394bdf85c3a53731ae42940e35 Mon Sep 17 00:00:00 2001 From: Rasmy Nguyen Date: Fri, 22 Nov 2024 14:55:28 -0500 Subject: [PATCH 04/17] fix: conditionally add esp cancellation reason --- includes/reader-activation/sync/class-metadata.php | 2 +- includes/reader-activation/sync/class-woocommerce.php | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/includes/reader-activation/sync/class-metadata.php b/includes/reader-activation/sync/class-metadata.php index 74c4665ab0..ab70be53a1 100644 --- a/includes/reader-activation/sync/class-metadata.php +++ b/includes/reader-activation/sync/class-metadata.php @@ -242,7 +242,7 @@ public static function get_payment_fields() { 'next_payment_date' => 'Next Payment Date', // Total value spent by this customer on the site. 'total_paid' => 'Total Paid', - 'cancellation_reason' => 'Cancellation Reason', + 'cancellation_reason' => 'Subscription Cancellation Reason', ]; } diff --git a/includes/reader-activation/sync/class-woocommerce.php b/includes/reader-activation/sync/class-woocommerce.php index 30b094d98a..66fe988992 100644 --- a/includes/reader-activation/sync/class-woocommerce.php +++ b/includes/reader-activation/sync/class-woocommerce.php @@ -270,7 +270,6 @@ private static function get_order_metadata( $order, $payment_page_url = false ) $metadata['recurring_payment'] = $current_subscription->get_total(); $metadata['last_payment_amount'] = $current_subscription->get_total(); $metadata['last_payment_date'] = $current_subscription->get_date( 'last_order_date_paid' ) ? $current_subscription->get_date( 'last_order_date_paid' ) : gmdate( Metadata::DATE_FORMAT ); - $metadata['cancellation_reason'] = $current_subscription->get_meta( WooCommerce_Subscriptions::CANCELLATION_REASON_META_KEY ); // When a WC Subscription is terminated, the next payment date is set to 0. We don't want to sync that – the next payment date should remain as it was // in the event of cancellation. @@ -286,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 ); + if ( ! empty( $cancellation_reason ) ) { + $metadata['cancellation_reason'] = $cancellation_reason; + } } // Clear out any payment-related fields that don't relate to the current order. From a80e024e7864d5bddc1b7460f34c265609a05391 Mon Sep 17 00:00:00 2001 From: Rasmy Nguyen Date: Fri, 22 Nov 2024 14:56:20 -0500 Subject: [PATCH 05/17] fix: remove uneeded comment --- includes/plugins/class-woocommerce-subscriptions.php | 1 - 1 file changed, 1 deletion(-) diff --git a/includes/plugins/class-woocommerce-subscriptions.php b/includes/plugins/class-woocommerce-subscriptions.php index a331f2eb89..797c64949e 100644 --- a/includes/plugins/class-woocommerce-subscriptions.php +++ b/includes/plugins/class-woocommerce-subscriptions.php @@ -1,7 +1,6 @@ Date: Fri, 22 Nov 2024 15:44:53 -0500 Subject: [PATCH 06/17] fix: add phpunit tests --- .../sync/class-woocommerce.php | 2 +- tests/mocks/wc-mocks.php | 9 ++++ .../class-woocommerce-subscriptions.php | 47 +++++++++++++++++++ .../reader-activation-sync-woocommerce.php | 1 + 4 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 tests/unit-tests/plugins/class-woocommerce-subscriptions.php diff --git a/includes/reader-activation/sync/class-woocommerce.php b/includes/reader-activation/sync/class-woocommerce.php index 66fe988992..89f1409aa0 100644 --- a/includes/reader-activation/sync/class-woocommerce.php +++ b/includes/reader-activation/sync/class-woocommerce.php @@ -287,7 +287,7 @@ private static function get_order_metadata( $order, $payment_page_url = false ) } // Record the cancellation reason if the subscription was cancelled. - $cancellation_reason = $current_subscription->get_meta( WooCommerce_Subscriptions::CANCELLATION_REASON_META_KEY ); + $cancellation_reason = $current_subscription->get_meta( WooCommerce_Subscriptions::CANCELLATION_REASON_META_KEY, '' ); if ( ! empty( $cancellation_reason ) ) { $metadata['cancellation_reason'] = $cancellation_reason; } diff --git a/tests/mocks/wc-mocks.php b/tests/mocks/wc-mocks.php index ec0ee1e795..753fd5a636 100644 --- a/tests/mocks/wc-mocks.php +++ b/tests/mocks/wc-mocks.php @@ -178,6 +178,9 @@ public function get_customer_id() { public function get_meta( $field_name ) { return isset( $this->meta[ $field_name ] ) ? $this->meta[ $field_name ] : ''; } + public function update_meta_data( $field_name, $value ) { + $this->meta[ $field_name ] = $value; + } public function has_status( $statuses ) { return in_array( $this->data['status'], $statuses ); } @@ -226,6 +229,9 @@ public function save() { } } +class WC_Subscriptions { +} + function wc_create_order( $data ) { return new WC_Order( $data ); } @@ -235,6 +241,9 @@ function wc_get_checkout_url() { function wcs_is_subscription( $order ) { return false; } +function wcs_create_subscription( $data = [] ) { + return new WC_Subscription( $data ); +} function wcs_get_subscriptions_for_order( $order ) { return []; } diff --git a/tests/unit-tests/plugins/class-woocommerce-subscriptions.php b/tests/unit-tests/plugins/class-woocommerce-subscriptions.php new file mode 100644 index 0000000000..ff31eeea99 --- /dev/null +++ b/tests/unit-tests/plugins/class-woocommerce-subscriptions.php @@ -0,0 +1,47 @@ +assertTrue( $is_active, 'WooCommerce Subscriptions integration should be active when Reader Activation is enabled.' ); + } + + /** + * Test WooCommerce_Subscriptions::maybe_record_cancelled_subscription. + */ + public function test_maybe_record_cancelled_subscription() { + $subscription = wcs_create_subscription(); + $this->assertEquals( + '', + $subscription->get_meta( WooCommerce_Subscriptions::CANCELLATION_REASON_META_KEY, '' ), + 'Cancellation reason meta should be empty before cancellation.' + ); + WooCommerce_Subscriptions::maybe_record_cancelled_subscription( 1, 'pending', 'active', $subscription ); + $this->assertEquals( + '', + $subscription->get_meta( WooCommerce_Subscriptions::CANCELLATION_REASON_META_KEY, '' ), + 'Cancellation reason meta should be empty when subscription status is not cancelled.' + ); + WooCommerce_Subscriptions::maybe_record_cancelled_subscription( 1, 'active', 'cancelled', $subscription ); + $this->assertEquals( + WooCommerce_Subscriptions::CANCELLATION_REASON_USER_CANCELLED, + $subscription->get_meta( WooCommerce_Subscriptions::CANCELLATION_REASON_META_KEY, '' ), + 'Cancellation reason meta should be set to user-cancelled when subscription is cancelled.' + ); + } +} diff --git a/tests/unit-tests/reader-activation-sync-woocommerce.php b/tests/unit-tests/reader-activation-sync-woocommerce.php index 186173351b..c9f3c8fa5c 100644 --- a/tests/unit-tests/reader-activation-sync-woocommerce.php +++ b/tests/unit-tests/reader-activation-sync-woocommerce.php @@ -120,6 +120,7 @@ public function test_payment_metadata_basic() { 'total_paid' => self::USER_DATA['meta_input']['wc_total_spent'] + $order_data['total'], 'account' => self::$user_id, 'registration_date' => $today, + 'cancellation_reason' => '', ], ], $contact_data From 8d0ec4d46eb3fca0afaac177a0e2437d56b14f77 Mon Sep 17 00:00:00 2001 From: Rasmy Nguyen Date: Fri, 22 Nov 2024 16:23:17 -0500 Subject: [PATCH 07/17] fix: move cancellation_reason in fields array --- includes/reader-activation/sync/class-metadata.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/reader-activation/sync/class-metadata.php b/includes/reader-activation/sync/class-metadata.php index ab70be53a1..3f419aba49 100644 --- a/includes/reader-activation/sync/class-metadata.php +++ b/includes/reader-activation/sync/class-metadata.php @@ -231,6 +231,7 @@ public static function get_payment_fields() { 'payment_page_utm' => 'Payment UTM: ', 'sub_start_date' => 'Current Subscription Start Date', 'sub_end_date' => 'Current Subscription End Date', + 'cancellation_reason' => 'Subscription Cancellation Reason', // At what interval does the recurring payment occur – e.g. day, week, month or year. 'billing_cycle' => 'Billing Cycle', // The total value of the recurring payment. @@ -242,7 +243,6 @@ public static function get_payment_fields() { 'next_payment_date' => 'Next Payment Date', // Total value spent by this customer on the site. 'total_paid' => 'Total Paid', - 'cancellation_reason' => 'Subscription Cancellation Reason', ]; } From 99dd7da880fb6ebbf204ba99843cc60b3482262d Mon Sep 17 00:00:00 2001 From: Rasmy Nguyen Date: Mon, 25 Nov 2024 12:17:20 -0500 Subject: [PATCH 08/17] fix: add feature flag --- .../class-subscriptions-meta.php} | 20 ++------ .../class-woocommerce-subscriptions.php | 9 ++-- .../class-woocommerce-subscriptions.php | 47 ------------------- .../class-subscriptions-meta.php | 46 ++++++++++++++++++ .../class-woocommerce-subscriptions.php | 33 +++++++++++++ 5 files changed, 90 insertions(+), 65 deletions(-) rename includes/plugins/{class-woocommerce-subscriptions.php => woocommerce-subscriptions/class-subscriptions-meta.php} (69%) delete mode 100644 tests/unit-tests/plugins/class-woocommerce-subscriptions.php create mode 100644 tests/unit-tests/plugins/woocommerce-subscriptions/class-subscriptions-meta.php create mode 100644 tests/unit-tests/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php diff --git a/includes/plugins/class-woocommerce-subscriptions.php b/includes/plugins/woocommerce-subscriptions/class-subscriptions-meta.php similarity index 69% rename from includes/plugins/class-woocommerce-subscriptions.php rename to includes/plugins/woocommerce-subscriptions/class-subscriptions-meta.php index 797c64949e..58d6de578e 100644 --- a/includes/plugins/class-woocommerce-subscriptions.php +++ b/includes/plugins/woocommerce-subscriptions/class-subscriptions-meta.php @@ -1,6 +1,6 @@ assertTrue( $is_active, 'WooCommerce Subscriptions integration should be active when Reader Activation is enabled.' ); - } - - /** - * Test WooCommerce_Subscriptions::maybe_record_cancelled_subscription. - */ - public function test_maybe_record_cancelled_subscription() { - $subscription = wcs_create_subscription(); - $this->assertEquals( - '', - $subscription->get_meta( WooCommerce_Subscriptions::CANCELLATION_REASON_META_KEY, '' ), - 'Cancellation reason meta should be empty before cancellation.' - ); - WooCommerce_Subscriptions::maybe_record_cancelled_subscription( 1, 'pending', 'active', $subscription ); - $this->assertEquals( - '', - $subscription->get_meta( WooCommerce_Subscriptions::CANCELLATION_REASON_META_KEY, '' ), - 'Cancellation reason meta should be empty when subscription status is not cancelled.' - ); - WooCommerce_Subscriptions::maybe_record_cancelled_subscription( 1, 'active', 'cancelled', $subscription ); - $this->assertEquals( - WooCommerce_Subscriptions::CANCELLATION_REASON_USER_CANCELLED, - $subscription->get_meta( WooCommerce_Subscriptions::CANCELLATION_REASON_META_KEY, '' ), - 'Cancellation reason meta should be set to user-cancelled when subscription is cancelled.' - ); - } -} diff --git a/tests/unit-tests/plugins/woocommerce-subscriptions/class-subscriptions-meta.php b/tests/unit-tests/plugins/woocommerce-subscriptions/class-subscriptions-meta.php new file mode 100644 index 0000000000..5c07a12cfd --- /dev/null +++ b/tests/unit-tests/plugins/woocommerce-subscriptions/class-subscriptions-meta.php @@ -0,0 +1,46 @@ +assertEquals( + '', + $subscription->get_meta( Subscriptions_Meta::CANCELLATION_REASON_META_KEY, '' ), + 'Cancellation reason meta should be empty before cancellation.' + ); + Subscriptions_Meta::maybe_record_cancelled_subscription_meta( 1, 'pending', 'active', $subscription ); + $this->assertEquals( + '', + $subscription->get_meta( Subscriptions_Meta::CANCELLATION_REASON_META_KEY, '' ), + 'Cancellation reason meta should be empty when subscription status is not cancelled.' + ); + Subscriptions_Meta::maybe_record_cancelled_subscription_meta( 1, 'active', 'cancelled', $subscription ); + $this->assertEquals( + Subscriptions_Meta::CANCELLATION_REASON_USER_CANCELLED, + $subscription->get_meta( Subscriptions_Meta::CANCELLATION_REASON_META_KEY, '' ), + 'Cancellation reason meta should be set to user-cancelled when subscription is cancelled.' + ); + } +} diff --git a/tests/unit-tests/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php b/tests/unit-tests/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php new file mode 100644 index 0000000000..13a80ff005 --- /dev/null +++ b/tests/unit-tests/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php @@ -0,0 +1,33 @@ +assertTrue( $is_active, 'WooCommerce Subscriptions integration should be active when WC_Subscriptions class exists.' ); + } + + /** + * Test WooCommerce_Subscriptions::is_enabled. + */ + public function test_is_enabled() { + $is_active = WooCommerce_Subscriptions::is_enabled(); + $this->assertFalse( $is_active, 'WooCommerce Subscriptions integration should be disabled when Feature Flag is not present.' ); + define( WooCommerce_Subscriptions::NEWSPACK_SUBSCRIPTIONS_EXPIRATION_FEATURE_FLAG, true ); + $is_active = WooCommerce_Subscriptions::is_enabled(); + $this->assertTrue( $is_active, 'WooCommerce Subscriptions integration should be enabled when Feature Flag is present.' ); + } +} From eda0e3590fcf5fc5ef09302bbd2c331e671d0da4 Mon Sep 17 00:00:00 2001 From: Rasmy Nguyen Date: Mon, 25 Nov 2024 13:47:17 -0500 Subject: [PATCH 09/17] fix: remove ff constant --- .../woocommerce-subscriptions/class-subscriptions-meta.php | 2 +- .../class-woocommerce-subscriptions.php | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/unit-tests/plugins/woocommerce-subscriptions/class-subscriptions-meta.php b/tests/unit-tests/plugins/woocommerce-subscriptions/class-subscriptions-meta.php index 5c07a12cfd..3717bce7ac 100644 --- a/tests/unit-tests/plugins/woocommerce-subscriptions/class-subscriptions-meta.php +++ b/tests/unit-tests/plugins/woocommerce-subscriptions/class-subscriptions-meta.php @@ -16,7 +16,7 @@ class Newspack_Test_Subscriptions_Meta extends WP_UnitTestCase { * Setup for the tests. */ public function set_up() { - define( WooCommerce_Subscriptions::NEWSPACK_SUBSCRIPTIONS_EXPIRATION_FEATURE_FLAG, true ); + define( 'NEWSPACK_SUBSCRIPTIONS_EXPIRATION', true ); WooCommerce_Subscriptions::init(); } diff --git a/tests/unit-tests/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php b/tests/unit-tests/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php index 13a80ff005..159c31626a 100644 --- a/tests/unit-tests/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php +++ b/tests/unit-tests/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php @@ -22,11 +22,14 @@ public function test_is_active() { /** * Test WooCommerce_Subscriptions::is_enabled. + * + * @runInSeparateProcess + * @preserveGlobalState disabled */ public function test_is_enabled() { $is_active = WooCommerce_Subscriptions::is_enabled(); $this->assertFalse( $is_active, 'WooCommerce Subscriptions integration should be disabled when Feature Flag is not present.' ); - define( WooCommerce_Subscriptions::NEWSPACK_SUBSCRIPTIONS_EXPIRATION_FEATURE_FLAG, true ); + define( 'NEWSPACK_SUBSCRIPTIONS_EXPIRATION', true ); $is_active = WooCommerce_Subscriptions::is_enabled(); $this->assertTrue( $is_active, 'WooCommerce Subscriptions integration should be enabled when Feature Flag is present.' ); } From b3b4024c9489263fe4d9c3cdd0f4c76f998de3d1 Mon Sep 17 00:00:00 2001 From: Rasmy Nguyen Date: Tue, 26 Nov 2024 16:53:39 -0500 Subject: [PATCH 10/17] fix: correctly sync esp data --- .../class-subscriptions-meta.php | 9 ++++----- includes/reader-activation/sync/class-woocommerce.php | 3 ++- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/includes/plugins/woocommerce-subscriptions/class-subscriptions-meta.php b/includes/plugins/woocommerce-subscriptions/class-subscriptions-meta.php index 58d6de578e..a85e59cea9 100644 --- a/includes/plugins/woocommerce-subscriptions/class-subscriptions-meta.php +++ b/includes/plugins/woocommerce-subscriptions/class-subscriptions-meta.php @@ -21,18 +21,17 @@ class Subscriptions_Meta { * Initialize hooks and filters. */ public static function init() { - add_action( 'woocommerce_subscription_status_changed', array( __CLASS__, 'maybe_record_cancelled_subscription_meta' ), 10, 4 ); + add_action( 'woocommerce_subscription_status_updated', array( __CLASS__, 'maybe_record_cancelled_subscription_meta' ), 10, 3 ); } /** * Record woo custom field for cancelled subscriptions. * - * @param int $id The subscription ID. - * @param string $from_status The status the subscription is changing from. - * @param string $to_status The status the subscription is changing to. * @param WC_Subscription $subscription The subscription object. + * @param string $to_status The status the subscription is changing to. + * @param string $from_status The status the subscription is changing from. */ - public static function maybe_record_cancelled_subscription_meta( $id, $from_status, $to_status, $subscription ) { + public static function maybe_record_cancelled_subscription_meta( $subscription, $to_status, $from_status ) { if ( ! WooCommerce_Subscriptions::is_active() ) { return; } diff --git a/includes/reader-activation/sync/class-woocommerce.php b/includes/reader-activation/sync/class-woocommerce.php index 89f1409aa0..3a8a709407 100644 --- a/includes/reader-activation/sync/class-woocommerce.php +++ b/includes/reader-activation/sync/class-woocommerce.php @@ -10,6 +10,7 @@ use Newspack\Donations; use Newspack\WooCommerce_Connection; use Newspack\WooCommerce_Order_UTM; +use Newspack\Subscriptions_Meta; defined( 'ABSPATH' ) || exit; @@ -287,7 +288,7 @@ private static function get_order_metadata( $order, $payment_page_url = false ) } // Record the cancellation reason if the subscription was cancelled. - $cancellation_reason = $current_subscription->get_meta( WooCommerce_Subscriptions::CANCELLATION_REASON_META_KEY, '' ); + $cancellation_reason = $current_subscription->get_meta( Subscriptions_Meta::CANCELLATION_REASON_META_KEY ); if ( ! empty( $cancellation_reason ) ) { $metadata['cancellation_reason'] = $cancellation_reason; } From ba22e30940247a5330ce38ce69e4439f836aab0d Mon Sep 17 00:00:00 2001 From: Rasmy Nguyen Date: Tue, 26 Nov 2024 16:59:27 -0500 Subject: [PATCH 11/17] fix: update unit test --- .../class-subscriptions-meta.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/unit-tests/plugins/woocommerce-subscriptions/class-subscriptions-meta.php b/tests/unit-tests/plugins/woocommerce-subscriptions/class-subscriptions-meta.php index 3717bce7ac..a8abcc6cd7 100644 --- a/tests/unit-tests/plugins/woocommerce-subscriptions/class-subscriptions-meta.php +++ b/tests/unit-tests/plugins/woocommerce-subscriptions/class-subscriptions-meta.php @@ -15,7 +15,7 @@ class Newspack_Test_Subscriptions_Meta extends WP_UnitTestCase { /** * Setup for the tests. */ - public function set_up() { + public static function set_up_before_class() { define( 'NEWSPACK_SUBSCRIPTIONS_EXPIRATION', true ); WooCommerce_Subscriptions::init(); } @@ -27,19 +27,19 @@ public function test_maybe_record_cancelled_subscription_meta() { $subscription = wcs_create_subscription(); $this->assertEquals( '', - $subscription->get_meta( Subscriptions_Meta::CANCELLATION_REASON_META_KEY, '' ), + $subscription->get_meta( Subscriptions_Meta::CANCELLATION_REASON_META_KEY ), 'Cancellation reason meta should be empty before cancellation.' ); - Subscriptions_Meta::maybe_record_cancelled_subscription_meta( 1, 'pending', 'active', $subscription ); + Subscriptions_Meta::maybe_record_cancelled_subscription_meta( $subscription, 'active', 'pending' ); $this->assertEquals( '', - $subscription->get_meta( Subscriptions_Meta::CANCELLATION_REASON_META_KEY, '' ), + $subscription->get_meta( Subscriptions_Meta::CANCELLATION_REASON_META_KEY ), 'Cancellation reason meta should be empty when subscription status is not cancelled.' ); - Subscriptions_Meta::maybe_record_cancelled_subscription_meta( 1, 'active', 'cancelled', $subscription ); + Subscriptions_Meta::maybe_record_cancelled_subscription_meta( $subscription, 'cancelled', 'active', $subscription ); $this->assertEquals( Subscriptions_Meta::CANCELLATION_REASON_USER_CANCELLED, - $subscription->get_meta( Subscriptions_Meta::CANCELLATION_REASON_META_KEY, '' ), + $subscription->get_meta( Subscriptions_Meta::CANCELLATION_REASON_META_KEY ), 'Cancellation reason meta should be set to user-cancelled when subscription is cancelled.' ); } From e5689aa85c0b038b70de87ffc4e34bea26aba87f Mon Sep 17 00:00:00 2001 From: Rasmy Nguyen Date: Wed, 27 Nov 2024 12:53:44 -0500 Subject: [PATCH 12/17] fix: add check for existing cancellation reason --- .../class-subscriptions-meta.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/includes/plugins/woocommerce-subscriptions/class-subscriptions-meta.php b/includes/plugins/woocommerce-subscriptions/class-subscriptions-meta.php index a85e59cea9..0648fb377b 100644 --- a/includes/plugins/woocommerce-subscriptions/class-subscriptions-meta.php +++ b/includes/plugins/woocommerce-subscriptions/class-subscriptions-meta.php @@ -36,9 +36,12 @@ public static function maybe_record_cancelled_subscription_meta( $subscription, 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; - $subscription->update_meta_data( self::CANCELLATION_REASON_META_KEY, $meta_value ); - $subscription->save(); + $meta_value = $subscription->get_meta( self::CANCELLATION_REASON_META_KEY, true ); + if ( ! $meta_value ) { + $meta_value = is_admin() ? self::CANCELLATION_REASON_ADMIN_CANCELLED : self::CANCELLATION_REASON_USER_CANCELLED; + $subscription->update_meta_data( self::CANCELLATION_REASON_META_KEY, $meta_value ); + $subscription->save(); + } } } } From 9bff2cd14aa5fad785b77e96c540107048fee39b Mon Sep 17 00:00:00 2001 From: Rasmy Nguyen Date: Wed, 27 Nov 2024 15:09:23 -0500 Subject: [PATCH 13/17] fix: init on plugin_loaded --- .../class-woocommerce-subscriptions.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/includes/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php b/includes/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php index 9109034d9a..135a396640 100644 --- a/includes/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php +++ b/includes/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php @@ -52,9 +52,9 @@ public static function is_active() { * Check if WooCommerce Subscriptions Integration is enabled. * * True if: - * - WooCommerce Subscriptions is active and, - * - Reader Activation is enabled and, - * - The NEWSPACK_SUBSCRIPTIONS_EXPIRATION feature flag is defined + * - WooCommerce Subscriptions is active and, + * - Reader Activation is enabled and, + * - NEWSPACK_SUBSCRIPTIONS_EXPIRATION is defined and true. * * @return bool */ From f321f5ff60dee550a436eec7eb260cf204b6c503 Mon Sep 17 00:00:00 2001 From: Rasmy Nguyen Date: Mon, 2 Dec 2024 17:04:11 -0500 Subject: [PATCH 14/17] fix: move active/enabled checks --- .../class-on-hold-duration.php | 4 ++++ .../class-subscriptions-meta.php | 4 ++++ .../class-woocommerce-subscriptions.php | 15 +++++---------- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/includes/plugins/woocommerce-subscriptions/class-on-hold-duration.php b/includes/plugins/woocommerce-subscriptions/class-on-hold-duration.php index 7a584505ee..14e64b3077 100644 --- a/includes/plugins/woocommerce-subscriptions/class-on-hold-duration.php +++ b/includes/plugins/woocommerce-subscriptions/class-on-hold-duration.php @@ -17,6 +17,10 @@ class On_Hold_Duration { * Initialize hooks and filters. */ public static function init() { + if ( ! WooCommerce_Subscriptions::is_enabled() ) { + return; + } + add_filter( 'woocommerce_subscription_settings', [ __CLASS__, 'add_on_hold_duration_setting' ], 11, 1 ); add_filter( 'wcs_default_retry_rules', [ __CLASS__, 'maybe_apply_on_hold_duration_rule' ], 99, 1 ); } diff --git a/includes/plugins/woocommerce-subscriptions/class-subscriptions-meta.php b/includes/plugins/woocommerce-subscriptions/class-subscriptions-meta.php index 0648fb377b..27eb7ed4ea 100644 --- a/includes/plugins/woocommerce-subscriptions/class-subscriptions-meta.php +++ b/includes/plugins/woocommerce-subscriptions/class-subscriptions-meta.php @@ -21,6 +21,10 @@ class Subscriptions_Meta { * Initialize hooks and filters. */ public static function init() { + if ( ! WooCommerce_Subscriptions::is_enabled() ) { + return; + } + add_action( 'woocommerce_subscription_status_updated', array( __CLASS__, 'maybe_record_cancelled_subscription_meta' ), 10, 3 ); } diff --git a/includes/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php b/includes/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php index 135a396640..31d3bd12bd 100644 --- a/includes/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php +++ b/includes/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php @@ -24,18 +24,13 @@ public static function init() { * Initialize WooCommerce Subscriptions Integration. */ public static function woocommerce_subscriptions_integration_init() { + include_once __DIR__ . '/class-on-hold-duration.php'; include_once __DIR__ . '/class-renewal.php'; - Renewal::init(); - - // To be included only if WooCommerce Subscriptions Integration is enabled. - // See is_enabled() method. - if ( self::is_enabled() ) { - include_once __DIR__ . '/class-on-hold-duration.php'; - include_once __DIR__ . '/class-subscriptions-meta.php'; + include_once __DIR__ . '/class-subscriptions-meta.php'; - On_Hold_Duration::init(); - Subscriptions_Meta::init(); - } + On_Hold_Duration::init(); + Renewal::init(); + Subscriptions_Meta::init(); } From ffca1d4e2634d780120555311e826ff9b0b4b113 Mon Sep 17 00:00:00 2001 From: Rasmy Nguyen Date: Mon, 2 Dec 2024 18:12:14 -0500 Subject: [PATCH 15/17] fix: handle pending-cancel to cancel transition --- .../class-subscriptions-meta.php | 32 ++++++++++++++----- .../sync/class-woocommerce.php | 4 +-- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/includes/plugins/woocommerce-subscriptions/class-subscriptions-meta.php b/includes/plugins/woocommerce-subscriptions/class-subscriptions-meta.php index 27eb7ed4ea..91545abdb4 100644 --- a/includes/plugins/woocommerce-subscriptions/class-subscriptions-meta.php +++ b/includes/plugins/woocommerce-subscriptions/class-subscriptions-meta.php @@ -13,9 +13,11 @@ * Main class. */ 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'; + const CANCELLATION_REASON_META_KEY = 'newspack_subscriptions_cancellation_reason'; + const CANCELLATION_REASON_ADMIN_CANCELLED = 'manually-cancelled'; + const CANCELLATION_REASON_ADMIN_PENDING_CANCEL = 'manually-pending-cancel'; + const CANCELLATION_REASON_USER_CANCELLED = 'user-cancelled'; + const CANCELLATION_REASON_USER_PENDING_CANCEL = 'user-pending-cancel'; /** * Initialize hooks and filters. @@ -36,16 +38,30 @@ public static function init() { * @param string $from_status The status the subscription is changing from. */ public static function maybe_record_cancelled_subscription_meta( $subscription, $to_status, $from_status ) { - if ( ! WooCommerce_Subscriptions::is_active() ) { + // We only care about active, cancelled, and pending statuses. + if ( ! in_array( $to_status, [ 'active', 'cancelled', 'pending-cancel' ], true ) || in_array( $from_status, [ 'cancelled', 'expired' ], true ) ) { return; } - if ( 'cancelled' === $to_status && ! in_array( $from_status, [ 'cancelled', 'expired' ], true ) ) { - $meta_value = $subscription->get_meta( self::CANCELLATION_REASON_META_KEY, true ); - if ( ! $meta_value ) { + $meta_value = $subscription->get_meta( self::CANCELLATION_REASON_META_KEY, true ); + if ( 'active' === $to_status && $meta_value ) { + $subscription->delete_meta_data( self::CANCELLATION_REASON_META_KEY ); + $subscription->save(); + } + if ( 'cancelled' === $to_status ) { + if ( self::CANCELLATION_REASON_USER_PENDING_CANCEL === $meta_value ) { + $subscription->update_meta_data( self::CANCELLATION_REASON_META_KEY, self::CANCELLATION_REASON_USER_CANCELLED ); + } elseif ( self::CANCELLATION_REASON_ADMIN_PENDING_CANCEL === $meta_value ) { + $subscription->update_meta_data( self::CANCELLATION_REASON_META_KEY, self::CANCELLATION_REASON_ADMIN_CANCELLED ); + } else { $meta_value = is_admin() ? self::CANCELLATION_REASON_ADMIN_CANCELLED : self::CANCELLATION_REASON_USER_CANCELLED; $subscription->update_meta_data( self::CANCELLATION_REASON_META_KEY, $meta_value ); - $subscription->save(); } + $subscription->save(); + } + if ( 'pending-cancel' === $to_status ) { + $meta_value = is_admin() ? self::CANCELLATION_REASON_ADMIN_PENDING_CANCEL : self::CANCELLATION_REASON_USER_PENDING_CANCEL; + $subscription->update_meta_data( self::CANCELLATION_REASON_META_KEY, $meta_value ); + $subscription->save(); } } } diff --git a/includes/reader-activation/sync/class-woocommerce.php b/includes/reader-activation/sync/class-woocommerce.php index 3a8a709407..bc28252c93 100644 --- a/includes/reader-activation/sync/class-woocommerce.php +++ b/includes/reader-activation/sync/class-woocommerce.php @@ -287,9 +287,9 @@ private static function get_order_metadata( $order, $payment_page_url = false ) } } - // Record the cancellation reason if the subscription was cancelled. + // Record the cancellation reason if meta exists and is not a pending cancellation. $cancellation_reason = $current_subscription->get_meta( Subscriptions_Meta::CANCELLATION_REASON_META_KEY ); - if ( ! empty( $cancellation_reason ) ) { + if ( ! empty( $cancellation_reason ) && ! in_array( $cancellation_reason, [ Subscriptions_Meta::CANCELLATION_REASON_USER_PENDING_CANCEL, Subscriptions_Meta::CANCELLATION_REASON_ADMIN_PENDING_CANCEL ], true ) ) { $metadata['cancellation_reason'] = $cancellation_reason; } } From 558d515eb2bf7de041e7946e377b9511958dc64c Mon Sep 17 00:00:00 2001 From: Rasmy Nguyen Date: Mon, 2 Dec 2024 18:25:07 -0500 Subject: [PATCH 16/17] fix: update unit tests --- .../class-woocommerce-subscriptions.php | 2 +- tests/mocks/wc-mocks.php | 3 ++ .../class-subscriptions-meta.php | 29 ++++++++++++++----- .../class-woocommerce-subscriptions.php | 21 +++++++++----- 4 files changed, 39 insertions(+), 16 deletions(-) diff --git a/includes/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php b/includes/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php index 31d3bd12bd..582bb79b51 100644 --- a/includes/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php +++ b/includes/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php @@ -49,7 +49,7 @@ public static function is_active() { * True if: * - WooCommerce Subscriptions is active and, * - Reader Activation is enabled and, - * - NEWSPACK_SUBSCRIPTIONS_EXPIRATION is defined and true. + * - The NEWSPACK_SUBSCRIPTIONS_EXPIRATION feature flag is defined and true. * * @return bool */ diff --git a/tests/mocks/wc-mocks.php b/tests/mocks/wc-mocks.php index 753fd5a636..b17ef505e4 100644 --- a/tests/mocks/wc-mocks.php +++ b/tests/mocks/wc-mocks.php @@ -181,6 +181,9 @@ public function get_meta( $field_name ) { public function update_meta_data( $field_name, $value ) { $this->meta[ $field_name ] = $value; } + public function delete_meta_data( $field_name ) { + unset( $this->meta[ $field_name ] ); + } public function has_status( $statuses ) { return in_array( $this->data['status'], $statuses ); } diff --git a/tests/unit-tests/plugins/woocommerce-subscriptions/class-subscriptions-meta.php b/tests/unit-tests/plugins/woocommerce-subscriptions/class-subscriptions-meta.php index a8abcc6cd7..e7dd9ee597 100644 --- a/tests/unit-tests/plugins/woocommerce-subscriptions/class-subscriptions-meta.php +++ b/tests/unit-tests/plugins/woocommerce-subscriptions/class-subscriptions-meta.php @@ -10,14 +10,17 @@ /** * Test WooCommerce Subscriptions integration functionality. + * + * @group WooCommerce_Subscriptions_Integration */ class Newspack_Test_Subscriptions_Meta extends WP_UnitTestCase { /** * Setup for the tests. */ public static function set_up_before_class() { - define( 'NEWSPACK_SUBSCRIPTIONS_EXPIRATION', true ); - WooCommerce_Subscriptions::init(); + if ( ! defined( 'NEWSPACK_SUBSCRIPTIONS_EXPIRATION' ) ) { + define( 'NEWSPACK_SUBSCRIPTIONS_EXPIRATION', true ); + } } /** @@ -28,19 +31,31 @@ public function test_maybe_record_cancelled_subscription_meta() { $this->assertEquals( '', $subscription->get_meta( Subscriptions_Meta::CANCELLATION_REASON_META_KEY ), - 'Cancellation reason meta should be empty before cancellation.' + 'Cancellation reason meta should be empty before any updates.' + ); + Subscriptions_Meta::maybe_record_cancelled_subscription_meta( $subscription, 'pending-cancel', 'cancelled' ); + $this->assertEquals( + '', + $subscription->get_meta( Subscriptions_Meta::CANCELLATION_REASON_META_KEY ), + 'Cancellation reason meta should be empty when subscription from-status is cancelled.' + ); + Subscriptions_Meta::maybe_record_cancelled_subscription_meta( $subscription, 'pending-cancel', 'active' ); + $this->assertEquals( + Subscriptions_Meta::CANCELLATION_REASON_USER_PENDING_CANCEL, + $subscription->get_meta( Subscriptions_Meta::CANCELLATION_REASON_META_KEY ), + 'Cancellation reason meta should be user-pending-cancel when subscription is updated to pending-cancel from active status.' ); - Subscriptions_Meta::maybe_record_cancelled_subscription_meta( $subscription, 'active', 'pending' ); + Subscriptions_Meta::maybe_record_cancelled_subscription_meta( $subscription, 'active', 'pending-cancel' ); $this->assertEquals( '', $subscription->get_meta( Subscriptions_Meta::CANCELLATION_REASON_META_KEY ), - 'Cancellation reason meta should be empty when subscription status is not cancelled.' + 'Cancellation reason meta should be reset when subscription is updated to active from pending-cancel status.' ); - Subscriptions_Meta::maybe_record_cancelled_subscription_meta( $subscription, 'cancelled', 'active', $subscription ); + Subscriptions_Meta::maybe_record_cancelled_subscription_meta( $subscription, 'cancelled', 'pending-cancel', $subscription ); $this->assertEquals( Subscriptions_Meta::CANCELLATION_REASON_USER_CANCELLED, $subscription->get_meta( Subscriptions_Meta::CANCELLATION_REASON_META_KEY ), - 'Cancellation reason meta should be set to user-cancelled when subscription is cancelled.' + 'Cancellation reason meta should be set to user-cancelled when subscription is cancelled from pending-cancel status.' ); } } diff --git a/tests/unit-tests/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php b/tests/unit-tests/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php index 159c31626a..a257014c6e 100644 --- a/tests/unit-tests/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php +++ b/tests/unit-tests/plugins/woocommerce-subscriptions/class-woocommerce-subscriptions.php @@ -10,8 +10,19 @@ /** * Test WooCommerce Subscriptions integration functionality. + * + * @group WooCommerce_Subscriptions_Integration */ class Newspack_Test_WooCommerce_Subscriptions extends WP_UnitTestCase { + /** + * Setup for the tests. + */ + public static function set_up_before_class() { + if ( ! defined( 'NEWSPACK_SUBSCRIPTIONS_EXPIRATION' ) ) { + define( 'NEWSPACK_SUBSCRIPTIONS_EXPIRATION', true ); + } + } + /** * Test WooCommerce_Subscriptions::is_active. */ @@ -22,15 +33,9 @@ public function test_is_active() { /** * Test WooCommerce_Subscriptions::is_enabled. - * - * @runInSeparateProcess - * @preserveGlobalState disabled */ public function test_is_enabled() { - $is_active = WooCommerce_Subscriptions::is_enabled(); - $this->assertFalse( $is_active, 'WooCommerce Subscriptions integration should be disabled when Feature Flag is not present.' ); - define( 'NEWSPACK_SUBSCRIPTIONS_EXPIRATION', true ); - $is_active = WooCommerce_Subscriptions::is_enabled(); - $this->assertTrue( $is_active, 'WooCommerce Subscriptions integration should be enabled when Feature Flag is present.' ); + $is_enabled = WooCommerce_Subscriptions::is_enabled(); + $this->assertTrue( $is_enabled, 'WooCommerce Subscriptions integration should be enabled when Feature Flag is present.' ); } } From f1c44e92f084049f133a422c6ca7006c4cae8acb Mon Sep 17 00:00:00 2001 From: Leo Germani Date: Fri, 6 Dec 2024 14:39:46 -0300 Subject: [PATCH 17/17] fix: avoid infinite loop --- .../woocommerce-subscriptions/class-subscriptions-meta.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/includes/plugins/woocommerce-subscriptions/class-subscriptions-meta.php b/includes/plugins/woocommerce-subscriptions/class-subscriptions-meta.php index 91545abdb4..dca7152c63 100644 --- a/includes/plugins/woocommerce-subscriptions/class-subscriptions-meta.php +++ b/includes/plugins/woocommerce-subscriptions/class-subscriptions-meta.php @@ -42,6 +42,9 @@ public static function maybe_record_cancelled_subscription_meta( $subscription, if ( ! in_array( $to_status, [ 'active', 'cancelled', 'pending-cancel' ], true ) || in_array( $from_status, [ 'cancelled', 'expired' ], true ) ) { return; } + + remove_action( 'woocommerce_subscription_status_updated', array( __CLASS__, 'maybe_record_cancelled_subscription_meta' ) ); + $meta_value = $subscription->get_meta( self::CANCELLATION_REASON_META_KEY, true ); if ( 'active' === $to_status && $meta_value ) { $subscription->delete_meta_data( self::CANCELLATION_REASON_META_KEY ); @@ -63,5 +66,7 @@ public static function maybe_record_cancelled_subscription_meta( $subscription, $subscription->update_meta_data( self::CANCELLATION_REASON_META_KEY, $meta_value ); $subscription->save(); } + + add_action( 'woocommerce_subscription_status_updated', array( __CLASS__, 'maybe_record_cancelled_subscription_meta' ), 10, 3 ); } }