Render user hovercards with context information about their ability to see the context object

Summary: Ref T13602. When rendering a user hovercard, pass the object on which the reference appears. If the user can't see the object, make it clear on the hovecard. Restyle the "nopermission" markup in mentions to make it more obvious what the style means: instead of grey text, use red with an explicit icon. Test Plan: {F8430398} Maniphest Tasks: T13602 Differential Revision: https://secure.phabricator.com/D21554
2024-12-22 21:40:55 +01:00 · 2021-02-13 12:51:06 -08:00 · 2021-02-13 12:51:06 -08:00 · 90903282c7
commit 90903282c7
parent 2aac3156f7
16 changed files with 212 additions and 69 deletions
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@ -9,8 +9,8 @@ return array(
  'names' => array(
    'conpherence.pkg.css' => '0e3cf785',
    'conpherence.pkg.js' => '020aebcf',
-    'core.pkg.css' => '970b3ceb',
-    'core.pkg.js' => '2fe70e3d',
+    'core.pkg.css' => '7cb6808c',
+    'core.pkg.js' => '079198f6',
    'dark-console.pkg.js' => '187792c2',
    'differential.pkg.css' => '5c459f92',
    'differential.pkg.js' => '5080baf4',
@ -101,7 +101,7 @@ return array(
    'rsrc/css/application/policy/policy-transaction-detail.css' => 'c02b8384',
    'rsrc/css/application/policy/policy.css' => 'ceb56a08',
    'rsrc/css/application/ponder/ponder-view.css' => '05a09d0a',
-    'rsrc/css/application/project/project-card-view.css' => '4e7371cd',
+    'rsrc/css/application/project/project-card-view.css' => 'a9f2c2dd',
    'rsrc/css/application/project/project-triggers.css' => 'cd9c8bb9',
    'rsrc/css/application/project/project-view.css' => '567858b3',
    'rsrc/css/application/releeph/releeph-core.css' => 'f81ff2db',
@ -114,7 +114,7 @@ return array(
    'rsrc/css/application/tokens/tokens.css' => 'ce5a50bd',
    'rsrc/css/application/uiexample/example.css' => 'b4795059',
    'rsrc/css/core/core.css' => 'b3ebd90d',
-    'rsrc/css/core/remarkup.css' => '24d48a73',
+    'rsrc/css/core/remarkup.css' => '5baa3bd9',
    'rsrc/css/core/syntax.css' => '548567f6',
    'rsrc/css/core/z-index.css' => 'ac3bfcd4',
    'rsrc/css/diviner/diviner-shared.css' => '4bd263b0',
@ -181,7 +181,7 @@ return array(
    'rsrc/css/phui/phui-segment-bar-view.css' => '5166b370',
    'rsrc/css/phui/phui-spacing.css' => 'b05cadc3',
    'rsrc/css/phui/phui-status.css' => 'e5ff8be0',
-    'rsrc/css/phui/phui-tag-view.css' => '8519160a',
+    'rsrc/css/phui/phui-tag-view.css' => 'fb811341',
    'rsrc/css/phui/phui-timeline-view.css' => '2d32d7a9',
    'rsrc/css/phui/phui-two-column-view.css' => 'f96d319f',
    'rsrc/css/phui/workboards/phui-workboard-color.css' => 'e86de308',
@ -460,8 +460,8 @@ return array(
    'rsrc/js/core/DraggableList.js' => '0169e425',
    'rsrc/js/core/Favicon.js' => '7930776a',
    'rsrc/js/core/FileUpload.js' => 'ab85e184',
-    'rsrc/js/core/Hovercard.js' => 'd9d29a5f',
-    'rsrc/js/core/HovercardList.js' => '10a5f4bf',
+    'rsrc/js/core/Hovercard.js' => '6199f752',
+    'rsrc/js/core/HovercardList.js' => 'de4b4919',
    'rsrc/js/core/KeyboardShortcut.js' => '1a844c06',
    'rsrc/js/core/KeyboardShortcutManager.js' => '81debc48',
    'rsrc/js/core/MultirowRowManager.js' => '5b54c823',
@ -486,7 +486,7 @@ return array(
    'rsrc/js/core/behavior-global-drag-and-drop.js' => '1cab0e9a',
    'rsrc/js/core/behavior-high-security-warning.js' => 'dae2d55b',
    'rsrc/js/core/behavior-history-install.js' => '6a1583a8',
-    'rsrc/js/core/behavior-hovercard.js' => '3f446c72',
+    'rsrc/js/core/behavior-hovercard.js' => '183738e6',
    'rsrc/js/core/behavior-keyboard-pager.js' => '1325b731',
    'rsrc/js/core/behavior-keyboard-shortcuts.js' => '42c44e8b',
    'rsrc/js/core/behavior-lightbox-attachments.js' => 'c7e748bf',
@ -671,7 +671,7 @@ return array(
    'javelin-behavior-pholio-mock-view' => '5aa1544e',
    'javelin-behavior-phui-dropdown-menu' => '5cf0501a',
    'javelin-behavior-phui-file-upload' => 'e150bd50',
-    'javelin-behavior-phui-hovercards' => '3f446c72',
+    'javelin-behavior-phui-hovercards' => '183738e6',
    'javelin-behavior-phui-selectable-list' => 'b26a41e4',
    'javelin-behavior-phui-submenu' => 'b5e9bff9',
    'javelin-behavior-phui-tab-group' => '242aa08b',
@ -807,7 +807,7 @@ return array(
    'phabricator-object-selector-css' => 'ee77366f',
    'phabricator-phtize' => '2f1db1ed',
    'phabricator-prefab' => '5793d835',
-    'phabricator-remarkup-css' => '24d48a73',
+    'phabricator-remarkup-css' => '5baa3bd9',
    'phabricator-search-results-css' => '9ea70ace',
    'phabricator-shaped-request' => '995f5102',
    'phabricator-slowvote-css' => '1694baed',
@ -859,8 +859,8 @@ return array(
    'phui-formation-view-css' => 'd2dec8ed',
    'phui-head-thing-view-css' => 'd7f293df',
    'phui-header-view-css' => '36c86a58',
-    'phui-hovercard' => 'd9d29a5f',
-    'phui-hovercard-list' => '10a5f4bf',
+    'phui-hovercard' => '6199f752',
+    'phui-hovercard-list' => 'de4b4919',
    'phui-hovercard-view-css' => '6ca90fa0',
    'phui-icon-set-selector-css' => '7aa5f3ec',
    'phui-icon-view-css' => '4cbc684a',
@ -886,7 +886,7 @@ return array(
    'phui-segment-bar-view-css' => '5166b370',
    'phui-spacing-css' => 'b05cadc3',
    'phui-status-list-view-css' => 'e5ff8be0',
-    'phui-tag-view-css' => '8519160a',
+    'phui-tag-view-css' => 'fb811341',
    'phui-theme-css' => '35883b37',
    'phui-timeline-view-css' => '2d32d7a9',
    'phui-two-column-view-css' => 'f96d319f',
@ -908,7 +908,7 @@ return array(
    'policy-edit-css' => '8794e2ed',
    'policy-transaction-detail-css' => 'c02b8384',
    'ponder-view-css' => '05a09d0a',
-    'project-card-view-css' => '4e7371cd',
+    'project-card-view-css' => 'a9f2c2dd',
    'project-triggers-css' => 'cd9c8bb9',
    'project-view-css' => '567858b3',
    'releeph-core' => 'f81ff2db',
@ -1025,14 +1025,6 @@ return array(
      'javelin-workflow',
      'phuix-icon-view',
    ),
-    '10a5f4bf' => array(
-      'javelin-install',
-      'javelin-dom',
-      'javelin-vector',
-      'javelin-request',
-      'javelin-uri',
-      'phui-hovercard',
-    ),
    '111bfd2d' => array(
      'javelin-install',
    ),
@ -1047,6 +1039,14 @@ return array(
      'javelin-stratcom',
      'javelin-util',
    ),
+    '183738e6' => array(
+      'javelin-behavior',
+      'javelin-behavior-device',
+      'javelin-stratcom',
+      'javelin-vector',
+      'phui-hovercard',
+      'phui-hovercard-list',
+    ),
    '1a844c06' => array(
      'javelin-install',
      'javelin-util',
@ -1269,14 +1269,6 @@ return array(
      'phabricator-drag-and-drop-file-upload',
      'phabricator-draggable-list',
    ),
-    '3f446c72' => array(
-      'javelin-behavior',
-      'javelin-behavior-device',
-      'javelin-stratcom',
-      'javelin-vector',
-      'phui-hovercard',
-      'phui-hovercard-list',
-    ),
    '407ee861' => array(
      'javelin-behavior',
      'javelin-uri',
@ -1528,6 +1520,13 @@ return array(
    '60cd9241' => array(
      'javelin-behavior',
    ),
+    '6199f752' => array(
+      'javelin-install',
+      'javelin-dom',
+      'javelin-vector',
+      'javelin-request',
+      'javelin-uri',
+    ),
    '6337cf26' => array(
      'javelin-behavior',
      'javelin-dom',
@ -2136,13 +2135,6 @@ return array(
      'javelin-util',
      'phabricator-shaped-request',
    ),
-    'd9d29a5f' => array(
-      'javelin-install',
-      'javelin-dom',
-      'javelin-vector',
-      'javelin-request',
-      'javelin-uri',
-    ),
    'da15d3dc' => array(
      'phui-oi-list-view-css',
    ),
@ -2155,6 +2147,14 @@ return array(
      'javelin-uri',
      'phabricator-notification',
    ),
+    'de4b4919' => array(
+      'javelin-install',
+      'javelin-dom',
+      'javelin-vector',
+      'javelin-request',
+      'javelin-uri',
+      'phui-hovercard',
+    ),
    'e150bd50' => array(
      'javelin-behavior',
      'javelin-stratcom',
--- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php
+++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php
@ -638,7 +638,9 @@ final class ManiphestTaskDetailController extends ManiphestController {
            'href' => $commit->getURI(),
            'sigil' => 'hovercard',
            'meta' => array(
-              'hoverPHID' => $commit->getPHID(),
+              'hovercardSpec' => array(
+                'objectPHID' => $commit->getPHID(),
+              ),
            ),
          ),
          $commit->getSummary());
@ -705,7 +707,9 @@ final class ManiphestTaskDetailController extends ManiphestController {
            'href' => $revision->getURI(),
            'sigil' => 'hovercard',
            'meta' => array(
-              'hoverPHID' => $revision->getPHID(),
+              'hovercardSpec' => array(
+                'objectPHID' => $revision->getPHID(),
+              ),
            ),
          ),
          $revision->getTitle());
--- a/src/applications/people/engineextension/PeopleHovercardEngineExtension.php
+++ b/src/applications/people/engineextension/PeopleHovercardEngineExtension.php
@ -47,12 +47,14 @@ final class PeopleHovercardEngineExtension
      return;
    }

+    $is_exiled = $hovercard->getIsExiled();
+
    $user_card = id(new PhabricatorUserCardView())
      ->setProfile($user)
-      ->setViewer($viewer);
+      ->setViewer($viewer)
+      ->setIsExiled($is_exiled);

    $hovercard->appendChild($user_card);
-
  }

 }
--- a/src/applications/people/markup/PhabricatorMentionRemarkupRule.php
+++ b/src/applications/people/markup/PhabricatorMentionRemarkupRule.php
@ -110,7 +110,6 @@ final class PhabricatorMentionRemarkupRule extends PhutilRemarkupRule {

      if ($exists) {
        $user = $actual_users[$username];
-        Javelin::initBehavior('phui-hovercards');

        // Check if the user has view access to the object she was mentioned in
        if ($policy_object) {
@ -159,8 +158,13 @@ final class PhabricatorMentionRemarkupRule extends PhutilRemarkupRule {
            ->setName('@'.$user->getUserName())
            ->setHref($user_href);

+          if ($context_object) {
+            $tag->setContextObject($context_object);
+          }
+
          if ($user_can_not_view) {
-            $tag->addClass('phabricator-remarkup-mention-nopermission');
+            $tag->setIcon('fa-eye-slash red');
+            $tag->setIsExiled(true);
          }

          if ($user->getIsDisabled()) {
--- a/src/applications/people/view/PhabricatorUserCardView.php
+++ b/src/applications/people/view/PhabricatorUserCardView.php
@ -5,6 +5,7 @@ final class PhabricatorUserCardView extends AphrontTagView {
  private $profile;
  private $viewer;
  private $tag;
+  private $isExiled;

  public function setProfile(PhabricatorUser $profile) {
    $this->profile = $profile;
@ -42,6 +43,15 @@ final class PhabricatorUserCardView extends AphrontTagView {
    );
  }

+  public function setIsExiled($is_exiled) {
+    $this->isExiled = $is_exiled;
+    return $this;
+  }
+
+  public function getIsExiled() {
+    return $this->isExiled;
+  }
+
  protected function getTagContent() {

    $user = $this->profile;
@ -108,6 +118,15 @@ final class PhabricatorUserCardView extends AphrontTagView {
      }
    }

+    if ($this->getIsExiled()) {
+      $body[] = $this->addItem(
+        'fa-eye-slash red',
+        pht('This user can not see this object.'),
+        array(
+          'project-card-item-exiled',
+        ));
+    }
+
    $classes[] = 'project-card-image';
    $image = phutil_tag(
      'img',
@ -160,17 +179,26 @@ final class PhabricatorUserCardView extends AphrontTagView {
    return $card;
  }

-  private function addItem($icon, $value) {
+  private function addItem($icon, $value, $classes = array()) {
+    $classes[] = 'project-card-item';
+
    $icon = id(new PHUIIconView())
      ->addClass('project-card-item-icon')
      ->setIcon($icon);
+
    $text = phutil_tag(
      'span',
      array(
        'class' => 'project-card-item-text',
      ),
      $value);
-    return phutil_tag_div('project-card-item', array($icon, $text));
+
+    return phutil_tag(
+      'div',
+      array(
+        'class' => implode(' ', $classes),
+      ),
+      array($icon, $text));
  }

 }
--- a/src/applications/phid/PhabricatorObjectHandle.php
+++ b/src/applications/phid/PhabricatorObjectHandle.php
@ -306,7 +306,9 @@ final class PhabricatorObjectHandle
    $attributes = array(
      'sigil' => 'hovercard',
      'meta' => array(
-        'hoverPHID' => $this->getPHID(),
+        'hovercardSpec' => array(
+          'objectPHID' => $this->getPHID(),
+        ),
      ),
    );

--- a/src/applications/search/controller/PhabricatorSearchHovercardController.php
+++ b/src/applications/search/controller/PhabricatorSearchHovercardController.php
@ -32,11 +32,19 @@ final class PhabricatorSearchHovercardController

    $object_phids = array();
    $handle_phids = array();
+    $context_phids = array();
    foreach ($cards as $card) {
      $object_phid = idx($card, 'objectPHID');

      $handle_phids[] = $object_phid;
      $object_phids[] = $object_phid;
+
+      $context_phid = idx($card, 'contextPHID');
+
+      if ($context_phid) {
+        $object_phids[] = $context_phid;
+        $context_phids[] = $context_phid;
+      }
    }

    $handles = id(new PhabricatorHandleQuery())
@ -50,11 +58,20 @@ final class PhabricatorSearchHovercardController
      ->execute();
    $objects = mpull($objects, null, 'getPHID');

+    $context_objects = array_select_keys($objects, $context_phids);
+
+    if ($context_objects) {
+      PhabricatorPolicyFilterSet::loadHandleViewCapabilities(
+        $viewer,
+        $handles,
+        $context_objects);
+    }
+
    $extensions =
      PhabricatorHovercardEngineExtension::getAllEnabledExtensions();

    $extension_maps = array();
-    foreach ($extensions as $key => $extension) {
+    foreach ($extensions as $extension_key => $extension) {
      $extension->setViewer($viewer);

      $extension_phids = array();
@ -64,18 +81,18 @@ final class PhabricatorSearchHovercardController
        }
      }

-      $extension_maps[$key] = $extension_phids;
+      $extension_maps[$extension_key] = $extension_phids;
    }

    $extension_data = array();
-    foreach ($extensions as $key => $extension) {
-      $extension_phids = $extension_maps[$key];
+    foreach ($extensions as $extension_key => $extension) {
+      $extension_phids = $extension_maps[$extension_key];
      if (!$extension_phids) {
-        unset($extensions[$key]);
+        unset($extensions[$extension_key]);
        continue;
      }

-      $extension_data[$key] = $extension->willRenderHovercards(
+      $extension_data[$extension_key] = $extension->willRenderHovercards(
        array_select_keys($objects, $extension_phids));
    }

@ -86,20 +103,35 @@ final class PhabricatorSearchHovercardController
      $handle = $handles[$object_phid];
      $object = idx($objects, $object_phid);

+      $context_phid = idx($card, 'contextPHID');
+      if ($context_phid) {
+        $context_object = idx($context_objects, $context_phid);
+      } else {
+        $context_object = null;
+      }
+
      $hovercard = id(new PHUIHovercardView())
        ->setUser($viewer)
        ->setObjectHandle($handle);

+      if ($context_object) {
+        if ($handle->hasCapabilities()) {
+          if (!$handle->hasViewCapability($context_object)) {
+            $hovercard->setIsExiled(true);
+          }
+        }
+      }
+
      if ($object) {
        $hovercard->setObject($object);

-        foreach ($extension_maps as $key => $extension_phids) {
-          if (isset($extension_phids[$phid])) {
-            $extensions[$key]->renderHovercard(
+        foreach ($extension_maps as $extension_key => $extension_phids) {
+          if (isset($extension_phids[$object_phid])) {
+            $extensions[$extension_key]->renderHovercard(
              $hovercard,
              $handle,
              $object,
-              $extension_data[$key]);
+              $extension_data[$extension_key]);
          }
        }
      }
@ -114,8 +146,8 @@ final class PhabricatorSearchHovercardController
        ));
    }

-    foreach ($results as $key => $hovercard) {
-      $results[$key] = phutil_tag('div',
+    foreach ($results as $result_key => $hovercard) {
+      $results[$result_key] = phutil_tag('div',
        array(
          'class' => 'ml',
        ),
--- a/src/infrastructure/graph/ManiphestTaskGraph.php
+++ b/src/infrastructure/graph/ManiphestTaskGraph.php
@ -69,7 +69,9 @@ final class ManiphestTaskGraph
          'href' => $object->getURI(),
          'sigil' => 'hovercard',
          'meta' => array(
-            'hoverPHID' => $object->getPHID(),
+            'hovercardSpec' => array(
+              'objectPHID' => $object->getPHID(),
+            ),
          ),
        ),
        $object->getTitle());
--- a/src/view/phui/PHUIHovercardView.php
+++ b/src/view/phui/PHUIHovercardView.php
@ -18,6 +18,7 @@ final class PHUIHovercardView extends AphrontTagView {
  private $fields = array();
  private $actions = array();
  private $badges = array();
+  private $isExiled;

  public function setObjectHandle(PhabricatorObjectHandle $handle) {
    $this->handle = $handle;
@ -43,6 +44,15 @@ final class PHUIHovercardView extends AphrontTagView {
    return $this;
  }

+  public function setIsExiled($is_exiled) {
+    $this->isExiled = $is_exiled;
+    return $this;
+  }
+
+  public function getIsExiled() {
+    return $this->isExiled;
+  }
+
  public function addField($label, $value) {
    $this->fields[] = array(
      'label' => $label,
--- a/src/view/phui/PHUITagView.php
+++ b/src/view/phui/PHUITagView.php
@ -44,6 +44,8 @@ final class PHUITagView extends AphrontTagView {
  private $shade;
  private $slimShady;
  private $border;
+  private $contextObject;
+  private $isExiled;

  public function setType($type) {
    $this->type = $type;
@ -127,6 +129,24 @@ final class PHUITagView extends AphrontTagView {
    return strlen($this->href) ? 'a' : 'span';
  }

+  public function setContextObject($context_object) {
+    $this->contextObject = $context_object;
+    return $this;
+  }
+
+  public function getContextObject() {
+    return $this->contextObject;
+  }
+
+  public function setIsExiled($is_exiled) {
+    $this->isExiled = $is_exiled;
+    return $this;
+  }
+
+  public function getIsExiled() {
+    return $this->isExiled;
+  }
+
  protected function getTagAttributes() {
    require_celerity_resource('phui-tag-view-css');

@ -155,6 +175,10 @@ final class PHUITagView extends AphrontTagView {
      $classes[] = 'phui-tag-'.$this->border;
    }

+    if ($this->getIsExiled()) {
+      $classes[] = 'phui-tag-exiled';
+    }
+
    $attributes = array(
      'href' => $this->href,
      'class' => $classes,
@ -170,10 +194,19 @@ final class PHUITagView extends AphrontTagView {
    if ($this->phid) {
      Javelin::initBehavior('phui-hovercards');

+      $hovercard_spec = array(
+        'objectPHID' => $this->phid,
+      );
+
+      $context_object = $this->getContextObject();
+      if ($context_object) {
+        $hovercard_spec['contextPHID'] = $context_object->getPHID();
+      }
+
      $attributes += array(
        'sigil' => 'hovercard',
        'meta' => array(
-          'hoverPHID' => $this->phid,
+          'hovercardSpec' => $hovercard_spec,
        ),
      );
    }
--- a/webroot/rsrc/css/application/project/project-card-view.css
+++ b/webroot/rsrc/css/application/project/project-card-view.css
@ -72,6 +72,17 @@
  color: {$greytext};
 }

+.project-card-view .project-card-item-exiled {
+  background-color: {$lightredbackground};
+  border-radius: 4px;
+  padding: 2px 8px;
+  margin: 2px 0;
+}
+
+.project-card-view .project-card-item-exiled .project-card-item-text {
+  color: {$red};
+}
+
 .project-card-view .project-card-item-icon {
  width: 20px;
 }
--- a/webroot/rsrc/css/core/remarkup.css
+++ b/webroot/rsrc/css/core/remarkup.css
@ -291,11 +291,6 @@ video.phabricator-media {
  color: {$greytext};
 }

-.phabricator-remarkup-mention-nopermission .phui-tag-core {
-  background: {$lightgreybackground};
-  color: {$lightgreytext};
-}
-
 .phabricator-remarkup .remarkup-note {
  margin: 16px 0;
  padding: 12px;
--- a/webroot/rsrc/css/phui/phui-tag-view.css
+++ b/webroot/rsrc/css/phui/phui-tag-view.css
@ -531,3 +531,15 @@ a.phui-tag-view:hover.phui-tag-disabled .phui-tag-core {
  color: {$blacktext};
  border-color: {$blacktext};
 }
+
+.phui-tag-exiled .phui-tag-core {
+  border-color: {$lightredborder};
+  color: {$red};
+  background: {$lightredbackground};
+}
+
+
+a.phui-tag-view.phui-tag-exiled:hover
+  .phui-tag-core.phui-tag-color-person {
+  border-color: {$red};
+}
--- a/webroot/rsrc/js/core/Hovercard.js
+++ b/webroot/rsrc/js/core/Hovercard.js
@ -13,6 +13,7 @@ JX.install('Hovercard', {
  properties: {
    hovercardKey: null,
    objectPHID: null,
+    contextPHID: null,
    isLoading: false,
    isLoaded: false,
    content: null
--- a/webroot/rsrc/js/core/HovercardList.js
+++ b/webroot/rsrc/js/core/HovercardList.js
@ -31,7 +31,8 @@ JX.install('HovercardList', {
      if (!(hovercard_key in this._cards)) {
        var card = new JX.Hovercard()
          .setHovercardKey(hovercard_key)
-          .setObjectPHID(spec.hoverPHID);
+          .setObjectPHID(spec.objectPHID)
+          .setContextPHID(spec.contextPHID || null);

        this._cards[hovercard_key] = card;
      }
@ -76,12 +77,18 @@ JX.install('HovercardList', {
    },

    _newHovercardKey: function(spec) {
-      return 'phid=' + spec.hoverPHID;
+      var parts = [
+        spec.objectPHID,
+        spec.contextPHID
+      ];
+
+      return parts.join('/');
    },

    _newCardRequest: function(card) {
      return {
-        objectPHID: card.getObjectPHID()
+        objectPHID: card.getObjectPHID(),
+        contextPHID: card.getContextPHID()
      };
    },

--- a/webroot/rsrc/js/core/behavior-hovercard.js
+++ b/webroot/rsrc/js/core/behavior-hovercard.js
@ -32,7 +32,7 @@ JX.behavior('phui-hovercards', function(config, statics) {
      }

      var node = e.getNode('hovercard');
-      var data = e.getNodeData('hovercard');
+      var data = e.getNodeData('hovercard').hovercardSpec;

      var card = cards.getCard(data);