D10965.diff
No OneTemporary
Actions

Size

40 KB

Referenced Files

None

Subscribers

None

D10965.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/resources/sql/autopatches/20141210.maniphestsubscribersmig.2.sql b/resources/sql/autopatches/20141210.maniphestsubscribersmig.2.sql
	new file mode 100644
	--- /dev/null
	+++ b/resources/sql/autopatches/20141210.maniphestsubscribersmig.2.sql
	@@ -0,0 +1,2 @@
	+ALTER TABLE {$NAMESPACE}_maniphest.maniphest_task
	+ DROP ccPHIDs;
	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',
	@@ -4071,9 +4070,9 @@
	'ManiphestSearchIndexer' => 'PhabricatorSearchDocumentIndexer',
	'ManiphestStatusConfigOptionType' => 'PhabricatorConfigJSONOptionType',
	'ManiphestSubpriorityController' => 'ManiphestController',
	- 'ManiphestSubscribeController' => 'ManiphestController',
	'ManiphestTask' => array(
	'ManiphestDAO',
	+ 'PhabricatorSubscribableInterface',
	'PhabricatorMarkupInterface',
	'PhabricatorPolicyInterface',
	'PhabricatorTokenReceiverInterface',
	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,6 @@
	private function buildActionView(ManiphestTask $task) {
	$viewer = $this->getRequest()->getUser();
	$viewer_phid = $viewer->getPHID();
	- $viewer_is_cc = in_array($viewer_phid, $task->getCCPHIDs());

	$id = $task->getID();
	$phid = $task->getPHID();
	@@ -420,26 +414,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 +464,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
	--- a/src/applications/maniphest/storage/ManiphestTaskSubscriber.php
	+++ b/src/applications/maniphest/storage/ManiphestTaskSubscriber.php
	@@ -1,5 +1,8 @@
	<?php

	+/**
	+ * Deprecated; delete me.
	+ */
	final class ManiphestTaskSubscriber extends ManiphestDAO {

	protected $taskPHID;
	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

Mime Type: text/plain
Expires: Thu, Nov 20, 10:40 AM (1 w, 12 h ago)
Storage Engine: amazon-s3
Storage Format: Encrypted (AES-256-CBC)
Storage Handle: phabricator/secure/nz/oy/4ldw52yzmo4b6fy4
Default Alt Text: D10965.diff (40 KB)

D10965.diffNo OneTemporaryActions

D10965.diffView Options

File Metadata

Event Timeline

D10965.diff
No OneTemporary
Actions

D10965.diff
View Options