Page MenuHomePhabricator
Files F14841632

D14350.id.diff
No OneTemporary
Actions

D14350.id.diff
View Options

diff --git a/src/applications/differential/controller/DifferentialRevisionOperationController.php b/src/applications/differential/controller/DifferentialRevisionOperationController.php
--- a/src/applications/differential/controller/DifferentialRevisionOperationController.php
+++ b/src/applications/differential/controller/DifferentialRevisionOperationController.php
@@ -18,86 +18,13 @@
$detail_uri = "/D{$id}";
- $repository = $revision->getRepository();
- if (!$repository) {
- return $this->rejectOperation(
- $revision,
- pht('No Repository'),
- pht(
- 'This revision is not associated with a known repository. Only '.
- 'revisions associated with a tracked repository can be landed '.
- 'automatically.'));
- }
-
- if (!$repository->canPerformAutomation()) {
- return $this->rejectOperation(
- $revision,
- pht('No Repository Automation'),
- pht(
- 'The repository this revision is associated with ("%s") is not '.
- 'configured to support automation. Configure automation for the '.
- 'repository to enable revisions to be landed automatically.',
- $repository->getMonogram()));
- }
-
- // TODO: At some point we should allow installs to give "land reviewed
- // code" permission to more users than "push any commit", because it is
- // a much less powerful operation. For now, just require push so this
- // doesn't do anything users can't do on their own.
- $can_push = PhabricatorPolicyFilter::hasCapability(
- $viewer,
- $repository,
- DiffusionPushCapability::CAPABILITY);
- if (!$can_push) {
- return $this->rejectOperation(
- $revision,
- pht('Unable to Push'),
- pht(
- 'You do not have permission to push to the repository this '.
- 'revision is associated with ("%s"), so you can not land it.',
- $repository->getMonogram()));
- }
-
$op = new DrydockLandRepositoryOperation();
-
- // Check for other operations. Eventually this should probably be more
- // general (e.g., it's OK to land to multiple different branches
- // simultaneously) but just put this in as a sanity check for now.
- $other_operations = id(new DrydockRepositoryOperationQuery())
- ->setViewer($viewer)
- ->withObjectPHIDs(array($revision->getPHID()))
- ->withOperationTypes(
- array(
- $op->getOperationConstant(),
- ))
- ->withOperationStates(
- array(
- DrydockRepositoryOperation::STATE_WAIT,
- DrydockRepositoryOperation::STATE_WORK,
- DrydockRepositoryOperation::STATE_DONE,
- ))
- ->execute();
-
- if ($other_operations) {
- $any_done = false;
- foreach ($other_operations as $operation) {
- if ($operation->isDone()) {
- $any_done = true;
- break;
- }
- }
-
- if ($any_done) {
- return $this->rejectOperation(
- $revision,
- pht('Already Complete'),
- pht('This revision has already landed.'));
- } else {
- return $this->rejectOperation(
- $revision,
- pht('Already In Flight'),
- pht('This revision is already landing.'));
- }
+ $barrier = $op->getBarrierToLanding($viewer, $revision);
+ if ($barrier) {
+ return $this->newDialog()
+ ->setTitle($barrier['title'])
+ ->appendParagraph($barrier['body'])
+ ->addCancelButton($detail_uri);
}
if ($request->isFormPost()) {
@@ -106,6 +33,7 @@
// occurs.
$diff = $revision->getActiveDiff();
+ $repository = $revision->getRepository();
$operation = DrydockRepositoryOperation::initializeNewOperation($op)
->setAuthorPHID($viewer->getPHID())
@@ -136,18 +64,4 @@
->addSubmitButton(pht('Mutate Repository Unpredictably'));
}
- private function rejectOperation(
- DifferentialRevision $revision,
- $title,
- $body) {
-
- $id = $revision->getID();
- $detail_uri = "/D{$id}";
-
- return $this->newDialog()
- ->setTitle($title)
- ->appendParagraph($body)
- ->addCancelButton($detail_uri);
- }
-
}
diff --git a/src/applications/differential/landing/DifferentialLandingActionMenuEventListener.php b/src/applications/differential/landing/DifferentialLandingActionMenuEventListener.php
--- a/src/applications/differential/landing/DifferentialLandingActionMenuEventListener.php
+++ b/src/applications/differential/landing/DifferentialLandingActionMenuEventListener.php
@@ -26,7 +26,9 @@
}
private function renderRevisionAction(PhutilEvent $event) {
- if (!$this->canUseApplication($event->getUser())) {
+ $viewer = $event->getUser();
+
+ if (!$this->canUseApplication($viewer)) {
return null;
}
@@ -40,11 +42,22 @@
if ($repository->canPerformAutomation()) {
$revision_id = $revision->getID();
+ $op = new DrydockLandRepositoryOperation();
+ $barrier = $op->getBarrierToLanding($viewer, $revision);
+
+ if ($barrier) {
+ $can_land = false;
+ } else {
+ $can_land = true;
+ }
+
$action = id(new PhabricatorActionView())
- ->setWorkflow(true)
->setName(pht('Land Revision'))
->setIcon('fa-fighter-jet')
- ->setHref("/differential/revision/operation/{$revision_id}/");
+ ->setHref("/differential/revision/operation/{$revision_id}/")
+ ->setWorkflow(true)
+ ->setDisabled(!$can_land);
+
$this->addActionMenuItems($event, $action);
}
@@ -54,7 +67,6 @@
->execute();
foreach ($strategies as $strategy) {
- $viewer = $event->getUser();
$action = $strategy->createMenuItem($viewer, $revision, $repository);
if ($action == null) {
continue;
diff --git a/src/applications/drydock/operation/DrydockLandRepositoryOperation.php b/src/applications/drydock/operation/DrydockLandRepositoryOperation.php
--- a/src/applications/drydock/operation/DrydockLandRepositoryOperation.php
+++ b/src/applications/drydock/operation/DrydockLandRepositoryOperation.php
@@ -189,4 +189,101 @@
return $diff;
}
+ public function getBarrierToLanding(
+ PhabricatorUser $viewer,
+ DifferentialRevision $revision) {
+
+ $repository = $revision->getRepository();
+ if (!$repository) {
+ return array(
+ 'title' => pht('No Repository'),
+ 'body' => pht(
+ 'This revision is not associated with a known repository. Only '.
+ 'revisions associated with a tracked repository can be landed '.
+ 'automatically.'),
+ );
+ }
+
+ if (!$repository->canPerformAutomation()) {
+ return array(
+ 'title' => pht('No Repository Automation'),
+ 'body' => pht(
+ 'The repository this revision is associated with ("%s") is not '.
+ 'configured to support automation. Configure automation for the '.
+ 'repository to enable revisions to be landed automatically.',
+ $repository->getMonogram()),
+ );
+ }
+
+ // TODO: At some point we should allow installs to give "land reviewed
+ // code" permission to more users than "push any commit", because it is
+ // a much less powerful operation. For now, just require push so this
+ // doesn't do anything users can't do on their own.
+ $can_push = PhabricatorPolicyFilter::hasCapability(
+ $viewer,
+ $repository,
+ DiffusionPushCapability::CAPABILITY);
+ if (!$can_push) {
+ return array(
+ 'title' => pht('Unable to Push'),
+ 'body' => pht(
+ 'You do not have permission to push to the repository this '.
+ 'revision is associated with ("%s"), so you can not land it.',
+ $repository->getMonogram()),
+ );
+ }
+
+ $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED;
+ if ($revision->getStatus() !== $status_accepted) {
+ return array(
+ 'title' => pht('Revision Not Accepted'),
+ 'body' => pht(
+ 'This revision is still under review. Only revisions which have '.
+ 'been accepted may land.'),
+ );
+ }
+
+ // Check for other operations. Eventually this should probably be more
+ // general (e.g., it's OK to land to multiple different branches
+ // simultaneously) but just put this in as a sanity check for now.
+ $other_operations = id(new DrydockRepositoryOperationQuery())
+ ->setViewer($viewer)
+ ->withObjectPHIDs(array($revision->getPHID()))
+ ->withOperationTypes(
+ array(
+ $this->getOperationConstant(),
+ ))
+ ->withOperationStates(
+ array(
+ DrydockRepositoryOperation::STATE_WAIT,
+ DrydockRepositoryOperation::STATE_WORK,
+ DrydockRepositoryOperation::STATE_DONE,
+ ))
+ ->execute();
+
+ if ($other_operations) {
+ $any_done = false;
+ foreach ($other_operations as $operation) {
+ if ($operation->isDone()) {
+ $any_done = true;
+ break;
+ }
+ }
+
+ if ($any_done) {
+ return array(
+ 'title' => pht('Already Complete'),
+ 'body' => pht('This revision has already landed.'),
+ );
+ } else {
+ return array(
+ 'title' => pht('Already In Flight'),
+ 'body' => pht('This revision is already landing.'),
+ );
+ }
+ }
+
+ return null;
+ }
+
}

File Metadata

Mime Type
text/plain
Expires
Sun, Feb 2, 5:22 PM (13 h, 42 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7084852
Default Alt Text
D14350.id.diff (9 KB)

Event Timeline

Phabricator · Built by Phacility