From b585146532c08d0b9464059494c634db6118e0df Mon Sep 17 00:00:00 2001
From: vrana <jakubv@fb.com>
Date: Tue, 2 Oct 2012 16:44:29 -0700
Subject: [PATCH] Display auditors in audit commit list

Summary: Users wonder who is actually doing all these audits.

Test Plan:
/owners/package/1/
/audit/view/packagecommits/?phid=PHID-OPKG-1

Reviewers: epriestley

Reviewed By: epriestley

CC: jungejason, aran, Korvin

Differential Revision: https://secure.phabricator.com/D3597
---
 .../PhabricatorAuditListController.php        |  1 +
 .../query/PhabricatorAuditCommitQuery.php     | 16 +++++++++++
 .../view/PhabricatorAuditCommitListView.php   | 27 +++++++++++++++++++
 .../PhabricatorOwnersDetailController.php     |  2 ++
 .../storage/PhabricatorRepositoryCommit.php   | 11 ++++++++
 5 files changed, 57 insertions(+)

diff --git a/src/applications/audit/controller/PhabricatorAuditListController.php b/src/applications/audit/controller/PhabricatorAuditListController.php
index 061493640a..49ce933013 100644
--- a/src/applications/audit/controller/PhabricatorAuditListController.php
+++ b/src/applications/audit/controller/PhabricatorAuditListController.php
@@ -412,6 +412,7 @@ final class PhabricatorAuditListController extends PhabricatorAuditController {
 
     $query = new PhabricatorAuditCommitQuery();
     $query->needCommitData(true);
+    $query->needAudits(true);
 
     $use_pager = ($this->filter != 'active');
 
diff --git a/src/applications/audit/query/PhabricatorAuditCommitQuery.php b/src/applications/audit/query/PhabricatorAuditCommitQuery.php
index f49b6803e6..2038dcf98b 100644
--- a/src/applications/audit/query/PhabricatorAuditCommitQuery.php
+++ b/src/applications/audit/query/PhabricatorAuditCommitQuery.php
@@ -27,6 +27,7 @@ final class PhabricatorAuditCommitQuery {
   private $identifiers = array();
 
   private $needCommitData;
+  private $needAudits;
 
   private $status     = 'status-any';
   const STATUS_ANY    = 'status-any';
@@ -62,6 +63,11 @@ final class PhabricatorAuditCommitQuery {
     return $this;
   }
 
+  public function needAudits($need) {
+    $this->needAudits = $need;
+    return $this;
+  }
+
   public function setOffset($offset) {
     $this->offset = $offset;
     return $this;
@@ -107,6 +113,16 @@ final class PhabricatorAuditCommitQuery {
       }
     }
 
+    if ($this->needAudits && $commits) {
+      $audits = id(new PhabricatorAuditComment())->loadAllWhere(
+        'targetPHID in (%Ls)',
+        mpull($commits, 'getPHID'));
+      $audits = mgroup($audits, 'getTargetPHID');
+      foreach ($commits as $commit) {
+        $commit->attachAudits(idx($audits, $commit->getPHID(), array()));
+      }
+    }
+
     return $commits;
 
   }
diff --git a/src/applications/audit/view/PhabricatorAuditCommitListView.php b/src/applications/audit/view/PhabricatorAuditCommitListView.php
index 715293c32a..1cc4788a65 100644
--- a/src/applications/audit/view/PhabricatorAuditCommitListView.php
+++ b/src/applications/audit/view/PhabricatorAuditCommitListView.php
@@ -57,6 +57,11 @@ final class PhabricatorAuditCommitListView extends AphrontView {
         $phids[$commit->getAuthorPHID()] = true;
       }
       $phids[$commit->getPHID()] = true;
+      if ($commit->getAudits()) {
+        foreach ($commit->getAudits() as $audit) {
+          $phids[$audit->getActorPHID()] = true;
+        }
+      }
     }
     return array_keys($phids);
   }
@@ -77,12 +82,20 @@ final class PhabricatorAuditCommitListView extends AphrontView {
       if ($commit->getAuthorPHID()) {
         $author_name = $this->getHandle($commit->getAuthorPHID())->renderLink();
       }
+      $auditors = array();
+      if ($commit->getAudits()) {
+        foreach ($commit->getAudits() as $audit) {
+          $actor_phid = $audit->getActorPHID();
+          $auditors[$actor_phid] = $this->getHandle($actor_phid)->renderLink();
+        }
+      }
       $rows[] = array(
         $commit_name,
         $author_name,
         phutil_escape_html($commit->getCommitData()->getSummary()),
         PhabricatorAuditCommitStatusConstants::getStatusName(
           $commit->getAuditStatus()),
+        implode(', ', $auditors),
         phabricator_datetime($commit->getEpoch(), $this->user),
       );
     }
@@ -94,6 +107,7 @@ final class PhabricatorAuditCommitListView extends AphrontView {
         'Author',
         'Summary',
         'Audit Status',
+        'Auditors',
         'Date',
       ));
     $table->setColumnClasses(
@@ -103,8 +117,21 @@ final class PhabricatorAuditCommitListView extends AphrontView {
         'wide',
         '',
         '',
+        '',
       ));
 
+    if ($this->commits && reset($this->commits)->getAudits() === null) {
+      $table->setColumnVisibility(
+        array(
+          true,
+          true,
+          true,
+          true,
+          false,
+          true,
+        ));
+    }
+
     if ($this->noDataString) {
       $table->setNoDataString($this->noDataString);
     }
diff --git a/src/applications/owners/controller/PhabricatorOwnersDetailController.php b/src/applications/owners/controller/PhabricatorOwnersDetailController.php
index 59b541dd50..edc58ca108 100644
--- a/src/applications/owners/controller/PhabricatorOwnersDetailController.php
+++ b/src/applications/owners/controller/PhabricatorOwnersDetailController.php
@@ -164,6 +164,7 @@ final class PhabricatorOwnersDetailController
       ->withPackagePHIDs(array($package->getPHID()))
       ->withStatus(PhabricatorAuditCommitQuery::STATUS_OPEN)
       ->needCommitData(true)
+      ->needAudits(true)
       ->setLimit(10);
     $attention_commits = $attention_query->execute();
     if ($attention_commits) {
@@ -187,6 +188,7 @@ final class PhabricatorOwnersDetailController
     $all_query = id(new PhabricatorAuditCommitQuery())
       ->withPackagePHIDs(array($package->getPHID()))
       ->needCommitData(true)
+      ->needAudits(true)
       ->setLimit(100);
     $all_commits = $all_query->execute();
 
diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommit.php b/src/applications/repository/storage/PhabricatorRepositoryCommit.php
index 3badbe3089..433b439dc8 100644
--- a/src/applications/repository/storage/PhabricatorRepositoryCommit.php
+++ b/src/applications/repository/storage/PhabricatorRepositoryCommit.php
@@ -27,6 +27,7 @@ final class PhabricatorRepositoryCommit extends PhabricatorRepositoryDAO {
   protected $auditStatus = PhabricatorAuditCommitStatusConstants::NONE;
 
   private $commitData;
+  private $audits;
   private $isUnparsed;
 
   public function setIsUnparsed($is_unparsed) {
@@ -71,6 +72,16 @@ final class PhabricatorRepositoryCommit extends PhabricatorRepositoryDAO {
     return $this->commitData;
   }
 
+  public function attachAudits(array $audits) {
+    assert_instances_of($audits, 'PhabricatorAuditComment');
+    $this->audits = $audits;
+    return $this;
+  }
+
+  public function getAudits() {
+    return $this->audits;
+  }
+
   public function save() {
     if (!$this->mailKey) {
       $this->mailKey = Filesystem::readRandomCharacters(20);