[Feat] Global backups & cross server backups#1198
Open
RichardAnderson wants to merge 7 commits into
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughBackups are now listed at project or server scope, database backup files record source metadata, and cross-server restores validate project, readiness, storage, engine, and version compatibility. Backup actions, restore targeting, broadcasts, file metadata, and selector sizing are updated. ChangesBackup management
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant RestoreBackup
participant TargetDatabase
participant BackupFile
participant RestoreDatabaseJob
Operator->>RestoreBackup: select target server and database
RestoreBackup->>TargetDatabase: validate project and readiness
RestoreBackup->>BackupFile: validate compatibility
RestoreBackup->>RestoreDatabaseJob: dispatch restore
RestoreDatabaseJob->>BackupFile: update status and restored_to
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds project-level (“global”) backups and enables restoring database backups across servers within the same project, with compatibility checks (engine match and target version >= source). It also enhances PostgreSQL backup/restore scripts for safer cross-server restores and standardizes dropdown popover widths.
Changes:
- Introduced a global backups index (project-scoped) backed by
BackupTable/VitoTableand added sidebar navigation. - Captured source DB engine/version on backup files and enforced cross-server restore constraints (project-only, cloud-only, engine/version compatible, target server/database ready).
- Improved PostgreSQL dump/restore behavior and normalized various UI dropdown popovers to use trigger-width sizing; centralized
formatBytes.
Reviewed changes
Copilot reviewed 38 out of 39 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Feature/BackupTest.php | Adds coverage for global backups scoping, metadata capture, and cross-server restore rules. |
| resources/views/ssh/services/database/postgresql/restore.blade.php | Forces psql to stop on first error during restore. |
| resources/views/ssh/services/database/postgresql/backup.blade.php | Produces dumps more suitable for cross-server restore (--clean, --if-exists, etc.). |
| resources/js/types/backup-file.d.ts | Exposes database_engine / database_version to the frontend. |
| resources/js/pages/users/components/user-select.tsx | Makes popover content match trigger width. |
| resources/js/pages/sites/stats.tsx | Uses shared formatBytes utility instead of local implementation. |
| resources/js/pages/sites/components/site-select.tsx | Makes popover content match trigger width (with min width). |
| resources/js/pages/sites/components/isolated-user-select.tsx | Makes popover content match trigger width. |
| resources/js/pages/services/components/install.tsx | Makes popover content match trigger width. |
| resources/js/pages/servers/components/templates.tsx | Makes popover content match trigger width. |
| resources/js/pages/servers/components/server-select.tsx | Makes popover content match trigger width (with min width). |
| resources/js/pages/servers/components/create-server.tsx | Makes popover content match trigger width. |
| resources/js/pages/backups/index.tsx | Migrates backups listing to VitoTable and supports global vs per-server layout. |
| resources/js/pages/backups/files.tsx | Hides DB-only columns when viewing file backups. |
| resources/js/pages/backups/components/restore-backup.tsx | Adds “restore to server” for DB backups and displays source engine/version. |
| resources/js/pages/backups/components/file-columns.tsx | Adds source metadata + size column for backup files and uses shared formatBytes. |
| resources/js/pages/backups/components/create-backup.tsx | Supports creating backups from global context (server selectable). |
| resources/js/pages/backups/components/columns.tsx | Removes legacy DataTable columns implementation (migrated to table/actions pattern). |
| resources/js/pages/backups/components/backup-actions.tsx | New actions dropdown for backup rows (edit/enable/files/delete). |
| resources/js/lib/utils.ts | Adds shared formatBytes(bytes, decimals) helper. |
| resources/js/components/project-select.tsx | Makes popover content match trigger width (with min width). |
| resources/js/components/multi-select.tsx | Makes popover content match trigger width. |
| resources/js/components/app-sidebar.tsx | Adds “Backups” entry to the app sidebar (global backups). |
| database/migrations/2026_07_14_120631_add_source_database_metadata_to_backup_files_table.php | Adds database_engine and database_version columns to backup_files. |
| database/factories/BackupFileFactory.php | Adds factory helper to seed source DB metadata. |
| app/Tables/BackupTable.php | New InertiaTable-backed table for backups (global + per-server). |
| app/Services/Database/Mysql.php | Ensures version command returns a single match (head -n 1). |
| app/Services/Database/AbstractDatabase.php | Captures source engine/version, makes size capture resilient, and fixes temp path for cross-server restore. |
| app/Models/Project.php | Adds backups() relationship via servers (has-many-through). |
| app/Models/BackupFile.php | Adds version normalization + cross-server restore compatibility checks; tempPath supports target server. |
| app/Jobs/Backup/RestoreFileJob.php | Broadcasts parent backup updates after file updates. |
| app/Jobs/Backup/RestoreDatabaseJob.php | Uses target server user for temp path cleanup; broadcasts parent backup updates. |
| app/Jobs/Backup/DeleteFileJob.php | Ensures parent backup updates are broadcast on delete/failure paths. |
| app/Http/Resources/BackupFileResource.php | Includes source DB metadata in API resource. |
| app/Http/Controllers/BackupFileController.php | Enforces route-model consistency (404 on mismatches) and tightens authorization target. |
| app/Http/Controllers/BackupController.php | Adds global backups route + migrates per-server listing to BackupTable. |
| app/Enums/BackupType.php | Implements table display contract for InertiaTable enum rendering. |
| app/Actions/Backup/RunBackup.php | Broadcasts parent backup updates and improves file-size capture robustness. |
| app/Actions/Backup/RestoreBackup.php | Enables cross-server restore validation and broadcasts parent backup updates. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Services/Database/AbstractDatabase.php (1)
279-295: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDownloaded restore temp file is never cleaned up on success.
restoreBackup()downloads the backup into$tempPathon the target server but never deletes it afterwards, unlikerunBackup()which explicitly callsos()->deleteFile()after the upload (line 270).RestoreDatabaseJob::handle()doesn't clean up either — onlyfailed()deletes the temp file. Every successful restore therefore leaves a.sql.gzdump under/home/{sshUser}/on the target server permanently, and with cross-server restores now possible, this can accumulate across multiple servers and eventually exhaust disk space.🐛 Proposed fix — mirror the cleanup pattern from `runBackup()`
public function restoreBackup(BackupFile $backupFile, string $database): void { $tempPath = $backupFile->tempPath($this->service->server); $backupFile->backup->storage->provider()->ssh($this->service->server)->download( $backupFile->path(), $tempPath, ); $this->service->server->ssh()->exec( view($this->getScriptView('restore'), [ 'database' => $database, 'path' => $tempPath, ]), 'restore-database' ); + + $this->service->server->os()->deleteFile($tempPath); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Services/Database/AbstractDatabase.php` around lines 279 - 295, Update restoreBackup() to delete the downloaded temporary backup file from the target server after the restore command completes, mirroring runBackup()’s os()->deleteFile() cleanup pattern. Keep the existing download and restore flow unchanged, and ensure cleanup occurs on successful execution.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/Actions/Backup/RestoreBackup.php`:
- Around line 88-117: Update the validation closure around the Database model
lookup to null-check the loaded server relationship before accessing project_id
or isReady(). Treat a missing server as an invalid selection by failing
validation, then return before the existing project and readiness checks; keep
the remaining database and restore compatibility validation unchanged.
In `@app/Http/Controllers/BackupController.php`:
- Around line 47-50: Ensure hierarchical route integrity in BackupController by
applying scoped implicit binding via the controller’s ScopeBindings attribute,
or add abort_unless($backup->server_id === $server->id, 404) to show, update,
run, enable, disable, and destroy before authorization or other actions.
Preserve the existing behavior for matching server and backup records.
In `@app/Jobs/Backup/DeleteFileJob.php`:
- Around line 27-30: Null-check backup and server relationships in the
asynchronous job flows before dereferencing them, returning early when either is
missing. In app/Jobs/Backup/DeleteFileJob.php lines 27-30 and 55-58, guard the
backup/server accesses and server logging; in app/Jobs/Backup/RestoreFileJob.php
lines 42-45, 66-69, and 83-91, guard server retrieval, failure handling, and
socket-event dispatch respectively. Use the existing job callbacks and preserve
normal behavior when both relationships exist.
In `@app/Models/BackupFile.php`:
- Around line 101-137: Reduce the reported complexity of
restoreCompatibilityError by extracting the database engine and version
validation into a focused private helper, such as
versionCompatibilityError(Service $service): ?string. Preserve the existing
validation order, error messages, and return behavior while having
restoreCompatibilityError delegate the service-specific checks to that helper.
In `@app/Services/Database/AbstractDatabase.php`:
- Around line 241-251: In runBackup, narrow the catch around database_version
capture from Throwable to the specific SSH-related exception used by the
service, such as SSHError, while retaining the existing logging and null
fallback. Allow unrelated errors from version() or normalizeVersion() to
propagate instead of being silently swallowed.
In `@resources/js/pages/backups/components/create-backup.tsx`:
- Around line 66-70: Update the onValueChange handler to preserve the current
database backup type when the newly selected server supports database backups;
otherwise continue setting the type to file. Keep clearing the database
selection when changing servers, and use the existing server capability data and
selected-server state to determine support.
In `@tests/Feature/BackupTest.php`:
- Around line 677-741: Add a dedicated test alongside
test_backup_run_succeeds_with_null_version_when_version_query_yields_nothing
that configures the SSH/version command used by AbstractDatabase::runBackup() to
throw an exception, then runs the backup and asserts the backup file is CREATED
with database_engine set and database_version null. Reuse the existing setup and
factory pattern while specifically exercising the exception-catching path.
---
Outside diff comments:
In `@app/Services/Database/AbstractDatabase.php`:
- Around line 279-295: Update restoreBackup() to delete the downloaded temporary
backup file from the target server after the restore command completes,
mirroring runBackup()’s os()->deleteFile() cleanup pattern. Keep the existing
download and restore flow unchanged, and ensure cleanup occurs on successful
execution.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c0da9a68-ad23-4553-9075-c2fec0b0bf9c
📒 Files selected for processing (39)
app/Actions/Backup/RestoreBackup.phpapp/Actions/Backup/RunBackup.phpapp/Enums/BackupType.phpapp/Http/Controllers/BackupController.phpapp/Http/Controllers/BackupFileController.phpapp/Http/Resources/BackupFileResource.phpapp/Jobs/Backup/DeleteFileJob.phpapp/Jobs/Backup/RestoreDatabaseJob.phpapp/Jobs/Backup/RestoreFileJob.phpapp/Models/BackupFile.phpapp/Models/Project.phpapp/Services/Database/AbstractDatabase.phpapp/Services/Database/Mysql.phpapp/Tables/BackupTable.phpdatabase/factories/BackupFileFactory.phpdatabase/migrations/2026_07_14_120631_add_source_database_metadata_to_backup_files_table.phpresources/js/components/app-sidebar.tsxresources/js/components/multi-select.tsxresources/js/components/project-select.tsxresources/js/lib/utils.tsresources/js/pages/backups/components/backup-actions.tsxresources/js/pages/backups/components/columns.tsxresources/js/pages/backups/components/create-backup.tsxresources/js/pages/backups/components/file-columns.tsxresources/js/pages/backups/components/restore-backup.tsxresources/js/pages/backups/files.tsxresources/js/pages/backups/index.tsxresources/js/pages/servers/components/create-server.tsxresources/js/pages/servers/components/server-select.tsxresources/js/pages/servers/components/templates.tsxresources/js/pages/services/components/install.tsxresources/js/pages/sites/components/isolated-user-select.tsxresources/js/pages/sites/components/site-select.tsxresources/js/pages/sites/stats.tsxresources/js/pages/users/components/user-select.tsxresources/js/types/backup-file.d.tsresources/views/ssh/services/database/postgresql/backup.blade.phpresources/views/ssh/services/database/postgresql/restore.blade.phptests/Feature/BackupTest.php
💤 Files with no reviewable changes (1)
- resources/js/pages/backups/components/columns.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit