Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev/core#704 Fix loss of links for recurrings with no payment_processor_id #13935

Merged
merged 1 commit into from
Apr 4, 2019

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Apr 2, 2019

Overview

Fixes 5.8 regression where the cancel link became unavailable on recurring contributions with no attached payment processor id

In 5.8 changes were made that remove the cancel links from recurring payments where the payment processor object doesn't load.

This is appropriate for cases where there IS a processor but it's disabled. However, it is not
unknown for sites to import contribution_recur records from elsewhere as data records rather than 'functional records' & it is appropriate to be able to edit those.

Before

Screenshot 2019-04-02 15 13 09

After

Screenshot 2019-04-02 15 09 36

Technical Details

We already have a relevant pattern - loading payment processor 0 loads the manual processor
(class is CRM_Core_Manual) which has functionality appropriate to non-automated flows
(also known as the paylater processor).

This PR switches to the function CRM_Contribute_BAO_ContributionRecur::getPaymentProcessorObject
which is a skinny wrapper on CRM_Contribute_BAO_ContributionRecur::getPaymentProcessor - which itself
was not actually called from core prior to this change (we didn't remove it as it was better than
functions in play & hence intended to start using it again). No processor is loaded
for an inactive processor so links do not appear there.

Comments

@civibot
Copy link

civibot bot commented Apr 2, 2019

(Standard links)

@civibot civibot bot added the master label Apr 2, 2019
*
* @return array|null
*/
public static function getPaymentProcessor($id, $mode = NULL) {
public static function getPaymentProcessor($id) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is safe - function not called from anywhere prior to this PR (we kept it when it stopped being called as a 'better' choice

…or_id

In 5.8 changes were made that remove the cancel links from recurring payments where the payment processor object
doesn't load. This is appropriate for cases where there IS a processor but it's disabled. However, it is not
unknown for sites to import contribution_recur records from elsewhere as data records rather than 'functional
records' & it is appropriate to be able to edit those.

We already have a relevant patter - loading payment processor 0 loads the manual processor
(class is CRM_Core_Manual) which has functionality appropriate to non-automated flows
(also known as the paylater processor).

This PR switches to the function CRM_Contribute_BAO_ContributionRecur::getPaymentProcessorObject
which is a skinny wrapper on CRM_Contribute_BAO_ContributionRecur::getPaymentProcessor - which itself
was not actually called from core prior to this change (we didn't remove it as it was better than
functions in play & hence intended to start using it again). No processor is loaded
for an inactive processor so links do not appear there.
@eileenmcnaughton
Copy link
Contributor Author

@mattwire I'd like to get this merged - I have follow on cleanup but I wanted to consolidate a preferred function first

@mattwire
Copy link
Contributor

mattwire commented Apr 4, 2019

Tested and working

@mattwire mattwire merged commit 45cf788 into civicrm:master Apr 4, 2019
@eileenmcnaughton eileenmcnaughton deleted the recur_cancel_load branch April 4, 2019 23:23
@eileenmcnaughton
Copy link
Contributor Author

thanks @mattwire

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants