Fix/linting errors #9
Conversation
… handling and data structures
… handling and data structures
WalkthroughThis PR modernizes API route handlers by updating to Promise-based dynamic route parameters (Next.js 15 pattern), introduces structured type definitions for checkout requests and API responses, hardens error handling with safe JSON parsing, improves time value normalization, enhances gift flow processing, refactors category/experience data fetching with fallback strategies, replaces anchor tags with Next.js Link components, and improves overall type safety throughout the codebase. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant OldHandler as Old API Handler
participant NewHandler as New API Handler
participant Backend
Note over OldHandler: Handler({ params: { id } })
Client->>OldHandler: PATCH /api/reviews/[id]/helpful
OldHandler->>OldHandler: Use params.id directly
OldHandler->>Backend: Fetch with id
Backend-->>OldHandler: Response
OldHandler->>OldHandler: response.json() (unsafe)
OldHandler-->>Client: Result
Note over NewHandler: Handler(Request, context: { params: Promise<{id}> })
Client->>NewHandler: PATCH /api/reviews/[id]/helpful
NewHandler->>NewHandler: await context.params → { id }
NewHandler->>Backend: Fetch with id
Backend-->>NewHandler: Response
NewHandler->>NewHandler: parseJsonSafely(response)
Note right of NewHandler: Safe extraction with fallback
NewHandler-->>Client: Result
sequenceDiagram
participant Client
participant CheckoutAPI as POST /api/checkout
participant BookingBackend as Booking Service
participant GiftBackend as Gift Service
participant Response
Client->>CheckoutAPI: POST with items, customer, gift details
CheckoutAPI->>CheckoutAPI: Type check to CheckoutRequestBody
CheckoutAPI->>CheckoutAPI: Extract giftDetails by itemId if isGift
alt Gift found
CheckoutAPI->>CheckoutAPI: Populate receiverEmail, giftMessage
else Gift not found
CheckoutAPI->>Response: Error 400
end
CheckoutAPI->>CheckoutAPI: Normalize times via normaliseTimeValue
CheckoutAPI->>BookingBackend: POST booking payload
BookingBackend-->>CheckoutAPI: Response (ok/error with message)
alt Booking success & isGift
CheckoutAPI->>GiftBackend: POST gift redemption with cookies
GiftBackend-->>CheckoutAPI: Redemption response
end
CheckoutAPI->>Response: Final 200/error response
Response-->>Client: Result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Areas requiring extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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: 7
🧹 Nitpick comments (20)
src/components/forgot-password-form.tsx (1)
116-121: Replace anchor tags with Next.jsLinkcomponent for internal navigation.For client-side navigation in a Next.js 15 application, use the
Linkcomponent instead of raw<a>tags to enable client-side transitions and avoid full-page reloads.Apply this diff to replace the anchor tags:
import { cn } from "@/lib/utils"; import { Button } from "@/components/ui/button"; +import Link from "next/link"; import { Card, CardContent,Then update the navigation links:
- <a - href="/auth/login" + <Link + href="/auth/login" className="ml-auto text-sm underline-offset-4 hover:underline" > Remember your password? - </a> + </Link>- <a href="/auth/signup" className="underline underline-offset-4"> + <Link href="/auth/signup" className="underline underline-offset-4"> Sign up - </a> + </Link>Also applies to: 137-139
src/app/auth/reset-password/page.tsx (1)
21-23: Correctly addresses the Next.js 15 requirement foruseSearchParams().The Suspense boundary is properly placed around
ResetPasswordForm, which uses theuseSearchParams()hook. This is required by Next.js 15 App Router to handle dynamic rendering and prevents hydration errors.The fallback UI is functional and appropriate for a linting fix. Optionally, you could enhance it to match the Card-based design system used in the form itself for visual consistency, but since the fallback is rarely shown (searchParams are typically available immediately), the current implementation is acceptable.
If you'd like to match the design system more closely, consider this enhancement:
- <Suspense fallback={<div className="text-center text-sm text-muted-foreground">Loading reset form...</div>}> + <Suspense fallback={ + <Card> + <CardContent className="pt-6"> + <div className="text-center text-sm text-muted-foreground"> + Loading reset form... + </div> + </CardContent> + </Card> + }> <ResetPasswordForm /> </Suspense>Note: This would require importing
CardandCardContentfrom your UI components.src/app/gift/redeem/page.tsx (3)
275-281: Clarify the Suspense fallback message.The Suspense boundary is correctly used here (required for
useSearchParamsin Next.js 15), but the fallback message "Loading gift details..." is misleading. This boundary handlesuseSearchParamsinitialization, not the actual gift data fetching—that's managed by internal state (line 63) and displays separately (lines 180-183).Consider a more accurate message:
- <Suspense fallback={<div className="text-center text-sm text-muted-foreground">Loading gift details...</div>}> + <Suspense fallback={<div className="text-center text-sm text-muted-foreground">Loading...</div>}>
92-97: Consider simplifying the error message extraction.The current error parsing logic is defensive but verbose. You can simplify it while maintaining type safety:
- const errorData = await response.json().catch(() => null); - const errorMessage = - errorData && typeof errorData === "object" && "message" in errorData - ? (errorData as { message?: string }).message - : undefined; - throw new Error(errorMessage || "Something went wrong"); + const errorData = await response.json().catch(() => ({})); + const errorMessage = + typeof errorData === "object" && errorData && "message" in errorData && typeof errorData.message === "string" + ? errorData.message + : "Something went wrong"; + throw new Error(errorMessage);Or extract to a helper function if this pattern repeats:
function extractErrorMessage(errorData: unknown): string { if (typeof errorData === "object" && errorData && "message" in errorData && typeof errorData.message === "string") { return errorData.message; } return "Something went wrong"; }
124-130: Extract the inline type definition to improve readability.The inline type assertion on lines 124-126 makes the code harder to read. Consider defining a proper interface:
Add this interface near the top of the file (after the
redeemGiftPropsinterface):interface AvailabilityResponse { availableTimes?: ApiTime[]; fullyBooked?: boolean; }Then simplify the fetch:
- const data = (await response.json().catch(() => null)) as - | { availableTimes?: ApiTime[]; fullyBooked?: boolean } - | null; + const data = (await response.json().catch(() => null)) as AvailabilityResponse | null;src/components/reset-password-form.tsx (1)
26-30: Good type safety improvement.The interface provides clear structure for API responses. The index signature allows flexibility for additional properties.
Consider adding runtime validation using a library like Zod to ensure the parsed JSON actually matches this interface, rather than relying solely on type assertions:
import { z } from 'zod'; const ApiResponseSchema = z.object({ error: z.string().optional(), message: z.string().optional(), }).passthrough(); type ApiResponseData = z.infer<typeof ApiResponseSchema>;Then at line 95, use:
data = ApiResponseSchema.safeParse(await response.json()).data ?? null;src/components/listings/BookingForm.tsx (1)
82-88: useEffect dependency change may cause extra resets when callback identity changesIncluding
onDateChangein the dependency array fixes the lint warning and avoids stale callbacks, but it also means:
setTime(undefined)andonDateChange(date)now run whenever the parent passes a newonDateChangereference, even ifdatehasn’t changed.If the parent passes an inline function (not wrapped in
useCallback), this could unexpectedly clear the selected time on re-renders. Consider:
- Ensuring the parent wraps
onDateChangeinuseCallback, or- Moving the
setTime(undefined)behind a guard that only runs whendateactually changes, if this becomes a UX issue.src/app/api/reviews/[id]/unhelpful/route.ts (1)
4-17: Safe JSON parsing and updated PATCH handler look solid; consider deduplicating helper
parseJsonSafelycorrectly:
- Checks
content-typefor JSON,- Catches
.json()failures (e.g., empty/invalid bodies),- Returns
nullwhen parsing isn’t safe, which is handled viadata?.message ?? default.- Using
await context.paramsand interpolatingidinto the backend URL aligns with the other review routes and keeps the handler consistent.- Success path
NextResponse.json(data ?? {}, { status: 200 })gives a defined payload even for 204/empty responses, which is reasonable.Since
parseJsonSafelyis duplicated in the “helpful” route, consider moving it to a shared utility (e.g.,@/lib/api/parseJsonSafely) to avoid drift in future edits.Also applies to: 19-57
src/components/listings/tabs/ReviewsTab.tsx (1)
185-271: Per‑review helpful/unhelpful handling is consistent and user‑aware
isHelpful/isUnhelpfulcorrectly guard againstuserbeing null and missing arrays via optional chaining and?? false.- The button handlers:
- Redirect unauthenticated users to
/auth/loginbefore doing anything,- Delegate to
onHelpful/onUnhelpfulwhen provided, keeping side effects at a higher level.- Disabling buttons when the current user has already marked helpful/unhelpful is a sensible UX choice.
If this list grows large, you could later extract the mapped block into a small
<ReviewItem>component to avoid redefining handlers per item, but that’s strictly an optional cleanup.src/app/api/reviews/[id]/helpful/route.ts (1)
4-17: Helpful route mirrors unhelpful route patterns; good consistency, minor duplication
parseJsonSafelyand the updated PATCH signature mirror the unhelpful route, which keeps behavior and error handling aligned across both endpoints.- Error responses use a clear default message and surface backend
data?.messagewhen available, which should help debugging without leaking raw responses.As with the unhelpful route, consider extracting
parseJsonSafelyinto a shared helper to avoid divergence between these two files in the future.Also applies to: 20-57
src/app/api/reviews/[id]/route.ts (1)
37-57: PUT/DELETE header forwarding is good; consider reusing safe JSON parsing
- For PUT and DELETE, forwarding
cookieandauthorizationfrom the incoming request improves auth consistency with the helpful/unhelpful endpoints.- All three handlers still use
await response.json()directly. If the backend ever returns non‑JSON or an empty body on error, these calls could throw before you reach the error handling.Given you already have
parseJsonSafelyin the helpful/unhelpful routes, consider:
- Using the same helper here to guard
response.json(), and- Deriving error messages from parsed data when available, with a sensible fallback.
This would make error handling more robust and consistent across all review routes.
Also applies to: 78-97, 70-71, 109-110
src/app/api/checkout/route.ts (1)
2-61: Typed checkout models and error handling are strong; verify error response shape and statusPositives:
CheckoutCustomer,CheckoutItem, andCheckoutRequestBodysignificantly clarify the expected payload shape.normaliseTimeValuecleanly handles bothstringandApiTimeinputs and guarantees a stringselectedTimefor the downstream API.- Duration normalization via
durationValueand theNumber.isFiniteguard preventsNaNfrom leaking into payloads.isApiErrorResponseplusresponse.json().catch(() => null)gives a safer, more informative error message (Failed to create/redeem booking: …) when the backend returns structured errors.Two things to double‑check:
Error response property name and status:
- On failure, the route now returns:
return NextResponse.json({ message }, { status: 500 });- Other API routes in this codebase (e.g., forgot‑password/login) appear to use
{ error: errorMessage }and often propagate the backend status code.- Please verify the frontend callers for
/api/checkoutexpectmessagerather thanerror, and whether they rely on distinguishing 4xx vs 5xx viastatus. If they currently readerroror expect non‑500 statuses from validation failures, this change could silently break error handling.Time normalization contract with the backend:
normaliseTimeValuereturns""whenitem.timeis missing. Confirm the bookings API treats empty string equivalently to “no time provided” and doesn’t requirenullor omission instead.If either of these assumptions doesn’t hold, small tweaks (e.g., returning
{ error }and/or propagatingresponse.status) should resolve it.Also applies to: 63-84, 86-90, 111-116, 133-138, 148-157
src/components/landing/DiscoverSection.tsx (1)
11-20: Slug-based identity is good; key fallback can be simplifiedAdding a required
slugfield and passing it through toListingCardis a solid move toward slug‑based routing/identification. Givenslugis non‑optional,key={exp.slug ?? index}can be simplified tokey={exp.slug}— the?? indexfallback will never trigger under the current type and doesn’t help with duplicate slugs.Also applies to: 23-103, 123-126
src/contexts/CartContext.tsx (1)
4-4: LocalStorage rehydration is well-typed; consider recomputing totalsTyping the persisted shape via
PersistedCartItem/PersistedCartStateand convertingdatestrings back intoDateinstances before dispatchingLOAD_CARTmakes the cart state much safer to work with.As a follow‑up improvement, you could ignore persisted
subtotal/taxes/fees/totalvalues and recompute them fromitemsWithDatesusingcalculateCartTotalswhen loading, so any future changes to pricing logic don’t leave old carts with stale totals.Also applies to: 158-175
src/components/listings/ExperienceDetails.tsx (1)
131-143: Availability parsing is resilient; optionally treat non-OK responses as failuresCatching JSON parse errors and defaulting to
data?.availableTimes ?? experience.availableTimes?.times ?? []plusBoolean(data?.fullyBooked)makes the availability check tolerant of malformed responses.For consistency with your other fetches (which check
response.ok), you might optionally treat non‑OK statuses here as failures (throw on!response.ok), letting thecatchpath handle them and ensuring you never depend on the shape of an error payload.src/app/experiences/category/[slug]/page.tsx (4)
1-6: CategoryType definition and imports are fine; consider centralizing the category listThe union
"activities" | "occasions" | "recipients"matches usages, but it’s repeated later inisCategoryType. You could deriveCategoryTypefrom a singleCATEGORY_TYPESarray to avoid drift if new categories are added.+const CATEGORY_TYPES = ["activities", "occasions", "recipients"] as const; +type CategoryType = (typeof CATEGORY_TYPES)[number]; - -type CategoryType = "activities" | "occasions" | "recipients";And then reuse
CATEGORY_TYPESinisCategoryType.
31-158: Fallback subcategories and metadata are robust; consider moving to a config moduleThe hard‑coded
fallbackSubcategoriesandcategoryMetagive a solid UX when the API fails. The tradeoff is a large literal block here and duplicated image paths between fallback data andfallbackImage. Extracting these objects into a shared config/module would keep this page leaner and make future content tweaks easier.
160-219: Good defensive fetching; watch out for the N+1 experiences counter
getExperienceCountandfetchSubcategoriesare defensive (returning 0 or fallback data on any failure), which is good for resilience. However, callinggetExperienceCountonce per entity yields an N+1 pattern against/experiences/by-category/.... If there are many subcategories, this can become slow or brittle.If the backend can be adjusted, a single endpoint that returns subcategories and their experience counts in one payload (or accepts multiple IDs) would be a worthwhile follow‑up to reduce round‑trips.
234-261: Rendering logic and error handling look solid
- Using
categoryMeta[categorySlug]forSectionHeaderkeeps copy centralized.endpoint={/experiences/subcategory/${categorySlug}/${subCategory.slug}}matches the subcategory page route signature.- On fetch or mapping failure, you log and then
notFound(), which is a reasonable fallback for a category page.Only minor nit:
"${subCategory.experienceCount}+ experiences"will read slightly odd for low counts (e.g.,1+). If you expect small counts, you might want to special‑case singular/plural in a later polish pass.src/app/experiences/subcategory/[mainCategory]/[subCategorySlug]/page.tsx (1)
21-62: Experience → Listing mapper is well-structured; minor UX polish possible
mapExperiencesToListingscleanly centralizes transformation into theListingshape with sensible defaults for missing image, location, duration, and price. This improves readability versus in‑line mapping.Only optional UX tweaks:
- For location, you currently require both
cityandstateto show anything. If the API often has just one of them, you might consider falling back to whichever is present instead of"Location not specified".If you want to tune the location behavior, confirm how often
cityvsstateare populated in your CMS data, then adjust the condition accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (31)
src/app/api/checkout/route.ts(5 hunks)src/app/api/reviews/[id]/helpful/route.ts(2 hunks)src/app/api/reviews/[id]/route.ts(6 hunks)src/app/api/reviews/[id]/unhelpful/route.ts(2 hunks)src/app/api/users/forgot-password/route.ts(2 hunks)src/app/api/users/login/route.ts(2 hunks)src/app/api/users/profile/route.ts(3 hunks)src/app/api/users/reset-password/route.ts(2 hunks)src/app/auth/forgot-password/page.tsx(1 hunks)src/app/auth/reset-password/page.tsx(2 hunks)src/app/auth/signup/page.tsx(1 hunks)src/app/cart/error.tsx(1 hunks)src/app/cart/page.tsx(2 hunks)src/app/checkout/error.tsx(1 hunks)src/app/checkout/page.tsx(3 hunks)src/app/experiences/category/[slug]/page.tsx(3 hunks)src/app/experiences/page.tsx(0 hunks)src/app/experiences/subcategory/[mainCategory]/[subCategorySlug]/page.tsx(4 hunks)src/app/gift/redeem/page.tsx(7 hunks)src/components/ListingCard.tsx(0 hunks)src/components/categories/CategoryPage.tsx(0 hunks)src/components/forgot-password-form.tsx(1 hunks)src/components/landing/DiscoverSection.tsx(10 hunks)src/components/listings/BookingForm.tsx(1 hunks)src/components/listings/ExperienceDetails.tsx(9 hunks)src/components/listings/tabs/LocationTab.tsx(0 hunks)src/components/listings/tabs/ReviewsTab.tsx(2 hunks)src/components/reset-password-form.tsx(3 hunks)src/components/ui/textarea.tsx(2 hunks)src/contexts/CartContext.tsx(2 hunks)src/types/index.ts(1 hunks)
💤 Files with no reviewable changes (4)
- src/components/ListingCard.tsx
- src/app/experiences/page.tsx
- src/components/listings/tabs/LocationTab.tsx
- src/components/categories/CategoryPage.tsx
🧰 Additional context used
🧬 Code graph analysis (11)
src/app/auth/signup/page.tsx (1)
src/app/auth/forgot-password/page.tsx (1)
LoginPage(5-21)
src/app/auth/reset-password/page.tsx (1)
src/components/reset-password-form.tsx (1)
ResetPasswordForm(32-271)
src/app/api/reviews/[id]/unhelpful/route.ts (1)
src/app/api/reviews/[id]/helpful/route.ts (1)
PATCH(20-64)
src/app/gift/redeem/page.tsx (2)
src/types/index.ts (1)
ApiTime(381-385)src/app/auth/reset-password/page.tsx (1)
Page(6-27)
src/app/auth/forgot-password/page.tsx (1)
src/app/auth/signup/page.tsx (1)
LoginPage(5-21)
src/app/api/reviews/[id]/helpful/route.ts (1)
src/app/api/reviews/[id]/unhelpful/route.ts (1)
PATCH(20-64)
src/contexts/CartContext.tsx (1)
src/types/index.ts (1)
CartItem(448-467)
src/app/api/checkout/route.ts (2)
src/types/index.ts (1)
ApiTime(381-385)src/app/api/users/forgot-password/route.ts (1)
POST(10-45)
src/app/experiences/subcategory/[mainCategory]/[subCategorySlug]/page.tsx (2)
src/types/index.ts (1)
Listing(1-12)src/core/config/common.settings.ts (1)
SETTINGS(1-3)
src/app/experiences/category/[slug]/page.tsx (1)
src/core/config/common.settings.ts (1)
SETTINGS(1-3)
src/components/listings/ExperienceDetails.tsx (1)
src/types/index.ts (4)
Review(234-249)ReviewStat(251-259)ApiTime(381-385)ApiExperience(392-432)
🔇 Additional comments (34)
src/components/ui/textarea.tsx (1)
1-23: LGTM! Clean linting fixes.The formatting improvements are appropriate:
- Semicolons added for consistency
- The
interface→typechange on Line 5 is semantically correct sinceTextareaPropsis a simple alias without additional propertiessrc/components/forgot-password-form.tsx (1)
77-77: Revert the apostrophe escape—it will render literally as'in the UI.In JSX text content (not HTML attributes),
'is not decoded and will display as the literal string "'" to users instead of an apostrophe. The original unescaped apostrophe is correct. If a linting tool flagged this, it may be misconfigured for JSX.Apply this diff to revert the change:
- Enter your email address and we'll send you a link to reset your + Enter your email address and we'll send you a link to reset yourLikely an incorrect or invalid review comment.
src/app/auth/reset-password/page.tsx (1)
1-1: LGTM! Required import for Suspense boundary.The Suspense import is correctly added to support wrapping components that use
useSearchParams(), which is required by Next.js 15 App Router to avoid hydration errors.src/app/gift/redeem/page.tsx (2)
102-109: Excellent error handling and fallback logic improvements.The enhanced error handling across the component is well-implemented:
- Safe initialization of
availableTimeswith fallback (line 102)- Graceful error message extraction (lines 104-108)
- Multi-level fallback strategy for availability fetch (lines 127-130)
- Fallback to gift data when availability fetch fails (lines 133-136)
- User-facing error display (lines 175-179)
This ensures the component gracefully degrades and provides helpful feedback to users.
Also applies to: 127-137, 175-179
146-161: LGTM! Booking data construction is correct.The
CartItemconstruction properly maps the gift data and booking selections, with appropriate field assignments:
- Uses
bookingData.pricefor current price- Links back to the original gift via
redeemedBookingId: gift._id- Preserves
oldPricefrom the gift's original amountsrc/components/reset-password-form.tsx (4)
24-24: LGTM!Proper Next.js import for client-side navigation.
92-96: Excellent improvements to JSON parsing.The changes enhance code quality:
- More descriptive variable naming (
contentTypevsct)- Safe JSON parsing with
.catch(() => null)handles malformed responses- Explicit type annotations improve readability
The error handling is robust and prevents crashes from invalid JSON.
99-102: LGTM!Robust error handling with proper fallback chain:
data?.error→data?.message→ default message. The optional chaining safely handles null responses.
257-259: Using Next.js Link component for client-side navigation.The
/auth/loginroute is correctly defined atsrc/app/auth/login/page.tsx. The Link component change enables client-side navigation and prefetching as intended.src/app/auth/forgot-password/page.tsx (1)
2-2: LGTM! Client-side navigation enabled.Replacing the anchor tag with Next.js
Linkcomponent enables faster client-side navigation and follows Next.js best practices.Also applies to: 9-16
src/app/auth/signup/page.tsx (1)
2-2: LGTM! Consistent navigation pattern.This change mirrors the navigation improvement in the forgot-password page, ensuring consistent client-side routing across authentication flows.
Also applies to: 9-16
src/app/api/users/forgot-password/route.ts (3)
25-29: LGTM! Safe JSON parsing implemented.The content-type check and safe JSON parsing with
.catch(() => null)properly handles non-JSON responses and malformed JSON.
32-37: LGTM! Improved error handling.Error message extraction from the response data with a clear fallback message improves error reporting.
38-38: LGTM! Proper error typing.Using
error: unknownin the catch block follows TypeScript best practices for error handling.src/app/api/users/profile/route.ts (2)
53-64: LGTM! Clean error handling in GET.The use of
parseJsonSafelyand proper error message extraction with fallback improves code quality.
106-117: LGTM! Consistent error handling in PUT.The PUT handler follows the same robust pattern as GET, ensuring consistent error handling across HTTP methods.
src/app/api/users/reset-password/route.ts (1)
32-40: LGTM! Consistent error handling and response pattern.The error handling and response structure align well with the patterns established in other route handlers.
src/app/api/users/login/route.ts (1)
36-54: I need to gather more information about Node.js version requirements for Next.js 15.2.1 and whengetSetCookie()was introduced.I need to verify when
getSetCookie()was specifically added to Node.js to confirm it's available in Node.js 18.18.0+.Based on my verification, I've found important information about the code's approach to handling cookies. Let me provide the rewritten review comment:
The fallback for multiple Set-Cookie headers may not capture all cookies.
Next.js 15 requires Node.js 18.18.0 minimum, and Node.js 18.18.2 supports the
getSetCookie()method. The optional chaining approach is correct as a safeguard. However, there's a subtle issue with the fallback:getSetCookie()returns an array containing all Set-Cookie header values, which wasn't possible prior to its implementation, but using.get('set-cookie')only returns one of the set-cookie headers when multiple exist in a response.The current fallback will miss additional cookies if
getSetCookie()is unavailable. For full compatibility, consider usingresponse.headers.raw()?.['set-cookie']as an additional fallback on older runtimes, or document that multiple cookies are only guaranteed whengetSetCookie()is available.src/app/cart/page.tsx (1)
48-49: Escaped apostrophes in UI copy are correct and safeUsing
'in the empty-cart and order-summary messages is valid JSX/HTML, avoids quoting issues, and doesn’t change behavior. Looks good.Also applies to: 231-232
src/components/listings/tabs/ReviewsTab.tsx (1)
68-74: Auth‑gate for “Add a Review” dialog is clear and minimalRedirecting unauthenticated users to
/auth/loginand returning early before opening the dialog keeps the UX straightforward and prevents unauthenticated submissions. Implementation is concise and readable.src/app/api/reviews/[id]/route.ts (1)
5-15: Context‑based params and GET caching behavior look correctSwitching GET to
context: { params: Promise<{ id: string }> }and awaitingcontext.paramsis consistent with the other review routes, and addingcache: "no-store"ensures review details aren’t cached unexpectedly. No issues spotted with the URL construction.Also applies to: 10-15
src/types/index.ts (1)
392-432: Stronger typing forApiExperience.reviewslooks goodSwitching
reviewstoReview[]aligns this API surface with the sharedReviewmodel and removesany-style looseness. This should help catch mismatches at compile time without changing runtime behavior.src/app/checkout/page.tsx (3)
43-79: Gift details state wiring looks consistentUsing a
Record<string, GiftDetails>keyed byitem.idties the gift inputs, review display, and thegiftspayload inhandleSubmittogether cleanly. This should scale fine to multiple gift items without extra complexity.
95-140: Improved checkout error handling is solidClearing the previous error, safely attempting to parse a JSON error payload, and falling back to a generic message if there’s no usable
messagefield makes the submit flow much more robust. Normalizing unknown errors to a string before storing inerroralso avoids leaking raw objects into the UI. The only behavioral note is that non‑conforming error payloads will show the generic “Something went wrong”/“One or more bookings failed.” message, which seems acceptable.
265-270: Escaped apostrophe in label is correctSwitching to
Recipient's Email (Optional)is the right way to handle the apostrophe in JSX/HTML text; no issues here.src/components/listings/ExperienceDetails.tsx (3)
3-4: Review/data fetch refactor withuseCallbacklooks correctMemoizing
fetchReviewsandfetchDataand driving them via effects keyed onexperienceId(andsortOptionfor reviews) gives predictable re-fetch behavior without obvious dependency issues. Pagination and stats updates remain consistent, andavailableTimesis initialized from eitherinitialDataor the fetched experience as expected.Also applies to: 46-78, 80-104, 105-116
176-204: Error handling for review mutations is appropriately defensiveUsing
error: unknownin thecatchclauses for add/helpful/unhelpful review actions and logging a clear context-specific message keeps these paths type-safe without impacting user flows. Since failures here aren’t critical to the primary booking experience, logging-only behavior is reasonable.Also applies to: 206-242
262-263: Location formatter now correctly typedChanging
formatLocationto acceptApiExperience["location"]instead of an untyped value keeps it aligned with the API model and removes anany-like hole in the type system. Implementation itself is straightforward and fine.src/app/experiences/category/[slug]/page.tsx (2)
14-20: CategoryEntity shape looks consistent with downstream usageThe interface aligns with how you consume entities (name, slug, description?, image?). No issues from a typing or usage standpoint.
221-232: Type guard + slug validation are cleanDefining
isCategoryTypeand early‑rejecting invalid slugs withnotFound()keeps the route surface explicit and narrowsslugtoCategoryTypebefore use. No issues here.src/app/experiences/subcategory/[mainCategory]/[subCategorySlug]/page.tsx (4)
1-19: Imports, PageProps, and SubcategoryEntity are coherentThe imports and
PagePropssignature align with your usage (await params).SubcategoryEntitymatches how you populatesubcategoryId/name/description/image. Nothing problematic here.
98-125: Experiences fetch + empty state handling look correctThe experiences fetch against
/experiences/by-category/${mainCategory}/${subcategoryId}with a guardedresponse.okcheck and a safe default to[]is sound. The empty array check and rendering aCategoryPagewithresults={0}andlistings={[]}give a clear, non-error empty state, and the non-empty path viamapExperiencesToListingsis straightforward.
137-139: Top-level error handling is appropriateCatching unexpected errors, logging
"Error pre-fetching subcategory data:", and then usingnotFound()is a reasonable fail-safe for this route.
64-96: Backend implementation is external; manual verification neededThe
/experiences/by-categoryendpoint is called at line 102, but the backend service (CMS_API athttp://localhost:3000) is not part of this repository. The code pattern—defaultingsubcategoryIdto the slug, then overwriting with the CMS_idwhen available, with slug as fallback—appears intentional, and the same pattern is used elsewhere (e.g., category page).To confirm the endpoint supports both identifier formats (CMS
_idand fallbacksubCategorySlug), check the backend API documentation or implementation to ensure it handles both parameter types correctly and doesn't silently fail for slug-based queries when ID enrichment is unavailable.
| interface ApiResponseData { | ||
| message?: string; | ||
| error?: string; | ||
| [key: string]: unknown; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract duplicated ApiResponseData interface to a shared types file.
The ApiResponseData interface is duplicated across multiple API route files (forgot-password, profile, login, reset-password). This violates the DRY principle and makes maintenance harder.
Create a shared types file (e.g., src/types/api.ts or src/lib/api-types.ts):
export interface ApiResponseData {
message?: string;
error?: string;
[key: string]: unknown;
}Then import it in each route file:
import { NextRequest, NextResponse } from "next/server";
import { config } from "@/lib/config";
+import { ApiResponseData } from "@/types/api";
-interface ApiResponseData {
- message?: string;
- error?: string;
- [key: string]: unknown;
-}🤖 Prompt for AI Agents
In src/app/api/users/forgot-password/route.ts around lines 4 to 8, the
ApiResponseData interface is duplicated across multiple route files; extract
this shared type into a new shared file (e.g., src/types/api.ts or
src/lib/api-types.ts) exporting the interface, remove the local definition from
this file, and replace it with an import of the exported ApiResponseData in this
file (and similarly update profile, login, reset-password route files to import
the shared type).
| let data: ApiResponseData | null = null; | ||
| const contentType = response.headers.get("content-type") || ""; | ||
| if (contentType.includes("application/json")) { | ||
| data = (await response.json().catch(() => null)) as ApiResponseData | null; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use the parseJsonSafely helper for consistency.
This inline JSON parsing duplicates the logic from profile/route.ts. Once the shared utility is created (as suggested in profile route review), replace this with:
- let data: ApiResponseData | null = null;
- const contentType = response.headers.get("content-type") || "";
- if (contentType.includes("application/json")) {
- data = (await response.json().catch(() => null)) as ApiResponseData | null;
- }
+ const data = await parseJsonSafely(response);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/app/api/users/login/route.ts around lines 22 to 26, the code does inline
JSON parsing which duplicates logic from profile/route.ts; replace this block to
call the shared parseJsonSafely helper instead: import parseJsonSafely from its
module, call it with the response (or response.text()/headers if the helper
expects them), assign the returned value to data (typed ApiResponseData | null),
and remove the manual content-type check and .catch() usage; ensure the import
is added at the top and adjust any types so data remains ApiResponseData | null.
| interface ApiResponseData { | ||
| message?: string; | ||
| error?: string; | ||
| [key: string]: unknown; | ||
| } | ||
|
|
||
| const parseJsonSafely = async ( | ||
| response: Response | ||
| ): Promise<ApiResponseData | null> => { | ||
| const contentType = response.headers.get("content-type") || ""; | ||
| if (!contentType.includes("application/json")) { | ||
| return null; | ||
| } | ||
|
|
||
| const result = await response.json().catch(() => null); | ||
| if (result && typeof result === "object") { | ||
| return result as ApiResponseData; | ||
| } | ||
| return null; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract parseJsonSafely helper and ApiResponseData to shared utilities.
The parseJsonSafely helper is well-designed and should be reused across all API routes. Currently, forgot-password, login, and reset-password routes duplicate this logic inline.
Create a shared utilities file (e.g., src/lib/api-utils.ts):
export interface ApiResponseData {
message?: string;
error?: string;
[key: string]: unknown;
}
export const parseJsonSafely = async (
response: Response
): Promise<ApiResponseData | null> => {
const contentType = response.headers.get("content-type") || "";
if (!contentType.includes("application/json")) {
return null;
}
const result = await response.json().catch(() => null);
if (result && typeof result === "object") {
return result as ApiResponseData;
}
return null;
};Then import in all route files:
import { NextRequest, NextResponse } from "next/server";
import { config } from "@/lib/config";
+import { ApiResponseData, parseJsonSafely } from "@/lib/api-utils";
-interface ApiResponseData {
- message?: string;
- error?: string;
- [key: string]: unknown;
-}
-
-const parseJsonSafely = async (
- response: Response
-): Promise<ApiResponseData | null> => {
- // ...
-};🤖 Prompt for AI Agents
In src/app/api/users/profile/route.ts around lines 4 to 23, the local
ApiResponseData interface and parseJsonSafely function should be extracted to a
shared utility file; create src/lib/api-utils.ts exporting the ApiResponseData
interface and parseJsonSafely function (same implementation), then remove the
duplicated declarations from this file and replace them with an import from
'src/lib/api-utils'; also update the other routes that duplicated the logic
(forgot-password, login, reset-password and any others) to import the shared
exports instead of inlining the helper so all routes reuse the single
implementation.
| let data: ApiResponseData | null = null; | ||
| const contentType = response.headers.get("content-type") || ""; | ||
| if (contentType.includes("application/json")) { | ||
| data = (await response.json().catch(() => null)) as ApiResponseData | null; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use the parseJsonSafely helper for consistency.
This inline JSON parsing duplicates logic that should be in a shared utility. Once the shared utility is created, replace with:
- let data: ApiResponseData | null = null;
- const contentType = response.headers.get("content-type") || "";
- if (contentType.includes("application/json")) {
- data = (await response.json().catch(() => null)) as ApiResponseData | null;
- }
+ const data = await parseJsonSafely(response);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/app/api/users/reset-password/route.ts around lines 25 to 29, the code
performs inline JSON parsing which duplicates shared logic; replace it with the
parseJsonSafely helper: import parseJsonSafely at the top of the file, call it
with the Response (or response.text()/response as the helper expects) to obtain
ApiResponseData | null, and remove the manual content-type check and
.json().catch(() => null) block so the helper handles content-type detection and
safe parsing while preserving the ApiResponseData | null type.
| <h1 className="text-2xl font-bold text-gray-900 mb-2"> | ||
| Something went wrong! | ||
| </h1> | ||
| <p className="text-gray-600 mb-6"> | ||
| We encountered an error while loading your cart. Please try again. | ||
| We encountered an error while loading your cart. Please try again: {error.message} | ||
| </p> |
There was a problem hiding this comment.
Don’t append raw error.message in cart error UI
Showing {error.message} directly in the cart error text can leak internal implementation details to users. Prefer a generic message and log the underlying error instead (or gate detailed output to development).
🤖 Prompt for AI Agents
In src/app/cart/error.tsx around lines 18 to 23, the component currently renders
the raw error.message into the UI which may leak internal details; replace the
interpolated {error.message} with a generic user-facing sentence (e.g., "Please
try again later."), remove any direct exposure of the error string in
production, and instead log the full error for diagnostics (e.g.,
console.error(error) or send to your error-tracking service). If you want to
surface details while developing, gate the raw message behind a development
check (process.env.NODE_ENV === 'development' or next/config) so only dev builds
show error.message. Ensure the UI change removes the interpolation and keep
logging/capture of the original error elsewhere.
| <h1 className="text-2xl font-bold text-gray-900 mb-2"> | ||
| Checkout Error | ||
| </h1> | ||
| <p className="text-gray-600 mb-6"> | ||
| We encountered an error during checkout. Please try again or return to your cart. | ||
| We encountered an error during checkout. Please try again or return to | ||
| your cart. {error.message} | ||
| </p> |
There was a problem hiding this comment.
Avoid surfacing raw error.message to end users
Interpolating error.message into the checkout error copy can expose internal details (stack fragments, backend messages) in production. Consider logging it and showing a generic, user‑friendly message instead (or only showing the raw message in development).
🤖 Prompt for AI Agents
In src/app/checkout/error.tsx around lines 18 to 24, the component is directly
interpolating error.message into UI which can leak internal details; instead log
the full error (using existing logger or console.error) and replace the
interpolated value with a generic user-facing sentence like "Please contact
support or try again" — if you need to surface the actual message during
development only, conditionally render error.message when process.env.NODE_ENV
=== 'development' (or use your app's isDev flag) while keeping production output
generic.
| <Link href="/terms-of-service">Terms of Service</Link> and{" "} | ||
| <Link href="/privacy-policy">Privacy Policy</Link>. |
There was a problem hiding this comment.
🧩 Analysis chain
Consistent use of Next.js Link components.
Good replacement of anchor tags for Terms of Service and Privacy Policy links. Verify that these routes exist in your application.
🏁 Script executed:
#!/bin/bash
# Description: Verify the policy routes exist in the application
# Search for terms of service page/route
echo "=== Terms of Service route ==="
fd -e tsx -e ts -e jsx -e js . | xargs rg -l "terms-of-service" | head -10
echo ""
echo "=== Privacy Policy route ==="
fd -e tsx -e ts -e jsx -e js . | xargs rg -l "privacy-policy" | head -10Length of output: 340
🏁 Script executed:
#!/bin/bash
# Description: Verify actual route pages exist for terms-of-service and privacy-policy
echo "=== Checking for Terms of Service page ==="
fd -type f \( -name "page.tsx" -o -name "page.ts" -o -name "route.ts" \) | xargs grep -l "terms-of-service" 2>/dev/null || echo "No explicit terms-of-service page found"
fd -path "*/terms-of-service/*" -type f | head -10
echo ""
echo "=== Checking for Privacy Policy page ==="
fd -type f \( -name "page.tsx" -o -name "page.ts" -o -name "route.ts" \) | xargs grep -l "privacy-policy" 2>/dev/null || echo "No explicit privacy-policy page found"
fd -path "*/privacy-policy/*" -type f | head -10
echo ""
echo "=== Checking app directory structure ==="
fd -path "*/app/*" -type d | grep -E "(terms|privacy|policy)" | head -10
echo ""
echo "=== Checking pages directory structure (if exists) ==="
fd -path "*/pages/*" -type f | grep -E "(terms|privacy|policy)" | head -10Length of output: 1278
🏁 Script executed:
#!/bin/bash
# Description: Verify actual route pages exist for terms-of-service and privacy-policy
echo "=== Directory structure check ==="
find . -type d -name "terms-of-service" 2>/dev/null
find . -type d -name "privacy-policy" 2>/dev/null
echo ""
echo "=== Looking for page files in app directory ==="
find . -path "*/app/terms-of-service/*" -name "page.*" 2>/dev/null
find . -path "*/app/privacy-policy/*" -name "page.*" 2>/dev/null
echo ""
echo "=== All directories under app ==="
find ./app -type d 2>/dev/null | head -20
echo ""
echo "=== Looking for any route/page files ==="
find . -name "page.tsx" -o -name "page.ts" 2>/dev/null | head -20Length of output: 593
Routes for Terms of Service and Privacy Policy do not exist in the application.
The links at lines 266-267 point to /terms-of-service and /privacy-policy, but these routes are not present in your app directory. The links will result in 404 errors. Either create these route pages (src/app/terms-of-service/page.tsx and src/app/privacy-policy/page.tsx) or update the links to point to existing routes.
🤖 Prompt for AI Agents
In src/components/reset-password-form.tsx around lines 266-267 the Link
components point to /terms-of-service and /privacy-policy which do not exist and
will 404; fix by either adding the missing route pages at
src/app/terms-of-service/page.tsx and src/app/privacy-policy/page.tsx (implement
basic content and export default a React component) or change the hrefs to
existing routes in the app (e.g., /legal/terms or /legal/privacy) ensuring the
paths match your app directory structure and updating any navigation/constants
accordingly.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Performance