mirror of
https://we.phorge.it/source/phorge.git
synced 2025-02-02 09:58:24 +01:00
Don't mark subscribes/mentions as "Lock Overridden"
Summary: See PHI1209. When a task is in "Hard Lock" mode, it's still possible to apply some changes to it. Notably: - You can subscribe/unsubscribe. - You can mention it on another object. - You can add a relationship from some other object to it (e.g., select it as a "Parent Task" for some other task). Currently, these types of edits will show a "Lock Overridden" timeline emblem icon. However, they should not: you didn't override a lock to make these changes, they just bypass locks. For now, special case these cases (self subscribe/unsubscribe + inverse edge edits) so they don't get the little icon, since I think this list is exhaustive today. Some day we should modularize this, but we'd need code like this anyway (since TYPE_SUBSCRIBE is not modular yet), and this seems unlikely to cause problems even if it's a bit rough. Test Plan: - Hard-locked a task. - Subscribed/unsubscribed, mentioned, relationship'd it as a non-author. No timeline emblems. - Soft-locked a task. - Subscribed/unsubscribed, mentioned, relationship'd it, no timeline emblems. - Clicked "Edit", answered "yes" to the override prompt, edited it. Got a timeline emblem. - Added some comments and stuff to a normal non-locked task, no emblems. Reviewers: amckinley Reviewed By: amckinley Differential Revision: https://secure.phabricator.com/D20513
This commit is contained in:
parent
ee89c2fad9
commit
3e0391c574
1 changed files with 53 additions and 1 deletions
|
@ -1164,7 +1164,10 @@ abstract class PhabricatorApplicationTransactionEditor
|
|||
|
||||
foreach ($xactions as $xaction) {
|
||||
if ($was_locked) {
|
||||
$xaction->setIsLockOverrideTransaction(true);
|
||||
$is_override = $this->isLockOverrideTransaction($xaction);
|
||||
if ($is_override) {
|
||||
$xaction->setIsLockOverrideTransaction(true);
|
||||
}
|
||||
}
|
||||
|
||||
$xaction->setObjectPHID($object->getPHID());
|
||||
|
@ -5204,6 +5207,55 @@ abstract class PhabricatorApplicationTransactionEditor
|
|||
return $xaction->getBodyForMail();
|
||||
}
|
||||
|
||||
private function isLockOverrideTransaction(
|
||||
PhabricatorApplicationTransaction $xaction) {
|
||||
|
||||
// See PHI1209. When an object is locked, certain types of transactions
|
||||
// can still be applied without requiring a policy check, like subscribing
|
||||
// or unsubscribing. We don't want these transactions to show the "Lock
|
||||
// Override" icon in the transaction timeline.
|
||||
|
||||
// We could test if a transaction did no direct policy checks, but it may
|
||||
// have done additional policy checks during validation, so this is not a
|
||||
// reliable test (and could cause false negatives, where edits which did
|
||||
// override a lock are not marked properly).
|
||||
|
||||
// For now, do this in a narrow way and just check against a hard-coded
|
||||
// list of non-override transaction situations. Some day, this should
|
||||
// likely be modularized.
|
||||
|
||||
|
||||
// Inverse edge edits don't interact with locks.
|
||||
if ($this->getIsInverseEdgeEditor()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// For now, all edits other than subscribes always override locks.
|
||||
$type = $xaction->getTransactionType();
|
||||
if ($type !== PhabricatorTransactions::TYPE_SUBSCRIBERS) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Subscribes override locks if they affect any users other than the
|
||||
// acting user.
|
||||
|
||||
$acting_phid = $this->getActingAsPHID();
|
||||
|
||||
$old = array_fuse($xaction->getOldValue());
|
||||
$new = array_fuse($xaction->getNewValue());
|
||||
$add = array_diff_key($new, $old);
|
||||
$rem = array_diff_key($old, $new);
|
||||
|
||||
$all = $add + $rem;
|
||||
foreach ($all as $phid) {
|
||||
if ($phid !== $acting_phid) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
|
||||
/* -( Extensions )--------------------------------------------------------- */
|
||||
|
||||
|
|
Loading…
Add table
Reference in a new issue