Skip to content

fix(unread): prevent negative totals, fix edge cases, and add test coverage#78

Open
DSBaibhav wants to merge 4 commits into
0rigin-c0de:mainfrom
DSBaibhav:fix/unread-count-edge-cases
Open

fix(unread): prevent negative totals, fix edge cases, and add test coverage#78
DSBaibhav wants to merge 4 commits into
0rigin-c0de:mainfrom
DSBaibhav:fix/unread-count-edge-cases

Conversation

@DSBaibhav
Copy link
Copy Markdown

@DSBaibhav DSBaibhav commented May 15, 2026 •
edited
Loading

Summary

Resolved the unread count edge cases described in the issue.

Changes

server/services/unreadService.js

  • Clamp server totals to ≥ 0 on both write and read (prevents negative totals)
  • Reset Redis client promise on error/failure so reconnects work correctly
  • Add isUsingRedis() diagnostic helper and resetForTesting() for unit tests
  • Document Redis vs in-memory fallback behavior in JSDoc

server/routes/notifications.js

  • Validate required body params — return 400 when friend_id, server_id, or channel_id are missing
  • Add GET /unread_storage_mode route to inspect which backend is active

frontend/src/store/unreadSlice.js

  • Clamp server total with Math.max(0, ...) in clear_channel_unread
  • Remove server entry only when total reaches exactly 0 (not ≤ 0)

frontend/src/components/notifications/NotificationListener.jsx

  • Re-fetch unread summary on socket reconnect (covers device sleep/wake and server restarts)
  • Wrap fetchUnreadSummary in try/catch with non-OK HTTP status handling
  • Explicitly call socket.connect() since socket now uses autoConnect: false

server/scripts/test-unread-edge-cases.mjs (new)

  • 20 automated assertions covering all edge cases
  • Run with: npm run test:unread

Test Results

Results: 20 passed, 0 failed ✅

Acceptance Criteria

  • Clearing unread counts never produces negative server totals
  • DM unread counts clear correctly when the active conversation is open
  • Server channel unread counts clear correctly when the active channel is open
  • Redis and memory fallback behavior are documented
  • Edge cases are covered by automated tests

DSBaibhav added 2 commits May 16, 2026 01:09
@DSBaibhav
fix(unread): prevent negative totals and fix edge cases
9e2d8fe 
- unreadService.js: clamp server totals to >= 0 on write and read;
  reset Redis client promise on error/failure so reconnects work;
  add isUsingRedis() diagnostic helper and resetForTesting() for tests;
  document Redis vs in-memory fallback behavior in JSDoc

- notifications.js: validate required body params (400 on missing
  friend_id / server_id / channel_id); add GET /unread_storage_mode
  diagnostic route to inspect active storage backend

- unreadSlice.js: clamp server total with Math.max(0, ...) in
  clear_channel_unread so double-clearing never produces a negative
  total; remove server entry only when total reaches exactly 0

- NotificationListener.jsx: add socket 'connect' handler to re-fetch
  unread summary on reconnect (covers device sleep/wake, server
  restart, Redis loss); wrap fetchUnreadSummary in try/catch with
  non-OK HTTP status handling; add .catch() to fire-and-forget
  mark-read fetches; call socket.connect() explicitly since socket
  now uses autoConnect:false

- test-unread-edge-cases.mjs: new 20-assertion automated test script
  covering DM and server channel edge cases (double-clear, multi-user
  isolation, cross-server isolation, memory reset / reconnect sim)

Closes #<issue-number>

Test: npm run test:unread (20/20 passing)
@DSBaibhav
fix(dx): fix blank screen on startup and improve dev experience
ff0b82a 
- Socket.jsx: switch from socketIO.connect() to socketIO() with
  autoConnect:false to prevent crash-on-import when backend is
  unreachable; add polling transport fallback and reconnection config;
  add DEV console logs for connect/disconnect/error events

- supabaseClient.js: guard createClient() with URL validation and
  try/catch so an invalid/missing SUPABASE_URL never crashes the app
  at module-load time (was causing blank white/black screen)

- App.jsx: wrap root with ErrorBoundary so any render crash shows a
  visible error message instead of a completely blank page

- ErrorBoundary.jsx: new class component that catches React tree
  errors and displays a styled fallback with the error message and a
  reload button

- Register.jsx: fix date-of-birth validation — month dropdown now
  stores the full month name instead of a numeric index; validation
  uses ISO YYYY-MM-DD format so new Date() parses reliably across all
  browsers (fixes 'Please enter a valid date' on every valid date)

- dev-server.mjs: new development startup script that auto-spins up
  mongodb-memory-server when no real MONGO_URI is set, so the server
  can run locally without any database configuration; also injects a
  dev-only ACCESS_TOKEN when none is provided
@DSBaibhav DSBaibhav requested a review from 0rigin-c0de as a code owner May 15, 2026 19:51
@netlify
Copy link
Copy Markdown

netlify Bot commented May 15, 2026 •
edited
Loading

👷 Deploy request for piperchat01 pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 3ce1809

@DSBaibhav DSBaibhav force-pushed the fix/unread-count-edge-cases branch from 3ce1809 to 23d9212 Compare May 18, 2026 22:16
@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented May 18, 2026

@DSBaibhav is attempting to deploy a commit to the Sunil Kumar's projects Team on Vercel.

A member of the Team first needs to authorize it.

@DSBaibhav
Copy link
Copy Markdown
Author

DSBaibhav commented May 18, 2026

Hey @0rigin-c0de,just following up on this PR whenever you get time to review it. Please let me know if any changes or improvements are needed from my side. Thanks!

@DSBaibhav
chore: merge upstream/main into fix/unread-count-edge-cases
58033d4 
Resolve conflicts across all conflicting files:

- frontend/src/App.jsx: keep ErrorBoundary + add Vercel Analytics
- frontend/src/components/notifications/NotificationListener.jsx:
  adopt upstream API_BASE_URL from config; keep reconnect handler,
  notification prefs re-sync, full error handling and JSDoc comments
- frontend/src/components/socket/Socket.jsx:
  adopt SOCKET_URL from config with VITE env var fallback chain;
  keep autoConnect:false, transport fallbacks, withCredentials, debug helpers
- server/package.json: adopt src/ entry point and new deps
  (logtail, compression, helmet, rate-limit, winston);
  keep dev:local, watch:local, test:unread, gmail:oauth-setup scripts
- server/src/routes/notifications.js: adopt logger; keep input
  validation and /unread_storage_mode diagnostic endpoint
- server/src/services/unreadService.js: adopt config/logger pattern;
  keep Redis promise-reset-on-error, clamping to >=0,
  clearServerChannelUnread algorithm, isUsingRedis(), resetForTesting()
@DSBaibhav
Copy link
Copy Markdown
Author

DSBaibhav commented May 24, 2026

Hey @0rigin-c0de,I have resolved all the conflicts,just following up please review this PR whenever you get the time under GSSoC'26

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@0rigin-c0de 0rigin-c0de Awaiting requested review from 0rigin-c0de 0rigin-c0de is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@DSBaibhav