Mutations and performance improvements#3122
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces many direct dhive/pin/private-key and Redux-thunk flows with auth-aware Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Component (e.g., QuickProfile)
participant Hook as useFollowUserAction
participant SDK as `@ecency/sdk` (mutation)
participant QC as QueryClient
participant Redux as Redux (optimistic actions/toasts)
UI->>Hook: handleFollowUser({ following })
Hook->>Redux: dispatch FOLLOW (optimistic)
Hook->>SDK: mutateAsync({ following })
SDK-->>Hook: success / error
alt success
Hook->>Redux: dispatch FOLLOW_SUCCESS
Hook->>QC: invalidate/refetch affected queries
Hook->>Redux: dispatch TOAST_NOTIFICATION (success)
else error
Hook->>Redux: dispatch FOLLOW_FAIL
Hook->>Redux: dispatch TOAST_NOTIFICATION (failure)
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
src/components/hiveAuthModal/hooks/useHiveAuth.ts (2)
435-445: PIN and pinCode validation is thorough.Both the raw
pinand the derivedpinCodeare validated before use in decryption, addressing the previously identified concern aboutgetDigitPinCodereturningundefined.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/hiveAuthModal/hooks/useHiveAuth.ts` around lines 435 - 445, The PIN and derived pinCode are both validated in useHiveAuth: ensure the pre-decryption checks remain intact by keeping the existing guards that throw errors when pin is falsy or when getDigitPinCode(pin) returns a falsy value; specifically, retain the two checks around pin and pinCode (the call to getDigitPinCode and the two throws using intl.formatMessage) so decryption never runs with an undefined pinCode.
399-413: Broadcast-in-progress flag and ref-based reads look correct.The
didIncrementBroadcastInProgresslocal flag properly addresses the previously identified race condition where_broadcastInProgressCountcould be decremented infinallywithout ever being incremented. Readingaccountandpinfrom refs at the top ofbroadcastis the right approach.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/hiveAuthModal/hooks/useHiveAuth.ts` around lines 399 - 413, No code changes required: the broadcast function correctly uses a local didIncrementBroadcastInProgress flag to avoid decrementing _broadcastInProgressCount when it was never incremented, and it reads fresh values from refs (currentAccountRef and pinHashRef) to avoid stale closures; keep the current logic in broadcast, including the assert and the account/pin ref reads and the _broadcastInProgressCount increment/decrement guard.src/providers/queries/notificationQueries.ts (1)
78-83:⚠️ Potential issue | 🟠 MajorInvalidate filtered notification caches consistently.
notificationsQueryKeyis built withfilter = undefinedat Lines 78–83. If any screens calluseNotificationsQuerywith a non-undefined filter, those caches won’t be invalidated after mark‑read, leaving stale filtered lists. Consider threading the active filter into this hook or invalidating by a broader base key/predicate.✅ One possible fix (requires updating call sites)
-export const useNotificationReadMutation = () => { +export const useNotificationReadMutation = (filter?: NotificationFilters) => { ... - const notificationsQueryKey = getNotificationsInfiniteQueryOptions( - username, - authCode, - undefined, - FETCH_LIMIT, - ).queryKey; + const notificationsQueryKey = getNotificationsInfiniteQueryOptions( + username, + authCode, + filter, + FETCH_LIMIT, + ).queryKey;To verify filtered usage:
#!/bin/bash # Find all useNotificationsQuery call sites and check if a filter is passed rg -n "useNotificationsQuery\\s*\\(" -C2 src🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/providers/queries/notificationQueries.ts` around lines 78 - 83, notificationsQueryKey is being built with filter = undefined (using getNotificationsInfiniteQueryOptions) so invalidate calls won't match caches created by useNotificationsQuery when a filter is provided; update the invalidation to include the active filter or use a broader predicate: either (A) accept a filter param in this module/hook and build notificationsQueryKey via getNotificationsInfiniteQueryOptions(username, authCode, filter, FETCH_LIMIT) so invalidation targets the exact filtered key (refer to notificationsQueryKey and getNotificationsInfiniteQueryOptions), or (B) call queryClient.invalidateQueries with a predicate/partial key that matches the base notifications key (e.g., the same queryKey[0]/base key returned by getNotificationsInfiniteQueryOptions) so all filtered variants created by useNotificationsQuery are invalidated after mark-read. Ensure you update callers of notifications invalidation or the hook signature accordingly (see useNotificationsQuery for where filters originate).
🧹 Nitpick comments (2)
src/components/hiveAuthModal/hooks/useHiveAuth.ts (1)
650-653: TheMath.max(0, ...)is now redundant with thedidIncrementBroadcastInProgressguard.With the flag ensuring we only decrement when we actually incremented, the counter can never go below zero from this path. The
Math.maxis harmless defensive coding—fine to keep, but a plain_broadcastInProgressCount--would be equivalent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/hiveAuthModal/hooks/useHiveAuth.ts` around lines 650 - 653, The finally block currently decrements _broadcastInProgressCount using Math.max(0, _broadcastInProgressCount - 1) despite already guarding with the didIncrementBroadcastInProgress flag; replace that defensive expression with a simple decrement to reflect the guard — update the finally clause where didIncrementBroadcastInProgress is checked so it does _broadcastInProgressCount-- (or equivalent) instead of Math.max(...) to simplify the logic around _broadcastInProgressCount.src/components/promote/promoteView.tsx (1)
110-110:_renderDropdownname no longer reflects its implementation.The function only renders a
<Text>label now; the "Dropdown" name is a leftover that could confuse future readers.✏️ Suggested rename
- const _renderDropdown = (accountName) => <Text style={styles.dropdownText}>{accountName}</Text>; + const _renderAccountLabel = (accountName) => <Text style={styles.dropdownText}>{accountName}</Text>;- rightComponent={() => _renderDropdown(currentAccountName)} + rightComponent={() => _renderAccountLabel(currentAccountName)}Also applies to: 128-128
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/promote/promoteView.tsx` at line 110, The helper _renderDropdown no longer renders a dropdown and should be renamed to reflect its behavior (e.g., _renderAccountLabel or renderAccountText); update the function declaration and all uses of _renderDropdown (and the similar occurrence around line 128) to the new name, keeping the implementation unchanged (returning <Text style={styles.dropdownText}>{accountName}</Text>) and ensure imports/exports or references in the component match the new identifier.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/hiveAuthModal/hooks/useHiveAuth.ts`:
- Around line 588-607: In useHiveAuth, before applying the re-auth credentials
from auth to the account read from currentAccountRef (freshAccount), validate
that freshAccount.name === username (the username captured earlier) and skip the
Redux update if they differ; if mismatched, log a clear warning including both
names and do not call updateCurrentAccount, otherwise proceed with building
updatedLocal (encrypting auth.token with pinCode and setting hiveAuthExpiry) and
dispatching updateCurrentAccount as currently implemented.
- Around line 507-515: The callback that calls
getHiveAuthUri/extractPreferredScheme can return early on error but leaves
HAS.broadcast/_cdWait awaiting a response and keeps _broadcastInProgressCount
elevated; modify the callback used with HAS.broadcast so that when
getHiveAuthUri fails or returns null you immediately abort the broadcast (use an
AbortController or invoke the broadcast's reject path) before calling
setStatusText/setStatus to ensure _broadcastInProgressCount is decremented and
_cdWait does not hang; locate the logic around getHiveAuthUri,
extractPreferredScheme, setStatusText, setStatus and integrate an abort signal
or local timeout wrapper that triggers broadcast cancellation when the signing
app cannot be opened.
In `@src/providers/queries/notificationQueries.ts`:
- Around line 75-76: The useMemo call memoizing getDigitPinCode(pinHash) must
guard against a missing pinHash to avoid decryptKey being called with undefined;
update the useMemo in notificationQueries.ts so it returns null/undefined
(consistent with useAuth.ts and useClaimPointsMutation.ts) when pinHash is
falsy, and only invoke getDigitPinCode(pinHash) when pinHash is truthy; ensure
callers of the memoized value handle the nullable result.
In `@src/providers/queries/postQueries/tipsQueries.ts`:
- Around line 59-113: Before performing transfers in the mutationFn inside
useMutation (TipParams handler), add guards to validate recipient, author, and
permlink: trim each value and ensure they are non-empty strings, throwing a
clear Error (e.g., "Tip recipient is required", "Tip author is required", "Tip
permlink is required") if any are invalid; also ensure recipient matches
expected address format if applicable, and use the validated/sanitized author
and permlink when building the memo variable and when calling
transferPointMutation.mutateAsync, transferMutation.mutateAsync, and
transferEngineMutation.mutateAsync to avoid malformed memos or invalid
transfers.
---
Duplicate comments:
In `@src/components/hiveAuthModal/hooks/useHiveAuth.ts`:
- Around line 435-445: The PIN and derived pinCode are both validated in
useHiveAuth: ensure the pre-decryption checks remain intact by keeping the
existing guards that throw errors when pin is falsy or when getDigitPinCode(pin)
returns a falsy value; specifically, retain the two checks around pin and
pinCode (the call to getDigitPinCode and the two throws using
intl.formatMessage) so decryption never runs with an undefined pinCode.
- Around line 399-413: No code changes required: the broadcast function
correctly uses a local didIncrementBroadcastInProgress flag to avoid
decrementing _broadcastInProgressCount when it was never incremented, and it
reads fresh values from refs (currentAccountRef and pinHashRef) to avoid stale
closures; keep the current logic in broadcast, including the assert and the
account/pin ref reads and the _broadcastInProgressCount increment/decrement
guard.
In `@src/providers/queries/notificationQueries.ts`:
- Around line 78-83: notificationsQueryKey is being built with filter =
undefined (using getNotificationsInfiniteQueryOptions) so invalidate calls won't
match caches created by useNotificationsQuery when a filter is provided; update
the invalidation to include the active filter or use a broader predicate: either
(A) accept a filter param in this module/hook and build notificationsQueryKey
via getNotificationsInfiniteQueryOptions(username, authCode, filter,
FETCH_LIMIT) so invalidation targets the exact filtered key (refer to
notificationsQueryKey and getNotificationsInfiniteQueryOptions), or (B) call
queryClient.invalidateQueries with a predicate/partial key that matches the base
notifications key (e.g., the same queryKey[0]/base key returned by
getNotificationsInfiniteQueryOptions) so all filtered variants created by
useNotificationsQuery are invalidated after mark-read. Ensure you update callers
of notifications invalidation or the hook signature accordingly (see
useNotificationsQuery for where filters originate).
---
Nitpick comments:
In `@src/components/hiveAuthModal/hooks/useHiveAuth.ts`:
- Around line 650-653: The finally block currently decrements
_broadcastInProgressCount using Math.max(0, _broadcastInProgressCount - 1)
despite already guarding with the didIncrementBroadcastInProgress flag; replace
that defensive expression with a simple decrement to reflect the guard — update
the finally clause where didIncrementBroadcastInProgress is checked so it does
_broadcastInProgressCount-- (or equivalent) instead of Math.max(...) to simplify
the logic around _broadcastInProgressCount.
In `@src/components/promote/promoteView.tsx`:
- Line 110: The helper _renderDropdown no longer renders a dropdown and should
be renamed to reflect its behavior (e.g., _renderAccountLabel or
renderAccountText); update the function declaration and all uses of
_renderDropdown (and the similar occurrence around line 128) to the new name,
keeping the implementation unchanged (returning <Text
style={styles.dropdownText}>{accountName}</Text>) and ensure imports/exports or
references in the component match the new identifier.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/components/hiveAuthModal/hooks/useHiveAuth.tssrc/components/organisms/quickProfileModal/children/quickProfileContent.tsxsrc/components/organisms/quickProfileModal/container/quickProfileModal.tsxsrc/components/promote/promoteView.tsxsrc/containers/pointsContainer.tssrc/providers/queries/notificationQueries.tssrc/providers/queries/postQueries/tipsQueries.tssrc/screens/searchResult/screen/tabs/communities/container/communitiesResultsContainer.ts
Summary by CodeRabbit
New Features
Improvements
Bug Fixes