From 346ffc51e100f8867b16d7e265fc5d13670d4028 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 16 Jul 2021 08:44:14 -0700 Subject: [PATCH] Modularize "HarbormasterBuildableTransaction" Summary: Ref T13072. Trivially convert this into a modular transaction type. Test Plan: Issued commands to a buildable. Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam Maniphest Tasks: T13072 Differential Revision: https://secure.phabricator.com/D21694 --- src/__phutil_library_map__.php | 6 +- .../HarbormasterBuildableActionController.php | 10 +-- ...HarbormasterBuildableTransactionEditor.php | 56 --------------- .../HarbormasterBuildableTransaction.php | 72 +------------------ ...arbormasterBuildableMessageTransaction.php | 65 +++++++++++++++++ .../HarbormasterBuildableTransactionType.php | 4 ++ 6 files changed, 80 insertions(+), 133 deletions(-) create mode 100644 src/applications/harbormaster/xaction/buildable/HarbormasterBuildableMessageTransaction.php create mode 100644 src/applications/harbormaster/xaction/buildable/HarbormasterBuildableTransactionType.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index cd50024b6f..4cddbeeb2a 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1475,6 +1475,7 @@ phutil_register_library_map(array( 'HarbormasterBuildableEngine' => 'applications/harbormaster/engine/HarbormasterBuildableEngine.php', 'HarbormasterBuildableInterface' => 'applications/harbormaster/interface/HarbormasterBuildableInterface.php', 'HarbormasterBuildableListController' => 'applications/harbormaster/controller/HarbormasterBuildableListController.php', + 'HarbormasterBuildableMessageTransaction' => 'applications/harbormaster/xaction/buildable/HarbormasterBuildableMessageTransaction.php', 'HarbormasterBuildablePHIDType' => 'applications/harbormaster/phid/HarbormasterBuildablePHIDType.php', 'HarbormasterBuildableQuery' => 'applications/harbormaster/query/HarbormasterBuildableQuery.php', 'HarbormasterBuildableSearchAPIMethod' => 'applications/harbormaster/conduit/HarbormasterBuildableSearchAPIMethod.php', @@ -1483,6 +1484,7 @@ phutil_register_library_map(array( 'HarbormasterBuildableTransaction' => 'applications/harbormaster/storage/HarbormasterBuildableTransaction.php', 'HarbormasterBuildableTransactionEditor' => 'applications/harbormaster/editor/HarbormasterBuildableTransactionEditor.php', 'HarbormasterBuildableTransactionQuery' => 'applications/harbormaster/query/HarbormasterBuildableTransactionQuery.php', + 'HarbormasterBuildableTransactionType' => 'applications/harbormaster/xaction/buildable/HarbormasterBuildableTransactionType.php', 'HarbormasterBuildableViewController' => 'applications/harbormaster/controller/HarbormasterBuildableViewController.php', 'HarbormasterBuildkiteBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterBuildkiteBuildStepImplementation.php', 'HarbormasterBuildkiteBuildableInterface' => 'applications/harbormaster/interface/HarbormasterBuildkiteBuildableInterface.php', @@ -7730,14 +7732,16 @@ phutil_register_library_map(array( 'HarbormasterBuildableActionController' => 'HarbormasterController', 'HarbormasterBuildableEngine' => 'Phobject', 'HarbormasterBuildableListController' => 'HarbormasterController', + 'HarbormasterBuildableMessageTransaction' => 'HarbormasterBuildableTransactionType', 'HarbormasterBuildablePHIDType' => 'PhabricatorPHIDType', 'HarbormasterBuildableQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'HarbormasterBuildableSearchAPIMethod' => 'PhabricatorSearchEngineAPIMethod', 'HarbormasterBuildableSearchEngine' => 'PhabricatorApplicationSearchEngine', 'HarbormasterBuildableStatus' => 'Phobject', - 'HarbormasterBuildableTransaction' => 'PhabricatorApplicationTransaction', + 'HarbormasterBuildableTransaction' => 'PhabricatorModularTransaction', 'HarbormasterBuildableTransactionEditor' => 'PhabricatorApplicationTransactionEditor', 'HarbormasterBuildableTransactionQuery' => 'PhabricatorApplicationTransactionQuery', + 'HarbormasterBuildableTransactionType' => 'PhabricatorModularTransactionType', 'HarbormasterBuildableViewController' => 'HarbormasterController', 'HarbormasterBuildkiteBuildStepImplementation' => 'HarbormasterBuildStepImplementation', 'HarbormasterBuildkiteHookController' => 'HarbormasterController', diff --git a/src/applications/harbormaster/controller/HarbormasterBuildableActionController.php b/src/applications/harbormaster/controller/HarbormasterBuildableActionController.php index cfed19a6c1..b0c74139db 100644 --- a/src/applications/harbormaster/controller/HarbormasterBuildableActionController.php +++ b/src/applications/harbormaster/controller/HarbormasterBuildableActionController.php @@ -111,18 +111,14 @@ final class HarbormasterBuildableActionController ->setContinueOnNoEffect(true) ->setContinueOnMissingFields(true); + $xaction_type = HarbormasterBuildableMessageTransaction::TRANSACTIONTYPE; + $xaction = id(new HarbormasterBuildableTransaction()) - ->setTransactionType(HarbormasterBuildableTransaction::TYPE_COMMAND) + ->setTransactionType($xaction_type) ->setNewValue($action); $editor->applyTransactions($buildable, array($xaction)); - $build_editor = id(new HarbormasterBuildTransactionEditor()) - ->setActor($viewer) - ->setContentSourceFromRequest($request) - ->setContinueOnNoEffect(true) - ->setContinueOnMissingFields(true); - foreach ($can_send as $build) { $build->sendMessage( $viewer, diff --git a/src/applications/harbormaster/editor/HarbormasterBuildableTransactionEditor.php b/src/applications/harbormaster/editor/HarbormasterBuildableTransactionEditor.php index 63121813bb..895c89f858 100644 --- a/src/applications/harbormaster/editor/HarbormasterBuildableTransactionEditor.php +++ b/src/applications/harbormaster/editor/HarbormasterBuildableTransactionEditor.php @@ -11,60 +11,4 @@ final class HarbormasterBuildableTransactionEditor return pht('Harbormaster Buildables'); } - public function getTransactionTypes() { - $types = parent::getTransactionTypes(); - - $types[] = HarbormasterBuildableTransaction::TYPE_COMMAND; - - return $types; - } - - protected function getCustomTransactionOldValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case HarbormasterBuildableTransaction::TYPE_COMMAND: - return null; - } - - return parent::getCustomTransactionOldValue($object, $xaction); - } - - protected function getCustomTransactionNewValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - 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_COMMAND: - return; - } - - return parent::applyCustomInternalTransaction($object, $xaction); - } - - protected function applyCustomExternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case HarbormasterBuildableTransaction::TYPE_COMMAND: - return; - } - - return parent::applyCustomExternalTransaction($object, $xaction); - } - } diff --git a/src/applications/harbormaster/storage/HarbormasterBuildableTransaction.php b/src/applications/harbormaster/storage/HarbormasterBuildableTransaction.php index a4a2f1ab14..e3b937df1e 100644 --- a/src/applications/harbormaster/storage/HarbormasterBuildableTransaction.php +++ b/src/applications/harbormaster/storage/HarbormasterBuildableTransaction.php @@ -1,9 +1,7 @@ getAuthorPHID(); - - $old = $this->getOldValue(); - $new = $this->getNewValue(); - - switch ($this->getTransactionType()) { - case self::TYPE_COMMAND: - switch ($new) { - case HarbormasterBuildMessageRestartTransaction::MESSAGETYPE: - return pht( - '%s restarted this buildable.', - $this->renderHandleLink($author_phid)); - case HarbormasterBuildMessageResumeTransaction::MESSAGETYPE: - return pht( - '%s resumed this buildable.', - $this->renderHandleLink($author_phid)); - case HarbormasterBuildMessagePauseTransaction::MESSAGETYPE: - return pht( - '%s paused this buildable.', - $this->renderHandleLink($author_phid)); - case HarbormasterBuildMessageAbortTransaction::MESSAGETYPE: - return pht( - '%s aborted this buildable.', - $this->renderHandleLink($author_phid)); - } - } - return parent::getTitle(); + public function getBaseTransactionClass() { + return 'HarbormasterBuildableTransactionType'; } - public function getIcon() { - $author_phid = $this->getAuthorPHID(); - - $old = $this->getOldValue(); - $new = $this->getNewValue(); - - switch ($this->getTransactionType()) { - case self::TYPE_COMMAND: - switch ($new) { - case HarbormasterBuildMessageRestartTransaction::MESSAGETYPE: - return 'fa-backward'; - case HarbormasterBuildMessageResumeTransaction::MESSAGETYPE: - return 'fa-play'; - case HarbormasterBuildMessagePauseTransaction::MESSAGETYPE: - return 'fa-pause'; - case HarbormasterBuildMessageAbortTransaction::MESSAGETYPE: - return 'fa-exclamation-triangle'; - } - } - - return parent::getIcon(); - } - - public function getColor() { - $author_phid = $this->getAuthorPHID(); - - $old = $this->getOldValue(); - $new = $this->getNewValue(); - - switch ($this->getTransactionType()) { - case self::TYPE_COMMAND: - switch ($new) { - case HarbormasterBuildMessagePauseTransaction::MESSAGETYPE: - return 'red'; - } - } - return parent::getColor(); - } } diff --git a/src/applications/harbormaster/xaction/buildable/HarbormasterBuildableMessageTransaction.php b/src/applications/harbormaster/xaction/buildable/HarbormasterBuildableMessageTransaction.php new file mode 100644 index 0000000000..641e9f8bc1 --- /dev/null +++ b/src/applications/harbormaster/xaction/buildable/HarbormasterBuildableMessageTransaction.php @@ -0,0 +1,65 @@ +getNewValue(); + + switch ($new) { + case HarbormasterBuildMessageRestartTransaction::MESSAGETYPE: + return pht( + '%s restarted this buildable.', + $this->renderAuthor()); + case HarbormasterBuildMessageResumeTransaction::MESSAGETYPE: + return pht( + '%s resumed this buildable.', + $this->renderAuthor()); + case HarbormasterBuildMessagePauseTransaction::MESSAGETYPE: + return pht( + '%s paused this buildable.', + $this->renderAuthor()); + case HarbormasterBuildMessageAbortTransaction::MESSAGETYPE: + return pht( + '%s aborted this buildable.', + $this->renderAuthor()); + } + + return parent::getTitle(); + } + + public function getIcon() { + $new = $this->getNewValue(); + + switch ($new) { + case HarbormasterBuildMessageRestartTransaction::MESSAGETYPE: + return 'fa-backward'; + case HarbormasterBuildMessageResumeTransaction::MESSAGETYPE: + return 'fa-play'; + case HarbormasterBuildMessagePauseTransaction::MESSAGETYPE: + return 'fa-pause'; + case HarbormasterBuildMessageAbortTransaction::MESSAGETYPE: + return 'fa-exclamation-triangle'; + } + + return parent::getIcon(); + } + + public function getColor() { + $new = $this->getNewValue(); + + switch ($new) { + case HarbormasterBuildMessagePauseTransaction::MESSAGETYPE: + return 'red'; + } + + return parent::getColor(); + } + +} diff --git a/src/applications/harbormaster/xaction/buildable/HarbormasterBuildableTransactionType.php b/src/applications/harbormaster/xaction/buildable/HarbormasterBuildableTransactionType.php new file mode 100644 index 0000000000..0299313104 --- /dev/null +++ b/src/applications/harbormaster/xaction/buildable/HarbormasterBuildableTransactionType.php @@ -0,0 +1,4 @@ +