From 616a355d5471fdc9f4dec976ab87ff271d0fd537 Mon Sep 17 00:00:00 2001 From: ignacionelson Date: Thu, 17 Sep 2026 00:22:29 -0300 Subject: [PATCH] Give each client a folder of their own, standing in for the root A client who may create folders creates them at the top of the library, beside the ones staff made, and their uploads land at the root too. An administrator opening /files gets one flat pile with nothing saying which parts belong to whom. With the new "Give each client a folder of their own" setting, every new client gets a folder named after them and it acts as their root: what they upload and any folder they create goes inside it. /files becomes a list of clients rather than a pile. The sentence this feature has to keep true: **the home is a default location, not a boundary.** Folder::scopeVisibleToClient is untouched, so a folder staff shared with a client still reaches them and sits beside their own. Making the home a jail would have silently revoked every share that already exists -- a data-access change wearing the clothes of a tidying-up feature. There is a test named after that rule. What the client sees is the *inside* of their folder, not a folder wearing their own name, which is not information to them. The breadcrumb is trimmed of it for the same reason: "Invoices", not "Acme Ltd / Invoices". Some decisions worth naming: - **A column, not a convention.** `folders.home_for_user_id`, unique. Matching on the name breaks the moment two clients share one, and `created_by` plus a null parent catches every root folder a client ever made themselves. The question is asked on each upload and each portal listing and the answer has to be exact. - **created_by is the client**, because that is how scopeVisibleToClient already grants somebody their own folder -- no assignment row to keep in step with it. That is also why this writes the row rather than calling FolderService::create(), which takes created_by from auth()->id(). - **On model events**, not in the services that make and rename clients. There are nine of those (ClientAccounts, ClientProvisioning, the profile screen, two update endpoints, AccountConversion, invitations, LDAP, social) and a rule repeated in nine places is missing from the tenth. - **Turning the setting on creates nothing.** Existing clients get a folder when an administrator presses a button that says how many are waiting, and it reports created/total/already-had afterwards. Somebody should be able to switch this on, look, and switch it off without having reorganised a library. It moves no files either. - **Nobody deletes a home from a folder screen**, staff included, and the client cannot rename theirs -- they own it, so ownership alone would have let them, and its name follows the account anyway. - **The name always follows the client**, over a hand-typed one. A folder still called "Acme Ltd" under an account now called something else misleads the administrator the feature exists for. Verified in a real browser as well as in tests: the screen mounts, the panel reads "24 of your existing clients have no folder yet", and pressing the button answers "24 of 24 clients got a folder. 0 already had one." --- .../Controllers/ClientSettingsController.php | 50 ++++ app/Modules/Files/FilesServiceProvider.php | 43 +++ app/Modules/Files/FolderPolicy.php | 21 ++ .../Files/Folders/ClientHomeFolders.php | 210 +++++++++++++++ .../Controllers/ChunkedUploadsController.php | 15 ++ .../Http/Controllers/MyFilesController.php | 63 ++++- .../Http/Controllers/MyFoldersController.php | 9 + app/Modules/Files/Models/Folder.php | 6 + app/Modules/Platform/Settings/Setting.php | 21 ++ ..._add_home_for_user_id_to_folders_table.php | 47 ++++ .../js/pages/system/settings/clients.tsx | 59 +++++ routes/settings.php | 5 + .../Feature/Clients/ClientsManagementTest.php | 1 + tests/Feature/Files/ClientHomeFoldersTest.php | 249 ++++++++++++++++++ .../Feature/Groups/MembershipRequestsTest.php | 1 + 15 files changed, 797 insertions(+), 3 deletions(-) create mode 100644 app/Modules/Files/Folders/ClientHomeFolders.php create mode 100644 database/migrations/2026_09_17_090000_add_home_for_user_id_to_folders_table.php create mode 100644 tests/Feature/Files/ClientHomeFoldersTest.php diff --git a/app/Modules/Clients/Http/Controllers/ClientSettingsController.php b/app/Modules/Clients/Http/Controllers/ClientSettingsController.php index 17a651a2..843c9729 100644 --- a/app/Modules/Clients/Http/Controllers/ClientSettingsController.php +++ b/app/Modules/Clients/Http/Controllers/ClientSettingsController.php @@ -7,6 +7,7 @@ namespace App\Modules\Clients\Http\Controllers; use App\Http\Controllers\Controller; use App\Modules\Audit\Action; use App\Modules\Audit\ActivityLogger; +use App\Modules\Files\Folders\ClientHomeFolders; use App\Modules\Groups\Models\Group; use App\Modules\Platform\Settings\Setting; use App\Modules\Platform\Settings\Settings; @@ -21,6 +22,7 @@ class ClientSettingsController extends Controller public function __construct( private readonly Settings $settings, private readonly ActivityLogger $activity, + private readonly ClientHomeFolders $homeFolders, ) {} public function edit(): Response @@ -34,6 +36,11 @@ class ClientSettingsController extends Controller 'client_invitation_expiry_hours' => $this->settings->get(Setting::ClientInvitationExpiryHours), 'default_client_storage_quota_mb' => (int) $this->settings->get(Setting::DefaultClientStorageQuotaMb), 'clients_can_preview_files' => $this->settings->get(Setting::ClientsCanPreviewFiles), + 'clients_home_folders' => $this->settings->get(Setting::ClientsHomeFolders), + // What the button beside the switch would actually do, so it can + // say "3 clients have no folder yet" instead of asking somebody + // to press it and find out. + 'clients_without_home' => $this->homeFolders->pendingCount(), 'groups' => Group::query()->orderBy('name')->get() ->map(fn (Group $group): array => ['id' => $group->id, 'name' => $group->name]) ->all(), @@ -51,6 +58,7 @@ class ClientSettingsController extends Controller 'client_invitation_expiry_hours' => ['required', 'integer', 'min:1', 'max:720'], 'default_client_storage_quota_mb' => ['required', 'integer', 'min:0'], 'clients_can_preview_files' => ['required', 'boolean'], + 'clients_home_folders' => ['required', 'boolean'], ]); $this->settings->set(Setting::ClientsCanRegister, $validated['clients_can_register']); @@ -61,9 +69,51 @@ class ClientSettingsController extends Controller $this->settings->set(Setting::ClientInvitationExpiryHours, (int) $validated['client_invitation_expiry_hours']); $this->settings->set(Setting::DefaultClientStorageQuotaMb, (int) $validated['default_client_storage_quota_mb']); $this->settings->set(Setting::ClientsCanPreviewFiles, $validated['clients_can_preview_files']); + // Saving the switch deliberately creates nothing. Existing clients + // get a folder when somebody presses the button, so that turning + // this on, looking at it, and turning it off again leaves the + // library exactly as it was. + $this->settings->set(Setting::ClientsHomeFolders, $validated['clients_home_folders']); $this->activity->log(Action::SettingsUpdated, context: ['section' => 'clients']); return back(); } + + /** + * Create the missing home folders, on request. + * + * Its own endpoint rather than part of saving the form, because it is a + * different kind of act: the form records a preference, this one writes + * a folder for every client on the installation. Wrapping the second + * inside the first would mean an administrator could not try the + * setting without committing to it. + */ + public function backfillHomeFolders(Request $request): RedirectResponse + { + abort_unless($this->homeFolders->enabled(), 403, 'Client folders are switched off.'); + + $result = $this->homeFolders->backfill(); + + $this->activity->log(Action::SettingsUpdated, context: [ + 'section' => 'clients', + 'action' => 'client_home_folders_backfill', + 'created' => $result['created'], + 'total' => $result['total'], + ]); + + // Counts rather than "Done": on an installation with hundreds of + // clients the administrator wants to know how many there were and + // how many are new, and the difference between "created 200" and + // "created 0, they already had one" is the whole answer. + return back()->with('success', trans_choice( + '{0}Every client already had a folder.|[1,*]:created of :total clients got a folder. :existing already had one.', + $result['created'], + [ + 'created' => (string) $result['created'], + 'total' => (string) $result['total'], + 'existing' => (string) $result['existing'], + ], + )); + } } diff --git a/app/Modules/Files/FilesServiceProvider.php b/app/Modules/Files/FilesServiceProvider.php index 4e1e0333..767fb72d 100644 --- a/app/Modules/Files/FilesServiceProvider.php +++ b/app/Modules/Files/FilesServiceProvider.php @@ -5,7 +5,9 @@ declare(strict_types=1); namespace App\Modules\Files; use App\Modules\Files\Access\ClientIdentityScope; +use App\Models\User; use App\Modules\Files\Events\FileBecameAvailable; +use App\Modules\Files\Folders\ClientHomeFolders; use App\Modules\Files\Events\FileWasStored; use App\Modules\Files\Listeners\AnnounceAvailableFile; use App\Modules\Files\Access\StaffLibraryScope; @@ -62,6 +64,8 @@ class FilesServiceProvider extends ServiceProvider Gate::policy(File::class, FilePolicy::class); Gate::policy(Folder::class, FolderPolicy::class); + $this->keepClientHomeFolders(); + // Cached renditions are written once and never revisited, so // whoever changes how they render has to say so — otherwise the // change is invisible on every file anyone has already looked at. @@ -166,4 +170,43 @@ class FilesServiceProvider extends ServiceProvider ]); } } + + /** + * Give a new client their home folder, and keep its name in step. + * + * On model events rather than in the handful of services that create + * and rename clients, because there are more of those than anyone + * remembers: ClientAccounts for the staff screens, the API and the + * control plane; ClientProvisioning for self-registration, LDAP, + * social sign-in and invitation redemption; the profile screen and two + * update endpoints for a rename; AccountConversion for a staff member + * becoming a client. A rule that had to be repeated in nine places + * would be missing from the tenth. + * + * Here rather than in User::booted() so the identity model does not + * have to know the files module exists -- the dependency points one + * way, and this is the end that cares. + * + * Both listeners are cheap when the feature is off: `created` asks the + * setting and returns, and `updated` asks whether the name actually + * changed before it asks anything else. + */ + private function keepClientHomeFolders(): void + { + User::created(function (User $user): void { + if ($user->isClient()) { + $this->app->make(ClientHomeFolders::class)->ensureFor($user); + } + }); + + User::updated(function (User $user): void { + // wasChanged, not isDirty: by `updated` the write has happened + // and isDirty is empty. A save that did not touch the name -- + // which is most of them, every sign-in timestamp included -- + // costs one array lookup and stops here. + if ($user->isClient() && $user->wasChanged('name')) { + $this->app->make(ClientHomeFolders::class)->syncName($user); + } + }); + } } diff --git a/app/Modules/Files/FolderPolicy.php b/app/Modules/Files/FolderPolicy.php index abefd40f..aefb7490 100644 --- a/app/Modules/Files/FolderPolicy.php +++ b/app/Modules/Files/FolderPolicy.php @@ -38,6 +38,17 @@ class FolderPolicy public function update(User $user, Folder $folder): bool { if (! $user->isStaff()) { + // A client owns their home folder -- created_by is them, which + // is how they can see it at all -- so ownership alone would let + // them rename it. It is structure rather than something of + // theirs to arrange: its name follows the account, and the + // administrator reading /files relies on that. Renaming it is + // refused rather than allowed and then silently overwritten the + // next time the account is edited. + if ($folder->isHome()) { + return false; + } + return $folder->isOwnedBy($user) && $user->can('create_own_folders'); } @@ -48,6 +59,16 @@ class FolderPolicy public function delete(User $user, Folder $folder): bool { + // Nobody deletes a home folder from a folder screen, staff + // included. Deleting one cascades over everything the client has, + // and it would leave their portal pointing at a folder that is not + // there -- an account still gets erased through the erasure flow, + // which is where destroying somebody's content is the declared + // intent rather than a side effect of tidying a tree. + if ($folder->isHome()) { + return false; + } + if (! $user->isStaff()) { return $folder->isOwnedBy($user) && $user->can('create_own_folders'); } diff --git a/app/Modules/Files/Folders/ClientHomeFolders.php b/app/Modules/Files/Folders/ClientHomeFolders.php new file mode 100644 index 00000000..08adfcf6 --- /dev/null +++ b/app/Modules/Files/Folders/ClientHomeFolders.php @@ -0,0 +1,210 @@ +id()` — the creator here is whoever pressed a + * button, and the owner has to be the client. + */ +class ClientHomeFolders +{ + public function __construct( + private readonly Settings $settings, + ) {} + + public function enabled(): bool + { + return (bool) $this->settings->get(Setting::ClientsHomeFolders); + } + + /** + * This client's home, or null if they have none. + * + * Asked of the column and not of the setting: a home that exists keeps + * working after the switch is turned off again. The folder is real, + * it holds real files, and pretending it is not there would strand + * them somewhere no listing looks. + */ + public function for(?User $client): ?Folder + { + if ($client === null || ! $client->isClient()) { + return null; + } + + return Folder::query()->where('home_for_user_id', $client->id)->first(); + } + + /** + * Give this client a home if the installation wants them to have one. + * + * Idempotent, and safe to call on a client who already has one. Returns + * the folder either way, or null when the feature is off. + */ + public function ensureFor(User $client): ?Folder + { + if (! $client->isClient() || ! $this->enabled()) { + return null; + } + + return $this->create($client); + } + + /** + * Create the row, or hand back the one that is already there. + * + * The unique index on home_for_user_id is what actually guarantees one + * home per client; this check only avoids raising on the ordinary + * second call. Two administrators pressing the backfill button at the + * same moment is exactly the race the index is there for. + */ + private function create(User $client): Folder + { + return DB::transaction(function () use ($client): Folder { + $existing = $this->for($client); + + if ($existing !== null) { + return $existing; + } + + return Folder::query()->create([ + 'name' => $this->nameFor($client), + 'parent_id' => null, + // Root, so an administrator sees it at the top of /files -- + // which is the whole point of the feature. + 'path' => '/', + 'created_by' => $client->id, + 'home_for_user_id' => $client->id, + ]); + }); + } + + /** + * Keep the folder's name in step with the client's. + * + * Always, including over a name somebody typed by hand. That was the + * product decision (2026-09-17) and it is the defensible one: the + * folder exists to say whose things these are, so a folder still + * called "Acme Ltd" after the account became "Acme Holdings" is + * actively misleading to the administrator the feature is for. A + * client cannot rename it anyway -- see FolderPolicy. + */ + public function syncName(User $client): void + { + $home = $this->for($client); + + if ($home === null) { + return; + } + + $name = $this->nameFor($client); + + if ($home->name !== $name) { + $home->update(['name' => $name]); + } + } + + /** + * How many clients would get a folder if the button were pressed. + * + * A count and not the rows: the settings screen only needs the number, + * and an installation with thousands of clients should not load them + * all to render one sentence. + */ + public function pendingCount(): int + { + return User::query() + ->where('type', UserType::Client) + ->whereNotExists(fn ($q) => $q + ->selectRaw('1') + ->from('folders') + ->whereColumn('folders.home_for_user_id', 'users.id') + ->whereNull('folders.deleted_at')) + ->count(); + } + + /** + * Every client without a home gets one. + * + * Deliberately a button rather than something switching the setting on + * does by itself: it writes a folder per client, and an administrator + * trying the feature out should be able to turn it on, look, and change + * their mind without having reorganised anything. + * + * Reports counts rather than staying quiet, because on an installation + * with hundreds of clients "it worked" is not a useful answer -- the + * administrator wants to know how many there were and how many are new. + * + * @return array{total: int, created: int, existing: int} + */ + public function backfill(): array + { + $clients = User::query()->where('type', UserType::Client)->orderBy('id')->get(); + $created = 0; + $existing = 0; + + foreach ($clients as $client) { + if ($this->for($client) !== null) { + $existing++; + + continue; + } + + $this->create($client); + $created++; + } + + return [ + 'total' => $clients->count(), + 'created' => $created, + 'existing' => $existing, + ]; + } + + /** + * A blank name would render as an unclickable sliver in the tree, so + * the address stands in -- every account has one, and it identifies + * the person as well as a name does. + */ + private function nameFor(User $client): string + { + $name = trim($client->name); + + return $name !== '' ? $name : $client->email; + } +} diff --git a/app/Modules/Files/Http/Controllers/ChunkedUploadsController.php b/app/Modules/Files/Http/Controllers/ChunkedUploadsController.php index ea74c60a..968489eb 100644 --- a/app/Modules/Files/Http/Controllers/ChunkedUploadsController.php +++ b/app/Modules/Files/Http/Controllers/ChunkedUploadsController.php @@ -10,6 +10,7 @@ use App\Modules\Audit\Action; use App\Modules\Audit\ActivityLogger; use App\Modules\Clients\ClientStorageUsage; use App\Modules\Files\Models\File; +use App\Modules\Files\Folders\ClientHomeFolders; use App\Modules\Files\Models\Folder; use App\Modules\Files\Notifications\AdminClientUploadedNotification; use App\Modules\Files\Uploads\LocalPartStore; @@ -57,6 +58,7 @@ class ChunkedUploadsController extends Controller private readonly Notifier $notifier, private readonly PermissionChecker $permissions, private readonly ActivityLogger $activity, + private readonly ClientHomeFolders $homeFolders, private readonly FileVersions $versions, ) {} @@ -90,6 +92,19 @@ class ChunkedUploadsController extends Controller assert($user !== null); $folder = isset($validated['folder_id']) ? Folder::query()->whereKey($validated['folder_id'])->first() : null; + + // A client uploading without naming a folder lands in their own, + // where this installation gives them one. That is what makes the + // home a root rather than just another folder: nothing in the + // portal has to be told about it for their files to end up there. + // + // Only when no folder was named. A client who picked a destination + // picked it, and uploadableBy() below is still what decides whether + // they may -- this chooses a default, it never grants anything. + if ($folder === null) { + $folder = $this->homeFolders->for($user); + } + abort_unless(Folder::uploadableBy($user, $folder), 403); // One session per file, and a person uploads a handful at a time. diff --git a/app/Modules/Files/Http/Controllers/MyFilesController.php b/app/Modules/Files/Http/Controllers/MyFilesController.php index d606a84b..e8a62323 100644 --- a/app/Modules/Files/Http/Controllers/MyFilesController.php +++ b/app/Modules/Files/Http/Controllers/MyFilesController.php @@ -17,6 +17,7 @@ use App\Modules\Files\DownloadLimitScope; use App\Modules\Files\Editing\ApplyFileEdits; use App\Modules\Files\Editing\FileExpiry; use App\Modules\Files\Folders\BreadcrumbBuilder; +use App\Modules\Files\Folders\ClientHomeFolders; use App\Modules\Files\Models\Category; use App\Modules\Files\Models\File; use App\Modules\Files\Models\Folder; @@ -70,6 +71,7 @@ class MyFilesController extends Controller private readonly PublicThemeRegistry $themes, private readonly CapabilityRegistry $capabilities, private readonly BreadcrumbBuilder $breadcrumbs, + private readonly ClientHomeFolders $homeFolders, private readonly CommentingRules $commenting, private readonly VisibleCommentScope $comments, private readonly DownloadAllowance $allowance, @@ -106,6 +108,17 @@ class MyFilesController extends Controller // folder they created themselves, anywhere in that visible tree. $visibleIds = array_values(Folder::query()->visibleToClient($client)->pluck('id')->map(fn ($id): int => (int) $id)->all()); + // Where this installation gives clients a folder of their own, it + // stands in for the root: the client opens the portal and sees what + // is inside it, not a folder named after themselves that they have + // to click through. Their own name is not information to them. + // + // It does not replace what else they can see. Folders staff shared + // with them still sit alongside -- the home is where their own + // things live, not a boundary around them. + $home = $this->homeFolders->for($client); + $homeId = $home?->id; + // A search term, a category filter, or an owner filter all switch to // a flat, global view across everything the client may see — same // convention as the staff library (FoldersController) uses for @@ -146,7 +159,24 @@ class MyFilesController extends Controller if ($current === null) { $folders = Folder::query() ->whereIn('id', $visibleIds) - ->where(fn ($q) => $q->whereNull('parent_id')->orWhereNotIn('parent_id', $visibleIds)) + ->where(function ($q) use ($visibleIds, $homeId): void { + // The home's own children, standing in for the root's. + if ($homeId !== null) { + $q->where('parent_id', $homeId); + } + + // Plus the top of every other subtree they can see, + // with the home itself removed -- it is the level + // they are looking at, not something inside it. + $q->{$homeId === null ? 'where' : 'orWhere'}(function ($outer) use ($visibleIds, $homeId): void { + $outer->where(fn ($inner) => $inner + ->whereNull('parent_id')->orWhereNotIn('parent_id', $visibleIds)); + + if ($homeId !== null) { + $outer->where('id', '!=', $homeId); + } + }); + }) ->orderBy('name'); } else { $folders = Folder::query() @@ -161,7 +191,12 @@ class MyFilesController extends Controller // own listing) show here with no folder context. $filesQuery = File::query()->visibleToClient($client); if ($current === null) { - $filesQuery->where(fn (Builder $q) => $q->whereNull('folder_id')->orWhereNotIn('folder_id', $visibleIds)); + $filesQuery->where(fn (Builder $q) => $q + ->whereNull('folder_id') + ->orWhereNotIn('folder_id', $visibleIds) + // Files sitting directly in the home belong to this + // level too, for the same reason its subfolders do. + ->when($homeId !== null, fn (Builder $w) => $w->orWhere('folder_id', $homeId))); } else { $filesQuery->where('folder_id', $current->id); } @@ -239,7 +274,10 @@ class MyFilesController extends Controller return Inertia::render("portal/themes/{$this->themeKey()}/my-files", [ 'folder' => $current === null ? null : ['id' => $current->id, 'name' => $current->name], - 'breadcrumb' => $flat ? [] : $this->breadcrumbs->visible($current, $visibleIds), + // Trimmed of the home, which is the root here and so is not a + // step in the trail -- a client browsing their own subfolder + // should see "Invoices", not "Acme Ltd / Invoices". + 'breadcrumb' => $flat ? [] : $this->trimHome($this->breadcrumbs->visible($current, $visibleIds), $home), 'folders' => $folderRows->map(fn (Folder $folder): array => [ 'id' => $folder->id, 'name' => $folder->name, @@ -578,4 +616,23 @@ class MyFilesController extends Controller return $this->themes->resolve(is_string($value) ? $value : 'default', $this->capabilities); } + + /** + * Drop the home folder from the front of a breadcrumb. + * + * Only from the front, and only when it is actually there: a folder + * shared with the client from elsewhere in the library has a trail of + * its own that the home has nothing to do with. + * + * @param list $trail + * @return list + */ + private function trimHome(array $trail, ?Folder $home): array + { + if ($home === null || $trail === [] || $trail[0]['id'] !== $home->id) { + return $trail; + } + + return array_slice($trail, 1); + } } diff --git a/app/Modules/Files/Http/Controllers/MyFoldersController.php b/app/Modules/Files/Http/Controllers/MyFoldersController.php index 7b490f8a..5028d581 100644 --- a/app/Modules/Files/Http/Controllers/MyFoldersController.php +++ b/app/Modules/Files/Http/Controllers/MyFoldersController.php @@ -7,6 +7,7 @@ namespace App\Modules\Files\Http\Controllers; use App\Http\Controllers\Controller; use App\Modules\Audit\Action; use App\Modules\Audit\ActivityLogger; +use App\Modules\Files\Folders\ClientHomeFolders; use App\Modules\Files\Folders\FolderService; use App\Modules\Files\Models\File; use App\Modules\Files\Models\Folder; @@ -30,6 +31,7 @@ class MyFoldersController extends Controller public function __construct( private readonly FolderService $folders, private readonly ActivityLogger $activity, + private readonly ClientHomeFolders $homeFolders, ) {} public function store(Request $request): RedirectResponse @@ -52,6 +54,13 @@ class MyFoldersController extends Controller $parent = Folder::query()->visibleToClient($client)->whereKey($validated['parent_id'])->firstOrFail(); } + // No parent named means the top of what this client sees -- which, + // where the installation gives them a home, is inside it rather + // than at the root of the library. Without this a client creating a + // folder would put it beside the staff folders, which is precisely + // the mess the home folder exists to end. + $parent ??= $this->homeFolders->for($client); + $folder = $this->folders->create($validated['name'], $parent); $this->activity->log(Action::FolderCreated, subject: $folder); diff --git a/app/Modules/Files/Models/Folder.php b/app/Modules/Files/Models/Folder.php index ab471048..0f176337 100644 --- a/app/Modules/Files/Models/Folder.php +++ b/app/Modules/Files/Models/Folder.php @@ -112,6 +112,12 @@ class Folder extends Model return $this->created_by === $user->id; } + /** Whether this folder stands in as some client's root. */ + public function isHome(): bool + { + return $this->home_for_user_id !== null; + } + /** * Self or any ancestor is public — the inheritance every file in this * folder's subtree relies on (File::isEffectivelyPublic()), and what diff --git a/app/Modules/Platform/Settings/Setting.php b/app/Modules/Platform/Settings/Setting.php index c44a0e9b..68ef5e93 100644 --- a/app/Modules/Platform/Settings/Setting.php +++ b/app/Modules/Platform/Settings/Setting.php @@ -44,6 +44,22 @@ enum Setting: string // PreviewKind and FileThumbnailController::preview. case ClientsCanPreviewFiles = 'clients_can_preview_files'; + // Give every new client a folder of their own, named after them, and + // treat it as their root: their uploads and the folders they create + // land inside it instead of at the top of the library. What the + // administrator gets out of it is a /files that reads as one folder + // per client rather than a flat pile. + // + // It is NOT a boundary. A folder staff share with a client is still + // theirs to see, alongside their own -- making the home a jail would + // silently break every share that already exists. See ClientHomeFolders. + // + // Off by default, and turning it on changes nothing that already + // exists: existing clients get a home only when an administrator asks + // for one, with the button beside the switch. A setting that quietly + // reorganised a library on save would be one nobody could try. + case ClientsHomeFolders = 'clients_home_folders'; + // Maximum upload size in MB (0 = unlimited). case MaxFileSizeMb = 'max_file_size_mb'; @@ -402,6 +418,7 @@ enum Setting: string self::ClientsCanRegister, self::ClientsAutoApprove, + self::ClientsHomeFolders, self::EmailNotificationsEnabled, self::DiscourageSearchIndexing, self::PublicListingEnabled, @@ -481,6 +498,10 @@ enum Setting: string // configuration names one — see ScanningConfig. self::VirusScanningEnabled, self::OrphanFilesAutoDeleteEnabled => false, + // Off, because switching it on is a change to how a library is + // laid out and that should be somebody's decision rather than + // something an upgrade did to them overnight. + self::ClientsHomeFolders => false, self::CheckForUpdates, self::FetchNews, diff --git a/database/migrations/2026_09_17_090000_add_home_for_user_id_to_folders_table.php b/database/migrations/2026_09_17_090000_add_home_for_user_id_to_folders_table.php new file mode 100644 index 00000000..a3b0f1be --- /dev/null +++ b/database/migrations/2026_09_17_090000_add_home_for_user_id_to_folders_table.php @@ -0,0 +1,47 @@ +foreignId('home_for_user_id')->nullable()->after('created_by') + ->constrained('users')->nullOnDelete(); + $table->unique('home_for_user_id', 'folders_home_for_user_id_unique'); + }); + } + + public function down(): void + { + Schema::table('folders', function (Blueprint $table): void { + $table->dropUnique('folders_home_for_user_id_unique'); + $table->dropConstrainedForeignId('home_for_user_id'); + }); + } +}; diff --git a/resources/js/pages/system/settings/clients.tsx b/resources/js/pages/system/settings/clients.tsx index c8efad52..904d4a92 100644 --- a/resources/js/pages/system/settings/clients.tsx +++ b/resources/js/pages/system/settings/clients.tsx @@ -5,6 +5,7 @@ import { FormEventHandler } from 'react'; import Heading from '@/components/heading'; import InputError from '@/components/input-error'; import { SaveButton } from '@/components/save-button'; +import { Button } from '@/components/ui/button'; import { Checkbox } from '@/components/ui/checkbox'; import { Input } from '@/components/ui/input'; import { Label } from '@/components/ui/label'; @@ -21,6 +22,8 @@ interface ClientSettingsProps { client_invitation_expiry_hours: number; default_client_storage_quota_mb: number; clients_can_preview_files: boolean; + clients_home_folders: boolean; + clients_without_home: number; groups: { id: number; name: string }[]; } @@ -33,6 +36,8 @@ export default function ClientSettings({ client_invitation_expiry_hours, default_client_storage_quota_mb, clients_can_preview_files, + clients_home_folders, + clients_without_home, groups, }: ClientSettingsProps) { const { t } = useTranslation(); @@ -51,8 +56,14 @@ export default function ClientSettings({ client_invitation_expiry_hours: String(client_invitation_expiry_hours), default_client_storage_quota_mb: String(default_client_storage_quota_mb), clients_can_preview_files: clients_can_preview_files, + clients_home_folders: clients_home_folders, }); + // Its own request, and its own spinner. Saving the form records the + // preference; this writes a folder per client, so the two must not look + // like one action. + const backfill = useForm({}); + const submit: FormEventHandler = (e) => { e.preventDefault(); patch(route('system-settings.clients.update')); @@ -214,6 +225,54 @@ export default function ClientSettings({ +
+ setData('clients_home_folders', checked === true)} + /> +
+ +

+ {t( + 'New clients get a folder named after them, and it acts as their root: what they upload and any folder they create goes inside it. In the file library you see one folder per client instead of everything at the top level. Clients still see anything you share with them, wherever it lives.', + )} +

+
+
+ + + {clients_home_folders && ( +
+

{t('Clients created before you turned this on')}

+

+ {clients_without_home === 0 + ? t('Every client already has a folder.') + : t( + ':count of your existing clients have no folder yet. Creating them does not move any file — each client keeps what they already have, and new uploads go into the new folder.', + { count: String(clients_without_home) }, + )} +

+ {clients_without_home > 0 && ( + + )} +
+ )} + diff --git a/routes/settings.php b/routes/settings.php index 2c4ff92d..f8dba7bc 100644 --- a/routes/settings.php +++ b/routes/settings.php @@ -180,6 +180,11 @@ Route::middleware('auth')->group(function () { Route::patch('system/settings/security', [SecuritySettingsController::class, 'update'])->name('system-settings.security.update'); Route::get('system/settings/clients', [ClientSettingsController::class, 'edit'])->name('system-settings.clients.edit'); Route::patch('system/settings/clients', [ClientSettingsController::class, 'update'])->name('system-settings.clients.update'); + // A POST rather than part of the PATCH above: it writes a folder per + // client, which is not something saving a preferences form should do + // as a side effect. See ClientSettingsController::backfillHomeFolders. + Route::post('system/settings/clients/home-folders', [ClientSettingsController::class, 'backfillHomeFolders']) + ->name('system-settings.clients.home-folders'); Route::get('system/settings/uploads', [UploadSettingsController::class, 'edit'])->name('system-settings.uploads.edit'); Route::patch('system/settings/uploads', [UploadSettingsController::class, 'update'])->name('system-settings.uploads.update'); Route::get('system/settings/downloads', [DownloadSettingsController::class, 'edit'])->name('system-settings.downloads.edit'); diff --git a/tests/Feature/Clients/ClientsManagementTest.php b/tests/Feature/Clients/ClientsManagementTest.php index 72618b63..b69f6c3c 100644 --- a/tests/Feature/Clients/ClientsManagementTest.php +++ b/tests/Feature/Clients/ClientsManagementTest.php @@ -156,6 +156,7 @@ test('staff can update client settings and they take effect', function () { 'client_invitation_expiry_hours' => 72, 'default_client_storage_quota_mb' => 0, 'clients_can_preview_files' => true, + 'clients_home_folders' => false, ])->assertRedirect()->assertSessionDoesntHaveErrors(); Auth::logout(); diff --git a/tests/Feature/Files/ClientHomeFoldersTest.php b/tests/Feature/Files/ClientHomeFoldersTest.php new file mode 100644 index 00000000..3a761bdc --- /dev/null +++ b/tests/Feature/Files/ClientHomeFoldersTest.php @@ -0,0 +1,249 @@ +admin = User::factory()->create(); + // Explicitly, never assumed: the Settings cache outlives a + // RefreshDatabase rollback, so a test that wants the default has to say + // so. See the settings-cache note in the test helpers. + app(Settings::class)->set(Setting::ClientsHomeFolders, false); +}); + +function enableHomeFolders(): void +{ + app(Settings::class)->set(Setting::ClientsHomeFolders, true); +} + +function shareFolderWithClient(Folder $folder, User $client): void +{ + FolderAssignment::query()->create([ + 'folder_id' => $folder->id, + 'assignable_type' => $client->getMorphClass(), + 'assignable_id' => $client->id, + ]); +} + +function clientWhoCanUpload(string $name = 'Acme Ltd'): User +{ + $client = User::factory()->client()->create(['name' => $name]); + + foreach ([Permission::Upload, Permission::CreateOwnFolders] as $permission) { + RolePermission::query()->firstOrCreate(['role_id' => $client->role_id, 'permission' => $permission->value]); + } + + return $client->refresh(); +} + +test('a new client gets a folder named after them, and only when the setting is on', function () { + $before = User::factory()->client()->create(['name' => 'Before Ltd']); + expect(app(ClientHomeFolders::class)->for($before))->toBeNull(); + + enableHomeFolders(); + + $after = User::factory()->client()->create(['name' => 'After Ltd']); + $home = app(ClientHomeFolders::class)->for($after); + + expect($home)->not->toBeNull() + ->and($home->name)->toBe('After Ltd') + // At the root, which is the entire point: this is what an + // administrator opening /files is meant to see. + ->and($home->parent_id)->toBeNull() + // Owned by the client, because that is how scopeVisibleToClient + // grants them a folder -- no assignment row to keep in step. + ->and($home->created_by)->toBe($after->id); + + // Staff who made the account did not accidentally become the owner. + expect($home->created_by)->not->toBe($this->admin->id); +}); + +test('the folder is created however the account was made, not just on one screen', function () { + enableHomeFolders(); + + // Through the service the staff screens, the API and the control plane + // all share -- a path that never calls a controller. + $client = app(ClientAccounts::class)->create( + name: 'Service Made', + email: 'service@example.test', + password: 'a-sufficiently-long-password', + ); + + expect(app(ClientHomeFolders::class)->for($client))->not->toBeNull(); +}); + +test('the home is a default location, never a boundary', function () { + enableHomeFolders(); + $client = clientWhoCanUpload(); + + // A folder staff shared with this client, nowhere near their home. + $shared = app(FolderService::class)->create('Contracts', null); + shareFolderWithClient($shared, $client); + + $this->actingAs($client)->get('/my-files')->assertOk()->assertInertia( + fn (AssertableInertia $page) => $page + // The shared folder is still right there. If this ever fails, + // the feature has started revoking access rather than + // organising it. + ->where('folders', fn ($folders) => collect($folders)->pluck('name')->contains('Contracts')), + ); +}); + +test('the client sees inside their folder, not the folder itself', function () { + enableHomeFolders(); + $client = clientWhoCanUpload(); + $home = app(ClientHomeFolders::class)->for($client); + + $inside = app(FolderService::class)->create('Invoices', $home); + $inside->update(['created_by' => $client->id]); + + $this->actingAs($client)->get('/my-files')->assertOk()->assertInertia(function (AssertableInertia $page) { + $names = collect($page->toArray()['props']['folders'])->pluck('name'); + + // Their own name is not information to them. + expect($names)->toContain('Invoices') + ->and($names)->not->toContain('Acme Ltd'); + }); +}); + +test('a folder the client creates without choosing a parent lands in their home', function () { + enableHomeFolders(); + $client = clientWhoCanUpload(); + $home = app(ClientHomeFolders::class)->for($client); + + $this->actingAs($client)->post('/my-folders', ['name' => 'Receipts'])->assertRedirect(); + + expect(Folder::query()->where('name', 'Receipts')->sole()->parent_id)->toBe($home->id); +}); + +test('the client cannot rename or delete their own home folder', function () { + enableHomeFolders(); + $client = clientWhoCanUpload(); + $home = app(ClientHomeFolders::class)->for($client); + + // They own it -- created_by is them -- so without the guard both of + // these would be allowed by ownership alone. + expect($home->isOwnedBy($client))->toBeTrue(); + + $this->actingAs($client)->patch("/my-folders/{$home->id}", ['name' => 'Something else'])->assertForbidden(); + $this->actingAs($client)->delete("/my-folders/{$home->id}")->assertForbidden(); + + expect($home->refresh()->name)->toBe('Acme Ltd'); +}); + +test('renaming the client renames the folder, even over a hand-typed name', function () { + enableHomeFolders(); + $client = clientWhoCanUpload(); + $home = app(ClientHomeFolders::class)->for($client); + + // Somebody renamed it by hand. The product decision is that the + // account still wins: a folder called "Acme Ltd" under an account now + // called something else misleads the administrator it exists for. + $home->update(['name' => 'Hand typed']); + + $client->update(['name' => 'Acme Holdings']); + + expect($home->refresh()->name)->toBe('Acme Holdings'); +}); + +test('the backfill is idempotent and reports what it did', function () { + User::factory()->client()->count(3)->create(); + enableHomeFolders(); + $fresh = User::factory()->client()->create(); + + $first = app(ClientHomeFolders::class)->backfill(); + + // Four clients; the one created after the switch already had one. + expect($first['total'])->toBe(4) + ->and($first['created'])->toBe(3) + ->and($first['existing'])->toBe(1); + + // Pressing the button twice must converge, not accumulate. + $second = app(ClientHomeFolders::class)->backfill(); + + expect($second['created'])->toBe(0) + ->and($second['existing'])->toBe(4) + ->and(Folder::query()->whereNotNull('home_for_user_id')->count())->toBe(4); + + expect(app(ClientHomeFolders::class)->for($fresh))->not->toBeNull(); +}); + +test('the backfill button is refused while the setting is off', function () { + User::factory()->client()->create(); + + $this->actingAs($this->admin)->post('/system/settings/clients/home-folders')->assertForbidden(); + + expect(Folder::query()->whereNotNull('home_for_user_id')->count())->toBe(0); +}); + +test('the settings screen says how many clients are still without a folder', function () { + enableHomeFolders(); + User::factory()->client()->count(2)->create(); + // Created while the setting was on, so it already has one. + expect(Folder::query()->whereNotNull('home_for_user_id')->count())->toBe(2); + + User::factory()->client()->count(3)->create(); + Folder::query()->whereNotNull('home_for_user_id')->limit(3)->delete(); + + $this->actingAs($this->admin)->get('/system/settings/clients')->assertInertia( + fn (AssertableInertia $page) => $page->where('clients_without_home', 3), + ); +}); + +test('a client upload with no folder chosen lands in their home', function () { + enableHomeFolders(); + $client = clientWhoCanUpload(); + $home = app(ClientHomeFolders::class)->for($client); + + $this->actingAs($client); + + // The real intake path, with no folder_id in the body -- which is what + // the portal sends when the client just picks a file and uploads. + $session = $this->postJson('/uploads', [ + 'filename' => 'note.txt', + 'size' => 11, + 'type' => 'text/plain', + ])->assertOk()->json('uploadId'); + + // Parts go to a signed URL, not to the bare path -- see the portal's + // own upload flow. + $url = $this->getJson("/uploads/{$session}/parts/1/sign")->assertOk()->json('url'); + $this->call('PUT', $url, [], [], [], ['CONTENT_TYPE' => 'application/octet-stream'], 'hello-world'); + $this->postJson("/uploads/{$session}/complete")->assertOk(); + + expect(File::query()->where('uploaded_by', $client->id)->sole()->folder_id)->toBe($home->id); +}); + +test('turning the setting off leaves an existing home working', function () { + enableHomeFolders(); + $client = clientWhoCanUpload(); + $home = app(ClientHomeFolders::class)->for($client); + + app(Settings::class)->set(Setting::ClientsHomeFolders, false); + + // The folder is real and holds real files. Pretending it is gone would + // strand them somewhere no listing looks. + expect(app(ClientHomeFolders::class)->for($client->refresh())->id)->toBe($home->id); +}); diff --git a/tests/Feature/Groups/MembershipRequestsTest.php b/tests/Feature/Groups/MembershipRequestsTest.php index f0ba20d4..d1f0236d 100644 --- a/tests/Feature/Groups/MembershipRequestsTest.php +++ b/tests/Feature/Groups/MembershipRequestsTest.php @@ -182,6 +182,7 @@ test('the client settings screen validates the group options', function () { 'client_invitation_expiry_hours' => 72, 'default_client_storage_quota_mb' => 0, 'clients_can_preview_files' => true, + 'clients_home_folders' => false, ])->assertSessionDoesntHaveErrors(); expect(app(Settings::class)->get(Setting::ClientsAutoGroup))->toBe($group->id);