Simplify Owners interfaces to Audit

Summary:
  - Owners has "by user" commit views, but these are supplanted by the Audit views. Just nuke them.
  - Owners has "by package" commit views; consolidate these onto the package detail pages and link into Audit for full details.

Test Plan: Browsed all the Owners interfaces, clicked "View All ... Commits" buttons.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T904

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

src/__phutil_library_map__.php

@@ -657,7 +657,6 @@ phutil_register_library_map(array(
     'PhabricatorObjectHandleStatus' => 'applications/phid/handle/const/status',
     'PhabricatorObjectSelectorDialog' => 'view/control/objectselector',
     'PhabricatorOwnerPathQuery' => 'applications/owners/query/path',
-    'PhabricatorOwnerRelatedListController' => 'applications/owners/controller/relatedlist',
     'PhabricatorOwnersController' => 'applications/owners/controller/base',
     'PhabricatorOwnersDAO' => 'applications/owners/storage/base',
     'PhabricatorOwnersDeleteController' => 'applications/owners/controller/delete',
@@ -1415,7 +1414,6 @@ phutil_register_library_map(array(
     'PhabricatorOAuthUnlinkController' => 'PhabricatorAuthController',
     'PhabricatorObjectGraph' => 'AbstractDirectedGraph',
     'PhabricatorObjectHandleStatus' => 'PhabricatorObjectHandleConstants',
-    'PhabricatorOwnerRelatedListController' => 'PhabricatorOwnersController',
     'PhabricatorOwnersController' => 'PhabricatorController',
     'PhabricatorOwnersDAO' => 'PhabricatorLiskDAO',
     'PhabricatorOwnersDeleteController' => 'PhabricatorOwnersController',

src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php

@@ -328,11 +328,6 @@ class AphrontDefaultApplicationConfiguration
         'new/' => 'PhabricatorOwnersEditController',
         'package/(?P<id>\d+)/' => 'PhabricatorOwnersDetailController',
         'delete/(?P<id>\d+)/' => 'PhabricatorOwnersDeleteController',
-        '(?P<scope>related|attention)/' => array(
-          '' => 'PhabricatorOwnerRelatedListController',
-          '(?P<view>package|owner)/'
-            => 'PhabricatorOwnerRelatedListController',
-        ),
       ),
 
       '/audit/' => array(

src/applications/owners/controller/base/PhabricatorOwnersController.php

@@ -72,16 +72,6 @@ abstract class PhabricatorOwnersController extends PhabricatorController {
     $nav->addLabel('Packages');
     $nav->addFilters($package_views);
 
-    $nav->addSpacer();
-    $nav->addLabel('Related Commits');
-    $related_views = $this->getRelatedViews();
-    $nav->addFilters($related_views);
-
-    $nav->addSpacer();
-    $nav->addLabel('Commits Need Attention');
-    $attention_views = $this->getAttentionViews();
-    $nav->addFilters($attention_views);
-
     $filter = $this->getSideNavFilter();
     $nav->selectFilter($filter, 'view/owned');
 
@@ -92,26 +82,4 @@ abstract class PhabricatorOwnersController extends PhabricatorController {
     return array();
   }
 
-  protected function getRelatedViews() {
-    $related_views = array(
-      array('name' => 'By Package',
-            'key'  => 'related/package'),
-      array('name' => 'By Package Owner',
-            'key'  => 'related/owner'),
-          );
-
-    return $related_views;
-  }
-
-  protected function getAttentionViews() {
-    $attention_views = array(
-      array('name' => 'By Package',
-            'key'  => 'attention/package'),
-      array('name' => 'By Package Owner',
-            'key'  => 'attention/owner'),
-          );
-
-    return $attention_views;
-  }
-
 }

src/applications/owners/controller/detail/PhabricatorOwnersDetailController.php

@@ -83,17 +83,6 @@ class PhabricatorOwnersDetailController extends PhabricatorOwnersController {
       $package->getAuditingEnabled() ? 'Enabled' : 'Disabled',
     );
 
-    $rows[] = array(
-      'Related Commits',
-      phutil_render_tag(
-        'a',
-        array(
-          'href' => '/owners/related/package/?phid='.$package->getPHID(),
-        ),
-        phutil_escape_html('Related Commits'))
-    );
-
-
     $path_links = array();
     foreach ($paths as $path) {
       $callsign = $handles[$path->getRepositoryPHID()]->getName();
@@ -143,8 +132,86 @@ class PhabricatorOwnersDetailController extends PhabricatorOwnersController {
     $key = 'package/'.$package->getID();
     $this->setSideNavFilter($key);
 
+    $commit_views = array();
+
+    $commit_uri = id(new PhutilURI('/audit/view/packagecommits/'))
+      ->setQueryParams(
+        array(
+          'phid'    => $package->getPHID(),
+        ));
+
+    $attention_query = id(new PhabricatorAuditCommitQuery())
+      ->withPackagePHIDs(array($package->getPHID()))
+      ->withStatus(PhabricatorAuditCommitQuery::STATUS_OPEN)
+      ->needCommitData(true)
+      ->setLimit(10);
+    $attention_commits = $attention_query->execute();
+    if ($attention_commits) {
+      $view = new PhabricatorAuditCommitListView();
+      $view->setUser($user);
+      $view->setCommits($attention_commits);
+
+      $commit_views[] = array(
+        'view'    => $view,
+        'header'  => 'Commits in this Package that Need Attention',
+        'button'  => phutil_render_tag(
+          'a',
+          array(
+            'href'  => $commit_uri->alter('status', 'open'),
+            'class' => 'button grey',
+          ),
+          'View All Problem Commits'),
+      );
+    }
+
+    $all_query = id(new PhabricatorAuditCommitQuery())
+      ->withPackagePHIDs(array($package->getPHID()))
+      ->needCommitData(true)
+      ->setLimit(100);
+    $all_commits = $all_query->execute();
+
+    $view = new PhabricatorAuditCommitListView();
+    $view->setUser($user);
+    $view->setCommits($all_commits);
+    $view->setNoDataString('No commits in this package.');
+
+    $commit_views[] = array(
+      'view'    => $view,
+      'header'  => 'Recent Commits in Package',
+      'button'  => phutil_render_tag(
+        'a',
+        array(
+          'href'  => $commit_uri,
+          'class' => 'button grey',
+        ),
+        'View All Package Commits'),
+    );
+
+    $phids = array();
+    foreach ($commit_views as $commit_view) {
+      $phids[] = $commit_view['view']->getRequiredHandlePHIDs();
+    }
+    $phids = array_mergev($phids);
+    $handles = id(new PhabricatorObjectHandleData($phids))->loadHandles();
+
+    $commit_panels = array();
+    foreach ($commit_views as $commit_view) {
+      $commit_panel = new AphrontPanelView();
+      $commit_panel->setHeader(phutil_escape_html($commit_view['header']));
+      if (isset($commit_view['button'])) {
+        $commit_panel->addButton($commit_view['button']);
+      }
+      $commit_view['view']->setHandles($handles);
+      $commit_panel->appendChild($commit_view['view']);
+
+      $commit_panels[] = $commit_panel;
+    }
+
     return $this->buildStandardPageResponse(
-      $panel,
+      array(
+        $panel,
+        $commit_panels,
+      ),
       array(
         'title' => "Package '".$package->getName()."'",
       ));

src/applications/owners/controller/detail/__init__.php

@@ -7,6 +7,8 @@
 
 
 phutil_require_module('phabricator', 'aphront/response/404');
+phutil_require_module('phabricator', 'applications/audit/query/commit');
+phutil_require_module('phabricator', 'applications/audit/view/commitlist');
 phutil_require_module('phabricator', 'applications/owners/controller/base');
 phutil_require_module('phabricator', 'applications/owners/storage/package');
 phutil_require_module('phabricator', 'applications/phid/handle/data');
@@ -15,6 +17,7 @@ phutil_require_module('phabricator', 'view/control/table');
 phutil_require_module('phabricator', 'view/layout/panel');
 
 phutil_require_module('phutil', 'markup');
+phutil_require_module('phutil', 'parser/uri');
 phutil_require_module('phutil', 'utils');
 
 

src/applications/owners/controller/relatedlist/PhabricatorOwnerRelatedListController.php

@@ -1,377 +0,0 @@
-<?php
-
-/*
- * Copyright 2012 Facebook, Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-class PhabricatorOwnerRelatedListController
-   extends PhabricatorOwnersController {
-
-  private $request;
-  private $user;
-  private $scope;
-  private $view;
-  private $searchPHID;
-  private $auditStatus;
-
-  public function willProcessRequest(array $data) {
-    $this->scope = idx($data, 'scope', 'related');
-    $this->view = idx($data, 'view', 'owner');
-  }
-
-  public function processRequest() {
-    $this->request = $this->getRequest();
-
-    if ($this->request->isFormPost()) {
-      $phids = $this->request->getArr('search_phids');
-      $phid = head($phids);
-      $status = $this->request->getStr('search_status');
-      return id(new AphrontRedirectResponse())
-        ->setURI(
-          $this->request
-            ->getRequestURI()
-            ->alter('phid', $phid)
-            ->alter('status', $status));
-    }
-
-    $this->user = $this->request->getUser();
-    $this->searchPHID = nonempty($this->request->getStr('phid'), null);
-    if ($this->view === 'owner' && !$this->searchPHID) {
-      $this->searchPHID = $this->user->getPHID();
-    }
-    $this->auditStatus = $this->request->getStr('status', 'needaudit');
-
-    $search_view = $this->renderSearchView();
-    $list_panel = $this->renderListPanel();
-
-    $side_nav_filter = $this->scope.'/'.$this->view;
-    $this->setSideNavFilter($side_nav_filter);
-
-    return $this->buildStandardPageResponse(
-      array(
-        $search_view,
-        $list_panel,
-      ),
-      array(
-        'title' => 'Related Commits',
-      ));
-  }
-
-  protected function getRelatedViews() {
-    $related_views = parent::getRelatedViews();
-    if ($this->searchPHID) {
-      $query = $this->getQueryString();
-      foreach ($related_views as &$view) {
-        // Pass on the query string to the filter item with the same view.
-        if (preg_match('/'.preg_quote($this->view, '/').'$/', $view['key'])) {
-          $view['uri'] = $view['key'].$query;
-          $view['relative'] = true;
-        }
-      }
-    }
-    return $related_views;
-  }
-
-  protected function getAttentionViews() {
-    $related_views = parent::getAttentionViews();
-    if ($this->searchPHID) {
-      $query = $this->getQueryString();
-      foreach ($related_views as &$view) {
-        // Pass on the query string to the filter item with the same view.
-        if (preg_match('/'.preg_quote($this->view, '/').'$/', $view['key'])) {
-          $view['uri'] = $view['key'].$query;
-          $view['relative'] = true;
-        }
-      }
-    }
-    return $related_views;
-  }
-
-  private function getQueryString() {
-    $query = null;
-    if ($this->searchPHID) {
-      $query = '/?phid='.$this->searchPHID;
-    }
-    return $query;
-  }
-
-  private function renderListPanel() {
-    if (!$this->searchPHID) {
-       return id(new AphrontErrorView())
-        ->setSeverity(AphrontErrorView::SEVERITY_NOTICE)
-        ->setTitle('No search specified. Please add one from above.');
-    }
-
-    $package = new PhabricatorOwnersPackage();
-    $owner = new PhabricatorOwnersOwner();
-    $relationship = id(new PhabricatorOwnersPackageCommitRelationship());
-
-    switch ($this->view) {
-      case 'package':
-        $package = $package->loadOneWhere(
-          "phid = %s",
-          $this->searchPHID);
-        if ($this->scope === 'attention' && !$package->getAuditingEnabled()) {
-          return id(new AphrontErrorView())
-            ->setSeverity(AphrontErrorView::SEVERITY_NOTICE)
-            ->setTitle("Package doesn't have auditing enabled. ".
-                       "Please choose another one.");
-        }
-        $packages = array($package);
-        break;
-      case 'owner':
-        $data = queryfx_all(
-          $package->establishConnection('r'),
-          'SELECT p.* FROM %T p JOIN %T o ON p.id = o.packageID
-            WHERE o.userPHID = %s GROUP BY p.id',
-          $package->getTableName(),
-          $owner->getTableName(),
-          $this->searchPHID);
-        $packages = $package->loadAllFromArray($data);
-        break;
-      default:
-        throw new Exception('view of the page not recognized.');
-    }
-
-    $status_arr = $this->getStatusArr();
-
-    $offset = $this->request->getInt('offset', 0);
-    $pager = new AphrontPagerView();
-    $pager->setPageSize(50);
-    $pager->setOffset($offset);
-    $pager->setURI($this->request->getRequestURI(), 'offset');
-
-    $packages = mpull($packages, null, 'getPHID');
-    if ($packages) {
-      $data = queryfx_all(
-        $relationship->establishConnection('r'),
-        'SELECT commitPHID, packagePHID, auditStatus, auditReasons FROM %T
-          WHERE packagePHID in (%Ls) AND auditStatus in (%Ls)
-          ORDER BY id DESC
-          LIMIT %d, %d',
-        $relationship->getTableName(),
-        array_keys($packages),
-        $status_arr,
-        $pager->getOffset(),
-        $pager->getPageSize() + 1);
-    } else {
-      $data = array();
-    }
-
-    $data = $pager->sliceResults($data);
-    $data = ipull($data, null, 'commitPHID');
-
-    $list_panel = $this->renderCommitTable($data, $packages);
-    $list_panel->appendChild($pager);
-
-    return $list_panel;
-  }
-
-  private function getStatusArr() {
-    switch ($this->scope) {
-      case 'related':
-        $status_arr =
-          array_keys(PhabricatorAuditStatusConstants::getStatusNameMap());
-        break;
-      case 'attention':
-        switch ($this->auditStatus) {
-          case 'needaudit':
-            $status_arr =
-              array(
-                PhabricatorAuditStatusConstants::AUDIT_REQUIRED,
-                PhabricatorAuditStatusConstants::CONCERNED,
-              );
-            break;
-          case 'accepted':
-            $status_arr =
-              array(
-                PhabricatorAuditStatusConstants::ACCEPTED,
-              );
-            break;
-          case 'all':
-            $status_arr =
-              array(
-                PhabricatorAuditStatusConstants::AUDIT_REQUIRED,
-                PhabricatorAuditStatusConstants::CONCERNED,
-                PhabricatorAuditStatusConstants::ACCEPTED,
-              );
-            break;
-          default:
-            throw new Exception("Status {$this->auditStatus} not recognized");
-        }
-        break;
-
-      default:
-        throw new Exception("view {$this->view} not recognized");
-    }
-    return $status_arr;
-  }
-
-  private function renderSearchView() {
-    if ($this->searchPHID) {
-      $loader = new PhabricatorObjectHandleData(array($this->searchPHID));
-      $handles = $loader->loadHandles();
-      $handle = $handles[$this->searchPHID];
-
-      $view_items = array(
-        $this->searchPHID => $handle->getFullName(),
-      );
-    } else {
-      $view_items = array();
-    }
-
-    switch ($this->view) {
-      case 'package':
-        $datasource = '/typeahead/common/packages/';
-        $label = 'Package';
-        break;
-      case 'owner':
-        $datasource = '/typeahead/common/users/';
-        $label = 'Owner';
-        break;
-      default:
-        throw new Exception('view of the page not recognized.');
-    }
-
-    $search_form = id(new AphrontFormView())
-      ->setUser($this->user)
-      ->appendChild(
-        id(new AphrontFormTokenizerControl())
-          ->setDatasource($datasource)
-          ->setLabel($label)
-          ->setName('search_phids')
-          ->setValue($view_items)
-          ->setLimit(1));
-
-    if ($this->scope === 'attention') {
-      $select_map = array(
-        'needaudit' => 'Needs Audit',
-        'accepted' => 'Accepted',
-        'all' => 'All',
-      );
-      $select = id(new AphrontFormSelectControl())
-        ->setLabel('Audit Status')
-        ->setName('search_status')
-        ->setOptions($select_map)
-        ->setValue($this->auditStatus);
-
-      $search_form->appendChild($select);
-    }
-
-    $search_form->appendChild(
-      id(new AphrontFormSubmitControl())
-        ->setValue('Search'));
-
-    $search_view = new AphrontListFilterView();
-    $search_view->appendChild($search_form);
-    return $search_view;
-  }
-
-  private function renderCommitTable($data, array $packages) {
-    $commit_phids = array_keys($data);
-    $loader = new PhabricatorObjectHandleData($commit_phids);
-    $handles = $loader->loadHandles();
-    $objects = $loader->loadObjects();
-
-    $rows = array();
-    foreach ($commit_phids as $commit_phid) {
-      $handle = $handles[$commit_phid];
-      $object = $objects[$commit_phid];
-      $commit_data = $object->getCommitData();
-      $relationship = $data[$commit_phid];
-      $package_phid = $relationship['packagePHID'];
-      $package = $packages[$package_phid];
-
-      $epoch = $handle->getTimeStamp();
-      $date = phabricator_date($epoch, $this->user);
-      $time = phabricator_time($epoch, $this->user);
-      $commit_link = phutil_render_tag(
-        'a',
-        array(
-          'href' => $handle->getURI(),
-        ),
-        phutil_escape_html($handle->getName()));
-
-      $package_link = phutil_render_tag(
-        'a',
-        array(
-          'href' => '/owners/package/'.$package->getID().'/',
-        ),
-        phutil_escape_html($package->getName()));
-
-      $row = array(
-        $commit_link,
-        $package_link,
-        $date,
-        $time,
-        phutil_escape_html($commit_data->getSummary()),
-      );
-
-      if ($this->scope === 'attention') {
-        $reasons = json_decode($relationship['auditReasons'], true);
-        $reasons = array_map('phutil_escape_html', $reasons);
-        $reasons = implode($reasons, '<br>');
-
-        $row = array_merge(
-          $row,
-          array(
-            $reasons,
-          ));
-      }
-
-      $rows[] = $row;
-    }
-
-    $commit_table = new AphrontTableView($rows);
-
-    $headers = array(
-      'Commit',
-      'Package',
-      'Date',
-      'Time',
-      'Summary',
-    );
-    if ($this->scope === 'attention') {
-      $headers = array_merge(
-        $headers,
-        array(
-          'Audit Reasons',
-        ));
-    }
-    $commit_table->setHeaders($headers);
-
-    $column_classes =
-      array(
-        '',
-        '',
-        '',
-        'right',
-        'wide',
-      );
-    if ($this->scope === 'attention') {
-      $column_classes = array_merge(
-        $column_classes,
-        array(
-          '',
-        ));
-    }
-    $commit_table->setColumnClasses($column_classes);
-
-    $list_panel = new AphrontPanelView();
-    $list_panel->appendChild($commit_table);
-
-    return $list_panel;
-  }
-}

src/applications/owners/controller/relatedlist/__init__.php

@@ -1,32 +0,0 @@
-<?php
-/**
- * This file is automatically generated. Lint this module to rebuild it.
- * @generated
- */
-
-
-
-phutil_require_module('phabricator', 'aphront/response/redirect');
-phutil_require_module('phabricator', 'applications/audit/constants/status');
-phutil_require_module('phabricator', 'applications/owners/controller/base');
-phutil_require_module('phabricator', 'applications/owners/storage/owner');
-phutil_require_module('phabricator', 'applications/owners/storage/package');
-phutil_require_module('phabricator', 'applications/owners/storage/packagecommitrelationship');
-phutil_require_module('phabricator', 'applications/phid/handle/data');
-phutil_require_module('phabricator', 'storage/queryfx');
-phutil_require_module('phabricator', 'view/control/pager');
-phutil_require_module('phabricator', 'view/control/table');
-phutil_require_module('phabricator', 'view/form/base');
-phutil_require_module('phabricator', 'view/form/control/select');
-phutil_require_module('phabricator', 'view/form/control/submit');
-phutil_require_module('phabricator', 'view/form/control/tokenizer');
-phutil_require_module('phabricator', 'view/form/error');
-phutil_require_module('phabricator', 'view/layout/listfilter');
-phutil_require_module('phabricator', 'view/layout/panel');
-phutil_require_module('phabricator', 'view/utils');
-
-phutil_require_module('phutil', 'markup');
-phutil_require_module('phutil', 'utils');
-
-
-phutil_require_source('PhabricatorOwnerRelatedListController.php');