diff --git a/app/Http/Controllers/Api/Client/Servers/BackupController.php b/app/Http/Controllers/Api/Client/Servers/BackupController.php index a9e454223..03ffd1685 100644 --- a/app/Http/Controllers/Api/Client/Servers/BackupController.php +++ b/app/Http/Controllers/Api/Client/Servers/BackupController.php @@ -211,7 +211,7 @@ class BackupController extends ClientApiController throw new BadRequestHttpException('This server is not currently in a state that allows for a backup to be restored.'); } - if (!$backup->is_successful && !$backup->completed_at) { + if (!$backup->is_successful && is_null($backup->completed_at)) { throw new BadRequestHttpException('This backup cannot be restored at this time: not completed or failed.'); } diff --git a/app/Http/Requests/Api/Remote/ReportBackupCompleteRequest.php b/app/Http/Requests/Api/Remote/ReportBackupCompleteRequest.php index a90a2b2b9..0c96b3f02 100644 --- a/app/Http/Requests/Api/Remote/ReportBackupCompleteRequest.php +++ b/app/Http/Requests/Api/Remote/ReportBackupCompleteRequest.php @@ -12,7 +12,7 @@ class ReportBackupCompleteRequest extends FormRequest public function rules() { return [ - 'successful' => 'present|boolean', + 'successful' => 'required|boolean', 'checksum' => 'nullable|string|required_if:successful,true', 'checksum_type' => 'nullable|string|required_if:successful,true', 'size' => 'nullable|numeric|required_if:successful,true', diff --git a/app/Models/Backup.php b/app/Models/Backup.php index 95503ecb0..8b3413147 100644 --- a/app/Models/Backup.php +++ b/app/Models/Backup.php @@ -64,7 +64,7 @@ class Backup extends Model * @var array */ protected $attributes = [ - 'is_successful' => true, + 'is_successful' => false, 'is_locked' => false, 'checksum' => null, 'bytes' => 0, diff --git a/app/Repositories/Eloquent/BackupRepository.php b/app/Repositories/Eloquent/BackupRepository.php index b53547e44..fef80c48e 100644 --- a/app/Repositories/Eloquent/BackupRepository.php +++ b/app/Repositories/Eloquent/BackupRepository.php @@ -25,7 +25,10 @@ class BackupRepository extends EloquentRepository return $this->getBuilder() ->withTrashed() ->where('server_id', $server) - ->where('is_successful', true) + ->where(function ($query) { + $query->whereNull('completed_at') + ->orWhere('is_successful', '=', true); + }) ->where('created_at', '>=', Carbon::now()->subSeconds($seconds)->toDateTimeString()) ->get() ->toBase(); diff --git a/app/Services/Backups/DeleteBackupService.php b/app/Services/Backups/DeleteBackupService.php index 80d6374b1..66eefe675 100644 --- a/app/Services/Backups/DeleteBackupService.php +++ b/app/Services/Backups/DeleteBackupService.php @@ -63,7 +63,7 @@ class DeleteBackupService // I also don't really see any reason you'd have a locked, failed backup to keep // around. The logic that updates the backup to the failed state will also remove // the lock, so this condition should really never happen. - if ($backup->is_locked && ($backup->completed_at && $backup->is_successful)) { + if ($backup->is_locked && ($backup->is_successful && !is_null($backup->completed_at))) { throw new BackupLockedException(); } diff --git a/app/Services/Backups/InitiateBackupService.php b/app/Services/Backups/InitiateBackupService.php index 347740dc1..f51279f69 100644 --- a/app/Services/Backups/InitiateBackupService.php +++ b/app/Services/Backups/InitiateBackupService.php @@ -132,8 +132,12 @@ class InitiateBackupService } } - // Check if the server has reached or exceeded it's backup limit. - $successful = $server->backups()->where('is_successful', true); + // Check if the server has reached or exceeded its backup limit. + // completed_at == null will cover any ongoing backups, while is_successful == true will cover any completed backups. + $successful = $server->backups()->where(function ($query) { + $query->whereNull('completed_at') + ->orWhere('is_successful', true); + }); if (!$server->backup_limit || $successful->count() >= $server->backup_limit) { // Do not allow the user to continue if this server is already at its limit and can't override. if (!$override || $server->backup_limit <= 0) { diff --git a/database/migrations/2021_08_03_210600_change_successful_field_to_default_to_false_on_backups_table.php b/database/migrations/2021_08_03_210600_change_successful_field_to_default_to_false_on_backups_table.php new file mode 100644 index 000000000..1dddb7f3b --- /dev/null +++ b/database/migrations/2021_08_03_210600_change_successful_field_to_default_to_false_on_backups_table.php @@ -0,0 +1,37 @@ +boolean('is_successful')->after('uuid')->default(false)->change(); + }); + + // Convert currently processing backups to the new format so things don't break. + DB::table('backups')->select('id')->where('is_successful', 1)->whereNull('completed_at')->update([ + 'is_successful' => 0, + ]); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::table('backups', function (Blueprint $table) { + $table->boolean('is_successful')->after('uuid')->default(true)->change(); + }); + } +} diff --git a/resources/scripts/components/server/backups/BackupRow.tsx b/resources/scripts/components/server/backups/BackupRow.tsx index 55a9aff98..42ece8f9f 100644 --- a/resources/scripts/components/server/backups/BackupRow.tsx +++ b/resources/scripts/components/server/backups/BackupRow.tsx @@ -44,7 +44,7 @@ export default ({ backup, className }: Props) => {
- {backup.completedAt ? + {backup.completedAt !== null ? backup.isLocked ? : @@ -55,7 +55,7 @@ export default ({ backup, className }: Props) => {
- {!backup.isSuccessful && + {backup.completedAt !== null && !backup.isSuccessful && Failed @@ -63,7 +63,7 @@ export default ({ backup, className }: Props) => {

{backup.name}

- {(backup.completedAt && backup.isSuccessful) && + {(backup.completedAt !== null && backup.isSuccessful) && }