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

CRM-19710 - Preserve is_email_receipt parameter through to email sent #10000

Merged
merged 1 commit into from
Mar 17, 2017

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Mar 16, 2017

@eileenmcnaughton
Copy link
Contributor Author

@agileware I reviewed #9487 & kept the test & some lines, but I felt we could do better in terms of rationalising the logic. Note this probably does a bit less than yours did. I focussed on the tested flow, but the logic feels right to me - ie. when loadRelatedObjects is called set the detail of the contributionPage in the $_relatedObjects property & dip into that when needed.

Plus, set the is_email_receipt in one place really explicitly, rather than all these interactions with it. Then extra logic can be added from the other functions.

Finally - I can't see that it is valid to call sendconfirmation with is_email_receipt not set to 1, that does not seem like it's intent, or the way in which it is called.

@eileenmcnaughton
Copy link
Contributor Author

Note, I attributed the commit to you since you did the most important part, so, you'd better make sure you are happy with it.

@agileware
Copy link
Contributor

@eileenmcnaughton looks good. Thanks for following through with this!

@eileenmcnaughton
Copy link
Contributor Author

ok I'll merge - would be great if you can test the rc once cut too

@eileenmcnaughton eileenmcnaughton merged commit bf4f0b1 into civicrm:master Mar 17, 2017
@eileenmcnaughton eileenmcnaughton deleted the agile branch March 17, 2017 00:44
monishdeb pushed a commit to monishdeb/civicrm-core that referenced this pull request May 2, 2017
CRM-19710 - Preserve is_email_receipt parameter through to email sent
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.

4 participants