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