fix(memberships): ensure user membership is linked to the correct subscription #3768

dkoo · 2025-02-20T23:59:31Z

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:

Yet another attempt to fix the WooCommerce Memberships bug addressed in #3050, #3060, #3268, #3338, #3380, and #3393. This bug exists in the WooCommerce Memberships plugin itself, but there's no indication that the issue will be addressed upstream, so we're applying a patch here as it continues to affect live sites.

The crux of the issue: if a user purchases more than one subscription for a product required by a user membership plan, and the first subscription has an Expired status, their user membership will remain linked to the expired subscription. Even if they subsequently purchase a new subscription for the same product, the membership will remain linked to the expired subscription, and will remain in expired status—so the user won't be able to access restricted content despite having purchased the required subscription.

This PR addresses the issue by checking the user's memberships upon login and subscription status change. If a user membership requires a subscription to specific products and the user has active subscriptions for those products, the membership will be relinked to the most recent active subscription, and the expiration date will be cleared. This should trigger the membership to automatically reactivate, granting the user access to any restricted content defined in the membership plan.

How to test the changes in this Pull Request:

Publish a membership plan that requires a subscription product and is tied the subscription length. Optionally add content restrictions to test user access to the restricted content.

On release, as a reader, purchase a subscription with the required product.
As an admin, change the subscription's status to expired.
As the reader, purchase a second subscription with the required product.
As an admin, find the user's membership (in WooCommerce > Memberships).
Observe that the user membership remains linked to the initial expired subscription, and remains in "Expired" status, meaning that the reader won't be able to access any content restricted by the membership plan.
Check out this branch.
As the reader, log out and then log back into the site again to trigger the reader_logged_in data event.
Confirm that the user membership has been relinked to the active subscription, that the membership now has an "Active" status with no expiration date, and that the reader can now access restricted content.

As an admin, set the active subscription to expired status and confirm that the membership also changes to expired status again.
As the reader, purchase a subscription for the required product again. Confirm that the user membership has been once again relinked to the new active subscription, that the membership now has an "Active" status with no expiration date, and that the reader can now access restricted content.

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

…ive subscription

dkoo · 2025-02-21T19:15:00Z

As discussed with @miguelpeixe, #3769 gets us close to a fix but doesn't solve the issue for user memberships linked to expired subscriptions.

miguelpeixe

I ran into a few issues that I commented inline.

Do we still need the handle_wc_memberships_expire_user_membership() logic here?

includes/plugins/wc-memberships/class-memberships.php

miguelpeixe · 2025-02-21T19:38:12Z

includes/plugins/wc-memberships/class-memberships.php

+					$updated         = $subscription_membership->set_subscription_id( $active_subscription_id );
+					$membership_plan = $subscription_membership->get_plan();
+
+					// Reset end date for plans with access set to subscription length.
+					if ( $membership_plan->is_access_length_type( 'subscription' ) && ! empty( $subscription_membership->get_end_date() ) ) {
+						$subscription_membership->set_end_date( '' );
+					}


With the hook priority at the default (10), the set_subscription_id() works and the membership is relinked, but set_end_date() doesn't persist and the membership remains with the expired status.

After changing the hook priority to 11 the end date resets, but the status remains "expired". Maybe we should be running a $subscription_membership->update_status( 'active', $note ); here too?

Thanks for catching this! I can confirm this is the case. Maybe the data events approach was working because it happened after all other WC Memberships hooks had done their thing

f436f1a is now working for me. As for whether we still need handle_wc_memberships_expire_user_membership(), I repurposed it as another trigger to check and possibly relink the membership, which should prevent the "Expired" status from being set if the membership is linked to the wrong subscription. WDYT?

3fad077 updates handle_wc_memberships_expire_user_membership() to check_user_memberships_on_expire() and groups it with the other check_user_memberships_on_* methods, for clarity. It also only prevents the expiration if that specific membership should be relinked to an active subscription.

Co-authored-by: Miguel Peixe <[email protected]>

…ership

dkoo · 2025-02-21T21:40:23Z

I'm not sure why the unit test job is failing now—tests run fine locally 😕

miguelpeixe · 2025-02-21T21:44:29Z

I'm not sure why the unit test job is failing now

Either CI or .org is unstable and unable to download WP. It's been doing that for me all day.

miguelpeixe

I repurposed it as another trigger to check and possibly relink the membership, which should prevent the "Expired" status from being set if the membership is linked to the wrong subscription. WDYT?

Good call!

It's working well for me now! 🎉

miguelpeixe · 2025-02-21T22:19:34Z

includes/plugins/wc-memberships/class-memberships.php

@@ -7,7 +7,10 @@

 namespace Newspack;

+use Newspack\Data_Events;


This is unused now

Thanks, removed in 835649b!

dkoo · 2025-02-21T22:32:08Z

Holding this until a non-Friday

## [5.14.3](v5.14.2...v5.14.3) (2025-02-24) ### Bug Fixes * **memberships:** ensure user membership is linked to the correct subscription ([#3768](#3768)) ([a942616](a942616))

matticbot · 2025-02-24T17:14:03Z

🎉 This PR is included in version 5.14.3 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

# [6.0.0-alpha.2](v6.0.0-alpha.1...v6.0.0-alpha.2) (2025-02-27) ### Bug Fixes * **memberships:** ensure user membership is linked to the correct subscription ([#3768](#3768)) ([a942616](a942616)) * undo forcing WC order attribution to off ([#3771](#3771)) ([c9cb52a](c9cb52a)) * **woo:** page template meta leaking to other types ([#3782](#3782)) ([325a21c](325a21c)) ### Features * add corrections customize settings ([#3751](#3751)) ([11dbc5e](11dbc5e)) * **corrections:** Unit tests for Corrections functionality ([#3776](#3776)) ([ae58933](ae58933)) * **my-account:** add change email template ([#3772](#3772)) ([32bef3c](32bef3c)) * **my-account:** add pending email change state ([#3763](#3763)) ([c9ba046](c9ba046)) * **my-account:** verify email change ([#3764](#3764)) ([b50c980](b50c980))

matticbot · 2025-02-27T12:46:14Z

🎉 This PR is included in version 6.0.0-alpha.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

dkoo added 2 commits February 20, 2025 16:22

fix(memberships): ensure user membership is linked to the correct act…

31bcc65

…ive subscription

fix: data event name

f75a58c

dkoo added the [Status] Needs Review The issue or pull request needs to be reviewed label Feb 20, 2025

dkoo self-assigned this Feb 20, 2025

dkoo requested a review from a team as a code owner February 20, 2025 23:59

chore: log the membership ID, not the plan ID

9a9bac1

miguelpeixe mentioned this pull request Feb 21, 2025

fix(memberships): reset end date on subscription #3769

Closed

6 tasks

refactor: use standard hooks instead of async data events

6cdc81a

miguelpeixe requested changes Feb 21, 2025

View reviewed changes

dkoo and others added 3 commits February 21, 2025 14:09

Update includes/plugins/wc-memberships/class-memberships.php

780a995

Co-authored-by: Miguel Peixe <[email protected]>

Update includes/plugins/wc-memberships/class-memberships.php

05ed19c

Co-authored-by: Miguel Peixe <[email protected]>

fix: explicitly set relinked membership to active status

f436f1a

dkoo requested a review from miguelpeixe February 21, 2025 21:25

fix: when checking membership on expire, make sure we check THAT memb…

3fad077

…ership

miguelpeixe approved these changes Feb 21, 2025

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 Feb 21, 2025

chore: unuse unused use

835649b

dkoo merged commit a942616 into release Feb 24, 2025
9 checks passed

dkoo deleted the hotfix/check-membership-linked-subscription branch February 24, 2025 17:10

matticbot added the released label Feb 24, 2025

miguelpeixe mentioned this pull request Feb 25, 2025

fix(woo): page template meta leaking to other types #3781

Closed

6 tasks

matticbot added the released on @alpha label Feb 27, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(memberships): ensure user membership is linked to the correct subscription #3768

fix(memberships): ensure user membership is linked to the correct subscription #3768

dkoo commented Feb 20, 2025 •

edited

Loading

dkoo commented Feb 21, 2025

miguelpeixe left a comment

miguelpeixe Feb 21, 2025

dkoo Feb 21, 2025

dkoo Feb 21, 2025

dkoo Feb 21, 2025

dkoo commented Feb 21, 2025

miguelpeixe commented Feb 21, 2025

miguelpeixe left a comment

miguelpeixe Feb 21, 2025

dkoo Feb 21, 2025

dkoo commented Feb 21, 2025

matticbot commented Feb 24, 2025

matticbot commented Feb 27, 2025

		@@ -7,7 +7,10 @@

		namespace Newspack;

		use Newspack\Data_Events;

fix(memberships): ensure user membership is linked to the correct subscription #3768

fix(memberships): ensure user membership is linked to the correct subscription #3768

Conversation

dkoo commented Feb 20, 2025 • edited Loading

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Other information:

dkoo commented Feb 21, 2025

miguelpeixe left a comment

Choose a reason for hiding this comment

miguelpeixe Feb 21, 2025

Choose a reason for hiding this comment

dkoo Feb 21, 2025

Choose a reason for hiding this comment

dkoo Feb 21, 2025

Choose a reason for hiding this comment

dkoo Feb 21, 2025

Choose a reason for hiding this comment

dkoo commented Feb 21, 2025

miguelpeixe commented Feb 21, 2025

miguelpeixe left a comment

Choose a reason for hiding this comment

miguelpeixe Feb 21, 2025

Choose a reason for hiding this comment

dkoo Feb 21, 2025

Choose a reason for hiding this comment

dkoo commented Feb 21, 2025

matticbot commented Feb 24, 2025

matticbot commented Feb 27, 2025

dkoo commented Feb 20, 2025 •

edited

Loading