-
-
Notifications
You must be signed in to change notification settings - Fork 825
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
Initial refactor of PayPal core processor to stop using doDirectPayment/doTransferCheckout #20030
Conversation
(Standard links)
|
|
||
if ($this->_paymentProcessor['billing_mode'] == 4) { | ||
$this->doPaymentRedirectToPayPal($params, $component); | ||
// redirect calls CiviExit() so execution is stopped |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Core doPayment()
assumes that doTransferCheckout() returns but for PayPal it does not because CRM_Utils_System::redirect()
is called at the end of the function.
I gave the code a quick skim & this makes sense - I haven't gone through more carefully/tested at this stage |
@mattwire it's a looong time since we consolidated on doPayment but it used to be the case that doTransferCheckout & doDirectPayment were directly called. Until your PR yesterday we never actually put a deprecation notice in them! I think I'd feel more comfortable is we added them back with a deprecation notice & a call to the renamed function just in case |
…nt/doTransferCheckout
814ea19
to
92112a9
Compare
@eileenmcnaughton Ok that makes sense. I've added the legacy methods with deprecated warnings. |
92112a9
to
ecd3687
Compare
thanks @mattwire |
OK - I've read through this a couple more times & I'm pretty comfortable that it is just moving the furniture and won't change (break) anything |
Overview
PayPal core payment processor is calling
doDirectPayment
anddoTransferCheckout
via parentdoPayment()
method. This PR re-implementsdoPayment()
directly on the PayPal class and renames thedoDirectPayment
/doTransferCheckout
methods to make it clear that they have no dependency on legacy core code.Before
Relies on legacy core code to call legacy methods.
After
Self-contained code using standard
doPayment()
as entry-point.Technical Details
This would allow for further refactoring/simplification in the future. It is now obvious that the methods are internal to PayPal.
Comments