1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-26 16:52:41 +01:00

Give policy name rendering explicit "text name", "capability link", and "transaction link" pathways

Summary:
Ref T13411. This cleans up policy name rendering. We ultimately render into three contexts:

  - Plain text contexts, like `bin/policy show`.
  - Transaction contexts, where we're showing a policy change. In these cases, we link some policies (like project policies and custom policies) but the links go directly to the relevant object or a minimal explanation of the change. We don't link policies like "All Users".
  - Capability contexts, where we're describing a capability, like "Can Push" or cases in Applicaitons. In these cases, we link all policies to the full policy explanation flow.

Test Plan:
  - Used `bin/policy show` to examine the policy of an object with a project policy, no longer saw HTML.
  - Viewed the transaction logs of Applications (ModularTransactions) and Tasks (not ModularTransactions) with policy edits, including project and custom policies.
  - Clicked "Custom Policy" in both logs, got consistent dialogs.
  - Viewed application detail pages, saw all capabities linked to explanatory capability dialogs. The value of having this dialog is that the user can get a full explanation of special rules even if the policy is something mundane like "All Users".

Maniphest Tasks: T13411

Differential Revision: https://secure.phabricator.com/D20804
This commit is contained in:
epriestley 2019-09-12 07:23:43 -07:00
parent c9b0d107f0
commit 506f93b4a3
8 changed files with 146 additions and 50 deletions

View file

@ -4195,6 +4195,7 @@ phutil_register_library_map(array(
'PhabricatorPolicyManagementWorkflow' => 'applications/policy/management/PhabricatorPolicyManagementWorkflow.php', 'PhabricatorPolicyManagementWorkflow' => 'applications/policy/management/PhabricatorPolicyManagementWorkflow.php',
'PhabricatorPolicyPHIDTypePolicy' => 'applications/policy/phid/PhabricatorPolicyPHIDTypePolicy.php', 'PhabricatorPolicyPHIDTypePolicy' => 'applications/policy/phid/PhabricatorPolicyPHIDTypePolicy.php',
'PhabricatorPolicyQuery' => 'applications/policy/query/PhabricatorPolicyQuery.php', 'PhabricatorPolicyQuery' => 'applications/policy/query/PhabricatorPolicyQuery.php',
'PhabricatorPolicyRef' => 'applications/policy/view/PhabricatorPolicyRef.php',
'PhabricatorPolicyRequestExceptionHandler' => 'aphront/handler/PhabricatorPolicyRequestExceptionHandler.php', 'PhabricatorPolicyRequestExceptionHandler' => 'aphront/handler/PhabricatorPolicyRequestExceptionHandler.php',
'PhabricatorPolicyRule' => 'applications/policy/rule/PhabricatorPolicyRule.php', 'PhabricatorPolicyRule' => 'applications/policy/rule/PhabricatorPolicyRule.php',
'PhabricatorPolicySearchEngineExtension' => 'applications/policy/engineextension/PhabricatorPolicySearchEngineExtension.php', 'PhabricatorPolicySearchEngineExtension' => 'applications/policy/engineextension/PhabricatorPolicySearchEngineExtension.php',
@ -10675,6 +10676,7 @@ phutil_register_library_map(array(
'PhabricatorPolicyManagementWorkflow' => 'PhabricatorManagementWorkflow', 'PhabricatorPolicyManagementWorkflow' => 'PhabricatorManagementWorkflow',
'PhabricatorPolicyPHIDTypePolicy' => 'PhabricatorPHIDType', 'PhabricatorPolicyPHIDTypePolicy' => 'PhabricatorPHIDType',
'PhabricatorPolicyQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorPolicyQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'PhabricatorPolicyRef' => 'Phobject',
'PhabricatorPolicyRequestExceptionHandler' => 'PhabricatorRequestExceptionHandler', 'PhabricatorPolicyRequestExceptionHandler' => 'PhabricatorRequestExceptionHandler',
'PhabricatorPolicyRule' => 'Phobject', 'PhabricatorPolicyRule' => 'Phobject',
'PhabricatorPolicySearchEngineExtension' => 'PhabricatorSearchEngineExtension', 'PhabricatorPolicySearchEngineExtension' => 'PhabricatorSearchEngineExtension',

View file

@ -60,8 +60,10 @@ final class PhabricatorPolicyManagementShowWorkflow
$console->writeOut("__%s__\n\n", pht('CAPABILITIES')); $console->writeOut("__%s__\n\n", pht('CAPABILITIES'));
foreach ($policies as $capability => $policy) { foreach ($policies as $capability => $policy) {
$ref = $policy->newRef($viewer);
$console->writeOut(" **%s**\n", $capability); $console->writeOut(" **%s**\n", $capability);
$console->writeOut(" %s\n", $policy->renderDescription()); $console->writeOut(" %s\n", $ref->getPolicyDisplayName());
$console->writeOut(" %s\n", $console->writeOut(" %s\n",
PhabricatorPolicy::getPolicyExplanation($viewer, $policy->getPHID())); PhabricatorPolicy::getPolicyExplanation($viewer, $policy->getPHID()));
$console->writeOut("\n"); $console->writeOut("\n");

View file

@ -48,7 +48,8 @@ final class PhabricatorPolicyQuery
$policies = self::loadPolicies($viewer, $object); $policies = self::loadPolicies($viewer, $object);
foreach ($policies as $capability => $policy) { foreach ($policies as $capability => $policy) {
$policies[$capability] = $policy->renderDescription(); $policies[$capability] = $policy->newRef($viewer)
->newCapabilityLink($object, $capability);
} }
return $policies; return $policies;

View file

@ -276,32 +276,22 @@ final class PhabricatorPolicy
} }
} }
public function renderDescription() { public function newRef(PhabricatorUser $viewer) {
if ($this->getHref()) { return id(new PhabricatorPolicyRef())
$desc = javelin_tag( ->setViewer($viewer)
'a', ->setPolicy($this);
array( }
'href' => $this->getHref(),
'class' => 'policy-link',
'sigil' => $this->getWorkflow() ? 'workflow' : null,
),
$this->getName());
} else {
$desc = $this->getName();
}
switch ($this->getType()) { public function isProjectPolicy() {
case PhabricatorPolicyType::TYPE_PROJECT: return ($this->getType() === PhabricatorPolicyType::TYPE_PROJECT);
return pht('%s (Project)', $desc); }
case PhabricatorPolicyType::TYPE_CUSTOM:
return $desc; public function isCustomPolicy() {
case PhabricatorPolicyType::TYPE_MASKED: return ($this->getType() === PhabricatorPolicyType::TYPE_CUSTOM);
return pht( }
'%s (You do not have permission to view policy details.)',
$desc); public function isMaskedPolicy() {
default: return ($this->getType() === PhabricatorPolicyType::TYPE_MASKED);
return $desc;
}
} }
/** /**

View file

@ -0,0 +1,99 @@
<?php
final class PhabricatorPolicyRef
extends Phobject {
private $viewer;
private $policy;
public function setViewer(PhabricatorUser $viewer) {
$this->viewer = $viewer;
return $this;
}
public function getViewer() {
return $this->viewer;
}
public function setPolicy(PhabricatorPolicy $policy) {
$this->policy = $policy;
return $this;
}
public function getPolicy() {
return $this->policy;
}
public function getPolicyDisplayName() {
$policy = $this->getPolicy();
return $policy->getFullName();
}
public function newTransactionLink(
$mode,
PhabricatorApplicationTransaction $xaction) {
$policy = $this->getPolicy();
if ($policy->isCustomPolicy()) {
$uri = urisprintf(
'/transactions/%s/%s/',
$mode,
$xaction->getPHID());
$workflow = true;
} else {
$uri = $policy->getHref();
$workflow = false;
}
return $this->newLink($uri, $workflow);
}
public function newCapabilityLink($object, $capability) {
$policy = $this->getPolicy();
$uri = urisprintf(
'/policy/explain/%s/%s/',
$object->getPHID(),
$capability);
return $this->newLink($uri, true);
}
private function newLink($uri, $workflow) {
$policy = $this->getPolicy();
$name = $policy->getName();
if ($uri !== null) {
$name = javelin_tag(
'a',
array(
'href' => $uri,
'sigil' => ($workflow ? 'workflow' : null),
),
$name);
}
$hint = $this->getPolicyTypeHint();
if ($hint !== null) {
$name = pht('%s (%s)', $name, $hint);
}
return $name;
}
private function getPolicyTypeHint() {
$policy = $this->getPolicy();
if ($policy->isProjectPolicy()) {
return pht('Project');
}
if ($policy->isMaskedPolicy()) {
return pht('You do not have permission to view policy details.');
}
return null;
}
}

View file

@ -445,19 +445,15 @@ abstract class PhabricatorApplicationTransaction
$policy = PhabricatorPolicy::newFromPolicyAndHandle( $policy = PhabricatorPolicy::newFromPolicyAndHandle(
$phid, $phid,
$this->getHandleIfExists($phid)); $this->getHandleIfExists($phid));
$ref = $policy->newRef($this->getViewer());
if ($this->renderingTarget == self::TARGET_HTML) { if ($this->renderingTarget == self::TARGET_HTML) {
switch ($policy->getType()) { $output = $ref->newTransactionLink($state, $this);
case PhabricatorPolicyType::TYPE_CUSTOM:
$policy->setHref('/transactions/'.$state.'/'.$this->getPHID().'/');
$policy->setWorkflow(true);
break;
default:
break;
}
$output = $policy->renderDescription();
} else { } else {
$output = hsprintf('%s', $policy->getFullName()); $output = $ref->getPolicyDisplayName();
} }
return $output; return $output;
} }

View file

@ -215,17 +215,16 @@ abstract class PhabricatorModularTransactionType
$phid, $phid,
$handles[$phid]); $handles[$phid]);
$ref = $policy->newRef($viewer);
if ($this->isTextMode()) { if ($this->isTextMode()) {
return $this->renderValue($policy->getFullName()); $name = $ref->getPolicyDisplayName();
} else {
$storage = $this->getStorage();
$name = $ref->newTransactionLink($mode, $storage);
} }
$storage = $this->getStorage(); return $this->renderValue($name);
if ($policy->getType() == PhabricatorPolicyType::TYPE_CUSTOM) {
$policy->setHref('/transactions/'.$mode.'/'.$storage->getPHID().'/');
$policy->setWorkflow(true);
}
return $this->renderValue($policy->renderDescription());
} }
final protected function renderHandleList(array $phids) { final protected function renderHandleList(array $phids) {

View file

@ -63,16 +63,23 @@ For detailed help on managing and stripping MFA, see the instructions in
Unlocking Objects Unlocking Objects
================= =================
If you aren't sure who owns an object, or no user account has access to an If you aren't sure who owns an object, you can inspect the policies from the
object, you can directly change object policies from the CLI: CLI:
```
$ ./bin/policy show <object>
```
To identify the object you want to examine, you can specify an object
name (like `T123`) or a PHID as the `<object>` parameter.
If examining the policy isn't helpful, or no user account has access to an
object, you can then directly change object policies from the CLI:
``` ```
$ ./bin/policy unlock <object> [--view ...] [--edit ...] [--owner ...] $ ./bin/policy unlock <object> [--view ...] [--edit ...] [--owner ...]
``` ```
To identify the object you want to unlock, you can specify an object name (like
`T123`) or a PHID as the `<object>` parameter.
Use the `--view` and `--edit` flags (and, for some objects, the `--owner` Use the `--view` and `--edit` flags (and, for some objects, the `--owner`
flag) to specify new policies for the object. flag) to specify new policies for the object.