Skip to content

Conversation

@tvarohohlavy
Copy link
Contributor

@tvarohohlavy tvarohohlavy commented Dec 22, 2025

This pull request improves the backup notification system by including the backup schedule name in both log messages and user notifications. This enhancement provides clearer context for backup operations, making it easier to identify which schedule triggered a backup in logs and notifications.

Logging and Notification Improvements:

  • Updated log messages in backups.service.ts to include the backup schedule name, providing more detailed context for backup start, completion, warning, and failure events.
  • Modified backup notification payloads to include the scheduleName property, ensuring that notifications sent to users specify the relevant backup schedule.

Notification Message Formatting:

  • Enhanced the buildNotificationMessage function in notifications.service.ts to add the schedule name to notification bodies for various backup events, improving the clarity of notification content.

Related Issues:
Fixes #213

Summary by CodeRabbit

  • Improvements
    • Backup notifications and logs now include schedule names for clearer identification of each backup operation.
    • Schedule names are included in start, progress, completion (success/warning), and failure messages and notification payloads for more informative and consistent status updates.

✏️ Tip: You can customize this high-level summary in your review settings.

Copilot AI review requested due to automatic review settings December 22, 2025 10:56
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 22, 2025

Walkthrough

Schedule names are added to backup log messages and included conditionally in notification bodies; notification body construction was refactored to an array-based pattern with filtering for consistent inclusion of the Schedule line.

Changes

Cohort / File(s) Change Summary
Backup schedule logging & payloads
app/server/modules/backups/backups.service.ts
Adds schedule.name into user-facing log messages and includes scheduleName in start, progress, completion (success/warning), and failure notification payloads; error logs now include schedule context.
Notification body formatting
app/server/modules/notifications/notifications.service.ts
Adds conditional Schedule: ${context.scheduleName} line across start/success/warning/failure/default branches and refactors body construction to an array-plus-filter join pattern for consistent formatting.

Possibly related PRs

  • feat: naming backup schedules #103 — Propagates schedule.name through schema/DTOs/services and the client; strongly connected to adding schedule name to notifications/logs.
  • feat: partial success warning status #74 — Modifies the same notification and backup service areas, including notification message construction and completion handling that overlap with these changes.

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main changes: adding schedule name to backup notifications and logs, which directly aligns with the changeset.
Linked Issues check ✅ Passed The PR fulfills issue #213 by adding the backup schedule name to notifications and logs across multiple service modules.
Out of Scope Changes check ✅ Passed All changes are focused on adding schedule name to notifications and logs in the two specified service files, with no unrelated modifications.

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 45187ba and 84ce2fb.

📒 Files selected for processing (1)
  • app/server/modules/backups/backups.service.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx,json}

📄 CodeRabbit inference engine (AGENTS.md)

Use Biome for code formatting and linting with bunx biome check --write ., format only with bunx biome format --write ., or lint with bunx biome lint .

Files:

  • app/server/modules/backups/backups.service.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Use tabs (not spaces) for indentation with a line width of 120 characters
Use double quotes for strings
Do not auto-organize imports - imports organization is disabled in Biome
All imports must include file extensions when targeting Node/Bun, as the project uses "type": "module"

Files:

  • app/server/modules/backups/backups.service.ts
app/server/modules/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

Server follows a modular service-oriented architecture with controller-service-database pattern in each module

Files:

  • app/server/modules/backups/backups.service.ts
🧠 Learnings (1)
📚 Learning: 2025-12-28T17:31:39.157Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.157Z
Learning: Applies to app/server/jobs/**/*.ts : New scheduled jobs must extend `Job` class, implement `run()` method, and be registered in `app/server/modules/lifecycle/startup.ts` with a cron expression

Applied to files:

  • app/server/modules/backups/backups.service.ts
🧬 Code graph analysis (1)
app/server/modules/backups/backups.service.ts (3)
app/server/utils/logger.ts (1)
  • logger (36-41)
app/server/index.ts (1)
  • error (42-44)
app/server/utils/errors.ts (1)
  • toMessage (16-19)
🔇 Additional comments (3)
app/server/modules/backups/backups.service.ts (3)

239-255: LGTM! Schedule name added to backup start logs and notifications.

The inclusion of schedule.name in the log message and scheduleName in the notification payload enhances observability and addresses the linked issue #213. The changes are consistent with the existing format and safe to access since the schedule object is already loaded.


344-369: LGTM! Schedule name added to backup completion logs and notifications.

The changes consistently include schedule.name in both warning and success completion logs, and add scheduleName to the completion notification payload. The format is consistent with the start log message, maintaining clarity across the backup lifecycle.


370-401: LGTM! Schedule name added to backup error logs and failure notifications.

The error log and failure notification now include the schedule name, completing the enhancement across all backup states (start, completion, failure). This provides consistent context throughout the backup lifecycle and aligns with the PR objectives.


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

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

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request enhances backup observability by including the backup schedule name in log messages and notification content. This makes it easier for users and administrators to identify which schedule triggered a backup event across different monitoring channels.

Key Changes:

  • Enhanced log messages to include schedule names in backup lifecycle events (start, completion, warnings, and failures)
  • Added scheduleName to notification payloads and message bodies across all backup event types

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
app/server/modules/backups/backups.service.ts Updated logger statements to include schedule names and added scheduleName property to all notification payloads sent during backup operations
app/server/modules/notifications/notifications.service.ts Modified buildNotificationMessage function to conditionally include schedule name in notification bodies for start, success, warning, failure, and default event types

After thoroughly reviewing the changes, I found no issues with this pull request. The implementation is:

  • Consistent: The schedule name is added to all relevant log messages and notification types
  • Well-structured: Uses conditional rendering (context.scheduleName ? ...) to handle optional schedule names
  • Type-safe: The scheduleName property is properly typed as optional in the context interface
  • Following existing patterns: The changes align with the existing code style and structure in both files

The changes successfully achieve the stated goal of improving backup notification and logging context without introducing any bugs or issues.

@tvarohohlavy
Copy link
Contributor Author

@nicotsx I am working on the backup mirrors support in export and import PRs and realized there are no notifications for them.
Basically you get Backup complete, but then mirrors are happening without any notification.
Is it something you would want have notification for? If yes, how should it look? Thanks

@nicotsx nicotsx force-pushed the schedule-nms-in-ntfcts-and-lgs branch from 048d0bd to 45187ba Compare December 29, 2025 08:05
@nicotsx
Copy link
Owner

nicotsx commented Dec 29, 2025

@tvarohohlavy let's discuss it next year :) I have opened an issue #259 to track it. I think we should have this indeed but I'm not sure about the UI yet

@tvarohohlavy
Copy link
Contributor Author

@nicotsx I have fixed the code style issues so its probably ready for merge as you have approved it already. Thanks

@nicotsx nicotsx merged commit 94c84d7 into nicotsx:main Jan 2, 2026
3 checks passed
@tvarohohlavy tvarohohlavy deleted the schedule-nms-in-ntfcts-and-lgs branch January 2, 2026 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Name of backup into notifications

2 participants