From 846d625ed0eb67bc22906d2b3e55ea320e8f7fd7 Mon Sep 17 00:00:00 2001 From: Ryan McElroy Date: Sun, 8 May 2011 00:35:29 -0700 Subject: [PATCH] [differential] gmail-compatible emails Summary: Gmail ignores text inside of [square brackets] when deciding what to group together. This diff does two things to create the right behavior for gmail: 1. put the verb text inside of [square brackets] so different verbs don't break gmail threading. 2. Add the Diff ID to the email thread, so different diffs with the same name don't group together. Furthermore, to aid in distinguishing who is doing what when the from field can't be spoofed, this diff adds the usename just before the verb. This works quite well in the english language. For example: [Differential] [rm requested a review of] D1: [admin] Create arcconfig for code reviews [Differential] [rm commented on] D1: [admin] Create arcconfig for code reviews It's almost like a complete sentence. All it's missing is a period. Test Plan: Did it live on my test setup. Received emails with subjects that looked right. Verified that gmail grouped the emails despite the different actions taking place (tested: comments, planned changes, request review). Reviewed By: epriestley Reviewers: epriestley, jungejason CC: aran, epriestley, rm Differential Revision: 251 --- .../differential/mail/comment/DifferentialCommentMail.php | 7 +++++-- .../editor/transaction/ManiphestTransactionEditor.php | 6 ++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/applications/differential/mail/comment/DifferentialCommentMail.php b/src/applications/differential/mail/comment/DifferentialCommentMail.php index 42eb8c7155..060264b166 100644 --- a/src/applications/differential/mail/comment/DifferentialCommentMail.php +++ b/src/applications/differential/mail/comment/DifferentialCommentMail.php @@ -45,9 +45,12 @@ class DifferentialCommentMail extends DifferentialMail { } protected function renderSubject() { + $verb = ucwords($this->getVerb()); $revision = $this->getRevision(); - $verb = $this->getVerb(); - return ucwords($verb).': '.$revision->getTitle(); + $title = $revision->getTitle(); + $id = $revision->getID(); + $subject = "[{$verb}] D{$id}: {$title}"; + return $subject; } protected function getVerb() { diff --git a/src/applications/maniphest/editor/transaction/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/transaction/ManiphestTransactionEditor.php index 8d4ac3430d..ade1eca2f6 100644 --- a/src/applications/maniphest/editor/transaction/ManiphestTransactionEditor.php +++ b/src/applications/maniphest/editor/transaction/ManiphestTransactionEditor.php @@ -17,6 +17,7 @@ */ class ManiphestTransactionEditor { + const SUBJECT_PREFIX = '[Maniphest]'; public function applyTransactions($task, array $transactions) { @@ -174,10 +175,11 @@ class ManiphestTransactionEditor { " ".$task_uri."\n"; $thread_id = 'getPHID().'>'; + $task_id = $task->getID(); + $title = $task->getTitle(); id(new PhabricatorMetaMTAMail()) - ->setSubject( - '[Maniphest] T'.$task->getID().' '.$action.': '.$task->getTitle()) + ->setSubject(self::SUBJECT_PREFIX." [{$action}] T{$task_id}: {$title}") ->setFrom($transaction->getAuthorPHID()) ->addTos($email_to) ->addCCs($email_cc)