From 7e91f42b02b0a8e91d77f038fd0c9ac4ac47ff19 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 24 May 2017 16:28:36 +0000 Subject: [PATCH] Issue commands to Nuance items, at least roughly Summary: Ref T12738. This makes clicking "Throw In Trash" technically do something, sort of. In Nuance, the default mode of operation for actions is asynchronous -- so you don't have to wait for a response from Twitter or GitHub after you mash the "send default reply tweet" / "close this pull request with a nice response" button and can move directly to the next item instead. In the future, some operations will attempt to apply synchronously (e.g., local actions like "ignore this item forever"). This fakes our way through that for now. There's also no connection to the action actually doing anything yet, but I'll probably rig that up next. Test Plan: {F4975227} Reviewers: chad Reviewed By: chad Maniphest Tasks: T12738 Differential Revision: https://secure.phabricator.com/D18010 --- resources/sql/autopatches/20170524.nuance.01.command.sql | 8 ++++++++ resources/sql/autopatches/20170524.nuance.02.commandstatus.sql | 5 +++++ src/applications/nuance/application/PhabricatorNuanceApplication.php | 2 ++ src/applications/nuance/controller/NuanceItemActionController.php | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- src/applications/nuance/controller/NuanceItemViewController.php | 34 ---------------------------------- src/applications/nuance/controller/NuanceQueueWorkController.php | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- src/applications/nuance/item/NuanceFormItemType.php | 4 ++++ src/applications/nuance/item/NuanceItemType.php | 8 +------- src/applications/nuance/query/NuanceItemCommandQuery.php | 13 +++++++++++++ src/applications/nuance/storage/NuanceItemCommand.php | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----- src/applications/nuance/storage/NuanceQueue.php | 4 ++++ src/applications/nuance/worker/NuanceItemUpdateWorker.php | 23 +++++++++++++++++++++-- 12 file(s) changed, 247 insertion(s)(+), 51 deletion(s)(-) diff --git a/resources/sql/autopatches/20170524.nuance.01.command.sql b/resources/sql/autopatches/20170524.nuance.01.command.sql new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20170524.nuance.01.command.sql @@ -0,0 +1,8 @@ +ALTER TABLE {$NAMESPACE}_nuance.nuance_itemcommand + ADD dateCreated INT UNSIGNED NOT NULL; + +ALTER TABLE {$NAMESPACE}_nuance.nuance_itemcommand + ADD dateModified INT UNSIGNED NOT NULL; + +ALTER TABLE {$NAMESPACE}_nuance.nuance_itemcommand + ADD queuePHID VARBINARY(64); diff --git a/resources/sql/autopatches/20170524.nuance.02.commandstatus.sql b/resources/sql/autopatches/20170524.nuance.02.commandstatus.sql new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20170524.nuance.02.commandstatus.sql @@ -0,0 +1,5 @@ +ALTER TABLE {$NAMESPACE}_nuance.nuance_itemcommand + ADD status VARCHAR(64) NOT NULL; + +UPDATE {$NAMESPACE}_nuance.nuance_itemcommand + SET status = 'done' WHERE status = ''; diff --git a/src/applications/nuance/application/PhabricatorNuanceApplication.php b/src/applications/nuance/application/PhabricatorNuanceApplication.php --- a/src/applications/nuance/application/PhabricatorNuanceApplication.php +++ b/src/applications/nuance/application/PhabricatorNuanceApplication.php @@ -52,6 +52,8 @@ $this->getEditRoutePattern('edit/') => 'NuanceQueueEditController', 'view/(?P[1-9]\d*)/' => 'NuanceQueueViewController', 'work/(?P[1-9]\d*)/' => 'NuanceQueueWorkController', + 'action/(?P[1-9]\d*)/(?P[^/]+)/(?P[1-9]\d*)/' + => 'NuanceItemActionController', ), ), '/action/' => array( diff --git a/src/applications/nuance/controller/NuanceItemActionController.php b/src/applications/nuance/controller/NuanceItemActionController.php --- a/src/applications/nuance/controller/NuanceItemActionController.php +++ b/src/applications/nuance/controller/NuanceItemActionController.php @@ -6,6 +6,14 @@ $viewer = $this->getViewer(); $id = $request->getURIData('id'); + if (!$request->validateCSRF()) { + return new Aphront400Response(); + } + + // NOTE: This controller can be reached from an individual item (usually + // by a user) or while working through a queue (usually by staff). When + // a command originates from a queue, the URI will have a queue ID. + $item = id(new NuanceItemQuery()) ->setViewer($viewer) ->withIDs(array($id)) @@ -14,13 +22,69 @@ return new Aphront404Response(); } + $cancel_uri = $item->getURI(); + + $queue_id = $request->getURIData('queueID'); + $queue = null; + if ($queue_id) { + $queue = id(new NuanceQueueQuery()) + ->setViewer($viewer) + ->withIDs(array($queue_id)) + ->executeOne(); + if (!$queue) { + return new Aphront404Response(); + } + + $item_queue = $item->getQueue(); + if (!$item_queue || ($item_queue->getPHID() != $queue->getPHID())) { + return $this->newDialog() + ->setTitle(pht('Wrong Queue')) + ->appendParagraph( + pht( + 'You are trying to act on this item from the wrong queue: it '. + 'is currently in a different queue.')) + ->addCancelButton($cancel_uri); + } + } + $action = $request->getURIData('action'); $impl = $item->getImplementation(); $impl->setViewer($viewer); $impl->setController($this); - return $impl->buildActionResponse($item, $action); + $command = NuanceItemCommand::initializeNewCommand() + ->setItemPHID($item->getPHID()) + ->setAuthorPHID($viewer->getPHID()) + ->setCommand($action); + + if ($queue) { + $command->setQueuePHID($queue->getPHID()); + } + + $command->save(); + + // TODO: Here, we should check if the command should be tried immediately, + // and just defer it to the daemons if not. If we're going to try to apply + // the command directly, we should first acquire the worker lock. If we + // can not, we should defer the command even if it's an immediate command. + // For the moment, skip all this stuff by deferring unconditionally. + + $should_defer = true; + if ($should_defer) { + $item->scheduleUpdate(); + } else { + // ... + } + + if ($queue) { + $done_uri = $queue->getWorkURI(); + } else { + $done_uri = $item->getURI(); + } + + return id(new AphrontRedirectResponse()) + ->setURI($done_uri); } } diff --git a/src/applications/nuance/controller/NuanceItemViewController.php b/src/applications/nuance/controller/NuanceItemViewController.php --- a/src/applications/nuance/controller/NuanceItemViewController.php +++ b/src/applications/nuance/controller/NuanceItemViewController.php @@ -26,14 +26,12 @@ $curtain = $this->buildCurtain($item); $content = $this->buildContent($item); - $commands = $this->buildCommands($item); $timeline = $this->buildTransactionTimeline( $item, new NuanceItemTransactionQuery()); $main = array( - $commands, $content, $timeline, ); @@ -89,38 +87,6 @@ $impl->setViewer($viewer); return $impl->buildItemView($item); - } - - private function buildCommands(NuanceItem $item) { - $viewer = $this->getViewer(); - - $commands = id(new NuanceItemCommandQuery()) - ->setViewer($viewer) - ->withItemPHIDs(array($item->getPHID())) - ->execute(); - $commands = msort($commands, 'getID'); - - if (!$commands) { - return null; - } - - $rows = array(); - foreach ($commands as $command) { - $rows[] = array( - $command->getCommand(), - ); - } - - $table = id(new AphrontTableView($rows)) - ->setHeaders( - array( - pht('Command'), - )); - - return id(new PHUIObjectBoxView()) - ->setHeaderText(pht('Pending Commands')) - ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) - ->setTable($table); } } diff --git a/src/applications/nuance/controller/NuanceQueueWorkController.php b/src/applications/nuance/controller/NuanceQueueWorkController.php --- a/src/applications/nuance/controller/NuanceQueueWorkController.php +++ b/src/applications/nuance/controller/NuanceQueueWorkController.php @@ -64,12 +64,14 @@ $impl = $item->getImplementation() ->setViewer($viewer); + $commands = $this->buildCommands($item); $work_content = $impl->buildItemWorkView($item); $view = id(new PHUITwoColumnView()) ->setCurtain($curtain) ->setMainColumn( array( + $commands, $work_content, $timeline, )); @@ -94,12 +96,15 @@ $item_id = $item->getID(); + $action_uri = "queue/action/{$id}/{$command_key}/{$item_id}/"; + $action_uri = $this->getApplicationURI($action_uri); + $curtain->addAction( id(new PhabricatorActionView()) ->setName($command->getName()) ->setIcon($command->getIcon()) - ->setHref("queue/command/{$id}/{$command_key}/{$item_id}/")) - ->setWorkflow(true); + ->setHref($action_uri) + ->setWorkflow(true)); } $curtain->addAction( @@ -118,6 +123,64 @@ ->setHref($this->getApplicationURI("queue/view/{$id}/"))); return $curtain; + } + + private function buildCommands(NuanceItem $item) { + $viewer = $this->getViewer(); + + $commands = id(new NuanceItemCommandQuery()) + ->setViewer($viewer) + ->withItemPHIDs(array($item->getPHID())) + ->withStatuses( + array( + NuanceItemCommand::STATUS_ISSUED, + NuanceItemCommand::STATUS_EXECUTING, + NuanceItemCommand::STATUS_FAILED, + )) + ->execute(); + $commands = msort($commands, 'getID'); + + if (!$commands) { + return null; + } + + $rows = array(); + foreach ($commands as $command) { + $icon = $command->getStatusIcon(); + $color = $command->getStatusColor(); + + $rows[] = array( + $command->getID(), + id(new PHUIIconView()) + ->setIcon($icon, $color), + $viewer->renderHandle($command->getAuthorPHID()), + $command->getCommand(), + phabricator_datetime($command->getDateCreated(), $viewer), + ); + } + + $table = id(new AphrontTableView($rows)) + ->setHeaders( + array( + pht('ID'), + null, + pht('Actor'), + pht('Command'), + pht('Date'), + )) + ->setColumnClasses( + array( + null, + 'icon', + null, + 'pri', + 'wide right', + )); + + return id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Pending Commands')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->setTable($table); } } diff --git a/src/applications/nuance/item/NuanceFormItemType.php b/src/applications/nuance/item/NuanceFormItemType.php --- a/src/applications/nuance/item/NuanceFormItemType.php +++ b/src/applications/nuance/item/NuanceFormItemType.php @@ -47,4 +47,8 @@ ); } + protected function handleAction(NuanceItem $item, $action) { + return null; + } + } diff --git a/src/applications/nuance/item/NuanceItemType.php b/src/applications/nuance/item/NuanceItemType.php --- a/src/applications/nuance/item/NuanceItemType.php +++ b/src/applications/nuance/item/NuanceItemType.php @@ -95,13 +95,7 @@ } final public function buildActionResponse(NuanceItem $item, $action) { - $response = $this->handleAction($item, $action); - - if ($response === null) { - return new Aphront404Response(); - } - - return $response; + return $this->handleAction($item, $action); } protected function handleAction(NuanceItem $item, $action) { diff --git a/src/applications/nuance/query/NuanceItemCommandQuery.php b/src/applications/nuance/query/NuanceItemCommandQuery.php --- a/src/applications/nuance/query/NuanceItemCommandQuery.php +++ b/src/applications/nuance/query/NuanceItemCommandQuery.php @@ -5,6 +5,7 @@ private $ids; private $itemPHIDs; + private $statuses; public function withIDs(array $ids) { $this->ids = $ids; @@ -13,6 +14,11 @@ public function withItemPHIDs(array $item_phids) { $this->itemPHIDs = $item_phids; + return $this; + } + + public function withStatuses(array $statuses) { + $this->statuses = $statuses; return $this; } @@ -39,6 +45,13 @@ $conn, 'itemPHID IN (%Ls)', $this->itemPHIDs); + } + + if ($this->statuses !== null) { + $where[] = qsprintf( + $conn, + 'status IN (%Ls)', + $this->statuses); } return $where; diff --git a/src/applications/nuance/storage/NuanceItemCommand.php b/src/applications/nuance/storage/NuanceItemCommand.php --- a/src/applications/nuance/storage/NuanceItemCommand.php +++ b/src/applications/nuance/storage/NuanceItemCommand.php @@ -4,30 +4,84 @@ extends NuanceDAO implements PhabricatorPolicyInterface { + const STATUS_ISSUED = 'issued'; + const STATUS_EXECUTING = 'executing'; + const STATUS_DONE = 'done'; + const STATUS_FAILED = 'failed'; + protected $itemPHID; protected $authorPHID; + protected $queuePHID; protected $command; - protected $parameters; + protected $status; + protected $parameters = array(); public static function initializeNewCommand() { - return new self(); + return id(new self()) + ->setStatus(self::STATUS_ISSUED); } protected function getConfiguration() { return array( - self::CONFIG_TIMESTAMPS => false, self::CONFIG_SERIALIZATION => array( 'parameters' => self::SERIALIZATION_JSON, ), self::CONFIG_COLUMN_SCHEMA => array( 'command' => 'text64', + 'status' => 'text64', + 'queuePHID' => 'phid?', ), self::CONFIG_KEY_SCHEMA => array( - 'key_item' => array( - 'columns' => array('itemPHID'), + 'key_pending' => array( + 'columns' => array('itemPHID', 'status'), ), ), ) + parent::getConfiguration(); + } + + public static function getStatusMap() { + return array( + self::STATUS_ISSUED => array( + 'name' => pht('Issued'), + 'icon' => 'fa-clock-o', + 'color' => 'bluegrey', + ), + self::STATUS_EXECUTING => array( + 'name' => pht('Executing'), + 'icon' => 'fa-play', + 'color' => 'green', + ), + self::STATUS_DONE => array( + 'name' => pht('Done'), + 'icon' => 'fa-check', + 'color' => 'blue', + ), + self::STATUS_FAILED => array( + 'name' => pht('Failed'), + 'icon' => 'fa-times', + 'color' => 'red', + ), + ); + } + + private function getStatusSpec() { + $map = self::getStatusMap(); + return idx($map, $this->getStatus(), array()); + } + + public function getStatusIcon() { + $spec = $this->getStatusSpec(); + return idx($spec, 'icon', 'fa-question'); + } + + public function getStatusColor() { + $spec = $this->getStatusSpec(); + return idx($spec, 'color', 'indigo'); + } + + public function getStatusName() { + $spec = $this->getStatusSpec(); + return idx($spec, 'name', $this->getStatus()); } diff --git a/src/applications/nuance/storage/NuanceQueue.php b/src/applications/nuance/storage/NuanceQueue.php --- a/src/applications/nuance/storage/NuanceQueue.php +++ b/src/applications/nuance/storage/NuanceQueue.php @@ -43,6 +43,10 @@ return '/nuance/queue/view/'.$this->getID().'/'; } + public function getWorkURI() { + return '/nuance/queue/work/'.$this->getID().'/'; + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/applications/nuance/worker/NuanceItemUpdateWorker.php b/src/applications/nuance/worker/NuanceItemUpdateWorker.php --- a/src/applications/nuance/worker/NuanceItemUpdateWorker.php +++ b/src/applications/nuance/worker/NuanceItemUpdateWorker.php @@ -61,12 +61,31 @@ $commands = id(new NuanceItemCommandQuery()) ->setViewer($viewer) ->withItemPHIDs(array($item->getPHID())) + ->withStatuses( + array( + NuanceItemCommand::STATUS_ISSUED, + )) ->execute(); $commands = msort($commands, 'getID'); foreach ($commands as $command) { - $impl->applyCommand($item, $command); - $command->delete(); + $command + ->setStatus(NuanceItemCommand::STATUS_EXECUTING) + ->save(); + + try { + $impl->applyCommand($item, $command); + + $command + ->setStatus(NuanceItemCommand::STATUS_DONE) + ->save(); + } catch (Exception $ex) { + $command + ->setStatus(NuanceItemCommand::STATUS_FAILED) + ->save(); + + throw $ex; + } } } -- tangled.sh