diff --git a/app/Modules/Files/Http/Controllers/MyFilesController.php b/app/Modules/Files/Http/Controllers/MyFilesController.php index 78c87b37..2c30f42d 100644 --- a/app/Modules/Files/Http/Controllers/MyFilesController.php +++ b/app/Modules/Files/Http/Controllers/MyFilesController.php @@ -446,6 +446,10 @@ class MyFilesController extends Controller 'categories' => $file->categories->pluck('id')->all(), ], 'can_delete' => Gate::forUser($client)->allows('delete', $file), + // Their own root, where the installation gives them one. The + // form offers no "No folder" beside it: there is no such place + // for this client, and update() resolves it here anyway. + 'home_folder_id' => $this->homeFolders->for($client)?->id, 'can_publish' => $client->can('upload_public'), // The public links on this file, and where to make and revoke // one — the same shape the staff screen uses. A file marked @@ -544,6 +548,15 @@ class MyFilesController extends Controller $folderId = isset($validated['folder_id']) ? (int) $validated['folder_id'] : null; + // "No folder" means the top of what this client sees, which on an + // installation that gives them a home folder is inside it — not the + // root of the library, beside the staff folders. Uploading and + // creating a folder already resolve it this way; the editor did + // not, so a client could move their own file out of their home and + // into the administrator's root by choosing "No folder" (reported + // by binghuo). + $folderId ??= $this->homeFolders->for($client)?->id; + // The client rule, not the staff one: somewhere they could have // uploaded it in the first place. Same check the upload path makes, // so moving a file cannot reach a folder that uploading it could diff --git a/resources/js/pages/portal/edit-file.tsx b/resources/js/pages/portal/edit-file.tsx index 95e9209a..068d1795 100644 --- a/resources/js/pages/portal/edit-file.tsx +++ b/resources/js/pages/portal/edit-file.tsx @@ -51,6 +51,8 @@ interface PortalEditFileProps { }; can_delete: boolean; can_publish: boolean; + /** This client's own root, where the installation gives them one. Null otherwise. */ + home_folder_id: number | null; /** The public links on this file, newest first, and where to make or revoke one. */ share_links: PortalShareLink[]; share_link_store_url: string; @@ -80,6 +82,7 @@ export default function PortalEditFile({ file, can_delete, can_publish, + home_folder_id, share_links, share_link_store_url, can_set_expiration, @@ -95,7 +98,9 @@ export default function PortalEditFile({ const form = useForm({ name: file.name, description: file.description ?? '', - folder_id: file.folder_id === null ? 'root' : String(file.folder_id), + // A file with no folder belongs in this client's own root where + // there is one — which is also what the server does with it. + folder_id: file.folder_id === null ? (home_folder_id === null ? 'root' : String(home_folder_id)) : String(file.folder_id), public: file.public, commentable: file.commentable, expires_at: file.expires_at ?? '', @@ -170,7 +175,10 @@ export default function PortalEditFile({ - {t('No folder')} + {/* Not offered where this client has a home + folder: "no folder" would mean the root + of the library, which is not theirs. */} + {home_folder_id === null && {t('No folder')}} {folders.map((folder) => ( diff --git a/tests/Feature/Files/ClientHomeFoldersTest.php b/tests/Feature/Files/ClientHomeFoldersTest.php index 3a761bdc..c80d657b 100644 --- a/tests/Feature/Files/ClientHomeFoldersTest.php +++ b/tests/Feature/Files/ClientHomeFoldersTest.php @@ -247,3 +247,61 @@ test('turning the setting off leaves an existing home working', function () { // strand them somewhere no listing looks. expect(app(ClientHomeFolders::class)->for($client->refresh())->id)->toBe($home->id); }); + +test('a client editing their own file cannot send it to the library root', function () { + // Reported by binghuo: choosing "No folder" in the portal's file editor + // moved the file out of the client's home and into the administrator's + // root, beside the staff folders — the exact mess the home folder + // exists to end. Uploading and creating a folder already resolved an + // absent folder to the home; the editor did not. + enableHomeFolders(); + $client = clientWhoCanUpload(); + RolePermission::query()->create(['role_id' => $client->role_id, 'permission' => Permission::EditFiles->value]); + $home = app(ClientHomeFolders::class)->for($client); + + $file = File::factory()->create(['uploaded_by' => $client->id, 'folder_id' => $home->id, 'name' => 'Theirs']); + + $this->actingAs($client)->patch("/my-files/{$file->id}", [ + 'name' => 'Theirs', + 'folder_id' => null, + ])->assertSessionHasNoErrors(); + + expect($file->refresh()->folder_id)->toBe($home->id); +}); + +test('with no home folder, no folder still means no folder', function () { + // The installation that never turned this on keeps what it had: a + // client's file sits at the root of the library because that is where + // every client's file sits there. + app(Settings::class)->set(Setting::ClientsHomeFolders, false); + $client = clientWhoCanUpload(); + RolePermission::query()->create(['role_id' => $client->role_id, 'permission' => Permission::EditFiles->value]); + $folder = app(FolderService::class)->create('Somewhere', null); + shareFolderWithClient($folder, $client); + + $file = File::factory()->create(['uploaded_by' => $client->id, 'folder_id' => $folder->id]); + + $this->actingAs($client)->patch("/my-files/{$file->id}", [ + 'name' => 'Theirs', + 'folder_id' => null, + ])->assertSessionHasNoErrors(); + + expect($file->refresh()->folder_id)->toBeNull(); +}); + +test('the editor offers no "no folder" where the client has a home', function () { + enableHomeFolders(); + $client = clientWhoCanUpload(); + RolePermission::query()->create(['role_id' => $client->role_id, 'permission' => Permission::EditFiles->value]); + $home = app(ClientHomeFolders::class)->for($client); + + $file = File::factory()->create(['uploaded_by' => $client->id, 'folder_id' => null]); + + $this->actingAs($client)->get("/my-files/{$file->id}/edit")->assertInertia( + fn (AssertableInertia $page) => $page + ->component('portal/edit-file') + // The page hides the option and preselects this, so the form + // cannot post the root even by accident. + ->where('home_folder_id', $home->id), + ); +});