Mark notifications as read in Differential if we also sent an email

Summary: See D3789. Same thing for Differential.

Test Plan: Created a new revision and made a comment. Verified reviewer got popup notifications but the in-app notifications were delivered already marked as read.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1403

Differential Revision: https://secure.phabricator.com/D3790
This commit is contained in:
epriestley
2012-10-23 12:03:11 -07:00
parent 6b39af4022
commit 7749c5abf3
3 changed files with 81 additions and 58 deletions

View File

@@ -558,8 +558,9 @@ final class DifferentialCommentEditor extends PhabricatorEditor {
$xherald_header = HeraldTranscript::loadXHeraldRulesHeader( $xherald_header = HeraldTranscript::loadXHeraldRulesHeader(
$revision->getPHID()); $revision->getPHID());
$mailed_phids = array();
if (!$this->noEmail) { if (!$this->noEmail) {
id(new DifferentialCommentMail( $mail = id(new DifferentialCommentMail(
$revision, $revision,
$actor_handle, $actor_handle,
$comment, $comment,
@@ -575,6 +576,8 @@ final class DifferentialCommentEditor extends PhabricatorEditor {
->setXHeraldRulesHeader($xherald_header) ->setXHeraldRulesHeader($xherald_header)
->setParentMessageID($this->parentMessageID) ->setParentMessageID($this->parentMessageID)
->send(); ->send();
$mailed_phids = $mail->getRawMail()->buildRecipientList();
} }
$event_data = array( $event_data = array(
@@ -586,12 +589,13 @@ final class DifferentialCommentEditor extends PhabricatorEditor {
'feedback_content' => $comment->getContent(), 'feedback_content' => $comment->getContent(),
'actor_phid' => $actor_phid, 'actor_phid' => $actor_phid,
); );
// TODO: Get rid of this
id(new PhabricatorTimelineEvent('difx', $event_data)) id(new PhabricatorTimelineEvent('difx', $event_data))
->recordEvent(); ->recordEvent();
// TODO: Move to a daemon?
id(new PhabricatorFeedStoryPublisher()) id(new PhabricatorFeedStoryPublisher())
->setStoryType(PhabricatorFeedStoryTypeConstants::STORY_DIFFERENTIAL) ->setStoryType('PhabricatorFeedStoryDifferential')
->setStoryData($event_data) ->setStoryData($event_data)
->setStoryTime(time()) ->setStoryTime(time())
->setStoryAuthorPHID($actor_phid) ->setStoryAuthorPHID($actor_phid)
@@ -607,9 +611,10 @@ final class DifferentialCommentEditor extends PhabricatorEditor {
array($revision->getAuthorPHID()), array($revision->getAuthorPHID()),
$revision->getReviewers(), $revision->getReviewers(),
$revision->getCCPHIDs())) $revision->getCCPHIDs()))
->setMailRecipientPHIDs($mailed_phids)
->publish(); ->publish();
// TODO: Move to a daemon? // TODO: Move to workers
PhabricatorSearchDifferentialIndexer::indexRevision($revision); PhabricatorSearchDifferentialIndexer::indexRevision($revision);
return $comment; return $comment;

View File

@@ -420,8 +420,62 @@ final class DifferentialRevisionEditor extends PhabricatorEditor {
id(new PhabricatorTimelineEvent('difx', $event_data)) id(new PhabricatorTimelineEvent('difx', $event_data))
->recordEvent(); ->recordEvent();
$mailed_phids = array();
if (!$this->silentUpdate) {
$revision->loadRelationships();
if ($add['rev']) {
$message = id(new DifferentialNewDiffMail(
$revision,
$actor_handle,
$changesets))
->setIsFirstMailAboutRevision($is_new)
->setIsFirstMailToRecipients(true)
->setToPHIDs(array_keys($add['rev']));
if ($is_new) {
// The first time we send an email about a revision, put the CCs in
// the "CC:" field of the same "Review Requested" email that reviewers
// get, so you don't get two initial emails if you're on a list that
// is CC'd.
$message->setCCPHIDs(array_keys($add['ccs']));
}
$mail[] = $message;
}
// 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(
$revision,
$actor_handle,
$changesets))
->setIsFirstMailToRecipients(true)
->setToPHIDs(array_keys($add['ccs']));
}
foreach ($mail as $message) {
$message->setHeraldTranscriptURI($xscript_uri);
$message->setXHeraldRulesHeader($xscript_header);
$message->send();
$mailed_phids[] = $message->getRawMail()->buildRecipientList();
}
$mailed_phids = array_mergev($mailed_phids);
}
id(new PhabricatorFeedStoryPublisher()) id(new PhabricatorFeedStoryPublisher())
->setStoryType(PhabricatorFeedStoryTypeConstants::STORY_DIFFERENTIAL) ->setStoryType('PhabricatorFeedStoryDifferential')
->setStoryData($event_data) ->setStoryData($event_data)
->setStoryTime(time()) ->setStoryTime(time())
->setStoryAuthorPHID($revision->getAuthorPHID()) ->setStoryAuthorPHID($revision->getAuthorPHID())
@@ -436,62 +490,11 @@ final class DifferentialRevisionEditor extends PhabricatorEditor {
array($revision->getAuthorPHID()), array($revision->getAuthorPHID()),
$revision->getReviewers(), $revision->getReviewers(),
$revision->getCCPHIDs())) $revision->getCCPHIDs()))
->setMailRecipientPHIDs($mailed_phids)
->publish(); ->publish();
// TODO: Move this into a worker task thing. // TODO: Move this into a worker task thing.
PhabricatorSearchDifferentialIndexer::indexRevision($revision); PhabricatorSearchDifferentialIndexer::indexRevision($revision);
if ($this->silentUpdate) {
return;
}
$revision->loadRelationships();
if ($add['rev']) {
$message = id(new DifferentialNewDiffMail(
$revision,
$actor_handle,
$changesets))
->setIsFirstMailAboutRevision($is_new)
->setIsFirstMailToRecipients(true)
->setToPHIDs(array_keys($add['rev']));
if ($is_new) {
// The first time we send an email about a revision, put the CCs in
// the "CC:" field of the same "Review Requested" email that reviewers
// get, so you don't get two initial emails if you're on a list that
// is CC'd.
$message->setCCPHIDs(array_keys($add['ccs']));
}
$mail[] = $message;
}
// 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(
$revision,
$actor_handle,
$changesets))
->setIsFirstMailToRecipients(true)
->setToPHIDs(array_keys($add['ccs']));
}
foreach ($mail as $message) {
$message->setHeraldTranscriptURI($xscript_uri);
$message->setXHeraldRulesHeader($xscript_header);
$message->send();
}
} }
public static function addCCAndUpdateRevision( public static function addCCAndUpdateRevision(

View File

@@ -35,6 +35,15 @@ abstract class DifferentialMail {
protected $replyHandler; protected $replyHandler;
protected $parentMessageID; protected $parentMessageID;
private $rawMail;
public function getRawMail() {
if (!$this->rawMail) {
throw new Exception("Call send() before getRawMail()!");
}
return $this->rawMail;
}
protected function renderSubject() { protected function renderSubject() {
$revision = $this->getRevision(); $revision = $this->getRevision();
$title = $revision->getTitle(); $title = $revision->getTitle();
@@ -187,6 +196,10 @@ abstract class DifferentialMail {
$this->prepareBody(); $this->prepareBody();
$this->rawMail = clone $template;
$this->rawMail->addTos($to_phids);
$this->rawMail->addCCs($cc_phids);
$mails = $reply_handler->multiplexMail($template, $to_handles, $cc_handles); $mails = $reply_handler->multiplexMail($template, $to_handles, $cc_handles);
$original_translator = PhutilTranslator::getInstance(); $original_translator = PhutilTranslator::getInstance();
@@ -236,6 +249,8 @@ abstract class DifferentialMail {
} }
PhutilTranslator::setInstance($original_translator); PhutilTranslator::setInstance($original_translator);
return $this;
} }
protected function getMailTags() { protected function getMailTags() {