Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F13985948
D10965.id26331.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
42 KB
Referenced Files
None
Subscribers
None
D10965.id26331.diff
View Options
diff --git a/resources/sql/autopatches/20141210.maniphestsubscribersmig.1.sql b/resources/sql/autopatches/20141210.maniphestsubscribersmig.1.sql
new file mode 100644
--- /dev/null
+++ b/resources/sql/autopatches/20141210.maniphestsubscribersmig.1.sql
@@ -0,0 +1,9 @@
+INSERT IGNORE INTO {$NAMESPACE}_maniphest.edge (src, type, dst)
+ SELECT taskPHID, 21, subscriberPHID
+ FROM {$NAMESPACE}_maniphest.maniphest_tasksubscriber
+ WHERE subscriberPHID != '';
+
+INSERT IGNORE INTO {$NAMESPACE}_maniphest.edge (src, type, dst)
+ SELECT subscriberPHID, 22, taskPHID
+ FROM {$NAMESPACE}_maniphest.maniphest_tasksubscriber
+ WHERE subscriberPHID != '';
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -1001,7 +1001,6 @@
'ManiphestSearchIndexer' => 'applications/maniphest/search/ManiphestSearchIndexer.php',
'ManiphestStatusConfigOptionType' => 'applications/maniphest/config/ManiphestStatusConfigOptionType.php',
'ManiphestSubpriorityController' => 'applications/maniphest/controller/ManiphestSubpriorityController.php',
- 'ManiphestSubscribeController' => 'applications/maniphest/controller/ManiphestSubscribeController.php',
'ManiphestTask' => 'applications/maniphest/storage/ManiphestTask.php',
'ManiphestTaskDescriptionPreviewController' => 'applications/maniphest/controller/ManiphestTaskDescriptionPreviewController.php',
'ManiphestTaskDetailController' => 'applications/maniphest/controller/ManiphestTaskDetailController.php',
@@ -1021,7 +1020,6 @@
'ManiphestTaskStatus' => 'applications/maniphest/constants/ManiphestTaskStatus.php',
'ManiphestTaskStatusDatasource' => 'applications/maniphest/typeahead/ManiphestTaskStatusDatasource.php',
'ManiphestTaskStatusTestCase' => 'applications/maniphest/constants/__tests__/ManiphestTaskStatusTestCase.php',
- 'ManiphestTaskSubscriber' => 'applications/maniphest/storage/ManiphestTaskSubscriber.php',
'ManiphestTransaction' => 'applications/maniphest/storage/ManiphestTransaction.php',
'ManiphestTransactionComment' => 'applications/maniphest/storage/ManiphestTransactionComment.php',
'ManiphestTransactionEditor' => 'applications/maniphest/editor/ManiphestTransactionEditor.php',
@@ -4072,9 +4070,9 @@
'ManiphestSearchIndexer' => 'PhabricatorSearchDocumentIndexer',
'ManiphestStatusConfigOptionType' => 'PhabricatorConfigJSONOptionType',
'ManiphestSubpriorityController' => 'ManiphestController',
- 'ManiphestSubscribeController' => 'ManiphestController',
'ManiphestTask' => array(
'ManiphestDAO',
+ 'PhabricatorSubscribableInterface',
'PhabricatorMarkupInterface',
'PhabricatorPolicyInterface',
'PhabricatorTokenReceiverInterface',
@@ -4104,7 +4102,6 @@
'ManiphestTaskStatus' => 'ManiphestConstants',
'ManiphestTaskStatusDatasource' => 'PhabricatorTypeaheadDatasource',
'ManiphestTaskStatusTestCase' => 'PhabricatorTestCase',
- 'ManiphestTaskSubscriber' => 'ManiphestDAO',
'ManiphestTransaction' => 'PhabricatorApplicationTransaction',
'ManiphestTransactionComment' => 'PhabricatorApplicationTransactionComment',
'ManiphestTransactionEditor' => 'PhabricatorApplicationTransactionEditor',
diff --git a/src/applications/herald/adapter/HeraldManiphestTaskAdapter.php b/src/applications/herald/adapter/HeraldManiphestTaskAdapter.php
--- a/src/applications/herald/adapter/HeraldManiphestTaskAdapter.php
+++ b/src/applications/herald/adapter/HeraldManiphestTaskAdapter.php
@@ -139,7 +139,7 @@
case self::FIELD_ASSIGNEE:
return $this->getTask()->getOwnerPHID();
case self::FIELD_CC:
- return $this->getTask()->getCCPHIDs();
+ return $this->getTask()->getSubscriberPHIDs();
case self::FIELD_PROJECTS:
return PhabricatorEdgeQuery::loadDestinationPHIDs(
$this->getTask()->getPHID(),
diff --git a/src/applications/maniphest/application/PhabricatorManiphestApplication.php b/src/applications/maniphest/application/PhabricatorManiphestApplication.php
--- a/src/applications/maniphest/application/PhabricatorManiphestApplication.php
+++ b/src/applications/maniphest/application/PhabricatorManiphestApplication.php
@@ -66,8 +66,6 @@
),
'export/(?P<key>[^/]+)/' => 'ManiphestExportController',
'subpriority/' => 'ManiphestSubpriorityController',
- 'subscribe/(?P<action>add|rem)/(?P<id>[1-9]\d*)/'
- => 'ManiphestSubscribeController',
),
);
}
diff --git a/src/applications/maniphest/conduit/ManiphestConduitAPIMethod.php b/src/applications/maniphest/conduit/ManiphestConduitAPIMethod.php
--- a/src/applications/maniphest/conduit/ManiphestConduitAPIMethod.php
+++ b/src/applications/maniphest/conduit/ManiphestConduitAPIMethod.php
@@ -62,6 +62,8 @@
$task->setDescription((string)$request->getValue('description'));
$changes[ManiphestTransaction::TYPE_STATUS] =
ManiphestTaskStatus::getDefaultStatus();
+ $changes[PhabricatorTransactions::TYPE_SUBSCRIBERS] =
+ array('+' => array($request->getUser()->getPHID()));
} else {
$comments = $request->getValue('comments');
@@ -111,7 +113,8 @@
$ccs = $request->getValue('ccPHIDs');
if ($ccs !== null) {
- $changes[ManiphestTransaction::TYPE_CCS] = $ccs;
+ $changes[PhabricatorTransactions::TYPE_SUBSCRIBERS] =
+ array('=' => array_fuse($ccs));
}
$transactions = array();
@@ -228,6 +231,13 @@
$event->setUser($request->getUser());
$event->setConduitRequest($request);
PhutilEventEngine::dispatchEvent($event);
+
+ // reload the task now that we've done all the fun stuff
+ return id(new ManiphestTaskQuery())
+ ->setViewer($request->getUser())
+ ->withPHIDs(array($task->getPHID()))
+ ->needSubscriberPHIDs(true)
+ ->executeOne();
}
protected function buildTaskInfoDictionaries(array $tasks) {
@@ -265,7 +275,7 @@
'phid' => $task->getPHID(),
'authorPHID' => $task->getAuthorPHID(),
'ownerPHID' => $task->getOwnerPHID(),
- 'ccPHIDs' => $task->getCCPHIDs(),
+ 'ccPHIDs' => $task->getSubscriberPHIDs(),
'status' => $task->getStatus(),
'statusName' => ManiphestTaskStatus::getTaskStatusName(
$task->getStatus()),
diff --git a/src/applications/maniphest/conduit/ManiphestCreateTaskConduitAPIMethod.php b/src/applications/maniphest/conduit/ManiphestCreateTaskConduitAPIMethod.php
--- a/src/applications/maniphest/conduit/ManiphestCreateTaskConduitAPIMethod.php
+++ b/src/applications/maniphest/conduit/ManiphestCreateTaskConduitAPIMethod.php
@@ -28,7 +28,7 @@
protected function execute(ConduitAPIRequest $request) {
$task = ManiphestTask::initializeNewTask($request->getUser());
- $this->applyRequest($task, $request, $is_new = true);
+ $task = $this->applyRequest($task, $request, $is_new = true);
return $this->buildTaskInfoDictionary($task);
}
diff --git a/src/applications/maniphest/conduit/ManiphestInfoConduitAPIMethod.php b/src/applications/maniphest/conduit/ManiphestInfoConduitAPIMethod.php
--- a/src/applications/maniphest/conduit/ManiphestInfoConduitAPIMethod.php
+++ b/src/applications/maniphest/conduit/ManiphestInfoConduitAPIMethod.php
@@ -32,6 +32,7 @@
$task = id(new ManiphestTaskQuery())
->setViewer($request->getUser())
->withIDs(array($task_id))
+ ->needSubscriberPHIDs(true)
->executeOne();
if (!$task) {
throw new ConduitException('ERR_BAD_TASK');
diff --git a/src/applications/maniphest/conduit/ManiphestQueryConduitAPIMethod.php b/src/applications/maniphest/conduit/ManiphestQueryConduitAPIMethod.php
--- a/src/applications/maniphest/conduit/ManiphestQueryConduitAPIMethod.php
+++ b/src/applications/maniphest/conduit/ManiphestQueryConduitAPIMethod.php
@@ -64,6 +64,7 @@
$query = new ManiphestTaskQuery();
$query->setViewer($request->getUser());
+ $query->needSubscriberPHIDs(true);
$task_ids = $request->getValue('ids');
if ($task_ids) {
diff --git a/src/applications/maniphest/conduit/ManiphestUpdateConduitAPIMethod.php b/src/applications/maniphest/conduit/ManiphestUpdateConduitAPIMethod.php
--- a/src/applications/maniphest/conduit/ManiphestUpdateConduitAPIMethod.php
+++ b/src/applications/maniphest/conduit/ManiphestUpdateConduitAPIMethod.php
@@ -38,11 +38,13 @@
$task = id(new ManiphestTaskQuery())
->setViewer($request->getUser())
->withIDs(array($id))
+ ->needSubscriberPHIDs(true)
->executeOne();
} else {
$task = id(new ManiphestTaskQuery())
->setViewer($request->getUser())
->withPHIDs(array($phid))
+ ->needSubscriberPHIDs(true)
->executeOne();
}
@@ -58,7 +60,7 @@
throw new ConduitException('ERR-BAD-TASK');
}
- $this->applyRequest($task, $request, $is_new = false);
+ $task = $this->applyRequest($task, $request, $is_new = false);
return $this->buildTaskInfoDictionary($task);
}
diff --git a/src/applications/maniphest/controller/ManiphestBatchEditController.php b/src/applications/maniphest/controller/ManiphestBatchEditController.php
--- a/src/applications/maniphest/controller/ManiphestBatchEditController.php
+++ b/src/applications/maniphest/controller/ManiphestBatchEditController.php
@@ -18,6 +18,7 @@
PhabricatorPolicyCapability::CAN_VIEW,
PhabricatorPolicyCapability::CAN_EDIT,
))
+ ->needSubscriberPHIDs(true)
->execute();
$actions = $request->getStr('actions');
@@ -171,8 +172,8 @@
'priority' => ManiphestTransaction::TYPE_PRIORITY,
'add_project' => ManiphestTransaction::TYPE_PROJECTS,
'remove_project' => ManiphestTransaction::TYPE_PROJECTS,
- 'add_ccs' => ManiphestTransaction::TYPE_CCS,
- 'remove_ccs' => ManiphestTransaction::TYPE_CCS,
+ 'add_ccs' => PhabricatorTransactions::TYPE_SUBSCRIBERS,
+ 'remove_ccs' => PhabricatorTransactions::TYPE_SUBSCRIBERS,
);
$edge_edit_types = array(
@@ -215,8 +216,8 @@
case ManiphestTransaction::TYPE_PROJECTS:
$current = $task->getProjectPHIDs();
break;
- case ManiphestTransaction::TYPE_CCS:
- $current = $task->getCCPHIDs();
+ case PhabricatorTransactions::TYPE_SUBSCRIBERS:
+ $current = $task->getSubscriberPHIDs();
break;
}
}
@@ -246,7 +247,7 @@
continue 2;
}
break;
- case ManiphestTransaction::TYPE_CCS:
+ case PhabricatorTransactions::TYPE_SUBSCRIBERS:
if (empty($value)) {
continue 2;
}
@@ -274,12 +275,7 @@
}
break;
case ManiphestTransaction::TYPE_PROJECTS:
- case ManiphestTransaction::TYPE_CCS:
- $remove_actions = array(
- 'remove_project' => true,
- 'remove_ccs' => true,
- );
- $is_remove = isset($remove_actions[$action['action']]);
+ $is_remove = $action['action'] == 'remove_project';
$current = array_fill_keys($current, true);
$value = array_fill_keys($value, true);
@@ -309,6 +305,39 @@
$value = array_keys($new);
break;
+ case PhabricatorTransactions::TYPE_SUBSCRIBERS:
+ $is_remove = $action['action'] == 'remove_ccs';
+
+ $current = array_fill_keys($current, true);
+
+ $new = array();
+ $did_something = false;
+
+ if ($is_remove) {
+ foreach ($value as $phid) {
+ if (isset($current[$phid])) {
+ $new[$phid] = true;
+ $did_something = true;
+ }
+ }
+ if ($new) {
+ $value = array('-' => array_keys($new));
+ }
+ } else {
+ $new = array();
+ foreach ($value as $phid) {
+ $new[$phid] = true;
+ $did_something = true;
+ }
+ if ($new) {
+ $value = array('+' => array_keys($new));
+ }
+ }
+ if (!$did_something) {
+ continue 2;
+ }
+
+ break;
}
$value_map[$type] = $value;
diff --git a/src/applications/maniphest/controller/ManiphestSubscribeController.php b/src/applications/maniphest/controller/ManiphestSubscribeController.php
deleted file mode 100644
--- a/src/applications/maniphest/controller/ManiphestSubscribeController.php
+++ /dev/null
@@ -1,51 +0,0 @@
-<?php
-
-final class ManiphestSubscribeController extends ManiphestController {
-
- private $id;
- private $action;
-
- public function willProcessRequest(array $data) {
- $this->id = $data['id'];
- $this->action = $data['action'];
- }
-
- public function processRequest() {
-
- $request = $this->getRequest();
- $user = $request->getUser();
-
- $task = id(new ManiphestTaskQuery())
- ->setViewer($user)
- ->withIDs(array($this->id))
- ->executeOne();
- if (!$task) {
- return new Aphront404Response();
- }
-
- $ccs = $task->getCCPHIDs();
- switch ($this->action) {
- case 'add':
- $ccs[] = $user->getPHID();
- break;
- case 'rem':
- $ccs = array_diff($ccs, array($user->getPHID()));
- break;
- default:
- return new Aphront400Response();
- }
-
- $xaction = id(new ManiphestTransaction())
- ->setTransactionType(ManiphestTransaction::TYPE_CCS)
- ->setNewValue($ccs);
-
- $editor = id(new ManiphestTransactionEditor())
- ->setActor($user)
- ->setContentSourceFromRequest($request)
- ->setContinueOnNoEffect(true)
- ->setContinueOnMissingFields(true)
- ->applyTransactions($task, array($xaction));
-
- return id(new AphrontRedirectResponse())->setURI('/T'.$task->getID());
- }
-}
diff --git a/src/applications/maniphest/controller/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/ManiphestTaskDetailController.php
--- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php
+++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php
@@ -23,6 +23,7 @@
$task = id(new ManiphestTaskQuery())
->setViewer($user)
->withIDs(array($this->id))
+ ->needSubscriberPHIDs(true)
->executeOne();
if (!$task) {
return new Aphront404Response();
@@ -65,12 +66,6 @@
$edges = idx($query->execute(), $phid);
$phids = array_fill_keys($query->getDestinationPHIDs(), true);
- foreach ($task->getCCPHIDs() as $phid) {
- $phids[$phid] = true;
- }
- foreach ($task->getProjectPHIDs() as $phid) {
- $phids[$phid] = true;
- }
if ($task->getOwnerPHID()) {
$phids[$task->getOwnerPHID()] = true;
}
@@ -139,12 +134,12 @@
$resolution_types = ManiphestTaskStatus::getTaskStatusMap();
$transaction_types = array(
- PhabricatorTransactions::TYPE_COMMENT => pht('Comment'),
- ManiphestTransaction::TYPE_STATUS => pht('Change Status'),
- ManiphestTransaction::TYPE_OWNER => pht('Reassign / Claim'),
- ManiphestTransaction::TYPE_CCS => pht('Add CCs'),
- ManiphestTransaction::TYPE_PRIORITY => pht('Change Priority'),
- ManiphestTransaction::TYPE_PROJECTS => pht('Associate Projects'),
+ PhabricatorTransactions::TYPE_COMMENT => pht('Comment'),
+ ManiphestTransaction::TYPE_STATUS => pht('Change Status'),
+ ManiphestTransaction::TYPE_OWNER => pht('Reassign / Claim'),
+ PhabricatorTransactions::TYPE_SUBSCRIBERS => pht('Add CCs'),
+ ManiphestTransaction::TYPE_PRIORITY => pht('Change Priority'),
+ ManiphestTransaction::TYPE_PROJECTS => pht('Associate Projects'),
);
// Remove actions the user doesn't have permission to take.
@@ -265,11 +260,11 @@
->setValue(pht('Submit')));
$control_map = array(
- ManiphestTransaction::TYPE_STATUS => 'resolution',
- ManiphestTransaction::TYPE_OWNER => 'assign_to',
- ManiphestTransaction::TYPE_CCS => 'ccs',
- ManiphestTransaction::TYPE_PRIORITY => 'priority',
- ManiphestTransaction::TYPE_PROJECTS => 'projects',
+ ManiphestTransaction::TYPE_STATUS => 'resolution',
+ ManiphestTransaction::TYPE_OWNER => 'assign_to',
+ PhabricatorTransactions::TYPE_SUBSCRIBERS => 'ccs',
+ ManiphestTransaction::TYPE_PRIORITY => 'priority',
+ ManiphestTransaction::TYPE_PROJECTS => 'projects',
);
$projects_source = new PhabricatorProjectDatasource();
@@ -289,7 +284,7 @@
'limit' => 1,
'placeholder' => $users_source->getPlaceholderText(),
),
- ManiphestTransaction::TYPE_CCS => array(
+ PhabricatorTransactions::TYPE_SUBSCRIBERS => array(
'id' => 'cc-tokenizer',
'src' => $mailable_source->getDatasourceURI(),
'placeholder' => $mailable_source->getPlaceholderText(),
@@ -397,7 +392,7 @@
private function buildActionView(ManiphestTask $task) {
$viewer = $this->getRequest()->getUser();
$viewer_phid = $viewer->getPHID();
- $viewer_is_cc = in_array($viewer_phid, $task->getCCPHIDs());
+ $viewer_is_cc = in_array($viewer_phid, $task->getSubscriberPHIDs());
$id = $task->getID();
$phid = $task->getPHID();
@@ -420,26 +415,6 @@
->setDisabled(!$can_edit)
->setWorkflow(!$can_edit));
- if ($task->getOwnerPHID() === $viewer_phid) {
- $view->addAction(
- id(new PhabricatorActionView())
- ->setName(pht('Automatically Subscribed'))
- ->setDisabled(true)
- ->setIcon('fa-check-circle'));
- } else {
- $action = $viewer_is_cc ? 'rem' : 'add';
- $name = $viewer_is_cc ? pht('Unsubscribe') : pht('Subscribe');
- $icon = $viewer_is_cc ? 'fa-minus-circle' : 'fa-plus-circle';
-
- $view->addAction(
- id(new PhabricatorActionView())
- ->setName($name)
- ->setHref("/maniphest/subscribe/{$action}/{$id}/")
- ->setRenderAsForm(true)
- ->setUser($viewer)
- ->setIcon($icon));
- }
-
$view->addAction(
id(new PhabricatorActionView())
->setName(pht('Merge Duplicates In'))
@@ -490,14 +465,6 @@
pht('Priority'),
ManiphestTaskPriority::getTaskPriorityName($task->getPriority()));
- $handles = $this->getLoadedHandles();
- $cc_handles = array_select_keys($handles, $task->getCCPHIDs());
- $subscriber_html = id(new SubscriptionListStringBuilder())
- ->setObjectPHID($task->getPHID())
- ->setHandles($cc_handles)
- ->buildPropertyString();
- $view->addProperty(pht('Subscribers'), $subscriber_html);
-
$view->addProperty(
pht('Author'),
$this->getHandle($task->getAuthorPHID())->renderLink());
diff --git a/src/applications/maniphest/controller/ManiphestTaskEditController.php b/src/applications/maniphest/controller/ManiphestTaskEditController.php
--- a/src/applications/maniphest/controller/ManiphestTaskEditController.php
+++ b/src/applications/maniphest/controller/ManiphestTaskEditController.php
@@ -38,6 +38,7 @@
PhabricatorPolicyCapability::CAN_EDIT,
))
->withIDs(array($this->id))
+ ->needSubscriberPHIDs(true)
->executeOne();
if (!$task) {
return new Aphront404Response();
@@ -215,7 +216,7 @@
$task->setDescription($new_desc);
$task->setPriority($request->getInt('priority'));
$task->setOwnerPHID($owner_phid);
- $task->setCCPHIDs($request->getArr('cc'));
+ $task->attachSubscriberPHIDs($request->getArr('cc'));
$task->attachProjectPHIDs($request->getArr('projects'));
} else {
@@ -227,7 +228,8 @@
$changes[ManiphestTransaction::TYPE_OWNER] = $owner_phid;
}
- $changes[ManiphestTransaction::TYPE_CCS] = $request->getArr('cc');
+ $changes[PhabricatorTransactions::TYPE_SUBSCRIBERS] =
+ array('=' => $request->getArr('cc'));
if ($can_edit_projects) {
$projects = $request->getArr('projects');
@@ -436,19 +438,20 @@
}
} else {
if (!$task->getID()) {
- $task->setCCPHIDs(array(
+ $task->attachSubscriberPHIDs(array(
$user->getPHID(),
));
if ($template_id) {
$template_task = id(new ManiphestTaskQuery())
->setViewer($user)
->withIDs(array($template_id))
+ ->needSubscriberPHIDs(true)
->executeOne();
if ($template_task) {
$cc_phids = array_unique(array_merge(
- $template_task->getCCPHIDs(),
+ $template_task->getSubscriberPHIDs(),
array($user->getPHID())));
- $task->setCCPHIDs($cc_phids);
+ $task->attachSubscriberPHIDs($cc_phids);
$task->attachProjectPHIDs($template_task->getProjectPHIDs());
$task->setOwnerPHID($template_task->getOwnerPHID());
$task->setPriority($template_task->getPriority());
@@ -486,7 +489,7 @@
$phids = array_merge(
array($task->getOwnerPHID()),
- $task->getCCPHIDs(),
+ $task->getSubscriberPHIDs(),
$task->getProjectPHIDs());
if ($parent_task) {
@@ -512,8 +515,8 @@
$assigned_value = array();
}
- if ($task->getCCPHIDs()) {
- $cc_value = array_select_keys($handles, $task->getCCPHIDs());
+ if ($task->getSubscriberPHIDs()) {
+ $cc_value = array_select_keys($handles, $task->getSubscriberPHIDs());
} else {
$cc_value = array();
}
diff --git a/src/applications/maniphest/controller/ManiphestTransactionPreviewController.php b/src/applications/maniphest/controller/ManiphestTransactionPreviewController.php
--- a/src/applications/maniphest/controller/ManiphestTransactionPreviewController.php
+++ b/src/applications/maniphest/controller/ManiphestTransactionPreviewController.php
@@ -58,22 +58,19 @@
}
$transaction->setNewValue($value);
break;
- case ManiphestTransaction::TYPE_CCS:
+ case PhabricatorTransactions::TYPE_SUBSCRIBERS:
if ($value) {
$value = json_decode($value);
}
if (!$value) {
$value = array();
}
- $phids = $value;
-
- foreach ($task->getCCPHIDs() as $cc_phid) {
+ $phids = array();
+ foreach ($value as $cc_phid) {
$phids[] = $cc_phid;
- $value[] = $cc_phid;
}
-
- $transaction->setOldValue($task->getCCPHIDs());
- $transaction->setNewValue($value);
+ $transaction->setOldValue(array());
+ $transaction->setNewValue($phids);
break;
case ManiphestTransaction::TYPE_PROJECTS:
if ($value) {
diff --git a/src/applications/maniphest/controller/ManiphestTransactionSaveController.php b/src/applications/maniphest/controller/ManiphestTransactionSaveController.php
--- a/src/applications/maniphest/controller/ManiphestTransactionSaveController.php
+++ b/src/applications/maniphest/controller/ManiphestTransactionSaveController.php
@@ -9,6 +9,7 @@
$task = id(new ManiphestTaskQuery())
->setViewer($user)
->withIDs(array($request->getStr('taskID')))
+ ->needSubscriberPHIDs(true)
->executeOne();
if (!$task) {
return new Aphront404Response();
@@ -33,7 +34,7 @@
$cc_transaction = new ManiphestTransaction();
$cc_transaction
- ->setTransactionType(ManiphestTransaction::TYPE_CCS);
+ ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS);
$transaction = new ManiphestTransaction();
$transaction
@@ -64,7 +65,7 @@
'+' => array_fuse($projects),
));
break;
- case ManiphestTransaction::TYPE_CCS:
+ case PhabricatorTransactions::TYPE_SUBSCRIBERS:
// Accumulate the new explicit CCs into the array that we'll add in
// the CC transaction later.
$added_ccs = array_merge($added_ccs, $request->getArr('ccs'));
@@ -135,19 +136,21 @@
if (!$user_owns_task) {
// If we aren't making the user the new task owner and they aren't the
// existing task owner, add them to CC unless they're aleady CC'd.
- if (!in_array($user->getPHID(), $task->getCCPHIDs())) {
+ if (!in_array($user->getPHID(), $task->getSubscriberPHIDs())) {
$added_ccs[] = $user->getPHID();
}
}
// Evade no-effect detection in the new editor stuff until we can switch
// to subscriptions.
- $added_ccs = array_filter(array_diff($added_ccs, $task->getCCPHIDs()));
+ $added_ccs = array_filter(array_diff(
+ $added_ccs,
+ $task->getSubscriberPHIDs()));
if ($added_ccs) {
// We've added CCs, so include a CC transaction.
- $all_ccs = array_merge($task->getCCPHIDs(), $added_ccs);
- $cc_transaction->setNewValue($all_ccs);
+ $all_ccs = array_merge($task->getSubscriberPHIDs(), $added_ccs);
+ $cc_transaction->setNewValue(array('=' => $all_ccs));
$transactions[] = $cc_transaction;
}
diff --git a/src/applications/maniphest/editor/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/ManiphestTransactionEditor.php
--- a/src/applications/maniphest/editor/ManiphestTransactionEditor.php
+++ b/src/applications/maniphest/editor/ManiphestTransactionEditor.php
@@ -23,7 +23,6 @@
$types[] = ManiphestTransaction::TYPE_TITLE;
$types[] = ManiphestTransaction::TYPE_DESCRIPTION;
$types[] = ManiphestTransaction::TYPE_OWNER;
- $types[] = ManiphestTransaction::TYPE_CCS;
$types[] = ManiphestTransaction::TYPE_SUBPRIORITY;
$types[] = ManiphestTransaction::TYPE_PROJECT_COLUMN;
$types[] = ManiphestTransaction::TYPE_MERGED_INTO;
@@ -62,8 +61,6 @@
return $object->getDescription();
case ManiphestTransaction::TYPE_OWNER:
return nonempty($object->getOwnerPHID(), null);
- case ManiphestTransaction::TYPE_CCS:
- return array_values(array_unique($object->getCCPHIDs()));
case ManiphestTransaction::TYPE_PROJECT_COLUMN:
// These are pre-populated.
return $xaction->getOldValue();
@@ -82,8 +79,6 @@
switch ($xaction->getTransactionType()) {
case ManiphestTransaction::TYPE_PRIORITY:
return (int)$xaction->getNewValue();
- case ManiphestTransaction::TYPE_CCS:
- return array_values(array_unique($xaction->getNewValue()));
case ManiphestTransaction::TYPE_OWNER:
return nonempty($xaction->getNewValue(), null);
case ManiphestTransaction::TYPE_STATUS:
@@ -106,10 +101,6 @@
$new = $xaction->getNewValue();
switch ($xaction->getTransactionType()) {
- case ManiphestTransaction::TYPE_CCS:
- sort($old);
- sort($new);
- return ($old !== $new);
case ManiphestTransaction::TYPE_PROJECT_COLUMN:
$new_column_phids = $new['columnPHIDs'];
$old_column_phids = $old['columnPHIDs'];
@@ -155,8 +146,6 @@
}
return $object->setOwnerPHID($phid);
- case ManiphestTransaction::TYPE_CCS:
- return $object->setCCPHIDs($xaction->getNewValue());
case ManiphestTransaction::TYPE_SUBPRIORITY:
$data = $xaction->getNewValue();
$new_sub = $this->getNextSubpriority(
@@ -433,10 +422,6 @@
protected function getMailCC(PhabricatorLiskDAO $object) {
$phids = array();
- foreach ($object->getCCPHIDs() as $phid) {
- $phids[] = $phid;
- }
-
foreach (parent::getMailCC($object) as $phid) {
$phids[] = $phid;
}
@@ -561,22 +546,16 @@
HeraldAdapter $adapter,
HeraldTranscript $transcript) {
- // TODO: Convert these to transactions. The way Maniphest deals with these
- // transactions is currently unconventional and messy.
+ $this->heraldEmailPHIDs = $adapter->getEmailPHIDs();
+ $xactions = array();
- $save_again = false;
$cc_phids = $adapter->getCcPHIDs();
if ($cc_phids) {
- $existing_cc = $object->getCCPHIDs();
- $new_cc = array_unique(array_merge($cc_phids, $existing_cc));
- $object->setCCPHIDs($new_cc);
- $object->save();
+ $xactions[] = id(new ManiphestTransaction())
+ ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS)
+ ->setNewValue(array('+' => $cc_phids));
}
- $this->heraldEmailPHIDs = $adapter->getEmailPHIDs();
-
- $xactions = array();
-
$assign_phid = $adapter->getAssignPHID();
if ($assign_phid) {
$xactions[] = id(new ManiphestTransaction())
diff --git a/src/applications/maniphest/lipsum/PhabricatorManiphestTaskTestDataGenerator.php b/src/applications/maniphest/lipsum/PhabricatorManiphestTaskTestDataGenerator.php
--- a/src/applications/maniphest/lipsum/PhabricatorManiphestTaskTestDataGenerator.php
+++ b/src/applications/maniphest/lipsum/PhabricatorManiphestTaskTestDataGenerator.php
@@ -28,8 +28,8 @@
$this->generateTaskStatus();
$changes[ManiphestTransaction::TYPE_PRIORITY] =
$this->generateTaskPriority();
- $changes[ManiphestTransaction::TYPE_CCS] =
- $this->getCCPHIDs();
+ $changes[PhabricatorTransactions::TYPE_SUBSCRIBERS] =
+ array('=' => $this->getCCPHIDs());
$transactions = array();
foreach ($changes as $type => $value) {
$transaction = clone $template;
diff --git a/src/applications/maniphest/mail/ManiphestReplyHandler.php b/src/applications/maniphest/mail/ManiphestReplyHandler.php
--- a/src/applications/maniphest/mail/ManiphestReplyHandler.php
+++ b/src/applications/maniphest/mail/ManiphestReplyHandler.php
@@ -106,14 +106,14 @@
break;
case 'unsubscribe':
$is_unsub = true;
- $ttype = ManiphestTransaction::TYPE_CCS;
- $ccs = $task->getCCPHIDs();
+ $ttype = PhabricatorTransactions::TYPE_SUBSCRIBERS;
+ $ccs = $task->getSubscriberPHIDs();
foreach ($ccs as $k => $phid) {
if ($phid == $user->getPHID()) {
unset($ccs[$k]);
}
}
- $new_value = array_values($ccs);
+ $new_value = array('=' => array_values($ccs));
break;
}
@@ -135,7 +135,7 @@
}
$ccs = $mail->loadCCPHIDs();
- $old_ccs = $task->getCCPHIDs();
+ $old_ccs = $task->getSubscriberPHIDs();
$new_ccs = array_merge($old_ccs, $ccs);
if (!$is_unsub) {
$new_ccs[] = $user->getPHID();
@@ -144,8 +144,9 @@
if (array_diff($new_ccs, $old_ccs)) {
$cc_xaction = clone $template;
- $cc_xaction->setTransactionType(ManiphestTransaction::TYPE_CCS);
- $cc_xaction->setNewValue($new_ccs);
+ $cc_xaction->setTransactionType(
+ PhabricatorTransactions::TYPE_SUBSCRIBERS);
+ $cc_xaction->setNewValue(array('=' => $new_ccs));
$xactions[] = $cc_xaction;
}
diff --git a/src/applications/maniphest/mail/ManiphestTaskMailReceiver.php b/src/applications/maniphest/mail/ManiphestTaskMailReceiver.php
--- a/src/applications/maniphest/mail/ManiphestTaskMailReceiver.php
+++ b/src/applications/maniphest/mail/ManiphestTaskMailReceiver.php
@@ -17,6 +17,7 @@
$results = id(new ManiphestTaskQuery())
->setViewer($viewer)
->withIDs(array($id))
+ ->needSubscriberPHIDs(true)
->execute();
return head($results);
diff --git a/src/applications/maniphest/query/ManiphestTaskQuery.php b/src/applications/maniphest/query/ManiphestTaskQuery.php
--- a/src/applications/maniphest/query/ManiphestTaskQuery.php
+++ b/src/applications/maniphest/query/ManiphestTaskQuery.php
@@ -50,6 +50,8 @@
const ORDER_MODIFIED = 'order-modified';
const ORDER_TITLE = 'order-title';
+ private $needSubscriberPHIDs;
+
const DEFAULT_PAGE_SIZE = 1000;
public function withAuthors(array $authors) {
@@ -178,6 +180,11 @@
return $this;
}
+ public function needSubscriberPHIDs($bool) {
+ $this->needSubscriberPHIDs = $bool;
+ return $this;
+ }
+
public function loadPage() {
// TODO: (T603) It is possible for a user to find the PHID of a project
// they can't see, then query for tasks in that project and deduce the
@@ -196,7 +203,6 @@
$where[] = $this->buildPrioritiesWhereClause($conn);
$where[] = $this->buildAuthorWhereClause($conn);
$where[] = $this->buildOwnerWhereClause($conn);
- $where[] = $this->buildSubscriberWhereClause($conn);
$where[] = $this->buildProjectWhereClause($conn);
$where[] = $this->buildAnyProjectWhereClause($conn);
$where[] = $this->buildAnyUserProjectWhereClause($conn);
@@ -332,12 +338,14 @@
}
protected function didFilterPage(array $tasks) {
+ $phids = mpull($tasks, 'getPHID');
+
// TODO: Eventually, we should make this optional and introduce a
// needProjectPHIDs() method, but for now there's a lot of code which
// assumes the data is always populated.
$edge_query = id(new PhabricatorEdgeQuery())
- ->withSourcePHIDs(mpull($tasks, 'getPHID'))
+ ->withSourcePHIDs($phids)
->withEdgeTypes(
array(
PhabricatorProjectObjectHasProjectEdgeType::EDGECONST,
@@ -345,8 +353,19 @@
$edge_query->execute();
foreach ($tasks as $task) {
- $phids = $edge_query->getDestinationPHIDs(array($task->getPHID()));
- $task->attachProjectPHIDs($phids);
+ $project_phids = $edge_query->getDestinationPHIDs(
+ array($task->getPHID()));
+ $task->attachProjectPHIDs($project_phids);
+ }
+
+ if ($this->needSubscriberPHIDs) {
+ $subscriber_sets = id(new PhabricatorSubscribersQuery())
+ ->withObjectPHIDs($phids)
+ ->execute();
+ foreach ($tasks as $task) {
+ $subscribers = idx($subscriber_sets, $task->getPHID(), array());
+ $task->attachSubscriberPHIDs($subscribers);
+ }
}
return $tasks;
@@ -497,17 +516,6 @@
$fulltext_results);
}
- private function buildSubscriberWhereClause(AphrontDatabaseConnection $conn) {
- if (!$this->subscriberPHIDs) {
- return null;
- }
-
- return qsprintf(
- $conn,
- 'subscriber.subscriberPHID IN (%Ls)',
- $this->subscriberPHIDs);
- }
-
private function buildProjectWhereClause(AphrontDatabaseConnection $conn) {
if (!$this->projectPHIDs && !$this->includeNoProject) {
return null;
@@ -708,11 +716,14 @@
}
if ($this->subscriberPHIDs) {
- $subscriber_dao = new ManiphestTaskSubscriber();
$joins[] = qsprintf(
$conn_r,
- 'JOIN %T subscriber ON subscriber.taskPHID = task.phid',
- $subscriber_dao->getTableName());
+ 'JOIN %T e_ccs ON e_ccs.src = task.phid '.
+ 'AND e_ccs.type = %s '.
+ 'AND e_ccs.dst in (%Ls)',
+ PhabricatorEdgeConfig::TABLE_NAME_EDGE,
+ PhabricatorEdgeConfig::TYPE_OBJECT_HAS_SUBSCRIBER,
+ $this->subscriberPHIDs);
}
switch ($this->groupBy) {
diff --git a/src/applications/maniphest/search/ManiphestSearchIndexer.php b/src/applications/maniphest/search/ManiphestSearchIndexer.php
--- a/src/applications/maniphest/search/ManiphestSearchIndexer.php
+++ b/src/applications/maniphest/search/ManiphestSearchIndexer.php
@@ -54,21 +54,6 @@
$task->getDateCreated());
}
- // We need to load handles here since non-users may subscribe (mailing
- // lists, e.g.)
- $ccs = $task->getCCPHIDs();
- $handles = id(new PhabricatorHandleQuery())
- ->setViewer(PhabricatorUser::getOmnipotentUser())
- ->withPHIDs($ccs)
- ->execute();
- foreach ($ccs as $cc) {
- $doc->addRelationship(
- PhabricatorSearchRelationship::RELATIONSHIP_SUBSCRIBER,
- $handles[$cc]->getPHID(),
- $handles[$cc]->getType(),
- time());
- }
-
return $doc;
}
diff --git a/src/applications/maniphest/storage/ManiphestTask.php b/src/applications/maniphest/storage/ManiphestTask.php
--- a/src/applications/maniphest/storage/ManiphestTask.php
+++ b/src/applications/maniphest/storage/ManiphestTask.php
@@ -2,6 +2,7 @@
final class ManiphestTask extends ManiphestDAO
implements
+ PhabricatorSubscribableInterface,
PhabricatorMarkupInterface,
PhabricatorPolicyInterface,
PhabricatorTokenReceiverInterface,
@@ -17,7 +18,6 @@
protected $authorPHID;
protected $ownerPHID;
- protected $ccPHIDs = array();
protected $status;
protected $priority;
@@ -33,10 +33,10 @@
protected $attached = array();
protected $projectPHIDs = array();
- private $subscribersNeedUpdate;
protected $ownerOrdering;
+ private $subscriberPHIDs = self::ATTACHABLE;
private $groupByProjectPHID = self::ATTACHABLE;
private $customFields = self::ATTACHABLE;
private $edgeProjectPHIDs = self::ATTACHABLE;
@@ -56,7 +56,8 @@
->setAuthorPHID($actor->getPHID())
->setViewPolicy($view_policy)
->setEditPolicy($edit_policy)
- ->attachProjectPHIDs(array());
+ ->attachProjectPHIDs(array())
+ ->attachSubscriberPHIDs(array());
}
public function getConfiguration() {
@@ -141,8 +142,8 @@
return PhabricatorPHID::generateNewPHID(ManiphestTaskPHIDType::TYPECONST);
}
- public function getCCPHIDs() {
- return array_values(nonempty($this->ccPHIDs, array()));
+ public function getSubscriberPHIDs() {
+ return $this->assertAttached($this->subscriberPHIDs);
}
public function getProjectPHIDs() {
@@ -154,15 +155,13 @@
return $this;
}
- public function setCCPHIDs(array $phids) {
- $this->ccPHIDs = array_values($phids);
- $this->subscribersNeedUpdate = true;
+ public function attachSubscriberPHIDs(array $phids) {
+ $this->subscriberPHIDs = $phids;
return $this;
}
public function setOwnerPHID($phid) {
$this->ownerPHID = nonempty($phid, null);
- $this->subscribersNeedUpdate = true;
return $this;
}
@@ -194,13 +193,6 @@
$result = parent::save();
- if ($this->subscribersNeedUpdate) {
- // If we've changed the subscriber PHIDs for this task, update the link
- // table.
- ManiphestTaskSubscriber::updateTaskSubscribers($this);
- $this->subscribersNeedUpdate = false;
- }
-
return $result;
}
@@ -217,6 +209,22 @@
}
+/* -( PhabricatorSubscribableInterface )----------------------------------- */
+
+
+ public function isAutomaticallySubscribed($phid) {
+ return ($phid == $this->getOwnerPHID());
+ }
+
+ public function shouldShowSubscribersProperty() {
+ return true;
+ }
+
+ public function shouldAllowSubscription($phid) {
+ return true;
+ }
+
+
/* -( Markup Interface )--------------------------------------------------- */
diff --git a/src/applications/maniphest/storage/ManiphestTaskSubscriber.php b/src/applications/maniphest/storage/ManiphestTaskSubscriber.php
deleted file mode 100644
--- a/src/applications/maniphest/storage/ManiphestTaskSubscriber.php
+++ /dev/null
@@ -1,59 +0,0 @@
-<?php
-
-final class ManiphestTaskSubscriber extends ManiphestDAO {
-
- protected $taskPHID;
- protected $subscriberPHID;
-
- public function getConfiguration() {
- return array(
- self::CONFIG_IDS => self::IDS_MANUAL,
- self::CONFIG_TIMESTAMPS => false,
- self::CONFIG_COLUMN_SCHEMA => array(
- 'id' => null,
- ),
- self::CONFIG_KEY_SCHEMA => array(
- 'PRIMARY' => array(
- 'columns' => array('subscriberPHID', 'taskPHID'),
- 'unique' => true,
- ),
- 'taskPHID' => array(
- 'columns' => array('taskPHID', 'subscriberPHID'),
- 'unique' => true,
- ),
- ),
- );
- }
-
- public static function updateTaskSubscribers(ManiphestTask $task) {
- $dao = new ManiphestTaskSubscriber();
- $conn = $dao->establishConnection('w');
-
- $sql = array();
- $subscribers = $task->getCCPHIDs();
- $subscribers[] = $task->getOwnerPHID();
- $subscribers = array_unique($subscribers);
-
- foreach ($subscribers as $subscriber_phid) {
- $sql[] = qsprintf(
- $conn,
- '(%s, %s)',
- $task->getPHID(),
- $subscriber_phid);
- }
-
- queryfx(
- $conn,
- 'DELETE FROM %T WHERE taskPHID = %s',
- $dao->getTableName(),
- $task->getPHID());
- if ($sql) {
- queryfx(
- $conn,
- 'INSERT INTO %T (taskPHID, subscriberPHID) VALUES %Q',
- $dao->getTableName(),
- implode(', ', $sql));
- }
- }
-
-}
diff --git a/src/applications/maniphest/storage/ManiphestTransaction.php b/src/applications/maniphest/storage/ManiphestTransaction.php
--- a/src/applications/maniphest/storage/ManiphestTransaction.php
+++ b/src/applications/maniphest/storage/ManiphestTransaction.php
@@ -7,7 +7,6 @@
const TYPE_STATUS = 'status';
const TYPE_DESCRIPTION = 'description';
const TYPE_OWNER = 'reassign';
- const TYPE_CCS = 'ccs';
const TYPE_PROJECTS = 'projects';
const TYPE_PRIORITY = 'priority';
const TYPE_EDGE = 'edge';
@@ -21,6 +20,12 @@
// NOTE: this type is deprecated. Keep it around for legacy installs
// so any transactions render correctly.
const TYPE_ATTACH = 'attach';
+ /**
+ * TYPE_CCS is legacy and depracted in favor of
+ * PhabricatorTransactions::TYPE_SUBSCRIBERS; keep it around for legacy
+ * transaction-rendering.
+ */
+ const TYPE_CCS = 'ccs';
const MAILTAG_STATUS = 'maniphest-status';
diff --git a/src/applications/search/controller/PhabricatorSearchAttachController.php b/src/applications/search/controller/PhabricatorSearchAttachController.php
--- a/src/applications/search/controller/PhabricatorSearchAttachController.php
+++ b/src/applications/search/controller/PhabricatorSearchAttachController.php
@@ -143,6 +143,7 @@
$targets = id(new ManiphestTaskQuery())
->setViewer($user)
->withPHIDs(array_keys($phids))
+ ->needSubscriberPHIDs(true)
->execute();
if (empty($targets)) {
@@ -156,9 +157,13 @@
->setContinueOnMissingFields(true);
$cc_vector = array();
- $cc_vector[] = $task->getCCPHIDs();
+ // since we loaded this via a generic object query, go ahead and get the
+ // attach the cc phids now
+ $task->attachSubscriberPHIDs(
+ PhabricatorSubscribersQuery::loadSubscribersForPHID($task->getPHID()));
+ $cc_vector[] = $task->getSubscriberPHIDs();
foreach ($targets as $target) {
- $cc_vector[] = $target->getCCPHIDs();
+ $cc_vector[] = $target->getSubscriberPHIDs();
$cc_vector[] = array(
$target->getAuthorPHID(),
$target->getOwnerPHID(),
@@ -178,8 +183,8 @@
$all_ccs = array_unique($all_ccs);
$add_ccs = id(new ManiphestTransaction())
- ->setTransactionType(ManiphestTransaction::TYPE_CCS)
- ->setNewValue($all_ccs);
+ ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS)
+ ->setNewValue(array('=' => $all_ccs));
$merged_from_txn = id(new ManiphestTransaction())
->setTransactionType(ManiphestTransaction::TYPE_MERGED_FROM)
diff --git a/src/applications/subscriptions/controller/PhabricatorSubscriptionsListController.php b/src/applications/subscriptions/controller/PhabricatorSubscriptionsListController.php
--- a/src/applications/subscriptions/controller/PhabricatorSubscriptionsListController.php
+++ b/src/applications/subscriptions/controller/PhabricatorSubscriptionsListController.php
@@ -23,8 +23,6 @@
if ($object instanceof PhabricatorSubscribableInterface) {
$subscriber_phids = PhabricatorSubscribersQuery::loadSubscribersForPHID(
$phid);
- } else if ($object instanceof ManiphestTask) {
- $subscriber_phids = $object->getCCPHIDs();
}
$handle_phids = $subscriber_phids;
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Mon, Oct 21, 11:45 PM (3 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6713244
Default Alt Text
D10965.id26331.diff (42 KB)
Attached To
Mode
D10965: Maniphest - use subscribers framework properly
Attached
Detach File
Event Timeline
Log In to Comment