Mostly modernize lint views and delete dead code

Summary:
This is a mostly-faithful modernization of the Diffusion lint interfaces. It:

  - Makes them policy aware;
  - removes the last callsites for old/dead code (crumbs, nav).

It's a little rough, but should be perfectly usable. At some point this should get another pass, but probably after we make it easier to populate the lint data.

Test Plan: See screenshots.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, FacebookPOC

Differential Revision: https://secure.phabricator.com/D7065

src/applications/diffusion/controller/DiffusionController.php

@@ -25,53 +25,6 @@ abstract class DiffusionController extends PhabricatorController {
     return $this->diffusionRequest;
   }
 
-  final protected function buildSideNav($selected, $has_change_view) {
-    $nav = new AphrontSideNavFilterView();
-    $nav->setBaseURI(new PhutilURI(''));
-
-    $navs = array(
-      'history' => pht('History View'),
-      'browse'  => pht('Browse View'),
-      'change'  => pht('Change View'),
-    );
-
-    if (!$has_change_view) {
-      unset($navs['change']);
-    }
-
-    $drequest = $this->getDiffusionRequest();
-    $branch = $drequest->loadBranch();
-
-    if ($branch && $branch->getLintCommit()) {
-      $navs['lint'] = pht('Lint View');
-    }
-
-    $selected_href = null;
-    foreach ($navs as $action => $name) {
-      $href = $drequest->generateURI(
-        array(
-          'action' => $action,
-        ));
-      if ($action == $selected) {
-        $selected_href = $href;
-      }
-
-      $nav->addFilter($href, $name, $href);
-    }
-    $nav->selectFilter($selected_href, null);
-
-    // TODO: URI encoding might need to be sorted out for this link.
-
-    $nav->addFilter(
-      '',
-      pht("Search Owners \xE2\x86\x97"),
-      '/owners/view/search/'.
-        '?repository='.phutil_escape_uri($drequest->getCallsign()).
-        '&path='.phutil_escape_uri('/'.$drequest->getPath()));
-
-    return $nav;
-  }
-
   public function buildCrumbs(array $spec = array()) {
     $crumbs = $this->buildApplicationCrumbs();
     $crumb_list = $this->buildCrumbList($spec);
@@ -167,19 +120,6 @@ abstract class DiffusionController extends PhabricatorController {
     $crumb = new PhabricatorCrumbView();
     $view = $spec['view'];
 
-    $path = null;
-    if (isset($spec['path'])) {
-      $path = $drequest->getPath();
-    }
-
-    if ($raw_commit) {
-      $commit_link = DiffusionView::linkCommit(
-        $repository,
-        $raw_commit);
-    } else {
-      $commit_link = '';
-    }
-
     switch ($view) {
       case 'history':
         $view_name = pht('History');
@@ -195,74 +135,10 @@ abstract class DiffusionController extends PhabricatorController {
         break;
     }
 
-    $uri_params = array(
-      'action' => $view,
-    );
-
     $crumb = id(new PhabricatorCrumbView())
       ->setName($view_name);
 
-    if ($view == 'browse' || $view == 'change' || $view == 'history') {
-      $crumb_list[] = $crumb;
-      return $crumb_list;
-    }
-
-    $crumb->setHref($drequest->generateURI(
-      array(
-        'path' => '',
-      ) + $uri_params));
     $crumb_list[] = $crumb;
-
-    $path_parts = explode('/', $path);
-    do {
-      $last = array_pop($path_parts);
-    } while (count($path_parts) && $last == '');
-
-    $path_sections = array();
-    $thus_far = '';
-    foreach ($path_parts as $path_part) {
-      $thus_far .= $path_part.'/';
-      $path_sections[] = '/';
-      $path_sections[] = phutil_tag(
-        'a',
-        array(
-          'href' => $drequest->generateURI(
-            array(
-              'path' => $thus_far,
-            ) + $uri_params),
-        ),
-        $path_part);
-    }
-
-    $path_sections[] = '/'.$last;
-
-    $crumb_list[] = id(new PhabricatorCrumbView())
-      ->setName($path_sections);
-
-    $last_crumb = array_pop($crumb_list);
-
-    if ($raw_commit) {
-      $jump_link = phutil_tag(
-        'a',
-        array(
-          'href' => $drequest->generateURI(
-            array(
-              'commit' => '',
-            ) + $uri_params),
-        ),
-        pht('Jump to HEAD'));
-
-      $name = $last_crumb->getName();
-      $name = hsprintf('%s @ %s (%s)', $name, $commit_link, $jump_link);
-      $last_crumb->setName($name);
-    } else if ($spec['view'] != 'lint') {
-      $name = $last_crumb->getName();
-      $name = hsprintf('%s @ HEAD', $name);
-      $last_crumb->setName($name);
-    }
-
-    $crumb_list[] = $last_crumb;
-
     return $crumb_list;
   }
 

src/applications/diffusion/controller/DiffusionLintController.php

@@ -2,6 +2,10 @@
 
 final class DiffusionLintController extends DiffusionController {
 
+  public function shouldAllowPublic() {
+    return true;
+  }
+
   public function processRequest() {
     $request = $this->getRequest();
     $user = $this->getRequest()->getUser();
@@ -17,7 +21,9 @@ final class DiffusionLintController extends DiffusionController {
     $owners = array();
     if (!$drequest) {
       if (!$request->getArr('owner')) {
-        $owners[$user->getPHID()] = $user->getFullName();
+        if ($user->isLoggedIn()) {
+          $owners[$user->getPHID()] = $user->getFullName();
+        }
       } else {
         $phids = $request->getArr('owner');
         $phid = reset($phids);
@@ -29,16 +35,23 @@ final class DiffusionLintController extends DiffusionController {
     $codes = $this->loadLintCodes(array_keys($owners));
 
     if ($codes && !$drequest) {
+      // TODO: Build some real Query classes for this stuff.
+
       $branches = id(new PhabricatorRepositoryBranch())->loadAllWhere(
         'id IN (%Ld)',
         array_unique(ipull($codes, 'branchID')));
 
-      $repositories = id(new PhabricatorRepository())->loadAllWhere(
+      $repositories = id(new PhabricatorRepositoryQuery())
-        'id IN (%Ld)',
+        ->setViewer($user)
-        array_unique(mpull($branches, 'getRepositoryID')));
+        ->withIDs(mpull($branches, 'getRepositoryID'))
+        ->execute();
 
       $drequests = array();
       foreach ($branches as $id => $branch) {
+        if (empty($repositories[$branch->getRepositoryID()])) {
+          continue;
+        }
+
         $drequests[$id] = DiffusionRequest::newFromDictionary(array(
           'user' => $user,
           'repository' => $repositories[$branch->getRepositoryID()],
@@ -48,11 +61,18 @@ final class DiffusionLintController extends DiffusionController {
     }
 
     $rows = array();
+    $total = 0;
     foreach ($codes as $code) {
       if (!$this->diffusionRequest) {
-        $drequest = $drequests[$code['branchID']];
+        $drequest = idx($drequests, $code['branchID']);
       }
 
+      if (!$drequest) {
+        continue;
+      }
+
+      $total += $code['n'];
+
       $rows[] = array(
         hsprintf(
           '<a href="%s">%s</a>',
@@ -95,16 +115,7 @@ final class DiffusionLintController extends DiffusionController {
     $content = array();
 
     $link = null;
-    if ($this->diffusionRequest) {
+    if (!$this->diffusionRequest) {
-      $link = hsprintf(
-        '<a href="%s">%s</a>',
-        $drequest->generateURI(array(
-          'action' => 'lint',
-          'lint' => '',
-        )),
-        pht('Switch to List View'));
-
-    } else {
       $form = id(new AphrontFormView())
         ->setUser($user)
         ->setMethod('GET')
@@ -122,7 +133,7 @@ final class DiffusionLintController extends DiffusionController {
     }
 
     $content[] = id(new AphrontPanelView())
-      ->setHeader(pht('%d Lint Message(s)', array_sum(ipull($codes, 'n'))))
+      ->setNoBackground(true)
       ->setCaption($link)
       ->appendChild($table);
 
@@ -133,21 +144,45 @@ final class DiffusionLintController extends DiffusionController {
         'path'   => true,
         'view'   => 'lint',
       ));
+
     if ($this->diffusionRequest) {
       $title[] = $drequest->getCallsign();
-      $content = $this->buildSideNav('lint', false)
-        ->setCrumbs($crumbs)
-        ->appendChild($content);
     } else {
-      array_unshift($content, $crumbs);
+      $crumbs->addCrumb(
+        id(new PhabricatorCrumbView())
+          ->setName(pht('All Lint')));
     }
 
+    if ($this->diffusionRequest) {
+      $branch = $drequest->loadBranch();
+
+      $header = id(new PHUIHeaderView())
+        ->setHeader($this->renderPathLinks($drequest, 'lint'))
+        ->setUser($user)
+        ->setPolicyObject($drequest->getRepository());
+      $actions = $this->buildActionView($drequest);
+      $properties = $this->buildPropertyView(
+        $drequest,
+        $branch,
+        $total);
+    } else {
+      $header = null;
+      $actions = null;
+      $properties = null;
+    }
+
+
     return $this->buildApplicationPage(
-      $content,
+      array(
+        $crumbs,
+        $header,
+        $actions,
+        $properties,
+        $content,
+      ),
       array(
         'title' => $title,
-        'device' => true,
+      ));
-        ));
   }
 
   private function loadLintCodes(array $owner_phids) {
@@ -233,4 +268,81 @@ final class DiffusionLintController extends DiffusionController {
       implode(' AND ', $where));
   }
 
+  protected function buildActionView(DiffusionRequest $drequest) {
+    $viewer = $this->getRequest()->getUser();
+
+    $view = id(new PhabricatorActionListView())
+      ->setUser($viewer);
+
+    $list_uri = $drequest->generateURI(
+      array(
+        'action' => 'lint',
+        'lint' => '',
+      ));
+
+    $view->addAction(
+      id(new PhabricatorActionView())
+        ->setName(pht('View As List'))
+        ->setHref($list_uri)
+        ->setIcon('transcript'));
+
+    $history_uri = $drequest->generateURI(
+      array(
+        'action' => 'history',
+      ));
+
+    $view->addAction(
+      id(new PhabricatorActionView())
+        ->setName(pht('View History'))
+        ->setHref($history_uri)
+        ->setIcon('history'));
+
+    $browse_uri = $drequest->generateURI(
+      array(
+        'action' => 'browse',
+      ));
+
+    $view->addAction(
+      id(new PhabricatorActionView())
+        ->setName(pht('Browse Content'))
+        ->setHref($browse_uri)
+        ->setIcon('file'));
+
+    return $view;
+  }
+
+  protected function buildPropertyView(
+    DiffusionRequest $drequest,
+    PhabricatorRepositoryBranch $branch,
+    $total) {
+
+    $viewer = $this->getRequest()->getUser();
+
+    $view = id(new PhabricatorPropertyListView())
+      ->setUser($viewer);
+
+    $callsign = $drequest->getRepository()->getCallsign();
+    $lint_commit = $branch->getLintCommit();
+
+    $view->addProperty(
+      pht('Lint Commit'),
+      phutil_tag(
+        'a',
+        array(
+          'href' => $drequest->generateURI(
+            array(
+              'action' => 'commit',
+              'commit' => $lint_commit,
+            )),
+        ),
+        $drequest->getRepository()->formatCommitName($lint_commit)));
+
+    $view->addProperty(
+      pht('Total Messages'),
+      pht('%s', new PhutilNumber($total)));
+
+    return $view;
+  }
+
+
 }

src/applications/diffusion/controller/DiffusionLintDetailsController.php

@@ -68,35 +68,23 @@ final class DiffusionLintDetailsController extends DiffusionController {
       ->setHasMorePages(count($messages) >= $limit)
       ->setURI($this->getRequest()->getRequestURI(), 'offset');
 
-    $lint = $drequest->getLint();
-    $link = hsprintf(
-      '<a href="%s">%s</a>',
-      $drequest->generateURI(array(
-        'action' => 'lint',
-        'lint' => null,
-      )),
-      pht('Switch to Grouped View'));
-
     $content[] = id(new AphrontPanelView())
-      ->setHeader(
+      ->setNoBackground(true)
-        ($lint != '' ? $lint." \xC2\xB7 " : '').
-        pht('%d Lint Message(s)', count($messages)))
-      ->setCaption($link)
       ->appendChild($table)
       ->appendChild($pager);
 
-    $nav = $this->buildSideNav('lint', false);
-    $nav->appendChild($content);
     $crumbs = $this->buildCrumbs(
       array(
         'branch' => true,
         'path'   => true,
         'view'   => 'lint',
       ));
-    $nav->setCrumbs($crumbs);
 
     return $this->buildApplicationPage(
-      $nav,
+      array(
+        $crumbs,
+        $content,
+      ),
       array(
         'device' => true,
         'title' =>