diff --git a/resources/sql/patches/20140514.harbormasterbuildabletransaction.sql b/resources/sql/patches/20140514.harbormasterbuildabletransaction.sql new file mode 100644 --- /dev/null +++ b/resources/sql/patches/20140514.harbormasterbuildabletransaction.sql @@ -0,0 +1,21 @@ +CREATE TABLE {$NAMESPACE}_harbormaster.harbormaster_buildabletransaction ( + id INT UNSIGNED NOT NULL PRIMARY KEY AUTO_INCREMENT, + phid VARCHAR(64) NOT NULL COLLATE utf8_bin, + authorPHID VARCHAR(64) NOT NULL COLLATE utf8_bin, + objectPHID VARCHAR(64) NOT NULL COLLATE utf8_bin, + viewPolicy VARCHAR(64) NOT NULL COLLATE utf8_bin, + editPolicy VARCHAR(64) NOT NULL COLLATE utf8_bin, + commentPHID VARCHAR(64) COLLATE utf8_bin, + commentVersion INT UNSIGNED NOT NULL, + transactionType VARCHAR(32) NOT NULL COLLATE utf8_bin, + oldValue LONGTEXT NOT NULL COLLATE utf8_bin, + newValue LONGTEXT NOT NULL COLLATE utf8_bin, + contentSource LONGTEXT NOT NULL COLLATE utf8_bin, + metadata LONGTEXT NOT NULL COLLATE utf8_bin, + dateCreated INT UNSIGNED NOT NULL, + dateModified INT UNSIGNED NOT NULL, + + UNIQUE KEY `key_phid` (phid), + KEY `key_object` (objectPHID) + +) ENGINE=InnoDB, COLLATE utf8_general_ci; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -739,6 +739,9 @@ 'HarbormasterBuildableListController' => 'applications/harbormaster/controller/HarbormasterBuildableListController.php', 'HarbormasterBuildableQuery' => 'applications/harbormaster/query/HarbormasterBuildableQuery.php', 'HarbormasterBuildableSearchEngine' => 'applications/harbormaster/query/HarbormasterBuildableSearchEngine.php', + 'HarbormasterBuildableTransaction' => 'applications/harbormaster/storage/HarbormasterBuildableTransaction.php', + 'HarbormasterBuildableTransactionEditor' => 'applications/harbormaster/editor/HarbormasterBuildableTransactionEditor.php', + 'HarbormasterBuildableTransactionQuery' => 'applications/harbormaster/query/HarbormasterBuildableTransactionQuery.php', 'HarbormasterBuildableViewController' => 'applications/harbormaster/controller/HarbormasterBuildableViewController.php', 'HarbormasterCapabilityManagePlans' => 'applications/harbormaster/capability/HarbormasterCapabilityManagePlans.php', 'HarbormasterCommandBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php', @@ -3441,6 +3444,9 @@ 'HarbormasterBuildableListController' => 'HarbormasterController', 'HarbormasterBuildableQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'HarbormasterBuildableSearchEngine' => 'PhabricatorApplicationSearchEngine', + 'HarbormasterBuildableTransaction' => 'PhabricatorApplicationTransaction', + 'HarbormasterBuildableTransactionEditor' => 'PhabricatorApplicationTransactionEditor', + 'HarbormasterBuildableTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'HarbormasterBuildableViewController' => 'HarbormasterController', 'HarbormasterCapabilityManagePlans' => 'PhabricatorPolicyCapability', 'HarbormasterCommandBuildStepImplementation' => 'HarbormasterBuildStepImplementation', diff --git a/src/applications/harbormaster/controller/HarbormasterBuildActionController.php b/src/applications/harbormaster/controller/HarbormasterBuildActionController.php --- a/src/applications/harbormaster/controller/HarbormasterBuildActionController.php +++ b/src/applications/harbormaster/controller/HarbormasterBuildActionController.php @@ -55,22 +55,19 @@ } if ($request->isDialogFormPost() && $can_issue) { - - // Issue the new build command. - id(new HarbormasterBuildCommand()) - ->setAuthorPHID($viewer->getPHID()) - ->setTargetPHID($build->getPHID()) - ->setCommand($command) - ->save(); - - // Schedule a build update. We may already have stuff in queue (in which - // case this will just no-op), but we might also be dealing with a - // stopped build, which won't restart unless we deal with this. - PhabricatorWorker::scheduleTask( - 'HarbormasterBuildWorker', - array( - 'buildID' => $build->getID() - )); + $editor = id(new HarbormasterBuildableTransactionEditor()) + ->setActor($viewer) + ->setContentSourceFromRequest($request) + ->setContinueOnNoEffect(true) + ->setContinueOnMissingFields(true); + + $xaction = id(new HarbormasterBuildableTransaction()) + ->setTransactionType(HarbormasterBuildableTransaction::TYPE_COMMAND) + ->setBuildPHID($build->getPHID()) + ->setNewValue($command); + + $buildable = $build->getBuildable()->attachBuilds(array($build)); + $editor->applyTransactions($build->getBuildable(), array($xaction)); return id(new AphrontRedirectResponse())->setURI($return_uri); } diff --git a/src/applications/harbormaster/controller/HarbormasterBuildableActionController.php b/src/applications/harbormaster/controller/HarbormasterBuildableActionController.php --- a/src/applications/harbormaster/controller/HarbormasterBuildableActionController.php +++ b/src/applications/harbormaster/controller/HarbormasterBuildableActionController.php @@ -56,19 +56,17 @@ $return_uri = $buildable->getMonogram(); if ($request->isDialogFormPost() && $issuable) { - foreach ($issuable as $build) { - id(new HarbormasterBuildCommand()) - ->setAuthorPHID($viewer->getPHID()) - ->setTargetPHID($build->getPHID()) - ->setCommand($command) - ->save(); - - PhabricatorWorker::scheduleTask( - 'HarbormasterBuildWorker', - array( - 'buildID' => $build->getID() - )); - } + $editor = id(new HarbormasterBuildableTransactionEditor()) + ->setActor($viewer) + ->setContentSourceFromRequest($request) + ->setContinueOnNoEffect(true) + ->setContinueOnMissingFields(true); + + $xaction = id(new HarbormasterBuildableTransaction()) + ->setTransactionType(HarbormasterBuildableTransaction::TYPE_COMMAND) + ->setNewValue($command); + + $editor->applyTransactions($buildable, array($xaction)); return id(new AphrontRedirectResponse())->setURI($return_uri); } diff --git a/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php b/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php --- a/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php @@ -46,6 +46,15 @@ $box = id(new PHUIObjectBoxView()) ->setHeader($header); + $xactions = id(new HarbormasterBuildableTransactionQuery()) + ->setViewer($viewer) + ->withObjectPHIDs(array($buildable->getPHID())) + ->execute(); + $timeline = id(new PhabricatorApplicationTransactionView()) + ->setUser($viewer) + ->setObjectPHID($buildable->getPHID()) + ->setTransactions($xactions); + $actions = $this->buildActionList($buildable); $this->buildPropertyLists($box, $buildable, $actions); @@ -57,6 +66,7 @@ $crumbs, $box, $build_list, + $timeline, ), array( 'title' => $title, diff --git a/src/applications/harbormaster/editor/HarbormasterBuildableTransactionEditor.php b/src/applications/harbormaster/editor/HarbormasterBuildableTransactionEditor.php new file mode 100644 --- /dev/null +++ b/src/applications/harbormaster/editor/HarbormasterBuildableTransactionEditor.php @@ -0,0 +1,125 @@ +getTransactionType()) { + case HarbormasterBuildableTransaction::TYPE_CREATE: + case HarbormasterBuildableTransaction::TYPE_COMMAND: + return null; + } + + return parent::getCustomTransactionOldValue($object, $xaction); + } + + protected function getCustomTransactionNewValue( + PhabricatorLiskDAO $object, + PhabricatorApplicationTransaction $xaction) { + + switch ($xaction->getTransactionType()) { + case HarbormasterBuildableTransaction::TYPE_CREATE: + return true; + case HarbormasterBuildableTransaction::TYPE_COMMAND: + return $xaction->getNewValue(); + } + + return parent::getCustomTransactionNewValue($object, $xaction); + } + + protected function applyCustomInternalTransaction( + PhabricatorLiskDAO $object, + PhabricatorApplicationTransaction $xaction) { + + switch ($xaction->getTransactionType()) { + case HarbormasterBuildableTransaction::TYPE_CREATE: + return; + case HarbormasterBuildableTransaction::TYPE_COMMAND: + return $this->executeBuildableCommand($object, $xaction); + } + + return parent::applyCustomInternalTransaction($object, $xaction); + } + + private function executeBuildableCommand( + HarbormasterBuildable $buildable, + HarbormasterBuildableTransaction $xaction) { + + if ($xaction->getBuildPHID() == null) { + $builds = $buildable->getBuilds(); + } else { + $phid = $xaction->getBuildPHID(); + foreach ($buildable->getBuilds() as $build) { + if ($phid == $build->getPHID()) { + $builds = array($build); // TODO better way + } + } + } + + $command = $xaction->getNewValue(); + + $issuable = array(); + foreach ($builds as $build) { + switch ($command) { + case HarbormasterBuildCommand::COMMAND_RESTART: + if ($build->canRestartBuild()) { + $issuable[] = $build; + } + break; + case HarbormasterBuildCommand::COMMAND_STOP: + if ($build->canStopBuild()) { + $issuable[] = $build; + } + break; + case HarbormasterBuildCommand::COMMAND_RESUME: + if ($build->canResumeBuild()) { + $issuable[] = $build; + } + break; + default: + throw new Exception("Unknown command $command"); + } + } + // TODO if $issuable is empty - no-effect exception. + + foreach ($issuable as $build) { + id(new HarbormasterBuildCommand()) + ->setAuthorPHID($xaction->getAuthorPHID()) + ->setTargetPHID($build->getPHID()) + ->setCommand($command) + ->save(); + + PhabricatorWorker::scheduleTask( + 'HarbormasterBuildWorker', + array( + 'buildID' => $build->getID() + )); + } + } + + protected function applyCustomExternalTransaction( + PhabricatorLiskDAO $object, + PhabricatorApplicationTransaction $xaction) { + + switch ($xaction->getTransactionType()) { + case HarbormasterBuildableTransaction::TYPE_CREATE: + case HarbormasterBuildableTransaction::TYPE_COMMAND: + return; + } + + return parent::applyCustomExternalTransaction($object, $xaction); + } + +} diff --git a/src/applications/harbormaster/engine/HarbormasterBuildEngine.php b/src/applications/harbormaster/engine/HarbormasterBuildEngine.php --- a/src/applications/harbormaster/engine/HarbormasterBuildEngine.php +++ b/src/applications/harbormaster/engine/HarbormasterBuildEngine.php @@ -408,8 +408,10 @@ // can look at the results themselves, and other users generally don't // care about the outcome. - if ($did_update && !$buildable->getIsManualBuildable()) { - + $should_publish = $did_update && + $new_status != HarbormasterBuildable::STATUS_BUILDING && + !$buildable->getIsManualBuildable(); + if ($should_publish) { $object = id(new PhabricatorObjectQuery()) ->setViewer($viewer) ->withPHIDs(array($buildable->getBuildablePHID())) diff --git a/src/applications/harbormaster/query/HarbormasterBuildableTransactionQuery.php b/src/applications/harbormaster/query/HarbormasterBuildableTransactionQuery.php new file mode 100644 --- /dev/null +++ b/src/applications/harbormaster/query/HarbormasterBuildableTransactionQuery.php @@ -0,0 +1,13 @@ +setMetadataValue('harbormaster:buildPHID', $phid); + return $this; + } + public function getBuildPHID() { + return $this->getMetadataValue('harbormaster:buildPHID'); + } + + public function getApplicationName() { + return 'harbormaster'; + } + + public function getApplicationTransactionType() { + return HarbormasterPHIDTypeBuildable::TYPECONST; + } + + public function getTitle() { + $author_phid = $this->getAuthorPHID(); + + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + switch ($this->getTransactionType()) { + case self::TYPE_CREATE: + return pht( + '%s created this buildable.', + $this->renderHandleLink($author_phid)); + case self::TYPE_COMMAND: + $target = pht('this buildable'); + if ($this->getBuildPHID()) { + $target = $this->renderHandleLink($this->getBuildPHID()); + } + switch ($new) { + case HarbormasterBuildCommand::COMMAND_RESTART: + return pht( + '%s restarted %s.', + $this->renderHandleLink($author_phid), + $target); + case HarbormasterBuildCommand::COMMAND_RESUME: + return pht( + '%s resumed %s.', + $this->renderHandleLink($author_phid), + $target); + case HarbormasterBuildCommand::COMMAND_STOP: + return pht( + '%s stopped %s.', + $this->renderHandleLink($author_phid), + $target); + } + } + return parent::getTitle(); + } + + public function getIcon() { + $author_phid = $this->getAuthorPHID(); + + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + switch ($this->getTransactionType()) { + case self::TYPE_CREATE: + return 'fa-plus'; + case self::TYPE_COMMAND: + switch ($new) { + case HarbormasterBuildCommand::COMMAND_RESTART: + return 'fa-backward'; + case HarbormasterBuildCommand::COMMAND_RESUME: + return 'fa-play'; + case HarbormasterBuildCommand::COMMAND_STOP: + return 'fa-stop'; + } + } + + return parent::getIcon(); + } + + public function getColor() { + $author_phid = $this->getAuthorPHID(); + + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + switch ($this->getTransactionType()) { + case self::TYPE_CREATE: + return 'green'; + case self::TYPE_COMMAND: + switch ($new) { + case HarbormasterBuildCommand::COMMAND_STOP: + return 'red'; + } + } + return parent::getColor(); + } + + public function getRequiredHandlePHIDs() { + $phids = array(); + if ($this->getBuildPHID()) { + $phids[] = $this->getBuildPHID(); + } + return array_merge($phids, parent::getRequiredHandlePHIDs()); + } +} diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -448,12 +448,13 @@ return true; case PhabricatorTransactions::TYPE_BUILDABLE: switch ($this->getNewValue()) { - case HarbormasterBuildable::STATUS_PASSED: - // For now, just never send mail when builds pass. We might let + case HarbormasterBuildable::STATUS_FAILED: + // For now, only ever send mail when builds fail. We might let // you customize this later, but in most cases this is probably // completely uninteresting. - return true; + return false; } + return true; } return $this->shouldHide(); @@ -465,13 +466,14 @@ return true; case PhabricatorTransactions::TYPE_BUILDABLE: switch ($this->getNewValue()) { - case HarbormasterBuildable::STATUS_PASSED: + case HarbormasterBuildable::STATUS_FAILED: // For now, don't notify on build passes either. These are pretty // high volume and annoying, with very little present value. We // might want to turn them back on in the specific case of // build successes on the current document? - return true; + return false; } + return true; } return $this->shouldHide(); @@ -645,6 +647,12 @@ case PhabricatorTransactions::TYPE_BUILDABLE: switch ($this->getNewValue()) { + case HarbormasterBuildable::STATUS_BUILDING: + return pht( + '%s started building %s.', + $this->renderHandleLink($author_phid), + $this->renderHandleLink( + $this->getMetadataValue('harbormaster:buildablePHID'))); case HarbormasterBuildable::STATUS_PASSED: return pht( '%s completed building %s.', @@ -722,6 +730,13 @@ } case PhabricatorTransactions::TYPE_BUILDABLE: switch ($this->getNewValue()) { + case HarbormasterBuildable::STATUS_BUILDING: + return pht( + '%s started building %s for %s.', + $this->renderHandleLink($author_phid), + $this->renderHandleLink( + $this->getMetadataValue('harbormaster:buildablePHID')), + $this->renderHandleLink($object_phid)); case HarbormasterBuildable::STATUS_PASSED: return pht( '%s completed building %s for %s.',