Commit dec03cf0 authored by epriestley's avatar epriestley
Browse files

Prepare a replacement for Controller->renderHandlesForPHIDs()

Summary:
Ref T7689. This gives HandleLists `renderList()` and `renderHandle()` methods, which return views that can perform just-in-time data fetching and generally look and feel like other rendering code, instead of being odd pseudo-functional methods on `Controller`.

Also converts callsites on the Maniphest detail page to use these methods.

Next changes will wipe out more of the callsites.

Test Plan:
  - Viewed Maniphest detail page with many relevant handles.
  - Created a new subtask.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7689

Differential Revision: https://secure.phabricator.com/D12205
parent 580590fc
......@@ -1165,7 +1165,9 @@ phutil_register_library_map(array(
'PHUIFormLayoutView' => 'view/form/PHUIFormLayoutView.php',
'PHUIFormMultiSubmitControl' => 'view/form/control/PHUIFormMultiSubmitControl.php',
'PHUIFormPageView' => 'view/form/PHUIFormPageView.php',
'PHUIHandleListView' => 'applications/phid/view/PHUIHandleListView.php',
'PHUIHandleTagListView' => 'applications/phid/view/PHUIHandleTagListView.php',
'PHUIHandleView' => 'applications/phid/view/PHUIHandleView.php',
'PHUIHeaderView' => 'view/phui/PHUIHeaderView.php',
'PHUIIconExample' => 'applications/uiexample/examples/PHUIIconExample.php',
'PHUIIconView' => 'view/phui/PHUIIconView.php',
......@@ -4423,7 +4425,9 @@ phutil_register_library_map(array(
'PHUIFormLayoutView' => 'AphrontView',
'PHUIFormMultiSubmitControl' => 'AphrontFormControl',
'PHUIFormPageView' => 'AphrontView',
'PHUIHandleListView' => 'AphrontTagView',
'PHUIHandleTagListView' => 'AphrontTagView',
'PHUIHandleView' => 'AphrontView',
'PHUIHeaderView' => 'AphrontView',
'PHUIIconExample' => 'PhabricatorUIExample',
'PHUIIconView' => 'AphrontTagView',
......
......@@ -85,10 +85,6 @@ final class ManiphestTaskDetailController extends ManiphestController {
$phids = array_keys($phids);
$handles = $user->loadHandles($phids);
// TODO: This is double-loading because we have a separate call to
// renderHandlesForPHIDs(). Clean this up in the next pass.
$this->loadHandles($phids);
$info_view = null;
if ($parent_task) {
$info_view = new PHUIInfoView();
......@@ -100,8 +96,8 @@ final class ManiphestTaskDetailController extends ManiphestController {
->setText(pht('Create Another Subtask')));
$info_view->appendChild(hsprintf(
'Created a subtask of <strong>%s</strong>',
$handles[$parent_task->getPHID()]->renderLink()));
'Created a subtask of <strong>%s</strong>.',
$handles->renderHandle($parent_task->getPHID())));
} else if ($workflow == 'create') {
$info_view = new PHUIInfoView();
$info_view->setSeverity(PHUIInfoView::SEVERITY_NOTICE);
......@@ -457,7 +453,7 @@ final class ManiphestTaskDetailController extends ManiphestController {
$view->addProperty(
pht('Assigned To'),
$task->getOwnerPHID()
? $handles[$task->getOwnerPHID()]->renderLink()
? $handles->renderHandle($task->getOwnerPHID())
: phutil_tag('em', array(), pht('None')));
$view->addProperty(
......@@ -466,7 +462,7 @@ final class ManiphestTaskDetailController extends ManiphestController {
$view->addProperty(
pht('Author'),
$handles[$task->getAuthorPHID()]->renderLink());
$handles->renderHandle($task->getAuthorPHID()));
$source = $task->getOriginalEmailSource();
if ($source) {
......@@ -504,7 +500,7 @@ final class ManiphestTaskDetailController extends ManiphestController {
->execute();
foreach ($commit_phids as $phid) {
$revisions_commits[$phid] = $handles[$phid]->renderLink();
$revisions_commits[$phid] = $handles->renderHandle($phid);
$revision_phid = key($drev_edges[$phid][$commit_drev]);
$revision_handle = $handles->getHandleIfExists($revision_phid);
if ($revision_handle) {
......@@ -520,9 +516,10 @@ final class ManiphestTaskDetailController extends ManiphestController {
foreach ($edge_types as $edge_type => $edge_name) {
if ($edges[$edge_type]) {
$edge_handles = $viewer->loadHandles(array_keys($edges[$edge_type]));
$view->addProperty(
$edge_name,
$this->renderHandlesForPHIDs(array_keys($edges[$edge_type])));
$edge_handles->renderList());
}
}
......
......@@ -26,6 +26,7 @@ final class PhabricatorHandleList
private $phids;
private $handles;
private $cursor;
private $map;
public function setHandlePool(PhabricatorHandlePool $pool) {
$this->handlePool = $pool;
......@@ -71,6 +72,33 @@ final class PhabricatorHandleList
}
/* -( Rendering )---------------------------------------------------------- */
/**
* Return a @{class:PHUIHandleListView} which can render the handles in
* this list.
*/
public function renderList() {
return id(new PHUIHandleListView())
->setHandleList($this);
}
/**
* Return a @{class:PHUIHandleView} which can render a specific handle.
*/
public function renderHandle($phid) {
if (!isset($this[$phid])) {
throw new Exception(
pht('Trying to render a handle which does not exist!'));
}
return id(new PHUIHandleView())
->setHandleList($this)
->setHandlePHID($phid);
}
/* -( Iterator )----------------------------------------------------------- */
......@@ -99,10 +127,15 @@ final class PhabricatorHandleList
public function offsetExists($offset) {
if ($this->handles === null) {
$this->loadHandles();
// NOTE: We're intentionally not loading handles here so that isset()
// checks do not trigger fetches. This gives us better bulk loading
// behavior, particularly when invoked through methods like renderHandle().
if ($this->map === null) {
$this->map = array_fill_keys($this->phids, true);
}
return isset($this->handles[$offset]);
return isset($this->map[$offset]);
}
public function offsetGet($offset) {
......
<?php
/**
* Convenience class for rendering a list of handles.
*
* This class simplifies rendering a list of handles and improves loading and
* caching semantics in the rendering pipeline by delaying bulk loads until the
* last possible moment.
*/
final class PHUIHandleListView
extends AphrontTagView {
private $handleList;
private $inline;
public function setHandleList(PhabricatorHandleList $list) {
$this->handleList = $list;
return $this;
}
public function setInline($inline) {
$this->inline = $inline;
return $this;
}
public function getInline() {
return $this->inline;
}
protected function getTagName() {
// TODO: It would be nice to render this with a proper <ul />, at least in
// block mode, but don't stir the waters up too much for now.
return 'span';
}
protected function getTagContent() {
$items = array();
foreach ($this->handleList as $handle) {
$items[] = $handle->renderLink();
}
if ($this->getInline()) {
$items = phutil_implode_html(', ', $items);
} else {
$items = phutil_implode_html(phutil_tag('br'), $items);
}
return $items;
}
}
<?php
/**
* Convenience class for rendering a single handle.
*
* This class simplifies rendering a single handle, and improves loading and
* caching semantics in the rendering pipeline by loading data at the last
* moment.
*/
final class PHUIHandleView
extends AphrontView {
private $handleList;
private $handlePHID;
public function setHandleList(PhabricatorHandleList $list) {
$this->handleList = $list;
return $this;
}
public function setHandlePHID($phid) {
$this->handlePHID = $phid;
return $this;
}
public function render() {
return $this->handleList[$this->handlePHID]->renderLink();
}
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment