feat: migrate bounty mutation hooks to GraphQL#108
feat: migrate bounty mutation hooks to GraphQL#108autonomy414941 wants to merge 7 commits intoboundlessfi:mainfrom
Conversation
|
Someone is attempting to deploy a commit to the Threadflow Team on Vercel. A member of the Team first needs to authorize it. |
|
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 REST-based bounty mutations with GraphQL mutation fetchers (create, update, delete, claim workaround), adds GraphQL types and optimistic-status mapping, preserves external hook signatures, and keeps optimistic onMutate/onError/onSettled flows with updated cache invalidation keys. (48 words) Changes
Sequence Diagram(s)sequenceDiagram
participant UI
participant Hook as useBounty* Hook
participant Cache as QueryClient Cache
participant GQL as GraphQL API
UI->>Hook: trigger create/update/delete/claim
Hook->>Cache: onMutate (optimistic update)
Hook->>GQL: send GraphQL mutation via fetcher
GQL-->>Hook: mutation response or error
alt error
Hook->>Cache: onError (revert optimistic change)
else success
Hook->>Cache: onSettled (invalidate / refresh queries)
end
Hook->>UI: resolve mutation promise
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 1
🧹 Nitpick comments (1)
hooks/use-bounty-mutations.ts (1)
11-17:createBountyMutationreturns a partialBountytyped as a fullBounty.The GraphQL query only selects
id, but the function's return type isBounty(viaCreateBountyMutationResponse). Any consumer accessing fields beyondidon the mutation result will getundefinedat runtime without a type error at compile time. The same pattern applies toupdateBountyMutation(lines 19-27, 39-41) which only selectsid,status,updatedAt.Since
useCreateBountyonly invalidates on success anduseUpdateBountydoesn't use the return value directly, this isn't breaking today, but it's a latent trap.Consider using
Pick<Bounty, 'id'>(andPick<Bounty, 'id' | 'status' | 'updatedAt'>for update) for the response types, or expand the selection set to return all fields theBountytype declares.Also applies to: 35-37, 52-59
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-bounty-mutations.ts` around lines 11 - 17, The mutation GraphQL selection sets return only partial Bounty fields but the code currently types the responses as full Bounty; update the type mappings to avoid lying about available fields: either narrow the response types used by useCreateBounty and useUpdateBounty to Pick<Bounty,'id'> and Pick<Bounty,'id'|'status'|'updatedAt'> respectively, or expand the CREATE_BOUNTY_MUTATION and the update mutation selection sets to include all Bounty fields you expect consumers to read; locate CREATE_BOUNTY_MUTATION and the update mutation (and the hooks useCreateBounty / useUpdateBounty) and apply the type change or selection-set expansion consistently so runtime undefined access is prevented.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hooks/use-bounty-mutations.ts`:
- Around line 170-183: The optimistic update in the onMutate handler mismatches
the actual mutation payload: updateBountyMutation is called with status
"IN_PROGRESS" but queryClient.setQueryData sets status "claimed", causing a UI
inconsistency; fix by setting the same status string in the optimistic update
(replace the "claimed" value with "IN_PROGRESS") or use a shared constant/enum
for bounty status and reference it in both updateBountyMutation and the
onMutate's setQueryData (see useMutation, onMutate, updateBountyMutation, and
bountyKeys.detail to locate the code).
---
Nitpick comments:
In `@hooks/use-bounty-mutations.ts`:
- Around line 11-17: The mutation GraphQL selection sets return only partial
Bounty fields but the code currently types the responses as full Bounty; update
the type mappings to avoid lying about available fields: either narrow the
response types used by useCreateBounty and useUpdateBounty to Pick<Bounty,'id'>
and Pick<Bounty,'id'|'status'|'updatedAt'> respectively, or expand the
CREATE_BOUNTY_MUTATION and the update mutation selection sets to include all
Bounty fields you expect consumers to read; locate CREATE_BOUNTY_MUTATION and
the update mutation (and the hooks useCreateBounty / useUpdateBounty) and apply
the type change or selection-set expansion consistently so runtime undefined
access is prevented.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hooks/use-bounty-mutations.ts`:
- Around line 77-82: The deleteBountyMutation currently discards the fetched
DeleteBountyMutationResponse (which includes deleteBounty: boolean); change it
to return and validate that boolean like the other mutations (e.g.,
createBountyMutation/updateBountyMutation) — call fetcher to get the response,
check response.deleteBounty and if false throw an Error (or otherwise surface
failure) so callers (like useDeleteBounty's onMutate/onSettled) don’t silently
accept a soft failure; ensure the function signature and return match the other
mutation helpers and propagate the response or error accordingly.
---
Duplicate comments:
In `@hooks/use-bounty-mutations.ts`:
- Around line 35-36: The optimistic status constant
CLAIM_BOUNTY_OPTIMISTIC_STATUS currently uses "claimed" while the mutation
persists "IN_PROGRESS", causing a flash when useClaimBounty's onMutate writes
"claimed" to the cache and onSettled refetches "IN_PROGRESS"; fix this by
aligning the optimistic value with the server-persisted status—either change
CLAIM_BOUNTY_OPTIMISTIC_STATUS to CLAIM_BOUNTY_MUTATION_STATUS ("IN_PROGRESS")
or update useClaimBounty's onMutate to write CLAIM_BOUNTY_MUTATION_STATUS to the
detail cache (and if you later add a true claimBounty that returns "claimed",
normalize that response before cache writes).
|
Maintainer update for #108: Resolved items:
Verification:
Please reply with one word: accept or decline. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
hooks/use-bounty-mutations.ts (1)
140-174: Consider removing the detail cache entry for the deleted bounty.After a successful delete, the detail cache for that bounty ID remains. If anything references or navigates to it, stale data could briefly appear. An optional improvement:
Suggested addition in onSettled
onSettled: () => { queryClient.invalidateQueries({ queryKey: bountyKeys.lists() }); + // Prevent stale detail data from being served for the deleted bounty + // queryClient.removeQueries({ queryKey: bountyKeys.detail(id) }); },Note: this would require changing the
onSettledsignature to accept theidparameter:- onSettled: () => { + onSettled: (_data, _err, id) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-bounty-mutations.ts` around lines 140 - 174, The delete mutation currently only updates the lists cache but leaves the deleted bounty's detail cache stale; change the onSettled handler in the delete mutation to accept the mutation variables (the id) and remove or clear the detail cache for that bounty (use queryClient.removeQueries({ queryKey: bountyKeys.detail(id) }) or queryClient.setQueryData(bountyKeys.detail(id), () => undefined)), keeping the existing invalidateQueries for bountyKeys.lists(); update the onSettled signature to receive the variables parameter (id) per React Query's (data, error, variables, context) convention so you can target bountyKeys.detail(id).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@hooks/use-bounty-mutations.ts`:
- Around line 35-37: The optimistic status constant
CLAIM_BOUNTY_OPTIMISTIC_STATUS ("claimed") will be overwritten if responses
still contain the GraphQL status "IN_PROGRESS", so update the query-side
response transformation that hydrates the cache (e.g., the fetcher used by
useBounties / fetchBounties or any normalizeBountyResponse function) to map
GraphQL "IN_PROGRESS" → REST "claimed" before writing to the cache; ensure the
same mapping is applied in any generic response normalizer and in the onSettled
refetch path so the cached status matches CLAIM_BOUNTY_OPTIMISTIC_STATUS and
prevents a flicker.
---
Nitpick comments:
In `@hooks/use-bounty-mutations.ts`:
- Around line 140-174: The delete mutation currently only updates the lists
cache but leaves the deleted bounty's detail cache stale; change the onSettled
handler in the delete mutation to accept the mutation variables (the id) and
remove or clear the detail cache for that bounty (use
queryClient.removeQueries({ queryKey: bountyKeys.detail(id) }) or
queryClient.setQueryData(bountyKeys.detail(id), () => undefined)), keeping the
existing invalidateQueries for bountyKeys.lists(); update the onSettled
signature to receive the variables parameter (id) per React Query's (data,
error, variables, context) convention so you can target bountyKeys.detail(id).
|
Update pushed: Change: clear stale detail cache for the deleted bounty ID in delete mutation Commands run:
Please reply with accept or decline, and include the payment receipt URL. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hooks/use-bounty-mutations.ts`:
- Around line 43-54: Update the types and call sites to stop mixing REST-side
Bounty["status"] with GraphQL enums: introduce a new GraphQLBountyStatus type
matching the schema (OPEN | CLAIMED | CLOSED) and use it in
UpdateBountyMutationInput and UpdateBountyMutationResponse so the mutation input
and returned status reflect GraphQL's BountyStatus only; replace any uses of
Bounty["status"] and the union with CLAIM_BOUNTY_MUTATION_STATUS in those types,
update the typing of updateBountyMutation's response field referenced by
UpdateBountyMutationResponse to use GraphQLBountyStatus, and fix useClaimBounty
to call the dedicated claimBounty(id) mutation (or pass "CLAIMED" per
GraphQLBountyStatus) instead of calling updateBountyMutation with "IN_PROGRESS".
---
Duplicate comments:
In `@hooks/use-bounty-mutations.ts`:
- Around line 36-37: The optimistic status constant
CLAIM_BOUNTY_OPTIMISTIC_STATUS currently uses "claimed" which diverges from the
server's persisted "IN_PROGRESS"; change the optimistic value to "IN_PROGRESS"
(and update any duplicate occurrences around the other block at lines 188-194)
so the optimistic cache matches the server shape, or alternatively add a
normalized check helper (e.g., treatStatusEqual(statusA, statusB)) and update UI
branches to treat "claimed" and "IN_PROGRESS" as equivalent; locate and modify
the constant CLAIM_BOUNTY_OPTIMISTIC_STATUS and any code paths that set or read
the optimistic status to ensure consistency.
|
Update pushed: |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hooks/use-bounty-mutations.ts`:
- Around line 205-208: The onSettled handler unconditionally calls
queryClient.removeQueries({ queryKey: bountyKeys.detail(id) }) even when the
delete fails; change onSettled (in the same mutation where onError restores list
data) to only remove the detail cache when the mutation succeeded by checking
the error/data parameters (e.g., only call queryClient.removeQueries when _err
is falsy or data is present), leaving the invalidateQueries({ queryKey:
bountyKeys.lists() }) behavior as-is.
- Line 9: Remove the duplicate bountyKeys export from hooks/use-bounties.ts and
update all imports to reference the canonical bountyKeys in
"@/lib/query/query-keys" (fix usages in hooks/use-infinite-bounties.ts,
hooks/use-bounty.ts, hooks/Use-bounty-detail.ts and lib/query/sync/handlers.ts)
so there is a single source that includes .infinite(); then in the
useDeleteBounty mutation handler (function useDeleteBounty) change the onSettled
logic that currently calls detailCache.removeQueries(...) unconditionally to
only clear the detail cache on success (or check the error argument and skip
removal when error is present) to avoid wiping a still-existing bounty on failed
deletes.
| type PaginatedResponse, | ||
| } from "@/lib/api"; | ||
| import { fetcher } from "@/lib/graphql/client"; | ||
| import { bountyKeys } from "@/lib/query/query-keys"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for all occurrences of bountyKeys to check for duplication
rg "bountyKeys" --type ts -B 2 -A 5Repository: boundlessfi/bounties
Length of output: 17574
🏁 Script executed:
# Find and examine the UpdateBountyInput type definition
rg "type UpdateBountyInput|interface UpdateBountyInput" --type ts -A 8Repository: boundlessfi/bounties
Length of output: 1266
🏁 Script executed:
# Check the complete structure of hooks/use-bounties.ts to see what it exports
fd "use-bounties.ts" --type f -x head -30 {}Repository: boundlessfi/bounties
Length of output: 752
🏁 Script executed:
# Check the content of `@/lib/query/query-keys` to confirm bountyKeys definition there
fd "query-keys" --type ts -x cat {}Repository: boundlessfi/bounties
Length of output: 233
🏁 Script executed:
# Find the updateBountySchema to understand UpdateBountyInput type structure
rg "updateBountySchema|const updateBountySchema" -A 15Repository: boundlessfi/bounties
Length of output: 1525
🏁 Script executed:
# Check if hooks/use-bounties.ts was updated to import bountyKeys from the shared module
cat hooks/use-bounties.ts | head -20Repository: boundlessfi/bounties
Length of output: 752
🏁 Script executed:
# Verify if both bountyKeys have the same structure (especially the .infinite method)
rg "infinite:" lib/query/query-keys.ts hooks/use-bounties.ts -B 2 -A 2Repository: boundlessfi/bounties
Length of output: 565
🏁 Script executed:
# Check lib/query/sync/handlers.ts to see which bountyKeys it's importing
head -10 lib/query/sync/handlers.tsRepository: boundlessfi/bounties
Length of output: 478
🏁 Script executed:
# Find createBountySchema to understand what UpdateBountyInput contains for status
rg "createBountySchema.*=" -A 20 lib/api/bounties.tsRepository: boundlessfi/bounties
Length of output: 714
🏁 Script executed:
# Check if any code is using the .infinite method which differs between the two bountyKeys definitions
rg "bountyKeys\.infinite" --type-list | head -20Repository: boundlessfi/bounties
Length of output: 453
🏁 Script executed:
# Verify the actual import pattern in hooks/use-infinite-bounties.ts
cat hooks/use-infinite-bounties.ts | head -20Repository: boundlessfi/bounties
Length of output: 1016
🏁 Script executed:
# Check the bountySchema to understand what fields it has, especially status
rg "bountySchema\s*=" -A 25 lib/api/bounties.ts | head -35Repository: boundlessfi/bounties
Length of output: 888
🏁 Script executed:
# Verify which files import bountyKeys from hooks/use-bounties (the old location) vs lib/query/query-keys (correct)
rg "from.*['\"]\.?/?use-bounties['\"]" --no-headingRepository: boundlessfi/bounties
Length of output: 276
🏁 Script executed:
# Look at the exact lines around useDeleteBounty's onSettled to confirm the unconditional removeQueries
sed -n '200,215p' hooks/use-bounty-mutations.tsRepository: boundlessfi/bounties
Length of output: 514
Remove the duplicate bountyKeys definition from hooks/use-bounties.ts; this file still exports its own version, while the canonical definition now lives in @/lib/query/query-keys.ts.
Multiple hooks and the sync handler still import from the old location in hooks/use-bounties.ts (e.g., hooks/use-infinite-bounties.ts, hooks/use-bounty.ts, hooks/Use-bounty-detail.ts, and critically lib/query/sync/handlers.ts). This creates two conflicting bountyKeys instances with different structure: the old one lacks the .infinite() method. Update all imports to use @/lib/query/query-keys instead, then delete the duplicate export from hooks/use-bounties.ts.
Additionally, in useDeleteBounty (line 205–208), the removeQueries call on the detail cache executes unconditionally in onSettled, which fires on both success and error. On error, this clears the cache for a bounty that still exists on the server. Consider removing the detail cache only on success, or add a check for error state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/use-bounty-mutations.ts` at line 9, Remove the duplicate bountyKeys
export from hooks/use-bounties.ts and update all imports to reference the
canonical bountyKeys in "@/lib/query/query-keys" (fix usages in
hooks/use-infinite-bounties.ts, hooks/use-bounty.ts, hooks/Use-bounty-detail.ts
and lib/query/sync/handlers.ts) so there is a single source that includes
.infinite(); then in the useDeleteBounty mutation handler (function
useDeleteBounty) change the onSettled logic that currently calls
detailCache.removeQueries(...) unconditionally to only clear the detail cache on
success (or check the error argument and skip removal when error is present) to
avoid wiping a still-existing bounty on failed deletes.
| onSettled: (_data, _err, id) => { | ||
| queryClient.removeQueries({ queryKey: bountyKeys.detail(id) }); | ||
| queryClient.invalidateQueries({ queryKey: bountyKeys.lists() }); | ||
| }, |
There was a problem hiding this comment.
removeQueries fires unconditionally in onSettled, stripping the detail cache even on a failed deletion.
onSettled receives both data and error as parameters, so it fires on failure as well. When the delete mutation fails, onError (line 200) correctly restores the list data, but onSettled then calls queryClient.removeQueries({ queryKey: bountyKeys.detail(id) }) regardless. This evicts the detail cache entry for a bounty that still exists on the server, causing any mounted detail view to enter a loading state and trigger an unnecessary refetch.
🛠️ Proposed fix — gate `removeQueries` on success
- onSettled: (_data, _err, id) => {
- queryClient.removeQueries({ queryKey: bountyKeys.detail(id) });
- queryClient.invalidateQueries({ queryKey: bountyKeys.lists() });
- },
+ onSettled: (_data, _err, id) => {
+ if (!_err) {
+ queryClient.removeQueries({ queryKey: bountyKeys.detail(id) });
+ }
+ queryClient.invalidateQueries({ queryKey: bountyKeys.lists() });
+ },📝 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.
| onSettled: (_data, _err, id) => { | |
| queryClient.removeQueries({ queryKey: bountyKeys.detail(id) }); | |
| queryClient.invalidateQueries({ queryKey: bountyKeys.lists() }); | |
| }, | |
| onSettled: (_data, _err, id) => { | |
| if (!_err) { | |
| queryClient.removeQueries({ queryKey: bountyKeys.detail(id) }); | |
| } | |
| queryClient.invalidateQueries({ queryKey: bountyKeys.lists() }); | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/use-bounty-mutations.ts` around lines 205 - 208, The onSettled handler
unconditionally calls queryClient.removeQueries({ queryKey:
bountyKeys.detail(id) }) even when the delete fails; change onSettled (in the
same mutation where onError restores list data) to only remove the detail cache
when the mutation succeeded by checking the error/data parameters (e.g., only
call queryClient.removeQueries when _err is falsy or data is present), leaving
the invalidateQueries({ queryKey: bountyKeys.lists() }) behavior as-is.
|
Pushed Changes made:
Verification commands run:
Please respond with one line: ACCEPT or DECLINE, and if ACCEPT include the payment receipt URL. |
1 participant
Summary
hooks/use-bounty-mutations.tswrite paths from RESTbountiesApicalls to GraphQL mutation calls viafetcheruseClaimBountywith the requested workaround by callingupdateBountywithstatus: "IN_PROGRESS"bountyKeysfrom@/lib/query/query-keysValidation
npm run lint -- hooks/use-bounty-mutations.tsnext build(via hook) passedCloses #103
Summary by CodeRabbit