Summary:
Ref T5681. Ref T6860. This doesn't do anything interesting on its own, just makes the next diff smaller.
In the next diff, policies become aware of the types of objects they're acting on. We need to specify which object type all the "Default View/Edit" settings are for so they get the right rules.
For example, a rule like "Allow task author" is OK for "View Policy" on a task, and also OK for "Default View Policy" on ManiphestApplication. But it's not OK for "Can Create Tasks" on ManiphestApplication.
So annotate all the "template"/"default" policies with their types. The next diff will use these to let you select appropriate rules for the given object type.
Test Plan:
- Used `grep` to find these.
- This change has no effect.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5681, T6860
Differential Revision: https://secure.phabricator.com/D13251
Summary:
Ref T8498. Allow ApplicationEmail addresses to be put into spaces:
- You can only see and send to addresses in Spaces you have access to.
- Objects are created into the same space their address is associated with.
Test Plan:
- Used `bin/mail receive-test` to send mail to various `xyz-bugs@...` addresses.
- Saw objects created in the proper space.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8498
Differential Revision: https://secure.phabricator.com/D13247
Summary: Ref T8449. Get rid of the `S123` stuff in headers, it feels like clutter.
Test Plan: Looked at objects in Spaces.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8449
Differential Revision: https://secure.phabricator.com/D13243
Summary:
Ref T8377. This adds a standard disable/enable feature to Spaces, with a couple of twists:
- You can't create new stuff in an archived space, and you can't move stuff into an archived space.
- We don't show results from an archived space by default in ApplicationSearch queries. You can still find these objects if you explicitly search for "Spaces: <the archived space>".
So this is a "put it in a box in the attic" sort of operation, but that seems fairly nice/reasonable.
Test Plan:
- Archived and activated spaces.
- Used ApplicationSearch, which omitted archived objects by default but allowed searches for them, specifically, to succeed.
- Tried to create objects into an archived space (this is not allowed).
- Edited objects in an archived space (this is OK).
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8377
Differential Revision: https://secure.phabricator.com/D13238
Summary:
Ref T8449. Try out some more subtle behaviors:
- Make the "Space" control part of the policy control, so the UI shows "Visible To: [Space][Policy]". I think this helps make the role of spaces more clear. It also makes them easier to implement.
- Don't show the default space in headers: instead, show nothing.
- If the user has access to only one space, pretend spaces don't exist (no edit controls, no header stuff).
This might be confusing, but I think most of the time it will all align fairly well with user expectation.
Test Plan:
- Viewed a list of pastes (saw Space with non-default space, no space with default space, no space with user in only one space).
- Viewed a paste (saw Space with non-default space, saw no space with default space, saw no space with user in only one space).
- Edited spaces on objects (control as privileged user, no control as locked user).
- Created a new paste in a space (got space select as privileged user, no select as locked user).
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8449
Differential Revision: https://secure.phabricator.com/D13229
Summary: See D13185, the `testMethodVisibility()` test was not being executed.
Test Plan: `arc unit`
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D13186
Summary: Ref T8441. Does what it says, provided other conditions (like using the new SearchField stuff) are fulfilled.
Test Plan:
{F473836}
{F473837}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8441
Differential Revision: https://secure.phabricator.com/D13171
Summary:
Ref T8424. This adds crude integration with Paste's edit/view workflows: you can change the space a Paste appears in, see transactions, and get a policy callout.
Lots of rough edges and non-obviousness but it pretty much works.
Test Plan:
- Created and updated Pastes.
- Moved them between spaces, saw policy effects.
- Read transactions.
- Looked at feed.
- Faked query to return no spaces, saw control and other stuff vanish.
- Faked query to return no spaces, created pastes.
- Tried to submit bad values and got errors.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8424
Differential Revision: https://secure.phabricator.com/D13159
Summary:
Ref T8424. No UI or interesting behavior yet, but integrates Spaces checks:
- `PolicyFilter` now checks Spaces.
- `PolicyAwareQuery` now automatically adds Spaces constraints.
There's one interesting design decision here: **spaces are stronger than automatic capabilities**. That means that you can't see a task in a space you don't have permission to access, //even if you are the owner//.
I //think// this is desirable. Particularly, we need to do this in order to exclude objects at the query level, which potentially makes policy filtering for spaces hugely more efficient. I also like Spaces being very strong, conceptually.
It's possible that we might want to change this; this would reduce our access to optimizations but might be a little friendlier or make more sense to users later on.
For now, at least, I'm pursuing the more aggressive line. If we stick with this, we probably need to make some additional UI affordances (e.g., show when an owner can't see a task).
This also means that you get a hard 404 instead of a policy exception when you try to access something in a space you can't see. I'd slightly prefer to show you a policy exception instead, but think this is generally a reasonable tradeoff to get the high-performance filtering at the Query layer.
Test Plan:
- Added and executed unit tests.
- Put objects in spaces and viewed them with multiple users.
- Made the default space visible/invisible, viewed objects.
- Checked the services panel and saw `spacePHID` constraints.
- Verified that this adds only one query to each page.
Reviewers: btrahan, chad
Reviewed By: btrahan
Subscribers: chad, epriestley
Maniphest Tasks: T8424
Differential Revision: https://secure.phabricator.com/D13156
Summary:
Ref T8424. I'm using Paste as a testbed application because Spaces make some degree of sense for it but it's also flat/simple.
This doesn't do anything interesting or useful and mostly just making the next (more interesting) diff smaller.
Test Plan:
- Ran `bin/storage upgrade -f`.
- Browsed pastes.
- Created a paste.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8424
Differential Revision: https://secure.phabricator.com/D13154
Summary: Ref T8377. These were picked up by tests.
Test Plan: `arc unit --everything`
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T8377
Differential Revision: https://secure.phabricator.com/D13130
Summary: got some errors in my sandbox when I updated. this fixes them and I think is the right naming convention.
Test Plan: /settings/panel/emailpreferences/ started working again! /spaces/ also loaded unit tests passed without errors
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T8377
Differential Revision: https://secure.phabricator.com/D13103
