Add the ability to re-purchase a license to extend it

Hello,

First, thanks for your work on this module. I have just retested the 8.x-2.0-alpha8 and it works well.

I am also interested in this feature request for the French Drupal association https://github.com/Drupal-FR/site-drupalfr.

I don't think we will go on the commerce recurring module because of what I see it is more for automated billing and more complex. Also I think having people manually buying their membership before expiration should be a possibility.

Thank you for the really awesome work : )

Here is a patch to get started with. It's just a rough draft but it did work in my use case.

It requires the patch from Recurring Period which I just submitted which is referenced here.

Trying to get the test passed.

Thank you for your quick feedback!

I will have another go at this.

The reason I usedBundleFieldDefinition is for the life of me I couldn’t get the value of product variation in the License Class in the presave method.

Also, Above you mention the grantable:

- got license? no:
    - do you have the grantable? yes: may not purchase

Can you please explain what exactly is the grantable?

@joachim Sorry it has taken me awhile to get back to you. Thank you for committing that getter.

One question I had is where should I add the checkbox and date field to allow a renewal within a specified time period?

I thought the the form on the Product type setting would be a good place to add it.

Yes, I think you are right. It is the product variation type. I was able to get this together tonite.

Its not a full patch but something to get started with. For some reason I can't save the interval value.

I wanted to hide the interval when allow_renewal was unchecked but its it not being hidden.

If anyone else would like to jump in feel free.

Thank you for those suggestions. I've incorporated them into this new patch.

I think everything works except that when a license is renewed, a duplicate license is issued.

I've tried deleting it with $this->delete() but then I get an error message that a string is not translatable.

I believe the Renew column is being set correctly on the original license though.

Any suggestions would be welcome.

I had a quick question regarding one set of your comments above.

When the user places the order, you need to find the existing license, and attach it to the order item.

You are then working with just one license, and here you simply need to extend its expiry date.

I was going to move this logic to the LicenseAvailabilityCheckerExistingRights.php file. I'm guessing the existing license gets attached to $entity or $this .

If so, which class variable do we attach it to or replace?

Had another go at it. This was working on my server though I haven't added any functionality to account for deletions.

I wanted to throw something out there though just to make sure I was going in the right direction.

Had to pull the changes from dev first.

@joachim, did you have comments about my last patch?

I know it failed testing but I was wondering if I was on the right track or if you had any other comments.

Hello,

Thanks all for your work on this issue, I have tested patch from comment #22.

I works but there are some warnings.

On the complete step when it is the first time a license is created for the user for a product:

Notice: Undefined offset: 0 in Drupal\commerce_license\EventSubscriber\LicenseOrderSyncSubscriber->onCartOrderTransition() (line 114 of modules/contrib/commerce_license/src/EventSubscriber/LicenseOrderSyncSubscriber.php).
Drupal\commerce_license\EventSubscriber\LicenseOrderSyncSubscriber->onCartOrderTransition(Object, 'commerce_order.place.pre_transition', Object)
call_user_func(Array, Object, 'commerce_order.place.pre_transition', Object) (Line: 111)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('commerce_order.place.pre_transition', Object) (Line: 339)
Drupal\state_machine\Plugin\Field\FieldType\StateItem->dispatchTransitionEvent('pre_transition') (Line: 308)
Drupal\state_machine\Plugin\Field\FieldType\StateItem->preSave() (Line: 233)
Drupal\Core\Field\FieldItemList->delegateMethod('preSave') (Line: 191)
Drupal\Core\Field\FieldItemList->preSave() (Line: 768)
Drupal\Core\Entity\ContentEntityStorageBase->invokeFieldMethod('preSave', Object) (Line: 718)
Drupal\Core\Entity\ContentEntityStorageBase->invokeHook('presave', Object) (Line: 88)
Drupal\commerce\CommerceContentEntityStorage->invokeHook('presave', Object) (Line: 435)
Drupal\Core\Entity\EntityStorageBase->doPreSave(Object) (Line: 587)
Drupal\Core\Entity\ContentEntityStorageBase->doPreSave(Object) (Line: 89)
Drupal\commerce_order\OrderStorage->doPreSave(Object) (Line: 389)
Drupal\Core\Entity\EntityStorageBase->save(Object) (Line: 820)
Drupal\Core\Entity\Sql\SqlContentEntityStorage->save(Object) (Line: 391)
Drupal\Core\Entity\Entity->save() (Line: 130)
Drupal\commerce_checkout\Plugin\Commerce\CheckoutFlow\CheckoutFlowBase->redirectToStep('complete') (Line: 173)
Drupal\commerce_payment\Plugin\Commerce\CheckoutPane\PaymentProcess->buildPaneForm(Array, Object, Array) (Line: 556)
Drupal\commerce_checkout\Plugin\Commerce\CheckoutFlow\CheckoutFlowWithPanesBase->buildForm(Array, Object, 'payment')
call_user_func_array(Array, Array) (Line: 518)
Drupal\Core\Form\FormBuilder->retrieveForm('commerce_checkout_flow_multistep_default', Object) (Line: 275)
Drupal\Core\Form\FormBuilder->buildForm('commerce_checkout_flow_multistep_default', Object) (Line: 216)
Drupal\Core\Form\FormBuilder->getForm(Object, 'payment') (Line: 94)
Drupal\commerce_checkout\Controller\CheckoutController->formPage(Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 582)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 151)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 99)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 78)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 52)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 666)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

Notice: Trying to get property of non-object in Drupal\commerce_license\EventSubscriber\LicenseOrderSyncSubscriber->onCartOrderTransition() (line 114 of modules/contrib/commerce_license/src/EventSubscriber/LicenseOrderSyncSubscriber.php).
Drupal\commerce_license\EventSubscriber\LicenseOrderSyncSubscriber->onCartOrderTransition(Object, 'commerce_order.place.pre_transition', Object)
call_user_func(Array, Object, 'commerce_order.place.pre_transition', Object) (Line: 111)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('commerce_order.place.pre_transition', Object) (Line: 339)
Drupal\state_machine\Plugin\Field\FieldType\StateItem->dispatchTransitionEvent('pre_transition') (Line: 308)
Drupal\state_machine\Plugin\Field\FieldType\StateItem->preSave() (Line: 233)
Drupal\Core\Field\FieldItemList->delegateMethod('preSave') (Line: 191)
Drupal\Core\Field\FieldItemList->preSave() (Line: 768)
Drupal\Core\Entity\ContentEntityStorageBase->invokeFieldMethod('preSave', Object) (Line: 718)
Drupal\Core\Entity\ContentEntityStorageBase->invokeHook('presave', Object) (Line: 88)
Drupal\commerce\CommerceContentEntityStorage->invokeHook('presave', Object) (Line: 435)
Drupal\Core\Entity\EntityStorageBase->doPreSave(Object) (Line: 587)
Drupal\Core\Entity\ContentEntityStorageBase->doPreSave(Object) (Line: 89)
Drupal\commerce_order\OrderStorage->doPreSave(Object) (Line: 389)
Drupal\Core\Entity\EntityStorageBase->save(Object) (Line: 820)
Drupal\Core\Entity\Sql\SqlContentEntityStorage->save(Object) (Line: 391)
Drupal\Core\Entity\Entity->save() (Line: 130)
Drupal\commerce_checkout\Plugin\Commerce\CheckoutFlow\CheckoutFlowBase->redirectToStep('complete') (Line: 173)
Drupal\commerce_payment\Plugin\Commerce\CheckoutPane\PaymentProcess->buildPaneForm(Array, Object, Array) (Line: 556)
Drupal\commerce_checkout\Plugin\Commerce\CheckoutFlow\CheckoutFlowWithPanesBase->buildForm(Array, Object, 'payment')
call_user_func_array(Array, Array) (Line: 518)
Drupal\Core\Form\FormBuilder->retrieveForm('commerce_checkout_flow_multistep_default', Object) (Line: 275)
Drupal\Core\Form\FormBuilder->buildForm('commerce_checkout_flow_multistep_default', Object) (Line: 216)
Drupal\Core\Form\FormBuilder->getForm(Object, 'payment') (Line: 94)
Drupal\commerce_checkout\Controller\CheckoutController->formPage(Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 582)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 151)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 99)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 78)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 52)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 666)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

And when selecting a product on which you already have a license, there is still the message "Renewals of this license are not allowed before expiration" even though the product is added to the cart.

Is there anything I can do to help resolve the issue. I saw @joachim review. Maybe you can guide me a little bit to do the points.

Also a question, this new bahavior will be for all licences. Would it be better to have a settings per licence to allow this behavior?

Thanks for your reply,

Ok, I though the behaviour would be set sitewide but if it can be set per product it's good.

Ok I'll wait for reply.

Hi,

@joachim: I am at Drupal Europe. Would it be possible to sprint together (or at least having a brefing on this issue) to get this issue done?

Ok,

I am already there and I am registered on the spreadsheet: https://docs.google.com/spreadsheets/d/1OxtAsRs_SeKQynL5bJ_5AEmRInLdyG7m...

I will attend your session.

Until friday, can you give me direction on what needs to be done please? If you prefer to see that directly, it's ok. I have other subjects to sprint on.

@joachim,

Ok, I found why the die() where not executed...

With the patch, the service LicenseAvailabilityCheckerExistingRights has been removed so after cancelling code deletion, we needed to clear the cache for Drupal to detect it exists again...

- LicenseAvailabilityCheckerExistingRights needs to know to step out of the way if the product is configured to allow renewals. Otherwise it acts as normal to prevent a repurchase.

=> OK

In commerce_license/src/LicenseAvailabilityCheckerExistingRights.php :

  public function check(PurchasableEntityInterface $entity, $quantity, Context $context) {
    // Don't do an availability check on recurring orders.
    // Very ugly workaround for the lack of any information about the order at
    // this point.
    // See https://www.drupal.org/project/commerce/issues/2937041
    $backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS);
    foreach ($backtrace as $backtrace_call) {
      // If the availability check is being done for an order being saved or
      // created by Commerce Recurring's RecurringOrderManager, then it's a
      // recurring order, and we shouldn't do anything.
      if (isset($backtrace_call['class']) && $backtrace_call['class'] == RecurringOrderManager::class) {
        return;
      }
    }

// Handle licence renewal.
    $product_variation_type_id = $entity->bundle();
    $product_variation_type = ProductVariationType::load($product_variation_type_id);
    $allow_renewal = $product_variation_type->getThirdPartySetting('commerce_license', 'allow_renewal', FALSE);
    if ($allow_renewal) {
      return;
//      $allow_renewal_window_interval = $product_variation_type->getThirdPartySetting('commerce_license', 'interval');
//      $allow_renewal_window_period = $product_variation_type->getThirdPartySetting('commerce_license', 'period');
    }

I will go to a session, but I need to discuss a little with you for the content of LicenseRenewalCartEventSubscriber.

@joachim,

Sorry, by "state", I meant the key/value API. Not a state in the state machine.

Do you still think a state of the state machine should be done?

Here is a WIP with comment 40 taken into account.

Another patch where I worked into the LicenseRenewalCartEventSubscriber.

The right license is selected but after the addition to the cart. The cart is empty.

I think I will need to discuss a little bit with you @joachim.

Thanks for the review!

1. Done: there is no type in the interval module to inherit from.
2. Done
3. I can't because I need a object that has the "sub" function to remove the timestamp interval. (and I didn't found how from the interval object print the corresponding timestamp)
4. Done
5. Done
6. Done
7. Done: but same result => no more item in the cart
8. I have used an uid instead of an account object to be aligned with the createFromProductVariation method. Do you still want an account?
9. Done

OK, I found why I have an empty cart...

"You already have the member role. You may not purchase the adhésion product." => commerce_license/src/Plugin/Commerce/LicenseType/Role.php

Someone else is emptying it.

OK I found why LicenseAvailabilityCheckerExistingRights was still in the way. See attached patch and interdiff.

    $renewal_window_start_time = new \DateTimeImmutable(date('r', \Drupal::time()->getRequestTime()));

Otherwise there will be a fatal error.

Now we need to see how to properly extends license expiration time.

Almost done.

The problem I have to extend the expiration timestamp is that in commerce_license/src/EventSubscriber/LicenseOrderSyncSubscriber.php, at the end of onCartOrderTransition method:

 // Attempt to activate and confirm the license.
      // TODO: This needs to be expanded for synchronizable licenses.
      // TODO: how does a license type plugin indicate that it's not able to
      // activate? And how do we notify the order at this point?
      $transition = $license->getState()->getWorkflow()->getTransition('activate');
      $license->getState()->applyTransition($transition);
      $license->save();

      $transition = $license->getState()->getWorkflow()->getTransition('confirm');
      $license->getState()->applyTransition($transition);
      $license->save();

So the activate transition is triggered first so in License presave method:

// Renewal completed.
        dpm($this->original->state->value);
        dpm($this->state->value);
        if ($this->original->state->value == 'renewal_in_progress') {
          $this->setExpiresTime($this->calculateExpirationTime($this->getExpiresTime()));
        }

$this->original->state->value is equal to pending and not renewal_in_progress.

And the other thing to handle is remoing item from the cart.

Annnddd!!! Done.

Thanks for your help @joachim.

Renewing is ok. Removing an order item is handled.

Now it needs a (final) review and automated tests.

Here a patch that at least fixes the existing tests.

Point 6 from previous comment taken into account.

As seen together the problem is that we need the transition because we do not have more info in the presave method.

Also a query on order item referencing the license won't work because of this feature. We would have the order items from the previous orders that will reference the license.

Tests that needs to be done:

• For a non renewable license: you can't renew
• For a renewable license:
  1. buy a license
  2. complete the order
  3. buy the same license in the renewable window
  4. complete the order
  5. see that the expires has been augmented of the license duration
• For a renewable license: test that you can't renew outsite the renewable window
• For a renewable license:
  1. buy a license
  2. complete the order
  3. buy the same license in the renewable window
  4. stop at the cart state
  5. remove the product from the cart
  6. see that the license in the active state and the expire time has been untouched

@joachim,

I am currently in spectrum C (thunder sprint room). If we can meet to see how to do the tests, it would speed up closing this issue.

Here are WIP tests.

We already found 3 bugs.

We need to figure out why the license is not updated.

Putting the list of tests to implement in the issue summary.

Hello,

Here is a new patch that provides the remaining tests.

The tests implying cart manipulation still fail for an unknown reason. The order item is not the right one or not well detected.

I have updated the issue summary.

Debugging the calls.

It is after the cart save that the order item is removed and it seems because of a bug in the existing license detection.

Because the existing license is in the "renewal_in_progress" state.

I continue testing and will provide a new patch by the end of the day of it is a success.

I made the tests passing but after a last manual tests I found bugs.

When renewing a license the user lost is role.

The expiration time of the license is not updated correctly (but the tests about that are green...) Searching why.

Ok.

I found out why it was not working anymore, it was because in my manual tests I used another order type than the default one.

I have added a message when trying to renew outside the renewal window for better UX.

@joachim: only stay the "problem" when in the renewal process the user lost the license grant. Is it a problem?

Thanks for the review.

Fix tests due to the addition of the drupal_set_message in License.php.

@joachim, Thanks for the review.

Here is a new patch where I moved the drupal_set_message where you asked.

Also I had chosen License.php canRenew method to know directly if the license was renewable and to get the license renewal window start.

About the lost of the license grant in the renewal process:

As soon as you add a product in your cart, if you already have a license that you can renew, this license is used in the order item and saved to have its state changed into "renewal in progress" state.

But in License.php::preSave(), has the state is changed and not set to "active" we fall into:

 // The state is being moved away from 'active'.
      if (isset($this->original) && $this->original->state->value == 'active') {
        // The license is revoked.
        $this->getTypePlugin()->revokeLicense($this);
      }

And so the grant (in my use case, the role) is removed.

@joachim: Thanks for your feedback. I will wait for your feedback on the best architecture for this feature.

Hi joachim,

Hum why not.

So with this solution, no need for the new state renewal in progress. OK.

I don't know when I will have the time to remake a patch.

Hi. To me this is the most valuable of all featured requests, as it falls into the most basic of use case: I have a role license, I want to be able to renew it before it expires so I continue to get all the fun features I bought without a day of missing out.

The patch is working for me: my user gets the state of "Renewal in Progress" with the correctly updated expires date. (Although I will be continuing to test this heavily throughout the day and night.)

This patch provides a great alternative to having to be forced to use Recurring in order to let users continue with their level of site access without a day of interrupted service, as they cannot add role licenses to carts when they already have that role. This module should be able to handle renewals without the aid of Recurring, imo.

Please help me to understand how to get this feature request into the module, as the module is somewhat unusable on its own without it due to the glitching of not being able to reup before expiration.

Thank you so much.

Hi,

Last patch don't just extend the time of the existent license, but create a new license. Then, after renewal, you have 2 licenses, one that is going to expire soon. And the second that will expire later (the new one).

But, when expiration date of the old license arrive:

- this license becomes expired AND its user receives a mail message warning about it (even when he has a second license, what is renewing thew old and now expired license).

- if the license is granting an user role, when the old license becomes expired, its user loose his role.

I think both are not designed behaviors and the second is, in fact, a major issue.

Hi there,

I had a look at patch 64, made a bunch of testing, and AFAIK in my local version now, most of the renewable feature seem to be ok, with very few changes.

All of the listed tests above that need to be done are passed successfully.
I then changed messages as following :
- replaced all drupal_set_message with Drupal::messenger().
- added a "renewalStartTimeMessage" when the license is renewable but out of the renew dates window...
- in that same case, removed the NotPurchasableMessage (You may not purchase the @product-label product. )
- added a "You have an existing license for @product-label until @expires-time. This will be extended until @extended-date when you complete this order." if ($existing_license && $existing_license->canRenew())

Still to be done IMHO for renewable licenses :

- when a product variation type providing a license has « Activate license when order is placed » parameter set to true, expire time is not updated :
=> when licence added to cart, state changes to renewal in progress
=> when order in completed with payment, state changes back to active
=> but expire time is not updated & validating order afterwards does not update expire time neither

- add in the README that orders that contain a license have to be validated before renewing them, otherwise if you’re in the renewable window and purchase it again, this will create a duplicate instead
- or add a contrainst to prevent these dupes ?
- remove Drupal\commerce_order\Plugin\Validation\Constraint message @product-label is not available with a quantity of @quantity. (in fact this one is not specific to renewable licenses)
Thank you.

Here's a patch, based on @Grimreaper #64 one.

Fix CartManagerTestTrait fatal error : it has been moved to Drupal\Tests\commerce_cart\Traits\CartManagerTestTrait.
(Todo it is deprecated, not sure how to rewrite the test)
Fix (hopefully) most coding standards messages.

My bad, I forgot a couple of CartManagerTestTrait wrong use declarations. Provides a new one.

Looked at the tests results :
- some report a problem similar to this Commerce Stripe issue about :

Commerce 2.15 introduced a new submodule commerce_number_pattern. If this is not correctly setup in the tests they will fail with errors such as Drupal\Core\Config\Schema\SchemaIncompleteException: No schema for commerce_number_pattern.commerce_number_pattern.order_default

- other failed tests refer to an existing issue : Tests broken for 8.x-2.x.

I may not pollute the current issue with these. The provided patch IMHO fixes most of current "renewable license" issue and I hope will be tested / reviewed soon.

When testing the feature, please remember that : orders that contain a license have to be validated before renewing its license, otherwise if you’re in the renewable window and purchase it again, this will create a duplicate.

My version now ships also with following patches.

• Don't delete renewed licenses when deleting orders.
• Order Deletion Error
• Deleted Users error

After some more days developing and testing, I can confirm that all tests listed are passed, with @grimreaper work and patch 76 above.

So I'm going to stick with this for now and push to production.

Wow Thanks for the patch Kojo.

I can confirm its working in my local dev environment.

It did not create duplicate licenses with my own limited testing.

The patch applied cleanly to the latest dev version.

I'm not sure this is correct but I think this updated patch solves all of the testing problems except one. Most of them were related to missing modules.

I wasn't sure how to fix the empty array but I thought I would throw what I have out there to get the ball rolling.

I forgot to include some untracked files in the last patch.

The test needed to bypass the access check to query storage.

Wow (my turn ;-) thanks for handling the tests failure, Pmagunia !

I tested on a fresh install of demo project, plus License 2.x-dev and #86 applies smoothy.

I performed a few tests :

- Set up product, variations, orders and all the stuff...

- Purchased normal product, fine. Purchased non renewable license, fine. Purchased renewable license, fine.

- Finally re-purchased the same one in the renewable window. Message informing the license will be renewed is displayed, and the license is extended. I tried with Interval based on reference date then with rolling interval. I tried both with admin and anonymous accounts.

IMHO, RTBC.

Great work. I've tested on a product with a renewable license and rolling interval and all it worked fine. Renewal was allowed and interval extended correctly.

core: "8.9.1"
commerce: "2.x-dev",
commerce_cart_advanced: "^1.0@beta",
commerce_cart_flyout: "^1.7",
commerce_cart_redirection: "^2.1",
commerce_license: "2.x-dev",
commerce_paypal: "1.x-dev",
commerce_reports: "1.x-dev",
commerce_shipping: "2.x-dev"

I'm using the patch on #86 on a project and its working fine. Thanks!

How to avoid the queue job that was created before expiring the license that had been renewed?

Patched the AdvancedQueue plugin 'commerce_license_expire'.

to do not expire the license that was renewed before the queue being processed.

if ($license->getExpiresTime() > $this->time->getRequestTime()) {
      return JobResult::failure('License is not expired.');
}

And I think the expire time should be re-calculated from now if it was already expired.

Any time estimate when this would be committed to the dev version?

I'm starting a new project where this module/feature would be an excellent fit...

Thank you for all the hard work everyone!

I need the very same functionality but with unlimited licenses. Because I am using Commerce Subscription and it requires to set the license type to unlimited, since intervals are managed by subscriptions instead of license itself (as far as I understand).
Patch applied smoothly but did nothing to my installation, people still can not upgrade their subscription / renew license because since it is unlimited it just says they may not pursache the same license again.

Any solution to this? Thanks in advance!

I have resolved the issue by using the hook_entity_insert() in my Custom module. Below is the hook implementation.

/**
 * Implements hook_entity_insert().
 * 
 * License expires recalculate for an existing license. 
 */
function MODULE_entity_insert(Drupal\Core\Entity\EntityInterface $entity) {
  if ($entity->getEntityType()->id() == 'commerce_license') {
    $uid = $entity->getCurrentUserId();

    // Check license type already purchased by the same user or not.
    $plugin_id = $entity->getExpirationPluginType();
    $request_time = \Drupal::service('datetime.time')->getRequestTime();

    $expiration_type_plugin = $entity->getExpirationPluginType();
    $query = \Drupal::service('entity_type.manager')->getStorage('commerce_license')
      ->getQuery()
      ->condition('state', 'active')
      ->condition('expiration_type__target_plugin_id', $plugin_id)
      ->condition('uid', $uid)
      ->condition('type', $entity->type->value)
      ->condition('expires', $request_time, '>=')
      ->condition('expires', 0, '<>')
      ->sort('license_id', 'DESC')
      ->range(0, 1);
    $license_ids = $query->execute();
    
    foreach ($license_ids as $license_id) {
      $expiration_plugin = $entity->getExpirationPlugin(); // Plugin type.
      $oldLicense = Drupal\commerce_license\Entity\License::load($license_id);
      $request_time = $oldLicense->getExpiresTime();

      // Get similar licenses.
      $start_date = (new \DateTimeImmutable('@' . $request_time))
        ->setTimezone(new \DateTimeZone(commerce_licence_get_user_timezone($entity->getOwner())));
      $expiration_date = $expiration_plugin->calculateDate($start_date);
  
      // The returned date is either \DateTimeImmutable or
      // \Drupal\recurring_period\Plugin\RecurringPeriod\RecurringPeriodInterface::UNLIMITED.
      if (is_object($expiration_date)) {
        $expiration_date = $expiration_date->format('U');
      }
      
      $entity->setExpiresTime($expiration_date);
      $entity->save();
    }
  }
}

Has this still not made it to DEV? The patch at #91 won't apply (via Composer) to the current DEV release (11 Feb 21)

@robcarr, have you tried #86? So far it was applying smoothly to DEV.

Also tried the patch at #86 and that fails too 🤔

I've just tried to unpick the patches at #86 and #91, and manually apply the text changes (taken most of the day). The patch files are long (obviously lots of work) and many changes are just unrelated code formatting changes to the whole module - which is an unhelpful distraction to trying to solve this significant issue.

However, I lose track about half way through patch #91 with the changes in src/LicenseAvailabilityCheckerExistingRights.php with the new function introduced:

private function checkPurchasable($entity, $user, $unsetNotPurchasableMessage) {
    $license_type_plugin = $entity->get('license_type')->first()->getTargetInstance();
    $existing_rights_result = $license_type_plugin->checkUserHasExistingRights($user);

    if (!$existing_rights_result->hasExistingRights()) {
      return AvailabilityResult::neutral();
    }

    // Show a message that includes the reason from the rights check.
    if ($user->id() == $this->currentUser->id()) {
      $rights_check_message = $existing_rights_result->getOwnerUserMessage();
    }
    else {
      $rights_check_message = $existing_rights_result->getOtherUserMessage();
    }
    $message = $rights_check_message . ' ' . t("You may not purchase the @product-label product.", [
      '@product-label' => $purchased_entity->label(),
    ]);

    return AvailabilityResult::unavailable($message);
  }

How is $unsetNotPurchasableMessage dealt with?

Also could not make sense of the patches on quite a few of the tests which have clearly changed a lot and most of the changes are already in DEV.

I haven't tested the patch particularly thoroughly, but it seems functional and should apply to the current DEV release (and prob alpha20 too). I'd be grateful if anyone with a bit more expertise could review it, especially the tests and the changes to src/LicenseAvailabilityCheckerExistingRights.php

Sorry last patch had loads of trailing white space errors so wouldn't apply. One attached to this post should apply and work, but if anyone can review the tests listed in the patch at #91, plus the private function checkPurchasable() function in src/LicenseAvailabilityCheckerExistingRights.php in this patch then that may push this issue closer to a solution.

Although the patch at #101 applies locally, it won't apply via Composer and d.o. plus the test has come back with loads of errors. I'm probably a little out of my depth here... 🆘

I'm making lots of mistakes (aka learning) here... attached is yet another patch file generated by a different tool and seems to include full path to module from webroot, so it may not apply to a vanilla D8/9 installation. My previous patches didn't include the 3 new pages created, so they're in this one.

The patch at #104 (commerce-license-ability-to-re-purchase-a-license-to-extend-it-102.patch) just fails everywhere. I hope it's of some use to someone else who can take this forward.

The other problem is that the code in my patches just don't work. Without having much knowledge of the Commerce framework, I tried my best to manually map across a lot of the code from the older patches; clearly my lack of expertise failed in this area.

The renewal functionality is really important to make this module more relevant to real world solutions. I'm also looking at more flexible ways to send expiry notifications (eg, 30 days before expiry), but such functionality depends on this issue being addressed.

Latest updated patch. Tried this out on a few scenarios, so appears functional. However, needs work on tests.

Previous patch failed to include new files 🙄

Added back trait LicenseOrderCompletionTestTrait (/tests/src/Kernel/LicenseOrderCompletionTestTrait.php) to try and get the patch through some tests. This trait was removed in alpha19 (probably for good reason), but causes a critical failure if not included.

The patch at #110 fails a few tests.

If a license is being renewed, but during checkout the item is deleted from the cart, then the whole user's license is deleted. This is current erroneous behaviour on this patch (tested manually too) and reason test fails.

Unsure of reasons for failures in 2-4, one of which being that the base table commerce_license doesn't exist...

Now need to focus on /src/EventSubscriber/LicenseRenewalCartEventSubscriber.php and add public function onCartOrderItemRemove(CartOrderItemRemoveEvent $event). Thought this might work:

public static function getSubscribedEvents() {

    $events = [
      CartEvents::CART_ORDER_ITEM_REMOVE => ['onCartOrderItemRemove', 10],
      CartEvents::CART_ENTITY_ADD => ['onCartEntityAdd', 0],
    ];
    return $events;
  }

I can't get that to trigger a public function onCartOrderItemRemove(CartOrderItemRemoveEvent $event), so not sure where else to look to catch cart item deletions. 🤔

I improved #110.

CommerceOrderSyncRenewalTest was failing because the "activate_on_place" setting was not enabled so not extending the expiry. I added a line to enable "activate_on_place" for the product variation which fixed it.

Regarding removing the item from the cart. There is already commerce_license_commerce_order_item_delete() to delete the license unconditionally. I improved that part. If the license is in the renewal_in_progress state, then it will first put the license in the renewal_cancelled state then back to the active state. We can't put the license in active state directly because of the way License::presave() acts if license transits from renewal_in_progress to active state by extending the expiry time.

I could not fully solve failing of CommerceAvailabilityExistingRightsTest. Need to fix that.

IMPORTANT: The patch is incompatible with the latest release, 8.x-2.0alpha21. It always create new license when attempting to renew the existing one. I think it is happening because of the change appeared in this commit.

Today I found I was wrong with my statement yesterday. It works with 8.x-2.0-alpha21 too. Improved CommerceAvailabilityExistingRightsTest by adding to install the schema for commerce_license entity, which fixed the error happened #110.

Thank you so much for your work on the patch Junaid. I've tested it and it works really well, including the case if the license is removed from the cart. Fantastic progress.

Would be really helpful if someone else could review and double check.

+1 for RTBC

@robcarr glad to hear you found it working.

However, we found it having a con which may require more changing.

We have a production having multiple variations allowing to purchase roles license for 1 year, 2 year and 3 year.

On further testing, we found it is not allowing to repurchase the role license with a different product variation of the same product.

For example, Let's assume I purchased 1 year membership before. Then I am only able to re-purchase 1 year member, not 2 or 3 year memberships!

@all Any inputs and or improvement on the patch are highly appreciated.

Thanks @junaidpv that's working fine for me too.

+1 for RTBC

The only workaround for your patch is to let license expire, which would allow a license of any duration to be re-purchased. Not ideal, but would work. I'm tempted in my current project to only offer 12 month licenses to be purchased.

The patch at #115 no longer works with latest DEV (6 Aug 21)

Re-roll attached. Seems to work, but needs a bit more testing.

Still has the issue where users can only renew for the same duration as currently held.

I think we could quickly get into diminishing returns to allow users to renew a different license (Product Variation). It looks incredibly complex.

If the patch provides acceptable functionality (and a few others can review it), then this might be the most pragmatic way to allow users to renew, as it fulfils the majority of use cases. The caveats would be:

1. Users can only renew their existing license for the same duration, eg, renew a 12 month license for another 12 months.
2. If users wish to move to a different license plan (PV), they have to wait until their current license has expired

The 2nd caveat is the same functionality as offered by the Commerce License module anyway (ie, the license has to expire before renewal is possible), but this patch at least permits renewal prior to expiry in the majority of cases.

Updated patch attached. Last one failed testing (see #3223616: unit tests fail using recent versions of commerce)

Patch #124 seems to be failing testing of Drupal\Tests\commerce_license\Kernel\CommerceOrderSyncRenewalTest::testRemovingProductFromCart at line 390:

   // Remove the item from the cart.
    $this->cartManager->removeOrderItem($cart_order, $order_item);

Resulting in 2 errors:

• Drupal\Core\Entity\EntityStorageException: Attempted to save order 1 with version 2. Current version is 3.
• Drupal\Tests\commerce_license\Kernel\CommerceOrderSyncRenewalTest: test runner returned a non-zero

With no experience of automated testing, I've spent a day scratching my head and getting nowhere with the test code. With a manual test of the 'failing' function (ie, deleting a renewing license from the basket) it works fine, so it seems there's something wrong with the test, rather the patched module code itself.

@joachim Agree. But that was only way I found as said in #114.

caused major error:

Drupal\Core\Entity\EntityStorageException: $string ("You already have one item similar to @product-label in your cart.") must be a string. in Drupal\Core\Entity\Sql\SqlContentEntityStorage->save() (line 846 of drupal8.9/html/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php).

I upgrade the commerce_license
I goto a product page like product/3 and I get WSOD with the above in my logs

Re-roll from #115.

In PHPUnit 9, arraySubsetAsserts is removed. I added a dev package to composer.json which replaces that function.

@robcarr the version error in the test you were seeing was because the cart needed to be reloaded.

Is the inability to renew a license with a different variation a blocker?

EDIT: Didn't mean to add the same file 2x. They are identical.

Added Role label.

Now with all Drupal tests.

@pmagunia thank you for your work on an updated patch. I'm currently working an a different project and I've had to park the project utilising Commerce/Commerce License, but will hopefully test again soon.

I don't believe that the the inability to renew a license with a different variation is a blocker. From a usability perspective, a user wishing to renew and shift to a different plan would just have to let the current plan (License) expire. I think the patch's implementation would suit the majority of cases. It would better to commit this patch to the module and raise a separate issue if this become significant to others.

Thanks for the updated patch pmagunia, I've given it a test and it seems to be working well!

I was getting the same issue as SocialNicheGuru with the previous patch I was using (#124) and also the latest patch.
Drupal\Core\Entity\EntityStorageException: $string ("You already have one item similar to @product-label in your cart.") must be a string. in Drupal\Core\Entity\Sql\SqlContentEntityStorage->save() (line 846 of drupal8.9/html/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php).

I found that it's caused by this line and is caused by $message already being TranslatableMarkup:
\Drupal::messenger()->addError(t($message));

I've updated it to this in the patch - it's the only change from #132:
\Drupal::messenger()->addError($message);

I'm not sure if we should be covering the case where a user tries to add the same product to their cart when the license is already "renewal_in_progress". I tried this and got some undesired behaviour.

The product was added to the cart and the quantity was increased even though LicenseMultiplesCartEventSubscriber is supposed to force the quantity. There weren't any messages about the item being added to the cart either.

The reason for this is due to the LicenseRenewalCartEventSubscriber trying to apply the "renewal_in_progress" transition again to the license. This causes an exception in the Renderer and skips the other event subscribers:
The transition "renewal_in_progress" is currently not allowed. (Current state: "renewal_in_progress".)

Therefore I've added a check to make sure that the license isn't already in the state before applying it. This allows the process to continue as normal. I've done some manual testing and this seems to be working well.

One loophole that I've found is that if the user is logged out, adds the product to the basket, and then logs in within the checkout flow. This creates a new basket and the event subscriber checks are bypassed.

I'm not sure if this is the right approach, but is there any way we could check the contents of the basket when it's assigned to the user on login?

Patch fails to apply for me on Drupal 9.3.6

Rerolling as it did not apply on 9.3.6.

@robotjox, It appears, you added previous patch file in your patch :) Please correct it. Thanks!

oops, sorry about that! Will try to get a new patch up today

Hope this works better...

Another thing I've noticed is that the license is revoked when the license is being renewed. I don't think this is desirable as if the user decides to leave the checkout without renewing, then their license will remain revoked.

I think we just need to update the condition around when a license is being changed from the 'active' state to also check the renewal state before revoking the license. I can update the patch when I get a chance.

We'd looked at that in #114. It's an important scenario to catch, as abandoned carts are quite a frequent occurrence. Thanks for all your work @joekers

The changes to the LicenseOrderProcessorMultiples result in breaking carts with multiple products with (potentially different) licenses being added to a cart - so only a single product with a license can be purchased at a time. This specifically breaks the usage of pado or vado where multiple licenses (e.g. roles) should be bought together.

Moved the latest patch (#141) to the issue fork, hid the tremendous amount of patch files and changed LicenseOrderProcessorMultiples to allow multiple different licenses in a single order.

Took a look at the test issues and figured to leave it be as long as they seem to stem from the dev branch.

Would be great if we could continue working on the issue fork (with the open MR). Patch can be applied just like it is possible with patch files using the plain diff link.

Applies fixes for:

• issue which revoke license when product is added to cart. So, abandoned cart wont be a problem now.
• issue which incorrectly sets the renewal time when product is removed from cart.

@joekers, @robcarr . Please have a look and try to see if it works for you as well.

As already stated earlier, it is not possible to repurchase a different variation. Fixing that will require a major rewrite of the module.

We wanted to fix that in our project. I prepared a patch specifically for our project. We have only three PVs granting role for 1, 2 and 3 years. This hack helped to work with them. I guess, it wont work if you are having more than one type of PVs and more attributes.

Please replace PV bundle name in this patch "our_project_membership_pv" with the corresponding PV bundle name in your project.

Looking forward to hear if anybody found this helpful and feedback.

This (large) patch/MR (with some modifications) has been committed.

The primary additional changes are:

1. Better handling during delete of an order item
2. Handle more cases were renewal_in_progress should be checked alongside active, e.g. during the deletion of a license or when cron is processing expiring licenses
3. Fix/Update more tests

This patch appears to be partly responsible for critical error in 3.0-rc1: https://www.drupal.org/project/commerce_license/issues/3362663