From 75e56cb25d4d90b81e91a795391ceec5ccdd387e Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 1 Aug 2013 12:19:18 -0700
Subject: [PATCH] Publish create object stories into Asana sort of, but not
 really

Summary: Ref T2852. Current code works fine, but although we want to drop creation stories, we really only want to drop the story text, not the other effects of the creation story. Also generalize this mechanism so we don't have Asana-specific code in the publishers.

Test Plan: Used `bin/feed republish` to publish creation and non-creation stories. Verified creation story published no text.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2852

Differential Revision: https://secure.phabricator.com/D6639
---
 ...alDoorkeeperRevisionFeedStoryPublisher.php | 16 ++++-------
 ...sionDoorkeeperCommitFeedStoryPublisher.php |  7 +++++
 .../engine/DoorkeeperFeedStoryPublisher.php   |  1 +
 .../worker/DoorkeeperFeedWorkerAsana.php      | 28 +++++++++++--------
 4 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/src/applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php b/src/applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php
index d11e73dc78..1b9750a100 100644
--- a/src/applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php
+++ b/src/applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php
@@ -4,23 +4,19 @@ final class DifferentialDoorkeeperRevisionFeedStoryPublisher
   extends DoorkeeperFeedStoryPublisher {
 
   public function canPublishStory(PhabricatorFeedStory $story, $object) {
-    if (!($object instanceof DifferentialRevision)) {
-      return false;
-    }
+    return ($object instanceof DifferentialRevision);
+  }
 
-    // Don't publish the "create" story, since pushing the object into Asana
-    // naturally generates a notification which effectively serves the same
-    // purpose as the "create" story.
+  public function isStoryAboutObjectCreation($object) {
+    $story = $this->getFeedStory();
 
     $action = $story->getStoryData()->getValue('action');
     switch ($action) {
       case DifferentialAction::ACTION_CREATE:
-        return false;
+        return true;
       default:
-        break;
+        return false;
     }
-
-    return true;
   }
 
   public function willPublishStory($object) {
diff --git a/src/applications/diffusion/doorkeeper/DiffusionDoorkeeperCommitFeedStoryPublisher.php b/src/applications/diffusion/doorkeeper/DiffusionDoorkeeperCommitFeedStoryPublisher.php
index 77fc99a302..6ded4ad2f8 100644
--- a/src/applications/diffusion/doorkeeper/DiffusionDoorkeeperCommitFeedStoryPublisher.php
+++ b/src/applications/diffusion/doorkeeper/DiffusionDoorkeeperCommitFeedStoryPublisher.php
@@ -15,6 +15,13 @@ final class DiffusionDoorkeeperCommitFeedStoryPublisher
     return ($object instanceof PhabricatorRepositoryCommit);
   }
 
+  public function isStoryAboutObjectCreation($object) {
+    // TODO: Although creation stories exist, they currently don't have a
+    // primary object PHID set, so they'll never make it here because they
+    // won't pass `canPublishStory()`.
+    return false;
+  }
+
   public function willPublishStory($commit) {
     $requests = id(new PhabricatorAuditQuery())
       ->withCommitPHIDs(array($commit->getPHID()))
diff --git a/src/applications/doorkeeper/engine/DoorkeeperFeedStoryPublisher.php b/src/applications/doorkeeper/engine/DoorkeeperFeedStoryPublisher.php
index 460c82fa8b..428a22be67 100644
--- a/src/applications/doorkeeper/engine/DoorkeeperFeedStoryPublisher.php
+++ b/src/applications/doorkeeper/engine/DoorkeeperFeedStoryPublisher.php
@@ -35,6 +35,7 @@ abstract class DoorkeeperFeedStoryPublisher {
     return $object;
   }
 
+  abstract public function isStoryAboutObjectCreation($object);
   abstract public function getOwnerPHID($object);
   abstract public function getActiveUserPHIDs($object);
   abstract public function getPassiveUserPHIDs($object);
diff --git a/src/applications/doorkeeper/worker/DoorkeeperFeedWorkerAsana.php b/src/applications/doorkeeper/worker/DoorkeeperFeedWorkerAsana.php
index d4f7486ae6..f23653a744 100644
--- a/src/applications/doorkeeper/worker/DoorkeeperFeedWorkerAsana.php
+++ b/src/applications/doorkeeper/worker/DoorkeeperFeedWorkerAsana.php
@@ -476,20 +476,24 @@ final class DoorkeeperFeedWorkerAsana extends FeedPushWorker {
 
     $sub_editor->save();
 
+    // Don't publish the "create" story, since pushing the object into Asana
+    // naturally generates a notification which effectively serves the same
+    // purpose as the "create" story.
+    if (!$publisher->isStoryAboutObjectCreation($object)) {
+      // Post the feed story itself to the main Asana task. We do this last
+      // because everything else is idempotent, so this is the only effect we
+      // can't safely run more than once.
 
-    // Post the feed story itself to the main Asana task. We do this last
-    // because everything else is idempotent, so this is the only effect we
-    // can't safely run more than once.
+      $text = $publisher->getStoryText($object);
 
-    $text = $publisher->getStoryText($object);
-
-    $this->makeAsanaAPICall(
-      $oauth_token,
-      'tasks/'.$parent_ref->getObjectID().'/stories',
-      'POST',
-      array(
-        'text' => $text,
-      ));
+      $this->makeAsanaAPICall(
+        $oauth_token,
+        'tasks/'.$parent_ref->getObjectID().'/stories',
+        'POST',
+        array(
+          'text' => $text,
+        ));
+    }
   }
 
   private function lookupAsanaUserIDs($all_phids) {