1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2025-01-02 02:40:58 +01:00

Fix some CC handling in Maniphest

Summary:
Fixes T6932. Fixes some issues from D11303.

  - When claiming a task, if it was previously unassigned, we would try to CC `null`.
  - When claiming a task, if the current owner was already CC'd, the viewer would incorrectly be warned about all subscribers being CC'd.

Test Plan:
  - Claimed an unclaimed task.
  - Claimed a task with owner CC'd.

Reviewers: btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T6932

Differential Revision: https://secure.phabricator.com/D11336
This commit is contained in:
epriestley 2015-01-11 17:57:20 -08:00
parent 698b7f9ea3
commit 5b3b9b7182

View file

@ -22,11 +22,8 @@ final class ManiphestTransactionSaveController extends ManiphestController {
$action = $request->getStr('action'); $action = $request->getStr('action');
$added_ccs = array(); $implicit_ccs = array();
$explicit_ccs = array();
$cc_transaction = new ManiphestTransaction();
$cc_transaction
->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS);
$transaction = new ManiphestTransaction(); $transaction = new ManiphestTransaction();
$transaction $transaction
@ -58,7 +55,7 @@ final class ManiphestTransactionSaveController extends ManiphestController {
case PhabricatorTransactions::TYPE_SUBSCRIBERS: case PhabricatorTransactions::TYPE_SUBSCRIBERS:
// Accumulate the new explicit CCs into the array that we'll add in // Accumulate the new explicit CCs into the array that we'll add in
// the CC transaction later. // the CC transaction later.
$added_ccs = $request->getArr('ccs'); $explicit_ccs = $request->getArr('ccs');
// Throw away the primary transaction. // Throw away the primary transaction.
$transaction = null; $transaction = null;
@ -91,7 +88,9 @@ final class ManiphestTransactionSaveController extends ManiphestController {
// If this is actually no-op, don't generate the side effect. // If this is actually no-op, don't generate the side effect.
} else { } else {
// Otherwise, when a task is reassigned, move the previous owner to CC. // Otherwise, when a task is reassigned, move the previous owner to CC.
$added_ccs[] = $task->getOwnerPHID(); if ($task->getOwnerPHID()) {
$implicit_ccs[] = $task->getOwnerPHID();
}
} }
} }
@ -127,14 +126,25 @@ final class ManiphestTransactionSaveController extends ManiphestController {
// If we aren't making the user the new task owner and they aren't the // 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. // existing task owner, add them to CC unless they're aleady CC'd.
if (!in_array($user->getPHID(), $task->getSubscriberPHIDs())) { if (!in_array($user->getPHID(), $task->getSubscriberPHIDs())) {
$added_ccs[] = $user->getPHID(); $implicit_ccs[] = $user->getPHID();
} }
} }
if ($added_ccs) { if ($implicit_ccs || $explicit_ccs) {
// We've added CCs, so include a CC transaction.
$all_ccs = array_merge($task->getSubscriberPHIDs(), $added_ccs); // TODO: These implicit CC rules should probably be handled inside the
$cc_transaction->setNewValue(array('=' => $all_ccs)); // Editor, eventually.
$all_ccs = array_fuse($implicit_ccs) + array_fuse($explicit_ccs);
$cc_transaction = id(new ManiphestTransaction())
->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS)
->setNewValue(array('+' => $all_ccs));
if (!$explicit_ccs) {
$cc_transaction->setIgnoreOnNoEffect(true);
}
$transactions[] = $cc_transaction; $transactions[] = $cc_transaction;
} }