feat: Implement escrow management and fee calculation with new hooks, types, services, and UI components. - #160
Merged
Conversation
… types, services, and UI components.
|
@Franklivania is attempting to deploy a commit to the Threadflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe PR introduces comprehensive escrow management functionality, adding new components ( Changes
Sequence DiagramsequenceDiagram
participant User
participant BountyDetailClient
participant useEscrowPool
participant useEscrowSlots
participant EscrowService
participant EscrowDetailPanel
participant UI
User->>BountyDetailClient: View bounty detail (bountyId)
BountyDetailClient->>useEscrowPool: useEscrowPool(bountyId)
useEscrowPool->>EscrowService: getPool(bountyId)
EscrowService-->>useEscrowPool: EscrowPool data
useEscrowPool-->>BountyDetailClient: pool object
BountyDetailClient->>useEscrowSlots: useEscrowSlots(bountyId)
useEscrowSlots->>EscrowService: getSlots(bountyId)
EscrowService-->>useEscrowSlots: EscrowSlot[] array
useEscrowSlots-->>BountyDetailClient: slots array
BountyDetailClient->>EscrowDetailPanel: Render with poolId=bountyId
EscrowDetailPanel->>UI: Display pool summary & slot table
UI-->>User: Show escrow details
BountyDetailClient->>UI: Render FeeCalculator in sidebar
User->>UI: Enter bounty amount & type
UI->>EscrowService: calculateFee(amount, subType)
EscrowService-->>UI: FeeBreakdown
UI-->>User: Display platform fee, insurance fee, net payout
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (9)
hooks/__tests__/use-escrow.test.tsx (2)
10-14: Consider creating QueryClient per test for better isolation.The shared module-level
QueryClientmay cause test pollution if other test files are added. Creating a fresh instance per test ensures complete isolation:♻️ Suggested improvement
-const queryClient = new QueryClient(); - -const wrapper = ({ children }: { children: React.ReactNode }) => ( - <QueryClientProvider client={queryClient}>{children}</QueryClientProvider> -); +const createWrapper = () => { + const queryClient = new QueryClient({ + defaultOptions: { + queries: { retry: false }, + }, + }); + return ({ children }: { children: React.ReactNode }) => ( + <QueryClientProvider client={queryClient}>{children}</QueryClientProvider> + ); +}; // In each test: -const { result } = renderHook(() => useEscrowPool("1"), { wrapper }); +const { result } = renderHook(() => useEscrowPool("1"), { wrapper: createWrapper() });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/__tests__/use-escrow.test.tsx` around lines 10 - 14, The current test uses a shared module-level QueryClient which can cause cross-test pollution; change to create a fresh QueryClient per test by moving the QueryClient instantiation into the test setup (e.g., inside a beforeEach or by making the wrapper a factory) so that QueryClient is re-created for each test, update the wrapper to accept or create that per-test QueryClient, and ensure QueryClientProvider receives the fresh instance in each test run (refer to QueryClient, wrapper, and QueryClientProvider to locate the change).
78-114: Add test coverage for MILESTONE_BASED bounty type.Tests cover
FIXED_PRICE(5%+1%) andCOMPETITION(8%+2%) but omitMILESTONE_BASED(6%+1%). Adding this test ensures all three fee calculation paths are verified:💚 Missing test case
it("should return fee breakdown for MILESTONE_BASED", async () => { const { result } = renderHook( () => useFeeCalculation(1000, "MILESTONE_BASED"), { wrapper }, ); await waitFor(() => { expect(result.current.isSuccess).toBe(true); }); expect(result.current.data).toEqual({ grossAmount: 1000, platformFee: 60, insuranceFee: 10, netPayout: 930, }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/__tests__/use-escrow.test.tsx` around lines 78 - 114, Add a new test case in hooks/__tests__/use-escrow.test.tsx to cover the MILESTONE_BASED path of useFeeCalculation: call renderHook(() => useFeeCalculation(1000, "MILESTONE_BASED"), { wrapper }), wait for result.current.isSuccess to be true, then assert result.current.data equals { grossAmount: 1000, platformFee: 60, insuranceFee: 10, netPayout: 930 }; place this test alongside the existing FIXED_PRICE and COMPETITION tests to ensure all bounty types are covered.components/bounty/bounty-card.tsx (2)
131-145: Extract duplicatedlockedAmountcalculation.The
lockedAmountlogic (pool.status === "Fully Released" ? 0 : pool.totalAmount - pool.releasedAmount) is duplicated here and inescrow-detail-panel.tsx(lines 62-65). Consider extracting this to a shared utility function or computing it within theEscrowStatuscomponent itself.♻️ Potential utility extraction
// lib/utils/escrow.ts export function calculateLockedAmount(pool: EscrowPool): number { return pool.status === "Fully Released" ? 0 : pool.totalAmount - pool.releasedAmount; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/bounty/bounty-card.tsx` around lines 131 - 145, The duplicated lockedAmount computation (pool.status === "Fully Released" ? 0 : pool.totalAmount - pool.releasedAmount) used when rendering <EscrowStatus> should be extracted into a shared helper and used in both bounty-card.tsx and escrow-detail-panel.tsx (or computed inside the EscrowStatus component); create a utility like calculateLockedAmount(pool: EscrowPool) (or add a lockedAmount prop computed inside EscrowStatus) and replace the inline expression in bounty-card.tsx (and the duplicate in escrow-detail-panel.tsx) with a call to that helper (or remove the prop and let EscrowStatus derive it from pool/status/amounts).
88-90: Consider performance impact of fetching escrow data per card.Calling
useEscrowPool(bounty.id)in eachBountyCardcan trigger N parallel requests when rendering a bounty list. While React Query will cache and dedupe, the initial load may cause a waterfall of requests.Consider:
- Prefetching pool data at the list level and passing it down
- Using a batch query hook that fetches multiple pools at once
- Accepting an optional
poolprop to avoid fetching when parent already has the data🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/bounty/bounty-card.tsx` around lines 88 - 90, Calling useEscrowPool(bounty.id) inside each BountyCard causes many parallel fetches on list render; refactor so the parent list fetches or batches escrow data and BountyCard optionally consumes it: add an optional prop pool to BountyCard (or create a batch hook useEscrowPools(ids)) and update callers to prefetch/cache pools at the list level (or call useEscrowPools once) then pass pool into BountyCard to avoid per-card network requests and rely on React Query only for misses.components/bounty/fee-calculator.tsx (1)
49-57: Consider validating for non-negative amounts.The
min="0"attribute doesn't prevent negative values entered via keyboard. While the service likely handles this, consider clamping the value:♻️ Suggested validation
- const amount = Number(debouncedAmountStr) || 0; + const amount = Math.max(0, Number(debouncedAmountStr) || 0);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/bounty/fee-calculator.tsx` around lines 49 - 57, The amount input allows negative numbers via keyboard despite min="0"; update the onChange/onBlur handling in the FeeCalculator component (where Input, amountInput, and setAmountInput are used) to parse the incoming value to a number, clamp it to a non-negative value (e.g., Math.max(0, parsedValue)), and then set the state (convert back to string if you keep amountInput as a string); alternatively enforce the clamp on blur/submit to ensure negative values are not stored or sent to the fee calculation logic.components/bounty/escrow-status.tsx (1)
50-58: Consider defensive fallback for unknown status.While TypeScript ensures
statusmatchesEscrowPoolStatus, a defensive fallback prevents runtime crashes if the type and config ever drift:♻️ Optional defensive pattern
- const { icon: Icon, colorClass, bgClass, variant } = config[status]; + const statusConfig = config[status] ?? config["Escrowed"]; + const { icon: Icon, colorClass, bgClass, variant } = statusConfig;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/bounty/escrow-status.tsx` around lines 50 - 58, The component EscrowStatus currently indexes config with status directly (const { icon: Icon, colorClass, bgClass, variant } = config[status]) which can throw if config lacks that key at runtime; change this to a defensive lookup: resolve a fallback entry when config[status] is undefined and destructure from that fallback (e.g., fallbackIcon/fallback classes or a defaultConfig object), ensuring you still reference EscrowStatus, status, config and EscrowStatusProps so unknown or drifting statuses render a safe default UI instead of crashing.components/bounty-detail/bounty-detail-client.tsx (1)
77-85: Verify whether FeeCalculator should use bounty context.The
FeeCalculatoris rendered as a standalone tool without any props from the current bounty (amount, type). This appears intentional as an estimator, but verify if it would be more useful to pre-populate values from the bounty being viewed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/bounty-detail/bounty-detail-client.tsx` around lines 77 - 85, The FeeCalculator is currently standalone and likely should be pre-populated from the viewed bounty; update the rendering to pass the current bounty into FeeCalculator (e.g., pass bounty or specific fields like amount and type from the bounty variable used in this file) and then modify the FeeCalculator component to accept an optional prop (bounty or { amount, type }) and use it to initialize its inputs/state when provided; also update the FeeCalculator prop types/interface and any related tests to handle both standalone and pre-populated usage so existing estimator behavior remains backward compatible.components/bounty/escrow-detail-panel.tsx (1)
162-172: Hardcoded testnet URL should be configurable.The Stellar Explorer link points to a testnet URL with a mock account pattern. This should be environment-configurable for production deployment:
♻️ Suggested approach
// Use environment variable or config const STELLAR_EXPLORER_BASE = process.env.NEXT_PUBLIC_STELLAR_EXPLORER_URL ?? "https://stellar.expert/explorer/testnet"; // In component: href={`${STELLAR_EXPLORER_BASE}/account/${pool.poolId}`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/bounty/escrow-detail-panel.tsx` around lines 162 - 172, Replace the hardcoded testnet Explorer link with a configurable base URL: add a constant (e.g. STELLAR_EXPLORER_BASE) that reads process.env.NEXT_PUBLIC_STELLAR_EXPLORER_URL with a fallback to the current testnet URL, then update the anchor href in escrow-detail-panel to use `${STELLAR_EXPLORER_BASE}/account/${poolId}` (remove the hardcoded "escrow_mock_" prefix) while keeping the target, rel, and ExternalLink usage intact so production can point to a different Stellar Explorer.lib/services/escrow.ts (1)
110-112: Minor: Potential floating-point rounding inconsistency in fee calculation.Rounding
platformFeeandinsuranceFeeseparately before computingnetPayoutcan causegrossAmount ≠ platformFee + insuranceFee + netPayoutdue to accumulated rounding differences. For a mock service this is acceptable, but when migrating to production, consider rounding only at the final step or calculatingnetPayoutas a residual.♻️ Suggested approach for production
- const platformFee = Number((amount * platformRate).toFixed(2)); - const insuranceFee = Number((amount * insuranceRate).toFixed(2)); - const netPayout = Number((amount - platformFee - insuranceFee).toFixed(2)); + const platformFee = Math.round(amount * platformRate * 100) / 100; + const insuranceFee = Math.round(amount * insuranceRate * 100) / 100; + // Derive netPayout as residual to ensure amounts sum correctly + const netPayout = Math.round((amount - platformFee - insuranceFee) * 100) / 100;Using
Math.roundwith multiplication/division is more explicit about the rounding behavior thantoFixed+Numberconversion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/services/escrow.ts` around lines 110 - 112, The current code rounds platformFee and insuranceFee individually using toFixed before computing netPayout, which can create rounding drift; instead compute raw fees from amount * platformRate and amount * insuranceRate, round the final values using Math.round(amountInCents / 100) style (or round fees then compute netPayout as the residual: netPayout = amount - roundedPlatformFee - roundedInsuranceFee) so grossAmount always equals platformFee + insuranceFee + netPayout; update the calculations around platformFee, insuranceFee, and netPayout (referencing amount, platformRate, insuranceRate, platformFee, insuranceFee, netPayout) to use consistent rounding (prefer integer cents math) as described.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@components/bounty-detail/bounty-detail-client.tsx`:
- Around line 77-85: The FeeCalculator is currently standalone and likely should
be pre-populated from the viewed bounty; update the rendering to pass the
current bounty into FeeCalculator (e.g., pass bounty or specific fields like
amount and type from the bounty variable used in this file) and then modify the
FeeCalculator component to accept an optional prop (bounty or { amount, type })
and use it to initialize its inputs/state when provided; also update the
FeeCalculator prop types/interface and any related tests to handle both
standalone and pre-populated usage so existing estimator behavior remains
backward compatible.
In `@components/bounty/bounty-card.tsx`:
- Around line 131-145: The duplicated lockedAmount computation (pool.status ===
"Fully Released" ? 0 : pool.totalAmount - pool.releasedAmount) used when
rendering <EscrowStatus> should be extracted into a shared helper and used in
both bounty-card.tsx and escrow-detail-panel.tsx (or computed inside the
EscrowStatus component); create a utility like calculateLockedAmount(pool:
EscrowPool) (or add a lockedAmount prop computed inside EscrowStatus) and
replace the inline expression in bounty-card.tsx (and the duplicate in
escrow-detail-panel.tsx) with a call to that helper (or remove the prop and let
EscrowStatus derive it from pool/status/amounts).
- Around line 88-90: Calling useEscrowPool(bounty.id) inside each BountyCard
causes many parallel fetches on list render; refactor so the parent list fetches
or batches escrow data and BountyCard optionally consumes it: add an optional
prop pool to BountyCard (or create a batch hook useEscrowPools(ids)) and update
callers to prefetch/cache pools at the list level (or call useEscrowPools once)
then pass pool into BountyCard to avoid per-card network requests and rely on
React Query only for misses.
In `@components/bounty/escrow-detail-panel.tsx`:
- Around line 162-172: Replace the hardcoded testnet Explorer link with a
configurable base URL: add a constant (e.g. STELLAR_EXPLORER_BASE) that reads
process.env.NEXT_PUBLIC_STELLAR_EXPLORER_URL with a fallback to the current
testnet URL, then update the anchor href in escrow-detail-panel to use
`${STELLAR_EXPLORER_BASE}/account/${poolId}` (remove the hardcoded
"escrow_mock_" prefix) while keeping the target, rel, and ExternalLink usage
intact so production can point to a different Stellar Explorer.
In `@components/bounty/escrow-status.tsx`:
- Around line 50-58: The component EscrowStatus currently indexes config with
status directly (const { icon: Icon, colorClass, bgClass, variant } =
config[status]) which can throw if config lacks that key at runtime; change this
to a defensive lookup: resolve a fallback entry when config[status] is undefined
and destructure from that fallback (e.g., fallbackIcon/fallback classes or a
defaultConfig object), ensuring you still reference EscrowStatus, status, config
and EscrowStatusProps so unknown or drifting statuses render a safe default UI
instead of crashing.
In `@components/bounty/fee-calculator.tsx`:
- Around line 49-57: The amount input allows negative numbers via keyboard
despite min="0"; update the onChange/onBlur handling in the FeeCalculator
component (where Input, amountInput, and setAmountInput are used) to parse the
incoming value to a number, clamp it to a non-negative value (e.g., Math.max(0,
parsedValue)), and then set the state (convert back to string if you keep
amountInput as a string); alternatively enforce the clamp on blur/submit to
ensure negative values are not stored or sent to the fee calculation logic.
In `@hooks/__tests__/use-escrow.test.tsx`:
- Around line 10-14: The current test uses a shared module-level QueryClient
which can cause cross-test pollution; change to create a fresh QueryClient per
test by moving the QueryClient instantiation into the test setup (e.g., inside a
beforeEach or by making the wrapper a factory) so that QueryClient is re-created
for each test, update the wrapper to accept or create that per-test QueryClient,
and ensure QueryClientProvider receives the fresh instance in each test run
(refer to QueryClient, wrapper, and QueryClientProvider to locate the change).
- Around line 78-114: Add a new test case in hooks/__tests__/use-escrow.test.tsx
to cover the MILESTONE_BASED path of useFeeCalculation: call renderHook(() =>
useFeeCalculation(1000, "MILESTONE_BASED"), { wrapper }), wait for
result.current.isSuccess to be true, then assert result.current.data equals {
grossAmount: 1000, platformFee: 60, insuranceFee: 10, netPayout: 930 }; place
this test alongside the existing FIXED_PRICE and COMPETITION tests to ensure all
bounty types are covered.
In `@lib/services/escrow.ts`:
- Around line 110-112: The current code rounds platformFee and insuranceFee
individually using toFixed before computing netPayout, which can create rounding
drift; instead compute raw fees from amount * platformRate and amount *
insuranceRate, round the final values using Math.round(amountInCents / 100)
style (or round fees then compute netPayout as the residual: netPayout = amount
- roundedPlatformFee - roundedInsuranceFee) so grossAmount always equals
platformFee + insuranceFee + netPayout; update the calculations around
platformFee, insuranceFee, and netPayout (referencing amount, platformRate,
insuranceRate, platformFee, insuranceFee, netPayout) to use consistent rounding
(prefer integer cents math) as described.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9a33da9d-7db9-41ce-9b4e-e9bdf0e86a07
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
components/bounty-detail/bounty-detail-client.tsxcomponents/bounty/bounty-card.tsxcomponents/bounty/escrow-detail-panel.tsxcomponents/bounty/escrow-status.tsxcomponents/bounty/fee-calculator.tsxhooks/__tests__/use-escrow.test.tsxhooks/use-escrow.tsjest.config.jslib/services/escrow.tstypes/escrow.ts
|
@Franklivania 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! 🚀 |
This was referenced Apr 24, 2026
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.
Integrate Escrow UI for Bounty Fund Management (#143)
Description
This pull request introduces the Escrow UI, providing functionality for both creators and contributors to seamlessly view the state of bounty fund pools, release slots, and expected fee breakdowns. The data retrieval relies on a mocked service patterned identically to existing services (
ComplianceService) to make integration with runtime TypeScript contract bindings seamless once available (139).Closes #143
Commits
feat(escrow): add escrow tracking components and typesfeat(escrow): implement fee breakdown calculatorfeat(escrow): add use-escrow data hooks backed by mock servicetest(escrow): add unit tests for use-escrow query hooksfix(test): configure jest to handle .tsx test filesfeat(bounty): integrate escrow status and panel into bounty detail and card viewsChanges
EscrowPool,EscrowSlot, andFeeBreakdown.EscrowServiceto simulate on-chain escrow state.useEscrowPool,useEscrowSlots, anduseFeeCalculationcustom hooks.EscrowStatus: Compact badge displaying escrow state (Escrowed, Released, Partial, Refunded).EscrowDetailPanel: A comprehensive view displaying pool details, remaining lock, expiration date, and individual slot recipients.FeeCalculator: A sidebar widget allowing creators to calculate platform and insurance fee splits dynamically.BountyCardandBountyDetailClientviews.testMatchto detect.tsxtest files alongside.tsand created a comprehensive test suite covering the query hooks' logic paths.Acceptance Criteria Met
Related Issues
Fixes #143
Depends on #139 mapping completion for API hooks.