Conversation
Signed-off-by: prxt6529 <prxt@6529.io>
WalkthroughAdds a PriorityAlert notification end-to-end: OpenAPI enum, TypeScript type, new React component, integration into NotificationItem and priority ordering, plus unit tests covering rendering and navigation. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Application
participant Item as NotificationItem
participant Comp as NotificationPriorityAlert
participant Drop as Drop Component
Note right of Item `#E8F8F5`: New switch case for PriorityAlert
App->>Item: Pass notification (cause: PriorityAlert)
Item->>Item: switch(notification.cause)
Item->>Comp: render NotificationPriorityAlert(props)
alt related_drops present
Comp->>Comp: derive isDirectMessage, compute routes
Comp->>Drop: render Drop (full content, handlers)
Drop->>Comp: user action (reply / quote / content click)
Comp->>App: call onReply/onQuote/onDropContentClick
Comp->>Router: router.push(getWaveRoute(...)) (navigation)
else no related_drops
Comp->>Comp: render compact header (avatar, handle, timestamp)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
components/brain/notifications/priority-alert/NotificationPriorityAlert.tsx (1)
87-87: Consider using FontAwesome icon instead of emoji.The emoji 🚨 may render inconsistently across platforms and devices.
As per coding guidelines, use a FontAwesome icon for consistent rendering:
-<span className="tw-text-iron-400">sent a priority alert 🚨</span> +<span className="tw-text-iron-400"> + sent a priority alert <FontAwesomeIcon icon={faExclamationTriangle} /> +</span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
generated/models/ApiNotificationCause.tsis excluded by!**/generated/**
📒 Files selected for processing (5)
components/brain/notifications/NotificationItem.tsx(2 hunks)components/brain/notifications/index.tsx(3 hunks)components/brain/notifications/priority-alert/NotificationPriorityAlert.tsx(1 hunks)openapi.yaml(1 hunks)types/feed.types.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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
Files:
types/feed.types.tscomponents/brain/notifications/priority-alert/NotificationPriorityAlert.tsxcomponents/brain/notifications/NotificationItem.tsxcomponents/brain/notifications/index.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use FontAwesome for icons
Use TailwindCSS for styling
Files:
components/brain/notifications/priority-alert/NotificationPriorityAlert.tsxcomponents/brain/notifications/NotificationItem.tsxcomponents/brain/notifications/index.tsx
🧬 Code graph analysis (3)
types/feed.types.ts (2)
generated/models/ApiProfileMin.ts (1)
ApiProfileMin(16-133)generated/models/ApiDrop.ts (1)
ApiDrop(28-227)
components/brain/notifications/priority-alert/NotificationPriorityAlert.tsx (9)
types/feed.types.ts (1)
INotificationPriorityAlert(135-143)types/dropInteractionTypes.ts (1)
ActiveDropState(8-12)components/waves/drops/Drop.tsx (1)
DropInteractionParams(13-16)helpers/waves/drop.helpers.ts (1)
ExtendedDrop(16-20)hooks/useDeviceInfo.ts (1)
useDeviceInfo(23-93)helpers/navigation.helpers.ts (1)
getWaveRoute(21-69)generated/models/ApiDrop.ts (1)
ApiDrop(28-227)helpers/image.helpers.ts (1)
getScaledImageUri(17-45)helpers/Helpers.ts (1)
getTimeAgoShort(563-589)
components/brain/notifications/NotificationItem.tsx (1)
components/brain/notifications/priority-alert/NotificationPriorityAlert.tsx (1)
NotificationPriorityAlert(18-120)
⏰ 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 (4)
openapi.yaml (1)
5566-5566: LGTM! Clean enum addition.The new PRIORITY_ALERT value is properly formatted and extends the notification system consistently.
components/brain/notifications/index.tsx (1)
25-25: LGTM! Correct priority assignment.Priority value 8 properly places PriorityAlert as the highest priority notification, which aligns with its intended urgency.
types/feed.types.ts (1)
135-154: LGTM! Type definition follows existing patterns.The new INotificationPriorityAlert type is properly structured and consistently integrated into the TypedNotification union.
components/brain/notifications/NotificationItem.tsx (1)
89-98: LGTM! Clean integration.The PriorityAlert case is properly wired with consistent prop passing.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
components/brain/notifications/priority-alert/NotificationPriorityAlert.tsx (2)
73-75: Replaceas anycast with proper typing.The
as anycast bypasses TypeScript's type safety when accessing nested wave properties. The optional chaining on[0]is also redundant since line 34 already ensuresrelated_drops[0]exists.Consider defining a proper type for the wave object with the chat scope structure, or create a type guard to safely access
is_direct_message:-const baseWave = notification.related_drops[0]?.wave as any; -const isDirectMessage = - baseWave?.chat?.scope?.group?.is_direct_message ?? false; +const baseWave = notification.related_drops[0].wave; +const isDirectMessage = + (baseWave as any)?.chat?.scope?.group?.is_direct_message ?? false;Better yet, define a type for the extended wave structure and use a type guard:
interface WaveWithChat { id: string; chat?: { scope?: { group?: { is_direct_message: boolean; }; }; }; } const hasDirectMessageInfo = (wave: any): wave is WaveWithChat => { return wave?.chat?.scope?.group?.is_direct_message !== undefined; }; const baseWave = notification.related_drops[0].wave; const isDirectMessage = hasDirectMessageInfo(baseWave) ? baseWave.chat.scope.group.is_direct_message : false;
90-93: Replaceas anycast with proper typing.Similar to lines 73-75, this
as anycast bypasses type safety. Use the same type guard or typed interface approach as suggested for thebaseWaveto safely access theis_direct_messageproperty.
🧹 Nitpick comments (3)
components/brain/notifications/priority-alert/NotificationPriorityAlert.tsx (2)
34-71: Extract duplicated header rendering logic.The header rendering code (avatar, handle, message, and timestamp) at lines 36-69 is duplicated at lines 106-137. Extract this into a reusable function or component to eliminate the duplication.
Example refactor:
+const renderHeader = () => ( + <div className="tw-flex tw-gap-x-2 tw-items-center"> + <div className="tw-h-7 tw-w-7"> + {notification.related_identity.pfp ? ( + <img + src={getScaledImageUri( + notification.related_identity.pfp, + ImageScale.W_AUTO_H_50 + )} + alt="#" + className="tw-flex-shrink-0 tw-object-contain tw-h-full tw-w-full tw-rounded-md tw-bg-iron-800 tw-ring-1 tw-ring-iron-700" + /> + ) : ( + <div className="tw-flex-shrink-0 tw-object-contain tw-h-full tw-w-full tw-rounded-md tw-bg-iron-800 tw-ring-1 tw-ring-iron-700" /> + )} + </div> + <span className="tw-text-sm tw-font-normal tw-text-iron-50"> + <Link + href={`/${notification.related_identity.handle}`} + className="tw-no-underline tw-font-semibold"> + {notification.related_identity.handle} + </Link>{" "} + <span className="tw-text-iron-400">sent a priority alert 🚨</span>{" "} + <span className="tw-text-sm tw-text-iron-300 tw-font-normal tw-whitespace-nowrap"> + <span className="tw-font-bold tw-mr-1 tw-text-xs tw-text-iron-400"> + • + </span>{" "} + {getTimeAgoShort(notification.created_at)} + </span> + </span> + </div> +); + if (!notification.related_drops || notification.related_drops.length === 0) { return ( <div className="tw-w-full tw-flex tw-gap-x-3"> <div className="tw-w-full tw-flex tw-flex-col tw-space-y-2"> - <div className="tw-flex tw-gap-x-2 tw-items-center"> - ... (entire header block) - </div> + {renderHeader()} </div> </div> ); } ... return ( <div className="tw-w-full tw-flex tw-gap-x-3"> <div className="tw-w-full tw-flex tw-flex-col tw-space-y-2"> - <div className="tw-flex tw-gap-x-2 tw-items-center"> - ... (entire header block) - </div> + {renderHeader()} {notification.related_drops[0] && ( <Drop ... /> )} </div> </div> );
139-139: Remove redundant existence check.The check
notification.related_drops[0] &&at line 139 is redundant because line 34 already ensuresrelated_dropsexists and has at least one element. This code path is unreachable ifrelated_drops[0]doesn't exist.Apply this diff:
-{notification.related_drops[0] && ( +{( <Drop drop={{ type: DropSize.FULL, ...notification.related_drops[0], stableKey: "", stableHash: "", }}__tests__/components/brain/notifications/priority-alert/NotificationPriorityAlert.test.tsx (1)
82-97: Add safety check for mock calls access.Line 91 accesses
DropMock.mock.calls[0][0]without verifying that the Drop component was called. If the component fails to render the Drop for any reason, this test will throw an error instead of failing with a clear assertion.Apply this diff:
it("uses router in reply and quote handlers", () => { render( <NotificationPriorityAlert notification={baseNotification} activeDrop={null} onReply={jest.fn()} onQuote={jest.fn()} /> ); + expect(DropMock).toHaveBeenCalled(); const props = DropMock.mock.calls[0][0]; props.onReplyClick(5); props.onQuoteClick({ wave: { id: "w" }, serial_no: 6 } as any); const router = (useRouter as jest.Mock).mock.results[0].value; expect(router.push).toHaveBeenCalledWith("/waves?wave=w&serialNo=5/"); expect(router.push).toHaveBeenCalledWith("/waves?wave=w&serialNo=6/"); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
__tests__/components/brain/notifications/item/NotificationItem.test.tsx(2 hunks)__tests__/components/brain/notifications/priority-alert/NotificationPriorityAlert.test.tsx(1 hunks)components/brain/notifications/priority-alert/NotificationPriorityAlert.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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
Files:
__tests__/components/brain/notifications/priority-alert/NotificationPriorityAlert.test.tsx__tests__/components/brain/notifications/item/NotificationItem.test.tsxcomponents/brain/notifications/priority-alert/NotificationPriorityAlert.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use FontAwesome for icons
Use TailwindCSS for styling
Files:
__tests__/components/brain/notifications/priority-alert/NotificationPriorityAlert.test.tsx__tests__/components/brain/notifications/item/NotificationItem.test.tsxcomponents/brain/notifications/priority-alert/NotificationPriorityAlert.tsx
__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/priority-alert/NotificationPriorityAlert.test.tsx__tests__/components/brain/notifications/item/NotificationItem.test.tsx
__tests__/components/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (tests/AGENTS.md)
Use
@testing-library/reactand@testing-library/user-eventfor React component tests
Files:
__tests__/components/brain/notifications/priority-alert/NotificationPriorityAlert.test.tsx__tests__/components/brain/notifications/item/NotificationItem.test.tsx
🧠 Learnings (7)
📚 Learning: 2025-09-28T12:33:30.950Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: __tests__/AGENTS.md:0-0
Timestamp: 2025-09-28T12:33:30.950Z
Learning: Applies to __tests__/components/**/*.{ts,tsx,js,jsx} : Use `testing-library/react` and `testing-library/user-event` for React component tests
Applied to files:
__tests__/components/brain/notifications/priority-alert/NotificationPriorityAlert.test.tsx
📚 Learning: 2025-09-28T12:33:07.561Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: __mocks__/AGENTS.md:0-0
Timestamp: 2025-09-28T12:33:07.561Z
Learning: Applies to __mocks__/**/__mocks__/**/*.{js,jsx,ts,tsx} : Document non-obvious expected behaviour directly in the mock file
Applied to files:
__tests__/components/brain/notifications/priority-alert/NotificationPriorityAlert.test.tsx__tests__/components/brain/notifications/item/NotificationItem.test.tsx
📚 Learning: 2025-09-28T12:33:07.561Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: __mocks__/AGENTS.md:0-0
Timestamp: 2025-09-28T12:33:07.561Z
Learning: Applies to __mocks__/**/__mocks__/**/*.{js,jsx,ts,tsx} : Keep mocks up to date with the real implementations they represent
Applied to files:
__tests__/components/brain/notifications/item/NotificationItem.test.tsx
📚 Learning: 2025-09-28T12:33:07.561Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: __mocks__/AGENTS.md:0-0
Timestamp: 2025-09-28T12:33:07.561Z
Learning: Applies to __mocks__/**/__mocks__/**/*.{js,jsx,ts,tsx} : Keep mock implementations minimal—only what’s necessary for the test scenarios
Applied to files:
__tests__/components/brain/notifications/item/NotificationItem.test.tsx
📚 Learning: 2025-09-28T12:33:07.561Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: __mocks__/AGENTS.md:0-0
Timestamp: 2025-09-28T12:33:07.561Z
Learning: Applies to __mocks__/**/__mocks__/**/*.{js,jsx,ts,tsx} : Mock only external dependencies or heavy functionality; avoid over-mocking internal logic
Applied to files:
__tests__/components/brain/notifications/item/NotificationItem.test.tsx
📚 Learning: 2025-09-28T12:33:07.561Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: __mocks__/AGENTS.md:0-0
Timestamp: 2025-09-28T12:33:07.561Z
Learning: Applies to __mocks__/**/*.{test,spec}.{js,jsx,ts,tsx} : In tests, use jest.mock('module') with a bare module specifier to load the corresponding manual mock
Applied to files:
__tests__/components/brain/notifications/item/NotificationItem.test.tsx
📚 Learning: 2025-09-28T12:33:07.561Z
Learnt from: CR
Repo: 6529-Collections/6529seize-frontend PR: 0
File: __mocks__/AGENTS.md:0-0
Timestamp: 2025-09-28T12:33:07.561Z
Learning: Use Jest’s built-in mocking for module replacement; keep manual mocks simple and lightweight
Applied to files:
__tests__/components/brain/notifications/item/NotificationItem.test.tsx
🧬 Code graph analysis (2)
__tests__/components/brain/notifications/priority-alert/NotificationPriorityAlert.test.tsx (1)
components/brain/notifications/priority-alert/NotificationPriorityAlert.tsx (1)
NotificationPriorityAlert(18-164)
components/brain/notifications/priority-alert/NotificationPriorityAlert.tsx (9)
types/feed.types.ts (1)
INotificationPriorityAlert(135-143)types/dropInteractionTypes.ts (1)
ActiveDropState(8-12)components/waves/drops/Drop.tsx (1)
DropInteractionParams(13-16)helpers/waves/drop.helpers.ts (1)
ExtendedDrop(16-20)hooks/useDeviceInfo.ts (1)
useDeviceInfo(23-93)helpers/image.helpers.ts (1)
getScaledImageUri(17-45)helpers/Helpers.ts (1)
getTimeAgoShort(563-589)helpers/navigation.helpers.ts (1)
getWaveRoute(21-69)generated/models/ApiDrop.ts (1)
ApiDrop(28-227)
⏰ 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 (6)
__tests__/components/brain/notifications/item/NotificationItem.test.tsx (2)
8-8: LGTM!The mock for
NotificationPriorityAlertfollows the same minimal pattern as the existing mocks for quoted and replied components, keeping it simple and focused on the test scenario.
22-25: LGTM!The test case for priority alert notifications is consistent with the existing test patterns and properly verifies that the priority alert component is rendered when the notification cause is
PriorityAlert.__tests__/components/brain/notifications/priority-alert/NotificationPriorityAlert.test.tsx (4)
6-28: LGTM!The mocks are minimal and focused on what's necessary for testing, following the learnings guidance. The Drop mock properly captures props for later verification, and the device/navigation mocks provide appropriate defaults.
Based on learnings
30-47: LGTM!The base notification fixture provides realistic test data with all required fields, including the priority alert cause, related identity, and related drops.
50-62: LGTM!The test properly verifies the component renders with a drop, checking for the sender handle, priority alert message, and drop presence. Uses
@testing-library/reactas per coding guidelines.As per coding guidelines
64-80: LGTM!The test correctly verifies the fallback rendering when
related_dropsis empty, ensuring the header renders without the Drop component.
|
2 participants
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Tests
API