From de66a8ece185b6769bc5b55d82ea8a11a1b18a17 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 18 Nov 2019 21:53:33 -0800 Subject: [PATCH] Remove "stronger/weaker" policy color hints from object headers Summary: Fixes T13461. Some applications provide hints about policy strength in the header, but these hints are inconsistent and somewhat confusing. They don't make much sense for modern objects with Custom Forms, which don't have a single "default" policy. Remove this feature since it seems to be confusing things more than illuminating them. Test Plan: - Viewed various objects, no longer saw colored policy hints. - Grepped for all removed symbols. Maniphest Tasks: T13461 Differential Revision: https://secure.phabricator.com/D20918 --- resources/celerity/map.php | 6 +- src/__phutil_library_map__.php | 2 - .../codex/PhrictionDocumentPolicyCodex.php | 17 ----- .../policy/codex/PhabricatorPolicyCodex.php | 4 -- .../PhabricatorPolicyStrengthConstants.php | 9 --- .../PhabricatorPolicyExplainController.php | 68 ------------------- src/view/phui/PHUIHeaderView.php | 47 ------------- webroot/rsrc/css/phui/phui-header-view.css | 27 -------- 8 files changed, 3 insertions(+), 177 deletions(-) delete mode 100644 src/applications/policy/constants/PhabricatorPolicyStrengthConstants.php diff --git a/resources/celerity/map.php b/resources/celerity/map.php index ffe4024ed7..23f002e36b 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' => '77de226f', + 'core.pkg.css' => 'b88ac037', 'core.pkg.js' => '705aec2c', 'differential.pkg.css' => '607c84be', 'differential.pkg.js' => '1b97518d', @@ -155,7 +155,7 @@ return array( 'rsrc/css/phui/phui-form-view.css' => '01b796c0', 'rsrc/css/phui/phui-form.css' => '159e2d9c', 'rsrc/css/phui/phui-head-thing.css' => 'd7f293df', - 'rsrc/css/phui/phui-header-view.css' => 'b500eeea', + 'rsrc/css/phui/phui-header-view.css' => 'be09cc83', 'rsrc/css/phui/phui-hovercard.css' => '6ca90fa0', 'rsrc/css/phui/phui-icon-set-selector.css' => '7aa5f3ec', 'rsrc/css/phui/phui-icon.css' => '4cbc684a', @@ -843,7 +843,7 @@ return array( 'phui-form-css' => '159e2d9c', 'phui-form-view-css' => '01b796c0', 'phui-head-thing-view-css' => 'd7f293df', - 'phui-header-view-css' => 'b500eeea', + 'phui-header-view-css' => 'be09cc83', 'phui-hovercard' => '074f0783', 'phui-hovercard-view-css' => '6ca90fa0', 'phui-icon-set-selector-css' => '7aa5f3ec', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 0927f450b2..682dfff366 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -4218,7 +4218,6 @@ phutil_register_library_map(array( 'PhabricatorPolicyRule' => 'applications/policy/rule/PhabricatorPolicyRule.php', 'PhabricatorPolicyRulesView' => 'applications/policy/view/PhabricatorPolicyRulesView.php', 'PhabricatorPolicySearchEngineExtension' => 'applications/policy/engineextension/PhabricatorPolicySearchEngineExtension.php', - 'PhabricatorPolicyStrengthConstants' => 'applications/policy/constants/PhabricatorPolicyStrengthConstants.php', 'PhabricatorPolicyTestCase' => 'applications/policy/__tests__/PhabricatorPolicyTestCase.php', 'PhabricatorPolicyTestObject' => 'applications/policy/__tests__/PhabricatorPolicyTestObject.php', 'PhabricatorPolicyType' => 'applications/policy/constants/PhabricatorPolicyType.php', @@ -10730,7 +10729,6 @@ phutil_register_library_map(array( 'PhabricatorPolicyRule' => 'Phobject', 'PhabricatorPolicyRulesView' => 'AphrontView', 'PhabricatorPolicySearchEngineExtension' => 'PhabricatorSearchEngineExtension', - 'PhabricatorPolicyStrengthConstants' => 'PhabricatorPolicyConstants', 'PhabricatorPolicyTestCase' => 'PhabricatorTestCase', 'PhabricatorPolicyTestObject' => array( 'Phobject', diff --git a/src/applications/phriction/codex/PhrictionDocumentPolicyCodex.php b/src/applications/phriction/codex/PhrictionDocumentPolicyCodex.php index 748bec705b..f4536cad7c 100644 --- a/src/applications/phriction/codex/PhrictionDocumentPolicyCodex.php +++ b/src/applications/phriction/codex/PhrictionDocumentPolicyCodex.php @@ -41,23 +41,6 @@ final class PhrictionDocumentPolicyCodex ->executeOne(); } - public function compareToDefaultPolicy(PhabricatorPolicy $policy) { - $root_policy = $this->getDefaultPolicy(); - $strongest_policy = $this->getStrongestPolicy(); - - // Note that we never return 'weaker', because Phriction documents can - // never have weaker permissions than their parents. If this object has - // been set to weaker permissions anyway, return 'adjusted'. - if ($root_policy == $strongest_policy) { - $strength = null; - } else if ($strongest_policy->isStrongerThan($root_policy)) { - $strength = PhabricatorPolicyStrengthConstants::STRONGER; - } else { - $strength = PhabricatorPolicyStrengthConstants::ADJUSTED; - } - return $strength; - } - private function getStrongestPolicy() { $ancestors = $this->getObject()->getAncestors(); $ancestors[] = $this->getObject(); diff --git a/src/applications/policy/codex/PhabricatorPolicyCodex.php b/src/applications/policy/codex/PhabricatorPolicyCodex.php index 8dee2a38d1..9bccb842bb 100644 --- a/src/applications/policy/codex/PhabricatorPolicyCodex.php +++ b/src/applications/policy/codex/PhabricatorPolicyCodex.php @@ -40,10 +40,6 @@ abstract class PhabricatorPolicyCodex $this->capability); } - public function compareToDefaultPolicy(PhabricatorPolicy $policy) { - return null; - } - final protected function newRule() { return new PhabricatorPolicyCodexRuleDescription(); } diff --git a/src/applications/policy/constants/PhabricatorPolicyStrengthConstants.php b/src/applications/policy/constants/PhabricatorPolicyStrengthConstants.php deleted file mode 100644 index 9bc3c81ca2..0000000000 --- a/src/applications/policy/constants/PhabricatorPolicyStrengthConstants.php +++ /dev/null @@ -1,9 +0,0 @@ -getViewer(); - - - $strength = null; - if ($object instanceof PhabricatorPolicyCodexInterface) { - $codex = id(PhabricatorPolicyCodex::newFromObject($object, $viewer)) - ->setCapability($capability); - $strength = $codex->compareToDefaultPolicy($policy); - $default_policy = $codex->getDefaultPolicy(); - } else { - $default_policy = PhabricatorPolicyQuery::getDefaultPolicyForObject( - $viewer, - $object, - $capability); - - if ($default_policy) { - if ($default_policy->getPHID() == $policy->getPHID()) { - return; - } - - if ($default_policy->getPHID() != $policy->getPHID()) { - if ($default_policy->isStrongerThan($policy)) { - $strength = PhabricatorPolicyStrengthConstants::WEAKER; - } else if ($policy->isStrongerThan($default_policy)) { - $strength = PhabricatorPolicyStrengthConstants::STRONGER; - } else { - $strength = PhabricatorPolicyStrengthConstants::ADJUSTED; - } - } - } - } - - if (!$strength) { - return; - } - - if ($strength == PhabricatorPolicyStrengthConstants::WEAKER) { - $info = pht( - 'This object has a less restrictive policy ("%s") than the default '. - 'policy for similar objects (which is "%s").', - $policy->getShortName(), - $default_policy->getShortName()); - } else if ($strength == PhabricatorPolicyStrengthConstants::STRONGER) { - $info = pht( - 'This object has a more restrictive policy ("%s") than the default '. - 'policy for similar objects (which is "%s").', - $policy->getShortName(), - $default_policy->getShortName()); - } else { - $info = pht( - 'This object has a different policy ("%s") than the default policy '. - 'for similar objects (which is "%s").', - $policy->getShortName(), - $default_policy->getShortName()); - } - - return $info; - } - private function getCapabilityName($capability) { $capability_name = $capability; $capobj = PhabricatorPolicyCapability::getCapabilityByKey($capability); @@ -344,11 +281,6 @@ final class PhabricatorPolicyExplainController $object_section->appendRulesView($rules_view); } - $strength = $this->getStrengthInformation($object, $policy, $capability); - if ($strength) { - $object_section->appendHint($strength); - } - return $object_section; } diff --git a/src/view/phui/PHUIHeaderView.php b/src/view/phui/PHUIHeaderView.php index 54f4fa58ee..465768ae16 100644 --- a/src/view/phui/PHUIHeaderView.php +++ b/src/view/phui/PHUIHeaderView.php @@ -469,53 +469,6 @@ final class PHUIHeaderView extends AphrontTagView { $container_classes[] = 'policy-header-callout'; $phid = $object->getPHID(); - // If we're going to show the object policy, try to determine if the object - // policy differs from the default policy. If it does, we'll call it out - // as changed. - if (!$use_space_policy) { - $strength = null; - if ($object instanceof PhabricatorPolicyCodexInterface) { - $codex = id(PhabricatorPolicyCodex::newFromObject($object, $viewer)) - ->setCapability($view_capability); - $strength = $codex->compareToDefaultPolicy($policy); - } else { - $default_policy = PhabricatorPolicyQuery::getDefaultPolicyForObject( - $viewer, - $object, - $view_capability); - - if ($default_policy) { - if ($default_policy->getPHID() != $policy->getPHID()) { - if ($default_policy->isStrongerThan($policy)) { - $strength = PhabricatorPolicyStrengthConstants::WEAKER; - } else if ($policy->isStrongerThan($default_policy)) { - $strength = PhabricatorPolicyStrengthConstants::STRONGER; - } else { - $strength = PhabricatorPolicyStrengthConstants::ADJUSTED; - } - } - } - } - - if ($strength) { - if ($strength == PhabricatorPolicyStrengthConstants::WEAKER) { - // The policy has strictly been weakened. For example, the - // default might be "All Users" and the current policy is "Public". - $container_classes[] = 'policy-adjusted-weaker'; - } else if ($strength == PhabricatorPolicyStrengthConstants::STRONGER) { - // The policy has strictly been strengthened, and is now more - // restrictive than the default. For example, "All Users" has - // been replaced with "No One". - $container_classes[] = 'policy-adjusted-stronger'; - } else { - // The policy has been adjusted but not strictly strengthened - // or weakened. For example, "Members of X" has been replaced with - // "Members of Y". - $container_classes[] = 'policy-adjusted-different'; - } - } - } - $policy_name = array($policy->getShortName()); $policy_icon = $policy->getIcon().' bluegrey'; diff --git a/webroot/rsrc/css/phui/phui-header-view.css b/webroot/rsrc/css/phui/phui-header-view.css index 1d851f04ee..e621d38134 100644 --- a/webroot/rsrc/css/phui/phui-header-view.css +++ b/webroot/rsrc/css/phui/phui-header-view.css @@ -213,33 +213,6 @@ body .phui-header-shell.phui-bleed-header -webkit-font-smoothing: auto; } -.policy-header-callout.policy-adjusted-weaker { - background: {$sh-greenbackground}; -} - -.policy-header-callout.policy-adjusted-weaker .policy-link, -.policy-header-callout.policy-adjusted-weaker .phui-icon-view { - color: {$sh-greentext}; -} - -.policy-header-callout.policy-adjusted-stronger { - background: {$sh-redbackground}; -} - -.policy-header-callout.policy-adjusted-stronger .policy-link, -.policy-header-callout.policy-adjusted-stronger .phui-icon-view { - color: {$sh-redtext}; -} - -.policy-header-callout.policy-adjusted-different { - background: {$sh-orangebackground}; -} - -.policy-header-callout.policy-adjusted-different .policy-link, -.policy-header-callout.policy-adjusted-different .phui-icon-view { - color: {$sh-orangetext}; -} - .policy-header-callout.policy-adjusted-special { background: {$sh-indigobackground}; }