diff --git a/src/applications/maniphest/controller/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/ManiphestTaskDetailController.php --- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php +++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php @@ -224,6 +224,7 @@ $comment_form = new AphrontFormView(); $comment_form ->setUser($user) + ->setWorkflow(true) ->setAction('/maniphest/transaction/save/') ->setEncType('multipart/form-data') ->addHiddenInput('taskID', $task->getID()) diff --git a/src/applications/maniphest/controller/ManiphestTransactionSaveController.php b/src/applications/maniphest/controller/ManiphestTransactionSaveController.php --- a/src/applications/maniphest/controller/ManiphestTransactionSaveController.php +++ b/src/applications/maniphest/controller/ManiphestTransactionSaveController.php @@ -1,18 +1,11 @@ getRequest(); $user = $request->getUser(); - // TODO: T603 This doesn't require CAN_EDIT because non-editors can still - // leave comments, probably? For now, this just nondisruptive. Smooth this - // out once policies are more clear. - $task = id(new ManiphestTaskQuery()) ->setViewer($user) ->withIDs(array($request->getStr('taskID'))) @@ -21,6 +14,8 @@ return new Aphront404Response(); } + $task_uri = '/'.$task->getMonogram(); + $transactions = array(); $action = $request->getStr('action'); @@ -135,14 +130,6 @@ $transactions[] = $transaction; } - if ($request->getStr('comments')) { - $transactions[] = id(new ManiphestTransaction()) - ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT) - ->attachComment( - id(new ManiphestTransactionComment()) - ->setContent($request->getStr('comments'))); - } - // When you interact with a task, we add you to the CC list so you get // further updates, and possibly assign the task to you if you took an // ownership action (closing it) but it's currently unowned. We also move @@ -208,6 +195,15 @@ $transactions[] = $cc_transaction; } + $comments = $request->getStr('comments'); + if (strlen($comments) || !$transactions) { + $transactions[] = id(new ManiphestTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT) + ->attachComment( + id(new ManiphestTransactionComment()) + ->setContent($comments)); + } + $event = new PhabricatorEvent( PhabricatorEventType::TYPE_MANIPHEST_WILLEDITTASK, array( @@ -226,7 +222,15 @@ ->setActor($user) ->setContentSourceFromRequest($request) ->setContinueOnMissingFields(true) - ->applyTransactions($task, $transactions); + ->setContinueOnNoEffect($request->isContinueRequest()); + + try { + $editor->applyTransactions($task, $transactions); + } catch (PhabricatorApplicationTransactionNoEffectException $ex) { + return id(new PhabricatorApplicationTransactionNoEffectResponse()) + ->setCancelURI($task_uri) + ->setException($ex); + } $draft = id(new PhabricatorDraft())->loadOneWhere( 'authorPHID = %s AND draftKey = %s', @@ -247,8 +251,7 @@ $event->setAphrontRequest($request); PhutilEventEngine::dispatchEvent($event); - return id(new AphrontRedirectResponse()) - ->setURI('/T'.$task->getID()); + return id(new AphrontRedirectResponse())->setURI($task_uri); } } diff --git a/src/applications/maniphest/storage/ManiphestTask.php b/src/applications/maniphest/storage/ManiphestTask.php --- a/src/applications/maniphest/storage/ManiphestTask.php +++ b/src/applications/maniphest/storage/ManiphestTask.php @@ -119,6 +119,10 @@ return $this; } + public function getMonogram() { + return 'T'.$this->getID(); + } + public function attachGroupByProjectPHID($phid) { $this->groupByProjectPHID = $phid; return $this; diff --git a/src/applications/maniphest/storage/ManiphestTransaction.php b/src/applications/maniphest/storage/ManiphestTransaction.php --- a/src/applications/maniphest/storage/ManiphestTransaction.php +++ b/src/applications/maniphest/storage/ManiphestTransaction.php @@ -709,5 +709,20 @@ return $tags; } + public function getNoEffectDescription() { + + switch ($this->getTransactionType()) { + case self::TYPE_STATUS: + return pht('The task already has the selected status.'); + case self::TYPE_OWNER: + return pht('The task already has the selected owner.'); + case self::TYPE_PROJECTS: + return pht('The task is already associated with those projects.'); + case self::TYPE_PRIORITY: + return pht('The task already has the selected priority.'); + } + + return parent::getNoEffectDescription(); + } }