Only disable "delete backup" when backup hasn't failed

[Boy132]

Currently, if a failed backup is locked there is no way to unlock or delete it. This makes a failed backup deletable, even if it's locked.

[coderabbitai]

📝 Walkthrough

Walkthrough

Updated the DeleteAction disabled condition in BackupResource::defaultTable: previously disabled when a backup was is_locked; now disabled only when is_locked and status is not Failed. This permits deleting locked backups with Failed status. No public API signatures changed.

Changes

Cohort / File(s)	Summary
Backup delete-action guard app/Filament/Server/Resources/Backups/BackupResource.php	Adjusted DeleteAction disabled predicate from is_locked to is_locked && status != Failed, enabling deletion of locked backups in Failed state; all other statuses remain disabled when locked.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant Table as BackupResource Table
    participant Delete as DeleteAction
    participant Guard as Disabled Condition

    User->>Table: Open Backups list
    User->>Delete: Attempt Delete on a backup
    Delete->>Guard: Evaluate (is_locked && status != Failed)?
    alt Guard returns true (disabled)
        Delete-->>User: Delete unavailable
    else Guard returns false (enabled)
        Delete->>Table: Proceed with delete flow
        Table-->>User: Deletion executed
    end

    note over Guard: New logic allows delete when is_locked && status == Failed

Loading

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title clearly and concisely summarizes the primary change by indicating that the delete action is only disabled for backups that have not failed, which matches the updated conditional logic in the code. It avoids unnecessary details and uses readable phrasing that a teammate scanning the history can understand.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check	✅ Passed	The pull request description clearly outlines the existing issue of locked, failed backups being undeletable and states the change to make such backups deletable, directly reflecting the code modifications to the DeleteAction disabled condition.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

app/Filament/Server/Resources/Backups/BackupResource.php (2)

90-90: Nit: fix class casing to match import (Textarea)

Avoid case-mismatch with the imported Filament component.

Apply:

-                TextArea::make('ignored')
+                Textarea::make('ignored')

---

251-254: Use status enum for the 'failed' activity property for consistency

Keeps telemetry aligned with the UI’s status-based logic and prevents drift if failure semantics evolve.

-                            Activity::event('server:backup.delete')
-                                ->subject($backup)
-                                ->property(['name' => $backup->name, 'failed' => !$backup->is_successful])
+                            Activity::event('server:backup.delete')
+                                ->subject($backup)
+                                ->property(['name' => $backup->name, 'failed' => $backup->status === BackupStatus::Failed])
                                 ->log();

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1900c04 and 391d245.

📒 Files selected for processing (1)

• app/Filament/Server/Resources/Backups/BackupResource.php (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

app/Filament/Server/Resources/Backups/BackupResource.php (1)

app/Models/Backup.php (1)

• Backup (36-107)

🔇 Additional comments (2)

app/Filament/Server/Resources/Backups/BackupResource.php (2)

230-230: Condition change LGTM — enables deleting locked failed backups as intended

This precisely matches the PR goal: deletion is disabled only when locked and not Failed; still hidden during InProgress.

---

233-255: Service allows deletion of failed locked backups
Verified that in DeleteBackupService::handle(), the BackupLockedException is only thrown when $backup->is_locked && $backup->is_successful && $backup->completed_at are true, so failed backups (where is_successful is false) can still be deleted even if locked.