Skip to content

Notifications component cleanup#1545

Merged
simo6529 merged 5 commits intomainfrom
notifications-component-cleanup
Oct 15, 2025
Merged

Notifications component cleanup#1545
simo6529 merged 5 commits intomainfrom
notifications-component-cleanup

Conversation

@simo6529
Copy link
Copy Markdown
Collaborator

@simo6529 simo6529 commented Oct 15, 2025

Summary by CodeRabbit

  • New Features

    • Notifications: clearer error messages, loading-timeout notice, sticky-to-bottom behavior, smoother infinite scroll, and actionable prompts (retry, re-auth, switch from proxy).
  • Refactor

    • Notifications rebuilt into modular hooks and subcomponents for clearer structure and maintainability.
  • Tests

    • Updated test imports/mocks to align with the new notifications module layout.
  • Documentation

    • Added ticket and state board entry documenting the notifications refactor.
  • Chores

    • .gitignore updated to include git hooks directory.

Signed-off-by: Simo <simo@6529.io>
Signed-off-by: Simo <simo@6529.io>
Signed-off-by: Simo <simo@6529.io>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Oct 15, 2025

Walkthrough

Refactors the notifications feature by removing the monolithic Notifications.tsx and replacing it with a modular implementation: index.tsx component, controller and scroll hooks, subcomponents, and utilities. Tests and imports updated to use the new barrel; .gitignore and codex docs/ticket entries added.

Changes

Cohort / File(s) Summary
Git config
/.gitignore
Adds .githooks/ to ignores; toggles /config/env.schema.runtime.cjs entry (no net effect).
Tests
/__tests__/components/brain/BrainMobile.test.tsx, /__tests__/components/brain/notifications/Notifications.test.tsx
Update mocks/import paths to use @/components/brain/notifications barrel instead of nested component paths.
Docs / Codex
/codex/STATE.md, /codex/tickets/TKT-0009.md
Adds TKT-0009 ticket file and updates STATE.md with the new ticket entry documenting the notifications refactor.
Notifications — main components
Deleted:
/components/brain/notifications/Notifications.tsx
Added/Updated:
/components/brain/notifications/index.tsx, /components/brain/notifications/NotificationsContainer.tsx
Removes legacy monolithic component; adds new index.tsx as the exported Notifications component and updates container to import from the new barrel.
Notifications — hooks
/components/brain/notifications/hooks/useNotificationsController.ts, /components/brain/notifications/hooks/useNotificationsScroll.ts
Adds useNotificationsController (auth, queries, error/timeouts, handlers, content state) and useNotificationsScroll (infinite/prepend, bottom-stick, observers, scroll handlers).
Notifications — subcomponents
/components/brain/notifications/subcomponents/NotificationsContent.tsx, /components/brain/notifications/subcomponents/NotificationsStateMessage.tsx
Adds NotificationsContent (renders loader, states, wrapper, empty view) and NotificationsStateMessage (centered message + optional action button).
Notifications — utils
/components/brain/notifications/utils/constants.ts, /components/brain/notifications/utils/getNotificationErrorDetails.ts
Adds shared constants (scroll threshold, timeouts, messages) and getNotificationErrorDetails utility to normalize error messages and unauthorized detection.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Page as Page/Container
  participant Comp as Notifications (index)
  participant Ctrl as useNotificationsController
  participant Scroll as useNotificationsScroll
  participant API as Notifications Query API
  participant Auth as Auth Context
  participant Toast as Toast/UI

  User->>Page: Open Notifications view
  Page->>Comp: Render
  Comp->>Ctrl: Initialize controller
  Ctrl->>Auth: Read auth/profile/proxy state
  Ctrl->>API: useNotificationsQuery (fetch)
  alt Unauthorized
    Ctrl->>Comp: contentState { showAuthRetry }
    Ctrl->>Toast: show unauthorized
  else Loading
    Ctrl->>Comp: contentState { showLoader }
  else Error
    Ctrl->>Comp: contentState { showError, message }
  else Success
    Ctrl->>Comp: items, pagination, handlers
  end
  Comp->>Scroll: init with items/pagination/filter
  User-->>Comp: Scroll event
  Comp->>Scroll: handleScroll
  alt Near top & hasNextPage
    Scroll->>API: fetchNextPage
    API-->>Ctrl: more items
    Ctrl-->>Comp: updated items
  else Near bottom
    Scroll-->>Comp: maintain bottom-stick
  end
Loading
sequenceDiagram
  autonumber
  participant Router as URL/Router
  participant Ctrl as useNotificationsController
  participant API as Notifications Query API
  participant Toast as Toast/UX

  Router-->>Ctrl: reload param present
  Ctrl->>API: refetch notifications
  Ctrl->>API: markAllAsRead (on first auth)
  API-->>Ctrl: Result/Error
  alt 401 Unauthorized
    Ctrl->>Toast: show unauthorized message
    Ctrl-->>Router: clear reload param
  else Other error
    Ctrl->>Toast: show error message
    Ctrl-->>Router: clear reload param
  else Success
    Ctrl-->>Router: clear reload param
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • ragnep
  • GelatoGenesis

Poem

"I hopped through code with nimble paws,
Split one file into handy claws.
Hooks now dance and components play,
Errors hush and scrolls obey.
A rabbit cheers—modular hooray! 🐇"

Pre-merge checks and finishing touches

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The title “Notifications component cleanup” mentions the affected component but is overly broad and does not reflect the extensive refactor, including removal of the legacy Notifications.tsx and introduction of new hooks, utilities, and subcomponents. It fails to convey the core architectural change that constitutes the bulk of this pull request. A vague title may obscure the significant updates from reviewers and future maintainers. Therefore, it is unclear whether the current title adequately summarizes the main changes. Please revise the title to clearly reflect the primary scope of the changes by specifying the refactor rather than a generic cleanup. For example, you might use “Refactor Notifications component into modular subcomponents and hooks for improved clarity.” A more descriptive title will help reviewers and future maintainers quickly understand the core purpose of this pull request.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch notifications-component-cleanup

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a2189d2 and 699e171.

📒 Files selected for processing (1)
  • components/brain/notifications/index.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • components/brain/notifications/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Analyze (javascript-typescript)

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.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (5)
codex/tickets/TKT-0009.md (1)

31-31: Link the current PR in the ticket.

The coding guidelines specify that pull requests should be linked back to their tickets. Since PR #1545 is now open, please update the placeholder to reference it.

Apply this change:

-- Primary PR: _(add when available)_
+- Primary PR: [#1545](https://github.com/6529-Collections/6529seize-frontend/pull/1545)
codex/STATE.md (1)

15-15: Link PR #1545 in the PRs column.

The coding guidelines require pull requests to be linked back to their tickets and mirrored in STATE.md. Since PR #1545 is associated with TKT-0009, please update the PRs column.

Based on learnings

Apply this change:

-| TKT-0009 | Refactor Brain notifications shell for modular clarity | In-Progress | P1 | simo6529 | — | 2025-10-15 |
+| TKT-0009 | Refactor Brain notifications shell for modular clarity | In-Progress | P1 | simo6529 | [#1545](https://github.com/6529-Collections/6529seize-frontend/pull/1545) | 2025-10-15 |
components/brain/notifications/index.tsx (1)

30-33: Stabilize activeFilter key to avoid unnecessary scroll resets

Sort before join (and memoize) so key doesn’t change due to array order.

Apply this diff:

+import { useMemo } from "react";
@@
-  const activeFilterKey =
-    activeFilter?.cause?.join("|") ?? "notifications-filter-all";
+  const activeFilterKey = useMemo(
+    () =>
+      activeFilter?.cause
+        ? [...activeFilter.cause].sort().join("|")
+        : "notifications-filter-all",
+    [activeFilter]
+  );
components/brain/notifications/hooks/useNotificationsScroll.ts (1)

98-101: Reset prepending state on filter change

If a prepend fetch fails, the flag can linger. Reset it when filters change.

Apply this diff:

 useEffect(() => {
   hasInitializedScrollRef.current = false;
   isPinnedToBottomRef.current = true;
+  isPrependingRef.current = false;
 }, [activeFilterKey]);
components/brain/notifications/hooks/useNotificationsController.ts (1)

100-116: Harden onSuccess to avoid unhandled rejections

If removeAllDeliveredNotifications throws, the promise from onSuccess rejects. Wrap in try/catch.

Apply this diff:

   const { mutateAsync: markAllAsRead } = useMutation({
     mutationFn: async () =>
       await commonApiPostWithoutBodyAndResponse({
         endpoint: `notifications/read`,
       }),
     onSuccess: async () => {
-      invalidateNotifications();
-      await removeAllDeliveredNotifications();
+      try {
+        invalidateNotifications();
+        await removeAllDeliveredNotifications();
+      } catch (e) {
+        console.error("Failed to clear delivered notifications:", e);
+      }
     },
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between adbd8e3 and f122e30.

📒 Files selected for processing (14)
  • .gitignore (1 hunks)
  • __tests__/components/brain/BrainMobile.test.tsx (1 hunks)
  • __tests__/components/brain/notifications/Notifications.test.tsx (1 hunks)
  • codex/STATE.md (1 hunks)
  • codex/tickets/TKT-0009.md (1 hunks)
  • components/brain/notifications/Notifications.tsx (0 hunks)
  • components/brain/notifications/NotificationsContainer.tsx (2 hunks)
  • components/brain/notifications/hooks/useNotificationsController.ts (1 hunks)
  • components/brain/notifications/hooks/useNotificationsScroll.ts (1 hunks)
  • components/brain/notifications/index.tsx (1 hunks)
  • components/brain/notifications/subcomponents/NotificationsContent.tsx (1 hunks)
  • components/brain/notifications/subcomponents/NotificationsStateMessage.tsx (1 hunks)
  • components/brain/notifications/utils/constants.ts (1 hunks)
  • components/brain/notifications/utils/getNotificationErrorDetails.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • components/brain/notifications/Notifications.tsx
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{ts,tsx}: Do not include any comments in the code
Use react-query for data fetching
Always add readonly before props

**/*.{ts,tsx}: Use TypeScript for source code and follow existing code style and naming conventions
Adhere to clean code standards as measured by SonarQube

Files:

  • components/brain/notifications/NotificationsContainer.tsx
  • components/brain/notifications/index.tsx
  • __tests__/components/brain/notifications/Notifications.test.tsx
  • components/brain/notifications/hooks/useNotificationsController.ts
  • components/brain/notifications/utils/getNotificationErrorDetails.ts
  • components/brain/notifications/subcomponents/NotificationsContent.tsx
  • components/brain/notifications/subcomponents/NotificationsStateMessage.tsx
  • components/brain/notifications/hooks/useNotificationsScroll.ts
  • components/brain/notifications/utils/constants.ts
  • __tests__/components/brain/BrainMobile.test.tsx
**/*.tsx

📄 CodeRabbit inference engine (.cursorrules)

**/*.tsx: Use FontAwesome for icons
Use TailwindCSS for styling

Implement React components as functional components using hooks (no class components)

Files:

  • components/brain/notifications/NotificationsContainer.tsx
  • components/brain/notifications/index.tsx
  • __tests__/components/brain/notifications/Notifications.test.tsx
  • components/brain/notifications/subcomponents/NotificationsContent.tsx
  • components/brain/notifications/subcomponents/NotificationsStateMessage.tsx
  • __tests__/components/brain/BrainMobile.test.tsx
codex/tickets/**/*.md

📄 CodeRabbit inference engine (AGENTS.md)

Author new tickets with the provided template, keep YAML front matter alphabetical, and log timestamped updates as work progresses

Files:

  • codex/tickets/TKT-0009.md
codex/tickets/**

📄 CodeRabbit inference engine (AGENTS.md)

codex/tickets/**: Never edit tickets marked Done; open a new ticket if new scope emerges
Link pull requests back to their tickets and mirror merged PR references in both the ticket log and STATE.md

Files:

  • codex/tickets/TKT-0009.md
__tests__/**

📄 CodeRabbit inference engine (tests/AGENTS.md)

Place Jest test suites under the __tests__ directory mirroring source folders (e.g., components, contexts, hooks, utils)

Files:

  • __tests__/components/brain/notifications/Notifications.test.tsx
  • __tests__/components/brain/BrainMobile.test.tsx
__tests__/components/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (tests/AGENTS.md)

Use @testing-library/react and @testing-library/user-event for React component tests

Files:

  • __tests__/components/brain/notifications/Notifications.test.tsx
  • __tests__/components/brain/BrainMobile.test.tsx
{**/__tests__/**,**/*.test.tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Place tests in tests directories or alongside components as ComponentName.test.tsx

Files:

  • __tests__/components/brain/notifications/Notifications.test.tsx
  • __tests__/components/brain/BrainMobile.test.tsx
{**/__tests__/**,**/*.test.{ts,tsx}}

📄 CodeRabbit inference engine (AGENTS.md)

Mock external dependencies and APIs in tests

Files:

  • __tests__/components/brain/notifications/Notifications.test.tsx
  • __tests__/components/brain/BrainMobile.test.tsx
codex/STATE.md

📄 CodeRabbit inference engine (AGENTS.md)

Keep codex/STATE.md in sync with tickets under codex/tickets/ so the board remains auditable

Files:

  • codex/STATE.md
🧠 Learnings (1)
📚 Learning: 2025-10-14T05:39:48.871Z
Learnt from: CR
PR: 6529-Collections/6529seize-frontend#0
File: AGENTS.md:0-0
Timestamp: 2025-10-14T05:39:48.871Z
Learning: Applies to codex/STATE.md : Keep codex/STATE.md in sync with tickets under codex/tickets/ so the board remains auditable

Applied to files:

  • codex/STATE.md
🧬 Code graph analysis (5)
components/brain/notifications/index.tsx (5)
types/dropInteractionTypes.ts (1)
  • ActiveDropState (8-12)
components/brain/notifications/hooks/useNotificationsController.ts (1)
  • useNotificationsController (64-384)
components/brain/notifications/hooks/useNotificationsScroll.ts (1)
  • useNotificationsScroll (34-233)
components/brain/notifications/NotificationsCauseFilter.tsx (1)
  • NotificationsCauseFilter (31-137)
components/brain/notifications/subcomponents/NotificationsContent.tsx (1)
  • NotificationsContent (29-114)
components/brain/notifications/hooks/useNotificationsController.ts (11)
components/brain/notifications/NotificationsCauseFilter.tsx (1)
  • NotificationFilter (8-11)
types/feed.types.ts (1)
  • TypedNotification (135-143)
components/auth/Auth.tsx (1)
  • AuthContext (83-93)
components/brain/my-stream/layout/LayoutContext.tsx (1)
  • useLayout (455-455)
components/notifications/NotificationsContext.tsx (1)
  • useNotificationsContext (235-243)
components/react-query-wrapper/ReactQueryWrapper.tsx (1)
  • ReactQueryWrapperContext (184-212)
contexts/TitleContext.tsx (1)
  • useSetTitle (197-205)
services/api/common-api.ts (1)
  • commonApiPostWithoutBodyAndResponse (193-209)
hooks/useNotificationsQuery.tsx (1)
  • useNotificationsQuery (77-148)
components/brain/notifications/utils/getNotificationErrorDetails.ts (1)
  • getNotificationErrorDetails (8-36)
components/brain/notifications/utils/constants.ts (3)
  • LOAD_TIMEOUT_MS (2-2)
  • LOAD_TIMEOUT_MESSAGE (5-6)
  • DEFAULT_ERROR_MESSAGE (3-4)
components/brain/notifications/utils/getNotificationErrorDetails.ts (1)
components/brain/notifications/utils/constants.ts (1)
  • DEFAULT_ERROR_MESSAGE (3-4)
components/brain/notifications/subcomponents/NotificationsContent.tsx (6)
types/feed.types.ts (1)
  • TypedNotification (135-143)
types/dropInteractionTypes.ts (1)
  • ActiveDropState (8-12)
components/common/SpinnerLoader.tsx (1)
  • SpinnerLoader (8-57)
components/brain/notifications/subcomponents/NotificationsStateMessage.tsx (1)
  • NotificationsStateMessage (15-32)
components/brain/my-stream/layout/MyStreamNoItems.tsx (1)
  • MyStreamNoItems (4-126)
components/brain/notifications/NotificationsWrapper.tsx (1)
  • NotificationsWrapper (21-79)
components/brain/notifications/hooks/useNotificationsScroll.ts (3)
types/feed.types.ts (1)
  • TypedNotification (135-143)
components/brain/notifications/utils/constants.ts (1)
  • STICK_TO_BOTTOM_SCROLL_THRESHOLD_PX (1-1)
components/brain/constants.ts (1)
  • NEAR_TOP_SCROLL_THRESHOLD_PX (1-1)
🪛 LanguageTool
codex/tickets/TKT-0009.md

[grammar] ~17-~17: There might be a mistake here.
Context: ...to a dedicated Notifications hook layer. - [x] Extract presentational state renderi...

(QB_NEW_EN)


[grammar] ~25-~25: There might be a mistake here.
Context: ...}and keep runtime behaviour identical. - [ ] Existing imports ofNotifications` ...

(QB_NEW_EN)


[grammar] ~31-~31: There might be a mistake here.
Context: ...nks - Primary PR: (add when available) - Follow-ups: _(reference additional ticke...

(QB_NEW_EN)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (15)
components/brain/notifications/utils/getNotificationErrorDetails.ts (3)

1-6: LGTM!

The import and interface definition follow the coding guidelines correctly, with proper use of readonly modifiers.


11-14: Status extraction handles multiple error shapes effectively.

The multi-level optional chaining correctly handles various error object structures (direct status, response.status, cause.status), providing robust fallback logic.


16-36: Error handling branches are comprehensive and defensive.

The function correctly handles Error instances, strings, and fallback cases, with appropriate message sanitization and authorization detection via both status code (401) and message content.

codex/tickets/TKT-0009.md (1)

1-8: LGTM!

The YAML front matter is correctly alphabetized and contains all necessary metadata fields.

components/brain/notifications/NotificationsContainer.tsx (1)

8-8: LGTM!

The import path correctly references the new barrel export, maintaining compatibility while improving module structure.

__tests__/components/brain/notifications/Notifications.test.tsx (1)

97-97: LGTM!

The import path correctly uses the barrel export, maintaining test compatibility with the refactored module structure.

__tests__/components/brain/BrainMobile.test.tsx (1)

49-49: LGTM!

The mock path correctly references the barrel export, maintaining test isolation while aligning with the refactored module structure.

components/brain/notifications/utils/constants.ts (1)

1-6: LGTM!

The constants provide sensible default values for scroll behavior (32px threshold), loading timeout (15 seconds), and clear user-facing error messages.

components/brain/notifications/subcomponents/NotificationsStateMessage.tsx (3)

1-4: LGTM!

The client directive and minimal imports are correctly structured for a Next.js client component.


5-13: LGTM!

The type definitions correctly use readonly modifiers per the coding guidelines, with appropriate optional action prop.


15-32: LGTM!

The component correctly implements a functional React component with proper TypeScript types, TailwindCSS styling, conditional rendering, and accessibility considerations (button type and focus states).

components/brain/notifications/index.tsx (1)

45-81: Wiring and composition look solid

Clean split between controller, scroll hook, and content. Props and flags are correctly threaded.

components/brain/notifications/hooks/useNotificationsScroll.ts (1)

195-227: Scroll logic and guards look good

Near-top trigger, bottom pin tracking, and gating conditions are correct.

components/brain/notifications/subcomponents/NotificationsContent.tsx (1)

29-114: State rendering is clear and correct

Good separation of loading/error/empty/content states; props and handlers are consistent.

components/brain/notifications/hooks/useNotificationsController.ts (1)

333-383: Controller outputs and UI flags look consistent

Memoized contentState/handlers/pagination and query wiring are coherent.

Signed-off-by: Simo <simo@6529.io>
Signed-off-by: Simo <simo@6529.io>
@sonarqubecloud
Copy link
Copy Markdown

@simo6529 simo6529 merged commit b68d351 into main Oct 15, 2025
8 checks passed
@simo6529 simo6529 deleted the notifications-component-cleanup branch October 15, 2025 11:34
@coderabbitai coderabbitai Bot mentioned this pull request Oct 17, 2025
@coderabbitai coderabbitai Bot mentioned this pull request Nov 12, 2025
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.

2 participants