diff --git a/src/applications/differential/controller/DifferentialController.php b/src/applications/differential/controller/DifferentialController.php --- a/src/applications/differential/controller/DifferentialController.php +++ b/src/applications/differential/controller/DifferentialController.php @@ -2,6 +2,10 @@ abstract class DifferentialController extends PhabricatorController { + private $packageChangesetMap; + private $pathPackageMap; + private $authorityPackages; + public function buildSideNavView($for_app = false) { $viewer = $this->getRequest()->getUser(); @@ -21,48 +25,98 @@ return $this->buildSideNavView(true)->getMenu(); } - protected function buildTableOfContents( - array $changesets, - array $visible_changesets, - array $coverage) { - $viewer = $this->getViewer(); + protected function buildPackageMaps(array $changesets) { + assert_instances_of($changesets, 'DifferentialChangeset'); - $toc_view = id(new PHUIDiffTableOfContentsListView()) - ->setViewer($viewer) - ->setBare(true); + $this->packageChangesetMap = array(); + $this->pathPackageMap = array(); + $this->authorityPackages = array(); + + if (!$changesets) { + return; + } + + $viewer = $this->getViewer(); $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 (!$have_owners) { + return; + } - if (!$repository_phid) { - $have_owners = false; - } else { - if ($viewer->getPHID()) { - $packages = id(new PhabricatorOwnersPackageQuery()) - ->setViewer($viewer) - ->withStatuses(array(PhabricatorOwnersPackage::STATUS_ACTIVE)) - ->withAuthorityPHIDs(array($viewer->getPHID())) - ->execute(); - $toc_view->setAuthorityPackages($packages); - } - - $paths = mpull($changesets, 'getOwnersFilename'); - - $control_query = id(new PhabricatorOwnersPackageQuery()) - ->setViewer($viewer) - ->withStatuses(array(PhabricatorOwnersPackage::STATUS_ACTIVE)) - ->withControl($repository_phid, $paths); - $control_query->execute(); + $changeset = head($changesets); + $diff = $changeset->getDiff(); + $repository_phid = $diff->getRepositoryPHID(); + if (!$repository_phid) { + return; + } + + if ($viewer->getPHID()) { + $packages = id(new PhabricatorOwnersPackageQuery()) + ->setViewer($viewer) + ->withStatuses(array(PhabricatorOwnersPackage::STATUS_ACTIVE)) + ->withAuthorityPHIDs(array($viewer->getPHID())) + ->execute(); + $this->authorityPackages = $packages; + } + + $paths = mpull($changesets, 'getOwnersFilename'); + + $control_query = id(new PhabricatorOwnersPackageQuery()) + ->setViewer($viewer) + ->withStatuses(array(PhabricatorOwnersPackage::STATUS_ACTIVE)) + ->withControl($repository_phid, $paths); + $control_query->execute(); + + foreach ($changesets as $changeset) { + $changeset_path = $changeset->getOwnersFilename(); + + $packages = $control_query->getControllingPackagesForPath( + $repository_phid, + $changeset_path); + + $this->pathPackageMap[$changeset_path] = $packages; + foreach ($packages as $package) { + $this->packageChangesetMap[$package->getPHID()][] = $changeset; } } + } + + protected function getAuthorityPackages() { + if ($this->authorityPackages === null) { + throw new PhutilInvalidStateException('buildPackageMaps'); + } + return $this->authorityPackages; + } + + protected function getChangesetPackages(DifferentialChangeset $changeset) { + if ($this->pathPackageMap === null) { + throw new PhutilInvalidStateException('buildPackageMaps'); + } + + $path = $changeset->getOwnersFilename(); + return idx($this->pathPackageMap, $path, array()); + } + + protected function getPackageChangesets($package_phid) { + if ($this->packageChangesetMap === null) { + throw new PhutilInvalidStateException('buildPackageMaps'); + } + + return idx($this->packageChangesetMap, $package_phid, array()); + } + + protected function buildTableOfContents( + array $changesets, + array $visible_changesets, + array $coverage) { + $viewer = $this->getViewer(); + + $toc_view = id(new PHUIDiffTableOfContentsListView()) + ->setViewer($viewer) + ->setBare(true) + ->setAuthorityPackages($this->getAuthorityPackages()); foreach ($changesets as $changeset_id => $changeset) { $is_visible = isset($visible_changesets[$changeset_id]); @@ -78,12 +132,8 @@ ->setCoverage(idx($coverage, $filename)) ->setCoverageID($coverage_id); - if ($have_owners) { - $packages = $control_query->getControllingPackagesForPath( - $repository_phid, - $changeset->getOwnersFilename()); - $item->setPackages($packages); - } + $packages = $this->getChangesetPackages($changeset); + $item->setPackages($packages); $toc_view->addItem($item); } diff --git a/src/applications/differential/controller/DifferentialDiffViewController.php b/src/applications/differential/controller/DifferentialDiffViewController.php --- a/src/applications/differential/controller/DifferentialDiffViewController.php +++ b/src/applications/differential/controller/DifferentialDiffViewController.php @@ -118,6 +118,8 @@ $changesets = $diff->loadChangesets(); $changesets = msort($changesets, 'getSortKey'); + $this->buildPackageMaps($changesets); + $table_of_contents = $this->buildTableOfContents( $changesets, $changesets, diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -326,11 +326,21 @@ $other_view = $this->renderOtherRevisions($other_revisions); } + $this->buildPackageMaps($changesets); + $toc_view = $this->buildTableOfContents( $changesets, $visible_changesets, $target->loadCoverageMap($viewer)); + // Attach changesets to each reviewer so we can show which Owners package + // reviewers own no files. + foreach ($revision->getReviewers() as $reviewer) { + $reviewer_phid = $reviewer->getReviewerPHID(); + $reviewer_changesets = $this->getPackageChangesets($reviewer_phid); + $reviewer->attachChangesets($reviewer_changesets); + } + $tab_group = id(new PHUITabGroupView()) ->addTab( id(new PHUITabView()) diff --git a/src/applications/differential/storage/DifferentialReviewer.php b/src/applications/differential/storage/DifferentialReviewer.php --- a/src/applications/differential/storage/DifferentialReviewer.php +++ b/src/applications/differential/storage/DifferentialReviewer.php @@ -12,6 +12,7 @@ protected $voidedPHID; private $authority = array(); + private $changesets = self::ATTACHABLE; protected function getConfiguration() { return array( @@ -54,6 +55,15 @@ return $this->assertAttachedKey($this->authority, $cache_fragment); } + public function attachChangesets(array $changesets) { + $this->changesets = $changesets; + return $this; + } + + public function getChangesets() { + return $this->assertAttached($this->changesets); + } + public function isResigned() { $status_resigned = DifferentialReviewerStatus::STATUS_RESIGNED; return ($this->getReviewerStatus() == $status_resigned); diff --git a/src/applications/differential/view/DifferentialReviewersView.php b/src/applications/differential/view/DifferentialReviewersView.php --- a/src/applications/differential/view/DifferentialReviewersView.php +++ b/src/applications/differential/view/DifferentialReviewersView.php @@ -150,6 +150,12 @@ $item->setIcon($icon, $color, $label); $item->setTarget($handle->renderHovercardLink()); + if ($reviewer->isPackage()) { + if (!$reviewer->getChangesets()) { + $item->setNote(pht('(Owns No Changed Paths)')); + } + } + $view->addItem($item); }