1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-30 01:10:58 +01:00

When a task changes status, update blocked tasks

Summary:
Ref T5008. Three notes:

  - I'm not hiding these even if the status change is open -> open or closed -> closed. I think these are OK, but might be a little spammy.
  - These show in feed, but shouldn't, since they're very redundant with stories which will almost always appear adjacently. Probably a bit spammy, see TODO. We can't hide them from feed without also squelching the notifications right now, which I //don't// want to do.
  - You get a notification even if you're on the original task which changed status. This is definitely spammy, see other TODO.

Test Plan: {F156217}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5008

Differential Revision: https://secure.phabricator.com/D9166
This commit is contained in:
epriestley 2014-05-19 12:23:17 -07:00
parent 3d457a53be
commit cc9ee66ef3
2 changed files with 128 additions and 1 deletions

View file

@ -19,6 +19,7 @@ final class ManiphestTransactionEditor
$types[] = ManiphestTransaction::TYPE_EDGE; $types[] = ManiphestTransaction::TYPE_EDGE;
$types[] = ManiphestTransaction::TYPE_SUBPRIORITY; $types[] = ManiphestTransaction::TYPE_SUBPRIORITY;
$types[] = ManiphestTransaction::TYPE_PROJECT_COLUMN; $types[] = ManiphestTransaction::TYPE_PROJECT_COLUMN;
$types[] = ManiphestTransaction::TYPE_UNBLOCK;
$types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
$types[] = PhabricatorTransactions::TYPE_EDIT_POLICY; $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
@ -84,6 +85,7 @@ final class ManiphestTransactionEditor
case ManiphestTransaction::TYPE_EDGE: case ManiphestTransaction::TYPE_EDGE:
case ManiphestTransaction::TYPE_SUBPRIORITY: case ManiphestTransaction::TYPE_SUBPRIORITY:
case ManiphestTransaction::TYPE_PROJECT_COLUMN: case ManiphestTransaction::TYPE_PROJECT_COLUMN:
case ManiphestTransaction::TYPE_UNBLOCK:
return $xaction->getNewValue(); return $xaction->getNewValue();
} }
} }
@ -244,6 +246,63 @@ final class ManiphestTransactionEditor
} }
} }
protected function applyFinalEffects(
PhabricatorLiskDAO $object,
array $xactions) {
// When we change the status of a task, update tasks this tasks blocks
// with a message to the effect of "alincoln resolved blocking task Txxx."
$unblock_xaction = null;
foreach ($xactions as $xaction) {
switch ($xaction->getTransactionType()) {
case ManiphestTransaction::TYPE_STATUS:
$unblock_xaction = $xaction;
break;
}
}
if ($unblock_xaction !== null) {
$blocked_phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
$object->getPHID(),
PhabricatorEdgeConfig::TYPE_TASK_DEPENDED_ON_BY_TASK);
if ($blocked_phids) {
// In theory we could apply these through policies, but that seems a
// little bit surprising. For now, use the actor's vision.
$blocked_tasks = id(new ManiphestTaskQuery())
->setViewer($this->getActor())
->withPHIDs($blocked_phids)
->execute();
$old = $unblock_xaction->getOldValue();
$new = $unblock_xaction->getNewValue();
foreach ($blocked_tasks as $blocked_task) {
$xactions = array();
$xactions[] = id(new ManiphestTransaction())
->setTransactionType(ManiphestTransaction::TYPE_UNBLOCK)
->setOldValue(array($object->getPHID() => $old))
->setNewValue(array($object->getPHID() => $new));
// TODO: We should avoid notifiying users about these indirect
// changes if they are getting a notification about the current
// change, so you don't get a pile of extra notifications if you are
// subscribed to this task.
id(new ManiphestTransactionEditor())
->setActor($this->getActor())
->setContentSource($this->getContentSource())
->setContinueOnNoEffect(true)
->setContinueOnMissingFields(true)
->applyTransactions($blocked_task, $xactions);
}
}
}
return $xactions;
}
protected function shouldSendMail( protected function shouldSendMail(
PhabricatorLiskDAO $object, PhabricatorLiskDAO $object,
array $xactions) { array $xactions) {

View file

@ -14,6 +14,8 @@ final class ManiphestTransaction
const TYPE_SUBPRIORITY = 'subpriority'; const TYPE_SUBPRIORITY = 'subpriority';
const TYPE_PROJECT_COLUMN = 'projectcolumn'; const TYPE_PROJECT_COLUMN = 'projectcolumn';
const TYPE_UNBLOCK = 'unblock';
// NOTE: this type is deprecated. Keep it around for legacy installs // NOTE: this type is deprecated. Keep it around for legacy installs
// so any transactions render correctly. // so any transactions render correctly.
const TYPE_ATTACH = 'attach'; const TYPE_ATTACH = 'attach';
@ -34,6 +36,7 @@ final class ManiphestTransaction
switch ($this->getTransactionType()) { switch ($this->getTransactionType()) {
case self::TYPE_PROJECT_COLUMN: case self::TYPE_PROJECT_COLUMN:
case self::TYPE_EDGE: case self::TYPE_EDGE:
case self::TYPE_UNBLOCK:
return false; return false;
} }
@ -87,7 +90,11 @@ final class ManiphestTransaction
array_keys(idx($old, 'FILE', array())), array_keys(idx($old, 'FILE', array())),
)); ));
break; break;
case self::TYPE_UNBLOCK:
foreach (array_keys($new) as $phid) {
$phids[] = $phid;
}
break;
} }
return $phids; return $phids;
@ -233,6 +240,22 @@ final class ManiphestTransaction
case self::TYPE_EDGE: case self::TYPE_EDGE:
case self::TYPE_ATTACH: case self::TYPE_ATTACH:
return pht('Attached'); return pht('Attached');
case self::TYPE_UNBLOCK:
$old_status = head($old);
$new_status = head($new);
$old_closed = ManiphestTaskStatus::isClosedStatus($old_status);
$new_closed = ManiphestTaskStatus::isClosedStatus($new_status);
if ($old_closed && !$new_closed) {
return pht('Block');
} else if (!$old_closed && $new_closed) {
return pht('Unblock');
} else {
return pht('Blocker');
}
} }
return parent::getActionName(); return parent::getActionName();
@ -290,6 +313,9 @@ final class ManiphestTransaction
case self::TYPE_ATTACH: case self::TYPE_ATTACH:
return 'fa-thumb-tack'; return 'fa-thumb-tack';
case self::TYPE_UNBLOCK:
return 'fa-shield';
} }
return parent::getIcon(); return parent::getIcon();
@ -352,6 +378,38 @@ final class ManiphestTransaction
$new_name); $new_name);
} }
case self::TYPE_UNBLOCK:
$blocker_phid = key($new);
$old_status = head($old);
$new_status = head($new);
$old_closed = ManiphestTaskStatus::isClosedStatus($old_status);
$new_closed = ManiphestTaskStatus::isClosedStatus($new_status);
$old_name = ManiphestTaskStatus::getTaskStatusName($old_status);
$new_name = ManiphestTaskStatus::getTaskStatusName($new_status);
if ($old_closed && !$new_closed) {
return pht(
'%s reopened blocking task %s as "%s".',
$this->renderHandleLink($author_phid),
$this->renderHandleLink($blocker_phid),
$new_name);
} else if (!$old_closed && $new_closed) {
return pht(
'%s closed blocking task %s as "%s".',
$this->renderHandleLink($author_phid),
$this->renderHandleLink($blocker_phid),
$new_name);
} else {
return pht(
'%s changed the status of blocking task %s from "%s" to "%s".',
$this->renderHandleLink($author_phid),
$this->renderHandleLink($blocker_phid),
$old_name,
$new_name);
}
case self::TYPE_OWNER: case self::TYPE_OWNER:
if ($author_phid == $new) { if ($author_phid == $new) {
return pht( return pht(
@ -551,6 +609,16 @@ final class ManiphestTransaction
$new_name); $new_name);
} }
case self::TYPE_UNBLOCK:
// TODO: We should probably not show these in feed; they're highly
// redundant. For now, just use the normal titles. Right now, we can't
// publish something to noficiations without also publishing it to feed.
// Fix that, then stop these from rendering in feed only.
break;
case self::TYPE_OWNER: case self::TYPE_OWNER:
if ($author_phid == $new) { if ($author_phid == $new) {
return pht( return pht(