Notification toast

[feruzm]

Summary by CodeRabbit

• New Features

  • Notification batching: quick successive notifications are grouped into a single aggregated alert with pluralized copy.
  • Single sound per burst and single clickable alert that opens/toggles the notifications panel.
  • Permission requested once per batch; if denied, a single in-app toast is shown instead.

• Bug Fixes

  • Pending notifications cleared when disconnecting to avoid stale alerts.

[coderabbitai]

Warning

Rate limit exceeded

@feruzm has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 13 minutes and 53 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 13 minutes and 53 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d89ea809-ed9d-4a58-933f-1c20f251c733

📥 Commits

Reviewing files that changed from the base of the PR and between 8f1cc00 and cc09b08.

📒 Files selected for processing (1)

• apps/web/src/api/notifications-ws-api.ts

📝 Walkthrough

Walkthrough

Implements 500ms burst aggregation for incoming WebSocket notifications: messages are queued, flushed as a single batched browser or in‑app notification with one sound, and pending timers/messages are cleared on disconnect. Adds i18n key for batched text and renames the settings label.

Changes

Cohort / File(s)	Summary
Notification Burst Aggregation apps/web/src/api/notifications-ws-api.ts	Added pendingMessages queue and burstTimer (500ms). Replaced per-message handling with queueNotification() and flushPendingNotifications() to produce one aggregated toast/body, request permission once per flush, play a single sound, create one browser Notification on "granted" (with onclick toggling UI), or emit one in-app info() and toggle UI when not shown. disconnect() clears timer and drops queued messages.
Localization Updates apps/web/src/features/i18n/locales/en-US.json	Renamed notifications.settings value to "Sound/Popup Settings" and added notifications.new-notifications-batch: "{{count}} new notifications" for batched messages.

Sequence Diagram(s)

sequenceDiagram
    participant WS as WebSocket
    participant API as notifications-ws-api
    participant Sound as Sound Player
    participant App as App/EventBus
    participant Browser as Browser Notification API

    WS->>API: incoming message(s)
    API->>API: enqueue into pendingMessages
    API-->>API: start/reset burstTimer (500ms)
    Note right of API: After burstTimer expires
    API->>API: flushPendingNotifications()
    API->>App: emit aggregated i18n string / in-app toast
    API->>Sound: play single notification sound
    API->>Browser: request Notification permission (once)
    Browser-->>API: permission "granted" / "denied"
    alt granted
        API->>Browser: create single Browser Notification (onclick toggles UI)
    else denied
        API->>App: emit in-app toast and toggle notifications UI if needed
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested labels

patch

Poem

🐰 I queued the pings in a cozy heap,
Waited half a beat before the leap.
One chime, one notice, tidy and neat,
No more scatter, no repeated bleat.
Hop, burrow, rest — users sleep sweet. 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'Notification toast' is vague and generic, using non-descriptive terms that don't clearly convey the specific changes made to the notification system.	Consider a more descriptive title like 'Batch WebSocket notifications with 500ms debounce' or 'Implement notification message batching and debouncing' to better reflect the main technical change.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch toast

---

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]

♻️ Duplicate comments (1)

apps/web/src/api/notifications-ws-api.ts (1)

252-257: ⚠️ Potential issue | 🟡 Minor

Add error handling for the async flush call.

flushPendingNotifications() is async but called without error handling. An unhandled promise rejection could occur if the flush fails.

🛡️ Proposed fix

     if (!this.burstTimer) {
       this.burstTimer = setTimeout(() => {
         this.burstTimer = null;
-        this.flushPendingNotifications();
+        this.flushPendingNotifications().catch((error) => {
+          console.error("notifications burst flush failed", error);
+        });
       }, BURST_WINDOW_MS);
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/api/notifications-ws-api.ts` around lines 252 - 257, The
setTimeout callback starts an async operation without handling rejections: in
the block that sets this.burstTimer and calls this.flushPendingNotifications()
(the code referencing this.burstTimer, BURST_WINDOW_MS and the
flushPendingNotifications method), ensure the async call is awaited or its
Promise is handled — for example call flushPendingNotifications().catch(err =>
process/log error) or invoke an async IIFE and await inside a try/catch — so any
rejection is caught and logged instead of causing an unhandled promise
rejection.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@apps/web/src/api/notifications-ws-api.ts`:
- Around line 252-257: The setTimeout callback starts an async operation without
handling rejections: in the block that sets this.burstTimer and calls
this.flushPendingNotifications() (the code referencing this.burstTimer,
BURST_WINDOW_MS and the flushPendingNotifications method), ensure the async call
is awaited or its Promise is handled — for example call
flushPendingNotifications().catch(err => process/log error) or invoke an async
IIFE and await inside a try/catch — so any rejection is caught and logged
instead of causing an unhandled promise rejection.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 957647ac-f770-4b6f-80ef-84e91eb971b4

📥 Commits

Reviewing files that changed from the base of the PR and between f895e9a and 8f1cc00.

📒 Files selected for processing (1)

• apps/web/src/api/notifications-ws-api.ts