Email notifications can be configured for Differential and Maniphest in users "Email Preferences". It would be great if "Audit" notifications could be too. Right now the workflow is really spammy. When doing post-commit reviews you usually want to send messages only if concerns are raised about a changeset and an action is expected from the committer. But you do not want to send emails when closing an audit request on a regular changeset, which happens most of the time.
- Mentioned In
- T6344: Changes to E-mail Preferences adversely affected Audits
T6331: Closing a revision via a commit generates superfluous "mention" in Task
rPac83ff706023: Audit - add mail tags
T3956: audit updates don't trigger notifications
T6170: How should I create audits without always emailing all project members
- Mentioned Here
- T4896: Move Audit to ApplicationTransactions
The way this works in Differential and Maniphest is that each email gets some tags, like "comment", "cc", "close", etc. Users then specify which types of email they want as email, and which types they want as just notifications. If an email has any tag the user is interested in, we send it to them. This has seemed to work pretty well, and do a good job of balancing flexibility with simplicity.
D3856 is a similar change, which built this feature out for Differential and expanded it for Maniphest. You can follow the pattern in that diff.
This mail is generated by PhabricatorAuditCommentEditor->sendMail(). It currently does not put any tags on the mail. The implementation will be roughly:
- Add some new audit mail tags (like D3856).
- Add some preferences for them (also like D3856).
- In PhabricatorAuditCommentEditor->sendMail(), build a list of those tags and set them on the mail.
To test this, you'll need to import a repository. You can then view commits and take actions on the commits.
Some reasonable tags might be: closed, cc'd, comment, auditors, accept, raise concern and accept.
The roadmap mostly just has big items -- this is small enough that it probably wouldn't make the cut.
One customer has expressed some interest in this, so it might end up in a contract and then implemented shortly. If not, it will probably be a while.
Does the user not want other comments, or do they want all comments?
Assuming they want all comments, that should be covered by "comment = email, accept = notify (or ignore)". We deliver the highest-strength notification which is applicable to the update, so if the update changes three things (say, adding a CC, adding a comment, accepting) and any of them are set to "email", we'll send you an email about the whole thing.
After T4896, this just requires:
- Implement PhabricatorAuditEditor->getMailTagsMap() to look something like DifferentialTransactionEditor->getMailTagsMap(), except that the constants should be in Audit somewhere (maybe just on the editor) and the tags won't map exactly.
- Implement PhabricatorAuditTransaction->getMailTags() to return the appropriate tags.
- Settings → Email Preferences should show the new options.
- The X-Phabricator-Mail-Tags header in email should reflect the relevant tags for the mail (for example, have the comment tag if there's a comment, and not have it otherwise).
- Changing your settings should filter what mail you receive.