From c7af663523e556ca08a6b6df30e6fd0932f8dd46 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 18 Sep 2017 14:14:52 -0700 Subject: [PATCH] Align most revision actions to the new "Draft" state Summary: Ref T2543. Most actions are not available for drafts. Authors can "Request Review" (move out of draft to become a normal revision) or "Abandon". Non-authors can't do anything (maybe we'll let them do something later -- like "Commandeer"? -- if there's a good reason). Test Plan: Viewed a draft revision as an author and non-author, saw fewer actions available. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T2543 Differential Revision: https://secure.phabricator.com/D18626 --- .../xaction/DifferentialRevisionAbandonTransaction.php | 3 ++- .../xaction/DifferentialRevisionAcceptTransaction.php | 3 ++- .../xaction/DifferentialRevisionActionTransaction.php | 5 +++-- .../xaction/DifferentialRevisionCloseTransaction.php | 3 ++- .../DifferentialRevisionCommandeerTransaction.php | 8 +++++++- .../DifferentialRevisionPlanChangesTransaction.php | 8 +++++++- .../xaction/DifferentialRevisionReclaimTransaction.php | 3 ++- .../xaction/DifferentialRevisionRejectTransaction.php | 8 +++++++- .../xaction/DifferentialRevisionReopenTransaction.php | 3 ++- .../DifferentialRevisionRequestReviewTransaction.php | 9 +++++++-- .../xaction/DifferentialRevisionResignTransaction.php | 8 +++++++- 11 files changed, 48 insertions(+), 13 deletions(-) diff --git a/src/applications/differential/xaction/DifferentialRevisionAbandonTransaction.php b/src/applications/differential/xaction/DifferentialRevisionAbandonTransaction.php index b7b0e7f123..6232594aae 100644 --- a/src/applications/differential/xaction/DifferentialRevisionAbandonTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionAbandonTransaction.php @@ -10,7 +10,8 @@ final class DifferentialRevisionAbandonTransaction return pht('Abandon Revision'); } - protected function getRevisionActionDescription() { + protected function getRevisionActionDescription( + DifferentialRevision $revision) { return pht('This revision will be abandoned and closed.'); } diff --git a/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php b/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php index f01ce4b487..d52e2dbeb7 100644 --- a/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php @@ -10,7 +10,8 @@ final class DifferentialRevisionAcceptTransaction return pht("Accept Revision \xE2\x9C\x94"); } - protected function getRevisionActionDescription() { + protected function getRevisionActionDescription( + DifferentialRevision $revision) { return pht('These changes will be approved.'); } diff --git a/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php b/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php index ba769c2be5..a1e66379f7 100644 --- a/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php @@ -52,7 +52,8 @@ abstract class DifferentialRevisionActionTransaction return DifferentialRevisionEditEngine::ACTIONGROUP_REVISION; } - protected function getRevisionActionDescription() { + protected function getRevisionActionDescription( + DifferentialRevision $revision) { return null; } @@ -103,7 +104,7 @@ abstract class DifferentialRevisionActionTransaction if ($label !== null) { $field->setCommentActionLabel($label); - $description = $this->getRevisionActionDescription(); + $description = $this->getRevisionActionDescription($revision); $field->setActionDescription($description); $group_key = $this->getRevisionActionGroupKey(); diff --git a/src/applications/differential/xaction/DifferentialRevisionCloseTransaction.php b/src/applications/differential/xaction/DifferentialRevisionCloseTransaction.php index d71b30950a..b1c796dca4 100644 --- a/src/applications/differential/xaction/DifferentialRevisionCloseTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionCloseTransaction.php @@ -10,7 +10,8 @@ final class DifferentialRevisionCloseTransaction return pht('Close Revision'); } - protected function getRevisionActionDescription() { + protected function getRevisionActionDescription( + DifferentialRevision $revision) { return pht('This revision will be closed.'); } diff --git a/src/applications/differential/xaction/DifferentialRevisionCommandeerTransaction.php b/src/applications/differential/xaction/DifferentialRevisionCommandeerTransaction.php index 34cedcbb73..4169c3e4dc 100644 --- a/src/applications/differential/xaction/DifferentialRevisionCommandeerTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionCommandeerTransaction.php @@ -10,7 +10,8 @@ final class DifferentialRevisionCommandeerTransaction return pht('Commandeer Revision'); } - protected function getRevisionActionDescription() { + protected function getRevisionActionDescription( + DifferentialRevision $revision) { return pht('You will take control of this revision and become its author.'); } @@ -65,6 +66,11 @@ final class DifferentialRevisionCommandeerTransaction 'been closed. You can only commandeer open revisions.')); } + if ($object->isDraft()) { + throw new Exception( + pht('You can not commandeer a draft revision.')); + } + if ($this->isViewerRevisionAuthor($object, $viewer)) { throw new Exception( pht( diff --git a/src/applications/differential/xaction/DifferentialRevisionPlanChangesTransaction.php b/src/applications/differential/xaction/DifferentialRevisionPlanChangesTransaction.php index 6e584ceb36..231adc5bf8 100644 --- a/src/applications/differential/xaction/DifferentialRevisionPlanChangesTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionPlanChangesTransaction.php @@ -10,7 +10,8 @@ final class DifferentialRevisionPlanChangesTransaction return pht('Plan Changes'); } - protected function getRevisionActionDescription() { + protected function getRevisionActionDescription( + DifferentialRevision $revision) { return pht( 'This revision will be removed from review queues until it is revised.'); } @@ -55,6 +56,11 @@ final class DifferentialRevisionPlanChangesTransaction } protected function validateAction($object, PhabricatorUser $viewer) { + if ($object->isDraft()) { + throw new Exception( + pht('You can not plan changes to a draft revision.')); + } + if ($object->isChangePlanned()) { throw new Exception( pht( diff --git a/src/applications/differential/xaction/DifferentialRevisionReclaimTransaction.php b/src/applications/differential/xaction/DifferentialRevisionReclaimTransaction.php index 4767924445..4a4744e2e0 100644 --- a/src/applications/differential/xaction/DifferentialRevisionReclaimTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionReclaimTransaction.php @@ -10,7 +10,8 @@ final class DifferentialRevisionReclaimTransaction return pht('Reclaim Revision'); } - protected function getRevisionActionDescription() { + protected function getRevisionActionDescription( + DifferentialRevision $revision) { return pht('This revision will be reclaimed and reopened.'); } diff --git a/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php b/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php index b4c71209aa..a15ca0641b 100644 --- a/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php @@ -10,7 +10,8 @@ final class DifferentialRevisionRejectTransaction return pht("Request Changes \xE2\x9C\x98"); } - protected function getRevisionActionDescription() { + protected function getRevisionActionDescription( + DifferentialRevision $revision) { return pht('This revision will be returned to the author for updates.'); } @@ -72,6 +73,11 @@ final class DifferentialRevisionRejectTransaction 'not own.')); } + if ($object->isDraft()) { + throw new Exception( + pht('You can not request changes to a draft revision.')); + } + if ($this->isViewerFullyRejected($object, $viewer)) { throw new Exception( pht( diff --git a/src/applications/differential/xaction/DifferentialRevisionReopenTransaction.php b/src/applications/differential/xaction/DifferentialRevisionReopenTransaction.php index 1d28433429..4fc9f3a6f1 100644 --- a/src/applications/differential/xaction/DifferentialRevisionReopenTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionReopenTransaction.php @@ -10,7 +10,8 @@ final class DifferentialRevisionReopenTransaction return pht('Reopen Revision'); } - protected function getRevisionActionDescription() { + protected function getRevisionActionDescription( + DifferentialRevision $revision) { return pht('This revision will be reopened for review.'); } diff --git a/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php b/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php index d102b0c0c2..465b5234d5 100644 --- a/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php @@ -10,8 +10,13 @@ final class DifferentialRevisionRequestReviewTransaction return pht('Request Review'); } - protected function getRevisionActionDescription() { - return pht('This revision will be returned to reviewers for feedback.'); + protected function getRevisionActionDescription( + DifferentialRevision $revision) { + if ($revision->isDraft()) { + return pht('This revision will be submitted to reviewers for feedback.'); + } else { + return pht('This revision will be returned to reviewers for feedback.'); + } } public function getColor() { diff --git a/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php b/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php index c7805ad35f..40a512202d 100644 --- a/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php @@ -10,7 +10,8 @@ final class DifferentialRevisionResignTransaction return pht('Resign as Reviewer'); } - protected function getRevisionActionDescription() { + protected function getRevisionActionDescription( + DifferentialRevision $revision) { return pht('You will resign as a reviewer for this change.'); } @@ -63,6 +64,11 @@ final class DifferentialRevisionResignTransaction 'been closed. You can only resign from open revisions.')); } + if ($object->isDraft()) { + throw new Exception( + pht('You can not resign from a draft revision.')); + } + $resigned = DifferentialReviewerStatus::STATUS_RESIGNED; if ($this->getViewerReviewerStatus($object, $viewer) == $resigned) { throw new Exception(