Notification Improvements

[sailalithkanumuri8]

Description

Resolves #231

Improved notification handling, added streaks, and adjusted notification design to better match the figma.

Checklist

• Title is same as the ticket title
• The ticket is mentioned above
• The changes fulfill the success criteria of the ticket
• Relevant developers have been assigned
• Relevant reviewers (EM, etc.) have been requested to review

[netlify]

✅ Deploy Preview for bog-ican ready!

Name	Link
🔨 Latest commit	1112770
🔍 Latest deploy log	https://app.netlify.com/projects/bog-ican/deploys/69bdf3bec9ed250008f360c8
😎 Deploy Preview	https://deploy-preview-242--bog-ican.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[greptile-apps]

Greptile Summary

This PR improves the notification system by adding streak-risk warnings, 24-hour time formatting, per-type notification toggles, an early reminder window setting, and a redesigned NotificationBanner toast component that better matches the Figma design. It also gates the Notifications settings behind the user's PIN when one is set.

Key changes:

• NotificationBanner.tsx — new toast component for in-app notification display; uses hardcoded hex colors via inline styles and text-[11px] instead of project CSS variables and rem units
• NotificationSettingsModal.tsx — expanded UI for notification types, early window, and 24-hour clock; earlyWindow input fires a DB mutation on every keystroke with no debounce
• notification.ts (service) — adds streak warning notification logic and a per-user medication-log cache to avoid redundant DB calls; streak warning check is nested inside the regular notification send path, meaning it can be silently skipped when the regular notification already exists in the database
• SettingsModal.tsx — PIN gate before opening notification settings; handlePinVerifiedForNotif is declared async unnecessarily
• tsconfig.json — corrects JSX mode from "react-jsx" to "preserve" (appropriate for Next.js/SWC)

Areas needing improvement per checklist:

• ❌ No hardcoded colors/sizes — NotificationBanner.tsx uses hardcoded hex values (#4DBDBA, #B7BDEF, #1E2353, etc.) via inline styles; NotificationSettingsModal.tsx new sections continue the same pattern
• ❌ rem units — text-[11px] in NotificationBanner.tsx should be text-[1.1rem]
• ❌ Handles error responses — earlyWindow has no debounce; a fast typist triggers many redundant mutations

Score: 68 / 100

Confidence Score: 3/5

• Functional but has style violations and a subtle streak-warning edge case that should be addressed before merge.
• Core logic (Ably subscription, streak warning, PIN gating, settings persistence) is sound. Main concerns are widespread hardcoded colors/px units in new components against project conventions, the missing debounce on the early-window input, and the streak warning being silently skipped if the regular notification record already exists. No security issues or data-loss risks identified.
• src/components/NotificationBanner.tsx (hardcoded colors and px units throughout), src/components/modals/NotificationSettingsModal.tsx (hardcoded colors + no debounce on earlyWindow input), src/services/notification.ts (streak warning coupling)

Important Files Changed

Filename	Overview
src/services/notification.ts	Major additions: streak warning logic, 24-hour time formatting, and a logs cache for performance. Streak warning is coupled to the regular notification send path, meaning it can be silently skipped if a regular notification already exists for that window.
src/components/NotificationBanner.tsx	New component for toast-based notification display. Uses pervasive hardcoded hex colors via inline styles and hardcoded pixel units (text-[11px]) instead of CSS variables and rem units per project conventions.
src/components/hooks/useNotifications.tsx	New hook that subscribes to Ably real-time notifications and renders toast banners. Cleanup logic is sound; uses a flag to suppress errors during teardown. Contains one console.error which is acceptable for error reporting.
src/components/modals/NotificationSettingsModal.tsx	Expanded with notification type toggles, 24-hour clock toggle, and early window input. Early window input fires a DB mutation on every keystroke (no debounce), and new JSX adds more hardcoded hex colors.
src/components/modals/SettingsModal.tsx	Adds PIN-gate flow before opening NotificationSettingsModal. Logic is straightforward; handlePinVerifiedForNotif is unnecessarily async.
src/db/models/settings.ts	Adds use24HourTime boolean field and changes default types to exclude streak_warning. Clean schema update.
src/db/models/notification.ts	Extracts regular notification types to constants.ts and adds streak_warning to the full NOTIFICATION_TYPES array. Re-exports are consistent.
src/app/api/v1/notifications/simulate/route.ts	New dev-only endpoint to simulate any notification type. Correctly gated to NODE_ENV=development and uses withAuth for authentication.
tsconfig.json	Changes jsx from "react-jsx" to "preserve". In a Next.js/SWC project this is the correct setting — SWC handles JSX transformation, so TypeScript should leave JSX intact.

Sequence Diagram

sequenceDiagram
    participant Cron as Cron Job
    participant NS as NotificationService
    participant DB as Database (DAO)
    participant Ably as Ably Realtime
    participant Hook as useNotifications (client)
    participant Toast as react-hot-toast

    Cron->>NS: checkAndSendNotifications()
    NS->>DB: getAllWithNotificationsEnabled()
    loop per user
        NS->>DB: getMedicationsByUserId()
        NS->>DB: getMedicationLogs() [cached per med]
        NS->>DB: getPetByUserId()
        NS->>NS: compute hasTakenMedToday, lastDose, shouldCheckStreak
        loop per medication × doseTime
            NS->>DB: NotificationDAO.exists(type)
            alt notification not yet sent
                NS->>DB: NotificationDAO.create(type)
                NS->>Ably: publishToUser(medication-notification)
                alt isLastDose && shouldCheckStreak
                    NS->>DB: NotificationDAO.exists(streak_warning)
                    alt streak warning not yet sent
                        NS->>DB: NotificationDAO.create(streak_warning)
                        NS->>Ably: publishToUser(streak_warning)
                    end
                end
            end
        end
    end

    Ably-->>Hook: message event (medication-notification)
    Hook->>Toast: toast.custom(NotificationBanner)
    Hook->>DB: NotificationHTTPClient.markDelivered()

Loading

Comments Outside Diff (1)

1. src/services/notification.ts, line 222-265 (link)

   Streak warning silently skipped when regular notification already exists

   The streak warning check is nested inside the alreadyExists guard. When alreadyExists is true, the loop hits continue on line 191, bypassing the streak warning entirely.

   In normal operation this is OK because each notification type fires in a 1-minute time window (e.g. diffMinutes >= 0 && diffMinutes < 1), and the cron only enters that window once. However, if:

   • The cron fires twice within the same minute (e.g. during retries or a restart), OR
   • The regular notification was created manually / by a different code path

   …then the streak warning for that dose will never be sent, even if shouldCheckStreak is true and no streak warning record exists yet. It may be worth extracting the streak warning check to run independently after the typesToCheck loop, guarded only by isLastDose and shouldCheckStreak, so it isn't silently skipped by a pre-existing regular notification.

_{Last reviewed commit: "Initial notification..."}