Page MenuHomePhabricator

When a change removes recipients from an object, send them one last email
ClosedPublic

Authored by epriestley on Feb 7 2018, 2:11 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Apr 19, 2:48 AM
Unknown Object (File)
Thu, Apr 11, 8:17 AM
Unknown Object (File)
Sat, Mar 30, 5:49 AM
Unknown Object (File)
Mar 23 2024, 5:03 AM
Unknown Object (File)
Mar 23 2024, 5:03 AM
Unknown Object (File)
Mar 10 2024, 9:29 AM
Unknown Object (File)
Mar 6 2024, 1:57 PM
Unknown Object (File)
Feb 3 2024, 5:59 PM
Subscribers

Details

Summary

Depends on D19018. Fixes T4776. Ref T13053. When you remove someone from an object's recipient list (for example, by removing them a reviewer, auditor, subscriber, owner or author) we currently do not send them mail about it because they're no longer connected to the object.

In many of these cases (Commandeer, Reassign) the actual action in the UI adds them back to the object somehow (as a reviewer or subscriber, respectively) so this doesn't actually matter. However, there's no recovery mechanism for reviewer or subscriber removal.

This is slightly bad from a policy/threat viewpoint since it means an attacker can remove all the recipients of an object "somewhat" silently. This isn't really silent, but it's less un-silent than it should be.

It's also just not very good from a human interaction perspective: if Alice removes Bob as a reviewer, possibly "against his will", he should be notified about that. In the good case, Alice wrote a nice goodbye note that he should get to read. In the bad case, he should get a chance to correct the mistake.

Also add a removed(@user) mail stamp so you can route these locally if you want.

Test Plan
  • Created and edited some different objects without catching anything broken.
  • Removed subscribers from tasks, saw the final email include the removed recipients with a removed() stamp.

I'm not totally sure this doesn't have any surprising behavior or break any weird objects, but I think anything that crops up should be easy to fix.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable