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

Bugfix: Template information not stored in correct scope #60

Closed
wants to merge 2 commits into from

Conversation

jscharlach
Copy link

If EmailTemplate was used to render multiple different templates in rapid succession it would always use the last template called for every render. This update moves the non-static data into a scope object.

If EmailTemplate was used to render multiple different templates in rapid succession it would always use the last template called for every render. This update moves the non-static data into a scope object.
@jasonsims
Copy link
Contributor

@jscharlach thanks for this! I think this actually relates to #46 that we were unable to reproduce. It looks like the scope changes broke the test for batch emails though. Can you fix the broken build as part of this PR?

@niftylettuce
Copy link
Collaborator

@jscharlach can you fix the broken build? /cc @jasonsims

@niftylettuce
Copy link
Collaborator

@jscharlach any luck?

@jscharlach
Copy link
Author

Gah. These messages got stuck in my spam folder somehow. I'll take a look
tomorrow.

Sorry for the delay.
On May 27, 2014 12:50 PM, "Nick Baugh" notifications@github.com wrote:

@jscharlach https://github.com/jscharlach any luck?


Reply to this email directly or view it on GitHubhttps://github.com//pull/60#issuecomment-44302833
.

@jscharlach
Copy link
Author

I've fixed the issue that I caused with batch template processing and slightly refactored my changes.

@jasonsims
Copy link
Contributor

Thanks @jscharlach! The tests are still failing but only for node v0.8. Since we're dropping support that version anyway I'll include this in the v1.0 branch before merging it to master.

@jasonsims jasonsims mentioned this pull request May 30, 2014
@jasonsims
Copy link
Contributor

This is now merged into v1.0.0 and has been published to npm

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.

None yet

3 participants