fix: show notification inhibitor in details#3064
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 29 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughAdds an “Inhibited By” lookup to ChangesInhibited By feature
Sequence Diagram(s)sequenceDiagram
participant NotificationDetails
participant ReactQuery
participant API
NotificationDetails->>NotificationDetails: check status === inhibited and parent_id
NotificationDetails->>ReactQuery: useQuery(parent_id)
ReactQuery->>API: getNotificationSendHistoryById(parent_id)
API-->>ReactQuery: inhibitor notification data
ReactQuery-->>NotificationDetails: data / isLoading
NotificationDetails->>NotificationDetails: render Inhibited By / Loading / Unknown
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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.
🧹 Nitpick comments (2)
src/components/Notifications/NotificationSendHistory/__tests__/NotificationDetails.unit.test.tsx (1)
43-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding tests for loading and error states.
The current tests cover the happy path (inhibited → fetch + render) and the negative case (non-inhibited → no fetch). Adding tests for the loading state (assert "Loading..." appears before resolution) and error state (mock rejection → "Unknown" or error text) would strengthen coverage for the conditional rendering branches in lines 196-218.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Notifications/NotificationSendHistory/__tests__/NotificationDetails.unit.test.tsx` around lines 43 - 69, Add unit tests in NotificationDetails.unit.test.tsx to cover the conditional branches in NotificationDetails for the async fetch flow: one test should assert the loading UI appears while mockGetNotificationSendHistoryById is pending, and another should mock a rejected resolution and verify the error/fallback display (such as "Unknown" or the expected error text). Use renderNotificationDetails and the existing mockGetNotificationSendHistoryById setup so the new tests exercise the same inhibited-history path without changing the current happy-path or non-inhibited assertions.src/components/Notifications/NotificationSendHistory/NotificationDetails.tsx (1)
196-218: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider distinguishing query error from "no data" in the Inhibited By field.
When the inhibitor query fails (e.g., 404 or network error),
inhibitorisundefinedandisLoadingInhibitorisfalse, so the UI shows "Unknown" — indistinguishable from a successful query that returned no data. Adding anisErrorcheck from the query would let you display a more accurate message like "Failed to load" for error states.♻️ Optional: surface error state
- const { data: inhibitor, isLoading: isLoadingInhibitor } = useQuery({ + const { data: inhibitor, isLoading: isLoadingInhibitor, isError: isInhibitorError } = useQuery({ queryKey: ["notification_send_history_inhibitor", notification.parent_id], enabled: shouldFetchInhibitor, queryFn: () => getNotificationSendHistoryById(notification.parent_id!) });) : isLoadingInhibitor ? ( "Loading..." + ) : isInhibitorError ? ( + "Failed to load" ) : ( "Unknown" )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Notifications/NotificationSendHistory/NotificationDetails.tsx` around lines 196 - 218, The Inhibited By display in NotificationDetails currently treats query failures the same as empty results by falling back to "Unknown" when inhibitor is undefined and isLoadingInhibitor is false. Update the inhibitor query handling in NotificationDetails to expose and check the query’s isError state alongside inhibitor and isLoadingInhibitor, and render a distinct error message such as "Failed to load" when the fetch fails while keeping "Unknown" only for successful no-data cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@src/components/Notifications/NotificationSendHistory/__tests__/NotificationDetails.unit.test.tsx`:
- Around line 43-69: Add unit tests in NotificationDetails.unit.test.tsx to
cover the conditional branches in NotificationDetails for the async fetch flow:
one test should assert the loading UI appears while
mockGetNotificationSendHistoryById is pending, and another should mock a
rejected resolution and verify the error/fallback display (such as "Unknown" or
the expected error text). Use renderNotificationDetails and the existing
mockGetNotificationSendHistoryById setup so the new tests exercise the same
inhibited-history path without changing the current happy-path or non-inhibited
assertions.
In
`@src/components/Notifications/NotificationSendHistory/NotificationDetails.tsx`:
- Around line 196-218: The Inhibited By display in NotificationDetails currently
treats query failures the same as empty results by falling back to "Unknown"
when inhibitor is undefined and isLoadingInhibitor is false. Update the
inhibitor query handling in NotificationDetails to expose and check the query’s
isError state alongside inhibitor and isLoadingInhibitor, and render a distinct
error message such as "Failed to load" when the fetch fails while keeping
"Unknown" only for successful no-data cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e6160312-24d8-4c4b-94bd-f206f3bbab20
📒 Files selected for processing (4)
src/components/Notifications/NotificationSendHistory/NotificationDetails.stories.tsxsrc/components/Notifications/NotificationSendHistory/NotificationDetails.tsxsrc/components/Notifications/NotificationSendHistory/__fixtures__/notificationDetails.tssrc/components/Notifications/NotificationSendHistory/__tests__/NotificationDetails.unit.test.tsx
7fce455 to
a30a5b1
Compare
df4e1c3 to
1e551eb
Compare
1e551eb to
936e7ed
Compare
936e7ed to
c6bc144
Compare
2 participants
Notification history can mark a send as inhibited when another related notification already fired.
The backend records that inhibitor as
parent_id, but the details panel only resolved silence metadata, so inhibited notifications did not show what caused the suppression.Resolve the parent send-history record for inhibited notifications and render it as
Inhibited Byso users can see the resource that triggered the inhibition.Summary by CodeRabbit
New Features
Bug Fixes
Tests