Feat: implementation logic for single claim model#111
Conversation
|
@Bosun-Josh121 is attempting to deploy a commit to the Threadflow Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughReorders and restructures the bounty detail sidebar CTA markup (adds outer spacing container, separators, and moves Meta after Reward) and applies formatting/indentation edits to bounty mutation hooks; no public API/signature changes and no behavioral changes to CTA handlers or mutation logic. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/mock-bounty.ts (1)
186-193:⚠️ Potential issue | 🟡 MinorBounty "6" has
status: "claimed"but noclaimInfo.Bounty "6" is also in a "claimed" state, but unlike bounty "2", it lacks a
claimInfoobject. This meansClaimantProfilewill not render for this bounty while the CTA still shows "Already Claimed." Consider adding mockclaimInfohere too for testing consistency, or leaving a comment explaining why it's intentionally omitted (e.g., different claiming model).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/mock-bounty.ts` around lines 186 - 193, Bounty object with id "6" has status "claimed" but is missing the claimInfo used by ClaimantProfile; update the bounty (the object for "6" in mock-bounty.ts) to include a mock claimInfo structure matching the shape used by ClaimantProfile (e.g., claimant id/name/avatar/claimedAt/amount or the same fields used in bounty "2"), or if intentional, add an inline comment on that bounty explaining why claimInfo is omitted (e.g., different claiming model) so tests and developers aren’t confused.
🧹 Nitpick comments (3)
components/bounty-detail/bounty-detail-sidebar-cta.tsx (2)
186-188: ClaimantProfile rendered based onstatus === "claimed"butclaimInfomay be absent.The guard here checks
bounty.status === "claimed"to renderClaimantProfile, butClaimantProfileitself returnsnullwhenclaimInfois missing (line 276). This works safely — no crash — but the user sees a "claimed" status with no claimant info at all (no card, no message). For bounties like mock bounty "6" (status "claimed", noclaimInfo), this could be confusing.Consider rendering a minimal fallback message (e.g., "Claimed by an unknown contributor") when
claimInfois absent but the bounty is claimed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/bounty-detail/bounty-detail-sidebar-cta.tsx` around lines 186 - 188, The component currently renders <ClaimantProfile claimInfo={bounty.claimInfo} /> whenever bounty.status === "claimed", but <ClaimantProfile> returns null if claimInfo is missing, leaving no UI for claimed bounties with missing data; update the render logic in bounty-detail-sidebar-cta so that when bounty.status === "claimed" you check bounty.claimInfo and render <ClaimantProfile claimInfo={bounty.claimInfo} /> if present, otherwise render a minimal fallback UI (e.g., a small message or card reading "Claimed by an unknown contributor") so users still see that the bounty is claimed even without claimInfo.
210-266:MobileCTAduplicateshandleActionand label logic fromSidebarCTA; both also instantiate separateuseClaimBountyhooks.The
handleAction,ctaLabel/label, andclaimMutationsetup inMobileCTA(lines 212–241) is nearly identical toSidebarCTA(lines 19, 35–63). Since both components are mounted simultaneously (CSSlg:hidden/lg:blockdoesn't unmount), you get two independent mutation instances — theirisPendingstates won't stay in sync across a breakpoint change.Consider extracting the shared claim logic (mutation, handler, label) into a custom hook (e.g.,
useClaimAction(bounty)) that both components consume, ensuring a single mutation instance and eliminating duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/bounty-detail/bounty-detail-sidebar-cta.tsx` around lines 210 - 266, Extract the duplicated claim logic from MobileCTA and SidebarCTA into a new custom hook (e.g., useClaimAction) that wraps useClaimBounty and exposes the shared symbols: claimMutation (or mutate/claim), isPending, a unified handleAction(bounty → opens dialog or calls mutate), and a label/ctaLabel function; then replace the internal claimMutation, handleAction, and label implementations in both MobileCTA and SidebarCTA to call useClaimAction(bounty) so both components share one mutation instance and identical label/behavior (keep SubmissionDialog state handling in each component if needed, but use the hook for mutation/label/handler).lib/api/bounties.ts (1)
54-56: Redundant claim fields: old flatclaimedAt/claimedBycoexist with new nestedclaimInfo.Lines 54–55 retain the legacy flat
claimedAtandclaimedByfields while line 56 introduces the newclaimInfoobject that encapsulates the same data in a richer form. The UI components now read fromclaimInfo, making the flat fields effectively dead weight in the schema. This dual representation creates ambiguity about which is the source of truth.Consider deprecating or removing the flat fields if the backend will migrate to the
claimInfostructure, or document why both are needed during a transition period.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/api/bounties.ts` around lines 54 - 56, The schema includes redundant flat fields claimedAt and claimedBy alongside the new claimInfo object; remove (or mark deprecated) the flat fields from the Zod schema and update any code that reads claimedAt/claimedBy to use claimInfo (or migrate values into claimInfo on input). Specifically, edit the Zod schema declarations in lib/api/bounties.ts to drop claimedAt and claimedBy (or leave them only in a transitional comment), ensure parsing/validation and any callers of the bounty schema (e.g., create/update handlers and downstream consumers) read claimInfo.{claimedAt,claimedBy,...} instead, and add a migration/compatibility step if you must accept legacy payloads by mapping claimedAt/claimedBy into claimInfo before validating.
🤖 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 108-125: The fallback branch in the update logic uses a brittle
validity check (data && data.projectName) and patches the cache with hardcoded
mock claimant values; update the check to verify the actual claim outcome (e.g.,
data?.claimInfo?.claimedBy or data?.status === "claimed") and remove or replace
the hardcoded mock values in the fallback path (the object merged into old with
status: "claimed" and claimInfo.claimedBy) — if you must keep a local fallback,
add a clear TODO comment above the fallback that warns to replace mock
userId/username/avatarUrl before production, otherwise drop the fallback and let
the real API response (data) determine the cached value.
In `@types/bounty.ts`:
- Around line 8-13: The TypeScript union ClaimingModel and the Zod enum
claimingModelSchema are out of sync because ClaimingModel includes "milestone"
but claimingModelSchema does not; update the Zod schema in lib/api/bounties.ts
(claimingModelSchema) to include "milestone" in its enum values so validation
accepts the same set as the ClaimingModel type (alternatively remove "milestone"
from the ClaimingModel type if the feature is intentionally unsupported).
---
Outside diff comments:
In `@lib/mock-bounty.ts`:
- Around line 186-193: Bounty object with id "6" has status "claimed" but is
missing the claimInfo used by ClaimantProfile; update the bounty (the object for
"6" in mock-bounty.ts) to include a mock claimInfo structure matching the shape
used by ClaimantProfile (e.g., claimant id/name/avatar/claimedAt/amount or the
same fields used in bounty "2"), or if intentional, add an inline comment on
that bounty explaining why claimInfo is omitted (e.g., different claiming model)
so tests and developers aren’t confused.
---
Nitpick comments:
In `@components/bounty-detail/bounty-detail-sidebar-cta.tsx`:
- Around line 186-188: The component currently renders <ClaimantProfile
claimInfo={bounty.claimInfo} /> whenever bounty.status === "claimed", but
<ClaimantProfile> returns null if claimInfo is missing, leaving no UI for
claimed bounties with missing data; update the render logic in
bounty-detail-sidebar-cta so that when bounty.status === "claimed" you check
bounty.claimInfo and render <ClaimantProfile claimInfo={bounty.claimInfo} /> if
present, otherwise render a minimal fallback UI (e.g., a small message or card
reading "Claimed by an unknown contributor") so users still see that the bounty
is claimed even without claimInfo.
- Around line 210-266: Extract the duplicated claim logic from MobileCTA and
SidebarCTA into a new custom hook (e.g., useClaimAction) that wraps
useClaimBounty and exposes the shared symbols: claimMutation (or mutate/claim),
isPending, a unified handleAction(bounty → opens dialog or calls mutate), and a
label/ctaLabel function; then replace the internal claimMutation, handleAction,
and label implementations in both MobileCTA and SidebarCTA to call
useClaimAction(bounty) so both components share one mutation instance and
identical label/behavior (keep SubmissionDialog state handling in each component
if needed, but use the hook for mutation/label/handler).
In `@lib/api/bounties.ts`:
- Around line 54-56: The schema includes redundant flat fields claimedAt and
claimedBy alongside the new claimInfo object; remove (or mark deprecated) the
flat fields from the Zod schema and update any code that reads
claimedAt/claimedBy to use claimInfo (or migrate values into claimInfo on
input). Specifically, edit the Zod schema declarations in lib/api/bounties.ts to
drop claimedAt and claimedBy (or leave them only in a transitional comment),
ensure parsing/validation and any callers of the bounty schema (e.g.,
create/update handlers and downstream consumers) read
claimInfo.{claimedAt,claimedBy,...} instead, and add a migration/compatibility
step if you must accept legacy payloads by mapping claimedAt/claimedBy into
claimInfo before validating.
…r-single-claim-model
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
hooks/use-bounty-mutations.ts (1)
99-124: Consider addingonSettledto re-validate the detail cache.The hook has no
onSettledhandler, so the detail cache is only updated via thesetQueryDatacall inonSuccess. If the claim API response is ever partial or the cache is not pre-populated (theif (!old) return oldguard silently no-ops), the detail entry won't be refreshed. Adding anonSettledinvalidation keeps the detail in sync consistently, matching the pattern already used inuseUpdateBounty.♻️ Proposed addition
onSuccess: (data, id) => { queryClient.setQueryData<Bounty>(bountyKeys.detail(id), data); queryClient.invalidateQueries({ queryKey: bountyKeys.lists() }); }, + onSettled: (_data, _err, id) => { + queryClient.invalidateQueries({ queryKey: bountyKeys.detail(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 99 - 124, The useClaimBounty hook only updates the detail cache in onSuccess via queryClient.setQueryData and skips when old is missing, so add an onSettled handler to always revalidate the bounty detail; implement onSettled: call queryClient.invalidateQueries({ queryKey: bountyKeys.detail(id) }) (or pass the id param from the mutation variables) and also keep the existing queryClient.invalidateQueries({ queryKey: bountyKeys.lists() }) behavior to ensure the detail cache is refreshed whether the mutation succeeds or fails.
🤖 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 104-122: The onSuccess handler contains dead fallback logic: since
mutationFn returns Promise<Bounty>, data is always a non-null Bounty, so remove
the unreachable if (data) { ... } else { ... } branch and simplify the update to
always setQueryData<Bounty>(bountyKeys.detail(id), () => data); keep the
subsequent queryClient.invalidateQueries({ queryKey: bountyKeys.lists() }) call;
update references are in the onSuccess callback, queryClient.setQueryData,
bountyKeys.detail and bountyKeys.lists, and the Bounty/mutationFn contract.
---
Nitpick comments:
In `@hooks/use-bounty-mutations.ts`:
- Around line 99-124: The useClaimBounty hook only updates the detail cache in
onSuccess via queryClient.setQueryData and skips when old is missing, so add an
onSettled handler to always revalidate the bounty detail; implement onSettled:
call queryClient.invalidateQueries({ queryKey: bountyKeys.detail(id) }) (or pass
the id param from the mutation variables) and also keep the existing
queryClient.invalidateQueries({ queryKey: bountyKeys.lists() }) behavior to
ensure the detail cache is refreshed whether the mutation succeeds or fails.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
components/bounty-detail/bounty-detail-sidebar-cta.tsxhooks/use-bounty-mutations.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- components/bounty-detail/bounty-detail-sidebar-cta.tsx
2 participants
Description
This PR implements the Single Claim Model logic for the bounty details page, bridging the gap between an open bounty and a claimed state.
Closes #79
Key Changes
ClaimInfotype to theBountyinterface (types/bounty.ts) and updated the Zod schema (lib/api/bounties.ts) to validate and pass the new data structure.ClaimantProfilecomponent in the sidebar (bounty-detail-sidebar-cta.tsx) that displays the avatar, username, and timestamp of the user who claimed the bounty.mockBountiesto include a fully mockedclaimInfoobject for UI testing.Acceptance Criteria Met
Testing Steps
/bounty/1)./bounty/2). Verify the Claimant Profile renders correctly on load.Summary by CodeRabbit