Review AI suggestions for PRs #443 and #444 #446
Conversation
Security improvements: - Add URL validation to prevent open redirect vulnerabilities in checkout routes - Use generic error messages for 500 responses to avoid exposing sensitive details - Add JSON parsing validation with proper 400 responses for malformed requests - Validate item existence before recording views to prevent data pollution - Add customer ID validation for Polar checkout (consistent with Stripe) Bug fixes: - Fix pagination issues (restore standard pagination UI, fix page reset on search) - Fix webhook handlers to properly detect sponsor ad renewals (check subscription metadata) - Fix admin stats to show global counts instead of page-scoped counts - Fix modal closing logic to check operation success before closing - Fix collection card spinner to only show on regular left-click navigation - Fix item view recording to validate item existence for all users - Fix limit parameter validation to handle NaN and invalid values properly - Fix isActive consistency in admin collections page i18n improvements: - Replace hardcoded strings with translation keys in collection components - Add missing translations for aria-labels and status messages Code quality: - Remove unused props and imports - Improve error handling consistency across API routes - Add proper existence checks before database operations - Fix cache revalidation to use correct identifiers (slug vs id) Affected areas: - API routes (sponsor ads, collections, items, payments, webhooks) - Admin pages (collections, sponsorships) - UI components (collections, categories, billing) - Webhook handlers (Stripe, Polar)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRefactors admin collections UI data fetching and modal flows, tightens admin API auth/validation and error handling, simplifies category listing props/pagination, adds i18n keys and localized strings, and hardens multiple public/payment/sponsor-ad routes with JSON/URL validation and improved webhook handling. Changes
Sequence Diagram(s)(Skipped — changes span multiple independent UI/API improvements rather than a single new multi-component sequential flow.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
Innocent-Akim
changed the title
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 23 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="app/api/sponsor-ads/checkout/route.ts">
<violation number="1" location="app/api/sponsor-ads/checkout/route.ts:235">
P1: The error message is exposed to clients in 500 responses, which contradicts the PR's stated security improvement. Internal error messages may reveal sensitive details like database errors or file paths. Use a generic message instead.</violation>
</file>
<file name="app/api/payment/[subscriptionId]/route.ts">
<violation number="1" location="app/api/payment/[subscriptionId]/route.ts:30">
P2: Destructuring `body` without validating it's a non-null object can throw if the JSON body is `null`, causing a 500 error instead of the intended 400. Consider adding an object validation check before destructuring.</violation>
</file>
<file name="components/collections/collection-card.tsx">
<violation number="1" location="components/collections/collection-card.tsx:109">
P2: Hardcoded string 'View collection' should use the translation system for i18n consistency. The PR description mentions replacing hardcoded strings with translation keys in collection components, but this string was missed. Consider using `t('VIEW_COLLECTION')` or similar.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/billing/expired-plan-banner.tsx (1)
267-267: Hardcoded fallback string not i18n-friendly.The fallback
'Subscription'should use a translation key for consistency with the i18n changes in this PR.- const planName = PAYMENT_PLAN_NAMES[planStatus.planId as PaymentPlan] || 'Subscription'; + const planName = PAYMENT_PLAN_NAMES[planStatus.planId as PaymentPlan] || t('DEFAULT_SUBSCRIPTION_NAME');Based on learnings, avoid hard-coded English strings; use next-intl messages where relevant.
app/api/payment/[subscriptionId]/route.ts (1)
23-35: Consider using Zod for type-safe request validation.The codebase consistently uses Zod for API request validation (stripe, lemonsqueezy, and sponsor-ad routes all follow this pattern). Replace the manual type checking with a schema-based approach.
Define a schema in
lib/validations/payment.ts:Zod schema approach
import { z } from 'zod'; import { PaymentProvider } from '@/lib/constants'; export const updateSubscriptionPaymentSchema = z.object({ enabled: z.boolean(), paymentProvider: z.nativeEnum(PaymentProvider).optional().default(PaymentProvider.STRIPE) });Then in the route handler:
- let body: unknown; - try { - body = await request.json(); - } catch (parseError) { - return NextResponse.json({ error: 'Invalid JSON in request body' }, { status: 400 }); - } - - const { enabled, paymentProvider } = body as { enabled?: unknown; paymentProvider?: string }; - const provider = paymentProvider || 'stripe'; - - if (typeof enabled !== 'boolean') { - return NextResponse.json({ error: 'Invalid request body. "enabled" must be a boolean.' }, { status: 400 }); - } - - const validProviders = Object.values(PaymentProvider) as string[]; - if (!validProviders.includes(provider)) { - return NextResponse.json( - { error: `Invalid payment provider. Must be one of: ${validProviders.join(', ')}` }, - { status: 400 } - ); - } + const body = await request.json(); + const result = updateSubscriptionPaymentSchema.safeParse(body); + + if (!result.success) { + return NextResponse.json( + { error: 'Invalid request body', details: result.error.format() }, + { status: 400 } + ); + } + + const { enabled, paymentProvider: provider } = result.data;
🧹 Nitpick comments (12)
app/api/extract/route.ts (1)
152-153: Consider explicit error handling for successful response parsing (optional).The successful response JSON parsing relies on the outer try-catch for error handling. While this is acceptable and aligns with the overall error handling pattern, adding explicit error handling here would provide clearer error messages if the platform API returns an invalid JSON response for a successful status code.
However, since the platform API should always return valid JSON for successful responses, the current approach is adequate.
🔎 Optional: Add explicit error handling
- const data = await response.json(); - return NextResponse.json(data); + try { + const data = await response.json(); + return NextResponse.json(data); + } catch { + return NextResponse.json( + { success: false, error: 'Invalid response format from platform API' }, + { status: 500 } + ); + }app/api/admin/collections/[id]/route.ts (1)
247-249: Redundant null check onbeforeUpdate.
beforeUpdateis guaranteed to be truthy at this point due to the check at line 239-241. The condition can be simplified.🔎 Suggested simplification
- if (beforeUpdate && beforeUpdate.slug !== (updateData.slug || id)) { + if (beforeUpdate.slug !== (updateData.slug || id)) { revalidatePath(`/collections/${beforeUpdate.slug}`); // old slug }app/[locale]/categories/listing-categories.tsx (2)
19-19: UnusedpaginationTypedestructured from hook.
paginationTypeis destructured but no longer used after pagination removal. Consider removing it to avoid confusion.- const { layoutHome = LayoutHome.HOME_ONE, paginationType } = useLayoutTheme(); + const { layoutHome = LayoutHome.HOME_ONE } = useLayoutTheme();
51-109: Consider extracting duplicate breadcrumb component.
HomeOneLayoutandHomeTwoLayoutcontain nearly identical breadcrumb markup (lines 57-99 and 117-159). Consider extracting a sharedCategoriesBreadcrumbcomponent to reduce duplication and improve maintainability.Also applies to: 111-165
app/[locale]/admin/collections/page.tsx (2)
44-44: Magic number for limit may not scale.The hardcoded
limit=1000assumes collections won't exceed this count. Consider extracting to a constant or using a dedicated stats endpoint.+const STATS_FETCH_LIMIT = 1000; + const { data: allCollectionsData } = useQuery({ queryKey: ['admin', 'collections', 'stats'], queryFn: async () => { const response = await serverClient.get<{ success: boolean; collections: Collection[]; total: number; - }>('/api/admin/collections?limit=1000&includeInactive=true'); + }>(`/api/admin/collections?limit=${STATS_FETCH_LIMIT}&includeInactive=true`);
93-98: Nativeconfirm()blocks the UI.Using
window.confirm()is functional but blocks the main thread. Consider using a confirmation modal component for better UX consistency with the rest of the admin interface.app/api/admin/sponsor-ads/route.ts (1)
121-125: Consider using structured logging instead of console.log.These debug logs expose potentially sensitive data (full query results and stats). Consider using the structured
Loggerclass from@/lib/loggerwith appropriate log levels, or remove these logs before merging to production.app/api/stripe/webhook/route.ts (1)
625-655: Consider extracting shared metadata resolution logic.Both
isSponsorAdSubscriptionandgetSponsorAdIdduplicate the same metadata extraction pattern. Consider extracting a helper function to reduce duplication and ensure consistent behavior.🔎 Proposed refactor
+/** + * Extract metadata from various possible locations in Stripe webhook data + */ +function extractMetadata(data: Record<string, unknown>): Record<string, string> | undefined { + return ( + (data.metadata as Record<string, string> | undefined) || + ((data.subscription_data as Record<string, unknown>)?.metadata as Record<string, string> | undefined) || + ((data.subscription as Record<string, unknown>)?.metadata as Record<string, string> | undefined) + ); +} + function isSponsorAdSubscription(data: Record<string, unknown>): boolean { - const metadata = data.metadata as Record<string, string> | undefined; - const subscriptionDataMetadata = (data.subscription_data as Record<string, unknown>)?.metadata as - | Record<string, string> - | undefined; - const subscriptionMetadata = (data.subscription as Record<string, unknown>)?.metadata as - | Record<string, string> - | undefined; - - return ( - metadata?.type === 'sponsor_ad' || - subscriptionDataMetadata?.type === 'sponsor_ad' || - subscriptionMetadata?.type === 'sponsor_ad' - ); + const metadata = extractMetadata(data); + return metadata?.type === 'sponsor_ad'; } function getSponsorAdId(data: Record<string, unknown>): string | null { - const metadata = data.metadata as Record<string, string> | undefined; - const subscriptionDataMetadata = (data.subscription_data as Record<string, unknown>)?.metadata as - | Record<string, string> - | undefined; - const subscriptionMetadata = (data.subscription as Record<string, unknown>)?.metadata as - | Record<string, string> - | undefined; - - return metadata?.sponsorAdId || subscriptionDataMetadata?.sponsorAdId || subscriptionMetadata?.sponsorAdId || null; + const metadata = extractMetadata(data); + return metadata?.sponsorAdId || null; }app/api/polar/webhook/handlers.ts (1)
152-166: Inconsistent email data construction pattern.
handleSubscriptionCreated(lines 152-166) andhandleSubscriptionUpdated(lines 210-224) constructemailDatadirectly without usingcreateEmailData, while other handlers likehandleSubscriptionCancelled(lines 275-293) use the{...createEmailData(..., emailConfig)}spread pattern. This inconsistency could lead to missing fields (e.g.,companyName,companyUrl,supportEmailare manually added vs. automatically included viacreateEmailData).🔎 Proposed fix for handleSubscriptionCreated (apply similar pattern to handleSubscriptionUpdated)
- const emailData = { - customerName: customerInfo.customerName, - customerEmail: customerInfo.customerEmail, - planName, - amount, - currency: data.currency || DEFAULT_CURRENCY, - billingPeriod, - nextBillingDate: data.current_period_end ? formatBillingDate(data.current_period_end) : undefined, - subscriptionId: data.id || '', - manageSubscriptionUrl: `${APP_URL}/settings/subscription`, - companyName: emailConfig.companyName, - companyUrl: emailConfig.companyUrl, - supportEmail: emailConfig.supportEmail || '', - features: getSubscriptionFeatures(planName) - }; + const emailData = { + ...createEmailData( + { + customerName: customerInfo.customerName, + customerEmail: customerInfo.customerEmail, + planName, + amount, + currency: data.currency || DEFAULT_CURRENCY, + billingPeriod, + nextBillingDate: data.current_period_end ? formatBillingDate(data.current_period_end) : undefined, + subscriptionId: data.id || '', + manageSubscriptionUrl: `${APP_URL}/settings/subscription`, + features: getSubscriptionFeatures(planName) + }, + emailConfig + ) + };Also applies to: 210-224
app/api/sponsor-ads/user/route.ts (1)
56-57: Consider using the same limit validation pattern as other routes.
app/api/sponsor-ads/route.tsuses a more robust validation pattern withNumber()andNumber.isFinite()checks. While Zod validation catches invalid values here, using consistent validation patterns across routes improves maintainability.app/api/admin/sponsor-ads/[id]/reject/route.ts (1)
73-73: Consider explicit JSON error handling for consistency.Line 73 silently converts invalid JSON to an empty object with
.catch(() => ({})). While Zod validation catches missing fields,app/api/sponsor-ads/user/route.ts(lines 155-161) explicitly returns a 400 error for invalid JSON. Consider using the explicit pattern for consistency and clearer error messages.🔎 Proposed fix
const { id } = await params; - const body = await request.json().catch(() => ({})); + let body: Record<string, unknown> = {}; + try { + body = await request.json(); + } catch { + return NextResponse.json( + { success: false, error: 'Invalid JSON in request body' }, + { status: 400 } + ); + }app/api/admin/sponsor-ads/[id]/route.ts (1)
102-114: Consider using typed errors instead of string matching.The error handling correctly distinguishes between expected 404 errors and unexpected 500 errors, which is good for both UX and security. However, checking
error.message === 'Sponsor ad not found'is brittle and lacks type safety.💡 Future improvement: typed error classes
Consider introducing typed error classes in the service layer for better maintainability:
// In lib/errors.ts or similar export class NotFoundError extends Error { constructor(message: string) { super(message); this.name = 'NotFoundError'; } } // In the service throw new NotFoundError('Sponsor ad not found'); // In the route handler if (error instanceof NotFoundError) { return NextResponse.json({ success: false, error: error.message }, { status: 404 }); }This provides type safety and makes the error handling contract explicit between the service and route layers.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
app/[locale]/admin/collections/page.tsxapp/[locale]/admin/sponsorships/page.tsxapp/[locale]/categories/listing-categories.tsxapp/[locale]/categories/page.tsxapp/api/admin/collections/[id]/route.tsapp/api/admin/collections/route.tsapp/api/admin/sponsor-ads/[id]/reject/route.tsapp/api/admin/sponsor-ads/[id]/route.tsapp/api/admin/sponsor-ads/route.tsapp/api/extract/route.tsapp/api/items/[slug]/views/route.tsapp/api/payment/[subscriptionId]/route.tsapp/api/polar/webhook/handlers.tsapp/api/sponsor-ads/checkout/route.tsapp/api/sponsor-ads/route.tsapp/api/sponsor-ads/user/[id]/cancel/route.tsapp/api/sponsor-ads/user/route.tsapp/api/stripe/webhook/route.tscomponents/admin/collections/assign-items-modal.tsxcomponents/billing/expired-plan-banner.tsxcomponents/categories-grid.tsxcomponents/collections/collection-card.tsxcomponents/collections/collection-detail.tsx
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Node.js >= 20.19.0
Files:
components/billing/expired-plan-banner.tsxcomponents/categories-grid.tsxapp/api/extract/route.tsapp/api/admin/collections/route.tscomponents/collections/collection-detail.tsxcomponents/collections/collection-card.tsxapp/api/items/[slug]/views/route.tsapp/api/sponsor-ads/user/[id]/cancel/route.tsapp/api/admin/sponsor-ads/route.tsapp/api/polar/webhook/handlers.tsapp/api/admin/collections/[id]/route.tsapp/[locale]/categories/page.tsxapp/api/sponsor-ads/user/route.tsapp/api/sponsor-ads/route.tsapp/api/sponsor-ads/checkout/route.tsapp/api/admin/sponsor-ads/[id]/route.tsapp/api/stripe/webhook/route.tsapp/[locale]/categories/listing-categories.tsxapp/api/admin/sponsor-ads/[id]/reject/route.tsapp/[locale]/admin/sponsorships/page.tsxcomponents/admin/collections/assign-items-modal.tsxapp/[locale]/admin/collections/page.tsxapp/api/payment/[subscriptionId]/route.ts
components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
components/**/*.{ts,tsx}: UI components should be placed in the components/** directory
Keep React components mostly presentational and lean, avoiding complex business logic
Files:
components/billing/expired-plan-banner.tsxcomponents/categories-grid.tsxcomponents/collections/collection-detail.tsxcomponents/collections/collection-card.tsxcomponents/admin/collections/assign-items-modal.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Prefer TypeScript files over JavaScript
Run pnpm tsc --noEmit for TypeScript type-checking
Files:
components/billing/expired-plan-banner.tsxcomponents/categories-grid.tsxapp/api/extract/route.tsapp/api/admin/collections/route.tscomponents/collections/collection-detail.tsxcomponents/collections/collection-card.tsxapp/api/items/[slug]/views/route.tsapp/api/sponsor-ads/user/[id]/cancel/route.tsapp/api/admin/sponsor-ads/route.tsapp/api/polar/webhook/handlers.tsapp/api/admin/collections/[id]/route.tsapp/[locale]/categories/page.tsxapp/api/sponsor-ads/user/route.tsapp/api/sponsor-ads/route.tsapp/api/sponsor-ads/checkout/route.tsapp/api/admin/sponsor-ads/[id]/route.tsapp/api/stripe/webhook/route.tsapp/[locale]/categories/listing-categories.tsxapp/api/admin/sponsor-ads/[id]/reject/route.tsapp/[locale]/admin/sponsorships/page.tsxcomponents/admin/collections/assign-items-modal.tsxapp/[locale]/admin/collections/page.tsxapp/api/payment/[subscriptionId]/route.ts
components/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Place UI and layout components in components/**
Files:
components/billing/expired-plan-banner.tsxcomponents/categories-grid.tsxcomponents/collections/collection-detail.tsxcomponents/collections/collection-card.tsxcomponents/admin/collections/assign-items-modal.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer TypeScript for all code
Files:
components/billing/expired-plan-banner.tsxcomponents/categories-grid.tsxapp/api/extract/route.tsapp/api/admin/collections/route.tscomponents/collections/collection-detail.tsxcomponents/collections/collection-card.tsxapp/api/items/[slug]/views/route.tsapp/api/sponsor-ads/user/[id]/cancel/route.tsapp/api/admin/sponsor-ads/route.tsapp/api/polar/webhook/handlers.tsapp/api/admin/collections/[id]/route.tsapp/[locale]/categories/page.tsxapp/api/sponsor-ads/user/route.tsapp/api/sponsor-ads/route.tsapp/api/sponsor-ads/checkout/route.tsapp/api/admin/sponsor-ads/[id]/route.tsapp/api/stripe/webhook/route.tsapp/[locale]/categories/listing-categories.tsxapp/api/admin/sponsor-ads/[id]/reject/route.tsapp/[locale]/admin/sponsorships/page.tsxcomponents/admin/collections/assign-items-modal.tsxapp/[locale]/admin/collections/page.tsxapp/api/payment/[subscriptionId]/route.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: Follow Prettier config with tabs, 4-space tabWidth, and 120-char printWidth
Prefer async/await over raw Promise chains in TypeScript/JavaScript code
Validate input with Zod where appropriate; follow existing schemas in lib/validations
Keep i18n-friendly: avoid hard-coded English strings in logic; use next-intl messages where relevant
Files:
components/billing/expired-plan-banner.tsxcomponents/categories-grid.tsxapp/api/extract/route.tsapp/api/admin/collections/route.tscomponents/collections/collection-detail.tsxcomponents/collections/collection-card.tsxapp/api/items/[slug]/views/route.tsapp/api/sponsor-ads/user/[id]/cancel/route.tsapp/api/admin/sponsor-ads/route.tsapp/api/polar/webhook/handlers.tsapp/api/admin/collections/[id]/route.tsapp/[locale]/categories/page.tsxapp/api/sponsor-ads/user/route.tsapp/api/sponsor-ads/route.tsapp/api/sponsor-ads/checkout/route.tsapp/api/admin/sponsor-ads/[id]/route.tsapp/api/stripe/webhook/route.tsapp/[locale]/categories/listing-categories.tsxapp/api/admin/sponsor-ads/[id]/reject/route.tsapp/[locale]/admin/sponsorships/page.tsxcomponents/admin/collections/assign-items-modal.tsxapp/[locale]/admin/collections/page.tsxapp/api/payment/[subscriptionId]/route.ts
**/components/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/components/**/*.{jsx,tsx}: For forms, prefer react-hook-form + Zod; follow patterns in existing auth/profile forms
Place business logic in lib/services or lib/repositories, not in components; keep components mostly presentational and data-fetching
Files:
components/billing/expired-plan-banner.tsxcomponents/categories-grid.tsxcomponents/collections/collection-detail.tsxcomponents/collections/collection-card.tsxcomponents/admin/collections/assign-items-modal.tsx
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Routes and pages should be placed in app/[locale]/** and app/api/** directories following Next.js App Router conventions
Files:
app/api/extract/route.tsapp/api/admin/collections/route.tsapp/api/items/[slug]/views/route.tsapp/api/sponsor-ads/user/[id]/cancel/route.tsapp/api/admin/sponsor-ads/route.tsapp/api/polar/webhook/handlers.tsapp/api/admin/collections/[id]/route.tsapp/[locale]/categories/page.tsxapp/api/sponsor-ads/user/route.tsapp/api/sponsor-ads/route.tsapp/api/sponsor-ads/checkout/route.tsapp/api/admin/sponsor-ads/[id]/route.tsapp/api/stripe/webhook/route.tsapp/[locale]/categories/listing-categories.tsxapp/api/admin/sponsor-ads/[id]/reject/route.tsapp/[locale]/admin/sponsorships/page.tsxapp/[locale]/admin/collections/page.tsxapp/api/payment/[subscriptionId]/route.ts
app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Next.js App Router under app/[locale]/** and app/api/**
Files:
app/api/extract/route.tsapp/api/admin/collections/route.tsapp/api/items/[slug]/views/route.tsapp/api/sponsor-ads/user/[id]/cancel/route.tsapp/api/admin/sponsor-ads/route.tsapp/api/polar/webhook/handlers.tsapp/api/admin/collections/[id]/route.tsapp/[locale]/categories/page.tsxapp/api/sponsor-ads/user/route.tsapp/api/sponsor-ads/route.tsapp/api/sponsor-ads/checkout/route.tsapp/api/admin/sponsor-ads/[id]/route.tsapp/api/stripe/webhook/route.tsapp/[locale]/categories/listing-categories.tsxapp/api/admin/sponsor-ads/[id]/reject/route.tsapp/[locale]/admin/sponsorships/page.tsxapp/[locale]/admin/collections/page.tsxapp/api/payment/[subscriptionId]/route.ts
app/api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
For API routes: put shared logic in lib/services or lib/repositories; keep handlers thin with validation, service call, and result mapping to HTTP response
Files:
app/api/extract/route.tsapp/api/admin/collections/route.tsapp/api/items/[slug]/views/route.tsapp/api/sponsor-ads/user/[id]/cancel/route.tsapp/api/admin/sponsor-ads/route.tsapp/api/polar/webhook/handlers.tsapp/api/admin/collections/[id]/route.tsapp/api/sponsor-ads/user/route.tsapp/api/sponsor-ads/route.tsapp/api/sponsor-ads/checkout/route.tsapp/api/admin/sponsor-ads/[id]/route.tsapp/api/stripe/webhook/route.tsapp/api/admin/sponsor-ads/[id]/reject/route.tsapp/api/payment/[subscriptionId]/route.ts
🧠 Learnings (4)
📚 Learning: 2025-11-30T21:22:22.296Z
Learnt from: CR
Repo: ever-works/ever-works-website-template PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-30T21:22:22.296Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Keep i18n-friendly: avoid hard-coded English strings in logic; use next-intl messages where relevant
Applied to files:
components/billing/expired-plan-banner.tsxcomponents/collections/collection-detail.tsx
📚 Learning: 2025-11-24T18:35:04.571Z
Learnt from: CR
Repo: ever-works/ever-works-website-template PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T18:35:04.571Z
Learning: Applies to app/**/*.{ts,tsx,js,jsx} : Use Next.js App Router under app/[locale]/** and app/api/**
Applied to files:
components/categories-grid.tsx
📚 Learning: 2025-11-30T21:22:22.296Z
Learnt from: CR
Repo: ever-works/ever-works-website-template PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-30T21:22:22.296Z
Learning: Applies to app/api/**/*.{ts,tsx} : For API routes: put shared logic in lib/services or lib/repositories; keep handlers thin with validation, service call, and result mapping to HTTP response
Applied to files:
app/api/admin/collections/route.tsapp/api/admin/collections/[id]/route.tsapp/api/admin/sponsor-ads/[id]/route.ts
📚 Learning: 2025-11-24T18:34:28.233Z
Learnt from: CR
Repo: ever-works/ever-works-website-template PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T18:34:28.233Z
Learning: Applies to app/**/*.{ts,tsx} : Routes and pages should be placed in app/[locale]/** and app/api/** directories following Next.js App Router conventions
Applied to files:
app/[locale]/categories/page.tsx
🧬 Code graph analysis (9)
components/categories-grid.tsx (2)
lib/paginate.ts (1)
totalPages(3-5)components/universal-pagination.tsx (1)
UniversalPagination(10-72)
components/collections/collection-card.tsx (2)
types/collection.ts (1)
Collection(4-15)components/collections/index.ts (1)
CollectionCard(1-1)
app/api/items/[slug]/views/route.ts (1)
lib/repositories/item.repository.ts (1)
ItemRepository(6-283)
app/api/sponsor-ads/user/[id]/cancel/route.ts (4)
app/api/admin/sponsor-ads/[id]/reject/route.ts (1)
POST(61-124)app/api/sponsor-ads/checkout/route.ts (1)
POST(101-239)app/api/sponsor-ads/user/route.ts (1)
POST(147-203)app/api/admin/sponsor-ads/[id]/approve/route.ts (1)
POST(42-118)
app/api/sponsor-ads/route.ts (2)
lib/services/sponsor-ad.service.ts (1)
sponsorAdService(431-431)lib/logger.ts (1)
error(113-123)
app/api/sponsor-ads/checkout/route.ts (2)
lib/logger.ts (1)
error(113-123)lib/payment/types/payment-types.ts (1)
CheckoutSessionParams(332-334)
app/[locale]/categories/listing-categories.tsx (3)
components/home-two/home-two-layout.tsx (1)
HomeTwoLayout(44-83)components/home-two/index.ts (1)
HomeTwoLayout(6-6)lib/content.ts (1)
Category(123-125)
components/admin/collections/assign-items-modal.tsx (6)
hooks/use-admin-items.ts (1)
useAdminItems(114-261)lib/logger.ts (1)
error(113-123)hooks/use-toast.ts (1)
toast(193-193)components/ui/modal.tsx (5)
Modal(21-139)ModalContent(141-143)ModalHeader(145-147)ModalBody(149-151)ModalFooter(153-155)lib/paginate.ts (1)
totalPages(3-5)components/universal-pagination.tsx (1)
UniversalPagination(10-72)
app/[locale]/admin/collections/page.tsx (10)
types/collection.ts (1)
Collection(4-15)hooks/use-admin-collections.ts (1)
useAdminCollections(99-219)lib/api/server-api-client.ts (1)
apiUtils(476-530)lib/repositories/collection.repository.ts (1)
assignItems(145-211)lib/editor/components/primitive/card/card.tsx (1)
CardBody(74-74)lib/paginate.ts (1)
totalPages(3-5)components/universal-pagination.tsx (1)
UniversalPagination(10-72)scripts/clone.cjs (1)
path(5-5)components/admin/collections/collection-form.tsx (1)
CollectionForm(16-180)components/admin/collections/assign-items-modal.tsx (1)
AssignItemsModal(20-186)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Greptile Review
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Lint and Build
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (36)
app/api/items/[slug]/views/route.ts (2)
44-51: LGTM! Item existence validation improves security.The pre-check to validate item existence before recording views is well-implemented and aligns with the PR's security objectives. Returning a 404 for non-existent items prevents invalid view records and the generic error message appropriately avoids leaking system details.
53-57: LGTM! Efficient owner exclusion using consolidated item fetch.The refactored owner-exclusion logic correctly reuses the item fetched earlier, eliminating the redundant fetch mentioned in the summary. The flow is clear and the session/ownership check is properly positioned before cookie and view-recording operations.
app/api/extract/route.ts (1)
140-149: Robust error handling for platform API failures.The improved error handling gracefully handles multiple failure scenarios with a well-designed fallback chain: attempts JSON parsing first, falls back to status text, and uses a default message as the final fallback. Preserving the original status code is appropriate for a proxy endpoint.
components/collections/collection-card.tsx (5)
1-7: LGTM!The imports are correctly organized, and the 'use client' directive is appropriately placed for this interactive component using hooks.
14-16: LGTM!The hook setup is clean and follows React best practices. The
isNavigatingstate andpathnametracking enable proper spinner behavior during navigation.
18-21: LGTM!The useEffect properly handles cleanup when the pathname changes, preventing the spinner from getting stuck during back navigation or other route changes.
26-31: LGTM!The click handler correctly prevents the navigation spinner from showing when users open the link in a new tab (Cmd/Ctrl+click) or use other navigation modifiers. This provides an excellent user experience.
131-135: LGTM!The spinner overlay is properly implemented with good visual feedback during navigation. The backdrop blur and z-index ensure it's clearly visible to users.
components/billing/expired-plan-banner.tsx (1)
78-78: Good i18n improvement for accessibility.The change from a hardcoded "Dismiss" string to
tCommon('CLOSE')correctly follows i18n best practices. Based on learnings, this ensures the aria-label is translatable for non-English locales.Also applies to: 241-241
components/collections/collection-detail.tsx (2)
135-135: Good i18n consistency for filter status messages.The translation hook and keys (
SHOWING_TOTAL_ITEMS,FILTER_STATUS) are correctly implemented with proper interpolation parameters. This aligns with the broader i18n effort in this PR.Also applies to: 228-230
389-393: Consistent i18n usage across both layout variants.The HOME_ONE layout correctly mirrors the i18n approach used in
CollectionHomeTwoFilters, ensuring consistent user experience across layout options.components/admin/collections/assign-items-modal.tsx (2)
62-73: Good error handling and modal closing logic.The modal now only closes on successful save (
onClose()called inside the success path), which aligns with the PR objective for modal closing logic fixes. Error messages are properly displayed via toast.
40-46: Helpful comment documenting refetch behavior.The comment clarifying that React Query handles page/search changes via query key is good documentation for future maintainers.
app/api/admin/collections/[id]/route.ts (2)
238-252: Good cache invalidation strategy for slug changes.The pre-fetch before update enables proper cache invalidation when slugs change. Revalidating both old and new slug paths ensures users don't see stale content.
322-326: Good: Explicit 404 before delete operation.Fetching the collection before deletion ensures proper error handling and enables slug-based cache invalidation. This aligns with the PR's item-existence validation improvements.
app/api/admin/collections/route.ts (1)
284-293: Good JSON parsing validation.The try/catch around
request.json()properly handles malformed request bodies with a 400 response, aligning with the PR's objective for JSON parsing validation. The error is logged for debugging without leaking details to clients.app/[locale]/categories/page.tsx (1)
23-30: Simplified data flow for categories page.The page now correctly fetches only what's needed (categories) and passes a minimal prop surface to
ListingCategories. This aligns with the related changes inlisting-categories.tsxthat removed pagination support.app/[locale]/categories/listing-categories.tsx (1)
12-16: Clean interface simplification.The
ListingCategoriesPropsinterface is now minimal and focused, containing only the necessary props after removing pagination support.app/[locale]/admin/collections/page.tsx (2)
36-51: Good approach for accurate global stats.Fetching all collections separately for stats ensures the dashboard displays accurate totals regardless of pagination, which was a bug fix objective in this PR.
76-90: Modal closes only on successful operations.The form submission handlers now correctly close the modal only after successful create/update operations, aligning with the PR's modal closing logic fix objective.
app/api/payment/[subscriptionId]/route.ts (1)
22-28: Excellent error handling for malformed JSON.The try-catch wrapper prevents unhandled exceptions from bubbling up as 500 errors and provides a clear, secure 400 response. This aligns well with the PR's security objectives.
components/categories-grid.tsx (3)
36-39: Good use of functional update pattern.The conditional state update prevents unnecessary re-renders when the page value from the URL matches the current state. This is a solid performance optimization.
101-118: Excellent pagination handler implementation.The callback correctly:
- Guards against non-standard pagination types
- Manages URL parameters (removes 'page' when returning to page 1)
- Controls scroll behavior with
scroll: falsefollowed by manual smooth scrolling- Includes all dependencies in the
useCallbackarray
229-232: Clean integration of standard pagination.The UniversalPagination component is correctly integrated with proper conditional rendering and appropriate props. The placement after the grid and the check for
totalPages > 1ensure a good user experience.app/api/sponsor-ads/route.ts (1)
53-56: Robust limit validation implemented.The validation logic correctly handles edge cases:
Number.isFinite()rejectsNaN,Infinity, and-InfinityMath.floor()handles decimal inputsMath.min/maxclamps to valid range[1, 50]- Non-numeric strings fall back to default
10app/api/stripe/webhook/route.ts (1)
625-638: Extended metadata inspection improves compatibility with various Stripe event shapes.The function now correctly checks metadata from three possible locations: top-level
data.metadata,data.subscription_data.metadata, anddata.subscription.metadata. This handles different Stripe webhook payload variants (checkout sessions, invoices, subscriptions).app/api/sponsor-ads/checkout/route.ts (2)
28-58: Solid implementation for open redirect prevention.The
validateRedirectUrlfunction correctly:
- Resolves relative URLs against the allowed origin
- Validates protocol, hostname, and port match
- Handles malformed URLs gracefully by returning
null- Rejects protocol-relative URLs like
//evil.com/path(URL constructor resolves tohttps://evil.com/path, which fails hostname check)This security hardening aligns with the PR objectives.
378-380: Customer ID validation now consistent with Stripe checkout.The Polar checkout now validates
customerIdbefore proceeding, matching the Stripe checkout behavior (lines 291-293). This consistency improvement aligns with the PR objectives.app/api/polar/webhook/handlers.ts (1)
492-507: Improved error handling for sponsor ad operations.The sponsor ad handlers now explicitly throw errors when operations fail, allowing the webhook handler to return an error response. This enables payment providers to retry failed webhooks, improving reliability.
app/api/sponsor-ads/user/route.ts (1)
155-161: Good addition of JSON parsing guard.Explicit handling of malformed JSON with a 400 response aligns with the PR's security improvement goals. This prevents raw parsing errors from being exposed.
app/api/admin/sponsor-ads/[id]/reject/route.ts (1)
106-123: Well-structured error handling with appropriate status codes.The error handling correctly differentiates:
- 400 for validation errors ("Cannot reject")
- 404 for not found errors
- 500 with generic message for unexpected errors
This pattern is consistent with the PR's security improvement goals.
app/api/admin/sponsor-ads/route.ts (1)
32-32: No changes needed—the status enum is correct as designed.The sponsor-ad workflow never included an "approved" status. The enum correctly implements a two-stage process:
pending_payment(user submission awaiting payment) andpending(payment received, awaiting admin review), withactivestatus when an admin approves it. This design is consistent across the type definition (lib/types/sponsor-ad.ts), database schema (lib/db/schema.ts), Swagger documentation, and validation array.The "approved" status exists only in the separate item submission workflow, not in the sponsor-ads system. The Swagger enum on line 32 and validStatuses array on line 89 are correctly aligned.
Likely an incorrect or invalid review comment.
app/api/admin/sponsor-ads/[id]/route.ts (1)
30-56: LGTM! Next.js 15 async params correctly implemented.The GET handler properly implements Next.js 15's async request APIs by typing
paramsasPromise<{ id: string }>and awaiting it before use. The admin authentication check, error handling, and generic error messages align well with the PR's security objectives.app/[locale]/admin/sponsorships/page.tsx (3)
64-67: LGTM! Pagination reset on search change.The unconditional page reset when the search term changes (including when cleared) provides consistent UX behavior and aligns with standard pagination patterns.
74-80: LGTM! useCallback refactoring improves performance.Wrapping handlers in
useCallbackwith correct dependencies prevents unnecessary rerenders of child components. All dependency arrays are complete and appropriate.Also applies to: 90-100, 136-142, 144-154, 156-161
102-109: Error handling is correct and appropriately implemented.The three verification points are all confirmed:
approveSponsorAdreturns the expected type:{ success: boolean; requiresForceApprove?: boolean }- Error handling is sound — the hook wraps mutations in try-catch, returns
success: falsefor failures, and deliberately skips toast notifications forPAYMENT_NOT_RECEIVED(handled as a UI flow via the modal)- User feedback is provided: success toast ("Sponsor ad approved and activated"), error toasts for failures (except PAYMENT_NOT_RECEIVED), and the button shows
isLoading={isSubmitting}stateThe conditional modal closing (only closing when
result.successis true) correctly prevents the modal from dismissing on failed operations, which improves the user experience by keeping the context visible and allowing retry.
Greptile SummaryThis PR implements security hardening, bug fixes, and i18n improvements across checkout flows, webhooks, admin pages, and UI components. Security Enhancements:
Bug Fixes:
i18n Improvements:
Code Quality:
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant CheckoutAPI as Checkout API
participant Validator as URL Validator
participant Provider as Payment Provider
participant Webhook as Webhook Handler
participant SponsorService as Sponsor Ad Service
User->>CheckoutAPI: POST /api/sponsor-ads/checkout
Note over CheckoutAPI: Auth & ownership checks
CheckoutAPI->>Validator: validateRedirectUrl(successUrl)
Validator->>Validator: Parse URL & check origin
alt URL from different origin
Validator-->>CheckoutAPI: null (rejected)
Note over CheckoutAPI: Log security warning
CheckoutAPI->>CheckoutAPI: Use default URL
else URL from same origin
Validator-->>CheckoutAPI: validated URL
end
CheckoutAPI->>Validator: validateRedirectUrl(cancelUrl)
Validator-->>CheckoutAPI: validated/null
alt Stripe Provider
CheckoutAPI->>Provider: getCustomerId(user)
Provider-->>CheckoutAPI: customerId
CheckoutAPI->>Provider: create checkout session
else Polar Provider
CheckoutAPI->>Provider: getCustomerId(user)
Note over CheckoutAPI: Now validates customer ID exists
alt Customer ID missing
CheckoutAPI-->>User: 500 error (generic message)
end
Provider-->>CheckoutAPI: customerId
CheckoutAPI->>Provider: create checkout session
end
Note over CheckoutAPI,Provider: Metadata includes type='sponsor_ad'
Provider-->>CheckoutAPI: checkout URL
CheckoutAPI-->>User: Checkout session created
User->>Provider: Complete payment
Provider->>Webhook: Webhook event (payment succeeded)
Webhook->>Webhook: Check metadata.type or subscription.metadata.type
alt type === 'sponsor_ad'
Webhook->>SponsorService: confirmPayment(sponsorAdId)
Note over SponsorService: Activates sponsor ad
SponsorService-->>Webhook: Success
else Regular subscription
Webhook->>Webhook: Handle as subscription
end
Webhook-->>Provider: 200 OK
|
- Add missing 404 handling in cancel sponsor ad route - Complete internationalization of assign items modal - Refactor categories-grid to use exported totalPages function
There was a problem hiding this comment.
1 issue found across 8 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="app/[locale]/admin/collections/page.tsx">
<violation number="1" location="app/[locale]/admin/collections/page.tsx:137">
P2: Concatenating numbers with translated strings breaks i18n word order. Consider using interpolation like `t('TOTAL_COUNT', { count: total })` to allow translators to control the full phrase structure.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/api/sponsor-ads/checkout/route.ts (1)
109-114: Add JSON parsing validation with 400 response for malformed requests.The PR description states "Add JSON parsing validation with proper 400 responses for malformed requests," but the current implementation doesn't wrap
request.json()in a try-catch block. If the request contains malformed JSON, it will throw and be caught by the outer catch block (line 232), returning a 500 status instead of the expected 400 status.Additionally, per coding guidelines, input should be validated with Zod schemas from
lib/validations.🔎 Proposed fix to add JSON parsing validation
+ let body; + try { + body = await request.json(); + } catch { + return NextResponse.json( + { success: false, error: 'Invalid JSON in request body' }, + { status: 400 } + ); + } + - const body = await request.json(); const { sponsorAdId, successUrl, cancelUrl } = body; if (!sponsorAdId) { return NextResponse.json({ success: false, error: 'Sponsor ad ID is required' }, { status: 400 }); }
🧹 Nitpick comments (1)
app/[locale]/admin/collections/page.tsx (1)
46-46: Consider API optimization for statistics.Fetching all collections (up to 1000) on every page load just to compute two aggregate numbers is inefficient. This creates unnecessary network overhead and client-side processing.
Consider adding a dedicated
/api/admin/collections/statsendpoint that returns pre-computed or efficiently calculated aggregate statistics:// Server-side (proposed API) GET /api/admin/collections/stats Response: { total: number, active: number, totalItemsAssigned: number } // Client-side usage const { data: stats } = useQuery({ queryKey: ['admin', 'collections', 'stats'], queryFn: async () => { const response = await serverClient.get('/api/admin/collections/stats'); return response.data; } });This would be more scalable and performant, especially as the number of collections grows.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/[locale]/admin/collections/page.tsxapp/api/payment/[subscriptionId]/route.tsapp/api/sponsor-ads/checkout/route.tsapp/api/sponsor-ads/user/[id]/cancel/route.tscomponents/admin/collections/assign-items-modal.tsxcomponents/categories-grid.tsxcomponents/collections/collection-card.tsxmessages/en.json
🚧 Files skipped from review as they are similar to previous changes (3)
- components/categories-grid.tsx
- app/api/sponsor-ads/user/[id]/cancel/route.ts
- app/api/payment/[subscriptionId]/route.ts
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Node.js >= 20.19.0
Files:
components/admin/collections/assign-items-modal.tsxapp/[locale]/admin/collections/page.tsxcomponents/collections/collection-card.tsxapp/api/sponsor-ads/checkout/route.ts
components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
components/**/*.{ts,tsx}: UI components should be placed in the components/** directory
Keep React components mostly presentational and lean, avoiding complex business logic
Files:
components/admin/collections/assign-items-modal.tsxcomponents/collections/collection-card.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Prefer TypeScript files over JavaScript
Run pnpm tsc --noEmit for TypeScript type-checking
Files:
components/admin/collections/assign-items-modal.tsxapp/[locale]/admin/collections/page.tsxcomponents/collections/collection-card.tsxapp/api/sponsor-ads/checkout/route.ts
components/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Place UI and layout components in components/**
Files:
components/admin/collections/assign-items-modal.tsxcomponents/collections/collection-card.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer TypeScript for all code
Files:
components/admin/collections/assign-items-modal.tsxapp/[locale]/admin/collections/page.tsxcomponents/collections/collection-card.tsxapp/api/sponsor-ads/checkout/route.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: Follow Prettier config with tabs, 4-space tabWidth, and 120-char printWidth
Prefer async/await over raw Promise chains in TypeScript/JavaScript code
Validate input with Zod where appropriate; follow existing schemas in lib/validations
Keep i18n-friendly: avoid hard-coded English strings in logic; use next-intl messages where relevant
Files:
components/admin/collections/assign-items-modal.tsxapp/[locale]/admin/collections/page.tsxcomponents/collections/collection-card.tsxapp/api/sponsor-ads/checkout/route.ts
**/components/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/components/**/*.{jsx,tsx}: For forms, prefer react-hook-form + Zod; follow patterns in existing auth/profile forms
Place business logic in lib/services or lib/repositories, not in components; keep components mostly presentational and data-fetching
Files:
components/admin/collections/assign-items-modal.tsxcomponents/collections/collection-card.tsx
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Routes and pages should be placed in app/[locale]/** and app/api/** directories following Next.js App Router conventions
Files:
app/[locale]/admin/collections/page.tsxapp/api/sponsor-ads/checkout/route.ts
app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Next.js App Router under app/[locale]/** and app/api/**
Files:
app/[locale]/admin/collections/page.tsxapp/api/sponsor-ads/checkout/route.ts
app/api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
For API routes: put shared logic in lib/services or lib/repositories; keep handlers thin with validation, service call, and result mapping to HTTP response
Files:
app/api/sponsor-ads/checkout/route.ts
🧠 Learnings (1)
📚 Learning: 2025-11-30T21:22:22.296Z
Learnt from: CR
Repo: ever-works/ever-works-website-template PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-30T21:22:22.296Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Keep i18n-friendly: avoid hard-coded English strings in logic; use next-intl messages where relevant
Applied to files:
components/admin/collections/assign-items-modal.tsxapp/[locale]/admin/collections/page.tsxcomponents/collections/collection-card.tsx
🧬 Code graph analysis (3)
app/[locale]/admin/collections/page.tsx (5)
types/collection.ts (1)
Collection(4-15)hooks/use-admin-collections.ts (1)
useAdminCollections(99-219)lib/api/server-api-client.ts (2)
serverClient(452-452)apiUtils(476-530)lib/repositories/collection.repository.ts (1)
assignItems(145-211)components/universal-pagination.tsx (1)
UniversalPagination(10-72)
components/collections/collection-card.tsx (1)
types/collection.ts (1)
Collection(4-15)
app/api/sponsor-ads/checkout/route.ts (2)
lib/services/sponsor-ad.service.ts (1)
sponsorAdService(431-431)lib/payment/types/payment-types.ts (1)
CheckoutSessionParams(332-334)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Lint and Build
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
app/api/sponsor-ads/checkout/route.ts (4)
28-58: LGTM! Strong security implementation.The URL validation helper effectively prevents open redirect vulnerabilities by verifying that URLs belong to the application domain. The implementation correctly:
- Uses URL constructor for parsing
- Validates protocol, hostname, and port match
- Returns null for invalid formats
- Has appropriate error handling
158-179: LGTM! Excellent security hardening.The URL validation implementation properly prevents open redirect vulnerabilities by:
- Validating both success and cancel URLs against the allowed origin
- Logging security warnings with context (userId, sponsorAdId) when invalid URLs are provided
- Falling back to safe default URLs when validation fails
This aligns well with the PR's security objectives.
232-237: LGTM! Past security concerns addressed.The error handling now correctly uses a generic error message for 500 responses, avoiding exposure of sensitive details to clients. The full error is still logged to console for debugging purposes, which is the right approach.
This resolves the concerns raised in previous review comments from cubic-dev-ai[bot] and coderabbitai[bot].
377-379: LGTM! Good validation addition.The customer ID validation for Polar checkout ensures that a valid customer exists before creating a subscription. This is consistent with the Stripe implementation (lines 290-292) and prevents potential issues with missing or invalid customer references.
components/collections/collection-card.tsx (1)
1-138: LGTM! Previous i18n issue has been resolved.The hardcoded "View collection" string has been properly replaced with
t('VIEW_COLLECTION'). The component now follows i18n best practices throughout, and the navigation spinner implementation with proper guards for middle-click and modifier keys is well done.messages/en.json (1)
92-152: LGTM! Translation keys added for collection management.The new translation keys properly support the internationalization requirements for collection management features. All entries follow the established pattern and naming conventions.
components/admin/collections/assign-items-modal.tsx (1)
1-187: LGTM! All hardcoded strings have been properly internationalized.All previously flagged hardcoded strings have been replaced with appropriate translation keys using both
commonandlistingnamespaces. The component now follows i18n best practices, and the logic for state management, item selection, and data fetching is well-structured.app/[locale]/admin/collections/page.tsx (1)
100-118: Good implementation of assign flow with proper fallback logic.The
handleAssignfunction properly prefers items already stored on the collection, with a sensible fallback to fetch if not present. ThehandleAssignSaveimplementation is clean and delegates to the hook properly.
- Increase max limit for collections endpoint (100 -> 1000) - Add missing 'EDIT' translation key in common section - Improve pagination parameter validation for collections
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
app/api/admin/clients/dashboard/route.ts (2)
253-256: Consider extracting pagination validation to a shared utility.The pagination validation logic is robust with proper NaN handling and bounds checking. However, similar validation appears in other admin routes (e.g.,
app/api/admin/collections/route.tslines 136-153).💡 Potential refactor to reduce duplication
Consider creating a shared utility function in
lib/validationsorlib/utils:export function validatePagination( page: string | null, limit: string | null, maxLimit: number = 100 ): { page: number; limit: number } | { error: string } { const rawPage = Number(page ?? 1); const rawLimit = Number(limit ?? 10); const validPage = Number.isFinite(rawPage) && rawPage > 0 ? Math.floor(rawPage) : 1; const validLimit = Number.isFinite(rawLimit) ? Math.min(Math.max(1, Math.floor(rawLimit)), maxLimit) : 10; return { page: validPage, limit: validLimit }; }Then use it in multiple routes:
const { page, limit } = validatePagination( searchParams.get('page'), searchParams.get('limit'), 100 );
266-284: Consider extracting date parsing utility for reusability.The
parseDateBoundfunction provides robust date handling with UTC normalization and is well-implemented. If date range filtering is used in other admin endpoints, consider extracting this to a shared utility module.💡 Suggested location
Create
lib/utils/date-parsing.ts:/** * Parse date parameter for filtering with proper UTC handling. * Treats YYYY-MM-DD as date-only in UTC. * @param v - Date string from query parameter * @param bound - 'start' for beginning of day, 'end' for end of day * @returns Parsed Date or undefined if invalid */ export function parseDateBound(v: string | null, bound: 'start' | 'end'): Date | undefined { if (!v) return undefined; // Handle YYYY-MM-DD format as UTC date-only if (/^\d{4}-\d{2}-\d{2}$/.test(v)) { const [y, m, d] = v.split('-').map(Number); return new Date( Date.UTC( y, m - 1, d, bound === 'end' ? 23 : 0, bound === 'end' ? 59 : 0, bound === 'end' ? 59 : 0, bound === 'end' ? 999 : 0 ) ); } // Fallback to generic Date parsing const d = new Date(v); return Number.isNaN(d.getTime()) ? undefined : d; }app/api/admin/collections/route.ts (1)
136-153: Inline pagination validation duplicates logic.The pagination validation logic here is similar to the validation in
app/api/admin/clients/dashboard/route.ts(lines 253-256), though with different approaches—this uses explicit 400 responses while the dashboard route coerces invalid values.Consider the shared utility approach suggested in the dashboard route review to standardize pagination handling across admin endpoints.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/api/admin/clients/dashboard/route.tsapp/api/admin/collections/route.tsmessages/en.json
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Node.js >= 20.19.0
Files:
app/api/admin/collections/route.tsapp/api/admin/clients/dashboard/route.ts
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Routes and pages should be placed in app/[locale]/** and app/api/** directories following Next.js App Router conventions
Files:
app/api/admin/collections/route.tsapp/api/admin/clients/dashboard/route.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Prefer TypeScript files over JavaScript
Run pnpm tsc --noEmit for TypeScript type-checking
Files:
app/api/admin/collections/route.tsapp/api/admin/clients/dashboard/route.ts
app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Next.js App Router under app/[locale]/** and app/api/**
Files:
app/api/admin/collections/route.tsapp/api/admin/clients/dashboard/route.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer TypeScript for all code
Files:
app/api/admin/collections/route.tsapp/api/admin/clients/dashboard/route.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: Follow Prettier config with tabs, 4-space tabWidth, and 120-char printWidth
Prefer async/await over raw Promise chains in TypeScript/JavaScript code
Validate input with Zod where appropriate; follow existing schemas in lib/validations
Keep i18n-friendly: avoid hard-coded English strings in logic; use next-intl messages where relevant
Files:
app/api/admin/collections/route.tsapp/api/admin/clients/dashboard/route.ts
app/api/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
For API routes: put shared logic in lib/services or lib/repositories; keep handlers thin with validation, service call, and result mapping to HTTP response
Files:
app/api/admin/collections/route.tsapp/api/admin/clients/dashboard/route.ts
🧠 Learnings (1)
📚 Learning: 2025-11-30T21:22:22.296Z
Learnt from: CR
Repo: ever-works/ever-works-website-template PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-30T21:22:22.296Z
Learning: Applies to app/api/**/*.{ts,tsx} : For API routes: put shared logic in lib/services or lib/repositories; keep handlers thin with validation, service call, and result mapping to HTTP response
Applied to files:
app/api/admin/collections/route.ts
🧬 Code graph analysis (2)
app/api/admin/collections/route.ts (3)
types/collection.ts (2)
CollectionListOptions(36-43)CreateCollectionRequest(17-24)lib/repositories/collection.repository.ts (1)
collectionRepository(269-269)lib/cache-invalidation.ts (1)
invalidateContentCaches(57-70)
app/api/admin/clients/dashboard/route.ts (2)
lib/auth/admin-guard.ts (1)
checkAdminAuth(9-47)lib/db/queries/client.queries.ts (1)
getAdminDashboardData(633-842)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Greptile Review
🔇 Additional comments (7)
messages/en.json (3)
92-92: LGTM! Well-placed translation key.The
VIEW_COLLECTIONkey is appropriately named, clearly worded, and logically positioned near related collection navigation keys.
190-190: LGTM! Generic action key appropriately placed.The
EDITkey fills a gap in generic action translations and is correctly positioned alongside other common actions (CANCEL, DELETE, SAVING). This generic placement allows reuse across different UI contexts.
138-152: Collection management translations are well-structured and properly implemented.The translation keys follow consistent naming conventions and are clear and user-friendly. Both parameterized keys are correctly consumed in their respective components:
SELECTED_COUNTis properly interpolated inassign-items-modal.tsxwith the count parameterDELETE_COLLECTION_CONFIRMis properly interpolated in the collections admin page with the collection nameThe i18n implementation correctly follows next-intl patterns.
app/api/admin/clients/dashboard/route.ts (2)
244-247: LGTM! Consistent auth pattern.The route correctly uses
checkAdminAuth()for authentication, which aligns with the auth-first pattern recommended in the PR objectives and implemented across other admin routes.
329-330: LGTM! Appropriate error handling for security.The generic error message prevents information leakage while proper error logging is maintained. This aligns with the PR's security hardening objectives.
app/api/admin/collections/route.ts (2)
297-303: Excellent security practice with JSON parsing guard.The try-catch block around
request.json()with a 400 response for malformed JSON is a good security practice that prevents cryptic error messages and aligns with the PR's objective to "validate JSON parsing with 400 responses for malformed requests."
331-342: LGTM! Well-differentiated error handling.The error handling appropriately distinguishes between:
- 409 for "already exists" conflicts
- 400 for validation errors (containing "must")
- 500 for generic server errors
This aligns with the PR's objective to "standardize API error handling."
| > | ||
| {layoutHome === LayoutHome.HOME_ONE && <HomeOneLayout categories={props.categories} />} | ||
| {layoutHome === LayoutHome.HOME_TWO && <HomeTwoLayout categories={props.categories} />} | ||
| {paginationType === 'standard' && totalPages(props.categories.length) > 1 && ( |
| const start = 0; | ||
| const basePath = "/categories"; | ||
| // Calculate pagination info | ||
| const page = 1; |
There was a problem hiding this comment.
@Innocent-Akim why pagination REMOVED!??? What happened with "tags" that was passed to listing categories component?
| .optional(), | ||
| icon_url: z.string().trim().optional(), | ||
| isActive: z.boolean().optional() | ||
| name: z |
There was a problem hiding this comment.
@Innocent-Akim not sure but somehow your PRs contain a lot of reformatting without actual changes, can it be that either you or other devs using different formatting style?
|
@Innocent-Akim I merged with Squash, if need more fixes please base on develop branch |
Security improvements:
Bug fixes:
i18n improvements:
Code quality:
Affected areas:
Summary
Related Issues
Description
Changes Made
Screenshots / Videos
Type of Change
Implementation Details
Testing Instructions
Deployment Notes
Checklist
Additional Context
Summary by cubic
Strengthens security across checkout, webhooks, and API routes, fixes pagination and admin inconsistencies, and aligns UI with i18n. Also prevents bad analytics by validating items before counting views.
Bug Fixes
Refactors
Written for commit 024e951. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Enhancements
✏️ Tip: You can customize this high-level summary in your review settings.