Conversation
- Add POST /api/v1/database-servers/{id}/restore endpoint with validation
for snapshot compatibility, schema name format, and app database protection
- Create RestoreRequest form request and RestoreResource API resource
- Extract isAppDatabase() to DatabaseServer model, removing duplication
from RestoreModal and the controller
- Replace RuntimeException with ValidationException in TriggerBackupAction
so the framework handles the 422 response automatically
📝 WalkthroughWalkthroughAdds a restore API endpoint and supporting pieces: a RestoreRequest, RestoreResource, controller restore method, BackupJobFactory restore validations, DatabaseServer::isAppDatabase, route, and tests; adjusts TriggerBackupAction to throw ValidationException for missing backup config. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant API as "DatabaseServerController"
participant Auth as "Authorization"
participant DB as "Models (Snapshot, DatabaseServer)"
participant Factory as "BackupJobFactory"
participant Queue as "Job Queue"
Client->>API: POST /api/v1/database-servers/{id}/restore (snapshot_id, schema_name)
API->>Auth: authorize(restore)
Auth-->>API: authorized
API->>DB: find Snapshot by ID
DB-->>API: snapshot
API->>API: validate snapshot.database_type == server.database_type
API->>DB: DatabaseServer::isAppDatabase(schema_name)
DB-->>API: isAppDatabase result
API->>Factory: createRestore(snapshot, server, schema_name)
Factory-->>API: Restore instance
API->>Queue: dispatch ProcessRestoreJob
Queue-->>API: job queued
API-->>Client: 202 RestoreResource (restore + snapshot + target_server + job)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
app/Http/Controllers/Api/V1/DatabaseServerController.php (1)
116-116: Redundant relation loading.The
BackupJobFactory::createRestore()method already loads these relations (seeapp/Services/Backup/BackupJobFactory.php:137). This line can be removed to avoid unnecessary database queries.♻️ Proposed fix
ProcessRestoreJob::dispatch($restore->id); - $restore->load(['job', 'snapshot', 'targetServer']); - return response()->json([ 'message' => 'Restore started successfully!', 'restore' => new RestoreResource($restore), ], 202);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Http/Controllers/Api/V1/DatabaseServerController.php` at line 116, The line calling $restore->load(['job', 'snapshot', 'targetServer']) is redundant because BackupJobFactory::createRestore() already eager-loads those relations; remove that load call from DatabaseServerController::(the method containing $restore) to avoid the extra DB query and rely on the relations already populated by BackupJobFactory::createRestore().app/Http/Requests/Api/V1/RestoreRequest.php (1)
44-49: Consider adding custom messages for other validation rules.Currently only
schema_name.regexhas a custom message. For API consistency, you might want to add messages forsnapshot_id.required,snapshot_id.exists, andschema_name.requiredto provide clearer error responses.💡 Example messages
public function messages(): array { return [ 'schema_name.regex' => 'Database name can only contain letters, numbers, and underscores.', + 'snapshot_id.required' => 'A snapshot ID is required.', + 'snapshot_id.exists' => 'The specified snapshot does not exist.', + 'schema_name.required' => 'A schema/database name is required.', ]; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Http/Requests/Api/V1/RestoreRequest.php` around lines 44 - 49, In RestoreRequest::messages() add explicit custom error messages for the other validation keys so API responses are consistent: include entries for 'snapshot_id.required', 'snapshot_id.exists', and 'schema_name.required' alongside the existing 'schema_name.regex' message, make the messages clear and user-facing (e.g., "Snapshot ID is required", "Snapshot not found", "Schema name is required") and ensure the keys exactly match the validation rules defined in the rules() method so Laravel maps them correctly.app/Models/DatabaseServer.php (1)
304-306: Consider hostname normalization for edge cases.The host comparison uses strict equality, which may not match when the app config uses
127.0.0.1but the server useslocalhost(or vice versa). If this is a concern in your deployment, you could normalize both values.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Models/DatabaseServer.php` around lines 304 - 306, The host comparison in DatabaseServer (the block comparing $this->host, $this->port, $schemaName to $appDbHost, $appDbPort, $appDbDatabase) should normalize hostnames before strict comparison to handle 'localhost' vs '127.0.0.1' and casing/whitespace differences; update the check to normalize both $this->host and $appDbHost (e.g., trim/lowercase and resolve to an IP via gethostbyname or a small normalizeHost(string $h): string helper) and then compare the normalized hosts along with the ports and schema name in the existing method to preserve behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/Feature/Api/RestoreApiTest.php`:
- Around line 30-57: The test sets Queue::fake() but never asserts a job was
enqueued; update the test in RestoreApiTest (the 'authenticated users can
trigger a restore' test) to assert the restore job is dispatched by adding
Queue::assertPushed(ProcessRestoreJob::class) after the POST and response
assertions so the test verifies the ProcessRestoreJob is queued when calling the
/database-servers/{id}/restore endpoint with snapshot_id and schema_name.
- Around line 103-115: The test "restore rejects nonexistent snapshot" uses
'nonexistent-id' which may fail format validation; instead create a real
Snapshot record for $server (or capture an existing snapshot key), store its
id/key, delete the record so the ID is well-formed but missing, then call the
POST to /api/v1/database-servers/{$server->id}/restore with that deleted
snapshot id (snapshot_id) and assert Unprocessable with snapshot_id validation
error; update the test function (test('restore rejects nonexistent snapshot'))
to create-and-delete the snapshot so the request reaches the "snapshot does not
exist" path.
---
Nitpick comments:
In `@app/Http/Controllers/Api/V1/DatabaseServerController.php`:
- Line 116: The line calling $restore->load(['job', 'snapshot', 'targetServer'])
is redundant because BackupJobFactory::createRestore() already eager-loads those
relations; remove that load call from DatabaseServerController::(the method
containing $restore) to avoid the extra DB query and rely on the relations
already populated by BackupJobFactory::createRestore().
In `@app/Http/Requests/Api/V1/RestoreRequest.php`:
- Around line 44-49: In RestoreRequest::messages() add explicit custom error
messages for the other validation keys so API responses are consistent: include
entries for 'snapshot_id.required', 'snapshot_id.exists', and
'schema_name.required' alongside the existing 'schema_name.regex' message, make
the messages clear and user-facing (e.g., "Snapshot ID is required", "Snapshot
not found", "Schema name is required") and ensure the keys exactly match the
validation rules defined in the rules() method so Laravel maps them correctly.
In `@app/Models/DatabaseServer.php`:
- Around line 304-306: The host comparison in DatabaseServer (the block
comparing $this->host, $this->port, $schemaName to $appDbHost, $appDbPort,
$appDbDatabase) should normalize hostnames before strict comparison to handle
'localhost' vs '127.0.0.1' and casing/whitespace differences; update the check
to normalize both $this->host and $appDbHost (e.g., trim/lowercase and resolve
to an IP via gethostbyname or a small normalizeHost(string $h): string helper)
and then compare the normalized hosts along with the ports and schema name in
the existing method to preserve behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1a00bae9-a35e-42eb-ae0a-7fa4a5749f40
📒 Files selected for processing (9)
app/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Http/Requests/Api/V1/RestoreRequest.phpapp/Http/Resources/RestoreResource.phpapp/Livewire/DatabaseServer/RestoreModal.phpapp/Models/DatabaseServer.phpapp/Services/Backup/TriggerBackupAction.phproutes/api.phptests/Feature/Api/RestoreApiTest.phptests/Feature/Services/Backup/TriggerBackupActionTest.php
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-app-image
- GitHub Check: ci
🧰 Additional context used
📓 Path-based instructions (13)
**/{app,tests}/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
**/{app,tests}/**/*.php: Use Eloquent models andModel::query()before suggesting raw database queries. Avoid usingDB::. Generate code that leverages Laravel's ORM capabilities.
Never append to static properties in services/classes, as they accumulate in memory across requests (Octane issue).
Files:
app/Http/Resources/RestoreResource.phpapp/Models/DatabaseServer.phptests/Feature/Api/RestoreApiTest.phpapp/Livewire/DatabaseServer/RestoreModal.phpapp/Http/Requests/Api/V1/RestoreRequest.phptests/Feature/Services/Backup/TriggerBackupActionTest.phpapp/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Services/Backup/TriggerBackupAction.php
**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.php: Use constructor property promotion in__construct()with PHP 8+ syntax (e.g.,public function __construct(public GitHub $github) { }). Do not allow empty constructors with zero parameters unless private.
Always use explicit return type declarations for methods and functions. Use appropriate PHP type hints for method parameters.
Use curly braces for control structures, even for single-line bodies.
Prefer PHPDoc blocks over inline comments. Never use comments within code itself unless logic is exceptionally complex.
Add useful array shape type definitions in PHPDoc blocks when appropriate.
Files:
app/Http/Resources/RestoreResource.phpapp/Models/DatabaseServer.phptests/Feature/Api/RestoreApiTest.phpapp/Livewire/DatabaseServer/RestoreModal.phpapp/Http/Requests/Api/V1/RestoreRequest.phptests/Feature/Services/Backup/TriggerBackupActionTest.phproutes/api.phpapp/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Services/Backup/TriggerBackupAction.php
**/{app,routes}/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
Use Laravel's built-in authentication and authorization features (gates, policies, Sanctum, etc.).
Files:
app/Http/Resources/RestoreResource.phpapp/Models/DatabaseServer.phpapp/Livewire/DatabaseServer/RestoreModal.phpapp/Http/Requests/Api/V1/RestoreRequest.phproutes/api.phpapp/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Services/Backup/TriggerBackupAction.php
**/{app,resources/views}/**/*.{php,blade.php}
📄 CodeRabbit inference engine (CLAUDE.md)
When generating links to other pages, prefer named routes and the
route()function.
Files:
app/Http/Resources/RestoreResource.phpapp/Models/DatabaseServer.phpapp/Livewire/DatabaseServer/RestoreModal.phpapp/Http/Requests/Api/V1/RestoreRequest.phpapp/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Services/Backup/TriggerBackupAction.php
**/{app,routes,tests}/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
Never use
env()function directly outside of config files. Always useconfig('app.name')instead ofenv('APP_NAME').
Files:
app/Http/Resources/RestoreResource.phpapp/Models/DatabaseServer.phptests/Feature/Api/RestoreApiTest.phpapp/Livewire/DatabaseServer/RestoreModal.phpapp/Http/Requests/Api/V1/RestoreRequest.phptests/Feature/Services/Backup/TriggerBackupActionTest.phproutes/api.phpapp/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Services/Backup/TriggerBackupAction.php
{lang/**/*.json,**/{app,resources/views}/**/*.{php,blade.php}}
📄 CodeRabbit inference engine (CLAUDE.md)
Translations use Laravel's JSON translation files with the
__('...')helper. Translations live inlang/{locale}.json. Use typographic apostrophes ('U+2019) instead of ASCII apostrophes in translation values to avoid HTML encoding artifacts.
Files:
app/Http/Resources/RestoreResource.phpapp/Models/DatabaseServer.phpapp/Livewire/DatabaseServer/RestoreModal.phpapp/Http/Requests/Api/V1/RestoreRequest.phpapp/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Services/Backup/TriggerBackupAction.php
**/*.{php,js,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use descriptive names for variables and methods (e.g.,
isRegisteredForDiscounts, notdiscount()).
Files:
app/Http/Resources/RestoreResource.phpapp/Models/DatabaseServer.phptests/Feature/Api/RestoreApiTest.phpapp/Livewire/DatabaseServer/RestoreModal.phpapp/Http/Requests/Api/V1/RestoreRequest.phptests/Feature/Services/Backup/TriggerBackupActionTest.phproutes/api.phpapp/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Services/Backup/TriggerBackupAction.php
**/{app/Models,database/migrations}/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
Database models use ULIDs instead of auto-incrementing integers for primary keys.
Files:
app/Models/DatabaseServer.php
**/Models/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
**/Models/**/*.php: All database models must use proper Eloquent relationship methods with return type hints. Prefer relationship methods over raw queries or manual joins. Use eager loading to prevent N+1 query problems.
Use casts via acasts()method on models rather than the$castsproperty to follow existing conventions.
Files:
app/Models/DatabaseServer.php
**/{tests,database/factories}/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
Use factories for creating models in tests. Check if the factory has custom states before manually setting up the model.
Files:
tests/Feature/Api/RestoreApiTest.phptests/Feature/Services/Backup/TriggerBackupActionTest.php
**/tests/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
Use faker methods such as
$this->faker->word()orfake()->randomDigit()in tests. Follow existing conventions whether to use$this->fakerorfake().
Files:
tests/Feature/Api/RestoreApiTest.phptests/Feature/Services/Backup/TriggerBackupActionTest.php
**/Livewire/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
**/Livewire/**/*.php: Use#[Validate]attributes or Form objects for validation in Livewire components. Form objects encapsulate validation logic.
Use class-based Livewire components for all interactive pages and modal interactions. Public properties are automatically bound to views. Usewire:modelfor two-way binding.
Files:
app/Livewire/DatabaseServer/RestoreModal.php
**/Requests/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
Always create Form Request classes for validation rather than inline validation. Include both validation rules and custom error messages. Check sibling Form Requests to see if the application uses array or string-based validation rules.
Files:
app/Http/Requests/Api/V1/RestoreRequest.php
🧠 Learnings (11)
📚 Learning: 2026-02-25T10:48:17.811Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 132
File: app/Console/Commands/RecoverAgentLeasesCommand.php:44-48
Timestamp: 2026-02-25T10:48:17.811Z
Learning: When reviewing PHP code, especially with foreign keys that use cascadeOnDelete and are non-nullable, assume child relations exist at runtime (the database will delete children when the parent is deleted). Do not rely on null-safe operators for these relations, as PHPStan already models them as non-null. This guideline applies broadly to PHP files that define models with foreign keys using cascade delete; verify there are no unnecessary null checks or optional chaining on such relations.
Applied to files:
app/Http/Resources/RestoreResource.phpapp/Models/DatabaseServer.phptests/Feature/Api/RestoreApiTest.phpapp/Livewire/DatabaseServer/RestoreModal.phpapp/Http/Requests/Api/V1/RestoreRequest.phptests/Feature/Services/Backup/TriggerBackupActionTest.phproutes/api.phpapp/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Services/Backup/TriggerBackupAction.php
📚 Learning: 2026-03-10T22:19:40.736Z
Learnt from: CR
Repo: David-Crty/databasement PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T22:19:40.736Z
Learning: When creating tests, use `php artisan make:test` with `--pest` option for Pest tests or `--unit` for unit tests. Most tests should be feature tests.
Applied to files:
tests/Feature/Api/RestoreApiTest.php
📚 Learning: 2026-02-17T08:43:03.630Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 114
File: tests/Integration/SshTunnelTest.php:37-46
Timestamp: 2026-02-17T08:43:03.630Z
Learning: In the databasement project, integration tests use RefreshDatabase (configured in tests/Pest.php), which automatically rolls back all database changes after each test. Therefore, explicit model deletion calls like $server->delete(), $volume->delete(), or $sshConfig->delete() are unnecessary in integration tests.
Applied to files:
tests/Feature/Api/RestoreApiTest.php
📚 Learning: 2026-03-02T13:13:34.079Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 132
File: tests/Unit/Services/Backup/DTO/BackupConfigTest.php:34-45
Timestamp: 2026-03-02T13:13:34.079Z
Learning: In tests/Unit/Services/Backup/DTO/BackupConfigTest.php and similar DTO symmetry tests, prioritize asserting execution-critical fields (database connection, compression, paths) over display-only metadata (volume.name, volume.config, database.serverName) that don't affect backup execution.
Applied to files:
tests/Feature/Api/RestoreApiTest.phptests/Feature/Services/Backup/TriggerBackupActionTest.php
📚 Learning: 2026-02-13T11:05:37.072Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 105
File: tests/Feature/Services/Backup/Databases/SqliteDatabaseTest.php:8-11
Timestamp: 2026-02-13T11:05:37.072Z
Learning: Adopt a global afterEach hook in tests/Pest.php (or equivalent Pest bootstrap) to clean up temporary directories created during tests. Specifically handle temp dirs named with the prefixes sqlite-db-test-*, backup-task-test-*, restore-task-test-*, and volume-test-*, so individual test files don’t need their own cleanup logic. This applies to all PHP test files under the tests directory.
Applied to files:
tests/Feature/Api/RestoreApiTest.phptests/Feature/Services/Backup/TriggerBackupActionTest.php
📚 Learning: 2026-02-18T09:45:52.485Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 116
File: app/Livewire/DatabaseServer/ConnectionStatus.php:18-18
Timestamp: 2026-02-18T09:45:52.485Z
Learning: In Livewire components, Eloquent model properties (e.g., public DatabaseServer $server) are automatically locked by the framework to prevent client-side ID tampering. The #[Locked] attribute is only needed for scalar properties (int, string, bool, etc.) that require protection from client-side mutation. Apply this guidance to all Livewire PHP components; use #[Locked] only on primitive properties that you want to shield from client manipulation, and rely on automatic locking for Eloquent model properties.
Applied to files:
app/Livewire/DatabaseServer/RestoreModal.php
📚 Learning: 2026-03-10T22:19:40.736Z
Learnt from: CR
Repo: David-Crty/databasement PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T22:19:40.736Z
Learning: Applies to **/Requests/**/*.php : Always create Form Request classes for validation rather than inline validation. Include both validation rules and custom error messages. Check sibling Form Requests to see if the application uses array or string-based validation rules.
Applied to files:
app/Http/Requests/Api/V1/RestoreRequest.php
📚 Learning: 2026-02-25T10:48:46.954Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 132
File: app/Services/Agent/AgentJobPayloadBuilder.php:63-76
Timestamp: 2026-02-25T10:48:46.954Z
Learning: In app/Services/Agent/AgentJobPayloadBuilder.php and similar service classes, prefer fail-fast error handling over defensive null-safe operators when upstream callers (e.g., TriggerBackupAction, RunScheduledBackups) guarantee certain relationships exist. For example, in resolveBackupPath(), $server->backup is intentionally accessed without the null-safe operator because backup existence is enforced at call sites; using the null-safe operator would hide bugs that violate this invariant.
Applied to files:
tests/Feature/Services/Backup/TriggerBackupActionTest.phpapp/Services/Backup/TriggerBackupAction.php
📚 Learning: 2026-03-10T22:19:40.736Z
Learnt from: CR
Repo: David-Crty/databasement PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T22:19:40.736Z
Learning: Applies to **/routes/web.php : Use `Route::livewire()` for full-page Livewire components in route definitions (e.g., `Route::livewire('database-servers', \App\Livewire\DatabaseServer\Index::class)`).
Applied to files:
routes/api.php
📚 Learning: 2026-02-04T12:26:27.374Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 72
File: routes/web.php:66-81
Timestamp: 2026-02-04T12:26:27.374Z
Learning: In Laravel Livewire v4, prefer routing full-page Livewire components using Route::livewire('/path', ComponentClass::class) for all full-page components (whether class-based or view-based). While Route::get('/path', ComponentClass::class) remains technically functional for backward compatibility, it is not recommended in v4. Apply this guideline in all route files under routes (e.g., routes/web.php, routes/api.php) to ensure consistent, future-proof routing for Livewire components.
Applied to files:
routes/api.php
📚 Learning: 2026-02-25T10:46:42.914Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 132
File: app/Http/Controllers/Api/V1/AgentController.php:175-206
Timestamp: 2026-02-25T10:46:42.914Z
Learning: In the agent job processing flow (app/Http/Controllers/Api/V1/AgentController.php), state updates in endpoints like ack() and fail() are not wrapped in DB::transaction(). The codebase relies on the scheduled agent:recover-leases command to handle recovery of incomplete or failed jobs, rather than using strict transactional consistency for each endpoint.
Applied to files:
app/Services/Backup/TriggerBackupAction.php
🪛 PHPMD (2.15.0)
app/Http/Resources/RestoreResource.php
[warning] 19-19: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (12)
routes/api.php (1)
15-16: LGTM!The new restore route follows the established pattern of the existing backup route, using consistent naming and middleware.
app/Models/DatabaseServer.php (1)
274-307: LGTM!The centralized
isAppDatabasemethod cleanly consolidates the app-database detection logic. The driver-to-type mapping correctly handles MySQL, MariaDB, and PostgreSQL.app/Livewire/DatabaseServer/RestoreModal.php (1)
172-176: LGTM!Clean refactor delegating the app-database check to the centralized
DatabaseServer::isAppDatabase()method.app/Http/Resources/RestoreResource.php (1)
1-41: LGTM!Well-structured API resource with proper use of
whenLoaded()for conditional relation inclusion. The$requestparameter flagged by PHPMD is required by theJsonResource::toArray()interface signature, so the static analysis warning is a false positive.app/Http/Requests/Api/V1/RestoreRequest.php (1)
1-50: LGTM!Clean form request with appropriate conditional validation based on database type. The validation rules correctly mirror the Livewire component's schema validation logic.
app/Http/Controllers/Api/V1/DatabaseServerController.php (2)
57-71: LGTM!Clean simplification of the backup method. The
ValidationExceptionthrown byTriggerBackupActionwill automatically propagate to Laravel's exception handler, which converts it to a 422 response.
80-122: Well-structured restore endpoint with proper validation and authorization.The restore method correctly:
- Authorizes the action
- Validates snapshot compatibility with target server
- Prevents restoring over the application database
- Dispatches the restore job asynchronously
app/Services/Backup/TriggerBackupAction.php (1)
29-33: LGTM!Good change to use
ValidationExceptioninstead ofRuntimeException. This allows Laravel's exception handler to automatically return a proper 422 response with structured error messages for API consumers, while existing Livewire callers that catch\Throwablecontinue to work correctly.tests/Feature/Services/Backup/TriggerBackupActionTest.php (1)
15-24: LGTM!Test correctly updated to expect
ValidationExceptionmatching the implementation change.tests/Feature/Api/RestoreApiTest.php (3)
8-14: Auth gating coverage looks good.These two tests lock down both the
401and403branches for the new endpoint.Also applies to: 16-28
59-68: Required-field validation coverage is solid.This keeps the endpoint's base
422contract pinned down.
70-86: Nice coverage of the restore compatibility rules.The type-mismatch and schema-format cases are both worth having here.
Also applies to: 88-101
| test('restore rejects nonexistent snapshot', function () { | ||
| $user = User::factory()->create(); | ||
| $server = DatabaseServer::factory()->create(); | ||
|
|
||
| $response = $this->actingAs($user, 'sanctum') | ||
| ->postJson("/api/v1/database-servers/{$server->id}/restore", [ | ||
| 'snapshot_id' => 'nonexistent-id', | ||
| 'schema_name' => 'testdb', | ||
| ]); | ||
|
|
||
| $response->assertUnprocessable() | ||
| ->assertJsonValidationErrors(['snapshot_id']); | ||
| }); |
There was a problem hiding this comment.
Use a valid-but-missing snapshot id in this test.
'nonexistent-id' can fail on format/type validation before it ever exercises the "snapshot does not exist" path. Capturing a real snapshot key and deleting the record first makes the test match its name.
Suggested tweak
test('restore rejects nonexistent snapshot', function () {
$user = User::factory()->create();
$server = DatabaseServer::factory()->create();
+ $snapshot = Snapshot::factory()->forServer($server)->create();
+ $missingSnapshotId = $snapshot->getKey();
+ $snapshot->delete();
$response = $this->actingAs($user, 'sanctum')
->postJson("/api/v1/database-servers/{$server->id}/restore", [
- 'snapshot_id' => 'nonexistent-id',
+ 'snapshot_id' => $missingSnapshotId,
'schema_name' => 'testdb',
]);
$response->assertUnprocessable()
->assertJsonValidationErrors(['snapshot_id']);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/Feature/Api/RestoreApiTest.php` around lines 103 - 115, The test
"restore rejects nonexistent snapshot" uses 'nonexistent-id' which may fail
format validation; instead create a real Snapshot record for $server (or capture
an existing snapshot key), store its id/key, delete the record so the ID is
well-formed but missing, then call the POST to
/api/v1/database-servers/{$server->id}/restore with that deleted snapshot id
(snapshot_id) and assert Unprocessable with snapshot_id validation error; update
the test function (test('restore rejects nonexistent snapshot')) to
create-and-delete the snapshot so the request reaches the "snapshot does not
exist" path.
There was a problem hiding this comment.
The validation rule is 'snapshot_id' => ['required', 'string', 'exists:snapshots,id']. The exists rule just checks if the value exists in the snapshots table, it doesn't care about format.
'nonexistent-id' is a string that won't exist, so the exists rule fails with a validation error. The test already passes and correctly reaches the "snapshot does not exist" path.
There was a problem hiding this comment.
@David-Crty, you're right — the exists:snapshots,id rule only performs a database lookup and doesn't impose any format constraints. Since 'nonexistent-id' satisfies both required and string, it correctly reaches and fails the exists check, which is exactly the path the test is meant to cover. My concern was unfounded — apologies for the noise!
🧠 Learnings used
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 114
File: tests/Integration/SshTunnelTest.php:37-46
Timestamp: 2026-02-17T08:43:03.630Z
Learning: In the databasement project, integration tests use RefreshDatabase (configured in tests/Pest.php), which automatically rolls back all database changes after each test. Therefore, explicit model deletion calls like $server->delete(), $volume->delete(), or $sshConfig->delete() are unnecessary in integration tests.
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 132
File: tests/Unit/Services/Backup/DTO/BackupConfigTest.php:34-45
Timestamp: 2026-03-02T13:13:34.079Z
Learning: In tests/Unit/Services/Backup/DTO/BackupConfigTest.php and similar DTO symmetry tests, prioritize asserting execution-critical fields (database connection, compression, paths) over display-only metadata (volume.name, volume.config, database.serverName) that don't affect backup execution.
Learnt from: CR
Repo: David-Crty/databasement PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T22:19:40.736Z
Learning: When creating tests, use `php artisan make:test` with `--pest` option for Pest tests or `--unit` for unit tests. Most tests should be feature tests.
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 105
File: tests/Feature/Services/Backup/Databases/SqliteDatabaseTest.php:8-11
Timestamp: 2026-02-13T11:05:37.072Z
Learning: Adopt a global afterEach hook in tests/Pest.php (or equivalent Pest bootstrap) to clean up temporary directories created during tests. Specifically handle temp dirs named with the prefixes sqlite-db-test-*, backup-task-test-*, restore-task-test-*, and volume-test-*, so individual test files don’t need their own cleanup logic. This applies to all PHP test files under the tests directory.
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 132
File: app/Console/Commands/RecoverAgentLeasesCommand.php:44-48
Timestamp: 2026-02-25T10:48:17.811Z
Learning: When reviewing PHP code, especially with foreign keys that use cascadeOnDelete and are non-nullable, assume child relations exist at runtime (the database will delete children when the parent is deleted). Do not rely on null-safe operators for these relations, as PHPStan already models them as non-null. This guideline applies broadly to PHP files that define models with foreign keys using cascade delete; verify there are no unnecessary null checks or optional chaining on such relations.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #158 +/- ##
============================================
+ Coverage 91.54% 91.84% +0.29%
- Complexity 1720 1725 +5
============================================
Files 158 160 +2
Lines 6364 6413 +49
============================================
+ Hits 5826 5890 +64
+ Misses 538 523 -15 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…tabase protection in `BackupJobFactory`
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/Http/Controllers/Api/V1/DatabaseServerController.php (1)
102-104: Localize the new API success message.This is a new user-facing string in
app/**, so it should go through the JSON translation flow instead of being hardcoded in English.Suggested change
return response()->json([ - 'message' => 'Restore started successfully!', + 'message' => __('Restore started successfully!'), 'restore' => new RestoreResource($restore), ], 202);As per coding guidelines, "translations use Laravel's JSON translation files with the
__('...')helper."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Http/Controllers/Api/V1/DatabaseServerController.php` around lines 102 - 104, Replace the hardcoded English success message in DatabaseServerController where the response()->json([... 'message' => 'Restore started successfully!', ...]) is returned with a call to Laravel's JSON translation helper (use __('Restore started successfully!')) so the message is localized; keep the rest of the payload (the 'restore' => new RestoreResource($restore)) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/Services/Backup/BackupJobFactory.php`:
- Around line 130-139: The ValidationException thrown by
BackupJobFactory::withMessages() produces field-level errors that the
RestoreModal currently hides by only using $e->getMessage(); update
app/Livewire/DatabaseServer/RestoreModal.php (the catch block around lines
handling exceptions) to detect if ($e instanceof
\Illuminate\Validation\ValidationException) and pull the field messages via
$e->errors() (or extract the first relevant field error like
errors()['snapshot_id'][0] / errors()['schema_name'][0]) and present that to the
user instead of $e->getMessage(); this keeps the factory HTTP-agnostic and
provides actionable field-level feedback in the modal.
---
Nitpick comments:
In `@app/Http/Controllers/Api/V1/DatabaseServerController.php`:
- Around line 102-104: Replace the hardcoded English success message in
DatabaseServerController where the response()->json([... 'message' => 'Restore
started successfully!', ...]) is returned with a call to Laravel's JSON
translation helper (use __('Restore started successfully!')) so the message is
localized; keep the rest of the payload (the 'restore' => new
RestoreResource($restore)) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: adb18dcc-2d03-4776-8c8f-9dd584706805
📒 Files selected for processing (4)
app/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Livewire/DatabaseServer/RestoreModal.phpapp/Services/Backup/BackupJobFactory.phptests/Feature/Api/RestoreApiTest.php
💤 Files with no reviewable changes (1)
- app/Livewire/DatabaseServer/RestoreModal.php
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Feature/Api/RestoreApiTest.php
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-app-image
- GitHub Check: ci
🧰 Additional context used
📓 Path-based instructions (7)
**/{app,tests}/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
**/{app,tests}/**/*.php: Use Eloquent models andModel::query()before suggesting raw database queries. Avoid usingDB::. Generate code that leverages Laravel's ORM capabilities.
Never append to static properties in services/classes, as they accumulate in memory across requests (Octane issue).
Files:
app/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Services/Backup/BackupJobFactory.php
**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.php: Use constructor property promotion in__construct()with PHP 8+ syntax (e.g.,public function __construct(public GitHub $github) { }). Do not allow empty constructors with zero parameters unless private.
Always use explicit return type declarations for methods and functions. Use appropriate PHP type hints for method parameters.
Use curly braces for control structures, even for single-line bodies.
Prefer PHPDoc blocks over inline comments. Never use comments within code itself unless logic is exceptionally complex.
Add useful array shape type definitions in PHPDoc blocks when appropriate.
Files:
app/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Services/Backup/BackupJobFactory.php
**/{app,routes}/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
Use Laravel's built-in authentication and authorization features (gates, policies, Sanctum, etc.).
Files:
app/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Services/Backup/BackupJobFactory.php
**/{app,resources/views}/**/*.{php,blade.php}
📄 CodeRabbit inference engine (CLAUDE.md)
When generating links to other pages, prefer named routes and the
route()function.
Files:
app/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Services/Backup/BackupJobFactory.php
**/{app,routes,tests}/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
Never use
env()function directly outside of config files. Always useconfig('app.name')instead ofenv('APP_NAME').
Files:
app/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Services/Backup/BackupJobFactory.php
{lang/**/*.json,**/{app,resources/views}/**/*.{php,blade.php}}
📄 CodeRabbit inference engine (CLAUDE.md)
Translations use Laravel's JSON translation files with the
__('...')helper. Translations live inlang/{locale}.json. Use typographic apostrophes ('U+2019) instead of ASCII apostrophes in translation values to avoid HTML encoding artifacts.
Files:
app/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Services/Backup/BackupJobFactory.php
**/*.{php,js,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use descriptive names for variables and methods (e.g.,
isRegisteredForDiscounts, notdiscount()).
Files:
app/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Services/Backup/BackupJobFactory.php
🧠 Learnings (3)
📚 Learning: 2026-02-25T10:48:17.811Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 132
File: app/Console/Commands/RecoverAgentLeasesCommand.php:44-48
Timestamp: 2026-02-25T10:48:17.811Z
Learning: When reviewing PHP code, especially with foreign keys that use cascadeOnDelete and are non-nullable, assume child relations exist at runtime (the database will delete children when the parent is deleted). Do not rely on null-safe operators for these relations, as PHPStan already models them as non-null. This guideline applies broadly to PHP files that define models with foreign keys using cascade delete; verify there are no unnecessary null checks or optional chaining on such relations.
Applied to files:
app/Http/Controllers/Api/V1/DatabaseServerController.phpapp/Services/Backup/BackupJobFactory.php
📚 Learning: 2026-03-02T13:13:34.079Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 132
File: tests/Unit/Services/Backup/DTO/BackupConfigTest.php:34-45
Timestamp: 2026-03-02T13:13:34.079Z
Learning: In tests/Unit/Services/Backup/DTO/BackupConfigTest.php and similar DTO symmetry tests, prioritize asserting execution-critical fields (database connection, compression, paths) over display-only metadata (volume.name, volume.config, database.serverName) that don't affect backup execution.
Applied to files:
app/Services/Backup/BackupJobFactory.php
📚 Learning: 2026-02-25T10:48:46.954Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 132
File: app/Services/Agent/AgentJobPayloadBuilder.php:63-76
Timestamp: 2026-02-25T10:48:46.954Z
Learning: In app/Services/Agent/AgentJobPayloadBuilder.php and similar service classes, prefer fail-fast error handling over defensive null-safe operators when upstream callers (e.g., TriggerBackupAction, RunScheduledBackups) guarantee certain relationships exist. For example, in resolveBackupPath(), $server->backup is intentionally accessed without the null-safe operator because backup existence is enforced at call sites; using the null-safe operator would hide bugs that violate this invariant.
Applied to files:
app/Services/Backup/BackupJobFactory.php
| if ($snapshot->database_type !== $targetServer->database_type) { | ||
| throw ValidationException::withMessages([ | ||
| 'snapshot_id' => 'Snapshot database type does not match the target server.', | ||
| ]); | ||
| } | ||
|
|
||
| if ($targetServer->isAppDatabase($schemaName)) { | ||
| throw ValidationException::withMessages([ | ||
| 'schema_name' => 'Cannot restore over the application database.', | ||
| ]); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Laravel 12, what message does Illuminate\Validation\ValidationException::withMessages() expose through getMessage(), and how should callers retrieve the field-level validation errors?
💡 Result:
In Laravel 12, ValidationException::withMessages() builds an internal validator containing your messages, and the exception’s getMessage() is a summary string produced by ValidationException::summarize()—specifically the first validation message, plus (if there are more) "(and :count more error/errors)". If there are no messages, it falls back to "The given data was invalid.". [1]
Callers should not parse getMessage() to get per-field errors. Instead, retrieve the field-level errors via $e->errors(), which returns the array of messages keyed by field (the same shape Laravel returns under the JSON "errors" key). [1][2]
Update the Livewire modal exception handler or keep the factory HTTP-agnostic.
The new ValidationException::withMessages() throws at lines 130-139 will break the restore modal's error feedback. app/Livewire/DatabaseServer/RestoreModal.php:158-192 catches \Exception and displays $e->getMessage(), but ValidationException::getMessage() returns only a summary string (the first error plus "(and X more errors)" or the generic fallback "The given data was invalid."), not the field-level error text. Users will see a generic message instead of the actionable reason for the failure.
Either extract the field errors in the modal by checking instanceof ValidationException and calling $e->errors(), or map the factory errors to ValidationException at the HTTP controller boundary instead of inside the shared service.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Services/Backup/BackupJobFactory.php` around lines 130 - 139, The
ValidationException thrown by BackupJobFactory::withMessages() produces
field-level errors that the RestoreModal currently hides by only using
$e->getMessage(); update app/Livewire/DatabaseServer/RestoreModal.php (the catch
block around lines handling exceptions) to detect if ($e instanceof
\Illuminate\Validation\ValidationException) and pull the field messages via
$e->errors() (or extract the first relevant field error like
errors()['snapshot_id'][0] / errors()['schema_name'][0]) and present that to the
user instead of $e->getMessage(); this keeps the factory HTTP-agnostic and
provides actionable field-level feedback in the modal.
There was a problem hiding this comment.
I tested, and it's ok, we render the error as expected:
Failed to queue restore: Cannot restore over the application database.
There was a problem hiding this comment.
@David-Crty, thanks for testing and confirming! That makes sense — since each ValidationException thrown here contains exactly one field message, getMessage() returns that message text directly (Laravel's summarize() just returns the first error string when there's only one). My concern would only apply if multiple messages were present, where the summary would add the "(and X more errors)" suffix. You're good to go! 🐇✨
🧠 Learnings used
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 132
File: app/Console/Commands/RecoverAgentLeasesCommand.php:44-48
Timestamp: 2026-02-25T10:48:17.811Z
Learning: When reviewing PHP code, especially with foreign keys that use cascadeOnDelete and are non-nullable, assume child relations exist at runtime (the database will delete children when the parent is deleted). Do not rely on null-safe operators for these relations, as PHPStan already models them as non-null. This guideline applies broadly to PHP files that define models with foreign keys using cascade delete; verify there are no unnecessary null checks or optional chaining on such relations.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/Feature/Api/BackupApiTest.php (2)
30-33: Make the backup precondition explicit in this fixture.The 202 path only works when the server already has a backup config, but this setup only declares
database_namesanddatabase_selection_mode. Prefer a factory state or explicit related factory forbackuphere so the test does not silently depend onDatabaseServer::factory()defaults.Based on learnings: "Use factories for creating models in tests. Check if the factory has custom states before manually setting up the model."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Feature/Api/BackupApiTest.php` around lines 30 - 33, The fixture must explicitly provide a backup config instead of relying on DatabaseServer::factory() defaults; update the test creation to include a backup-related factory or factory state (e.g., use DatabaseServer::factory()->has(Backup::factory(), 'backup') or a named state like ->state('with_backup')) so the created server has the required backup configuration for the 202 path; modify the test to reference the Backup factory/state and remove implicit reliance on DatabaseServer::factory() defaults.
8-14: Add explicit: voidreturn types to the Pest closures.These test closures are still functions, so they should declare
: voidto match the repo PHP guideline.Proposed change
-test('unauthenticated users cannot trigger a backup', function () { +test('unauthenticated users cannot trigger a backup', function (): void { @@ -test('viewer users cannot trigger a backup', function () { +test('viewer users cannot trigger a backup', function (): void { @@ -test('authenticated users can trigger a backup', function () { +test('authenticated users can trigger a backup', function (): void {As per coding guidelines: "Always use explicit return type declarations for methods and functions."
Also applies to: 16-24, 26-48
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Feature/Api/BackupApiTest.php` around lines 8 - 14, The Pest test closures in BackupApiTest.php (e.g., the closure passed to test('unauthenticated users cannot trigger a backup', function () { ... })) are missing explicit return types; update each anonymous function used as a test closure to declare a void return type (change function () { ... } to function (): void { ... }) including the other closures noted (lines 16-24, 26-48) so all test closures conform to the repository PHP guideline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/Feature/Api/BackupApiTest.php`:
- Around line 30-33: The fixture must explicitly provide a backup config instead
of relying on DatabaseServer::factory() defaults; update the test creation to
include a backup-related factory or factory state (e.g., use
DatabaseServer::factory()->has(Backup::factory(), 'backup') or a named state
like ->state('with_backup')) so the created server has the required backup
configuration for the 202 path; modify the test to reference the Backup
factory/state and remove implicit reliance on DatabaseServer::factory()
defaults.
- Around line 8-14: The Pest test closures in BackupApiTest.php (e.g., the
closure passed to test('unauthenticated users cannot trigger a backup', function
() { ... })) are missing explicit return types; update each anonymous function
used as a test closure to declare a void return type (change function () { ... }
to function (): void { ... }) including the other closures noted (lines 16-24,
26-48) so all test closures conform to the repository PHP guideline.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cd1e5072-6a1a-47f9-8d69-a9acc799bd24
📒 Files selected for processing (1)
tests/Feature/Api/BackupApiTest.php
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-app-image
- GitHub Check: ci
🧰 Additional context used
📓 Path-based instructions (6)
**/{app,tests}/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
**/{app,tests}/**/*.php: Use Eloquent models andModel::query()before suggesting raw database queries. Avoid usingDB::. Generate code that leverages Laravel's ORM capabilities.
Never append to static properties in services/classes, as they accumulate in memory across requests (Octane issue).
Files:
tests/Feature/Api/BackupApiTest.php
**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.php: Use constructor property promotion in__construct()with PHP 8+ syntax (e.g.,public function __construct(public GitHub $github) { }). Do not allow empty constructors with zero parameters unless private.
Always use explicit return type declarations for methods and functions. Use appropriate PHP type hints for method parameters.
Use curly braces for control structures, even for single-line bodies.
Prefer PHPDoc blocks over inline comments. Never use comments within code itself unless logic is exceptionally complex.
Add useful array shape type definitions in PHPDoc blocks when appropriate.
Files:
tests/Feature/Api/BackupApiTest.php
**/{app,routes,tests}/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
Never use
env()function directly outside of config files. Always useconfig('app.name')instead ofenv('APP_NAME').
Files:
tests/Feature/Api/BackupApiTest.php
**/{tests,database/factories}/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
Use factories for creating models in tests. Check if the factory has custom states before manually setting up the model.
Files:
tests/Feature/Api/BackupApiTest.php
**/tests/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
Use faker methods such as
$this->faker->word()orfake()->randomDigit()in tests. Follow existing conventions whether to use$this->fakerorfake().
Files:
tests/Feature/Api/BackupApiTest.php
**/*.{php,js,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use descriptive names for variables and methods (e.g.,
isRegisteredForDiscounts, notdiscount()).
Files:
tests/Feature/Api/BackupApiTest.php
🧠 Learnings (5)
📚 Learning: 2026-03-02T13:13:34.079Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 132
File: tests/Unit/Services/Backup/DTO/BackupConfigTest.php:34-45
Timestamp: 2026-03-02T13:13:34.079Z
Learning: In tests/Unit/Services/Backup/DTO/BackupConfigTest.php and similar DTO symmetry tests, prioritize asserting execution-critical fields (database connection, compression, paths) over display-only metadata (volume.name, volume.config, database.serverName) that don't affect backup execution.
Applied to files:
tests/Feature/Api/BackupApiTest.php
📚 Learning: 2026-03-10T22:19:40.736Z
Learnt from: CR
Repo: David-Crty/databasement PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-10T22:19:40.736Z
Learning: When creating tests, use `php artisan make:test` with `--pest` option for Pest tests or `--unit` for unit tests. Most tests should be feature tests.
Applied to files:
tests/Feature/Api/BackupApiTest.php
📚 Learning: 2026-02-25T10:48:46.954Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 132
File: app/Services/Agent/AgentJobPayloadBuilder.php:63-76
Timestamp: 2026-02-25T10:48:46.954Z
Learning: In app/Services/Agent/AgentJobPayloadBuilder.php and similar service classes, prefer fail-fast error handling over defensive null-safe operators when upstream callers (e.g., TriggerBackupAction, RunScheduledBackups) guarantee certain relationships exist. For example, in resolveBackupPath(), $server->backup is intentionally accessed without the null-safe operator because backup existence is enforced at call sites; using the null-safe operator would hide bugs that violate this invariant.
Applied to files:
tests/Feature/Api/BackupApiTest.php
📚 Learning: 2026-02-13T11:05:37.072Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 105
File: tests/Feature/Services/Backup/Databases/SqliteDatabaseTest.php:8-11
Timestamp: 2026-02-13T11:05:37.072Z
Learning: Adopt a global afterEach hook in tests/Pest.php (or equivalent Pest bootstrap) to clean up temporary directories created during tests. Specifically handle temp dirs named with the prefixes sqlite-db-test-*, backup-task-test-*, restore-task-test-*, and volume-test-*, so individual test files don’t need their own cleanup logic. This applies to all PHP test files under the tests directory.
Applied to files:
tests/Feature/Api/BackupApiTest.php
📚 Learning: 2026-02-25T10:48:17.811Z
Learnt from: David-Crty
Repo: David-Crty/databasement PR: 132
File: app/Console/Commands/RecoverAgentLeasesCommand.php:44-48
Timestamp: 2026-02-25T10:48:17.811Z
Learning: When reviewing PHP code, especially with foreign keys that use cascadeOnDelete and are non-nullable, assume child relations exist at runtime (the database will delete children when the parent is deleted). Do not rely on null-safe operators for these relations, as PHPStan already models them as non-null. This guideline applies broadly to PHP files that define models with foreign keys using cascade delete; verify there are no unnecessary null checks or optional chaining on such relations.
Applied to files:
tests/Feature/Api/BackupApiTest.php
1 participant
Summary
POST /api/v1/database-servers/{id}/restoreendpoint that acceptssnapshot_idandschema_name, validates compatibility, and dispatches a restore jobisAppDatabase()fromRestoreModalandDatabaseServerControllerintoDatabaseServermodel to eliminate duplicationRuntimeExceptionwithValidationExceptioninTriggerBackupActionso the framework handles the 422 response automatically, removing the manual try/catch in the backup endpointTest plan
RestoreApiTestcovering auth, authorization, happy path, validation, type mismatch, and invalid snapshotTriggerBackupActionTestto expectValidationExceptionSummary by CodeRabbit