diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 6e9b9fcd0c..bb522ae3e1 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -268,6 +268,7 @@ phutil_register_library_map(array( 'DifferentialLintStatus' => 'applications/differential/constants/DifferentialLintStatus.php', 'DifferentialLocalCommitsView' => 'applications/differential/view/DifferentialLocalCommitsView.php', 'DifferentialMail' => 'applications/differential/mail/DifferentialMail.php', + 'DifferentialMailPhase' => 'applications/differential/constants/DifferentialMailPhase.php', 'DifferentialManiphestTasksFieldSpecification' => 'applications/differential/field/specification/DifferentialManiphestTasksFieldSpecification.php', 'DifferentialNewDiffMail' => 'applications/differential/mail/DifferentialNewDiffMail.php', 'DifferentialPathFieldSpecification' => 'applications/differential/field/specification/DifferentialPathFieldSpecification.php', diff --git a/src/applications/differential/constants/DifferentialMailPhase.php b/src/applications/differential/constants/DifferentialMailPhase.php new file mode 100644 index 0000000000..b8c80b00b9 --- /dev/null +++ b/src/applications/differential/constants/DifferentialMailPhase.php @@ -0,0 +1,25 @@ +getRevision()->getStatus(); if ($status != ArcanistDifferentialRevisionStatus::NEEDS_REVISION && diff --git a/src/applications/differential/field/specification/DifferentialCommitsFieldSpecification.php b/src/applications/differential/field/specification/DifferentialCommitsFieldSpecification.php index 41958397cf..4071faba02 100644 --- a/src/applications/differential/field/specification/DifferentialCommitsFieldSpecification.php +++ b/src/applications/differential/field/specification/DifferentialCommitsFieldSpecification.php @@ -50,7 +50,7 @@ final class DifferentialCommitsFieldSpecification return $revision->getCommitPHIDs(); } - public function renderValueForMail() { + public function renderValueForMail($phase) { $revision = $this->getRevision(); if ($revision->getStatus() != ArcanistDifferentialRevisionStatus::CLOSED) { diff --git a/src/applications/differential/field/specification/DifferentialFieldSpecification.php b/src/applications/differential/field/specification/DifferentialFieldSpecification.php index 1edd7fc27f..728be2d079 100644 --- a/src/applications/differential/field/specification/DifferentialFieldSpecification.php +++ b/src/applications/differential/field/specification/DifferentialFieldSpecification.php @@ -375,11 +375,12 @@ abstract class DifferentialFieldSpecification { * Return plain text to render in e-mail messages. The text may span * multiple lines. * + * @return int One of DifferentialMailPhase constants. * @return string|null Plain text, or null for no message. * * @task mail */ - public function renderValueForMail() { + public function renderValueForMail($phase) { return null; } diff --git a/src/applications/differential/field/specification/DifferentialManiphestTasksFieldSpecification.php b/src/applications/differential/field/specification/DifferentialManiphestTasksFieldSpecification.php index b24d1a86e0..4758301a84 100644 --- a/src/applications/differential/field/specification/DifferentialManiphestTasksFieldSpecification.php +++ b/src/applications/differential/field/specification/DifferentialManiphestTasksFieldSpecification.php @@ -156,4 +156,23 @@ final class DifferentialManiphestTasksFieldSpecification return $task_phids; } + public function renderValueForMail($phase) { + if ($phase == DifferentialMailPhase::COMMENT) { + return null; + } + + if (!$this->maniphestTasks) { + return null; + } + + $handles = id(new PhabricatorObjectHandleData($this->maniphestTasks)) + ->loadHandles(); + $body = array(); + $body[] = 'MANIPHEST TASKS'; + foreach ($handles as $handle) { + $body[] = ' '.PhabricatorEnv::getProductionURI($handle->getURI()); + } + return implode("\n", $body); + } + } diff --git a/src/applications/differential/field/specification/DifferentialReviewersFieldSpecification.php b/src/applications/differential/field/specification/DifferentialReviewersFieldSpecification.php index 1117ade5cc..b624620033 100644 --- a/src/applications/differential/field/specification/DifferentialReviewersFieldSpecification.php +++ b/src/applications/differential/field/specification/DifferentialReviewersFieldSpecification.php @@ -164,4 +164,22 @@ final class DifferentialReviewersFieldSpecification return array(); } + public function renderValueForMail($phase) { + if ($phase == DifferentialMailPhase::COMMENT) { + return null; + } + + if (!$this->reviewers) { + return null; + } + + $handles = id(new PhabricatorObjectHandleData($this->reviewers)) + ->loadHandles(); + $handles = array_select_keys( + $handles, + array($this->getRevision()->getPrimaryReviewer())) + $handles; + $names = mpull($handles, 'getName'); + return 'Reviewers: '.implode(', ', $names); + } + } diff --git a/src/applications/differential/field/specification/DifferentialRevisionIDFieldSpecification.php b/src/applications/differential/field/specification/DifferentialRevisionIDFieldSpecification.php index 0e2c23e046..8cefeceb4b 100644 --- a/src/applications/differential/field/specification/DifferentialRevisionIDFieldSpecification.php +++ b/src/applications/differential/field/specification/DifferentialRevisionIDFieldSpecification.php @@ -104,7 +104,7 @@ final class DifferentialRevisionIDFieldSpecification return 'D'.$revision->getID(); } - public function renderValueForMail() { + public function renderValueForMail($phase) { $uri = PhabricatorEnv::getProductionURI('/D'.$this->id); return "REVISION DETAIL\n {$uri}"; } diff --git a/src/applications/differential/field/specification/DifferentialSummaryFieldSpecification.php b/src/applications/differential/field/specification/DifferentialSummaryFieldSpecification.php index 0dd1cdf98c..6c4ddb4687 100644 --- a/src/applications/differential/field/specification/DifferentialSummaryFieldSpecification.php +++ b/src/applications/differential/field/specification/DifferentialSummaryFieldSpecification.php @@ -74,4 +74,16 @@ final class DifferentialSummaryFieldSpecification return (string)$value; } + public function renderValueForMail($phase) { + if ($phase != DifferentialMailPhase::WELCOME) { + return null; + } + + if ($this->summary == '') { + return null; + } + + return $this->summary; + } + } diff --git a/src/applications/differential/field/specification/DifferentialTestPlanFieldSpecification.php b/src/applications/differential/field/specification/DifferentialTestPlanFieldSpecification.php index 4f3dd83479..1f1092b9e7 100644 --- a/src/applications/differential/field/specification/DifferentialTestPlanFieldSpecification.php +++ b/src/applications/differential/field/specification/DifferentialTestPlanFieldSpecification.php @@ -107,6 +107,18 @@ final class DifferentialTestPlanFieldSpecification return $value; } + public function renderValueForMail($phase) { + if ($phase != DifferentialMailPhase::WELCOME) { + return null; + } + + if ($this->plan == '') { + return null; + } + + return "TEST PLAN\n".preg_replace('/^/m', ' ', $this->plan); + } + private function isRequired() { return PhabricatorEnv::getEnvConfig('differential.require-test-plan-field'); } diff --git a/src/applications/differential/mail/DifferentialCCWelcomeMail.php b/src/applications/differential/mail/DifferentialCCWelcomeMail.php index 85776461de..2dd66816de 100644 --- a/src/applications/differential/mail/DifferentialCCWelcomeMail.php +++ b/src/applications/differential/mail/DifferentialCCWelcomeMail.php @@ -29,8 +29,6 @@ final class DifferentialCCWelcomeMail extends DifferentialReviewRequestMail { $body = array(); $body[] = "{$actor} added you to the CC list for the revision \"{$name}\"."; - $body[] = $this->renderReviewersLine(); - $body[] = null; $body[] = $this->renderReviewRequestBody(); diff --git a/src/applications/differential/mail/DifferentialCommentMail.php b/src/applications/differential/mail/DifferentialCommentMail.php index 7fec3f4973..d5821617d0 100644 --- a/src/applications/differential/mail/DifferentialCommentMail.php +++ b/src/applications/differential/mail/DifferentialCommentMail.php @@ -161,18 +161,7 @@ final class DifferentialCommentMail extends DifferentialMail { $body[] = null; } - $selector = DifferentialFieldSelector::newSelector(); - $aux_fields = $selector->sortFieldsForMail( - $selector->getFieldSpecifications()); - - foreach ($aux_fields as $field) { - $field->setRevision($this->getRevision()); - $text = $field->renderValueForMail(); - if ($text !== null) { - $body[] = $text; - $body[] = null; - } - } + $body[] = $this->renderAuxFields(DifferentialMailPhase::COMMENT); return implode("\n", $body); } diff --git a/src/applications/differential/mail/DifferentialMail.php b/src/applications/differential/mail/DifferentialMail.php index bd5721b391..fae1cda58c 100644 --- a/src/applications/differential/mail/DifferentialMail.php +++ b/src/applications/differential/mail/DifferentialMail.php @@ -354,11 +354,6 @@ EOTEXT; return $this->changesets; } - protected function getManiphestTaskPHIDs() { - return $this->getRevision()->getAttachedPHIDs( - PhabricatorPHIDConstants::PHID_TYPE_TASK); - } - public function setInlineComments(array $inline_comments) { assert_instances_of($inline_comments, 'PhabricatorInlineCommentInterface'); $this->inlineComments = $inline_comments; @@ -369,6 +364,24 @@ EOTEXT; return $this->inlineComments; } + protected function renderAuxFields($phase) { + $selector = DifferentialFieldSelector::newSelector(); + $aux_fields = $selector->sortFieldsForMail( + $selector->getFieldSpecifications()); + + $body = array(); + foreach ($aux_fields as $field) { + $field->setRevision($this->getRevision()); + $text = $field->renderValueForMail($phase); + if ($text !== null) { + $body[] = $text; + $body[] = null; + } + } + + return implode("\n", $body); + } + public function renderRevisionDetailLink() { $uri = $this->getRevisionURI(); return "REVISION DETAIL\n {$uri}"; diff --git a/src/applications/differential/mail/DifferentialNewDiffMail.php b/src/applications/differential/mail/DifferentialNewDiffMail.php index f3167caa64..cee44b1ff2 100644 --- a/src/applications/differential/mail/DifferentialNewDiffMail.php +++ b/src/applications/differential/mail/DifferentialNewDiffMail.php @@ -44,8 +44,6 @@ final class DifferentialNewDiffMail extends DifferentialReviewRequestMail { } else { $body[] = "{$actor} updated the revision \"{$name}\"."; } - $body[] = $this->renderReviewersLine(); - $body[] = null; $body[] = $this->renderReviewRequestBody(); diff --git a/src/applications/differential/mail/DifferentialReviewRequestMail.php b/src/applications/differential/mail/DifferentialReviewRequestMail.php index 878140b09b..94cf686cbf 100644 --- a/src/applications/differential/mail/DifferentialReviewRequestMail.php +++ b/src/applications/differential/mail/DifferentialReviewRequestMail.php @@ -40,47 +40,21 @@ abstract class DifferentialReviewRequestMail extends DifferentialMail { $this->setChangesets($changesets); } - protected function renderReviewersLine() { - $reviewers = $this->getRevision()->getReviewers(); - $handles = id(new PhabricatorObjectHandleData($reviewers))->loadHandles(); - return 'Reviewers: '.$this->renderHandleList($handles, $reviewers); - } - protected function renderReviewRequestBody() { $revision = $this->getRevision(); $body = array(); - if ($this->isFirstMailToRecipients()) { - if ($revision->getSummary() != '') { - $body[] = $this->formatText($revision->getSummary()); - $body[] = null; - } - - if ($revision->getTestPlan() != '') { - $body[] = 'TEST PLAN'; - $body[] = $this->formatText($revision->getTestPlan()); - $body[] = null; - } - } else { + if (!$this->isFirstMailToRecipients()) { if (strlen($this->getComments())) { $body[] = $this->formatText($this->getComments()); $body[] = null; } } - $body[] = $this->renderRevisionDetailLink(); - $body[] = null; - - $task_phids = $this->getManiphestTaskPHIDs(); - if ($task_phids) { - $handles = id(new PhabricatorObjectHandleData($task_phids)) - ->loadHandles(); - $body[] = 'MANIPHEST TASKS'; - foreach ($handles as $handle) { - $body[] = ' '.PhabricatorEnv::getProductionURI($handle->getURI()); - } - $body[] = null; - } + $phase = ($this->isFirstMailToRecipients() ? + DifferentialMailPhase::WELCOME : + DifferentialMailPhase::UPDATE); + $body[] = $this->renderAuxFields($phase); $changesets = $this->getChangesets(); if ($changesets) {