mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-26 00:32:42 +01:00
When a task card is edited, emit update events for old boards and parent boards
Summary: Ref T4900. When a card is edited, we currently emit an update notification for all the projects the task is tagged with. This isn't quite the right set: - We want to emit notifications for projects the task //was previously// tagged with, so it can be removed from boards it should no longer be part of. - We want to emit notifications for ancestors of projects the task is or was tagged with, so parent project boards can be updated. - However, we don't need to emit notifications for projects that don't actually have workboards. Adjust the notification set to align better to these rules. Test Plan: - Removal of Parent Project: Edited a task on board "A > B", removing the "B" project tag. Saw board A update in another window. - Normal Update: Edited a task title on board X, saw board X update in another window. - Used `bin/aphlict debug` to inspect the notification set, saw generally sensible-seeming data going over the wire. Reviewers: amckinley Maniphest Tasks: T4900 Differential Revision: https://secure.phabricator.com/D20680
This commit is contained in:
parent
7e09da3313
commit
7d41535010
4 changed files with 78 additions and 32 deletions
|
@ -412,7 +412,7 @@ return array(
|
||||||
'rsrc/js/application/phortune/phortune-credit-card-form.js' => 'd12d214f',
|
'rsrc/js/application/phortune/phortune-credit-card-form.js' => 'd12d214f',
|
||||||
'rsrc/js/application/policy/behavior-policy-control.js' => '0eaa33a9',
|
'rsrc/js/application/policy/behavior-policy-control.js' => '0eaa33a9',
|
||||||
'rsrc/js/application/policy/behavior-policy-rule-editor.js' => '9347f172',
|
'rsrc/js/application/policy/behavior-policy-rule-editor.js' => '9347f172',
|
||||||
'rsrc/js/application/projects/WorkboardBoard.js' => '75727403',
|
'rsrc/js/application/projects/WorkboardBoard.js' => 'b46d88c5',
|
||||||
'rsrc/js/application/projects/WorkboardCard.js' => '0392a5d8',
|
'rsrc/js/application/projects/WorkboardCard.js' => '0392a5d8',
|
||||||
'rsrc/js/application/projects/WorkboardCardTemplate.js' => '84f82dad',
|
'rsrc/js/application/projects/WorkboardCardTemplate.js' => '84f82dad',
|
||||||
'rsrc/js/application/projects/WorkboardColumn.js' => 'c3d24e63',
|
'rsrc/js/application/projects/WorkboardColumn.js' => 'c3d24e63',
|
||||||
|
@ -743,7 +743,7 @@ return array(
|
||||||
'javelin-view-renderer' => '9aae2b66',
|
'javelin-view-renderer' => '9aae2b66',
|
||||||
'javelin-view-visitor' => '308f9fe4',
|
'javelin-view-visitor' => '308f9fe4',
|
||||||
'javelin-websocket' => 'fdc13e4e',
|
'javelin-websocket' => 'fdc13e4e',
|
||||||
'javelin-workboard-board' => '75727403',
|
'javelin-workboard-board' => 'b46d88c5',
|
||||||
'javelin-workboard-card' => '0392a5d8',
|
'javelin-workboard-card' => '0392a5d8',
|
||||||
'javelin-workboard-card-template' => '84f82dad',
|
'javelin-workboard-card-template' => '84f82dad',
|
||||||
'javelin-workboard-column' => 'c3d24e63',
|
'javelin-workboard-column' => 'c3d24e63',
|
||||||
|
@ -1549,18 +1549,6 @@ return array(
|
||||||
'javelin-uri',
|
'javelin-uri',
|
||||||
'javelin-request',
|
'javelin-request',
|
||||||
),
|
),
|
||||||
75727403 => array(
|
|
||||||
'javelin-install',
|
|
||||||
'javelin-dom',
|
|
||||||
'javelin-util',
|
|
||||||
'javelin-stratcom',
|
|
||||||
'javelin-workflow',
|
|
||||||
'phabricator-draggable-list',
|
|
||||||
'javelin-workboard-column',
|
|
||||||
'javelin-workboard-header-template',
|
|
||||||
'javelin-workboard-card-template',
|
|
||||||
'javelin-workboard-order-template',
|
|
||||||
),
|
|
||||||
'78bc5d94' => array(
|
'78bc5d94' => array(
|
||||||
'javelin-behavior',
|
'javelin-behavior',
|
||||||
'javelin-uri',
|
'javelin-uri',
|
||||||
|
@ -1900,6 +1888,18 @@ return array(
|
||||||
'b347a301' => array(
|
'b347a301' => array(
|
||||||
'javelin-behavior',
|
'javelin-behavior',
|
||||||
),
|
),
|
||||||
|
'b46d88c5' => array(
|
||||||
|
'javelin-install',
|
||||||
|
'javelin-dom',
|
||||||
|
'javelin-util',
|
||||||
|
'javelin-stratcom',
|
||||||
|
'javelin-workflow',
|
||||||
|
'phabricator-draggable-list',
|
||||||
|
'javelin-workboard-column',
|
||||||
|
'javelin-workboard-header-template',
|
||||||
|
'javelin-workboard-card-template',
|
||||||
|
'javelin-workboard-order-template',
|
||||||
|
),
|
||||||
'b49fd60c' => array(
|
'b49fd60c' => array(
|
||||||
'multirow-row-manager',
|
'multirow-row-manager',
|
||||||
'trigger-rule',
|
'trigger-rule',
|
||||||
|
|
|
@ -3,6 +3,7 @@
|
||||||
final class ManiphestTransactionEditor
|
final class ManiphestTransactionEditor
|
||||||
extends PhabricatorApplicationTransactionEditor {
|
extends PhabricatorApplicationTransactionEditor {
|
||||||
|
|
||||||
|
private $oldProjectPHIDs;
|
||||||
private $moreValidationErrors = array();
|
private $moreValidationErrors = array();
|
||||||
|
|
||||||
public function getEditorApplicationClass() {
|
public function getEditorApplicationClass() {
|
||||||
|
@ -378,6 +379,11 @@ final class ManiphestTransactionEditor
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
$send_notifications = PhabricatorNotificationClient::isEnabled();
|
||||||
|
if ($send_notifications) {
|
||||||
|
$this->oldProjectPHIDs = $this->loadProjectPHIDs($object);
|
||||||
|
}
|
||||||
|
|
||||||
return $results;
|
return $results;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -859,14 +865,61 @@ final class ManiphestTransactionEditor
|
||||||
return array_values($phid_list);
|
return array_values($phid_list);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
protected function didApplyTransactions($object, array $xactions) {
|
protected function didApplyTransactions($object, array $xactions) {
|
||||||
// TODO: This should include projects which the object was previously
|
$send_notifications = PhabricatorNotificationClient::isEnabled();
|
||||||
// associated with but no longer is (so it can be removed from those
|
if ($send_notifications) {
|
||||||
// boards) but currently does not.
|
$old_phids = $this->oldProjectPHIDs;
|
||||||
|
$new_phids = $this->loadProjectPHIDs($object);
|
||||||
|
|
||||||
|
// We want to emit update notifications for all old and new tagged
|
||||||
|
// projects, and all parents of those projects. For example, if an
|
||||||
|
// edit removes project "A > B" from a task, the "A" workboard should
|
||||||
|
// receive an update event.
|
||||||
|
|
||||||
|
$project_phids = array_fuse($old_phids) + array_fuse($new_phids);
|
||||||
|
$project_phids = array_keys($project_phids);
|
||||||
|
|
||||||
|
$projects = id(new PhabricatorProjectQuery())
|
||||||
|
->setViewer(PhabricatorUser::getOmnipotentUser())
|
||||||
|
->withPHIDs($project_phids)
|
||||||
|
->execute();
|
||||||
|
|
||||||
|
$notify_projects = array();
|
||||||
|
foreach ($projects as $project) {
|
||||||
|
$notify_projects[$project->getPHID()] = $project;
|
||||||
|
foreach ($project->getAncestorProjects() as $ancestor) {
|
||||||
|
$notify_projects[$ancestor->getPHID()] = $ancestor;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
foreach ($notify_projects as $key => $project) {
|
||||||
|
if (!$project->getHasWorkboard()) {
|
||||||
|
unset($notify_projects[$key]);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
$notify_phids = array_keys($notify_projects);
|
||||||
|
|
||||||
|
if ($notify_phids) {
|
||||||
|
$data = array(
|
||||||
|
'type' => 'workboards',
|
||||||
|
'subscribers' => $notify_phids,
|
||||||
|
);
|
||||||
|
|
||||||
|
PhabricatorNotificationClient::tryToPostMessage($data);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return $xactions;
|
||||||
|
}
|
||||||
|
|
||||||
|
private function loadProjectPHIDs(ManiphestTask $task) {
|
||||||
|
if (!$task->getPHID()) {
|
||||||
|
return array();
|
||||||
|
}
|
||||||
|
|
||||||
$edge_query = id(new PhabricatorEdgeQuery())
|
$edge_query = id(new PhabricatorEdgeQuery())
|
||||||
->withSourcePHIDs(array($object->getPHID()))
|
->withSourcePHIDs(array($task->getPHID()))
|
||||||
->withEdgeTypes(
|
->withEdgeTypes(
|
||||||
array(
|
array(
|
||||||
PhabricatorProjectObjectHasProjectEdgeType::EDGECONST,
|
PhabricatorProjectObjectHasProjectEdgeType::EDGECONST,
|
||||||
|
@ -874,18 +927,7 @@ final class ManiphestTransactionEditor
|
||||||
|
|
||||||
$edge_query->execute();
|
$edge_query->execute();
|
||||||
|
|
||||||
$project_phids = $edge_query->getDestinationPHIDs();
|
return $edge_query->getDestinationPHIDs();
|
||||||
|
|
||||||
if ($project_phids) {
|
|
||||||
$data = array(
|
|
||||||
'type' => 'workboards',
|
|
||||||
'subscribers' => $project_phids,
|
|
||||||
);
|
|
||||||
|
|
||||||
PhabricatorNotificationClient::tryToPostMessage($data);
|
|
||||||
}
|
|
||||||
|
|
||||||
return $xactions;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -37,4 +37,8 @@ final class PhabricatorNotificationClient extends Phobject {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public static function isEnabled() {
|
||||||
|
return (bool)PhabricatorNotificationServerRef::getEnabledAdminServers();
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -711,7 +711,7 @@ JX.install('WorkboardBoard', {
|
||||||
// Compare the server state to the client state, and add or remove
|
// Compare the server state to the client state, and add or remove
|
||||||
// cards on the client as necessary to synchronize them.
|
// cards on the client as necessary to synchronize them.
|
||||||
|
|
||||||
if (update_map[card_phid][column_phid]) {
|
if (update_map[card_phid] && update_map[card_phid][column_phid]) {
|
||||||
if (!card) {
|
if (!card) {
|
||||||
column.newCard(card_phid);
|
column.newCard(card_phid);
|
||||||
column.markForRedraw();
|
column.markForRedraw();
|
||||||
|
|
Loading…
Reference in a new issue