From 2c626f72a9efc40cfac4455d6e0ea604e4793ac9 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 4 Jun 2014 15:17:19 -0700 Subject: [PATCH] Fix unsubscribing from projects in a gross, hacky way Summary: Fixes T5261. This fix isn't very good. Two better fixes would be: # Add some sort of `setRole(SUBSCRIPTIONS)` method to `ObjectQuery`, which gets passed down until it reaches `ProjectQuery`, and `ProjectQuery` knows that it needs to load more data. This feels OK, but is a very general approach and I don't think we have many/any other use cases right now. I //think// this is the right way in the long run, but I'd like to have more use cases in mind before implementing it. # Add some sort of `loadAllTheSubscriptionStuffYouNeed()` method to `PhabricatorSubscribableInterface`. This feels OK-ish too, but kind of yuck, and doesn't lend itself to proper batching, and is silly if we do the above instead, which I think we probably will. For now, just fix the issue without committing to an infrastructure direction. I think (1) is the right way to go eventually, but I want a better second use case before writing it, since I might be crazy. Test Plan: Unsubscribed from a project. Reviewers: joshuaspence Reviewed By: joshuaspence Subscribers: epriestley Maniphest Tasks: T5261 Differential Revision: https://secure.phabricator.com/D9377 --- ...PhabricatorSubscriptionsEditController.php | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php b/src/applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php index c65a4b1a39..10c8cbe47a 100644 --- a/src/applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php +++ b/src/applications/subscriptions/controller/PhabricatorSubscriptionsEditController.php @@ -37,10 +37,24 @@ final class PhabricatorSubscriptionsEditController ->withPHIDs(array($phid)) ->executeOne(); - $object = id(new PhabricatorObjectQuery()) - ->setViewer($user) - ->withPHIDs(array($phid)) - ->executeOne(); + if (phid_get_type($phid) == PhabricatorProjectPHIDTypeProject::TYPECONST) { + // TODO: This is a big hack, but a weak argument for adding some kind + // of "load for role" feature to ObjectQuery, and also not a really great + // argument for adding some kind of "load extra stuff" feature to + // SubscriberInterface. Do this for now and wait for the best way forward + // to become more clear? + + $object = id(new PhabricatorProjectQuery()) + ->setViewer($user) + ->withPHIDs(array($phid)) + ->needWatchers(true) + ->executeOne(); + } else { + $object = id(new PhabricatorObjectQuery()) + ->setViewer($user) + ->withPHIDs(array($phid)) + ->executeOne(); + } if (!($object instanceof PhabricatorSubscribableInterface)) { return $this->buildErrorResponse(