From 22b31f567149faf3db865cd0791c1867a38fc166 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 1 Jul 2026 11:14:20 +0200 Subject: [PATCH] fix(backups): require valid S3 storage selection Preserve S3 backups when a single valid storage is available, require explicit selection when multiple storages exist, and disable S3 when none are available. Make backup action controls responsive on narrow screens. --- app/Livewire/Project/Database/BackupEdit.php | 37 ++++++++-- .../components/modal-confirmation.blade.php | 6 +- .../project/database/backup-edit.blade.php | 59 ++++++++------- .../database/backup-executions.blade.php | 26 ++++--- .../database/backup/execution.blade.php | 2 +- .../database/scheduled-backups.blade.php | 2 +- .../views/livewire/settings-backup.blade.php | 2 +- tests/Feature/BackupEditValidationTest.php | 73 ++++++++++++++++++- 8 files changed, 157 insertions(+), 50 deletions(-) diff --git a/app/Livewire/Project/Database/BackupEdit.php b/app/Livewire/Project/Database/BackupEdit.php index 99426c120..a387b6f88 100644 --- a/app/Livewire/Project/Database/BackupEdit.php +++ b/app/Livewire/Project/Database/BackupEdit.php @@ -3,10 +3,12 @@ namespace App\Livewire\Project\Database; use App\Jobs\DatabaseBackupJob; +use App\Models\S3Storage; use App\Models\ScheduledDatabaseBackup; use App\Models\ServiceDatabase; use Exception; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; +use Illuminate\Support\Collection; use Livewire\Attributes\Locked; use Livewire\Attributes\Validate; use Livewire\Component; @@ -18,7 +20,7 @@ class BackupEdit extends Component public ScheduledDatabaseBackup $backup; #[Locked] - public $s3s; + public $availableS3Storages; #[Locked] public $parameters; @@ -69,7 +71,7 @@ class BackupEdit extends Component public bool $disableLocalBackup = false; #[Validate(['nullable', 'integer'])] - public ?int $s3StorageId = 1; + public ?int $s3StorageId = null; #[Validate(['nullable', 'string'])] public ?string $databasesToBackup = null; @@ -222,10 +224,18 @@ private function customValidate() } // S3 backup cannot be enabled without a valid S3 storage owned by the team - $availableS3Ids = collect($this->s3s)->pluck('id'); + $availableS3Ids = $this->availableS3StorageIds(); if ($this->backup->save_s3 && ! $availableS3Ids->contains($this->backup->s3_storage_id)) { - $this->backup->save_s3 = $this->saveS3 = false; - $this->backup->s3_storage_id = $this->s3StorageId = null; + if ($availableS3Ids->isEmpty()) { + $this->backup->s3_storage_id = $this->s3StorageId = null; + $this->backup->save_s3 = $this->saveS3 = false; + } elseif ($this->backup->s3_storage_id === null && $availableS3Ids->count() === 1) { + $this->backup->s3_storage_id = $this->s3StorageId = $availableS3Ids->first(); + } else { + $this->backup->s3_storage_id = $this->s3StorageId = null; + + throw new Exception('Please select a valid S3 storage to enable S3 backups.'); + } } // Validate that disable_local_backup can only be true when S3 backup is enabled @@ -240,6 +250,23 @@ private function customValidate() $this->validate(); } + private function availableS3StorageIds(): Collection + { + $storages = collect($this->availableS3Storages); + $storageIds = $storages->pluck('id')->filter()->all(); + $teamIds = $storages->pluck('team_id')->reject(fn ($teamId) => $teamId === null)->unique()->values()->all(); + + if (empty($storageIds) || empty($teamIds)) { + return collect(); + } + + return S3Storage::query() + ->whereKey($storageIds) + ->whereIn('team_id', $teamIds) + ->where('is_usable', true) + ->pluck('id'); + } + public function submit() { try { diff --git a/resources/views/components/modal-confirmation.blade.php b/resources/views/components/modal-confirmation.blade.php index 4629e3b96..a25e0141f 100644 --- a/resources/views/components/modal-confirmation.blade.php +++ b/resources/views/components/modal-confirmation.blade.php @@ -129,7 +129,11 @@ } }" @keydown.escape.window="if (modalOpen) { modalOpen = false; resetModal(); }" :class="{ 'z-40': modalOpen }" - class="relative w-auto h-auto"> + @class([ + 'relative h-auto', + 'w-full' => $buttonFullWidth, + 'w-auto' => ! $buttonFullWidth, + ])> @if (isset($trigger))
{{ $trigger }} diff --git a/resources/views/livewire/project/database/backup-edit.blade.php b/resources/views/livewire/project/database/backup-edit.blade.php index 8a4b89e5b..edea7b80f 100644 --- a/resources/views/livewire/project/database/backup-edit.blade.php +++ b/resources/views/livewire/project/database/backup-edit.blade.php @@ -1,32 +1,37 @@
-
+

Scheduled Backup

- - Save - - @if (str($status)->startsWith('running')) - Backup Now - @endif - @if ($backup->database_id !== 0) - - @endif +
+ + Save + + @if (str($status)->startsWith('running')) + Backup Now + @endif + @if ($backup->database_id !== 0) +
+ +
+ @endif +
-
+
- @if ($s3s->count() > 0) + @if ($availableS3Storages->count() > 0) @else @endif - @if ($backup->save_s3) + @if ($saveS3) @else @@ -34,11 +39,11 @@ helper="When enabled, backup files will be deleted from local storage immediately after uploading to S3. This requires S3 backup to be enabled." /> @endif
- @if ($backup->save_s3) -
+ @if ($saveS3) +
- @foreach ($s3s as $s3) + @foreach ($availableS3Storages as $s3) @endforeach @@ -80,7 +85,7 @@ @endif @endif
-
+
@@ -98,7 +103,7 @@

Local Backup Retention

-
+
@@ -111,10 +116,10 @@
- @if ($backup->save_s3) + @if ($saveS3)

S3 Storage Retention

-
+
diff --git a/resources/views/livewire/project/database/backup-executions.blade.php b/resources/views/livewire/project/database/backup-executions.blade.php index b6d88a2fd..15d42a7a5 100644 --- a/resources/views/livewire/project/database/backup-executions.blade.php +++ b/resources/views/livewire/project/database/backup-executions.blade.php @@ -1,7 +1,7 @@
@isset($backup) -
-

Executions ({{ $executions_count }})

+
+

Executions ({{ $executions_count }})

@if ($executions_count > 0)
@@ -21,13 +21,15 @@
@endif - Cleanup Failed Backups - +
+ Cleanup Failed Backups + +
@@ -87,7 +89,7 @@ class="flex flex-col gap-4">
Location: {{ data_get($execution, 'filename', 'N/A') }}
-
+
Backup Availability:
@@ -154,9 +156,9 @@ class="flex flex-col gap-4"> 
{{ data_get($execution, 'message') }}
@endif -
+
@if (data_get($execution, 'status') === 'success') - Download @endif @php diff --git a/resources/views/livewire/project/database/backup/execution.blade.php b/resources/views/livewire/project/database/backup/execution.blade.php index 3e689645f..23c108e8c 100644 --- a/resources/views/livewire/project/database/backup/execution.blade.php +++ b/resources/views/livewire/project/database/backup/execution.blade.php @@ -6,7 +6,7 @@
- +
diff --git a/resources/views/livewire/project/database/scheduled-backups.blade.php b/resources/views/livewire/project/database/scheduled-backups.blade.php index 12b36ffa1..b8241569c 100644 --- a/resources/views/livewire/project/database/scheduled-backups.blade.php +++ b/resources/views/livewire/project/database/scheduled-backups.blade.php @@ -216,7 +216,7 @@ class="px-3 py-1 rounded-md text-xs font-medium tracking-wide shadow-xs bg-gray- @if ($type === 'service-database' && $selectedBackup)
+ :available-s3-storages="$s3s" :status="data_get($database, 'status')" />
diff --git a/resources/views/livewire/settings-backup.blade.php b/resources/views/livewire/settings-backup.blade.php index 0d2fd6ceb..10fea55b7 100644 --- a/resources/views/livewire/settings-backup.blade.php +++ b/resources/views/livewire/settings-backup.blade.php @@ -27,7 +27,7 @@
- +
diff --git a/tests/Feature/BackupEditValidationTest.php b/tests/Feature/BackupEditValidationTest.php index 8894f0f69..4bd39d2c3 100644 --- a/tests/Feature/BackupEditValidationTest.php +++ b/tests/Feature/BackupEditValidationTest.php @@ -4,6 +4,7 @@ use App\Models\Environment; use App\Models\InstanceSettings; use App\Models\Project; +use App\Models\S3Storage; use App\Models\ScheduledDatabaseBackup; use App\Models\Server; use App\Models\StandaloneDocker; @@ -43,6 +44,20 @@ function createBackupForEditValidationTest(Team $team, array $overrides = []): S ], $overrides)); } +function createS3StorageForBackupEditValidationTest(Team|int $team, string $name = 'Backup Edit S3'): S3Storage +{ + return S3Storage::create([ + 'name' => $name, + 'region' => 'us-east-1', + 'key' => 'test-key', + 'secret' => 'test-secret', + 'bucket' => 'test-bucket', + 'endpoint' => 'https://s3.example.com', + 'is_usable' => true, + 'team_id' => $team instanceof Team ? $team->id : $team, + ]); +} + beforeEach(function () { if (InstanceSettings::find(0) === null) { $settings = new InstanceSettings; @@ -60,7 +75,7 @@ function createBackupForEditValidationTest(Team $team, array $overrides = []): S it('disables S3 backup when saved without a selected S3 storage', function () { $backup = createBackupForEditValidationTest($this->team); - Livewire::test(BackupEdit::class, ['backup' => $backup->fresh(), 's3s' => $this->team->s3s]) + Livewire::test(BackupEdit::class, ['backup' => $backup->fresh(), 'availableS3Storages' => $this->team->s3s]) ->call('submit') ->assertDispatched('success'); @@ -74,7 +89,7 @@ function createBackupForEditValidationTest(Team $team, array $overrides = []): S 'disable_local_backup' => true, ]); - Livewire::test(BackupEdit::class, ['backup' => $backup->fresh(), 's3s' => $this->team->s3s]) + Livewire::test(BackupEdit::class, ['backup' => $backup->fresh(), 'availableS3Storages' => $this->team->s3s]) ->call('submit') ->assertDispatched('success'); @@ -83,3 +98,57 @@ function createBackupForEditValidationTest(Team $team, array $overrides = []): S expect($backup->s3_storage_id)->toBeNull(); expect($backup->disable_local_backup)->toBeFalsy(); }); + +it('keeps S3 enabled by selecting the only available team storage when none is selected yet', function () { + createS3StorageForBackupEditValidationTest(Team::factory()->create()); + $s3 = createS3StorageForBackupEditValidationTest($this->team); + $backup = createBackupForEditValidationTest($this->team, [ + 'save_s3' => false, + 's3_storage_id' => null, + ]); + + Livewire::test(BackupEdit::class, ['backup' => $backup->fresh(), 'availableS3Storages' => $this->team->s3s]) + ->set('saveS3', true) + ->call('instantSave') + ->assertDispatched('success'); + + $backup->refresh(); + expect($backup->save_s3)->toBeTruthy(); + expect($backup->s3_storage_id)->toBe($s3->id); +}); + +it('requires an explicit S3 selection when multiple storages are available', function () { + createS3StorageForBackupEditValidationTest($this->team, 'First S3'); + createS3StorageForBackupEditValidationTest($this->team, 'Second S3'); + $backup = createBackupForEditValidationTest($this->team, [ + 'save_s3' => false, + 's3_storage_id' => null, + ]); + + Livewire::test(BackupEdit::class, ['backup' => $backup->fresh(), 'availableS3Storages' => $this->team->s3s]) + ->set('saveS3', true) + ->call('instantSave') + ->assertDispatched('error'); + + $backup->refresh(); + expect($backup->save_s3)->toBeFalsy(); + expect($backup->s3_storage_id)->toBeNull(); +}); + +it('accepts the S3 storage scope passed to the component', function () { + $s3 = createS3StorageForBackupEditValidationTest(0); + $backup = createBackupForEditValidationTest($this->team, [ + 'save_s3' => false, + 's3_storage_id' => null, + ]); + + Livewire::test(BackupEdit::class, ['backup' => $backup->fresh(), 'availableS3Storages' => collect([$s3])]) + ->set('saveS3', true) + ->set('s3StorageId', $s3->id) + ->call('instantSave') + ->assertDispatched('success'); + + $backup->refresh(); + expect($backup->save_s3)->toBeTruthy(); + expect($backup->s3_storage_id)->toBe($s3->id); +});