From 6010e80e5c5a79f81132dcb4fe1cdd5ee118662e Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 17 Aug 2015 10:14:22 -0700 Subject: [PATCH] Show packages in table of contents views in Diffusion and Differential Summary: Fixes T8004. - For paths which are part of a package, show the package. - Highlight paths which are part of a package you (the viewer) have authority over. Test Plan: {F725418} - Viewed owned and unowned chagnes in Diffusion and Differential. Reviewers: chad Reviewed By: chad Maniphest Tasks: T8004 Differential Revision: https://secure.phabricator.com/D13923 --- .../controller/DifferentialController.php | 42 +++++++++++++++++++ .../differential/storage/DifferentialDiff.php | 8 +++- .../controller/DiffusionCommitController.php | 38 ++++++++++++++++- .../view/PHUIDiffTableOfContentsListView.php | 38 ++++++++++++++++- 4 files changed, 121 insertions(+), 5 deletions(-) diff --git a/src/applications/differential/controller/DifferentialController.php b/src/applications/differential/controller/DifferentialController.php index 0ae33f4e59..eed059e265 100644 --- a/src/applications/differential/controller/DifferentialController.php +++ b/src/applications/differential/controller/DifferentialController.php @@ -30,6 +30,39 @@ abstract class DifferentialController extends PhabricatorController { $toc_view = id(new PHUIDiffTableOfContentsListView()) ->setUser($viewer); + $have_owners = PhabricatorApplication::isClassInstalledForViewer( + 'PhabricatorOwnersApplication', + $viewer); + if ($have_owners) { + $repository_phid = null; + if ($changesets) { + $changeset = head($changesets); + $diff = $changeset->getDiff(); + $repository_phid = $diff->getRepositoryPHID(); + } + + if (!$repository_phid) { + $have_owners = false; + } else { + if ($viewer->getPHID()) { + $packages = id(new PhabricatorOwnersPackageQuery()) + ->setViewer($viewer) + ->withAuthorityPHIDs(array($viewer->getPHID())) + ->execute(); + $toc_view->setAuthorityPackages($packages); + } + + // TODO: For Subversion, we should adjust these paths to be relative to + // the repository root where possible. + $paths = mpull($changesets, 'getFilename'); + + $control_query = id(new PhabricatorOwnersPackageQuery()) + ->setViewer($viewer) + ->withControl($repository_phid, $paths); + $control_query->execute(); + } + } + foreach ($changesets as $changeset_id => $changeset) { $is_visible = isset($visible_changesets[$changeset_id]); $anchor = $changeset->getAnchorName(); @@ -44,6 +77,15 @@ abstract class DifferentialController extends PhabricatorController { ->setCoverage(idx($coverage, $filename)) ->setCoverageID($coverage_id); + if ($have_owners) { + $package = $control_query->getControllingPackageForPath( + $repository_phid, + $changeset->getFilename()); + if ($package) { + $item->setPackage($package); + } + } + $toc_view->addItem($item); } diff --git a/src/applications/differential/storage/DifferentialDiff.php b/src/applications/differential/storage/DifferentialDiff.php index e48ed5630d..7a5bc24203 100644 --- a/src/applications/differential/storage/DifferentialDiff.php +++ b/src/applications/differential/storage/DifferentialDiff.php @@ -101,9 +101,15 @@ final class DifferentialDiff if (!$this->getID()) { return array(); } - return id(new DifferentialChangeset())->loadAllWhere( + $changesets = id(new DifferentialChangeset())->loadAllWhere( 'diffID = %d', $this->getID()); + + foreach ($changesets as $changeset) { + $changeset->attachDiff($this); + } + + return $changesets; } public function save() { diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index 927ad7f453..b91a7dc804 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -1066,6 +1066,7 @@ final class DiffusionCommitController extends DiffusionController { } private function buildTableOfContents(array $changesets) { + $drequest = $this->getDiffusionRequest(); $viewer = $this->getViewer(); $toc_view = id(new PHUIDiffTableOfContentsListView()) @@ -1074,9 +1075,33 @@ final class DiffusionCommitController extends DiffusionController { // TODO: This is hacky, we just want access to the linkX() methods on // DiffusionView. $diffusion_view = id(new DiffusionEmptyResultView()) - ->setDiffusionRequest($this->getDiffusionRequest()); + ->setDiffusionRequest($drequest); - // TODO: Restore package stuff here. + $have_owners = PhabricatorApplication::isClassInstalledForViewer( + 'PhabricatorOwnersApplication', + $viewer); + + if (!$changesets) { + $have_owners = false; + } + + if ($have_owners) { + if ($viewer->getPHID()) { + $packages = id(new PhabricatorOwnersPackageQuery()) + ->setViewer($viewer) + ->withAuthorityPHIDs(array($viewer->getPHID())) + ->execute(); + $toc_view->setAuthorityPackages($packages); + } + + $repository = $drequest->getRepository(); + $repository_phid = $repository->getPHID(); + + $control_query = id(new PhabricatorOwnersPackageQuery()) + ->setViewer($viewer) + ->withControl($repository_phid, mpull($changesets, 'getFilename')); + $control_query->execute(); + } foreach ($changesets as $changeset_id => $changeset) { $path = $changeset->getFilename(); @@ -1095,6 +1120,15 @@ final class DiffusionCommitController extends DiffusionController { $browse_link, )); + if ($have_owners) { + $package = $control_query->getControllingPackageForPath( + $repository_phid, + $changeset->getFilename()); + if ($package) { + $item->setPackage($package); + } + } + $toc_view->addItem($item); } diff --git a/src/infrastructure/diff/view/PHUIDiffTableOfContentsListView.php b/src/infrastructure/diff/view/PHUIDiffTableOfContentsListView.php index 66677ee598..a7e3270c6e 100644 --- a/src/infrastructure/diff/view/PHUIDiffTableOfContentsListView.php +++ b/src/infrastructure/diff/view/PHUIDiffTableOfContentsListView.php @@ -3,12 +3,23 @@ final class PHUIDiffTableOfContentsListView extends AphrontView { private $items = array(); + private $authorityPackages; public function addItem(PHUIDiffTableOfContentsItemView $item) { $this->items[] = $item; return $this; } + public function setAuthorityPackages(array $authority_packages) { + assert_instances_of($authority_packages, 'PhabricatorOwnersPackage'); + $this->authorityPackages = $authority_packages; + return $this; + } + + public function getAuthorityPackages() { + return $this->authorityPackages; + } + public function render() { $this->requireResource('differential-core-view-css'); $this->requireResource('differential-table-of-contents-css'); @@ -16,12 +27,34 @@ final class PHUIDiffTableOfContentsListView extends AphrontView { Javelin::initBehavior('phabricator-tooltips'); + if ($this->getAuthorityPackages()) { + $authority = mpull($this->getAuthorityPackages(), null, 'getPHID'); + } else { + $authority = array(); + } + $items = $this->items; $rows = array(); + $rowc = array(); foreach ($items as $item) { $item->setUser($this->getUser()); $rows[] = $item->render(); + + $have_authority = false; + + $package = $item->getPackage(); + if ($package) { + if (isset($authority[$package->getPHID()])) { + $have_authority = true; + } + } + + if ($have_authority) { + $rowc[] = 'highlighted'; + } else { + $rowc[] = null; + } } // Check if any item has content in these columns. If no item does, we'll @@ -60,6 +93,7 @@ final class PHUIDiffTableOfContentsListView extends AphrontView { $reveal_link); $table = id(new AphrontTableView($rows)) + ->setRowClasses($rowc) ->setHeaders( array( null, @@ -69,7 +103,7 @@ final class PHUIDiffTableOfContentsListView extends AphrontView { pht('Path'), pht('Coverage (All)'), pht('Coverage (Touched)'), - null, + pht('Package'), )) ->setColumnClasses( array( @@ -80,7 +114,7 @@ final class PHUIDiffTableOfContentsListView extends AphrontView { 'differential-toc-file wide', 'differential-toc-cov', 'differential-toc-cov', - 'center', + null, )) ->setColumnVisibility( array(