Page MenuHomePhabricator
Files F13189495

D18410.diff
No OneTemporary
Actions

D18410.diff
View Options

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 @@
+<?php
+
+final class DifferentialRevisionStatusTransaction
+ extends DifferentialRevisionTransactionType {
+
+ const TRANSACTIONTYPE = 'differential.revision.status';
+
+ public function generateOldValue($object) {
+ return $object->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);
+ }
+
+}

File Metadata

Mime Type
text/plain
Expires
Sun, May 12, 6:07 AM (1 w, 4 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6290484
Default Alt Text
D18410.diff (11 KB)

Event Timeline

Phabricator · Built by Phacility