diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -765,6 +765,7 @@ 'DiffusionCommitRevisionAcceptedHeraldField' => 'applications/diffusion/herald/DiffusionCommitRevisionAcceptedHeraldField.php', 'DiffusionCommitRevisionAcceptingReviewersHeraldField' => 'applications/diffusion/herald/DiffusionCommitRevisionAcceptingReviewersHeraldField.php', 'DiffusionCommitRevisionHeraldField' => 'applications/diffusion/herald/DiffusionCommitRevisionHeraldField.php', + 'DiffusionCommitRevisionQuery' => 'applications/diffusion/query/DiffusionCommitRevisionQuery.php', 'DiffusionCommitRevisionReviewersHeraldField' => 'applications/diffusion/herald/DiffusionCommitRevisionReviewersHeraldField.php', 'DiffusionCommitRevisionSubscribersHeraldField' => 'applications/diffusion/herald/DiffusionCommitRevisionSubscribersHeraldField.php', 'DiffusionCommitSearchConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionCommitSearchConduitAPIMethod.php', @@ -6427,6 +6428,7 @@ 'DiffusionCommitRevisionAcceptedHeraldField' => 'DiffusionCommitHeraldField', 'DiffusionCommitRevisionAcceptingReviewersHeraldField' => 'DiffusionCommitHeraldField', 'DiffusionCommitRevisionHeraldField' => 'DiffusionCommitHeraldField', + 'DiffusionCommitRevisionQuery' => 'Phobject', 'DiffusionCommitRevisionReviewersHeraldField' => 'DiffusionCommitHeraldField', 'DiffusionCommitRevisionSubscribersHeraldField' => 'DiffusionCommitHeraldField', 'DiffusionCommitSearchConduitAPIMethod' => 'PhabricatorSearchEngineAPIMethod', diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -158,18 +158,6 @@ return '/'.$this->getMonogram(); } - public function loadIDsByCommitPHIDs($phids) { - if (!$phids) { - return array(); - } - $revision_ids = queryfx_all( - $this->establishConnection('r'), - 'SELECT * FROM %T WHERE commitPHID IN (%Ls)', - self::TABLE_COMMIT, - $phids); - return ipull($revision_ids, 'revisionID', 'commitPHID'); - } - public function loadCommitPHIDs() { if (!$this->getID()) { return ($this->commits = array()); diff --git a/src/applications/diffusion/controller/DiffusionBlameController.php b/src/applications/diffusion/controller/DiffusionBlameController.php --- a/src/applications/diffusion/controller/DiffusionBlameController.php +++ b/src/applications/diffusion/controller/DiffusionBlameController.php @@ -36,7 +36,9 @@ $commit_map = mpull($commits, 'getCommitIdentifier', 'getPHID'); - $revision_map = $this->loadRevisionsForCommits($commits); + $revision_map = DiffusionCommitRevisionQuery::loadRevisionMapForCommits( + $viewer, + $commits); $base_href = (string)$drequest->generateURI( array( @@ -267,45 +269,4 @@ } } - private function loadRevisionsForCommits(array $commits) { - if (!$commits) { - return array(); - } - - $commit_phids = mpull($commits, 'getPHID'); - - $edge_query = id(new PhabricatorEdgeQuery()) - ->withSourcePHIDs($commit_phids) - ->withEdgeTypes( - array( - DiffusionCommitHasRevisionEdgeType::EDGECONST, - )); - $edge_query->execute(); - - $revision_phids = $edge_query->getDestinationPHIDs(); - if (!$revision_phids) { - return array(); - } - - $viewer = $this->getViewer(); - - $revisions = id(new DifferentialRevisionQuery()) - ->setViewer($viewer) - ->withPHIDs($revision_phids) - ->execute(); - $revisions = mpull($revisions, null, 'getPHID'); - - $map = array(); - foreach ($commit_phids as $commit_phid) { - $revision_phids = $edge_query->getDestinationPHIDs( - array( - $commit_phid, - )); - - $map[$commit_phid] = array_select_keys($revisions, $revision_phids); - } - - return $map; - } - } diff --git a/src/applications/diffusion/controller/DiffusionBrowseController.php b/src/applications/diffusion/controller/DiffusionBrowseController.php --- a/src/applications/diffusion/controller/DiffusionBrowseController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseController.php @@ -1106,11 +1106,7 @@ $history_table = id(new DiffusionHistoryTableView()) ->setViewer($viewer) ->setDiffusionRequest($drequest) - ->setHistory($history); - - $history_table->loadRevisions(); - - $history_table + ->setHistory($history) ->setParents($results['parents']) ->setFilterParents(true) ->setIsHead(true) diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -807,8 +807,6 @@ ->setDiffusionRequest($drequest) ->setHistory($merges); - $history_table->loadRevisions(); - $panel = id(new PHUIObjectBoxView()) ->setHeaderText(pht('Merged Changes')) ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) diff --git a/src/applications/diffusion/controller/DiffusionCompareController.php b/src/applications/diffusion/controller/DiffusionCompareController.php --- a/src/applications/diffusion/controller/DiffusionCompareController.php +++ b/src/applications/diffusion/controller/DiffusionCompareController.php @@ -299,11 +299,7 @@ $history_table = id(new DiffusionHistoryTableView()) ->setUser($viewer) ->setDiffusionRequest($drequest) - ->setHistory($history); - - $history_table->loadRevisions(); - - $history_table + ->setHistory($history) ->setParents($results['parents']) ->setFilterParents(true) ->setIsHead(!$pager->getOffset()) diff --git a/src/applications/diffusion/controller/DiffusionGraphController.php b/src/applications/diffusion/controller/DiffusionGraphController.php --- a/src/applications/diffusion/controller/DiffusionGraphController.php +++ b/src/applications/diffusion/controller/DiffusionGraphController.php @@ -40,7 +40,6 @@ ->setDiffusionRequest($drequest) ->setHistory($history); - $graph->loadRevisions(); $show_graph = !strlen($drequest->getPath()); if ($show_graph) { $graph->setParents($history_results['parents']); diff --git a/src/applications/diffusion/controller/DiffusionHistoryController.php b/src/applications/diffusion/controller/DiffusionHistoryController.php --- a/src/applications/diffusion/controller/DiffusionHistoryController.php +++ b/src/applications/diffusion/controller/DiffusionHistoryController.php @@ -40,7 +40,6 @@ ->setDiffusionRequest($drequest) ->setHistory($history); - $history_list->loadRevisions(); $header = $this->buildHeader($drequest); $crumbs = $this->buildCrumbs( diff --git a/src/applications/diffusion/controller/DiffusionRepositoryController.php b/src/applications/diffusion/controller/DiffusionRepositoryController.php --- a/src/applications/diffusion/controller/DiffusionRepositoryController.php +++ b/src/applications/diffusion/controller/DiffusionRepositoryController.php @@ -440,17 +440,13 @@ $history_table = id(new DiffusionHistoryTableView()) ->setUser($viewer) ->setDiffusionRequest($drequest) - ->setHistory($history); - - // TODO: Super sketchy. - $history_table->loadRevisions(); + ->setHistory($history) + ->setIsHead(true); if ($history_results) { $history_table->setParents($history_results['parents']); } - $history_table->setIsHead(true); - $panel = id(new PHUIObjectBoxView()) ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->addClass('diffusion-mobile-view'); diff --git a/src/applications/diffusion/query/DiffusionCommitRevisionQuery.php b/src/applications/diffusion/query/DiffusionCommitRevisionQuery.php new file mode 100644 --- /dev/null +++ b/src/applications/diffusion/query/DiffusionCommitRevisionQuery.php @@ -0,0 +1,49 @@ +withSourcePHIDs($commit_phids) + ->withEdgeTypes( + array( + DiffusionCommitHasRevisionEdgeType::EDGECONST, + )); + $edge_query->execute(); + + $revision_phids = $edge_query->getDestinationPHIDs(); + if (!$revision_phids) { + return array(); + } + + $revisions = id(new DifferentialRevisionQuery()) + ->setViewer($viewer) + ->withPHIDs($revision_phids) + ->execute(); + $revisions = mpull($revisions, null, 'getPHID'); + + $map = array(); + foreach ($commit_phids as $commit_phid) { + $revision_phids = $edge_query->getDestinationPHIDs( + array( + $commit_phid, + )); + + $map[$commit_phid] = array_select_keys($revisions, $revision_phids); + } + + return $map; + } + +} diff --git a/src/applications/diffusion/view/DiffusionHistoryListView.php b/src/applications/diffusion/view/DiffusionHistoryListView.php --- a/src/applications/diffusion/view/DiffusionHistoryListView.php +++ b/src/applications/diffusion/view/DiffusionHistoryListView.php @@ -104,16 +104,17 @@ $diff_tag = null; if ($show_revisions && $commit) { - $d_id = idx($this->getRevisions(), $commit->getPHID()); - if ($d_id) { + $revisions = $this->getRevisionsForCommit($commit); + if ($revisions) { + $revision = head($revisions); $diff_tag = id(new PHUITagView()) - ->setName('D'.$d_id) + ->setName($revision->getMonogram()) ->setType(PHUITagView::TYPE_SHADE) ->setColor(PHUITagView::COLOR_BLUE) - ->setHref('/D'.$d_id) + ->setHref($revision->getURI()) ->setBorder(PHUITagView::BORDER_NONE) ->setSlimShady(true); - } + } } $build_view = null; diff --git a/src/applications/diffusion/view/DiffusionHistoryTableView.php b/src/applications/diffusion/view/DiffusionHistoryTableView.php --- a/src/applications/diffusion/view/DiffusionHistoryTableView.php +++ b/src/applications/diffusion/view/DiffusionHistoryTableView.php @@ -127,6 +127,20 @@ 'tip' => $name, )); + $revision_link = null; + if ($commit) { + $revisions = $this->getRevisionsForCommit($commit); + if ($revisions) { + $revision = head($revisions); + $revision_link = phutil_tag( + 'a', + array( + 'href' => $revision->getURI(), + ), + $revision->getMonogram()); + } + } + $rows[] = array( $graph ? $graph[$ii++] : null, $browse, @@ -135,9 +149,7 @@ $history->getCommitIdentifier()), $build, $audit_view, - ($commit ? - self::linkRevision(idx($this->getRevisions(), $commit->getPHID())) : - null), + $revision_link, $author, $summary, $committed, diff --git a/src/applications/diffusion/view/DiffusionHistoryView.php b/src/applications/diffusion/view/DiffusionHistoryView.php --- a/src/applications/diffusion/view/DiffusionHistoryView.php +++ b/src/applications/diffusion/view/DiffusionHistoryView.php @@ -9,6 +9,7 @@ private $isTail; private $parents; private $filterParents; + private $revisionMap; public function setHistory(array $history) { assert_instances_of($history, 'DiffusionPathChange'); @@ -20,24 +21,6 @@ return $this->history; } - public function loadRevisions() { - $commit_phids = array(); - foreach ($this->history as $item) { - if ($item->getCommit()) { - $commit_phids[] = $item->getCommit()->getPHID(); - } - } - - // TODO: Get rid of this. - $this->revisions = id(new DifferentialRevision()) - ->loadIDsByCommitPHIDs($commit_phids); - return $this; - } - - public function getRevisions() { - return $this->revisions; - } - public function setHandles(array $handles) { assert_instances_of($handles, 'PhabricatorObjectHandle'); $this->handles = $handles; @@ -98,4 +81,30 @@ public function render() {} + final protected function getRevisionsForCommit( + PhabricatorRepositoryCommit $commit) { + + if ($this->revisionMap === null) { + $this->revisionMap = $this->newRevisionMap(); + } + + return idx($this->revisionMap, $commit->getPHID(), array()); + } + + private function newRevisionMap() { + $history = $this->history; + + $commits = array(); + foreach ($history as $item) { + $commit = $item->getCommit(); + if ($commit) { + $commits[] = $commit; + } + } + + return DiffusionCommitRevisionQuery::loadRevisionMapForCommits( + $this->getViewer(), + $commits); + } + } diff --git a/src/applications/diffusion/view/DiffusionView.php b/src/applications/diffusion/view/DiffusionView.php --- a/src/applications/diffusion/view/DiffusionView.php +++ b/src/applications/diffusion/view/DiffusionView.php @@ -169,19 +169,6 @@ $detail); } - final public static function linkRevision($id) { - if (!$id) { - return null; - } - - return phutil_tag( - 'a', - array( - 'href' => "/D{$id}", - ), - "D{$id}"); - } - final public static function renderName($name) { $email = new PhutilEmailAddress($name); if ($email->getDisplayName() && $email->getDomainName()) {