Open
Conversation
|
@Oluwatos94 is attempting to deploy a commit to the Threadflow Team on Vercel. A member of the Team first needs to authorize it. |
|
@Oluwatos94 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! 🚀 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds Spark Credits end-to-end: types, mock service, two Next.js API routes, client wrappers and React Query hooks, UI components (balance, history, explainer), and profile/navbar integration to display balance and history. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as User Browser
participant Component as CreditBalance Component
participant Hook as useSparkCreditsBalance Hook
participant Client as sparkCreditsApi.fetchBalance
participant Route as API Route<br/>/api/spark-credits/{userId}
participant Service as SparkCreditsService
Browser->>Component: mount with userId
Component->>Hook: call useSparkCreditsBalance(userId)
Hook->>Client: fetchBalance(userId)
Client->>Route: GET /api/spark-credits/{userId}
Route->>Service: SparkCreditsService.getBalance(userId)
Service-->>Route: { userId, balance }
Route-->>Client: JSON response
Client-->>Hook: SparkCreditsBalance
Hook-->>Component: data -> render balance
sequenceDiagram
participant Browser as User Browser
participant Page as Profile Credits Tab
participant Hook as useSparkCreditsHistory Hook
participant Client as sparkCreditsApi.fetchHistory
participant Route as API Route<br/>/api/spark-credits/{userId}/history
participant Service as SparkCreditsService
Browser->>Page: open "Spark Credits" tab
Page->>Hook: useSparkCreditsHistory(userId, limit)
Hook->>Client: fetchHistory(userId, {limit, offset})
Client->>Route: GET /api/spark-credits/{userId}/history?limit=20&offset=0
Route->>Service: getCreditHistory(userId, limit, offset)
Service-->>Route: { events[], totalCount, hasMore }
Route-->>Client: JSON response
Client-->>Hook: SparkCreditsHistoryResponse
Hook-->>Page: render CreditHistory(events)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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: 5
🧹 Nitpick comments (5)
lib/services/spark-credits.ts (1)
83-96: Harden service pagination inputs (defense in depth).Even if routes validate inputs, sanitizing
limit/offsethere prevents bad behavior from any direct callers.🛡️ Suggested hardening
static async getCreditHistory( userId: string, limit = 20, offset = 0, ): Promise<SparkCreditsHistoryResponse> { await new Promise((resolve) => setTimeout(resolve, 100)); + const safeLimit = + Number.isInteger(limit) && limit > 0 ? Math.min(limit, 100) : 20; + const safeOffset = + Number.isInteger(offset) && offset >= 0 ? offset : 0; const events = MOCK_CREDIT_EVENTS[userId] ?? []; const totalCount = events.length; - const sliced = events.slice(offset, offset + limit); + const sliced = events.slice(safeOffset, safeOffset + safeLimit); return { events: sliced, totalCount, - hasMore: offset + limit < totalCount, + hasMore: safeOffset + safeLimit < totalCount, }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/services/spark-credits.ts` around lines 83 - 96, The getCreditHistory method accepts unvalidated limit/offset which can cause unexpected slices or DoS; inside SparkCreditsService.getCreditHistory (function getCreditHistory) sanitize inputs by coercing limit and offset to integers, enforce offset >= 0 and limit >= 1, and cap limit to a sensible maximum (e.g., 100) before using them to slice MOCK_CREDIT_EVENTS[userId]; ensure non-numeric values default to the provided defaults and that hasMore is computed from the sanitized values.app/api/spark-credits/[userId]/route.ts (1)
4-10: Add explicit error handling for service failures.Wrap the service call so this route consistently returns JSON error payloads on failure.
🛠️ Suggested refactor
export async function GET( _request: NextRequest, context: { params: Promise<{ userId: string }> }, ) { - const { userId } = await context.params; - const balance = await SparkCreditsService.getBalance(userId); - return NextResponse.json(balance); + try { + const { userId } = await context.params; + const balance = await SparkCreditsService.getBalance(userId); + return NextResponse.json(balance); + } catch { + return NextResponse.json( + { error: "Failed to fetch spark credit balance" }, + { status: 500 }, + ); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/spark-credits/`[userId]/route.ts around lines 4 - 10, The GET route calls SparkCreditsService.getBalance without error handling; wrap the await SparkCreditsService.getBalance(userId) call in a try/catch inside the exported GET function and on failure return a consistent JSON error response using NextResponse.json (include a descriptive message and the error details where safe) with an appropriate HTTP status (e.g., 500), and optionally map known service errors to specific statuses (e.g., 404 for not found); keep the successful path returning NextResponse.json(balance) unchanged.hooks/use-spark-credits.ts (1)
8-9: Query key doesn't includeoffsetfor future pagination.The API (
lib/api/spark-credits.ts) and server route (app/api/spark-credits/[userId]/history/route.ts) both supportoffsetfor pagination, but the query key only includeslimit. If offset-based pagination is added later, the key should includeoffsetto avoid cache collisions.Since
hasMorein the response suggests pagination may be implemented, consider future-proofing:🔮 Future-proof query key
- history: (userId: string, limit?: number) => - [...SPARK_CREDITS_KEYS.all, "history", userId, limit] as const, + history: (userId: string, limit?: number, offset?: number) => + [...SPARK_CREDITS_KEYS.all, "history", userId, { limit, offset }] as const,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-spark-credits.ts` around lines 8 - 9, The query key for spark credits history is missing offset which can cause cache collisions when paginating; update the history function (SPARK_CREDITS_KEYS.history) signature in hooks/use-spark-credits.ts to accept an optional offset (e.g., history: (userId: string, limit?: number, offset?: number)) and include offset in the returned tuple (append offset after limit) so the key becomes [...SPARK_CREDITS_KEYS.all, "history", userId, limit, offset] as const; keep the order and as const typing so React Query can distinguish different pages.components/reputation/credit-history.tsx (2)
71-75: Consider defensive date parsing forcreatedAt.If
event.createdAtcontains an invalid or malformed date string,new Date(event.createdAt)will produce anInvalid Dateobject, causingformatDistanceToNowto throw or display unexpected output. Consider adding validation or a try-catch with a fallback.🛡️ Suggested defensive approach
- <span> - {formatDistanceToNow(new Date(event.createdAt), { - addSuffix: true, - })} - </span> + <span> + {(() => { + const date = new Date(event.createdAt); + return isNaN(date.getTime()) + ? "Unknown date" + : formatDistanceToNow(date, { addSuffix: true }); + })()} + </span>Alternatively, extract this to a helper function for cleaner JSX.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/reputation/credit-history.tsx` around lines 71 - 75, Wrap the date construction and formatting used in the JSX (where formatDistanceToNow(new Date(event.createdAt), { addSuffix: true }) is called) with defensive parsing: validate event.createdAt before calling new Date (e.g., try parsing and check isValid or that resulting Date is not "Invalid Date"), and when invalid use a fallback (e.g., Date.now() or a placeholder like "unknown") so formatDistanceToNow never receives an invalid Date; extract this logic into a small helper function (e.g., formatEventAge(createdAt)) and call that from the JSX to keep the component clean.
89-95: Consider formatting numeric values for readability.
event.amountandevent.balanceare displayed as raw numbers. For larger values, consider usingtoLocaleString()or a number formatting utility to improve readability (e.g.,1,234instead of1234).✨ Example formatting
- <span>{event.amount}</span> + <span>{event.amount.toLocaleString()}</span>- <span>{event.balance} balance</span> + <span>{event.balance.toLocaleString()} balance</span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/reputation/credit-history.tsx` around lines 89 - 95, Format the numeric displays for readability by converting event.amount and event.balance to locale-formatted strings before rendering: replace raw usage of event.amount and event.balance with a formatted value (e.g., use Number(event.amount).toLocaleString() or Intl.NumberFormat) and ensure the sign rendering around event.amount (the existing isEarned ? "+" : "-") still applies to the formatted absolute value so you don't double-sign; update both spans that reference event.amount and event.balance accordingly (the span showing amount next to the first Zap icon and the span showing balance next to the second Zap icon).
🤖 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/spark-credits/`[userId]/history/route.ts:
- Around line 10-20: Parse and validate the search params used for pagination
before calling SparkCreditsService.getCreditHistory: ensure the parsed limit and
offset are finite integers (use Math.floor or similar), fall back to defaults
when parsing fails (limit default 20, offset default 0), clamp limit to a
reasonable max (e.g., 100) and enforce non-negative offset; replace the current
direct Number(...) usage with this validated/clamped values so
getCreditHistory(userId, limit, offset) always receives safe integers.
In `@app/profile/`[userId]/page.tsx:
- Around line 5-8: The import of useSparkCreditsBalance is unused; edit the
import statement in page.tsx to remove useSparkCreditsBalance and only import
useSparkCreditsHistory from "@/hooks/use-spark-credits" (i.e., change the
imported symbols to avoid the unused binding).
In `@components/global-navbar.tsx`:
- Around line 95-98: The NavRankBadge and CreditBalance components are using a
hardcoded userId="user-1"; replace that with the real authenticated user ID
source (e.g., the session/user object or auth hook) so each signed-in user sees
their own data: locate NavRankBadge and CreditBalance usages in
global-navbar.tsx and pass the actual id (for example user.id or currentUser.id)
obtained from your auth context/prop (e.g., useAuth(), useSession(), or a parent
prop like currentUser) and ensure you handle the unauthenticated case (hide
widgets or return null) before rendering.
In `@components/reputation/credit-balance.tsx`:
- Around line 20-27: The component collapses fetch errors into a zero-balance UX
by using data?.balance ?? 0; update the render logic in the block that reads
useSparkCreditsBalance (and the similar block at the 52-61 region) to check the
hook's error state (e.g., isError or error) before defaulting: if isLoading show
the Skeleton, if isError show an explicit error/placeholder state (or return
null) instead of treating balance as 0, and only compute balance = data?.balance
?? 0 and isLow = balance <= 1 when there is no error; reference
useSparkCreditsBalance, data, isLoading, isError/error, balance, and isLow to
locate and change the logic.
In `@lib/services/spark-credits.ts`:
- Around line 8-12: The mock credits data is inconsistent: MOCK_CREDITS_DB has
"user-1": 5 while the latest event (evt-1) records a running balance of 6,
causing contradictory UI state; update either MOCK_CREDITS_DB or the event data
so both agree (e.g., set MOCK_CREDITS_DB["user-1"] to 6 or adjust evt-1 to 5)
and ensure any other mock balances/events in the same block (the entries around
evt-1) are consistent as well; locate symbols MOCK_CREDITS_DB and evt-1 to make
the change.
---
Nitpick comments:
In `@app/api/spark-credits/`[userId]/route.ts:
- Around line 4-10: The GET route calls SparkCreditsService.getBalance without
error handling; wrap the await SparkCreditsService.getBalance(userId) call in a
try/catch inside the exported GET function and on failure return a consistent
JSON error response using NextResponse.json (include a descriptive message and
the error details where safe) with an appropriate HTTP status (e.g., 500), and
optionally map known service errors to specific statuses (e.g., 404 for not
found); keep the successful path returning NextResponse.json(balance) unchanged.
In `@components/reputation/credit-history.tsx`:
- Around line 71-75: Wrap the date construction and formatting used in the JSX
(where formatDistanceToNow(new Date(event.createdAt), { addSuffix: true }) is
called) with defensive parsing: validate event.createdAt before calling new Date
(e.g., try parsing and check isValid or that resulting Date is not "Invalid
Date"), and when invalid use a fallback (e.g., Date.now() or a placeholder like
"unknown") so formatDistanceToNow never receives an invalid Date; extract this
logic into a small helper function (e.g., formatEventAge(createdAt)) and call
that from the JSX to keep the component clean.
- Around line 89-95: Format the numeric displays for readability by converting
event.amount and event.balance to locale-formatted strings before rendering:
replace raw usage of event.amount and event.balance with a formatted value
(e.g., use Number(event.amount).toLocaleString() or Intl.NumberFormat) and
ensure the sign rendering around event.amount (the existing isEarned ? "+" :
"-") still applies to the formatted absolute value so you don't double-sign;
update both spans that reference event.amount and event.balance accordingly (the
span showing amount next to the first Zap icon and the span showing balance next
to the second Zap icon).
In `@hooks/use-spark-credits.ts`:
- Around line 8-9: The query key for spark credits history is missing offset
which can cause cache collisions when paginating; update the history function
(SPARK_CREDITS_KEYS.history) signature in hooks/use-spark-credits.ts to accept
an optional offset (e.g., history: (userId: string, limit?: number, offset?:
number)) and include offset in the returned tuple (append offset after limit) so
the key becomes [...SPARK_CREDITS_KEYS.all, "history", userId, limit, offset] as
const; keep the order and as const typing so React Query can distinguish
different pages.
In `@lib/services/spark-credits.ts`:
- Around line 83-96: The getCreditHistory method accepts unvalidated
limit/offset which can cause unexpected slices or DoS; inside
SparkCreditsService.getCreditHistory (function getCreditHistory) sanitize inputs
by coercing limit and offset to integers, enforce offset >= 0 and limit >= 1,
and cap limit to a sensible maximum (e.g., 100) before using them to slice
MOCK_CREDIT_EVENTS[userId]; ensure non-numeric values default to the provided
defaults and that hasMore is computed from the sanitized values.
🪄 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: 66e5444d-048d-499c-aad6-18e670882fbc
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
app/api/spark-credits/[userId]/history/route.tsapp/api/spark-credits/[userId]/route.tsapp/profile/[userId]/page.tsxcomponents/global-navbar.tsxcomponents/reputation/credit-balance.tsxcomponents/reputation/credit-explainer.tsxcomponents/reputation/credit-history.tsxhooks/use-spark-credits.tslib/api/spark-credits.tslib/services/spark-credits.tstypes/spark-credits.ts
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 Spark Credits UI
Add credit balance display, history, and explainer components for the Spark Credits system.
Changes:
Note: Credit gate in hooks/use-claim-bounty.ts is deferred pending #140.
closes #144
Summary by CodeRabbit