diff --git a/src/applications/maniphest/editor/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/ManiphestTransactionEditor.php index a899192006..5225f5ea06 100644 --- a/src/applications/maniphest/editor/ManiphestTransactionEditor.php +++ b/src/applications/maniphest/editor/ManiphestTransactionEditor.php @@ -17,6 +17,7 @@ final class ManiphestTransactionEditor $types[] = ManiphestTransaction::TYPE_ATTACH; $types[] = ManiphestTransaction::TYPE_EDGE; $types[] = ManiphestTransaction::TYPE_SUBPRIORITY; + $types[] = ManiphestTransaction::TYPE_PROJECT_COLUMN; $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY; @@ -57,6 +58,7 @@ final class ManiphestTransactionEditor case ManiphestTransaction::TYPE_ATTACH: return $object->getAttached(); case ManiphestTransaction::TYPE_EDGE: + case ManiphestTransaction::TYPE_PROJECT_COLUMN: // These are pre-populated. return $xaction->getOldValue(); case ManiphestTransaction::TYPE_SUBPRIORITY: @@ -83,6 +85,7 @@ final class ManiphestTransactionEditor case ManiphestTransaction::TYPE_ATTACH: case ManiphestTransaction::TYPE_EDGE: case ManiphestTransaction::TYPE_SUBPRIORITY: + case ManiphestTransaction::TYPE_PROJECT_COLUMN: return $xaction->getNewValue(); } } @@ -101,6 +104,12 @@ final class ManiphestTransactionEditor sort($old); sort($new); return ($old !== $new); + case ManiphestTransaction::TYPE_PROJECT_COLUMN: + $new_column_phids = $new['columnPHIDs']; + $old_column_phids = $old['columnPHIDs']; + sort($new_column_phids); + sort($old_column_phids); + return ($old !== $new); } return parent::transactionHasEffect($object, $xaction); @@ -157,6 +166,9 @@ final class ManiphestTransactionEditor $data['newSubpriorityBase']); $object->setSubpriority($new_sub); return; + case ManiphestTransaction::TYPE_PROJECT_COLUMN: + // these do external (edge) updates + return; } } @@ -186,11 +198,59 @@ final class ManiphestTransactionEditor protected function applyCustomExternalTransaction( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { + + switch ($xaction->getTransactionType()) { + case ManiphestTransaction::TYPE_PROJECT_COLUMN: + $new = $xaction->getNewValue(); + $old = $xaction->getOldValue(); + $src = $object->getPHID(); + $dst = head($new['columnPHIDs']); + $edges = $old['columnPHIDs']; + $edge_type = PhabricatorEdgeConfig::TYPE_OBJECT_HAS_COLUMN; + // NOTE: Normally, we expect only one edge to exist, but this works in + // a general way so it will repair any stray edges. + $remove = array(); + $edge_missing = true; + foreach ($edges as $phid) { + if ($phid == $dst) { + $edge_missing = false; + } else { + $remove[] = $phid; + } + } + + $add = array(); + if ($edge_missing) { + $add[] = $dst; + } + + // This should never happen because of the code in + // transactionHasEffect, but keep it for maximum conservativeness + if (!$add && !$remove) { + return; + } + + $editor = id(new PhabricatorEdgeEditor()) + ->setActor($this->getActor()) + ->setSuppressEvents(true); + + foreach ($add as $phid) { + $editor->addEdge($src, $edge_type, $phid); + } + foreach ($remove as $phid) { + $editor->removeEdge($src, $edge_type, $phid); + } + $editor->save(); + break; + default: + break; + } } protected function shouldSendMail( PhabricatorLiskDAO $object, array $xactions) { + $should_mail = true; if (count($xactions) == 1) { $xaction = head($xactions); diff --git a/src/applications/maniphest/storage/ManiphestTransaction.php b/src/applications/maniphest/storage/ManiphestTransaction.php index 627066cddd..b4c1a8bb1a 100644 --- a/src/applications/maniphest/storage/ManiphestTransaction.php +++ b/src/applications/maniphest/storage/ManiphestTransaction.php @@ -13,6 +13,7 @@ final class ManiphestTransaction const TYPE_EDGE = 'edge'; const TYPE_ATTACH = 'attach'; const TYPE_SUBPRIORITY = 'subpriority'; + const TYPE_PROJECT_COLUMN = 'projectcolumn'; public function getApplicationName() { return 'maniphest'; @@ -26,6 +27,20 @@ final class ManiphestTransaction return new ManiphestTransactionComment(); } + public function shouldGenerateOldValue() { + $generate = true; + switch ($this->getTransactionType()) { + case self::TYPE_PROJECT_COLUMN: + case self::TYPE_EDGE: + $generate = false; + break; + default: + $generate = true; + break; + } + return $generate; + } + public function getRequiredHandlePHIDs() { $phids = parent::getRequiredHandlePHIDs(); @@ -51,6 +66,10 @@ final class ManiphestTransaction nonempty($new, array()), )); break; + case self::TYPE_PROJECT_COLUMN: + $phids[] = $new['projectPHID']; + $phids[] = head($new['columnPHIDs']); + break; case self::TYPE_EDGE: $phids = array_mergev( array( @@ -188,6 +207,9 @@ final class ManiphestTransaction case self::TYPE_PROJECTS: return pht('Changed Projects'); + case self::TYPE_PROJECT_COLUMN: + return pht('Changed Project Column'); + case self::TYPE_PRIORITY: if ($old == ManiphestTaskPriority::getDefaultPriority()) { return pht('Triaged'); @@ -237,6 +259,9 @@ final class ManiphestTransaction case self::TYPE_PROJECTS: return 'project'; + case self::TYPE_PROJECT_COLUMN: + return 'workboard'; + case self::TYPE_PRIORITY: if ($old == ManiphestTaskPriority::getDefaultPriority()) { return 'normal-priority'; @@ -428,6 +453,16 @@ final class ManiphestTransaction $this->renderHandleList($removed)); } + case self::TYPE_PROJECT_COLUMN: + $project_phid = $new['projectPHID']; + $column_phid = head($new['columnPHIDs']); + return pht( + '%s moved this task to %s on the %s workboard.', + $this->renderHandleLink($author_phid), + $this->renderHandleLink($column_phid), + $this->renderHandleLink($project_phid)); + break; + } @@ -620,6 +655,15 @@ final class ManiphestTransaction $this->renderHandleList($removed)); } + case self::TYPE_PROJECT_COLUMN: + $project_phid = $new['projectPHID']; + $column_phid = head($new['columnPHIDs']); + return pht( + '%s moved this task to %s on the %s workboard.', + $this->renderHandleLink($author_phid), + $this->renderHandleLink($column_phid), + $this->renderHandleLink($project_phid)); + break; } return parent::getTitleForFeed($story); diff --git a/src/applications/project/controller/PhabricatorProjectMoveController.php b/src/applications/project/controller/PhabricatorProjectMoveController.php index be17d3362d..105e81cdeb 100644 --- a/src/applications/project/controller/PhabricatorProjectMoveController.php +++ b/src/applications/project/controller/PhabricatorProjectMoveController.php @@ -49,12 +49,15 @@ final class PhabricatorProjectMoveController ->execute(); $columns = mpull($columns, null, 'getPHID'); - if (empty($columns[$column_phid])) { + $column = idx($columns, $column_phid); + if (!$column) { // User is trying to drop this object into a nonexistent column, just kick // them out. return new Aphront404Response(); } + $xactions = array(); + $edge_type = PhabricatorEdgeConfig::TYPE_OBJECT_HAS_COLUMN; $query = id(new PhabricatorEdgeQuery()) @@ -66,11 +69,14 @@ final class PhabricatorProjectMoveController $edge_phids = $query->getDestinationPHIDs(); - $this->rewriteEdges( - $object->getPHID(), - $edge_type, - $column_phid, - $edge_phids); + $xactions[] = id(new ManiphestTransaction()) + ->setTransactionType(ManiphestTransaction::TYPE_PROJECT_COLUMN) + ->setNewValue(array( + 'columnPHIDs' => array($column->getPHID()), + 'projectPHID' => $column->getProjectPHID())) + ->setOldValue(array( + 'columnPHIDs' => $edge_phids, + 'projectPHID' => $column->getProjectPHID())); if ($after_phid) { $after_task = id(new ManiphestTaskQuery()) @@ -84,60 +90,22 @@ final class PhabricatorProjectMoveController $after_pri = $after_task->getPriority(); $after_sub = $after_task->getSubpriority(); - $xactions = array(id(new ManiphestTransaction()) + $xactions[] = id(new ManiphestTransaction()) ->setTransactionType(ManiphestTransaction::TYPE_SUBPRIORITY) ->setNewValue(array( 'newPriority' => $after_pri, - 'newSubpriorityBase' => $after_sub))); - $editor = id(new ManiphestTransactionEditor()) - ->setActor($viewer) - ->setContinueOnMissingFields(true) - ->setContinueOnNoEffect(true) - ->setContentSourceFromRequest($request); - - $editor->applyTransactions($object, $xactions); + 'newSubpriorityBase' => $after_sub)); } + $editor = id(new ManiphestTransactionEditor()) + ->setActor($viewer) + ->setContinueOnMissingFields(true) + ->setContinueOnNoEffect(true) + ->setContentSourceFromRequest($request); + + $editor->applyTransactions($object, $xactions); + return id(new AphrontAjaxResponse())->setContent(array()); } - private function rewriteEdges($src, $edge_type, $dst, array $edges) { - $viewer = $this->getRequest()->getUser(); - - // NOTE: Normally, we expect only one edge to exist, but this works in a - // general way so it will repair any stray edges. - - $remove = array(); - $edge_missing = true; - foreach ($edges as $phid) { - if ($phid == $dst) { - $edge_missing = false; - } else { - $remove[] = $phid; - } - } - - $add = array(); - if ($edge_missing) { - $add[] = $dst; - } - - if (!$add && !$remove) { - return; - } - - $editor = id(new PhabricatorEdgeEditor()) - ->setActor($viewer) - ->setSuppressEvents(true); - - foreach ($add as $phid) { - $editor->addEdge($src, $edge_type, $phid); - } - foreach ($remove as $phid) { - $editor->removeEdge($src, $edge_type, $phid); - } - - $editor->save(); - } - } diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index 387fe6b57c..365a34ddde 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -743,20 +743,11 @@ abstract class PhabricatorApplicationTransactionEditor "You can not apply transactions which already have commentVersions!"); } - $exempt_types = array( - // CustomField logic currently prefills these before we enter the - // transaction editor. - PhabricatorTransactions::TYPE_CUSTOMFIELD => true, - - // TODO: Remove this, this edge type is encumbered with a bunch of - // legacy nonsense. - ManiphestTransaction::TYPE_EDGE => true, - ); - - if (empty($exempt_types[$xaction->getTransactionType()])) { - if ($xaction->getOldValue() !== null) { + if (!$xaction->shouldGenerateOldValue()) { + if ($xaction->getOldValue() === null) { throw new Exception( - "You can not apply transactions which already have oldValue!"); + 'You can not apply transactions which should already have '. + 'oldValue but do not!'); } } diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php index 151f346f06..047a862c77 100644 --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -49,6 +49,14 @@ abstract class PhabricatorApplicationTransaction return $this->ignoreOnNoEffect; } + public function shouldGenerateOldValue() { + switch ($this->getTransactionType()) { + case PhabricatorTransactions::TYPE_CUSTOMFIELD: + return false; + } + return true; + } + abstract public function getApplicationTransactionType(); private function getApplicationObjectTypeName() { diff --git a/webroot/rsrc/js/application/projects/behavior-project-boards.js b/webroot/rsrc/js/application/projects/behavior-project-boards.js index 478f950d14..55a1e02700 100644 --- a/webroot/rsrc/js/application/projects/behavior-project-boards.js +++ b/webroot/rsrc/js/application/projects/behavior-project-boards.js @@ -28,10 +28,13 @@ JX.behavior('project-boards', function(config) { var data = { objectPHID: JX.Stratcom.getData(item).objectPHID, - columnPHID: JX.Stratcom.getData(list.getRootNode()).columnPHID, - afterPHID: after && JX.Stratcom.getData(after).objectPHID + columnPHID: JX.Stratcom.getData(list.getRootNode()).columnPHID }; + if (after) { + data.afterPHID = JX.Stratcom.getData(after).objectPHID; + } + var workflow = new JX.Workflow(config.moveURI, data) .setHandler(function(response) { onresponse(response);