Subscription with end date does not expire

Created on 12 April 2019, over 5 years ago
Updated 9 July 2024, 6 months ago

Problem/Motivation

Running the latest commerce_recurring for Drupal 8. When I provide an end date to the subscription payments continue to process and subscription remains active.

I suspect the subscriptions should cancel or go deactivated once the end date is reached. Any ideas why they might not be?

Steps to reproduce

I have products with prepaid subscriptions. If I have a purchase, then the subscription becomes active. Then edit the subscription from /admin/commerce/subscriptions, adding an end date in the future. I made it 5 minutes ahead of the time i was editing. Wait till after that time. Trigger cron. Check the state of the subscription, and it's still active.

Looking at cron, there's no check for active subscriptions with an end-date in the past.

🐛 Bug report
Status

Needs work

Version

1.0

Component

Code

Created by

🇺🇸United States rjdjohnston

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇮🇱Israel jsacksick

    So I'm guessing the query should filter out subscription with NULL end dates? Is it NULL or 0?

  • 🇮🇱Israel jsacksick

    So as soon as we address #29, we also should refactor the following code:

    +    if (!in_array($subscription->getState()->getId(), ['pending', 'trial', 'active'], TRUE)) {
    +      return JobResult::failure(sprintf('Unsupported subscription status. Supported statuses: ("trial", "pending", "active"), Actual: "%s").', $subscription->getState()->getId()));
    +    }

    To use the isTransitionAllowed() method.

  • 🇮🇱Israel jsacksick

    The attached patch addresses the concerns from comment #29.

    Regarding:

    Because the cancellation is enqueued now, is there any possibility that a draft order will be triggered before the cancellation? If that were to happen, then the customer would be charged, receive a receipt, and immediately have the subscription cancelled.

    I think you're right, but even if the subscription was to expire right away (instead of via a queue), the open recurring orders are closed & renewed prior to applying changes to the subscriptions, due to:

    $this->enqueueOrders($recurring_queue);
        $this->enqueueSubscriptions($recurring_queue);

    So we'd need to change the orders and not queue the expiration. The problem is, not queuing the subscription expiration has a performance impact... This could work for installs with a relatively low number of subscriptions to expire... But let's imagine a 1000 subscriptions that have to be expired... The cron job could potentially crash...

  • 🇮🇱Israel jsacksick

    Disregard the patch from #32, it's incomplete...

  • 🇮🇱Israel jsacksick

    Removing the KickstartPrague tag.

  • 🇮🇱Israel jsacksick

    Also... wondering if this shouldn't be closed in favor of Support stopping billing after a number of billing periods Needs work ... These 2 issues are kinda related...

    I'm thinking that the isRenewable() method introduced there should check whether the subscription has an end date in the past.

  • 🇮🇱Israel jsacksick

    Also, one thing that isn't really clear to me with the changes introduced here is... When is the end date actually set? Is it expected to set it via custom code or from the UI manually?

  • 🇮🇱Israel jsacksick

    Back to this again and sorry for the noise...
    The end time is set when the subscription is canceled or scheduled for cancelation... I'm now wondering why we have an expire transition? What would be the usecase? What is the practical difference between expiring a subscription and canceling it?
    Also when a subscription is expired, there is no way to "unexpire", so in practice, it is the same as a cancelation?

  • First commit to issue fork.
  • Pipeline finished with Success
    5 months ago
    Total: 190s
    #232425
  • 🇳🇴Norway zaporylie

    I did some manual testing around this bug report and I agree with #35 about the isRenewable approach.

    This MR is where test implementation was done. Current WIP still lacks automated tests, but I validated this approach with manual testing with rather positive results.

    Still needs work but feedback is welcome.

  • Pipeline finished with Success
    5 months ago
    Total: 183s
    #239464
  • 🇯🇴Jordan Anas_maw

    Revamp patch in #33 to work on latest version

Production build 0.71.5 2024