Differential D11049 Diff 26531 src/applications/maniphest/controller/ManiphestTransactionSaveController.php

Changeset View

Standalone View

src/applications/maniphest/controller/ManiphestTransactionSaveController.php

Show All 16 Lines	public function processRequest() {
}		}

$task_uri = '/'.$task->getMonogram();		$task_uri = '/'.$task->getMonogram();

$transactions = array();		$transactions = array();

$action = $request->getStr('action');		$action = $request->getStr('action');

// Compute new CCs added by @mentions. Several things can cause CCs to		// Compute new CCs added by @mentions. Several things can cause CCs to
// be added as side effects: mentions, explicit CCs, users who aren't		// be added as side effects: mentions, explicit CCs, users who aren't
// CC'd interacting with the task, and ownership changes. We build up a		// CC'd interacting with the task, and ownership changes. We build up a
// list of all the CCs and then construct a transaction for them at the		// list of all the CCs and then construct a transaction for them at the
// end if necessary.		// end if necessary.
$added_ccs = PhabricatorMarkupEngine::extractPHIDsFromMentions(		$added_ccs = PhabricatorMarkupEngine::extractPHIDsFromMentions(
$user,		$user,
array(		array(
$request->getStr('comments'),		$request->getStr('comments'),
));		));
		epriestleyUnsubmitted Not Done Inline Actions Replace this code with `$added_ccs = array();`, then... epriestley: Replace this code with `$added_ccs = array();`, then...

$cc_transaction = new ManiphestTransaction();		$cc_transaction = new ManiphestTransaction();
$cc_transaction		$cc_transaction
->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS);		->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS);

$transaction = new ManiphestTransaction();		$transaction = new ManiphestTransaction();
$transaction		$transaction
->setTransactionType($action);		->setTransactionType($action);
Show All 19 Lines	switch ($action) {
->setNewValue(		->setNewValue(
array(		array(
'+' => array_fuse($projects),		'+' => array_fuse($projects),
));		));
break;		break;
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 = array_merge($added_ccs, $request->getArr('ccs'));		$added_ccs = array_merge($added_ccs, $request->getArr('ccs'));
		epriestleyUnsubmitted Not Done Inline Actions Change this to `$added_ccs = $request->getArr('ccs');`. epriestley: Change this to `$added_ccs = $request->getArr('ccs');`.

// Throw away the primary transaction.		// Throw away the primary transaction.
$transaction = null;		$transaction = null;
break;		break;
case ManiphestTransaction::TYPE_PRIORITY:		case ManiphestTransaction::TYPE_PRIORITY:
$transaction->setNewValue($request->getInt('priority'));		$transaction->setNewValue($request->getInt('priority'));
break;		break;
case PhabricatorTransactions::TYPE_COMMENT:		case PhabricatorTransactions::TYPE_COMMENT:
▲ Show 20 Lines • Show All 63 Lines • ▼ Show 20 Lines	public function processRequest() {

// Evade no-effect detection in the new editor stuff until we can switch		// Evade no-effect detection in the new editor stuff until we can switch
// to subscriptions.		// to subscriptions.
$added_ccs = array_filter(array_diff(		$added_ccs = array_filter(array_diff(
$added_ccs,		$added_ccs,
$task->getSubscriberPHIDs()));		$task->getSubscriberPHIDs()));

if ($added_ccs) {		if ($added_ccs) {
		$users = id(new PhabricatorPeopleQuery())
		->withPHIDs($added_ccs)
		->loadPage();
		$users = mpull($users, null, 'getPHID');

		// Check if the added_ccs have CAN_VIEW permission on the object
		foreach ($added_ccs as $key => $added_cc) {
		if (!PhabricatorPolicyFilter::hasCapability(
		$users[$added_cc],
		$task,
		PhabricatorPolicyCapability::CAN_VIEW)
		) {
		unset($added_ccs[$key]);
		}
		}

		if ($added_ccs) {
// We've added CCs, so include a CC transaction.		// We've added CCs, so include a CC transaction.
$all_ccs = array_merge($task->getSubscriberPHIDs(), $added_ccs);		$all_ccs = array_merge($task->getSubscriberPHIDs(), $added_ccs);
$cc_transaction->setNewValue(array('=' => $all_ccs));		$cc_transaction->setNewValue(array('=' => $all_ccs));
$transactions[] = $cc_transaction;		$transactions[] = $cc_transaction;
}		}
		}
		epriestleyUnsubmitted Not Done Inline Actions I think this should be unnecessary. Even though this code isn't fully modernized, it ultimately goes through the PhabricatorApplicationTransactionEditor pathway, so it should hit the other change. Or was that not the case? Oh, is this handling "Add CCs: ..." explicitly? Let's just do mentions first in this diff, I think the various ways to "Add CCs:" will be a bit more complicated. See note later on. epriestley: I //think// this should be unnecessary. Even though this code isn't fully modernized, it…
		fabeAuthorUnsubmitted Not Done Inline Actions It just seems to be used when commenting on a task. If i remove i can comment an mention users having no permission and they will get added. Manually adding cc's via the "Edit task" function is unaffected by this. The comment above is quite suspicious: "// Evade no-effect detection in the new editor stuff until we can switch to subscriptions." fabe: It just seems to be used when commenting on a task. If i remove i can comment an mention users…
		epriestleyUnsubmitted Not Done Inline Actions Let's not do this yet -- this prevents explicit CC's, right? We can move that to another diff later on. epriestley: Let's not do this yet -- this prevents explicit CC's, right? We can move that to another diff…
		fabeAuthorUnsubmitted Not Done Inline Actions No this is actually needed for checking the policy when commenting on a task. Every other place i found runs through the TransactionEditor but not the task comments. fabe: No this is actually needed for checking the policy when commenting on a task. Every other place…

$comments = $request->getStr('comments');		$comments = $request->getStr('comments');
if (strlen($comments) \|\| !$transactions) {		if (strlen($comments) \|\| !$transactions) {
$transactions[] = id(new ManiphestTransaction())		$transactions[] = id(new ManiphestTransaction())
->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)		->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
->attachComment(		->attachComment(
id(new ManiphestTransactionComment())		id(new ManiphestTransactionComment())
->setContent($comments));		->setContent($comments));
Show All 15 Lines	public function processRequest() {

$editor = id(new ManiphestTransactionEditor())		$editor = id(new ManiphestTransactionEditor())
->setActor($user)		->setActor($user)
->setContentSourceFromRequest($request)		->setContentSourceFromRequest($request)
->setContinueOnMissingFields(true)		->setContinueOnMissingFields(true)
->setContinueOnNoEffect($request->isContinueRequest());		->setContinueOnNoEffect($request->isContinueRequest());

try {		try {
$editor->applyTransactions($task, $transactions);		$editor->applyTransactions($task, $transactions);
		epriestleyUnsubmitted Not Done Inline Actions I still don't think the block above is required -- we'll go into the transaction editor here, and it seems to works fine locally for me. When this block is removed I can still "Add CCs", but that's expected and desirable (for now). `@mentioning` users correctly greys out the ones who can't see the object. Am I just doing something wrong? epriestley: I still don't think the block above is required -- we'll go into the transaction editor here…
} catch (PhabricatorApplicationTransactionNoEffectException $ex) {		} catch (PhabricatorApplicationTransactionNoEffectException $ex) {
return id(new PhabricatorApplicationTransactionNoEffectResponse())		return id(new PhabricatorApplicationTransactionNoEffectResponse())
->setCancelURI($task_uri)		->setCancelURI($task_uri)
->setException($ex);		->setException($ex);
}		}

$draft = id(new PhabricatorDraft())->loadOneWhere(		$draft = id(new PhabricatorDraft())->loadOneWhere(
'authorPHID = %s AND draftKey = %s',		'authorPHID = %s AND draftKey = %s',
Show All 21 Lines