Allow processors (inc. paypal rest) to not support cancel recurring - #210
Allow processors (inc. paypal rest) to not support cancel recurring#210michaelmcandrew wants to merge 2 commits into
Conversation
|
@michaelmcandrew I don't think this is the right way to do it - you still want to be able to "cancel" the subscription in CiviCRM but you don't want to notify the payment processor - you should set |
|
What @mattwire says seems right - if that works then do that.... note I think the behaviour for Paypal rest on recurrings is the same as other processors in Omnipay (ie it's supported but should not 'do' anything) so you don't have to implement it in a processor specific way |
|
Thanks for the pointers, both! You were right, @mattwire. After a bit more testing we realised that the fix I suggested above resulted in the cancel this subscription link being removed from the receipts. On further investigation, it looks like CRM_Core_Payment::doCancelRecurring() is expecting CRM_Core_Payment_OmnipayMultiProcessor::cancelSubscription() to return true to avoid throwing an exception. But it returns void, which I think is ultimately the source of the errors we were experiencing (and this is likely omnipay wide). From my reading of CRM_Core_Payment::doCancelRecurring(), the best solution here would be to remove the empty CRM_Core_Payment_OmnipayMultiProcessor::cancelSubscription(). Let me know what you think and I will update the patch. Also @eileenmcnaughton - do any of the omnipay processors communicate with the payment processor to cancel recurring contributions. My reading of the code suggested that they do not, and if that is the case, then I think we should probably set |
|
do any of the omnipay processors communicate with the payment processor to cancel recurring contributions. - the best solution here would be to remove the empty CRM_Core_Payment_OmnipayMultiProcessor::cancelSubscription(). |
8f31154 to
679df0f
Compare
The current PayPal Checkout implementation uses reference transactions to deliver recurring payments.
Therefore, no action is needed on the Payment processor side to cancel a recurring transaction.
So I wanted to set CRM_Core_Payment_OmnipayMultiProcessor::supportsCancelRecurring() to return FALSE.
Currently, with it set to TRUE, cancelling a recurring payment is broken for anoymous users. Setting it to FALSE fixes the issue.
I added a meta data field to define this to PayPal Checkout.
I didn't want to add it to other payment processors but since the default for undefined meta data is false, I thought it might be a good idea to be able to default the default when none exists.
To do this, I added another param to ::getProcessorTypeMetadata() to allow it to return other values (TRUE in this case).
I also added a commit to comply with civicrm coding standards. The interesting stuff is in this commit: 11bcd79.
Interested to know what you think about this approach.