From a8271ecd4010393ffa612c76a261b4179fbcf5ba Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 30 Mar 2015 06:35:32 -0700 Subject: [PATCH] Remove most callsites to Controller->renderHandlesForPHIDs() Summary: Ref T7689. This moves most of the easy/testable callsites off `Controller->renderHandlesForPHIDs()`. Test Plan: - Viewed a file; viewed author; viewed "attached" tab. - Viewed a mock; viewed attached tasks. - Viewed a credential; viewed "Used By". - Viewed a paste; viewed author; viewed forks; viewed forked from. - Viewed a dashboard; viewed panel list. - Viewed a dashboard panel; viewed "Appears On". - Viewed a Phortune account; viewed "Members"; viewed payment methods. - Viewed a Phortune merchant account; viewed "Members". - Viewed Phortune account switcher; viewed "Accounts". - I just removed "Members:" here since it felt kind of out-of-place anyway. - Viewed a Phragment fragment, viewed "Latest Version", viewed "Snapshots". - Viewed a Phargment snapshot, viewed "Fragment". Reviewers: btrahan Reviewed By: btrahan Subscribers: hach-que, epriestley Maniphest Tasks: T7689 Differential Revision: https://secure.phabricator.com/D12207 --- .../PhabricatorDashboardManageController.php | 5 +-- ...habricatorDashboardPanelViewController.php | 3 +- .../PhabricatorFileInfoController.php | 10 ++---- .../PassphraseCredentialViewController.php | 3 +- .../PhabricatorPasteViewController.php | 14 ++++---- .../people/storage/PhabricatorUser.php | 34 +++++++++++++++++-- .../controller/PholioMockViewController.php | 6 ++-- .../PhortuneAccountListController.php | 2 -- .../PhortuneAccountViewController.php | 8 +---- .../PhortuneMerchantViewController.php | 4 +-- .../controller/PhragmentController.php | 10 ++---- .../controller/PhragmentCreateController.php | 12 +++++-- .../PhragmentSnapshotViewController.php | 7 +--- .../controller/PhragmentVersionController.php | 7 +--- 14 files changed, 61 insertions(+), 64 deletions(-) diff --git a/src/applications/dashboard/controller/PhabricatorDashboardManageController.php b/src/applications/dashboard/controller/PhabricatorDashboardManageController.php index 41028fac61..1ee4a533a6 100644 --- a/src/applications/dashboard/controller/PhabricatorDashboardManageController.php +++ b/src/applications/dashboard/controller/PhabricatorDashboardManageController.php @@ -165,12 +165,9 @@ final class PhabricatorDashboardManageController pht('Editable By'), $descriptions[PhabricatorPolicyCapability::CAN_EDIT]); - $panel_phids = $dashboard->getPanelPHIDs(); - $this->loadHandles($panel_phids); - $properties->addProperty( pht('Panels'), - $this->renderHandlesForPHIDs($panel_phids)); + $viewer->renderHandleList($dashboard->getPanelPHIDs())); return $properties; } diff --git a/src/applications/dashboard/controller/PhabricatorDashboardPanelViewController.php b/src/applications/dashboard/controller/PhabricatorDashboardPanelViewController.php index 91e664a9ae..01bd083f8f 100644 --- a/src/applications/dashboard/controller/PhabricatorDashboardPanelViewController.php +++ b/src/applications/dashboard/controller/PhabricatorDashboardPanelViewController.php @@ -163,7 +163,6 @@ final class PhabricatorDashboardPanelViewController $dashboard_phids = PhabricatorEdgeQuery::loadDestinationPHIDs( $panel->getPHID(), PhabricatorDashboardPanelHasDashboardEdgeType::EDGECONST); - $this->loadHandles($dashboard_phids); $does_not_appear = pht( 'This panel does not appear on any dashboards.'); @@ -171,7 +170,7 @@ final class PhabricatorDashboardPanelViewController $properties->addProperty( pht('Appears On'), $dashboard_phids - ? $this->renderHandlesForPHIDs($dashboard_phids) + ? $viewer->renderHandleList($dashboard_phids) : phutil_tag('em', array(), $does_not_appear)); return $properties; diff --git a/src/applications/files/controller/PhabricatorFileInfoController.php b/src/applications/files/controller/PhabricatorFileInfoController.php index 27a67005d9..3d9731f6b4 100644 --- a/src/applications/files/controller/PhabricatorFileInfoController.php +++ b/src/applications/files/controller/PhabricatorFileInfoController.php @@ -39,11 +39,6 @@ final class PhabricatorFileInfoController extends PhabricatorFileController { $phid = $file->getPHID(); - $handle_phids = array_merge( - array($file->getAuthorPHID()), - $file->getObjectPHIDs()); - - $this->loadHandles($handle_phids); $header = id(new PHUIHeaderView()) ->setUser($user) ->setPolicyObject($file) @@ -185,7 +180,6 @@ final class PhabricatorFileInfoController extends PhabricatorFileController { $request = $this->getRequest(); $user = $request->getUser(); - $properties = id(new PHUIPropertyListView()); $properties->setActionList($actions); $box->addPropertyList($properties, pht('Details')); @@ -193,7 +187,7 @@ final class PhabricatorFileInfoController extends PhabricatorFileController { if ($file->getAuthorPHID()) { $properties->addProperty( pht('Author'), - $this->getHandle($file->getAuthorPHID())->renderLink()); + $user->renderHandle($file->getAuthorPHID())); } $properties->addProperty( @@ -270,7 +264,7 @@ final class PhabricatorFileInfoController extends PhabricatorFileController { $attached->addProperty( pht('Attached To'), - $this->renderHandlesForPHIDs($phids)); + $user->renderHandleList($phids)); } diff --git a/src/applications/passphrase/controller/PassphraseCredentialViewController.php b/src/applications/passphrase/controller/PassphraseCredentialViewController.php index 8edd785a94..cabd721c89 100644 --- a/src/applications/passphrase/controller/PassphraseCredentialViewController.php +++ b/src/applications/passphrase/controller/PassphraseCredentialViewController.php @@ -189,10 +189,9 @@ final class PassphraseCredentialViewController extends PassphraseController { PhabricatorCredentialsUsedByObjectEdgeType::EDGECONST); if ($used_by_phids) { - $this->loadHandles($used_by_phids); $properties->addProperty( pht('Used By'), - $this->renderHandlesForPHIDs($used_by_phids)); + $viewer->renderHandleList($used_by_phids)); } $description = $credential->getDescription(); diff --git a/src/applications/paste/controller/PhabricatorPasteViewController.php b/src/applications/paste/controller/PhabricatorPasteViewController.php index 92d150c3a7..0b18f94501 100644 --- a/src/applications/paste/controller/PhabricatorPasteViewController.php +++ b/src/applications/paste/controller/PhabricatorPasteViewController.php @@ -176,35 +176,35 @@ final class PhabricatorPasteViewController extends PhabricatorPasteController { PhabricatorPaste $paste, array $child_phids, PhabricatorActionListView $actions) { + $viewer = $this->getViewer(); - $user = $this->getRequest()->getUser(); $properties = id(new PHUIPropertyListView()) - ->setUser($user) + ->setUser($viewer) ->setObject($paste) ->setActionList($actions); $properties->addProperty( pht('Author'), - $this->getHandle($paste->getAuthorPHID())->renderLink()); + $viewer->renderHandle($paste->getAuthorPHID())); $properties->addProperty( pht('Created'), - phabricator_datetime($paste->getDateCreated(), $user)); + phabricator_datetime($paste->getDateCreated(), $viewer)); if ($paste->getParentPHID()) { $properties->addProperty( pht('Forked From'), - $this->getHandle($paste->getParentPHID())->renderLink()); + $viewer->renderHandle($paste->getParentPHID())); } if ($child_phids) { $properties->addProperty( pht('Forks'), - $this->renderHandlesForPHIDs($child_phids)); + $viewer->renderHandleList($child_phids)); } $descriptions = PhabricatorPolicyQuery::renderPolicyDescriptions( - $user, + $viewer, $paste); return $properties; diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index 82548701f3..ea138042b2 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -1,7 +1,8 @@ List of PHIDs to load. * @return PhabricatorHandleList Handle list object. + * @task handle */ public function loadHandles(array $phids) { if ($this->handlePool === null) { @@ -821,6 +823,34 @@ EOBODY; } + /** + * Get a @{class:PHUIHandleView} for a single handle. + * + * This benefits from the viewer's internal handle pool. + * + * @param phid PHID to render a handle for. + * @return PHUIHandleView View of the handle. + * @task handle + */ + public function renderHandle($phid) { + return $this->loadHandles(array($phid))->renderHandle($phid); + } + + + /** + * Get a @{class:PHUIHandleListView} for a list of handles. + * + * This benefits from the viewer's internal handle pool. + * + * @param list List of PHIDs to render. + * @return PHUIHandleListView View of the handles. + * @task handle + */ + public function renderHandleList(array $phids) { + return $this->loadHandles($phids)->renderList(); + } + + /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/applications/pholio/controller/PholioMockViewController.php b/src/applications/pholio/controller/PholioMockViewController.php index 3f67817ccf..3487a21dbf 100644 --- a/src/applications/pholio/controller/PholioMockViewController.php +++ b/src/applications/pholio/controller/PholioMockViewController.php @@ -42,8 +42,6 @@ final class PholioMockViewController extends PholioController { $mock->getPHID(), PholioMockHasTaskEdgeType::EDGECONST); $this->setManiphestTaskPHIDs($phids); - $phids[] = $mock->getAuthorPHID(); - $this->loadHandles($phids); $engine = id(new PhabricatorMarkupEngine()) ->setViewer($user); @@ -167,7 +165,7 @@ final class PholioMockViewController extends PholioController { $properties->addProperty( pht('Author'), - $this->getHandle($mock->getAuthorPHID())->renderLink()); + $user->renderHandle($mock->getAuthorPHID())); $properties->addProperty( pht('Created'), @@ -176,7 +174,7 @@ final class PholioMockViewController extends PholioController { if ($this->getManiphestTaskPHIDs()) { $properties->addProperty( pht('Maniphest Tasks'), - $this->renderHandlesForPHIDs($this->getManiphestTaskPHIDs())); + $user->renderHandleList($this->getManiphestTaskPHIDs())); } $properties->invokeWillRenderEvent(); diff --git a/src/applications/phortune/controller/PhortuneAccountListController.php b/src/applications/phortune/controller/PhortuneAccountListController.php index 89fdf58b9d..8c7d765b9d 100644 --- a/src/applications/phortune/controller/PhortuneAccountListController.php +++ b/src/applications/phortune/controller/PhortuneAccountListController.php @@ -36,12 +36,10 @@ final class PhortuneAccountListController extends PhortuneController { foreach ($accounts as $account) { $this->loadHandles($account->getMemberPHIDs()); - $members = $this->renderHandlesForPHIDs($account->getMemberPHIDs(), ','); $item = id(new PHUIObjectItemView()) ->setObjectName(pht('Account %d', $account->getID())) ->setHeader($account->getName()) ->setHref($this->getApplicationURI($account->getID().'/')) - ->addAttribute(pht('Members: %s', $members)) ->setObject($account); $payment_list->addItem($item); diff --git a/src/applications/phortune/controller/PhortuneAccountViewController.php b/src/applications/phortune/controller/PhortuneAccountViewController.php index 2a50c2ef0f..7991a260b3 100644 --- a/src/applications/phortune/controller/PhortuneAccountViewController.php +++ b/src/applications/phortune/controller/PhortuneAccountViewController.php @@ -57,11 +57,9 @@ final class PhortuneAccountViewController extends PhortuneController { ->setObject($account) ->setUser($viewer); - $this->loadHandles($account->getMemberPHIDs()); - $properties->addProperty( pht('Members'), - $this->renderHandlesForPHIDs($account->getMemberPHIDs())); + $viewer->renderHandleList($account->getMemberPHIDs())); $status_items = $this->getStatusItemsForAccount($account, $invoices); $status_view = new PHUIStatusListView(); @@ -137,10 +135,6 @@ final class PhortuneAccountViewController extends PhortuneController { ->withAccountPHIDs(array($account->getPHID())) ->execute(); - if ($methods) { - $this->loadHandles(mpull($methods, 'getAuthorPHID')); - } - foreach ($methods as $method) { $id = $method->getID(); diff --git a/src/applications/phortune/controller/PhortuneMerchantViewController.php b/src/applications/phortune/controller/PhortuneMerchantViewController.php index 675a1afc47..94a53742b0 100644 --- a/src/applications/phortune/controller/PhortuneMerchantViewController.php +++ b/src/applications/phortune/controller/PhortuneMerchantViewController.php @@ -135,11 +135,9 @@ final class PhortuneMerchantViewController $view->addProperty(pht('Status'), $status_view); - $this->loadHandles($merchant->getMemberPHIDs()); - $view->addProperty( pht('Members'), - $this->renderHandlesForPHIDs($merchant->getMemberPHIDs())); + $viewer->renderHandleList($merchant->getMemberPHIDs())); $view->invokeWillRenderEvent(); diff --git a/src/applications/phragment/controller/PhragmentController.php b/src/applications/phragment/controller/PhragmentController.php index 00ca5aa566..096ee21a1f 100644 --- a/src/applications/phragment/controller/PhragmentController.php +++ b/src/applications/phragment/controller/PhragmentController.php @@ -57,21 +57,15 @@ abstract class PhragmentController extends PhabricatorController { $viewer = $this->getRequest()->getUser(); - $phids = array(); - $phids[] = $fragment->getLatestVersionPHID(); - $snapshot_phids = array(); $snapshots = id(new PhragmentSnapshotQuery()) ->setViewer($viewer) ->withPrimaryFragmentPHIDs(array($fragment->getPHID())) ->execute(); foreach ($snapshots as $snapshot) { - $phids[] = $snapshot->getPHID(); $snapshot_phids[] = $snapshot->getPHID(); } - $this->loadHandles($phids); - $file = null; $file_uri = null; if (!$fragment->isDirectory()) { @@ -183,7 +177,7 @@ abstract class PhragmentController extends PhabricatorController { } $properties->addProperty( pht('Latest Version'), - $this->renderHandlesForPHIDs(array($fragment->getLatestVersionPHID()))); + $viewer->renderHandle($fragment->getLatestVersionPHID())); } else { $properties->addProperty( pht('Type'), @@ -193,7 +187,7 @@ abstract class PhragmentController extends PhabricatorController { if (count($snapshot_phids) > 0) { $properties->addProperty( pht('Snapshots'), - $this->renderHandlesForPHIDs($snapshot_phids)); + $viewer->renderHandleList($snapshot_phids)); } return id(new PHUIObjectBoxView()) diff --git a/src/applications/phragment/controller/PhragmentCreateController.php b/src/applications/phragment/controller/PhragmentCreateController.php index 5c9aee5207..d899754a1c 100644 --- a/src/applications/phragment/controller/PhragmentCreateController.php +++ b/src/applications/phragment/controller/PhragmentCreateController.php @@ -43,8 +43,11 @@ final class PhragmentCreateController extends PhragmentController { $errors[] = pht('The fragment name can not contain \'/\'.'); } - $file = id(new PhabricatorFile())->load($v_fileid); - if ($file === null) { + $file = id(new PhabricatorFileQuery()) + ->setViewer($viewer) + ->withIDs(array($v_fileid)) + ->executeOne(); + if (!$file) { $errors[] = pht('The specified file doesn\'t exist.'); } @@ -115,9 +118,12 @@ final class PhragmentCreateController extends PhragmentController { $box = id(new PHUIObjectBoxView()) ->setHeaderText('Create Fragment') - ->setValidationException(null) ->setForm($form); + if ($error_view) { + $box->setInfoView($error_view); + } + return $this->buildApplicationPage( array( $crumbs, diff --git a/src/applications/phragment/controller/PhragmentSnapshotViewController.php b/src/applications/phragment/controller/PhragmentSnapshotViewController.php index 006ba448c7..545e8806eb 100644 --- a/src/applications/phragment/controller/PhragmentSnapshotViewController.php +++ b/src/applications/phragment/controller/PhragmentSnapshotViewController.php @@ -90,11 +90,6 @@ final class PhragmentSnapshotViewController extends PhragmentController { $viewer = $this->getRequest()->getUser(); - $phids = array(); - $phids[] = $snapshot->getPrimaryFragmentPHID(); - - $this->loadHandles($phids); - $header = id(new PHUIHeaderView()) ->setHeader(pht('"%s" Snapshot', $snapshot->getName())) ->setPolicyObject($snapshot) @@ -146,7 +141,7 @@ final class PhragmentSnapshotViewController extends PhragmentController { $snapshot->getName()); $properties->addProperty( pht('Fragment'), - $this->renderHandlesForPHIDs(array($snapshot->getPrimaryFragmentPHID()))); + $viewer->renderHandle($snapshot->getPrimaryFragmentPHID())); return id(new PHUIObjectBoxView()) ->setHeader($header) diff --git a/src/applications/phragment/controller/PhragmentVersionController.php b/src/applications/phragment/controller/PhragmentVersionController.php index ceebd3fb96..9267fdc5b1 100644 --- a/src/applications/phragment/controller/PhragmentVersionController.php +++ b/src/applications/phragment/controller/PhragmentVersionController.php @@ -33,11 +33,6 @@ final class PhragmentVersionController extends PhragmentController { $crumbs = $this->buildApplicationCrumbsWithPath($parents); $crumbs->addTextCrumb(pht('View Version %d', $version->getSequence())); - $phids = array(); - $phids[] = $version->getFilePHID(); - - $this->loadHandles($phids); - $file = id(new PhabricatorFileQuery()) ->setViewer($viewer) ->withPHIDs(array($version->getFilePHID())) @@ -71,7 +66,7 @@ final class PhragmentVersionController extends PhragmentController { ->setActionList($actions); $properties->addProperty( pht('File'), - $this->renderHandlesForPHIDs(array($version->getFilePHID()))); + $viewer->renderHandle($version->getFilePHID())); $box = id(new PHUIObjectBoxView()) ->setHeader($header)