From d7c712a85585b81440a0b91ffc01f162f40c03e0 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 8 Aug 2013 12:01:48 -0700 Subject: [PATCH] Remove actor as a follower from unowned Asana subtasks after touching them Summary: Ref T2852. Asana adds the actor as a follower when they create a task, so subtasks currently have up to two followers (the actor and the reviewer) when they should have only one (the reviewer). Simply removing the actor is an effective remedy for this because unfollowing tasks occurs with sneaky ninja stealth in Asana and doesn't generate notifications or even transaction activity. Test Plan: Synchronized a revision without this patch, saw two followers on the subtask. Synchronized a revision after this patch, saw the "removeFollowers" fire and only one follower on the subtask, with no record of the removal in notifications or the transaction log. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T2852 Differential Revision: https://secure.phabricator.com/D6700 --- .../worker/DoorkeeperFeedWorkerAsana.php | 33 +++++++++++++++---- 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/src/applications/doorkeeper/worker/DoorkeeperFeedWorkerAsana.php b/src/applications/doorkeeper/worker/DoorkeeperFeedWorkerAsana.php index 3f3cf7e209..49166b43a8 100644 --- a/src/applications/doorkeeper/worker/DoorkeeperFeedWorkerAsana.php +++ b/src/applications/doorkeeper/worker/DoorkeeperFeedWorkerAsana.php @@ -194,8 +194,9 @@ final class DoorkeeperFeedWorkerAsana extends FeedPushWorker { array_keys($phid_aid_map)); $try_users = array_filter($try_users); - list($possessed_user, $oauth_token) = $this->findAnyValidAsanaAccessToken( - $try_users); + $access_info = $this->findAnyValidAsanaAccessToken($try_users); + list($possessed_user, $possessed_asana_id, $oauth_token) = $access_info; + if (!$oauth_token) { throw new PhabricatorWorkerPermanentFailureException( 'Unable to find any Asana user with valid credentials to '. @@ -425,7 +426,6 @@ final class DoorkeeperFeedWorkerAsana extends FeedPushWorker { } // For each user that we don't have a subtask for, create a new subtask. - foreach ($need_subtasks as $user_phid => $is_completed) { $subtask = $this->makeAsanaAPICall( $oauth_token, @@ -472,6 +472,27 @@ final class DoorkeeperFeedWorkerAsana extends FeedPushWorker { )); } + foreach ($user_to_ref_map as $user_phid => $ref) { + // For each subtask, if the acting user isn't the same user as the subtask + // owner, remove the acting user as a follower. Currently, the acting user + // will be added as a follower only when they create the task, but this + // may change in the future (e.g., closing the task may also mark them + // as a follower). Wipe every subtask to be sure. The intent here is to + // leave only the owner as a follower so that the acting user doesn't + // receive notifications about changes to subtask state. Note that + // removing followers is silent in all cases in Asana and never produces + // any kind of notification, so this isn't self-defeating. + if ($user_phid != $possessed_user->getPHID()) { + $this->makeAsanaAPICall( + $oauth_token, + 'tasks/'.$ref->getObjectID().'/removeFollowers', + 'POST', + array( + 'followers' => array($possessed_asana_id), + )); + } + } + // Update edges on our side. $sub_editor->save(); @@ -527,7 +548,7 @@ final class DoorkeeperFeedWorkerAsana extends FeedPushWorker { private function findAnyValidAsanaAccessToken(array $user_phids) { if (!$user_phids) { - return array(null, null); + return array(null, null, null); } $provider = $this->getProvider(); @@ -572,11 +593,11 @@ final class DoorkeeperFeedWorkerAsana extends FeedPushWorker { ->withPHIDs(array($account->getUserPHID())) ->executeOne(); if ($user) { - return array($user, $token); + return array($user, $account->getAccountID(), $token); } } - return array(null, null); + return array(null, null, null); } private function makeAsanaAPICall($token, $action, $method, array $params) {