Skip to content

Commit 669db41

Browse files
committed
refactor: simplify scheduled restore schema and add schedule guards
- Drop redundant scheduled_restores.last_restore_id; derive latest restore via hasOne()->latestOfMany() on the existing scheduled_restore_id FK. - Switch source/target FKs on scheduled_restores from RESTRICT to CASCADE so deleting a database server cleans up its scheduled restores. - Guard backup-schedule deletion when referenced by a scheduled restore and surface the relation in the Configuration > Backup UI alongside the existing server badge. - Remove the schedule "Run now" button: it only triggers backups, which is now ambiguous because schedules can also be used by scheduled restores.
1 parent 803c610 commit 669db41

11 files changed

Lines changed: 84 additions & 102 deletions

File tree

app/Console/Commands/RunScheduledRestores.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ public function handle(BackupJobFactory $backupJobFactory, LatestSnapshotResolve
7676

7777
$scheduledRestore->forceFill([
7878
'last_executed_at' => now(),
79-
'last_restore_id' => $restore->id,
8079
'last_skip_reason' => null,
8180
])->save();
8281

app/Http/Resources/ScheduledRestoreResource.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ public function toArray(Request $request): array
2727
'options' => $this->options,
2828
'enabled' => $this->enabled,
2929
'last_executed_at' => $this->last_executed_at,
30-
'last_restore_id' => $this->last_restore_id,
3130
'last_skip_reason' => $this->last_skip_reason,
3231
'created_at' => $this->created_at,
3332
'updated_at' => $this->updated_at,

app/Livewire/Configuration/Backup.php

Lines changed: 17 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
use App\Jobs\VerifySnapshotFileJob;
77
use App\Livewire\Forms\ConfigurationForm;
88
use App\Models\BackupSchedule;
9-
use App\Services\Backup\TriggerBackupAction;
109
use App\Services\CurrentOrganization;
1110
use App\Traits\Toast;
1211
use Illuminate\Contracts\View\View;
@@ -133,7 +132,7 @@ public function deleteSchedule(): void
133132
return;
134133
}
135134

136-
$schedule = BackupSchedule::withCount('backups')->findOrFail($this->deleteScheduleId);
135+
$schedule = BackupSchedule::withCount(['backups', 'scheduledRestores'])->findOrFail($this->deleteScheduleId);
137136

138137
if ($schedule->backups_count > 0) {
139138
$this->error(__('Cannot delete a schedule that is in use by database servers.'));
@@ -143,48 +142,21 @@ public function deleteSchedule(): void
143142
return;
144143
}
145144

145+
if ($schedule->scheduled_restores_count > 0) {
146+
$this->error(__('Cannot delete a schedule that is in use by scheduled restores.'));
147+
$this->showDeleteScheduleModal = false;
148+
$this->deleteScheduleId = null;
149+
150+
return;
151+
}
152+
146153
$schedule->delete();
147154
$this->showDeleteScheduleModal = false;
148155
$this->deleteScheduleId = null;
149156

150157
$this->success(__('Backup schedule deleted.'));
151158
}
152159

153-
public function runSchedule(string $scheduleId, TriggerBackupAction $action): void
154-
{
155-
abort_unless(auth()->user()->isAdmin(), Response::HTTP_FORBIDDEN);
156-
157-
$schedule = BackupSchedule::findOrFail($scheduleId);
158-
159-
$backups = $schedule->backups()
160-
->whereRelation('databaseServer', 'backups_enabled', true)
161-
->with(['databaseServer', 'volume'])
162-
->get();
163-
164-
$totalSnapshots = 0;
165-
$errors = [];
166-
167-
foreach ($backups as $backup) {
168-
try {
169-
$userId = auth()->id();
170-
$result = $action->execute($backup, is_int($userId) ? $userId : null);
171-
$totalSnapshots += count($result['snapshots']);
172-
} catch (\Throwable $e) {
173-
$errors[] = $backup->databaseServer->name.': '.$e->getMessage();
174-
}
175-
}
176-
177-
if ($totalSnapshots > 0) {
178-
$this->success(
179-
trans_choice(':count backup started successfully!|:count backups started successfully!', $totalSnapshots)
180-
);
181-
}
182-
183-
if (! empty($errors)) {
184-
$this->error(implode('; ', $errors));
185-
}
186-
}
187-
188160
// --- Computed Properties ---
189161

190162
/**
@@ -198,10 +170,15 @@ public function backupSchedules(): Collection
198170
$query->whereRelation('databaseServer', 'backups_enabled', true);
199171
},
200172
'backups as total_backups_count',
173+
'scheduledRestores as scheduled_restores_count',
201174
])
202-
->with(['backups' => function ($query) {
203-
$query->whereRelation('databaseServer', 'backups_enabled', true);
204-
}, 'backups.databaseServer:id,name'])
175+
->with([
176+
'backups' => function ($query) {
177+
$query->whereRelation('databaseServer', 'backups_enabled', true);
178+
},
179+
'backups.databaseServer:id,name',
180+
'scheduledRestores:id,name,backup_schedule_id',
181+
])
205182
->orderBy('name')
206183
->get();
207184
}

app/Models/BackupSchedule.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,12 @@ public function backups(): HasMany
2828
{
2929
return $this->hasMany(Backup::class);
3030
}
31+
32+
/**
33+
* @return HasMany<ScheduledRestore, BackupSchedule>
34+
*/
35+
public function scheduledRestores(): HasMany
36+
{
37+
return $this->hasMany(ScheduledRestore::class);
38+
}
3139
}

app/Models/ScheduledRestore.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use Illuminate\Database\Eloquent\Model;
99
use Illuminate\Database\Eloquent\Relations\BelongsTo;
1010
use Illuminate\Database\Eloquent\Relations\HasMany;
11+
use Illuminate\Database\Eloquent\Relations\HasOne;
1112

1213
/**
1314
* @mixin IdeHelperScheduledRestore
@@ -33,7 +34,6 @@ class ScheduledRestore extends Model
3334
'options',
3435
'enabled',
3536
'last_executed_at',
36-
'last_restore_id',
3737
'last_skip_reason',
3838
];
3939

@@ -74,11 +74,11 @@ public function targetServer(): BelongsTo
7474
}
7575

7676
/**
77-
* @return BelongsTo<Restore, ScheduledRestore>
77+
* @return HasOne<Restore, ScheduledRestore>
7878
*/
79-
public function lastRestore(): BelongsTo
79+
public function lastRestore(): HasOne
8080
{
81-
return $this->belongsTo(Restore::class, 'last_restore_id');
81+
return $this->hasOne(Restore::class)->latestOfMany();
8282
}
8383

8484
/**

database/factories/ScheduledRestoreFactory.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ public function definition(): array
2626
'options' => null,
2727
'enabled' => true,
2828
'last_executed_at' => null,
29-
'last_restore_id' => null,
3029
'last_skip_reason' => null,
3130
];
3231
}

database/migrations/2026_05_15_000001_create_scheduled_restores.php

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,12 @@ public function up(): void
1919
$table->json('options')->nullable();
2020
$table->boolean('enabled')->default(true);
2121
$table->timestamp('last_executed_at')->nullable();
22-
$table->ulid('last_restore_id')->nullable();
2322
$table->string('last_skip_reason')->nullable();
2423
$table->timestamps();
2524

26-
$table->foreign('source_server_id')->references('id')->on('database_servers')->restrictOnDelete();
27-
$table->foreign('target_server_id')->references('id')->on('database_servers')->restrictOnDelete();
25+
$table->foreign('source_server_id')->references('id')->on('database_servers')->cascadeOnDelete();
26+
$table->foreign('target_server_id')->references('id')->on('database_servers')->cascadeOnDelete();
2827
$table->foreign('backup_schedule_id')->references('id')->on('backup_schedules')->restrictOnDelete();
29-
$table->foreign('last_restore_id')->references('id')->on('restores')->nullOnDelete();
3028

3129
$table->index(['enabled', 'source_server_id']);
3230
});

resources/views/livewire/configuration/backup.blade.php

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,19 @@
3535
</x-slot:content>
3636
</x-popover>
3737
@endif
38+
@if ($schedule->scheduled_restores_count > 0)
39+
<x-popover>
40+
<x-slot:trigger>
41+
<span class="badge badge-outline badge-info cursor-default">
42+
<x-icon name="o-calendar" class="w-3 h-3" />
43+
{{ trans_choice(':count scheduled restore|:count scheduled restores', $schedule->scheduled_restores_count) }}
44+
</span>
45+
</x-slot:trigger>
46+
<x-slot:content>
47+
{{ $schedule->scheduledRestores->pluck('name')->join(', ') }}
48+
</x-slot:content>
49+
</x-popover>
50+
@endif
3851
</span>
3952
</x-slot:label>
4053
<div class="flex flex-wrap items-center gap-3">
@@ -45,16 +58,21 @@
4558
<span class="text-sm text-base-content/60 min-w-0">{{ \App\Support\Formatters::cronTranslation($schedule->expression) }}</span>
4659
@if ($this->isAdmin)
4760
<div class="flex items-center gap-0.5 shrink-0 ml-auto">
48-
@if ($schedule->backups_count > 0)
49-
<x-button icon="bi.database-fill-up" class="btn-ghost btn-sm text-info" wire:click="runSchedule('{{ $schedule->id }}')" spinner="runSchedule('{{ $schedule->id }}')" :tooltip-left="__('Run now')" />
50-
@endif
5161
<x-button icon="o-pencil-square" class="btn-ghost btn-sm" wire:click="openScheduleModal('{{ $schedule->id }}')" :tooltip-left="__('Edit')" />
52-
@if ($schedule->total_backups_count > 0)
62+
@if ($schedule->total_backups_count > 0 || $schedule->scheduled_restores_count > 0)
5363
<x-popover>
5464
<x-slot:trigger>
5565
<x-button icon="o-trash" class="btn-ghost btn-sm opacity-40" disabled />
5666
</x-slot:trigger>
57-
<x-slot:content>{{ __('In use by servers') }}</x-slot:content>
67+
<x-slot:content>
68+
@if ($schedule->total_backups_count > 0 && $schedule->scheduled_restores_count > 0)
69+
{{ __('In use by servers and scheduled restores') }}
70+
@elseif ($schedule->total_backups_count > 0)
71+
{{ __('In use by servers') }}
72+
@else
73+
{{ __('In use by scheduled restores') }}
74+
@endif
75+
</x-slot:content>
5876
</x-popover>
5977
@else
6078
<x-button icon="o-trash" class="btn-ghost btn-sm text-error hover:bg-error/10" wire:click="confirmDeleteSchedule('{{ $schedule->id }}')" :tooltip-left="__('Delete')" />

tests/Feature/Configuration/BackupTest.php

Lines changed: 13 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,11 @@
33
use App\Enums\UserRole;
44
use App\Facades\AppConfig;
55
use App\Jobs\CleanupExpiredSnapshotsJob;
6-
use App\Jobs\ProcessBackupJob;
76
use App\Jobs\VerifySnapshotFileJob;
87
use App\Livewire\Configuration\Backup;
98
use App\Models\BackupSchedule;
109
use App\Models\DatabaseServer;
11-
use App\Models\Snapshot;
10+
use App\Models\ScheduledRestore;
1211
use App\Models\User;
1312
use Illuminate\Support\Facades\Queue;
1413
use Livewire\Livewire;
@@ -130,6 +129,18 @@
130129
$this->assertDatabaseHas('backup_schedules', ['id' => $schedule->id]);
131130
});
132131

132+
test('cannot delete a backup schedule that is referenced by a scheduled restore', function () {
133+
$schedule = BackupSchedule::factory()->create();
134+
ScheduledRestore::factory()->create(['backup_schedule_id' => $schedule->id]);
135+
136+
Livewire::actingAs(User::factory()->create(['role' => UserRole::Admin]))
137+
->test(Backup::class)
138+
->call('confirmDeleteSchedule', $schedule->id)
139+
->call('deleteSchedule');
140+
141+
$this->assertDatabaseHas('backup_schedules', ['id' => $schedule->id]);
142+
});
143+
133144
test('schedule name must be unique', function () {
134145
Livewire::actingAs(User::factory()->create(['role' => UserRole::Admin]))
135146
->test(Backup::class)
@@ -164,49 +175,6 @@
164175
->assertForbidden();
165176
});
166177

167-
test('admin can run a schedule to trigger backups for all its servers', function () {
168-
Queue::fake();
169-
170-
$schedule = BackupSchedule::factory()->create();
171-
$server = DatabaseServer::factory()->create(['database_names' => ['app']]);
172-
$server->backups->first()->update(['backup_schedule_id' => $schedule->id]);
173-
174-
Livewire::actingAs(User::factory()->create(['role' => UserRole::Admin]))
175-
->test(Backup::class)
176-
->call('runSchedule', $schedule->id);
177-
178-
Queue::assertPushed(ProcessBackupJob::class);
179-
});
180-
181-
test('running a schedule skips servers with backups disabled', function () {
182-
Queue::fake();
183-
184-
$schedule = BackupSchedule::factory()->create();
185-
$enabledServer = DatabaseServer::factory()->create(['database_names' => ['app'], 'backups_enabled' => true]);
186-
$disabledServer = DatabaseServer::factory()->create(['database_names' => ['app'], 'backups_enabled' => false]);
187-
$enabledServer->backups->first()->update(['backup_schedule_id' => $schedule->id]);
188-
$disabledServer->backups->first()->update(['backup_schedule_id' => $schedule->id]);
189-
190-
Livewire::actingAs(User::factory()->create(['role' => UserRole::Admin]))
191-
->test(Backup::class)
192-
->call('runSchedule', $schedule->id);
193-
194-
Queue::assertPushed(ProcessBackupJob::class, function ($job) use ($enabledServer) {
195-
return Snapshot::find($job->snapshotId)->database_server_id === $enabledServer->id;
196-
});
197-
198-
Queue::assertNotPushed(ProcessBackupJob::class, function ($job) use ($disabledServer) {
199-
return Snapshot::find($job->snapshotId)->database_server_id === $disabledServer->id;
200-
});
201-
});
202-
203-
test('non-admin cannot run a schedule', function () {
204-
Livewire::actingAs(User::factory()->create(['role' => UserRole::Member]))
205-
->test(Backup::class)
206-
->call('runSchedule', 'fake-id')
207-
->assertForbidden();
208-
});
209-
210178
test('admin can run cleanup manually', function () {
211179
Queue::fake();
212180

tests/Feature/Console/RunScheduledRestoresTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@
9090
->and($restore->triggered_by_user_id)->toBeNull();
9191

9292
$scheduled->refresh();
93-
expect($scheduled->last_restore_id)->toBe($restore->id)
93+
expect($scheduled->lastRestore?->id)->toBe($restore->id)
9494
->and($scheduled->last_executed_at)->not->toBeNull()
9595
->and($scheduled->last_skip_reason)->toBeNull();
9696
});

0 commit comments

Comments
 (0)