feat: Implement Earnings & Payout Tracking#110
Open
Oluwatos94 wants to merge 12 commits intoboundlessfi:mainfrom
Open
feat: Implement Earnings & Payout Tracking#110Oluwatos94 wants to merge 12 commits intoboundlessfi:mainfrom
Oluwatos94 wants to merge 12 commits intoboundlessfi:mainfrom
Conversation
fix reviews
fix reviews
|
@Oluwatos94 is attempting to deploy a commit to the Threadflow Team on Vercel. A member of the Team first needs to authorize it. |
|
Caution Review failedAn error occurred during the review process. Please try again later. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds an EarningsSummary UI and related types, updates profile claims/earnings logic, tightens GraphQL tooling and client typings, refines submission error handling, extends completion-history API with auth and richer fields, and updates tests and package deps. (50 words) Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (16)
package.json (1)
93-101: Dual test runners: both Jest and Vitest are listed.
jest@^30.2.0andvitest@^4.0.18are both indevDependencies. The test file reviewed (my-claims.test.ts) imports from@jest/globals, so it targets Jest. Having two test frameworks adds confusion and dependency bloat — consider converging on one.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 93 - 101, The repo currently lists both jest and vitest in package.json while tests (e.g. my-claims.test.ts) import from `@jest/globals`; remove the duplicate test runner to avoid confusion—either delete the "vitest" entry from devDependencies and any vitest scripts/config (preferred since tests target Jest), run a reinstall and update CI/scripts to use jest, or if you want to standardize on Vitest, change my-claims.test.ts imports from `@jest/globals` to the equivalent vitest imports and update package.json scripts and config to use "vitest" instead of "jest".components/reputation/__tests__/my-claims.test.ts (1)
16-58: Add test coverage for the new "Expired" claim section.The
CLAIM_SECTIONSconfiguration incomponents/reputation/my-claims.tsxincludes a new "Expired" section with statusesexpired,forfeited, andoverdue. The test currently has no claims with these statuses, leaving this section untested. The dynamic length assertion at line 29 masks this gap. Add expired/forfeited/overdue claims to the test data and verify they are grouped correctly.🧪 Proposed test data additions
{ bountyId: "6", title: "Completed B", status: "closed" }, { bountyId: "7", title: "Unknown", status: "queued" }, + { bountyId: "8", title: "Expired A", status: "expired" }, + { bountyId: "9", title: "Expired B", status: "forfeited" }, + { bountyId: "10", title: "Expired C", status: "overdue" }, ];And add an assertion:
+ const expiredGroup = groups.find( + (group) => group.section.title === "Expired", + ); + expect(expiredGroup?.claims.map((claim) => claim.bountyId)).toEqual([ + "8", + "9", + "10", + ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/reputation/__tests__/my-claims.test.ts` around lines 16 - 58, Update the test that calls getClaimsBySection to include sample claims with statuses "expired", "forfeited", and "overdue" so the new "Expired" section from CLAIM_SECTIONS is exercised; then add assertions that the group whose section.title === "Expired" contains those bountyIds and that those ids are not present in other sections (and adjust the grouped length checks as needed). Reference getClaimsBySection and CLAIM_SECTIONS when locating the test to modify.components/reputation/earnings-summary.tsx (1)
6-15: Type and component share the nameEarningsSummary.TypeScript allows this since types and values occupy separate namespaces, but it creates ambiguity at import sites (the profile page already aliases the type as
EarningsSummaryType). Consider renaming the type toEarningsSummaryDataor similar to avoid the need for aliasing.Also applies to: 17-19, 21-21
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/reputation/earnings-summary.tsx` around lines 6 - 15, The exported type EarningsSummary conflicts with the React component name EarningsSummary; rename the type to EarningsSummaryData (update the type declaration named EarningsSummary to EarningsSummaryData), update the component’s props and any local references that use the old type (e.g., props or generics that reference EarningsSummary), and update all import sites to import the new name (replace previous aliasing like EarningsSummaryType with EarningsSummaryData) so the component remains exported as EarningsSummary while the data type is unambiguous; ensure the export statement and any usage in related modules (the profile page and other files referencing the type) are updated accordingly.app/transparency/page.tsx (1)
101-138: Minor:statsError ? "—"branches are dead code.The
statCardsarray computes"—"whenstatsErroris true (lines 104, 113, 122, 131), but the entire stats grid is hidden via{!statsError && ...}on line 178. The error-case values are never rendered. Consider removing the ternary's error branch for clarity, or removing the grid-hiding guard if you want to show dashes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/transparency/page.tsx` around lines 101 - 138, The statCards entries include a statsError ? "—" branch that never renders because the stats grid is already gated by {!statsError && ...}; update statCards to remove the dead statsError ternaries and only condition on stats (e.g., value: stats ? ... : fallback) for each card (statCards, referencing stats and stats.totalFundsDistributed, stats.totalContributorsPaid, stats.totalProjectsFunded, stats.averagePayoutTimeDays), or if you intend to show dashes on error instead, remove the {!statsError && ...} guard so the error-case "—" values can actually render—pick one approach and make the corresponding change consistently.components/reputation/my-claims.tsx (1)
98-101: Hardcoded"$"currency may be incorrect for non-USD rewards.The
MyClaimtype doesn't carry acurrencyfield, soformatCurrencyalways uses"$". If bounties can pay in USDC, ETH, or other tokens, this will display misleading amounts (e.g.,$ 500.00for a USDC bounty). Consider adding acurrencyfield toMyClaimfor consistency withEarningsSummary.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/reputation/my-claims.tsx` around lines 98 - 101, The UI always formats claim.rewardAmount with a hardcoded "$" via formatCurrency, which is wrong for non-USD tokens; update the MyClaim type to include a currency field (e.g., currency: string) consistent with EarningsSummary, ensure the data layer that constructs MyClaim (API/mapper) populates that currency (USDC, ETH, USD, etc.), and then change the component rendering (where claim.rewardAmount is used) to call formatCurrency(claim.rewardAmount, claim.currency || "$") so the correct symbol/token is shown (with a sensible fallback).lib/graphql/client.ts (1)
57-75: Fragile error shape assertion.The
catchblock castserrorto an inline type to accessresponse.errors[].extensions.status. Ifgraphql-requestchanges its error shape across versions, this will silently skip auth handling. Consider using the library'sClientErrortype for safer narrowing.Proposed improvement
+import { GraphQLClient, ClientError } from "graphql-request"; ... } catch (error: unknown) { - const err = error as { - response?: { errors?: { extensions?: { status?: number } }[] }; - }; - if (err?.response?.errors) { - err.response.errors.forEach((gqlError) => { - const status = (gqlError?.extensions?.status as number) || 500; + if (error instanceof ClientError && error.response?.errors) { + error.response.errors.forEach((gqlError) => { + const status = + (gqlError?.extensions?.["status"] as number) ?? 500; if (isAuthStatus(status)) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/graphql/client.ts` around lines 57 - 75, The catch block currently narrows error with a fragile inline assertion to access response.errors[].extensions.status; replace this with a proper type guard using graphql-request's ClientError (import ClientError) and check instanceof ClientError before iterating error.response.errors, so auth handling runs only when the shape matches; retain the same logic using clearAccessToken(), isAuthStatus(), and window.dispatchEvent(...) but ensure you throw the original error (not the asserted type) if it's not a ClientError or after handling, and handle missing response/errors defensively.lib/api/bounties.ts (2)
114-121:submitpayload sendscontributorIdthat the server immediately discards.The server route at
app/api/bounties/[id]/submit/route.ts(line 23) destructurescontributorIdinto_clientContributorIdand ignores it, using the authenticated session's ID instead. Including it in the client API surface suggests it has an effect, which it doesn't. Remove it from the signature to avoid confusion and to close the door on any future accidental reliance on it.submit: ( id: string, - data: SubmissionFormValue & { contributorId: string }, + data: SubmissionFormValue, ): Promise<{ success: boolean; data: Submission }> => post<{ success: boolean; data: Submission }>( `${BOUNTIES_ENDPOINT}/${id}/submit`, data, ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/api/bounties.ts` around lines 114 - 121, The client API function submit in lib/api/bounties.ts currently accepts and sends a contributorId (signature: submit(id: string, data: SubmissionFormValue & { contributorId: string })) even though the server route destructures and ignores it; remove contributorId from the submit signature and payload so the function becomes submit(id: string, data: SubmissionFormValue): Promise<{ success: boolean; data: Submission }>, and update any call sites to stop passing contributorId; this prevents implying the client-provided contributorId has any effect and avoids accidental reliance on it.
54-57: Type names collide with existing exports fromlib/types.ts.
BountyType,BountyStatus, andDifficultyLevelare already defined inlib/types.ts(andClaimingModelis implicitly there viaBounty['claimingModel']). Consumers that import from both files will need to alias one set, and the two definitions can silently diverge if either file is updated independently.Consider re-exporting from
lib/types.tsor consolidating all Bounty-related types into a single source of truth.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/api/bounties.ts` around lines 54 - 57, The file defines BountyType, BountyStatus, DifficultyLevel and ClaimingModel that collide with exports from lib/types.ts; instead of keeping duplicate z.infer types here, import the canonical types from lib/types.ts and re-export them (or move the schema definitions into lib/types.ts and export z.infer types there), removing the duplicate declarations (references: BountyType, BountyStatus, DifficultyLevel, ClaimingModel and the local bountyTypeSchema/difficultySchema/claimingModelSchema symbols) so consumers use a single source of truth.hooks/use-reputation.ts (1)
11-12:undefinedin query key whenlimitis omitted from the key factory.Calling
REPUTATION_KEYS.completionHistory(userId)withoutlimityields[..., undefined]. In practice the hook always passes the default 50, but calling the key factory directly (e.g., ininvalidateQueries) without a limit produces a subtly different key that won't match cached entries.Consider making
limitrequired in the key factory, or defaulting it there:- completionHistory: (userId: string, limit?: number) => - [...REPUTATION_KEYS.all, "completion-history", userId, limit] as const, + completionHistory: (userId: string, limit: number = DEFAULT_COMPLETION_HISTORY_LIMIT) => + [...REPUTATION_KEYS.all, "completion-history", userId, limit] as const,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-reputation.ts` around lines 11 - 12, REPUTATION_KEYS.completionHistory currently allows limit to be undefined which produces a key containing undefined; change the factory to supply a concrete default (e.g., make the parameter signature completionHistory(userId: string, limit = 50)) or make limit required so callers must pass it, and update any call sites that rely on the implicit default; this ensures completionHistory(...) always returns a stable key (refer to REPUTATION_KEYS.completionHistory).components/bounty-detail/submission-dialog.tsx (1)
84-87:useFieldArraywithas neverbypasses type safety for theattachmentsfield.
useFieldArrayis designed for arrays of objects; using it withstring[]requires theas neverescape hatch, which hides any type mismatch. If the field shape ever changes, TypeScript won't catch the breakage here.For primitive arrays, prefer
watch/setValuedirectly:- const { fields, append, remove } = useFieldArray({ - control: form.control, - name: "attachments" as never, - }); + const attachments = form.watch("attachments") ?? []; + const appendAttachment = () => { + form.setValue("attachments", [...attachments, ""], { shouldValidate: false }); + }; + const removeAttachment = (index: number) => { + form.setValue( + "attachments", + attachments.filter((_, i) => i !== index), + { shouldValidate: true }, + ); + };And replace
append("" as never)→appendAttachment(),remove(index)→removeAttachment(index).Also applies to: 278-279
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/bounty-detail/submission-dialog.tsx` around lines 84 - 87, The useFieldArray call for the "attachments" field is bypassing type safety by casting to never; replace useFieldArray(control: form.control, name: "attachments") with a primitive-array approach using form.watch("attachments") and form.setValue("attachments", ...) and implement helper functions appendAttachment() and removeAttachment(index) that update the watched attachment string[] (instead of using append("" as never) and remove(index)); update the other occurrence that uses append/remove for the same "attachments" field to call these new helpers so TypeScript correctly enforces the attachments: string[] shape.app/api/bounties/[id]/submit/route.ts (2)
26-31:.flatten()is deprecated in Zod v4.The
.flatten()method onZodErroris deprecated in Zod 4; the replacement is the top-levelz.treeifyError()function.- const fieldErrors = parsed.error.flatten().fieldErrors; + const fieldErrors = z.treeifyError(parsed.error); return NextResponse.json( { error: "Validation failed", fieldErrors },Note:
z.treeifyErrorreturns a nested tree structure rather than a flat{ fieldName: string[] }map, so the API response shape and any client-side consumers that readfieldErrorswill need to be updated consistently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/bounties/`[id]/submit/route.ts around lines 26 - 31, The code uses parsed.error.flatten() (in route.ts handling parsed from Zod) which is deprecated in Zod v4; replace the flatten() call with z.treeifyError(parsed.error) to get the new nested error tree and then either (a) adapt the API response to return that nested shape (e.g., return fieldErrors = z.treeifyError(parsed.error)) or (b) convert the tree into the previous flat { field: string[] } map before returning to preserve client compatibility; update the return statement that currently sets fieldErrors and any client-side consumers accordingly.
46-57: HardcodedallowedModelsduplicatesclaimingModelSchema— derive from the schema instead.This list must be kept in sync with
claimingModelSchemainlib/api/bounties.tsmanually. If a new model is added to the schema, this check will silently let it through or block it depending on edit order.♻️ Proposed fix
- const allowedModels = [ - "single-claim", - "competition", - "multi-winner", - "application", - ]; - if (!allowedModels.includes(bounty.claimingModel)) { + // claimingModelSchema is the single source of truth for valid models + const { claimingModelSchema } = await import("@/components/bounty/forms/schemas"); + if (!claimingModelSchema.options.includes(bounty.claimingModel)) {Or, more simply, export the allowed values array from
lib/api/bounties.tsand import it here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/bounties/`[id]/submit/route.ts around lines 46 - 57, The hardcoded allowedModels array in route.ts duplicates the claimingModelSchema; remove that local list and instead import the canonical allowed values from lib/api/bounties.ts (either export the allowed array or derive it from claimingModelSchema there), then replace the check so it uses the imported array (e.g., importedAllowedClaimingModels.includes(bounty.claimingModel)); update the reference to the symbol allowedModels in this file to the imported name and ensure the export from lib/api/bounties.ts is the single source of truth.components/bounty/forms/schemas.ts (1)
70-79:submissionFormSchemauses deprecated Zod v4 APIs — migrate before the next major.
z.string().url()(and similar method forms like.email()) are deprecated in Zod v4; they still work but "will be removed in the next major version." Additionally, themessagestring parameter (e.g.,.min(20, "...")) has been replaced byerror; the oldmessageparameter is still supported but deprecated.Both patterns appear in the new schema:
- githubUrl: z.string().url("Must be a valid URL").optional().or(z.literal("")), - demoUrl: z.string().url("Must be a valid URL").optional().or(z.literal("")), + githubUrl: z.union([z.url({ error: "Must be a valid URL" }), z.literal("")]).optional(), + demoUrl: z.union([z.url({ error: "Must be a valid URL" }), z.literal("")]).optional(), explanation: z .string() - .min(20, "Explanation must be at least 20 characters") - .max(5000, "Explanation must not exceed 5,000 characters"), - attachments: z.array(z.string().url("Must be a valid URL")).optional(), + .min(20, { error: "Explanation must be at least 20 characters" }) + .max(5000, { error: "Explanation must not exceed 5,000 characters" }), + attachments: z.array(z.url({ error: "Must be a valid URL" })).optional(),The same pattern also applies to the pre-existing schemas earlier in the file (
milestoneSchema,markdownContentSchema, etc.), though those are out of scope for this PR.app/profile/[userId]/page.tsx (3)
123-126: Fragile error-shape assumption.The cast
error as { status?: number; message?: string }and the substring checkmessage?.includes("404")are brittle. If the error shape fromuseContributorReputationchanges (e.g., wraps the message differently, or uses astatusCodefield), 404 detection silently breaks and every not-found shows "Something went wrong" instead.Consider centralising a type guard (e.g.,
isNotFoundError(error)) that handles known error shapes in one place.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/profile/`[userId]/page.tsx around lines 123 - 126, The current 404 detection in the error handling block (using a direct cast to `{ status?: number; message?: string }` and `message?.includes("404")`) is brittle; implement a central type guard function named `isNotFoundError(error): boolean` that checks common shapes returned by `useContributorReputation` (e.g., `error.status`, `error.statusCode`, `error.code === 'NOT_FOUND'`, `error.message` containing 'not found' or '404', and `error.response?.status === 404`) and then replace the inline cast/check in the page component with a call to `isNotFoundError(error)` so 404 detection is robust across different error shapes.
128-141: Duplicated "Profile Not Found" block.Lines 128-141 and 157-170 render identical markup. Extract a small
ProfileNotFoundcomponent or a shared JSX variable to keep them in sync.Also applies to: 157-170
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/profile/`[userId]/page.tsx around lines 128 - 141, Duplicate JSX for the "Profile Not Found" UI appears in the isNotFound branches (render block using AlertCircle, heading "Profile Not Found", paragraph and Button/Link); extract that markup into a shared React component (e.g., ProfileNotFound) or a JSX variable and replace both inline blocks with <ProfileNotFound /> to avoid duplication and keep behavior/styling in one place; ensure the new component accepts any props needed (e.g., className or returnUrl) and is imported/defined in the same module so both branches (the isNotFound checks around the existing rendering logic) render the shared component.
45-49:useBounties()fetches all bounties then filters client-side.Lines 58-61 filter the full bounty list by
claimedBy === userId. As the bounty count grows, this will transfer and process unnecessary data on every profile visit. Consider adding a server-side filter (e.g.,useBounties({ claimedBy: userId })) when the API supports it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/profile/`[userId]/page.tsx around lines 45 - 49, useBounties() currently fetches all bounties then filters them client-side (the code that filters by claimedBy === userId); change the hook invocation to request a server-side filter so only relevant records are returned (e.g., call useBounties({ claimedBy: userId }) or equivalent query param), remove the client-side filter on bountyResponse, and update any downstream uses of bountyResponse/isBountiesLoading/bountiesError to expect the already-filtered list; if the API lacks this option, add a server endpoint or extend useBounties to accept and forward a claimedBy parameter to the backend.
🤖 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/api/reputation/`[userId]/completion-history/route.ts:
- Around line 41-43: The GET handler currently exposes any user's completion
history without auth; update the GET function to call getCurrentUser() (same
auth flow used by the submit route), verify the returned user exists, and then
enforce access rules: if you intend to restrict reads, require that
currentUser.id === params.userId and return 403 for mismatches; if cross-user
reads are allowed only for authenticated callers, ensure the caller is
authenticated and proceed, otherwise return 401. Locate the GET function and the
params.userId reference to add this check and return appropriate 401/403
responses consistent with the submit route behavior.
- Around line 19-21: The current code uses placeholder fallbacks that
misrepresent data: replace the substitutions so claimedAt, completionTimeHours,
and pointsEarned return null when real values can't be computed instead of using
bounty.createdAt, 0, or reward; update the logic in the route handler where
claimedAt is assigned (currently using bounty.claimedAt ?? bounty.createdAt),
where completionTimeHours is computed/assigned (currently set to 0), and where
pointsEarned is set (currently equal to reward) to return null when the true
value is unknown, and adjust the BountyCompletionRecord type/schema to allow
nullable claimedAt, completionTimeHours, and pointsEarned so consumers receive
honest nulls rather than misleading placeholders.
- Line 24: The generated record id is unstable because it includes the paginated
index (id: `completion-${bounty.id}-${index}` where index = offset + i); remove
the offset-dependent part and generate a deterministic id per bounty instead
(e.g., use `completion-${bounty.id}`) so the same bounty returns the same id
across pages—update the id construction where `id:
\`completion-${bounty.id}-${index}\`` is set (use bounty.id alone or another
stable bounty-unique value).
In `@app/profile/`[userId]/page.tsx:
- Around line 55-96: The current earningsSummary computation filters
userBounties by reputation.stats.earningsCurrency (default "USDC"), silently
excluding bounties in other currencies; update earningsSummary (and related
variables userBounties, totalEarned, pendingAmount, payoutHistory derived from
bountyResponse?.data and deriveBountyStatus) to either (A) aggregate earnings
per currency (group bounties by rewardCurrency and produce totals/payoutHistory
per currency) or (B) include all currencies in the single summary but
mark/outline non-primary-currency entries (e.g., add currency field on each
payoutHistory row and keep totals grouped by currency), and surface a UI flag
when reputation.stats.earningsCurrency is set so the frontend can show a note
that multiple currencies exist; pick one approach and ensure the dependency
array and returned shape of earningsSummary are updated to reflect per-currency
results.
- Around line 116-117: The Skeleton components in app/profile/[userId]/page.tsx
use an invalid Tailwind class `h-100`; replace those usages (the two Skeletons
around the profile layout and the other occurrence mentioned) with a valid
option: either change `h-100` to an arbitrary-value class like `h-[25rem]` on
the Skeleton components, or add a `100: "25rem"` height entry to the Tailwind
theme heights in app/globals.css (inside the `@theme` inline block) and then keep
`h-100`; update both occurrences to the chosen approach so the Skeletons have an
explicit height.
In `@codegen.ts`:
- Line 4: The codegen config is referencing an external schema path ("schema:
\"../boundless-nestjs/src/schema.gql\"") which will break CI when the sibling
repo isn't present; change the value of schema in codegen.ts to point to a
schema file committed in this repo (for example lib/graphql/schema.gql) or
replace it with an introspection URL endpoint, add the chosen schema file to the
repository, and update any scripts/CI steps so codegen uses the new local path
or reachable URL.
In `@components/bounty-detail/submission-dialog.tsx`:
- Around line 301-309: The icon-only remove Button rendered inside the
submission dialog (Button with onClick={() => remove(index)} containing the
Trash2 icon) lacks an accessible label; update that Button to include an
appropriate aria-label (e.g., aria-label={`Remove submission ${index + 1}`}) or
a visually-hidden text alternative so screen readers can identify its purpose,
keeping the onClick and classNames unchanged and applying the label to the
Button component that wraps the Trash2 icon.
- Around line 45-51: The default submission form currently hardcodes
mockWalletInfo.address in getBaseDefaults (SubmissionFormValue) which would
submit a fake wallet in production; replace the hardcoded mock address with a
safe default (e.g., empty string or pull from the real wallet provider) and add
a runtime guard: if process.env.NODE_ENV !== "development" and
mockWalletInfo.address is still used, throw an error or log a clear TODO so the
mock cannot ship to production; update getBaseDefaults and any initialization
that references mockWalletInfo.address to use the real wallet accessor (or empty
fallback) and include the TODO/error check to prevent accidental production use.
- Around line 153-172: The success-state dialog rendered when submitted returns
a Dialog with DialogContent but uses a plain <h3> ("Submission Sent!") instead
of the library's DialogTitle, so screen readers won't announce the dialog;
update the component to use the DialogTitle component (import it if missing) in
place of or wrapping the <h3> used in the submitted branch (the JSX around
Dialog, DialogContent, and the heading that references bountyTitle) and preserve
the existing classes/visual styling so the dialog receives an accessible name
while keeping the rest of the content unchanged.
- Around line 142-148: The catch block in submission-dialog.tsx swallows
server-side field validation errors by only reading err.message; update error
handling so the HTTP client (bountiesApi.submit) includes the parsed response
body on error (e.g., attach response.body or fieldErrors to the thrown error),
then in the catch in the submit handler detect that structure (e.g.,
err.fieldErrors or err.response?.body?.fieldErrors) and call
form.setError(fieldName, { message: fieldError }) for each field before showing
a toast; keep a fallback to toast the general err.message when no fieldErrors
are present.
In `@components/reputation/earnings-summary.tsx`:
- Around line 22-23: The current derivation of currencySymbol is a no-op
(currencySymbol uses earnings.currency === "USDC" ? "USDC" : earnings.currency)
so either simplify it to directly use earnings.currency or, if you intended to
show a symbol for USDC, replace the ternary with a mapping that returns the
actual symbol (e.g., "$") for the "USDC" key; update the variable currencySymbol
(and any uses) accordingly and remove the redundant branch so currencySymbol
reflects the intended display value.
In `@lib/graphql/client.ts`:
- Around line 39-41: Add a client-only directive so this module cannot be
imported on the server: insert the "use client" directive at the top of the file
to enforce client-side usage; ensure the existing GraphQLClient creation
(symbols: graphQLClient, GraphQLClient, url, NEXT_PUBLIC_GRAPHQL_URL) remains
unchanged and uses the existing env fallback
(process.env.NEXT_PUBLIC_GRAPHQL_URL || "/api/graphql"), or if you prefer SSR
support instead, wrap the URL creation in a runtime check (e.g., only use
"/api/graphql" when window is defined) to avoid constructing GraphQLClient on
the server.
In `@package.json`:
- Line 80: The package.json currently pairs
`@graphql-codegen/typescript-react-query`@6 with graphql-request@7 which is
ESM-only and incompatible with CJS toolchains; either pin graphql-request to a
5.x release (downgrade the "graphql-request" dependency to ^5) or remove the
built-in fetcher and configure a custom fetcher for the generated React Query
hooks (update codegen configuration that references the typescript-react-query
plugin to use a custom fetcher). Ensure you update package.json dependency entry
for "graphql-request" or the codegen config used by
`@graphql-codegen/typescript-react-query` so the bundle/toolchain remains
CJS-compatible.
In `@types/participation.ts`:
- Around line 27-29: The test in lib/store.test.ts constructs a Submission but
omits the now-required fields from the Participation/Submission interface;
update the test's Submission object to include valid explanation (string) and
walletAddress (string) properties so it matches the interface used in
types/participation.ts and satisfies TypeScript strict checks; locate the
Submission creation in lib/store.test.ts and add these fields with appropriate
sample values.
---
Nitpick comments:
In `@app/api/bounties/`[id]/submit/route.ts:
- Around line 26-31: The code uses parsed.error.flatten() (in route.ts handling
parsed from Zod) which is deprecated in Zod v4; replace the flatten() call with
z.treeifyError(parsed.error) to get the new nested error tree and then either
(a) adapt the API response to return that nested shape (e.g., return fieldErrors
= z.treeifyError(parsed.error)) or (b) convert the tree into the previous flat {
field: string[] } map before returning to preserve client compatibility; update
the return statement that currently sets fieldErrors and any client-side
consumers accordingly.
- Around line 46-57: The hardcoded allowedModels array in route.ts duplicates
the claimingModelSchema; remove that local list and instead import the canonical
allowed values from lib/api/bounties.ts (either export the allowed array or
derive it from claimingModelSchema there), then replace the check so it uses the
imported array (e.g.,
importedAllowedClaimingModels.includes(bounty.claimingModel)); update the
reference to the symbol allowedModels in this file to the imported name and
ensure the export from lib/api/bounties.ts is the single source of truth.
In `@app/profile/`[userId]/page.tsx:
- Around line 123-126: The current 404 detection in the error handling block
(using a direct cast to `{ status?: number; message?: string }` and
`message?.includes("404")`) is brittle; implement a central type guard function
named `isNotFoundError(error): boolean` that checks common shapes returned by
`useContributorReputation` (e.g., `error.status`, `error.statusCode`,
`error.code === 'NOT_FOUND'`, `error.message` containing 'not found' or '404',
and `error.response?.status === 404`) and then replace the inline cast/check in
the page component with a call to `isNotFoundError(error)` so 404 detection is
robust across different error shapes.
- Around line 128-141: Duplicate JSX for the "Profile Not Found" UI appears in
the isNotFound branches (render block using AlertCircle, heading "Profile Not
Found", paragraph and Button/Link); extract that markup into a shared React
component (e.g., ProfileNotFound) or a JSX variable and replace both inline
blocks with <ProfileNotFound /> to avoid duplication and keep behavior/styling
in one place; ensure the new component accepts any props needed (e.g., className
or returnUrl) and is imported/defined in the same module so both branches (the
isNotFound checks around the existing rendering logic) render the shared
component.
- Around line 45-49: useBounties() currently fetches all bounties then filters
them client-side (the code that filters by claimedBy === userId); change the
hook invocation to request a server-side filter so only relevant records are
returned (e.g., call useBounties({ claimedBy: userId }) or equivalent query
param), remove the client-side filter on bountyResponse, and update any
downstream uses of bountyResponse/isBountiesLoading/bountiesError to expect the
already-filtered list; if the API lacks this option, add a server endpoint or
extend useBounties to accept and forward a claimedBy parameter to the backend.
In `@app/transparency/page.tsx`:
- Around line 101-138: The statCards entries include a statsError ? "—" branch
that never renders because the stats grid is already gated by {!statsError &&
...}; update statCards to remove the dead statsError ternaries and only
condition on stats (e.g., value: stats ? ... : fallback) for each card
(statCards, referencing stats and stats.totalFundsDistributed,
stats.totalContributorsPaid, stats.totalProjectsFunded,
stats.averagePayoutTimeDays), or if you intend to show dashes on error instead,
remove the {!statsError && ...} guard so the error-case "—" values can actually
render—pick one approach and make the corresponding change consistently.
In `@components/bounty-detail/submission-dialog.tsx`:
- Around line 84-87: The useFieldArray call for the "attachments" field is
bypassing type safety by casting to never; replace useFieldArray(control:
form.control, name: "attachments") with a primitive-array approach using
form.watch("attachments") and form.setValue("attachments", ...) and implement
helper functions appendAttachment() and removeAttachment(index) that update the
watched attachment string[] (instead of using append("" as never) and
remove(index)); update the other occurrence that uses append/remove for the same
"attachments" field to call these new helpers so TypeScript correctly enforces
the attachments: string[] shape.
In `@components/reputation/__tests__/my-claims.test.ts`:
- Around line 16-58: Update the test that calls getClaimsBySection to include
sample claims with statuses "expired", "forfeited", and "overdue" so the new
"Expired" section from CLAIM_SECTIONS is exercised; then add assertions that the
group whose section.title === "Expired" contains those bountyIds and that those
ids are not present in other sections (and adjust the grouped length checks as
needed). Reference getClaimsBySection and CLAIM_SECTIONS when locating the test
to modify.
In `@components/reputation/earnings-summary.tsx`:
- Around line 6-15: The exported type EarningsSummary conflicts with the React
component name EarningsSummary; rename the type to EarningsSummaryData (update
the type declaration named EarningsSummary to EarningsSummaryData), update the
component’s props and any local references that use the old type (e.g., props or
generics that reference EarningsSummary), and update all import sites to import
the new name (replace previous aliasing like EarningsSummaryType with
EarningsSummaryData) so the component remains exported as EarningsSummary while
the data type is unambiguous; ensure the export statement and any usage in
related modules (the profile page and other files referencing the type) are
updated accordingly.
In `@components/reputation/my-claims.tsx`:
- Around line 98-101: The UI always formats claim.rewardAmount with a hardcoded
"$" via formatCurrency, which is wrong for non-USD tokens; update the MyClaim
type to include a currency field (e.g., currency: string) consistent with
EarningsSummary, ensure the data layer that constructs MyClaim (API/mapper)
populates that currency (USDC, ETH, USD, etc.), and then change the component
rendering (where claim.rewardAmount is used) to call
formatCurrency(claim.rewardAmount, claim.currency || "$") so the correct
symbol/token is shown (with a sensible fallback).
In `@hooks/use-reputation.ts`:
- Around line 11-12: REPUTATION_KEYS.completionHistory currently allows limit to
be undefined which produces a key containing undefined; change the factory to
supply a concrete default (e.g., make the parameter signature
completionHistory(userId: string, limit = 50)) or make limit required so callers
must pass it, and update any call sites that rely on the implicit default; this
ensures completionHistory(...) always returns a stable key (refer to
REPUTATION_KEYS.completionHistory).
In `@lib/api/bounties.ts`:
- Around line 114-121: The client API function submit in lib/api/bounties.ts
currently accepts and sends a contributorId (signature: submit(id: string, data:
SubmissionFormValue & { contributorId: string })) even though the server route
destructures and ignores it; remove contributorId from the submit signature and
payload so the function becomes submit(id: string, data: SubmissionFormValue):
Promise<{ success: boolean; data: Submission }>, and update any call sites to
stop passing contributorId; this prevents implying the client-provided
contributorId has any effect and avoids accidental reliance on it.
- Around line 54-57: The file defines BountyType, BountyStatus, DifficultyLevel
and ClaimingModel that collide with exports from lib/types.ts; instead of
keeping duplicate z.infer types here, import the canonical types from
lib/types.ts and re-export them (or move the schema definitions into
lib/types.ts and export z.infer types there), removing the duplicate
declarations (references: BountyType, BountyStatus, DifficultyLevel,
ClaimingModel and the local
bountyTypeSchema/difficultySchema/claimingModelSchema symbols) so consumers use
a single source of truth.
In `@lib/graphql/client.ts`:
- Around line 57-75: The catch block currently narrows error with a fragile
inline assertion to access response.errors[].extensions.status; replace this
with a proper type guard using graphql-request's ClientError (import
ClientError) and check instanceof ClientError before iterating
error.response.errors, so auth handling runs only when the shape matches; retain
the same logic using clearAccessToken(), isAuthStatus(), and
window.dispatchEvent(...) but ensure you throw the original error (not the
asserted type) if it's not a ClientError or after handling, and handle missing
response/errors defensively.
In `@package.json`:
- Around line 93-101: The repo currently lists both jest and vitest in
package.json while tests (e.g. my-claims.test.ts) import from `@jest/globals`;
remove the duplicate test runner to avoid confusion—either delete the "vitest"
entry from devDependencies and any vitest scripts/config (preferred since tests
target Jest), run a reinstall and update CI/scripts to use jest, or if you want
to standardize on Vitest, change my-claims.test.ts imports from `@jest/globals` to
the equivalent vitest imports and update package.json scripts and config to use
"vitest" instead of "jest".
There was a problem hiding this comment.
♻️ Duplicate comments (3)
app/profile/[userId]/page.tsx (1)
219-219:⚠️ Potential issue | 🟡 Minor
h-100is still not a valid Tailwind v4 utility class.Lines 116–117 were fixed to
h-[25rem], but this occurrence was missed. Apply the same fix here.Proposed fix
- <Skeleton className="h-100 w-full" /> + <Skeleton className="h-[25rem] w-full" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/profile/`[userId]/page.tsx at line 219, Replace the invalid Tailwind class on the Skeleton component: locate the JSX element <Skeleton className="h-100 w-full" /> and change the className to use the same fixed utility used earlier (e.g., replace "h-100" with "h-[25rem]" so it becomes "h-[25rem] w-full").app/api/reputation/[userId]/completion-history/route.ts (2)
51-56: Auth added but authorization is missing — any authenticated user can read any other user's history.
getCurrentUser()is now called (good), but the endpoint doesn't verify thatcurrentUser.id === userId. Depending on the platform's privacy model, this may expose another user's full bounty/completion history to any logged-in user.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/reputation/`[userId]/completion-history/route.ts around lines 51 - 56, The endpoint calls getCurrentUser() but never authorizes access by comparing the authenticated user's id to the requested userId; update the route handler to verify currentUser.id === userId (or allow privileged roles if your policy requires) and if it doesn't match return a 403 (e.g., NextResponse.json({ error: "Forbidden" }, { status: 403 })); locate the authorization check around getCurrentUser(), currentUser, and the userId parameter in this route (route.ts) and enforce the equality check before returning any other user-specific completion history.
17-18: Placeholder fallbacks forclaimedAtandpointsEarnedremain.
claimedAtstill falls back tobounty.createdAt(bounty creation ≠ claim time) andpointsEarnedis still hardcoded to0. These were flagged in a prior review. If the real values aren't available yet, consider documenting a TODO or making theBountyCompletionRecordtype acceptnullfor these fields so consumers can distinguish "unknown" from "zero."Also applies to: 41-42
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/reputation/`[userId]/completion-history/route.ts around lines 17 - 18, The code is using placeholder fallbacks (bounty.claimedAt => bounty.createdAt and pointsEarned => 0) which hide "unknown" values; update the logic to set claimedAt and pointsEarned to null when the real values are unavailable (instead of falling back to createdAt or zero), and update the BountyCompletionRecord type to allow these fields to be null (or add a TODO comment noting the absence of real data and why null is used) so consumers can distinguish unknown vs zero; specifically change assignments that reference bounty.claimedAt/bounty.createdAt and the hardcoded pointsEarned to return null when not present and adjust BountyCompletionRecord to accept null for claimedAt and pointsEarned (or add a clear TODO if you intend to populate them later).
🧹 Nitpick comments (3)
lib/store.test.ts (1)
9-13: Consider adding aBountyStore.reset()method for test isolation.The comment acknowledges that global state persists across tests and tests rely on execution order (e.g., "update" tests depend on prior "add" tests having run). A
reset()orclear()method onBountyStorecalled inbeforeEachwould make these tests independent and order-safe.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/store.test.ts` around lines 9 - 13, Add a reset/clear method to the BountyStore singleton and call it in tests' beforeEach to ensure isolation: implement a BountyStore.reset() (or clear()) that empties internal storage and resets any ID counters used by methods like addBounty and updateBounty, then update lib/store.test.ts to call beforeEach(() => BountyStore.reset()) so each test (e.g., tests referencing addBounty, updateBounty) runs order-independent.components/reputation/earnings-summary.tsx (1)
6-15: Type and component share the same nameEarningsSummary, causing awkward imports for consumers.The profile page is forced to alias the type:
type EarningsSummary as EarningsSummaryType. Renaming the type (e.g.,EarningsSummaryData) or the component (e.g.,EarningsSummaryPanel) would remove this ambiguity.Also applies to: 21-21
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/reputation/earnings-summary.tsx` around lines 6 - 15, The exported type EarningsSummary conflicts with the component of the same name; rename one to avoid ambiguous imports (e.g., change the type EarningsSummary to EarningsSummaryData or rename the component to EarningsSummaryPanel), then update all references/exports/usages in this file (including the component declaration that currently shares the name and any prop typings/consumers) to the new identifier (ensure the type used in prop definitions, imports in consumer files, and any tests are updated accordingly).app/profile/[userId]/page.tsx (1)
45-49:useBounties()fetches all bounties and filters client-side.This call retrieves the entire bounty list just to filter by
claimedBy === userId. As the platform scales, this could become a performance bottleneck. Consider adding a server-side filter parameter (e.g.,useBounties({ claimedBy: userId })) to reduce payload size.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/profile/`[userId]/page.tsx around lines 45 - 49, Currently useBounties() fetches all bounties and filters client-side by claimedBy === userId; change the call to pass a server-side filter (e.g., useBounties({ claimedBy: userId })) so the API returns only bounties for that user, and remove any subsequent client-side filtering; update the useBounties hook implementation to accept and forward a filter param (claimedBy) to the backend query and ensure components using bountyResponse / isBountiesLoading / bountiesError keep the same shape.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/api/reputation/`[userId]/completion-history/route.ts:
- Around line 51-56: The endpoint calls getCurrentUser() but never authorizes
access by comparing the authenticated user's id to the requested userId; update
the route handler to verify currentUser.id === userId (or allow privileged roles
if your policy requires) and if it doesn't match return a 403 (e.g.,
NextResponse.json({ error: "Forbidden" }, { status: 403 })); locate the
authorization check around getCurrentUser(), currentUser, and the userId
parameter in this route (route.ts) and enforce the equality check before
returning any other user-specific completion history.
- Around line 17-18: The code is using placeholder fallbacks (bounty.claimedAt
=> bounty.createdAt and pointsEarned => 0) which hide "unknown" values; update
the logic to set claimedAt and pointsEarned to null when the real values are
unavailable (instead of falling back to createdAt or zero), and update the
BountyCompletionRecord type to allow these fields to be null (or add a TODO
comment noting the absence of real data and why null is used) so consumers can
distinguish unknown vs zero; specifically change assignments that reference
bounty.claimedAt/bounty.createdAt and the hardcoded pointsEarned to return null
when not present and adjust BountyCompletionRecord to accept null for claimedAt
and pointsEarned (or add a clear TODO if you intend to populate them later).
In `@app/profile/`[userId]/page.tsx:
- Line 219: Replace the invalid Tailwind class on the Skeleton component: locate
the JSX element <Skeleton className="h-100 w-full" /> and change the className
to use the same fixed utility used earlier (e.g., replace "h-100" with
"h-[25rem]" so it becomes "h-[25rem] w-full").
---
Nitpick comments:
In `@app/profile/`[userId]/page.tsx:
- Around line 45-49: Currently useBounties() fetches all bounties and filters
client-side by claimedBy === userId; change the call to pass a server-side
filter (e.g., useBounties({ claimedBy: userId })) so the API returns only
bounties for that user, and remove any subsequent client-side filtering; update
the useBounties hook implementation to accept and forward a filter param
(claimedBy) to the backend query and ensure components using bountyResponse /
isBountiesLoading / bountiesError keep the same shape.
In `@components/reputation/earnings-summary.tsx`:
- Around line 6-15: The exported type EarningsSummary conflicts with the
component of the same name; rename one to avoid ambiguous imports (e.g., change
the type EarningsSummary to EarningsSummaryData or rename the component to
EarningsSummaryPanel), then update all references/exports/usages in this file
(including the component declaration that currently shares the name and any prop
typings/consumers) to the new identifier (ensure the type used in prop
definitions, imports in consumer files, and any tests are updated accordingly).
In `@lib/store.test.ts`:
- Around line 9-13: Add a reset/clear method to the BountyStore singleton and
call it in tests' beforeEach to ensure isolation: implement a
BountyStore.reset() (or clear()) that empties internal storage and resets any ID
counters used by methods like addBounty and updateBounty, then update
lib/store.test.ts to call beforeEach(() => BountyStore.reset()) so each test
(e.g., tests referencing addBounty, updateBounty) runs order-independent.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
app/api/bounties/[id]/submit/route.tsapp/api/reputation/[userId]/completion-history/route.tsapp/profile/[userId]/page.tsxcodegen.tscomponents/bounty-detail/submission-dialog.tsxcomponents/reputation/earnings-summary.tsxlib/graphql/client.tslib/store.test.tspackage.json
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/graphql/client.ts
- components/bounty-detail/submission-dialog.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat: implement earnings & payout tracking in My Claims
Adds an EarningsSummary component to the My Claims tab on the profile page, showing total earned, pending amount (by currency), and a payout history list with processing/completed statuses. Earnings data is derived from the user's bounty claims, with a fallback to reputation stats when no claim data is available.
closes #84
Summary by CodeRabbit
New Features
Improvements