From 64e4b3aef449942618a2052513749323f515fd56 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 6 Oct 2013 17:07:08 -0700 Subject: [PATCH] Remove loadMemberPHIDs from PhabricatorProject Summary: Ref T603. Move toward stamping out all the Project / ProjectProfile query irregularities with respect to policies. - Fixes a bug with Asana publishing when the remote task is deleted. - Fixes an issue with Herald commit rules. Test Plan: - Viewed projects; - edited projects; - added and removed members from projects; - republished Asana-bridged feed stories about commits. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T603 Differential Revision: https://secure.phabricator.com/D7251 --- .../DiffusionDoorkeeperCommitFeedStoryPublisher.php | 7 ++++++- .../doorkeeper/worker/DoorkeeperFeedWorkerAsana.php | 8 ++++++-- src/applications/herald/adapter/HeraldCommitAdapter.php | 2 ++ .../PhabricatorProjectMembersEditController.php | 3 ++- .../project/editor/PhabricatorProjectEditor.php | 3 +-- src/applications/project/storage/PhabricatorProject.php | 9 --------- 6 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/applications/diffusion/doorkeeper/DiffusionDoorkeeperCommitFeedStoryPublisher.php b/src/applications/diffusion/doorkeeper/DiffusionDoorkeeperCommitFeedStoryPublisher.php index 35fdbce886..75be3898cc 100644 --- a/src/applications/diffusion/doorkeeper/DiffusionDoorkeeperCommitFeedStoryPublisher.php +++ b/src/applications/diffusion/doorkeeper/DiffusionDoorkeeperCommitFeedStoryPublisher.php @@ -83,7 +83,12 @@ final class DiffusionDoorkeeperCommitFeedStoryPublisher $request_phids = PhabricatorOwnersOwner::loadAffiliatedUserPHIDs( array($object->getID())); } else if ($object instanceof PhabricatorProject) { - $request_phids = $object->loadMemberPHIDs(); + $project = id(new PhabricatorProjectQuery()) + ->setViewer($this->getViewer()) + ->withIDs(array($object->getID())) + ->needMembers(true) + ->executeOne(); + $request_phids = $project->getMemberPHIDs(); } else { // Dunno what this is. $request_phids = array(); diff --git a/src/applications/doorkeeper/worker/DoorkeeperFeedWorkerAsana.php b/src/applications/doorkeeper/worker/DoorkeeperFeedWorkerAsana.php index 80df37905a..2a7aeb0aa8 100644 --- a/src/applications/doorkeeper/worker/DoorkeeperFeedWorkerAsana.php +++ b/src/applications/doorkeeper/worker/DoorkeeperFeedWorkerAsana.php @@ -209,8 +209,12 @@ final class DoorkeeperFeedWorkerAsana extends DoorkeeperFeedWorker { // Add the silent followers first so that a user who is both a reviewer and // a CC gets silently added and then implicitly skipped by then noisy add. // They will get a subtask notification. - $this->addFollowers($oauth_token, $task_id, $silent_followers, true); - $this->addFollowers($oauth_token, $task_id, $noisy_followers); + + // We only do this if the task still exists. + if (empty($extra_data['gone'])) { + $this->addFollowers($oauth_token, $task_id, $silent_followers, true); + $this->addFollowers($oauth_token, $task_id, $noisy_followers); + } $dst_phid = $parent_ref->getExternalObject()->getPHID(); diff --git a/src/applications/herald/adapter/HeraldCommitAdapter.php b/src/applications/herald/adapter/HeraldCommitAdapter.php index fe4424e457..c1199c9f2c 100644 --- a/src/applications/herald/adapter/HeraldCommitAdapter.php +++ b/src/applications/herald/adapter/HeraldCommitAdapter.php @@ -150,6 +150,8 @@ final class HeraldCommitAdapter extends HeraldAdapter { $object = new HeraldCommitAdapter(); + $commit->attachRepository($repository); + $object->repository = $repository; $object->commit = $commit; $object->commitData = $commit_data; diff --git a/src/applications/project/controller/PhabricatorProjectMembersEditController.php b/src/applications/project/controller/PhabricatorProjectMembersEditController.php index d5ae0b064e..98067f435c 100644 --- a/src/applications/project/controller/PhabricatorProjectMembersEditController.php +++ b/src/applications/project/controller/PhabricatorProjectMembersEditController.php @@ -16,6 +16,7 @@ final class PhabricatorProjectMembersEditController $project = id(new PhabricatorProjectQuery()) ->setViewer($user) ->withIDs(array($this->id)) + ->needMembers(true) ->requireCapabilities( array( PhabricatorPolicyCapability::CAN_VIEW, @@ -30,7 +31,7 @@ final class PhabricatorProjectMembersEditController $profile = new PhabricatorProjectProfile(); } - $member_phids = $project->loadMemberPHIDs(); + $member_phids = $project->getMemberPHIDs(); $errors = array(); if ($request->isFormPost()) { diff --git a/src/applications/project/editor/PhabricatorProjectEditor.php b/src/applications/project/editor/PhabricatorProjectEditor.php index 0e45ba7151..f3b3f2a5a9 100644 --- a/src/applications/project/editor/PhabricatorProjectEditor.php +++ b/src/applications/project/editor/PhabricatorProjectEditor.php @@ -213,8 +213,7 @@ final class PhabricatorProjectEditor extends PhabricatorEditor { $xaction->setOldValue($project->getStatus()); break; case PhabricatorProjectTransactionType::TYPE_MEMBERS: - $member_phids = $project->loadMemberPHIDs(); - $project->attachMemberPHIDs($member_phids); + $member_phids = $project->getMemberPHIDs(); $old_value = array_values($member_phids); $xaction->setOldValue($old_value); diff --git a/src/applications/project/storage/PhabricatorProject.php b/src/applications/project/storage/PhabricatorProject.php index 40074e801d..78cebb50cf 100644 --- a/src/applications/project/storage/PhabricatorProject.php +++ b/src/applications/project/storage/PhabricatorProject.php @@ -112,15 +112,6 @@ final class PhabricatorProject extends PhabricatorProjectDAO return $this->assertAttached($this->memberPHIDs); } - public function loadMemberPHIDs() { - if (!$this->getPHID()) { - return array(); - } - return PhabricatorEdgeQuery::loadDestinationPHIDs( - $this->getPHID(), - PhabricatorEdgeConfig::TYPE_PROJ_MEMBER); - } - public function setPhrictionSlug($slug) { // NOTE: We're doing a little magic here and stripping out '/' so that