Conversation
Contributor
📝 WalkthroughWalkthroughStandardizes pending-state handling for subscription and entry-delete UI buttons, adds a guard to skip author-link enhancement inside archived tweets, and rewrites the notifications "mark as read" mutation to support infinite-query shapes, unified optimistic updates, and explicit unread-count cache management. Also bumps SDK and wallets package versions and changelogs. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client Component
participant Mutation as MarkNotificationsRead Mutation
participant Cache as Query Cache
participant Server as API Server
Client->>Mutation: trigger mark-as-read (id?|all)
Mutation->>Cache: getQueriesData(QueryKeys.notifications._prefix)
Mutation->>Mutation: detect isInfiniteData or finite
Mutation->>Cache: store previousData (for rollback)
alt Infinite data
Mutation->>Cache: map pages/items -> markNotificationRead
else Finite data
Mutation->>Cache: update notification list
end
Mutation->>Cache: adjust unreadCount cache (decrement)
Mutation->>Server: send mutation request
alt Success
Server-->>Mutation: server unread count
Mutation->>Cache: align unreadCount & data with server
else Error
Mutation->>Cache: restore previousData and unreadCount
end
Cache-->>Client: UI reflects updated cache
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/sdk/src/modules/notifications/mutations/use-mark-notifications-read.ts (1)
109-150: Redundant invalidation:onSuccess+onSettledboth invalidate the same queries when marking all.
onSettled(line 146) always invalidatesQueryKeys.notifications._prefix, which already covers the!variables.idinvalidation inonSuccess(line 127). TheonSuccessinvalidation is redundant sinceonSettledfires immediately after.If the intent is to always refetch after settle, the
onSuccessblock at lines 126-130 can be removed. Alternatively, if you want to avoid refetching on single-notification success (since the optimistic update already handled it), makeonSettledconditional instead:Option A: Remove redundant onSuccess invalidation (simpler)
onSuccess?.(unreadCount); - - // If marking all notifications, invalidate to ensure fresh data - if (!variables.id) { - queryClient.invalidateQueries({ - queryKey: QueryKeys.notifications._prefix, - }); - } },Option B: Only invalidate onSettled when marking all (preserves optimistic-only for single)
onSettled: () => { - queryClient.invalidateQueries({ - queryKey: QueryKeys.notifications._prefix, - }); + // Only invalidate for "mark all" to ensure fresh data; + // single-notification marking relies on the optimistic update },Remove the
onSuccessinvalidation and move it here:+ onSettled: (_data, _error, variables) => { + if (!variables.id) { + queryClient.invalidateQueries({ + queryKey: QueryKeys.notifications._prefix, + }); + } + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/src/modules/notifications/mutations/use-mark-notifications-read.ts` around lines 109 - 150, The onSuccess handler duplicates invalidation already done in onSettled: remove the conditional invalidation in onSuccess (the block checking if (!variables.id) and calling queryClient.invalidateQueries with QueryKeys.notifications._prefix) so only onSettled performs the global refetch, or alternatively change onSettled to only invalidate when variables.id is falsy (check variables.id) and keep the current onSuccess behavior; update the handlers around the mutation (onSuccess, onSettled) accordingly and keep use of QueryKeys.notifications._prefix and variables.id as the determining symbols.apps/web/src/app/communities/_components/subscription-btn/index.tsx (1)
54-65: Good UX improvement—subscribe button stays visible during pending.One minor note:
mutateAsync(line 59) returns a promise, and if it rejects, this becomes an unhandled rejection since there's no.catch(). The same pattern exists on the unsubscribe button (line 46), so this is pre-existing. Consider usingmutateinstead ofmutateAsyncif you don't need the returned promise—mutateswallows rejections and routes them through the mutation'sonErrorcallback.♻️ Suggested: use `mutate` instead of `mutateAsync` to avoid unhandled rejections
- onClick={() => subscribeMutation.mutateAsync({ community: community.name })} + onClick={() => subscribeMutation.mutate({ community: community.name })}Same applies to the unsubscribe button at line 46:
- onClick={() => unsubscribeMutation.mutateAsync({ community: community.name })} + onClick={() => unsubscribeMutation.mutate({ community: community.name })}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/communities/_components/subscription-btn/index.tsx` around lines 54 - 65, The subscribe/unsubscribe handlers currently call subscribeMutation.mutateAsync and unsubscribeMutation.mutateAsync which can create unhandled promise rejections; replace those calls with subscribeMutation.mutate(...) and unsubscribeMutation.mutate(...) (passing the same payload { community: community.name }) so rejections are handled by the mutation's onError callback, or alternatively wrap the mutateAsync calls in try/catch if you intentionally need the returned promise; update the Button onClick handlers where subscribeMutation.mutateAsync and unsubscribeMutation.mutateAsync are used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/sdk/src/modules/notifications/mutations/use-mark-notifications-read.ts`:
- Around line 97-103: When optimistically decrementing unread count using
QueryKeys.notifications.unreadCount, first verify the target notification's
cached read status instead of unconditionally subtracting 1: use
queryClient.getQueryData to fetch the cached notifications list (the same cache
key your list uses), find the notification by id, and only push/set the
unreadKey to currentUnread - 1 if that notification exists and its read field
!== 1; otherwise leave the unread count unchanged (still record previousData and
restore on error as before). Ensure you still handle the id falsy branch
(mark-all) by setting to 0 as currently done.
---
Nitpick comments:
In `@apps/web/src/app/communities/_components/subscription-btn/index.tsx`:
- Around line 54-65: The subscribe/unsubscribe handlers currently call
subscribeMutation.mutateAsync and unsubscribeMutation.mutateAsync which can
create unhandled promise rejections; replace those calls with
subscribeMutation.mutate(...) and unsubscribeMutation.mutate(...) (passing the
same payload { community: community.name }) so rejections are handled by the
mutation's onError callback, or alternatively wrap the mutateAsync calls in
try/catch if you intentionally need the returned promise; update the Button
onClick handlers where subscribeMutation.mutateAsync and
unsubscribeMutation.mutateAsync are used.
In
`@packages/sdk/src/modules/notifications/mutations/use-mark-notifications-read.ts`:
- Around line 109-150: The onSuccess handler duplicates invalidation already
done in onSettled: remove the conditional invalidation in onSuccess (the block
checking if (!variables.id) and calling queryClient.invalidateQueries with
QueryKeys.notifications._prefix) so only onSettled performs the global refetch,
or alternatively change onSettled to only invalidate when variables.id is falsy
(check variables.id) and keep the current onSuccess behavior; update the
handlers around the mutation (onSuccess, onSettled) accordingly and keep use of
QueryKeys.notifications._prefix and variables.id as the determining symbols.
feruzm
added
the
patch
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/web/src/features/post-renderer/components/utils/hiveAuthorLinkEnhancer.tsx (1)
21-23: Set the processed marker before the archived-tweet early return to avoid repeated traversal.The
el.dataset.enhanced = "true"assignment at line 24 is never reached for skipped elements. On every subsequentapplyAuthorLinkscall these elements pass the line-13 guard, go through the DOM-connection check, and then re-executeel.closest(".markdown-view")?.textContent?.includes(…)— which reads the entire container's text content — for nothing.♻️ Proposed fix
+ el.dataset.enhanced = "true"; + // Skip mentions inside archived tweet blocks if (el.closest(".markdown-view")?.textContent?.includes("Archived Tweet from")) return; - - el.dataset.enhanced = "true";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/post-renderer/components/utils/hiveAuthorLinkEnhancer.tsx` around lines 21 - 23, In applyAuthorLinks, mark elements as processed before any early returns for archived tweets so skipped nodes don't get re-evaluated on subsequent runs: set el.dataset.enhanced = "true" immediately after passing the initial guard (the same place where el.dataset.enhanced is currently assigned) or move that assignment to just before the archived-tweet check (the if that calls el.closest(".markdown-view")?.textContent?.includes("Archived Tweet from")), ensuring the function still returns early for archived tweets but that those elements are flagged to avoid repeated DOM traversal.packages/sdk/src/modules/notifications/mutations/use-mark-notifications-read.ts (1)
74-95: InnerisInfiniteDatacheck is redundant — thepredicatealready guarantees it.Because the returned data in each tuple from
getQueriesDatacan be of varying structures, theTDatageneric is advisory — if you provide a specific type it is assumed you are certain each tuple's data entry is of that type. TypeScript will typedataasInfiniteNotificationData | undefinedafter thepredicatefilter, so at runtime every non-undefinedentry is guaranteed to satisfyisInfiniteData. TheisInfiniteData(data)call insideforEachcan be dropped in favour of just the null-guard.♻️ Proposed simplification
infiniteQueries.forEach(([queryKey, data]) => { - if (data && isInfiniteData(data)) { + if (data) { previousData.push([queryKey, data]); const updatedData: InfiniteNotificationData = {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/src/modules/notifications/mutations/use-mark-notifications-read.ts` around lines 74 - 95, The inner isInfiniteData check is redundant because getQueriesData was called with a predicate that already ensures each non-undefined data is InfiniteNotificationData; remove the isInfiniteData(data) condition inside the forEach and only guard against undefined (i.e., if (data) { ... }), keeping the rest of the logic that pushes to previousData and creates updatedData using markNotificationRead, then call queryClient.setQueryData(queryKey, updatedData); this simplifies the loop while retaining getQueriesData, predicate, InfiniteNotificationData, previousData, markNotificationRead, and queryClient.setQueryData usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@packages/sdk/src/modules/notifications/mutations/use-mark-notifications-read.ts`:
- Around line 97-117: The review confirms the optimistic unread-count decrement
in useMarkNotificationsRead.ts is correct: ensure no changes are made to the
logic in the block around QueryKeys.notifications.unreadCount, currentUnread,
and the isUnread check (which inspects the captured infiniteQueries pages) —
keep the existing pre-update snapshot usage so that queryClient.setQueryData
only decrements when the notification (id) was originally unread.
---
Nitpick comments:
In
`@apps/web/src/features/post-renderer/components/utils/hiveAuthorLinkEnhancer.tsx`:
- Around line 21-23: In applyAuthorLinks, mark elements as processed before any
early returns for archived tweets so skipped nodes don't get re-evaluated on
subsequent runs: set el.dataset.enhanced = "true" immediately after passing the
initial guard (the same place where el.dataset.enhanced is currently assigned)
or move that assignment to just before the archived-tweet check (the if that
calls el.closest(".markdown-view")?.textContent?.includes("Archived Tweet
from")), ensuring the function still returns early for archived tweets but that
those elements are flagged to avoid repeated DOM traversal.
In
`@packages/sdk/src/modules/notifications/mutations/use-mark-notifications-read.ts`:
- Around line 74-95: The inner isInfiniteData check is redundant because
getQueriesData was called with a predicate that already ensures each
non-undefined data is InfiniteNotificationData; remove the isInfiniteData(data)
condition inside the forEach and only guard against undefined (i.e., if (data) {
... }), keeping the rest of the logic that pushes to previousData and creates
updatedData using markNotificationRead, then call
queryClient.setQueryData(queryKey, updatedData); this simplifies the loop while
retaining getQueriesData, predicate, InfiniteNotificationData, previousData,
markNotificationRead, and queryClient.setQueryData usage.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes ECENCY-NEXT-13YF
Fixes #662
Summary by CodeRabbit
New Features
Bug Fixes
Chores