From 892a2d1b61c4898937a347078bd70dc157d9f3f4 Mon Sep 17 00:00:00 2001 From: vrana Date: Wed, 13 Jun 2012 20:52:10 -0700 Subject: [PATCH] Make Thread-Topic human readable Summary: Some e-mail clients display this header and it needs to be constant. This is somehow involved but I doubt that there is a simpler solution. Test Plan: Applied SQL patch. Commented on revision, commented on commit, changed package. Verified that the `Thread-Topic` has constant and human readable value. Reviewers: epriestley Reviewed By: epriestley CC: ola, aran, Korvin Differential Revision: https://secure.phabricator.com/D2745 --- resources/sql/patches/threadtopic.sql | 14 ++++++++++++++ .../audit/editor/PhabricatorAuditCommentEditor.php | 13 +++++++++---- .../differential/mail/DifferentialMail.php | 5 +++-- .../differential/storage/DifferentialRevision.php | 9 +++++++++ .../editor/ManiphestTransactionEditor.php | 2 +- .../maniphest/storage/ManiphestTask.php | 9 +++++++++ src/applications/owners/mail/PackageMail.php | 4 ++-- .../owners/storage/PhabricatorOwnersPackage.php | 9 +++++++++ .../PhabricatorRepositoryCommitHeraldWorker.php | 3 ++- .../setup/sql/PhabricatorBuiltinPatchList.php | 4 ++++ 10 files changed, 62 insertions(+), 10 deletions(-) create mode 100644 resources/sql/patches/threadtopic.sql diff --git a/resources/sql/patches/threadtopic.sql b/resources/sql/patches/threadtopic.sql new file mode 100644 index 0000000000..cce2ff80fb --- /dev/null +++ b/resources/sql/patches/threadtopic.sql @@ -0,0 +1,14 @@ +ALTER TABLE {$NAMESPACE}_differential.differential_revision + ADD originalTitle varchar(255) NOT NULL AFTER title; +UPDATE {$NAMESPACE}_differential.differential_revision SET + originalTitle = title; + +ALTER TABLE {$NAMESPACE}_owners.owners_package + ADD originalName varchar(255) NOT NULL AFTER name; +UPDATE {$NAMESPACE}_owners.owners_package SET + originalName = name; + +ALTER TABLE {$NAMESPACE}_maniphest.maniphest_task + ADD originalTitle text NOT NULL AFTER title; +UPDATE {$NAMESPACE}_maniphest.maniphest_task SET + originalTitle = title; diff --git a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php index 4b06aa03be..53f76bec3f 100644 --- a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php @@ -395,7 +395,9 @@ final class PhabricatorAuditCommentEditor { $prefix = PhabricatorEnv::getEnvConfig('metamta.diffusion.subject-prefix'); - $threading = self::getMailThreading($commit->getPHID()); + $repository = id(new PhabricatorRepository()) + ->load($commit->getRepositoryID()); + $threading = self::getMailThreading($repository, $commit); list($thread_id, $thread_topic) = $threading; $body = $this->renderMailBody( @@ -452,10 +454,13 @@ final class PhabricatorAuditCommentEditor { } } - public static function getMailThreading($phid) { + public static function getMailThreading( + PhabricatorRepository $repository, + PhabricatorRepositoryCommit $commit) { + return array( - 'diffusion-audit-'.$phid, - 'Diffusion Audit '.$phid, + 'diffusion-audit-'.$commit->getPHID(), + 'Commit r'.$repository->getCallsign().$commit->getCommitIdentifier(), ); } diff --git a/src/applications/differential/mail/DifferentialMail.php b/src/applications/differential/mail/DifferentialMail.php index 7014e2743b..2d1628f023 100644 --- a/src/applications/differential/mail/DifferentialMail.php +++ b/src/applications/differential/mail/DifferentialMail.php @@ -328,8 +328,9 @@ EOTEXT; } protected function getThreadTopic() { - $phid = $this->getRevision()->getPHID(); - return "Differential Revision {$phid}"; + $id = $this->getRevision()->getID(); + $title = $this->getRevision()->getOriginalTitle(); + return "D{$id}: {$title}"; } public function setComment($comment) { diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php index 218470b656..44a19ae684 100644 --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -19,6 +19,7 @@ final class DifferentialRevision extends DifferentialDAO { protected $title; + protected $originalTitle; protected $status; protected $summary; @@ -60,6 +61,14 @@ final class DifferentialRevision extends DifferentialDAO { ) + parent::getConfiguration(); } + public function setTitle($title) { + $this->title = $title; + if (!$this->getID()) { + $this->originalTitle = $title; + } + return $this; + } + public function loadCommitPHIDs() { if (!$this->getID()) { return ($this->commits = array()); diff --git a/src/applications/maniphest/editor/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/ManiphestTransactionEditor.php index d9307b3aa2..ec6f4518e7 100644 --- a/src/applications/maniphest/editor/ManiphestTransactionEditor.php +++ b/src/applications/maniphest/editor/ManiphestTransactionEditor.php @@ -278,7 +278,7 @@ final class ManiphestTransactionEditor { ->setVarySubjectPrefix("[{$action}]") ->setFrom($transaction->getAuthorPHID()) ->setParentMessageID($this->parentMessageID) - ->addHeader('Thread-Topic', 'Maniphest Task '.$task->getPHID()) + ->addHeader('Thread-Topic', "T{$task_id}: ".$task->getOriginalTitle()) ->setThreadID($thread_id, $is_create) ->setRelatedPHID($task->getPHID()) ->setIsBulk(true) diff --git a/src/applications/maniphest/storage/ManiphestTask.php b/src/applications/maniphest/storage/ManiphestTask.php index d95084896d..cf0d88d77e 100644 --- a/src/applications/maniphest/storage/ManiphestTask.php +++ b/src/applications/maniphest/storage/ManiphestTask.php @@ -31,6 +31,7 @@ final class ManiphestTask extends ManiphestDAO { protected $subpriority; protected $title; + protected $originalTitle; protected $description; protected $originalEmailSource; protected $mailKey; @@ -107,6 +108,14 @@ final class ManiphestTask extends ManiphestDAO { return $this; } + public function setTitle($title) { + $this->title = $title; + if (!$this->getID()) { + $this->originalTitle = $title; + } + return $this; + } + public function attachAuxiliaryAttributes(array $attrs) { if ($this->auxiliaryDirty) { throw new Exception( diff --git a/src/applications/owners/mail/PackageMail.php b/src/applications/owners/mail/PackageMail.php index 6be89d2df0..700f83843e 100644 --- a/src/applications/owners/mail/PackageMail.php +++ b/src/applications/owners/mail/PackageMail.php @@ -24,7 +24,7 @@ abstract class PackageMail { protected $paths; protected $mailTo; - public function __construct($package) { + public function __construct(PhabricatorOwnersPackage $package) { $this->package = $package; } @@ -206,7 +206,7 @@ abstract class PackageMail { private function getMailThreading() { return array( 'package-'.$this->getPackage()->getPHID(), - 'package '.$this->getPackage()->getPHID(), + 'Package '.$this->getPackage()->getOriginalName(), ); } diff --git a/src/applications/owners/storage/PhabricatorOwnersPackage.php b/src/applications/owners/storage/PhabricatorOwnersPackage.php index 39f45c14c3..82e2deae3a 100644 --- a/src/applications/owners/storage/PhabricatorOwnersPackage.php +++ b/src/applications/owners/storage/PhabricatorOwnersPackage.php @@ -20,6 +20,7 @@ final class PhabricatorOwnersPackage extends PhabricatorOwnersDAO { protected $phid; protected $name; + protected $originalName; protected $auditingEnabled; protected $description; protected $primaryOwnerPHID; @@ -79,6 +80,14 @@ final class PhabricatorOwnersPackage extends PhabricatorOwnersDAO { return $this->oldAuditingEnabled; } + public function setName($name) { + $this->name = $name; + if (!$this->getID()) { + $this->originalName = $name; + } + return $this; + } + public function loadOwners() { if (!$this->getID()) { return array(); diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php index bcc0b658b7..d6f58b5a63 100644 --- a/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php +++ b/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php @@ -153,7 +153,8 @@ EOBODY; $prefix = PhabricatorEnv::getEnvConfig('metamta.diffusion.subject-prefix'); $threading = PhabricatorAuditCommentEditor::getMailThreading( - $commit->getPHID()); + $repository, + $commit); list($thread_id, $thread_topic) = $threading; $template = new PhabricatorMetaMTAMail(); diff --git a/src/infrastructure/setup/sql/PhabricatorBuiltinPatchList.php b/src/infrastructure/setup/sql/PhabricatorBuiltinPatchList.php index ac48990cf2..5c3c664595 100644 --- a/src/infrastructure/setup/sql/PhabricatorBuiltinPatchList.php +++ b/src/infrastructure/setup/sql/PhabricatorBuiltinPatchList.php @@ -887,6 +887,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList { 'type' => 'sql', 'name' => $this->getPatchPath('ldapinfo.sql'), ), + 'threadtopic.sql' => array( + 'type' => 'sql', + 'name' => $this->getPatchPath('threadtopic.sql'), + ), ); }