From a90ab7f4033defd9dd3a364339c3737c7ca288ec Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 28 Dec 2016 13:05:04 -0800 Subject: [PATCH] Restore "Close" and "Reopen" actions to Differential on EditEngine Summary: Ref T11114. This restores these actions as selectable in the comment area. This does not implement one special rule ("Closing a revision in response to a commit is OK from any status.") but I have a note about that separately. Test Plan: Closed and reopened revisions. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11114 Differential Revision: https://secure.phabricator.com/D17108 --- src/__phutil_library_map__.php | 4 + ...DifferentialRevisionAbandonTransaction.php | 2 +- .../DifferentialRevisionCloseTransaction.php | 78 +++++++++++++++++++ .../DifferentialRevisionReopenTransaction.php | 68 ++++++++++++++++ 4 files changed, 151 insertions(+), 1 deletion(-) create mode 100644 src/applications/differential/xaction/DifferentialRevisionCloseTransaction.php create mode 100644 src/applications/differential/xaction/DifferentialRevisionReopenTransaction.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 6e779b2a7d..6929ce05e3 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -511,6 +511,7 @@ phutil_register_library_map(array( 'DifferentialRevisionAuthorHeraldField' => 'applications/differential/herald/DifferentialRevisionAuthorHeraldField.php', 'DifferentialRevisionAuthorProjectsHeraldField' => 'applications/differential/herald/DifferentialRevisionAuthorProjectsHeraldField.php', 'DifferentialRevisionCloseDetailsController' => 'applications/differential/controller/DifferentialRevisionCloseDetailsController.php', + 'DifferentialRevisionCloseTransaction' => 'applications/differential/xaction/DifferentialRevisionCloseTransaction.php', 'DifferentialRevisionContentAddedHeraldField' => 'applications/differential/herald/DifferentialRevisionContentAddedHeraldField.php', 'DifferentialRevisionContentHeraldField' => 'applications/differential/herald/DifferentialRevisionContentHeraldField.php', 'DifferentialRevisionContentRemovedHeraldField' => 'applications/differential/herald/DifferentialRevisionContentRemovedHeraldField.php', @@ -544,6 +545,7 @@ phutil_register_library_map(array( 'DifferentialRevisionReclaimTransaction' => 'applications/differential/xaction/DifferentialRevisionReclaimTransaction.php', 'DifferentialRevisionRelationship' => 'applications/differential/relationships/DifferentialRevisionRelationship.php', 'DifferentialRevisionRelationshipSource' => 'applications/search/relationship/DifferentialRevisionRelationshipSource.php', + 'DifferentialRevisionReopenTransaction' => 'applications/differential/xaction/DifferentialRevisionReopenTransaction.php', 'DifferentialRevisionRepositoryHeraldField' => 'applications/differential/herald/DifferentialRevisionRepositoryHeraldField.php', 'DifferentialRevisionRepositoryProjectsHeraldField' => 'applications/differential/herald/DifferentialRevisionRepositoryProjectsHeraldField.php', 'DifferentialRevisionRepositoryTransaction' => 'applications/differential/xaction/DifferentialRevisionRepositoryTransaction.php', @@ -5180,6 +5182,7 @@ phutil_register_library_map(array( 'DifferentialRevisionAuthorHeraldField' => 'DifferentialRevisionHeraldField', 'DifferentialRevisionAuthorProjectsHeraldField' => 'DifferentialRevisionHeraldField', 'DifferentialRevisionCloseDetailsController' => 'DifferentialController', + 'DifferentialRevisionCloseTransaction' => 'DifferentialRevisionActionTransaction', 'DifferentialRevisionContentAddedHeraldField' => 'DifferentialRevisionHeraldField', 'DifferentialRevisionContentHeraldField' => 'DifferentialRevisionHeraldField', 'DifferentialRevisionContentRemovedHeraldField' => 'DifferentialRevisionHeraldField', @@ -5213,6 +5216,7 @@ phutil_register_library_map(array( 'DifferentialRevisionReclaimTransaction' => 'DifferentialRevisionActionTransaction', 'DifferentialRevisionRelationship' => 'PhabricatorObjectRelationship', 'DifferentialRevisionRelationshipSource' => 'PhabricatorObjectRelationshipSource', + 'DifferentialRevisionReopenTransaction' => 'DifferentialRevisionActionTransaction', 'DifferentialRevisionRepositoryHeraldField' => 'DifferentialRevisionHeraldField', 'DifferentialRevisionRepositoryProjectsHeraldField' => 'DifferentialRevisionHeraldField', 'DifferentialRevisionRepositoryTransaction' => 'DifferentialRevisionTransactionType', diff --git a/src/applications/differential/xaction/DifferentialRevisionAbandonTransaction.php b/src/applications/differential/xaction/DifferentialRevisionAbandonTransaction.php index 5033ecaf50..473a47f514 100644 --- a/src/applications/differential/xaction/DifferentialRevisionAbandonTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionAbandonTransaction.php @@ -45,7 +45,7 @@ final class DifferentialRevisionAbandonTransaction pht( 'You can not abandon this revision because you are not the '. 'author. You can only abandon revisions you own. You can change '. - 'this behavior by adjusting the "%s" setting in Config', + 'this behavior by adjusting the "%s" setting in Config.', $config_key)); } } diff --git a/src/applications/differential/xaction/DifferentialRevisionCloseTransaction.php b/src/applications/differential/xaction/DifferentialRevisionCloseTransaction.php new file mode 100644 index 0000000000..ebbd0069a1 --- /dev/null +++ b/src/applications/differential/xaction/DifferentialRevisionCloseTransaction.php @@ -0,0 +1,78 @@ +isClosed(); + } + + public function applyInternalEffects($object, $value) { + $status_closed = ArcanistDifferentialRevisionStatus::CLOSED; + $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; + + $old_status = $object->getStatus(); + + $object->setStatus($status_closed); + + $was_accepted = ($old_status == $status_accepted); + + $object->setProperty( + DifferentialRevision::PROPERTY_CLOSED_FROM_ACCEPTED, + $was_accepted); + } + + protected function validateAction($object, PhabricatorUser $viewer) { + if ($object->isClosed()) { + throw new Exception( + pht( + 'You can not close this revision because it has already been '. + 'closed. Only open revisions can be closed.')); + } + + $config_key = 'differential.always-allow-close'; + if (!PhabricatorEnv::getEnvConfig($config_key)) { + if (!$this->isViewerRevisionAuthor($object, $viewer)) { + throw new Exception( + pht( + 'You can not close this revision because you are not the '. + 'author. You can only close revisions you own. You can change '. + 'this behavior by adjusting the "%s" setting in Config.', + $config_key)); + } + } + } + + public function getTitle() { + return pht( + '%s closed this revision.', + $this->renderAuthor()); + } + + public function getTitleForFeed() { + return pht( + '%s closed %s.', + $this->renderAuthor(), + $this->renderObject()); + } + +} diff --git a/src/applications/differential/xaction/DifferentialRevisionReopenTransaction.php b/src/applications/differential/xaction/DifferentialRevisionReopenTransaction.php new file mode 100644 index 0000000000..f1e77bbb48 --- /dev/null +++ b/src/applications/differential/xaction/DifferentialRevisionReopenTransaction.php @@ -0,0 +1,68 @@ +isClosed(); + } + + public function applyInternalEffects($object, $value) { + $object->setStatus(ArcanistDifferentialRevisionStatus::NEEDS_REVIEW); + } + + protected function validateAction($object, PhabricatorUser $viewer) { + // Note that we're testing for "Closed", exactly, not just any closed + // status. + $status_closed = ArcanistDifferentialRevisionStatus::CLOSED; + if ($object->getStatus() != $status_closed) { + throw new Exception( + pht( + 'You can not reopen this revision because it is not closed. '. + 'Only closed revisions can be reopened.')); + } + + $config_key = 'differential.allow-reopen'; + if (!PhabricatorEnv::getEnvConfig($config_key)) { + throw new Exception( + pht( + 'You can not reopen this revision because configuration prevents '. + 'any revision from being reopened. You can change this behavior '. + 'by adjusting the "%s" setting in Config.', + $config_key)); + } + } + + public function getTitle() { + return pht( + '%s reopened this revision.', + $this->renderAuthor()); + } + + public function getTitleForFeed() { + return pht( + '%s reopened %s.', + $this->renderAuthor(), + $this->renderObject()); + } + +}