-
Notifications
You must be signed in to change notification settings - Fork 26
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
[#4895] outgoing email logging improvements #4905
[#4895] outgoing email logging improvements #4905
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4905 +/- ##
=======================================
Coverage 96.57% 96.57%
=======================================
Files 757 757
Lines 25728 25730 +2
Branches 3379 3379
=======================================
+ Hits 24847 24849 +2
Misses 616 616
Partials 265 265 ☔ View full report in Codecov by Sentry. |
@@ -302,3 +303,33 @@ def test_cancel_and_change_instructions(self): | |||
with self.subTest(type="plain text"): | |||
self.assertIn(cancel_link, message_text) | |||
self.assertNotIn(change_link, message_text) | |||
|
|||
def test_headers_present_in_confirmation_email(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to have this test in the submissions app instead of appointments, since it's not something exclusive or special to appointments - even for non-appointment submissions we expect these confirmation emails (with custom headers) to be sent.
"Content-Language": submission.language_code, | ||
X_OF_CONTENT_TYPE_HEADER: EmailContentTypeChoices.submission, | ||
X_OF_CONTENT_UUID_HEADER: str(submission.uuid), | ||
X_OF_EVENT_HEADER: EmailEventChoices.cosign_confirmation, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This email isn't a cosign confirmation email, but rather a cosign request. To be fair, I think the original code defining the event is wrong and should be renamed to 'cosign-request', since the cosign confirmation is not much different from the "normal" confirmation email (if I remember correctly).
03d38b9
to
0fdd871
Compare
It is rather a cosign request email, since the cosign confirmation itself is not much different from the "normal" confirmation email.
0fdd871
to
9441219
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one!
Closes #4895
Changes
Add custom headers to confirmation and cosign emails
Checklist
Check off the items that are completed or not relevant.
Impact on features
Release management
I have updated the translations assets (you do NOT need to provide translations)
./bin/makemessages_js.sh
./bin/compilemessages_js.sh
Commit hygiene