From cb531207796367ba92843a04e61e129ced0278df Mon Sep 17 00:00:00 2001 From: denkfabrik-li <274324701+denkfabrik-li@users.noreply.github.com> Date: Fri, 28 Aug 2026 02:18:29 +0200 Subject: [PATCH] Show the portal dashboard the files a client can actually open clientDashboard() restates the assignment half of File::scopeVisibleToClient in a whereHas of its own. The scope is the single source of truth for client file access and ends in notExpired(), which the copy leaves off, so the two disagree in both directions. Over: an expired file stays counted and keeps its name on the dashboard after /my-files has stopped listing it and the download answers 403. Under: everything that reaches a client another way is missing -- a file inside a folder shared with them, a file they uploaded through the portal themselves, and a revision, which owns no assignment row at all and inherits its original's recipients through SharingIdentity. Replaced by the scope itself, which is what /my-files runs. The existing test for the page is unchanged and still passes: a directly assigned, unexpired file counts exactly as before. Two tests, one for each direction. Without the fix both go red. --- .../Http/Controllers/DashboardController.php | 22 ++++---- tests/Feature/Audit/DashboardTest.php | 50 +++++++++++++++++++ 2 files changed, 62 insertions(+), 10 deletions(-) diff --git a/app/Modules/Audit/Http/Controllers/DashboardController.php b/app/Modules/Audit/Http/Controllers/DashboardController.php index b8931b45..ba875a6b 100644 --- a/app/Modules/Audit/Http/Controllers/DashboardController.php +++ b/app/Modules/Audit/Http/Controllers/DashboardController.php @@ -497,23 +497,25 @@ class DashboardController extends Controller private function clientDashboard(User $client): Response { - $assignedFiles = File::query()->whereHas('assignments', function ($query) use ($client): void { - $query->where(function ($direct) use ($client): void { - $direct->where('assignable_type', User::class)->where('assignable_id', $client->id); - })->orWhere(function ($viaGroup) use ($client): void { - $viaGroup->where('assignable_type', Group::class) - ->whereIn('assignable_id', $client->memberOfGroups()->pluck('groups.id')); - }); - }); + // File::scopeVisibleToClient is the single source of truth for + // client file access, and this page has to agree with the portal it + // introduces. Restating the assignment half here made it disagree + // in both directions: it counted expired files, which the scope + // ends by excluding and /my-files therefore never shows, and it + // missed everything that reaches a client another way — a file in a + // folder shared with them, their own portal upload, and a revision, + // which owns no assignment row and inherits its original's + // recipients. + $visibleFiles = File::query()->visibleToClient($client); return Inertia::render('portal/dashboard', [ - 'files_count' => (clone $assignedFiles)->count(), + 'files_count' => (clone $visibleFiles)->count(), 'groups_count' => $client->memberOfGroups()->where('public', true)->count(), 'storage' => [ 'used_bytes' => $this->storageUsage->usedBytes($client), 'quota_bytes' => $this->storageUsage->quotaBytes($client) ?: null, ], - 'latest_files' => $assignedFiles->orderByDesc('created_at')->limit(5)->get() + 'latest_files' => $visibleFiles->orderByDesc('created_at')->limit(5)->get() ->map(fn (File $file): array => [ 'id' => $file->id, 'name' => $file->name, diff --git a/tests/Feature/Audit/DashboardTest.php b/tests/Feature/Audit/DashboardTest.php index db13433a..a3e03e53 100644 --- a/tests/Feature/Audit/DashboardTest.php +++ b/tests/Feature/Audit/DashboardTest.php @@ -7,6 +7,7 @@ use App\Modules\Audit\Action; use App\Modules\Audit\ActivityLog; use App\Modules\Audit\ActivityOrigin; use App\Modules\Files\Models\File; +use App\Modules\Files\Models\FolderAssignment; use App\Modules\Files\Models\ShareLink; use App\Modules\Groups\Models\Group; use App\Modules\Identity\Models\Role; @@ -317,6 +318,55 @@ test('clients get the portal dashboard with their own numbers', function () { expect((string) $response->getContent())->not->toContain('Hidden'); }); +test('the portal dashboard does not count a file the client can no longer open', function () { + // File::scopeVisibleToClient ends in notExpired(), so /my-files stops + // listing an expired file and the download is refused. The dashboard + // restated the assignment half of that scope without its ending, and + // went on offering the file's name. + $client = User::factory()->client()->create(); + $file = File::factory()->create([ + 'uploaded_by' => $this->admin->id, + 'name' => 'old-offer', + 'expires_at' => now()->subDay(), + ]); + shareFileWith($file, $client); + + $this->actingAs($client)->get("/files/{$file->id}/download")->assertForbidden(); + + $this->actingAs($client)->get('/dashboard')->assertInertia( + fn (AssertableInertia $page) => $page + ->where('files_count', 0) + ->has('latest_files', 0), + ); +}); + +test('the portal dashboard counts a file that reaches the client through a folder', function () { + // The other direction of the same substitution: a shared folder and a + // client's own upload are two of the three ways into + // scopeVisibleToClient that the hand-rolled query left out, so the + // number was under the truth as well as over it. + $client = User::factory()->client()->create(); + $folder = makeFolder('Shared with them'); + FolderAssignment::query()->create([ + 'folder_id' => $folder->id, + 'assignable_type' => $client->getMorphClass(), + 'assignable_id' => $client->id, + ]); + + File::factory()->create([ + 'uploaded_by' => $this->admin->id, + 'name' => 'in-the-folder', + 'folder_id' => $folder->id, + ]); + File::factory()->create(['uploaded_by' => $client->id, 'name' => 'their-own-upload']); + + $this->actingAs($client)->get('/dashboard')->assertInertia( + fn (AssertableInertia $page) => $page + ->where('files_count', 2) + ->has('latest_files', 2), + ); +}); + test('the recent-activity widget carries origin so an actorless entry is not mislabelled', function () { // An anonymous entry (a public/share-link download) and a system entry // are both actor_name null; only `origin` separates "Anonymous" from