Intent to ship email fixes #747#1147
Merged
jaredlockhart merged 13 commits intomozilla:masterfrom Apr 15, 2019
glasserc:747-intent-to-ship-email
Merged
Intent to ship email fixes #747#1147jaredlockhart merged 13 commits intomozilla:masterfrom glasserc:747-intent-to-ship-email
jaredlockhart merged 13 commits intomozilla:masterfrom
glasserc:747-intent-to-ship-email
Conversation
- No need to pass all these template arguments separately since they're largely available on the experiment themselves. - Do some weird stuff to try to minimize number of extra blank lines when risk/technical complexity are absent. - Disable autoescaping since this is a text email. - Add tests.
Collaborator
|
@glasserc Can you please include screenshots of all the releant UI changes? Thanks! |
jaredlockhart
requested changes
Apr 4, 2019
Collaborator
jaredlockhart
left a comment
jaredlockhart
left a comment
There was a problem hiding this comment.
Awesome @glasserc this is great. I like the modal, I like the behaviour. A couple changes listed below.
Contributor
Author
|
Oops, yeah, screenshots...
|
Contributor
Author
|
|
Thanks @jaredkerim for pointing this out.
Thanks @jaredkerim for the suggestion.
Thanks @jaredkerim for the suggestion.
Thanks @jaredkerim for the suggestion.
These utility methods in the test suite do not need 100% coverage.
Adding a CC requires using the EmailMessage API directly instead of the send_mail wrapper.
jaredlockhart
approved these changes
Apr 15, 2019
Collaborator
jaredlockhart
left a comment
jaredlockhart
left a comment
There was a problem hiding this comment.
Awesome this looks good @glasserc! I like the patterns you used here, we should probably adopt them for more parts of the project (text templates for emails, API calls from modals).
| if (resp.status == 200) { | ||
| // Rather than trying to update the DOM to match the new state, | ||
| // just refresh the page. | ||
| location.reload(); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.