From 6b76c11192f8bc62b2a2c6f3c03f8647a9a974c9 Mon Sep 17 00:00:00 2001 From: ignacionelson Date: Fri, 21 Aug 2026 12:39:16 -0300 Subject: [PATCH 1/3] Answer "what happened to this file?" on the file's own page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A file's history was only reachable from the library list, through the details panel's Activity tab — so anyone who arrived at the file from a link, a search or a notification had to go back and find the row they came from to ask what had happened to it. The file's own page now carries an Activity tab of its own, next to General and Sharing: the twenty most recent entries, fetched only if the tab is opened, and a link to the full history. It is behind the same view_actions_log permission as everywhere else. That full history is now filterable, which is the point of sending somebody to it. The action list is built from the file's own log rather than from the eighty-odd actions the software can record — all but a handful of which can never apply to a file — and each option carries its count. Downloads are three separate actions on purpose (a signed-in recipient, a public link, the public group listing), so "All downloads" asks that question once instead of three times; the group only appears when the file's log actually holds more than one of its members. Narrowing by who acted and by date range works the same as it does on the main activity log, the reader's own calendar day included. --- .../Controllers/FileDetailsController.php | 195 +++++++++++++++++- .../Http/Controllers/FilesController.php | 5 + resources/js/pages/activity/subject.tsx | 90 +++++++- resources/js/pages/files/edit.tsx | 92 ++++++++- tests/Feature/Files/DetailsPanelTest.php | 25 +++ .../Feature/Files/FileActivityHistoryTest.php | 65 ++++++ 6 files changed, 453 insertions(+), 19 deletions(-) diff --git a/app/Modules/Files/Http/Controllers/FileDetailsController.php b/app/Modules/Files/Http/Controllers/FileDetailsController.php index a841d93d..d9af023a 100644 --- a/app/Modules/Files/Http/Controllers/FileDetailsController.php +++ b/app/Modules/Files/Http/Controllers/FileDetailsController.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace App\Modules\Files\Http\Controllers; use App\Http\Controllers\Controller; +use App\Models\User; use App\Modules\Audit\Action; use App\Modules\Audit\ActivityLog; use App\Modules\Audit\ActivityPresenter; @@ -18,10 +19,15 @@ use App\Modules\Files\Models\File; use App\Modules\Files\Models\Folder; use App\Modules\Files\Models\ShareLink; use App\Modules\Files\Versions\FileVersionLinks; +use App\Modules\Platform\Localization\LocalDay; +use App\Modules\Platform\Localization\TimezoneRegistry; +use Carbon\Carbon; +use Illuminate\Database\Eloquent\Builder; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Illuminate\Support\Collection; use Illuminate\Support\Facades\Gate; +use Illuminate\Validation\Rule; use Inertia\Inertia; use Inertia\Response; @@ -34,6 +40,29 @@ class FileDetailsController extends Controller /** Raw rows considered when grouping downloads() by actor — see that method's docblock. */ private const DOWNLOADS_SUMMARY_LIMIT = 500; + /** + * Filters that stand for a question rather than for one logged action. + * + * "Who downloaded this file?" is not one action: a signed-in recipient, + * somebody following a public link and a visitor to a public group + * listing are recorded separately, on purpose, because *how* a file + * left matters. But nobody reading a file's history wants to ask the + * question three times, so these offer it once — and are still only + * shown when the file's own log has more than one of the members. + * + * @var array}> + */ + private const ACTION_GROUPS = [ + 'downloads' => [ + 'label' => 'All downloads', + 'actions' => [Action::FileDownloaded, Action::ShareLinkDownloaded, Action::PublicFileDownloaded], + ], + 'previews' => [ + 'label' => 'All previews', + 'actions' => [Action::FilePreviewed, Action::PublicFilePreviewed], + ], + ]; + public function __construct( private readonly ActivityPresenter $presenter, private readonly DownloadPresenter $downloadPresenter, @@ -41,6 +70,7 @@ class FileDetailsController extends Controller private readonly CommentingRules $commenting, private readonly FileVersionLinks $versionLinks, private readonly DownloadAllowance $allowance, + private readonly TimezoneRegistry $timezones, ) {} public function show(Request $request, File $file): JsonResponse @@ -148,7 +178,15 @@ class FileDetailsController extends Controller Gate::forUser($viewer)->authorize('view', $file); abort_unless($viewer->can('view_actions_log'), 403); - return $this->renderHistory($file->getMorphClass(), $file->id, $file->name, route('files.edit', $file, false)); + return $this->renderHistory( + $request, + $file->getMorphClass(), + $file->id, + $file->name, + route('files.edit', $file, false).'?tab=activity', + 'files.activity.history', + ['file' => $file->id], + ); } /** @@ -304,15 +342,35 @@ class FileDetailsController extends Controller Gate::forUser($viewer)->authorize('view', $folder); abort_unless($viewer->can('view_actions_log'), 403); - return $this->renderHistory($folder->getMorphClass(), $folder->id, $folder->name, route('files.index', ['folder' => $folder->id], false)); + return $this->renderHistory( + $request, + $folder->getMorphClass(), + $folder->id, + $folder->name, + route('files.index', ['folder' => $folder->id], false), + 'folders.activity.history', + ['folder' => $folder->id], + ); } - private function renderHistory(string $morphClass, int $subjectId, string $subjectName, string $backUrl): Response - { - $entries = ActivityLog::query() - ->where('subject_type', $morphClass) - ->where('subject_id', $subjectId) - ->orderByDesc('created_at')->orderByDesc('id') + /** + * @param array $routeParams + */ + private function renderHistory( + Request $request, + string $morphClass, + int $subjectId, + string $subjectName, + string $backUrl, + string $routeName, + array $routeParams, + ): Response { + $viewer = $request->user(); + assert($viewer !== null); + + $filters = $this->validatedHistoryFilters($request); + + $entries = $this->historyQuery($morphClass, $subjectId, $filters, $viewer) ->paginate(25) ->withQueryString(); @@ -327,8 +385,129 @@ class FileDetailsController extends Controller 'next' => $entries->nextPageUrl(), 'total' => $entries->total(), ], + 'filters' => $filters, + 'action_options' => $this->actionOptions($morphClass, $subjectId), 'subject_name' => $subjectName, 'back_url' => $backUrl, + 'route_name' => $routeName, + 'route_params' => $routeParams, ]); } + + /** + * The actions this subject's history actually contains, with how many + * times each happened. + * + * Built from the log rather than from `Action::cases()`: the enum has + * over eighty members and all but a handful can never appear against a + * file, so offering them all would be a dropdown you scroll past the + * answer in. What is here is what happened. + * + * @return list + */ + private function actionOptions(string $morphClass, int $subjectId): array + { + /** @var array $counts */ + $counts = ActivityLog::query() + ->where('subject_type', $morphClass) + ->where('subject_id', $subjectId) + ->selectRaw('action, count(*) as total') + ->groupBy('action') + ->pluck('total', 'action') + ->map(fn ($total): int => (int) $total) + ->all(); + + $options = []; + + foreach (self::ACTION_GROUPS as $key => $group) { + $present = array_filter($group['actions'], fn (Action $action): bool => isset($counts[$action->value])); + + // One member present means the group would filter to exactly + // what its member already offers, under a vaguer name. + if (count($present) < 2) { + continue; + } + + $options[] = [ + 'key' => $key, + 'label' => $group['label'], + 'count' => array_sum(array_map(fn (Action $action): int => $counts[$action->value], $present)), + ]; + } + + // Enum order, not count order, so the list does not rearrange + // itself under the reader every time the file is downloaded. + foreach (Action::cases() as $action) { + if (! isset($counts[$action->value])) { + continue; + } + + $options[] = [ + 'key' => $action->value, + 'label' => $action->description(), + 'count' => $counts[$action->value], + ]; + } + + return $options; + } + + /** + * @return array{action: ?string, actor: ?string, from: ?string, to: ?string} + */ + private function validatedHistoryFilters(Request $request): array + { + $validated = $request->validate([ + 'action' => ['nullable', Rule::in([ + ...array_keys(self::ACTION_GROUPS), + ...array_column(Action::cases(), 'value'), + ])], + 'actor' => ['nullable', 'string', 'max:255'], + 'from' => ['nullable', 'date'], + 'to' => ['nullable', 'date', 'after_or_equal:from'], + ]); + + return [ + 'action' => $validated['action'] ?? null, + 'actor' => $validated['actor'] ?? null, + 'from' => $validated['from'] ?? null, + 'to' => $validated['to'] ?? null, + ]; + } + + /** + * @param array{action: ?string, actor: ?string, from: ?string, to: ?string} $filters + * @return Builder + */ + private function historyQuery(string $morphClass, int $subjectId, array $filters, User $viewer): Builder + { + $timezone = $this->timezones->resolve($viewer); + + return ActivityLog::query() + ->where('subject_type', $morphClass) + ->where('subject_id', $subjectId) + ->when($filters['action'], function (Builder $query, string $action): void { + $group = self::ACTION_GROUPS[$action] ?? null; + + $group === null + ? $query->where('action', $action) + : $query->whereIn('action', array_map(fn (Action $member): string => $member->value, $group['actions'])); + }) + // Matched on the name snapshotted onto the entry, the same as + // the main log: an account deleted since is still findable by + // the name it acted under, which is the whole point of the + // snapshot. + ->when($filters['actor'], fn (Builder $query, string $actor) => $query->where('actor_name', 'like', "%{$actor}%")) + // The reader's own calendar day, not the UTC one — see LocalDay. + ->when( + $filters['from'] !== null ? LocalDay::start($filters['from'], $timezone) : null, + fn (Builder $query, Carbon $from) => $query->where('created_at', '>=', $from), + ) + ->when( + $filters['to'] !== null ? LocalDay::end($filters['to'], $timezone) : null, + fn (Builder $query, Carbon $to) => $query->where('created_at', '<=', $to), + ) + ->orderByDesc('created_at') + ->orderByDesc('id'); + } } diff --git a/app/Modules/Files/Http/Controllers/FilesController.php b/app/Modules/Files/Http/Controllers/FilesController.php index 8ab8d8c9..9d3e0736 100644 --- a/app/Modules/Files/Http/Controllers/FilesController.php +++ b/app/Modules/Files/Http/Controllers/FilesController.php @@ -206,6 +206,11 @@ class FilesController extends Controller 'can_update' => Gate::forUser($viewer)->allows('update', $file), 'can_delete' => Gate::forUser($viewer)->allows('delete', $file), 'can_manage_public' => $viewer->can('upload_public'), + // Whether this page offers its Activity tab. The file's own + // page is where somebody lands from a link, a search or a + // notification, so "what happened to this file" has to be + // answerable here and not only from the library's list. + 'can_view_activity' => $viewer->can('view_actions_log'), // The per-file switch only does anything while the comment // scope is `selected`; under every other value the page hides // it rather than offer a control with no current effect. diff --git a/resources/js/pages/activity/subject.tsx b/resources/js/pages/activity/subject.tsx index 44045b5d..caf92073 100644 --- a/resources/js/pages/activity/subject.tsx +++ b/resources/js/pages/activity/subject.tsx @@ -3,11 +3,15 @@ import { Head, Link } from '@inertiajs/react'; import { ArrowLeft } from 'lucide-react'; import Heading from '@/components/heading'; +import { FilterField, ListToolbar } from '@/components/list-toolbar'; import { Pagination, PaginationMeta } from '@/components/pagination'; import { TableShell } from '@/components/table-shell'; import { Badge } from '@/components/ui/badge'; import { Button } from '@/components/ui/button'; +import { Input } from '@/components/ui/input'; +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/components/ui/select'; import { useFormatDate } from '@/hooks/use-format-date'; +import { ALL, useListQuery } from '@/hooks/use-list-query'; import { useTranslation } from '@/hooks/use-translation'; import AppLayout from '@/layouts/app-layout'; import { activityActorLabel as actorLabel } from '@/lib/activity-actor'; @@ -23,17 +27,51 @@ interface ActivityEntry { replacements: Record; } +interface Filters { + action: string | null; + actor: string | null; + from: string | null; + to: string | null; +} + interface ActivitySubjectProps { entries: ActivityEntry[]; pagination: PaginationMeta; + filters: Filters; + /** Only the actions this subject's own history contains, with their counts. */ + action_options: { key: string; label: string; count: number }[]; subject_name: string; back_url: string; + /** This same page, for the filter links — a file's history and a folder's are different routes. */ + route_name: string; + route_params: Record; } -export default function ActivitySubject({ entries, pagination, subject_name, back_url }: ActivitySubjectProps) { +export default function ActivitySubject({ + entries, + pagination, + filters, + action_options, + subject_name, + back_url, + route_name, + route_params, +}: ActivitySubjectProps) { const { t } = useTranslation(); const { dateTime } = useFormatDate(); + const { values, set, reset, hasFilters } = useListQuery( + route_name, + { + action: filters.action ?? ALL, + actor: filters.actor ?? '', + from: filters.from ?? '', + to: filters.to ?? '', + }, + { action: ALL, actor: '', from: '', to: '' }, + route_params, + ); + const breadcrumbs: BreadcrumbItem[] = [ { title: t('Activity log'), href: '/activity' }, { title: subject_name, href: back_url }, @@ -45,7 +83,16 @@ export default function ActivitySubject({ entries, pagination, subject_name, bac
- + {/* Once a filter is on, "every recorded action" is no + longer what the table shows — say how many it does. */} +
+ + + + + + + set('actor', e.target.value, true)} + /> + + + + set('from', e.target.value)} /> + + + + set('to', e.target.value)} /> + + + {t('No activity recorded yet.')}} + emptyMessage={<>{hasFilters ? t('No activity matches these filters.') : t('No activity recorded yet.')}} > {entries.map((entry) => ( diff --git a/resources/js/pages/files/edit.tsx b/resources/js/pages/files/edit.tsx index dea11752..c70fc28e 100644 --- a/resources/js/pages/files/edit.tsx +++ b/resources/js/pages/files/edit.tsx @@ -1,7 +1,7 @@ import { type BreadcrumbItem } from '@/types'; import { Head, router, useForm, usePage } from '@inertiajs/react'; -import { Check, Copy, Download, X } from 'lucide-react'; -import { FormEventHandler, useState } from 'react'; +import { Check, Copy, Download, Loader2, X } from 'lucide-react'; +import { FormEventHandler, useEffect, useState } from 'react'; import { CommentThread } from '@/components/comments/comment-thread'; import { ConfirmDialog } from '@/components/confirm-dialog'; @@ -22,6 +22,7 @@ import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@ import { useFormatDate } from '@/hooks/use-format-date'; import { useTranslation } from '@/hooks/use-translation'; import AppLayout from '@/layouts/app-layout'; +import { activityActorLabel } from '@/lib/activity-actor'; import { categoryColor } from '@/lib/category-colors'; import { formatBytes } from '@/lib/format-bytes'; import { isThumbnailable } from '@/lib/thumbnails'; @@ -47,7 +48,19 @@ interface CategoryTag { color: string; } -type Tab = 'general' | 'sharing' | 'links' | 'versions' | 'comments'; +type Tab = 'general' | 'sharing' | 'links' | 'versions' | 'comments' | 'activity'; + +/** One line of this file's history, as /files/{id}/activity presents it. */ +interface ActivityEntry { + id: number; + created_at: string; + actor_name: string | null; + actor_type: string | null; + /** Separates an unauthenticated visitor from the scheduler; both have no actor. */ + origin: string; + template: string; + replacements: Record; +} interface FilesEditProps { file: { @@ -84,6 +97,7 @@ interface FilesEditProps { can_update: boolean; can_delete: boolean; can_manage_public: boolean; + can_view_activity: boolean; can_set_commentable: boolean; comments_enabled: boolean; assigned_clients: Named[]; @@ -109,6 +123,7 @@ export default function FilesEdit({ can_update, can_delete, can_manage_public, + can_view_activity, can_set_commentable, comments_enabled, assigned_clients, @@ -121,7 +136,7 @@ export default function FilesEdit({ can_limit_downloads, }: FilesEditProps) { const { t } = useTranslation(); - const { date } = useFormatDate(); + const { date, dateTime } = useFormatDate(); const pageErrors = usePage().props.errors as Record; // A comment notification and the activity log both land here, so honour // ?tab=comments. Somebody who may read the file but not edit it gets the @@ -130,8 +145,13 @@ export default function FilesEdit({ const requested = new URLSearchParams(window.location.search).get('tab'); if (requested === 'comments' && comments_enabled) return 'comments'; + if (requested === 'activity' && can_view_activity) return 'activity'; - return can_update ? 'general' : 'comments'; + if (can_update) return 'general'; + + // Whatever is left: somebody who may read the file but not edit it + // has at most these two, and Comments is the one they came for. + return comments_enabled ? 'comments' : 'activity'; }); const [target, setTarget] = useState(''); const [shareExpiresAt, setShareExpiresAt] = useState(''); @@ -145,6 +165,25 @@ export default function FilesEdit({ // (an already-populated slug counts as touched so we never clobber a // deliberate value). const [slugTouched, setSlugTouched] = useState(file.slug !== ''); + // The file's own history, fetched from the same endpoint the library's + // details panel reads — the most recent 20 entries plus how many there + // are in total, so the link below can say what "all" amounts to. Null + // until the tab has been opened: a page that nobody opens the tab on + // should not pay for the query. + const [activity, setActivity] = useState(null); + const [activityTotal, setActivityTotal] = useState(0); + + useEffect(() => { + if (tab !== 'activity' || activity !== null || !can_view_activity) return; + + fetch(route('files.activity', file.id), { headers: { Accept: 'application/json' }, credentials: 'same-origin' }) + .then((r) => r.json()) + .then((d) => { + setActivity(d.entries); + setActivityTotal(d.total); + }); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [tab]); const breadcrumbs: BreadcrumbItem[] = [ { title: t('All files'), href: '/files' }, @@ -278,12 +317,13 @@ export default function FilesEdit({
- {(can_update || comments_enabled) && ( + {(can_update || comments_enabled || can_view_activity) && ( )}
- {tab === 'comments' ? ( + {tab === 'activity' ? ( +
+ {activity === null ? ( +
+ {t('Loading…')} +
+ ) : activity.length === 0 ? ( +

{t('No activity recorded yet.')}

+ ) : ( +
+ {activity.map((entry) => ( +
+

+ {t(activityActorLabel(entry).key)}{' '} + {t(entry.template, entry.replacements)} +

+

{dateTime(entry.created_at)}

+
+ ))} +
+ )} + {/* Offered whenever there is any history at all, the + same as the library's details panel: the full + page is filterable and paged, not merely longer. */} + {activityTotal > 0 && ( + + )} +
+ ) : tab === 'comments' ? ( // The same thread the library's slide-over renders — // one component, so the two can never disagree about // what a comment looks like or who may write one. diff --git a/tests/Feature/Files/DetailsPanelTest.php b/tests/Feature/Files/DetailsPanelTest.php index 458be497..20488748 100644 --- a/tests/Feature/Files/DetailsPanelTest.php +++ b/tests/Feature/Files/DetailsPanelTest.php @@ -15,6 +15,7 @@ use App\Modules\Identity\Models\RolePermission; use App\Modules\Platform\Settings\Setting; use App\Modules\Platform\Settings\Settings; use Illuminate\Http\UploadedFile; +use Inertia\Testing\AssertableInertia; use Illuminate\Support\Facades\Storage; use Illuminate\Support\Str; @@ -236,3 +237,27 @@ test('a file with no limit says so rather than reporting a zero', function () { ->assertJsonPath('expires_at', null) ->assertJsonPath('expired', false); }); + +test("the file's own page carries an activity tab, behind the same permission", function () { + $this->actingAs($this->admin)->post('/files', [ + 'file' => UploadedFile::fake()->create('a.pdf', 10, 'application/pdf'), 'name' => '', 'description' => '', + ]); + $file = File::query()->sole(); + + $this->actingAs($this->admin)->get("/files/{$file->id}")->assertOk()->assertInertia( + fn (AssertableInertia $page) => $page->component('files/edit')->where('can_view_activity', true), + ); + + // The uploader of *this* file, but with no view_actions_log: the page + // still loads, without the tab that would show the log. + $role = Role::query()->create(['name' => 'No Log']); + foreach (['upload', 'edit_own_files'] as $permission) { + RolePermission::query()->create(['role_id' => $role->id, 'permission' => $permission]); + } + $noLog = User::factory()->create(['role_id' => $role->id]); + $file->update(['uploaded_by' => $noLog->id]); + + $this->actingAs($noLog)->get("/files/{$file->id}")->assertOk()->assertInertia( + fn (AssertableInertia $page) => $page->component('files/edit')->where('can_view_activity', false), + ); +}); diff --git a/tests/Feature/Files/FileActivityHistoryTest.php b/tests/Feature/Files/FileActivityHistoryTest.php index 5563a943..a4a02a64 100644 --- a/tests/Feature/Files/FileActivityHistoryTest.php +++ b/tests/Feature/Files/FileActivityHistoryTest.php @@ -197,3 +197,68 @@ test('the details panel downloads summary is bounded but reports the true total' ->and($response->json('downloaders'))->toHaveCount(1) ->and($response->json('downloaders.0.count'))->toBe(500); }); + +test('the file activity history filters by action, and offers only the actions that happened', function () { + $file = uploadImageFile($this->admin); + $client = User::factory()->client()->create(['name' => 'Downloading Client']); + + $log = function (Action $action, ?User $actor = null) use ($file): void { + ActivityLog::create([ + 'action' => $action, + 'subject_type' => $file->getMorphClass(), + 'subject_id' => $file->id, + 'subject_name' => $file->name, + 'actor_id' => $actor?->id, + 'actor_name' => $actor?->name, + 'actor_type' => $actor === null ? null : 'client', + 'created_at' => now(), + ]); + }; + + $log(Action::FileDownloaded, $client); + $log(Action::FileDownloaded, $client); + $log(Action::ShareLinkDownloaded); + $log(Action::FilePreviewed, $client); + + $options = $this->actingAs($this->admin)->get("/files/{$file->id}/activity/history") + ->assertInertia(fn (AssertableInertia $page) => $page->component('activity/subject')) + ->viewData('page')['props']['action_options']; + + $byKey = collect($options)->keyBy('key'); + + // The upload happened; the eighty-odd actions that cannot apply to a + // file are not offered at all. + expect($byKey->keys()->all())->toEqualCanonicalizing(['downloads', 'file.uploaded', 'file.downloaded', 'file.previewed', 'share_link.downloaded']) + ->and($byKey['downloads']['count'])->toBe(3) + ->and($byKey['file.downloaded']['count'])->toBe(2); + + // A single preview action means no "All previews" group: it would + // filter to exactly what its one member already offers. + expect($byKey->has('previews'))->toBeFalse(); + + // The group covers all three download flavours, one of them anonymous. + $this->actingAs($this->admin)->get("/files/{$file->id}/activity/history?action=downloads")->assertInertia( + fn (AssertableInertia $page) => $page->has('entries', 3)->where('filters.action', 'downloads'), + ); + + $this->actingAs($this->admin)->get("/files/{$file->id}/activity/history?action=file.previewed")->assertInertia( + fn (AssertableInertia $page) => $page->has('entries', 1)->where('entries.0.actor_name', 'Downloading Client'), + ); + + // Narrowing by who acted, and by day, works the same as the main log. + $this->actingAs($this->admin)->get("/files/{$file->id}/activity/history?actor=Downloading")->assertInertia( + fn (AssertableInertia $page) => $page->has('entries', 3), + ); + + $this->actingAs($this->admin)->get("/files/{$file->id}/activity/history?from=".now()->addDay()->toDateString())->assertInertia( + fn (AssertableInertia $page) => $page->has('entries', 0), + ); + + // An action that exists but never touched this file filters to nothing + // rather than erroring; a value that is neither action nor group is + // rejected outright. + $this->actingAs($this->admin)->get("/files/{$file->id}/activity/history?action=user.created")->assertInertia( + fn (AssertableInertia $page) => $page->has('entries', 0), + ); + $this->actingAs($this->admin)->get("/files/{$file->id}/activity/history?action=nonsense")->assertSessionHasErrors('action'); +}); From 7d1903f9db40b655345eba1e9ce79c5ab39c53a6 Mon Sep 17 00:00:00 2001 From: ignacionelson Date: Fri, 21 Aug 2026 12:39:22 -0300 Subject: [PATCH 2/3] Let the download history be searched MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The installation-wide download history listed every download newest first and offered nothing else, so "did that client ever actually download the contract?" meant paging through everything that had happened since. It now filters by file name, by who downloaded it, and by date range, in the same toolbar every other list uses: the query string carries the filters, so a narrowed view is a link somebody can be sent. Both names are matched against what the entry snapshotted rather than through a join, so a file or an account deleted since is still findable by the name it went out under — often exactly what this page is being asked. The filters narrow the viewer's already-scoped query rather than replacing it, so a client-scoped staffer cannot search their way to a download of a file outside their library. --- .../Http/Controllers/DownloadsController.php | 77 ++++++++++++++++--- resources/js/pages/activity/downloads.tsx | 68 +++++++++++++++- tests/Feature/Audit/DownloadsTest.php | 66 ++++++++++++++++ 3 files changed, 199 insertions(+), 12 deletions(-) diff --git a/app/Modules/Audit/Http/Controllers/DownloadsController.php b/app/Modules/Audit/Http/Controllers/DownloadsController.php index 91059f1b..e368f360 100644 --- a/app/Modules/Audit/Http/Controllers/DownloadsController.php +++ b/app/Modules/Audit/Http/Controllers/DownloadsController.php @@ -5,12 +5,17 @@ declare(strict_types=1); namespace App\Modules\Audit\Http\Controllers; use App\Http\Controllers\Controller; +use App\Models\User; use App\Modules\Audit\Action; use App\Modules\Audit\ActivityLog; use App\Modules\Audit\ActivityLogScope; use App\Modules\Audit\DownloadPresenter; use App\Modules\Files\Models\File; +use App\Modules\Platform\Localization\LocalDay; +use App\Modules\Platform\Localization\TimezoneRegistry; use App\Support\Pagination; +use Carbon\Carbon; +use Illuminate\Database\Eloquent\Builder; use Illuminate\Http\Request; use Inertia\Inertia; use Inertia\Response; @@ -27,6 +32,7 @@ class DownloadsController extends Controller public function __construct( private readonly DownloadPresenter $presenter, private readonly ActivityLogScope $scope, + private readonly TimezoneRegistry $timezones, ) {} public function index(Request $request): Response @@ -34,15 +40,9 @@ class DownloadsController extends Controller $viewer = $request->user(); assert($viewer !== null); - // A download row names the file and says who fetched it from which - // IP, so it needs the viewer's library scope applied — not just - // `view_actions_log`. See ActivityLogScope for the full reasoning. - $entries = $this->scope - ->apply(ActivityLog::query(), $viewer) - ->where('subject_type', (new File)->getMorphClass()) - ->whereIn('action', [Action::FileDownloaded, Action::ShareLinkDownloaded, Action::PublicFileDownloaded]) - ->orderByDesc('created_at') - ->orderByDesc('id') + $filters = $this->validatedFilters($request); + + $entries = $this->filteredQuery($filters, $viewer) ->paginate(25) ->withQueryString(); @@ -63,6 +63,65 @@ class DownloadsController extends Controller ]; })->all(), 'pagination' => Pagination::meta($entries), + 'filters' => $filters, ]); } + + /** + * @return array{file: ?string, user: ?string, from: ?string, to: ?string} + */ + private function validatedFilters(Request $request): array + { + $validated = $request->validate([ + 'file' => ['nullable', 'string', 'max:255'], + 'user' => ['nullable', 'string', 'max:255'], + 'from' => ['nullable', 'date'], + 'to' => ['nullable', 'date', 'after_or_equal:from'], + ]); + + return [ + 'file' => $validated['file'] ?? null, + 'user' => $validated['user'] ?? null, + 'from' => $validated['from'] ?? null, + 'to' => $validated['to'] ?? null, + ]; + } + + /** + * @param array{file: ?string, user: ?string, from: ?string, to: ?string} $filters + * @return Builder + */ + private function filteredQuery(array $filters, User $viewer): Builder + { + $timezone = $this->timezones->resolve($viewer); + + // A download row names the file and says who fetched it from which + // IP, so it needs the viewer's library scope applied — not just + // `view_actions_log`. See ActivityLogScope for the full reasoning. + return $this->scope + ->apply(ActivityLog::query(), $viewer) + ->where('subject_type', (new File)->getMorphClass()) + ->whereIn('action', [Action::FileDownloaded, Action::ShareLinkDownloaded, Action::PublicFileDownloaded]) + // Both names are matched on what the entry snapshotted, not on + // a join: a file or an account deleted since is still findable + // by the name it went out under, which is often exactly what + // this page is being asked. + ->when($filters['file'], fn (Builder $query, string $file) => $query->where('subject_name', 'like', "%{$file}%")) + // Only rows with a real account can match a name. The two + // anonymous flavours ("Public link", "Public listing") are + // labels this page prints, not stored values, so a search for + // them finds nothing rather than something arbitrary. + ->when($filters['user'], fn (Builder $query, string $user) => $query->where('actor_name', 'like', "%{$user}%")) + // The viewer's own calendar day, not the UTC one — see LocalDay. + ->when( + $filters['from'] !== null ? LocalDay::start($filters['from'], $timezone) : null, + fn (Builder $query, Carbon $from) => $query->where('created_at', '>=', $from), + ) + ->when( + $filters['to'] !== null ? LocalDay::end($filters['to'], $timezone) : null, + fn (Builder $query, Carbon $to) => $query->where('created_at', '<=', $to), + ) + ->orderByDesc('created_at') + ->orderByDesc('id'); + } } diff --git a/resources/js/pages/activity/downloads.tsx b/resources/js/pages/activity/downloads.tsx index 30218c1d..74b5bf6d 100644 --- a/resources/js/pages/activity/downloads.tsx +++ b/resources/js/pages/activity/downloads.tsx @@ -3,10 +3,13 @@ import { Head, Link } from '@inertiajs/react'; import { ArrowLeft } from 'lucide-react'; import Heading from '@/components/heading'; +import { FilterField, ListToolbar } from '@/components/list-toolbar'; import { Pagination, PaginationMeta } from '@/components/pagination'; import { Badge } from '@/components/ui/badge'; import { Button } from '@/components/ui/button'; +import { Input } from '@/components/ui/input'; import { useFormatDate } from '@/hooks/use-format-date'; +import { useListQuery } from '@/hooks/use-list-query'; import { useTranslation } from '@/hooks/use-translation'; import AppLayout from '@/layouts/app-layout'; @@ -20,9 +23,19 @@ interface DownloadEntry { file_url?: string | null; } +interface Filters { + file: string | null; + user: string | null; + from: string | null; + to: string | null; +} + interface ActivityDownloadsProps { entries: DownloadEntry[]; pagination: PaginationMeta; + // Only the installation-wide page filters: a single file's history is + // already narrowed to the one thing its filters would ask about. + filters?: Filters; // Present only when scoped to a single file/folder (the details // panel's "View all downloads" destination). Absent for the // installation-wide /downloads page, which shows a File column @@ -31,12 +44,23 @@ interface ActivityDownloadsProps { back_url?: string; } -export default function ActivityDownloads({ entries, pagination, subject_name, back_url }: ActivityDownloadsProps) { +export default function ActivityDownloads({ entries, pagination, filters, subject_name, back_url }: ActivityDownloadsProps) { const { t } = useTranslation(); const { dateTime } = useFormatDate(); const scoped = subject_name !== undefined && back_url !== undefined; + const { values, set, reset, hasFilters } = useListQuery( + 'downloads.index', + { + file: filters?.file ?? '', + user: filters?.user ?? '', + from: filters?.from ?? '', + to: filters?.to ?? '', + }, + { file: '', user: '', from: '', to: '' }, + ); + const breadcrumbs: BreadcrumbItem[] = scoped ? [ { title: t('Activity log'), href: '/activity' }, @@ -45,7 +69,11 @@ export default function ActivityDownloads({ entries, pagination, subject_name, b : [{ title: t('Download history'), href: '/downloads' }]; const title = scoped ? t('Download history for :name', { name: subject_name }) : t('Download history'); - const description = scoped ? t('Every download, newest first') : t('Every download across the installation, newest first'); + const description = scoped + ? t('Every download, newest first') + : hasFilters + ? t(':count matching this filter, newest first', { count: pagination.total }) + : t('Every download across the installation, newest first'); return ( @@ -64,6 +92,40 @@ export default function ActivityDownloads({ entries, pagination, subject_name, b )}
+ {filters !== undefined && ( + + + set('file', e.target.value, true)} + /> + + + + set('user', e.target.value, true)} + /> + + + + set('from', e.target.value)} /> + + + + set('to', e.target.value)} /> + + + )} +
@@ -78,7 +140,7 @@ export default function ActivityDownloads({ entries, pagination, subject_name, b {entries.length === 0 && ( )} diff --git a/tests/Feature/Audit/DownloadsTest.php b/tests/Feature/Audit/DownloadsTest.php index 6fa6c287..1b5e25c5 100644 --- a/tests/Feature/Audit/DownloadsTest.php +++ b/tests/Feature/Audit/DownloadsTest.php @@ -178,3 +178,69 @@ test('a client-scoped viewer does see entries for their own clients and their ow // permission alone, so it never points at a 403. ->and($rows->firstWhere('file_name', 'quarterly-report')['file_url'])->not->toBeNull(); }); + +test('the installation-wide download history filters by file, by account and by date', function () { + $report = uploadImageFile($this->admin, 'quarterly-report.jpg'); + $photo = uploadImageFile($this->admin, 'holiday-photo.jpg'); + $client = User::factory()->client()->create(['name' => 'Acme Design']); + + $log = function ($file, ?User $actor, string $when) { + ActivityLog::create([ + 'action' => $actor === null ? Action::ShareLinkDownloaded : Action::FileDownloaded, + 'subject_type' => $file->getMorphClass(), + 'subject_id' => $file->id, + 'subject_name' => $file->name, + 'actor_id' => $actor?->id, + 'actor_name' => $actor?->name, + 'actor_type' => $actor === null ? null : 'client', + 'created_at' => $when, + ]); + }; + + $log($report, $client, '2026-08-10 09:00:00'); + $log($report, $this->admin, '2026-08-12 09:00:00'); + $log($photo, $client, '2026-08-14 09:00:00'); + // Anonymous: no account name to match, so the user filter can never + // return it — which is what the column already says on screen. + $log($photo, null, '2026-08-16 09:00:00'); + + $entries = fn (string $query) => $this->actingAs($this->admin)->get("/downloads{$query}")->assertOk(); + + $entries('?file=report')->assertInertia( + fn (AssertableInertia $page) => $page->has('entries', 2)->where('filters.file', 'report'), + ); + + $entries('?user=Acme')->assertInertia( + fn (AssertableInertia $page) => $page->has('entries', 2)->where('entries.0.actor_name', 'Acme Design'), + ); + + // Both filters at once narrow to the one row that satisfies each. + $entries('?file=report&user=Acme')->assertInertia(fn (AssertableInertia $page) => $page->has('entries', 1)); + + $entries('?from=2026-08-13')->assertInertia(fn (AssertableInertia $page) => $page->has('entries', 2)); + $entries('?to=2026-08-11')->assertInertia(fn (AssertableInertia $page) => $page->has('entries', 1)); + $entries('?from=2026-08-11&to=2026-08-15')->assertInertia(fn (AssertableInertia $page) => $page->has('entries', 2)); + + // A range that ends before it starts is rejected rather than silently + // returning nothing. + $this->actingAs($this->admin)->get('/downloads?from=2026-08-15&to=2026-08-11')->assertSessionHasErrors('to'); +}); + +test('a client-scoped viewer cannot widen the download history with a filter', function () { + $secret = uploadImageFile($this->admin, 'board-minutes-confidential.jpg'); + $this->actingAs($this->admin)->get("/files/{$secret->id}/download")->assertOk(); + + $manager = User::factory()->role(SystemRole::ClientManager)->create(); + $manager->assignedClients()->sync([]); + + // Out of this viewer's library scope, so it stays invisible however + // precisely it is searched for: the filters narrow the scoped query, + // they do not replace it. + $this->actingAs($manager)->get('/downloads?file=board-minutes')->assertInertia( + fn (AssertableInertia $page) => $page->has('entries', 0), + ); + + $this->actingAs($this->admin)->get('/downloads?file=board-minutes')->assertInertia( + fn (AssertableInertia $page) => $page->has('entries', 1), + ); +}); From cca3d9c314a7650867a4d95bed2f5f27b60904a5 Mon Sep 17 00:00:00 2001 From: ignacionelson Date: Fri, 21 Aug 2026 13:12:26 -0300 Subject: [PATCH 3/3] Group only the downloads, which are the actions that need it The grouped filter had a second member, "All previews", built on a public-preview action that does not exist: previewing is recorded one way today, so its own option already answers "who previewed this?" in full. Static analysis caught the reference; the group would have been unreachable even if it had compiled, since a group with a single present member is deliberately not offered. Previews get a group here the day a second way to preview a file is recorded separately, and the test now pins the single-member case on a file whose log holds one flavour of download. --- .../Controllers/FileDetailsController.php | 14 ++++++------- .../Feature/Files/FileActivityHistoryTest.php | 21 ++++++++++++++++--- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/app/Modules/Files/Http/Controllers/FileDetailsController.php b/app/Modules/Files/Http/Controllers/FileDetailsController.php index d9af023a..3c1340ed 100644 --- a/app/Modules/Files/Http/Controllers/FileDetailsController.php +++ b/app/Modules/Files/Http/Controllers/FileDetailsController.php @@ -47,20 +47,20 @@ class FileDetailsController extends Controller * somebody following a public link and a visitor to a public group * listing are recorded separately, on purpose, because *how* a file * left matters. But nobody reading a file's history wants to ask the - * question three times, so these offer it once — and are still only - * shown when the file's own log has more than one of the members. + * question three times, so this offers it once — and only when the + * file's own log holds more than one of the members, since otherwise + * it would filter to exactly what its single member already offers. * - * @var array}> + * Previewing has one action today, so it needs no group; give it one + * here if a second way to preview a file is ever recorded separately. + * + * @var array}> */ private const ACTION_GROUPS = [ 'downloads' => [ 'label' => 'All downloads', 'actions' => [Action::FileDownloaded, Action::ShareLinkDownloaded, Action::PublicFileDownloaded], ], - 'previews' => [ - 'label' => 'All previews', - 'actions' => [Action::FilePreviewed, Action::PublicFilePreviewed], - ], ]; public function __construct( diff --git a/tests/Feature/Files/FileActivityHistoryTest.php b/tests/Feature/Files/FileActivityHistoryTest.php index a4a02a64..e2f9e146 100644 --- a/tests/Feature/Files/FileActivityHistoryTest.php +++ b/tests/Feature/Files/FileActivityHistoryTest.php @@ -232,9 +232,24 @@ test('the file activity history filters by action, and offers only the actions t ->and($byKey['downloads']['count'])->toBe(3) ->and($byKey['file.downloaded']['count'])->toBe(2); - // A single preview action means no "All previews" group: it would - // filter to exactly what its one member already offers. - expect($byKey->has('previews'))->toBeFalse(); + // A file whose log holds only one flavour of download gets no group: + // it would filter to exactly what its single member already offers. + $simple = uploadImageFile($this->admin, 'simple.jpg'); + ActivityLog::create([ + 'action' => Action::FileDownloaded, + 'subject_type' => $simple->getMorphClass(), + 'subject_id' => $simple->id, + 'subject_name' => $simple->name, + 'actor_id' => $this->admin->id, + 'actor_name' => $this->admin->name, + 'actor_type' => 'staff', + 'created_at' => now(), + ]); + + $simpleOptions = $this->actingAs($this->admin)->get("/files/{$simple->id}/activity/history") + ->viewData('page')['props']['action_options']; + + expect(collect($simpleOptions)->pluck('key')->all())->toEqualCanonicalizing(['file.uploaded', 'file.downloaded']); // The group covers all three download flavours, one of them anonymous. $this->actingAs($this->admin)->get("/files/{$file->id}/activity/history?action=downloads")->assertInertia(
- {t('No downloads recorded yet.')} + {hasFilters ? t('No downloads match these filters.') : t('No downloads recorded yet.')}