diff --git a/resources/sql/autopatches/20140514.harbormasterbuildabletransaction.sql b/resources/sql/autopatches/20140514.harbormasterbuildabletransaction.sql new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20140514.harbormasterbuildabletransaction.sql @@ -0,0 +1,43 @@ +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; + +CREATE TABLE {$NAMESPACE}_harbormaster.harbormaster_buildtransaction ( + 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 @@ -738,6 +738,9 @@ 'HarbormasterBuildStepTransactionQuery' => 'applications/harbormaster/query/HarbormasterBuildStepTransactionQuery.php', 'HarbormasterBuildTarget' => 'applications/harbormaster/storage/build/HarbormasterBuildTarget.php', 'HarbormasterBuildTargetQuery' => 'applications/harbormaster/query/HarbormasterBuildTargetQuery.php', + 'HarbormasterBuildTransaction' => 'applications/harbormaster/storage/HarbormasterBuildTransaction.php', + 'HarbormasterBuildTransactionEditor' => 'applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php', + 'HarbormasterBuildTransactionQuery' => 'applications/harbormaster/query/HarbormasterBuildTransactionQuery.php', 'HarbormasterBuildViewController' => 'applications/harbormaster/controller/HarbormasterBuildViewController.php', 'HarbormasterBuildWorker' => 'applications/harbormaster/worker/HarbormasterBuildWorker.php', 'HarbormasterBuildable' => 'applications/harbormaster/storage/HarbormasterBuildable.php', @@ -746,6 +749,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', @@ -3427,6 +3433,9 @@ 1 => 'PhabricatorPolicyInterface', ), 'HarbormasterBuildTargetQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', + 'HarbormasterBuildTransaction' => 'PhabricatorApplicationTransaction', + 'HarbormasterBuildTransactionEditor' => 'PhabricatorApplicationTransactionEditor', + 'HarbormasterBuildTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'HarbormasterBuildViewController' => 'HarbormasterController', 'HarbormasterBuildWorker' => 'HarbormasterWorker', 'HarbormasterBuildable' => @@ -3439,6 +3448,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,17 @@ } if ($request->isDialogFormPost() && $can_issue) { + $editor = id(new HarbormasterBuildTransactionEditor()) + ->setActor($viewer) + ->setContentSourceFromRequest($request) + ->setContinueOnNoEffect(true) + ->setContinueOnMissingFields(true); - // 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() - )); + $xaction = id(new HarbormasterBuildTransaction()) + ->setTransactionType(HarbormasterBuildTransaction::TYPE_COMMAND) + ->setNewValue($command); + + $editor->applyTransactions($build, array($xaction)); return id(new AphrontRedirectResponse())->setURI($return_uri); } diff --git a/src/applications/harbormaster/controller/HarbormasterBuildViewController.php b/src/applications/harbormaster/controller/HarbormasterBuildViewController.php --- a/src/applications/harbormaster/controller/HarbormasterBuildViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterBuildViewController.php @@ -103,11 +103,21 @@ $targets[] = $this->buildLog($build, $build_target); } + $xactions = id(new HarbormasterBuildTransactionQuery()) + ->setViewer($viewer) + ->withObjectPHIDs(array($build->getPHID())) + ->execute(); + $timeline = id(new PhabricatorApplicationTransactionView()) + ->setUser($viewer) + ->setObjectPHID($build->getPHID()) + ->setTransactions($xactions); + return $this->buildApplicationPage( array( $crumbs, $box, - $targets + $targets, + $timeline, ), array( 'title' => $title, 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,18 +56,29 @@ $return_uri = $buildable->getMonogram(); if ($request->isDialogFormPost() && $issuable) { + $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)); + + $build_editor = id(new HarbormasterBuildTransactionEditor()) + ->setActor($viewer) + ->setContentSourceFromRequest($request) + ->setContinueOnNoEffect(true) + ->setContinueOnMissingFields(true); + foreach ($issuable as $build) { - id(new HarbormasterBuildCommand()) - ->setAuthorPHID($viewer->getPHID()) - ->setTargetPHID($build->getPHID()) - ->setCommand($command) - ->save(); - - PhabricatorWorker::scheduleTask( - 'HarbormasterBuildWorker', - array( - 'buildID' => $build->getID() - )); + $xaction = id(new HarbormasterBuildTransaction()) + ->setTransactionType(HarbormasterBuildTransaction::TYPE_COMMAND) + ->setNewValue($command); + $build_editor->applyTransactions($build, 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/HarbormasterBuildTransactionEditor.php b/src/applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php new file mode 100644 --- /dev/null +++ b/src/applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php @@ -0,0 +1,106 @@ +getTransactionType()) { + case HarbormasterBuildTransaction::TYPE_CREATE: + case HarbormasterBuildTransaction::TYPE_COMMAND: + return null; + } + + return parent::getCustomTransactionOldValue($object, $xaction); + } + + protected function getCustomTransactionNewValue( + PhabricatorLiskDAO $object, + PhabricatorApplicationTransaction $xaction) { + + switch ($xaction->getTransactionType()) { + case HarbormasterBuildTransaction::TYPE_CREATE: + return true; + case HarbormasterBuildTransaction::TYPE_COMMAND: + return $xaction->getNewValue(); + } + + return parent::getCustomTransactionNewValue($object, $xaction); + } + + protected function applyCustomInternalTransaction( + PhabricatorLiskDAO $object, + PhabricatorApplicationTransaction $xaction) { + + switch ($xaction->getTransactionType()) { + case HarbormasterBuildTransaction::TYPE_CREATE: + return; + case HarbormasterBuildTransaction::TYPE_COMMAND: + return $this->executeBuildCommand($object, $xaction); + } + + return parent::applyCustomInternalTransaction($object, $xaction); + } + + private function executeBuildCommand( + HarbormasterBuild $build, + HarbormasterBuildTransaction $xaction) { + + $command = $xaction->getNewValue(); + + switch ($command) { + case HarbormasterBuildCommand::COMMAND_RESTART: + $issuable = $build->canRestartBuild(); + break; + case HarbormasterBuildCommand::COMMAND_STOP: + $issuable = $build->canStopBuild(); + break; + case HarbormasterBuildCommand::COMMAND_RESUME: + $issuable = $build->canResumeBuild(); + break; + default: + throw new Exception("Unknown command $command"); + } + + if (!$issuable) { + return; + } + + 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 HarbormasterBuildTransaction::TYPE_CREATE: + case HarbormasterBuildTransaction::TYPE_COMMAND: + return; + } + + return parent::applyCustomExternalTransaction($object, $xaction); + } + +} 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,68 @@ +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: + case HarbormasterBuildableTransaction::TYPE_COMMAND: + return; + } + + return parent::applyCustomInternalTransaction($object, $xaction); + } + + 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/HarbormasterBuildTransactionQuery.php b/src/applications/harbormaster/query/HarbormasterBuildTransactionQuery.php new file mode 100644 --- /dev/null +++ b/src/applications/harbormaster/query/HarbormasterBuildTransactionQuery.php @@ -0,0 +1,13 @@ +getAuthorPHID(); + + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + switch ($this->getTransactionType()) { + case self::TYPE_CREATE: + return pht( + '%s created this build.', + $this->renderHandleLink($author_phid)); + case self::TYPE_COMMAND: + switch ($new) { + case HarbormasterBuildCommand::COMMAND_RESTART: + return pht( + '%s restarted this build.', + $this->renderHandleLink($author_phid)); + case HarbormasterBuildCommand::COMMAND_RESUME: + return pht( + '%s resumed this build.', + $this->renderHandleLink($author_phid)); + case HarbormasterBuildCommand::COMMAND_STOP: + return pht( + '%s stopped this build.', + $this->renderHandleLink($author_phid)); + } + } + 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(); + } +} diff --git a/src/applications/harbormaster/storage/HarbormasterBuildableTransaction.php b/src/applications/harbormaster/storage/HarbormasterBuildableTransaction.php new file mode 100644 --- /dev/null +++ b/src/applications/harbormaster/storage/HarbormasterBuildableTransaction.php @@ -0,0 +1,87 @@ +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: + switch ($new) { + case HarbormasterBuildCommand::COMMAND_RESTART: + return pht( + '%s restarted this buildable.', + $this->renderHandleLink($author_phid)); + case HarbormasterBuildCommand::COMMAND_RESUME: + return pht( + '%s resumed this buildable.', + $this->renderHandleLink($author_phid)); + case HarbormasterBuildCommand::COMMAND_STOP: + return pht( + '%s stopped this buildable.', + $this->renderHandleLink($author_phid)); + } + } + 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(); + } +} 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.',