Allow audit comments to be added from Diffusion

Summary:
This is intended to supplant the existing "audit edit" interface. I've changed
them to both drive down the same write pathway, but the UIs are still different.
I'll fully merge them in a future diff.

Add a comment box (like Maniphest and Differential) to Diffusion. When users
make comments, their comments appear on the commit. Any audits triggers they are
responsible for are updated to reflect actions they take, as well.

Currently, audits can only be triggered by packages, but I intend to allow them
to be triggered by users and projects (via herald rules) in an upcoming diff.
Thus some of the language like "projects, users or packages" when the code is
clearly dealing only with "packagePHID".

Test Plan: Made audit updates via commit interface and via existing edit
interface. Verified both interfaces updated correctly, and that audit
responsibility rules were applied properly.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T904

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

src/__phutil_library_map__.php

@@ -441,7 +441,9 @@ phutil_register_library_map(array(
     'MetaMTANotificationType' => 'applications/metamta/constants/notificationtype',
     'Phabricator404Controller' => 'applications/base/controller/404',
     'PhabricatorAuditActionConstants' => 'applications/audit/constants/action',
+    'PhabricatorAuditAddCommentController' => 'applications/audit/controller/addcomment',
     'PhabricatorAuditComment' => 'applications/audit/storage/auditcomment',
+    'PhabricatorAuditCommentEditor' => 'applications/audit/editor/comment',
     'PhabricatorAuditController' => 'applications/audit/controller/base',
     'PhabricatorAuditDAO' => 'applications/audit/storage/base',
     'PhabricatorAuditEditController' => 'applications/audit/controller/edit',
@@ -1230,6 +1232,7 @@ phutil_register_library_map(array(
     'ManiphestView' => 'AphrontView',
     'MetaMTANotificationType' => 'MetaMTAConstants',
     'Phabricator404Controller' => 'PhabricatorController',
+    'PhabricatorAuditAddCommentController' => 'PhabricatorAuditController',
     'PhabricatorAuditComment' => 'PhabricatorAuditDAO',
     'PhabricatorAuditController' => 'PhabricatorController',
     'PhabricatorAuditDAO' => 'PhabricatorLiskDAO',

src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php

@@ -350,6 +350,7 @@ class AphrontDefaultApplicationConfiguration
         'view/(?P<filter>[^/]+)/$' => 'PhabricatorAuditListController',
 
         'edit/$' => 'PhabricatorAuditEditController',
+        'addcomment/$' => 'PhabricatorAuditAddCommentController',
       ),
 
       '/xhpast/' => array(

src/applications/audit/constants/action/PhabricatorAuditActionConstants.php

@@ -26,7 +26,7 @@ class PhabricatorAuditActionConstants {
     static $map = array(
       self::COMMENT => 'Comment',
       self::CONCERN => 'Raise Concern',
-      self::ACCEPT  => 'Accept',
+      self::ACCEPT  => 'Accept Commit',
     );
 
     return $map;

src/applications/audit/controller/addcomment/PhabricatorAuditAddCommentController.php

@@ -0,0 +1,54 @@
+<?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.
+ */
+
+final class PhabricatorAuditAddCommentController
+  extends PhabricatorAuditController {
+
+  public function processRequest() {
+    $request = $this->getRequest();
+    $user = $request->getUser();
+
+    if (!$request->isFormPost()) {
+      return new Aphront403Response();
+    }
+
+    $commit_phid = $request->getStr('commit');
+    $commit = id(new PhabricatorRepositoryCommit())->loadOneWhere(
+      'phid = %s',
+      $commit_phid);
+
+    if (!$commit) {
+      return new Aphront404Response();
+    }
+
+    $comment = id(new PhabricatorAuditComment())
+      ->setAction($request->getStr('action'))
+      ->setContent($request->getStr('content'));
+
+    id(new PhabricatorAuditCommentEditor($commit))
+      ->setUser($user)
+      ->addComment($comment);
+
+    $phids = array($commit_phid);
+    $handles = id(new PhabricatorObjectHandleData($phids))->loadHandles();
+    $uri = $handles[$commit_phid]->getURI();
+
+    return id(new AphrontRedirectResponse())->setURI($uri);
+  }
+
+}

src/applications/audit/controller/addcomment/__init__.php

@@ -0,0 +1,21 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'aphront/response/403');
+phutil_require_module('phabricator', 'aphront/response/404');
+phutil_require_module('phabricator', 'aphront/response/redirect');
+phutil_require_module('phabricator', 'applications/audit/controller/base');
+phutil_require_module('phabricator', 'applications/audit/editor/comment');
+phutil_require_module('phabricator', 'applications/audit/storage/auditcomment');
+phutil_require_module('phabricator', 'applications/phid/handle/data');
+phutil_require_module('phabricator', 'applications/repository/storage/commit');
+
+phutil_require_module('phutil', 'utils');
+
+
+phutil_require_source('PhabricatorAuditAddCommentController.php');

src/applications/audit/controller/edit/PhabricatorAuditEditController.php

@@ -1,7 +1,7 @@
 <?php
 
 /*
- * Copyright 2011 Facebook, Inc.
+ * 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.
@@ -228,53 +228,21 @@ class PhabricatorAuditEditController extends PhabricatorAuditController {
 
   private function saveAuditComments() {
     $action = $this->request->getStr('action');
-    $status_map = PhabricatorAuditActionConstants::getStatusNameMap();
-    $status = idx($status_map, $action, null);
-    if ($status === null) {
-      return $this->buildStandardPageResponse(
-        id(new AphrontErrorView())
-          ->setSeverity(AphrontErrorView::SEVERITY_ERROR)
-          ->setTitle("Action {$action} is invalid."),
-        array(
-          'title' => 'Audit a Commit',
-        ));
+
+    $commit = id(new PhabricatorRepositoryCommit())->loadOneWhere(
+      'phid = %s',
+      $this->commitPHID);
+    if (!$commit) {
+      throw new Exception("No such commit!");
     }
 
-    id(new PhabricatorAuditComment())
-      ->setActorPHID($this->user->getPHID())
-      ->setTargetPHID($this->commitPHID)
+    $comment = id(new PhabricatorAuditComment())
       ->setAction($action)
-      ->setContent($this->request->getStr('comments'))
-      ->save();
+      ->setContent($this->request->getStr('comments'));
 
-    // Update the audit status for all the relationships <commit, package>
-    // where the package is owned by the user. When a user owns several
-    // packages and a commit touches all of them,It should be good enough for
-    // the user to approve it once to get all the relationships automatically
-    // updated.
-    $owned_packages = id(new PhabricatorOwnersOwner())->loadAllWhere(
-      'userPHID = %s',
-      $this->user->getPHID());
-    $owned_package_ids = mpull($owned_packages, 'getPackageID');
-
-    $conn_r = id(new PhabricatorOwnersPackage())->establishConnection('r');
-    $owned_package_phids = queryfx_all(
-      $conn_r,
-      'SELECT `phid` FROM %T WHERE id IN (%Ld)',
-      id(new PhabricatorOwnersPackage())->getTableName(),
-      $owned_package_ids);
-    $owned_package_phids = ipull($owned_package_phids, 'phid');
-
-    $relationships = id(new PhabricatorOwnersPackageCommitRelationship())
-      ->loadAllWhere(
-        'commitPHID = %s AND packagePHID IN (%Ls)',
-        $this->commitPHID,
-        $owned_package_phids);
-
-    foreach ($relationships as $relationship) {
-      $relationship->setAuditStatus($status);
-      $relationship->save();
-    }
+    $editor = id(new PhabricatorAuditCommentEditor($commit))
+      ->setUser($this->user)
+      ->addComment($comment);
 
     return id(new AphrontRedirectResponse())
       ->setURI(sprintf('/audit/edit/?c-phid=%s&p-phid=%s',

src/applications/audit/controller/edit/__init__.php

@@ -11,12 +11,14 @@ phutil_require_module('phabricator', 'aphront/response/redirect');
 phutil_require_module('phabricator', 'applications/audit/constants/action');
 phutil_require_module('phabricator', 'applications/audit/constants/status');
 phutil_require_module('phabricator', 'applications/audit/controller/base');
+phutil_require_module('phabricator', 'applications/audit/editor/comment');
 phutil_require_module('phabricator', 'applications/audit/storage/auditcomment');
 phutil_require_module('phabricator', 'applications/differential/storage/revision');
 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', 'applications/repository/storage/commit');
 phutil_require_module('phabricator', 'storage/queryfx');
 phutil_require_module('phabricator', 'view/form/base');
 phutil_require_module('phabricator', 'view/form/control/divider');

src/applications/audit/controller/list/__init__.php

@@ -6,34 +6,14 @@
 
 
 
-phutil_require_module('phabricator', 'aphront/response/404');
-phutil_require_module('phabricator', 'aphront/response/redirect');
-phutil_require_module('phabricator', 'applications/audit/constants/action');
-phutil_require_module('phabricator', 'applications/audit/constants/status');
 phutil_require_module('phabricator', 'applications/audit/controller/base');
 phutil_require_module('phabricator', 'applications/audit/query/audit');
-phutil_require_module('phabricator', 'applications/audit/storage/auditcomment');
 phutil_require_module('phabricator', 'applications/audit/view/list');
-phutil_require_module('phabricator', 'applications/differential/storage/revision');
-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/form/base');
-phutil_require_module('phabricator', 'view/form/control/divider');
-phutil_require_module('phabricator', 'view/form/control/markup');
-phutil_require_module('phabricator', 'view/form/control/select');
-phutil_require_module('phabricator', 'view/form/control/static');
-phutil_require_module('phabricator', 'view/form/control/submit');
-phutil_require_module('phabricator', 'view/form/control/textarea');
-phutil_require_module('phabricator', 'view/form/error');
 phutil_require_module('phabricator', 'view/layout/panel');
 phutil_require_module('phabricator', 'view/layout/sidenavfilter');
-phutil_require_module('phabricator', 'view/utils');
 
-phutil_require_module('phutil', 'markup');
 phutil_require_module('phutil', 'parser/uri');
 phutil_require_module('phutil', 'utils');
 

src/applications/audit/editor/comment/PhabricatorAuditCommentEditor.php

@@ -0,0 +1,115 @@
+<?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.
+ */
+
+final class PhabricatorAuditCommentEditor {
+
+  private $commit;
+  private $user;
+
+  public function __construct(PhabricatorRepositoryCommit $commit) {
+    $this->commit = $commit;
+    return $this;
+  }
+
+  public function setUser(PhabricatorUser $user) {
+    $this->user = $user;
+    return $this;
+  }
+
+  public function addComment(PhabricatorAuditComment $comment) {
+
+    $commit = $this->commit;
+    $user = $this->user;
+
+    $comment
+      ->setActorPHID($user->getPHID())
+      ->setTargetPHID($commit->getPHID())
+      ->save();
+
+    // When a user submits an audit comment, we update all the audit requests
+    // they have authority over to reflect the most recent status. The general
+    // idea here is that if audit has triggered for, e.g., several packages, but
+    // a user owns all of them, they can clear the audit requirement in one go
+    // without auditing the commit for each trigger.
+
+    $audit_phids = self::loadAuditPHIDsForUser($this->user);
+    $audit_phids = array_fill_keys($audit_phids, true);
+
+    $relationships = id(new PhabricatorOwnersPackageCommitRelationship())
+      ->loadAllWhere(
+        'commitPHID = %s',
+        $commit->getPHID());
+
+    $action = $comment->getAction();
+    $status_map = PhabricatorAuditActionConstants::getStatusNameMap();
+    $status = idx($status_map, $action, null);
+
+    // Status may be empty for updates which don't affect status, like
+    // "comment".
+    if ($status) {
+      foreach ($relationships as $relationship) {
+        if (empty($audit_phids[$relationship->getPackagePHID()])) {
+          continue;
+        }
+        $relationship->setAuditStatus($status);
+        $relationship->save();
+      }
+    }
+
+    // TODO: News feed.
+    // TODO: Search index.
+    // TODO: Email.
+  }
+
+
+  /**
+   * Load the PHIDs for all objects the user has the authority to act as an
+   * audit for. This includes themselves, and any packages they are an owner
+   * of.
+   */
+  public static function loadAuditPHIDsForUser(PhabricatorUser $user) {
+    $phids = array();
+
+    // The user can audit on their own behalf.
+    $phids[$user->getPHID()] = true;
+
+    // The user can audit on behalf of all packages they own.
+    $owned_packages = id(new PhabricatorOwnersOwner())->loadAllWhere(
+      'userPHID = %s',
+      $user->getPHID());
+    if ($owned_packages) {
+      $packages = id(new PhabricatorOwnersPackage())->loadAllWhere(
+        'id IN (%Ld)',
+        mpull($owned_packages, 'getPackageID'));
+      foreach (mpull($packages, 'getPHID') as $phid) {
+        $phids[$phid] = true;
+      }
+    }
+
+    // The user can audit on behalf of all projects they are a member of.
+    $query = new PhabricatorProjectQuery();
+    $query->setMembers(array($user->getPHID));
+    $projects = $query->execute();
+    foreach ($projects as $project) {
+      $phids[$project->getPHID()] = true;
+    }
+
+    return array_keys($phids);
+  }
+
+}

src/applications/audit/editor/comment/__init__.php

@@ -0,0 +1,18 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'applications/audit/constants/action');
+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/project/query/project');
+
+phutil_require_module('phutil', 'utils');
+
+
+phutil_require_source('PhabricatorAuditCommentEditor.php');

src/applications/diffusion/controller/commit/DiffusionCommitController.php

@@ -216,6 +216,8 @@ class DiffusionCommitController extends DiffusionController {
       $content[] = $change_list;
     }
 
+    $content[] = $this->buildAddCommentView($commit);
+
     return $this->buildStandardPageResponse(
       $content,
       array(
@@ -329,4 +331,42 @@ class DiffusionCommitController extends DiffusionController {
     return $view;
   }
 
+  private function buildAddCommentView($commit) {
+    $user = $this->getRequest()->getUser();
+
+    $is_serious = PhabricatorEnv::getEnvConfig('phabricator.serious-business');
+
+    $form = id(new AphrontFormView())
+      ->setUser($user)
+      ->setAction('/audit/addcomment/')
+      ->addHiddenInput('commit', $commit->getPHID())
+      ->appendChild(
+        id(new AphrontFormSelectControl())
+          ->setLabel('Action')
+          ->setName('action')
+          ->setOptions(PhabricatorAuditActionConstants::getActionNameMap()))
+      ->appendChild(
+        id(new AphrontFormTextAreaControl())
+          ->setLabel('Comments')
+          ->setName('content')
+          ->setCaption(phutil_render_tag(
+            'a',
+            array(
+              'href' => PhabricatorEnv::getDoclink(
+                'article/Remarkup_Reference.html'),
+              'tabindex' => '-1',
+              'target' => '_blank',
+            ),
+            'Formatting Reference')))
+      ->appendChild(
+        id(new AphrontFormSubmitControl())
+          ->setValue($is_serious ? 'Submit' : 'Cook the Books'));
+
+    $panel = new AphrontPanelView();
+    $panel->setHeader($is_serious ? 'Audit Commit' : 'Creative Accounting');
+    $panel->appendChild($form);
+
+    return $panel;
+  }
+
 }

src/applications/diffusion/controller/commit/__init__.php

@@ -6,6 +6,7 @@
 
 
 
+phutil_require_module('phabricator', 'applications/audit/constants/action');
 phutil_require_module('phabricator', 'applications/audit/query/audit');
 phutil_require_module('phabricator', 'applications/audit/storage/auditcomment');
 phutil_require_module('phabricator', 'applications/audit/view/list');
@@ -22,7 +23,12 @@ phutil_require_module('phabricator', 'applications/phid/handle/data');
 phutil_require_module('phabricator', 'applications/repository/constants/repositorytype');
 phutil_require_module('phabricator', 'applications/repository/storage/repository');
 phutil_require_module('phabricator', 'infrastructure/celerity/api');
+phutil_require_module('phabricator', 'infrastructure/env');
 phutil_require_module('phabricator', 'storage/queryfx');
+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/textarea');
 phutil_require_module('phabricator', 'view/form/error');
 phutil_require_module('phabricator', 'view/layout/panel');
 phutil_require_module('phabricator', 'view/utils');