Skip to content

Bugfix/notification#88

Merged
HerrKanin merged 2 commits into
mainfrom
bugfix/notification
Apr 16, 2026
Merged

Bugfix/notification#88
HerrKanin merged 2 commits into
mainfrom
bugfix/notification

Conversation

@HerrKanin

@HerrKanin HerrKanin commented Apr 16, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

Release Notes

  • New Features

    • Unread notifications are now displayed in chronological order, with newest notifications appearing first.
    • Notifications related to a specific incident are automatically marked as read when the incident is viewed.
  • Refactor

    • Enhanced backend notification system for improved state management and retrieval efficiency.

@coderabbitai

coderabbitai Bot commented Apr 16, 2026

Copy link
Copy Markdown

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

The PR refactors notification-read marking from client-side to server-side. When accessing an incident, the controller now marks all unread notifications for that incident as read before returning the incident data. Repository and service layer methods have been updated to query and mark notifications by incident ID, with unread notifications now sorted by creation date descending.

Changes

Cohort / File(s) Summary
Incident Notification Integration
src/main/java/org/example/team6backend/incident/controller/IncidentController.java
Injected NotificationService dependency; getIncidentById now calls markNotificationAsReadForIncident() before fetching the incident to mark relevant unread notifications as read server-side.
Notification Service Layer
src/main/java/org/example/team6backend/notification/service/NotificationService.java, src/main/java/org/example/team6backend/notification/repository/NotificationRepository.java
Added markNotificationAsReadForIncident(userId, incidentId) with @Transactional; updated findByUserIdAndReadFalse() to sort descending by creation date; added findByUserIdAndIncidentIdAndReadFalse() to query notifications by incident.
Notification Controller
src/main/java/org/example/team6backend/notification/controller/NotificationController.java
Changed getUserNotifications() to call getUnreadNotifications() instead of getUserNotifications(), returning only unread notifications.
Client-Side UI
src/main/resources/templates/dashboard.html
Removed fetch PATCH call to mark notifications as read from openNotification() function; notification marking now handled server-side when incident is accessed.

Sequence Diagram

sequenceDiagram
    actor User
    participant Dashboard as Dashboard (UI)
    participant IncidentCtrl as IncidentController
    participant NotifService as NotificationService
    participant IncidentService as IncidentService
    participant NotifRepo as NotificationRepository
    participant DB as Database

    User->>Dashboard: Click notification
    Dashboard->>IncidentCtrl: GET /incidents/{incidentId}
    IncidentCtrl->>NotifService: markNotificationAsReadForIncident(userId, incidentId)
    NotifService->>NotifRepo: findByUserIdAndIncidentIdAndReadFalse(userId, incidentId)
    NotifRepo->>DB: Query unread notifications
    DB-->>NotifRepo: Unread notifications for incident
    NotifService->>NotifRepo: saveAll(marked as read)
    NotifRepo->>DB: Update notifications
    DB-->>NotifRepo: Confirmed
    IncidentCtrl->>IncidentService: getById(incidentId, currentUser)
    IncidentService->>DB: Fetch incident
    DB-->>IncidentService: Incident data
    IncidentService-->>IncidentCtrl: Incident
    IncidentCtrl-->>Dashboard: Incident response
    Dashboard->>User: Display incident
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Poem

🐰✨ Notifications hop to the server-side now,
Where incidents mark them read—oh, how!
No more client-side chatter and fuss,
The backend takes care of us,
Efficiency springs eternal, meow! 🌱

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Bugfix/notification' is vague and generic, using a branch name format that doesn't clearly describe the specific bug being fixed or the main change. Replace with a specific title describing the bug fix, such as 'Mark notifications as read when viewing incident' or 'Fix notification ordering and incident read-marking'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch bugfix/notification

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.

@HerrKanin
HerrKanin merged commit 913b4a5 into main Apr 16, 2026
2 of 3 checks passed
@kristinaxm
kristinaxm deleted the bugfix/notification branch April 23, 2026 17:30
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.

1 participant