diff --git a/src/applications/differential/controller/DifferentialRevisionLandController.php b/src/applications/differential/controller/DifferentialRevisionLandController.php index 6bb58e5f8c..aecc5644b7 100644 --- a/src/applications/differential/controller/DifferentialRevisionLandController.php +++ b/src/applications/differential/controller/DifferentialRevisionLandController.php @@ -68,6 +68,26 @@ final class DifferentialRevisionLandController extends DifferentialController { return id(new AphrontDialogResponse())->setDialog($dialog); } + $is_disabled = $this->pushStrategy->isActionDisabled( + $viewer, + $revision, + $revision->getRepository()); + if ($is_disabled) { + if (is_string($is_disabled)) { + $explain = $is_disabled; + } else { + $explain = pht("This action is not currently enabled."); + } + $dialog = id(new AphrontDialogView()) + ->setUser($viewer) + ->setTitle(pht("Can't land revision")) + ->appendChild($explain) + ->addCancelButton('/D'.$revision_id); + + return id(new AphrontDialogResponse())->setDialog($dialog); + } + + $prompt = hsprintf('%s

%s', pht( 'This will squash and rebase revision %s, and push it to '. diff --git a/src/applications/differential/landing/DifferentialLandingActionMenuEventListener.php b/src/applications/differential/landing/DifferentialLandingActionMenuEventListener.php index fdfea16ae0..f2fb2b16c1 100644 --- a/src/applications/differential/landing/DifferentialLandingActionMenuEventListener.php +++ b/src/applications/differential/landing/DifferentialLandingActionMenuEventListener.php @@ -1,5 +1,8 @@ getValue('object'); - - $actions = null; if ($object instanceof DifferentialRevision) { - $actions = $this->renderRevisionAction($event); + $this->renderRevisionAction($event); } - - $this->addActionMenuItems($event, $actions); } private function renderRevisionAction(PhutilEvent $event) { @@ -42,13 +41,15 @@ final class DifferentialLandingActionMenuEventListener ->setAncestorClass('DifferentialLandingStrategy') ->loadObjects(); foreach ($strategies as $strategy) { - $actions = $strategy->createMenuItems( - $event->getUser(), - $revision, - $repository); - $this->addActionMenuItems($event, $actions); + $viewer = $event->getUser(); + $action = $strategy->createMenuItem($viewer, $revision, $repository); + if ($action == null) + continue; + if ($strategy->isActionDisabled($viewer, $revision, $repository)) { + $action->setDisabled(true); + } + $this->addActionMenuItems($event, $action); } } } - diff --git a/src/applications/differential/landing/DifferentialLandingStrategy.php b/src/applications/differential/landing/DifferentialLandingStrategy.php index 0c7efad5f0..aea5d83719 100644 --- a/src/applications/differential/landing/DifferentialLandingStrategy.php +++ b/src/applications/differential/landing/DifferentialLandingStrategy.php @@ -8,9 +8,9 @@ abstract class DifferentialLandingStrategy { PhabricatorRepository $repository); /** - * returns PhabricatorActionView or an array of PhabricatorActionView or null. + * returns PhabricatorActionView or null. */ - abstract function createMenuItems( + abstract function createMenuItem( PhabricatorUser $viewer, DifferentialRevision $revision, PhabricatorRepository $repository); @@ -18,14 +18,43 @@ abstract class DifferentialLandingStrategy { /** * returns PhabricatorActionView which can be attached to the revision view. */ - protected function createActionView($revision, $name, $disabled = false) { + protected function createActionView($revision, $name) { $strategy = get_class($this); $revision_id = $revision->getId(); return id(new PhabricatorActionView()) ->setRenderAsForm(true) + ->setWorkflow(true) ->setName($name) - ->setHref("/differential/revision/land/{$revision_id}/{$strategy}/") - ->setDisabled($disabled); + ->setHref("/differential/revision/land/{$revision_id}/{$strategy}/"); + } + + /** + * Check if this action should be disabled, and explain why. + * + * By default, this method checks for push permissions, and for the + * revision being Accepted. + * + * @return FALSE for "not disabled"; + * Human-readable text explaining why, if it is disabled; + */ + public function isActionDisabled( + PhabricatorUser $viewer, + DifferentialRevision $revision, + PhabricatorRepository $repository) { + + $status = $revision->getStatus(); + if ($status != ArcanistDifferentialRevisionStatus::ACCEPTED) { + return pht("Only Accepted revisions can be landed."); + } + + if (!PhabricatorPolicyFilter::hasCapability( + $viewer, + $repository, + DiffusionCapabilityPush::CAPABILITY)) { + return pht("You do not have permissions to push to this repository."); + } + + return false; } /** diff --git a/src/applications/differential/landing/DifferentialLandingToGitHub.php b/src/applications/differential/landing/DifferentialLandingToGitHub.php index 7eb19ecf92..00c5362b22 100644 --- a/src/applications/differential/landing/DifferentialLandingToGitHub.php +++ b/src/applications/differential/landing/DifferentialLandingToGitHub.php @@ -47,7 +47,7 @@ final class DifferentialLandingToGitHub /** * returns PhabricatorActionView or an array of PhabricatorActionView or null. */ - public function createMenuItems( + public function createMenuItem( PhabricatorUser $viewer, DifferentialRevision $revision, PhabricatorRepository $repository) { diff --git a/src/applications/differential/landing/DifferentialLandingToHostedGit.php b/src/applications/differential/landing/DifferentialLandingToHostedGit.php index 874b0aff2c..8438492aba 100644 --- a/src/applications/differential/landing/DifferentialLandingToHostedGit.php +++ b/src/applications/differential/landing/DifferentialLandingToHostedGit.php @@ -105,7 +105,7 @@ final class DifferentialLandingToHostedGit $workspace->execxLocal("push origin HEAD:master"); } - public function createMenuItems( + public function createMenuItem( PhabricatorUser $viewer, DifferentialRevision $revision, PhabricatorRepository $repository) { @@ -123,14 +123,8 @@ final class DifferentialLandingToHostedGit return; } - $can_push = PhabricatorPolicyFilter::hasCapability( - $viewer, - $repository, - DiffusionCapabilityPush::CAPABILITY); - return $this->createActionView( $revision, - pht('Land to Hosted Repository'), - !$can_push); + pht('Land to Hosted Repository')); } } diff --git a/src/applications/differential/landing/DifferentialLandingToHostedMercurial.php b/src/applications/differential/landing/DifferentialLandingToHostedMercurial.php index fd533aaf8b..029f821814 100644 --- a/src/applications/differential/landing/DifferentialLandingToHostedMercurial.php +++ b/src/applications/differential/landing/DifferentialLandingToHostedMercurial.php @@ -93,7 +93,7 @@ final class DifferentialLandingToHostedMercurial $workspace->execxLocal("push -b default"); } - public function createMenuItems( + public function createMenuItem( PhabricatorUser $viewer, DifferentialRevision $revision, PhabricatorRepository $repository) { @@ -107,14 +107,8 @@ final class DifferentialLandingToHostedMercurial return; } - $can_push = PhabricatorPolicyFilter::hasCapability( - $viewer, - $repository, - DiffusionCapabilityPush::CAPABILITY); - return $this->createActionView( $revision, - pht('Land to Hosted Repository'), - !$can_push); + pht('Land to Hosted Repository')); } }