From a20f108034126ff7dc45604dec80319e1ec76172 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 7 Feb 2019 14:09:53 -0800 Subject: [PATCH] When an edit overrides an object lock, note it in the transaction record Summary: Ref T13244. See PHI1059. When you lock a task, users who can edit the task can currently override the lock by using "Edit Task" if they confirm that they want to do this. Mark these edits with an emblem, similar to the "MFA" and "Silent" emblems, so it's clear that they may have bent the rules. Also, make the "MFA" and "Silent" emblems more easily visible. Test Plan: Edited a locked task, overrode the lock, got marked for it. {F6195005} Reviewers: amckinley Reviewed By: amckinley Subscribers: aeiser Maniphest Tasks: T13244 Differential Revision: https://secure.phabricator.com/D20131 --- resources/celerity/map.php | 6 ++--- ...habricatorApplicationTransactionEditor.php | 14 +++++++++++ .../PhabricatorApplicationTransaction.php | 14 +++++++++++ .../PhabricatorApplicationTransactionView.php | 3 ++- src/view/phui/PHUIIconView.php | 14 +++++++++++ src/view/phui/PHUITimelineEventView.php | 23 +++++++++++++++++-- webroot/rsrc/css/phui/phui-icon.css | 21 +++++++++++++++++ 7 files changed, 89 insertions(+), 6 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 8312828c6e..cb44dd5585 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,7 +9,7 @@ return array( 'names' => array( 'conpherence.pkg.css' => '3c8a0668', 'conpherence.pkg.js' => '020aebcf', - 'core.pkg.css' => '08baca0c', + 'core.pkg.css' => '7a73ffc5', 'core.pkg.js' => '5c737607', 'differential.pkg.css' => 'b8df73d4', 'differential.pkg.js' => '67c9ea4c', @@ -157,7 +157,7 @@ return array( 'rsrc/css/phui/phui-header-view.css' => '93cea4ec', 'rsrc/css/phui/phui-hovercard.css' => '6ca90fa0', 'rsrc/css/phui/phui-icon-set-selector.css' => '7aa5f3ec', - 'rsrc/css/phui/phui-icon.css' => '281f964d', + 'rsrc/css/phui/phui-icon.css' => '4cbc684a', 'rsrc/css/phui/phui-image-mask.css' => '62c7f4d2', 'rsrc/css/phui/phui-info-view.css' => '37b8d9ce', 'rsrc/css/phui/phui-invisible-character-view.css' => 'c694c4a4', @@ -823,7 +823,7 @@ return array( 'phui-hovercard' => '074f0783', 'phui-hovercard-view-css' => '6ca90fa0', 'phui-icon-set-selector-css' => '7aa5f3ec', - 'phui-icon-view-css' => '281f964d', + 'phui-icon-view-css' => '4cbc684a', 'phui-image-mask-css' => '62c7f4d2', 'phui-info-view-css' => '37b8d9ce', 'phui-inline-comment-view-css' => '48acce5b', diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index 216c1e00a2..bd066e633b 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -1115,6 +1115,16 @@ abstract class PhabricatorApplicationTransactionEditor $transaction_open = true; } + // We can technically test any object for CAN_INTERACT, but we can + // run into some issues in doing so (for example, in project unit tests). + // For now, only test for CAN_INTERACT if the object is explicitly a + // lockable object. + + $was_locked = false; + if ($object instanceof PhabricatorEditEngineLockableInterface) { + $was_locked = !PhabricatorPolicyFilter::canInteract($actor, $object); + } + foreach ($xactions as $xaction) { $this->applyInternalEffects($object, $xaction); } @@ -1132,6 +1142,10 @@ abstract class PhabricatorApplicationTransactionEditor } foreach ($xactions as $xaction) { + if ($was_locked) { + $xaction->setIsLockOverrideTransaction(true); + } + $xaction->setObjectPHID($object->getPHID()); if ($xaction->getComment()) { $xaction->setPHID($xaction->generatePHID()); diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php index acfc4d0cfa..d71728a01f 100644 --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -169,6 +169,14 @@ abstract class PhabricatorApplicationTransaction return (bool)$this->getMetadataValue('core.mfa', false); } + public function setIsLockOverrideTransaction($override) { + return $this->setMetadataValue('core.lock-override', $override); + } + + public function getIsLockOverrideTransaction() { + return (bool)$this->getMetadataValue('core.lock-override', false); + } + public function attachComment( PhabricatorApplicationTransactionComment $comment) { $this->comment = $comment; @@ -1529,6 +1537,12 @@ abstract class PhabricatorApplicationTransaction return false; } } + + // Don't group lock override and non-override transactions together. + $is_override = $this->getIsLockOverrideTransaction(); + if ($is_override != $xaction->getIsLockOverrideTransaction()) { + return false; + } } return true; diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionView.php index c2b32aa190..4d738877b8 100644 --- a/src/applications/transactions/view/PhabricatorApplicationTransactionView.php +++ b/src/applications/transactions/view/PhabricatorApplicationTransactionView.php @@ -416,7 +416,8 @@ class PhabricatorApplicationTransactionView extends AphrontView { ->setColor($xaction->getColor()) ->setHideCommentOptions($this->getHideCommentOptions()) ->setIsSilent($xaction->getIsSilentTransaction()) - ->setIsMFA($xaction->getIsMFATransaction()); + ->setIsMFA($xaction->getIsMFATransaction()) + ->setIsLockOverride($xaction->getIsLockOverrideTransaction()); list($token, $token_removed) = $xaction->getToken(); if ($token) { diff --git a/src/view/phui/PHUIIconView.php b/src/view/phui/PHUIIconView.php index 8cc61ba2eb..d907cb3343 100644 --- a/src/view/phui/PHUIIconView.php +++ b/src/view/phui/PHUIIconView.php @@ -19,6 +19,7 @@ final class PHUIIconView extends AphrontTagView { private $iconColor; private $iconBackground; private $tooltip; + private $emblemColor; public function setHref($href) { $this->href = $href; @@ -66,6 +67,15 @@ final class PHUIIconView extends AphrontTagView { return $this; } + public function setEmblemColor($emblem_color) { + $this->emblemColor = $emblem_color; + return $this; + } + + public function getEmblemColor() { + return $this->emblemColor; + } + protected function getTagName() { $tag = 'span'; if ($this->href) { @@ -106,6 +116,10 @@ final class PHUIIconView extends AphrontTagView { $this->appendChild($this->text); } + if ($this->emblemColor) { + $classes[] = 'phui-icon-emblem phui-icon-emblem-'.$this->emblemColor; + } + $sigil = null; $meta = array(); if ($this->tooltip) { diff --git a/src/view/phui/PHUITimelineEventView.php b/src/view/phui/PHUITimelineEventView.php index 86628058fe..78a75a2063 100644 --- a/src/view/phui/PHUITimelineEventView.php +++ b/src/view/phui/PHUITimelineEventView.php @@ -31,6 +31,7 @@ final class PHUITimelineEventView extends AphrontView { private $pinboardItems = array(); private $isSilent; private $isMFA; + private $isLockOverride; public function setAuthorPHID($author_phid) { $this->authorPHID = $author_phid; @@ -197,6 +198,15 @@ final class PHUITimelineEventView extends AphrontView { return $this->isMFA; } + public function setIsLockOverride($is_override) { + $this->isLockOverride = $is_override; + return $this; + } + + public function getIsLockOverride() { + return $this->isLockOverride; + } + public function setReallyMajorEvent($me) { $this->reallyMajorEvent = $me; return $this; @@ -597,7 +607,8 @@ final class PHUITimelineEventView extends AphrontView { // not expect to have received any mail or notifications. if ($this->getIsSilent()) { $extra[] = id(new PHUIIconView()) - ->setIcon('fa-bell-slash', 'red') + ->setIcon('fa-bell-slash', 'white') + ->setEmblemColor('red') ->setTooltip(pht('Silent Edit')); } @@ -605,9 +616,17 @@ final class PHUITimelineEventView extends AphrontView { // provide a hint that it was extra authentic. if ($this->getIsMFA()) { $extra[] = id(new PHUIIconView()) - ->setIcon('fa-vcard', 'pink') + ->setIcon('fa-vcard', 'white') + ->setEmblemColor('pink') ->setTooltip(pht('MFA Authenticated')); } + + if ($this->getIsLockOverride()) { + $extra[] = id(new PHUIIconView()) + ->setIcon('fa-chain-broken', 'white') + ->setEmblemColor('violet') + ->setTooltip(pht('Lock Overridden')); + } } $extra = javelin_tag( diff --git a/webroot/rsrc/css/phui/phui-icon.css b/webroot/rsrc/css/phui/phui-icon.css index 4108074b08..5436bb04b1 100644 --- a/webroot/rsrc/css/phui/phui-icon.css +++ b/webroot/rsrc/css/phui/phui-icon.css @@ -183,3 +183,24 @@ a.phui-icon-view.phui-icon-square:hover { text-decoration: none; color: #fff; } + + +.phui-icon-emblem { + border-radius: 4px; +} + +.phui-timeline-extra .phui-icon-emblem { + padding: 4px 6px; +} + +.phui-icon-emblem-violet { + background-color: {$violet}; +} + +.phui-icon-emblem-red { + background-color: {$red}; +} + +.phui-icon-emblem-pink { + background-color: {$pink}; +}