From ab7a0912126e6e5b06db3cddf59eb5d0fed36162 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 7 Aug 2013 13:29:09 -0700 Subject: [PATCH] Fix text-mode rendering of object and Asana link views Summary: Ref T2852. Two issues: - Embeds (`T12`, `{T12}`) have some handle issues because handles run afoul of visibility checks under some configs. Make handles unconditionally visible. - Asana links don't render correctly into text mode. Give them a valid text mode rendering so they don't flip out. Test Plan: Made comments with `T12` and `http://app.asana.com/...` and published them to Asana. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T2852 Differential Revision: https://secure.phabricator.com/D6696 --- .../remarkup/DoorkeeperRemarkupRuleAsana.php | 16 ++++++++++------ .../phid/PhabricatorObjectHandle.php | 4 +++- .../rule/PhabricatorRemarkupRuleObject.php | 8 ++++---- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/applications/doorkeeper/remarkup/DoorkeeperRemarkupRuleAsana.php b/src/applications/doorkeeper/remarkup/DoorkeeperRemarkupRuleAsana.php index ea63971ce5..7307d0e8f3 100644 --- a/src/applications/doorkeeper/remarkup/DoorkeeperRemarkupRuleAsana.php +++ b/src/applications/doorkeeper/remarkup/DoorkeeperRemarkupRuleAsana.php @@ -61,12 +61,16 @@ final class DoorkeeperRemarkupRuleAsana 'id' => $tag_id, ) + $spec['tag']; - $view = id(new PhabricatorTagView()) - ->setID($tag_id) - ->setName($spec['href']) - ->setHref($spec['href']) - ->setType(PhabricatorTagView::TYPE_OBJECT) - ->setExternal(true); + if ($this->getEngine()->isTextMode()) { + $view = $spec['href']; + } else { + $view = id(new PhabricatorTagView()) + ->setID($tag_id) + ->setName($spec['href']) + ->setHref($spec['href']) + ->setType(PhabricatorTagView::TYPE_OBJECT) + ->setExternal(true); + } $engine->overwriteStoredText($spec['token'], $view); } diff --git a/src/applications/phid/PhabricatorObjectHandle.php b/src/applications/phid/PhabricatorObjectHandle.php index d9bfcda361..ace10a1986 100644 --- a/src/applications/phid/PhabricatorObjectHandle.php +++ b/src/applications/phid/PhabricatorObjectHandle.php @@ -231,7 +231,9 @@ final class PhabricatorObjectHandle } public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { - return false; + // NOTE: Handles are always visible, they just don't get populated with + // data if the user can't see the underlying object. + return true; } } diff --git a/src/infrastructure/markup/rule/PhabricatorRemarkupRuleObject.php b/src/infrastructure/markup/rule/PhabricatorRemarkupRuleObject.php index 57f52b3651..35e95ee4ba 100644 --- a/src/infrastructure/markup/rule/PhabricatorRemarkupRuleObject.php +++ b/src/infrastructure/markup/rule/PhabricatorRemarkupRuleObject.php @@ -26,11 +26,11 @@ abstract class PhabricatorRemarkupRuleObject protected function loadHandles(array $objects) { $phids = mpull($objects, 'getPHID'); - $query = new PhabricatorObjectHandleData($phids); - $viewer = $this->getEngine()->getConfig('viewer'); - $query->setViewer($viewer); - $handles = $query->loadHandles(); + $handles = id(new PhabricatorHandleQuery($phids)) + ->withPHIDs($phids) + ->setViewer($this->getEngine()->getConfig('viewer')) + ->execute(); $result = array(); foreach ($objects as $id => $object) {