search

Automattic / sensei

Sensei LMS - Online Courses, Quizzes, & Learning
https://senseilms.com
GNU General Public License v2.0
545 stars 199 forks source link

User with one active, one cancelled subscription cannot access content #2094

Closed andyadams closed 6 years ago

andyadams commented 6 years ago

Steps to Reproduce

  1. Restrict some course content via a subscription
  2. Create a user with one active, one cancelled subscription (both on the same subscription type)
  3. Try to visit the content, and it will be blocked

What I Expected

A user with an active subscription should have been able to access the content.

What Happened Instead

Because they also have a cancelled subscription, they cannot access the content.

WordPress / Sensei version

4.9.4 / 1.9.20

Browser / OS version

Any

Context / Source

We use Sensei in conjunction with Subscriptions and Memberships, and we've occasionally had customers who cancel their subscription (temporarily) and then sign up for a brand new subscription of the same type. They lose access to the content (it asks them to buy again) even with an active subscription.

(I think) I've debugged it down to class-sensei-wc.php, in the has_customer_bought_product function, line 1054:

https://github.com/Automattic/sensei/blob/master/includes/class-sensei-wc.php#L1054

Here, the code is checking to see if the user has a cancelled subscription, and if so it returns false immediately. Instead, it seems to me it should also check to see if there is an active subscription of the same type, and if so, continue.

The way it is, it will lock someone out of content even if they have an active subscription.

Ideally, customers would re-activate their old subscription, but in our experience they most often want to go through and re-purchase. I think this should probably handle this double-subscription case. Please let me know if you have any questions (or need a pull request) and I can help!

donnapep commented 6 years ago

Could you please provide a bit more detail? What type of subscription is it? Are there variations? Screenshots would be especially useful. Thx.

andyadams commented 6 years ago

Hi @donnapep!

  1. The bug applies any type of subscription, but ours is a monthly subscription in most cases.
  2. No variations.
  3. It's a virtual product.

The line of code has moved to here:

https://github.com/Automattic/sensei/blob/a736ab9ef7659e41c82e49c7bc49f1e74ceda970/includes/class-sensei-wc.php#L1044

$user_bought_subscription_but_cancelled = wcs_user_has_subscription( $user_id, $product_id, 'cancelled' ); will be true if the user has a cancelled subscription. If this is true, the function returns on line 1047.

BUT, what if the user also has an active subscription? In this case, the function should not return on line 1047, correct?

I'd provide screenshots, but I'm not sure which ones would help.

lisaleague commented 6 years ago

@donnapep just to add the to this, the use case is for upgrades and pretty typical scenario:

  1. User purchases standard access, then decides to upgrade to premium access
  2. User cancels standard subscription
  3. User purchases premium subscription
  4. User is locked out of Sensei course content when standard subscription expires, even though they have a valid premium subscription
andyadams commented 6 years ago

@donnapep I wanted to follow up to see if this is potentially going to be fixed soon? It's a potentially huge issue for our customers, and I at least would love to know if it's going to have a fix in the near future, or if I should work on a workaround (which seems like it may be painful, but I haven't dug in) in the meantime? Thanks!

donnapep commented 6 years ago

@andyadams I'm still having difficulty reproducing this one. If I could reproduce, I could put the fix in the next release. I'm starting to see why this combination of products is so complicated!

My notes so far:

Have I missed something in my setup?

lisaleague commented 6 years ago

@donnapep user access is restricted by lesson tag, not by course. This is a normal option in WooCommerce Memberships. Reason being is that both standard and premium get access to the same course, but premium gets access to additional content (lessons with specific tag) that standard does not. When the standard subscription membership expires, users with a valid premium subscription cannot access ANY lessons no matter the tag.

andyadams commented 6 years ago

@donnapep You've almost got it, but except for the last 2 steps:

Instead of:

Try this:

donnapep commented 6 years ago

I'm still having a lot of trouble reproducing this. After tracing through the wcs_user_has_subscription function, I realized that I don't have a WooCommerce product attached to the course, so I never end up with $has_subscription being set to true, which in turn sets $user_bought_subscription_but_cancelled to false.

In our docs, we actually recommend that a WooCommerce product not be attached to a course, so you could try removing it.

However, even after having attached the product to the course and going through the whole purchase, cancel, purchase again scenario, I still don't lose access to the course. It looks like this may require a tag team approach to figuring out how I can reproduce this, unless detaching the product from the course solves the issue.

donnapep commented 6 years ago

@andyadams How's it going with this one? I'm defining the work for the next release, so do let me know whether or not removing the WC product from the course resolved this issue. Thx.

andyadams commented 6 years ago

Hi @donnapep, removing the link from Course <-> Product does seem to fix the access issue in my initial testing. I'm going to need to go through our site a bit, because in the past when we didn't have a course linked to a product, other sections of the site would crash. I can't remember offhand where they were, but at least we're making progress.

For now, you can close and if I have anything more concrete & easily reproducible to report, I will. Thank you for your help, dedication, and follow-ups!