Page MenuHomePhabricator

Support Spaces in Pholio

Authored by epriestley on Jun 9 2015, 11:56 PM.


Maniphest Tasks
T8493: Integrate Spaces into more applications
Restricted Diffusion Commit
rPde0e0d995baa: Support Spaces in Pholio

Ref T8493. Add Spaces support to Pholio.

This is straightforward; Pholio has no clone/copy/fork or weird parent/child stuff going on.

Test Plan

Created a mock, put it in a space, looked at it as another user, searched for stuff in spaces, viewed Macros.

Diff Detail

rP Phabricator
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley updated this revision to Diff 31979.Jun 9 2015, 11:56 PM
epriestley retitled this revision from to Support Spaces in Pholio.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
btrahan accepted this revision.Jun 10 2015, 10:42 PM
btrahan edited edge metadata.

Just double checking the images that comprise a mock are also "space'd" properly by this change.


do these images end up being in the space as well?


user makes a mock with N images
user moves the mock to a space

...the N images are also in the space with respect to viewability?

This revision is now accepted and ready to land.Jun 10 2015, 10:42 PM

the N images are also in the space with respect to viewability

Yeah, we generally get this automatically: when you load an image, we also load and attach the mock. If you can't see the mock, we filter out the image. So moving a mock to a space you can't see is sufficient to hide the image, as well, for the same reason that setting the mock to "visible to: only me" is also sufficient to hide the image.

(If some future diff let you interact directly with images that a different viewer might have loaded [for example, we let you write Herald rules directly against Images or something], we'd need to implement PhabricatorExtendedPolicyInterface to avoid T7703, but there's no way to hit that currently and I don't anticipate adding one any time soon / ever.)

This revision was automatically updated to reflect the committed changes.