There is a very simple use case for this module to allow for "subscription" products to sites where a user is buying a role. For example I could buy a yearly subscription to the site and get granted a simple role. This is easy to do with the module out of the box with the role license class. There are some people using this workflow that don't require or want recurring billing, orders, etc. They simply just want to send a reminder email out and have the user come back and purchase another licenses to "renew" their membership or subscription.

Ideally if I buy a licenses, it would check to see if I already have one for this particular product and extend the expiration date instead or make the start of the new licenses start when the current one expires so there is no overlapping time.

I have spoken to bojanz in IRC about this and he recommended that I simply check for this inside hook_form_commerce_cart_add_to_cart_form_alter, look for existing licenses and set it to $form_state['line_item']->commerce_license. Then when it comes time to grant, we simply extend with ->renew()

This seems pretty straight forward, the only issue is that this will require users to be logged in before they could add these types of license products to the cart, which I don't think is the case for most people. Thoughts on that?

The alternative is instead of renewing/extending licenses I can post date the start of new licenses to exactly when the current ones expire. When the grant happens, we have a user on the order and it could technically all happing in the single save method on the license.

Would love some thoughts on all of this, specificity best practices so I can write this as a patch and new feature to the module and not a separate module. My main goal in this is to get this simple subscription type setup so I can end of life the Commerce Subscription Product module.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mrconnerton’s picture

Title: Option to renew license » Option to renew license or post date grant start and expire
mrconnerton’s picture

Issue summary: View changes
mrconnerton’s picture

I have attached the simplest patch which, upon setting the grant time for a license, checks to see if a license for that user and product is already active, and if it is, changes the start to when the previous one expires.

mrconnerton’s picture

What I plan to personally do next is add a configuration field on the product type to enable or disable this behaviour. Some warnings to users "Hey you already have an active license until X date. This license you are buying will start on X date"

This could go in core commerce_license module or it could just as easily be a custom class instead.

brad.curnow’s picture

Hi all,

Thanks for your work on this - I'm more of a webmaster than a developer so can't contribute code (though I could potentially contribute some $$ to help out) but I can offer my specific use case as to why we need this.

Basically we're offering role (and perhaps later, node) by weekly/monthly/yearly recurring subscription and would like to use PayPal recurring payments along with that. I was surprised that the choices for this when using Commerce were so sparse - as far as I can see the options are Commerce SP + PayPal, or Commerce Paypal IPN Rules integration. There seems to be no PayPal integration with the recurring framework module.

So, I was going to use Commerce SP but since it's being phased out figure I need to make use of something else. Not sure what the best practice (if there is any!) here is?

Cheers!

BC

mrconnerton’s picture

Hey Brad, thanks for the support. For your use case (as far as I can interpret) you more or less have ~3 options:

1) If you are set on using paypal AND having the subscriptions be managed by paypal then you need to look into commerce_paypal module. I did quickly find https://drupal.org/node/1780740 which is a very old patch for setting products to make the payment as a subscription. You would just have to dig into the issue queue there. Then how that ties back into commerce_license I'm not sure.

2) If you are not set on using paypal to managed subscriptions, then you can let drupal manage the subscriptions. commerce_recurring + card on file module seems to be a good solution. I believe there is an issue open to get paypal support for card on file as well. commerce_recurring will have its own rules you would use for granting roles, etc.

3) The other drupal solution for managing the subscriptions is commerce_license + https://drupal.org/project/commerce_license_billing which again uses card on file to auto charge people every billing period with commerce_license as the backend

brad.curnow’s picture

Thanks for the quick response!

Some food for thought indeed. I think I may also try with Ubercart for now - my preference is to use Commerce but for my current project we're not doing anything overly elaborate, so UC may be a good choice for the time being.

pinkonomy’s picture

I tried patch from #3 using patch -p3 etc but it failed patching:
I got this message: Hunk #1 FAILED at 125.
I had also another patch on this module.Could this be the issue for the failed patching?
When this patch will be in at least dev version of this module?
thanks

bojanz’s picture

This patch won't be committed.
The same thing is possible with a hook_entity_presave(), mrconnerton will post his final code
and we'll create documentation for his and other related approaches for semi-automatic renewals.

pinkonomy’s picture

I think this patch is needed since I found a bug in the "logic" of Commerce Linsence.
-A user buys a lisence and is assigned a role.
-Before the expiration date is reaching user buys a second License.
-When the first license expires,the role is removed from the user.
So user has no role even though the second license is still active.

Is my logic correct or am I missing something?
thanks

bojanz’s picture

Yes, that is correct. But the module can't make assumptions on how renewals happen (via commerce_license_billing, by creating a new license that starts where the old one ended, by manually renewing the existing license...)
We'll document all options.

pinkonomy’s picture

Okay thanks bojan)

dang42’s picture

My use case would only require that, on "renewal", the expriation date for the existing license be extended by a set period of time (one year, in this case...).

The association I'm working with doesn't need to know how many 1 year memberships an individual has had, just that they've been a member since "X date" and when that membership expires. Is this / will this be an option in the code referenced in #9?

Also, while the research I've done thus far leads me to think commerce_license is my best bet for what I need to implement, if there are other modules that might be better suited I'd love to hear about them.

Thank you very much for any / all help!

drupalixo’s picture

Thanks for the patch in #3 - the new granted date in second license is the expired date from first license. ok.
But the role is gone. How can I set the role (#10) again for the user? With rules?
My following module-code is not the right. What is incorrect?

/**
* Implements hook_entity_presave();
*/
function de_dc_role_renewed_entity_presave($entity, $type) {
   if(($entity->type == 'role') && ($entity->status == 3)){ // 3=expired 
       $lizuid = $entity->uid;
       $lizstatus = $entity->status;
       $lizprodid = $entity->product_id;
       $lizexpires = $entity->expires;
       $lizgranted = $entity->granted;
       
       $result = db_query("SELECT uid, status, product_id, granted FROM {commerce_license} WHERE uid = :uid AND status = :status AND product_id = :product_id AND granted = :granted", array( 
           ':uid' => $lizuid, 
           ':status' => 2,
           ':product_id' => $lizprodid,
           ':granted' => $lizexpires,
));   
      
    $data = array();
    foreach ($result as $record) {
    $data[] = $record;     
  }
  if ($data != '') { 
  db_update('commerce_license')
  ->fields(array('status' => 2))  // set status again to 2 = active...
  ->condition('uid', $lizuid)  
  ->execute(); 
  }        
 }
}


If it gives no solution in DC license, I must unfortunately take Ubercart, here are this functions out of the box ...

Thanks for help!

bojanz’s picture

So, to summarize, there are three basic use cases / solutions:
1) Commerce License Billing (renewal is automatic and done for you)
2) Creating a new license that starts right after the previous active one (mrconnerton's use case, doable with a hook_entity_presave())
3) Extending the duration of an existing license

The third one can be a bit tricky UX wise:
- are you buying a "renewal" product or the original one?
- do you allow anonymous checkout or not? (Much easier if you don't)
Commerce License Billing conceptually extends the duration of an existing license of course, since if the license represents a remote account, you can't just trash it because you can't find out the parameters again

@drupalixo
You are attempting to do #3.
You are doing a db_update against an entity base table, which should never be done. You just need to modify $entity->status instead.

drupalixo’s picture

Hi bojanz, (and all)
sorry for delay...

Thank you for your answer, but I tried it, I can't understand it..
The version 1.3 I tried also... no solution...
Why is #3 not in version 1.3 inclusive?
Do you have a sample-(module)-code for hook_entity_presave() in addition to #3?

Thanks for help!

garphy’s picture

FWIW, here's a way to implement (2) (from #15) through hook_entity_presave().


/**
 * Implements hook_entity_presave().
 */
function MODULE_entity_presave($entity, $type) {

  if ($type == 'commerce_license' && empty($entity->granted)) {
    $query = new EntityFieldQuery();
    $result = $query
      ->entityCondition('entity_type', 'commerce_license')
      ->propertyCondition('uid', $entity->uid)
      ->propertyCondition('product_id', $entity->product_id)
      ->propertyCondition('status', COMMERCE_LICENSE_ACTIVE)
      ->propertyOrderBy('expires', 'DESC')
      ->range(0, 1)
      ->execute();

    if (isset($result['commerce_license'])) {
      $current_id = array_shift(array_keys($result['commerce_license']));
      $license = entity_load_single('commerce_license', $current_id);
      $entity->granted = $license->expires;
      $duration = $entity->wrapper->product->commerce_license_duration->value();
      if ($duration > 0 && empty($entity->expires)) {
        $entity->expires = $entity->granted + $duration;
      }
    }

  }

}

The main pitfall to avoid : $this become $entity and don't forget to handle the expiration date as setting the granted date will prevent the base class code to calculate expiry date.

Feedback appreciated.

garphy’s picture

As mentioned in #10 & #11 there's a flaw that prevent option 2) from working.

Licence are always made "active" when checkout is complete, even if their granted date is not reached. In case of "Role license", the role would be given immediately. (In our use case, the role is already granted by the previous licence anyway).

When the first licence expire, the role is removed from user but the second licence never re-grant the role because it has already been "activated".

I'm puzzled at how to resolve this...

1) Prevent licence from become "active" when the granted date is not reached, this needs to make hook_cron activate() such deferred licences (and avoid races such as activating the next licence before revoking the current, which would leave us in the same situation).
2) Make role licence revoke() lookup for other licence of the same product id & user id and abort role removal. (but it would have to also check the granted date 'cause all licence are "active" whether or not the granted date is reached).

We have two bugs there ?

grahamvalue’s picture

Subscribing.

Is there any way to extend licenses/subscriptions/memberships on a site that uses commerce_license and commerce_paypal, without storing credit card information?

garphy’s picture

@serenitystocks.com: maybe by using https://www.drupal.org/project/commerce_cardonfile with a Paypal integration (don't now if there's any).

grahamvalue’s picture

Thank you, Garphy!

The question is essentially the same as the OP's - can a user renew or extend a license before it expires, without setting up a recurring subscription?

BTW, a big thank you to those behind the Commerce framework, as well as to the Drupal community (as always!).

garphy’s picture

@serenitystocks.com: It is what I'm trying to implement with the code in #17, but there's a design issue with the way license are activate during the checkout (see #18).

Another way of solving this is trying to load the same subscription during the checkout and simply alter its expiration date but I've not found a way to do this properly right now.

grahamvalue’s picture

Thank you for your help, Garphy!
Will wait for updates.

zebda’s picture

Is there any new on this subject? I'm also looking for a way for my users to renew their license.
So they have a license for a year, a month before it ends they want to renew it.
When they do this, a new year is added to the current license. So there is no overlap in the two licenses...is this possible yet.

pwsherman’s picture

I also need to extend a license (option 3 from #15).

stevep’s picture

According to Amey Mudras over on stackoverflow, there is a solution for 3 from #15 (see http://stackoverflow.com/questions/22046549/drupal-commerce-renew-role-s...)

He writes:

I had implemented such a solution before. There was a subscription for a year,two years & three years. The problem was how to maintain the subscription limit & how to extend it.

The solution was creating a hidden field within for every user. When a user first buys a package for a year we programatically make a entry of the end timestamp in the end field. When he buys one more package the current timestamp value of the field is added by additional timestamp

timestamp = timestamp + 1years timestamp
or

timestamp = timestamp + 2years timestamp

Conceptually this makes sense -- create a field, call it say 'endsub', and use that if endsub is not null, and greater than present date, and add to the workflow.

I'm not very familiar witih rules, but it seems doable. If I get it working I'll update here.

lawrencey’s picture

Hi Everyone,

Thanks for all the interesting discussion on this post. I've used that to come up with a customization to the CommerceLicenseRole save function, based on the presave logic at #17. I added the following query code in the case where the license is > COMMERCE_LICENSE_ACTIVE to determine if there is another active license for the same uid and product id. If so, I didn't delete the role. I can't see any drawbacks of this, but wondered if anyone else can spot if I've missed something here.

...
elseif ($this->status > COMMERCE_LICENSE_ACTIVE) {
// Custom code start - Check if there is another license still active for the same product and uid - If so, don't remove the role
$query = new EntityFieldQuery();
$result = $query
->entityCondition('entity_type', 'commerce_license')
->propertyCondition('uid', $this->uid)
->propertyCondition('product_id', $this->product_id)
->propertyCondition('status', COMMERCE_LICENSE_ACTIVE)
->propertyCondition('expires', time(), '>')
->propertyOrderBy('expires', 'DESC')
->range(0, 1)
->execute();

if (!isset($result['commerce_license'])) {
// No existing other active licenses for this user and product
// Wrapping if around exsting code below
// The owner of an inactive license must not have the role.
if (isset($owner->roles[$role])) {
unset($owner->roles[$role]);
$save_owner = TRUE;
}
}
// end of custom changes
}
->range(0, 1)
->execute();

if (!isset($result['commerce_license'])) {

A quick question of my own though - why is it that when I revoke a licence, the role is not removed? It seems to bypass the CommerceLicenceRole class altogether? I don't understand if that's by design and why that's so or if its a bug?

Thanks

millenniumtree’s picture

I'm trying to build a setup like Situation 2 from comment #15.
No recurring billing, multiple role licenses, with no 'overlap'.
I found a few issues already in trying to set this up (using hook_entity_presave)

1) The submitted code patches don't account for different products providing the same role (different durations). I have code to handle that.
2) If the latest license is already expired, the new license should start from the present, not from the last expired license. I solved that too.
3) When a license expires, it removes the role, even though the next consecutive license should re-add it. It does not.
4) For new 'extension' licenses that are granted in the future, the role is still added to the user on checkout - no action should be taken until the grant time.
5) If multiple products are purchased at the same time, providing the same role, the new extension licenses are all granted at the same time.

I'm trying to solve each of the above in one simple custom module, because we really need this to work.

pinkonomy’s picture

msypes’s picture

Add me to the list of people looking to handle license renewals. Either case 2 or 3 from comment #15 would work for me, as I'm just doing a simple in-house membership registration. (I'm in too much of a hurry at this time to risk going with membership_entity, which is currently in alpha and has 10% of this module's "sites using".

I'm going to experiment with garphy's code in #17 as others have, but I'm also thinking of possibly changing the pre-existing license's expiration and later removing the new license, perhaps in a cron task instead. This would handle the noted problems of deleting roles, yes?

asak’s picture

FileSize
6.06 KB

Yes. As far as I can tell - this is critical functionality for this module,
And after testing out various situations on a development site -
It seems this is the only thing missing for a complete implementation in real-world production.

So - I gave it a shot with a small module i made that handles multiple roles (instead of 1 roles as provided in the examples).

I used the code by garphy in #17,
and according to his concerns in #18 -
implemented the fix that lawrencey's code in #27 suggests.

Going over the issues millenniumtree found in #28 -
1) I implemented a fix for that, comparing the roles before making a decision.
2) Seems to work correctly
3) Seems to work correctly
4) Since we only consider active licenses - this is not an issue as far as i can tell
5) I didn't deal with this. For my use case, purchase is limited to 1 license per order.

My initial tests show good results.
Licenses are created, update and revoked correctly,
granting and revoking user roles as desired.

I'm attaching the complete module files here -
This isn't a patch solution, it's a complete module.

Feel free to break this up and take the code to implement (possibly) a more generic solution.

If anyone feels like giving this a spin and checking for any other/remaining issues -
That would be great :)

msypes’s picture

I will try out asak's code in the next few weeks, so thanks!
I can also report that in tests I've run so far, the idea of updating existing/active licenses to have the same expiration as the newly purchased one is working in my implementation, so there's at least back-up, hackish as it might be.

Grimreaper’s picture

Hello,

Thanks all for your code snippets, modules, suggestions, ...

I used the post of asak #31. To make this patch.

dtamajon’s picture

Hi,

I have checked this last patch, and I get an error on next line execution:

$this_license_role = $entity->wrapper->product->commerce_license_role->value();

Error is:

EntityMetadataWrapperException: Unknown data property commerce_license_role. en EntityStructureWrapper->getPropertyInfo() (line 335 of [mypath]entity.wrapper.inc).

dtamajon’s picture

I realised the presave function is called more than once, and in the firsts calls there is no a product_id. So it's working properly if adding this check to the initial condition:

function commerce_license_role_entity_presave($entity, $type) {
  if ($type == 'commerce_license' && empty($entity->granted) && !empty($entity->product_id)) {
Grimreaper’s picture

Hello,

Thank you for testing. But I cannot reproduce the bug.

- I have activated the module with the patch.
- added the commerce licence role management to product (default product type of drupal commerce)
- created a product with a role license
- added the product to my cart
- done the full process with the exemple payment module

I didn't have any error.

Could you give steps to reproduce please ? Thank you.

dtamajon’s picture

After this steps, with the checkout finished, start a second process with the same user, including the same product on the cart. When you do checkout, the error is raised.

On the first cycle (which you described) is working fine.

Grimreaper’s picture

Ok, I made a second cycle and I got no problem.

I think there is a detail missing.

dtamajon’s picture

Ok, I try to explain the full environment. Maybe there is something related to other functionality.

We are selling subscriptions, so when a user click for a product, Rules are raised in order to:

  • Delete any previous product in the cart
  • Redirect to the checkout, avoiding cart page

I have checked deactivating both rules, but I continue having the same problem.

I will try to review all Rules and defined checkout process in order to find a point which is raising presave before of time. When I have a conclusion, I will be back with results.

magicmyth’s picture

I got the same issue as dtamajon with Grimreaper's patch from #33. dtamajon fix in #35 resolved. This is the workflow that caused it for myself.

  • Install Commerce Kickstarter 2 (may need to update Inline Entity Form to latest dev to resolve other issues).
  • Install Commerce License Role
  • Create product type with License field
  • Purchase a license. Everything works as expected out of the box. Eg license activates role but additional license purchase does not begin at the end of existing license.
  • Apply patch.
  • Add license product to cart as before and the error mentioned in #34 is thrown. Note that Commerce Kickstart installs a module that creates an overlay message for go to checkout or continue shopping when an item is added to the cart.

BTW the patch in #33 is slightly broken for me and caused a different error to be thrown when accessing the cart with a license product in it. This was because the line parent::save(); had been nested within a condition statement in the CommerceLicenseRole::save when it should always be called at the end of that method. If some wants I could make an updated patch?

Grimreaper’s picture

Status: Active » Needs review
FileSize
5.29 KB

Hello,

I do not encounter the problem mentionned in the comments after #33. But I remade a patch with your suggestions as it does not break my use case.

Thanks for the review and the tests.

jami3z’s picture

I've implemented #41 but I am still having the issue where the expiry date of the current active licence with the same role is not extended and a new licence is created at the start of the existing expiry. According to the comments of the commerce_license_role_entity_presave() added by this patch

compare roles to see if we need to create a new license as usual, or modify the granted and expiring dates if this license according to an existing active license which grants the same roles as this one:

the existing active license should be updated?

Grimreaper’s picture

Hello,

The behavior you described is the one that is implemented in the patch. Yes, the comment is not very clear. I think because there is a typo in it.

It should be "of this license" and not "if this license".

The problem is that as soon as you add your product in your cart, a commerce license is created, so to avoid consistency problem, it is easier to modify the start and expire timestamp of the new license than deleting the new one and editing the old one.

Is it good for you or do you need some help/explanation ?

amh5514’s picture

Patch from #41 worked for me.

  • Role was added when product w/ license was purchased
  • Second product purchased, moving start and expire date consecutively with first active license
  • Role remained while first and second license was active
  • Role removed when second license expired.

Thank you!

The only thing I would suggest is changing the second license status to pending until the granted date and then switching it to active.

darksnow’s picture

I'm looking to implement something like this too.

Are any of these patches going to be committed? I don't want the maintenace overhead of having to manage these changes if commerce_license gets updated on my site.

In relation to that, for the patch in #41 can you confirm that I understand it correctly.

The code in hook_entity_presave() changes the start and expiry dates of the new license, based on any old licenses found. This prevent the new license overlapping the old and can be implemented in another custom module rather than modifying commerce_license.

The changes to CommerceLicenseRole.class.php are there just to handle multiple different roles (or the same role) from multiple licenses. Is this modification required to allow the presave hook to function?

I guess what I'm asking is, can I do this in a custom module, for maintenance reasons, rather than having to modify commerce_license at all?

Grimreaper’s picture

Status: Needs review » Reviewed & tested by the community

Hello,

I hope too that the patch will be merged.

@amh5514: Thanks for the test. Yes, that could be an improvement but the query:

$result = $query
      ->entityCondition('entity_type', 'commerce_license')
      ->propertyCondition('uid', $entity->uid)
      ->propertyCondition('status', COMMERCE_LICENSE_ACTIVE)
      ->propertyOrderBy('expires', 'DESC')
      ->range(0, 1)
      ->execute();

In the hook_entity_presave would not found the good licenses and then the renewal will not get the good dates.

I change the status in RTBC as it is ok for you.

@darksnow: Yes you understand the patch perfectly.

I think you can extract the changes in CommerceLicenseRole.class.php in a hook_entity_presave or hook_entity_update, I don't have test it as the part to assure consistency among roles was in this class. If you will make some code, I will be interested to see it.

Maybe if this patch is not commited I will make a sub-module in its own project because me too I don't want to lose time keeping an eye on this issue for maintenance.

darksnow’s picture

Thanks Grimreaper, I've just copied the hook implementation from the code above and it seems to be working.

A new license is being created and forward dated to it doesn't overlap the old one. I've not fully tested this as yet though, but it looks like the new license will reinstate the role revoked by the old license when the queue is run.

garphy’s picture

Status: Reviewed & tested by the community » Needs review
+/**
+ * Implements hook_entity_presave().
+ */
+function commerce_license_role_entity_presave($entity, $type) {
+  // we use this presave hook to set future
+  // granted & expires dates for license renewals,
+  // in case the user has an active license granting
+  // the same roles as the currently saved license:
+  if ($type == 'commerce_license' && empty($entity->granted) && !empty($entity->product_id)) {

Shouldn't we check that $entity->type === 'role' in order to avoid altering the behaviour of other kinds of commerce_license ?

garphy’s picture

+    $result = $query
+      ->entityCondition('entity_type', 'commerce_license')
+      ->propertyCondition('uid', $entity->uid)
+      ->propertyCondition('status', COMMERCE_LICENSE_ACTIVE)
+      ->propertyOrderBy('expires', 'DESC')
+      ->range(0, 1)
+      ->execute();

This query should also be restricted to ->propertyCondition('type', 'role') to avoid messing with another type of active license that could potentially exist it the database.

Grimreaper’s picture

Hello,

Thanks Garphy for the review. I have added the two conditions you suggest.

garphy’s picture

I thought the CommerceLicenseRole::save() method would also benefit from the same condition in its database query.

capfive’s picture

This is looking great! hope this can be pushed into a commit soon! good work guys!

darksnow’s picture

I might need to post a new issue for this, but how would I go about renewing the existing license rather than use it to set the expires date of the new one?

I've used the patch provided above successfully and my system now forward dates any newly purchased license so we have no overlap. I've not seen any issue with the system revoking a license when there's another valid one so things are fine.

I also have a system of notifications for license expiry. These emails should stop if a new license has been purchased, but since I'm creating a new license, the old one is still about to expire so subsequent emails get sent. Not ideal so I would rather the system just calls

$license->renew($license->expires + $duration);

It seems to me that I can't do that in entity presave because the renewal should happen after the license has been paid for. So my question is firstly, how do I prevent a new license being created if there's an existing one, and how do I subsequently renew the old one after payment?

garphy’s picture

I think you'd better add a check in your notification system to prevent notification if there's a valid non-expired licence following the current one.
By design, the license entity seems very closely related to the order / line item entity. Maybe if you find a way to create a order with a line item referencing the existing license?

garphy’s picture

Status: Needs review » Needs work

Crap! I think I found a flaw...

I wanted to administratively revoke an active license and the rôle was not removed because there's still an active license... the one I want to revoke.

A quick fix would be to exclude the license being saved from the checking query but still, this query would have to be improved : If we revoke an active license before the "granted" date of the potentially existing future active license, we still have to remove the role, which will be added back when the date is reached.

(and this would still cause trouble because the future license is already "active", by design so no update will be triggered and the role won't be added back).

Are we doing it wrong with this approach ? wouldn't it be much simpler to extend the "expires" date of the active license ? (which raise the issue of how a single license can relates to two or more orders).

I would love having @bojanz input on all of this.

garphy’s picture

New patch :
- exclude currently saved license from the checking query
- ensure the checking query only extract license which have "granted" date in the past.

darksnow’s picture

Thanks for the suggestion garphy, I've solved my problem but I agree with your comment in #55, I still think the best approach would be to extend the existing license.

Any idea how to go about doing this in a clean and consistent manner?

darksnow’s picture

As a follow up, I've started a new thread here https://www.drupal.org/node/2642574

This is to cover option 3 in #15, allowing an existing license to be renewed. This thread is getting long and has gone down the route of forward dating the new license when it's purchased. As discussed in the new thread I think using the existing renewal functionality is a better approach when the system spots an existing license. Can ideas and discussions for this be moved to the new thread?

johnrosswvsu’s picture

Please disregard. These are just the patch that worked for me. Got an issue when I have two licenses of the same type in my cart and the first one has a granted date later than the current time (or last used time).

levmyshkin’s picture

I used patch #3 and I get an error with anonymous checkout. New license was be created as renewal another license for user with uid 0, if license existed with the same product_id and commerce_license type for anonymous user. You should get email from checkout form and add it to query condition or use checkout only for registered users.

jsacksick’s picture

Status: Needs work » Needs review
FileSize
3.2 KB

The attached patch is doing the following :

The commerce_license_activate_orders function tries to find existing licenses and call the renew() method on it.
The line item is updated to reference the correct license.

jsacksick’s picture

Small fix inside commerce_license_get_active_license() :
Changed ->propertyCondition('expires', commerce_license_get_time(), '>=') to ->propertyCondition('expires', commerce_license_get_time(), '>'); .

  • jsacksick committed 6db2dda on 7.x-1.x
    Issue #2193305: Renew an existing license if it exists instead of...
jsacksick’s picture

Status: Needs review » Fixed
Kazanir’s picture

Status: Fixed » Needs work

We need to revert this. This patch assumes that any given line item of SKU ABC is intended to renew the most recent license of SKU ABC. This is one of several possible use cases for CL, but it is far from the only one -- many use cases have a user that has multiple "parallel" licenses for a single SKU (with differentiating metadata between them) and this change would break that use case completely.

I think the *right* thing to do here would be to allow a product field to enable this kind of behavior, and this would change the logic where an empty licenseable line item has a license attached to it. This logic currently lives in commerce_license_commerce_line_item_presave() and that's the right place to change this. Alternatively, a site-specific implementation could intervene with a prior _presave() hook and prevent the default license creation logic from needing to run.

  • Kazanir committed 78463ca on 7.x-1.x
    Revert "Issue #2193305: Renew an existing license if it exists instead...
Kazanir’s picture

Reverted in 78463ca.

garphy’s picture

#65 : fair point you raised there !

But AFAIK, this issue was never RTBC'd so it was a bit early to commit it.

rszrama’s picture

Just a note: we committed this to specifically address a problem on sites where an existing license when renewed manually was erasing any time remaining on an active license. That said, our test environment always assumed a 1:1 correlation between users and license products. We've deployed the patch as is to production, so let's work to find a mutually successful solution before rolling a new point release of the module... ideally non-disruptive to the approach in the patch, but we can notify our client of any changes if need be when we do finalize it.

jsacksick’s picture

#65 :

I think the *right* thing to do here would be to allow a product field to enable this kind of behavior

We should probably add a boolean field "commerce_license_product_renewable" in order to enable that behavior although what this field is used for might be difficult to understand for the end user.

If we decide to add that field, then I think we'd just need to add a condition inside commerce_license_activate_order_licenses().

The reason why the code that I added inside commerce_license_activate_order_licenses() doesn't live inside commerce_license_commerce_line_item_presave() is that at the moment the license gets created, the user may not be authenticated and we wouldn't be able to know if there's an active license for that particular user.
Furthermore, I don't think it'd be a good idea to move the code there since it will unnecessarily be evaluated each time a line item is saved whereas it's currently being evaluated once in commerce_license_activate_order_licenses
Any thoughts?

jamescook’s picture

FWIW we've been using #56 in production for ages - and it serves our purpose fine.

Just recently though we've been getting "strange" granted/expires for certain licenses.
A brief check reveals anonymous active licenses on the DB for whatever reason - with granted/expires set!

So for anonymous users their granted/expires dates were always being based on those values in the DB - see the code in #56.

Anonymous users can exist if payments occur very quickly - e.g. for us with stripe (and others). Well the whole checkout thing involves various changes from anonymous to "system user" to real or new real customer etc. etc. which is all "fairly fragile" with rules etc.

For now we check if

$entity->uid === 0

on entry and return.

Not sure if this cures our problem completely - hard to know how the anonymous licenses got on the DB in the first place etc.

dtamajon’s picture