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

Remove always true if #18946

Merged
merged 1 commit into from
Nov 9, 2020
Merged

Remove always true if #18946

merged 1 commit into from
Nov 9, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Remove always true if

Before

if (!empty($contributionParams['contribution_recur_id'])) { is always true, but checked

After

Poof

Technical Details

By the time the code reaches this point contributionParams['contribution_recur_id']
must be set. We can confirm this by the fact the call to getTemplateContribution
requires id to be passed in and uses it in a lookup. Also the
code just before repeatTransaction ensures it is called

Screen Shot 2020-11-09 at 12 48 46 PM

repeattransaction is called from only 1 place

Comments

By the time the code reaches this point contributionParams['contribution_recur_id']
must be set. We can confirm this by the fact the call to getTemplateContribution
requires id to be passed in and uses it in a lookup. Also the
code just before repeatTransaction ensures it is called
@civibot civibot bot added the master label Nov 8, 2020
@civibot
Copy link

civibot bot commented Nov 8, 2020

(Standard links)

@seamuslee001
Copy link
Contributor

I think this is ok but would appreciate someone like @mattwire just confirming this is all good

@mattwire
Copy link
Contributor

mattwire commented Nov 9, 2020

@eileenmcnaughton I wasn't sure about this. But tracing back the first lines of repeatTransaction:
if (!empty($contribution->id)) { return FALSE; }

prove that the entire function will not run if it was called via completeTransaction because the API call sets the contribution ID. And for repeatTransaction per earlier work we don't support calling without a recurring contribution ID.

@mattwire mattwire merged commit b3f9bca into civicrm:master Nov 9, 2020
@eileenmcnaughton
Copy link
Contributor Author

Yep these things can take a bit of figuring out - but then the code is more ready for further changes...

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.

3 participants