From fde4ccf9b2cf2bb0c4abd8d279be7500e5eaeaa0 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 18 Apr 2014 17:52:32 -0700 Subject: [PATCH] Use standard handle loading in Releeph Summary: Ref T3718. Move from unbatched / ad-hoc loading to standard stuff for handles. Test Plan: Looked at some requests and saw no changes. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T3718 Differential Revision: https://secure.phabricator.com/D8810 --- .../customfield/DifferentialCustomField.php | 13 --------- .../ReleephAuthorFieldSpecification.php | 15 +++++----- .../ReleephDependsOnFieldSpecification.php | 25 ++++------------ .../ReleephDiffSizeFieldSpecification.php | 2 +- .../ReleephFieldSpecification.php | 8 ----- .../ReleephIntentFieldSpecification.php | 16 ++++------ ...eleephOriginalCommitFieldSpecification.php | 10 +++++-- .../ReleephRequestorFieldSpecification.php | 18 ++++++++---- .../ReleephRevisionFieldSpecification.php | 29 +++++++------------ .../field/PhabricatorCustomField.php | 21 ++++++++++++++ 10 files changed, 72 insertions(+), 85 deletions(-) diff --git a/src/applications/differential/customfield/DifferentialCustomField.php b/src/applications/differential/customfield/DifferentialCustomField.php index f95ed86faf..7264ddc89a 100644 --- a/src/applications/differential/customfield/DifferentialCustomField.php +++ b/src/applications/differential/customfield/DifferentialCustomField.php @@ -31,19 +31,6 @@ abstract class DifferentialCustomField return parent::shouldEnableForRole($role); } - protected function renderHandleList(array $handles) { - if (!$handles) { - return null; - } - - $out = array(); - foreach ($handles as $handle) { - $out[] = $handle->renderLink(); - } - - return phutil_implode_html(phutil_tag('br'), $out); - } - public function getRequiredDiffPropertiesForRevisionView() { if ($this->getProxy()) { return $this->getProxy()->getRequiredDiffPropertiesForRevisionView(); diff --git a/src/applications/releeph/field/specification/ReleephAuthorFieldSpecification.php b/src/applications/releeph/field/specification/ReleephAuthorFieldSpecification.php index c7acd1d44e..b917b63ce6 100644 --- a/src/applications/releeph/field/specification/ReleephAuthorFieldSpecification.php +++ b/src/applications/releeph/field/specification/ReleephAuthorFieldSpecification.php @@ -11,24 +11,23 @@ final class ReleephAuthorFieldSpecification return 'Author'; } - public function renderPropertyViewValue(array $handles) { + public function getRequiredHandlePHIDsForPropertyView() { $pull = $this->getReleephRequest(); $commit = $pull->loadPhabricatorRepositoryCommit(); if (!$commit) { - return null; + return array(); } $author_phid = $commit->getAuthorPHID(); if (!$author_phid) { - return null; + return array(); } - $handle = id(new PhabricatorHandleQuery()) - ->setViewer($this->getUser()) - ->withPHIDs(array($author_phid)) - ->executeOne(); + return array($author_phid); + } - return $handle->renderLink(); + public function renderPropertyViewValue(array $handles) { + return $this->renderHandleList($handles); } } diff --git a/src/applications/releeph/field/specification/ReleephDependsOnFieldSpecification.php b/src/applications/releeph/field/specification/ReleephDependsOnFieldSpecification.php index 5636e8f112..e951b4bf11 100644 --- a/src/applications/releeph/field/specification/ReleephDependsOnFieldSpecification.php +++ b/src/applications/releeph/field/specification/ReleephDependsOnFieldSpecification.php @@ -10,25 +10,12 @@ final class ReleephDependsOnFieldSpecification return pht('Depends On'); } + public function getRequiredHandlePHIDsForPropertyView() { + return $this->getDependentRevisionPHIDs(); + } + public function renderPropertyViewValue(array $handles) { - $revision_phids = $this->getDependentRevisionPHIDs(); - if (!$revision_phids) { - return null; - } - - $links = array(); - $handles = id(new PhabricatorHandleQuery()) - ->setViewer($this->getUser()) - ->withPHIDs($revision_phids) - ->execute(); - foreach ($revision_phids as $revision_phid) { - $links[] = id(clone $handles[$revision_phid]) - // Hack to remove the strike-through rendering of diff links - ->setStatus(null) - ->renderLink(); - } - - return phutil_implode_html(phutil_tag('br'), $links); + return $this->renderHandleList($handles); } private function getDependentRevisionPHIDs() { @@ -36,7 +23,7 @@ final class ReleephDependsOnFieldSpecification ->getReleephRequest() ->loadDifferentialRevision(); if (!$revision) { - return null; + return array(); } return PhabricatorEdgeQuery::loadDestinationPHIDs( diff --git a/src/applications/releeph/field/specification/ReleephDiffSizeFieldSpecification.php b/src/applications/releeph/field/specification/ReleephDiffSizeFieldSpecification.php index 6b201e9cda..9c269d3c81 100644 --- a/src/applications/releeph/field/specification/ReleephDiffSizeFieldSpecification.php +++ b/src/applications/releeph/field/specification/ReleephDiffSizeFieldSpecification.php @@ -18,7 +18,7 @@ final class ReleephDiffSizeFieldSpecification public function renderPropertyViewValue(array $handles) { $diff_rev = $this->getReleephRequest()->loadDifferentialRevision(); if (!$diff_rev) { - return ''; + return null; } $diffs = $diff_rev->loadRelatives( diff --git a/src/applications/releeph/field/specification/ReleephFieldSpecification.php b/src/applications/releeph/field/specification/ReleephFieldSpecification.php index ea6a565d11..93e175a374 100644 --- a/src/applications/releeph/field/specification/ReleephFieldSpecification.php +++ b/src/applications/releeph/field/specification/ReleephFieldSpecification.php @@ -30,14 +30,6 @@ abstract class ReleephFieldSpecification return $value; } - public function slowlyLoadHandle($phid) { - // TODO: Remove this, it's transitional as fields modernize. - return id(new PhabricatorHandleQuery()) - ->withPHIDs(array($phid)) - ->setViewer($this->getUser()) - ->executeOne(); - } - abstract public function getName(); /* -( Storage )------------------------------------------------------------ */ diff --git a/src/applications/releeph/field/specification/ReleephIntentFieldSpecification.php b/src/applications/releeph/field/specification/ReleephIntentFieldSpecification.php index 504dd67a16..2c1288cab2 100644 --- a/src/applications/releeph/field/specification/ReleephIntentFieldSpecification.php +++ b/src/applications/releeph/field/specification/ReleephIntentFieldSpecification.php @@ -11,6 +11,12 @@ final class ReleephIntentFieldSpecification return 'Intent'; } + public function getRequiredHandlePHIDsForPropertyView() { + $pull = $this->getReleephRequest(); + $intents = $pull->getUserIntents(); + return array_keys($intents); + } + public function renderPropertyViewValue(array $handles) { $pull = $this->getReleephRequest(); @@ -21,16 +27,6 @@ final class ReleephIntentFieldSpecification return null; } - $user_phids = array_keys($intents); - if ($user_phids) { - $handles = id(new PhabricatorHandleQuery()) - ->withPHIDs($user_phids) - ->setViewer(PhabricatorUser::getOmnipotentUser()) - ->execute(); - } else { - $handles = array(); - } - $pushers = array(); $others = array(); diff --git a/src/applications/releeph/field/specification/ReleephOriginalCommitFieldSpecification.php b/src/applications/releeph/field/specification/ReleephOriginalCommitFieldSpecification.php index b1cef1b5ba..0d34c34eb7 100644 --- a/src/applications/releeph/field/specification/ReleephOriginalCommitFieldSpecification.php +++ b/src/applications/releeph/field/specification/ReleephOriginalCommitFieldSpecification.php @@ -11,9 +11,15 @@ final class ReleephOriginalCommitFieldSpecification return 'Commit'; } + public function getRequiredHandlePHIDsForPropertyView() { + return array( + $this->getReleephRequest()->getRequestCommitPHID(), + ); + } + + public function renderPropertyViewValue(array $handles) { - $pull = $this->getReleephRequest(); - return $this->slowlyLoadHandle($pull->getRequestCommitPHID())->renderLink(); + return $this->renderHandleList($handles); } } diff --git a/src/applications/releeph/field/specification/ReleephRequestorFieldSpecification.php b/src/applications/releeph/field/specification/ReleephRequestorFieldSpecification.php index 93b7a0fac0..abcd9bfb8a 100644 --- a/src/applications/releeph/field/specification/ReleephRequestorFieldSpecification.php +++ b/src/applications/releeph/field/specification/ReleephRequestorFieldSpecification.php @@ -11,13 +11,19 @@ final class ReleephRequestorFieldSpecification return 'Requestor'; } - public function renderPropertyViewValue(array $handles) { + public function getRequiredHandlePHIDsForPropertyView() { + $phids = array(); + $phid = $this->getReleephRequest()->getRequestUserPHID(); - $handle = id(new PhabricatorHandleQuery()) - ->setViewer($this->getUser()) - ->withPHIDs(array($phid)) - ->executeOne(); - return $handle->renderLink(); + if ($phid) { + $phids[] = $phid; + } + + return $phids; + } + + public function renderPropertyViewValue(array $handles) { + return $this->renderHandleList($handles); } public function shouldAppearOnCommitMessage() { diff --git a/src/applications/releeph/field/specification/ReleephRevisionFieldSpecification.php b/src/applications/releeph/field/specification/ReleephRevisionFieldSpecification.php index 5455f6d6db..a070ddd8a8 100644 --- a/src/applications/releeph/field/specification/ReleephRevisionFieldSpecification.php +++ b/src/applications/releeph/field/specification/ReleephRevisionFieldSpecification.php @@ -11,26 +11,19 @@ final class ReleephRevisionFieldSpecification return 'Revision'; } - public function renderPropertyViewValue(array $handles) { - $phid = $this - ->getReleephRequest() - ->loadRequestCommitDiffPHID(); - if (!$phid) { - return null; + public function getRequiredHandlePHIDsForPropertyView() { + $phids = array(); + + $phid = $this->getReleephRequest()->loadRequestCommitDiffPHID(); + if ($phid) { + $phids[] = $phid; } - $handles = $this->getReleephRequest()->getHandles(); - $handle = $handles[$phid]; - $link = $handle - // Hack to remove the strike-through rendering of diff links - ->setStatus(null) - ->renderLink(); - return phutil_tag( - 'div', - array( - 'class' => 'releeph-header-text-truncated', - ), - $link); + return $phids; + } + + public function renderPropertyViewValue(array $handles) { + return $this->renderHandleList($handles); } } diff --git a/src/infrastructure/customfield/field/PhabricatorCustomField.php b/src/infrastructure/customfield/field/PhabricatorCustomField.php index 2d1d45d709..2741cf6b25 100644 --- a/src/infrastructure/customfield/field/PhabricatorCustomField.php +++ b/src/infrastructure/customfield/field/PhabricatorCustomField.php @@ -5,6 +5,7 @@ * @task core Core Properties and Field Identity * @task proxy Field Proxies * @task context Contextual Data + * @task render Rendering Utilities * @task storage Field Storage * @task edit Integration with Edit Views * @task view Integration with Property Views @@ -467,6 +468,26 @@ abstract class PhabricatorCustomField { } +/* -( Rendering Utilities )------------------------------------------------ */ + + + /** + * @task render + */ + protected function renderHandleList(array $handles) { + if (!$handles) { + return null; + } + + $out = array(); + foreach ($handles as $handle) { + $out[] = $handle->renderLink(); + } + + return phutil_implode_html(phutil_tag('br'), $out); + } + + /* -( Storage )------------------------------------------------------------ */