From 1406d6cdd0d57e720b06a929245d6d9bfe35376d Mon Sep 17 00:00:00 2001 From: vrana Date: Wed, 6 Jun 2012 00:45:29 -0700 Subject: [PATCH] Drive Differential e-mail message by field specification Summary: Refactoring before the actual change. Test Plan: None yet. Reviewers: epriestley Reviewed By: epriestley CC: aran, Koolvin Differential Revision: https://secure.phabricator.com/D2661 --- .../DifferentialDefaultFieldSelector.php | 21 ++++++++++ .../selector/DifferentialFieldSelector.php | 5 +++ .../DifferentialBranchFieldSpecification.php | 17 ++++++++ .../DifferentialCommitsFieldSpecification.php | 32 ++++++++++++++- .../DifferentialFieldSpecification.php | 17 ++++++++ ...fferentialRevisionIDFieldSpecification.php | 5 +++ .../mail/DifferentialCommentMail.php | 41 ++++--------------- 7 files changed, 104 insertions(+), 34 deletions(-) diff --git a/src/applications/differential/field/selector/DifferentialDefaultFieldSelector.php b/src/applications/differential/field/selector/DifferentialDefaultFieldSelector.php index b5ec522fe7..98bb0f630b 100644 --- a/src/applications/differential/field/selector/DifferentialDefaultFieldSelector.php +++ b/src/applications/differential/field/selector/DifferentialDefaultFieldSelector.php @@ -88,5 +88,26 @@ final class DifferentialDefaultFieldSelector return array_values($map); } + public function sortFieldsForMail(array $fields) { + assert_instances_of($fields, 'DifferentialFieldSpecification'); + + $map = array(); + foreach ($fields as $field) { + $map[get_class($field)] = $field; + } + + $map = array_select_keys( + $map, + array( + 'DifferentialSummaryFieldSpecification', + 'DifferentialTestPlanFieldSpecification', + 'DifferentialRevisionIDFieldSpecification', + 'DifferentialBranchFieldSpecification', + 'DifferentialCommitsFieldSpecification', + )) + $map; + + return array_values($map); + } + } diff --git a/src/applications/differential/field/selector/DifferentialFieldSelector.php b/src/applications/differential/field/selector/DifferentialFieldSelector.php index d4070e5564..523b5e829e 100644 --- a/src/applications/differential/field/selector/DifferentialFieldSelector.php +++ b/src/applications/differential/field/selector/DifferentialFieldSelector.php @@ -33,4 +33,9 @@ abstract class DifferentialFieldSelector { return $fields; } + public function sortFieldsForMail(array $fields) { + assert_instances_of($fields, 'DifferentialFieldSpecification'); + return $fields; + } + } diff --git a/src/applications/differential/field/specification/DifferentialBranchFieldSpecification.php b/src/applications/differential/field/specification/DifferentialBranchFieldSpecification.php index ec8ceabc93..af1cbe89ca 100644 --- a/src/applications/differential/field/specification/DifferentialBranchFieldSpecification.php +++ b/src/applications/differential/field/specification/DifferentialBranchFieldSpecification.php @@ -38,4 +38,21 @@ final class DifferentialBranchFieldSpecification return phutil_escape_html($branch); } + public function renderValueForMail() { + $status = $this->getRevision()->getStatus(); + + if ($status != ArcanistDifferentialRevisionStatus::NEEDS_REVISION && + $status != ArcanistDifferentialRevisionStatus::ACCEPTED) { + return null; + } + + $diff = $this->getRevision()->loadActiveDiff(); + if ($diff) { + $branch = $diff->getBranch(); + if ($branch) { + return "BRANCH\n $branch"; + } + } + } + } diff --git a/src/applications/differential/field/specification/DifferentialCommitsFieldSpecification.php b/src/applications/differential/field/specification/DifferentialCommitsFieldSpecification.php index 245bf6049c..41958397cf 100644 --- a/src/applications/differential/field/specification/DifferentialCommitsFieldSpecification.php +++ b/src/applications/differential/field/specification/DifferentialCommitsFieldSpecification.php @@ -1,7 +1,7 @@ getCommitPHIDs(); } + public function renderValueForMail() { + $revision = $this->getRevision(); + + if ($revision->getStatus() != ArcanistDifferentialRevisionStatus::CLOSED) { + return null; + } + + $phids = $revision->loadCommitPHIDs(); + if (!$phids) { + return null; + } + + $body = array(); + $handles = id(new PhabricatorObjectHandleData($phids))->loadHandles(); + if (count($handles) == 1) { + $body[] = "COMMIT"; + } else { + // This is unlikely to ever happen since we'll send this mail the + // first time we discover a commit, but it's not impossible if data + // was migrated, etc. + $body[] = "COMMITS"; + } + + foreach ($handles as $handle) { + $body[] = ' '.PhabricatorEnv::getProductionURI($handle->getURI()); + } + + return implode("\n", $body); + } + } diff --git a/src/applications/differential/field/specification/DifferentialFieldSpecification.php b/src/applications/differential/field/specification/DifferentialFieldSpecification.php index 334eab01e4..1edd7fc27f 100644 --- a/src/applications/differential/field/specification/DifferentialFieldSpecification.php +++ b/src/applications/differential/field/specification/DifferentialFieldSpecification.php @@ -28,6 +28,7 @@ * @task edit Extending the Revision Edit Interface * @task view Extending the Revision View Interface * @task list Extending the Revision List Interface + * @task mail Extending the E-mail Interface * @task conduit Extending the Conduit View Interface * @task commit Extending Commit Messages * @task load Loading Additional Data @@ -367,6 +368,22 @@ abstract class DifferentialFieldSpecification { } +/* -( Extending the E-mail Interface )------------------------------------- */ + + + /** + * Return plain text to render in e-mail messages. The text may span + * multiple lines. + * + * @return string|null Plain text, or null for no message. + * + * @task mail + */ + public function renderValueForMail() { + return null; + } + + /* -( Extending the Conduit Interface )------------------------------------ */ diff --git a/src/applications/differential/field/specification/DifferentialRevisionIDFieldSpecification.php b/src/applications/differential/field/specification/DifferentialRevisionIDFieldSpecification.php index 96e8c956ce..0e2c23e046 100644 --- a/src/applications/differential/field/specification/DifferentialRevisionIDFieldSpecification.php +++ b/src/applications/differential/field/specification/DifferentialRevisionIDFieldSpecification.php @@ -104,4 +104,9 @@ final class DifferentialRevisionIDFieldSpecification return 'D'.$revision->getID(); } + public function renderValueForMail() { + $uri = PhabricatorEnv::getProductionURI('/D'.$this->id); + return "REVISION DETAIL\n {$uri}"; + } + } diff --git a/src/applications/differential/mail/DifferentialCommentMail.php b/src/applications/differential/mail/DifferentialCommentMail.php index 478e7b635c..7fec3f4973 100644 --- a/src/applications/differential/mail/DifferentialCommentMail.php +++ b/src/applications/differential/mail/DifferentialCommentMail.php @@ -161,40 +161,15 @@ final class DifferentialCommentMail extends DifferentialMail { $body[] = null; } - $body[] = $this->renderRevisionDetailLink(); - $body[] = null; + $selector = DifferentialFieldSelector::newSelector(); + $aux_fields = $selector->sortFieldsForMail( + $selector->getFieldSpecifications()); - $revision = $this->getRevision(); - $status = $revision->getStatus(); - - if ($status == ArcanistDifferentialRevisionStatus::NEEDS_REVISION || - $status == ArcanistDifferentialRevisionStatus::ACCEPTED) { - $diff = $revision->loadActiveDiff(); - if ($diff) { - $branch = $diff->getBranch(); - if ($branch) { - $body[] = "BRANCH\n $branch"; - $body[] = null; - } - } - } - - if ($status == ArcanistDifferentialRevisionStatus::CLOSED) { - $phids = $revision->loadCommitPHIDs(); - if ($phids) { - $handles = id(new PhabricatorObjectHandleData($phids))->loadHandles(); - if (count($handles) == 1) { - $body[] = "COMMIT"; - } else { - // This is unlikely to ever happen since we'll send this mail the - // first time we discover a commit, but it's not impossible if data - // was migrated, etc. - $body[] = "COMMITS"; - } - - foreach ($handles as $handle) { - $body[] = ' '.PhabricatorEnv::getProductionURI($handle->getURI()); - } + foreach ($aux_fields as $field) { + $field->setRevision($this->getRevision()); + $text = $field->renderValueForMail(); + if ($text !== null) { + $body[] = $text; $body[] = null; } }