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;