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#152 remove determination of source #19017

Merged
merged 1 commit into from
Dec 10, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Nov 22, 2020

Overview

This removes a chunk of code used to determine the source to be given to a contribution in completeOrder because I believe it's extraneous

Either
1 ) we are completing an existing contribution - in which case the source will already exist or
2 ) we are repeating a contribution - in which case the source will be derived from the template contribution
(with recurring appended).

Before

lotsa code

After

poof

Technical Details

Removing this brings us close to the point where contribution is unnessary as an input parameter on the function

I believe the logic dates back to when this function was part of the IPN and it may have been necessary to create a new contribution because ... 4.x

Comments

@civibot
Copy link

civibot bot commented Nov 22, 2020

(Standard links)

@civibot civibot bot added the master label Nov 22, 2020
@eileenmcnaughton eileenmcnaughton force-pushed the remove_recur branch 3 times, most recently from 0352136 to 0fa584e Compare November 23, 2020 00:19
This function call determines the source, requiring quite a bit of complexity - however I believe it is unnecessary because either
1 ) we are completing an existing contribution - in which case the source will already exist or
2 ) we are repeating a contribution - in which case the source will be derived from the template contribution
(with recurring appended).

Removing this brings us close to the point where contribution is unnessary as an input parameter on the function
@eileenmcnaughton
Copy link
Contributor Author

@mattwire did you spot this one - basically I think there is a bunch of code here & in the functions that call completeOrder to prepare for this that seems of no value

@eileenmcnaughton
Copy link
Contributor Author

@mattwire what do you think on this one - it seems to me we can simplify here

@@ -45,6 +47,7 @@ public function createRepeatMembershipOrder() {
$orderID = $this->callAPISuccess('Order', 'create', [
'total_amount' => '200',
'financial_type_id' => 'Donation',
'source' => 'Online Contribution: form payment',
Copy link
Contributor Author

@eileenmcnaughton eileenmcnaughton Dec 10, 2020

Choose a reason for hiding this comment

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

Test change is because the template contribution source is now used so we need to set it up in a valid/real-world sort of way - the source-is-empty-so-we-need-to-make-something-up is from an older flow IMHO

@mattwire
Copy link
Contributor

Yes, agreed.

@mattwire mattwire merged commit dc7f057 into civicrm:master Dec 10, 2020
@eileenmcnaughton eileenmcnaughton deleted the remove_recur branch December 11, 2020 01:23
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