Implement Multi-Winner Milestone Bounty Flow (Issue #173) - #186
Conversation
…#173) - Added MULTI_WINNER_MILESTONE bounty type and updated data models - Implemented MilestoneFunnel component for shared progress visualization - Added individual progress tracking for contributors - Created maintainer dashboard for milestone advancement and reward release - Integrated slot application process in sidebar
|
@Jopsan-gm 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:
📝 WalkthroughWalkthroughAdds frontend support for a new MULTI_WINNER_MILESTONE bounty type: types and mocks, milestone funnel and submission UI, maintainer dashboard, session-aware bounty-detail rendering, and slot-application CTA integrations. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BountyClient as BountyDetailClient
participant Auth as AuthClient
participant Server
participant Funnel as MilestoneFunnel
participant CTA as SidebarCTA/ApplicationDialog
User->>BountyClient: Open bounty page
BountyClient->>Auth: request session (authClient.useSession)
Auth-->>BountyClient: session (or null)
BountyClient->>Server: fetch bounty data
Server-->>BountyClient: bounty (may lack model‑4 fields)
alt session present
BountyClient->>Funnel: render with real contributors
else no session
BountyClient->>Funnel: render with mock fallback
end
User->>CTA: Click "Apply for Slot"
CTA->>ApplicationDialog: open dialog
User->>ApplicationDialog: submit application
ApplicationDialog->>Server: post application payload
Server-->>ApplicationDialog: ack
ApplicationDialog-->>User: success / close
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ 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 |
|
@Jopsan-gm 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: 12
🧹 Nitpick comments (5)
lib/mock-model4.ts (1)
1-61: LGTM — minor semantic note on mock data shape.Mock data is correctly typed. One observation:
MOCK_MODEL4_MILESTONESmarkm1andm2asisCompleted: trueglobally, whileMOCK_MODEL4_CONTRIBUTORSlists Bob atm2and David atm1. The types permit this, but the funnel UI incomponents/bounty/milestone-funnel.tsxwill show those contributor avatars under a milestone the funnel renders as "completed" (green checkmark), which may look inconsistent in the demo. Worth tweaking the mock if a screenshot is going into docs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/mock-model4.ts` around lines 1 - 61, The mock data marks milestones "m1" and "m2" as completed in MOCK_MODEL4_MILESTONES while MOCK_MODEL4_CONTRIBUTORS places live contributors at those same milestone IDs, causing a visual inconsistency in the milestone-funnel demo; update the mock so UI looks coherent by either moving contributors (e.g., change Bob/David currentMilestoneId to an incomplete milestone like "m3" or "m4") or toggling isCompleted on the corresponding milestones (MOCK_MODEL4_MILESTONES entries for "m1"/"m2") so contributors are not shown under milestones rendered as completed by components/bounty/milestone-funnel.tsx.components/bounty/bounty-header.tsx (1)
25-29: LGTM — but consider centralizing type config.The new entry is correct. As a follow-up (not blocking), the same
typeConfig/TYPE_CONFIGmapping is now duplicated acrosscomponents/bounty/bounty-header.tsx,components/bounty/github-bounty-card.tsx, andlib/bounty-config.ts, each with slightly diverging color schemes for the existing types (e.g.,MILESTONE_BASEDisbg-gray-700here,bg-secondary-500in the card, andbg-violet-500/10inlib/bounty-config.ts). Consider consolidating to a single source of truth.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/bounty/bounty-header.tsx` around lines 25 - 29, The TYPE_CONFIG/typeConfig mapping has been duplicated across components (including the new MULTI_WINNER_MILESTONE entry) causing inconsistent color schemes; extract a single exported central constant (e.g., export const TYPE_CONFIG) in the existing lib configuration module (lib/bounty-config) and update components (bounty-header.tsx and github-bounty-card.tsx) to import and use that constant instead of defining their own mappings; ensure the central mapping includes the new MULTI_WINNER_MILESTONE, pick the canonical color/class values there, and remove the local TYPE_CONFIG/typeConfig definitions so all components reference the single source of truth.components/bounty-detail/bounty-detail-sidebar-cta.tsx (1)
33-33: MergeUsersinto the existinglucide-reactimport.Minor: line 33 introduces a second
import { Users } from "lucide-react";even though there's already a multi-line import fromlucide-reactat lines 4–11. Consolidating avoids duplicate import statements.♻️ Proposed change
import { Github, Copy, Check, AlertCircle, XCircle, Loader2, + Users, } from "lucide-react"; @@ import { ApplicationDialog } from "@/components/bounty/application-dialog"; -import { Users } from "lucide-react"; import { Bounty } from "@/types/bounty";🤖 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` at line 33, There is a duplicate import of Users from "lucide-react"; remove the separate line importing Users and add Users to the existing multi-line import from "lucide-react" (i.e., include Users alongside the other icons in the existing import statement) so all lucide-react icons are consolidated into a single import.components/bounty/milestone-funnel.tsx (1)
92-96: Remove unnecessary optional chaining on required field.
contributor.userNameis non-optional inContributorProgress, so the?.and?? "?"operators add no value. Either remove them or relax the type ifuserNamecan legitimately be missing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/bounty/milestone-funnel.tsx` around lines 92 - 96, The AvatarFallback rendering uses unnecessary optional chaining and a nullish fallback for a required field; update the JSX in milestone-funnel's AvatarFallback so it directly uses contributor.userName.substring(0, 2).toUpperCase() (remove the ?. and ?? "?"), or if userName can actually be missing, relax the ContributorProgress type to make userName optional and keep appropriate null handling—target the contributor.userName expression inside AvatarFallback and adjust either the JSX or the ContributorProgress type accordingly.types/bounty.ts (1)
72-84: Consider modeling completion at the per-contributor level rather than per-milestone.
Milestone.isCompletedis a global flag, but the multi-winner flow has each contributor progressing independently — milestone 2 may be completed for contributor A but not B. The current shape forces consumers to combineMilestone.isCompletedwith the contributor'scurrentMilestoneIdindex, which is ambiguous (see follow-up comments inmilestone-submission-card.tsx).Additionally,
ContributorProgress.currentMilestoneId: stringcannot represent "has completed all milestones" or "not yet started," yet the funnel/dashboard need to render those states. Allowingnull(and ideally tracking the set of completed milestone ids per contributor) would remove the ambiguity.♻️ Suggested shape
export interface Milestone { id: string; title: string; description?: string; - isCompleted?: boolean; } export interface ContributorProgress { userId: string; userName: string; userAvatarUrl: string; - currentMilestoneId: string; + currentMilestoneId: string | null; + completedMilestoneIds?: string[]; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@types/bounty.ts` around lines 72 - 84, Milestone.isCompleted should be removed as a global flag and ContributorProgress should track progress per-user: update the Milestone interface (remove isCompleted) and change ContributorProgress (currently userId, userName, userAvatarUrl, currentMilestoneId) to make currentMilestoneId nullable (string | null) and add a completedMilestoneIds: string[] (or Set<string>) field so a contributor can represent "not started" (null), per-milestone completions, and "all done" by comparing completedMilestoneIds to the milestone list; update any consumers referencing Milestone.isCompleted or assuming non-null currentMilestoneId to use the new ContributorProgress fields (look for types Milestone and ContributorProgress in this diff).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/bounty-detail/bounty-detail-client.tsx`:
- Around line 103-155: The code is repeatedly casting bounty to Bounty to access
fields (milestones, contributorProgress) that the GraphQL fragment
(BountyFieldsFragment) doesn't actually include, hiding missing data; fix by
adding a single typed adapter function (e.g., getMilestoneData(bounty): {
milestones, contributorProgress }) used by MilestoneFunnel,
MilestoneSubmissionCard, and Model4MaintainerDashboard that safely reads
properties from the fragment and returns either the real fields or the
MOCK_MODEL4_* fallbacks, or alternatively update the GraphQL fragment/query used
by useBountyDetail to include milestones and contributorProgress so no casts are
needed; reference the adapter name getMilestoneData and the components
MilestoneFunnel, MilestoneSubmissionCard, and Model4MaintainerDashboard when
making the change.
- Around line 128-141: The current rendering of MilestoneSubmissionCard for
MULTI_WINNER_MILESTONE uses a fallback to MOCK_MODEL4_CONTRIBUTORS[0], which
exposes Alice's progress to any signed-in non-participant; change the logic to
first compute a matched contributor (e.g., const matched = ((bounty as
Bounty).contributorProgress || MOCK_MODEL4_CONTRIBUTORS).find(c => c.userId ===
session?.user?.id)) and only render MilestoneSubmissionCard when matched exists
(pass matched as contributorProgress and remove the MOCK_MODEL4_CONTRIBUTORS[0]
fallback); otherwise render a "You haven't applied yet" CTA or hide the card
entirely so non-participants cannot see or act on someone else's progress.
In `@components/bounty-detail/bounty-detail-sidebar-cta.tsx`:
- Around line 136-159: The onApply handler in the MULTI_WINNER_MILESTONE
ApplicationDialog is a stub that just console.logs and returns true, so wire it
to the real application flow: replace the stub in the onApply prop of
ApplicationDialog with a call to the actual slot-application mutation (e.g.,
applyForBountySlot / submitApplication API) and await its result, propagate
success/failure to ApplicationDialog.handleSubmit (so the dialog only closes on
real success), show user-facing feedback on error/success, and update the local
slot counter/cache (or refetch the bounty) on success; if the backend is not yet
available, instead disable the Button trigger for this branch or render a
visible “Demo only / Coming soon” indicator so users aren’t misled. Ensure you
modify the onApply prop in bounty-detail-sidebar-cta.tsx and coordinate with
components/bounty/application-dialog.tsx handleSubmit behavior to surface
errors.
- Around line 118-128: The component is casting the bounty prop to a broader
Bounty type to access totalSlotsOccupied and maxSlots, but those fields aren’t
selected on BountyFieldsFragment so they’re always undefined; update the GraphQL
BountyFieldsFragment to include totalSlotsOccupied and maxSlots, run codegen to
regenerate types, remove the unsafe cast in bounty-detail-sidebar-cta and read
the typed fields directly. Also replace the stubbed onApply handler (the
function used by ApplicationDialog) that only logs and returns true with a real
submission flow: call the backend application API, handle errors (keep dialog
open on failure and show feedback), and on success persist state by refetching
the bounty or applying an optimistic update to totalSlotsOccupied so the UI
reflects the new slot occupancy.
In `@components/bounty-detail/milestone-submission-card.tsx`:
- Around line 103-115: The action buttons in milestone-submission-card.tsx
(rendered when isCurrent) are missing handlers so View Task and Submit Work do
nothing; update the component to accept and use props like onViewTask (or
taskUrl) and onSubmitWork, wiring them to the respective Button elements (or
render an <a> with href for taskUrl), and bubble those props up from the parent
(e.g., BountyDetailClient) or mark them as required/default to a no-op with a
TODO comment so this is tracked; ensure you reference the Button instances that
include <ExternalLink/> and <Send/> and add proper onClick/onKey handlers and
aria-labels for accessibility.
- Around line 22-47: The code doesn't handle findIndex === -1 and allows a
milestone to be both "completed" and "locked"; change the index handling and the
three derived flags: compute a safeCurrentIndex = (currentIdx === -1 ?
milestones.length : currentIdx) where currentIdx is the result of
milestones.findIndex(m => m.id === contributorProgress.currentMilestoneId); then
derive isCurrent = index === safeCurrentIndex, isLocked = index >
safeCurrentIndex, and isCompleted = milestone.isCompleted || index <
safeCurrentIndex. Also update the UI count (where currentMilestoneIndex is
shown) to use Math.min(safeCurrentIndex + 1, milestones.length) so
finished/stale IDs render correctly. Ensure you update uses of
currentMilestoneIndex in the component (the findIndex, the display string, and
inside milestones.map) to use these new names/values.
- Line 75: The shadow utility uses rgba(var(--primary),0.3) but --primary is a
HEX token so rgba(var(--primary),...) will not parse; in
components/bounty-detail/milestone-submission-card.tsx find the conditional that
emits "bg-background border-primary text-primary
shadow-[0_0_10px_rgba(var(--primary),0.3)]" and replace the shadow part with a
valid color format — either convert to the OKLCH token form used elsewhere (e.g.
shadow-[0_0_10px_oklch(var(--primary) / 0.3)]) if you change --primary to an
OKLCH token, or hardcode explicit RGB values (e.g.
shadow-[0_0_10px_rgba(167,249,80,0.3)]) so the CSS parses correctly; update only
the shadow class in the same conditional.
In `@components/bounty-detail/model4-maintainer-dashboard.tsx`:
- Around line 138-151: The footer currently hardcodes "/ 5" which can mismatch
the bounty's actual cap; update the component to accept either a maxSlots prop
or the full bounty object (referencing contributors and the footer span that
renders "Total Winners Allowed") and replace the hardcoded 5 with "{maxSlots ??
bounty?.maxSlots ?? 5}" (or pass maxSlots directly) so the string becomes
"{contributors.length} / {maxSlots ?? 5}"; also update the component props/type
definition to include maxSlots or bounty and ensure callers pass the value.
- Around line 102-131: The action Buttons inside the maintainer dashboard are
inert; add explicit click handlers or mark them disabled with tooltips.
Implement handler functions (e.g., handleMessageClick,
handleViewSubmissionsClick, handleReleasePaymentClick, handleAdvanceClick) and
wire them to the respective Button onClick props (the Button containing
MessageSquare, the "View Submissions" Button, the "Release Payment" Button with
Coins, and the "Advance" Button with ArrowRight); for actions not yet
implemented, set the Button disabled and attach a tooltip like "Coming soon"
instead of leaving them as no-ops. Ensure Release Payment calls the
payment/mutation method or confirmation flow you have (or a stub that clearly
indicates disabled), and keep handlers located near the component so they’re
easy to find and test.
- Around line 49-53: Guard against empty milestones when computing
progressPercentage: ensure milestones.length > 0 before using it, e.g., compute
currentMilestoneIndex from milestones.findIndex(...) and then set
progressPercentage = milestones.length === 0 ? 0 : Math.max(0, Math.min(100,
((currentMilestoneIndex + 1) / milestones.length) * 100))). Update the code
around currentMilestoneIndex, milestones and progressPercentage (used in the
inline width style) to use this guarded-and-clamped value or early-return if no
milestones.
In `@components/bounty/milestone-funnel.tsx`:
- Around line 32-42: The rendering computes style width using `${100 /
milestones.length}%` which becomes Infinity when milestones is empty; add a
defensive guard at the top of the component render (before the milestones.map)
such as returning null or a simple empty/fallback UI when !milestones.length to
avoid division by zero and broken layout; update the component that contains
milestones.map (the function/component referencing milestones and the inline
style) to early-return a safe placeholder if the milestones array is empty.
In `@types/bounty.ts`:
- Around line 5-9: Two hardcoded filter arrays are missing the new union member
MULTI_WINNER_MILESTONE; update the arrays named bountyTypes and BOUNTY_TYPES to
include the string "MULTI_WINNER_MILESTONE" so they match the BountyType union
and the Record<BountyType,...> updates, ensuring the UI filter can select the
new bounty type.
---
Nitpick comments:
In `@components/bounty-detail/bounty-detail-sidebar-cta.tsx`:
- Line 33: There is a duplicate import of Users from "lucide-react"; remove the
separate line importing Users and add Users to the existing multi-line import
from "lucide-react" (i.e., include Users alongside the other icons in the
existing import statement) so all lucide-react icons are consolidated into a
single import.
In `@components/bounty/bounty-header.tsx`:
- Around line 25-29: The TYPE_CONFIG/typeConfig mapping has been duplicated
across components (including the new MULTI_WINNER_MILESTONE entry) causing
inconsistent color schemes; extract a single exported central constant (e.g.,
export const TYPE_CONFIG) in the existing lib configuration module
(lib/bounty-config) and update components (bounty-header.tsx and
github-bounty-card.tsx) to import and use that constant instead of defining
their own mappings; ensure the central mapping includes the new
MULTI_WINNER_MILESTONE, pick the canonical color/class values there, and remove
the local TYPE_CONFIG/typeConfig definitions so all components reference the
single source of truth.
In `@components/bounty/milestone-funnel.tsx`:
- Around line 92-96: The AvatarFallback rendering uses unnecessary optional
chaining and a nullish fallback for a required field; update the JSX in
milestone-funnel's AvatarFallback so it directly uses
contributor.userName.substring(0, 2).toUpperCase() (remove the ?. and ?? "?"),
or if userName can actually be missing, relax the ContributorProgress type to
make userName optional and keep appropriate null handling—target the
contributor.userName expression inside AvatarFallback and adjust either the JSX
or the ContributorProgress type accordingly.
In `@lib/mock-model4.ts`:
- Around line 1-61: The mock data marks milestones "m1" and "m2" as completed in
MOCK_MODEL4_MILESTONES while MOCK_MODEL4_CONTRIBUTORS places live contributors
at those same milestone IDs, causing a visual inconsistency in the
milestone-funnel demo; update the mock so UI looks coherent by either moving
contributors (e.g., change Bob/David currentMilestoneId to an incomplete
milestone like "m3" or "m4") or toggling isCompleted on the corresponding
milestones (MOCK_MODEL4_MILESTONES entries for "m1"/"m2") so contributors are
not shown under milestones rendered as completed by
components/bounty/milestone-funnel.tsx.
In `@types/bounty.ts`:
- Around line 72-84: Milestone.isCompleted should be removed as a global flag
and ContributorProgress should track progress per-user: update the Milestone
interface (remove isCompleted) and change ContributorProgress (currently userId,
userName, userAvatarUrl, currentMilestoneId) to make currentMilestoneId nullable
(string | null) and add a completedMilestoneIds: string[] (or Set<string>) field
so a contributor can represent "not started" (null), per-milestone completions,
and "all done" by comparing completedMilestoneIds to the milestone list; update
any consumers referencing Milestone.isCompleted or assuming non-null
currentMilestoneId to use the new ContributorProgress fields (look for types
Milestone and ContributorProgress in this diff).
🪄 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: 272e4220-bb15-4f76-92fb-825f7b4bfc97
📒 Files selected for processing (10)
components/bounty-detail/bounty-detail-client.tsxcomponents/bounty-detail/bounty-detail-sidebar-cta.tsxcomponents/bounty-detail/milestone-submission-card.tsxcomponents/bounty-detail/model4-maintainer-dashboard.tsxcomponents/bounty/bounty-header.tsxcomponents/bounty/github-bounty-card.tsxcomponents/bounty/milestone-funnel.tsxlib/bounty-config.tslib/mock-model4.tstypes/bounty.ts
| {bounty.type === "MULTI_WINNER_MILESTONE" && ( | ||
| <div className="flex items-center justify-between text-gray-400"> | ||
| <span className="flex items-center gap-1.5"> | ||
| <Users className="size-3.5" /> Slots | ||
| </span> | ||
| <span className="font-medium text-gray-200"> | ||
| {(bounty as unknown as Bounty).totalSlotsOccupied || 0} /{" "} | ||
| {(bounty as unknown as Bounty).maxSlots || 5} | ||
| </span> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether the GraphQL fragment selects multi-winner fields
fd -t f -e graphql -e gql | xargs rg -nC2 'fragment\s+BountyFields|BountyFieldsFragment' || true
fd -t f 'generated' lib/graphql | head
rg -nP '\b(maxSlots|totalSlotsOccupied|milestones|contributorProgress)\b' lib/graphql || trueRepository: boundlessfi/bounties
Length of output: 443
🏁 Script executed:
# Get the full BountyFields fragment definition
cat -n lib/graphql/operations/fragments.graphql | head -100Repository: boundlessfi/bounties
Length of output: 1485
🏁 Script executed:
# Find the BountyFieldsFragment type in generated.ts
rg -n 'type BountyFieldsFragment|interface BountyFieldsFragment' lib/graphql/generated.ts -A 20Repository: boundlessfi/bounties
Length of output: 676
🏁 Script executed:
# Check the bounty-detail-sidebar-cta.tsx file for imports and usage
head -40 components/bounty-detail/bounty-detail-sidebar-cta.tsxRepository: boundlessfi/bounties
Length of output: 1256
🏁 Script executed:
# Search for Bounty type definition in the codebase
fd -t f -e ts -e tsx | xargs rg -n 'type Bounty\s*=|interface Bounty\s*{' | grep -E 'types.*bounty|Bounty\.ts'Repository: boundlessfi/bounties
Length of output: 200
🏁 Script executed:
# Find and read the Bounty type definition
cat types/bounty.tsRepository: boundlessfi/bounties
Length of output: 2465
🏁 Script executed:
# Also check if totalSlotsOccupied or maxSlots are defined anywhere
rg -n 'totalSlotsOccupied|maxSlots' --type ts --type tsxRepository: boundlessfi/bounties
Length of output: 92
🏁 Script executed:
# Read the full bounty-detail-sidebar-cta.tsx file to check ApplicationDialog usage
wc -l components/bounty-detail/bounty-detail-sidebar-cta.tsxRepository: boundlessfi/bounties
Length of output: 123
🏁 Script executed:
# Get lines 130-165 to see ApplicationDialog usage
sed -n '130,165p' components/bounty-detail/bounty-detail-sidebar-cta.tsxRepository: boundlessfi/bounties
Length of output: 1261
🏁 Script executed:
# Also check if there's any ApplicationDialog component to see what onApply does
cat components/bounty/application-dialog.tsx | head -100Repository: boundlessfi/bounties
Length of output: 2584
Slot fields not on BountyFieldsFragment — display will always show defaults, and application handler does not persist submissions.
The bounty prop is typed as BountyFieldsFragment from GraphQL, which does not include totalSlotsOccupied or maxSlots. The as unknown as Bounty cast suppresses the type error but provides no runtime value—these properties remain undefined, causing the slot display to always fall back to 0 / 5 regardless of actual state. You must extend the GraphQL fragment to select these fields and regenerate types.
Additionally, the onApply handler (line 139–142) only logs and returns true without making an API call. The ApplicationDialog will close as if the application succeeded, but nothing is persisted. Implement the actual application submission or clearly indicate the feature is incomplete.
🤖 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 118 -
128, The component is casting the bounty prop to a broader Bounty type to access
totalSlotsOccupied and maxSlots, but those fields aren’t selected on
BountyFieldsFragment so they’re always undefined; update the GraphQL
BountyFieldsFragment to include totalSlotsOccupied and maxSlots, run codegen to
regenerate types, remove the unsafe cast in bounty-detail-sidebar-cta and read
the typed fields directly. Also replace the stubbed onApply handler (the
function used by ApplicationDialog) that only logs and returns true with a real
submission flow: call the backend application API, handle errors (keep dialog
open on failure and show feedback), and on success persist state by refetching
the bounty or applying an optimistic update to totalSlotsOccupied so the UI
reflects the new slot occupancy.
Benjtalkshow
left a comment
Benjtalkshow
left a comment
There was a problem hiding this comment.
Hey @Jopsan-gm, There's a pretty significant pile of work to do before this can ship though.
First two practical things:
CI is failing on the JSR install issue tracked in #187, which has been fixed on main. Please merge main back into your branch and push the result.
Please go through CodeRabbit's corrections carefully and address them, particularly the ones around the unsafe type casts, the stubbed onApply handler, the inert maintainer action buttons, the missing MULTI_WINNER_MILESTONE entries in the bounty type filter arrays, and the milestone state edge cases (findIndex === -1, empty milestones causing Infinity%/NaN% widths). Those are all real bugs, not stylistic notes.
A bigger concern that's worth surfacing in the PR description rather than hidden in the code:
The whole feature is currently rendered against MOCK_MODEL4_MILESTONES and MOCK_MODEL4_CONTRIBUTORS. The GraphQL fragment used by useBountyDetail doesn't select milestones, contributorProgress, maxSlots, or totalSlotsOccupied, so every bounty.field access for those goes through (bounty as unknown as Bounty) and falls back to mocks. That means right now this is a UI prototype against fixtures, not an implementation against the data layer. To turn it into a real feature you'd need the GraphQL fragment extended (and codegen rerun), the backend schema/resolvers updated to expose those fields, and the onApply handler wired to a real mutation. Worth being explicit in the PR description that this is "UI scaffolding only, backend integration follows in a separate PR" if that's the intent, otherwise reviewers will assume it works end-to-end.
The fallback in bounty-detail-client.tsx that shows MOCK_MODEL4_CONTRIBUTORS[0] (Alice) when the signed-in user isn't a contributor is the most user-visible symptom of this. As CodeRabbit points out, any logged-in non-participant currently sees Alice's progress as their own and can click "advance" or submit work as if it's their record. Even as a demo, that needs to be gated behind an actual participation check before merge.
Once main is merged in, CodeRabbit's findings are addressed, and the mock-vs-real-data story is clarified, ping me for another pass.
…ti-winner-milestones
- Merge origin/main to fix JSR install CI failure - Fix security bug: hide MilestoneSubmissionCard from non-participants (removed MOCK_MODEL4_CONTRIBUTORS[0] fallback that exposed Alice) - Add getMilestoneData() adapter to centralise unsafe casts - Fix findIndex === -1 edge case in MilestoneSubmissionCard - Decouple isCompleted from isLocked to prevent badge conflict - Fix invalid CSS shadow rgba(var(--primary),...) -> rgba(167,249,80,...) - Guard MilestoneFunnel against empty milestones (division by zero) - Guard Model4MaintainerDashboard progressPercentage against empty array - Mark all inert dashboard buttons as disabled with 'Coming soon' tooltip - Add maxSlots prop to Model4MaintainerDashboard (remove hardcoded /5) - Fix duplicate Users import in bounty-detail-sidebar-cta.tsx - Fix mock data: move Bob/David off completed milestones (m1/m2) to m3/m4 - Remove unnecessary optional chaining on required ContributorProgress.userName
There was a problem hiding this comment.
♻️ Duplicate comments (2)
components/bounty-detail/milestone-submission-card.tsx (1)
107-119:⚠️ Potential issue | 🟠 Major
View TaskandSubmit Workbuttons are still non-functional.Neither button has an
onClickhandler orhref. A user who reaches their current milestone can click them with no effect. Either wire them through props fromBountyDetailClientor hide them behind a "Coming soon" disabled state to set expectations, consistent with what the dashboard does for inert actions.🛠️ Suggested prop wiring
interface MilestoneSubmissionCardProps { milestones: Milestone[]; contributorProgress: ContributorProgress; className?: string; + onViewTask?: (milestone: Milestone) => void; + onSubmitWork?: (milestone: Milestone) => void; }- <Button - size="sm" - variant="outline" - className="h-8 text-xs border-primary/30 hover:bg-primary/10" - > - <ExternalLink className="size-3 mr-1.5" /> View Task - </Button> - <Button size="sm" className="h-8 text-xs font-bold"> - <Send className="size-3 mr-1.5" /> Submit Work - </Button> + <Button + size="sm" + variant="outline" + className="h-8 text-xs border-primary/30 hover:bg-primary/10" + onClick={() => onViewTask?.(milestone)} + disabled={!onViewTask} + > + <ExternalLink className="size-3 mr-1.5" /> View Task + </Button> + <Button + size="sm" + className="h-8 text-xs font-bold" + onClick={() => onSubmitWork?.(milestone)} + disabled={!onSubmitWork} + > + <Send className="size-3 mr-1.5" /> Submit Work + </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/bounty-detail/milestone-submission-card.tsx` around lines 107 - 119, The View Task and Submit Work buttons rendered when isCurrent is true are non-functional; update the milestone-submission-card component to either wire their actions through props passed from BountyDetailClient (e.g., accept and use onViewTask and onSubmitWork callbacks or a taskUrl prop and call it in the Button onClick / set href) or make them visibly disabled with a consistent "Coming soon" disabled state (set disabled and adjust className) to match dashboard behavior; locate the JSX that renders the Buttons (the Button components containing ExternalLink and Send) and implement the chosen fix so clicks either trigger the provided handlers/URL or the buttons are disabled with appropriate aria-label and styling.components/bounty/milestone-funnel.tsx (1)
50-53:⚠️ Potential issue | 🟡 Minor
rgba(var(--primary), 0.5)won't parse — same issue previously fixed inmilestone-submission-card.tsx.
--primaryinapp/globals.cssis a HEX color (#a7f950), and HEX values cannot be passed intorgba(). The shadow on completed milestones will silently drop. The sibling component already converted to explicit RGB on line 79; mirror that here.🛠️ Proposed fix
milestone.isCompleted - ? "bg-primary border-primary text-primary-foreground shadow-[0_0_15px_rgba(var(--primary),0.5)]" + ? "bg-primary border-primary text-primary-foreground shadow-[0_0_15px_rgba(167,249,80,0.5)]" : "bg-background border-gray-700 text-gray-500 group-hover:border-primary/50",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/bounty/milestone-funnel.tsx` around lines 50 - 53, The shadow color in the completed branch of the class string for milestone.isCompleted uses rgba(var(--primary),0.5), which fails because --primary is a HEX and cannot be fed into rgba(); update the completed-class string in milestone-funnel.tsx (the ternary that builds the class when milestone.isCompleted) to use the explicit RGB(A) value used in milestone-submission-card.tsx (the same RGB equivalent of `#a7f950` with 0.5 alpha) instead of rgba(var(--primary),0.5) so the shadow renders correctly.
🧹 Nitpick comments (1)
components/bounty-detail/bounty-detail-client.tsx (1)
115-157: Recommended: hoistgetMilestoneData(bounty)once per render.
getMilestoneData(bounty)is invoked four times across the three Model 4 sections. Each call returns a fresh object (and identical mock arrays via??), so this is harmless today, but extracting it once also makes the JSX slightly cleaner and avoids redundant casts.♻️ Proposed refactor
+ {bounty.type === "MULTI_WINNER_MILESTONE" && (() => { + const { milestones, contributorProgress } = getMilestoneData(bounty); + const myProgress = session?.user?.id + ? contributorProgress.find((c) => c.userId === session.user.id) + : undefined; + const isMaintainer = session?.user?.id === bounty.createdBy; + return ( + <> + <Card className="border-gray-800 bg-background-card/50 backdrop-blur-sm overflow-hidden"> + <CardHeader className="border-b border-gray-800/50 pb-4"> + <CardTitle className="text-lg font-bold flex items-center gap-2"> + Milestone Funnel + <span className="text-xs font-normal text-muted-foreground bg-primary/10 text-primary px-2 py-0.5 rounded-full"> + Multi-Winner + </span> + </CardTitle> + </CardHeader> + <CardContent className="pt-6"> + <MilestoneFunnel + milestones={milestones} + contributors={contributorProgress} + /> + </CardContent> + </Card> + {myProgress && ( + <MilestoneSubmissionCard + milestones={milestones} + contributorProgress={myProgress} + /> + )} + {isMaintainer && ( + <Model4MaintainerDashboard + milestones={milestones} + contributors={contributorProgress} + /> + )} + </> + ); + })()}Also nice that
MilestoneSubmissionCardis now properly gated on a realmyProgressmatch — the previous Alice-fallback issue is fully resolved.🤖 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 115 - 157, Hoist the repeated call to getMilestoneData(bounty) into a single const (e.g., const { milestones, contributorProgress } = getMilestoneData(bounty)) at the top of this render/JSX block and replace the four inline calls inside MilestoneFunnel, the anonymous IIFE that computes myProgress and MilestoneSubmissionCard, and Model4MaintainerDashboard with those hoisted variables; ensure you still compute myProgress from the hoisted contributorProgress (contributorProgress.find(c => c.userId === session.user.id)) and keep the existing guards (bounty.type === "MULTI_WINNER_MILESTONE" and session?.user?.id checks) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@components/bounty-detail/milestone-submission-card.tsx`:
- Around line 107-119: The View Task and Submit Work buttons rendered when
isCurrent is true are non-functional; update the milestone-submission-card
component to either wire their actions through props passed from
BountyDetailClient (e.g., accept and use onViewTask and onSubmitWork callbacks
or a taskUrl prop and call it in the Button onClick / set href) or make them
visibly disabled with a consistent "Coming soon" disabled state (set disabled
and adjust className) to match dashboard behavior; locate the JSX that renders
the Buttons (the Button components containing ExternalLink and Send) and
implement the chosen fix so clicks either trigger the provided handlers/URL or
the buttons are disabled with appropriate aria-label and styling.
In `@components/bounty/milestone-funnel.tsx`:
- Around line 50-53: The shadow color in the completed branch of the class
string for milestone.isCompleted uses rgba(var(--primary),0.5), which fails
because --primary is a HEX and cannot be fed into rgba(); update the
completed-class string in milestone-funnel.tsx (the ternary that builds the
class when milestone.isCompleted) to use the explicit RGB(A) value used in
milestone-submission-card.tsx (the same RGB equivalent of `#a7f950` with 0.5
alpha) instead of rgba(var(--primary),0.5) so the shadow renders correctly.
---
Nitpick comments:
In `@components/bounty-detail/bounty-detail-client.tsx`:
- Around line 115-157: Hoist the repeated call to getMilestoneData(bounty) into
a single const (e.g., const { milestones, contributorProgress } =
getMilestoneData(bounty)) at the top of this render/JSX block and replace the
four inline calls inside MilestoneFunnel, the anonymous IIFE that computes
myProgress and MilestoneSubmissionCard, and Model4MaintainerDashboard with those
hoisted variables; ensure you still compute myProgress from the hoisted
contributorProgress (contributorProgress.find(c => c.userId ===
session.user.id)) and keep the existing guards (bounty.type ===
"MULTI_WINNER_MILESTONE" and session?.user?.id checks) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0d784ec7-9e58-47f9-9310-0a51be7486a5
📒 Files selected for processing (6)
components/bounty-detail/bounty-detail-client.tsxcomponents/bounty-detail/bounty-detail-sidebar-cta.tsxcomponents/bounty-detail/milestone-submission-card.tsxcomponents/bounty-detail/model4-maintainer-dashboard.tsxcomponents/bounty/milestone-funnel.tsxlib/mock-model4.ts
✅ Files skipped from review due to trivial changes (1)
- lib/mock-model4.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- components/bounty-detail/model4-maintainer-dashboard.tsx
- components/bounty-detail/bounty-detail-sidebar-cta.tsx
|
Real progress on this round @Jopsan-gm . The
The type adapter you introduced in Once those land, this is good to go. |
…ulti-winner-milestones
- bounty-detail-sidebar-cta: replace all 6 'as unknown as Bounty' casts with typed BountyFieldsFragment & Partial<Bounty> prop; compute occupied/max once per block to avoid repeated inline casts; add TODO comment on onApply stub. - use-bounty-detail: widen return type to include Partial<Bounty> so callers can access optional Model-4 fields without unsafe assertions. - bounty-detail-client: call getMilestoneData once per MULTI_WINNER block instead of twice; remove now-redundant local Bounty import; update getMilestoneData signature to use the typed hook return type. - milestone-submission-card: guard against empty milestones array (avoids rendering '0 of 0' and potential stale-index crashes); add loading state to Submit Work button with visible spinner; add TODO on handleSubmitWork stub so it is not silently inert. Addresses CodeRabbit feedback on PR boundlessfi#186.
There was a problem hiding this comment.
🧹 Nitpick comments (5)
hooks/use-bounty-detail.ts (1)
19-24: LGTM — interim type-broadening is reasonable.The intersection with
Partial<Bounty>lets the new Model-4 fields flow withoutas unknown as Bountycasts at the callsites, and the doc comment correctly notes that real values come fromgetMilestoneDatamock fallbacks until the GraphQL fragment is extended.One small nit:
Partial<BountyQuery["bounty"]>is likely redundant withBountyFieldsFragment(the GraphQL fragment is whatBountyQuery["bounty"]resolves to). It can be dropped in a follow-up cleanup once the fragment is extended to cover the Model-4 fields, removing the need forPartial<Bounty>entirely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-bounty-detail.ts` around lines 19 - 24, The type union for the returned data includes a redundant Partial type; remove Partial<BountyQuery["bounty"]> from the intersection so the type is just BountyFieldsFragment & Partial<Bounty> | undefined (update the code at the declaration using data: data?.bounty to drop Partial<BountyQuery["bounty"]>), keeping the existing doc comment and leaving Partial<Bounty> in place until the GraphQL fragment (BountyFieldsFragment) is extended in a later cleanup.components/bounty-detail/milestone-submission-card.tsx (2)
41-48: Drop theconsole.logand guard against unmount during the simulated delay.Two small nits in
handleSubmitWork:
- The
console.log("[Coming soon] …")will fire on every click in production. Either remove it or gate behindprocess.env.NODE_ENV !== "production".- If the component unmounts during the 800ms
setTimeout,setIsSubmitting(false)will run on a stale instance. A simplemountedRef(orAbortController) guard avoids that — not critical for demo data, but trivial to add now and prevents a surprise once this is wired to a real mutation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/bounty-detail/milestone-submission-card.tsx` around lines 41 - 48, Remove the unconditional console.log from handleSubmitWork (or wrap it with a NODE_ENV check: process.env.NODE_ENV !== "production") and add a mounted guard so setIsSubmitting(false) is only called if the component is still mounted; create a mountedRef (set true on mount, false in cleanup) or use an AbortController, check mountedRef.current (or controller.signal) after the await new Promise(...) and before calling setIsSubmitting(false) so you avoid updating state on an unmounted instance.
134-140: Prefer an anchor orscrollIntoViewoverwindow.openfor in-page hash navigation.
window.open(#milestone-${id}, "_self")works but is an unusual idiom for jumping to an anchor on the same page — it triggers a full window navigation and some browsers/extensions treatwindow.opencalls suspiciously. A plain anchor (<a href={#milestone-${milestone.id}}>) styled as a button, ordocument.getElementById(...)?.scrollIntoView({ behavior: "smooth" }), is more idiomatic, keyboard/screen-reader friendly, and avoids the_selfsemantics.♻️ Suggested refactor
- <Button - size="sm" - variant="outline" - className="h-8 text-xs border-primary/30 hover:bg-primary/10" - onClick={() => - milestone.id && - window.open(`#milestone-${milestone.id}`, "_self") - } - > - <ExternalLink className="size-3 mr-1.5" /> View Task - </Button> + <Button + asChild + size="sm" + variant="outline" + className="h-8 text-xs border-primary/30 hover:bg-primary/10" + > + <a href={`#milestone-${milestone.id}`}> + <ExternalLink className="size-3 mr-1.5" /> View Task + </a> + </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/bounty-detail/milestone-submission-card.tsx` around lines 134 - 140, Replace the in-page navigation that uses window.open(`#milestone-${milestone.id}`, "_self") in the Button's onClick handler with an accessible anchor or a smooth scroll; specifically, change the Button (rendering ExternalLink + "View Task") to either wrap it in an <a href={`#milestone-${milestone.id}`}> so native hash navigation is used, or keep the Button but replace the onClick with document.getElementById(`milestone-${milestone.id}`)?.scrollIntoView({ behavior: "smooth" }); ensure you reference milestone.id and the Button/ExternalLink rendering so the update preserves styling and accessibility.components/bounty-detail/bounty-detail-client.tsx (2)
32-45: Adapter looks good; consider importingMilestone/ContributorProgressdirectly.The
getMilestoneDataadapter is exactly the centralization the previous review asked for — nice. Minor nit: the inlineimport("@/types/bounty").Milestone/ContributorProgressand theReturnType<(typeof import("@/hooks/use-bounty-detail"))["useBountyDetail"]>["data"]types are quite verbose. A top-levelimport type { Milestone, ContributorProgress } from "@/types/bounty"and a small alias for the bounty data type would read much more cleanly without changing behavior.🤖 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 32 - 45, Refactor getMilestoneData to use top-level type imports instead of inline imports: add a top-level "import type { Milestone, ContributorProgress } from '@/types/bounty'" and create a small alias for the hook return data (e.g., type BountyData = ReturnType<typeof import('@/hooks/use-bounty-detail')['useBountyDetail']>['data']) then change the function signature to accept "bounty: BountyData" and return "milestones: Milestone[]" and "contributorProgress: ContributorProgress[]"; update references inside getMilestoneData to use the new types and remove the verbose inline import types.
119-172: HoistgetMilestoneData(bounty)once for the Model-4 branch.The same
getMilestoneData(bounty)call is repeated inside three separate IIFEs, each gated onbounty.type === "MULTI_WINNER_MILESTONE". Computing it once in a singleifbranch would (a) avoid three calls per render, (b) flatten the IIFEs into straightforward JSX, and (c) make the gating intent more obvious.♻️ Suggested refactor
- {bounty.type === "MULTI_WINNER_MILESTONE" && - (() => { - const { milestones, contributorProgress } = - getMilestoneData(bounty); - return ( - <Card className="..."> - ... - <MilestoneFunnel - milestones={milestones} - contributors={contributorProgress} - /> - ... - </Card> - ); - })()} - - {bounty.type === "MULTI_WINNER_MILESTONE" && - session?.user?.id && - (() => { - const { milestones, contributorProgress } = - getMilestoneData(bounty); - const myProgress = contributorProgress.find( - (c) => c.userId === session.user.id, - ); - if (!myProgress) return null; - return ( - <MilestoneSubmissionCard - milestones={milestones} - contributorProgress={myProgress} - /> - ); - })()} - - {bounty.type === "MULTI_WINNER_MILESTONE" && - session?.user?.id === bounty.createdBy && - (() => { - const { milestones, contributorProgress } = - getMilestoneData(bounty); - return ( - <Model4MaintainerDashboard - milestones={milestones} - contributors={contributorProgress} - /> - ); - })()} + {bounty.type === "MULTI_WINNER_MILESTONE" && (() => { + const { milestones, contributorProgress } = getMilestoneData(bounty); + const myProgress = session?.user?.id + ? contributorProgress.find((c) => c.userId === session.user.id) + : undefined; + const isMaintainer = session?.user?.id === bounty.createdBy; + return ( + <> + <Card className="..."> + ... + <MilestoneFunnel + milestones={milestones} + contributors={contributorProgress} + /> + ... + </Card> + {myProgress && ( + <MilestoneSubmissionCard + milestones={milestones} + contributorProgress={myProgress} + /> + )} + {isMaintainer && ( + <Model4MaintainerDashboard + milestones={milestones} + contributors={contributorProgress} + /> + )} + </> + ); + })()}🤖 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 119 - 172, Multiple IIFEs call getMilestoneData(bounty) three times when bounty.type === "MULTI_WINNER_MILESTONE"; hoist a single const { milestones, contributorProgress } = getMilestoneData(bounty) inside one surrounding if (bounty.type === "MULTI_WINNER_MILESTONE") block and then render the three components (MilestoneFunnel, MilestoneSubmissionCard, Model4MaintainerDashboard) using that hoisted data, keeping the existing additional guards (session?.user?.id for MilestoneSubmissionCard and session?.user?.id === bounty.createdBy for Model4MaintainerDashboard) and returning null where appropriate.
🤖 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 32-45: Refactor getMilestoneData to use top-level type imports
instead of inline imports: add a top-level "import type { Milestone,
ContributorProgress } from '@/types/bounty'" and create a small alias for the
hook return data (e.g., type BountyData = ReturnType<typeof
import('@/hooks/use-bounty-detail')['useBountyDetail']>['data']) then change the
function signature to accept "bounty: BountyData" and return "milestones:
Milestone[]" and "contributorProgress: ContributorProgress[]"; update references
inside getMilestoneData to use the new types and remove the verbose inline
import types.
- Around line 119-172: Multiple IIFEs call getMilestoneData(bounty) three times
when bounty.type === "MULTI_WINNER_MILESTONE"; hoist a single const {
milestones, contributorProgress } = getMilestoneData(bounty) inside one
surrounding if (bounty.type === "MULTI_WINNER_MILESTONE") block and then render
the three components (MilestoneFunnel, MilestoneSubmissionCard,
Model4MaintainerDashboard) using that hoisted data, keeping the existing
additional guards (session?.user?.id for MilestoneSubmissionCard and
session?.user?.id === bounty.createdBy for Model4MaintainerDashboard) and
returning null where appropriate.
In `@components/bounty-detail/milestone-submission-card.tsx`:
- Around line 41-48: Remove the unconditional console.log from handleSubmitWork
(or wrap it with a NODE_ENV check: process.env.NODE_ENV !== "production") and
add a mounted guard so setIsSubmitting(false) is only called if the component is
still mounted; create a mountedRef (set true on mount, false in cleanup) or use
an AbortController, check mountedRef.current (or controller.signal) after the
await new Promise(...) and before calling setIsSubmitting(false) so you avoid
updating state on an unmounted instance.
- Around line 134-140: Replace the in-page navigation that uses
window.open(`#milestone-${milestone.id}`, "_self") in the Button's onClick
handler with an accessible anchor or a smooth scroll; specifically, change the
Button (rendering ExternalLink + "View Task") to either wrap it in an <a
href={`#milestone-${milestone.id}`}> so native hash navigation is used, or keep
the Button but replace the onClick with
document.getElementById(`milestone-${milestone.id}`)?.scrollIntoView({ behavior:
"smooth" }); ensure you reference milestone.id and the Button/ExternalLink
rendering so the update preserves styling and accessibility.
In `@hooks/use-bounty-detail.ts`:
- Around line 19-24: The type union for the returned data includes a redundant
Partial type; remove Partial<BountyQuery["bounty"]> from the intersection so the
type is just BountyFieldsFragment & Partial<Bounty> | undefined (update the code
at the declaration using data: data?.bounty to drop
Partial<BountyQuery["bounty"]>), keeping the existing doc comment and leaving
Partial<Bounty> in place until the GraphQL fragment (BountyFieldsFragment) is
extended in a later cleanup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d5142fab-89a7-4aa1-a707-8ff8a305421e
📒 Files selected for processing (4)
components/bounty-detail/bounty-detail-client.tsxcomponents/bounty-detail/bounty-detail-sidebar-cta.tsxcomponents/bounty-detail/milestone-submission-card.tsxhooks/use-bounty-detail.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- components/bounty-detail/bounty-detail-sidebar-cta.tsx
…leak
- use-bounty-filters: add MULTI_WINNER_MILESTONE entry to BOUNTY_TYPES
array so the filter UI can surface this bounty type. Omitting it made
all MULTI_WINNER_MILESTONE bounties invisible to the type filter.
- bounty-detail-client: split getMilestoneData into three focused helpers:
* getMilestones() — milestones with mock fallback, safe for public display
* getRealContributors() — ONLY real API data (no mock fallback), used by
the public MilestoneFunnel to prevent Alice/Bob/Charlie from appearing
to unauthenticated or unrelated visitors.
* getFullMilestoneData() — full data with mock fallback, used only in
authenticated sections (contributor progress card, maintainer dashboard)
where mocks are acceptable during prototyping.
- use-bounty-filters.test: add MULTI_WINNER_MILESTONE fixture bounty and
a dedicated toggleType test. Update all assertions affected by the new
fixture (counts, sort orders, org and reward range filters). All 22 tests
pass.
Addresses maintainer feedback on PR boundlessfi#186 (CodeRabbit + privacy concern).
…tivity - Filters: Add MULTI_WINNER_MILESTONE to discovery filters (Discover and Projects). - Dashboard: Enable Advance, Release Payment, and Remove actions with mock handlers and toast feedback. - Privacy: Prevent mock data leakage to unauthenticated users. - DX: Refactor complex type aliases for better maintainability. Passed TSC and fixed ESLint 'any' violations.
Benjtalkshow
left a comment
Benjtalkshow
left a comment
There was a problem hiding this comment.
Big improvement on this round. Two of the three are done cleanly: the filter arrays now include MULTI_WINNER_MILESTONE, and the unsafe casts in the sidebar are replaced with the typed intersection. Nice work.
The one still open is onApply in bounty-detail-sidebar-cta.tsx:154. Renaming the log to [Mock] doesn't fix the issue. The dialog still shows toast.success("Application submitted!") for a submission that never persists, which is exactly the trap I flagged. You used the [Coming soon] pattern for Submit Work in milestone-submission-card.tsx — please apply the same approach here: drop the toast, or disable the button with a Coming soon label until the mutation exists.
Once that's swapped out, this is good to merge.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (3)
components/projects/project-bounties.tsx (1)
17-29:⚠️ Potential issue | 🔴 CriticalSame unsafe cast / invalid enum value as
app/bounty/page.tsx— see root-cause comment there.
selectedTypeends up holding"MULTI_WINNER_MILESTONE"(typed asBountyTypevia the cast) and is sent to the backend throughparams.typeon Line 53. If the GraphQLBountyTypeenum lacks this value, the type filter will not work for Multi-Winner bounties on project pages either. Fix the codegen enum once and remove the cast in both files.Also applies to: 98-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/projects/project-bounties.tsx` around lines 17 - 29, The bountyTypes array uses an unsafe cast for "MULTI_WINNER_MILESTONE" which forces a string into BountyType (see constant bountyTypes and where selectedType/params.type is set), causing invalid enum values to be sent to the backend; fix the root cause by updating the generated BountyType enum to include MULTI_WINNER_MILESTONE (regenerate codegen) and then remove the cast in components/projects/project-bounties.tsx (and the duplicate site at lines ~98-100) so the array uses the real BountyType.Multipler/appropriate enum member instead of the string literal.components/bounty-detail/model4-maintainer-dashboard.tsx (1)
130-234:⚠️ Potential issue | 🟡 MinorInline action buttons aren't visibly marked "Coming soon".
The footer's "View All Applications" button is correctly
disabledwith a "Coming soon" tooltip — but the per-contributor actions (Message, View Submissions, Release Payment, Advance, Remove) only log[Coming soon] …to the console while showing a 1-second spinner. To a maintainer, "Release Payment" appearing to "process" looks indistinguishable from a real backend call, which is the exact UX trap raised in earlier review feedback. Either disable these buttons with the same Tooltip pattern, or show an inline toast like "Coming soon — backend not yet wired" so the no-op is unmistakable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/bounty-detail/model4-maintainer-dashboard.tsx` around lines 130 - 234, The per-contributor action buttons (Message, View Submissions, Release Payment, Advance, Remove) currently call handleAction and set loadingAction causing a spinner for a fake 1s "Coming soon" flow; change them to the same UX as the footer by disabling the buttons and wrapping them with a Tooltip/TooltipContent that reads "Coming soon — backend not yet wired" (or, alternatively, make their onClick call a no-op that triggers a toast message instead of setting loadingAction). Update the Button props and Tooltip/TooltipContent around the buttons (references: handleAction, loadingAction, Button, Tooltip, TooltipContent, Loader2) so no spinner appears and the maintainer clearly sees the action is disabled/upcoming.components/bounty-detail/bounty-detail-sidebar-cta.tsx (1)
145-170:⚠️ Potential issue | 🟠 Major"Coming soon" CTA still opens a working application form that fakes success.
The label communicates "Coming soon", but the button is not disabled — clicking it opens
ApplicationDialog, wherehandleSubmit(incomponents/bounty/application-dialog.tsx:72-86) awaitsonApply, seestrue, then closes the dialog and resets the form. From the user's perspective the application appeared to succeed, and they get no indication that nothing was persisted. This is the same silent no-op UX flagged in the prior review and does not match the maintainer dashboard's pattern (trulydisabledbutton + tooltip).Either disable the trigger and rely on a tooltip (mirroring
Model4MaintainerDashboard's "View All Applications"), or keep the dialog but haveonApplyreturnfalseand surface an inline "Submission API not yet available" message so the dialog stays open and the user is not misled.🛡️ Proposed fix — disable the trigger to match the maintainer pattern
- ) : bounty.type === "MULTI_WINNER_MILESTONE" ? ( - (() => { - const occupied = bounty.totalSlotsOccupied ?? 0; - const max = bounty.maxSlots ?? 5; - const isFull = occupied >= max; - return ( - <ApplicationDialog - bountyTitle={bounty.title} - onApply={async (data) => { - // Mock application delay - await new Promise((resolve) => setTimeout(resolve, 1500)); - console.log("[Coming soon] Applying for slot:", data); - return true; - }} - trigger={ - <Button - className="w-full h-11 font-bold tracking-wide" - disabled={!canAct || isFull} - size="lg" - > - {isFull ? "Slots Full" : "Apply for Slot [Coming soon]"} - </Button> - } - /> - ); - })() + ) : bounty.type === "MULTI_WINNER_MILESTONE" ? ( + (() => { + const occupied = bounty.totalSlotsOccupied ?? 0; + const max = bounty.maxSlots ?? 5; + const isFull = occupied >= max; + return ( + <TooltipProvider> + <Tooltip> + <TooltipTrigger asChild> + <span className="block"> + <Button + className="w-full h-11 font-bold tracking-wide" + disabled + size="lg" + > + {isFull ? "Slots Full" : "Apply for Slot"} + </Button> + </span> + </TooltipTrigger> + <TooltipContent>Coming soon</TooltipContent> + </Tooltip> + </TooltipProvider> + ); + })()🤖 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 145 - 170, The "Coming soon" CTA currently opens ApplicationDialog and fakes success; update the MULTI_WINNER_MILESTONE branch so the trigger does not allow a successful submission: either (preferred) make the Button truly disabled and wrap it with the same Tooltip pattern used in Model4MaintainerDashboard's "View All Applications" (so clicking cannot open ApplicationDialog), or keep the dialog but change the supplied onApply to immediately return false and surface an inline "Submission API not yet available" error (so handleSubmit in components/bounty/application-dialog.tsx will keep the dialog open). Target the ApplicationDialog trigger/Button in bounty-detail-sidebar-cta.tsx and, if choosing the dialog approach, update the onApply behavior and ApplicationDialog's handleSubmit to display the inline error instead of treating true as success.
🧹 Nitpick comments (4)
app/bounty/page.tsx (1)
110-115: Consolidate the widened-union + cast into a single helper to avoid drift.Same
BountyType | "MULTI_WINNER_MILESTONE"pattern is repeated incomponents/projects/project-bounties.tsx. Once the enum is fixed (see comment above), both call sites can drop the cast. Until then, consider extracting the union type and a smalltoFilterType()helper to one module so the workaround is auditable in one place — this echoes the reviewer's earlier feedback about consolidating(bounty as unknown as Bounty)casts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/bounty/page.tsx` around lines 110 - 115, The toggleType function uses the widened union BountyType | "MULTI_WINNER_MILESTONE" and an inline cast—extract that workaround into a single helper module: define the union alias (e.g., FilterableBountyType = BountyType | "MULTI_WINNER_MILESTONE") and a toFilterType(value) helper that returns the correct BountyType or "all", then replace the inline cast in toggleType and the other call site in components/projects/project-bounties.tsx to call toFilterType and pass its result into setSelectedType (keep setPage(1) behavior). This centralizes the cast/workaround so the code can drop it once the enum is fixed.components/bounty-detail/model4-maintainer-dashboard.tsx (1)
218-233: Destructive "Remove" lacks a confirmation step.When this is wired to a real mutation, a single click on the trash-style
UserMinusbutton will drop a contributor from a slot and (presumably) trigger reassignment/escrow side-effects. Worth pre-emptively gating it behind anAlertDialog(mirroring the cancel-bounty pattern inbounty-detail-sidebar-cta.tsx) before it leaves stub state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/bounty-detail/model4-maintainer-dashboard.tsx` around lines 218 - 233, The Remove action is destructive and needs a confirmation dialog before calling handleAction("Remove", ...); wrap the Button (the UserMinus icon/ Button component currently using onClick) inside an AlertDialog flow similar to the cancel-bounty pattern: use AlertDialog, AlertDialogTrigger (asChild) for the Button, show AlertDialogContent with a clear title, description and two actions (Cancel and Confirm Remove), and call handleAction("Remove", contributor.userName) only from the Confirm Remove button; ensure the disabled/ loading state (loadingAction) is applied to the Confirm button as well.components/bounty-detail/bounty-detail-sidebar-cta.tsx (1)
123-148: Repeated hardcoded?? 5default formaxSlots.
bounty.maxSlots ?? 5is duplicated at lines 126 and 148, and again atmodel4-maintainer-dashboard.tsx(default param). Consider extracting aDEFAULT_MAX_SLOTSconstant (or computing{ occupied, max, isFull }once at the top ofSidebarCTA) so the fallback is defined in one place and the slots row + CTA never disagree.🤖 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 123 - 148, Extract the duplicated fallback logic for bounty.maxSlots into a single source of truth: define a DEFAULT_MAX_SLOTS constant and/or compute { occupied, max, isFull } once at the top of the SidebarCTA component (use bounty.totalSlotsOccupied and bounty.maxSlots to derive these), then replace all occurrences of bounty.maxSlots ?? 5 with the computed max or DEFAULT_MAX_SLOTS and use isFull for CTA branching; ensure both the Slots row and the CTA use the same derived values so they never disagree (refer to symbols bounty.maxSlots, bounty.totalSlotsOccupied, FcfsClaimButton, and the MULTI_WINNER_MILESTONE conditional).components/bounty-detail/bounty-detail-client.tsx (1)
30-60: Adapter split looks good; consider tightening theBountyDatatype for clarity.The three adapters cleanly separate "structural milestones" (mock OK), "real contributors only" (public funnel), and "full data with mocks" (auth-gated sections). While
useBountyDetailalready explicitly intersects withPartial<Bounty>to surfacemilestonesandcontributorProgress, usingBountyData = ReturnType<typeof useBountyDetail>["data"]still couples these adapters to the full hook shape. A narrower type likePartial<Pick<Bounty, "milestones" | "contributorProgress">>would make the contract more explicit and easier to maintain if the hook evolves.🤖 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 30 - 60, Replace the broad BountyData alias with a narrower explicit type to decouple these adapters from the full hook shape: change BountyData to Partial<Pick<Bounty, "milestones" | "contributorProgress">> and update the parameter types for getMilestones, getRealContributors, and getFullMilestoneData to use that new type; ensure the Bounty type is imported/available where this file defines BountyData so the functions still return Milestone[] and ContributorProgress[] as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/bounty/page.tsx`:
- Around line 38-49: The BOUNTY_TYPES entry is using an unsafe cast
("MULTI_WINNER_MILESTONE" as unknown as BountyType) which will send an invalid
enum via queryParams.type to the backend; fix by adding MULTI_WINNER_MILESTONE
to the GraphQL schema and regenerating the TypeScript types so you can use
BountyType.MultiWinnerMilestone in BOUNTY_TYPES (replace the cast), then ensure
any usage that sets queryParams.type (where BountyType is propagated) uses the
real enum value; if you cannot update the schema right away, gate the UI filter
so selecting "Multi-Winner Milestone" is handled client-side (does not set
queryParams.type) and add a TODO comment referencing the schema/types regen
until the enum is added.
In `@app/leaderboard/page.tsx`:
- Around line 48-50: Replace the hardcoded currentUserId variable with the
actual session-derived user id by calling authClient.useSession() in this
component (same pattern used in bounty-detail-client.tsx and
fcfs-claim-button.tsx); extract the user id from the returned session (e.g.,
session.user.id or equivalent) and pass that value into LeaderboardTable and
UserRankSidebar instead of the static "user-1", handling the case where session
is null/undefined (fall back to undefined/null so components can render a
non-personalized view).
- Around line 137-139: The leaderboard's onRowClick handler currently calls
router.push with a non-existent `/user/${entry.contributor.userId}` route;
update the navigation to use the existing profile route by changing the
router.push target to `/profile/${entry.contributor.userId}` (locate the
onRowClick callback and the router.push call referencing
entry.contributor.userId) so clicks navigate to the implemented
/profile/[userId] page.
In `@components/bounty-detail/bounty-detail-client.tsx`:
- Around line 173-184: getFullMilestoneData currently falls back to
MOCK_MODEL4_CONTRIBUTORS and causes Model4MaintainerDashboard to show fake
contributors (and actions) when bounty.contributorProgress is empty; update the
rendering logic so that when contributorProgress is derived from the
MOCK_MODEL4_CONTRIBUTORS you either: (a) do not render Model4MaintainerDashboard
for real environments unless an explicit demo flag is enabled (add and check a
feature flag like isDemoMode or process.env.SHOW_DEMO_CONTRIBUTORS), or (b)
render the dashboard but pass an explicit demo prop and surface a prominent
"Demo data" banner inside Model4MaintainerDashboard; locate
getFullMilestoneData, MOCK_MODEL4_CONTRIBUTORS and the Model4MaintainerDashboard
call and implement the gate or banner to prevent real creators from acting on
mock contributors.
In `@components/bounty-detail/model4-maintainer-dashboard.tsx`:
- Around line 38-45: The loadingAction state key currently uses
`${action}-${userName}` which can collide for contributors sharing the same
display name; update the key generation in handleAction to use the unique
contributor.userId instead (e.g., `${action}-${contributor.userId}`) and change
all call sites that invoke handleAction to pass contributor.userId rather than
userName, then update the matched-spinner comparisons (the checks that compare
loadingAction to those keys) to compare against the
`${action}-${contributor.userId}` form so each row’s spinner is driven by the
unique userId.
In `@hooks/__tests__/use-bounty-filters.test.ts`:
- Around line 93-121: The test fixture sets the BountyFieldsFragment's type to
"MULTI_WINNER_MILESTONE", but the generated BountyType enum (from
lib/graphql/generated.ts) only contains COMPETITION, FIXED_PRICE, and
MILESTONE_BASED causing TS errors; fix by changing the fixture's type value in
the failing test to one of the existing enum values (e.g., "MILESTONE_BASED")
or, if the MULTI_WINNER_MILESTONE value is required, add that value to your
GraphQL schema and re-run codegen to regenerate BountyType so the new enum
includes MULTI_WINNER_MILESTONE.
---
Duplicate comments:
In `@components/bounty-detail/bounty-detail-sidebar-cta.tsx`:
- Around line 145-170: The "Coming soon" CTA currently opens ApplicationDialog
and fakes success; update the MULTI_WINNER_MILESTONE branch so the trigger does
not allow a successful submission: either (preferred) make the Button truly
disabled and wrap it with the same Tooltip pattern used in
Model4MaintainerDashboard's "View All Applications" (so clicking cannot open
ApplicationDialog), or keep the dialog but change the supplied onApply to
immediately return false and surface an inline "Submission API not yet
available" error (so handleSubmit in components/bounty/application-dialog.tsx
will keep the dialog open). Target the ApplicationDialog trigger/Button in
bounty-detail-sidebar-cta.tsx and, if choosing the dialog approach, update the
onApply behavior and ApplicationDialog's handleSubmit to display the inline
error instead of treating true as success.
In `@components/bounty-detail/model4-maintainer-dashboard.tsx`:
- Around line 130-234: The per-contributor action buttons (Message, View
Submissions, Release Payment, Advance, Remove) currently call handleAction and
set loadingAction causing a spinner for a fake 1s "Coming soon" flow; change
them to the same UX as the footer by disabling the buttons and wrapping them
with a Tooltip/TooltipContent that reads "Coming soon — backend not yet wired"
(or, alternatively, make their onClick call a no-op that triggers a toast
message instead of setting loadingAction). Update the Button props and
Tooltip/TooltipContent around the buttons (references: handleAction,
loadingAction, Button, Tooltip, TooltipContent, Loader2) so no spinner appears
and the maintainer clearly sees the action is disabled/upcoming.
In `@components/projects/project-bounties.tsx`:
- Around line 17-29: The bountyTypes array uses an unsafe cast for
"MULTI_WINNER_MILESTONE" which forces a string into BountyType (see constant
bountyTypes and where selectedType/params.type is set), causing invalid enum
values to be sent to the backend; fix the root cause by updating the generated
BountyType enum to include MULTI_WINNER_MILESTONE (regenerate codegen) and then
remove the cast in components/projects/project-bounties.tsx (and the duplicate
site at lines ~98-100) so the array uses the real
BountyType.Multipler/appropriate enum member instead of the string literal.
---
Nitpick comments:
In `@app/bounty/page.tsx`:
- Around line 110-115: The toggleType function uses the widened union BountyType
| "MULTI_WINNER_MILESTONE" and an inline cast—extract that workaround into a
single helper module: define the union alias (e.g., FilterableBountyType =
BountyType | "MULTI_WINNER_MILESTONE") and a toFilterType(value) helper that
returns the correct BountyType or "all", then replace the inline cast in
toggleType and the other call site in components/projects/project-bounties.tsx
to call toFilterType and pass its result into setSelectedType (keep setPage(1)
behavior). This centralizes the cast/workaround so the code can drop it once the
enum is fixed.
In `@components/bounty-detail/bounty-detail-client.tsx`:
- Around line 30-60: Replace the broad BountyData alias with a narrower explicit
type to decouple these adapters from the full hook shape: change BountyData to
Partial<Pick<Bounty, "milestones" | "contributorProgress">> and update the
parameter types for getMilestones, getRealContributors, and getFullMilestoneData
to use that new type; ensure the Bounty type is imported/available where this
file defines BountyData so the functions still return Milestone[] and
ContributorProgress[] as before.
In `@components/bounty-detail/bounty-detail-sidebar-cta.tsx`:
- Around line 123-148: Extract the duplicated fallback logic for bounty.maxSlots
into a single source of truth: define a DEFAULT_MAX_SLOTS constant and/or
compute { occupied, max, isFull } once at the top of the SidebarCTA component
(use bounty.totalSlotsOccupied and bounty.maxSlots to derive these), then
replace all occurrences of bounty.maxSlots ?? 5 with the computed max or
DEFAULT_MAX_SLOTS and use isFull for CTA branching; ensure both the Slots row
and the CTA use the same derived values so they never disagree (refer to symbols
bounty.maxSlots, bounty.totalSlotsOccupied, FcfsClaimButton, and the
MULTI_WINNER_MILESTONE conditional).
In `@components/bounty-detail/model4-maintainer-dashboard.tsx`:
- Around line 218-233: The Remove action is destructive and needs a confirmation
dialog before calling handleAction("Remove", ...); wrap the Button (the
UserMinus icon/ Button component currently using onClick) inside an AlertDialog
flow similar to the cancel-bounty pattern: use AlertDialog, AlertDialogTrigger
(asChild) for the Button, show AlertDialogContent with a clear title,
description and two actions (Cancel and Confirm Remove), and call
handleAction("Remove", contributor.userName) only from the Confirm Remove
button; ensure the disabled/ loading state (loadingAction) is applied to the
Confirm button as well.
🪄 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: 9f8c0bef-49fd-4939-90c3-9e0753dfedb6
📒 Files selected for processing (8)
app/bounty/page.tsxapp/leaderboard/page.tsxcomponents/bounty-detail/bounty-detail-client.tsxcomponents/bounty-detail/bounty-detail-sidebar-cta.tsxcomponents/bounty-detail/model4-maintainer-dashboard.tsxcomponents/projects/project-bounties.tsxhooks/__tests__/use-bounty-filters.test.tshooks/use-bounty-filters.ts
✅ Files skipped from review due to trivial changes (1)
- hooks/use-bounty-filters.ts
| const BOUNTY_TYPES: { | ||
| value: BountyType | "MULTI_WINNER_MILESTONE"; | ||
| label: string; | ||
| }[] = [ | ||
| { value: BountyType.FixedPrice, label: "Fixed Price" }, | ||
| { value: BountyType.MilestoneBased, label: "Milestone Based" }, | ||
| { value: BountyType.Competition, label: "Competition" }, | ||
| { | ||
| value: "MULTI_WINNER_MILESTONE" as unknown as BountyType, | ||
| label: "Multi-Winner Milestone", | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether MULTI_WINNER_MILESTONE is part of the generated GraphQL BountyType enum.
fd -t f 'generated' -e ts -e tsx | head -50
rg -nP -C3 '\benum\s+BountyType\b|BountyType\s*=\s*\{' --type=ts
rg -nP -C2 'MultiWinnerMilestone|MULTI_WINNER_MILESTONE' --type=ts --type=graphql -g '!**/__tests__/**'Repository: boundlessfi/bounties
Length of output: 515
🏁 Script executed:
# Get the complete BountyType enum definition
sed -n '789,810p' lib/graphql/generated.tsRepository: boundlessfi/bounties
Length of output: 661
🏁 Script executed:
# Check the app/bounty/page.tsx file for context around line 90 and the queryParams usage
sed -n '85,95p' app/bounty/page.tsxRepository: boundlessfi/bounties
Length of output: 471
🏁 Script executed:
# Search for any TODOs or comments about MULTI_WINNER_MILESTONE
rg -i 'TODO|FIXME|NOTE.*multi.*winner|multi.*winner.*milestone' app/bounty/page.tsx -B2 -A2Repository: boundlessfi/bounties
Length of output: 552
🏁 Script executed:
# Check if MULTI_WINNER_MILESTONE is referenced anywhere else in the codebase
rg 'MULTI_WINNER_MILESTONE' --type ts --type tsxRepository: boundlessfi/bounties
Length of output: 92
Unsafe cast of MULTI_WINNER_MILESTONE — invalid filter value will be sent to the backend.
The generated BountyType enum contains only Competition, FixedPrice, and MilestoneBased. The string "MULTI_WINNER_MILESTONE" is not a valid enum member. Casting it with as unknown as BountyType (lines 45, 105) hides this type error from TypeScript, but at runtime the value is spread directly into queryParams.type (line 90) and sent to the GraphQL backend as part of BountyQueryInput. The backend will reject this invalid enum value, causing the filter to silently fail or error.
Add MULTI_WINNER_MILESTONE to the GraphQL schema and regenerate the BountyType enum so BountyType.MultiWinnerMilestone can be used without casts. If that cannot be done immediately, gate the filter behind a client-side check that does not round-trip to the backend, or add a TODO comment to prevent silent shipping.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/bounty/page.tsx` around lines 38 - 49, The BOUNTY_TYPES entry is using an
unsafe cast ("MULTI_WINNER_MILESTONE" as unknown as BountyType) which will send
an invalid enum via queryParams.type to the backend; fix by adding
MULTI_WINNER_MILESTONE to the GraphQL schema and regenerating the TypeScript
types so you can use BountyType.MultiWinnerMilestone in BOUNTY_TYPES (replace
the cast), then ensure any usage that sets queryParams.type (where BountyType is
propagated) uses the real enum value; if you cannot update the schema right
away, gate the UI filter so selecting "Multi-Winner Milestone" is handled
client-side (does not set queryParams.type) and add a TODO comment referencing
the schema/types regen until the enum is added.
| // Fake current user ID for demo purposes | ||
| // In a real app this would come from auth context | ||
| const currentUserId = "user-1"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm this file used to derive currentUserId from authClient and this is a recent regression
git log --oneline -- app/leaderboard/page.tsx | head -20
git log -p -S 'authClient.useSession' -- app/leaderboard/page.tsx | head -80Repository: boundlessfi/bounties
Length of output: 152
🏁 Script executed:
#!/bin/bash
# Get current content of the leaderboard file
cat -n app/leaderboard/page.tsx
# Show the specific git diff for the commit mentioned
git show 8a934e8 -- app/leaderboard/page.tsxRepository: boundlessfi/bounties
Length of output: 11896
🏁 Script executed:
#!/bin/bash
# Search for authClient usage in the codebase
rg "authClient" --type tsx --type ts -A 3 -B 3
# Also search for useSession specifically
rg "useSession" --type tsx --type ts -A 2 -B 2Repository: boundlessfi/bounties
Length of output: 120
🏁 Script executed:
#!/bin/bash
# Search for authClient usage without file type restriction
rg "authClient" -A 3 -B 3 | head -100
# Also search for useSession
rg "useSession" -A 2 -B 2 | head -100
# Check if there are other uses of session/auth in the app
rg "auth.*Context|session.*Context" --type-list | head -20Repository: boundlessfi/bounties
Length of output: 10034
🏁 Script executed:
#!/bin/bash
# Check the commit message and surrounding context
git log --oneline -10
git show 8a934e8 --statRepository: boundlessfi/bounties
Length of output: 24350
Replace hardcoded currentUserId with actual user session derivation.
This file uses a hardcoded demo value (currentUserId = "user-1") that will break leaderboard personalization for real users—all visitors will see "user-1" highlighted in LeaderboardTable and have user-1's rank shown in UserRankSidebar. While the "[Coming soon] pattern" may be intentional for this page's current state, the leaderboard logic should derive the user ID from authClient.useSession(), as done in other components throughout the codebase (e.g., bounty-detail-client.tsx, fcfs-claim-button.tsx, use-notifications.ts).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/leaderboard/page.tsx` around lines 48 - 50, Replace the hardcoded
currentUserId variable with the actual session-derived user id by calling
authClient.useSession() in this component (same pattern used in
bounty-detail-client.tsx and fcfs-claim-button.tsx); extract the user id from
the returned session (e.g., session.user.id or equivalent) and pass that value
into LeaderboardTable and UserRankSidebar instead of the static "user-1",
handling the case where session is null/undefined (fall back to undefined/null
so components can render a non-personalized view).
| onRowClick={(entry) => | ||
| router.push(`/profile/${entry.contributor.userId}`) | ||
| router.push(`/user/${entry.contributor.userId}`) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for the route handler/page for /user/[id] vs /profile/[id]
fd -t d -a 'user' app
fd -t d -a 'profile' app
fd -e tsx -e ts . app/user 2>/dev/null
fd -e tsx -e ts . app/profile 2>/dev/null
# Find any other links to /profile/ or /user/ to gauge consistency
rg -nP "['\"]/(?:profile|user)/" --type=ts --type=tsxRepository: boundlessfi/bounties
Length of output: 411
🏁 Script executed:
#!/bin/bash
# Search for page.tsx files in app/user directory
find app -path "*/user/*/page.tsx" -o -path "*/user/page.tsx" 2>/dev/null | sort
# Check if app/user directory exists and list its contents
if [ -d "app/user" ]; then
echo "=== app/user directory contents ==="
find app/user -type f \( -name "*.tsx" -o -name "*.ts" \) 2>/dev/null | head -20
fi
# Check what's in app/profile for comparison
if [ -d "app/profile" ]; then
echo "=== app/profile directory contents ==="
find app/profile -type f \( -name "*.tsx" -o -name "*.ts" \) 2>/dev/null
fiRepository: boundlessfi/bounties
Length of output: 133
🏁 Script executed:
#!/bin/bash
# Search for route references in the codebase (without problematic file type flags)
echo "=== References to /user/ route ==="
rg "/user/" app --max-count=20
echo "=== References to /profile/ route ==="
rg "/profile/" app --max-count=20Repository: boundlessfi/bounties
Length of output: 340
🏁 Script executed:
#!/bin/bash
# Get the specific leaderboard page content around lines 137-139
if [ -f "app/leaderboard/page.tsx" ]; then
echo "=== app/leaderboard/page.tsx lines 130-150 ==="
sed -n '130,150p' app/leaderboard/page.tsx
fi
# Check UserRankSidebar location and content
echo "=== Finding UserRankSidebar ==="
find app -name "*UserRankSidebar*" -o -name "*rank*sidebar*" -o -name "*Rank*" 2>/dev/null | grep -i sidebarRepository: boundlessfi/bounties
Length of output: 816
🏁 Script executed:
#!/bin/bash
# Search for UserRankSidebar more broadly
echo "=== Searching for UserRankSidebar component ==="
rg "UserRankSidebar" app --max-count=10
echo "=== Searching for any Sidebar component ==="
find app -name "*Sidebar*" -o -name "*sidebar*" 2>/dev/null
echo "=== Searching in components directory ==="
find app/components -type f \( -name "*.tsx" -o -name "*.ts" \) 2>/dev/null | head -20Repository: boundlessfi/bounties
Length of output: 381
🏁 Script executed:
#!/bin/bash
# Check if there's a components directory and search for UserRank or Rank components
rg "export.*UserRankSidebar\|class UserRankSidebar\|function UserRankSidebar" app --max-count=5
# Also search for any route navigation in components
rg "router\.push.*user\|router\.push.*profile" app --max-count=10Repository: boundlessfi/bounties
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Find the actual components directory
echo "=== Locating components directory ==="
find . -maxdepth 3 -type d -name "components" 2>/dev/null
echo "=== Searching for user-rank-sidebar file ==="
fd "user-rank-sidebar" . 2>/dev/null
echo "=== Checking tsconfig or path aliases ==="
if [ -f "tsconfig.json" ]; then
cat tsconfig.json | grep -A 5 "paths\|baseUrl"
fi
echo "=== Full directory structure at root ==="
ls -la | head -20Repository: boundlessfi/bounties
Length of output: 1561
🏁 Script executed:
#!/bin/bash
# Search recursively for user-rank-sidebar
rg "user-rank-sidebar" . --type ts --type tsx 2>/dev/null
# Also try to find any file with UserRankSidebar definition
rg "class UserRankSidebar|function UserRankSidebar|const UserRankSidebar|export.*UserRankSidebar" . 2>/dev/nullRepository: boundlessfi/bounties
Length of output: 179
🏁 Script executed:
#!/bin/bash
# Read the user-rank-sidebar.tsx file to check for route references
echo "=== Contents of user-rank-sidebar.tsx ==="
cat components/leaderboard/user-rank-sidebar.tsx
# Also check if /app/user route exists at all
echo "=== Check for /app/user route ==="
ls -la app/ | grep userRepository: boundlessfi/bounties
Length of output: 8501
Fix broken /user/[userId] route reference; the route does not exist.
The leaderboard table's onRowClick handler navigates to /user/${userId}, but this route is not implemented. Only /profile/[userId] exists in the app. Users clicking leaderboard entries will encounter 404 errors. Change the route back to /profile/ or implement the /user/[userId] route.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/leaderboard/page.tsx` around lines 137 - 139, The leaderboard's
onRowClick handler currently calls router.push with a non-existent
`/user/${entry.contributor.userId}` route; update the navigation to use the
existing profile route by changing the router.push target to
`/profile/${entry.contributor.userId}` (locate the onRowClick callback and the
router.push call referencing entry.contributor.userId) so clicks navigate to the
implemented /profile/[userId] page.
| {bounty.type === "MULTI_WINNER_MILESTONE" && | ||
| session?.user?.id === bounty.createdBy && | ||
| (() => { | ||
| const { milestones, contributorProgress } = | ||
| getFullMilestoneData(bounty); | ||
| return ( | ||
| <Model4MaintainerDashboard | ||
| milestones={milestones} | ||
| contributors={contributorProgress} | ||
| /> | ||
| ); | ||
| })()} |
There was a problem hiding this comment.
Maintainer dashboard will display mock contributors when no real data is present.
getFullMilestoneData falls back to MOCK_MODEL4_CONTRIBUTORS for contributorProgress. When the bounty creator is signed in but bounty.contributorProgress is empty/undefined (real-data state), the dashboard will list Alice/Bob/etc. and expose mock-only "Release Payment"/"Advance" actions on them. The PR notes this is intentional during prototyping, but please add a clear gate (or a "Demo data" banner) before merging to a non-demo environment so creators don't act on mock contributors.
🤖 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 173 - 184,
getFullMilestoneData currently falls back to MOCK_MODEL4_CONTRIBUTORS and causes
Model4MaintainerDashboard to show fake contributors (and actions) when
bounty.contributorProgress is empty; update the rendering logic so that when
contributorProgress is derived from the MOCK_MODEL4_CONTRIBUTORS you either: (a)
do not render Model4MaintainerDashboard for real environments unless an explicit
demo flag is enabled (add and check a feature flag like isDemoMode or
process.env.SHOW_DEMO_CONTRIBUTORS), or (b) render the dashboard but pass an
explicit demo prop and surface a prominent "Demo data" banner inside
Model4MaintainerDashboard; locate getFullMilestoneData, MOCK_MODEL4_CONTRIBUTORS
and the Model4MaintainerDashboard call and implement the gate or banner to
prevent real creators from acting on mock contributors.
| const [loadingAction, setLoadingAction] = React.useState<string | null>(null); | ||
|
|
||
| const handleAction = async (action: string, userName: string) => { | ||
| setLoadingAction(`${action}-${userName}`); | ||
| console.log(`[Coming soon] ${action} for ${userName}`); | ||
| await new Promise((r) => setTimeout(r, 1000)); | ||
| setLoadingAction(null); | ||
| }; |
There was a problem hiding this comment.
loadingAction key collides on duplicate display names.
The key ${action}-${userName} uses the display name as identity. Two contributors with the same userName will share spinner state, and the matched-spinner branches at lines 182–183 / 205–206 will light up on both rows. Use contributor.userId instead.
🛠 Proposed fix
- const handleAction = async (action: string, userName: string) => {
- setLoadingAction(`${action}-${userName}`);
- console.log(`[Coming soon] ${action} for ${userName}`);
+ const handleAction = async (action: string, userId: string) => {
+ setLoadingAction(`${action}-${userId}`);
+ console.log(`[Coming soon] ${action} for ${userId}`);
await new Promise((r) => setTimeout(r, 1000));
setLoadingAction(null);
};…and update the call sites and the matched-spinner comparisons (lines 182, 205) to pass/compare contributor.userId.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/bounty-detail/model4-maintainer-dashboard.tsx` around lines 38 -
45, The loadingAction state key currently uses `${action}-${userName}` which can
collide for contributors sharing the same display name; update the key
generation in handleAction to use the unique contributor.userId instead (e.g.,
`${action}-${contributor.userId}`) and change all call sites that invoke
handleAction to pass contributor.userId rather than userName, then update the
matched-spinner comparisons (the checks that compare loadingAction to those
keys) to compare against the `${action}-${contributor.userId}` form so each
row’s spinner is driven by the unique userId.
|
@Jopsan-gm The "[Coming soon]" pattern is only half-applied. "Apply for Slot" has the suffix on the label, but the button is still clickable — it opens the full Same pattern is missing on Two UI deviations to tone down:
Please address all CodeRabbit findings as well. |
|
Gm @Benjtalkshow, can you check the changes? |
@Jopsan-gm , One last fix and we are good to go. One regression introduced by Still open from the last round:
|
Matches the pattern applied to the other maintainer dashboard actions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Benjtalkshow
left a comment
Benjtalkshow
left a comment
There was a problem hiding this comment.
LGTM!
All four items are clean. Test fixtures fixed, "[Coming soon]" suffix applied consistently to the dashboard buttons and Submit Work, the Release Payment color now matches the muted emerald pattern used elsewhere, and the funnel/submission-card/progress-bar shadows are all in line with the existing glows. Typecheck and lint pass locally.
Pushed a small commit (20ef410) on your branch to add the [Coming soon] suffix to the Remove tooltip so it matches the rest of the dashboard actions. Everything else looks ready — merging this in. Thanks for the thorough follow-up.
2 participants
Pull Request: Implement Multi-Winner Milestone Bounty Flow (Issue #173)
🎯 Description
This PR implements the "Model 4" bounty flow, which allows multiple contributors to work on a single bounty simultaneously through shared milestones. Each contributor tracks their progress independently, and sponsors can manage them through a new dedicated dashboard.
Key Changes:
BountyTypeand interfaces to support milestones and contributor progress.📸 Screenshots
🛠️ Technical Details
MULTI_WINNER_MILESTONEtoBountyType.MilestoneFunnel,MilestoneSubmissionCard, andModel4MaintainerDashboard.BountyDetailClientfor conditional rendering of Model 4 features.🔗 Related Issues
Closes #173
Summary by CodeRabbit