Move Audit to proper Subscriptions

Summary:
Ref T4896. Currently, subscriptions to commits are stored as auditors with a special "CC" type.

Instead, use normal subscriptions storage, reads and writes.

Test Plan:
  - Ran migration and verified data still looked good.
  - Viewed commits in UI and saw "subscribers".
  - Saw "Automatically Subscribed", clicked Subscribe/Unsubscribe on a non-authored commit, saw subscriptions update.
  - Pushed a commit through Herald rules and saw them trigger subscriptions and auditors.
  - Used "Add CCs".
  - Added CCs with mentions.

Reviewers: btrahan, joshuaspence

Reviewed By: btrahan, joshuaspence

Subscribers: epriestley

Maniphest Tasks: T4896

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

resources/sql/autopatches/20140731.audit.1.subscribers.php

@@ -0,0 +1,30 @@
+<?php
+
+$table = new PhabricatorRepositoryAuditRequest();
+$conn_w = $table->establishConnection('w');
+
+echo "Migrating Audit subscribers to subscriptions...\n";
+foreach (new LiskMigrationIterator($table) as $request) {
+  $id = $request->getID();
+
+  echo "Migrating auditor {$id}...\n";
+
+  if ($request->getAuditStatus() != 'cc') {
+    // This isn't a "subscriber", so skip it.
+    continue;
+  }
+
+  queryfx(
+    $conn_w,
+    'INSERT IGNORE INTO %T (src, type, dst) VALUES (%s, %d, %s)',
+    PhabricatorEdgeConfig::TABLE_NAME_EDGE,
+    $request->getCommitPHID(),
+    PhabricatorEdgeConfig::TYPE_OBJECT_HAS_SUBSCRIBER,
+    $request->getAuditorPHID());
+
+
+  // Wipe the row.
+  $request->delete();
+}
+
+echo "Done.\n";

src/__phutil_library_map__.php

@@ -4847,6 +4847,7 @@ phutil_register_library_map(array(
       'PhabricatorPolicyInterface',
       'PhabricatorFlaggableInterface',
       'PhabricatorTokenReceiverInterface',
+      'PhabricatorSubscribableInterface',
       'HarbormasterBuildableInterface',
       'PhabricatorCustomFieldInterface',
     ),

src/applications/audit/editor/PhabricatorAuditCommentEditor.php

@@ -93,6 +93,8 @@ final class PhabricatorAuditCommentEditor extends PhabricatorEditor {
       }
     }
 
+    $add_self_cc = false;
+
     if ($action == PhabricatorAuditActionConstants::CLOSE) {
       if (!PhabricatorEnv::getEnvConfig('audit.can-author-close-audit')) {
         throw new Exception('Cannot Close Audit without enabling'.
@@ -177,9 +179,9 @@ final class PhabricatorAuditCommentEditor extends PhabricatorEditor {
         $new_status = null;
         switch ($action) {
           case PhabricatorAuditActionConstants::COMMENT:
-          case PhabricatorAuditActionConstants::ADD_CCS:
           case PhabricatorAuditActionConstants::ADD_AUDITORS:
-            $new_status = PhabricatorAuditStatusConstants::AUDIT_NOT_REQUIRED;
+          case PhabricatorAuditActionConstants::ADD_CCS:
+            $add_self_cc = true;
             break;
           case PhabricatorAuditActionConstants::ACCEPT:
             $new_status = PhabricatorAuditStatusConstants::ACCEPTED;
@@ -193,18 +195,25 @@ final class PhabricatorAuditCommentEditor extends PhabricatorEditor {
             throw new Exception("Unknown or invalid action '{$action}'!");
         }
 
-        $request = id(new PhabricatorRepositoryAuditRequest())
+        if ($new_status !== null) {
-          ->setCommitPHID($commit->getPHID())
+          $request = id(new PhabricatorRepositoryAuditRequest())
-          ->setAuditorPHID($actor->getPHID())
+            ->setCommitPHID($commit->getPHID())
-          ->setAuditStatus($new_status)
+            ->setAuditorPHID($actor->getPHID())
-          ->setAuditReasons(array('Voluntary Participant'))
+            ->setAuditStatus($new_status)
-          ->save();
+            ->setAuditReasons(array('Voluntary Participant'))
-        $requests[] = $request;
+            ->save();
+          $requests[] = $request;
+        }
       }
     }
 
     $auditors = array();
     $ccs = array();
+
+    if ($add_self_cc) {
+      $ccs[] = $actor->getPHID();
+    }
+
     foreach ($comments as $comment) {
       $meta = $comment->getMetadata();
 
@@ -244,22 +253,17 @@ final class PhabricatorAuditCommentEditor extends PhabricatorEditor {
       }
     }
 
-    if ($ccs) {
-      foreach ($ccs as $cc_phid) {
-        $audit_cc = PhabricatorAuditStatusConstants::CC;
-        $requests[] = id (new PhabricatorRepositoryAuditRequest())
-          ->setCommitPHID($commit->getPHID())
-          ->setAuditorPHID($cc_phid)
-          ->setAuditStatus($audit_cc)
-          ->setAuditReasons(
-            array('Added by '.$actor->getUsername()))
-          ->save();
-      }
-    }
-
     $commit->updateAuditStatus($requests);
     $commit->save();
 
+    if ($ccs) {
+      id(new PhabricatorSubscriptionsEditor())
+        ->setActor($actor)
+        ->setObject($commit)
+        ->subscribeExplicit($ccs)
+        ->save();
+    }
+
     $content_source = PhabricatorContentSource::newForSource(
       PhabricatorContentSource::SOURCE_LEGACY,
       array());
@@ -288,15 +292,14 @@ final class PhabricatorAuditCommentEditor extends PhabricatorEditor {
     foreach ($requests as $request) {
       $status = $request->getAuditStatus();
       switch ($status) {
-      case PhabricatorAuditStatusConstants::RESIGNED:
+        case PhabricatorAuditStatusConstants::RESIGNED:
-      case PhabricatorAuditStatusConstants::NONE:
+        case PhabricatorAuditStatusConstants::NONE:
-      case PhabricatorAuditStatusConstants::AUDIT_NOT_REQUIRED:
+        case PhabricatorAuditStatusConstants::AUDIT_NOT_REQUIRED:
-      case PhabricatorAuditStatusConstants::CC:
+          $feed_dont_publish_phids[$request->getAuditorPHID()] = 1;
-        $feed_dont_publish_phids[$request->getAuditorPHID()] = 1;
+          break;
-        break;
+        default:
-      default:
+          unset($feed_dont_publish_phids[$request->getAuditorPHID()]);
-        unset($feed_dont_publish_phids[$request->getAuditorPHID()]);
+          break;
-        break;
       }
     }
     $feed_dont_publish_phids = array_keys($feed_dont_publish_phids);
@@ -452,9 +455,14 @@ final class PhabricatorAuditCommentEditor extends PhabricatorEditor {
       $email_cc[$other_comment->getActorPHID()] = true;
     }
 
+    $subscribers = PhabricatorSubscribersQuery::loadSubscribersForPHID(
+      $commit->getPHID());
+    foreach ($subscribers as $subscriber) {
+      $email_cc[$subscriber] = true;
+    }
+
     foreach ($requests as $request) {
       switch ($request->getAuditStatus()) {
-        case PhabricatorAuditStatusConstants::CC:
         case PhabricatorAuditStatusConstants::AUDIT_REQUIRED:
           $email_cc[$request->getAuditorPHID()] = true;
           break;

src/applications/diffusion/controller/DiffusionCommitController.php

@@ -1100,12 +1100,6 @@ final class DiffusionCommitController extends DiffusionController {
             'blue',
             pht('Closed'));
           break;
-        case PhabricatorAuditStatusConstants::CC:
-          $item->setIcon(
-            PHUIStatusItemView::ICON_INFO,
-            'dark',
-            pht('Subscribed'));
-          break;
         default:
           $item->setIcon(
             PHUIStatusItemView::ICON_QUESTION,

src/applications/diffusion/doorkeeper/DiffusionDoorkeeperCommitFeedStoryPublisher.php

@@ -69,10 +69,6 @@ final class DiffusionDoorkeeperCommitFeedStoryPublisher
 
     foreach ($requests as $request) {
       $status = $request->getAuditStatus();
-      if ($status == PhabricatorAuditStatusConstants::CC) {
-        // We handle these specially below.
-        continue;
-      }
 
       $object = idx($objects, $request->getAuditorPHID());
       if (!$object) {
@@ -133,13 +129,8 @@ final class DiffusionDoorkeeperCommitFeedStoryPublisher
   }
 
   public function getCCUserPHIDs($object) {
-    $ccs = array();
+    return PhabricatorSubscribersQuery::loadSubscribersForPHID(
-    foreach ($this->getAuditRequests() as $request) {
+      $object->getPHID());
-      if ($request->getAuditStatus() == PhabricatorAuditStatusConstants::CC) {
-        $ccs[] = $request->getAuditorPHID();
-      }
-    }
-    return $ccs;
   }
 
   public function getObjectTitle($object) {

src/applications/repository/storage/PhabricatorRepositoryCommit.php

@@ -6,6 +6,7 @@ final class PhabricatorRepositoryCommit
     PhabricatorPolicyInterface,
     PhabricatorFlaggableInterface,
     PhabricatorTokenReceiverInterface,
+    PhabricatorSubscribableInterface,
     HarbormasterBuildableInterface,
     PhabricatorCustomFieldInterface {
 
@@ -330,4 +331,24 @@ final class PhabricatorRepositoryCommit
     return $this;
   }
 
+
+/* -(  PhabricatorSubscribableInterface  )----------------------------------- */
+
+
+  public function isAutomaticallySubscribed($phid) {
+
+    // TODO: This should also list auditors, but handling that is a bit messy
+    // right now because we are not guaranteed to have the data.
+
+    return ($phid == $this->getAuthorPHID());
+  }
+
+  public function shouldShowSubscribersProperty() {
+    return true;
+  }
+
+  public function shouldAllowSubscription($phid) {
+    return true;
+  }
+
 }

src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php

@@ -244,7 +244,6 @@ final class PhabricatorRepositoryCommitHeraldWorker
 
     $maps = array(
       PhabricatorAuditStatusConstants::AUDIT_REQUIRED => $map,
-      PhabricatorAuditStatusConstants::CC => $ccmap,
     );
 
     foreach ($maps as $status => $map) {
@@ -281,6 +280,14 @@ final class PhabricatorRepositoryCommitHeraldWorker
 
     $commit->updateAuditStatus($requests);
     $commit->save();
+
+    if ($ccmap) {
+      id(new PhabricatorSubscriptionsEditor())
+        ->setActor(PhabricatorUser::getOmnipotentUser())
+        ->setObject($commit)
+        ->subscribeExplicit(array_keys($ccmap))
+        ->save();
+    }
   }