Split hooks/use-bounty-application.ts into domain files. Closes #276 - #311
Merged
Conversation
|
@Emmo00 is attempting to deploy a commit to the Threadflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthrough
ChangesBounty Hook Decomposition and GraphQL Fix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
@Emmo00 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
![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
🤖 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.
Inline comments:
In `@hooks/use-application-mutations.ts`:
- Around line 100-116: The decline mutation in use-application-mutations
currently only returns the input payload and a timestamp, so it never writes to
a real source of truth. Update the mutationFn for the decline flow to persist
the decline through the actual contract/API layer used by this hook, using the
existing bountyId, applicantAddress, and reason fields, and only return data
after that write succeeds. Keep the optimistic cache behavior, but ensure the
refetch invalidation in the same hook is backed by a real persistence call so
the declined applicant stays declined after refetch.
In `@hooks/use-milestone-mutations.ts`:
- Around line 157-160: The contributor mutation handlers in
use-milestone-mutations are still mocked, so optimistic milestone/slot updates
are lost after invalidateQueries refetches the bounty data. Update the relevant
mutationFn implementations for contributor add/remove flows to persist the
change through the real backend action instead of only awaiting delay and
returning the input, and keep the existing invalidateQueries calls so the cache
refresh reflects saved state. Locate the mock logic in the contributor mutation
functions and replace it with the actual persistence path used by the milestone
mutation hooks.
- Around line 101-109: In the milestone payout flow inside
use-milestone-mutations.ts, stop deriving amountToRelease from unsafe cache
fallbacks in the mutation handlers. In the releasePayment and related optimistic
update logic, read the bounty detail from the query cache only when it is
present and has a valid rewardAmount plus a non-empty milestones list; otherwise
fail fast by throwing or returning an error before calling
EscrowService.releasePayment. Use the existing mutation functions and
queryClient.getQueryData/ bountyKeys.detail lookup to locate the guard points,
and ensure the escrow update only runs with a finite, positive amount.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 00d1992b-ef8a-4241-8085-4412d5cac1f1
📒 Files selected for processing (7)
hooks/use-application-contracts.tshooks/use-application-mutations.tshooks/use-application-review-mutations.tshooks/use-bounty-application.tshooks/use-dispute-mutations.tshooks/use-milestone-mutations.tslib/server-graphql.ts
Comment on lines
+100
to
+116
| return useMutation({ | ||
| mutationFn: async ({ | ||
| bountyId, | ||
| applicantAddress, | ||
| reason, | ||
| }: { | ||
| bountyId: string; | ||
| applicantAddress: string; | ||
| reason?: string; | ||
| }) => { | ||
| return { | ||
| bountyId, | ||
| applicantAddress, | ||
| reason: reason?.trim() || undefined, | ||
| declinedAt: new Date().toISOString(), | ||
| }; | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
This decline mutation never reaches a source of truth.
Lines 110-115 only echo the payload and a timestamp. Since Lines 161-163 then invalidate the optimistic cache, the declined applicant will come back on the next refetch because no contract/API write happened here. This needs a real persistence path before the hook is safe to use.
Also applies to: 161-163
🤖 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 `@hooks/use-application-mutations.ts` around lines 100 - 116, The decline
mutation in use-application-mutations currently only returns the input payload
and a timestamp, so it never writes to a real source of truth. Update the
mutationFn for the decline flow to persist the decline through the actual
contract/API layer used by this hook, using the existing bountyId,
applicantAddress, and reason fields, and only return data after that write
succeeds. Keep the optimistic cache behavior, but ensure the refetch
invalidation in the same hook is backed by a real persistence call so the
declined applicant stays declined after refetch.
Comment on lines
+101
to
+109
| const previous = queryClient.getQueryData<ExtendedBountyQuery>( | ||
| bountyKeys.detail(bountyId), | ||
| ); | ||
| const totalAmount = previous?.bounty?.rewardAmount ?? 100; | ||
| const milestonesCount = previous?.bounty?.milestones?.length ?? 1; | ||
| const amountToRelease = totalAmount / milestonesCount; | ||
|
|
||
| await EscrowService.releasePayment(bountyId, amountToRelease); | ||
| return { contributorId, milestoneId, amountToRelease }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fail fast when payout inputs are missing.
Lines 104-106 and 120-122 derive amountToRelease from cache fallbacks. If the detail query is cold, this sends a hard-coded 100; if milestones is an empty array, milestones.length is 0, so the release amount becomes Infinity. Both values flow into EscrowService.releasePayment() and the optimistic escrow update.
Suggested direction
+function getReleaseAmount(bounty?: ExtendedBountyQuery["bounty"]) {
+ const totalAmount = bounty?.rewardAmount;
+ const milestonesCount = bounty?.milestones?.length;
+
+ if (totalAmount == null || !milestonesCount) {
+ throw new Error(
+ "Bounty reward and milestones must be loaded before releasing payment",
+ );
+ }
+
+ return totalAmount / milestonesCount;
+}
+
export function useReleasePayment(bountyId: string) {
const queryClient = useQueryClient();
return useMutation({
@@
const previous = queryClient.getQueryData<ExtendedBountyQuery>(
bountyKeys.detail(bountyId),
);
- const totalAmount = previous?.bounty?.rewardAmount ?? 100;
- const milestonesCount = previous?.bounty?.milestones?.length ?? 1;
- const amountToRelease = totalAmount / milestonesCount;
+ const amountToRelease = getReleaseAmount(previous?.bounty);
@@
const prevBounty = queryClient.getQueryData<ExtendedBountyQuery>(
bountyKeys.detail(bountyId),
);
- const totalAmount = prevBounty?.bounty?.rewardAmount ?? 100;
- const milestonesCount = prevBounty?.bounty?.milestones?.length ?? 1;
- const amountToRelease = totalAmount / milestonesCount;
+ const amountToRelease = getReleaseAmount(prevBounty?.bounty);Also applies to: 117-139
🤖 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 `@hooks/use-milestone-mutations.ts` around lines 101 - 109, In the milestone
payout flow inside use-milestone-mutations.ts, stop deriving amountToRelease
from unsafe cache fallbacks in the mutation handlers. In the releasePayment and
related optimistic update logic, read the bounty detail from the query cache
only when it is present and has a valid rewardAmount plus a non-empty milestones
list; otherwise fail fast by throwing or returning an error before calling
EscrowService.releasePayment. Use the existing mutation functions and
queryClient.getQueryData/ bountyKeys.detail lookup to locate the guard points,
and ensure the escrow update only runs with a finite, positive amount.
Comment on lines
+157
to
+160
| mutationFn: async ({ contributorId }: { contributorId: string }) => { | ||
| await delay(1000); | ||
| return { contributorId }; | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
These contributor mutations are still mocks.
Lines 157-159 and 222-224 only wait and return the input, but Lines 212-214 and 263-265 invalidate the authoritative bounty query. That means the optimistic milestone/slot updates disappear as soon as the refetch completes because nothing was persisted.
Also applies to: 212-214, 222-225, 263-265
🤖 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 `@hooks/use-milestone-mutations.ts` around lines 157 - 160, The contributor
mutation handlers in use-milestone-mutations are still mocked, so optimistic
milestone/slot updates are lost after invalidateQueries refetches the bounty
data. Update the relevant mutationFn implementations for contributor add/remove
flows to persist the change through the real backend action instead of only
awaiting delay and returning the input, and keep the existing invalidateQueries
calls so the cache refresh reflects saved state. Locate the mock logic in the
contributor mutation functions and replace it with the actual persistence path
used by the milestone mutation hooks.
Benjtalkshow
added a commit
to Johnpii1/bounties-fork
that referenced
this pull request
Jun 29, 2026
Reset branch onto current main so this PR no longer deletes the work from PRs boundlessfi#298, boundlessfi#311, and boundlessfi#313. Only changes left: - New hooks/__tests__/use-bounty-application.test.tsx with 12 cases covering happy and rollback paths for useApplyToBounty, useSelectApplicant, useApproveApplicationSubmission, useRequestRevisions, useApplyForSlot, useReleasePayment, useRemoveContributor, useDeclineApplicant, and useRaiseDispute. - Validation in useApplyToBounty that throws ApplicationError when applicantAddress is missing. Production guard, not test-only. Verified pnpm lint, pnpm tsc --noEmit, pnpm build, and the new test suite (12/12 passing) all clean.
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
2 participants
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.
Closes #276
Summary by CodeRabbit
New Features
Bug Fixes
Refactor