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

feat(subscriptions): limit network-sync'd subscriptions buying #112

Closed
wants to merge 1 commit into from

Conversation

adekbadek
Copy link
Member

@adekbadek adekbadek commented Jul 16, 2024

All Submissions:

Changes proposed in this Pull Request:

Restricts subscription buying if the user has already a subscription on another network site (if that subscription is tied to a synchronised membership plan).

image

See 1200133283036252-as-1207352955560208/f

How to test the changes in this Pull Request:

  1. Create a granted-on-subscription memberships plan on two network sites, and make them synchronised (set same Network ID in the plan settings)
  2. One one site, buy the subscription, which should grant you the plan
  3. On the second site, log in/register using the same email address
  4. On the second site, try to buy the membership-granting subscription – observe it's not allowed (see screenshot above)

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?

Copy link
Contributor

@leogermani leogermani left a comment

Choose a reason for hiding this comment

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

I understand the value of these new Data endpoints to audit and check the health of the sync across sites.

But for the regular operations of the plugin we shouldn't rely on it.

In this case we are making a request to the Hub, that will potentially make a dozen of requests to all the other sites. It can take a while and requests could fail.

The way to do it, following what the plugin does with everything else, is to sync the information about what subscriptions/memberships the user has across all sites.

So when a user tries to buy a subscription, we can check if they already have it by looking at local data, no need for external requests.

One way to do it is to store a user_meta whenever a user buys a subscriptions, recording everything we need (site id, plan, id, membership network id, etc).

Then you can add this meta to the list of user_meta we sync and we'll automatically have it available in all sites.

But looking at it I wonder if there isn't an even simpler solution. Because when a user buys a membership in one site, it gets propagated and they are granted the membership in all sites. So when they try to buy a subscription that will enable a membership can't we just check if they user already has that membership plan and halt there?

*/
public static function woocommerce_cart_product_cannot_be_purchased_message( $message, $product_data ) {
if ( ! self::can_buy_subscription( $product_data ) ) {
return __( 'You can only purchase one subscription in this network at a time.', 'newspack-network' );
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: Instead of just saying they can't buy the subscription, we could say that they already have that subscription, because they bought it in site X

@leogermani
Copy link
Contributor

closing in favor of #138

@leogermani leogermani closed this Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants