From 8374ec46fdabf4184734d2748395a90342eb98d6 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 24 May 2017 10:16:53 -0700 Subject: [PATCH] Make throwing things into the trash actually work in Nuance Summary: Ref T12738. Implements some modular behavior for Nuance commands. Test Plan: {F4975322} Reviewers: chad Reviewed By: chad Maniphest Tasks: T12738 Differential Revision: https://secure.phabricator.com/D18011 --- src/__phutil_library_map__.php | 6 + .../command/NuanceCommandImplementation.php | 107 ++++++++++++++++++ .../nuance/command/NuanceTrashCommand.php | 23 ++++ .../nuance/item/NuanceGitHubEventItemType.php | 3 + .../nuance/item/NuanceItemType.php | 40 ------- .../nuance/storage/NuanceItem.php | 1 - .../nuance/worker/NuanceItemUpdateWorker.php | 18 ++- .../xaction/NuanceItemCommandTransaction.php | 12 +- .../xaction/NuanceItemStatusTransaction.php | 25 ++++ 9 files changed, 187 insertions(+), 48 deletions(-) create mode 100644 src/applications/nuance/command/NuanceCommandImplementation.php create mode 100644 src/applications/nuance/command/NuanceTrashCommand.php create mode 100644 src/applications/nuance/xaction/NuanceItemStatusTransaction.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 45a9aa2b96..a01125ec38 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1601,6 +1601,7 @@ phutil_register_library_map(array( 'MultimeterLabel' => 'applications/multimeter/storage/MultimeterLabel.php', 'MultimeterSampleController' => 'applications/multimeter/controller/MultimeterSampleController.php', 'MultimeterViewer' => 'applications/multimeter/storage/MultimeterViewer.php', + 'NuanceCommandImplementation' => 'applications/nuance/command/NuanceCommandImplementation.php', 'NuanceConduitAPIMethod' => 'applications/nuance/conduit/NuanceConduitAPIMethod.php', 'NuanceConsoleController' => 'applications/nuance/controller/NuanceConsoleController.php', 'NuanceContentSource' => 'applications/nuance/contentsource/NuanceContentSource.php', @@ -1636,6 +1637,7 @@ phutil_register_library_map(array( 'NuanceItemRequestorTransaction' => 'applications/nuance/xaction/NuanceItemRequestorTransaction.php', 'NuanceItemSearchEngine' => 'applications/nuance/query/NuanceItemSearchEngine.php', 'NuanceItemSourceTransaction' => 'applications/nuance/xaction/NuanceItemSourceTransaction.php', + 'NuanceItemStatusTransaction' => 'applications/nuance/xaction/NuanceItemStatusTransaction.php', 'NuanceItemTransaction' => 'applications/nuance/storage/NuanceItemTransaction.php', 'NuanceItemTransactionComment' => 'applications/nuance/storage/NuanceItemTransactionComment.php', 'NuanceItemTransactionQuery' => 'applications/nuance/query/NuanceItemTransactionQuery.php', @@ -1690,6 +1692,7 @@ phutil_register_library_map(array( 'NuanceSourceTransactionType' => 'applications/nuance/xaction/NuanceSourceTransactionType.php', 'NuanceSourceViewController' => 'applications/nuance/controller/NuanceSourceViewController.php', 'NuanceTransaction' => 'applications/nuance/storage/NuanceTransaction.php', + 'NuanceTrashCommand' => 'applications/nuance/command/NuanceTrashCommand.php', 'NuanceWorker' => 'applications/nuance/worker/NuanceWorker.php', 'OwnersConduitAPIMethod' => 'applications/owners/conduit/OwnersConduitAPIMethod.php', 'OwnersEditConduitAPIMethod' => 'applications/owners/conduit/OwnersEditConduitAPIMethod.php', @@ -6714,6 +6717,7 @@ phutil_register_library_map(array( 'MultimeterLabel' => 'MultimeterDimension', 'MultimeterSampleController' => 'MultimeterController', 'MultimeterViewer' => 'MultimeterDimension', + 'NuanceCommandImplementation' => 'Phobject', 'NuanceConduitAPIMethod' => 'ConduitAPIMethod', 'NuanceConsoleController' => 'NuanceController', 'NuanceContentSource' => 'PhabricatorContentSource', @@ -6759,6 +6763,7 @@ phutil_register_library_map(array( 'NuanceItemRequestorTransaction' => 'NuanceItemTransactionType', 'NuanceItemSearchEngine' => 'PhabricatorApplicationSearchEngine', 'NuanceItemSourceTransaction' => 'NuanceItemTransactionType', + 'NuanceItemStatusTransaction' => 'NuanceItemTransactionType', 'NuanceItemTransaction' => 'NuanceTransaction', 'NuanceItemTransactionComment' => 'PhabricatorApplicationTransactionComment', 'NuanceItemTransactionQuery' => 'PhabricatorApplicationTransactionQuery', @@ -6822,6 +6827,7 @@ phutil_register_library_map(array( 'NuanceSourceTransactionType' => 'PhabricatorModularTransactionType', 'NuanceSourceViewController' => 'NuanceSourceController', 'NuanceTransaction' => 'PhabricatorModularTransaction', + 'NuanceTrashCommand' => 'NuanceCommandImplementation', 'NuanceWorker' => 'PhabricatorWorker', 'OwnersConduitAPIMethod' => 'ConduitAPIMethod', 'OwnersEditConduitAPIMethod' => 'PhabricatorEditEngineAPIMethod', diff --git a/src/applications/nuance/command/NuanceCommandImplementation.php b/src/applications/nuance/command/NuanceCommandImplementation.php new file mode 100644 index 0000000000..46220266ff --- /dev/null +++ b/src/applications/nuance/command/NuanceCommandImplementation.php @@ -0,0 +1,107 @@ +actor = $actor; + return $this; + } + + final public function getActor() { + return $this->actor; + } + + abstract public function getCommandName(); + abstract public function canApplyToItem(NuanceItem $item); + + abstract protected function executeCommand( + NuanceItem $item, + NuanceItemCommand $command); + + final public function applyCommand( + NuanceItem $item, + NuanceItemCommand $command) { + + $command_key = $command->getCommand(); + $implementation_key = $this->getCommandKey(); + if ($command_key !== $implementation_key) { + throw new Exception( + pht( + 'This command implementation("%s") can not apply a command of a '. + 'different type ("%s").', + $implementation_key, + $command_key)); + } + + if (!$this->canApplyToItem($item)) { + throw new Exception( + pht( + 'This command implementation ("%s") can not be applied to an '. + 'item of type "%s".', + $implementation_key, + $item->getItemType())); + } + + $this->transactionQueue = array(); + + $command_type = NuanceItemCommandTransaction::TRANSACTIONTYPE; + $command_xaction = $this->newTransaction($command_type); + + $result = $this->executeCommand($item, $command); + + $xactions = $this->transactionQueue; + $this->transactionQueue = array(); + + $command_xaction->setNewValue( + array( + 'command' => $command->getCommand(), + 'parameters' => $command->getParameters(), + 'result' => $result, + )); + + // TODO: Maybe preserve the actor's original content source? + $source = PhabricatorContentSource::newForSource( + PhabricatorDaemonContentSource::SOURCECONST); + + $actor = $this->getActor(); + + id(new NuanceItemEditor()) + ->setActor($actor) + ->setActingAsPHID($command->getAuthorPHID()) + ->setContentSource($source) + ->setContinueOnMissingFields(true) + ->setContinueOnNoEffect(true) + ->applyTransactions($item, $xactions); + } + + final public function getCommandKey() { + return $this->getPhobjectClassConstant('COMMANDKEY'); + } + + final public static function getAllCommands() { + return id(new PhutilClassMapQuery()) + ->setAncestorClass(__CLASS__) + ->setUniqueMethod('getCommandKey') + ->execute(); + } + + protected function newTransaction($type) { + $xaction = id(new NuanceItemTransaction()) + ->setTransactionType($type); + + $this->transactionQueue[] = $xaction; + + return $xaction; + } + + protected function newStatusTransaction($status) { + return $this->newTransaction(NuanceItemStatusTransaction::TRANSACTIONTYPE) + ->setNewValue($status); + } + +} diff --git a/src/applications/nuance/command/NuanceTrashCommand.php b/src/applications/nuance/command/NuanceTrashCommand.php new file mode 100644 index 0000000000..1f8a729277 --- /dev/null +++ b/src/applications/nuance/command/NuanceTrashCommand.php @@ -0,0 +1,23 @@ +getImplementation(); + return ($type instanceof NuanceFormItemType); + } + + protected function executeCommand( + NuanceItem $item, + NuanceItemCommand $command) { + $this->newStatusTransaction(NuanceItem::STATUS_CLOSED); + } + +} diff --git a/src/applications/nuance/item/NuanceGitHubEventItemType.php b/src/applications/nuance/item/NuanceGitHubEventItemType.php index b7b90690b7..b8bfb3ccab 100644 --- a/src/applications/nuance/item/NuanceGitHubEventItemType.php +++ b/src/applications/nuance/item/NuanceGitHubEventItemType.php @@ -329,6 +329,9 @@ final class NuanceGitHubEventItemType NuanceItem $item, NuanceItemCommand $command) { + // TODO: This code is no longer reachable, and has moved to + // CommandImplementation subclasses. + $action = $command->getCommand(); switch ($action) { case 'sync': diff --git a/src/applications/nuance/item/NuanceItemType.php b/src/applications/nuance/item/NuanceItemType.php index de64977cb3..3a65ad0195 100644 --- a/src/applications/nuance/item/NuanceItemType.php +++ b/src/applications/nuance/item/NuanceItemType.php @@ -106,46 +106,6 @@ abstract class NuanceItemType return $this->newWorkCommands($item); } - final public function applyCommand( - NuanceItem $item, - NuanceItemCommand $command) { - - $result = $this->handleCommand($item, $command); - - if ($result === null) { - return; - } - - $xaction = id(new NuanceItemTransaction()) - ->setTransactionType(NuanceItemCommandTransaction::TRANSACTIONTYPE) - ->setNewValue( - array( - 'command' => $command->getCommand(), - 'parameters' => $command->getParameters(), - 'result' => $result, - )); - - $viewer = $this->getViewer(); - - // TODO: Maybe preserve the actor's original content source? - $source = PhabricatorContentSource::newForSource( - PhabricatorDaemonContentSource::SOURCECONST); - - $editor = id(new NuanceItemEditor()) - ->setActor($viewer) - ->setActingAsPHID($command->getAuthorPHID()) - ->setContentSource($source) - ->setContinueOnMissingFields(true) - ->setContinueOnNoEffect(true) - ->applyTransactions($item, array($xaction)); - } - - protected function handleCommand( - NuanceItem $item, - NuanceItemCommand $command) { - return null; - } - final protected function newContentSource( NuanceItem $item, $agent_phid) { diff --git a/src/applications/nuance/storage/NuanceItem.php b/src/applications/nuance/storage/NuanceItem.php index b1deab3af2..09a106ca7a 100644 --- a/src/applications/nuance/storage/NuanceItem.php +++ b/src/applications/nuance/storage/NuanceItem.php @@ -9,7 +9,6 @@ final class NuanceItem const STATUS_IMPORTING = 'importing'; const STATUS_ROUTING = 'routing'; const STATUS_OPEN = 'open'; - const STATUS_ASSIGNED = 'assigned'; const STATUS_CLOSED = 'closed'; protected $status; diff --git a/src/applications/nuance/worker/NuanceItemUpdateWorker.php b/src/applications/nuance/worker/NuanceItemUpdateWorker.php index 5f33f885f9..30d5621872 100644 --- a/src/applications/nuance/worker/NuanceItemUpdateWorker.php +++ b/src/applications/nuance/worker/NuanceItemUpdateWorker.php @@ -68,13 +68,29 @@ final class NuanceItemUpdateWorker ->execute(); $commands = msort($commands, 'getID'); + $executors = NuanceCommandImplementation::getAllCommands(); foreach ($commands as $command) { $command ->setStatus(NuanceItemCommand::STATUS_EXECUTING) ->save(); try { - $impl->applyCommand($item, $command); + $command_key = $command->getCommand(); + + $executor = idx($executors, $command_key); + if (!$executor) { + throw new Exception( + pht( + 'Unable to execute command "%s": this command does not have '. + 'a recognized command implementation.', + $command_key)); + } + + $executor = clone $executor; + + $executor + ->setActor($viewer) + ->applyCommand($item, $command); $command ->setStatus(NuanceItemCommand::STATUS_DONE) diff --git a/src/applications/nuance/xaction/NuanceItemCommandTransaction.php b/src/applications/nuance/xaction/NuanceItemCommandTransaction.php index bf1cbdf8d6..6aa1d6473f 100644 --- a/src/applications/nuance/xaction/NuanceItemCommandTransaction.php +++ b/src/applications/nuance/xaction/NuanceItemCommandTransaction.php @@ -9,14 +9,14 @@ final class NuanceItemCommandTransaction return null; } - public function applyInternalEffects($object, $value) { - // TODO: Probably implement this. - } - public function getTitle() { + $spec = $this->getNewValue(); + $command_key = idx($spec, 'command', '???'); + return pht( - '%s applied a command to this item.', - $this->renderAuthor()); + '%s applied a "%s" command to this item.', + $this->renderAuthor(), + $command_key); } } diff --git a/src/applications/nuance/xaction/NuanceItemStatusTransaction.php b/src/applications/nuance/xaction/NuanceItemStatusTransaction.php new file mode 100644 index 0000000000..48c5a16e8c --- /dev/null +++ b/src/applications/nuance/xaction/NuanceItemStatusTransaction.php @@ -0,0 +1,25 @@ +getStatus(); + } + + public function applyInternalEffects($object, $value) { + $object->setStatus($value); + } + + public function getTitle() { + return pht( + '%s changed the status of this item from %s to %s.', + $this->renderAuthor(), + $this->renderOldValue(), + $this->renderNewValue()); + } + + +}