Skip to content

fix(channel): mark message notification viewed when it arrives after mount#2707

Merged
gbirman merged 1 commit into
mainfrom
fix/mark-message-notifications-race
Apr 21, 2026
Merged

fix(channel): mark message notification viewed when it arrives after mount#2707
gbirman merged 1 commit into
mainfrom
fix/mark-message-notifications-race

Conversation

@gbirman
Copy link
Copy Markdown
Contributor

@gbirman gbirman commented Apr 20, 2026

Summary

MarkMessageNotifications used onMount, which ran exactly once: it looked for a matching notification in the cache and, if missing, fell back to a websocket-only subscription. That fallback never fires for notifications that land in the cache via the subsequent paginated query fetch (cold start / full refresh), so viewed_at stayed null until the user reloaded the app.

Switch to createEffect with a marked guard so the mark-as-read happens as soon as the notification appears in the reactive notifications() memo — whether it came from the query or a websocket push.

Repro before

  1. Receive a channel message while the app is closed.
  2. Reopen the app and view the channel.
  3. Notification stays viewed_at: null; the unread badge on the channel in the sidebar persists until reload.

…mount

MarkMessageNotifications used onMount which ran exactly once. If the
matching notification was not yet in the cache at mount time (query
still loading on cold start), we set up a websocket-only subscription
as a fallback -- but that subscription never fires for notifications
that arrive via the subsequent query fetch, so viewed_at stayed null
until a full reload.

Switch to createEffect with a "marked" guard so the mark-as-read fires
as soon as the notification shows up in the reactive notifications()
memo, regardless of whether it came from the query or the socket.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 20, 2026

Warning

Rate limit exceeded

@gbirman has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 48 minutes and 45 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 48 minutes and 45 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a0f0e645-9d29-4fd6-b971-ec2d7524caec

📥 Commits

Reviewing files that changed from the base of the PR and between f9bb1f8 and df4b96e.

📒 Files selected for processing (1)
  • js/app/packages/notifications/components/MarkMessageNotifications.tsx

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.

@github-actions
Copy link
Copy Markdown

@gbirman gbirman merged commit 9303faa into main Apr 21, 2026
24 checks passed
@gbirman gbirman deleted the fix/mark-message-notifications-race branch April 21, 2026 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant