diff --git a/CHANGELOG.md b/CHANGELOG.md index 75099cde..57aa6a51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -239,6 +239,12 @@ a version is cut. (found, diagnosed and fixed by [@denkfabrik-li](https://github.com/denkfabrik-li) in [#1705](https://github.com/projectsend/projectsend/pull/1705)) +- **The dashboard's expired-files list says whose files it is showing.** For a staff role limited to + its own clients it lists that person's own uploads, since an expired file is already out of reach + of the clients it was shared with. It now says so — "Your expired files", and a line explaining + what is not in the list — rather than presenting a short list as though it were the whole picture. + A warning about what is due to be deleted is worth nothing if it is quietly narrower than it looks. + - **A limited staff role no longer reaches every client record, or every file name on the dashboard.** Two more places where holding a permission was treated as holding a boundary. The clients screen listed every client on the installation by name and email, and a role limited to diff --git a/app/Modules/Audit/Http/Controllers/DashboardController.php b/app/Modules/Audit/Http/Controllers/DashboardController.php index 13040904..0ff45a87 100644 --- a/app/Modules/Audit/Http/Controllers/DashboardController.php +++ b/app/Modules/Audit/Http/Controllers/DashboardController.php @@ -407,10 +407,11 @@ class DashboardController extends Controller // File::scopeVisibleToClient ends in notExpired(), so an // expired file belonging to one of their clients is not in // their library, and only their own expired uploads reach - // this list. Safe, and under-inclusive — telling them about - // a client's file that auto-delete is about to take would - // need a library query that keeps expired rows, which is a - // boundary to decide rather than to invent here. + // this list. Rather than widen the boundary — which would + // mean a library query that keeps expired rows, and + // scopeVisibleToClient is the single source of truth for + // client file access — the widget says what it is showing. + // `scoped` is how it knows to. 'count' => $this->library->files($viewer)->expired()->count(), 'files' => array_values($this->library->files($viewer)->expired()->orderBy('expires_at')->limit(10) ->get(['id', 'name', 'expires_at']) @@ -420,6 +421,10 @@ class DashboardController extends Controller 'expires_at' => $file->expires_at?->toIso8601String(), 'edit_url' => $canFiles ? route('files.edit', $file->id, false) : null, ])->all()), + // Whether this list is "everything expired" or "everything of + // yours that expired" — a widget whose whole job is warning + // about what is due to be deleted has to say which it means. + 'scoped' => $viewer->isClientScoped(), 'auto_delete_enabled' => (bool) $this->settings->get(Setting::ExpiredFilesAutoDeleteEnabled), // Schedule::command('projectsend:purge-expired-files')->daily() // runs at 00:00 — always "tonight" from whenever this loads. diff --git a/resources/js/components/dashboard-widgets/expired-files-widget.tsx b/resources/js/components/dashboard-widgets/expired-files-widget.tsx index 67a30110..37550130 100644 --- a/resources/js/components/dashboard-widgets/expired-files-widget.tsx +++ b/resources/js/components/dashboard-widgets/expired-files-widget.tsx @@ -13,6 +13,8 @@ export interface ExpiredFile { export interface ExpiredFilesSummary { count: number; files: ExpiredFile[]; + /** True when the viewer's role limits them to their own clients, in which case this lists only their own uploads. */ + scoped: boolean; auto_delete_enabled: boolean; next_run_at: string; } @@ -31,8 +33,16 @@ export function ExpiredFilesWidget({ expiredFiles }: { expiredFiles: ExpiredFile

{t('Next cleanup :date', { date: dateTime(expiredFiles.next_run_at) })}

)} + {/* A warning about what is due to be deleted has to say what it + covers. A limited role sees only its own uploads here, and + would otherwise read an empty list as "nothing to worry + about" on behalf of files it cannot see. */} + {expiredFiles.scoped &&

{t('Files you uploaded. Your clients\u2019 files are not listed here.')}

} + {expiredFiles.count === 0 ? ( -

{t('No expired files.')}

+

+ {expiredFiles.scoped ? t('None of your uploads have expired.') : t('No expired files.')} +

) : (
{expiredFiles.files.map((file) => ( diff --git a/resources/js/pages/dashboard.tsx b/resources/js/pages/dashboard.tsx index 23cc5316..466c0ba0 100644 --- a/resources/js/pages/dashboard.tsx +++ b/resources/js/pages/dashboard.tsx @@ -212,7 +212,11 @@ export default function Dashboard({ case 'expired_files': return ( expired_files && ( - + // Retitled rather than relabelled in WIDGET_LABELS: + // the same widget means different things to the two + // viewers, and only the server knows which one this + // is. + ) diff --git a/tests/Feature/Audit/DashboardTest.php b/tests/Feature/Audit/DashboardTest.php index 60786986..8cd06971 100644 --- a/tests/Feature/Audit/DashboardTest.php +++ b/tests/Feature/Audit/DashboardTest.php @@ -446,7 +446,11 @@ test('the statistics widgets name only files inside the viewer scope', function ->where('expired_files.files.0.name', 'My Own Expired') // The count has to agree with the list, or the number // describes files the list is not allowed to name. - ->where('expired_files.count', 1), + ->where('expired_files.count', 1) + // And the widget has to say which list it is: a warning about + // what is due to be deleted, showing only the viewer's own + // uploads, reads as "nothing to worry about" otherwise. + ->where('expired_files.scoped', true), ); }); @@ -461,6 +465,9 @@ test('an unscoped viewer still sees the whole installation in those widgets', fu $this->actingAs($this->admin)->get('/dashboard')->assertInertia( fn (AssertableInertia $page) => $page ->where('largest_files.0.name', 'Anything') - ->where('expired_files.count', 1), + ->where('expired_files.count', 1) + // Unscoped: the widget keeps its plain title and its plain + // meaning, everything expired on the installation. + ->where('expired_files.scoped', false), ); });