From d445b01dd458f6ed60634135d6a910a0dd67f4f0 Mon Sep 17 00:00:00 2001 From: ignacionelson Date: Wed, 16 Sep 2026 22:52:08 -0300 Subject: [PATCH] Make starting a scan a button in the header, and drop the box it lived in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "New scan" sits beside Quarantine as the screen's one action, in the primary colour, on every tab. The "Files already here" box it replaces is gone: it held an action under a Save button, a counter that the Activity tab now shows live, and a field that belongs with the other settings, which is where it is now. The button says why when it cannot be pressed — scanning is off, every file has already been checked, or a scan is already running — rather than sitting grey with no explanation. It refuses a second scan while one is working through the queue, which is a thing somebody would otherwise do by clicking twice. Starting one lands on the Activity tab. A button whose screen looks unchanged afterwards reads as a button that did nothing, and this one has somewhere worth looking. Checked against a real ClamAV: 42 files queued, 31 came back clean, the rest were the ones whose bytes are missing. The button then disabled itself, because there was nothing left to scan. --- .../VirusScanningSettingsController.php | 13 +++- .../pages/system/settings/virus-scanning.tsx | 62 +++++++++---------- .../Files/VirusScanningSettingsTest.php | 19 ++++++ 3 files changed, 59 insertions(+), 35 deletions(-) diff --git a/app/Modules/Files/Http/Controllers/VirusScanningSettingsController.php b/app/Modules/Files/Http/Controllers/VirusScanningSettingsController.php index 74fd9542..e472a22b 100644 --- a/app/Modules/Files/Http/Controllers/VirusScanningSettingsController.php +++ b/app/Modules/Files/Http/Controllers/VirusScanningSettingsController.php @@ -18,6 +18,7 @@ use App\Modules\Platform\Capabilities\CapabilityRegistry; use App\Modules\Platform\Settings\Setting; use App\Modules\Platform\Settings\Settings; use Illuminate\Http\JsonResponse; +use Illuminate\Support\Facades\Artisan; use Illuminate\Http\RedirectResponse; use Illuminate\Http\Request; use Illuminate\Support\Facades\Queue; @@ -188,9 +189,14 @@ class VirusScanningSettingsController extends Controller { abort_unless($this->config->enabled(), 422); - \Illuminate\Support\Facades\Artisan::queue('projectsend:scan-files', ['--existing' => true]); + Artisan::queue('projectsend:scan-files', ['--existing' => true]); - return back()->with('success', __('Scanning existing files has started. It runs in the background.')); + // Onto the tab that shows it happening rather than back where they + // were: somebody who just started a scan wants to watch it, and a + // screen that looks unchanged reads as a button that did nothing. + return redirect() + ->route('system-settings.virus-scanning.edit', ['tab' => 'activity']) + ->with('success', __('The scan has started.')); } /** @@ -294,6 +300,9 @@ class VirusScanningSettingsController extends Controller NotScannedReason::Encrypted->value, ]) ->count(), + // So the New scan button can refuse a second scan while one is + // still working through the queue. + 'queued' => Queue::size('scans'), ]; } diff --git a/resources/js/pages/system/settings/virus-scanning.tsx b/resources/js/pages/system/settings/virus-scanning.tsx index 81f8d4d0..f806338a 100644 --- a/resources/js/pages/system/settings/virus-scanning.tsx +++ b/resources/js/pages/system/settings/virus-scanning.tsx @@ -98,15 +98,35 @@ export default function VirusScanningSettings({ it — the quarantine screen answers 403 otherwise, and a button that leads to a refusal is worse than no button. */} - {auth.permissions.includes('release_quarantined_files') && ( - + )} + + {/* The screen's one action, where an action belongs. + Disabled rather than hidden when there is nothing + to do, so it can say why. */} + - )} + {counts.let_through > 0 && ( @@ -214,30 +234,6 @@ export default function VirusScanningSettings({ {tab === 'options' && (
-
- - -

- {t('Never scanned: :never · Being checked: :pending · In quarantine: :quarantined', { - never: counts.never_scanned, - pending: counts.pending, - quarantined: counts.quarantined, - })} -

- - -
-
@@ -325,7 +321,7 @@ export default function VirusScanningSettings({ onChange={(e) => setData('existing_rate_per_minute', Number(e.target.value))} />

- {t('Applies to the button above, so a backfill does not starve the scanner of new uploads.')} + {t('The pace of a New scan, so working through a whole library does not starve the scanner of new uploads.')}

diff --git a/tests/Feature/Files/VirusScanningSettingsTest.php b/tests/Feature/Files/VirusScanningSettingsTest.php index 7b3cd7c1..ea0311ee 100644 --- a/tests/Feature/Files/VirusScanningSettingsTest.php +++ b/tests/Feature/Files/VirusScanningSettingsTest.php @@ -389,3 +389,22 @@ test('a backfill counts as running even though it holds nothing back', function ->and($body['queued'])->toBe(1) ->and($body['running'])->toBeTrue(); }); + +test('starting a scan lands on the tab that shows it happening', function () { + // A button whose screen looks unchanged afterwards reads as a button + // that did nothing. + app(Settings::class)->set(Setting::VirusScanningEnabled, true); + app(Settings::class)->set(Setting::VirusScannerAddress, 'tcp://scanner.test:3310'); + + $this->actingAs($this->admin)->post('/system/settings/virus-scanning/scan-existing') + ->assertRedirect(route('system-settings.virus-scanning.edit', ['tab' => 'activity'])); +}); + +test('the screen says whether a scan is already under way', function () { + Illuminate\Support\Facades\Queue::fake(); + App\Modules\Files\Jobs\ScanFileJob::dispatch(1, true); + + $this->actingAs($this->admin)->get('/system/settings/virus-scanning')->assertInertia( + fn (AssertableInertia $page) => $page->where('counts.queued', 1), + ); +});