Canceling Renewals?

I also need this functionality - thanks for the module bpirkle! It works well. If you don't mind, I will likely roll this into a patch for commerce_recurring over the weekend as I need the provided functionality asap.

I think this could be useful, but bear in mind that it should expose the proper rule events that enable payment methods to update statuses as well.

Honestly I don't think a gzip is a good practice, so please always attach a patch as a first option for contributing.

I'll be happy to evaluate a patch.

We need this functionality as well. Any chance someone could roll a patch? @bpirkle it'd be great to see your latest code, perhaps rolled into a patch.

@selexin, does this work for the original scenario (assuming your needs were the same and met by the new patch)? I will give this a try, but curious to know if others have had positive results.

@torgosPizza, this does work well for the original scenario, however the original scenario very quickly became too simplified for the business rules of site that required this functionality. The main thing we also needed was support for a time based cancellation policy, which meant automatically monitoring payments in relation to recurring dates, and permitting/not permitting the cancellation action based on these dates. One day when I get more time I would like to contribute my implementation, however it will require some heavy refactoring so don't keep you hopes up sorry :(

Ah drat, that's what I was afraid of. Any chance you could give us the gist of it? Rather than work on discovering how, I'd love to know how (in general) you accomplished it. Thanks!

For sure. For cancellations we built a confirmation form that also captured the user's cancellation reason (to help refine the product). On form generation, the system runs the logic to check if the recurring entity can be immediately cancelled, or must recur at least once more, and a flag is stored in the form state to pass the result of this check on to the form submission handler. If the recurring entity has to recur at least once more a message is shown to the user stating that this 'cancellation' will not happen immediately, as they are subject to the ts& cs of the site etc. The user is NOT denied 'cancellation'. Instead, the submission handler will check for the flag mentioned before, and if found save a new boolean in the data array of the recurring entity (such as ->data['disable_on_next_payment'] = TRUE;). The recurring entity is left as-is, and the user is notified that their subscription will automatically cancel upon payment of their next and final due payment.
Now the real magic happens immediately after that last payment is made, I can't remember the exact hook (I think it was hook_commerce_payment_order_paid_in_full), but upon payment the system loads any recurring entities associated with the order and looks for the above flag in the R/E's data array. If found, it simply disables the recurring entity - effectively cancelling it.

Not really the right place, but since were on the topic I'll describe the other functionality we implemented around users controlled their recurring entity dates/statuses..
We also implemented a system to 'set your end date'. To do so we built a simple form that presents the user with a checkbox to either set their subscription to 'ongoing' (no end date), or select the end date of their subscription. This is like cancelling, but allows a time offset to set the cancellation date instead of being immediate. Again, the dates are subject to business rules and due payment dates etc.

As the business is selling a physical subscription based product, we also a implemented pause/resume system that allows the user temporarily suspend their subscription (in case they were going on holidays etc). To achieve this we built another form similar to the 'set end date' form, that takes two dates (pause/resume date) that are stored separately in another table. Again these dates are subject to business rule validation as to what dates can be set.
The actually pausing/resuming is simply controlled by a cron hook that checks every few hours for recurring entities that are due to pause/resume, and acts upon them accordingly by adjusting their status and due/end dates. The beauty of it is that the cron hooks fires before commerce_recurring's cron rules, allowing it to generate and charge orders as soon as they 'resume'.

Again, it would be awesome to release all the code for the stuff above, but it may not ever happen due to time and potential legal restrictions, and the fact it is currently in a very specific implementation :(

This is all great stuff and should be really helpful. So thanks!

HI - interested in helping out here as best I can.

We may need to start simple here as these items can get quite complex pretty quickly. Let's work a specification and build on the work from @bpirkle and @selexin. Our goal should be to roll a patch and get this functionality squared away.

@torgosPizza, I'll install the sample module and work on squaring away the spec. We can go from there.

Okay - I think that we need to keep things simple to get the basic functionality and look to provide flexibility as we go to allow extensibility to support more complex use cases.

Looks like we can build on the existing work by @bpirkle. Good. A few observations on things that we need to address:
1. Disable cancellation of the renewal if it is already cancelled.
2. The current implementation processes the cancellation and puts the end date as the current time - this is not the best as it will lead to potentially needing to offer a partial subscription refund. I think that when the cancellation is processed we need to put the end date as the same date as the due date so that the renewal does not occur but the current terms says active. We would put the recurring payment into a sort of 'expiring' state.
3. If we follow the approach in the second point above, what are your thoughts on needing to provide a mechanism for a user to reverse the cancellation request and reinstate the recurring payment?

Any other items to address before we get going on a patch?

Expanding on your second point @mxwitkowski, I think that the addition of a state property to recurring entities would be very beneficial to this and other functional requirements. Although some states can be assumed by existing properties (e.g. 'due' if due date > current date), being able to define and set states such as 'paused' and 'expiring' would help greatly with what has been discussed in this issue, and potential future functional requirements.
I imagine it working similarly to commerce_order's hook_commerce_order_state_info, with some default states provided by commerce_recurring, but the ability for additional modules to define their own states.
What are your thoughts on this? I'm happy to move this proposal into a separate issue and roll a patch as I see it as more of a generic feature addition.

@selexin - I like the idea of adding new states of the recurring order. I agree that having a more abstract approach versus the current is a good idea. We would need to be able to control these state changes via the Admin UI as well as with Rules too if possible.

Initial patch to port @bpirkle separate module into the Commerce Recurring module. Patch is against the most recent dev version. I changed the cancellation behavior to set the end date as the same date as the due date.

TODO:
1. Have not addressed the ability to cancel an already cancelled subscription
2. Need to test the changes end to end

Patch #12 worked well for me.

Two or three years ago the problem of cancelling subscriptions was resolved by a user who posted his code. I converted it into a module, which I have attached. I posted my code to this project and suggested that it should be included in the module proper, however it appears nothing was done and I can no longer find the post that had the original code or my module. The extra module also includes a number of other features which I consider essential.

I do wonder why commerce_recurring module still does not have a full release and would be keen to become a co-maintainer because I have a site that uses this module and keeps moving forward and features need to be added to the module to keep it alive.

Kent, your .zip file is not in patch format. And did you look at #12?

Berend, it is a separate module, not a patch. There are too many modifications for it to be viable as a patch. It consists mainly of additions with a few alterations to current code. Some other features include a recurring tab in the order admin section in which users can view the parent order and all its siblings. I haven't looked at #12, because I believe this question was answered a couple of years ago and nothing came of it. The suggested code had been sitting unused for 6 months or so and I decided that I could not wait and needed the functionality immediately so created an extra module. I believe that same code is the foundation of the commerce_recurring_bonus module: https://www.drupal.org/project/commerce_recurring_bonus which someone has created.

I guess the commerce_recurring module maintainers are busy doing other things and it appears that maintenance of the commerce_recurring module is not particularly active. The maintainer of the bonus module has experienced the same problem as I have as you can see here: https://www.drupal.org/node/2106571

I suggest that other devs download the extra module and test it. If people are happy then functionality can be added incrementally to the commerce_recurring module itself.

Commerce Recurring should have a full release by now given the number of people who have downloaded it. I'd like to be a co-maintainer so that I can integrate all three modules into the one module and answer some of the issues in the queue.

Please post patches in the relevant issues, a zip is not as useful as a patch.

I guess the commerce_recurring module maintainers are busy doing other things and it appears that maintenance of the commerce_recurring module is not particularly active.

Sorry, I wish I had more time to dedicate to this, honestly. To be honest, recurring is a quite complex module, the basic features are fully tested and very few of the patches in the queue add a test for the feature, meaning is a little harder to get stuff in, but being sure that the things that are there (mostly) work pays off.

The bonus module is not particularly useful in my opinion, I think all of it should be discussed in the queue and included, if relevant in the module.

I suggest that other devs download the extra module and test it. If people are happy then functionality can be added incrementally to the commerce_recurring module itself.

As said, please post it as patches, for what it sounds you've got there very useful fixes, and I'm happy to review them.

Commerce Recurring should have a full release by now given the number of people who have downloaded it.

The main blocker for this is the order recurring stuff, if we tag a stable release, and add support for orders later, the updates would be massive, if you want a stable release tagged, please help in #1821428: Clone line items rather than build new ones

I'd like to be a co-maintainer so that I can integrate all three modules into the one module and answer some of the issues in the queue.

You don't need to be a maintainer to do that! please by all means do post patches and answer stuff, I'm a strong believer in commit attribution and credit will be given when it's due!!! :D

Thanks for your reply. I understand that you have your standards for maintaining your module. I, like you, have limited time, but I do want to contribute back. I will simply create a new project. I've checked the bonus module and it does not have the code that is in the extra module.

but I do want to contribute back. I will simply create a new project.

Not sure why don't you provide patches instead?

Look, I simply don't have time to go through all your hoops and whistles. Let the community test the code. Any glitches will be soon found out. That's how the Drupal community works.

Look, I simply don't have time to go through all your hoops and whistles

Sorry I think we have a big misunderstanding here. I'm just saying that you have fixes for this module, I will be more than happy to review and merge the changes you might post, just not as a bulk zipfile. I don't think I'm being unreasonable.

Well, I am a little put off by the obstacles you raise such as:

To be honest, recurring is a quite complex module, the basic features are fully tested and very few of the patches in the queue add a test for the feature

Also, this code has been posted in an issue queue here before and yet it appears to have been deleted. I started using this module in good faith two or three years ago as a foundation for a complex recurring registration system. I come back now and find that the module is little changed from what it used to be and the code I posted for other people to use is no longer here. Those other people have missed out.

The zip file contains a completely self contained module that is separate from commerce_recurring and does not require any patching or integration. Given your attitude towards my contributing this code in the commerce_recurring module issue queue, I suspect that it is better for the Drupal community at large if it is presented as a separate module so that the community can test it separately. When it is more robust, then maybe it can be integrated into commerce_recurring but I suspect we will be at Drupal 8 by then.

One of the benefits of having a full release is that then the dev release can be used for the community to test new code, but commerce_recurring has no such thing.

I don't think I am being unreasonable, either.

@New Zeal: You seem fixed on a belief that an older issue was deleted, and this appears to be clouding your judgement of both this module's maintainers and the community as a whole. Let me clarify - project maintainers cannot delete issues out of an issue queue, only high-level site administrators can do that, and they have better things to be doing with their lives than go around deleting random issues.

If you had taken a moment to do an advanced search on the issue queue and searched for issues you followed, or filtered your personal issue queue for the Commerce Recurring Framework project you would have found two issues - this one and #1565770: How to manage subscriptions. I suspect it is this last one which you believed to have been deleted.

Well, I'm glad it hasn't been deleted. What annoys me is that the question raised in this thread has been answered before in that very thread you linked to: https://www.drupal.org/node/1565770 and here someone is having to ask it again and so we go through the process again.

The module maintainer dismissed the contents of that entire thread and all the contributions made with the comment:

At this point there are 6 sites using the 1.x branch of recurring framework, I'm dropping support for that version recommending to use and contribute to the 2.x

So, yeah, maybe the thread wasn't actually deleted, but in virtuality, I'd say it was with Closed (won't fix).

How different is 2.x that the code from that thread could not have been incorporated into it? Dozens of patches were supplied on that page, many which passed, and yet nothing was done with them!? :(

I don't think I'm being unreasonable, and I thank you for finding that issue so I can illustrate my point.

I would have to say that yes, the other issue (#1565770: How to manage subscriptions) should not have been closed but should have just been changed to reference the 7.x-2.x branch and rerolled. Given that the issue was closed a year ago, and this one is already up to 25 comments, lets just continue the work here and roll out improvements to the main module as individual patches.

BTW New Zeal, when you have a module with low usage and and a maintainer that admits to not having a huge amount of time to focus on it, forking it into your own module is not the way to go, you should instead work to provide tangible improvements in individual patches that can each be RTBC'd; also, read xkcd #927.

Ok, thanks, Damien. The underlying issue here is that I am using the 1.x and not the 2.x. I see that they are substantially different, and that might be why that thread was closed. I do wonder how many others like me are using that branch. Unfortunately usage stats do not differentiate between branches.

Patch #12 works for me too but as @mxwitkowski states the subscribers are able to cancel already cancelled subscriptions.

3. If we follow the approach in the second point above, what are your thoughts on needing to provide a mechanism for a user to reverse the cancellation request and reinstate the recurring payment?

The ability for subscribers to cancel and reinstate their subscription, as suggested by @mxwitkowski would be greatly beneficial as it would help reduce churn.

@nyleve101, glad to hear that the patch is working. I'll add the functionality for cancel/renewal and seek post a new patch in the coming days.

Just an FYI - I am not the most experienced programmer, so I'll get things as far as I can but will need help from people with more advanced knowledge and experience.

I do wonder how many others like me are using that branch. Unfortunately usage stats do not differentiate between branches.

Exactly 6 reported installs when I deprecated 1.x branch. 2.x is a complete rewrite. Stats show exclusively 2.x usage and not combined usage.

Updating the patch to check to see if a subscription is cancelled or set for cancellation.

Other fixes: Proper message setting code style, spelling mistake fix. Changing the return URL from my-subscriptions to what was set as the the default path for the view in commerce_recurring_ui/includes/views/handlers/commerce_recurring_ui.views_default.inc line 266

Sorry, small mistake in my latest patch, wrong path name.

Here is the latest.

Hiding previous patches and zip file.

Thanks @mxwitkowski and @VanD for working on this!

looks to me like the patch in #31 working well

Hi @mxwitkowski,

any luck on the patch for the user to reinstate the recurring payment after the user cancels it?

Thanks again

Hello,

Attaching patch that makes it possible to pause and resume paused subscription. Patch from #31 was used as base.

When you upload a new patch, please don't forget to change the status to "needs review" :) Thanks!

Excited to try the patch in #36. Looking over the patch I did notice one thing that I would probably change. You have it set the end date to the same date as the due date. I think you should probably have the end date set to the current date so that you can see when the user cancelled their payment and so that you can be sure that it will not be charged again on the due date. You mention it needs testing for this exact reason so making it the current date should ensure it works.

Hi @deggertsen. Attaching patch that takes your suggestions into account.

Changes are sponsored by nyleve101.

Correct me if I'm wrong, but it looks like this will probably need to be changed as well...

if ($recurring_entity->end_date == $recurring_entity->due_date) {
To something like this (will a less than work?):
if ($recurring_entity->end_date < $recurring_entity->due_date) {

The way the cancelling is set is to set $recurring_entity->end_date = $recurring_entity->due_date

However, if the user sets their own end date, then it's possible it may be less than, so the code should be

if ($recurring_entity->end_date <= $recurring_entity->due_date) {

Attached new patch.

I did another review of that patch and updated a few code style errors, and also added in watchdog message so if entity_save() fails, it doesn't fail silently and throws and error to watchdog.

Awesome. The patch looks good!

Great, please mark the issue as reviewed and tested by the community so we can commit it.

Can this be committed please?

Tagging to get some eyes on it during the sprint, since this patch is RTBC.

Thanks all for the work on this!

Here are some comments regarding the last patch. It would be awesome to have some tests around this too.

1. +++ b/commerce_recurring.module
   @@ -632,6 +635,115 @@ function commerce_recurring_uri($recurring_entity) {
   +function commerce_recurring_cancel_renewals_form($form, &$form_state) {
   

   Can we send the recurring entity id as a parameter here to avoid the arg(1) stuff?

2. +++ b/commerce_recurring.module
   @@ -632,6 +635,115 @@ function commerce_recurring_uri($recurring_entity) {
   +  if (user_is_anonymous()) {
   

   We should manage this as a permission thing, doesn't matter if it's anonymous, what matters is that has the "cancel own subscription renewals" permission

3. +++ b/commerce_recurring.module
   @@ -632,6 +635,115 @@ function commerce_recurring_uri($recurring_entity) {
   + * Determines if the current user can assess the specified recurring entity
   

   Comment needs fixing I think

4. +++ b/commerce_recurring.module
   @@ -632,6 +635,115 @@ function commerce_recurring_uri($recurring_entity) {
   +    // MXW - put an if in here if we are going to do immediate or at end of term
   +    // if immediate, then:
   ...
   +    // else
   +    commerce_recurring_expire_recurring($recurring_entity);
   

   This needs clean up

5. +++ b/commerce_recurring.module
   @@ -632,6 +635,115 @@ function commerce_recurring_uri($recurring_entity) {
   +    	//commerce_recurring_stop_recurring($recurring_entity);
   ...
   +    ¶
   

   Extra spacing

6. +++ b/commerce_recurring.module
   @@ -632,6 +635,115 @@ function commerce_recurring_uri($recurring_entity) {
   +  //$recurring_entity->end_date = $recurring_entity->due_date; // make this equal to the next recurrence date so it does not trigger – test this to make sure it works…
   +  $recurring_entity->end_date = time(); //changed to current time - so next recurrence won't trigger and also gives information, when user has cancelled subscription
   +	entity_save('commerce_recurring', $recurring_entity);
   

   This needs cleanup

7. +++ b/commerce_recurring.module
   @@ -648,3 +760,101 @@ function commerce_recurring_commerce_checkout_router($order, $checkout_page) {
   +function commerce_recurring_pause_renewals_form($form, &$form_state) {
   

   This has 80% same code than the cancel renewals, I think we should have one form with a "mode" parameter

8. +++ b/commerce_recurring.module
   @@ -648,3 +760,101 @@ function commerce_recurring_commerce_checkout_router($order, $checkout_page) {
   +function commerce_recurring_pause_renewals_form_submit($form, &$form_state) {
   

   General comment, we need a comment header for all these new functions

9. +++ b/commerce_recurring.module
   @@ -648,3 +760,101 @@ function commerce_recurring_commerce_checkout_router($order, $checkout_page) {
   +function commerce_recurring_pause_recurring($recurring_entity){
   +  $recurring_entity->status = 0;
   +  entity_save('commerce_recurring', $recurring_entity);
   +}
   +
   +function commerce_recurring_unpause_recurring($recurring_entity){
   +  $recurring_entity->status = 1;
   +  if ($recurring_entity->due_date < time()){
   +    $recurring_entity->due_date = time();
   +  }
   +  entity_save('commerce_recurring', $recurring_entity);
   +}
   

   Not sure if we need two different functions for this

10. +++ b/commerce_recurring_ui/commerce_recurring_ui.module
    @@ -28,6 +28,22 @@ function commerce_recurring_ui_menu() {
    +  $items['cancel-subscription-renewals'] = array(
    ...
    +  $items['pause-subscription-renewals'] = array(
    

    Here we need to handle a %uid optional wildcard to avoid the arg(1)

11. +++ b/commerce_recurring_ui/commerce_recurring_ui.module
    @@ -28,6 +28,22 @@ function commerce_recurring_ui_menu() {
    +    'description'      => 'Cancel subscription renewals',
    ...
    +    'description'      => 'Pause or unpause subscription renewals',
    

    If we add a description, it should have more info than the title, otherwise we can just remove it

12. +++ b/commerce_recurring_ui/commerce_recurring_ui.module
    @@ -28,6 +28,22 @@ function commerce_recurring_ui_menu() {
    +  );      ¶
    

    Extra tab

13. +++ b/commerce_recurring_ui/includes/views/commerce_recurring_ui.views.inc
    @@ -16,6 +16,14 @@ function commerce_recurring_ui_views_data() {
    +  	'real field' => 'pause',
    

    Extra space.

    Do we need "real_field" here?

14. +++ b/commerce_recurring_ui/includes/views/commerce_recurring_ui.views_default.inc
    @@ -141,7 +141,7 @@ function commerce_recurring_ui_views_default_views() {
    -  $handler->display->display_options['fields']['end_date']['date_format'] = 'raw time hence';
    +  $handler->display->display_options['fields']['end_date']['date_format'] = 'time span';
    
    @@ -184,7 +184,7 @@ function commerce_recurring_ui_views_default_views() {
    -  $handler->display->display_options['title'] = 'User recurring entities';
    +  $handler->display->display_options['title'] = 'Recurring Subscriptions';
    
    @@ -230,21 +230,24 @@ function commerce_recurring_ui_views_default_views() {
    -  $handler->display->display_options['fields']['end_date']['date_format'] = 'raw time hence';
    +  $handler->display->display_options['fields']['end_date']['date_format'] = 'time span';
    

    These seem like unrelated changes to me

Thanks for the review pcambra, I was able to clean up the code some. However I didn't do all of it.

If someone wants to take it further they are welcome, here is what is left from pcambra's list: 1, 7, 9, 10

#1 - changed to pass in recurring_entity like the other forms do
#7 - combined cancel/pause _form and _form_submit functions with $mode option
#9 - removed all 3 functions as they only had about 1-2 lines of unique code and moved into _form_submit function
#10 - same as #1

Also should these form functions be in the UI module instead? that is where their menu entries are and how the other admin form functions are structured. If not, I'd propose we move their menu hooks back to the main module, having them split apart doesn't make sense to me.

I also didn't post an interdiff, because with all the removal of code the interdiff was just a big confusing mess that was worse than just looking at the 2 patches independently.

Thanks @VanD and @smccabe!

Also should these form functions be in the UI module instead? that is where their menu entries are and how the other admin form functions are structured. If not, I'd propose we move their menu hooks back to the main module, having them split apart doesn't make sense to me.

Indeed, I think you're right here.

A bit more of review:

1. +++ b/commerce_recurring.module
   @@ -627,12 +632,132 @@ function commerce_recurring_uri($recurring_entity) {
   +  // If recurring entity end date is equal to due date, thus set to be cancelled.
   +  if ($recurring_entity && $recurring_entity->end_date != 0) {
   +    if ($recurring_entity->end_date <= $recurring_entity->due_date) {
   +      drupal_set_message(t("Subscription @id is set to be cancelled on @endDate.", array("@id" => $recurring_entity->id, "@endDate" => $recurring_entity->end_date)));
   +      drupal_goto($my_subscriptions_path);
   +    }
   +  }
   

   Do we need all these checks on the current date? Seems to me a little limiting.

2. +++ b/commerce_recurring.module
   @@ -627,12 +632,132 @@ function commerce_recurring_uri($recurring_entity) {
   +  // Confirm this user can act on this recurring entity
   +  if (!_commerce_recurring_can_cancel_subscription($recurring_entity)) {
   +    drupal_set_message(t("Permission denied for subscription @id.", array("@id" => $recurring_entity->id)));
   +    drupal_goto($my_subscriptions_path);
   +  }
   

   This should be handled in the access of the form instead.

3. +++ b/commerce_recurring.module
   @@ -627,12 +632,132 @@ function commerce_recurring_uri($recurring_entity) {
   +  if($mode == 'cancel') {
   ...
   +  else if($mode == 'pause') {
   ...
   +    else{
   

   Watch out for the missing spaces for coding standards

4. +++ b/commerce_recurring.module
   @@ -627,12 +632,132 @@ function commerce_recurring_uri($recurring_entity) {
   +  $form = confirm_form($form, $confirm_question, $my_subscriptions_path, t('This action cannot be undone.'), $confirm_button, t('Return to My Subscriptions'));
   ...
   +  return $form;
   

   Maybe return confirm_form ?

5. +++ b/commerce_recurring_ui/commerce_recurring_ui.module
   @@ -28,6 +28,20 @@ function commerce_recurring_ui_menu() {
   +  $items['cancel-subscription-renewals/%commerce_recurring'] = array(
   ...
   +  $items['pause-subscription-renewals/%commerce_recurring'] = array(
   

   I'm a little worried about the url here, shall we namespace it somehow with commerce-recurring?

6. +++ b/commerce_recurring_ui/includes/views/commerce_recurring_ui.views_default.inc
   @@ -237,14 +237,17 @@ function commerce_recurring_ui_views_default_views() {
   -  $handler->display->display_options['fields']['nothing']['label'] = '';
   -  $handler->display->display_options['fields']['nothing']['alter']['text'] = 'View orders';
   -  $handler->display->display_options['fields']['nothing']['alter']['make_link'] = TRUE;
   -  $handler->display->display_options['fields']['nothing']['alter']['path'] = 'user/[uid]/recurring/[id]/orders';
   +  $handler->display->display_options['fields']['nothing']['label'] = 'Actions';
   +  $handler->display->display_options['fields']['nothing']['alter']['text'] = '<a href="/user/[uid]/recurring/[id]/orders">View Orders</a>&nbsp;&nbsp;&nbsp;<a href="/cancel-subscription-renewals/[id]">Cancel Renewals</a>&nbsp;&nbsp;&nbsp;[pause]';
   

   Not sure if we want to do this, I think we should do a ctools dropbutton instead.

Ok, I moved everything into commerce_recurring_ui

#1 - I just removed this completely, if the recurring is set to cancel in the future, im not sure why we prevent you from pausing or cancelling it now. Seems like if we don't want you to cancel, we just don't give out cancel permissions at all
#2 - It already was, removed the redundant check from the form function. Also added pause permission as you might want pause or cancel but not both potentially.
#3 - Ah spaces.. my arch nemesis.
#4 - agree, done
#5 - changed to 'user/%/recurring/%recurring-entity/operation to match the existing stuff
#6 - Haven't worked with dropbuttom before, but mimic'd what was done in commerce_discount, also removed some half done handler stuff that was already there.

This is brilliant @smccabe !

1. +++ b/commerce_recurring_ui/commerce_recurring_ui.module
   @@ -28,10 +28,41 @@ function commerce_recurring_ui_menu() {
   +    'access arguments' => array('cancel own subscription renewals'),
   ...
   +    'access arguments' => array('pause own subscription renewals'),
   
   @@ -59,3 +90,106 @@ function commerce_recurring_ui_views_api() {
   +  // Confirm this user can act on this recurring entity
   +  if (!_commerce_recurring_ui_can_cancel_subscription($recurring_entity)) {
   +    drupal_set_message(t("Permission denied for subscription @id.", array("@id" => $recurring_entity->id)));
   +    drupal_goto($my_subscriptions_path);
   +  }
   
   +++ b/commerce_recurring_ui/includes/views/handlers/commerce_recurring_ui_handler_field_operations_dropbutton.inc
   @@ -0,0 +1,52 @@
   +
   

   This is what I meant with #3, if this is a requirement for accesing the form, it should be included in the access_arguments callback from hook_menu, you could pass the user there and avoid this custom check.

2. +++ b/commerce_recurring_ui/commerce_recurring_ui.module
   @@ -59,3 +90,106 @@ function commerce_recurring_ui_views_api() {
   +  if (!$recurring_entity) {
   +    drupal_set_message(t("Unable to find subscription @id.", array("@id" => $recurring_entity->id)));
   +    drupal_goto($my_subscriptions_path);
   +  }
   

   I think if we're using %commerce_recurring on the load from hook menu, there's no way this can be empty at this point?

3. +++ b/commerce_recurring_ui/commerce_recurring_ui.module
   @@ -59,3 +90,106 @@ function commerce_recurring_ui_views_api() {
   +    if($form_state['values']['mode'] == 'cancel') {
   ...
   +    else if($form_state['values']['mode'] == 'pause') {
   

   Missing spaces

4. +++ b/commerce_recurring_ui/commerce_recurring_ui.module
   @@ -59,3 +90,106 @@ function commerce_recurring_ui_views_api() {
   +      // Set end date to current time, which will allow expiry
   ...
   +  // Redirect back to subscription path
   

   Missing dot at the end of the comment

5. +++ b/commerce_recurring_ui/commerce_recurring_ui.module
   @@ -59,3 +90,106 @@ function commerce_recurring_ui_views_api() {
   +        if ($recurring_entity->due_date < time()){
   +          $recurring_entity->due_date = time();
   +        }
   

   Not for this issue but we might to open a followup integrating rules here, I can see how custom stuff can be wanted when pausing/cancelling

#1 - I think I see what you mean here, removed yet more code and converted to an access callback
#2 - You are correct, removed
#3 - Sigh.. added
#4 - Removed comment completely and cleaned up line below, was overly complicated for just setting a redirect back to the listings page
#5 - Agree, created #2602110: Allow cancels and pauses based on rules

Mostly just deleted stuff! The best kind of fixes

Great, a few minor tweaks and we can get this in.

1. +++ b/commerce_recurring_ui/commerce_recurring_ui.module
   @@ -59,3 +92,93 @@ function commerce_recurring_ui_views_api() {
   +  global $user;
   

   no need to do this, you can pass the user argument from hook_menu

2. +++ b/commerce_recurring_ui/includes/views/handlers/commerce_recurring_ui_handler_field_operations_dropbutton.inc
   @@ -0,0 +1,52 @@
   +      'href'  => '/user/' . $user->uid . '/recurring/' . $recurring_id .'/orders',
   ...
   +      'href'  => '/user/' . $user->uid . '/recurring/' . $recurring_id . '/cancel',
   ...
   +      'href'  => '/user/' . $user->uid . '/recurring/' . $recurring_id . '/pause',
   

   I guess we could use a $base_path here

We can have this in without tests so we can finally close this and then have a followup to have some testing on this, it will be useful to ensure #2602110: Allow cancels and pauses based on rules is working

#1 - Done
#2 - Done

Thanks for chasing this, @smccabe!

A couple of things after trying this patch out:

1. +++ b/commerce_recurring_ui/includes/views/handlers/commerce_recurring_ui_handler_field_operations_dropbutton.inc
   @@ -0,0 +1,52 @@
   +    global $user;
   

   I think this should come from the view instead, using global $user means that if I'm an admin and I visit some user "My subscriptions" page, I'd see my orders and not the user's.

2. +++ b/commerce_recurring_ui/includes/views/handlers/commerce_recurring_ui_handler_field_operations_dropbutton.inc
   @@ -0,0 +1,52 @@
   +      'href'  => base_path() . 'user/' . $user->uid . '/recurring/' . $recurring_id .'/orders',
   ...
   +      'href'  => base_path() . 'user/' . $user->uid . '/recurring/' . $recurring_id . '/cancel',
   ...
   +      'href'  => base_path() . 'user/' . $user->uid . '/recurring/' . $recurring_id . '/pause',
   

   base_path() here is not necessary, if someone doesn't have it configured it will go to "user/id/recurring/id/orders" but with no path at all, just removing it would work.

   Also, as they perform operations that change data, this is a potential security vulnerability, so they need to be protected by a token, see https://www.drupal.org/sandbox/davereid/1332490 for more info. If you want to see how these are implemented in a very similar fashion, check https://www.drupal.org/project/commerce_extra_panes

When clicking on "pause" of an active recurring order, I get these warnings:

    Notice: Undefined index: mode in commerce_recurring_ui_cancel_renewals_form_submit() (line 163 of /webs/contrib/drupalcommerce/sites/all/modules/commerce_recurring/commerce_recurring_ui/commerce_recurring_ui.module).
    Notice: Undefined index: mode in commerce_recurring_ui_cancel_renewals_form_submit() (line 167 of /webs/contrib/drupalcommerce/sites/all/modules/commerce_recurring/commerce_recurring_ui/commerce_recurring_ui.module).

Also, I don't have very clear what's the purpose of a "pause", if we don't have a "resume". Maybe the best thing would be to move the pause for now to a followup and provide just the cancel.

#1 - Fixed
#2a - removed
#2b - added on form, not sure if it is 100% correct
#3 - removed pause functionality, I don't know its purpose either.

For me, Hunk #1 failed when applying to the latest dev. But I will continue to test.

Also, submitting the Cancel form doesn't seem to change the status - at least, according to the UI. After submitting the form, the Status indicator is still a checkbox as opposed to an "X".

EDIT: The above issue is easily fixed just by setting $recurring_entity->status = FALSE in commerce_recurring_ui_cancel_renewals_form_submit(). I'd suggest adding that to the current patch.

@TorgosPizza - I'm not the original author, but my understanding was the cancel just removes the next payment date and that doesn't necessarily make it invalid right away? I do agree that still seeing the checkmark seems weird though. The below code seems to indicate changing the time and not the status is the intended way.

    // Set end date to current time, which will allow expiry
    $recurring_entity->end_date = time();

I had noticed that, but I'm not quite sure that is the correct approach. In theory, a subscription would actually end at the next billing cycle, as well, so even the current method of setting the end_date to the current time seems a bit misleading. If someone pre-pays for a month of a subscription access, it makes sense to let them finish out the rest of the month, and then just not bill them again.

Thoughts?

I looked into the code in more detail and I believe you are correct and cancel is the correct way to go. There is actually a cancel function that should be called so that the cancel rule gets triggered. This way we cancel the recurring payment and then the site owner can have their rules set up to cancel or continue or whatever they want for whatever they've tied to a recurring payment.

    commerce_recurring_stop_recurring($recurring_entity);

Brilliant find! I wasn't even aware that function exists. (I haven't really dived into the guts of Commerce Recurring yet.)

I'll apply this patch and see how things go. Thanks!

This looks really good, thanks @smccabe and @torgosPizza :), please let me know how the testing goes and I'm testing it on my side too.

+++ b/commerce_recurring_ui/commerce_recurring_ui.module
@@ -59,3 +80,62 @@ function commerce_recurring_ui_views_api() {
+function commerce_recurring_ui_cancel_renewals_form_submit($form, &$form_state) {
+  global $user;
+
+  if ($form_state['values']['confirm']) {
+    $recurring_entity_id = $form['id']['#value'];
+    $recurring_entity = commerce_recurring_load($recurring_entity_id);
+
+    commerce_recurring_stop_recurring($recurring_entity);
+  }
+
+  $form_state['redirect'] = 'user/' . $user->uid . '/user-recurring-entities';;
+}
\ No newline at end of file

We probably need a drupal_set_message for the user to confirm the operation?

Also, no newline at the end of the file

• Message added
• global $user replaced with uid from entity to work better with admin users
• newline added

Patch in #71 works perfectly for me. My subscription was canceled and subsequent orders were not made.

My only comment:
drupal_set_message(t('The subscription @id was cancelled, you will not be charged again for this subscription.', array('@id' => $recurring_entity_id)));

I might change @id to :title and then replace that with $recurring_entity->title to make it more user friendly. (I changed the default user-facing View as well for our purposes) because it'd be more user-friendly. But that can be addressed in a follow-up.

I copied a different area where it uses @id as well, but you're right, I should probably just change that in both places to be title, we've already loaded the whole entity anyways. I will post updated patch this evening

I should probably just change that in both places to be title, we've already loaded the whole entity anyways.

Are there other areas of the module that do this? If so, I'd consider that a follow up issue to improve user experience and commit this one as is, since it matches current messages. My two cents :)

Yeah I'd be okay with creating a follow-up just so we get this in. This feature is more important than continuing to pick nits :)

Added issue #2617512: Change messages to return title instead of id for the title stuff, lets get this committed!

Thanks everyone for the effort and patience put in this!

I've opened #2641582: Add tests for the cancel ui for the tests bit.