Feature/email secretary after decisions #582
Conversation
reviews/utils/review_utils.py
Outdated
| "decisions": review.decision_set.all() | ||
| } | ||
| msg_plain = render_to_string("mail/all_decisions_notify.txt", params) | ||
| send_mail(subject, msg_plain, settings.EMAIL_FROM, [secretary.email]) |
There was a problem hiding this comment.
As per #563:
[..] FETC npm (not users!) [..]
Thus, the recipient should be the general fetc email ;)
reviews/utils/review_utils.py
Outdated
| subject = _("FETC-GW {}: alle beoordelingen toegevoegd").format( | ||
| review.proposal.committee_prefixed_refnum(), | ||
| ) |
There was a problem hiding this comment.
You're translating the string, but not adding any translations...
Honestly, as the email is not translated, I'd just drop the _([..]) tag altogether
| {% for decision in decisions %} | ||
| Beoordelaar: {{ decision.reviewer.get_full_name }} | ||
| Beoordeling: {{ decision.get_go_display }} | ||
| Opmerkingen: {{ decision.comments|default:"geen"|safe }} |
There was a problem hiding this comment.
Reviewer comments can be hundreds of words. I think this table is a really cool addition, but the actual comments might be a bit much.
Rather, I would include a link to the review detail page, and also a link to the decide close review page.
| "proposal": review.proposal, | ||
| "decisions": review.decision_set.all() | ||
| } | ||
| msg_plain = render_to_string("mail/all_decisions_notify.txt", params) |
There was a problem hiding this comment.
Any translations existing in this input, e.g. decision.get_go_display, will render to the currently selected language.
If the user who triggered the update_go() happened to be set to English, all translations in here will also turn into English, which is a bit weird.
See proposal_utils.notify_local_staff() for an example of how we manually activate a language for email sending, and then reset it afterwards.
reviews/utils/review_utils.py
Outdated
| send_mail(subject, msg_plain, settings.EMAIL_FROM, ["[email protected]"]) | ||
|
|
There was a problem hiding this comment.
I should've clarified; you should use settings.EMAIL_FROM as the recipient.
It's kinda weird, I know, but EMAIL_FROM is the NPM email. Hardcoding this is a bad idea, as names (and thus, emails) in the UU tend to change. It's better if we only have to change the one value when that happens
Some fixes for a two small issues.
the fix for #581 is pretty self explanatory.
For the issue #563, I updated the
update_go()ofReview, as Ty suggested. Please check if this is in the correct spot ... In my testing this works as expected.Furthermore, I'd appreciate feedback on the text of the e-mail. It's pretty bare bones, but gets the job done, with an overview of all decisions.
PS. I accidentally blacked review_utils.py, but undid it, because it made the diff unreadable.