From a0fba642b382da2337f3c92371129fcb0290cecb Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 26 Oct 2015 19:58:37 +0000 Subject: [PATCH] Show the oldest non-failing revision land operation, or the newest failure Summary: Ref T182. - We just show the oldest operation right now, but we usually care about the oldest non-failure. - Only query for actual land operations when rendering the revision operations dialog (maybe eventually we'll show more stuff?). - For now, prevent multiple lands / repeated lands or queueing up lands while other lands are happening. Test Plan: Landed a revision. Tried to land it more / again. Reviewers: chad Reviewed By: chad Maniphest Tasks: T182 Differential Revision: https://secure.phabricator.com/D14338 --- ...ifferentialRevisionOperationController.php | 44 ++++++++++++++++++- .../DifferentialRevisionViewController.php | 15 ++++++- .../query/DrydockRepositoryOperationQuery.php | 13 ++++++ .../storage/DrydockRepositoryOperation.php | 4 ++ 4 files changed, 73 insertions(+), 3 deletions(-) diff --git a/src/applications/differential/controller/DifferentialRevisionOperationController.php b/src/applications/differential/controller/DifferentialRevisionOperationController.php index 6ec5b8edec..85f2a0b1b2 100644 --- a/src/applications/differential/controller/DifferentialRevisionOperationController.php +++ b/src/applications/differential/controller/DifferentialRevisionOperationController.php @@ -58,6 +58,48 @@ final class DifferentialRevisionOperationController $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.')); + } + } + if ($request->isFormPost()) { // NOTE: The operation is locked to the current active diff, so if the // revision is updated before the operation applies nothing sneaky @@ -65,8 +107,6 @@ final class DifferentialRevisionOperationController $diff = $revision->getActiveDiff(); - $op = new DrydockLandRepositoryOperation(); - $operation = DrydockRepositoryOperation::initializeNewOperation($op) ->setAuthorPHID($viewer->getPHID()) ->setObjectPHID($revision->getPHID()) diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index f6c90362df..f05b6ab97d 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -1047,6 +1047,10 @@ final class DifferentialRevisionViewController extends DifferentialController { $operations = id(new DrydockRepositoryOperationQuery()) ->setViewer($viewer) ->withObjectPHIDs(array($revision->getPHID())) + ->withOperationTypes( + array( + DrydockLandRepositoryOperation::OPCONST, + )) ->withOperationStates( array( DrydockRepositoryOperation::STATE_WAIT, @@ -1058,7 +1062,16 @@ final class DifferentialRevisionViewController extends DifferentialController { return null; } - $operation = head(msort($operations, 'getID')); + $state_fail = DrydockRepositoryOperation::STATE_FAIL; + + // We're going to show the oldest operation which hasn't failed, or the + // most recent failure if they're all failures. + $operations = msort($operations, 'getID'); + foreach ($operations as $operation) { + if ($operation->getOperationState() != $state_fail) { + break; + } + } $box_view = id(new PHUIObjectBoxView()) ->setHeaderText(pht('Active Operations')); diff --git a/src/applications/drydock/query/DrydockRepositoryOperationQuery.php b/src/applications/drydock/query/DrydockRepositoryOperationQuery.php index b72e654603..38e1b4787b 100644 --- a/src/applications/drydock/query/DrydockRepositoryOperationQuery.php +++ b/src/applications/drydock/query/DrydockRepositoryOperationQuery.php @@ -7,6 +7,7 @@ final class DrydockRepositoryOperationQuery extends DrydockQuery { private $objectPHIDs; private $repositoryPHIDs; private $operationStates; + private $operationTypes; public function withIDs(array $ids) { $this->ids = $ids; @@ -33,6 +34,11 @@ final class DrydockRepositoryOperationQuery extends DrydockQuery { return $this; } + public function withOperationTypes(array $types) { + $this->operationTypes = $types; + return $this; + } + public function newResultObject() { return new DrydockRepositoryOperation(); } @@ -139,6 +145,13 @@ final class DrydockRepositoryOperationQuery extends DrydockQuery { $this->operationStates); } + if ($this->operationTypes !== null) { + $where[] = qsprintf( + $conn, + 'operationType IN (%Ls)', + $this->operationTypes); + } + return $where; } diff --git a/src/applications/drydock/storage/DrydockRepositoryOperation.php b/src/applications/drydock/storage/DrydockRepositoryOperation.php index 6ee86bf5bb..32e6ded9f3 100644 --- a/src/applications/drydock/storage/DrydockRepositoryOperation.php +++ b/src/applications/drydock/storage/DrydockRepositoryOperation.php @@ -158,6 +158,10 @@ final class DrydockRepositoryOperation extends DrydockDAO return false; } + public function isDone() { + return ($this->getOperationState() === self::STATE_DONE); + } + public function getWorkingCopyMerges() { return $this->getImplementation()->getWorkingCopyMerges( $this);