More SDK Mutations, mute, subscribe, unsubscribe, account update, delete comment#653
More SDK Mutations, mute, subscribe, unsubscribe, account update, delete comment#653
Conversation
📝 WalkthroughWalkthroughThis PR introduces SDK-backed mutation hooks across web and SDK layers. Web-specific mutation wrappers are created that delegate to new SDK mutations via broadcast adapters. SDK mutations are added for comment deletion, post muting, and community subscription/unsubscription with post-broadcast cache invalidation. Export barrels are extended to expose these mutations. Changes
Sequence Diagram(s)sequenceDiagram
participant WebComponent
participant WebMutationWrapper as Web Mutation<br/>(useDeleteCommentMutation)
participant SDKMutation as SDK Mutation<br/>(useDeleteComment)
participant BroadcastAdapter
participant QueryCache
WebComponent->>WebMutationWrapper: mutate(payload)
WebMutationWrapper->>WebMutationWrapper: getActiveUser()
WebMutationWrapper->>WebMutationWrapper: createWebBroadcastAdapter()
WebMutationWrapper->>SDKMutation: mutate(username, auth)
SDKMutation->>BroadcastAdapter: buildDeleteCommentOp()
SDKMutation->>BroadcastAdapter: broadcast(operation)
BroadcastAdapter-->>SDKMutation: success
SDKMutation->>QueryCache: invalidateQueries(feeds)
SDKMutation->>QueryCache: invalidateQueries(discussions)
SDKMutation->>QueryCache: invalidateQueries(parent)
SDKMutation-->>WebMutationWrapper: result
WebMutationWrapper->>WebComponent: onSuccess(callback)
sequenceDiagram
participant WebComponent
participant WebMutationWrapper as Web Mutation<br/>(updateProfile)
participant SDKMutation as SDK Mutation<br/>(useAccountUpdate)
participant BroadcastAdapter
participant QueryCache
WebComponent->>WebMutationWrapper: mutate(profile)
WebMutationWrapper->>WebMutationWrapper: getActiveUser()
WebMutationWrapper->>WebMutationWrapper: createWebBroadcastAdapter()
WebMutationWrapper->>SDKMutation: mutate(username, auth)
SDKMutation->>QueryCache: optimisticUpdate(FullAccount)
SDKMutation->>BroadcastAdapter: buildAccountUpdateOp()
SDKMutation->>BroadcastAdapter: broadcast(operation)
BroadcastAdapter-->>SDKMutation: success
SDKMutation->>QueryCache: invalidateQueries(accounts/full)
SDKMutation-->>WebMutationWrapper: result
WebMutationWrapper->>WebComponent: onSuccess()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ 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}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/api/mutations/update-profile.ts (1)
51-71:⚠️ Potential issue | 🔴 CriticalCache key collision causes shallow merge to overwrite SDK's thorough profile update.
The SDK's
useAccountUpdate(lines 99–117 ofuse-account-update.ts) performs an optimistic cache update usingbuildProfileMetadata()which:
- Deep merges the profile with
R.mergeDeep()- Sanitizes the tokens array (removes
privateKey,username)- Sets
version = 2This wrapper performs a second update on the same query key
["get-account-full", account.name]using a shallow spread (...data.profile, ...profile), which overwrites the SDK's carefully constructed profile and loses:
- Deep merge behavior for nested profile properties
- Token sanitization (sensitive fields remain exposed)
- Version field
Since both mutations target identical query keys, the web wrapper's
onSuccessoverwrites the SDK's update after it completes. Consider removing the manual cache update here and relying on the SDK's cache management, or coordinate the merge strategies to use deep merging and token sanitization consistently.
🤖 Fix all issues with AI agents
In `@apps/web/src/api/mutations/subscribe-to-community.ts`:
- Around line 38-43: The code inconsistently handles a possibly nullish
community: mutationKey uses community?.name while mutationFn accesses
community.name directly; to fix, make the handling consistent by either (A)
guarding at the top of the function (e.g., return or throw if community is
null/undefined) so you can safely use community.name in mutationKey and inside
mutationFn, or (B) keep optional chaining everywhere and bail out of
subscribeMutation.mutateAsync / unsubscribeMutation.mutateAsync when
community?.name is undefined; update references to mutationKey, mutationFn,
subscribeMutation.mutateAsync and unsubscribeMutation.mutateAsync to follow the
chosen approach.
- Around line 37-46: The mutationKey uses optional chaining (`community?.name`)
while the mutationFn and its calls (`subscribeMutation.mutateAsync`,
`unsubscribeMutation.mutateAsync`) use `community.name`; since the parameter is
a non-nullable Community, make access consistent by removing the optional
chaining in the useMutation call—change the key to use `community.name` so
`mutationKey` and `mutationFn` both reference the same non-null property.
In `@packages/sdk/src/modules/communities/mutations/use-unsubscribe-community.ts`:
- Around line 58-65: The invalidation keys embed the possibly undefined variable
username (used in the onSuccess callback where auth.adapter.invalidateQueries is
called), which can produce keys like ["accounts","subscriptions",undefined] and
fail to match cache entries; update the invalidate logic in
use-unsubscribe-community (and mirror the same fix in use-subscribe-community)
to only include the ["accounts","subscriptions",username] key when username is a
defined non-empty string (e.g., build an array of keys conditionally or push
that key only if username) before calling auth.adapter.invalidateQueries,
leaving the ["communities", variables.community] key intact; ensure you
reference the auth.adapter.invalidateQueries call and the username variable when
making the change.
🧹 Nitpick comments (7)
apps/web/src/api/sdk-mutations/use-update-profile-mutation.ts (1)
63-74: Passingusername ?? ""triggers a wasteful query for a non-existent account.When no user is logged in,
usernameisundefined, and the fallback""is passed touseAccountUpdate. Inside the SDK hook (Line 69 ofuse-account-update.ts), this firesuseQuery(getAccountFullQueryOptions("")), sending a network request for an empty username. The mutation itself is guarded (throws ifdatais missing), but the query is still issued unnecessarily.Consider passing
undefined(and updatinguseAccountUpdateto acceptstring | undefined) or disabling the query when username is empty — consistent with how the other SDK mutations (useMutePost,useSubscribeCommunity, etc.) acceptstring | undefined.packages/sdk/src/modules/accounts/mutations/use-account-update.ts (1)
63-65:usernameparameter type is inconsistent with sibling mutations.
useAccountUpdatetakesusername: string, whileuseMutePost,useSubscribeCommunity, anduseUnsubscribeCommunityall acceptstring | undefined. This forces the web wrapper (use-update-profile-mutation.ts) to use theusername ?? ""fallback, which triggers a wasteful query for an empty string (see Line 69).Aligning the signature to
username: string | undefined(and disabling the query when undefined) would make the API consistent and avoid the unnecessary network request.Proposed signature change
export function useAccountUpdate( - username: string, + username: string | undefined, auth?: AuthContextV2 ) {packages/sdk/src/modules/posts/mutations/use-delete-comment.ts (1)
82-117: Consider tightening the type ofqueriesToInvalidate.The array mixes plain query key arrays with predicate objects (
{ predicate: ... }). Usingany[]works but loses type safety. A union type (e.g.,Array<string[] | { predicate: (query: any) => boolean }>) would make the intent clearer and catch accidental misuse.apps/web/src/api/sdk-mutations/use-unsubscribe-community-mutation.ts (1)
3-3: Unused type import:UnsubscribeCommunityPayload.
UnsubscribeCommunityPayloadis imported but never referenced in this file. The same applies toSubscribeCommunityPayloadin the subscribe wrapper andMutePostPayload/DeleteCommentPayloadin their respective wrappers.Proposed fix
-import { useUnsubscribeCommunity, type UnsubscribeCommunityPayload } from "@ecency/sdk"; +import { useUnsubscribeCommunity } from "@ecency/sdk";apps/web/src/api/sdk-mutations/use-subscribe-community-mutation.ts (1)
3-3: Unused type import:SubscribeCommunityPayload.Same as the unsubscribe wrapper — the type is imported but not used.
Proposed fix
-import { useSubscribeCommunity, type SubscribeCommunityPayload } from "@ecency/sdk"; +import { useSubscribeCommunity } from "@ecency/sdk";apps/web/src/api/sdk-mutations/use-mute-post-mutation.ts (1)
3-3: Unused type import:MutePostPayload.Proposed fix
-import { useMutePost, type MutePostPayload } from "@ecency/sdk"; +import { useMutePost } from "@ecency/sdk";apps/web/src/api/sdk-mutations/use-delete-comment-mutation.ts (1)
3-3: Unused type import:DeleteCommentPayload.Proposed fix
-import { useDeleteComment, type DeleteCommentPayload } from "@ecency/sdk"; +import { useDeleteComment } from "@ecency/sdk";
| return useMutation({ | ||
| mutationKey: ["subscribeToCommunity", activeUser?.username, community?.name], | ||
| mutationKey: ["subscribeToCommunity", community?.name], | ||
| mutationFn: async ({ isSubscribe }: { isSubscribe: boolean }) => { | ||
| if (!activeUser) { | ||
| throw new Error("Can`t subscribe w/o active user"); | ||
| } | ||
|
|
||
| if (isSubscribe) { | ||
| return [ | ||
| isSubscribe, | ||
| await broadcastPostingJSON(activeUser?.username, "community", [ | ||
| "subscribe", | ||
| { community: community.name } | ||
| ]) | ||
| ] as const; | ||
| await subscribeMutation.mutateAsync({ community: community.name }); | ||
| } else { | ||
| return [ | ||
| isSubscribe, | ||
| await broadcastPostingJSON(activeUser?.username, "community", [ | ||
| "unsubscribe", | ||
| { community: community.name } | ||
| ]) | ||
| ] as const; | ||
| await unsubscribeMutation.mutateAsync({ community: community.name }); | ||
| } | ||
| }, | ||
| onSuccess: ([isSubscribe]) => { | ||
| queryClient.setQueryData<Subscription[] | undefined>( | ||
| ["accounts", "subscriptions", activeUser?.username], | ||
| (data) => { | ||
| if (!data) { | ||
| return isSubscribe | ||
| ? [[community.name, community.title, "guest", ""]] | ||
| : data; | ||
| } | ||
|
|
||
| return isSubscribe | ||
| ? [...data, [community.name, community.title, "guest", ""]] | ||
| : data.filter(([u]) => u !== community.name); | ||
| } | ||
| ); | ||
| queryClient.invalidateQueries({ | ||
| queryKey: [QueryIdentifiers.COMMUNITY, community.name] | ||
| }); | ||
| }, | ||
| onError: (err) => error(...formatError(err)) | ||
| } | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's confirm the file exists and examine its full content
cat -n apps/web/src/api/mutations/subscribe-to-community.tsRepository: ecency/vision-next
Length of output: 1999
🏁 Script executed:
# Check how useSubscribeToCommunity is consumed
rg -n --type=ts --type=tsx -C3 'useSubscribeToCommunity' -g '!**/mutations/subscribe-to-community.ts'Repository: ecency/vision-next
Length of output: 89
🏁 Script executed:
# Check React Query version to understand mutation patterns
fd package.json -x grep -l 'react-query\|@tanstack/react-query' {} \;Repository: ecency/vision-next
Length of output: 177
🏁 Script executed:
# Search for callers without file type restriction
rg -n 'useSubscribeToCommunity' --type ts -C3 | grep -v 'subscribe-to-community.ts'Repository: ecency/vision-next
Length of output: 1647
🏁 Script executed:
# Also search in all files more broadly
rg -n 'useSubscribeToCommunity' -C3 | head -100Repository: ecency/vision-next
Length of output: 2858
🏁 Script executed:
# Check the React Query version in the main package.json
cat apps/web/package.json | grep -A2 -B2 '@tanstack/react-query'Repository: ecency/vision-next
Length of output: 294
🏁 Script executed:
# Examine the caller component more thoroughly
cat -n apps/web/src/app/communities/_components/subscription-btn/index.tsxRepository: ecency/vision-next
Length of output: 2357
🏁 Script executed:
# Check if Community type allows undefined/null
rg -n 'type Community|interface Community' --type ts -A5Repository: ecency/vision-next
Length of output: 17134
🏁 Script executed:
# Search for any other potential callers more broadly
find . -name '*.ts' -o -name '*.tsx' | xargs grep -l 'useSubscribeToCommunity' 2>/dev/null | grep -v node_modules | head -20Repository: ecency/vision-next
Length of output: 186
🏁 Script executed:
# Verify that Community cannot be undefined in actual usage
cat -n apps/web/src/entities/communities.ts | head -40Repository: ecency/vision-next
Length of output: 1215
🏁 Script executed:
# Double-check if there are any other patterns where useSubscribeToCommunity receives undefined community
rg -B5 -A5 'useSubscribeToCommunity' apps/web/src/app/communities/_components/subscription-btn/index.tsxRepository: ecency/vision-next
Length of output: 850
Fix optional chaining inconsistency in mutationKey and mutationFn.
Line 38 uses community?.name with optional chaining, but lines 41 and 43 access community.name directly. Since the community parameter is required (type Community is non-nullable), the optional chaining is unnecessary. Make the access consistent by removing the optional chaining operator on line 38: ["subscribeToCommunity", community.name].
🤖 Prompt for AI Agents
In `@apps/web/src/api/mutations/subscribe-to-community.ts` around lines 37 - 46,
The mutationKey uses optional chaining (`community?.name`) while the mutationFn
and its calls (`subscribeMutation.mutateAsync`,
`unsubscribeMutation.mutateAsync`) use `community.name`; since the parameter is
a non-nullable Community, make access consistent by removing the optional
chaining in the useMutation call—change the key to use `community.name` so
`mutationKey` and `mutationFn` both reference the same non-null property.
| mutationKey: ["subscribeToCommunity", community?.name], | ||
| mutationFn: async ({ isSubscribe }: { isSubscribe: boolean }) => { | ||
| if (!activeUser) { | ||
| throw new Error("Can`t subscribe w/o active user"); | ||
| } | ||
|
|
||
| if (isSubscribe) { | ||
| return [ | ||
| isSubscribe, | ||
| await broadcastPostingJSON(activeUser?.username, "community", [ | ||
| "subscribe", | ||
| { community: community.name } | ||
| ]) | ||
| ] as const; | ||
| await subscribeMutation.mutateAsync({ community: community.name }); | ||
| } else { | ||
| return [ | ||
| isSubscribe, | ||
| await broadcastPostingJSON(activeUser?.username, "community", [ | ||
| "unsubscribe", | ||
| { community: community.name } | ||
| ]) | ||
| ] as const; | ||
| await unsubscribeMutation.mutateAsync({ community: community.name }); |
There was a problem hiding this comment.
Inconsistent nullish handling of community.
Line 38 uses community?.name (optional chaining), implying community could be nullish. But lines 41 and 43 access community.name without optional chaining, which would throw a TypeError if community is indeed nullish.
Either guard consistently or drop the optional chaining on line 38 if community is guaranteed by the parameter type.
Proposed fix (drop optional chaining)
- mutationKey: ["subscribeToCommunity", community?.name],
+ mutationKey: ["subscribeToCommunity", community.name],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mutationKey: ["subscribeToCommunity", community?.name], | |
| mutationFn: async ({ isSubscribe }: { isSubscribe: boolean }) => { | |
| if (!activeUser) { | |
| throw new Error("Can`t subscribe w/o active user"); | |
| } | |
| if (isSubscribe) { | |
| return [ | |
| isSubscribe, | |
| await broadcastPostingJSON(activeUser?.username, "community", [ | |
| "subscribe", | |
| { community: community.name } | |
| ]) | |
| ] as const; | |
| await subscribeMutation.mutateAsync({ community: community.name }); | |
| } else { | |
| return [ | |
| isSubscribe, | |
| await broadcastPostingJSON(activeUser?.username, "community", [ | |
| "unsubscribe", | |
| { community: community.name } | |
| ]) | |
| ] as const; | |
| await unsubscribeMutation.mutateAsync({ community: community.name }); | |
| mutationKey: ["subscribeToCommunity", community.name], | |
| mutationFn: async ({ isSubscribe }: { isSubscribe: boolean }) => { | |
| if (isSubscribe) { | |
| await subscribeMutation.mutateAsync({ community: community.name }); | |
| } else { | |
| await unsubscribeMutation.mutateAsync({ community: community.name }); |
🤖 Prompt for AI Agents
In `@apps/web/src/api/mutations/subscribe-to-community.ts` around lines 38 - 43,
The code inconsistently handles a possibly nullish community: mutationKey uses
community?.name while mutationFn accesses community.name directly; to fix, make
the handling consistent by either (A) guarding at the top of the function (e.g.,
return or throw if community is null/undefined) so you can safely use
community.name in mutationKey and inside mutationFn, or (B) keep optional
chaining everywhere and bail out of subscribeMutation.mutateAsync /
unsubscribeMutation.mutateAsync when community?.name is undefined; update
references to mutationKey, mutationFn, subscribeMutation.mutateAsync and
unsubscribeMutation.mutateAsync to follow the chosen approach.
| async (_result: any, variables) => { | ||
| // Cache invalidation | ||
| if (auth?.adapter?.invalidateQueries) { | ||
| await auth.adapter.invalidateQueries([ | ||
| ["accounts", "subscriptions", username], | ||
| ["communities", variables.community] | ||
| ]); | ||
| } |
There was a problem hiding this comment.
Cache invalidation key includes potentially undefined username.
On Line 62, username (typed string | undefined) is embedded directly in the invalidation key: ["accounts", "subscriptions", username]. If username is undefined at the time of the callback, the key becomes ["accounts", "subscriptions", undefined], which won't match the cache entry set with the actual username string, silently skipping invalidation.
The same issue exists in use-subscribe-community.ts (Line 62) and use-mute-post.ts does not have this issue since it doesn't embed username in its invalidation keys.
While broadcast likely requires an authenticated user (so username is defined in practice), adding a guard would be safer:
Proposed guard
async (_result: any, variables) => {
// Cache invalidation
- if (auth?.adapter?.invalidateQueries) {
+ if (auth?.adapter?.invalidateQueries && username) {
await auth.adapter.invalidateQueries([
["accounts", "subscriptions", username],
["communities", variables.community]
]);
}
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async (_result: any, variables) => { | |
| // Cache invalidation | |
| if (auth?.adapter?.invalidateQueries) { | |
| await auth.adapter.invalidateQueries([ | |
| ["accounts", "subscriptions", username], | |
| ["communities", variables.community] | |
| ]); | |
| } | |
| async (_result: any, variables) => { | |
| // Cache invalidation | |
| if (auth?.adapter?.invalidateQueries && username) { | |
| await auth.adapter.invalidateQueries([ | |
| ["accounts", "subscriptions", username], | |
| ["communities", variables.community] | |
| ]); | |
| } |
🤖 Prompt for AI Agents
In `@packages/sdk/src/modules/communities/mutations/use-unsubscribe-community.ts`
around lines 58 - 65, The invalidation keys embed the possibly undefined
variable username (used in the onSuccess callback where
auth.adapter.invalidateQueries is called), which can produce keys like
["accounts","subscriptions",undefined] and fail to match cache entries; update
the invalidate logic in use-unsubscribe-community (and mirror the same fix in
use-subscribe-community) to only include the
["accounts","subscriptions",username] key when username is a defined non-empty
string (e.g., build an array of keys conditionally or push that key only if
username) before calling auth.adapter.invalidateQueries, leaving the
["communities", variables.community] key intact; ensure you reference the
auth.adapter.invalidateQueries call and the username variable when making the
change.
1 participant
Summary by CodeRabbit