WIP This adds default capability to the Files application
Details
- Reviewers
epriestley btrahan - Maniphest Tasks
- T6564: Set default permissions for file uploads to same as File application
- Commits
- Restricted Diffusion Commit
rP1a8d87699c15: Add default policy to Files application
Set default to public, go to Files page, see public preset. Upload File. Doesn't work.
Diff Detail
- Repository
- rP Phabricator
- Branch
- files-policy
- Lint
Lint Errors Severity Location Code Message Error src/applications/files/storage/PhabricatorFile.php:279 XHP5 Use of Undeclared Variable - Unit
Test Failures - Build Status
Buildable 3115 Build 3121: [Placeholder Plan] Wait for 30 Seconds
Time | Test | |
---|---|---|
0 ms | testFileStorageDelete | |
0 ms | testFileStorageDeleteSharedHandle | |
0 ms | testFileStorageReadWrite | |
0 ms | testFileStorageUploadDifferentFiles | |
0 ms | testFileStorageUploadSameFile | |
View Full Test Results (8 Failed · 5 Passed) |
Event Timeline
src/applications/files/storage/PhabricatorFile.php | ||
---|---|---|
279 | This is my main question. Setting $viewer here means I'd have to update a dozen callsites and pass $viewer everywhere. Is that the correctest way? newFromFileData is the main function that gets called everywhere. |
We can pull the application without needing the viewer like this, using PhabricatorUser::getOmnipotentUser():
$app = id(new PhabricatorApplicationQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) ->withClasses(array('PhabricatorFilesApplication')) ->executeOne();
It's slightly "better" to use a real user (and for many other applications there's some other reason we need a real user -- for example, to populate an author field), but there's no practical difference in this case. The big thing that using a real user does is make this fail in an obvious way if you messed something up and the user doesn't actually have access to use the application. However, Files is a required application and can not be uninstalled or have access limited, so it's moot in this case.
There are also a lot of cases where we create a file but don't necessarily have a meaningful viewer (for example, when pulling profile pictures from Facebook or Google during registration), so you wouldn't really be able to provide one there anyway.
So:
- You can just use PhabricatorUser::getOmnipotentUser() to load the $app.
- Maybe we'll refine this eventually and optionally let you pass a viewer in (after T5681?).
- You don't need to update every callsite, just the user-facing ones. Some of them are really deep in non-user-facing workflows and can get cleaned up eventually.
- Rest of this diff looks good.