diff --git a/src/applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php b/src/applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php index 1b9750a100..d24b997b1f 100644 --- a/src/applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php +++ b/src/applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php @@ -9,14 +9,17 @@ final class DifferentialDoorkeeperRevisionFeedStoryPublisher public function isStoryAboutObjectCreation($object) { $story = $this->getFeedStory(); - $action = $story->getStoryData()->getValue('action'); - switch ($action) { - case DifferentialAction::ACTION_CREATE: - return true; - default: - return false; - } + + return ($action == DifferentialAction::ACTION_CREATE); + } + + public function isStoryAboutObjectClosure($object) { + $story = $this->getFeedStory(); + $action = $story->getStoryData()->getValue('action'); + + return ($action == DifferentialAction::ACTION_CLOSE) || + ($action == DifferentialAction::ACTION_ABANDON); } public function willPublishStory($object) { diff --git a/src/applications/diffusion/doorkeeper/DiffusionDoorkeeperCommitFeedStoryPublisher.php b/src/applications/diffusion/doorkeeper/DiffusionDoorkeeperCommitFeedStoryPublisher.php index 6ded4ad2f8..feae1628ad 100644 --- a/src/applications/diffusion/doorkeeper/DiffusionDoorkeeperCommitFeedStoryPublisher.php +++ b/src/applications/diffusion/doorkeeper/DiffusionDoorkeeperCommitFeedStoryPublisher.php @@ -22,6 +22,29 @@ final class DiffusionDoorkeeperCommitFeedStoryPublisher return false; } + public function isStoryAboutObjectClosure($object) { + // TODO: This isn't quite accurate, but pretty close: check if this story + // is a close (which clearly is about object closure) or is an "Accept" and + // the commit is fully audited (which is almost certainly a closure). + // 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) { + return true; + } + + 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 428a22be67..bd6809d87f 100644 --- a/src/applications/doorkeeper/engine/DoorkeeperFeedStoryPublisher.php +++ b/src/applications/doorkeeper/engine/DoorkeeperFeedStoryPublisher.php @@ -36,6 +36,7 @@ abstract class DoorkeeperFeedStoryPublisher { } abstract public function isStoryAboutObjectCreation($object); + abstract public function isStoryAboutObjectClosure($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 f23653a744..3f3cf7e209 100644 --- a/src/applications/doorkeeper/worker/DoorkeeperFeedWorkerAsana.php +++ b/src/applications/doorkeeper/worker/DoorkeeperFeedWorkerAsana.php @@ -478,8 +478,10 @@ final class DoorkeeperFeedWorkerAsana extends FeedPushWorker { // 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)) { + // purpose as the "create" story. Similarly, "close" stories generate a + // close notification. + if (!$publisher->isStoryAboutObjectCreation($object) && + !$publisher->isStoryAboutObjectClosure($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.