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/financial#148 Simplify parameters passed to completeOrder #18479

Merged
merged 1 commit into from
Sep 19, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Sep 16, 2020

Overview

Simplifies the code in Event batch update to no longer interact with the BaseIPN class - specifically when updating an event to Completed

Before

$baseIPN validate called to retrieve 'objects' - but in practice doing the same as contribution->fetch() plus an incorrect guess at the relevant payment processor

After

just calls contribution->fetch()

Technical Details

Now that completeOrder has had most interaction with objects var cleaned up
we know only 2 objects are actually used

  • contribution - which we can fetch in-function
  • paymentProcessor - which we were actually only guessing

This alters it to only load & pass contribution. The value of passing in
processor seems lost in time. This code can only be reached for pay_later contributions - see

CRM_Contribute_BAO_Contribution::checkOnlinePendingContribution
and specifically

$dao->is_pay_later &&

and only when submitted via an online event (same function) so by definition there IS NO PAYMENT PROCESSOR that applies.

Given that we can just do the minimum to load the contribution and pass that in.

Testing notes

I added CRM_Event_Form_Task_BatchTest::testSubmit prior to starting cleanup on this code. It passes through the altered lines & should lock in 'no change of behaviour'

Comments

Original PR was closed by accident (by me) #18390

Now that completeOrder has had most interaction with objects var cleaned up
we know only 2 objects are actually used
- contribution - which we can fetch in-function
- paymentProcessor - which we were actually only guessing

This alters it to only load & pass contribution. The value of passing in
processor seems lost in time. This code can only be reached for pay_later contributions - see

CRM_Contribute_BAO_Contribution::checkOnlinePendingContribution
and specifically
https://github.com/civicrm/civicrm-core/blob/318c63045e82d30ed7fe8cdacb91a6d31fc5331b/CRM/Contribute/BAO/Contribution.php#L2033

and only when submitted via an online event (same function) so by definition there IS NO PAYMENT PROCESSOR that applies.

Given that we can just do the minimum to load the contribution and pass that in.
@civibot
Copy link

civibot bot commented Sep 16, 2020

(Standard links)

@civibot civibot bot added the master label Sep 16, 2020
@eileenmcnaughton
Copy link
Contributor Author

eileenmcnaughton commented Sep 16, 2020

@JoeMurray @monishdeb this is the last one - looks like I deleted the branch by accident - looking for the original PR now. This is a bit more tangential. It has good test cover but needs an r-run

@eileenmcnaughton eileenmcnaughton changed the title Simplify parameters passed to completeOrder dev/financial#148 Simplify parameters passed to completeOrder Sep 16, 2020
@mattwire mattwire merged commit 71e6c52 into civicrm:master Sep 19, 2020
@eileenmcnaughton eileenmcnaughton deleted the just_pay branch September 19, 2020 19:56
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