Split users apart from projects/packages in reviewer and audit UIs

Summary: Ref T1279. Show separate sections for "Reviewers" and "Project Reviewers" (Differential) and for "Auditors" and "Package/Project Auditors" (Diffusion/Audit).

Test Plan:
  - Looked at a commit. Saw separation.
  - Looked at a revision. Saw separation.

Reviewers: chad, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1279

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

src/__phutil_library_map__.php

@@ -392,6 +392,7 @@ phutil_register_library_map(array(
     'DifferentialPathFieldSpecification' => 'applications/differential/field/specification/DifferentialPathFieldSpecification.php',
     'DifferentialPeopleMenuEventListener' => 'applications/differential/events/DifferentialPeopleMenuEventListener.php',
     'DifferentialPrimaryPaneView' => 'applications/differential/view/DifferentialPrimaryPaneView.php',
+    'DifferentialProjectReviewersFieldSpecification' => 'applications/differential/field/specification/DifferentialProjectReviewersFieldSpecification.php',
     'DifferentialRawDiffRenderer' => 'applications/differential/render/DifferentialRawDiffRenderer.php',
     'DifferentialReleephRequestFieldSpecification' => 'applications/releeph/differential/DifferentialReleephRequestFieldSpecification.php',
     'DifferentialRemarkupRule' => 'applications/differential/remarkup/DifferentialRemarkupRule.php',
@@ -405,6 +406,7 @@ phutil_register_library_map(array(
     'DifferentialReviewer' => 'applications/differential/storage/DifferentialReviewer.php',
     'DifferentialReviewerStatus' => 'applications/differential/constants/DifferentialReviewerStatus.php',
     'DifferentialReviewersFieldSpecification' => 'applications/differential/field/specification/DifferentialReviewersFieldSpecification.php',
+    'DifferentialReviewersView' => 'applications/differential/view/DifferentialReviewersView.php',
     'DifferentialRevision' => 'applications/differential/storage/DifferentialRevision.php',
     'DifferentialRevisionCommentListView' => 'applications/differential/view/DifferentialRevisionCommentListView.php',
     'DifferentialRevisionCommentView' => 'applications/differential/view/DifferentialRevisionCommentView.php',
@@ -2487,6 +2489,7 @@ phutil_register_library_map(array(
     'DifferentialPathFieldSpecification' => 'DifferentialFieldSpecification',
     'DifferentialPeopleMenuEventListener' => 'PhutilEventListener',
     'DifferentialPrimaryPaneView' => 'AphrontView',
+    'DifferentialProjectReviewersFieldSpecification' => 'DifferentialFieldSpecification',
     'DifferentialReleephRequestFieldSpecification' => 'DifferentialFieldSpecification',
     'DifferentialRemarkupRule' => 'PhabricatorRemarkupRuleObject',
     'DifferentialReplyHandler' => 'PhabricatorMailReplyHandler',
@@ -2497,6 +2500,7 @@ phutil_register_library_map(array(
     'DifferentialReviewRequestMail' => 'DifferentialMail',
     'DifferentialReviewedByFieldSpecification' => 'DifferentialFieldSpecification',
     'DifferentialReviewersFieldSpecification' => 'DifferentialFieldSpecification',
+    'DifferentialReviewersView' => 'AphrontView',
     'DifferentialRevision' =>
     array(
       0 => 'DifferentialDAO',

src/applications/differential/field/selector/DifferentialDefaultFieldSelector.php

@@ -11,6 +11,7 @@ final class DifferentialDefaultFieldSelector
       new DifferentialRevisionStatusFieldSpecification(),
       new DifferentialAuthorFieldSpecification(),
       new DifferentialReviewersFieldSpecification(),
+      new DifferentialProjectReviewersFieldSpecification(),
       new DifferentialReviewedByFieldSpecification(),
       new DifferentialCCsFieldSpecification(),
       new DifferentialRepositoryFieldSpecification(),

src/applications/differential/field/specification/DifferentialProjectReviewersFieldSpecification.php

@@ -0,0 +1,42 @@
+<?php
+
+final class DifferentialProjectReviewersFieldSpecification
+  extends DifferentialFieldSpecification {
+
+  public function shouldAppearOnRevisionView() {
+    return true;
+  }
+
+  public function getRequiredHandlePHIDsForRevisionView() {
+    return $this->getRevision()->getReviewers();
+  }
+
+  public function renderLabelForRevisionView() {
+    return pht('Project Reviewers');
+  }
+
+  public function renderValueForRevisionView() {
+    $reviewers = array();
+    foreach ($this->getRevision()->getReviewerStatus() as $reviewer) {
+      if (!$reviewer->isUser()) {
+        $reviewers[] = $reviewer;
+      }
+    }
+
+    if (!$reviewers) {
+      return null;
+    }
+
+    $view = id(new DifferentialReviewersView())
+      ->setReviewers($reviewers)
+      ->setHandles($this->getLoadedHandles());
+
+    $diff = $this->getRevision()->loadActiveDiff();
+    if ($diff) {
+      $view->setActiveDiff($diff);
+    }
+
+    return $view;
+  }
+
+}

src/applications/differential/field/specification/DifferentialReviewersFieldSpecification.php

@@ -19,77 +19,27 @@ final class DifferentialReviewersFieldSpecification
   }
 
   public function renderValueForRevisionView() {
-    if (!$this->getReviewerPHIDs()) {
+    $reviewers = array();
+    foreach ($this->getRevision()->getReviewerStatus() as $reviewer) {
+      if ($reviewer->isUser()) {
+        $reviewers[] = $reviewer;
+      }
+    }
+
+    if (!$reviewers) {
       // Renders "None".
       return $this->renderUserList(array());
     }
 
-    $revision = $this->getRevision();
+    $view = id(new DifferentialReviewersView())
-    $reviewers = $revision->getReviewerStatus();
+      ->setReviewers($reviewers)
+      ->setHandles($this->getLoadedHandles());
 
-
+    $diff = $this->getRevision()->loadActiveDiff();
-    $diff = $revision->loadActiveDiff();
     if ($diff) {
-      $diff = $diff->getID();
+      $view->setActiveDiff($diff);
     }
 
-    $view = new PHUIStatusListView();
-    $handles = $this->getLoadedHandles();
-    foreach ($reviewers as $reviewer) {
-      $phid = $reviewer->getReviewerPHID();
-
-      $is_current = (!$diff) ||
-                    (!$reviewer->getDiffID()) ||
-                    ($diff == $reviewer->getDiffID());
-
-      $item = new PHUIStatusItemView();
-
-      switch ($reviewer->getStatus()) {
-        case DifferentialReviewerStatus::STATUS_ADDED:
-          $item->setIcon('open-dark', pht('Review Requested'));
-          break;
-
-        case DifferentialReviewerStatus::STATUS_ACCEPTED:
-          if ($is_current) {
-            $item->setIcon(
-              'accept-green',
-              pht('Accept'));
-          } else {
-            $item->setIcon(
-              'accept-dark',
-              pht('Accepted Prior Diff'));
-          }
-          break;
-
-        case DifferentialReviewerStatus::STATUS_REJECTED:
-          if ($is_current) {
-            $item->setIcon(
-              'reject-red',
-              pht('Requested Changes'));
-          } else {
-            $item->setIcon(
-              'reject-dark',
-              pht('Requested Changes to Prior Diff'));
-          }
-          break;
-
-        case DifferentialReviewerStatus::STATUS_COMMENTED:
-          if ($is_current) {
-            $item->setIcon(
-              'info-blue',
-              pht('Commented'));
-          } else {
-            $item->setIcon(
-              'info-dark',
-              pht('Commented Previously'));
-          }
-          break;
-
-      }
-
-      $item->setTarget($handles[$phid]->renderLink());
-      $view->addItem($item);
-    }
     return $view;
   }
 

src/applications/differential/storage/DifferentialReviewer.php

@@ -24,4 +24,9 @@ final class DifferentialReviewer {
     return $this->diffID;
   }
 
+  public function isUser() {
+    $user_type = PhabricatorPeoplePHIDTypeUser::TYPECONST;
+    return (phid_get_type($this->getReviewerPHID()) == $user_type);
+  }
+
 }

src/applications/differential/view/DifferentialReviewersView.php

@@ -0,0 +1,96 @@
+<?php
+
+final class DifferentialReviewersView extends AphrontView {
+
+  private $reviewers;
+  private $handles;
+  private $diff;
+
+  public function setReviewers(array $reviewers) {
+    assert_instances_of($reviewers, 'DifferentialReviewer');
+    $this->reviewers = $reviewers;
+    return $this;
+  }
+
+  public function setHandles(array $handles) {
+    assert_instances_of($handles, 'PhabricatorObjectHandle');
+    $this->handles = $handles;
+    return $this;
+  }
+
+  public function setActiveDiff(DifferentialDiff $diff) {
+    $this->diff = $diff;
+    return $this;
+  }
+
+  public function render() {
+    $view = new PHUIStatusListView();
+
+    foreach ($this->reviewers as $reviewer) {
+      $phid = $reviewer->getReviewerPHID();
+      $handle = $this->handles[$phid];
+
+      // If we're missing either the diff or action information for the
+      // reviewer, render information as current.
+      $is_current = (!$this->diff) ||
+                    (!$reviewer->getDiffID()) ||
+                    ($this->diff->getID() == $reviewer->getDiffID());
+
+      $item = new PHUIStatusItemView();
+
+
+      switch ($reviewer->getStatus()) {
+        case DifferentialReviewerStatus::STATUS_ADDED:
+          $item->setIcon('open-dark', pht('Review Requested'));
+          break;
+
+        case DifferentialReviewerStatus::STATUS_ACCEPTED:
+          if ($is_current) {
+            $item->setIcon(
+              'accept-green',
+              pht('Accepted'));
+          } else {
+            $item->setIcon(
+              'accept-dark',
+              pht('Accepted Prior Diff'));
+          }
+          break;
+
+        case DifferentialReviewerStatus::STATUS_REJECTED:
+          if ($is_current) {
+            $item->setIcon(
+              'reject-red',
+              pht('Requested Changes'));
+          } else {
+            $item->setIcon(
+              'reject-dark',
+              pht('Requested Changes to Prior Diff'));
+          }
+          break;
+
+        case DifferentialReviewerStatus::STATUS_COMMENTED:
+          if ($is_current) {
+            $item->setIcon(
+              'info-blue',
+              pht('Commented'));
+          } else {
+            $item->setIcon(
+              'info-dark',
+              pht('Commented Previously'));
+          }
+          break;
+
+        default:
+          $item->setIcon('question-dark', pht('%s?', $reviewer->getStatus()));
+          break;
+
+      }
+
+      $item->setTarget($handle->renderLink());
+      $view->addItem($item);
+    }
+
+    return $view;
+  }
+
+}

src/applications/diffusion/controller/DiffusionCommitController.php

@@ -473,7 +473,25 @@ final class DiffusionCommitController extends DiffusionController {
     }
 
     if ($audit_requests) {
-      $props['Auditors'] = $this->renderAuditStatusView($audit_requests);
+      $user_requests = array();
+      $other_requests = array();
+      foreach ($audit_requests as $audit_request) {
+        if ($audit_request->isUser()) {
+          $user_requests[] = $audit_request;
+        } else {
+          $other_requests[] = $audit_request;
+        }
+      }
+
+      if ($user_requests) {
+        $props['Auditors'] = $this->renderAuditStatusView(
+          $user_requests);
+      }
+
+      if ($other_requests) {
+        $props['Project/Package Auditors'] = $this->renderAuditStatusView(
+          $other_requests);
+      }
     }
 
     $props['Committed'] = phabricator_datetime($commit->getEpoch(), $user);
@@ -1012,6 +1030,11 @@ final class DiffusionCommitController extends DiffusionController {
         case PhabricatorAuditStatusConstants::CC:
           $item->setIcon('info-dark', pht('Subscribed'));
           break;
+        default:
+          $item->setIcon(
+            'question-dark',
+            pht('%s?', $request->getAuditStatus()));
+          break;
       }
 
       $note = array();

src/applications/repository/storage/PhabricatorRepositoryAuditRequest.php

@@ -16,4 +16,9 @@ final class PhabricatorRepositoryAuditRequest extends PhabricatorRepositoryDAO {
     ) + parent::getConfiguration();
   }
 
+  public function isUser() {
+    $user_type = PhabricatorPeoplePHIDTypeUser::TYPECONST;
+    return (phid_get_type($this->getAuditorPHID()) == $user_type);
+  }
+
 }

src/view/phui/PHUIStatusItemView.php

@@ -62,6 +62,7 @@ final class PHUIStatusItemView extends AphrontTagView {
         $icon->setMetadata(
           array(
             'tip' => $this->iconLabel,
+            'size' => 240,
           ));
       }
     }