From 615d27c8e97f80aee373298630d5423950421408 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 03 Apr 2018 15:39:13 +0000 Subject: [PATCH] Show an additional "Draft" tag on non-broadcasting revisions in a non-draft state Summary: Depends on D19284. Ref T13110. It's now possible to get a revision into a "Abandoned + But, Never Promoted From Draft" state. Show this in the header and provide the draft hint above the comment area. Also, remove `shouldBroadcast()`. The method `getShouldBroadcast()` now has the same meaning. Finally, migrate existing drafts to `shouldBroadcast = false` and default `shouldBroadcast` to `true`. If we don't do this, every older revision becomes a non-broadcasting revision because this flag was not explicitly set on revision creation before, only on promotion out of draft. Test Plan: Ran migration; abandoned draft revisions and ended up in a draft + abandoned state. Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam Maniphest Tasks: T13110 Differential Revision: https://secure.phabricator.com/D19285 --- resources/sql/autopatches/20180403.draft.01.broadcast.php | 20 ++++++++++++++++++++ src/applications/differential/controller/DifferentialRevisionViewController.php | 18 +++++++++++++++++- src/applications/differential/customfield/DifferentialDraftField.php | 2 +- src/applications/differential/editor/DifferentialTransactionEditor.php | 6 +++--- src/applications/differential/storage/DifferentialRevision.php | 15 +++++---------- 5 file(s) changed, 46 insertion(s)(+), 15 deletion(s)(-) diff --git a/resources/sql/autopatches/20180403.draft.01.broadcast.php b/resources/sql/autopatches/20180403.draft.01.broadcast.php new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20180403.draft.01.broadcast.php @@ -0,0 +1,20 @@ +establishConnection('w'); + +$drafts = $table->loadAllWhere( + 'status = %s', + DifferentialRevisionStatus::DRAFT); +foreach ($drafts as $draft) { + $properties = $draft->getProperties(); + + $properties[DifferentialRevision::PROPERTY_SHOULD_BROADCAST] = false; + + queryfx( + $conn, + 'UPDATE %T SET properties = %s WHERE id = %d', + id(new DifferentialRevision())->getTableName(), + phutil_json_encode($properties), + $draft->getID()); +} diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -525,10 +525,26 @@ $status_tag = id(new PHUITagView()) ->setName($revision->getStatusDisplayName()) ->setIcon($revision->getStatusIcon()) - ->setColor($revision->getStatusIconColor()) + ->setColor($revision->getStatusTagColor()) ->setType(PHUITagView::TYPE_SHADE); $view->addProperty(PHUIHeaderView::PROPERTY_STATUS, $status_tag); + + // If the revision is in a status other than "Draft", but not broadcasting, + // add an additional "Draft" tag to the header to make it clear that this + // revision hasn't promoted yet. + if (!$revision->getShouldBroadcast() && !$revision->isDraft()) { + $draft_status = DifferentialRevisionStatus::newForStatus( + DifferentialRevisionStatus::DRAFT); + + $draft_tag = id(new PHUITagView()) + ->setName($draft_status->getDisplayName()) + ->setIcon($draft_status->getIcon()) + ->setColor($draft_status->getTagColor()) + ->setType(PHUITagView::TYPE_SHADE); + + $view->addTag($draft_tag); + } return $view; } diff --git a/src/applications/differential/customfield/DifferentialDraftField.php b/src/applications/differential/customfield/DifferentialDraftField.php --- a/src/applications/differential/customfield/DifferentialDraftField.php +++ b/src/applications/differential/customfield/DifferentialDraftField.php @@ -101,7 +101,7 @@ public function getWarningsForDetailView() { $revision = $this->getObject(); - if (!$revision->isDraft()) { + if ($revision->getShouldBroadcast()) { return array(); } 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 @@ -487,7 +487,7 @@ PhabricatorLiskDAO $object, array $xactions) { - if (!$object->shouldBroadcast()) { + if (!$object->getShouldBroadcast()) { return false; } @@ -498,7 +498,7 @@ PhabricatorLiskDAO $object, array $xactions) { - if (!$object->shouldBroadcast()) { + if (!$object->getShouldBroadcast()) { return false; } @@ -1152,7 +1152,7 @@ // If the object is still a draft, prevent "Send me an email" and other // similar rules from acting yet. - if (!$object->shouldBroadcast()) { + if (!$object->getShouldBroadcast()) { $adapter->setForbiddenAction( HeraldMailableState::STATECONST, DifferentialHeraldStateReasons::REASON_DRAFT); diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -679,6 +679,10 @@ return $this->getStatusObject()->getIconColor(); } + public function getStatusTagColor() { + return $this->getStatusObject()->getTagColor(); + } + public function getStatusObject() { $status = $this->getStatus(); return DifferentialRevisionStatus::newForStatus($status); @@ -704,14 +708,6 @@ return $this; } - public function shouldBroadcast() { - if (!$this->isDraft()) { - return true; - } - - return false; - } - public function getHoldAsDraft() { return $this->getProperty(self::PROPERTY_DRAFT_HOLD, false); } @@ -721,7 +717,7 @@ } public function getShouldBroadcast() { - return $this->getProperty(self::PROPERTY_SHOULD_BROADCAST, false); + return $this->getProperty(self::PROPERTY_SHOULD_BROADCAST, true); } public function setShouldBroadcast($should_broadcast) { @@ -745,7 +741,6 @@ public function getRemovedLineCount() { return $this->getProperty(self::PROPERTY_LINES_REMOVED); } - public function getBuildableStatus($phid) { $buildables = $this->getProperty(self::PROPERTY_BUILDABLES); -- tangled.sh