Use standard infrastructure for Feed in Audit

Summary: Ref T4896. Instead of using custom stuff, use standard stuff.

Test Plan: Viewed a bunch of feed stories and published some over the Asana bridge.

Reviewers: joshuaspence, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4896

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

src/applications/audit/editor/PhabricatorAuditCommentEditor.php

@@ -245,27 +245,6 @@ final class PhabricatorAuditCommentEditor extends PhabricatorEditor {
       ->setExcludeMailRecipientPHIDs($this->getExcludeMailRecipientPHIDs())
       ->setDisableEmail($this->noEmail)
       ->applyTransactions($commit, $xactions);
-
-    $feed_dont_publish_phids = array();
-    foreach ($requests as $request) {
-      $status = $request->getAuditStatus();
-      switch ($status) {
-        case PhabricatorAuditStatusConstants::RESIGNED:
-        case PhabricatorAuditStatusConstants::NONE:
-        case PhabricatorAuditStatusConstants::AUDIT_NOT_REQUIRED:
-          $feed_dont_publish_phids[$request->getAuditorPHID()] = 1;
-          break;
-        default:
-          unset($feed_dont_publish_phids[$request->getAuditorPHID()]);
-          break;
-      }
-    }
-    $feed_dont_publish_phids = array_keys($feed_dont_publish_phids);
-
-    $feed_phids = array_diff($requests_phids, $feed_dont_publish_phids);
-    foreach ($comments as $comment) {
-      $this->publishFeedStory($comment, $feed_phids);
-    }
   }
 
 
@@ -305,32 +284,4 @@ final class PhabricatorAuditCommentEditor extends PhabricatorEditor {
     return array_keys($phids);
   }
 
-  private function publishFeedStory(
-    PhabricatorAuditComment $comment,
-    array $more_phids) {
-
-    $commit = $this->commit;
-    $actor = $this->getActor();
-
-    $related_phids = array_merge(
-      array(
-        $actor->getPHID(),
-        $commit->getPHID(),
-      ),
-      $more_phids);
-
-    id(new PhabricatorFeedStoryPublisher())
-      ->setRelatedPHIDs($related_phids)
-      ->setStoryAuthorPHID($actor->getPHID())
-      ->setStoryTime(time())
-      ->setStoryType(PhabricatorFeedStoryTypeConstants::STORY_AUDIT)
-      ->setStoryData(
-        array(
-          'commitPHID'    => $commit->getPHID(),
-          'action'        => $comment->getAction(),
-          'content'       => $comment->getContent(),
-        ))
-      ->publish();
-  }
-
 }

src/applications/audit/editor/PhabricatorAuditEditor.php

@@ -234,4 +234,10 @@ final class PhabricatorAuditEditor
     return implode("\n", $block);
   }
 
+  protected function shouldPublishFeedStory(
+    PhabricatorLiskDAO $object,
+    array $xactions) {
+    return true;
+  }
+
 }

src/applications/audit/storage/PhabricatorAuditTransaction.php

@@ -179,6 +179,93 @@ final class PhabricatorAuditTransaction
     return parent::getTitle();
   }
 
+  public function getTitleForFeed(PhabricatorFeedStory $story) {
+    $old = $this->getOldValue();
+    $new = $this->getNewValue();
+
+    $author_handle = $this->renderHandleLink($this->getAuthorPHID());
+    $object_handle = $this->renderHandleLink($this->getObjectPHID());
+
+    $type = $this->getTransactionType();
+
+    switch ($type) {
+      case PhabricatorAuditActionConstants::ADD_CCS:
+      case PhabricatorAuditActionConstants::ADD_AUDITORS:
+        if (!is_array($old)) {
+          $old = array();
+        }
+        if (!is_array($new)) {
+          $new = array();
+        }
+        $add = array_keys(array_diff_key($new, $old));
+        $rem = array_keys(array_diff_key($old, $new));
+        break;
+    }
+
+    switch ($type) {
+      case PhabricatorAuditActionConstants::INLINE:
+        return pht(
+          '%s added inline comments to %s.',
+          $author_handle,
+          $object_handle);
+
+      case PhabricatorAuditActionConstants::ADD_AUDITORS:
+        if ($add && $rem) {
+          return pht(
+            '%s edited auditors for %s; added: %s, removed: %s.',
+            $author_handle,
+            $object_handle,
+            $this->renderHandleList($add),
+            $this->renderHandleList($rem));
+        } else if ($add) {
+          return pht(
+            '%s added auditors to %s: %s.',
+            $author_handle,
+            $object_handle,
+            $this->renderHandleList($add));
+        } else if ($rem) {
+          return pht(
+            '%s removed auditors from %s: %s.',
+            $author_handle,
+            $object_handle,
+            $this->renderHandleList($rem));
+        } else {
+          return pht(
+            '%s added auditors to %s...',
+            $author_handle,
+            $object_handle);
+        }
+
+      case PhabricatorAuditActionConstants::ACTION:
+        switch ($new) {
+          case PhabricatorAuditActionConstants::ACCEPT:
+            return pht(
+              '%s accepted %s.',
+              $author_handle,
+              $object_handle);
+          case PhabricatorAuditActionConstants::CONCERN:
+            return pht(
+              '%s raised a concern with %s.',
+              $author_handle,
+              $object_handle);
+          case PhabricatorAuditActionConstants::RESIGN:
+            return pht(
+              '%s resigned from auditing %s.',
+              $author_handle,
+              $object_handle);
+          case PhabricatorAuditActionConstants::CLOSE:
+            return pht(
+              '%s closed the audit of %s.',
+              $author_handle,
+              $object_handle);
+        }
+
+    }
+
+    return parent::getTitleForFeed($story);
+  }
+
+
   // TODO: These two mail methods can likely be abstracted by introducing a
   // formal concept of "inline comment" transactions.
 

src/applications/auth/controller/PhabricatorAuthLoginController.php

@@ -94,7 +94,7 @@ final class PhabricatorAuthLoginController
       } else {
         return $this->renderError(
           pht(
-            'The external account ("%s") you just used to login is alerady '.
+            'The external account ("%s") you just used to login is already '.
             'associated with another Phabricator user account. Login to the '.
             'other Phabricator account and unlink the external account before '.
             'linking it to a new Phabricator account.',

src/applications/diffusion/doorkeeper/DiffusionDoorkeeperCommitFeedStoryPublisher.php

@@ -12,7 +12,9 @@ final class DiffusionDoorkeeperCommitFeedStoryPublisher
   }
 
   public function canPublishStory(PhabricatorFeedStory $story, $object) {
-    return ($object instanceof PhabricatorRepositoryCommit);
+    return
+      ($story instanceof PhabricatorApplicationTransactionFeedStory) &&
+      ($object instanceof PhabricatorRepositoryCommit);
   }
 
   public function isStoryAboutObjectCreation($object) {
@@ -29,17 +31,21 @@ final class DiffusionDoorkeeperCommitFeedStoryPublisher
     // After ApplicationTransactions, we could annotate feed stories more
     // explicitly.
 
-    $story = $this->getFeedStory();
-    $action = $story->getStoryData()->getValue('action');
-
-    if ($action == PhabricatorAuditActionConstants::CLOSE) {
-      return true;
-    }
-
     $fully_audited = PhabricatorAuditCommitStatusConstants::FULLY_AUDITED;
-    if (($action == PhabricatorAuditActionConstants::ACCEPT) &&
+
-        $object->getAuditStatus() == $fully_audited) {
+    $story = $this->getFeedStory();
-      return true;
+    $xaction = $story->getPrimaryTransaction();
+    switch ($xaction->getTransactionType()) {
+      case PhabricatorAuditActionConstants::ACTION:
+        switch ($xaction->getNewValue()) {
+          case PhabricatorAuditActionConstants::CLOSE:
+            return true;
+          case PhabricatorAuditActionConstants::ACCEPT:
+            if ($object->getAuditStatus() == $fully_audited) {
+              return true;
+            }
+            break;
+        }
     }
 
     return false;

src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php

@@ -30,7 +30,7 @@ class PhabricatorApplicationTransactionFeedStory
     return head($this->getValue('transactionPHIDs'));
   }
 
-  protected function getPrimaryTransaction() {
+  public function getPrimaryTransaction() {
     return $this->getObject($this->getPrimaryTransactionPHID());
   }