Page MenuHomePhabricator

Fix unsubscribing from projects in a gross, hacky way
ClosedPublic

Authored by epriestley on Jun 4 2014, 9:05 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Dec 7, 9:10 PM
Unknown Object (File)
Sat, Dec 7, 4:41 PM
Unknown Object (File)
Thu, Dec 5, 12:35 AM
Unknown Object (File)
Mon, Dec 2, 9:53 AM
Unknown Object (File)
Fri, Nov 22, 6:47 AM
Unknown Object (File)
Nov 19 2024, 1:29 AM
Unknown Object (File)
Nov 8 2024, 4:20 PM
Unknown Object (File)
Nov 3 2024, 5:44 PM
Subscribers

Details

Summary

Fixes T5261.

This fix isn't very good. Two better fixes would be:

  1. 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.
  2. 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.

Diff Detail

Repository
rP Phabricator
Branch
subscribers
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 850
Build 850: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

epriestley retitled this revision from to Fix unsubscribing from projects in a gross, hacky way.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: joshuaspence.
joshuaspence edited edge metadata.

Yeah, this fixes my issue.

This revision is now accepted and ready to land.Jun 4 2014, 9:55 PM
epriestley updated this revision to Diff 22351.

Closed by commit rP2c626f72a9ef (authored by @epriestley).