mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-26 16:52:41 +01:00
Workboards - add transactions for column changes
Summary: adds ManiphestTransaction::TYPE_PROJECT_COLUMN and makes it work. Had to clean up the Javascript ever so slightly as it was sending up the string "null" when it should just omit the data in these cases. Ref T4422. NOTE: this is overall a bit buggy - e.g. move a task Foo from column A to top of column B; refresh; task Foo is at bottom of column B and should be at top of column B - but I plan on additional diff or three to clean this up. Test Plan: dragged tasks around columns. clicked on those tasks and saw some nice transactions. Reviewers: epriestley CC: Korvin, epriestley, aran Maniphest Tasks: T4422 Differential Revision: https://secure.phabricator.com/D8366
This commit is contained in:
parent
83206242c9
commit
d1e64e64ff
6 changed files with 143 additions and 69 deletions
|
@ -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);
|
||||
|
|
|
@ -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);
|
||||
|
|
|
@ -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();
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -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!');
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -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() {
|
||||
|
|
|
@ -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);
|
||||
|
|
Loading…
Reference in a new issue