Notify for ship-it revocations
Review Request #12045 — Created Feb. 4, 2022 and updated
- Create boolean field in admin page for email
- Connect handler to revoke signal
- Prepare ship-it revoked email message
- Create new email template for text and HTML
- Modified email settings tests to include the new field.
- Added email test to verify that the email has actually been sent and contains the right text.
Summary | ID |
---|---|
dee2620629820bf3ca6f76b811d29484a4d9ab61 | |
57f846201057f8be07d382e8d5ffdd180edf95d0 | |
e3d46591923cf2b30e45228ee53ca7a9b91629f4 | |
ffc09ed5e8de08ffbef47a92c0e847d52fdde6ce | |
6423a47761c811a6754ea8a244dd389f78e8ea5d | |
cb19e9326b5b409dc0e8806df59820f8c970554e | |
3fc6e424712db0a621d8306f0fbe0c41cd6d506c |
Description | From | Last Updated |
---|---|---|
F401 'importlib' imported but unused |
reviewbot | |
F401 'django.utils.six' imported but unused |
reviewbot | |
E501 line too long (87 > 79 characters) |
reviewbot | |
E302 expected 2 blank lines, found 1 |
reviewbot | |
E302 expected 2 blank lines, found 1 |
reviewbot | |
W291 trailing whitespace |
reviewbot | |
F401 'reviewboard.reviews.views.build_diff_comment_fragments' imported but unused |
reviewbot | |
E128 continuation line under-indented for visual indent |
reviewbot | |
This should probably use the "Re:" logic from prepare_review_request_mail |
david | |
F401 'reviewboard.reviews.models.review_request' imported but unused |
reviewbot | |
E231 missing whitespace after ',' |
reviewbot | |
F811 redefinition of unused 'review_request' from line 17 |
reviewbot | |
Would it be clearer to write the label as "Send e-mails to users when the reviews' ship-it status are revoked"? |
sheenaNg | |
It looks good! One suggestion, could you include docstring for this method similar to other methods in this module? |
sheenaNg | |
Could you also include docstring for this method? |
sheenaNg | |
Is this supposed to be review.REVOKED_SHIP_IT_TEXT? |
sheenaNg | |
I noticed that you included On {{review_request.time_emailed}}, {{review_request.submitter|user_displayname}} wrote: {% quoted_email "notifications/review_request_email.txt" %}} in the email.txt template. I wonder if … |
sheenaNg | |
Please add a class docstring. |
david | |
The description needs to go on the next line. |
david | |
The description needs to go on the next line. |
david | |
E501 line too long (81 > 79 characters) |
reviewbot | |
Description for the return type doesn't need to be indented (align with EmailMessage) |
david | |
This needs a trailing comma. Please also use single quotes. |
david | |
E501 line too long (81 > 79 characters) |
reviewbot | |
E501 line too long (80 > 79 characters) |
reviewbot | |
E501 line too long (81 > 79 characters) |
reviewbot |
Commits: |
|
|||||||||
---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 2 (+2432 -1050) |
Checks run (1 failed, 1 succeeded)
flake8
-
reviewboard/__init__.py (Diff revision 2) -
Description: |
|
|||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
Commits: |
|
|||||||||||
Diff: |
Revision 3 (+140 -6) |
Checks run (1 failed, 1 succeeded)
flake8
-
-
-
reviewboard/notifications/email/signal_handlers.py (Diff revision 3) E302 expected 2 blank lines, found 1
Description: |
|
|||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commits: |
|
|||||||||||||
Diff: |
Revision 4 (+265 -13) |
Checks run (1 failed, 1 succeeded)
flake8
-
-
reviewboard/notifications/email/message.py (Diff revision 4) F401 'reviewboard.reviews.views.build_diff_comment_fragments' imported but unused
-
reviewboard/notifications/email/message.py (Diff revision 4) E128 continuation line under-indented for visual indent
-
reviewboard/notifications/email/signal_handlers.py (Diff revision 4) F401 'reviewboard.reviews.models.review_request' imported but unused
-
reviewboard/notifications/email/signal_handlers.py (Diff revision 4) E231 missing whitespace after ','
-
reviewboard/notifications/email/signal_handlers.py (Diff revision 4) F811 redefinition of unused 'review_request' from line 17
-
Looks like everything is on the right track!
-
reviewboard/notifications/email/message.py (Diff revision 4) This should probably use the "Re:" logic from
prepare_review_request_mail
Description: |
|
|||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commits: |
|
|||||||||||||||||
Diff: |
Revision 5 (+567 -17) |
Checks run (2 succeeded)
-
-
reviewboard/admin/forms/email_settings.py (Diff revision 5) Would it be clearer to write the label as "Send e-mails to users when the reviews' ship-it status are revoked"?
-
reviewboard/notifications/email/message.py (Diff revision 5) It looks good! One suggestion, could you include docstring for this method similar to other methods in this module?
-
reviewboard/notifications/email/signal_handlers.py (Diff revision 5) Could you also include docstring for this method?
-
reviewboard/notifications/email/signal_handlers.py (Diff revision 5) I noticed that the
send_review_published_mail
signal handler includes a parameter calledto_owner_only
that indicates whether or not the mail should only be sent to the review request
submitter.
I am just curious of why this field is not added to this method to enable the option to send it to the submitter only? -
reviewboard/templates/notifications/review_ship_it_revoked_email.html (Diff revision 5) Is this supposed to be
review.REVOKED_SHIP_IT_TEXT
? -
reviewboard/templates/notifications/review_ship_it_revoked_email.html (Diff revision 5) I noticed that you included
On {{review_request.time_emailed}}, {{review_request.submitter|user_displayname}} wrote: {% quoted_email "notifications/review_request_email.txt" %}}
in the email.txt template.I wonder if you should also include that in the .html similar to what is shown in the
review_email.html
block footer?
Testing Done: |
|
|||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commits: |
|
|||||||||||||||||||||
Diff: |
Revision 6 (+687 -17) |
Checks run (1 failed, 1 succeeded)
flake8
-
reviewboard/notifications/email/message.py (Diff revision 6) E501 line too long (81 > 79 characters)
-
reviewboard/notifications/email/signal_handlers.py (Diff revision 6) E501 line too long (81 > 79 characters)
-
reviewboard/notifications/tests/test_email_sending.py (Diff revision 6) E501 line too long (80 > 79 characters)
-
-
-
-
-
reviewboard/notifications/email/message.py (Diff revision 6) Description for the return type doesn't need to be indented (align with
EmailMessage
) -
reviewboard/notifications/email/message.py (Diff revision 6) This needs a trailing comma. Please also use single quotes.
Commits: |
|
|||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 7 (+736 -26) |
Checks run (1 failed, 1 succeeded)
flake8
Commits: |
|
|||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 8 (+737 -27) |