revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 08:52:39 +01:00
Code Issues Releases Wiki Activity

Remove all current Authors / Reviewers from "CCWelcome" mail

Browse source 
Summary:
We'll incorrectly send CCWelcome mail to users who would be added as CCs but are blocked by the new "$dont_add" stuff, for
example when a revision is updated and the user has a Herald rule which triggers them getting CC'd. See D2057.

Potentially a better fix for this would be to have "addCCs" return a list of the CCs it actually added, rather than duplicating the
logic of removing CCs in two places. However, that's not trivial since it's just a wrapper around alterRelationships() which is nasty
and would need a more complicated return type. I think this whole thing will get a refactoring pass at some point -- I want to build a
more generic "associations"-like datastore and replace some of the ad-hoc associations with it. So maybe I can clean it up when that
happens. For now, this should fix the immediate problem.

Test Plan: Updated a revision, didn't get CC welcomed.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2072
This commit is contained in:
epriestley 2012-04-02 10:41:03 -07:00
parent 49a0b3fab0
commit eeec726ded
1 changed files with 16 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

25
src/applications/differential/editor/revision/DifferentialRevisionEditor.php
View file

@ -461,13 +461,16 @@ final class DifferentialRevisionEditor {
$mail[] = $message;
}
// If you were added as a reviewer and a CC, just give you the reviewer
// email. We could go to greater lengths to prevent this, but there's
// bunch of stuff with list subscriptions anyway. You can still get two
// emails, but only if a revision is updated and you are added as a reviewer
// at the same time a list you are on is added as a CC, which is rare and
// reasonable.
$add['ccs'] = array_diff_key($add['ccs'], $add['rev']);
// If we added CCs, we want to send them an email, but only if they were not
// already a reviewer and were not added as one (in these cases, they got
// a "NewDiff" mail, either in the past or just a moment ago). You can still
// get two emails, but only if a revision is updated and you are added as a
// reviewer at the same time a list you are on is added as a CC, which is
// rare and reasonable.
$implied_ccs = self::getImpliedCCs($revision);
$implied_ccs = array_fill_keys($implied_ccs, true);
$add['ccs'] = array_diff_key($add['ccs'], $implied_ccs);
if (!$is_new && $add['ccs']) {
$mail[] = id(new DifferentialCCWelcomeMail(
@ -546,8 +549,7 @@ final class DifferentialRevisionEditor {
array $add_phids,
$reason_phid) {
$dont_add = $revision->getReviewers();
$dont_add[] = $revision->getAuthorPHID();
$dont_add = self::getImpliedCCs($revision);
$add_phids = array_diff($add_phids, $dont_add);
return self::alterRelationships(
@ -559,6 +561,11 @@ final class DifferentialRevisionEditor {
DifferentialRevision::RELATION_SUBSCRIBED);
}
private static function getImpliedCCs(DifferentialRevision $revision) {
return array_merge(
$revision->getReviewers(),
array($revision->getAuthorPHID()));
}
public static function alterReviewers(
DifferentialRevision $revision,