Always use cancelSubscription form for cancelling recurring contributions #18196

mattwire · 2020-08-19T21:31:38Z

Overview

Replaces #16717, follow up to #17146.

There was existing code to handle cancellation if the processor does not support cancellation.

You may still want to record cancellation reason, see instructions to manually cancel with your provider and send a notification email to the contributor.

But if the processor reports that it does not supportCancelRecurring the form is never displayed and the built in entity enable/disable via javascript is used. This only really makes sense if there is no payment processor.

Before

enable/disable javascript used if processor does not support cancelling recur (cancel link still available in list of recurs!).

After

Always display the cancellation form. Eg. for a recur with payment processor:

And without a payment processor:

Technical Details

Comments

This cleans up another inconsistency in the way civi handles things. @eileenmcnaughton This finishes off the cancellation stuff we were working on a few versions back.

civibot · 2020-08-19T21:31:40Z

(Standard links)

If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

eileenmcnaughton · 2020-09-03T22:23:12Z

I'm just going through triaging for 'needs test' & ok without test. I guess the logic is PRs need a test unless we can't see how they could be tested or we know there is existing relevant cover. I think this could be tested.

eileenmcnaughton · 2020-10-18T23:37:25Z

I took a look at this since a new PR #18787 got raised that addresses some of the same code / makes at least one change that is the same & put up a PR to address the 2 things that stalled this PR #18790

Reviewer's partial from civicrm#18196

seamuslee001 · 2020-10-27T00:49:03Z

Test fail relates i believe

eileenmcnaughton · 2020-10-27T02:36:04Z

Yep - there is another open PR related to these links which is where discussion is taking place #18787 - the tldr is that we also need to think about the situation where the appropriate links are different for someone dealing with their own recurring vs someone else's

mattwire · 2021-03-03T10:47:06Z

CRM_Contribute_Page_TabTest::testLinks
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'<span><a href="/index.php?q=civicrm/contact/view/contributionrecur&amp;reset=1&amp;id=1&amp;cid=3&amp;context=contribution" class="action-item crm-hover-button" title='View Recurring Payment' >View</a><a href="/index.php?q=civicrm/contribute/updaterecur&amp;reset=1&amp;action=update&amp;crid=1&amp;cid=3&amp;context=contribution" class="action-item crm-hover-button" title='Edit Recurring Payment' >Edit</a><a href="/index.php?q=civicrm/contribute/unsubscribe&amp;reset=1&amp;crid=1&amp;cid=3&amp;context=contribution" class="action-item crm-hover-button" title='Cancel' >Cancel</a></span>'
+'<span><a href="/index.php?q=civicrm/contact/view/contributionrecur&amp;reset=1&amp;id=1&amp;cid=3&amp;context=contribution" class="action-item crm-hover-button" title='View Recurring Payment' >View</a><a href="/index.php?q=civicrm/contribute/unsubscribe&amp;reset=1&amp;crid=1&amp;cid=3&amp;context=contribution" class="action-item crm-hover-button" title='Cancel' >Cancel</a></span>'

/home/jenkins/bknix-dfl/build/core-18196-7nonk/web/sites/all/modules/civicrm/tests/phpunit/CRM/Contribute/Page/TabTest.php:55
/home/jenkins/bknix-dfl/build/core-18196-7nonk/web/sites/all/modules/civicrm/tests/phpunit/CiviTest/CiviUnitTestCase.php:226
/home/jenkins/bknix-dfl/extern/phpunit7/phpunit7.phar:615

This picks up on two of the issues that were blocking on civicrm#18787 and civicrm#18196 (but does not solve all of them). It 1) adds the basis for unit test cover and 2) separates the function for retrieving links for self-service from back office user. I think there is still some jumbling together of these concepts but as long as we are careful to extend the tests as we extend the logic... This fixes what I suspect is an oldish regression whereby the cancel link for paypal std users was wrong

This splits out a minor code cleanup from civicrm#18196 (which can probably build on the unit test work in 20210)

This picks up on two of the issues that were blocking on civicrm#18787 and civicrm#18196 (but does not solve all of them). It 1) adds the basis for unit test cover and 2) separates the function for retrieving links for self-service from back office user. I think there is still some jumbling together of these concepts but as long as we are careful to extend the tests as we extend the logic... This fixes what I suspect is an oldish regression whereby the cancel link for paypal std users was wrong

mattwire · 2021-09-24T09:34:32Z

We can build on this test: #21588

jitendrapurohit · 2021-09-30T13:38:12Z

CRM/Contribute/Page/Tab.php

+        'title' => ts('Cancel'),
+        'url' => 'civicrm/contribute/unsubscribe',
+        'qs' => "reset=1&crid=%%crid%%&cid=%%cid%%&context={$context}",
+      ];


Does the above change mean we load Cancel link only for processors supporting cancelRecurring?

But from the description -

After
Always display the cancellation form. Eg. for a recur with payment processor:

it seems the cancel link should always be visible and condition should be checked on the form, i.e, for Send cancellation request to processor field? Is that correct?

With this PR applied, I see the Cancel link is not displayed for processors not having supportsCancelRecurring().

@mattwire can you pls clarify?

@jitendrapurohit You are right. I've have pushed a fix.

The "Cancel" link should always display but the form should disable "send cancellation request to processor" if processor does not supportsRecurring()

BettyDolfing · 2022-04-11T07:56:20Z

Test this please

jaapjansma · 2022-04-11T14:16:21Z

@mattwire can you explain what this PR changes? We see the exact same behaviour in https://dmaster.demo.civicrm.org (which does not have this patch). See screenshot below:

It looks like this PR is not solving anything (or does solve something which has been solved in the mean time).

…ions This is civicrm#18196 without the bit I was uncomfortable with

eileenmcnaughton · 2022-04-14T23:12:47Z

@mattwire I've always shied away from looking at this because of the unexplained changes in getAllProcessors - but the part where you alter the action makes sense to me - so I pulled that part out into a separate PR that I think you can probably merge
#23210

…ions This is civicrm#18196 without the bit I was uncomfortable with

…orm if cancel is not supported by processor

eileenmcnaughton · 2022-04-19T19:49:52Z

@mattwire pr template needs updating now - to explain what the point is of this change / why the PR still exists

mattwire · 2022-05-04T10:46:42Z

@eileenmcnaughton Yes, not sure that this bit is still required.

civibot bot added the master label Aug 19, 2020

mattwire changed the title ~~Cancelsubscriptionalways~~ Always use cancelSubscription form for cancelling recurring contributions Aug 19, 2020

mattwire force-pushed the cancelsubscriptionalways branch from 6347fd1 to 7d4ef13 Compare August 19, 2020 22:13

eileenmcnaughton mentioned this pull request Aug 20, 2020

dev/core#1961 Fix regression - cancel button not working on recurring contributions #18204

Merged

eileenmcnaughton added the needs-test label Sep 3, 2020

eileenmcnaughton mentioned this pull request Oct 18, 2020

Add test for recurring links and clean up method of retrieving recurring #18790

Merged

eileenmcnaughton mentioned this pull request Oct 18, 2020

dev/core#2034 Fix paypal standard cancel url and action links in dashboard #18787

Closed

eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Oct 20, 2020

Declare support for cancelRecurring in manual processor

b888a01

Reviewer's partial from civicrm#18196

eileenmcnaughton mentioned this pull request Oct 20, 2020

Declare support for cancelRecurring in manual processor #18804

Merged

mattwire force-pushed the cancelsubscriptionalways branch from 7d4ef13 to fbc4cd0 Compare October 26, 2020 21:07

mattwire force-pushed the cancelsubscriptionalways branch from fbc4cd0 to 1f988b0 Compare December 5, 2020 20:22

mattwire force-pushed the cancelsubscriptionalways branch 5 times, most recently from a3871e2 to f284c77 Compare February 1, 2021 20:44

mattwire force-pushed the cancelsubscriptionalways branch from f284c77 to 98ee7c9 Compare March 4, 2021 10:19

eileenmcnaughton mentioned this pull request May 3, 2021

Fix old-ish regression on paypal standard self-service cancel links #20210

Merged

eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request May 3, 2021

[REF] Reverse if statement

b1a53a8

This splits out a minor code cleanup from civicrm#18196 (which can probably build on the unit test work in 20210)

eileenmcnaughton mentioned this pull request May 3, 2021

[REF] Reverse if statement #20211

Merged

mattwire force-pushed the cancelsubscriptionalways branch from 98ee7c9 to 92a01cf Compare May 16, 2021 13:57

mattwire force-pushed the cancelsubscriptionalways branch from 92a01cf to 325617f Compare July 5, 2021 11:47

mattwire force-pushed the cancelsubscriptionalways branch from 325617f to 023a6cb Compare September 24, 2021 09:33

jitendrapurohit reviewed Sep 30, 2021

View reviewed changes

mattwire force-pushed the cancelsubscriptionalways branch from 023a6cb to bc1f2c5 Compare September 30, 2021 21:30

kartik1000 mentioned this pull request Oct 20, 2021

Added Cancel Recur Subscription test & setter for supports on Dummy processor #21895

Merged

mattwire force-pushed the cancelsubscriptionalways branch 2 times, most recently from 663b65e to d310941 Compare February 17, 2022 21:07

mattwire added has-test and removed needs-test labels Feb 17, 2022

mattwire force-pushed the cancelsubscriptionalways branch 3 times, most recently from ca0fa1e to edcf036 Compare February 23, 2022 18:59

jaapjansma added the needs-work-not-review-ready Submitter required to take action, will be closed after 1-2 weeks in this state label Apr 11, 2022

eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Apr 14, 2022

Always use cancelSubscription form for cancelling recurring contribut…

ba53b86

…ions This is civicrm#18196 without the bit I was uncomfortable with

eileenmcnaughton mentioned this pull request Apr 14, 2022

Always use cancelSubscription form for cancelling recurring contributions #23210

Merged

eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Apr 14, 2022

Always use cancelSubscription form for cancelling recurring contribut…

ca574dc

…ions This is civicrm#18196 without the bit I was uncomfortable with

Always use cancelSubscription form instead of enable/disable entity f…

cf2d3c2

…orm if cancel is not supported by processor

mattwire force-pushed the cancelsubscriptionalways branch from edcf036 to cf2d3c2 Compare April 19, 2022 09:49

mattwire closed this May 4, 2022

mattwire deleted the cancelsubscriptionalways branch May 4, 2022 10:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Always use cancelSubscription form for cancelling recurring contributions #18196

Always use cancelSubscription form for cancelling recurring contributions #18196

mattwire commented Aug 19, 2020 •

edited

Loading

civibot bot commented Aug 19, 2020

eileenmcnaughton commented Sep 3, 2020

eileenmcnaughton commented Oct 18, 2020 •

edited

Loading

seamuslee001 commented Oct 27, 2020

eileenmcnaughton commented Oct 27, 2020

mattwire commented Mar 3, 2021

mattwire commented Sep 24, 2021

jitendrapurohit Sep 30, 2021

mattwire Sep 30, 2021

mattwire Sep 30, 2021

BettyDolfing commented Apr 11, 2022

jaapjansma commented Apr 11, 2022 •

edited

Loading

eileenmcnaughton commented Apr 14, 2022

eileenmcnaughton commented Apr 19, 2022

mattwire commented May 4, 2022

Always use cancelSubscription form for cancelling recurring contributions #18196

Always use cancelSubscription form for cancelling recurring contributions #18196

Conversation

mattwire commented Aug 19, 2020 • edited Loading

Overview

Before

After

Technical Details

Comments

civibot bot commented Aug 19, 2020

eileenmcnaughton commented Sep 3, 2020

eileenmcnaughton commented Oct 18, 2020 • edited Loading

seamuslee001 commented Oct 27, 2020

eileenmcnaughton commented Oct 27, 2020

mattwire commented Mar 3, 2021

mattwire commented Sep 24, 2021

jitendrapurohit Sep 30, 2021

Choose a reason for hiding this comment

mattwire Sep 30, 2021

Choose a reason for hiding this comment

mattwire Sep 30, 2021

Choose a reason for hiding this comment

BettyDolfing commented Apr 11, 2022

jaapjansma commented Apr 11, 2022 • edited Loading

eileenmcnaughton commented Apr 14, 2022

eileenmcnaughton commented Apr 19, 2022

mattwire commented May 4, 2022

mattwire commented Aug 19, 2020 •

edited

Loading

eileenmcnaughton commented Oct 18, 2020 •

edited

Loading

jaapjansma commented Apr 11, 2022 •

edited

Loading