diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -575,6 +575,7 @@ 'DifferentialRevisionStatus' => 'applications/differential/constants/DifferentialRevisionStatus.php', 'DifferentialRevisionStatusDatasource' => 'applications/differential/typeahead/DifferentialRevisionStatusDatasource.php', 'DifferentialRevisionStatusFunctionDatasource' => 'applications/differential/typeahead/DifferentialRevisionStatusFunctionDatasource.php', + 'DifferentialRevisionStatusTransaction' => 'applications/differential/xaction/DifferentialRevisionStatusTransaction.php', 'DifferentialRevisionSummaryHeraldField' => 'applications/differential/herald/DifferentialRevisionSummaryHeraldField.php', 'DifferentialRevisionSummaryTransaction' => 'applications/differential/xaction/DifferentialRevisionSummaryTransaction.php', 'DifferentialRevisionTestPlanTransaction' => 'applications/differential/xaction/DifferentialRevisionTestPlanTransaction.php', @@ -5571,6 +5572,7 @@ 'DifferentialRevisionStatus' => 'Phobject', 'DifferentialRevisionStatusDatasource' => 'PhabricatorTypeaheadDatasource', 'DifferentialRevisionStatusFunctionDatasource' => 'PhabricatorTypeaheadCompositeDatasource', + 'DifferentialRevisionStatusTransaction' => 'DifferentialRevisionTransactionType', 'DifferentialRevisionSummaryHeraldField' => 'DifferentialRevisionHeraldField', 'DifferentialRevisionSummaryTransaction' => 'DifferentialRevisionTransactionType', 'DifferentialRevisionTestPlanTransaction' => 'DifferentialRevisionTransactionType', diff --git a/src/applications/differential/constants/DifferentialRevisionStatus.php b/src/applications/differential/constants/DifferentialRevisionStatus.php --- a/src/applications/differential/constants/DifferentialRevisionStatus.php +++ b/src/applications/differential/constants/DifferentialRevisionStatus.php @@ -32,6 +32,14 @@ return idx($this->spec, 'color.tag', 'bluegrey'); } + public function getTimelineIcon() { + return idx($this->spec, 'icon.timeline'); + } + + public function getTimelineColor() { + return idx($this->spec, 'color.timeline'); + } + public function getANSIColor() { return idx($this->spec, 'color.ansi'); } @@ -56,6 +64,10 @@ return ($this->key === self::NEEDS_REVIEW); } + public function isNeedsRevision() { + return ($this->key === self::NEEDS_REVISION); + } + public function isPublished() { return ($this->key === self::PUBLISHED); } @@ -116,19 +128,23 @@ 'name' => pht('Needs Review'), 'legacy' => 0, 'icon' => 'fa-code', + 'icon.timeline' => 'fa-undo', 'closed' => false, 'color.icon' => 'grey', 'color.tag' => 'bluegrey', 'color.ansi' => 'magenta', + 'color.timeline' => 'orange', ), self::NEEDS_REVISION => array( 'name' => pht('Needs Revision'), 'legacy' => 1, 'icon' => 'fa-refresh', + 'icon.timeline' => 'fa-times', 'closed' => false, 'color.icon' => 'red', 'color.tag' => 'red', 'color.ansi' => 'red', + 'color.timeline' => 'red', ), self::CHANGES_PLANNED => array( 'name' => pht('Changes Planned'), @@ -143,10 +159,12 @@ 'name' => pht('Accepted'), 'legacy' => 2, 'icon' => 'fa-check', + 'icon.timeline' => 'fa-check', 'closed' => $close_on_accept, 'color.icon' => 'green', 'color.tag' => 'green', 'color.ansi' => 'green', + 'color.timeline' => 'green', ), self::PUBLISHED => array( 'name' => pht('Closed'), diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -71,7 +71,6 @@ $types[] = DifferentialTransaction::TYPE_ACTION; $types[] = DifferentialTransaction::TYPE_INLINE; - $types[] = DifferentialTransaction::TYPE_STATUS; $types[] = DifferentialTransaction::TYPE_UPDATE; return $types; @@ -606,7 +605,8 @@ if ($new_status !== null && ($new_status != $old_status)) { $xaction = id(new DifferentialTransaction()) - ->setTransactionType(DifferentialTransaction::TYPE_STATUS) + ->setTransactionType( + DifferentialRevisionStatusTransaction::TRANSACTIONTYPE) ->setOldValue($old_status) ->setNewValue($new_status); diff --git a/src/applications/differential/storage/DifferentialTransaction.php b/src/applications/differential/storage/DifferentialTransaction.php --- a/src/applications/differential/storage/DifferentialTransaction.php +++ b/src/applications/differential/storage/DifferentialTransaction.php @@ -8,7 +8,6 @@ const TYPE_INLINE = 'differential:inline'; const TYPE_UPDATE = 'differential:update'; const TYPE_ACTION = 'differential:action'; - const TYPE_STATUS = 'differential:status'; const MAILTAG_REVIEWERS = 'differential-reviewers'; const MAILTAG_CLOSED = 'differential-committed'; @@ -42,6 +41,10 @@ } } + if ($xaction_type == 'differential:status') { + return new DifferentialRevisionStatusTransaction(); + } + return parent::newFallbackModularTransactionType(); } @@ -305,15 +308,6 @@ return DifferentialAction::getBasicStoryText($new, $author_handle); } break; - case self::TYPE_STATUS: - switch ($this->getNewValue()) { - case ArcanistDifferentialRevisionStatus::ACCEPTED: - return pht('This revision is now accepted and ready to land.'); - case ArcanistDifferentialRevisionStatus::NEEDS_REVISION: - return pht('This revision now requires changes to proceed.'); - case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW: - return pht('This revision now requires review to proceed.'); - } } return parent::getTitle(); @@ -457,21 +451,6 @@ $object_link); } break; - case self::TYPE_STATUS: - switch ($this->getNewValue()) { - case ArcanistDifferentialRevisionStatus::ACCEPTED: - return pht( - '%s is now accepted and ready to land.', - $object_link); - case ArcanistDifferentialRevisionStatus::NEEDS_REVISION: - return pht( - '%s now requires changes to proceed.', - $object_link); - case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW: - return pht( - '%s now requires review to proceed.', - $object_link); - } } return parent::getTitleForFeed(); @@ -483,16 +462,6 @@ return 'fa-comment'; case self::TYPE_UPDATE: return 'fa-refresh'; - case self::TYPE_STATUS: - switch ($this->getNewValue()) { - case ArcanistDifferentialRevisionStatus::ACCEPTED: - return 'fa-check'; - case ArcanistDifferentialRevisionStatus::NEEDS_REVISION: - return 'fa-times'; - case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW: - return 'fa-undo'; - } - break; case self::TYPE_ACTION: switch ($this->getNewValue()) { case DifferentialAction::ACTION_CLOSE: @@ -530,14 +499,12 @@ // Never group status changes with other types of actions, they're indirect // and don't make sense when combined with direct actions. - $type_status = self::TYPE_STATUS; - - if ($this->getTransactionType() == $type_status) { + if ($this->isStatusTransaction($this)) { return false; } foreach ($group as $xaction) { - if ($xaction->getTransactionType() == $type_status) { + if ($this->isStatusTransaction($xaction)) { return false; } } @@ -545,21 +512,25 @@ return parent::shouldDisplayGroupWith($group); } + private function isStatusTransaction($xaction) { + $old_status = 'differential:status'; + if ($xaction->getTransactionType() == $old_status) { + return true; + } + + $new_status = DifferentialRevisionStatusTransaction::TRANSACTIONTYPE; + if ($xaction->getTransactionType() == $new_status) { + return true; + } + + return false; + } + public function getColor() { switch ($this->getTransactionType()) { case self::TYPE_UPDATE: return PhabricatorTransactions::COLOR_SKY; - case self::TYPE_STATUS: - switch ($this->getNewValue()) { - case ArcanistDifferentialRevisionStatus::ACCEPTED: - return PhabricatorTransactions::COLOR_GREEN; - case ArcanistDifferentialRevisionStatus::NEEDS_REVISION: - return PhabricatorTransactions::COLOR_RED; - case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW: - return PhabricatorTransactions::COLOR_ORANGE; - } - break; case self::TYPE_ACTION: switch ($this->getNewValue()) { case DifferentialAction::ACTION_CLOSE: diff --git a/src/applications/differential/xaction/DifferentialRevisionStatusTransaction.php b/src/applications/differential/xaction/DifferentialRevisionStatusTransaction.php new file mode 100644 --- /dev/null +++ b/src/applications/differential/xaction/DifferentialRevisionStatusTransaction.php @@ -0,0 +1,74 @@ +getStatus(); + } + + public function applyInternalEffects($object, $value) { + $object->setStatus($value); + } + + public function getTitle() { + $new = $this->getNewValue(); + $status = DifferentialRevisionStatus::newForLegacyStatus($new); + + if ($status->isAccepted()) { + return pht('This revision is now accepted and ready to land.'); + } + + if ($status->isNeedsRevision()) { + return pht('This revision now requires changes to proceed.'); + } + + if ($status->isNeedsReview()) { + return pht('This revision now requires review to proceed.'); + } + + return null; + } + + public function getTitleForFeed() { + $status = $this->newStatusObject(); + + if ($status->isAccepted()) { + return pht( + '%s is now accepted and ready to land.', + $this->renderObject()); + } + + if ($status->isNeedsRevision()) { + return pht( + '%s now requires changes to proceed.', + $this->renderObject()); + } + + if ($status->isNeedsReview()) { + return pht( + '%s now requires review to proceed.', + $this->renderObject()); + } + + return null; + } + + public function getIcon() { + $status = $this->newStatusObject(); + return $status->getTimelineIcon(); + } + + public function getColor() { + $status = $this->newStatusObject(); + return $status->getTimelineColor(); + } + + private function newStatusObject() { + $new = $this->getNewValue(); + return DifferentialRevisionStatus::newForLegacyStatus($new); + } + +}