1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-14 02:42:40 +01:00

Distinguish between invalid/broken handles and filtered handles

Summary:
Ref T603. Currently, we render handles the user doesn't have permission to see in a manner identical to handles that don't exist. This is confusing, and not required by policies (which restrict content, but permit knowledge that an object exists).

Instead, render them in different styles. Bad/invalid objects look like:

  Unknown Object (Task)

Restricted objects look like:

  [o] Restricted Task

...where `[o]` is the padlock icon.

Test Plan:
{F71100}

{F71101}

It's possible this renders weird somewhere, but I wasn't immediately able to find any issues. Yell if you see something.

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: chad, aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D7334
This commit is contained in:
epriestley 2013-10-17 10:49:21 -07:00
parent cf8d7da292
commit 95c2b03fc8
5 changed files with 121 additions and 54 deletions

View file

@ -3523,7 +3523,7 @@ celerity_register_resource_map(array(
), ),
'phabricator-standard-page-view' => 'phabricator-standard-page-view' =>
array( array(
'uri' => '/res/59c804b1/rsrc/css/application/base/standard-page-view.css', 'uri' => '/res/eebd59cd/rsrc/css/application/base/standard-page-view.css',
'type' => 'css', 'type' => 'css',
'requires' => 'requires' =>
array( array(
@ -4297,7 +4297,7 @@ celerity_register_resource_map(array(
), array( ), array(
'packages' => 'packages' =>
array( array(
'deb31815' => '13fde8cd' =>
array( array(
'name' => 'core.pkg.css', 'name' => 'core.pkg.css',
'symbols' => 'symbols' =>
@ -4346,7 +4346,7 @@ celerity_register_resource_map(array(
41 => 'phabricator-tag-view-css', 41 => 'phabricator-tag-view-css',
42 => 'phui-list-view-css', 42 => 'phui-list-view-css',
), ),
'uri' => '/res/pkg/deb31815/core.pkg.css', 'uri' => '/res/pkg/13fde8cd/core.pkg.css',
'type' => 'css', 'type' => 'css',
), ),
'6041c6c8' => '6041c6c8' =>
@ -4538,15 +4538,15 @@ celerity_register_resource_map(array(
), ),
'reverse' => 'reverse' =>
array( array(
'aphront-dialog-view-css' => 'deb31815', 'aphront-dialog-view-css' => '13fde8cd',
'aphront-error-view-css' => 'deb31815', 'aphront-error-view-css' => '13fde8cd',
'aphront-list-filter-view-css' => 'deb31815', 'aphront-list-filter-view-css' => '13fde8cd',
'aphront-pager-view-css' => 'deb31815', 'aphront-pager-view-css' => '13fde8cd',
'aphront-panel-view-css' => 'deb31815', 'aphront-panel-view-css' => '13fde8cd',
'aphront-table-view-css' => 'deb31815', 'aphront-table-view-css' => '13fde8cd',
'aphront-tokenizer-control-css' => 'deb31815', 'aphront-tokenizer-control-css' => '13fde8cd',
'aphront-tooltip-css' => 'deb31815', 'aphront-tooltip-css' => '13fde8cd',
'aphront-typeahead-control-css' => 'deb31815', 'aphront-typeahead-control-css' => '13fde8cd',
'differential-changeset-view-css' => '7cd7e387', 'differential-changeset-view-css' => '7cd7e387',
'differential-core-view-css' => '7cd7e387', 'differential-core-view-css' => '7cd7e387',
'differential-inline-comment-editor' => '5e9e5c4e', 'differential-inline-comment-editor' => '5e9e5c4e',
@ -4560,7 +4560,7 @@ celerity_register_resource_map(array(
'differential-table-of-contents-css' => '7cd7e387', 'differential-table-of-contents-css' => '7cd7e387',
'diffusion-commit-view-css' => '270f4eb4', 'diffusion-commit-view-css' => '270f4eb4',
'diffusion-icons-css' => '270f4eb4', 'diffusion-icons-css' => '270f4eb4',
'global-drag-and-drop-css' => 'deb31815', 'global-drag-and-drop-css' => '13fde8cd',
'inline-comment-summary-css' => '7cd7e387', 'inline-comment-summary-css' => '7cd7e387',
'javelin-aphlict' => '6041c6c8', 'javelin-aphlict' => '6041c6c8',
'javelin-behavior' => '3e3be199', 'javelin-behavior' => '3e3be199',
@ -4635,56 +4635,56 @@ celerity_register_resource_map(array(
'javelin-util' => '3e3be199', 'javelin-util' => '3e3be199',
'javelin-vector' => '3e3be199', 'javelin-vector' => '3e3be199',
'javelin-workflow' => '3e3be199', 'javelin-workflow' => '3e3be199',
'lightbox-attachment-css' => 'deb31815', 'lightbox-attachment-css' => '13fde8cd',
'maniphest-task-summary-css' => '49898640', 'maniphest-task-summary-css' => '49898640',
'phabricator-action-list-view-css' => 'deb31815', 'phabricator-action-list-view-css' => '13fde8cd',
'phabricator-application-launch-view-css' => 'deb31815', 'phabricator-application-launch-view-css' => '13fde8cd',
'phabricator-busy' => '6041c6c8', 'phabricator-busy' => '6041c6c8',
'phabricator-content-source-view-css' => '7cd7e387', 'phabricator-content-source-view-css' => '7cd7e387',
'phabricator-core-css' => 'deb31815', 'phabricator-core-css' => '13fde8cd',
'phabricator-crumbs-view-css' => 'deb31815', 'phabricator-crumbs-view-css' => '13fde8cd',
'phabricator-drag-and-drop-file-upload' => '5e9e5c4e', 'phabricator-drag-and-drop-file-upload' => '5e9e5c4e',
'phabricator-dropdown-menu' => '6041c6c8', 'phabricator-dropdown-menu' => '6041c6c8',
'phabricator-file-upload' => '6041c6c8', 'phabricator-file-upload' => '6041c6c8',
'phabricator-filetree-view-css' => 'deb31815', 'phabricator-filetree-view-css' => '13fde8cd',
'phabricator-flag-css' => 'deb31815', 'phabricator-flag-css' => '13fde8cd',
'phabricator-hovercard' => '6041c6c8', 'phabricator-hovercard' => '6041c6c8',
'phabricator-jump-nav' => 'deb31815', 'phabricator-jump-nav' => '13fde8cd',
'phabricator-keyboard-shortcut' => '6041c6c8', 'phabricator-keyboard-shortcut' => '6041c6c8',
'phabricator-keyboard-shortcut-manager' => '6041c6c8', 'phabricator-keyboard-shortcut-manager' => '6041c6c8',
'phabricator-main-menu-view' => 'deb31815', 'phabricator-main-menu-view' => '13fde8cd',
'phabricator-menu-item' => '6041c6c8', 'phabricator-menu-item' => '6041c6c8',
'phabricator-nav-view-css' => 'deb31815', 'phabricator-nav-view-css' => '13fde8cd',
'phabricator-notification' => '6041c6c8', 'phabricator-notification' => '6041c6c8',
'phabricator-notification-css' => 'deb31815', 'phabricator-notification-css' => '13fde8cd',
'phabricator-notification-menu-css' => 'deb31815', 'phabricator-notification-menu-css' => '13fde8cd',
'phabricator-object-selector-css' => '7cd7e387', 'phabricator-object-selector-css' => '7cd7e387',
'phabricator-phtize' => '6041c6c8', 'phabricator-phtize' => '6041c6c8',
'phabricator-prefab' => '6041c6c8', 'phabricator-prefab' => '6041c6c8',
'phabricator-project-tag-css' => '49898640', 'phabricator-project-tag-css' => '49898640',
'phabricator-remarkup-css' => 'deb31815', 'phabricator-remarkup-css' => '13fde8cd',
'phabricator-shaped-request' => '5e9e5c4e', 'phabricator-shaped-request' => '5e9e5c4e',
'phabricator-side-menu-view-css' => 'deb31815', 'phabricator-side-menu-view-css' => '13fde8cd',
'phabricator-standard-page-view' => 'deb31815', 'phabricator-standard-page-view' => '13fde8cd',
'phabricator-tag-view-css' => 'deb31815', 'phabricator-tag-view-css' => '13fde8cd',
'phabricator-textareautils' => '6041c6c8', 'phabricator-textareautils' => '6041c6c8',
'phabricator-tooltip' => '6041c6c8', 'phabricator-tooltip' => '6041c6c8',
'phabricator-transaction-view-css' => 'deb31815', 'phabricator-transaction-view-css' => '13fde8cd',
'phabricator-zindex-css' => 'deb31815', 'phabricator-zindex-css' => '13fde8cd',
'phui-button-css' => 'deb31815', 'phui-button-css' => '13fde8cd',
'phui-form-css' => 'deb31815', 'phui-form-css' => '13fde8cd',
'phui-form-view-css' => 'deb31815', 'phui-form-view-css' => '13fde8cd',
'phui-header-view-css' => 'deb31815', 'phui-header-view-css' => '13fde8cd',
'phui-icon-view-css' => 'deb31815', 'phui-icon-view-css' => '13fde8cd',
'phui-list-view-css' => 'deb31815', 'phui-list-view-css' => '13fde8cd',
'phui-object-item-list-view-css' => 'deb31815', 'phui-object-item-list-view-css' => '13fde8cd',
'phui-property-list-view-css' => 'deb31815', 'phui-property-list-view-css' => '13fde8cd',
'phui-spacing-css' => 'deb31815', 'phui-spacing-css' => '13fde8cd',
'sprite-apps-large-css' => 'deb31815', 'sprite-apps-large-css' => '13fde8cd',
'sprite-gradient-css' => 'deb31815', 'sprite-gradient-css' => '13fde8cd',
'sprite-icons-css' => 'deb31815', 'sprite-icons-css' => '13fde8cd',
'sprite-menu-css' => 'deb31815', 'sprite-menu-css' => '13fde8cd',
'sprite-status-css' => 'deb31815', 'sprite-status-css' => '13fde8cd',
'syntax-highlighting-css' => 'deb31815', 'syntax-highlighting-css' => '13fde8cd',
), ),
)); ));

View file

@ -15,6 +15,16 @@ final class PhabricatorObjectHandle
private $complete; private $complete;
private $disabled; private $disabled;
private $objectName; private $objectName;
private $policyFiltered;
public function setPolicyFiltered($policy_filered) {
$this->policyFiltered = $policy_filered;
return $this;
}
public function getPolicyFiltered() {
return $this->policyFiltered;
}
public function setObjectName($object_name) { public function setObjectName($object_name) {
$this->objectName = $object_name; $this->objectName = $object_name;
@ -53,8 +63,12 @@ final class PhabricatorObjectHandle
public function getName() { public function getName() {
if ($this->name === null) { if ($this->name === null) {
if ($this->getPolicyFiltered()) {
return pht('Restricted %s', $this->getTypeName());
} else {
return pht('Unknown Object (%s)', $this->getTypeName()); return pht('Unknown Object (%s)', $this->getTypeName());
} }
}
return $this->name; return $this->name;
} }
@ -186,6 +200,7 @@ final class PhabricatorObjectHandle
$name = $this->getLinkName(); $name = $this->getLinkName();
} }
$classes = array(); $classes = array();
$classes[] = 'phui-handle';
$title = $this->title; $title = $this->title;
if ($this->status != PhabricatorObjectHandleStatus::STATUS_OPEN) { if ($this->status != PhabricatorObjectHandleStatus::STATUS_OPEN) {
@ -195,21 +210,30 @@ final class PhabricatorObjectHandle
if ($this->disabled) { if ($this->disabled) {
$classes[] = 'handle-disabled'; $classes[] = 'handle-disabled';
$title = 'disabled'; // Overwrite status. $title = pht('Disabled'); // Overwrite status.
} }
if ($this->getType() == PhabricatorPeoplePHIDTypeUser::TYPECONST) { if ($this->getType() == PhabricatorPeoplePHIDTypeUser::TYPECONST) {
$classes[] = 'phui-link-person'; $classes[] = 'phui-link-person';
} }
$uri = $this->getURI();
$icon = null;
if ($this->getPolicyFiltered()) {
$icon = id(new PHUIIconView())
->setSpriteSheet(PHUIIconView::SPRITE_ICONS)
->setSpriteIcon('lock-grey');
}
return phutil_tag( return phutil_tag(
'a', $uri ? 'a' : 'span',
array( array(
'href' => $this->getURI(), 'href' => $uri,
'class' => implode(' ', $classes), 'class' => implode(' ', $classes),
'title' => $title, 'title' => $title,
), ),
$name); array($icon, $name));
} }
public function getLinkName() { public function getLinkName() {

View file

@ -18,10 +18,12 @@ final class PhabricatorHandleQuery
return array(); return array();
} }
$objects = id(new PhabricatorObjectQuery()) $object_query = id(new PhabricatorObjectQuery())
->withPHIDs($phids) ->withPHIDs($phids)
->setViewer($this->getViewer()) ->setViewer($this->getViewer());
->execute();
$objects = $object_query->execute();
$filtered = $object_query->getPolicyFilteredPHIDs();
$groups = array(); $groups = array();
foreach ($phids as $phid) { foreach ($phids as $phid) {
@ -43,6 +45,8 @@ final class PhabricatorHandleQuery
->setPHID($phid); ->setPHID($phid);
if (isset($objects[$phid])) { if (isset($objects[$phid])) {
$handles[$phid]->setComplete(true); $handles[$phid]->setComplete(true);
} else if (isset($filtered[$phid])) {
$handles[$phid]->setPolicyFiltered(true);
} }
} }

View file

@ -34,6 +34,7 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery {
private $rawResultLimit; private $rawResultLimit;
private $capabilities; private $capabilities;
private $workspace = array(); private $workspace = array();
private $policyFilteredPHIDs = array();
/* -( Query Configuration )------------------------------------------------ */ /* -( Query Configuration )------------------------------------------------ */
@ -228,6 +229,17 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery {
$visible = $maybe_visible; $visible = $maybe_visible;
} else { } else {
$visible = $filter->apply($maybe_visible); $visible = $filter->apply($maybe_visible);
$policy_filtered = array();
foreach ($maybe_visible as $key => $object) {
if (empty($visible[$key])) {
$phid = $object->getPHID();
if ($phid) {
$policy_filtered[$phid] = $phid;
}
}
}
$this->addPolicyFilteredPHIDs($policy_filtered);
} }
if ($visible) { if ($visible) {
@ -305,6 +317,28 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery {
PhabricatorPolicyCapability::CAN_VIEW); PhabricatorPolicyCapability::CAN_VIEW);
} }
protected function addPolicyFilteredPHIDs(array $phids) {
$this->policyFilteredPHIDs += $phids;
if ($this->getParentQuery()) {
$this->getParentQuery()->addPolicyFilteredPHIDs($phids);
}
return $this;
}
/**
* Return a map of all object PHIDs which were loaded in the query but
* filtered out by policy constraints. This allows a caller to distinguish
* between objects which do not exist (or, at least, were filtered at the
* content level) and objects which exist but aren't visible.
*
* @return map<phid, phid> Map of object PHIDs which were filtered
* by policies.
* @task exec
*/
public function getPolicyFilteredPHIDs() {
return $this->policyFilteredPHIDs;
}
/* -( Query Workspace )---------------------------------------------------- */ /* -( Query Workspace )---------------------------------------------------- */

View file

@ -95,3 +95,8 @@ a.handle-disabled {
padding: 8px 16px; padding: 8px 16px;
background: {$lightyellow}; background: {$lightyellow};
} }
.phui-handle .phui-icon-view {
display: inline-block;
margin: 2px 2px -2px 0;
}