Add backup completed event, listener, and notification system #2041
Conversation
…n-dev#1885) Integrates new features to notify users of completed backups via panel and email, including configuration toggles and localization updates.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughIntroduces a notification system for completed backups that alerts server owners via in-app and email notifications. Adds a new event, listener, and notification class, with configuration option and admin toggle to control email delivery preferences. Changes
Sequence Diagram(s)sequenceDiagram
participant API as BackupStatusController
participant DB as Database
participant Listener as BackupCompletedListener
participant Panel as Panel Notification
participant Config as Config
participant Email as Mail Notification
API->>API: Compute $successful status
API->>DB: Begin transaction & save backup log
DB-->>API: Transaction complete
API->>Listener: Dispatch BackupCompleted(backup, successful)
Listener->>Listener: Load server & user relations
Listener->>Panel: Send panel notification<br/>(status: success/danger)
Panel-->>Listener: Notification sent to user
Listener->>Config: Check send_backup_notification
alt Email enabled
Listener->>Email: Send BackupCompleted<br/>notification email
Email-->>Listener: Email queued
end
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 review |
✅ Actions performedReview triggered.
|
![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 (1)
app/Listeners/Backup/BackupCompletedListener.php (1)
35-37: Consider email notifications for failed backups.Currently, email notifications are only sent for successful backups. Users might want to be notified via email when backups fail as well, especially for critical servers. Consider whether the config should control both success and failure notifications, or if there should be separate toggles.
Potential enhancement
// Send email notification if enabled and backup was successful -if ($event->successful && config()->get('panel.email.send_backup_notification', true)) { +if (config()->get('panel.email.send_backup_notification', true)) { $user->notify(new BackupCompletedNotification($event->backup)); }This would notify on both success and failure. The email template would need updating to handle both cases.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/Events/Backup/BackupCompleted.phpapp/Filament/Admin/Pages/Settings.phpapp/Http/Controllers/Api/Remote/Backups/BackupStatusController.phpapp/Listeners/Backup/BackupCompletedListener.phpapp/Notifications/BackupCompleted.phpconfig/panel.phplang/en/admin/setting.phplang/en/notifications.php
🧰 Additional context used
🧬 Code graph analysis (3)
app/Listeners/Backup/BackupCompletedListener.php (2)
app/Events/Backup/BackupCompleted.php (1)
BackupCompleted(9-17)app/Notifications/BackupCompleted.php (1)
BackupCompleted(13-34)
app/Events/Backup/BackupCompleted.php (3)
app/Models/Backup.php (1)
Backup(36-107)app/Events/Event.php (1)
Event(5-5)app/Notifications/BackupCompleted.php (2)
BackupCompleted(13-34)__construct(17-17)
app/Http/Controllers/Api/Remote/Backups/BackupStatusController.php (4)
app/Models/Backup.php (1)
Backup(36-107)app/Events/Backup/BackupCompleted.php (1)
BackupCompleted(9-17)app/Notifications/BackupCompleted.php (1)
BackupCompleted(13-34)app/Services/Activity/ActivityLogService.php (3)
log(136-155)transaction(173-182)event(48-53)
🔇 Additional comments (9)
lang/en/admin/setting.php (1)
121-121: LGTM! Translation entry is consistent.The new
backup_completedtranslation follows the existing pattern for notification labels and integrates well with the mail notifications section.lang/en/notifications.php (1)
9-12: LGTM! Translation keys are well-structured.The new backup notification translation keys follow Laravel conventions with proper placeholder syntax and integrate cleanly with the existing notification strings.
app/Http/Controllers/Api/Remote/Backups/BackupStatusController.php (1)
61-61: Good refactoring: computing $successful outside the transaction.Extracting the boolean value before the transaction improves readability and ensures consistent use of the same value throughout the operation.
config/panel.php (1)
51-52: LGTM! Configuration follows established patterns.The new
send_backup_notificationconfig option is consistent with existing email notification settings and provides appropriate defaults.app/Filament/Admin/Pages/Settings.php (1)
685-694: LGTM! Toggle implementation is consistent and well-structured.The new backup notification toggle follows the established pattern perfectly, with proper state casting, validation, and default value resolution.
app/Events/Backup/BackupCompleted.php (1)
1-17: LGTM! Clean event implementation.The event class is well-structured with appropriate use of promoted properties and the
SerializesModelstrait. The$successfulparameter enables listeners to handle success and failure cases differently.app/Listeners/Backup/BackupCompletedListener.php (2)
15-15: Good practice: eager loading relationships.Using
loadMissing()to preload the required relationships prevents N+1 queries and ensures data availability for both notifications.
21-32: LGTM! Panel notification implementation is solid.The Filament notification properly handles both success and failure cases with localized content and provides a clear action button for users to view their backups.
app/Notifications/BackupCompleted.php (1)
13-13: LGTM! Proper use of ShouldQueue.Implementing
ShouldQueueensures email notifications are sent asynchronously, preventing backup completion acknowledgment from being delayed by email delivery.
1 participant
Closes #1742