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

[Ref] Move sending the email back out of the recur function #18852

Merged
merged 1 commit into from
Nov 2, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Oct 23, 2020

Overview

[Ref] Move sending the email back out of the recur function

Before

The call to CRM_Contribute_BAO_ContributionPage::recurringNotify is within the recur function - it is only called from one place and variables are being wrangled / made confusing to pass through

After

The call to CRM_Contribute_BAO_ContributionPage::recurringNotify is back in 'Main' - we can start to stop passing in the $ids array (which we hope to fully deprecate as they can be calculated as easily in CompleteOrder)

Technical Details

This allows us not to pass in extra ids that are only required for that use and
steps us closer to not loading them

Comments

@civibot
Copy link

civibot bot commented Oct 23, 2020

(Standard links)

@civibot civibot bot added the master label Oct 23, 2020
@eileenmcnaughton eileenmcnaughton changed the title Move sending the email back out of the recur function [Ref] Move sending the email back out of the recur function Oct 23, 2020
@seamuslee001
Copy link
Contributor

@eileenmcnaughton looking at the 2 returns in the old lines 200 and 207 this might trigger this code when it wasn't previously being triggered

This allows us not to pass in extra ids that are only required for that use and
steps us closer to not loading them
@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 good call - I've switched them to FALSE

@seamuslee001
Copy link
Contributor

this looks fine now

@seamuslee001 seamuslee001 merged commit 7f57015 into civicrm:master Nov 2, 2020
@seamuslee001 seamuslee001 deleted the aip branch November 2, 2020 04:08
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