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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix email template rendering issue #2256

Merged
merged 2 commits into from Sep 25, 2018

Conversation

Projects
None yet
3 participants
@alexsanford
Contributor

alexsanford commented Sep 25, 2018

Fixes #2255

The problem was that when this action was set to fire earlier in #2217, the global $email_template variable was set earlier (in this code) but was never reset. This caused the handling here to always use an email template for subsequent calls, causing the template to be rendered on the front-end.

This PR resets the $email_template variable, and also future-proofs the template handler in case the email is ever rendered later (there was a separate issue here that would have caused late-rendered emails to render incorrectly on unsupported themes).

Kudos to @jom for his help in getting to the root of this one! 馃憦

Handle global $email_template properly
- Ensure that it is reset after it is used.
- Ensure that we do not mangle template loading for emails when
  $email_template is set.

@alexsanford alexsanford added this to the 1.12.0 milestone Sep 25, 2018

@alexsanford alexsanford self-assigned this Sep 25, 2018

@alexsanford alexsanford requested review from jom and donnapep Sep 25, 2018

@jom

jom approved these changes Sep 25, 2018

Code looks good. Unsupported and supported themes still seem to load the correct template and email notifications are sent. The email footer output is no longer an issue. 馃憤

@donnapep

Very nice. Looks good except for the tests.

@alexsanford

This comment has been minimized.

Contributor

alexsanford commented Sep 25, 2018

Tests are fixed! 馃檪

@alexsanford alexsanford merged commit 9a9a928 into master Sep 25, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@alexsanford alexsanford deleted the fix/course-complete-render-issue branch Sep 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment