Skip to content

Commit 4931039

Browse files
author
tuomaspelkonen
committed
Added subscriber view to Maniphest.
Summary: People want to see all the tasks they have subscribed to in one view. A new table was added for this to make queries faster. Test Plan: Tested that the view was initially empty. After running the reindex_maniphest.php script, I saw the correct tasks there. Added myself as a subscriber to one task and made sure the view was updated. Removed myself as a subscriber from one task and made sure the view was updated again. Reviewed By: epriestley Reviewers: epriestley, jungejason, codeblock CC: aran, rm, epriestley Differential Revision: 603
1 parent b5f1faf commit 4931039

File tree

10 files changed

+149
-3
lines changed

10 files changed

+149
-3
lines changed
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
CREATE TABLE phabricator_maniphest.maniphest_tasksubscriber (
2+
taskPHID varchar(64) BINARY NOT NULL,
3+
subscriberPHID varchar(64) BINARY NOT NULL,
4+
PRIMARY KEY (subscriberPHID, taskPHID),
5+
UNIQUE KEY (taskPHID, subscriberPHID)
6+
);

scripts/search/reindex_maniphest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
echo "Updating relationships for ".count($tasks)." tasks";
2727
foreach ($tasks as $task) {
2828
ManiphestTaskProject::updateTaskProjects($task);
29+
ManiphestTaskSubscriber::updateTaskSubscribers($task);
2930
echo '.';
3031
}
3132
echo "\nDone.\n";

src/__phutil_library_map__.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,7 @@
277277
'ManiphestTaskProject' => 'applications/maniphest/storage/taskproject',
278278
'ManiphestTaskQuery' => 'applications/maniphest/query',
279279
'ManiphestTaskStatus' => 'applications/maniphest/constants/status',
280+
'ManiphestTaskSubscriber' => 'applications/maniphest/storage/subscriber',
280281
'ManiphestTaskSummaryView' => 'applications/maniphest/view/tasksummary',
281282
'ManiphestTransaction' => 'applications/maniphest/storage/transaction',
282283
'ManiphestTransactionDetailView' => 'applications/maniphest/view/transactiondetail',
@@ -788,6 +789,7 @@
788789
'ManiphestTaskPriority' => 'ManiphestConstants',
789790
'ManiphestTaskProject' => 'ManiphestDAO',
790791
'ManiphestTaskStatus' => 'ManiphestConstants',
792+
'ManiphestTaskSubscriber' => 'ManiphestDAO',
791793
'ManiphestTaskSummaryView' => 'ManiphestView',
792794
'ManiphestTransaction' => 'ManiphestDAO',
793795
'ManiphestTransactionDetailView' => 'ManiphestView',

src/applications/maniphest/controller/tasklist/ManiphestTaskListController.php

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,10 @@ public function processRequest() {
5454

5555
$views = array(
5656
'User Tasks',
57-
'action' => 'Assigned',
58-
'created' => 'Created',
59-
'triage' => 'Need Triage',
57+
'action' => 'Assigned',
58+
'created' => 'Created',
59+
'subscribed' => 'Subscribed',
60+
'triage' => 'Need Triage',
6061
'<hr />',
6162
'All Tasks',
6263
'alltriage' => 'Need Triage',
@@ -70,6 +71,7 @@ public function processRequest() {
7071
$has_filter = array(
7172
'action' => true,
7273
'created' => true,
74+
'subscribed' => true,
7375
'triage' => true,
7476
);
7577

@@ -270,6 +272,9 @@ private function loadTasks(
270272
case 'created':
271273
$query->withAuthors($user_phids);
272274
break;
275+
case 'subscribed':
276+
$query->withSubscribers($user_phids);
277+
break;
273278
case 'triage':
274279
$query->withOwners($user_phids);
275280
$query->withPriority(ManiphestTaskPriority::PRIORITY_TRIAGE);

src/applications/maniphest/query/ManiphestTaskQuery.php

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ final class ManiphestTaskQuery {
2828
private $ownerPHIDs = array();
2929
private $includeUnowned = null;
3030
private $projectPHIDs = array();
31+
private $subscriberPHIDs = array();
3132

3233
private $status = 'status-any';
3334
const STATUS_ANY = 'status-any';
@@ -89,6 +90,11 @@ public function withPriority($priority) {
8990
return $this;
9091
}
9192

93+
public function withSubscribers(array $subscribers) {
94+
$this->subscriberPHIDs = $subscribers;
95+
return $this;
96+
}
97+
9298
public function setGroupBy($group) {
9399
$this->groupBy = $group;
94100
return $this;
@@ -139,6 +145,7 @@ public function execute() {
139145
$where[] = $this->buildPriorityWhereClause($conn);
140146
$where[] = $this->buildAuthorWhereClause($conn);
141147
$where[] = $this->buildOwnerWhereClause($conn);
148+
$where[] = $this->buildSubscriberWhereClause($conn);
142149
$where[] = $this->buildProjectWhereClause($conn);
143150

144151
$where = array_filter($where);
@@ -150,6 +157,7 @@ public function execute() {
150157

151158
$join = array();
152159
$join[] = $this->buildProjectJoinClause($conn);
160+
$join[] = $this->buildSubscriberJoinClause($conn);
153161

154162
$join = array_filter($join);
155163
if ($join) {
@@ -272,6 +280,17 @@ private function buildOwnerWhereClause($conn) {
272280
}
273281
}
274282

283+
private function buildSubscriberWhereClause($conn) {
284+
if (!$this->subscriberPHIDs) {
285+
return null;
286+
}
287+
288+
return qsprintf(
289+
$conn,
290+
'subscriber.subscriberPHID IN (%Ls)',
291+
$this->subscriberPHIDs);
292+
}
293+
275294
private function buildProjectWhereClause($conn) {
276295
if (!$this->projectPHIDs) {
277296
return null;
@@ -295,6 +314,18 @@ private function buildProjectJoinClause($conn) {
295314
$project_dao->getTableName());
296315
}
297316

317+
private function buildSubscriberJoinClause($conn) {
318+
if (!$this->subscriberPHIDs) {
319+
return null;
320+
}
321+
322+
$subscriber_dao = new ManiphestTaskSubscriber();
323+
return qsprintf(
324+
$conn,
325+
'JOIN %T subscriber ON subscriber.taskPHID = task.phid',
326+
$subscriber_dao->getTableName());
327+
}
328+
298329
private function buildOrderClause($conn) {
299330
$order = array();
300331

src/applications/maniphest/query/__init__.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88

99
phutil_require_module('phabricator', 'applications/maniphest/constants/owner');
10+
phutil_require_module('phabricator', 'applications/maniphest/storage/subscriber');
1011
phutil_require_module('phabricator', 'applications/maniphest/storage/task');
1112
phutil_require_module('phabricator', 'applications/maniphest/storage/taskproject');
1213
phutil_require_module('phabricator', 'storage/qsprintf');
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
<?php
2+
3+
/*
4+
* Copyright 2011 Facebook, Inc.
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
/**
20+
* @group maniphest
21+
*/
22+
final class ManiphestTaskSubscriber extends ManiphestDAO {
23+
24+
protected $taskPHID;
25+
protected $subscriberPHID;
26+
27+
public function getConfiguration() {
28+
return array(
29+
self::CONFIG_IDS => self::IDS_MANUAL,
30+
self::CONFIG_TIMESTAMPS => false,
31+
);
32+
}
33+
34+
public static function updateTaskSubscribers(ManiphestTask $task) {
35+
$dao = new ManiphestTaskSubscriber();
36+
$conn = $dao->establishConnection('w');
37+
38+
$sql = array();
39+
$subscribers = $task->getCCPHIDs();
40+
$subscribers[] = $task->getOwnerPHID();
41+
$subscribers = array_unique($subscribers);
42+
43+
foreach ($subscribers as $subscriber_phid) {
44+
$sql[] = qsprintf(
45+
$conn,
46+
'(%s, %s)',
47+
$task->getPHID(),
48+
$subscriber_phid);
49+
}
50+
51+
queryfx(
52+
$conn,
53+
'DELETE FROM %T WHERE taskPHID = %s',
54+
$dao->getTableName(),
55+
$task->getPHID());
56+
if ($sql) {
57+
queryfx(
58+
$conn,
59+
'INSERT INTO %T (taskPHID, subscriberPHID) VALUES %Q',
60+
$dao->getTableName(),
61+
implode(', ', $sql));
62+
}
63+
}
64+
65+
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?php
2+
/**
3+
* This file is automatically generated. Lint this module to rebuild it.
4+
* @generated
5+
*/
6+
7+
8+
9+
phutil_require_module('phabricator', 'applications/maniphest/storage/base');
10+
phutil_require_module('phabricator', 'storage/qsprintf');
11+
phutil_require_module('phabricator', 'storage/queryfx');
12+
13+
14+
phutil_require_source('ManiphestTaskSubscriber.php');

src/applications/maniphest/storage/task/ManiphestTask.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ class ManiphestTask extends ManiphestDAO {
3737
protected $attached = array();
3838
protected $projectPHIDs = array();
3939
private $projectsNeedUpdate;
40+
private $subscribersNeedUpdate;
4041

4142
protected $ownerOrdering;
4243

@@ -70,6 +71,18 @@ public function setProjectPHIDs(array $phids) {
7071
return $this;
7172
}
7273

74+
public function setCCPHIDs(array $phids) {
75+
$this->ccPHIDs = $phids;
76+
$this->subscribersNeedUpdate = true;
77+
return $this;
78+
}
79+
80+
public function setOwnerPHID($phid) {
81+
$this->ownerPHID = $phid;
82+
$this->subscribersNeedUpdate = true;
83+
return $this;
84+
}
85+
7386
public function save() {
7487
if (!$this->mailKey) {
7588
$this->mailKey = sha1(Filesystem::readRandomBytes(20));
@@ -84,6 +97,13 @@ public function save() {
8497
$this->projectsNeedUpdate = false;
8598
}
8699

100+
if ($this->subscribersNeedUpdate) {
101+
// If we've changed the subscriber PHIDs for this task, update the link
102+
// table.
103+
ManiphestTaskSubscriber::updateTaskSubscribers($this);
104+
$this->subscribersNeedUpdate = false;
105+
}
106+
87107
return $result;
88108
}
89109

src/applications/maniphest/storage/task/__init__.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88

99
phutil_require_module('phabricator', 'applications/maniphest/storage/base');
10+
phutil_require_module('phabricator', 'applications/maniphest/storage/subscriber');
1011
phutil_require_module('phabricator', 'applications/maniphest/storage/taskproject');
1112
phutil_require_module('phabricator', 'applications/phid/constants');
1213
phutil_require_module('phabricator', 'applications/phid/storage/phid');

0 commit comments

Comments
 (0)