From 2b0632b442a2761cb8f294cbca1292aa73778ddc Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 12 Jul 2020 09:23:21 -0700 Subject: [PATCH] Remove "DiffusionHistoryTableView" and "DiffusionHistoryView" Summary: Ref T13552. Currently, the "Browse" page shows a snippet of unmerged changes if you're looking at a non-default branch. Remove this for consistency with the simplified main "Browse" page. This is reachable via "Compare". Update the "Compare" page to use the new "CommitGraphView". Test Plan: - Looked at the "Browse" page of "stable". - Looked at the "Compare" page for "stable vs master". Maniphest Tasks: T13552 Differential Revision: https://secure.phabricator.com/D21414 --- src/__phutil_library_map__.php | 4 - .../controller/DiffusionBrowseController.php | 63 ------ .../controller/DiffusionCompareController.php | 16 +- .../view/DiffusionHistoryTableView.php | 208 ------------------ .../diffusion/view/DiffusionHistoryView.php | 117 ---------- 5 files changed, 3 insertions(+), 405 deletions(-) delete mode 100644 src/applications/diffusion/view/DiffusionHistoryTableView.php delete mode 100644 src/applications/diffusion/view/DiffusionHistoryView.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 60677143d5..97ea8abf05 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -863,8 +863,6 @@ phutil_register_library_map(array( 'DiffusionGitWireProtocolRefList' => 'applications/diffusion/protocol/DiffusionGitWireProtocolRefList.php', 'DiffusionHistoryController' => 'applications/diffusion/controller/DiffusionHistoryController.php', 'DiffusionHistoryQueryConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php', - 'DiffusionHistoryTableView' => 'applications/diffusion/view/DiffusionHistoryTableView.php', - 'DiffusionHistoryView' => 'applications/diffusion/view/DiffusionHistoryView.php', 'DiffusionHovercardEngineExtension' => 'applications/diffusion/engineextension/DiffusionHovercardEngineExtension.php', 'DiffusionIdentityAssigneeDatasource' => 'applications/diffusion/typeahead/DiffusionIdentityAssigneeDatasource.php', 'DiffusionIdentityAssigneeEditField' => 'applications/diffusion/editfield/DiffusionIdentityAssigneeEditField.php', @@ -6955,8 +6953,6 @@ phutil_register_library_map(array( 'DiffusionGitWireProtocolRefList' => 'Phobject', 'DiffusionHistoryController' => 'DiffusionController', 'DiffusionHistoryQueryConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod', - 'DiffusionHistoryTableView' => 'DiffusionHistoryView', - 'DiffusionHistoryView' => 'DiffusionView', 'DiffusionHovercardEngineExtension' => 'PhabricatorHovercardEngineExtension', 'DiffusionIdentityAssigneeDatasource' => 'PhabricatorTypeaheadCompositeDatasource', 'DiffusionIdentityAssigneeEditField' => 'PhabricatorTokenizerEditField', diff --git a/src/applications/diffusion/controller/DiffusionBrowseController.php b/src/applications/diffusion/controller/DiffusionBrowseController.php index e4e7a0c030..76efa41866 100644 --- a/src/applications/diffusion/controller/DiffusionBrowseController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseController.php @@ -292,7 +292,6 @@ final class DiffusionBrowseController extends DiffusionController { $empty_result = null; $browse_panel = null; - $branch_panel = null; if (!$results->isValidResults()) { $empty_result = new DiffusionEmptyResultView(); $empty_result->setDiffusionRequest($drequest); @@ -328,12 +327,6 @@ final class DiffusionBrowseController extends DiffusionController { ->setTable($browse_table) ->addClass('diffusion-mobile-view') ->setPager($pager); - - $path = $drequest->getPath(); - $is_branch = (!strlen($path) && $repository->supportsBranchComparison()); - if ($is_branch) { - $branch_panel = $this->buildBranchTable(); - } } $open_revisions = $this->buildOpenRevisions(); @@ -359,7 +352,6 @@ final class DiffusionBrowseController extends DiffusionController { ->setFooter( array( $bar, - $branch_panel, $empty_result, $browse_panel, $open_revisions, @@ -1074,59 +1066,4 @@ final class DiffusionBrowseController extends DiffusionController { return $file; } - private function buildBranchTable() { - $viewer = $this->getViewer(); - $drequest = $this->getDiffusionRequest(); - $repository = $drequest->getRepository(); - - $branch = $drequest->getBranch(); - $default_branch = $repository->getDefaultBranch(); - - if ($branch === $default_branch) { - return null; - } - - $pager = id(new PHUIPagerView()) - ->setPageSize(10); - - try { - $results = $this->callConduitWithDiffusionRequest( - 'diffusion.historyquery', - array( - 'commit' => $branch, - 'against' => $default_branch, - 'path' => $drequest->getPath(), - 'offset' => $pager->getOffset(), - 'limit' => $pager->getPageSize() + 1, - )); - } catch (Exception $ex) { - return null; - } - - $history = DiffusionPathChange::newFromConduit($results['pathChanges']); - $history = $pager->sliceResults($history); - - if (!$history) { - return null; - } - - $history_table = id(new DiffusionHistoryTableView()) - ->setViewer($viewer) - ->setDiffusionRequest($drequest) - ->setHistory($history) - ->setParents($results['parents']) - ->setFilterParents(true) - ->setIsHead(true) - ->setIsTail(!$pager->getHasMorePages()); - - $header = id(new PHUIHeaderView()) - ->setHeader(pht('%s vs %s', $branch, $default_branch)); - - return id(new PHUIObjectBoxView()) - ->setHeader($header) - ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) - ->addClass('diffusion-mobile-view') - ->setTable($history_table); - } - } diff --git a/src/applications/diffusion/controller/DiffusionCompareController.php b/src/applications/diffusion/controller/DiffusionCompareController.php index 7dc3fb14c5..26d888dfb2 100644 --- a/src/applications/diffusion/controller/DiffusionCompareController.php +++ b/src/applications/diffusion/controller/DiffusionCompareController.php @@ -285,7 +285,6 @@ final class DiffusionCompareController extends DiffusionController { $request = $this->getRequest(); $viewer = $this->getViewer(); $drequest = $this->getDiffusionRequest(); - $repository = $drequest->getRepository(); if (!$history) { return $this->renderStatusMessage( @@ -296,8 +295,8 @@ final class DiffusionCompareController extends DiffusionController { phutil_tag('strong', array(), $against_ref))); } - $history_table = id(new DiffusionHistoryTableView()) - ->setUser($viewer) + $history_view = id(new DiffusionCommitGraphView()) + ->setViewer($viewer) ->setDiffusionRequest($drequest) ->setHistory($history) ->setParents($results['parents']) @@ -305,15 +304,6 @@ final class DiffusionCompareController extends DiffusionController { ->setIsHead(!$pager->getOffset()) ->setIsTail(!$pager->getHasMorePages()); - $header = id(new PHUIHeaderView()) - ->setHeader(pht('Commits')); - - return id(new PHUIObjectBoxView()) - ->setHeader($header) - ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) - ->setTable($history_table) - ->addClass('diffusion-mobile-view') - ->setPager($pager); - + return $history_view; } } diff --git a/src/applications/diffusion/view/DiffusionHistoryTableView.php b/src/applications/diffusion/view/DiffusionHistoryTableView.php deleted file mode 100644 index bd4fd5490a..0000000000 --- a/src/applications/diffusion/view/DiffusionHistoryTableView.php +++ /dev/null @@ -1,208 +0,0 @@ -getDiffusionRequest(); - - $viewer = $this->getUser(); - - $buildables = $this->loadBuildables( - mpull($this->getHistory(), 'getCommit')); - $has_any_build = false; - - $show_revisions = PhabricatorApplication::isClassInstalledForViewer( - 'PhabricatorDifferentialApplication', - $viewer); - - $handles = $viewer->loadHandles($this->getRequiredHandlePHIDs()); - - $graph = null; - if ($this->getParents()) { - $parents = $this->getParents(); - - // If we're filtering parents, remove relationships which point to - // commits that are not part of the visible graph. Otherwise, we get - // a big tree of nonsense when viewing release branches like "stable" - // versus "master". - if ($this->getFilterParents()) { - foreach ($parents as $key => $nodes) { - foreach ($nodes as $nkey => $node) { - if (empty($parents[$node])) { - unset($parents[$key][$nkey]); - } - } - } - } - - $graph = id(new PHUIDiffGraphView()) - ->setIsHead($this->getIsHead()) - ->setIsTail($this->getIsTail()) - ->renderGraph($parents); - } - - $show_builds = PhabricatorApplication::isClassInstalledForViewer( - 'PhabricatorHarbormasterApplication', - $this->getUser()); - - $rows = array(); - $ii = 0; - foreach ($this->getHistory() as $history) { - $epoch = $history->getEpoch(); - - if ($epoch) { - $committed = $viewer->formatShortDateTime($epoch); - } else { - $committed = null; - } - - $data = $history->getCommitData(); - $author_phid = $committer = $committer_phid = null; - if ($data) { - $author_phid = $data->getCommitDetail('authorPHID'); - $committer_phid = $data->getCommitDetail('committerPHID'); - $committer = $data->getCommitDetail('committer'); - } - - if ($author_phid && isset($handles[$author_phid])) { - $author = $handles[$author_phid]->renderLink(); - } else { - $author = self::renderName($history->getAuthorName()); - } - - $different_committer = false; - if ($committer_phid) { - $different_committer = ($committer_phid != $author_phid); - } else if ($committer != '') { - $different_committer = ($committer != $history->getAuthorName()); - } - if ($different_committer) { - if ($committer_phid && isset($handles[$committer_phid])) { - $committer = $handles[$committer_phid]->renderLink(); - } else { - $committer = self::renderName($committer); - } - $author = hsprintf('%s/%s', $author, $committer); - } - - // We can show details once the message and change have been imported. - $partial_import = PhabricatorRepositoryCommit::IMPORTED_MESSAGE | - PhabricatorRepositoryCommit::IMPORTED_CHANGE; - - $commit = $history->getCommit(); - if ($commit && $commit->isPartiallyImported($partial_import) && $data) { - $summary = AphrontTableView::renderSingleDisplayLine( - $history->getSummary()); - } else { - $summary = phutil_tag('em', array(), pht("Importing\xE2\x80\xA6")); - } - - $build = null; - if ($show_builds) { - $buildable = idx($buildables, $commit->getPHID()); - if ($buildable !== null) { - $build = $this->renderBuildable($buildable); - $has_any_build = true; - } - } - - $browse = $this->linkBrowse( - $history->getPath(), - array( - 'commit' => $history->getCommitIdentifier(), - 'branch' => $drequest->getBranch(), - 'type' => $history->getFileType(), - )); - - $status = $commit->getAuditStatusObject(); - $icon = $status->getIcon(); - $color = $status->getColor(); - $name = $status->getName(); - - $audit_view = id(new PHUIIconView()) - ->setIcon($icon, $color) - ->addSigil('has-tooltip') - ->setMetadata( - array( - '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, - self::linkCommit( - $drequest->getRepository(), - $history->getCommitIdentifier()), - $build, - $audit_view, - $revision_link, - $author, - $summary, - $committed, - ); - } - - $view = new AphrontTableView($rows); - $view->setHeaders( - array( - null, - null, - pht('Commit'), - null, - null, - null, - pht('Author'), - pht('Details'), - pht('Committed'), - )); - $view->setColumnClasses( - array( - 'threads', - 'nudgeright', - '', - 'icon', - 'icon', - '', - '', - 'wide', - 'right', - )); - $view->setColumnVisibility( - array( - $graph ? true : false, - true, - true, - $has_any_build, - true, - $show_revisions, - )); - $view->setDeviceVisibility( - array( - $graph ? true : false, - true, - true, - true, - true, - true, - false, - true, - false, - )); - return $view->render(); - } - -} diff --git a/src/applications/diffusion/view/DiffusionHistoryView.php b/src/applications/diffusion/view/DiffusionHistoryView.php deleted file mode 100644 index 9fd760b2cc..0000000000 --- a/src/applications/diffusion/view/DiffusionHistoryView.php +++ /dev/null @@ -1,117 +0,0 @@ -history = $history; - return $this; - } - - public function getHistory() { - return $this->history; - } - - public function setHandles(array $handles) { - assert_instances_of($handles, 'PhabricatorObjectHandle'); - $this->handles = $handles; - return $this; - } - - public function getRequiredHandlePHIDs() { - $phids = array(); - foreach ($this->history as $item) { - $data = $item->getCommitData(); - if ($data) { - if ($data->getCommitDetail('authorPHID')) { - $phids[$data->getCommitDetail('authorPHID')] = true; - } - if ($data->getCommitDetail('committerPHID')) { - $phids[$data->getCommitDetail('committerPHID')] = true; - } - } - } - return array_keys($phids); - } - - public function setParents(array $parents) { - $this->parents = $parents; - return $this; - } - - public function getParents() { - return $this->parents; - } - - public function setIsHead($is_head) { - $this->isHead = $is_head; - return $this; - } - - public function getIsHead() { - return $this->isHead; - } - - public function setIsTail($is_tail) { - $this->isTail = $is_tail; - return $this; - } - - public function getIsTail() { - return $this->isTail; - } - - public function setFilterParents($filter_parents) { - $this->filterParents = $filter_parents; - return $this; - } - - public function getFilterParents() { - return $this->filterParents; - } - - 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) { - - // NOTE: The "commit" objects in the history list may be undiscovered, - // and thus not yet have PHIDs. Only load data for commits with PHIDs. - if (!$commit->getPHID()) { - continue; - } - - $commits[] = $commit; - } - } - - return DiffusionCommitRevisionQuery::loadRevisionMapForCommits( - $this->getViewer(), - $commits); - } - -}