Feat: Integrate Auth (supabase) + UI user pages#25
Conversation
…le management with form validation
… and submission improvements
… enhance DeleteAccount styling
…ance and reliability
…user profile fetching logic
…ons, and improve loading state handling
…earch Company" for improved clarity
… and integrate it into AccountSettings
…iew functionality, and loading state management
…enu components with improved avatar handling
|
Warning Rate limit exceeded@alexmarqs has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 37 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
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. WalkthroughAdds Supabase browser/server/admin clients and DB types; session middleware and middleware matcher; OAuth callback, GitHub/Google login UI, user CRUD (profile, avatar, delete) and React Query hooks; Plunk-backed email service and templates; many Radix/Tailwind UI primitives, settings/login pages, PWA support, env/CI and dependency updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Browser
participant NextServer as NextApp
participant Supabase
participant Plunk as EmailService
Note over Browser,NextServer: OAuth sign-in and callback
Browser->>Supabase: client initiates OAuth redirect (provider)
Provider-->>Browser: redirects back with code
Browser->>NextServer: GET /api/auth/callback?code=...
NextServer->>Supabase: exchange code for session via server/admin client
Supabase-->>NextServer: returns session + user info
alt newly created user
NextServer->>Plunk: enqueue welcome email (background via sendWelcomeEmail)
Plunk-->>NextServer: ack
end
NextServer-->>Browser: redirect to computed destination (considers env / x-forwarded-host)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 review |
✅ Actions performedReview triggered.
|
…ERVICE_ROLE_KEY and SUPABASE_URL
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (11)
.gitignore (2)
18-18: Fix trailing space so dist is actually ignored.As written, "dist " (with space) won’t match "dist".
Apply this diff:
-dist +dist
25-31: Harden env secrets hygiene.Current rules miss .env and most non-local variants, risking accidental commits of secrets.
Apply this diff:
# local env files -.env*.local +.env +.env.* +!.env.example +!.env.*.example +.env*.localpackages/analytics/src/client/providers.tsx (3)
25-27: Remove hardcoded PostHog distinct ID (merges all users into one profile).Bootstrapping with a fixed
distinctID: "user distinct id"will attribute every event in prod to the same user. Remove the bootstrap block and identify users from Supabase auth (and reset on sign-out).Apply:
capture_pageview: false, // Disable automatic pageview capture, as we capture manually // cookieless approach persistence: "memory", - bootstrap: { - distinctID: "user distinct id", - },Then, from your Supabase auth listener, call
posthog.identify(user.id, { email: user.email, ... })on sign-in andposthog.reset()on sign-out. Example outside this file:useEffect(() => { const { data: { subscription } } = supabase.auth.onAuthStateChange((_evt, session) => { if (session?.user) posthog.identify(session.user.id, { email: session.user.email }); else posthog.reset(); }); return () => subscription?.unsubscribe(); }, []);
12-19: Guard against missing PostHog key to avoid initializing with undefined.Apply:
useEffect(() => { if (!isProd) { console.log("Analytics disabled in non-production environment"); return; } - posthog.init(process.env.NEXT_PUBLIC_POSTHOG_KEY as string, { + const key = process.env.NEXT_PUBLIC_POSTHOG_KEY; + if (!key) { + console.warn("PostHog disabled: NEXT_PUBLIC_POSTHOG_KEY is not set"); + return; + } + posthog.init(key, {
1-68: Add PostHog identify/reset calls on auth transitionsNo usages of posthog.identify or posthog.reset found in the login/logout flows. After a successful login (e.g. in GoogleLogin.tsx, GithubLogin.tsx or centrally in useSession on the INITIAL_SESSION event), call
posthog.identify(session.user.id)and after logout (in UserMenu.tsx.handleLogout), call
posthog.reset()Ensure each is invoked exactly once per transition.
apps/web/src/lib/parser.ts (1)
31-35: Use the correct GitHub header name ("User-Agent")."UserAgent" is not a valid header key; some proxies/clients will drop it. Use "User-Agent" to avoid rate-limit/compat issues.
const response = await fetch(url, { headers: { - UserAgent: "Tech Companies in Portugal", + "User-Agent": "Tech Companies in Portugal", Accept: "application/vnd.github.html+json", "X-GitHub-Api-Version": "2022-11-28", },apps/web/src/components/CompaniesListFooter.tsx (1)
38-81: Make buttons actually disabled for a11y and keyboard users.Styling with pointer-events-none doesn’t disable keyboard activation. Use the native disabled attribute (keeps focus/ARIA semantics consistent).
<Button className={cn( isPreviousDisabled && "pointer-events-none text-muted-foreground", "!px-2 h-8", )} variant="outline" size="sm" + disabled={isPreviousDisabled} + aria-disabled={isPreviousDisabled} onClick={() => setSearchParams({ page: 1 })} > ... <Button className={cn( isPreviousDisabled && "pointer-events-none text-muted-foreground", "!px-2 h-8", )} variant="outline" size="sm" + disabled={isPreviousDisabled} + aria-disabled={isPreviousDisabled} onClick={() => setSearchParams({ page: currentPage - 1 })} > ... <Button variant="outline" size="sm" className={cn( isNextDisabled && "pointer-events-none text-muted-foreground", "!px-2 h-8", )} + disabled={isNextDisabled} + aria-disabled={isNextDisabled} onClick={() => setSearchParams({ page: currentPage + 1 })} > ... <Button variant="outline" size="sm" className={cn( isNextDisabled && "pointer-events-none text-muted-foreground", "!px-2 h-8", )} + disabled={isNextDisabled} + aria-disabled={isNextDisabled} onClick={() => setSearchParams({ page: totalPages })} >apps/web/src/components/CompaniesList.tsx (1)
33-35: Clamp page to avoid negative slicing and out-of-range indexes.If page is 0/NaN/>totalPages (e.g., user-edited URL), current logic can slice from the end or past bounds.
- const start = (page - 1) * PAGE_SIZE; - const end = start + PAGE_SIZE; + // defer until after filteredCompanies to compute totalPages and clamp page const filteredCompanies = useMemo( () => isDedicatedPage ? allCompanies : allCompanies.filter((company) => matchCompanies(company, query, category, location), ), [allCompanies, query, category, location, isDedicatedPage], ); - const paginatedCompanies = filteredCompanies.slice(start, end); - const totalPages = Math.ceil(filteredCompanies.length / PAGE_SIZE); + const totalPages = Math.max(1, Math.ceil(filteredCompanies.length / PAGE_SIZE)); + const safePage = Math.min(Math.max(page, 1), totalPages); + const start = (safePage - 1) * PAGE_SIZE; + const end = start + PAGE_SIZE; + const paginatedCompanies = filteredCompanies.slice(start, end);Also applies to: 46-48
apps/web/src/components/CompaniesListHeader.tsx (1)
57-68: Keyboard handlers won’t fire on non-focusable divs; add semantics and Space key support.Divs aren’t tabbable, so onKeyDown won’t run. Add role/button semantics, tabIndex, aria-disabled, and handle Space. Also guard clicks when “disabled”.
- <div - onClick={() => setSearchParams({ page: 1 })} + <div + role="button" + tabIndex={0} + aria-disabled={isPreviousDisabled} + onClick={() => { + if (isPreviousDisabled) return; + setSearchParams({ page: 1 }); + }} className={cn( "hover:text-foreground flex items-center justify-center hover:cursor-pointer", isPreviousDisabled && "pointer-events-none text-muted-foreground", )} onKeyDown={(e) => { - if (e.key === "Enter") { - setSearchParams({ page: 1 }); - } + if ((e.key === "Enter" || e.key === " " || e.code === "Space") && !isPreviousDisabled) { + e.preventDefault(); + setSearchParams({ page: 1 }); + } }} > ... - <div + <div + role="button" + tabIndex={0} + aria-disabled={isPreviousDisabled} onKeyDown={(e) => { - if (e.key === "Enter") { - setSearchParams({ page: currentPage - 1 }); - } + if ((e.key === "Enter" || e.key === " " || e.code === "Space") && !isPreviousDisabled) { + e.preventDefault(); + setSearchParams({ page: currentPage - 1 }); + } }} - onClick={() => setSearchParams({ page: currentPage - 1 })} + onClick={() => { + if (isPreviousDisabled) return; + setSearchParams({ page: currentPage - 1 }); + }} className={cn( ... - <div + <div + role="button" + tabIndex={0} + aria-disabled={isNextDisabled} onKeyDown={(e) => { - if (e.key === "Enter") { - setSearchParams({ page: currentPage + 1 }); - } + if ((e.key === "Enter" || e.key === " " || e.code === "Space") && !isNextDisabled) { + e.preventDefault(); + setSearchParams({ page: currentPage + 1 }); + } }} - onClick={() => setSearchParams({ page: currentPage + 1 })} + onClick={() => { + if (isNextDisabled) return; + setSearchParams({ page: currentPage + 1 }); + }} className={cn( ... - <div + <div + role="button" + tabIndex={0} + aria-disabled={isNextDisabled} onKeyDown={(e) => { - if (e.key === "Enter") { - setSearchParams({ page: totalPages }); - } + if ((e.key === "Enter" || e.key === " " || e.code === "Space") && !isNextDisabled) { + e.preventDefault(); + setSearchParams({ page: totalPages }); + } }} - onClick={() => setSearchParams({ page: totalPages })} + onClick={() => { + if (isNextDisabled) return; + setSearchParams({ page: totalPages }); + }} className={cn(Alternatively, replace these divs with real button elements and use the disabled attribute. I can provide that patch if preferred.
Also applies to: 71-79, 86-96, 100-109
apps/web/src/components/CompanyItem.tsx (1)
60-65: Use correct Tailwind class and ensure unique React keys
- Replace the non-existent
text-nowrapclass withwhitespace-nowrap.- Avoid key collisions by using
key={${location}-${index}}instead ofkey={location}.- <React.Fragment key={location}> - <p className="text-nowrap">{location}</p> + <React.Fragment key={`${location}-${index}`}> + <p className="whitespace-nowrap">{location}</p>apps/web/package.json (1)
5-17: Do not auto-write on lint; split lint vs lint:fix.Running
biome checkwith--write --unsafeinlintcan mutate code in CI. Makelintread-only and add a separate fixer script."scripts": { "dev": "next dev --turbopack", "build": "next build", "start": "next start", - "lint": "biome check . --write --unsafe", + "lint": "biome check .", + "lint:fix": "biome check . --write --unsafe", "format": "biome format --write .",
🧹 Nitpick comments (85)
.gitignore (3)
45-45: If you plan to version .cursor/mcp.json, switch to ignore-with-exception.Ignoring the directory hides all files, including a potential MCP config. If you want to keep only mcp.json tracked:
Apply this diff:
-.cursor +.cursor/* +!.cursor/mcp.json
39-44: Ignore ESLint cache.Prevents noisy changes and large cache files from being committed.
Apply this diff:
npm-debug.log* yarn-debug.log* yarn-error.log* .pnpm-debug.log* +.eslintcache
46-46: Add Supabase CLI ephemeral artifacts to .gitignore.These are generated locally and shouldn’t be committed.
Apply this diff:
+ +# supabase (CLI) artifacts +supabase/.temp/ +supabase/.branchesapps/web/src/app/error.tsx (1)
5-12: Optional: Expose Next.js reset and add a retry affordance.Supporting the
{ reset }prop lets users recover without a full reload and aligns with Next.js error boundary patterns.-export default function ErrorPage() { +export default function ErrorPage({ reset }: { reset: () => void }) { return ( - <EmptyState - title="Something went wrong" - description="Please try again later." - /> + <> + <EmptyState + title="Something went wrong" + description="Please try again later." + /> + <button type="button" onClick={() => reset()} aria-label="Try again"> + Try again + </button> + </> ); }apps/web/src/app/location/sitemap.ts (1)
9-15: URL-safety + deterministic order; drop unnecessary spread.Sort for stable sitemap output, encode location segments, and return the array directly.
Please confirm
availableLocationsare plain names (not already slugified); if so, encoding is needed.- const locationsRoutes = availableLocations.map((location) => ({ - url: `${APP_URL}/location/${location}`, - lastModified: updatedAtISODate, - })); - - return [...locationsRoutes]; + const locationsRoutes = availableLocations + .slice() + .sort((a, b) => a.localeCompare(b)) + .map((location) => ({ + url: `${APP_URL}/location/${encodeURIComponent(location)}`, + lastModified: updatedAtISODate, + })); + + return locationsRoutes;apps/web/src/app/company/sitemap.ts (1)
8-14: Deterministic order; drop unnecessary spread.Sort by slug for stable output and return the array directly.
Confirm
company.slugis guaranteed URL-safe (already slugified).- const companiesRoutes = companies.map((company) => ({ - url: `${APP_URL}/company/${company.slug}`, - lastModified: updatedAtISODate, - })); - - return [...companiesRoutes]; + const companiesRoutes = companies + .slice() + .sort((a, b) => a.slug.localeCompare(b.slug)) + .map((company) => ({ + url: `${APP_URL}/company/${company.slug}`, + lastModified: updatedAtISODate, + })); + + return companiesRoutes;apps/web/src/app/category/sitemap.ts (1)
9-15: URL-safety + deterministic order; drop unnecessary spread.Sort categories, encode segments, and return the array directly.
If
availableCategoriesare already slugified, skip encoding.- const categoriesRoutes = availableCategories.map((category) => ({ - url: `${APP_URL}/category/${category}`, - lastModified: updatedAtISODate, - })); - - return [...categoriesRoutes]; + const categoriesRoutes = availableCategories + .slice() + .sort((a, b) => a.localeCompare(b)) + .map((category) => ({ + url: `${APP_URL}/category/${encodeURIComponent(category)}`, + lastModified: updatedAtISODate, + })); + + return categoriesRoutes;apps/web/src/app/sitemap.ts (1)
5-11: Minor cleanup and import consistency.Return the array directly. Also align MetadataRoute import path with the other sitemap files.
- return [...routes]; + return routes;Outside this hunk (import line):
// Prefer consistency with other files import type { MetadataRoute } from "next";packages/analytics/src/client/providers.tsx (7)
23-25: Verify persistence strategy (“memory”) is intentional for production.
persistence: "memory"resets identity on reload and across tabs, which breaks cross-visit analytics unless you always identify via auth. If privacy requires cookieless, keep; otherwise consider cookie/localStorage or make it configurable.Option:
- persistence: "memory", + persistence: process.env.NEXT_PUBLIC_POSTHOG_PERSISTENCE ?? "memory",Expected: set NEXT_PUBLIC_POSTHOG_PERSISTENCE to "cookie" or "localStorage" in prod if desired.
44-53: Build URL with the URL API; avoid double serialization and include hash if needed.Apply:
useEffect(() => { if (pathname && posthog) { - let url = window.origin + pathname; - if (searchParams.toString()) { - url = `${url}?${searchParams.toString()}`; - } - - posthog.capture("$pageview", { $current_url: url }); + const query = searchParams.toString(); + const url = new URL(pathname + (query ? `?${query}` : ""), window.location.origin); + // If you want to include hash: url.hash = window.location.hash; + posthog.capture("$pageview", { $current_url: url.toString() }); } }, [pathname, searchParams, posthog]);
48-49: LGTM on the template literal change.Tiny readability win; combined with the above diff it can be simplified further.
31-36: Supabase x PostHog wiring (provider-level).Consider adding a small “AuthAnalyticsBridge” component under this provider to listen to Supabase auth state and call
identify/resetas noted above, so pageviews after login are immediately tied to the correct user.
18-23: Default EU host is fine; prefer env-only to avoid surprises.If you deploy outside EU or change regions per env, make host mandatory via env and drop the fallback:
- api_host: - process.env.NEXT_PUBLIC_POSTHOG_HOST || "https://eu.i.posthog.com", + api_host: process.env.NEXT_PUBLIC_POSTHOG_HOST as string,And fail fast if missing (similar to the key guard).
39-56: Effect dependencies and extra work.Minor:
searchParams.toString()was computed twice; the URL-API refactor addresses it. Keepingposthogin deps is fine; if it’s unstable, memoize the capture function instead.
11-29: Add Do-Not-Track/GPC respect (optional).If you want additional privacy, skip init when the user has DNT/GPC enabled.
Example:
const dnt = navigator.doNotTrack === "1" || (window as any).globalPrivacyControl === true; if (dnt) return;apps/web/src/components/Footer.tsx (2)
43-43: Add noopener to internal links opened in new tabsThese same-origin links still use
target="_blank"; addnoopenerto preventwindow.openeraccess and standardize with the rest of the app.Apply:
- rel="noreferrer" + rel="noopener noreferrer"
66-66: Add noopener to category links as wellMirror the recommendation for locations: include
noopener.- rel="noreferrer" + rel="noopener noreferrer"apps/web/src/components/LogoFooter.tsx (1)
17-21: Also include noopener on the external status-page link
noreferreroften impliesnoopener, but addingnoopeneris the safer, explicit default.- <a + <a href="https://techcompaniesportugal.openstatus.dev" target="_blank" - rel="noreferrer" + rel="noopener noreferrer" >apps/web/src/app/company/[slug]/page.tsx (1)
33-33: Simplify: always return Metadata from generateMetadataReturning
undefinedchanges the signature surface. An easy, type-stable alternative is to always returndefaultMetadatawhen the company isn’t found (the page itself already 404s).-}): Promise<Metadata | undefined> { +}): Promise<Metadata> { const { slug } = await params; const company = await getParsedCompanyBySlug(slug); - if (!company) { - return; - } + if (!company) { + return defaultMetadata; + }If you prefer the current signature, please run your typecheck pipeline to confirm
Promise<Metadata | undefined>is accepted by your Next.js version.apps/web/src/lib/parser.ts (1)
86-88: Set population change is fine; consider trimming and skipping empties.If any location strings come through with whitespace or as empty values, they’ll be added to the Set. Minor guard suggested below.
- for (const location of locations) { - availableLocations.add(location); - } + for (const location of locations) { + const v = location.trim(); + if (v) availableLocations.add(v); + }apps/web/src/components/FiltersPanelButton.tsx (3)
74-89: Use a non-submit button for “See results”.Form onSubmit is prevented; making this a submit button is unnecessary and can trigger submit handlers unexpectedly.
- <Button - type="submit" + <Button + type="button" variant="secondary"
22-24: Verify segment check with route groups.useSelectedLayoutSegment typically doesn’t return route group names like “(companies-list)”. If this never matches, the button won’t render. Consider usePathname() or passing a prop from the companies-list layout.
26-28: Confirm Suspense boundary for use().Calling use(promise) in a client component requires a Suspense boundary above to avoid “A component suspended while rendering...” errors. Ensure the parent wraps this with .
apps/web/src/components/FiltersButton.tsx (1)
15-23: Minor a11y: expose dialog intent.Add aria-haspopup="dialog" to the button. If feasible, also pass isOpen to set aria-expanded accordingly.
Example:
<Button onClick={() => setIsFilterOpen(true)} className="px-3 inline-flex hover:cursor-pointer space-x-1" aria-label="Open filters" aria-haspopup="dialog" >apps/web/src/components/SearchSideBar.tsx (2)
39-43: Restore a search landmark (moved from container to form).Since role="search" was removed from the container, consider adding it to the form for better navigation by assistive tech.
Example:
<form className="px-4 py-3 w-full" role="search" aria-label="Search form" onSubmit={(e) => e.preventDefault()} >
65-65: Microcopy nit.“Search Company” could be “Search companies” or “Search by company” for consistency with list context.
README.md (2)
35-36: Clarify “MCP” and dedupe Vercel in Tech stack.
- “MCP” is ambiguous here; expand it or replace with the specific Supabase capability you mean (e.g., “Auth, DB, Edge Functions”).
- Tech stack lists Vercel twice; consider removing one entry.
37-40: Tighten wording and punctuation in Roadmap.Use US punctuation for “e.g.” and “etc.” and consider a slightly clearer phrasing.
-## Roadmap 🛣️ - -- [ ] Notification jobs -- [ ] Move some logic to dedicated packages (e.g. emails, parsing, etc) +## Roadmap 🛣️ + +- [ ] Notification jobs +- [ ] Move some logic into dedicated packages (e.g., emails, parsing, etc.)apps/web/src/components/ui/input.tsx (1)
14-14: Ensure invalid-state styles win Tailwind precedence.
border-inputcan overridearia-[invalid=true]:border-destructivedue to class order. Move thearia-…utilities after the base border/ring classes.- "flex aria-[invalid=true]:border-destructive aria-[invalid=true]:focus-visible:ring-destructive/25 h-10 w-full rounded-md border border-input bg-background px-3 py-2 text-sm ring-offset-background file:border-0 file:bg-transparent file:text-sm file:font-medium placeholder:text-muted-foreground focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50", + "flex h-10 w-full rounded-md border border-input bg-background px-3 py-2 text-sm ring-offset-background file:border-0 file:bg-transparent file:text-sm file:font-medium placeholder:text-muted-foreground focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 aria-[invalid=true]:border-destructive aria-[invalid=true]:focus-visible:ring-destructive/25 disabled:cursor-not-allowed disabled:opacity-50",apps/web/src/app/api/og/_utils.ts (1)
15-16: Useresponse.okand broaden font format handling (optional).
response.okis more robust than strict=== 200.- Your regex only matches opentype/truetype; Google Fonts typically returns
woff2. Consider supporting it to avoid random failures.- if (response.status === 200) { + if (response.ok) { return await response.arrayBuffer(); }If helpful, here’s a safer extraction (outside this hunk):
// replace the current regex and fetch block const match = css.match(/src:\s*url\(([^)]+)\)\s*format\('(woff2|opentype|truetype)'\)/i); if (match?.[1]) { const resp = await fetch(match[1]); if (resp.ok) return await resp.arrayBuffer(); }apps/web/src/app/api/og/route.tsx (1)
122-124: Log the error and return explicit content type.Improves debuggability and ensures proper response headers.
- return new Response("Failed to generate the og image", { - status: 500, - }); + console.error("OG image generation failed:", e); + return new Response("Failed to generate the og image", { + status: 500, + headers: { "content-type": "text/plain; charset=utf-8" }, + });apps/web/src/components/CompanyItem.tsx (1)
81-84: Avoid potential key collisions for categories.If duplicates exist, keys must still be unique.
- {categoriesArray.map((category) => ( + {categoriesArray.map((category, index) => ( <Badge - key={category} + key={`${category}-${index}`} variant="secondary"Alternatively, de-duplicate upstream before rendering.
apps/web/.env.example (1)
2-6: Resolve dotenv-linter warnings (ordering + trailing newline).Reorder keys and add a newline at EOF to satisfy dotenv-linter and keep things tidy.
-NEXT_PUBLIC_POSTHOG_KEY= -NEXT_PUBLIC_POSTHOG_HOST= -NEXT_PUBLIC_SUPABASE_URL= -# Make sure to enable RLS and policies to use this key in client -NEXT_PUBLIC_SUPABASE_ANON_KEY= +NEXT_PUBLIC_POSTHOG_HOST= +NEXT_PUBLIC_POSTHOG_KEY= +# Make sure to enable RLS and policies to use this key in client +NEXT_PUBLIC_SUPABASE_ANON_KEY= +NEXT_PUBLIC_SUPABASE_URL= +apps/web/src/lib/metadata.ts (1)
40-41: Prefer absolute path for OG/Twitter image.Using "/api/og" avoids any relative-path surprises from nested routes and reads clearer.
- images: ["api/og"], + images: ["/api/og"],Repeat the same change for both twitter and openGraph images.
Also applies to: 49-50
apps/web/src/lib/supabase/middleware.ts (1)
4-5: Optional: centralize protected routes.Consider exporting protectedRoutes from a shared auth config to avoid drift with future pages (e.g., nested settings subroutes).
apps/web/src/middleware.ts (2)
5-6: Minor: drop unnecessary await.Middleware can return a Promise; no need to await.
-export async function middleware(request: NextRequest) { - return await updateSession(request); -} +export async function middleware(request: NextRequest) { + return updateSession(request); +}
17-18: Exclude /api and Next.js data routes from middleware.Prevents unnecessary auth checks and avoids interfering with API routes and data fetching endpoints.
- "/((?!_next/static|_next/image|favicon.ico|.*\\.(?:svg|png|jpg|jpeg|gif|webp)$).*)", + "/((?!api|_next/static|_next/image|_next/data|favicon.ico|.*\\.(?:svg|png|jpg|jpeg|gif|webp)$).*)",apps/web/src/components/CustomQueryClientProvider.tsx (1)
37-43: Make staleTime expression explicit.5 * 60 * 1000 is clearer than 60 * 5000.
- staleTime: 60 * 5000, // 5 minutes + staleTime: 5 * 60 * 1000, // 5 minutesapps/web/src/components/ui/alert-dialog.tsx (1)
101-127: Make Action/Cancel button variants configurable (defaulting to destructive/outline).Hard-coding variants limits reuse (e.g., non-destructive confirms). Allow overriding while keeping sensible defaults.
@@ -import { buttonVariants } from "@/components/ui/button"; +import { buttonVariants } from "@/components/ui/button"; +import type { VariantProps } from "class-variance-authority"; @@ -const AlertDialogAction = React.forwardRef< - React.ElementRef<typeof AlertDialogPrimitive.Action>, - React.ComponentPropsWithoutRef<typeof AlertDialogPrimitive.Action> ->(({ className, ...props }, ref) => ( +type ButtonVariant = VariantProps<typeof buttonVariants>["variant"]; +const AlertDialogAction = React.forwardRef< + React.ElementRef<typeof AlertDialogPrimitive.Action>, + React.ComponentPropsWithoutRef<typeof AlertDialogPrimitive.Action> & { + variant?: ButtonVariant; + } +>(({ className, variant = "destructive", ...props }, ref) => ( <AlertDialogPrimitive.Action ref={ref} - className={cn(buttonVariants({ variant: "destructive" }), className)} + className={cn(buttonVariants({ variant }), className)} {...props} /> )); @@ -const AlertDialogCancel = React.forwardRef< - React.ElementRef<typeof AlertDialogPrimitive.Cancel>, - React.ComponentPropsWithoutRef<typeof AlertDialogPrimitive.Cancel> ->(({ className, ...props }, ref) => ( +const AlertDialogCancel = React.forwardRef< + React.ElementRef<typeof AlertDialogPrimitive.Cancel>, + React.ComponentPropsWithoutRef<typeof AlertDialogPrimitive.Cancel> & { + variant?: ButtonVariant; + } +>(({ className, variant = "outline", ...props }, ref) => ( <AlertDialogPrimitive.Cancel ref={ref} - className={cn( - buttonVariants({ variant: "outline" }), + className={cn( + buttonVariants({ variant }), "mt-2 sm:mt-0", className, )} {...props} /> ));apps/web/src/lib/supabase/database.types.ts (1)
2-6: Typo in comment (“instanciate” → “instantiate”).Minor clarity fix in a top-of-file comment.
- // Allows to automatically instanciate createClient with right options + // Allows to automatically instantiate createClient with the right optionsapps/web/src/lib/supabase/server.ts (2)
6-12: Remove unnecessary async/await; add env guards; avoid non-null assertions for env.
- cookies() is synchronous in Next.js App Router; no need for async/await.
- Fail fast if required env vars are missing.
-export async function createClient() { - const cookieStore = await cookies(); +const requiredEnv = (name: string) => { + const v = process.env[name]; + if (!v) throw new Error(`Missing environment variable: ${name}`); + return v; +}; + +export function createClient() { + const cookieStore = cookies(); @@ - return createServerClient<Database>( - process.env.NEXT_PUBLIC_SUPABASE_URL!, - process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY!, + return createServerClient<Database>( + requiredEnv("NEXT_PUBLIC_SUPABASE_URL"), + requiredEnv("NEXT_PUBLIC_SUPABASE_ANON_KEY"), { cookies: {-export async function createAdminClient(accessToken?: string) { - return createSupabaseClient<Database>( - process.env.SUPABASE_URL!, - process.env.SUPABASE_SERVICE_ROLE_KEY!, +export function createAdminClient(_accessToken?: string) { + return createSupabaseClient<Database>( + requiredEnv("SUPABASE_URL"), + requiredEnv("SUPABASE_SERVICE_ROLE_KEY"), { auth: { autoRefreshToken: false, persistSession: false, }, }, ); }Also applies to: 33-44
33-33: Unused parameter in createAdminClient.Rename to underscore or remove to satisfy linters and avoid confusion.
-export async function createAdminClient(accessToken?: string) { +export function createAdminClient(_accessToken?: string) {apps/web/src/lib/types.ts (1)
30-35: Prefer ReactNode import over React namespace.Avoids relying on global React types and keeps types tree-shakable.
Apply this diff:
-export type SettingsTab = { +export type SettingsTab = { id: string; title: string; disabled?: boolean; - badge?: React.ReactNode; + badge?: ReactNode; };Add at the top of the file:
import type { ReactNode } from "react";apps/web/src/app/settings/page.tsx (2)
4-4: Fix typo in comment (“prefech” → “prefetch”).- // let's keep this page here for SSR, possibly to prefech some data on the server side, suspense queries etc. + // keep this page for SSR, possibly to prefetch some data on the server side, suspense queries, etc.
3-11: Confirm auth gating for /settings.If not already handled by middleware, add a server-side check and redirect unauthenticated users.
Example:
import { redirect } from "next/navigation"; import { createClient } from "@/lib/supabase/server"; export default async function SettingsPage() { const supabase = createClient(); const { data: { user } } = await supabase.auth.getUser(); if (!user) redirect("/login?from=settings"); return <Settings />; }apps/web/src/lib/supabase/client.ts (1)
1-9: Optional: memoize client + add explicit return type.Avoids multiple client instances across renders and clarifies API.
import { createBrowserClient } from "@supabase/ssr"; +import type { SupabaseClient } from "@supabase/supabase-js"; import type { Database } from "./database.types"; -export function createClient() { - return createBrowserClient<Database>( - process.env.NEXT_PUBLIC_SUPABASE_URL!, - process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY!, - ); -} +let client: SupabaseClient<Database> | null = null; +export function createClient(): SupabaseClient<Database> { + const url = process.env.NEXT_PUBLIC_SUPABASE_URL; + const anonKey = process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY; + if (!url || !anonKey) { + throw new Error( + "Missing NEXT_PUBLIC_SUPABASE_URL or NEXT_PUBLIC_SUPABASE_ANON_KEY" + ); + } + if (!client) client = createBrowserClient<Database>(url, anonKey); + return client; +}apps/web/src/components/GoHomeLoginButton.tsx (2)
6-6: Add explicit return type.Improves readability and avoids implicit any/JSX types drift.
-export const GoHomeLoginButton = () => { +export const GoHomeLoginButton = (): JSX.Element | null => {
9-14: Inline the conditional return.Small readability win.
- if (from !== "logout") { - return null; - } - - return <BackButton />; + return from === "logout" ? <BackButton /> : null;apps/web/src/components/Title.tsx (1)
10-10: Remove unnecessary braces around string literal.Minor cleanup.
- <h1 className={"text-3xl font-bold font-mono"}>{title}</h1> + <h1 className="text-3xl font-bold font-mono">{title}</h1>apps/web/src/components/Navbar.tsx (1)
44-52: Minor: use null instead of empty string for Suspense fallback.Returns a cleaner noop element.
Apply:
- <Suspense fallback={""}> + <Suspense fallback={null}>biome.json (2)
39-45: Enable a11y rules and set console policy.To catch regressions (e.g., accessible labels, stray console in prod), consider:
Apply:
"linter": { "enabled": true, "rules": { "recommended": true, + "a11y": { "recommended": true }, "suspicious": {}, "style": { "useBlockStatements": "off", "useImportType": "error", "noNonNullAssertion": "off" }, "correctness": { "noUnusedImports": "warn", "useExhaustiveDependencies": "error", "useHookAtTopLevel": "error" - } + }, + "complexity": {}, + "nursery": {}, + "performance": {}, + "style": { + "useImportType": "error", + "noNonNullAssertion": "off", + "noConsole": "warn" + } } },If “noConsole” is noisy for server-only code, we can add overrides scoped to api/ routes.
47-50: Scope hook rule to React code to avoid false positives.If you hit noise in non-React files, restrict this rule to TSX/React paths.
I can draft an overrides block targeting apps/web/src/**/*.{tsx,jsx}.
apps/web/src/components/settings/account/AccountSettings.tsx (1)
9-23: Lift profile fetching and add error state. We’re callinguseGetUserProfilein AccountSettings, AccountAvatar, and AccountName (three separate hooks fetching the same data). Instead, fetch once in AccountSettings:
- Destructure
isPending,data: profile, anderrorfromuseGetUserProfile().- Show
<Skeleton>while loading and an error alert UI iferroris truthy.- Provide
profileto children via props or a lightweightProfileContextto prevent duplicate network queries.apps/web/src/components/ExploreButton.tsx (2)
14-14: Preserve explicit label for accessibility and parity.Previous UI used “Back to all companies”; default BackButton label may be generic. Pass an explicit label (and href if needed).
Apply:
- return <BackButton />; + return <BackButton href="/" label="Back to all companies" />;
7-8: Tiny readability nit: use a Set for membership check.Apply:
- const isCompanyPage = - segment === "company" || segment === "location" || segment === "category"; + const isCompanyPage = new Set(["company", "location", "category"]).has(segment ?? "");apps/web/src/app/api/auth/callback/route.ts (1)
9-17: Add try/catch around session exchange and avoid noisy logs in production.Supabase usually returns { error }, but wrapping protects against unexpected throws. Consider structured logging.
Apply:
if (code) { - const supabase = await createClient(); - const { error } = await supabase.auth.exchangeCodeForSession(code); - - if (error) { - console.error("Error exchanging code for session:", error); - return NextResponse.redirect(`${origin}/auth/auth-code-error`); - } + try { + const supabase = await createClient(); + const { error } = await supabase.auth.exchangeCodeForSession(code); + if (error) { + console.error("auth.exchangeCodeForSession failed", { message: error.message }); + return NextResponse.redirect(`${origin}/auth/auth-code-error`); + } + } catch (e) { + console.error("auth.exchangeCodeForSession threw", { message: (e as Error).message }); + return NextResponse.redirect(`${origin}/auth/auth-code-error`); + } }apps/web/src/components/ui/sonner.tsx (1)
8-22: Preserve base styles while allowing consumers to extend/overrideRight now, any incoming
classNamefully replaces"toaster group". Consider merging class names and shallow-mergingtoastOptions.classNamesso consumers can override selectively.-"use client"; -import { Toaster as Sonner } from "sonner"; +\"use client\"; +import { Toaster as Sonner } from "sonner"; +import { cn } from "@/lib/utils"; @@ -const Toaster = ({ ...props }: ToasterProps) => { +const Toaster = ({ className, toastOptions, ...props }: ToasterProps) => { return ( <Sonner - className="toaster group" - toastOptions={{ - classNames: { + className={cn("toaster group", className)} + toastOptions={{ + ...toastOptions, + classNames: { toast: "group toast group-[.toaster]:bg-background group-[.toaster]:text-foreground group-[.toaster]:border-border group-[.toaster]:shadow-lg", description: "group-[.toast]:text-muted-foreground", actionButton: "group-[.toast]:bg-primary group-[.toast]:text-primary-foreground", cancelButton: "group-[.toast]:bg-muted group-[.toast]:text-muted-foreground", - }, + ...(toastOptions?.classNames ?? {}), + }, }} {...props} /> ); };apps/web/src/components/BackButton.tsx (2)
6-9: Type href to match Next.js Link and make it optionalUse
LinkProps["href"]for better type safety across string/URLObject.-import { Button, type ButtonProps } from "./ui/button"; +import { Button, type ButtonProps } from "./ui/button"; +import type { LinkProps } from "next/link"; @@ -type BackButtonProps = ButtonProps & { - href?: string; +type BackButtonProps = ButtonProps & { + href?: LinkProps["href"]; label?: string; };
11-21: Avoid double className when using Button asChildPassing
props.classNameon both Button (via forwarded props) and Link can conflict. Move layout classes to Button’sclassNameand keep Link clean. Also set defaults in destructuring to avoid repeated||.-export const BackButton = ({ href, label, ...props }: BackButtonProps) => { +export const BackButton = ({ + href = "/", + label = "Back to Home", + className, + ...props +}: BackButtonProps) => { return ( - <Button asChild aria-label={label || "Back to Home"} size="sm" {...props}> - <Link - href={href || "/"} - className={cn("flex items-center gap-2", props.className)} - > + <Button + asChild + aria-label={label} + size="sm" + className={cn("flex items-center gap-2", className)} + {...props} + > + <Link href={href}> <ArrowLeft aria-hidden="true" className="h-4 w-4 shrink-0" /> - {label || "Back to Home"} + {label} </Link> </Button> ); };apps/web/src/components/SocialIcons.tsx (3)
3-11: Export the SocialIcon type for reuseThis makes the union available to consumers (e.g., prop types elsewhere).
-type SocialIcon = "github" | "google"; +export type SocialIcon = "github" | "google";
12-50: Add an exhaustive default to catch future icon additionsHelps the compiler flag missing cases if
SocialIcongrows.export const SocialIcons = ({ icon, className, }: { icon: SocialIcon; className?: string; }) => { switch (icon) { case "github": return ( <svg aria-hidden="true" className={cn("h-4 w-4", className)} - role="img" viewBox="0 0 24 24" > @@ case "google": return ( <svg aria-hidden="true" className={cn("h-4 w-4", className)} - role="img" viewBox="0 0 24 24" > @@ ); + default: { + // exhaustive check + const _never: never = icon; + return null; + } } };
18-20: Remove redundant role attribute
role="img"is unnecessary (and a bit conflicting) whenaria-hidden="true"is present.- role="img"Also applies to: 28-30
apps/web/src/hooks/useSession.ts (1)
5-7: Memoize the Supabase client and clear loading on first auth event
WrapcreateClient()inuseMemoto avoid re-subscribing on every render, and replace theINITIAL_SESSION-only check with an unconditionalsetIsLoading(false)so loading isn’t tied solely to that event.-import { useEffect, useState } from "react"; +import { useEffect, useMemo, useState } from "react"; export function useSession() { - const supabase = createClient(); + const supabase = useMemo(() => createClient(), []); const [session, setSession] = useState<Session | null>(null); const [isLoading, setIsLoading] = useState(true); useEffect(() => { const { data: { subscription }, } = supabase.auth.onAuthStateChange((event, newSession) => { setSession(newSession); - if (event === "INITIAL_SESSION") { - setIsLoading(false); - } + setIsLoading(false); }); return () => subscription.unsubscribe(); }, [supabase]);apps/web/src/components/GoogleLogin.tsx (1)
10-47: Handle OAuth errors and avoid artificial spinner delayAdd error handling (toast) and drop the fixed 2s timeout. Optionally memoize the client and add basic accessibility.
"use client"; import { createClient } from "@/lib/supabase/client"; -import { useState } from "react"; +import { useMemo, useState } from "react"; import { Loader2 } from "lucide-react"; import { SocialIcons } from "./SocialIcons"; import { Button } from "./ui/button"; +import { toast } from "sonner"; export const GoogleLogin = () => { const [isLoading, setIsLoading] = useState(false); - const supabase = createClient(); + const supabase = useMemo(() => createClient(), []); const handleGoogleAuth = async () => { setIsLoading(true); - try { - await supabase.auth.signInWithOAuth({ + try { + const { error } = await supabase.auth.signInWithOAuth({ provider: "google", options: { - redirectTo: `${window.location.origin}/api/auth/callback`, + redirectTo: new URL("/api/auth/callback", window.location.origin).toString(), scopes: "openid email profile", }, }); + if (error) throw error; + } catch (err) { + const message = + err instanceof Error ? err.message : "Please try again."; + toast.error("Google sign-in failed", { description: message }); } finally { - setTimeout(() => { - setIsLoading(false); - }, 2000); + setIsLoading(false); } }; return ( <Button type="button" variant="outline" className="w-full" onClick={handleGoogleAuth} disabled={isLoading} + aria-busy={isLoading} > {isLoading ? ( <Loader2 className="w-4 h-4 mr-2 animate-spin" /> ) : ( <SocialIcons icon="google" className="mr-2" /> )} Continue with Google </Button> ); };apps/web/src/components/settings/index.tsx (2)
39-49: Disable tab via prop for a11y; don’t rely only on CSSLeverage the trigger’s disabled prop (Radix supports it) and expose aria-disabled. Avoid pointer-events-none so keyboard focus is also blocked by the native disabled behavior.
<TabsTrigger key={tab.id} value={tab.id} + disabled={tab.disabled} + aria-disabled={tab.disabled || undefined} className={cn( - "!bg-transparent data-[state=active]:border-b-2 data-[state=active]:border-primary font-mono px-0 py-2 flex items-center gap-2", - tab.disabled && "opacity-80 pointer-events-none", + "!bg-transparent data-[state=active]:border-b-2 data-[state=active]:border-primary font-mono px-0 py-2 flex items-center gap-2", + tab.disabled && "opacity-60", )} >
36-36: Remove non-existent Tailwind class “bold”Tailwind doesn’t ship “bold” by default; use font-semibold or drop it.
- <Tabs defaultValue="account" className="bold"> + <Tabs defaultValue="account">apps/web/src/components/GithubLogin.tsx (2)
6-9: Remove arbitrary 2s delay; surface OAuth start errorsThe timeout can cause unnecessary UI flicker and potential state updates after unmount. Provide user feedback on failure and clear loading immediately.
import { Loader2 } from "lucide-react"; import { SocialIcons } from "./SocialIcons"; import { Button } from "./ui/button"; +import { toast } from "sonner"; const handleGithubLogin = async () => { setIsLoading(true); try { await supabase.auth.signInWithOAuth({ provider: "github", options: { redirectTo: `${window.location.origin}/api/auth/callback`, scopes: "read:user user:email", }, }); + } catch (err) { + toast.error("Failed to start GitHub login. Please try again."); } finally { - setTimeout(() => { - setIsLoading(false); - }, 2000); + setIsLoading(false); } };Also applies to: 17-29
21-23: Make redirect base configurableUse NEXT_PUBLIC_SITE_URL when available to avoid mismatches behind proxies or multiple domains.
- redirectTo: `${window.location.origin}/api/auth/callback`, + redirectTo: `${process.env.NEXT_PUBLIC_SITE_URL ?? window.location.origin}/api/auth/callback`,Confirm NEXT_PUBLIC_SITE_URL is set in your envs (local, preview, prod).
apps/web/src/app/api/delete-user/route.ts (2)
17-26: Don’t swallow storage cleanup errors silentlyAt minimum, log the error for operational visibility.
- } catch {} + } catch (err) { + console.error("[api/delete-user] storage cleanup failed", err); + }
28-35: Consider cascading app-data cleanupIf you maintain a public users/profile table or related rows, ensure ON DELETE CASCADE or follow-up deletion to avoid orphans after auth deletion.
apps/web/src/components/settings/account/AccountAvatar.tsx (1)
83-86: Theming/a11y nits: mark busy; use theme tokenExpose aria-busy while uploading and use bg-background for theme parity.
- <Label - htmlFor="avatar-input" - className="relative flex-shrink-0 cursor-pointer inline-block outline-none rounded-full focus-within:ring-2 focus-within:ring-ring focus-within:ring-offset-2 focus-within:ring-offset-background" - > + <Label + htmlFor="avatar-input" + aria-busy={isMutatingUserProfile} + className="relative flex-shrink-0 cursor-pointer inline-block outline-none rounded-full focus-within:ring-2 focus-within:ring-ring focus-within:ring-offset-2 focus-within:ring-offset-background" + > @@ - <div - className="absolute bottom-0 right-0 h-6 w-6 rounded-full border-none flex items-center justify-center shadow-md p-1 bg-white" + <div + className="absolute bottom-0 right-0 h-6 w-6 rounded-full border-none flex items-center justify-center shadow-md p-1 bg-background" aria-hidden >Also applies to: 110-114
apps/web/src/app/login/page.tsx (1)
11-11: Prefer importing Metadata type from "next"Minor consistency tweak with Next.js docs.
-import type { Metadata } from "next/types"; +import type { Metadata } from "next";apps/web/src/components/settings/account/AccountName.tsx (2)
86-90: Tighten UX: cap length and disable Save when invalid.Mirror schema constraints in the input and avoid enabling Save for invalid values.
<Input disabled={isMutatingUserProfile} placeholder="Enter your full name" + maxLength={30} + autoComplete="name" {...field} />-<Button +<Button type="submit" - disabled={isMutatingUserProfile || !form.formState.isDirty} + disabled={ + isMutatingUserProfile || + !form.formState.isDirty || + !form.formState.isValid + } className="flex items-center gap-2" >Also applies to: 100-109
39-46: Avoid double success side-effects unless intentional.You have
onSuccessin the hook options (invalidates query) and another per-callonSuccess(reset + toast). Both will fire. If duplication is unintended, consolidate into one place.Also applies to: 63-66
apps/web/src/components/UserMenu.tsx (3)
34-45: Minor: reduce chance of setState-after-navigation; clear cache before redirect.Reorder so the cache is cleared before
router.replace, and consider not flipping state back after navigation.const handleLogout = async () => { try { setIsSigningOut(true); await supabase.auth.signOut(); - router.replace("/login?from=logout"); - queryClient.clear(); + queryClient.clear(); + router.replace("/login?from=logout"); } catch (error) { console.error("Unexpected error during logout:", error); } finally { - setIsSigningOut(false); + setIsSigningOut(false); // optional: skip if you observe warnings after route change } };
31-33: Stable avatar fallback initial.Ensure a visible fallback even when
full_nameis empty/undefined.const queryClient = useQueryClient(); +const initials = + userProfile?.full_name?.trim()?.[0]?.toUpperCase() ?? "?";- <AvatarFallback className="bg-muted text-muted-foreground font-medium text-xs"> - {userProfile?.full_name?.charAt(0)?.toUpperCase()} - </AvatarFallback> + <AvatarFallback className="bg-muted text-muted-foreground font-medium text-xs"> + {initials} + </AvatarFallback>Also applies to: 90-93
135-138: Allow logout while profile is loading.Disabling Logout on
isPending(profile query) blocks sign-out unnecessarily.- disabled={isPending || isSigningOut} + disabled={isSigningOut}apps/web/src/components/settings/account/DeleteAccount.tsx (2)
56-60: Reset confirmation input when dialog closes and autofocus field.Small UX polish: clear the typed text on close and focus the input on open.
-<AlertDialog> - <AlertDialogTrigger asChild> +<AlertDialog + onOpenChange={(open) => { + if (!open) setConfirmationText(""); + }} +> + <AlertDialogTrigger asChild> <Button variant="destructive">Delete</Button> </AlertDialogTrigger><Input id="confirmation" value={confirmationText} onChange={(e) => setConfirmationText(e.target.value)} className="w-full" + autoFocus />Also applies to: 70-81
31-42: Surface friendlier error text.Leaking raw
error.messagecan be noisy. Consider a generic toast with optional console log, or map known errors to user-friendly messages.apps/web/src/components/ui/avatar.tsx (1)
39-47: Avoid flash of fallback with a short delay.Radix
FallbacksupportsdelayMs. Adding a small delay smooths image loading transitions.<AvatarPrimitive.Fallback ref={ref} + delayMs={300} className={cn( "flex h-full w-full items-center justify-center rounded-full bg-muted", className, )} {...props} />apps/web/src/lib/db/users.ts (2)
12-21: ConsidermaybeSingle()or a clearer not-found pathIf a profile row doesn't exist,
.single()will throw. If that's a valid state in your app, prefer.maybeSingle()and handle null (or auto-seed). Otherwise, keep.single()and ensure a row is always created server-side on sign-up.- const { data, error } = await supabase + const { data, error } = await supabase .from("users") .select("*") .eq("id", session.user.id) - .single(); + .maybeSingle(); + if (!error && !data) { + throw new Error("User profile not found"); + }
75-85: Sign out locally after server-side deletionAfter a successful
/api/delete-user, consider signing out to clear local auth state and cached queries.if (!response.ok) { throw new Error("Failed to delete user"); } + // Best-effort local cleanup + await supabase.auth.signOut();apps/web/src/hooks/users.ts (2)
26-33: Avoid clobbering caller-providedmeta; merge insteadSpreading
optionsbefore settingmetaoverwrites any providedoptions.meta. Merge to preserve caller metadata while adding your errorMessage.- const resQuery = useQuery({ - queryKey: [UsersServerKeys.GET_USER_PROFILE], - queryFn: getUserProfile, - ...options, - meta: { - errorMessage: "Failed to get user profile", - }, - }); + const { meta: userMeta, ...rest } = options ?? {}; + const resQuery = useQuery({ + queryKey: [UsersServerKeys.GET_USER_PROFILE], + queryFn: getUserProfile, + ...rest, + meta: { ...(userMeta as Record<string, unknown>), errorMessage: "Failed to get user profile" }, + });
9-13: Default to invalidating the profile on mutations (unless caller overrides)Improves UX by keeping UI in sync after profile updates, avatar uploads, or deletion. Callers can still provide their own
onSuccess.import { type UseMutationOptions, type UseQueryOptions, useMutation, useQuery, + useQueryClient, } from "@tanstack/react-query"; @@ export const useMutateUserProfile = ( options?: UseMutationOptions< void, Error, { data: TablesUpdate<"users"> }, unknown >, ) => { - const resQuery = useMutation({ + const qc = useQueryClient(); + const resQuery = useMutation({ mutationFn: updateUserProfile, mutationKey: [UsersServerKeys.UPDATE_USER_PROFILE], - ...options, + onSuccess: async (...args) => { + await qc.invalidateQueries({ queryKey: [UsersServerKeys.GET_USER_PROFILE] }); + await options?.onSuccess?.(...args); + }, + ...options, }); @@ export const useMutateDeleteUser = ( options?: UseMutationOptions<void, Error, void, unknown>, ) => { - const resQuery = useMutation({ + const qc = useQueryClient(); + const resQuery = useMutation({ mutationFn: deleteUser, mutationKey: [UsersServerKeys.DELETE_USER], - ...options, + onSuccess: async (...args) => { + await qc.invalidateQueries(); + await options?.onSuccess?.(...args); + }, + ...options, }); @@ export const useUploadUserAvatar = ( options?: UseMutationOptions< { publicUrl: string }, Error, { file: File }, unknown >, ) => { - const resQuery = useMutation({ + const qc = useQueryClient(); + const resQuery = useMutation({ mutationFn: uploadUserAvatar, mutationKey: [UsersServerKeys.UPLOAD_USER_AVATAR], - ...options, + onSuccess: async (...args) => { + await qc.invalidateQueries({ queryKey: [UsersServerKeys.GET_USER_PROFILE] }); + await options?.onSuccess?.(...args); + }, + ...options, });Also applies to: 46-51, 58-63, 75-79
apps/web/src/components/ui/form.tsx (1)
158-165: Announce errors to assistive techAdd
role="alert"andaria-liveto ensure validation messages are announced.- <p + <p + role="alert" + aria-live="polite" ref={ref} id={formMessageId} className={cn("text-sm font-medium text-destructive", className)} {...props} >
…age and add server-only import in server.ts
… by refining fallback logic and increasing max upload size to 2MB
…ckage dependencies, and enhance styling for scrollbar visibility
…, add new email assets, and enhance footer component styling and structure
… update package dependencies
…nd auth callback route
…sed within the last 30 days
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (8)
apps/web/src/components/PWAInstallBanner.tsx (5)
40-43: iOS standalone detection incomplete.This check misses iOS devices that use
navigator.standalone. The fix was detailed in previous comments.
45-50: iOS detection logic is incorrect.The
!("standalone" in navigator)check suppresses Safari iOS. This issue was detailed in previous comments.
52-67: Event listener cleanup incomplete for iOS.The early return at line 66 prevents the
beforeinstallpromptlistener from being removed. This issue was detailed in previous comments.
69-86: Cleanup function unreachable on iOS.On iOS, the early return at line 66 prevents lines 78-86 from executing, so the
appinstalledlistener is never registered and cleanup never runs. Already flagged in previous comments.
126-126: Fixed minimum width causes mobile overflow.
min-w-[400px]will cause horizontal scrolling on phones narrower than 400px. This issue was detailed in previous comments.apps/web/src/app/api/auth/callback/route.ts (3)
12-16: Sanitize “next” to a same-origin absolute path; use it in all redirects.Current code concatenates
${origin}${next}directly. Normalize and restrict to a single-slash absolute path to prevent open-redirect/path confusion, then use the sanitized value everywhere.Apply:
- // if "next" is in params, use it as the redirect URL - const next = searchParams.get("next") ?? "/"; + // if "next" is in params, sanitize to a same-origin absolute path + const rawNext = searchParams.get("next") ?? "/"; + const sanitizedNext = (() => { + const s = String(rawNext).replace(/[\r\n]/g, "").trim(); + // allow only absolute paths with exactly one leading slash (no "//", no scheme) + if (!/^\/(?!\/)/.test(s)) return "/"; + return s.replace(/^\/+/, "/"); + })(); @@ - return NextResponse.redirect(`${origin}${next}`); + return NextResponse.redirect(`${origin}${sanitizedNext}`); @@ - return NextResponse.redirect(`${origin}${next}`); + return NextResponse.redirect(`${origin}${sanitizedNext}`);Also applies to: 43-46, 53-53
21-24: Validate X-Forwarded-Host and honor X-Forwarded-Proto safely; restrict to allowlist.Blindly trusting
x-forwarded-hostenables host-header spoofing and off-site redirects; also proto should be validated. Parse the first XFH value, compare against a known host (e.g., fromNEXT_PUBLIC_APP_URL), and use a vetted proto.Apply:
- const forwardedHost = request.headers.get("x-forwarded-host"); - const isLocalEnv = process.env.NODE_ENV === "development"; + const isLocalEnv = process.env.NODE_ENV === "development"; + // read and sanitize forwarded headers (first value only) + const forwardedHostHeader = request.headers.get("x-forwarded-host") || ""; + const forwardedHost = forwardedHostHeader.split(",")[0].trim().toLowerCase(); + const forwardedProtoRaw = request.headers.get("x-forwarded-proto") ?? "https"; + const forwardedProto = /^(https?)$/i.test(forwardedProtoRaw) + ? forwardedProtoRaw.toLowerCase() + : "https"; + // allow only our expected host (fallback to request origin when env not set) + const allowedHost = new URL(process.env.NEXT_PUBLIC_APP_URL || origin).host.toLowerCase(); @@ - if (forwardedHost) { - // in case we user load balancer or proxy, we need to redirect to the correct host - return NextResponse.redirect(`https://${forwardedHost}${next}`); - } + if (!isLocalEnv && forwardedHost && forwardedHost === allowedHost) { + // behind trusted proxy/load balancer + return NextResponse.redirect(`${forwardedProto}://${forwardedHost}${sanitizedNext}`); + }Set
NEXT_PUBLIC_APP_URL(e.g., https://techcompaniesportugal.fyi). Optionally support a comma-separatedALLOWED_REDIRECT_HOSTSif multiple domains are valid.Also applies to: 48-51
36-39: Align types for optional user name; avoid passing possibly-undefined to astringparam.
full_namemay be missing; make the parameter optional and coalesce in the template.Apply:
- sendWelcomeEmail( - data?.session?.user.email, - data?.session?.user.user_metadata.full_name, - ), + sendWelcomeEmail( + data.session.user.email, + data.session.user.user_metadata?.full_name, + ),-const sendWelcomeEmail = async (email: string, name = "there") => { +const sendWelcomeEmail = async (email: string, name?: string) => { const emailHtml = await render( WelcomeEmail({ - userFirstname: name, + userFirstname: name ?? "there", }), );Also applies to: 61-66
🧹 Nitpick comments (1)
apps/web/src/app/api/auth/callback/route.ts (1)
10-10: Rate limit the callback to reduce abuse.Implement a lightweight token-bucket (e.g., Upstash Ratelimit or in-memory for dev) keyed by IP or
session_state. Enforce ~5–10 req/min; on exceed, redirect to a soft error.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/src/actions/send-contact-message-action.ts(1 hunks)apps/web/src/app/api/auth/callback/route.ts(1 hunks)apps/web/src/components/PWAInstallBanner.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/actions/send-contact-message-action.ts
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/app/api/auth/callback/route.ts (3)
apps/web/src/lib/supabase/server.ts (1)
createClient(8-33)apps/web/src/emails/templates/welcome.tsx (1)
WelcomeEmail(21-168)apps/web/src/lib/email/index.ts (1)
emailService(31-33)
⏰ 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). (1)
- GitHub Check: Web E2E Tests
🔇 Additional comments (7)
apps/web/src/components/PWAInstallBanner.tsx (6)
1-11: LGTM!Clean imports and the
BeforeInstallPromptEventinterface correctly types the browser's install prompt API.
13-17: LGTM!State initialization is appropriate for managing the prompt lifecycle.
19-27: LGTM!Mobile detection combines user agent and viewport width checks appropriately.
89-110: LGTM!The install flow correctly handles the deferred prompt and tracks analytics.
112-119: LGTM!Dismiss handler correctly tracks analytics and stores the dismissal timestamp.
137-156: LGTM!Render logic appropriately shows iOS-specific instructions or an Install button based on platform capabilities.
apps/web/src/app/api/auth/callback/route.ts (1)
57-59: Good: explicit response for missing/failed code.The final redirect ensures the handler never falls through. LGTM.
…and user experience
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
apps/web/src/app/manifest.ts (1)
19-38: Icons: each declared size must point to an actual file; add maskable.All three entries reuse the same src. Provide correctly sized images per entry and include a maskable variant for 512×512.
Apply:
icons: [ { - src: "/assets/images/logo.png", + src: "/assets/images/icons/icon-192.png", sizes: "192x192", type: "image/png", purpose: "any", }, { - src: "/assets/images/logo.png", + src: "/assets/images/icons/icon-384.png", sizes: "384x384", type: "image/png", purpose: "any", }, { - src: "/assets/images/logo.png", + src: "/assets/images/icons/icon-512.png", sizes: "512x512", type: "image/png", - purpose: "any", + purpose: "any", }, + { + src: "/assets/images/icons/icon-512-maskable.png", + sizes: "512x512", + type: "image/png", + purpose: "maskable", + }, ],apps/web/src/components/PWAInstallBanner.tsx (1)
31-36: Critical: dismissal window check always fails (string vs integer).localStorage returns strings; Number.isInteger on a string is always false. Parse first and validate.
- const dismissed = localStorage.getItem("pwa-prompt-dismissed"); - if ( - dismissed && - Number.isInteger(dismissed) && - Date.now() - Number.parseInt(dismissed) < 1000 * 60 * 60 * 24 * 7 - ) { + const dismissed = localStorage.getItem("pwa-prompt-dismissed"); + const ts = dismissed ? Number.parseInt(dismissed, 10) : NaN; + if (!Number.isNaN(ts) && Date.now() - ts < 1000 * 60 * 60 * 24 * 7) { return; }apps/web/src/components/ContactButton.tsx (2)
106-115: Handle loading, error, and missing-email separately to avoid indefinite spinner.Currently any falsy
userEmailshows a spinner, masking fetch errors forever. Render explicit states.- const { data: userProfile } = useGetUserProfile(); + const { data: userProfile, isLoading, isError } = useGetUserProfile(); const userEmail = userProfile?.email; - if (!userEmail) { + if (isError) { + return ( + <div className="flex items-center justify-center text-red-500 text-sm py-4"> + <span>Failed to load user profile</span> + </div> + ); + } + + if (isLoading) { return ( <div className="flex items-center justify-center text-sm py-4"> <Loader2 className="h-6 w-6 animate-spin" /> </div> ); } + + if (!userEmail) { + return ( + <div className="flex items-center justify-center text-sm text-muted-foreground py-4"> + <span>No email associated with your account</span> + </div> + ); + }
117-120: Fix class typo to restore flex alignment.
tems-center→items-center.- <div className="flex tems-center gap-2 justify-center text-green-500 text-sm py-4"> + <div className="flex items-center gap-2 justify-center text-green-500 text-sm py-4">
🧹 Nitpick comments (7)
apps/web/src/app/manifest.ts (2)
8-14: Add a stable manifest id to avoid duplicate installs across paths.Chrome uses id to de-dupe installations. Use a stable, origin-relative id.
return { name: "Tech Companies Portugal", + id: "/", short_name: "Tech Companies PT",
15-15: Orientation: consider allowing any.Portrait-only can constrain tablets/desktops. Consider "any" unless you rely on portrait layouts.
- orientation: "portrait-primary", // evaluate change to "any"? + orientation: "any",apps/web/src/components/PWAInstallBanner.tsx (4)
41-45: iPadOS on Safari can report “Macintosh”; broaden iOS detection.Include the iPadOS heuristic (Mac UA + touch points). Extend the nav type accordingly.
- const nav = navigator as Navigator & { standalone?: boolean }; + const nav = navigator as Navigator & { + standalone?: boolean; + maxTouchPoints?: number; + }; @@ - const isIOSDevice = /iPad|iPhone|iPod/.test(navigator.userAgent); + const isIOSDevice = + /iPad|iPhone|iPod/.test(navigator.userAgent) || + (navigator.userAgent.includes("Macintosh") && + (nav.maxTouchPoints ?? 0) > 1);Also applies to: 49-52
64-69: Prefer ReturnType for browser timers.Avoid NodeJS.Timeout in client code to prevent type mismatches when DOM types are used without Node types.
- let timerIOS: NodeJS.Timeout | undefined; + let timerIOS: ReturnType<typeof setTimeout> | undefined;
80-83: Option: register appinstalled with { once: true }.Ensures auto-cleanup after first fire and simplifies listener management.
- if (!isIOSDevice) { - window.addEventListener("appinstalled", handleAppInstalled); - } + if (!isIOSDevice) { + window.addEventListener("appinstalled", handleAppInstalled, { once: true }); + } @@ - window.removeEventListener("appinstalled", handleAppInstalled); + // Optional: safe even if { once: true } already removed it + window.removeEventListener("appinstalled", handleAppInstalled);Also applies to: 89-94
107-118: Harden prompt flow with try/catch and analytics on failures.Some browsers can reject prompt() or userChoice; log failures and always clear deferredPrompt.
- // Show the prompt - deferredPrompt.prompt(); - - // Wait for the user to choose to install or not - const { outcome } = await deferredPrompt.userChoice; - - if (outcome === "accepted") { - setShowPrompt(false); - } - - setDeferredPrompt(null); + try { + await deferredPrompt.prompt(); + const { outcome } = await deferredPrompt.userChoice; + if (outcome === "accepted") { + setShowPrompt(false); + } + } catch (err) { + trackEvent("pwa_install_prompt_error", { + platform: isIOS ? "ios" : "android_chrome", + message: (err as Error)?.message ?? "unknown", + }); + } finally { + setDeferredPrompt(null); + }apps/web/src/components/ContactButton.tsx (1)
138-145: Add an accessible label for the textarea.Provide a label or aria-label; placeholders aren’t read as labels by screen readers.
- <Textarea + <label htmlFor="contact-message" className="sr-only"> + Message + </label> + <Textarea + id="contact-message" disabled={isPending} name="message" rows={4} placeholder="Your message" required />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/src/app/manifest.ts(1 hunks)apps/web/src/components/ContactButton.tsx(1 hunks)apps/web/src/components/PWAInstallBanner.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/components/ContactButton.tsx (4)
apps/web/src/lib/contexts/SessionContext.tsx (1)
useSession(59-65)apps/web/src/hooks/useMediaQuery.tsx (1)
useMediaQuery(7-23)apps/web/src/hooks/users.ts (1)
useGetUserProfile(23-36)apps/web/src/actions/send-contact-message-action.ts (1)
sendContactMessageAction(6-43)
⏰ 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). (1)
- GitHub Check: Web E2E Tests
… in sendContactMessageAction
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (10)
apps/web/src/components/CompaniesListHeader.tsx (1)
47-96: Pagination controls still lack keyboard accessibility attributes.The
onKeyDownhandlers have been added, but thedivelements still lacktabIndex={0}androle="button"attributes. WithouttabIndex, these divs remain unfocusable via keyboard, so theonKeyDownhandlers will never fire for keyboard users.Apply this diff to make the pagination controls keyboard accessible:
<div + role="button" + tabIndex={0} onClick={() => setSearchParams({ page: 1 })} className={cn( "hover:text-foreground flex items-center justify-center hover:cursor-pointer", isPreviousDisabled && "pointer-events-none text-muted-foreground", )} onKeyDown={(e) => { if (e.key === "Enter") { setSearchParams({ page: 1 }); } }} > <ChevronsLeft className="inline" size={18} /> </div> <div + role="button" + tabIndex={0} onKeyDown={(e) => { if (e.key === "Enter") { setSearchParams({ page: currentPage - 1 }); } }} onClick={() => setSearchParams({ page: currentPage - 1 })} className={cn( "hover:text-foreground flex items-center justify-center hover:cursor-pointer", isPreviousDisabled && "pointer-events-none text-muted-foreground", )} > <ChevronLeft className="inline" size={18} /> </div> <div + role="button" + tabIndex={0} onKeyDown={(e) => { if (e.key === "Enter") { setSearchParams({ page: currentPage + 1 }); } }} onClick={() => setSearchParams({ page: currentPage + 1 })} className={cn( "hover:text-foreground flex items-center justify-center hover:cursor-pointer", isNextDisabled && "pointer-events-none text-muted-foreground", )} > <ChevronRight className="inline" size={18} /> </div> <div + role="button" + tabIndex={0} onKeyDown={(e) => { if (e.key === "Enter") { setSearchParams({ page: totalPages }); } }} onClick={() => setSearchParams({ page: totalPages })} className={cn( "hover:text-foreground flex items-center justify-center hover:cursor-pointer", isNextDisabled && "pointer-events-none text-muted-foreground", )} > <ChevronsRight className="inline" size={18} /> </div>apps/web/package.json (1)
57-57: Critical: Invalid zod version4.0.14will cause installation failure.Zod version
4.0.14does not exist. The stable release line is v3.x (e.g.,3.23.8). This will causenpm installoryarn installto fail and block the build.Apply this diff to use a valid zod version:
- "zod": "4.0.14" + "zod": "^3.23.8"Verify the correct version at npmjs.com/package/zod.
apps/web/src/components/UserMenu.tsx (3)
35-47: Previously flagged issues remain unaddressed.This logout flow still has the issues identified in earlier reviews:
- Line 41: The stale comment should be removed (covered in past review).
- Lines 42-43: Errors are only logged to console without user-facing feedback (covered in past review).
Since these were already discussed, please refer to the previous review comments for the detailed analysis and suggested fixes.
146-150: Previously flagged: UseonSelectinstead ofonClickfor keyboard accessibility.This Radix DropdownMenuItem still uses
onClick(line 148), which was identified in a previous review as an accessibility issue. Radix menus requireonSelectto ensure keyboard interactions work correctly.Please refer to the earlier review comment for the detailed explanation and suggested fix.
146-157: Previously flagged: Incorrect disabled state logic.Line 149 still includes
isPendingin the disabled condition (disabled={isPending || isSigningOut}). As noted in the earlier review, profile loading state should not prevent users from logging out.Please refer to the previous review comment for the rationale and suggested fix (remove
isPendingfrom the condition).apps/web/src/components/ContactButton.tsx (4)
99-111: Distinguish loading from error states.The current logic shows a spinner for any falsy
userEmail, conflating "loading", "query error", and "missing email" scenarios. Users will see an indefinite spinner if the profile fetch fails.Destructure
isLoadingandisErrorfromuseGetUserProfileand handle each state explicitly:- const { data: userProfile } = useGetUserProfile(); + const { data: userProfile, isLoading, isError } = useGetUserProfile(); const userEmail = userProfile?.email; + if (isError) { + return ( + <div className="flex items-center justify-center text-red-500 text-sm py-4"> + <span>Failed to load user profile</span> + </div> + ); + } + - if (!userEmail) { + if (isLoading || !userEmail) { return ( <div className="flex items-center justify-center text-sm py-4"> <Loader2 className="h-6 w-6 animate-spin" /> </div> ); }
113-116: Fix CSS class typo (still present).Line 114 has
tems-centerwhich should beitems-centerfor proper flexbox alignment. This was previously marked as addressed but the typo remains in the current code.Apply this diff:
- <div className="flex tems-center gap-2 justify-center text-green-500 text-sm py-4"> + <div className="flex items-center gap-2 justify-center text-green-500 text-sm py-4">
118-132: Fix async transition and add timeout cleanup.Using an async function inside
startTransitionmeansisPendingwon't cover the network call, leaving the form enabled during submission and risking duplicate sends. Additionally, thesetTimeoutlacks cleanup on unmount.Apply these changes:
-import { useState, useTransition } from "react"; +import { useEffect, useRef, useState } from "react";const ContactForm = () => { const [isSubmitted, setSubmitted] = useState(false); - const [isPending, startTransition] = useTransition(); + const [isSubmitting, setSubmitting] = useState(false); + const timeoutRef = useRef<NodeJS.Timeout | null>(null); const { data: userProfile } = useGetUserProfile(); const userEmail = userProfile?.email; + useEffect(() => { + return () => { + if (timeoutRef.current) clearTimeout(timeoutRef.current); + }; + }, []);action={async (formData) => { - startTransition(async () => { - try { - await sendContactMessageAction(formData); - setSubmitted(true); - } catch (error) { - console.error(error); - toast.error("Failed to send message. Try again later."); - } finally { - setTimeout(() => setSubmitted(false), 2500); - } - }); + setSubmitting(true); + try { + await sendContactMessageAction(formData); + setSubmitted(true); + } catch (error) { + console.error(error); + toast.error("Failed to send message. Try again later."); + } finally { + if (timeoutRef.current) clearTimeout(timeoutRef.current); + timeoutRef.current = setTimeout(() => setSubmitted(false), 2500); + setSubmitting(false); + } }}
134-154: Update disabled states to useisSubmitting.After applying the fix for async transition, update the Textarea and Button to use
isSubmittinginstead ofisPending:<Textarea - disabled={isPending} + disabled={isSubmitting} name="message" rows={4} placeholder="Your message" required /> <Button variant="secondary" type="submit" className="w-full flex items-center gap-2" - disabled={isPending} + disabled={isSubmitting} > - {isPending ? ( + {isSubmitting ? ( <Loader2 className="h-4 w-4 animate-spin" /> ) : ( <Send size={14} /> )} Send </Button>apps/web/src/actions/send-contact-message-action.ts (1)
21-25: Add maximum length validation.While the message is now validated for presence, there's no upper limit. A contact form should enforce a reasonable maximum length to prevent abuse.
Apply this diff:
const message = formData.get("message")?.toString().trim(); if (!message || message.length === 0) { throw new Error("Message is required"); } + + if (message.length > 5000) { + throw new Error("Message is too long (max 5000 characters)"); + }
🧹 Nitpick comments (3)
apps/web/src/actions/send-contact-message-action.ts (3)
34-35: Add defensive handling for email parsing.Line 34 assumes
user.emailcontains an "@" character. While Supabase validates emails, defensive coding is recommended for robustness.Apply this diff:
- const firstPartEmail = user.email.split("@")[0]; + const firstPartEmail = user.email.includes("@") + ? user.email.split("@")[0] + : user.email; const fullName = user.user_metadata?.full_name;
37-58: Add error handling for email send operation.The
emailService.sendEmailcall lacks error handling. If the email service fails, users will receive a generic error instead of a clear message.Wrap the email send in a try-catch:
+ try { await emailService.sendEmail({ from: fromEmail, name: "Tech Companies Portugal", subject: "New message received — Tech Companies Portugal", body: ` <p>Hello 👋</p> <p>You have received a new message through the <strong>Tech Companies Portugal</strong> website.</p> <p><strong>From:</strong> ${firstPartEmail}${fullName ? ` (${fullName})` : ""}</p> <p><strong>Message:</strong></p> <blockquote style="border-left:3px solid #ccc;padding-left:8px;margin:10px 0;"> <p>${message}</p> </blockquote> <hr> <p style="font-size:12px;color:#666;"> Automated message from Tech Companies Portugal — ${new Date().toLocaleDateString()}. </p> `, to: toEmail, }); + } catch (error) { + console.error("Failed to send contact message:", error); + throw new Error("Failed to send message. Please try again later."); + }
7-7: Rate limiting needed for production.The TODO on line 7 correctly identifies that rate limiting is missing. Contact forms without rate limiting are vulnerable to spam and abuse.
This is important for production readiness. Would you like me to open a tracking issue for implementing rate limiting on this endpoint?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
apps/web/package.json(1 hunks)apps/web/src/actions/send-contact-message-action.ts(1 hunks)apps/web/src/components/AnimateNumber.tsx(0 hunks)apps/web/src/components/CompaniesHeader.tsx(1 hunks)apps/web/src/components/CompaniesListFooter.tsx(5 hunks)apps/web/src/components/CompaniesListHeader.tsx(5 hunks)apps/web/src/components/ContactButton.tsx(1 hunks)apps/web/src/components/FiltersButton.tsx(2 hunks)apps/web/src/components/FiltersPanelButton.tsx(2 hunks)apps/web/src/components/UserMenu.tsx(1 hunks)apps/web/src/lib/metadata.ts(3 hunks)
💤 Files with no reviewable changes (1)
- apps/web/src/components/AnimateNumber.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/components/FiltersPanelButton.tsx
🧰 Additional context used
🧬 Code graph analysis (5)
apps/web/src/components/UserMenu.tsx (5)
apps/web/src/lib/supabase/client.ts (1)
createClient(4-9)apps/web/src/lib/contexts/SessionContext.tsx (1)
useSession(59-65)apps/web/src/hooks/users.ts (1)
useGetUserProfile(23-36)packages/analytics/src/client/utils.ts (1)
trackEvent(6-13)apps/web/src/components/SocialIcons.tsx (1)
SocialIcons(5-51)
apps/web/src/components/ContactButton.tsx (4)
apps/web/src/lib/contexts/SessionContext.tsx (1)
useSession(59-65)apps/web/src/hooks/useMediaQuery.tsx (1)
useMediaQuery(7-23)apps/web/src/hooks/users.ts (1)
useGetUserProfile(23-36)apps/web/src/actions/send-contact-message-action.ts (1)
sendContactMessageAction(6-61)
apps/web/src/components/CompaniesListFooter.tsx (1)
apps/web/src/lib/utils.ts (1)
cn(5-7)
apps/web/src/components/FiltersButton.tsx (1)
apps/web/src/hooks/useSearchQueryParams.tsx (1)
useSearchQueryParams(5-26)
apps/web/src/actions/send-contact-message-action.ts (1)
apps/web/src/lib/email/index.ts (1)
emailService(31-33)
⏰ 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). (1)
- GitHub Check: Web E2E Tests
🔇 Additional comments (20)
apps/web/src/components/CompaniesListHeader.tsx (1)
9-9: LGTM: Import path correction.The import path has been correctly updated to reflect the actual location of the hook.
apps/web/src/components/CompaniesListFooter.tsx (2)
8-8: LGTM: Import path correction.The import path has been correctly updated to reflect the actual location of the hook.
33-84: LGTM: Styling updates.The addition of
border-noneto the Badge and Button elements is a minor cosmetic improvement. The pagination controls in this footer use properButtoncomponents, which are naturally keyboard accessible.apps/web/package.json (2)
9-9: Confirm intent of--unsafelint flag.The
--unsafeflag bypasses certain Biome safety checks. Please verify this aligns with your project's linting policy and that it's intentional.
20-56: Overall dependency additions look good for PR objectives.The new packages (Supabase, auth, email, UI, query, forms) align well with integrating Supabase authentication and building user-facing pages. Version selections are current and stable across the board—particularly solid choices for next@15.4.7 (includes security fixes per learnings), @tanstack/react-query@5.84.1 (stable v5), sonner@2.0.7, and react-hook-form@7.62.0. DevDependencies for email preview tooling are appropriate.
Action required: Fix the zod version blocker (line 57) before merging.
Also applies to: 58-74
apps/web/src/lib/metadata.ts (2)
21-28: LGTM! Enhanced keyword targeting for Portugal-focused searches.The keyword refinements improve SEO specificity by adding "portugal" to generic terms and expanding coverage with career and directory-focused keywords. This aligns well with the app's geographic focus.
44-44: Approved—no issues found.The OG image route exists at
apps/web/src/app/api/og/route.tsx, and Next.js metadata will correctly resolve the relative path"api/og"tohttps://techcompaniesportugal.fyi/api/ogusing the configuredmetadataBase. The syntax change from template literals to regular strings is safe and improves consistency.apps/web/src/components/FiltersButton.tsx (3)
1-2: LGTM: Import path correctly updated.The import path change from
"./hooks/useSearchQueryParams"to"../hooks/useSearchQueryParams"is correct given the file is located incomponents/and the hook is inhooks/. The removal of the X icon import aligns with the removal of the clear filters button.
19-25: LGTM: Button UI appropriately simplified for mobile.The changes make the button more compact for mobile:
size="sm"prop reduces button size- Removal of "Filters" text label creates a cleaner icon-only button
- Accessibility is maintained via
aria-label="Open filters"on line 18- Badge count still displays when filters are applied
These changes are consistent with mobile-first UI design patterns.
11-11: ****The removal of
setSearchParamsfrom FiltersButton does not represent missing filter-clearing functionality. The clear filters button is still present in the SearchSideBar component (line 131:setSearchParams(null)with aria-label "Reset filters"). This change appears to be an intentional refactor to consolidate the reset functionality in the main filter sidebar rather than the mobile filters button.Likely an incorrect or invalid review comment.
apps/web/src/components/UserMenu.tsx (3)
49-52: LGTM!The loading state logic correctly handles both session initialization and profile data fetching, providing a smooth user experience during async operations.
54-80: LGTM!The unauthenticated state is well-implemented with:
- Proper routing (Next.js Link for internal navigation)
- Secure external links with correct
relattributes- Analytics tracking for user interactions
- Good accessibility with
aria-labelattributes
82-131: LGTM!The dropdown menu structure and Settings navigation are well-implemented:
- Avatar component properly handles missing images with character fallback
- Privacy-conscious
referrerPolicy="no-referrer"on avatar image- Proper use of Next.js Link with
asChildpattern for the Settings item- Analytics tracking for interactions
- Good null-safety with optional chaining
apps/web/src/components/ContactButton.tsx (3)
32-41: LGTM: Proper feature gating.The authentication and route-based visibility logic correctly restricts the contact button to authenticated users on allowed pages.
43-64: LGTM: Clean Dialog implementation.The desktop Dialog pattern follows best practices with proper accessibility attributes via DialogHeader/DialogTitle/DialogDescription.
66-97: LGTM: Consistent Drawer implementation.The mobile Drawer provides equivalent functionality to the desktop Dialog with proper close affordance.
apps/web/src/actions/send-contact-message-action.ts (2)
6-19: LGTM: Proper authentication check.The action correctly validates the user is authenticated before proceeding, preventing unauthorized message submissions.
27-32: LGTM: Environment validation added.The environment variable validation properly prevents runtime crashes and provides clear error messages, addressing a previous critical concern.
apps/web/src/components/CompaniesHeader.tsx (2)
5-17: Good accessibility improvement with aria-labelledby.The
aria-labelledbyattribute properly references the heading'sid, creating a semantic relationship between the section and its heading. This is an appropriate accessibility enhancement.
13-16: The color contrast meets WCAG AA compliance for large text.This h1 element with
text-4xl font-bold(36px) qualifies as "large text" under WCAG standards. The measured contrast ratio of 3.18:1 exceeds the WCAG AA requirement of 3:1 for large text. No accessibility issue exists.
…and update environment variables
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
apps/web/package.json (1)
58-58: Invalid zod version — package installation will fail.Zod version
4.0.14does not exist. The latest stable release is in the v3.x family (e.g.,3.23.8). This will causenpm installoryarn installto fail. This issue was flagged in the previous review and remains unresolved.Apply this diff to use a valid stable zod version:
- "zod": "4.0.14" + "zod": "^3.23.8"Verify the correct version at npmjs.com/package/zod.
apps/web/src/actions/send-contact-message-action.ts (2)
56-77: CRITICAL: XSS vulnerability still present - sanitize all user input.This issue was flagged in a previous review but has not been addressed. Lines 64 and 68 directly interpolate user-controlled data (
firstPartEmail,fullName, andmessage) into HTML without sanitization. A malicious user can inject<script>tags or other HTML that could execute when the recipient opens the email.Apply this diff to escape HTML:
+// Escape HTML to prevent XSS +const escapeHtml = (text: string) => { + return text + .replace(/&/g, "&") + .replace(/</g, "<") + .replace(/>/g, ">") + .replace(/"/g, """) + .replace(/'/g, "'"); +}; + +const sanitizedMessage = escapeHtml(message); +const sanitizedFirstPartEmail = escapeHtml(firstPartEmail); +const sanitizedFullName = fullName ? escapeHtml(fullName) : ""; + await emailService.sendEmail({ from: fromEmail, name: "Tech Companies Portugal", subject: "New message received — Tech Companies Portugal", body: ` <p>Hello 👋</p> <p>You have received a new message through the <strong>Tech Companies Portugal</strong> website.</p> - <p><strong>From:</strong> ${firstPartEmail}${fullName ? ` (${fullName})` : ""}</p> + <p><strong>From:</strong> ${sanitizedFirstPartEmail}${sanitizedFullName ? ` (${sanitizedFullName})` : ""}</p> <p><strong>Message:</strong></p> <blockquote style="border-left:3px solid #ccc;padding-left:8px;margin:10px 0;"> - <p>${message}</p> + <p>${sanitizedMessage}</p> </blockquote> <hr> <p style="font-size:12px;color:#666;"> Automated message from Tech Companies Portugal — ${new Date().toLocaleDateString()}. </p> `, to: toEmail, });
40-44: Add maximum length validation for the message.While the code now validates that the message is non-empty (addressing part of the previous review), there's no maximum length check. A malicious user could submit extremely long messages, potentially causing issues with email delivery, storage, or abuse.
Apply this diff to add a max length check:
const message = formData.get("message")?.toString().trim(); if (!message || message.length === 0) { throw new Error("Message is required"); } + +if (message.length > 5000) { + throw new Error("Message is too long (max 5000 characters)"); +}
🧹 Nitpick comments (4)
apps/web/.env.example (1)
1-12: Consider optional formatting improvements.Static analysis suggests alphabetically ordering keys within their prefix groups and adding a blank line at the end of the file. These are stylistic improvements that enhance consistency but are not critical.
Apply this diff to address the formatting:
# For turbo builds, prefixes with NEXT_PUBLIC_ are automatically considered to force a rebuild NEXT_PUBLIC_POSTHOG_KEY= NEXT_PUBLIC_POSTHOG_HOST= NEXT_PUBLIC_SUPABASE_URL= # Make sure to enable RLS and policies to use this key in client NEXT_PUBLIC_SUPABASE_ANON_KEY= +ARCJET_KEY= +CONTACT_FROM_EMAIL= +CONTACT_TO_EMAIL= +PLUNK_API_KEY= SUPABASE_SERVICE_ROLE_KEY= SUPABASE_URL= -PLUNK_API_KEY= -CONTACT_FROM_EMAIL= -CONTACT_TO_EMAIL= -ARCJET_KEY= +apps/web/src/components/CompaniesHeader.tsx (3)
6-6: A11y: Good use of aria-labelledby; avoid ID collisions.If this header can render more than once, hardcoding id risks duplicates. Use React’s useId and allow an optional override.
Apply:
+import { useId } from "react" -export default function CompaniesHeader() { +export default function CompaniesHeader({ headingId }: { headingId?: string }) { + const autoId = useId() + const id = headingId ?? autoId return ( <section className="font-mono relative w-full overflow-hidden py-8 text-center" - data-testid="companies-header" - aria-labelledby="companies-heading" + data-testid="companies-header" + aria-labelledby={id} >And update the heading to use the same id (see next comment). Also consider moving tests to role-based queries (e.g., getByRole('region', { name: /find top tech companies/i })).
10-17: Make heading level configurable; ensure rotate utility exists.
- To prevent multiple H1s when used inside layouts that already render an H1, accept a headingLevel/as prop and render the appropriate tag (h1/h2/h3). Keep aria-labelledby pointing to this element.
- Tailwind’s -rotate-1 may not exist unless customized. Safer: -rotate-[1deg].
Example (sketch):
type HeadingTag = 'h1' | 'h2' | 'h3' | 'h4' export default function CompaniesHeader({ headingId, headingLevel = 'h1', }: { headingId?: string; headingLevel?: HeadingTag }) { const id = headingId ?? useId() const Heading = headingLevel as keyof JSX.IntrinsicElements return ( /* ... */ - <h1 id="companies-heading" className="text-4xl font-bold tracking-tight"> + <Heading id={id} className="text-4xl font-bold tracking-tight"> <span className="bg-orange-100/50 -rotate-[1deg] p-1 shadow-sm inline-block"> <span className="text-red-500/90">Find top tech companies</span>{" "} in Portugal </span> - </h1> + </Heading>Please confirm whether your Tailwind config defines rotate-1; if not, the arbitrary value form will compile reliably.
21-22: Verify nonstandard Tailwind size text-md (or switch to a default).text-md isn’t a Tailwind default. If your theme doesn’t define it, it will be ignored. Prefer text-base (or a responsive combo) unless you’ve extended the theme.
Option A (default Tailwind):
-<p className="text-md text-gray-600 max-w-2xl mx-auto font-semibold"> +<p className="text-base text-gray-600 max-w-2xl mx-auto font-semibold">Option B (responsive):
-<p className="text-md text-gray-600 max-w-2xl mx-auto font-semibold"> +<p className="text-base md:text-lg text-gray-600 max-w-2xl mx-auto font-semibold">If text-md is intentional via theme extension, ignore this change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
apps/web/.env.example(1 hunks)apps/web/package.json(1 hunks)apps/web/src/actions/send-contact-message-action.ts(1 hunks)apps/web/src/components/CompaniesHeader.tsx(1 hunks)apps/web/src/components/PWAInstallBanner.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/actions/send-contact-message-action.ts (1)
apps/web/src/lib/email/index.ts (1)
emailService(31-33)
🪛 dotenv-linter (3.3.0)
apps/web/.env.example
[warning] 3-3: [UnorderedKey] The NEXT_PUBLIC_POSTHOG_HOST key should go before the NEXT_PUBLIC_POSTHOG_KEY key
(UnorderedKey)
[warning] 6-6: [UnorderedKey] The NEXT_PUBLIC_SUPABASE_ANON_KEY key should go before the NEXT_PUBLIC_SUPABASE_URL key
(UnorderedKey)
[warning] 9-9: [UnorderedKey] The PLUNK_API_KEY key should go before the SUPABASE_SERVICE_ROLE_KEY key
(UnorderedKey)
[warning] 10-10: [UnorderedKey] The CONTACT_FROM_EMAIL key should go before the NEXT_PUBLIC_POSTHOG_HOST key
(UnorderedKey)
[warning] 11-11: [UnorderedKey] The CONTACT_TO_EMAIL key should go before the NEXT_PUBLIC_POSTHOG_HOST key
(UnorderedKey)
[warning] 12-12: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 12-12: [UnorderedKey] The ARCJET_KEY key should go before the CONTACT_FROM_EMAIL key
(UnorderedKey)
⏰ 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). (1)
- GitHub Check: Web E2E Tests
🔇 Additional comments (12)
apps/web/package.json (3)
9-9: Clarify the intent of--unsafeflag in the lint script.The lint script now runs with the
--unsafeflag, which may disable certain recommended checks in Biome. Confirm this is intentional and document why these checks are being skipped if this is a known trade-off.
16-17: Good: db:types script now correctly uses dotenv--separator.The
db:typesscript has been corrected to include the--separator after dotenv options and removes single quotes around$PROJECT_REF, allowing proper shell variable expansion. The newemail:devscript supports local email template preview, which aligns well with the new email templating feature.
20-58: Confirm @arcjet/next beta usage is acceptable; address version mismatch.@arcjet/next has no stable release available—only beta versions. The package.json specifies 1.0.0-beta.13, but npm shows version 1.0.0-beta.11 as the latest published. Verify that using an unreleased beta version aligns with your production stability requirements and clarify why beta.13 is specified when npm's main registry shows beta.11.
apps/web/src/actions/send-contact-message-action.ts (3)
1-6: LGTM!Imports and server directive are correctly configured for a Next.js server action.
21-32: LGTM!Authentication logic correctly validates the user session and throws an appropriate error if the user is not authenticated.
46-55: LGTM!Environment variable validation is correct. The derived sender information will need sanitization before being used in the email body (see next comment).
apps/web/src/components/PWAInstallBanner.tsx (6)
1-11: LGTM: Clean imports and proper type definition.The interface correctly types the non-standard
beforeinstallpromptevent API used by Chrome/Edge.
43-50: LGTM: Robust standalone detection.The combined check for both
display-mode: standaloneandnavigator.standaloneproperly handles PWA installation detection across browsers and older iOS versions.
64-97: LGTM: Event listeners properly cleaned up.The unified cleanup function correctly removes all event listeners and conditionally clears the iOS timer, preventing memory leaks on component unmount.
100-121: LGTM: Proper deferred prompt handling.The install flow correctly guards against missing prompts, triggers the native installation dialog, and cleans up state based on the user's choice.
123-130: LGTM: Dismissal properly tracked and persisted.The handler correctly stores the dismissal timestamp and tracks the event with platform context.
136-170: LGTM: Clean, accessible UI with proper responsive behavior.The banner correctly adapts between iOS (instruction-only) and Chrome/Edge (with install button), includes proper accessibility labels, and handles mobile layouts without overflow.
…improve localStorage handling
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/src/components/PWAInstallBanner.tsx(1 hunks)
⏰ 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). (1)
- GitHub Check: Web E2E Tests
🔇 Additional comments (1)
apps/web/src/components/PWAInstallBanner.tsx (1)
1-187: Well-structured PWA banner implementation with proper device detection and cleanup.The component demonstrates solid engineering: unified event listener cleanup addresses past memory leak concerns, proper iOS vs non-iOS handling with device-specific messaging, dismissal persistence with a 7-day window, and analytics integration. The type-safe approach with
BeforeInstallPromptEventinterface andReturnType<typeof setTimeout>for the timer shows attention to detail.Once the localStorage exception handling (flagged separately) is addressed, this implementation will be production-ready.
…attacks and improve localStorage error handling in PWAInstallBanner
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/src/app/api/auth/callback/route.ts (1)
13-13: Critical: Sanitize the "next" parameter to prevent open redirect attacks.The
nextparameter is used directly in redirects without validation, allowing attackers to craft protocol-relative URLs like?next=//evil.comthat bypass same-origin checks. This enables phishing attacks where users click what appears to be a legitimate OAuth callback URL but are redirected to an attacker-controlled site.Apply this diff to restrict
nextto safe same-origin absolute paths:- const next = searchParams.get("next") ?? "/"; + const rawNext = searchParams.get("next") ?? "/"; + // Only allow same-origin absolute paths like "/foo", reject protocol-relative "//..." or absolute URLs + const next = /^\/(?!\/)/.test(rawNext) ? rawNext : "/";This regex ensures the path starts with exactly one slash and rejects protocol-relative or external URLs.
🧹 Nitpick comments (4)
apps/web/src/components/PWAInstallBanner.tsx (2)
19-88: Solid initialization logic with proper cleanup.The detection flow correctly handles mobile/iOS identification, standalone state checks, and event listener lifecycle. All previously flagged memory leaks and iOS detection issues have been resolved. The unified cleanup at lines 78-87 properly removes listeners and clears timers.
Optional: Consider extracting magic numbers (7-day dismissal period at line 29, 2-second iOS delay at line 59) into named constants at the top of the file for easier maintenance.
126-160: Well-structured and accessible UI.The banner responsively adapts from full-width on mobile to a floating card on desktop, with proper
aria-labelattributes and conditional content based on platform capabilities.Optional: Consider adding
md:max-w-[400px]to line 127 to constrain the banner width on larger screens for a more polished desktop appearance.apps/web/src/app/api/auth/callback/route.ts (2)
33-38: Add error handling to the background email task to prevent unhandled rejections.If
sendWelcomeEmailfails (due to network issues, invalid email, or template errors), the promise rejection will be unhandled, creating noise in logs and monitoring.Apply this diff to catch and log email failures gracefully:
waitUntil( sendWelcomeEmail( data?.session?.user.email, data?.session?.user.user_metadata.full_name, - ), + ).catch((err) => { + console.error("[OAuth callback] Welcome email failed:", err); + }), );
34-36: Use optional chaining consistently for user_metadata access.While the default parameter in
sendWelcomeEmailhandles undefined safely, using optional chaining here improves type consistency and makes the nullable path explicit.Apply this diff:
sendWelcomeEmail( data?.session?.user.email, - data?.session?.user.user_metadata.full_name, + data?.session?.user.user_metadata?.full_name, ),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/src/actions/send-contact-message-action.ts(1 hunks)apps/web/src/app/api/auth/callback/route.ts(1 hunks)apps/web/src/components/PWAInstallBanner.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/actions/send-contact-message-action.ts
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/app/api/auth/callback/route.ts (3)
apps/web/src/lib/supabase/server.ts (1)
createClient(8-33)apps/web/src/emails/templates/welcome.tsx (1)
WelcomeEmail(21-168)apps/web/src/lib/email/index.ts (1)
emailService(31-33)
⏰ 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). (1)
- GitHub Check: Web E2E Tests
🔇 Additional comments (4)
apps/web/src/components/PWAInstallBanner.tsx (4)
1-11: Excellent setup and TypeScript interface.The
BeforeInstallPromptEventinterface correctly types the non-standard browser API, and all imports are clean and necessary.
90-111: Install flow correctly implements the PWA prompt API.The handler properly guards against missing prompts, tracks analytics with platform context, and manages the deferred prompt lifecycle.
113-120: Dismissal handling is clean and safe.The handler tracks the event, updates state, and safely persists the dismissal via the error-wrapped localStorage helper.
163-191: Robust localStorage helpers with comprehensive error handling.Both helpers correctly wrap storage access in try-catch blocks, validate parsed values, and provide safe defaults. This addresses all previously flagged SecurityError and dismissal logic issues.
Summary by CodeRabbit
New Features
UI/UX Improvements
Infrastructure