Setup Supabase SSR #84
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
Caution Review failedThe pull request is closed. WalkthroughReplaces auth-helpers with custom Supabase SSR/API clients and wrappers (withAuth, withSupabase, createMiddlewareClient), moves many API handlers and getServerSideProps to these wrappers, adds browser/server/static client factories, updates middleware and _app to local UserContext, and introduces targeted ts-ignore annotations and dependency updates. (≤50 words) Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Browser as Browser
participant MW as Next.js Middleware
participant MiddlewareClient as createMiddlewareClient
participant Supabase as Supabase SSR Client
participant Resp as NextResponse
Browser->>MW: HTTP request
MW->>MiddlewareClient: createMiddlewareClient(req)
MiddlewareClient-->>MW: { supabase, response }
MW->>Supabase: supabase.auth.getSession() -> supabase.auth.getUser()
alt authenticated
MW-->>Browser: return response (NextResponse) with cookies/headers
else unauthenticated
MW-->>Browser: redirect to /login?redirectedFrom=...
end
sequenceDiagram
autonumber
participant Client as HTTP Client
participant API as Next.js API Route (withAuth)
participant Factory as createServerClientForAPI
participant Supabase as Supabase Server Client
participant Handler as Wrapped Handler
Client->>API: HTTP request
API->>Factory: createServerClientForAPI({ req, res })
Factory-->>API: supabase
API->>Supabase: supabase.auth.getUser()
alt no user
API-->>Client: 401 Unauthorized
else user present
API->>Handler: handler(req,res,{ user, supabase })
Handler-->>Client: 200 / 4xx / 5xx response
end
sequenceDiagram
autonumber
participant Browser as Browser
participant GSSP as getServerSideProps (withSupabase)
participant Factory as createServerClientSSR
participant Supabase as Supabase SSR Client
participant Page as Page Component
Browser->>GSSP: Request page
GSSP->>Factory: createServerClientSSR(ctx)
Factory-->>GSSP: supabase
GSSP->>Supabase: supabase.auth.getUser()
alt no user
GSSP-->>Browser: redirect to /login?redirectedFrom=...
else user present
GSSP->>Page: return props (handler result)
Page-->>Browser: Rendered HTML
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/utils/useUser.tsx (1)
18-34: Fix context typing: default value masks missing provider; nullability mismatches.
- createContext with a concrete default object means
useUserDatawill never throw when provider is absent.billingDetailsandsupabaseare declared non-null but set tonull.Apply:
-const UserContext = createContext<{ - loading: boolean; - session: Session | null; - user: User | null; - billingDetails: IBillingInfo; - fetchBilling: () => void; - signOut: () => Promise<{ error: Error | null }>; - supabase: SupabaseClient<Database>; -}>({ - loading: true, - session: null, - user: null, - billingDetails: null, - fetchBilling: () => null, - signOut: () => null, - supabase: null, -}); +type UserContextValue = { + loading: boolean; + session: Session | null; + user: User | null; + billingDetails: IBillingInfo | null; + fetchBilling: () => void; + signOut: () => Promise<{ error: Error | null }>; + supabase: SupabaseClient<Database>; +}; + +const UserContext = createContext<UserContextValue | undefined>(undefined); @@ -const [billingDetails, setBillingDetails] = useState<IBillingInfo>(null); +const [billingDetails, setBillingDetails] = useState<IBillingInfo | null>(null); @@ -export const useUserData = () => { +export const useUserData = () => { const context = useContext(UserContext); if (context === undefined) { throw new Error(`useUserData must be used within a UserContextProvider.`); } return context; };Also applies to: 46-48, 125-129
apps/web/middleware.ts (1)
18-21: Preserve Supabase cookies on redirect.Cookies set via the SSR client are attached to
response. Redirecting with a freshNextResponsedrops them. Copy cookies into the redirect response.- return NextResponse.redirect(redirectUrl); + const redirectResponse = NextResponse.redirect(redirectUrl); + for (const { name, value } of response.cookies.getAll()) { + redirectResponse.cookies.set(name, value); + } + return redirectResponse;
🧹 Nitpick comments (8)
apps/web/utils/useSSR.ts (1)
4-12: LGTM. Consider explicit return type for clarity.Typing the return helps callers and keeps API stable.
-export async function getPage(supabase: SupabaseClient<Database>, id: string) { +export async function getPage( + supabase: SupabaseClient<Database>, + id: string +): Promise<Database["public"]["Tables"]["pages"]["Row"] | null> {apps/web/middleware.ts (1)
8-9: Redundant getSession call (result unused).If this is only to refresh cookies, add a comment; otherwise remove to save an auth round-trip.
apps/web/utils/supabase/server.ts (2)
10-15: Defensive cookie read to avoid undefined access.Guard against missing req.cookies.
- getAll() { - return Object.keys(context.req.cookies).map((name) => ({ - name, - value: context.req.cookies[name], - })); - }, + getAll() { + const jar = context.req?.cookies ?? {}; + return Object.entries(jar).map(([name, value]) => ({ name, value: String(value) })); + },
4-4: Type the context param (no any).Use concrete Next.js request/response types.
+import type { GetServerSidePropsContext, NextApiRequest, NextApiResponse } from "next"; -export function createServerClientSSR(context: { req: any; res: any }) { +export function createServerClientSSR( + context: + | GetServerSidePropsContext + | { req: NextApiRequest; res: NextApiResponse } +) {apps/web/utils/supabase/supabase-admin.ts (2)
24-31: Avoid extra auth round-trip; derive user from session.One call to getSession is enough; it already contains user.
- const { - data: { user }, - error, - } = await supabase.auth.getUser(); - - if (error) { - console.error("Error getting user:", error); - }
33-41: Inline user from session and simplify return.const { data: { session }, } = await supabase.auth.getSession(); - return { - supabase, - session, - user, - }; + const user = session?.user ?? null; + return { supabase, session, user };apps/web/utils/supabase/middleware.ts (2)
6-10: Lazy-init response to avoid unnecessary NextResponse allocation.-export function createMiddlewareClient(request: NextRequest) { - let response = NextResponse.next({ - request: { - headers: request.headers, - }, - }); +export function createMiddlewareClient(request: NextRequest) { + let response: NextResponse | undefined;
39-40: Always return a response; fall back if setAll didn’t run.- return { supabase, response }; + return { + supabase, + response: response ?? NextResponse.next({ request: { headers: request.headers } }), + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
apps/web/middleware.ts(1 hunks)apps/web/package.json(1 hunks)apps/web/pages/_app.tsx(2 hunks)apps/web/pages/api/auth/callback.ts(1 hunks)apps/web/utils/supabase/client.ts(1 hunks)apps/web/utils/supabase/middleware.ts(1 hunks)apps/web/utils/supabase/server.ts(1 hunks)apps/web/utils/supabase/supabase-admin.ts(3 hunks)apps/web/utils/useSSR.ts(1 hunks)apps/web/utils/useUser.tsx(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
apps/web/utils/useSSR.ts (1)
packages/supabase/types/index.ts (1)
Database(9-836)
apps/web/utils/supabase/client.ts (1)
packages/supabase/types/index.ts (1)
Database(9-836)
apps/web/utils/supabase/server.ts (1)
packages/supabase/types/index.ts (1)
Database(9-836)
apps/web/utils/supabase/middleware.ts (1)
packages/supabase/types/index.ts (1)
Database(9-836)
apps/web/pages/api/auth/callback.ts (2)
apps/web/utils/supabase/server.ts (1)
createServerClientSSR(4-49)apps/web/data/routes.data.ts (1)
ROUTES(1-26)
apps/web/middleware.ts (1)
apps/web/utils/supabase/middleware.ts (1)
createMiddlewareClient(5-40)
apps/web/utils/supabase/supabase-admin.ts (1)
apps/web/utils/supabase/server.ts (1)
createServerClientSSR(4-49)
apps/web/pages/_app.tsx (1)
apps/web/utils/useUser.tsx (1)
UserContextProvider(36-123)
apps/web/utils/useUser.tsx (3)
apps/web/utils/supabase/client.ts (1)
createClient(4-9)apps/web/data/user.interface.ts (1)
IBillingInfo(9-31)apps/web/data/routes.data.ts (1)
ROUTES(1-26)
🪛 GitHub Check: CodeQL
apps/web/pages/api/auth/callback.ts
[warning] 37-37: Server-side URL redirect
Untrusted URL redirection depends on a user-provided value.
🔇 Additional comments (7)
apps/web/package.json (1)
26-26: No action needed: @supabase/ssr ^0.7.0 is correctly constrained and no legacy auth-helpers found
Caret on 0.7.0 already restricts updates to <0.8.0, and searches returned no imports of@supabase/auth-helpers-*orcreatePages*Client.apps/web/pages/_app.tsx (1)
63-66: Support SSR session or rely on fallback. _app.tsx passespageProps.initialSession, but no page-levelgetServerSidePropsor_app.getInitialPropsreturns it—so it’s alwaysundefinedand falls back tonull. To enable SSR auth, have your SSR methods returninitialSession(e.g. viasupabase.auth.getSession()ingetServerSidePropsor_app.getInitialPropsusingcreateServerClientSSR); otherwise confirm your flow relies solely on client-side/middleware auth. [apps/web/pages/_app.tsx:63]apps/web/pages/api/auth/callback.ts (1)
12-33: Nice: explicit error handling around exchangeCodeForSession.Clear logs and user-facing errors improve debuggability and UX.
apps/web/utils/supabase/supabase-admin.ts (2)
22-22: Good switch to shared SSR client.
19-21: [waiting for script results]apps/web/utils/supabase/middleware.ts (2)
12-37: Middleware cookie bridge looks correct.Batch set via setAll; re-create response with updated headers; then mirror cookies to response.
21-24: No action needed:NextRequest.cookies.setis supported in Next.js 14.2.25 middleware.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/utils/useUser.tsx (1)
18-34: Fix context typing: defaults don’t match declared types; guard is ineffective.
billingDetailsis declared non-nullable but initialized withnull.supabaseis declared non-nullable but defaulted tonull.fetchBillingis typed() => voidbut returns a promise.useUserDatachecks forundefined, butcreateContextnever returnsundefineddue to a non-undefined default, making the guard unreachable.Refactor to use an
undefineddefault and nullable shapes where applicable.@@ -const UserContext = createContext<{ - loading: boolean; - session: Session | null; - user: User | null; - billingDetails: IBillingInfo; - fetchBilling: () => void; - signOut: () => Promise<{ error: Error | null }>; - supabase: SupabaseClient<Database>; -}>({ - loading: true, - session: null, - user: null, - billingDetails: null, - fetchBilling: () => null, - signOut: () => null, - supabase: null, -}); +type UserContextValue = { + loading: boolean; + session: Session | null; + user: User | null; + billingDetails: IBillingInfo | null; + fetchBilling: () => Promise<IBillingInfo | undefined>; + signOut: () => Promise<{ error: Error | null }>; + supabase: SupabaseClient<Database>; +}; + +const UserContext = createContext<UserContextValue | undefined>(undefined); @@ -const [billingDetails, setBillingDetails] = useState<IBillingInfo>(null); +const [billingDetails, setBillingDetails] = useState<IBillingInfo | null>(null); @@ -export const useUserData = () => { +export const useUserData = () => { const context = useContext(UserContext); - if (context === undefined) { + if (!context) { throw new Error(`useUserData must be used within a UserContextProvider.`); } return context; };Also applies to: 47-47, 127-131
♻️ Duplicate comments (3)
apps/web/pages/api/auth/callback.ts (1)
36-46: Harden redirect validation; block protocol-relative “//” and CRLF.This will address the CodeQL open-redirect finding and tighten checks.
- let redirectTo = ROUTES.PAGES; - if ( - typeof redirectedFrom === "string" && - redirectedFrom.startsWith("/") && - !redirectedFrom.includes("://") && - !redirectedFrom.includes("\\") - ) { - redirectTo = redirectedFrom; - } + const isSafeInternalPath = (p: unknown): p is string => { + if (typeof p !== "string") return false; + const s = p.trim(); + return ( + s.startsWith("/") && + !s.startsWith("//") && + !s.includes("://") && + !s.includes("\\") && + !/[\r\n]/.test(s) + ); + }; + + let redirectTo = ROUTES.PAGES; + if (isSafeInternalPath(redirectedFrom)) { + redirectTo = String(redirectedFrom).trim(); + }apps/web/utils/supabase/client.ts (1)
4-15: Resolved: env guards added; browser client wiring looks good.Non-null assertions removed in favor of runtime checks; typed
createBrowserClient<Database>return is correct.apps/web/utils/useUser.tsx (1)
63-73: Sign-out order + PostHog reset: good.Signs out before navigation, resets analytics, clears billing; matches prior recommendation.
🧹 Nitpick comments (5)
apps/web/pages/api/auth/callback.ts (3)
15-21: Avoid echoing raw auth errors to users.Keep detailed errors in logs; show a generic message to users.
- return res.redirect( - `/login?error=${encodeURIComponent(error.message)}` - ); + return res.redirect( + `/login?error=${encodeURIComponent("Login failed")}` + );
46-46: Return the redirect for clarity.Minor readability improvement; prevents accidental fall-through if code changes later.
- res.redirect(redirectTo); + return res.redirect(redirectTo);
3-3: Re-check SSR cookie security in createServerClientSSR.In apps/web/utils/supabase/server.ts, cookies are set with
httpOnly: false. That exposes auth cookies to JS (XSS risk). PreferhttpOnly: truefor server-managed auth cookies and use a browser client that relies on its own storage/token bridging.Would you like a follow-up patch to flip this and validate end-to-end login?
apps/web/utils/useUser.tsx (2)
100-112: Reset analytics and clear billing when user becomes null (cross-tab sign-out, expiry).Prevents stale identity/data if auth changes outside this tab.
if (user) { @@ posthog.identify(user.id, { email: user.email, name: user.user_metadata?.full_name, }); } else { + setBillingDetails(null); + posthog.reset(); setLoading(false); }
114-122: Optional: memoize context value to reduce downstream rerenders.Wrap
valueinuseMemoover its deps.-const value = { - loading, - session, - user, - billingDetails, - fetchBilling, - signOut, - supabase, -}; +const value = useMemo( + () => ({ + loading, + session, + user, + billingDetails, + fetchBilling, + signOut, + supabase, + }), + [loading, session, user, billingDetails, fetchBilling, signOut, supabase] +);Also applies to: 124-124
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/pages/api/auth/callback.ts(1 hunks)apps/web/utils/supabase/client.ts(1 hunks)apps/web/utils/useUser.tsx(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
apps/web/utils/supabase/client.ts (1)
packages/supabase/types/index.ts (1)
Database(9-836)
apps/web/utils/useUser.tsx (3)
apps/web/utils/supabase/client.ts (1)
createClient(4-15)apps/web/data/user.interface.ts (1)
IBillingInfo(9-31)apps/web/data/routes.data.ts (1)
ROUTES(1-26)
apps/web/pages/api/auth/callback.ts (2)
apps/web/utils/supabase/server.ts (1)
createServerClientSSR(4-49)apps/web/data/routes.data.ts (1)
ROUTES(1-26)
🪛 GitHub Check: CodeQL
apps/web/pages/api/auth/callback.ts
[warning] 46-46: Server-side URL redirect
Untrusted URL redirection depends on a user-provided value.
🔇 Additional comments (2)
apps/web/pages/api/auth/callback.ts (2)
22-27: Good guard when no session is created.
10-14: SSR client creation + code exchange flow looks correct.
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (11)
apps/web/pages/pages/[page_id]/edit.tsx (1)
19-27: Harden GSSP: normalize params to string and handle notFound/errors.getPage throws on error (throwOnError). Current code can 500 and also returns a possibly non-string page_id. Normalize and guard.
-export async function getServerSideProps(ctx: GetServerSidePropsContext) { - const { page_id } = ctx.params; - - const { supabase } = await getSupabaseServerClientForSSR(ctx); - const page = await getPage(supabase, page_id as string); +export async function getServerSideProps(ctx: GetServerSidePropsContext) { + const page_id = String(ctx.params?.page_id); + + const { supabase } = await getSupabaseServerClientForSSR(ctx); + const page = await getPage(supabase, page_id).catch((e) => { + console.error("Failed to load page:", e); + return null; + }); + + if (!page) { + return { notFound: true }; + } return { - props: { page_id, page }, + props: { page_id, page }, }; }apps/web/pages/pages/[page_id]/[post_id].tsx (1)
18-36: GSSP: normalize ids and return 404 when post isn’t found.Avoid returning undefined post to the component.
-export async function getServerSideProps(ctx: GetServerSidePropsContext) { - const { page_id, post_id } = ctx.params; - - const { supabase } = await getSupabaseServerClientForSSR(ctx); - const settings = await createOrRetrievePageSettings(String(page_id)); - const { data: post } = await supabase +export async function getServerSideProps(ctx: GetServerSidePropsContext) { + const page_id = String(ctx.params?.page_id); + const post_id = String(ctx.params?.post_id); + + const { supabase } = await getSupabaseServerClientForSSR(ctx); + const settings = await createOrRetrievePageSettings(page_id); + const { data: post } = await supabase .from("posts") .select("*") - .eq("id", post_id as string) - .single(); + .eq("id", post_id) + .maybeSingle(); + + if (!post) { + return { notFound: true }; + } return { props: { page_id, post_id, post, settings, }, }; }apps/web/pages/api/emails/subscribers/export-csv.ts (1)
18-24: Fix authorization check: page ownership result is ignored (potential data leak with admin client)You query page ownership with supabaseAdmin but don’t check the result. A user could export subscribers for a page they don’t own if they know the ID. Enforce ownership before exporting.
Apply:
- await supabaseAdmin - .from("pages") - .select("id") - .eq("id", page_id) - .eq("user_id", user.id) - .single(); + const { data: page, error: pageError } = await supabaseAdmin + .from("pages") + .select("id") + .eq("id", page_id) + .eq("user_id", user.id) + .single(); + if (pageError || !page) { + return res + .status(403) + .json({ error: { statusCode: 403, message: "Forbidden" } }); + }apps/web/pages/api/teams/invite/accept/index.ts (2)
18-26: Guard against accounts without an email before filtering invites by email.
Supabase users can exist without.ilike("email", user.email)withundefinedrisks runtime issues and unintended access behavior.Apply this diff to enforce an email presence check:
const { user, supabase } = await getSupabaseServerClientForAPI({ req, res, }); if (!user) { return res.status(401).json({ error: { statusCode: 401, message: "Unauthorized" }, }); } + if (!user.email) { + return res.status(400).json({ + error: { + statusCode: 400, + message: "Email is required to accept invitations for this account", + }, + }); + }
28-33: Validate invitation status before accepting.
Prevent accepting non-pending or already-consumed invites to avoid surprising states.Example tweak:
const { data: invite } = await supabase .from("team_invitations") .select("*") .eq("id", invite_id) .ilike("email", user.email) .single(); + if (!invite || invite.status !== "pending") { + return res.status(403).json({ + error: { statusCode: 403, message: "Invalid or already accepted invitation" }, + }); + }apps/web/pages/api/pages/settings/remove-domain.ts (2)
11-21: Propagate Vercel API errors and validate env.
Currently ignores non-2xx responses and missing envs.- const response = await fetch( + if (!process.env.VERCEL_PAGES_PROJECT_ID || !process.env.VERCEL_TEAM_ID || !process.env.VERCEL_AUTH_TOKEN) { + return res.status(500).json({ error: { statusCode: 500, message: "Vercel configuration missing" } }); + } + const response = await fetch( `https://api.vercel.com/v8/projects/${process.env.VERCEL_PAGES_PROJECT_ID}/domains/${domain}?teamId=${process.env.VERCEL_TEAM_ID}`, { headers: { Authorization: `Bearer ${process.env.VERCEL_AUTH_TOKEN}`, }, method: "DELETE", } ); - await response.json(); + if (!response.ok) { + const detail = await response.text(); + return res.status(response.status).json({ + error: { statusCode: response.status, message: `Vercel delete failed: ${detail}` }, + }); + }
7-10: Verify authorization to remove this domain.
Ensure the domain being removed belongs to a page/team the user owns before calling Vercel.If your schema stores custom domains (e.g.,
page_settings.custom_domain), add a check like:// Pseudocode (adapt to your schema) const { data: owned } = await supabase .from("page_settings") .select("id, page:page_id(owner_id)") .eq("custom_domain", domain) .single(); if (!owned || owned.page.owner_id !== user.id) { return res.status(403).json({ error: { statusCode: 403, message: "Forbidden" } }); }apps/web/pages/pages/[page_id]/analytics.tsx (1)
186-195: Block SSR access to non-owners + normalize range to a string (avoid array/NaN) + handle missing page.Without an SSR check, a non-owner can hit this route and receive analytics computed with the admin client. Also coerce
rangesafely and early-return on missing page.Apply:
-export async function getServerSideProps(ctx: GetServerSidePropsContext) { - const { range } = ctx.query; - const page_id = String(ctx.params?.page_id); - - const { supabase } = await getSupabaseServerClientForSSR(ctx); - const page = await getPage(supabase, page_id); +export async function getServerSideProps(ctx: GetServerSidePropsContext) { + const rangeParam = Array.isArray(ctx.query.range) ? ctx.query.range[0] : ctx.query.range; + const page_id = String(ctx.params?.page_id); + + const { supabase, user } = await getSupabaseServerClientForSSR(ctx); + const page = await getPage(supabase, page_id).catch(() => null); + if (!page) { + return { notFound: true }; + } + if (!user || page.user_id !== user.id) { + return { notFound: true }; + } @@ - const rangeNum = Number(range) || 7; + const rangeNum = Number(rangeParam) || 7;Also applies to: 190-195
apps/web/pages/pages/[page_id]/roadmap/[board_id]/settings.tsx (1)
341-352: Fix stage position when adding the first column.
Math.max(...[])yields-Infinity, causing a badposition. Initialize to 1 when empty and guard null positions.- const maxPosition = - Math.max(...boardColumns.map((col) => col.position)) + 1; + const maxPosition = + boardColumns.length > 0 + ? Math.max(...boardColumns.map((col) => col.position || 0)) + 1 + : 1;apps/web/pages/pages/[page_id]/roadmap/new.tsx (1)
129-136: Handle RPC errors and avoid orphaned boards (consider transaction).Both RPCs are fire-and-forget; failures leave a half-initialized board. Check errors and optionally clean up, or better: move board creation + initialization into one SQL RPC that runs in a transaction.
- // Initialize default stages for the board - await supabase.rpc("initialize_roadmap_columns", { board_id: board.id }); - - // Initialize default categories for the board - await supabase.rpc("initialize_roadmap_categories", { - board_id: board.id, - }); + // Initialize default stages for the board + const { error: colsError } = await supabase.rpc( + "initialize_roadmap_columns", + { board_id: board.id } + ); + if (colsError) { + console.error("Failed to initialize columns:", colsError); + setErrors({ general: "Failed to initialize default columns" }); + // Optional cleanup to avoid orphaned boards (ensure RLS allows it) + await supabase.from("roadmap_boards").delete().eq("id", board.id); + return; + } + + // Initialize default categories for the board + const { error: catsError } = await supabase.rpc( + "initialize_roadmap_categories", + { board_id: board.id } + ); + if (catsError) { + console.error("Failed to initialize categories:", catsError); + setErrors({ general: "Failed to initialize default categories" }); + await supabase.from("roadmap_boards").delete().eq("id", board.id); + return; + }If you prefer the transactional route, I can sketch a
create_roadmap_board_with_defaults(...)SQL function that inserts the board and seeds columns/categories atomically.apps/web/pages/api/teams/member/[id]/index.ts (1)
12-17: Normalize query param id (string | string[]) instead of String(id).Next.js query params can be arrays; String(id) on string[] produces "a,b". Normalize and pass the scalar.
Apply:
- const { id } = req.query; - if (!id) { + const { id } = req.query; + const memberId = Array.isArray(id) ? id[0] : id; + if (!memberId) { return res.status(400).json({ error: { statusCode: 400, message: "Invalid request" }, }); } @@ - .eq("id", String(id)) + .eq("id", memberId)Also applies to: 34-35
♻️ Duplicate comments (1)
apps/web/pages/api/auth/callback.ts (1)
39-46: Add CRLF guard to appease CodeQL and harden redirect further.You already block schemes and “//”. Also reject CR/LF to avoid header-splitting false positives.
if ( typeof redirectedFrom === "string" && redirectedFrom.startsWith("/") && !redirectedFrom.includes("://") && - !redirectedFrom.includes("\\") + !redirectedFrom.includes("\\") && + !redirectedFrom.includes("\r") && + !redirectedFrom.includes("\n") ) {
🧹 Nitpick comments (31)
apps/web/utils/supabase/static.ts (1)
1-10: Add strong typing for the client.Return a typed SupabaseClient for safer queries.
+import type { SupabaseClient } from "@supabase/supabase-js"; +import type { Database } from "@changes-page/supabase/types"; -import { createClient as createClientPrimitive } from "@supabase/supabase-js"; +import { createClient as createClientPrimitive } from "@supabase/supabase-js"; -export function createClient() { - const supabase = createClientPrimitive( +export function createClient(): SupabaseClient<Database> { + const supabase = createClientPrimitive<Database>( process.env.NEXT_PUBLIC_SUPABASE_URL!, - process.env.NEXT_PUBLIC_SUPABASE_PUBLISHABLE_KEY! + process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY! ); return supabase; }apps/web/pages/pages/[page_id]/edit.tsx (1)
62-73: Ensure saving state resets on all paths.setSaving(false) is only in the catch. Use finally so the spinner doesn’t stick if navigation fails mid-way.
- try { + try { await supabase .from("pages") .update(values as IPage) .match({ id: page_id }); return await router.push(ROUTES.PAGES); - } catch (e) { + } catch (e) { notifyError(); setSaving(false); - } + } finally { + setSaving(false); + }apps/web/pages/pages/[page_id]/[post_id].tsx (1)
57-59: Use strict equality.- if (newPost.status == PostStatus.published && !newPost.publication_date) { + if (newPost.status === PostStatus.published && !newPost.publication_date) {apps/web/utils/supabase/server.ts (1)
3-7: Import Next.js types as type-only from "next".Avoids accidental value import and matches Next’s guidance.
-import { - GetServerSidePropsContext, - NextApiRequest, - NextApiResponse, -} from "next/types"; +import type { + GetServerSidePropsContext, + NextApiRequest, + NextApiResponse, +} from "next";apps/web/pages/api/auth/callback.ts (1)
9-12: Nit: use ROUTES.LOGIN for consistency.- return res.redirect( - `/login?error=${encodeURIComponent("Missing or invalid code")}` - ); + return res.redirect( + `${ROUTES.LOGIN}?error=${encodeURIComponent("Missing or invalid code")}` + );apps/web/pages/api/ai/get-streaming-url.ts (2)
6-6: Rename handler to match file purposeFunction name doesn’t reflect the route. Rename for clarity.
Outside-diff suggestion:
// before const expandConcept = async (req, res) => { ... } export default expandConcept; // after const getStreamingUrl = async (req, res) => { ... } export default getStreamingUrl;Also applies to: 30-30
11-11: Validate input earlyEnsure workflowId exists and is a string to avoid downstream errors.
Outside-diff suggestion:
if (!workflowId || typeof workflowId !== "string") { return res.status(400).json({ error: { statusCode: 400, message: "Invalid workflowId" }, }); }apps/web/pages/api/billing/enable-email-notifications.ts (3)
49-52: Make Stripe item creation idempotent to avoid duplicates under racesTwo concurrent requests can add duplicate items. Use an idempotency key.
- await stripe.subscriptionItems.create({ - subscription: stripe_subscription_id, - price: process.env.EMAIL_NOTIFICATION_STRIPE_PRICE_ID, - }); + await stripe.subscriptionItems.create( + { + subscription: stripe_subscription_id, + price: process.env.EMAIL_NOTIFICATION_STRIPE_PRICE_ID, + }, + { idempotencyKey: `enable-email-${user.id}-${stripe_subscription_id}` } + );
56-56: Fix log tagMessage references a different route.
- console.log("createBillingSession", err); + console.log("enableEmailNotifications", err);
3-7: Unify Stripe import/instantiation styleMixed ESM/CJS reduces type safety. Prefer typed client with explicit apiVersion.
Outside-diff suggestion:
import Stripe from "stripe"; const stripe = new Stripe(process.env.STRIPE_SECRET_KEY!, { apiVersion: "2024-06-20" });apps/web/pages/api/ai/suggest-title.ts (1)
11-12: Validate input size/typeReject empty/non-string content to avoid wasted workflow runs.
Outside-diff suggestion:
if (typeof content !== "string" || !content.trim()) { return res.status(400).json({ error: { statusCode: 400, message: "Invalid content" } }); }apps/web/pages/api/emails/subscribers/export-csv.ts (2)
54-55: Correct Allow header for method mismatchCurrently advertises POST,PUT on a GET route.
- res.setHeader("Allow", "POST,PUT"); + res.setHeader("Allow", "GET");
16-16: Validate page_id formatDefensive check (e.g., UUID) prevents noisy admin queries.
Outside-diff suggestion:
const page_id = String(req.query.page_id); if (!/^[a-f0-9-]{36}$/i.test(page_id)) { return res.status(400).json({ error: { statusCode: 400, message: "Invalid page_id" } }); }apps/web/pages/integrations/zapier.tsx (1)
21-24: Load Zapier script via next/scriptImproves loading strategy and avoids blocking.
Outside-diff suggestion:
import Script from "next/script"; <Script src="https://cdn.zapier.com/packages/partner-sdk/v0/zapier-elements/zapier-elements.esm.js" type="module" strategy="afterInteractive" />apps/web/pages/api/billing/create-billing-portal.ts (2)
15-16: Validate and constrain return_url to first-party originAvoid open redirect via arbitrary return_url passed to Stripe.
Outside-diff suggestion:
const { return_url } = req.body; const appBase = new URL(getAppBaseURL()); let safeReturnUrl = `${appBase.origin}/account/billing`; try { if (return_url) { const u = new URL(return_url); if (u.origin === appBase.origin) safeReturnUrl = u.toString(); } } catch {} // use safeReturnUrl instead of return_url
2-8: Unify Stripe client instantiationPrefer typed ESM client with apiVersion.
Outside-diff suggestion:
import Stripe from "stripe"; const stripe = new Stripe(process.env.STRIPE_SECRET_KEY!, { apiVersion: "2024-06-20" });apps/web/pages/api/teams/invite/accept/index.ts (3)
41-49: Handle duplicate membership idempotently.
If a unique constraint exists on(team_id, user_id), repeated submissions will 500. Prefer 200/409 with a clean message.Example:
const { data: membership } = await supabaseAdmin .from("team_members") .insert({ team_id: invite.team_id, user_id: user.id, role: invite.role, }) .select() .single(); - if (!membership) { + if (!membership) { return res.status(500).json({ error: { statusCode: 500, message: "Failed to create invitation" }, }); }Or inspect
error.code === "23505"and treat as success.
60-64: Fix log context for easier debugging.
Message label doesn’t match the handler name.- console.log("inviteUser", err); + console.log("acceptInvite", err);
67-68: Correct the Allow header.
This handler only supports POST.- res.setHeader("Allow", "POST,PUT"); + res.setHeader("Allow", "POST");apps/web/pages/api/teams/invite/index.ts (2)
58-69: Prevent duplicate invitations.
Avoid spamming multiple pending invites for the same email/team pair.Pre-check before insert:
const { data: existing } = await supabaseAdmin .from("team_invitations") .select("id,status") .eq("team_id", team_id) .ilike("email", email) .in("status", ["pending"]) .maybeSingle(); if (existing) { return res.status(200).json({ ok: true }); // idempotent }
96-98: Correct the Allow header.
Only POST is supported in this handler.- res.setHeader("Allow", "POST,PUT"); + res.setHeader("Allow", "POST");apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx (1)
65-74: Prefer DB-side ordering for nested relations.
Sorting large nested arrays in memory can be heavier than letting PostgREST do it.Adjust the query with
order(..., { foreignTable }):const { data: board } = await supabase .from("roadmap_boards") .select(` *, roadmap_columns (*), roadmap_items (*, roadmap_categories (id, name, color), roadmap_votes (id)), roadmap_categories (*) `) .eq("id", board_id) .eq("page_id", page_id) .order("position", { ascending: true, foreignTable: "roadmap_columns" }) .order("position", { ascending: true, foreignTable: "roadmap_items" }) .order("created_at", { ascending: true, foreignTable: "roadmap_categories" }) .single();apps/web/pages/pages/[page_id]/roadmap/index.tsx (1)
149-156: Use shared date formatting util for consistency.
Other pages useDateTimehelpers; prefer that over rawtoLocaleDateStringto keep UX consistent.- {new Date(board.created_at).toLocaleDateString("en-US", { - year: "numeric", - month: "short", - day: "numeric", - })} + {/* e.g., if available */} + {DateTime.fromISO(board.created_at).toNiceFormat()}apps/web/pages/pages/[page_id]/analytics.tsx (1)
269-271: Pass a normalized range string to getPageAnalytics.Prevents accidental
string[]propagation and keeps the helper’s input predictable.- const metricsData = await Promise.all( - metrics.map((metric) => getPageAnalytics(page_id, metric, range)) - ); + const metricsData = await Promise.all( + metrics.map((metric) => getPageAnalytics(page_id, metric, String(rangeNum))) + );apps/web/utils/supabase/supabase-admin.ts (1)
1-68: Standardize public Supabase key env-var usage across factories
- In
apps/web/utils/supabase/server.ts(both SSR/API) andmiddleware.ts(and optionally instatic.ts), replace direct reads ofNEXT_PUBLIC_SUPABASE_ANON_KEYorNEXT_PUBLIC_SUPABASE_PUBLISHABLE_KEYwith a single fallback:const url = process.env.NEXT_PUBLIC_SUPABASE_URL!; const key = process.env.NEXT_PUBLIC_SUPABASE_PUBLISHABLE_KEY ?? process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY!; if (!key) throw new Error( 'Missing Supabase key: set NEXT_PUBLIC_SUPABASE_PUBLISHABLE_KEY or NEXT_PUBLIC_SUPABASE_ANON_KEY' ); return createServerClient<Database>(url, key, { … });- Update
apps/web/.env.example(and any other.env.example) to document bothNEXT_PUBLIC_SUPABASE_PUBLISHABLE_KEYandNEXT_PUBLIC_SUPABASE_ANON_KEY, preferring the former.- Fail fast when neither env-var is set.
apps/web/pages/pages/[page_id]/roadmap/new.tsx (5)
11-14: Guard against missing/invalid route param instead of String(undefined).Avoid coercing undefined to "undefined". Add a type check and early 404.
- const page_id = String(ctx.params?.page_id); + const raw = ctx.params?.page_id; + if (typeof raw !== "string") { + return { notFound: true } as const; + } + const page_id = raw;
25-30: Drop unused page from props to reduce payload.You fetch page only to gate 404; it isn’t consumed by the component. Don’t send it to the client.
return { props: { page_id, - page, }, };
70-77: Slugify: handle diacritics and produce cleaner slugs.Normalize and strip accents so titles like “Crème Brûlée” slug correctly.
- const generateSlugFromTitle = (title: string) => { - return title - .toLowerCase() - .replace(/[^a-z0-9\s-]/g, "") - .replace(/\s+/g, "-") - .replace(/-+/g, "-") - .replace(/^-|-$/g, ""); - }; + const generateSlugFromTitle = (title: string) => { + return title + .normalize("NFKD") + .replace(/[\u0300-\u036f]/g, "") // remove diacritics + .toLowerCase() + .replace(/[^a-z0-9\s-]/g, "") + .replace(/\s+/g, "-") + .replace(/-+/g, "-") + .replace(/^-|-$/g, ""); + };
86-91: Tighten slug validation (no leading/trailing or double hyphens).- } else if (!/^[a-z0-9-]+$/.test(formData.slug)) { - newErrors.slug = - "Slug can only contain lowercase letters, numbers, and hyphens"; + } else if ( + !/^(?!.*--)[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$/.test(formData.slug) + ) { + newErrors.slug = + "Slug must use lowercase letters/numbers with single hyphens (no leading/trailing hyphens).";
137-145: Navigation nit: don’t await push; prefer replace to avoid back to “New”.- // Redirect to the roadmap page - await router.push(`/pages/${page_id}/roadmap`); + // Redirect to the roadmap page + void router.replace(`/pages/${page_id}/roadmap`);apps/web/pages/api/teams/member/[id]/index.ts (1)
43-54: Rename memeberDetails → memberDetails (typo).Improves readability and prevents future copy-paste mistakes.
Apply:
- const { data: memeberDetails, error: memeberDetailsError } = + const { data: memberDetails, error: memberDetailsError } = await supabaseAdmin.auth.admin.getUserById(teamMember.user_id); - if (memeberDetailsError) { + if (memberDetailsError) { return res.status(500).json({ error: { statusCode: 500, message: "Internal server error" }, }); } @@ - email: memeberDetails.user.email, - name: memeberDetails.user.user_metadata.name, + email: memberDetails.user.email, + name: memberDetails.user.user_metadata.name,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
apps/web/pages/api/ai/get-streaming-url.ts(2 hunks)apps/web/pages/api/ai/suggest-title.ts(2 hunks)apps/web/pages/api/auth/callback.ts(1 hunks)apps/web/pages/api/billing/create-billing-portal.ts(2 hunks)apps/web/pages/api/billing/enable-email-notifications.ts(2 hunks)apps/web/pages/api/billing/index.ts(2 hunks)apps/web/pages/api/billing/redirect-to-checkout.ts(2 hunks)apps/web/pages/api/emails/subscribers/export-csv.ts(2 hunks)apps/web/pages/api/emails/subscribers/index.ts(1 hunks)apps/web/pages/api/pages/new.ts(2 hunks)apps/web/pages/api/pages/reactions.ts(2 hunks)apps/web/pages/api/pages/settings/add-domain.ts(1 hunks)apps/web/pages/api/pages/settings/check-domain.ts(1 hunks)apps/web/pages/api/pages/settings/index.ts(2 hunks)apps/web/pages/api/pages/settings/remove-domain.ts(1 hunks)apps/web/pages/api/posts/index.ts(2 hunks)apps/web/pages/api/teams/invite/accept/index.ts(2 hunks)apps/web/pages/api/teams/invite/index.ts(2 hunks)apps/web/pages/api/teams/member/[id]/index.ts(3 hunks)apps/web/pages/integrations/zapier.tsx(1 hunks)apps/web/pages/onboarding/open-page.tsx(1 hunks)apps/web/pages/pages/[page_id]/[post_id].tsx(2 hunks)apps/web/pages/pages/[page_id]/analytics.tsx(3 hunks)apps/web/pages/pages/[page_id]/audit-logs.tsx(1 hunks)apps/web/pages/pages/[page_id]/edit.tsx(2 hunks)apps/web/pages/pages/[page_id]/index.tsx(2 hunks)apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx(1 hunks)apps/web/pages/pages/[page_id]/roadmap/[board_id]/settings.tsx(1 hunks)apps/web/pages/pages/[page_id]/roadmap/index.tsx(3 hunks)apps/web/pages/pages/[page_id]/roadmap/new.tsx(2 hunks)apps/web/pages/pages/[page_id]/settings/[activeTab].tsx(4 hunks)apps/web/pages/pages/index.tsx(1 hunks)apps/web/utils/supabase/server.ts(1 hunks)apps/web/utils/supabase/static.ts(1 hunks)apps/web/utils/supabase/supabase-admin.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/pages/api/billing/index.ts
🧰 Additional context used
🧬 Code graph analysis (33)
apps/web/pages/api/ai/suggest-title.ts (1)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForAPI(39-67)
apps/web/pages/api/pages/settings/index.ts (1)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForAPI(39-67)
apps/web/pages/api/pages/new.ts (1)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForAPI(39-67)
apps/web/pages/integrations/zapier.tsx (1)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForSSR(10-37)
apps/web/pages/api/pages/settings/check-domain.ts (2)
apps/web/utils/rate-limit.ts (1)
apiRateLimiter(16-30)apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForAPI(39-67)
apps/web/pages/api/ai/get-streaming-url.ts (1)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForAPI(39-67)
apps/web/pages/api/billing/create-billing-portal.ts (1)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForAPI(39-67)
apps/web/pages/api/billing/redirect-to-checkout.ts (1)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForAPI(39-67)
apps/web/pages/pages/index.tsx (1)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForSSR(10-37)
apps/web/pages/onboarding/open-page.tsx (1)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForSSR(10-37)
apps/web/pages/api/teams/invite/index.ts (1)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForAPI(39-67)
apps/web/pages/api/pages/reactions.ts (1)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForAPI(39-67)
apps/web/pages/api/teams/member/[id]/index.ts (1)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForAPI(39-67)
apps/web/pages/api/billing/enable-email-notifications.ts (1)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForAPI(39-67)
apps/web/pages/pages/[page_id]/roadmap/index.tsx (2)
apps/web/pages/pages/[page_id]/index.tsx (1)
getServerSideProps(49-73)apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForSSR(10-37)
apps/web/pages/api/emails/subscribers/export-csv.ts (1)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForAPI(39-67)
apps/web/pages/api/pages/settings/add-domain.ts (1)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForAPI(39-67)
apps/web/pages/pages/[page_id]/settings/[activeTab].tsx (2)
apps/web/pages/pages/[page_id]/index.tsx (1)
getServerSideProps(49-73)apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForSSR(10-37)
apps/web/pages/pages/[page_id]/audit-logs.tsx (2)
apps/web/pages/pages/[page_id]/index.tsx (1)
getServerSideProps(49-73)apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForSSR(10-37)
apps/web/utils/supabase/server.ts (1)
packages/supabase/types/index.ts (1)
Database(9-836)
apps/web/pages/pages/[page_id]/[post_id].tsx (1)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForSSR(10-37)
apps/web/pages/pages/[page_id]/roadmap/[board_id]/settings.tsx (2)
apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx (1)
getServerSideProps(12-87)apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForSSR(10-37)
apps/web/pages/api/pages/settings/remove-domain.ts (1)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForAPI(39-67)
apps/web/pages/api/posts/index.ts (1)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForAPI(39-67)
apps/web/pages/pages/[page_id]/roadmap/new.tsx (4)
apps/web/pages/pages/[page_id]/[post_id].tsx (1)
getServerSideProps(18-37)apps/web/pages/pages/[page_id]/index.tsx (1)
getServerSideProps(49-73)apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForSSR(10-37)apps/web/utils/useUser.tsx (1)
useUserData(127-133)
apps/web/pages/pages/[page_id]/analytics.tsx (1)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForSSR(10-37)
apps/web/pages/api/teams/invite/accept/index.ts (1)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForAPI(39-67)
apps/web/pages/pages/[page_id]/edit.tsx (3)
apps/web/pages/pages/[page_id]/index.tsx (1)
getServerSideProps(49-73)apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForSSR(10-37)apps/web/utils/useSSR.ts (1)
getPage(4-13)
apps/web/pages/api/auth/callback.ts (2)
apps/web/utils/supabase/server.ts (1)
createServerClientForAPI(34-64)apps/web/data/routes.data.ts (1)
ROUTES(1-26)
apps/web/pages/api/emails/subscribers/index.ts (1)
apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForAPI(39-67)
apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx (2)
apps/web/pages/pages/[page_id]/index.tsx (1)
getServerSideProps(49-73)apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForSSR(10-37)
apps/web/utils/supabase/supabase-admin.ts (2)
packages/supabase/types/index.ts (1)
Database(9-836)apps/web/utils/supabase/server.ts (2)
createServerClientSSR(9-32)createServerClientForAPI(34-64)
apps/web/pages/pages/[page_id]/index.tsx (2)
apps/web/pages/pages/[page_id]/[post_id].tsx (1)
getServerSideProps(18-37)apps/web/utils/supabase/supabase-admin.ts (1)
getSupabaseServerClientForSSR(10-37)
🪛 GitHub Check: CodeQL
apps/web/pages/api/auth/callback.ts
[warning] 48-48: Server-side URL redirect
Untrusted URL redirection depends on a user-provided value.
🔇 Additional comments (29)
apps/web/pages/pages/[page_id]/[post_id].tsx (1)
63-69: Guard against nulluserbefore inserting audit logs
Wrap the Supabase insert in an explicitif (!user)check to avoid callinguser.idwhenusermay be null. For example:- await supabase.from("page_audit_logs").insert({ + if (!user) { + setSaving(false); + return notifyError("User not authenticated"); + } + await supabase.from("page_audit_logs").insert({ page_id: page_id, actor_id: user.id, action: `Updated Post: ${newPost.title}`, changes: newPost, });apps/web/pages/api/auth/callback.ts (1)
15-36: LGTM on error handling and safe redirects.Early return on missing/invalid code, robust try/catch around exchange, and sanitized redirect fix the earlier open-redirect issue. Nice.
apps/web/pages/api/ai/get-streaming-url.ts (1)
13-13: Require auth (401) instead of a no-op Supabase callYou’re instantiating the API client but not enforcing authentication. Gate the action on a real user to avoid unauthenticated access to signed streaming URLs.
[submit_essential_refactor]
Apply:- await getSupabaseServerClientForAPI({ req, res }); + const { user } = await getSupabaseServerClientForAPI({ req, res }); + if (!user) { + return res + .status(401) + .json({ error: { statusCode: 401, message: "Unauthorized" } }); + }apps/web/pages/integrations/zapier.tsx (1)
6-10: LGTM: SSR helper migration is correctSwapping to getSupabaseServerClientForSSR is consistent with the PR’s SSR split.
apps/web/pages/api/pages/reactions.ts (1)
3-3: LGTM! Consistent API client migration.The replacement of
getSupabaseServerClientwithgetSupabaseServerClientForAPImaintains consistency with the broader PR migration pattern for API routes. The function signature and logic remain unchanged, ensuring no breaking changes.Also applies to: 12-12
apps/web/pages/api/billing/redirect-to-checkout.ts (1)
4-4: LGTM! Proper API client helper usage.The migration to
getSupabaseServerClientForAPIis correct for this API route context. The authentication and billing flow logic remains intact while benefiting from the specialized API client implementation.Also applies to: 21-21
apps/web/pages/api/posts/index.ts (1)
5-5: LGTM! Clean migration to API-specific client.The migration from
getSupabaseServerClienttogetSupabaseServerClientForAPIfollows the established pattern across API routes. The destructuring of bothuserandsupabasefrom the client helper shows comprehensive usage of the returned object.Also applies to: 27-30
apps/web/pages/onboarding/open-page.tsx (1)
5-5: LGTM! Proper SSR context migration.The change from
getSupabaseServerClienttogetSupabaseServerClientForSSRis appropriate for this SSR context (getServerSideProps). This maintains the correct separation between SSR and API client usage patterns.Also applies to: 10-10
apps/web/pages/api/pages/settings/index.ts (1)
4-4: LGTM! Consistent API route migration.The migration to
getSupabaseServerClientForAPImaintains consistency with other API routes in this PR. The authentication check and page settings retrieval logic remain unchanged.Also applies to: 15-15
apps/web/pages/api/emails/subscribers/index.ts (2)
3-3: LGTM! Proper API client helper migration.The replacement with
getSupabaseServerClientForAPIfollows the established migration pattern for API routes.Also applies to: 11-11
13-13: Good defensive programming with explicit string conversion.The explicit conversion
String(req.query.page_id)improves type safety by ensuring the parameter is always a string when used in database queries, preventing potential type coercion issues.apps/web/pages/api/teams/invite/accept/index.ts (1)
4-4: Migration to API-specific Supabase helper looks good.
Import swap is correct and consistent with the new utilities.apps/web/pages/pages/[page_id]/audit-logs.tsx (1)
23-29: Confirm RLS/authorization for audit logs.
UsingsupabaseAdminbypasses RLS. EnsuregetPage’s access check is sufficient and logs aren’t exposed across tenants.Would you prefer switching to the user-scoped
supabasewith proper RLS policies forpage_audit_logs?apps/web/pages/api/teams/invite/index.ts (2)
7-7: Client migration is correct.
The import swap to the API helper matches the PR’s new utility surface.
22-25: LGTM on client creation and usage.
Rate limiting precedes auth; user guard follows immediately. Looks good.apps/web/pages/pages/[page_id]/analytics.tsx (3)
3-3: LGTM: correct Next.js types import.
8-8: LGTM: SSR helper import aligns with new SSR client pattern.
206-214: Date window filtering looks correct.Using in-memory Date comparisons for current/previous windows is fine after the single fetch constrained by
prevDate. No change needed.apps/web/pages/pages/[page_id]/index.tsx (2)
18-18: LGTM: Next.js types + SSR helper imports match the new SSR setup.Also applies to: 44-44
49-73: LGTM: ctx-based SSR migration is correct.Uses
getSupabaseServerClientForSSR(ctx), resolvespage_idfrom params, and returnsnotFoundwhen appropriate.apps/web/pages/pages/[page_id]/roadmap/[board_id]/settings.tsx (2)
8-8: LGTM: Next.js types + SSR helper imports align with migration.Also applies to: 15-15
108-116: Minor: activeTab state stays in sync once SSR value is sanitized.With the SSR fix above coercing
tabto a string, this effect remains correct. No change needed.apps/web/utils/supabase/supabase-admin.ts (2)
10-37: LGTM: SSR client helper returns typed supabase, session, and user.Error logging on
getUser()and nullable returns are appropriate.
39-67: LGTM: API client helper mirrors SSR pattern.Consistent return shape simplifies call sites.
apps/web/pages/pages/[page_id]/roadmap/new.tsx (2)
37-38: Confirmed browser Supabase client
useUserData().supabaseis instantiated viacreateClientinapps/web/utils/supabase/client.ts, which callscreateBrowserClient<Database>, ensuring it’s the client-side instance with proper cookie handling and RLS.
13-19: Enforce RLS‐bound client and null‐on‐deny in getPage
Confirm thatgetSupabaseServerClientForSSRis instantiated with the anon (RLS-enabled) key and thatgetPagereturnsnull(thus serving a 404) when a row is inaccessible. If not, add an explicit server-side permission check to avoid leaking page existence.apps/web/pages/api/teams/member/[id]/index.ts (1)
4-4: Good migration to API-scoped server client.Aligns with the new @supabase/ssr split and avoids leaking SSR cookie handling into API routes.
apps/web/pages/pages/index.tsx (2)
17-17: Correct helper for SSR.Using getSupabaseServerClientForSSR here is the right split from the API variant.
21-24: Confirm RLS/policies protect the pages query on SSR.This SSR query isn’t filtered by user; if RLS is off or the service client is used elsewhere by mistake, it could leak data. Ensure this path uses a non-admin client and RLS restricts rows to the viewer.
There was a problem hiding this comment.
Actionable comments posted: 28
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (36)
apps/web/pages/api/integrations/zapier/trigger-new-post.tsx (6)
23-27: Return after 400 to stop executionThe handler continues after sending the 400 response. Add an early return.
if (!page_secret_key) { - res - .status(400) - .json({ error: { statusCode: 400, message: "Invalid request" } }); + return res + .status(400) + .json({ error: { statusCode: 400, message: "Invalid request" } }); }
36-38: NaN pagination bug (Number(undefined) and ??)
Number(per_page) ?? 50yieldsNaN(not nullish) whenper_pageis absent;offsetcan also becomeNaN. Compute with defaults and clamp.- const offset = Number(page) * Number(per_page); - const limit = Number(per_page) ?? 50; + const pageParam = Array.isArray(page) ? page[0] : page; + const perPageParam = Array.isArray(per_page) ? per_page[0] : per_page; + const limit = Number.isFinite(Number(perPageParam)) && Number(perPageParam) > 0 ? Number(perPageParam) : 50; + const pageNum = Number.isFinite(Number(pageParam)) && Number(pageParam) > 0 ? Number(pageParam) : 0; + const offset = pageNum * limit;
39-45: Guard status filter and avoid applying .eq when status is missing/arrayCasting to
PostStatusis compile-time only; at runtimestatuscan beundefinedorstring[]. Conditionally apply the filter.- const { data: posts } = await supabaseAdmin - .from("posts") - .select("id,title,content,tags,created_at") - .eq("page_id", String(pageDetails.id)) - .eq("status", status as PostStatus) - .order("created_at", { ascending: false }) - .range(offset, limit - 1 + offset); + const statusParam = Array.isArray(status) ? status[0] : status; + let query = supabaseAdmin + .from("posts") + .select("id,title,content,tags,created_at") + .eq("page_id", String(pageDetails.id)); + if (typeof statusParam === "string" && statusParam.length) { + query = query.eq("status", statusParam as PostStatus); + } + const { data: posts } = await query + .order("created_at", { ascending: false }) + .range(offset, offset + limit - 1);
29-33: Handle not-found pageDetails explicitlyAvoid dereferencing
pageDetails.idwhen the secret is invalid.const pageDetails = await getPageByIntegrationSecret( String(page_secret_key) ); + if (!pageDetails?.id) { + return res + .status(404) + .json({ error: { statusCode: 404, message: "Page not found" } }); + }Also applies to: 47-47
34-34: Don’t log secrets
req.queryincludespage_secret_key. Redact or remove this log.- console.log("Zapier: get posts for", pageDetails?.id, "input", req.query); + console.log("Zapier: get posts for", pageDetails?.id, "input", { + page, + per_page, + status, + });
14-17: Response type doesn’t include url but you return itAlign the API response type with the payload.
- res: NextApiResponse< - | Pick<IPost, "id" | "title" | "content" | "tags" | "created_at">[] - | null - | IErrorResponse - > + res: NextApiResponse< + | (Pick<IPost, "id" | "title" | "content" | "tags" | "created_at"> & { url: string })[] + | null + | IErrorResponse + >Also applies to: 49-52
apps/web/utils/hooks/usePosts.ts (2)
9-13: Fix param type: defaulting a string to nullMake
pinnedPostIdnullable.-export default function usePagePosts( - pageId: string, - search = "", - pinnedPostId: string = null -) { +export default function usePagePosts( + pageId: string, + search = "", + pinnedPostId: string | null = null +) {
25-25: State type should allow null
useState<IPost>(null)is invalid under strict null checks.- const [pinnedPost, setPinnedPost] = useState<IPost>(null); + const [pinnedPost, setPinnedPost] = useState<IPost | null>(null);apps/web/components/layout/header.component.tsx (1)
64-82: Fix stale dependency: hasPendingInvites not in useMemo deps (pulse won’t update).navigation won’t recompute when invite state changes.
- }, [user, billingDetails]); + }, [user, billingDetails, hasPendingInvites]);apps/web/components/post/post.tsx (2)
312-321: ReactMarkdown: move remarkGfm to remarkPlugins and harden sanitize; drop ts-ignore.Current setup passes remarkGfm as a rehype plugin and relies on ts-ignore. Also, sanitize config should extend the default schema.
- <div className="prose dark:prose-invert text-gray-900 dark:text-gray-300 break-words"> - {/* @ts-ignore */} - <ReactMarkdown - rehypePlugins={[ - rehypeRaw, - // @ts-ignore - rehypeSanitize({ tagNames: ["div", "iframe"] }), - remarkGfm, - ]} - > + <div className="prose dark:prose-invert text-gray-900 dark:text-gray-300 break-words"> + <ReactMarkdown + remarkPlugins={[remarkGfm]} + rehypePlugins={[ + rehypeRaw, + [rehypeSanitize, markdownSchema], + ]} + >Add once near imports and top-level (outside this hunk):
// imports import rehypeSanitize from "rehype-sanitize"; import { defaultSchema } from "hast-util-sanitize"; // schema (top-level) const markdownSchema = { ...defaultSchema, tagNames: [...(defaultSchema.tagNames || []), "div", "iframe"], attributes: { ...(defaultSchema.attributes || {}), iframe: [ ...(defaultSchema.attributes?.iframe || []), "src", "width", "height", "allow", "allowfullscreen", "frameborder", "referrerpolicy", "loading", ], div: [...(defaultSchema.attributes?.div || []), "className"], }, };
192-201: Effect should depend on post props.Ensures reactions refresh when viewing a different post or when toggling allow_reactions.
- }, []); + }, [post.id, post.status, post.allow_reactions]);apps/web/components/layout/page.component.tsx (1)
171-185: Remove ts-ignore by using legacy anchor child or type the tab href.Safer than suppressing types.
- // @ts-ignore - <Link - key={tab.name} - href={tab.href} - className={classNames( - tab.current - ? "border-indigo-500 text-indigo-600 dark:text-indigo-400 dark:border-indigo-400" - : "border-transparent text-gray-500 hover:text-gray-700 hover:border-gray-300 dark:text-gray-400 dark:hover:text-gray-300", - "whitespace-nowrap pb-2 px-1 border-b-2 font-medium text-sm" - )} - aria-current={tab.current ? "page" : undefined} - > - {tab.name} - </Link> + <Link key={tab.name} href={tab.href} legacyBehavior passHref> + <a + className={classNames( + tab.current + ? "border-indigo-500 text-indigo-600 dark:text-indigo-400 dark:border-indigo-400" + : "border-transparent text-gray-500 hover:text-gray-700 hover:border-gray-300 dark:text-gray-400 dark:hover:text-gray-300", + "whitespace-nowrap pb-2 px-1 border-b-2 font-medium text-sm" + )} + aria-current={tab.current ? "page" : undefined} + > + {tab.name} + </a> + </Link>apps/web/components/dialogs/manage-team-dialog.component.tsx (1)
60-81: Handle Supabase errors on update/insert.Avoids silent failures and improves UX.
- if (team) { - await supabase - .from("teams") - .update({ - name: values.name, - image: values.image, - }) - .match({ id: team.id }); - onSuccess(); - } else { - await supabase - .from("teams") - .insert([ - { - name: values.name, - image: values.image, - owner_id: user.id, - }, - ]) - .select(); - onSuccess(); - } + if (team) { + const { error } = await supabase + .from("teams") + .update({ name: values.name, image: values.image }) + .eq("id", team.id); + if (error) return notifyError(error.message); + onSuccess(); + } else { + const { error } = await supabase + .from("teams") + .insert([{ name: values.name, image: values.image, owner_id: user.id }]); + if (error) return notifyError(error.message); + onSuccess(); + }apps/web/components/dialogs/date-time-prompt-dialog.component.tsx (1)
37-42: Preserve initialValue when opening.Current code clears value, making edits start empty.
- useEffect(() => { - if (open) { - setNaturalDateString(""); - setValue(null); - } - }, [open]); + useEffect(() => { + if (open) { + setNaturalDateString(""); + setValue(initialValue ?? null); + } + }, [open, initialValue]);apps/web/components/marketing/changelog.tsx (1)
11-11: Fix nullable state type for latestPostInitialize with null but typed as non-nullable. Use a union to avoid TS error and downstream non-null assertions.
- const [latestPost, setLatestPost] = useState<IPostWithUrl>(null); + const [latestPost, setLatestPost] = useState<IPostWithUrl | null>(null);apps/page/pages/_sites/[site]/roadmap/[roadmap_slug].tsx (1)
369-399: Enforce rel="noopener noreferrer" on external links rendered via MarkdownYou allow target/rel via sanitize, but nothing forces rel on target="_blank" links. Add a rehype plugin to auto-apply.
+import rehypeExternalLinks from "rehype-external-links"; ... <ReactMarkdown rehypePlugins={[ rehypeRaw, [ rehypeSanitize, { ...defaultSchema, attributes: { ...defaultSchema.attributes, a: [...(defaultSchema.attributes?.a||[]), ["target"], ["rel"]], code: [...(defaultSchema.attributes?.code||[]), ["className"]], span: [...(defaultSchema.attributes?.span||[]), ["className"]] } } ], + [rehypeExternalLinks, { target: "_blank", rel: ["noopener", "noreferrer"] }], ]} >apps/web/components/dialogs/ai-expand-concept-prompt-dialog.component.tsx (1)
37-39: Handle non-OK responses and stop streaming earlyCurrently continues to read the stream after an error toast.
- if (!response.ok) { - notifyError("Too many requests"); - } + if (!response.ok) { + notifyError("Too many requests"); + setLoading(false); + return; + }apps/web/pages/pages/[page_id]/edit.tsx (1)
61-71: Handle Supabase update errors instead of relying on try/catch.supabase-js returns
{ data, error }without throwing on PostgREST errors. Current code may redirect despite a failed update.- try { - await supabase - .from("pages") - .update(values as IPage) - .match({ id: page_id }); - - return await router.push(ROUTES.PAGES); - } catch (e) { - notifyError(); - setSaving(false); - } + const { error } = await supabase + .from("pages") + .update(values as IPage) + .eq("id", String(page_id)); + + if (error) { + notifyError(error.message); + setSaving(false); + return; + } + return await router.push(ROUTES.PAGES);apps/web/pages/api/billing/jobs/report-usage.ts (1)
41-86: Isolate per-user failures and stabilize idempotency.One Stripe error aborts the whole job; also use a deterministic idempotency key per user+timestamp to safely retry.
- for (const user of users ?? []) { - const subscription = user.stripe_subscription; + for (const user of users ?? []) { + try { + const subscription = user.stripe_subscription; console.log( `Job ${jobId} - Processing user ${user.id} with status ${subscription.status}` ); if ( subscription.status !== "active" && subscription.status !== "trialing" ) { continue; } console.log(`Reporting usage for user: ${user.id}`); - const timestamp = Math.floor(Date.now() / 1000); - const idempotencyKey = `${user.id}-report-job-${jobId}`; + const timestamp = Math.floor(Date.now() / 1000); + // Stable across retries for same (user,timestamp) + const idempotencyKey = `usage:${user.id}:${timestamp}`; const usageQuantity = await getPagesCount(user.id); console.log( `Update usage count for user: ${user.id} to ${usageQuantity}`, idempotencyKey ); const pagesSubscriptionItem = subscription.items?.data.find((item) => VALID_STRIPE_PRICE_IDS.includes(item.price.id) ); if (!pagesSubscriptionItem) { console.log(`No page subscription item found for user: ${user.id}`); continue; } await stripe.subscriptionItems.createUsageRecord( pagesSubscriptionItem.id, { quantity: usageQuantity, timestamp: timestamp, action: "set", }, { idempotencyKey, } ); + } catch (e) { + console.error(`Job ${jobId} - Failed for user ${user?.id}:`, e); + // continue with next user + } }apps/web/pages/onboarding/open-page.tsx (1)
21-22: Sanitize the user-controlled path before composing the redirect.path may be string[]/string and can include leading slashes or ../; normalize to a safe segment.
- const { path } = ctx.query; + const rawPath = ctx.query.path; + const safePath = Array.isArray(rawPath) ? rawPath.join("/") : String(rawPath ?? ""); + // strip leading slashes and any parent traversals + const normalizedPath = safePath.replace(/^\/+/, "").replace(/\.\.(\/|$)/g, ""); @@ return { redirect: { permanent: false, - destination: `${ROUTES.PAGES}/${pages[0].id}/${path ?? ""}`, + destination: `${ROUTES.PAGES}/${pages[0].id}/${normalizedPath}`, }, };Also applies to: 37-41
apps/web/pages/pages/[page_id]/roadmap/[board_id]/settings.tsx (1)
340-342: Fix position computation when adding first column.Math.max over an empty array yields -Infinity; inserted position becomes invalid.
- const maxPosition = - Math.max(...boardColumns.map((col) => col.position)) + 1; + const maxPosition = + (boardColumns.length + ? Math.max(...boardColumns.map((col) => col.position ?? 0)) + : 0) + 1;apps/web/pages/pages/[page_id]/roadmap/new.tsx (1)
117-135: RPC errors not handled; risk of half-initialized boards. Add checks/rollback or make it transactional.If either RPC fails, you redirect with a partially initialized board. Handle RPC errors and attempt rollback, or prefer a single SQL function that inserts + initializes atomically.
- // Initialize default stages for the board - await supabase.rpc("initialize_roadmap_columns", { board_id: board.id }); - - // Initialize default categories for the board - await supabase.rpc("initialize_roadmap_categories", { - board_id: board.id, - }); + // Initialize default stages for the board + const { error: colsErr } = await supabase.rpc("initialize_roadmap_columns", { board_id: board.id }); + if (colsErr) { + console.error("initialize_roadmap_columns failed:", colsErr); + // best-effort rollback to avoid half-initialized state + await supabase.from("roadmap_boards").delete().eq("id", board.id); + setErrors({ general: "Failed to initialize board columns" }); + return; + } + + // Initialize default categories for the board + const { error: catsErr } = await supabase.rpc("initialize_roadmap_categories", { board_id: board.id }); + if (catsErr) { + console.error("initialize_roadmap_categories failed:", catsErr); + await supabase.from("roadmap_boards").delete().eq("id", board.id); + setErrors({ general: "Failed to initialize board categories" }); + return; + }If you prefer, I can draft a
create_roadmap_board(board_input jsonb)SQL function that inserts and initializes within one transaction.apps/web/pages/api/pages/settings/add-domain.ts (1)
7-14: Validate and sanitize domain before use to avoid crashes and bad requests.
domaincan be undefined;endsWithwould throw. Also add basic format checks and forbidchanges.pagehere.- const { domain } = req.body; - - if (domain.endsWith("changes.page")) { - return res.status(400).json({ - success: false, - error: "forbidden", - }); - } + const rawDomain = req.body?.domain; + const domain = + typeof rawDomain === "string" ? rawDomain.trim().toLowerCase() : ""; + const isValid = + !!domain && + /^[a-z0-9.-]+$/i.test(domain) && + !domain.includes("..") && + !domain.endsWith("changes.page"); + if (!isValid) { + return res.status(400).json({ success: false, error: "invalid_domain" }); + }apps/web/pages/api/emails/subscribers/export-csv.ts (2)
11-19: Authorization check is not enforced; potential data leak.You ignore the result of the ownership check while using the admin client (bypasses RLS). Enforce ownership before querying subscribers.
- await supabaseAdmin - .from("pages") - .select("id") - .eq("id", page_id) - .eq("user_id", user.id) - .single(); + const { data: page, error: pageErr } = await supabaseAdmin + .from("pages") + .select("id") + .eq("id", page_id) + .eq("user_id", user.id) + .single(); + if (pageErr || !page) { + return res + .status(404) + .json({ error: { statusCode: 404, message: "Page not found" } }); + }
48-51: Fix Allow header for 405.Endpoint supports GET; header currently advertises POST,PUT.
- res.setHeader("Allow", "POST,PUT"); + res.setHeader("Allow", "GET"); res.status(405).end("Method Not Allowed");apps/web/pages/api/pages/settings/index.ts (1)
21-24: Fix Allow header for 405.This route handles GET.
- res.setHeader("Allow", "POST"); + res.setHeader("Allow", "GET"); res.status(405).end("Method Not Allowed");apps/web/pages/api/ai/get-streaming-url.ts (1)
5-12: Validate input and return 400 on missing workflowId.Avoid 500s for client errors.
if (req.method === "POST") { - const { workflowId } = req.body; + const { workflowId } = req.body ?? {}; + if (!workflowId || typeof workflowId !== "string") { + return res + .status(400) + .json({ error: { statusCode: 400, message: "workflowId required" } }); + }apps/web/pages/api/ai/suggest-title.ts (1)
5-12: Validate input and return 400 on missing content.Prevents unnecessary workflow calls and clearer client feedback.
if (req.method === "POST") { - const { content } = req.body; + const { content } = req.body ?? {}; + if (!content || typeof content !== "string") { + return res + .status(400) + .json({ error: { statusCode: 400, message: "content required" } }); + }apps/web/pages/api/teams/member/[id]/index.ts (2)
25-35: Block PII leakage: enforce team-based authorization before returning member details.You’re using the service-role client which bypasses RLS and can expose another user’s email/name to any authenticated subscriber who knows a team_members.id. Add an explicit membership/role check tying the requester to teamMember.team_id, return 403 when unauthorized, and 404 when the member doesn’t exist.
- const { data: teamMember, error: teamMemberError } = await supabaseAdmin - .from("team_members") - .select("*") - .eq("id", String(id)) - .single(); - - if (teamMemberError) { - return res.status(500).json({ - error: { statusCode: 500, message: "Internal server error" }, - }); - } + const { data: teamMember, error: teamMemberError } = await supabaseAdmin + .from("team_members") + .select("id,team_id,user_id") + .eq("id", id as string) + .single(); + if (teamMemberError || !teamMember) { + return res.status(404).json({ + error: { statusCode: 404, message: "Not found" }, + }); + } + // Ensure requester belongs to this team (or elevate with your role model) + const { data: requesterMembership } = await supabaseAdmin + .from("team_members") + .select("id") + .eq("team_id", teamMember.team_id) + .eq("user_id", user.id) + .maybeSingle(); + if (!requesterMembership) { + return res.status(403).json({ + error: { statusCode: 403, message: "Forbidden" }, + }); + } - const { data: memeberDetails, error: memeberDetailsError } = + const { data: memberDetails, error: memberDetailsError } = await supabaseAdmin.auth.admin.getUserById(teamMember.user_id); - if (memeberDetailsError) { + if (memberDetailsError) { return res.status(500).json({ error: { statusCode: 500, message: "Internal server error" }, }); } return res.status(200).json({ - email: memeberDetails.user.email, - name: memeberDetails.user.user_metadata.name, + email: memberDetails.user.email, + name: memberDetails.user.user_metadata?.name ?? null, });Also applies to: 37-48
9-13: Validate id type (string vs. string[]), and avoid String() coercion.Next.js query params can be arrays. Reject non-string ids early and use the raw string in the filter.
- const { id } = req.query; - if (!id) { + const { id } = req.query; + if (typeof id !== "string" || id.trim().length === 0) { return res.status(400).json({ error: { statusCode: 400, message: "Invalid request" }, }); } @@ - .eq("id", String(id)) + .eq("id", id)Also applies to: 28-28
apps/web/pages/api/posts/index.ts (2)
26-38: Prevent runtime crash: .trim() on possibly undefined. Sanitize inputs once and validate sanitized values.title/content may be missing; calling .trim() will throw. Normalize first and use the normalized values for both validation and insert.
- const isValid = await NewPostSchema.isValid({ - page_id, - title: title.trim(), - content: content.trim(), + const normalizedTitle = typeof title === "string" ? title.trim() : ""; + const normalizedContent = typeof content === "string" ? content.trim() : ""; + const isValid = await NewPostSchema.isValid({ + page_id, + title: normalizedTitle, + content: normalizedContent, tags, status, images_folder, publish_at, notes, allow_reactions, email_notified, publication_date, });
57-72: Use sanitized values in payload (avoid reintroducing unsanitized inputs).You validate trimmed values but insert the originals.
- const postPayload = { + const postPayload = { user_id: user.id, page_id, - title, - content, + title: normalizedTitle, + content: normalizedContent, tags, status, images_folder, publish_at, publication_date: publication_date ?? (status === PostStatus.published ? new Date().toISOString() : null), notes: notes ?? "", allow_reactions: allow_reactions ?? false, email_notified: email_notified ?? false, };apps/web/pages/api/pages/new.ts (1)
25-36: Avoid .trim() on possibly undefined; validate first, then trim for storage.Current code can throw when title/description are missing.
- const isValid = await NewPageSchema.isValid({ - url_slug, - title: title.trim(), - description: description.trim(), - type, - }); + const isValid = await NewPageSchema.isValid({ + url_slug, + title, + description, + type, + }); @@ - const data = await createPage({ + const data = await createPage({ user_id: user.id, url_slug, - title, - description, + title: title.trim(), + description: description.trim(), type, });Also applies to: 40-46
apps/web/pages/api/billing/index.ts (1)
18-20: Guard missing STRIPE_PRICE_ID.If undefined, stripe.prices.retrieve will throw. Validate and respond 500 with a concise message.
- const { unit_amount } = await stripe.prices.retrieve( - process.env.STRIPE_PRICE_ID - ); + const priceId = process.env.STRIPE_PRICE_ID; + if (!priceId) { + console.error("getBillingStatus: STRIPE_PRICE_ID missing"); + return res.status(500).json({ error: { statusCode: 500 } }); + } + const { unit_amount } = await stripe.prices.retrieve(priceId);apps/web/pages/api/teams/invite/accept/index.ts (2)
17-23: Guard against missing user.email and handle Supabase errors explicitly.user.email can be null; also check error from .single().
- const { data: invite } = await supabase + if (!user.email) { + return res.status(400).json({ + error: { statusCode: 400, message: "User email not available" }, + }); + } + const { data: invite, error: inviteErr } = await supabase .from("team_invitations") .select("*") .eq("id", invite_id) .ilike("email", user.email) .single(); + if (inviteErr) { + return res.status(500).json({ + error: { statusCode: 500, message: "Failed to fetch invitation" }, + }); + }
30-38: Handle duplicate membership idempotently and surface insert errors.Avoid failing on unique_violation (23505) and report real failures.
- const { data: membership } = await supabaseAdmin + const { data: membership, error: memErr } = await supabaseAdmin .from("team_members") .insert({ team_id: invite.team_id, user_id: user.id, role: invite.role, }) .select() .single(); - - if (!membership) { - return res.status(500).json({ - error: { statusCode: 500, message: "Failed to create invitation" }, - }); - } + if (memErr?.code === "23505") { + // membership already exists + } else if (memErr || !membership) { + return res.status(500).json({ + error: { statusCode: 500, message: "Failed to create membership" }, + }); + }
♻️ Duplicate comments (9)
apps/web/pages/pages/[page_id]/audit-logs.tsx (1)
18-36: Add notFound guard and param validation before querying audit logs.Align with sibling pages: treat missing/unauthorized page as 404 and avoid extra queries. Also guard against an accidental "undefined" page_id string.
-export const getServerSideProps = withSupabase(async (ctx, { supabase }) => { - const page_id = String(ctx.params?.page_id); +export const getServerSideProps = withSupabase(async (ctx, { supabase }) => { + const pid = ctx.params?.page_id; + const page_id = typeof pid === "string" ? pid : Array.isArray(pid) ? pid[0] : ""; + if (!page_id) return { notFound: true as const }; - const page = await getPage(supabase, page_id); + const page = await getPage(supabase, page_id).catch(() => null); + if (!page) return { notFound: true as const }; const { data: audit_logs } = await supabaseAdminapps/web/pages/pages/[page_id]/settings/[activeTab].tsx (2)
27-29: Add notFound handling if the page cannot be loaded.Bring this in line with other routes that guard on missing/forbidden pages.
- const page = await getPage(supabase, page_id); + const page = await getPage(supabase, page_id).catch((e) => { + console.error("Failed to get page", e); + return null; + }); + if (!page) { + return { notFound: true }; + }
35-36: Normalize activeTab to a string and default to "general".ctx.params?.activeTab can be string | string[] | undefined; array/undefined breaks equality checks.
- activeTab: ctx.params?.activeTab, + activeTab: (() => { + const raw = ctx.params?.activeTab; + const tab = Array.isArray(raw) ? raw[0] : raw; + return (tab ?? "general") as string; + })(),apps/web/pages/pages/[page_id]/roadmap/[board_id]/settings.tsx (1)
20-24: Enforce SSR ownership (avoid leaking board data via HTML).Ownership is only checked client-side; SSR still fetches and renders board/columns/categories for non-owners. Add a server-side ownership gate and harden tab parsing.
-export const getServerSideProps = withSupabase(async (ctx, { supabase }) => { - const page_id = String(ctx.params?.page_id); - const board_id = String(ctx.params?.board_id); - const { tab = "board" } = ctx.query; +export const getServerSideProps = withSupabase(async (ctx, { supabase, user }) => { + const page_id = String(ctx.params?.page_id); + const board_id = String(ctx.params?.board_id); + const tabParam = Array.isArray(ctx.query.tab) ? ctx.query.tab[0] : ctx.query.tab; + const tab = typeof tabParam === "string" ? tabParam : "board"; @@ if (!page) { return { notFound: true, }; } + if (!user || page.user_id !== user.id) { + return { notFound: true }; + }Also applies to: 34-36
apps/web/pages/pages/[page_id]/analytics.tsx (1)
215-219: Exclude null/empty visitor_ids from unique counts.Prevents inflated unique metrics.
- const visitors = new Set(currentPeriodViews.map((v) => v.visitor_id)).size; + const visitors = new Set( + currentPeriodViews.map((v) => v.visitor_id).filter(Boolean) + ).size; @@ - const prev_visitors = new Set(previousPeriodViews.map((v) => v.visitor_id)) - .size; + const prev_visitors = new Set( + previousPeriodViews.map((v) => v.visitor_id).filter(Boolean) + ).size;apps/web/pages/api/pages/settings/add-domain.ts (2)
18-31: Handle non-2xx Vercel responses and send JSON body properly.Check
response.okand surface Vercel error text/status; useJSON.stringifyfor the body.- const response = await fetch( + const response = await fetch( `https://api.vercel.com/v8/projects/${process.env.VERCEL_PAGES_PROJECT_ID}/domains?teamId=${process.env.VERCEL_TEAM_ID}`, { - body: `{\n "name": "${domain}"\n}`, + body: JSON.stringify({ name: domain }), headers: { Authorization: `Bearer ${process.env.VERCEL_AUTH_TOKEN}`, "Content-Type": "application/json", }, method: "POST", } ); - - const data = await response.json(); + if (!response.ok) { + const text = await response.text(); + return res + .status(response.status) + .json({ success: false, error: text || "vercel_error" }); + } + const data = await response.json();
4-6: Enforce HTTP method = POST.Reject non-POST to prevent misuse and align with semantics.
-const addDomain = withAuth(async (req, res, { user }) => { +const addDomain = withAuth(async (req, res, { user }) => { + if (req.method !== "POST") { + res.setHeader("Allow", "POST"); + return res.status(405).end("Method Not Allowed"); + }apps/web/pages/api/teams/member/[id]/index.ts (1)
37-48: Fix variable typo and optional chaining.Rename memeberDetails → memberDetails and guard missing name.
apps/web/pages/api/pages/settings/remove-domain.ts (1)
5-6: Add an explicit HTTP method guard (DELETE only).withAuth likely handles auth, but the route still accepts any method; enforce DELETE (or POST, if that’s the contract) and return 405 for others.
const removeDomain = withAuth<{ success: boolean }>( async (req, res, { user }) => { + if (req.method !== "DELETE") { + res.setHeader("Allow", "DELETE"); + return res.status(405).end("Method Not Allowed"); + } const { domain } = req.body;
apps/web/components/dialogs/ai-expand-concept-prompt-dialog.component.tsx
Show resolved
Hide resolved
| export const getServerSideProps = withSupabase(async (ctx, { supabase }) => { | ||
| const page_id = String(ctx.params?.page_id); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden param parsing and avoid shipping unused prop.
Guard against missing/array params and stop returning the unused page to the client.
-export const getServerSideProps = withSupabase(async (ctx, { supabase }) => {
- const page_id = String(ctx.params?.page_id);
+export const getServerSideProps = withSupabase(async (ctx, { supabase }) => {
+ const pageParam = ctx.params?.page_id;
+ if (!pageParam || Array.isArray(pageParam)) {
+ return { notFound: true };
+ }
+ const page_id = pageParam;
@@
return {
props: {
page_id,
- page,
},
};
});Also applies to: 24-30
🤖 Prompt for AI Agents
In apps/web/pages/pages/[page_id]/roadmap/new.tsx around lines 10-12 (and
similarly 24-30), the param parsing is not hardened and an unused `page` prop is
returned to the client; change the logic to explicitly validate
ctx.params?.page_id: if it's missing or an array, handle it (return notFound:
true or redirect) or normalize by picking the first element only after explicit
check, then assign page_id as a string; remove `page` from the props returned to
the client so only necessary values are sent; ensure any early-error paths
exit/return before using invalid params.
| export const getServerSideProps = withSupabase(async (ctx, { supabase }) => { | ||
| const page_id = String(ctx.params?.page_id); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Normalize page_id and return 404 when missing.
Avoid String(undefined) and early-return if the param is absent.
-export const getServerSideProps = withSupabase(async (ctx, { supabase }) => {
- const page_id = String(ctx.params?.page_id);
+export const getServerSideProps = withSupabase(async (ctx, { supabase }) => {
+ const pageParam = ctx.params?.page_id;
+ const page_id = Array.isArray(pageParam) ? pageParam[0] : pageParam;
+ if (!page_id) return { notFound: true };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const getServerSideProps = withSupabase(async (ctx, { supabase }) => { | |
| const page_id = String(ctx.params?.page_id); | |
| export const getServerSideProps = withSupabase(async (ctx, { supabase }) => { | |
| const pageParam = ctx.params?.page_id; | |
| const page_id = Array.isArray(pageParam) ? pageParam[0] : pageParam; | |
| if (!page_id) return { notFound: true }; |
🤖 Prompt for AI Agents
In apps/web/pages/pages/[page_id]/settings/[activeTab].tsx around lines 24-26,
the code uses String(ctx.params?.page_id) which turns undefined into "undefined"
and doesn't handle array params; change this to first validate that
ctx.params?.page_id exists, normalize it by handling string|string[] (if it's an
array take the first element), and if no page_id is present return a server-side
404 via { notFound: true } immediately before continuing.
| if (status) { | ||
| query = query.eq("status", String(status)); | ||
| query = query.eq("status", status as PostStatus); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard router.query.status (can be string[]/undefined)
Only apply the filter for a single string value.
- if (status) {
- query = query.eq("status", status as PostStatus);
- }
+ const statusParam = Array.isArray(status) ? status[0] : status;
+ if (typeof statusParam === "string" && statusParam.length) {
+ query = query.eq("status", statusParam as PostStatus);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (status) { | |
| query = query.eq("status", String(status)); | |
| query = query.eq("status", status as PostStatus); | |
| } | |
| // Only apply a status filter when `router.query.status` is a single non-empty string | |
| const statusParam = Array.isArray(status) ? status[0] : status; | |
| if (typeof statusParam === "string" && statusParam.length) { | |
| query = query.eq("status", statusParam as PostStatus); | |
| } |
🤖 Prompt for AI Agents
In apps/web/utils/hooks/usePosts.ts around lines 43 to 45, the code blindly
applies query.eq("status", status) but router.query.status can be undefined or
string[], so only apply the filter when status is a single string: check that
typeof status === "string" (or Array.isArray(status) === false and status !==
undefined) before calling query.eq and cast to PostStatus; otherwise skip adding
the status filter. Ensure the guard prevents passing an array/undefined into the
query.
| return handler(req, res, { supabase, user }); | ||
| } catch (error) { | ||
| console.error("Auth wrapper error:", error); | ||
| return res.status(500).json({ | ||
| error: { statusCode: 500, message: "Internal server error" }, | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Await the handler to catch async rejections in this try/catch.
Without await, thrown async errors may bypass the catch.
- return handler(req, res, { supabase, user });
+ await handler(req, res, { supabase, user });
+ return;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return handler(req, res, { supabase, user }); | |
| } catch (error) { | |
| console.error("Auth wrapper error:", error); | |
| return res.status(500).json({ | |
| error: { statusCode: 500, message: "Internal server error" }, | |
| }); | |
| } | |
| }; | |
| await handler(req, res, { supabase, user }); | |
| return; | |
| } catch (error) { | |
| console.error("Auth wrapper error:", error); | |
| return res.status(500).json({ | |
| error: { statusCode: 500, message: "Internal server error" }, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In apps/web/utils/withAuth.ts around lines 32 to 39, the call to handler(req,
res, { supabase, user }) is not awaited so async rejections can escape the
try/catch; update the code to await the handler invocation (e.g., use await
before calling the handler and return its result) so thrown async errors are
caught by the surrounding catch block.
arjunkomath
force-pushed
the
supabase-ssr-migration
branch
from
e47ee0e to
6378408
Compare
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/components/dialogs/ai-prood-read-dialog.component.tsx (2)
31-52: Fix error handling and streaming decode correctness.Currently proceeds after non-OK responses and may break multibyte chars across chunks.
@@ - if (!response.ok) { - notifyError("Too many requests"); - } + if (!response.ok) { + setLoading(false); + notifyError(`Request failed: ${response.status} ${response.statusText}`); + return; + } @@ - const decoder = new TextDecoder(); - let done = false; - - setLoading(false); + const decoder = new TextDecoder(); + let done = false; + let firstChunk = true; @@ - const { value, done: doneReading } = await reader.read(); + const { value, done: doneReading } = await reader.read(); done = doneReading; - const chunkValue = decoder.decode(value); - setResult((prev) => (prev ?? "") + chunkValue); + if (value?.length) { + const chunkValue = decoder.decode(value, { stream: true }); + if (firstChunk) { + setLoading(false); + firstChunk = false; + } + setResult((prev) => (prev ?? "") + chunkValue); + } } + // flush any remaining decoder state + const rest = decoder.decode(); + if (rest) setResult((prev) => (prev ?? "") + rest);
14-20: Abort in-flight stream on close/unmount to prevent leaks and setState-after-unmount.Reading continues after the dialog closes; hook up an AbortController and cleanup.
Add once after the button ref:
const abortRef = useRef<AbortController | null>(null);Wire it in proofRead:
const proofRead = useCallback(async (text) => { setLoading(true); + // cancel any in-flight request + abortRef.current?.abort(); + const controller = new AbortController(); + abortRef.current = controller; @@ - const response = await fetch(url, { + const response = await fetch(url, { method: "POST", + signal: controller.signal, headers: { "Content-Type": "application/json", },Clean up on dependency change/unmount:
useEffect(() => { if (open && content) { @@ } -}, [open, content]); + return () => abortRef.current?.abort(); +}, [open, content]);Abort immediately when the user clicks Close:
- onClick={() => setOpen(false)} + onClick={() => { abortRef.current?.abort(); setOpen(false); }}Also applies to: 21-29, 54-66, 141-149
apps/web/utils/useUser.tsx (1)
51-61: Type fetchBilling as a Promise and return null on failure.Currently it returns a Promise but the type implies void; also normalize the error path.
- const fetchBilling = useCallback(async () => { - return httpGet({ url: `/api/billing` }) - .then((billingDetails) => { - setBillingDetails(billingDetails); - return billingDetails; - }) - .catch((error) => { - console.error("Failed to get billing data:", error); - notifyError("Failed to fetch billing information"); - }); - }, []); + const fetchBilling = useCallback(async (): Promise<IBillingInfo | null> => { + try { + const details = await httpGet({ url: `/api/billing` }); + setBillingDetails(details); + return details; + } catch (error) { + console.error("Failed to get billing data:", error); + notifyError("Failed to fetch billing information"); + return null; + } + }, []);
♻️ Duplicate comments (3)
apps/web/pages/api/pages/settings/remove-domain.ts (2)
5-7: Add HTTP method guard (POST) and short-circuit before work.This route performs a destructive action but doesn’t restrict the method. Guard early.
Apply:
const removeDomain = withAuth<{ success: boolean }>( async (req, res, { user }) => { - const { domain } = req.body; + if (req.method !== "POST") { + res.setHeader("Allow", "POST"); + return res.status(405).end("Method Not Allowed"); + } + const { domain } = req.body;
28-36: Harden the Vercel call: timeout, encode domain, check response, handle 404 idempotently.Address CodeQL SSRF warning and upstream failure handling; current code always returns 200 and may throw on empty body.
Apply:
- const response = await fetch( - `https://api.vercel.com/v8/projects/${process.env.VERCEL_PAGES_PROJECT_ID}/domains/${domain}?teamId=${process.env.VERCEL_TEAM_ID}`, - { - headers: { - Authorization: `Bearer ${process.env.VERCEL_AUTH_TOKEN}`, - }, - method: "DELETE", - } - ); - - await response.json(); - - res.status(200).json({ - success: true, - }); + const { VERCEL_PAGES_PROJECT_ID, VERCEL_TEAM_ID, VERCEL_AUTH_TOKEN } = process.env; + if (!VERCEL_PAGES_PROJECT_ID || !VERCEL_TEAM_ID || !VERCEL_AUTH_TOKEN) { + console.error("removeDomain missing env"); + return res.status(500).json({ success: false }); + } + const controller = new AbortController(); + const t = setTimeout(() => controller.abort(), 10_000); + const url = new URL( + `/v8/projects/${encodeURIComponent(VERCEL_PAGES_PROJECT_ID)}/domains/${encodeURIComponent(domain)}`, + "https://api.vercel.com" + ); + url.searchParams.set("teamId", VERCEL_TEAM_ID); + const response = await fetch(url, { + headers: { Authorization: `Bearer ${VERCEL_AUTH_TOKEN}` }, + method: "DELETE", + signal: controller.signal, + }).catch((e) => { + console.error("removeDomain fetch failed", e); + return null; + }); + clearTimeout(t); + + if (!response) return res.status(502).json({ success: false }); + let body: any = null; + const ct = response.headers.get("content-type") || ""; + if (ct.includes("application/json")) { + try { body = await response.json(); } catch {} + } + if (response.ok || response.status === 404) { + return res.status(200).json({ success: true }); + } + console.error("removeDomain Vercel error", response.status, body); + return res.status(502).json({ success: false });Also applies to: 38-38, 40-42
apps/web/utils/useUser.tsx (1)
73-73: Add posthog to deps (minor).Module imports are stable, but adding posthog to the dependency array is harmless and satisfies previous review guidance.
- }, [supabase, router]); + }, [supabase, router, posthog]);
🧹 Nitpick comments (21)
apps/web/pages/api/billing/jobs/report-usage.ts (2)
8-11: Type for DB-stored Stripe subscription may be too strong; consider narrowing.If the JSON stored in users.stripe_subscription is a subset of Stripe.Subscription, typing it as the full Stripe object can mask shape drift. Consider narrowing to the fields you use.
-type UserWithSubscription = { - id: string; - stripe_subscription: Stripe.Subscription; -}; +type UserWithSubscription = { + id: string; + stripe_subscription: Pick<Stripe.Subscription, "status" | "items">; +};
57-58: Use integer seconds without string parsing.- const timestamp = parseInt(`${Date.now() / 1000}`); + const timestamp = Math.floor(Date.now() / 1000);apps/web/pages/api/pages/settings/remove-domain.ts (2)
9-9: Avoid logging PII; use structured logs.Don’t log raw user IDs/domains in plaintext.
Apply:
- console.log("removeDomain", user?.id, `domain: ${domain}`); + console.info("removeDomain requested", { userId: user?.id ? "present" : "absent" });
40-42: Ensure DB state is updated after successful deletion.If you store custom_domain in page_settings, clear it post-success to prevent drift.
I can wire the update (set custom_domain = null where page_id = data.page_id) and add a small transactional guard. Want a patch?
apps/web/components/dialogs/confirm-delete-dialog.component.tsx (1)
28-30: Replace TS ignores around Headless UIas={Fragment}with a typed alias.
Avoid blanket@ts-ignore. Use a typed ElementType to satisfyasgenerics.Add once (outside the ranges, near imports):
import type { ElementType } from "react"; const AsFragment = Fragment as unknown as ElementType;Apply within these ranges:
- {/* @ts-ignore */} - <Transition.Root show={open} as={Fragment}> + <Transition.Root show={open} as={AsFragment}>- // @ts-ignore - as={Fragment} + as={AsFragment}- // @ts-ignore - as={Fragment} + as={AsFragment}Also applies to: 37-41, 57-61
apps/web/components/dialogs/ai-suggest-title-prompt-dialog.component.tsx (4)
19-37: Harden effect: guard against stale updates and avoid auto-closing on error.
Prevents setting state after unmount/race and keeps dialog open so users can retry.- useEffect(() => { - if (open && content) { - setLoading(true); - setSuggestions([]); - - const text = convertMarkdownToPlainText(content); - - promptSuggestTitle(text) - .then((suggestions) => { - setSuggestions(suggestions); - setLoading(false); - }) - .catch(() => { - setLoading(false); - setOpen(false); - notifyError("Failed to suggest title, please contact support."); - }); - } - }, [open, content]); + useEffect(() => { + if (!open || !content) return; + let active = true; + setLoading(true); + setSuggestions([]); + + const text = convertMarkdownToPlainText(content); + promptSuggestTitle(text) + .then((titles) => { + if (!active) return; + setSuggestions(titles); + setLoading(false); + }) + .catch(() => { + if (!active) return; + setLoading(false); + notifyError("Failed to suggest title. Please try again."); + }); + + return () => { + active = false; + }; + }, [open, content]);
40-42: Remove TS ignores by typingFragmentforasprop.
Same pattern as other dialogs; avoids suppressing type-checks.Add once (outside the ranges, near imports):
import type { ElementType } from "react"; const AsFragment = Fragment as unknown as ElementType;Apply within these ranges:
- // @ts-ignore - <Transition.Root show={open} as={Fragment}> + <Transition.Root show={open} as={AsFragment}>- // @ts-ignore - as={Fragment} + as={AsFragment}- // @ts-ignore - as={Fragment} + as={AsFragment}Also applies to: 50-52, 70-72
92-96: Avoid multiline template literal that introduces unintended whitespace.
Keeps copy clean in UI.- {loading - ? "Loading..." - : `We've found ${suggestions.length} title suggestion(s) for - you`} + {loading + ? "Loading..." + : `We've found ${suggestions.length} title suggestion(s) for you`}
106-109: Prefer a stable key to index for the suggestions list.
Prevents unnecessary re-renders on list updates.- {suggestions.map((title, idx) => ( + {suggestions.map((title, idx) => ( <li - key={idx} + key={`${idx}-${title}`}apps/web/components/dialogs/ai-prood-read-dialog.component.tsx (5)
68-70: Remove // @ts-ignore and use a typed Fragment alias (or, at minimum, ts-expect-error with reason).Suppressing here hides real errors. Prefer a one-time typed alias to avoid sprinkling ignores.
Apply this diff:
- // @ts-ignore - <Transition.Root show={open} as={Fragment}> + <Transition.Root show={open} as={AsFragment}>Add once near imports (outside this hunk):
import type { ElementType } from "react"; const AsFragment = Fragment as unknown as ElementType;If you must keep a suppression, switch to:
// @ts-expect-error headlessui type hole for `as={Fragment}`
78-80: Same: replace // @ts-ignore with typed alias.Keep consistency and avoid broad suppressions.
- // @ts-ignore - as={Fragment} + as={AsFragment}
98-100: Same: replace // @ts-ignore with typed alias.- // @ts-ignore - as={Fragment} + as={AsFragment}
9-13: Add prop/ref types (tighten TS, improves DX).-export default function AiProofReadDialogComponent({ open, setOpen, content }) { +type Props = { + open: boolean; + setOpen: (open: boolean) => void; + content: string; +}; + +export default function AiProofReadDialogComponent({ open, setOpen, content }: Props) { @@ - const cancelButtonRef = useRef(null); + const cancelButtonRef = useRef<HTMLButtonElement>(null);
1-1: Filename typo: ai-prood-read → ai-proof-read.Helps discoverability and prevents future import typos.
apps/web/pages/free-tools/release-calendar.tsx (5)
84-87: Prefer crypto.randomUUID() over Date.now+Math.random.substr is legacy, and collisions are possible. Use crypto.randomUUID() with a small fallback.
Apply:
-const generateId = useCallback(() => { - return Date.now().toString(36) + Math.random().toString(36).substr(2); -}, []); +const generateId = useCallback(() => { + if (typeof crypto !== "undefined" && "randomUUID" in crypto) { + return crypto.randomUUID(); + } + // Fallback + return ( + Date.now().toString(36) + + Math.random().toString(36).slice(2) + + Math.random().toString(36).slice(2) + ); +}, []);
90-97: Minimal required-field validation OK, consider date validity too.Current check allows invalid dates like 2025-02-31. If you keep string dates, validate Y-M-D shape and ranges.
Example addition:
- if ( + const isYMD = /^\d{4}-\d{2}-\d{2}$/.test(formData.date); + if ( !formData.version.trim() || !formData.date.trim() || !formData.title.trim() ) { notifyError("Please fill in version, date, and title fields"); return; } + if (!isYMD) { + notifyError("Please use a valid date (YYYY-MM-DD)"); + return; + }
175-181: Avoid stale closure in togglePresentationMode.Side-effects should depend on the next state to prevent drift during rapid toggles.
Apply:
-const togglePresentationMode = useCallback(() => { - setPresentationMode((prev) => !prev); - if (!presentationMode) { - // Hide forms when entering presentation mode - setShowAddForm(false); - setEditingRelease(null); - } -}, [presentationMode]); +const togglePresentationMode = useCallback(() => { + setPresentationMode((prev) => { + const next = !prev; + if (next) { + setShowAddForm(false); + setEditingRelease(null); + } + return next; + }); +}, []);
185-193: Only attach Escape listener while in presentation mode.Slightly reduces event noise.
Apply:
-useEffect(() => { - const handleKeyPress = (event: KeyboardEvent) => { - if (event.key === "Escape" && presentationMode) { - setPresentationMode(false); - } - }; - document.addEventListener("keydown", handleKeyPress); - return () => document.removeEventListener("keydown", handleKeyPress); -}, [presentationMode]); +useEffect(() => { + if (!presentationMode) return; + const onKeyDown = (e: KeyboardEvent) => { + if (e.key === "Escape") setPresentationMode(false); + }; + document.addEventListener("keydown", onKeyDown); + return () => document.removeEventListener("keydown", onKeyDown); +}, [presentationMode]);
1368-1380: Use getStaticProps for this page.Props are static SEO strings; switching from getServerSideProps to getStaticProps reduces server load and enables CDN caching.
Example:
-export async function getServerSideProps() { +export async function getStaticProps() { return { props: { title: "Free Release Calendar Creator Online - Timeline Visualizer | Changes.page", description: "Create release calendars online with our free release calendar creator app. Plan software releases with interactive timeline and calendar views. Track milestones and manage release schedules.", keywords: "release calendar creator online, release calendar creator app, free release calendar, online release calendar, release timeline creator, software release calendar, release planning calendar, create release calendar, release calendar generator, release schedule creator, project release calendar, deployment calendar creator", canonicalUrl: "https://changes.page/free-tools/release-calendar", }, }; }apps/web/utils/useUser.tsx (2)
14-14: Import OK. Ensure IBillingInfo is nullable downstream.See my comment on Line 47 about using IBillingInfo | null for state and context.
75-99: Avoid setState after unmount in async/auth callbacks.Add a mounted flag so late resolves from getSession or auth listener don’t update state post-unmount.
- useEffect(() => { - // Get initial session - const getInitialSession = async () => { - const { - data: { session }, - } = await supabase.auth.getSession(); - setSession(session); - setUser(session?.user ?? null); - setLoading(false); - }; - - getInitialSession(); - - // Listen for auth changes - const { - data: { subscription }, - } = supabase.auth.onAuthStateChange(async (_, session) => { - setSession(session); - setUser(session?.user ?? null); - setLoading(false); - }); - - return () => subscription.unsubscribe(); - }, [supabase]); + useEffect(() => { + let mounted = true; + const apply = (s: Session | null) => { + if (!mounted) return; + setSession(s); + setUser(s?.user ?? null); + setLoading(false); + }; + const getInitialSession = async () => { + const { data: { session } } = await supabase.auth.getSession(); + apply(session); + }; + getInitialSession(); + const { data: { subscription } } = supabase.auth.onAuthStateChange((_, s) => { + apply(s); + }); + return () => { + mounted = false; + subscription.unsubscribe(); + }; + }, [supabase]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
apps/web/components/dialogs/ai-expand-concept-prompt-dialog.component.tsx(3 hunks)apps/web/components/dialogs/ai-prood-read-dialog.component.tsx(3 hunks)apps/web/components/dialogs/ai-suggest-title-prompt-dialog.component.tsx(4 hunks)apps/web/components/dialogs/confirm-delete-dialog.component.tsx(4 hunks)apps/web/components/dialogs/date-time-prompt-dialog.component.tsx(3 hunks)apps/web/components/dialogs/manage-team-dialog.component.tsx(3 hunks)apps/web/components/dialogs/warning-dialog.component.tsx(4 hunks)apps/web/components/dialogs/widget-code-dialog.tsx(3 hunks)apps/web/components/forms/post-form.component.tsx(3 hunks)apps/web/components/layout/blog-layout.component.tsx(1 hunks)apps/web/components/layout/header.component.tsx(1 hunks)apps/web/components/layout/page.component.tsx(2 hunks)apps/web/components/marketing/changelog.tsx(2 hunks)apps/web/components/post/post-options.tsx(2 hunks)apps/web/components/post/post.tsx(1 hunks)apps/web/components/roadmap/RoadmapItemModal.tsx(2 hunks)apps/web/next.config.js(0 hunks)apps/web/package.json(3 hunks)apps/web/pages/api/billing/jobs/report-usage.ts(3 hunks)apps/web/pages/api/integrations/zapier/trigger-new-post.tsx(2 hunks)apps/web/pages/api/pages/settings/remove-domain.ts(1 hunks)apps/web/pages/free-tools/release-calendar.tsx(27 hunks)apps/web/pages/pages/[page_id]/[post_id].tsx(3 hunks)apps/web/utils/hooks/usePosts.ts(2 hunks)apps/web/utils/useUser.tsx(5 hunks)
💤 Files with no reviewable changes (1)
- apps/web/next.config.js
🚧 Files skipped from review as they are similar to previous changes (17)
- apps/web/components/post/post.tsx
- apps/web/pages/api/integrations/zapier/trigger-new-post.tsx
- apps/web/components/dialogs/ai-expand-concept-prompt-dialog.component.tsx
- apps/web/components/layout/page.component.tsx
- apps/web/components/forms/post-form.component.tsx
- apps/web/components/dialogs/date-time-prompt-dialog.component.tsx
- apps/web/package.json
- apps/web/components/dialogs/manage-team-dialog.component.tsx
- apps/web/utils/hooks/usePosts.ts
- apps/web/components/dialogs/widget-code-dialog.tsx
- apps/web/components/layout/blog-layout.component.tsx
- apps/web/components/marketing/changelog.tsx
- apps/web/components/roadmap/RoadmapItemModal.tsx
- apps/web/components/layout/header.component.tsx
- apps/web/components/post/post-options.tsx
- apps/web/pages/pages/[page_id]/[post_id].tsx
- apps/web/components/dialogs/warning-dialog.component.tsx
🔇 Additional comments (22)
apps/web/pages/api/billing/jobs/report-usage.ts (2)
23-25: Unused req param rename — LGTM.
41-43: Direct assignment over double-cast — LGTM.Cleaner and type-checked now that the query overrides the return type.
apps/web/pages/api/pages/settings/remove-domain.ts (2)
1-3: Good direction: centralized auth + input validation.Nice move to withAuth + isFQDN; this aligns with the SSR migration.
5-6: Confirm withAuth enforces authentication and (optionally) allowed methods.If withAuth doesn’t 401 unauthenticated requests or guard methods, add those here.
Run:
#!/bin/bash # Inspect withAuth implementation and usages rg -n -C2 'export function withAuth|const withAuth' --apps --hidden rg -n -C2 'withAuth\<' apps/web/pages/api | sed -n '1,200p' rg -n -C2 'status\\(401\\)|Unauthorized' apps/web/utils/withAuth.ts rg -n -C2 'req\\.method' apps/web/utils/withAuth.tsapps/web/components/dialogs/confirm-delete-dialog.component.tsx (1)
2-2: Spinner import consolidation looks good.
Removes duplication and keeps UI imports consistent.apps/web/components/dialogs/ai-suggest-title-prompt-dialog.component.tsx (1)
1-2: Good additions for UX and preprocessing.
SpinnerWithSpacingandconvertMarkdownToPlainTextare appropriate here.apps/web/pages/free-tools/release-calendar.tsx (10)
1-7: Import cleanup looks good.Consolidated Heroicons import is fine and keeps bundle size reasonable given solid icons are already tree-shaken.
46-56: SSR-safe localStorage read.The window guard + try/catch is correct for Next.js pages.
60-67: Persisting to localStorage is handled safely.Guard + try/catch is good. No action needed.
110-116: Release update path uses functional setState correctly.Map/replace by id is clear and safe.
148-150: Delete handler is straightforward and efficient.Filters by id and toasts. Looks good.
167-171: Clear localStorage key on reset.Good cleanup symmetry with save/load.
256-266: Style maps are clear and consistent.Enums → class maps read well; defaults handled.
Also applies to: 271-281, 287-297, 302-312
623-629: Title tooltip concatenation is fine.Using a template string with optional description is OK; low risk since it’s not innerHTML.
1252-1281: onClick → handleEditRelease is a nice UX touch.No issues here; keep.
746-761: JSON-LD block looks correct.Static strings and props are serialized safely via JSON.stringify.
apps/web/utils/useUser.tsx (6)
2-2: LGTM on Supabase typings import.Importing Session, SupabaseClient, and User looks correct for the new client usage.
36-42: Provider props shape looks good.initialSession default and children typing align with SSR usage.
63-71: Sign-out order + analytics reset: good fix.Signing out before navigation and calling posthog.reset() prevents stale flashes.
120-121: Including signOut in context value: good.Matches the new API surface.
131-131: Runtime guard currently ineffective.The undefined check only works if the context’s default is undefined (see previous comment). After the refactor, this becomes valid.
16-16: Verify SSR-safe client factory.Confirm that ./supabase/client uses @supabase/ssr (or proper env guards) so calling createClient() during SSR won’t touch window/document.
#!/bin/bash set -euo pipefail # Inspect the supabase client factory and related utils fd -a 'apps' -t d echo "---- Supabase client implementations ----" fd -a 'client.ts' apps | sed -n '1,200p' | xargs -I{} sh -c 'echo ">>> {}"; sed -n "1,200p" "{}"' echo "---- Look for SSR helpers usage ----" rg -n -C2 '@supabase/ssr|createServerClient|createMiddlewareClient|createBrowserClient|auth-helpers' apps || true
| .select("id,stripe_subscription") | ||
| .filter("stripe_subscription->>status", "in", '("active","trialing")') | ||
| .overrideTypes<UserWithSubscription[]>(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Safer filtering and pagination needed; consider .in() and paging to avoid the 1,000-row cap.
- Optional: Prefer
.in()over raw'in'filter for readability. - Essential: Supabase limits select() to 1,000 rows by default; without pagination you’ll skip users beyond the first page, risking under-reporting usage. (supabase.com)
Apply this for clearer filtering:
- .select("id,stripe_subscription")
- .filter("stripe_subscription->>status", "in", '("active","trialing")')
- .overrideTypes<UserWithSubscription[]>();
+ .select("id,stripe_subscription")
+ .in("stripe_subscription->>status", ["active", "trialing"])
+ .overrideTypes<UserWithSubscription[]>();Example pagination pattern (sketch):
const pageSize = 1000;
let from = 0, to = pageSize - 1;
for (;;) {
const { data, error } = await supabaseAdmin
.from("users")
.select("id,stripe_subscription")
.in("stripe_subscription->>status", ["active", "trialing"])
.range(from, to)
.overrideTypes<UserWithSubscription[]>();
if (error) throw error;
if (!data?.length) break;
// process batch...
from += pageSize; to += pageSize;
}🤖 Prompt for AI Agents
In apps/web/pages/api/billing/jobs/report-usage.ts around lines 33–35 the query
uses a raw 'in' filter and no pagination, which risks missing users beyond
Supabase's 1,000-row default; change the filter to the client
.in("stripe_subscription->>status", ["active","trialing"]) form and implement
paginated fetches (e.g., loop with a pageSize like 1000 using .range(from, to)),
process each batch and break when data.length === 0, and propagate/throw on
errors so all matching users are fetched and handled safely.
| const { user } = await getSupabaseServerClient({ req, res }); | ||
| const removeDomain = withAuth<{ success: boolean }>( | ||
| async (req, res, { user }) => { | ||
| const { domain } = req.body; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Normalize and strictly validate domain input.
Trim/lowercase to avoid case or whitespace mismatches; keep FQDN validation.
Apply:
- const { domain } = req.body;
+ const raw = req.body?.domain;
+ const domain = typeof raw === "string" ? raw.trim().toLowerCase() : raw;
- if (
- typeof domain !== "string" ||
- !isFQDN(domain, { require_tld: true, allow_underscores: false })
- ) {
+ if (
+ typeof domain !== "string" ||
+ !isFQDN(domain, { require_tld: true, allow_underscores: false })
+ ) {
return res.status(400).json({ success: false });
}Also applies to: 11-16
🤖 Prompt for AI Agents
In apps/web/pages/api/pages/settings/remove-domain.ts around lines 7 and 11-16,
the handler currently reads domain directly from req.body without normalization
or strict validation; trim and lower-case the incoming domain before any
comparisons or DB calls (e.g. domain =
String(req.body.domain).trim().toLowerCase()), then validate it as a proper FQDN
(use a robust FQDN regex or an existing validator library, and handle punycode
if needed); if validation fails return a 400 with an explanatory message and do
not proceed with removal, otherwise continue using the normalized value for all
subsequent logic and DB operations.
| (a, b) => new Date(a.date).getTime() - new Date(b.date).getTime() | ||
| ); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Sort using local date parsing; also memoize.
Avoid new Date(ymd). Use the helper and compute once per releases change.
Also update imports to include useMemo.
-import { useCallback, useEffect, useState } from "react";
+import { useCallback, useEffect, useMemo, useState } from "react";
@@
-const sortedReleases = [...releases].sort(
- (a, b) => new Date(a.date).getTime() - new Date(b.date).getTime()
-);
+const sortedReleases = useMemo(
+ () =>
+ [...releases].sort(
+ (a, b) => parseYMDLocal(a.date).getTime() - parseYMDLocal(b.date).getTime()
+ ),
+ [releases]
+);🤖 Prompt for AI Agents
In apps/web/pages/free-tools/release-calendar.tsx around lines 249–251, avoid
calling new Date(...) inside the sort and recomputing dates on every render;
instead import useMemo and the existing YMD parsing helper, then memoize the
sorted list: map releases to include a single parsed timestamp using the helper,
sort by that timestamp, and return the sorted array from useMemo with [releases]
as dependency; also update the file imports to include useMemo and the helper.
| // @ts-ignore | ||
| return <UserContext.Provider value={value}>{children}</UserContext.Provider>; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove ts-ignore by fixing Context typing (real issue).
The ts-ignore hides mismatches: default context shape returns nulls and functions with wrong return types. Use an undefined default with a proper value type; this also makes the runtime guard in useUserData effective.
- // @ts-ignore
- return <UserContext.Provider value={value}>{children}</UserContext.Provider>;
+ return <UserContext.Provider value={value}>{children}</UserContext.Provider>;Apply this type refactor elsewhere in the file (outside these lines) to make it compile cleanly:
// Define once near the top
type UserContextValue = {
loading: boolean;
session: Session | null;
user: User | null;
billingDetails: IBillingInfo | null;
fetchBilling: () => Promise<IBillingInfo | null>;
signOut: () => Promise<{ error: AuthError | null }>;
supabase: SupabaseClient<Database>;
};
// Replace current createContext(...) with:
const UserContext = createContext<UserContextValue | undefined>(undefined);
// Optionally type the hook return:
export const useUserData = (): UserContextValue => {
const context = useContext(UserContext);
if (context === undefined) {
throw new Error(`useUserData must be used within a UserContextProvider.`);
}
return context;
}
// And add this import:
import { Session, SupabaseClient, User, AuthError } from "@supabase/supabase-js";🤖 Prompt for AI Agents
In apps/web/utils/useUser.tsx around lines 124-125, remove the // @ts-ignore by
fixing the Context typing: define a UserContextValue type (loading, session,
user, billingDetails, fetchBilling, signOut, supabase) and change createContext
to createContext<UserContextValue | undefined>(undefined); update the
useUserData hook to return UserContextValue and throw if context is undefined,
and import Session, SupabaseClient, User, AuthError from @supabase/supabase-js;
these changes align runtime guard with TS types so the Provider return no longer
needs ts-ignore.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
apps/web/pages/api/pages/settings/remove-domain.ts (4)
5-7: Add an explicit HTTP method guard (DELETE/POST).Destructive endpoint should 405 everything else.
async (req, res, { user }) => { + if (req.method !== "DELETE" && req.method !== "POST") { + res.setHeader("Allow", "DELETE, POST"); + return res.status(405).end("Method Not Allowed"); + } const { domain } = req.body;
7-7: Normalize and strictly validate input domain.Trim/lower-case to avoid DB mismatches; keep FQDN check. Consider IDN→ASCII if you accept IDNs.
- const { domain } = req.body; + const raw = req.body?.domain; + const domain = typeof raw === "string" ? raw.trim().toLowerCase() : raw; if ( typeof domain !== "string" || !isFQDN(domain, { require_tld: true, allow_underscores: false }) ) { return res.status(400).json({ success: false }); } - .eq("custom_domain", domain) + .eq("custom_domain", domain)Optional (if IDNs allowed):
// import { toASCII } from "punycode/"; const ascii = domain; // or toASCII(domain)Also applies to: 11-16, 22-22
18-25: Use maybeSingle() and return correct status codes.Differentiate DB errors (500) from ownership/absent rows (403/404).
- const { data, error } = await supabaseAdmin + const { data, error } = await supabaseAdmin .from("page_settings") .select("page_id,custom_domain, pages(user_id)") .eq("custom_domain", domain) - .single(); - if (!data || error || data?.pages?.user_id !== user.id) { - return res.status(400).json({ success: false }); - } + .maybeSingle(); + if (error) { + console.error("removeDomain DB error", error); + return res.status(500).json({ success: false }); + } + if (!data || data?.pages?.user_id !== user.id) { + return res.status(403).json({ success: false }); + }
28-37: Handle upstream failures, add timeout, encode the path segment (SSRF finding), and verify env vars.Avoid always returning 200; protect against hangs and path injection; fixes CodeQL SSRF alert by encoding the domain.
- const safeDomain = data.custom_domain; - const response = await fetch( - `https://api.vercel.com/v8/projects/${process.env.VERCEL_PAGES_PROJECT_ID}/domains/${safeDomain}?teamId=${process.env.VERCEL_TEAM_ID}`, - { - headers: { - Authorization: `Bearer ${process.env.VERCEL_AUTH_TOKEN}`, - }, - method: "DELETE", - } - ); - - await response.json(); - - res.status(200).json({ - success: true, - }); + const safeDomain = data.custom_domain; + const projectId = process.env.VERCEL_PAGES_PROJECT_ID; + const teamId = process.env.VERCEL_TEAM_ID; + const token = process.env.VERCEL_AUTH_TOKEN; + if (!projectId || !teamId || !token) { + console.error("Missing Vercel config env vars"); + return res.status(500).json({ success: false }); + } + const encoded = encodeURIComponent(safeDomain); + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 10_000); + const url = `https://api.vercel.com/v8/projects/${projectId}/domains/${encoded}?teamId=${teamId}`; + const response = await fetch(url, { + headers: { Authorization: `Bearer ${token}` }, + method: "DELETE", + signal: controller.signal, + }).catch((e) => { + console.error("removeDomain fetch failed", e); + return null; + }); + clearTimeout(timeout); + + if (!response) return res.status(502).json({ success: false }); + let body: any = null; + try { + body = await response.json(); + } catch {} + + // Treat 404 as idempotent success + if (!response.ok && response.status !== 404) { + console.error("removeDomain Vercel error", response.status, body); + return res.status(502).json({ success: false }); + } + + // reflect removal in DB + const { error: updError } = await supabaseAdmin + .from("page_settings") + .update({ custom_domain: null }) + .eq("page_id", data.page_id); + if (updError) { + console.error("removeDomain DB update error", updError); + return res.status(500).json({ success: false }); + } + return res.status(200).json({ success: true });Also applies to: 39-43
🧹 Nitpick comments (5)
apps/web/pages/api/pages/settings/remove-domain.ts (2)
9-9: Remove noisy console log or guard by NODE_ENV.Avoid logging identifiers in production.
- console.log("removeDomain", user?.id, `domain: ${domain}`); + if (process.env.NODE_ENV !== "production") { + console.debug("removeDomain", user?.id, `domain: ${domain}`); + }
20-23: Enforce uniqueness on custom_domain (case-insensitive).Prevents .single() conflicts and ensures lookups are deterministic.
Example migration:
CREATE UNIQUE INDEX IF NOT EXISTS page_settings_custom_domain_key ON page_settings (lower(custom_domain));apps/web/pages/api/billing/enable-email-notifications.ts (3)
70-70: Prefer 200 over 201 for idempotent PUT.PUT toggles a setting on an existing subscription; 200 (or 204) is more conventional.
- return res.status(201).json({ status: "ok" }); + return res.status(200).json({ status: "ok" });
71-76: Harden error handling: log as error and avoid leaking internals.Return a generic message; keep details in server logs.
- console.log("createBillingSession", err); - res - .status(500) - .json({ error: { statusCode: 500, message: err.message } }); + console.error("enableEmailNotifications", err); + res.status(500).json({ + error: { statusCode: 500, message: "Internal server error" }, + });
78-79: Unify response shape for 405.Keep JSON error format for clients.
- res.setHeader("Allow", "PUT"); - res.status(405).end("Method Not Allowed"); + res.setHeader("Allow", "PUT"); + res.status(405).json({ + error: { statusCode: 405, message: "Method Not Allowed" }, + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/pages/api/billing/enable-email-notifications.ts(1 hunks)apps/web/pages/api/pages/settings/remove-domain.ts(1 hunks)apps/web/utils/supabase/withSupabase.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/utils/supabase/withSupabase.ts
🔇 Additional comments (6)
apps/web/pages/api/pages/settings/remove-domain.ts (1)
5-7: Confirm withAuth guarantees a non-null user; otherwise add a 401 guard.Current code would throw on Line 24 if user is undefined.
Would you confirm that utils/withAuth responds 401 when no session? If not, add:
- async (req, res, { user }) => { + async (req, res, { user }) => { + if (!user) return res.status(401).json({ success: false });apps/web/pages/api/billing/enable-email-notifications.ts (5)
11-19: Solid env guard for priceId.Prevents misconfig-driven 500s and avoids hitting Stripe with bad input.
38-45: Good guard for missing Stripe subscription id.Avoids
retrieve(null)500s.
51-58: Idempotent “already added” check looks good.Short-circuits duplicates cleanly.
60-68: Nice use of idempotency key on create.Prevents dup items during retries.
3-3: Good move to withAuth; addresses unauthenticated access.This replaces ad-hoc checks and should return 401 upstream when no session.
Run to confirm withAuth enforces 401 on missing session:
#!/bin/bash rg -nC3 --type=ts 'export\s+const\s+withAuth|function\s+withAuth' apps | sed -n '1,120p'Also applies to: 7-9
| import { getUserById } from "../../../utils/useDatabase"; | ||
| import { withAuth } from "../../../utils/withAuth"; | ||
|
|
||
| const stripe = require("stripe")(process.env.STRIPE_SECRET_KEY); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use typed Stripe client and pin API version.
Improves type-safety and future-proofing; avoid require and set apiVersion.
-const stripe = require("stripe")(process.env.STRIPE_SECRET_KEY);
+const stripe = new Stripe(process.env.STRIPE_SECRET_KEY as string, {
+ apiVersion: "2024-06-20",
+});Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/pages/api/billing/enable-email-notifications.ts around line 5,
replace the untyped require call with the typed Stripe client and pin the API
version: import the Stripe class from 'stripe', instantiate it via new
Stripe(process.env.STRIPE_SECRET_KEY, { apiVersion: '<PINNED_VERSION>' }), and
ensure the secret key is asserted/validated (or throw a clear error) so the
client is fully typed and the API version is explicit for stability.
| if ( | ||
| !stripe_subscription || | ||
| (stripe_subscription as unknown as Stripe.Subscription)?.status === | ||
| "canceled" | ||
| ) { | ||
| return res.status(400).json({ | ||
| error: { | ||
| statusCode: 400, | ||
| message: | ||
| "You have canceled your subscription. Please reactivate it to continue using this feature.", | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Bug: “canceled subscription” check uses stale DB snapshot and blocks valid users.
Treating a missing stripe_subscription object as canceled is incorrect if stripe_subscription_id exists. Check status from Stripe after retrieve, not from DB.
Apply:
- if (
- !stripe_subscription ||
- (stripe_subscription as unknown as Stripe.Subscription)?.status ===
- "canceled"
- ) {
- return res.status(400).json({
- error: {
- statusCode: 400,
- message:
- "You have canceled your subscription. Please reactivate it to continue using this feature.",
- },
- });
- }And after retrieving the subscription (below Line 49), add:
+ if (subscription.status === "canceled") {
+ return res.status(400).json({
+ error: {
+ statusCode: 400,
+ message:
+ "You have canceled your subscription. Please reactivate it to continue using this feature.",
+ },
+ });
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ( | |
| !stripe_subscription || | |
| (stripe_subscription as unknown as Stripe.Subscription)?.status === | |
| "canceled" | |
| ) { | |
| return res.status(400).json({ | |
| error: { | |
| statusCode: 400, | |
| message: | |
| "You have canceled your subscription. Please reactivate it to continue using this feature.", | |
| }, | |
| }); | |
| } | |
| const { stripe_subscription_id, stripe_subscription } = | |
| await getUserById(user.id); | |
| - if ( | |
| - !stripe_subscription || | |
| - (stripe_subscription as unknown as Stripe.Subscription)?.status === | |
| - "canceled" | |
| - ) { | |
| - return res.status(400).json({ | |
| - error: { | |
| - statusCode: 400, | |
| - message: | |
| - "You have canceled your subscription. Please reactivate it to continue using this feature.", | |
| - }, | |
| - }); | |
| - } | |
| const subscription = await stripe.subscriptions.retrieve( | |
| stripe_subscription_id | |
| ); | |
| if (subscription.status === "canceled") { | |
| return res.status(400).json({ | |
| error: { | |
| statusCode: 400, | |
| message: | |
| "You have canceled your subscription. Please reactivate it to continue using this feature.", | |
| }, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In apps/web/pages/api/billing/enable-email-notifications.ts around lines 24–36,
the current code treats a missing stripe_subscription DB object as a canceled
subscription and blocks users; change the logic so you only treat a subscription
as canceled after retrieving the live subscription from Stripe. Remove the early
return that assumes cancellation when stripe_subscription is null; instead,
after the Stripe retrieval (around line 49), if you have a
stripe_subscription_id but no local stripe_subscription, fetch the subscription
from Stripe and then check its status (if status === "canceled" return the 400
JSON), and if Stripe returns an active subscription proceed normally; keep the
original 400 response shape for canceled cases.
| const subscription = await stripe.subscriptions.retrieve( | ||
| stripe_subscription_id | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add a quick guard for missing STRIPE_SECRET_KEY before Stripe calls.
Prevents “No API key provided” runtime errors with clearer 500.
- const subscription = await stripe.subscriptions.retrieve(
+ if (!process.env.STRIPE_SECRET_KEY) {
+ return res.status(500).json({
+ error: {
+ statusCode: 500,
+ message: "Internal error: Stripe secret not configured.",
+ },
+ });
+ }
+ const subscription = await stripe.subscriptions.retrieve(
stripe_subscription_id
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const subscription = await stripe.subscriptions.retrieve( | |
| stripe_subscription_id | |
| ); | |
| if (!process.env.STRIPE_SECRET_KEY) { | |
| return res.status(500).json({ | |
| error: { | |
| statusCode: 500, | |
| message: "Internal error: Stripe secret not configured.", | |
| }, | |
| }); | |
| } | |
| const subscription = await stripe.subscriptions.retrieve( | |
| stripe_subscription_id | |
| ); |
🤖 Prompt for AI Agents
In apps/web/pages/api/billing/enable-email-notifications.ts around lines 47 to
49, add a guard that checks process.env.STRIPE_SECRET_KEY (or the configured env
var used to instantiate Stripe) before calling stripe.subscriptions.retrieve; if
the key is missing, immediately respond with a 500 error and a clear message
(and avoid invoking any Stripe client methods) so the runtime “No API key
provided” error is prevented and a clearer server error is returned.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
apps/web/pages/pages/[page_id]/roadmap/new.tsx (1)
131-141: Handle RPC failures and avoid redirecting on partial initialization.If either RPC fails, users get redirected to a broken board. Handle errors and gate the redirect.
- // Initialize default stages for the board - await supabase.rpc("initialize_roadmap_columns", { board_id: board.id }); - - // Initialize default categories for the board - await supabase.rpc("initialize_roadmap_categories", { - board_id: board.id, - }); - - // Redirect to the roadmap page - await router.push(`/pages/${page_id}/roadmap`); + // Initialize defaults in parallel + const [{ error: colError }, { error: catError }] = await Promise.all([ + supabase.rpc("initialize_roadmap_columns", { board_id: board.id }), + supabase.rpc("initialize_roadmap_categories", { board_id: board.id }), + ]); + if (colError || catError) { + console.error("Error initializing board defaults", { colError, catError }); + setErrors({ general: "Failed to initialize board defaults" }); + return; + } + + // Redirect after successful initialization + await router.push(`/pages/${page_id}/roadmap`);apps/web/utils/useUser.tsx (1)
58-68: Ensure fetchBilling returns a Promise of IBillingInfo | null on all paths.The catch branch currently returns undefined. Return null and tighten typing.
-const fetchBilling = useCallback(async () => { - return httpGet({ url: `/api/billing` }) - .then((billingDetails) => { - setBillingDetails(billingDetails); - return billingDetails; - }) - .catch((error) => { - console.error("Failed to get billing data:", error); - notifyError("Failed to fetch billing information"); - }); -}, []); +const fetchBilling = useCallback(async (): Promise<IBillingInfo | null> => { + try { + const billingDetails = await httpGet<IBillingInfo>({ url: `/api/billing` }); + setBillingDetails(billingDetails); + return billingDetails; + } catch (error) { + console.error("Failed to get billing data:", error); + notifyError("Failed to fetch billing information"); + return null; + } +}, []);apps/web/utils/useDatabase.ts (1)
209-224: Make createOrRetrievePageSettings idempotent to avoid race on concurrent calls.Two parallel requests can both miss and then insert, causing a unique-violation. Use upsert on page_id and return a single row.
-export const createOrRetrievePageSettings = async (page_id: string) => { +export const createOrRetrievePageSettings = async (page_id: string) => { const { data: pageSettings } = await supabaseAdmin .from("page_settings") .select("*") .eq("page_id", page_id) .single(); if (pageSettings) return pageSettings; - const { data: newPageSettings, error: createPageSettingsError } = - await supabaseAdmin.from("page_settings").insert([{ page_id }]).select(); + const { data: newPageSettings, error: createPageSettingsError } = + await supabaseAdmin + .from("page_settings") + .upsert({ page_id }, { onConflict: "page_id" }) + .select() + .single(); if (createPageSettingsError) throw createPageSettingsError; - return newPageSettings[0]; + return newPageSettings; };apps/web/pages/pages/[page_id]/roadmap/[board_id]/settings.tsx (2)
20-45: Enforce SSR ownership to avoid leaking board data.Only the page owner should receive the board and settings SSR payload.
-export const getServerSideProps = withSupabase(async (ctx, { supabase }) => { +export const getServerSideProps = withSupabase(async (ctx, { supabase, user }) => { const { page_id } = ctx.params; if (!page_id || Array.isArray(page_id)) { return { notFound: true }; } const { board_id } = ctx.params; if (!board_id || Array.isArray(board_id)) { return { notFound: true }; } - const { tab = "board" } = ctx.query; + const tabParam = Array.isArray(ctx.query.tab) ? ctx.query.tab[0] : ctx.query.tab; + const tab = typeof tabParam === "string" ? tabParam : "board"; const page = await getPage(supabase, page_id).catch((e) => { console.error("Failed to get page", e); return null; }); if (!page) { return { notFound: true, }; } + if (!user || page.user_id !== user.id) { + return { notFound: true }; + } const settings = await createOrRetrievePageSettings(page_id);
347-358: Fix -Infinity when adding first column.Math.max on an empty array yields -Infinity.
- const maxPosition = - Math.max(...boardColumns.map((col) => col.position)) + 1; + const maxExisting = boardColumns.length + ? Math.max(...boardColumns.map((col) => col.position || 0)) + : 0; + const maxPosition = maxExisting + 1;
♻️ Duplicate comments (6)
apps/web/pages/pages/[page_id]/roadmap/new.tsx (1)
27-31: Stop serializing unusedpageprop to the client.
pageisn’t used by the component but will be serialized over the wire. Drop it frompropsto reduce payload and avoid leaking fields unnecessarily. (Same ask as earlier review.)Apply:
return { props: { page_id, - page, }, };apps/web/utils/useUser.tsx (1)
70-80: Sign-out sequence and PostHog reset — nice cleanup.Sign-out first, reset analytics, clear billing, then navigate. Matches previous guidance.
apps/web/pages/pages/[page_id]/settings/[activeTab].tsx (1)
38-39: Normalize activeTab to a string.ctx.params?.activeTab can be string | string[] | undefined; normalize for equality checks.
- activeTab: ctx.params?.activeTab, + activeTab: Array.isArray(ctx.params?.activeTab) + ? ctx.params.activeTab[0] + : (ctx.params?.activeTab ?? "general"),apps/web/pages/pages/[page_id]/[post_id].tsx (1)
18-29: Add SSR authorization and 404 on missing/foreign post.Gate by page owner and ensure the post belongs to the page.
-export const getServerSideProps = withSupabase(async (ctx, { supabase }) => { +export const getServerSideProps = withSupabase(async (ctx, { supabase, user }) => { const { page_id, post_id } = ctx.params; if (!page_id || Array.isArray(page_id)) { return { notFound: true }; } if (!post_id || Array.isArray(post_id)) { return { notFound: true }; } - const settings = await createOrRetrievePageSettings(page_id); - const { data: post } = await supabase + const page = await getPage(supabase, page_id).catch(() => null); + if (!page || page.user_id !== user.id) { + return { notFound: true }; + } + + const settings = await createOrRetrievePageSettings(page_id); + const { data: post, error: postError } = await supabase .from("posts") .select("*") - .eq("id", post_id) + .eq("id", post_id) + .eq("page_id", page_id) .single(); + + if (postError || !post) { + return { notFound: true }; + }apps/web/pages/pages/[page_id]/analytics.tsx (2)
218-221: Exclude null/empty visitor_ids from “unique visitors”.Nulls/empties collapse to one Set entry and skew counts.
- const visitors = new Set(currentPeriodViews.map((v) => v.visitor_id)).size; + const visitors = new Set( + currentPeriodViews.map((v) => v.visitor_id).filter(Boolean) + ).size; @@ - const prev_visitors = new Set(previousPeriodViews.map((v) => v.visitor_id)) - .size; + const prev_visitors = new Set( + previousPeriodViews.map((v) => v.visitor_id).filter(Boolean) + ).size;
186-196: Add SSR ownership check and 404 on missing page.Restrict analytics to the page owner and handle missing pages.
-export const getServerSideProps = withSupabase(async (ctx, { supabase }) => { +export const getServerSideProps = withSupabase(async (ctx, { supabase, user }) => { const page_id = ctx.params?.page_id; if (!page_id || Array.isArray(page_id)) { return { notFound: true }; } - const page = await getPage(supabase, page_id); + const page = await getPage(supabase, page_id).catch(() => null); + if (!page || page.user_id !== user.id) { + return { notFound: true }; + }
🧹 Nitpick comments (9)
apps/web/pages/pages/[page_id]/roadmap/new.tsx (3)
245-257: Disable submit when Supabase client isn’t ready.If
useUserData()returns a transientundefinedclient, clicking submit will throw. Disable the button until the client is available.- <button - type="submit" - disabled={isCreating} + <button + type="submit" + disabled={isCreating || !supabase} className="inline-flex justify-center py-2 px-4 border border-transparent shadow-sm text-sm font-medium rounded-md text-white bg-indigo-600 hover:bg-indigo-700 focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-indigo-500 disabled:opacity-50 disabled:cursor-not-allowed"
178-186: Add basic HTML validation to inputs.Mirror your client-side checks in the DOM for better UX and no-JS safety.
- <input + <input + required type="text" id="title" value={formData.title} onChange={(e) => handleInputChange("title", e.target.value)}- <input + <input + required + pattern="^[a-z0-9-]+$" type="text" id="slug" value={formData.slug} onChange={(e) => handleInputChange("slug", e.target.value)}Also applies to: 223-231
72-79: Cap auto-generated slug length to avoid oversized URLs.Keep slugs reasonable (e.g., ≤64 chars) when auto-generating from title.
- return title + return title .toLowerCase() .replace(/[^a-z0-9\s-]/g, "") .replace(/\s+/g, "-") .replace(/-+/g, "-") - .replace(/^-|-$/g, ""); + .replace(/^-|-$/g, "") + .slice(0, 64);apps/web/utils/useUser.tsx (2)
121-129: Memoize the context value to prevent avoidable re-renders.Stabilize the Provider value with useMemo.
-const value: UserContextValue = { - loading, - session, - user, - billingDetails, - fetchBilling, - signOut, - supabase, -}; +const value: UserContextValue = useMemo( + () => ({ + loading, + session, + user, + billingDetails, + fetchBilling, + signOut, + supabase, + }), + [loading, session, user, billingDetails, fetchBilling, signOut, supabase] +);Also applies to: 11-16
70-79: Toast only on success; surface error message otherwise.Minor UX polish: if signOut returns an error, show notifyError instead of a success toast.
- await router.replace(ROUTES.HOME); - - notifyInfo("Logout completed"); + await router.replace(ROUTES.HOME); + if (error) { + notifyError(error.message ?? "Failed to logout"); + } else { + notifyInfo("Logout completed"); + }apps/web/utils/useDatabase.ts (1)
355-358: Handle invalid/zero ranges explicitly.range || 7 turns 0 into 7. Clamp to positive integers for predictability.
- const date = new Date(Date.now() - (range || 7) * 24 * 60 * 60 * 1000); + const days = Number.isFinite(range) && range > 0 ? Math.floor(range) : 7; + const date = new Date(Date.now() - days * 24 * 60 * 60 * 1000);apps/web/pages/pages/[page_id]/[post_id].tsx (1)
71-71: Remove redundant String() cast.page_id is already a string.
- page_id: String(page_id), + page_id,apps/web/pages/pages/[page_id]/roadmap/index.tsx (2)
151-158: Avoid SSR/CSR date drift; pin timezone or preformat on servertoLocaleDateString without a fixed timeZone can render different days between server and client depending on host tz, causing hydration warnings. Pin to UTC (or your intended zone), or preformat in GSSP.
Apply this diff to pin timezone:
- {new Date(board.created_at).toLocaleDateString( - "en-US", - { - year: "numeric", - month: "short", - day: "numeric", - } - )} + {new Date(board.created_at).toLocaleDateString("en-US", { + timeZone: "UTC", + year: "numeric", + month: "short", + day: "numeric", + })}If you’re seeing hydration warnings in the console, this should resolve them. Alternatively, precompute a display string in getServerSideProps and pass it down.
161-170: A11y: mark decorative SVG as hidden from screen readersThis chevron is purely decorative; add aria-hidden to avoid noise for AT users.
Apply this diff:
- <svg + <svg + aria-hidden="true" className="h-5 w-5 text-gray-400 group-hover:text-gray-500" fill="currentColor" viewBox="0 0 20 20" >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/web/pages/pages/[page_id]/[post_id].tsx(3 hunks)apps/web/pages/pages/[page_id]/analytics.tsx(5 hunks)apps/web/pages/pages/[page_id]/audit-logs.tsx(2 hunks)apps/web/pages/pages/[page_id]/edit.tsx(1 hunks)apps/web/pages/pages/[page_id]/index.tsx(3 hunks)apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx(3 hunks)apps/web/pages/pages/[page_id]/roadmap/[board_id]/settings.tsx(3 hunks)apps/web/pages/pages/[page_id]/roadmap/index.tsx(4 hunks)apps/web/pages/pages/[page_id]/roadmap/new.tsx(2 hunks)apps/web/pages/pages/[page_id]/settings/[activeTab].tsx(2 hunks)apps/web/utils/useDatabase.ts(4 hunks)apps/web/utils/useUser.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/web/pages/pages/[page_id]/edit.tsx
- apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx
- apps/web/pages/pages/[page_id]/audit-logs.tsx
- apps/web/pages/pages/[page_id]/index.tsx
🧰 Additional context used
🧬 Code graph analysis (8)
apps/web/pages/pages/[page_id]/settings/[activeTab].tsx (3)
apps/web/utils/supabase/withSupabase.ts (1)
withSupabase(11-32)apps/web/utils/useSSR.ts (1)
getPage(4-13)apps/web/utils/useDatabase.ts (1)
createOrRetrievePageSettings(209-224)
apps/web/pages/pages/[page_id]/analytics.tsx (3)
apps/web/pages/pages/[page_id]/index.tsx (1)
getServerSideProps(49-75)apps/web/utils/supabase/withSupabase.ts (1)
withSupabase(11-32)apps/web/utils/useSSR.ts (1)
getPage(4-13)
apps/web/pages/pages/[page_id]/roadmap/[board_id]/settings.tsx (3)
apps/web/pages/pages/[page_id]/roadmap/[board_id].tsx (1)
getServerSideProps(12-93)apps/web/utils/supabase/withSupabase.ts (1)
withSupabase(11-32)apps/web/utils/useDatabase.ts (1)
createOrRetrievePageSettings(209-224)
apps/web/pages/pages/[page_id]/roadmap/index.tsx (1)
apps/web/utils/supabase/withSupabase.ts (1)
withSupabase(11-32)
apps/web/pages/pages/[page_id]/[post_id].tsx (2)
apps/web/utils/supabase/withSupabase.ts (1)
withSupabase(11-32)apps/web/utils/useDatabase.ts (1)
createOrRetrievePageSettings(209-224)
apps/web/utils/useDatabase.ts (1)
packages/supabase/admin.ts (1)
supabaseAdmin(4-7)
apps/web/pages/pages/[page_id]/roadmap/new.tsx (2)
apps/web/pages/pages/[page_id]/index.tsx (1)
getServerSideProps(49-75)apps/web/utils/supabase/withSupabase.ts (1)
withSupabase(11-32)
apps/web/utils/useUser.tsx (4)
apps/web/data/user.interface.ts (1)
IBillingInfo(9-31)packages/supabase/types/index.ts (1)
Database(9-836)apps/web/utils/supabase/client.ts (1)
createClient(4-15)apps/web/data/routes.data.ts (1)
ROUTES(1-26)
🔇 Additional comments (6)
apps/web/pages/pages/[page_id]/roadmap/new.tsx (1)
10-14: Param parsing looks good.Guarding against missing/array
page_idis correct and avoids invalid SSR inputs.apps/web/utils/useUser.tsx (2)
48-55: LGTM: lazy client init and nullable billing state.Using a lazy initializer for Supabase and typing billingDetails as nullable avoids SSR pitfalls and matches runtime usage.
107-119: LGTM: post-login identify and billing fetch.Identifying after user is set and fetching billing once is consistent; loading guards are reasonable.
apps/web/utils/useDatabase.ts (1)
333-334: No action needed: all getPageAnalytics calls pass a numeric range. Confirmed thatgetServerSidePropsconvertsctx.query.rangeto a Number (default 7) and the sole call inanalytics.tsxuses this numericrange.apps/web/pages/pages/[page_id]/roadmap/index.tsx (2)
4-4: LGTM: type-only JSX importType-only import avoids affecting the runtime bundle. Looks good.
9-9: LGTM: SSR wrapper adoptionUsing withSupabase here keeps auth/session handling consistent across pages.
| await supabase.from("posts").update(newPost).match({ id: post_id }); | ||
|
|
||
| await supabase.from("page_audit_logs").insert({ | ||
| page_id: page_id, | ||
| page_id: String(page_id), | ||
| actor_id: user.id, |
There was a problem hiding this comment.
Do not ignore update errors; constrain by page_id.
Without throwOnError/checking, failures are silently ignored; also add page_id filter.
- await supabase.from("posts").update(newPost).match({ id: post_id });
+ await supabase
+ .from("posts")
+ .update(newPost)
+ .eq("id", post_id)
+ .eq("page_id", page_id)
+ .throwOnError();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await supabase.from("posts").update(newPost).match({ id: post_id }); | |
| await supabase.from("page_audit_logs").insert({ | |
| page_id: page_id, | |
| page_id: String(page_id), | |
| actor_id: user.id, | |
| // Constrain updates by both post ID and page ID, and throw on error instead of silently ignoring | |
| await supabase | |
| .from("posts") | |
| .update(newPost) | |
| .eq("id", post_id) | |
| .eq("page_id", page_id) | |
| .throwOnError(); | |
| await supabase.from("page_audit_logs").insert({ | |
| page_id: String(page_id), | |
| actor_id: user.id, |
🤖 Prompt for AI Agents
In apps/web/pages/pages/[page_id]/[post_id].tsx around lines 68 to 72, the
supabase update call ignores errors and only matches by post id; change the
update to constrain by both post_id and page_id (e.g., .match({ id: post_id,
page_id: String(page_id) }) or .eq for each) and surface failures by using
throwOnError() or capturing the result ({ data, error }) and handling/throwing
the error before proceeding to the insert; ensure you only run the audit insert
after a successful update.
| export const getServerSideProps = withSupabase(async (ctx, { supabase }) => { | ||
| const { page_id } = ctx.params; | ||
| if (!page_id || Array.isArray(page_id)) { | ||
| return { notFound: true }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard ctx.params before destructuring to avoid runtime/TS issues
ctx.params is typed as optional in Next. Destructuring it directly can error under strict types or on edge cases. Guard first, then narrow.
Apply this diff:
-export const getServerSideProps = withSupabase(async (ctx, { supabase }) => {
- const { page_id } = ctx.params;
- if (!page_id || Array.isArray(page_id)) {
+export const getServerSideProps = withSupabase(async (ctx, { supabase }) => {
+ const page_id = ctx.params?.page_id;
+ if (!page_id || Array.isArray(page_id)) {
return { notFound: true };
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const getServerSideProps = withSupabase(async (ctx, { supabase }) => { | |
| const { page_id } = ctx.params; | |
| if (!page_id || Array.isArray(page_id)) { | |
| return { notFound: true }; | |
| } | |
| export const getServerSideProps = withSupabase(async (ctx, { supabase }) => { | |
| const page_id = ctx.params?.page_id; | |
| if (!page_id || Array.isArray(page_id)) { | |
| return { notFound: true }; | |
| } |
🤖 Prompt for AI Agents
In apps/web/pages/pages/[page_id]/roadmap/index.tsx around lines 12 to 16, avoid
destructuring ctx.params directly because ctx.params is optional; first check
that ctx.params exists and is the right shape, e.g. assign const params =
ctx.params; then validate params.page_id (ensure it exists and is not an array)
and return { notFound: true } if invalid; only after those guards destructure or
read page_id and continue with the rest of the function.
| export const getServerSideProps = withSupabase(async (ctx, { supabase }) => { | ||
| const { page_id } = ctx.params; | ||
| if (!page_id || Array.isArray(page_id)) { | ||
| return { notFound: true }; | ||
| } | ||
|
|
||
| const { supabase } = await getSupabaseServerClient({ req, res }); | ||
| const page = await getPage(supabase, page_id); | ||
| const settings = await createOrRetrievePageSettings(String(page_id)); | ||
| const settings = await createOrRetrievePageSettings(page_id); | ||
|
|
||
| return { |
There was a problem hiding this comment.
Enforce SSR ownership and 404 on missing page.
Avoid leaking settings to non-owners and handle missing pages.
-export const getServerSideProps = withSupabase(async (ctx, { supabase }) => {
+export const getServerSideProps = withSupabase(async (ctx, { supabase, user }) => {
const { page_id } = ctx.params;
if (!page_id || Array.isArray(page_id)) {
return { notFound: true };
}
- const page = await getPage(supabase, page_id);
+ const page = await getPage(supabase, page_id).catch(() => null);
+ if (!page || page.user_id !== user.id) {
+ return { notFound: true };
+ }
const settings = await createOrRetrievePageSettings(page_id);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const getServerSideProps = withSupabase(async (ctx, { supabase }) => { | |
| const { page_id } = ctx.params; | |
| if (!page_id || Array.isArray(page_id)) { | |
| return { notFound: true }; | |
| } | |
| const { supabase } = await getSupabaseServerClient({ req, res }); | |
| const page = await getPage(supabase, page_id); | |
| const settings = await createOrRetrievePageSettings(String(page_id)); | |
| const settings = await createOrRetrievePageSettings(page_id); | |
| return { | |
| export const getServerSideProps = withSupabase(async (ctx, { supabase, user }) => { | |
| const { page_id } = ctx.params; | |
| if (!page_id || Array.isArray(page_id)) { | |
| return { notFound: true }; | |
| } | |
| const page = await getPage(supabase, page_id).catch(() => null); | |
| if (!page || page.user_id !== user.id) { | |
| return { notFound: true }; | |
| } | |
| const settings = await createOrRetrievePageSettings(page_id); | |
| return { | |
| props: { | |
| // …existing props… | |
| }, | |
| }; | |
| }); |
| const UserContext = createContext<{ | ||
| loading: boolean; | ||
| session: Session | null; | ||
| user: User | null; | ||
| billingDetails: IBillingInfo; | ||
| fetchBilling: () => void; | ||
| fetchUser: () => Promise<IUser>; | ||
| signOut: () => Promise<{ error: Error | null }>; | ||
| billingDetails: IBillingInfo | null; | ||
| fetchBilling: () => Promise<IBillingInfo | null>; | ||
| signOut: () => Promise<{ error: AuthError | null }>; | ||
| supabase: SupabaseClient<Database>; | ||
| }>({ | ||
| loading: true, | ||
| session: null, | ||
| user: null, | ||
| billingDetails: null, | ||
| fetchBilling: () => null, | ||
| fetchUser: () => null, | ||
| signOut: () => null, | ||
| supabase: null, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix Context typing, remove ts-ignore, and align the hook (use undefined default).
Current default value violates the declared types (functions returning Promise vs. null; supabase typed non-null but set to null) and forces the ts-ignore. Use a properly typed value and undefined default to restore type-safety.
Apply:
@@
-import {
- createContext,
- useCallback,
- useContext,
- useEffect,
- useState,
-} from "react";
+import {
+ createContext,
+ useCallback,
+ useContext,
+ useEffect,
+ useMemo,
+ useState,
+} from "react";
@@
-const UserContext = createContext<{
- loading: boolean;
- session: Session | null;
- user: User | null;
- billingDetails: IBillingInfo | null;
- fetchBilling: () => Promise<IBillingInfo | null>;
- signOut: () => Promise<{ error: AuthError | null }>;
- supabase: SupabaseClient<Database>;
-}>({
- loading: true,
- session: null,
- user: null,
- billingDetails: null,
- fetchBilling: () => null,
- signOut: () => null,
- supabase: null,
-});
+type UserContextValue = {
+ loading: boolean;
+ session: Session | null;
+ user: User | null;
+ billingDetails: IBillingInfo | null;
+ fetchBilling: () => Promise<IBillingInfo | null>;
+ signOut: () => Promise<{ error: AuthError | null }>;
+ supabase: SupabaseClient<Database>;
+};
+
+const UserContext = createContext<UserContextValue | undefined>(undefined);
@@
-const value = {
+const value: UserContextValue = {
loading,
session,
user,
billingDetails,
fetchBilling,
signOut,
supabase,
};
@@
-// @ts-ignore
return <UserContext.Provider value={value}>{children}</UserContext.Provider>;
@@
-export const useUserData = () => {
+export const useUserData = (): UserContextValue => {
const context = useContext(UserContext);
if (context === undefined) {
throw new Error(`useUserData must be used within a UserContextProvider.`);
}
return context;
};Also applies to: 131-132, 135-141
| // Listen for auth changes | ||
| const { | ||
| data: { subscription }, | ||
| } = supabase.auth.onAuthStateChange(async (_, session) => { | ||
| setSession(session); | ||
| setUser(session?.user ?? null); | ||
| setLoading(false); | ||
| }); | ||
|
|
||
| return () => subscription.unsubscribe(); | ||
| }, [supabase]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle cross-tab sign-out/token revocation in onAuthStateChange.
If the user signs out elsewhere, PostHog identity and billing remain stale. Reset on SIGNED_OUT and redirect home; add router to deps.
-const {
- data: { subscription },
-} = supabase.auth.onAuthStateChange(async (_, session) => {
+const {
+ data: { subscription },
+} = supabase.auth.onAuthStateChange(async (event, session) => {
setSession(session);
setUser(session?.user ?? null);
setLoading(false);
+ if (event === "SIGNED_OUT") {
+ setBillingDetails(null);
+ posthog.reset();
+ await router.replace(ROUTES.HOME);
+ }
});
@@
-}, [supabase]);
+}, [supabase, router]);Also applies to: 82-106
🤖 Prompt for AI Agents
In apps/web/utils/useUser.tsx around lines 82 to 106, the supabase
onAuthStateChange handler doesn't handle cross-tab sign-out/token revocation:
update the callback to inspect the event, and when event === "SIGNED_OUT" clear
PostHog identity (reset identify/anonymous id) and reset any in-memory
billing/user state, call setSession(null) and setUser(null) (or equivalent),
then redirect to the home page via next/router (inject/use router and add it to
the effect deps). Also ensure the effect cleanup still unsubscribes the
subscription and add router to the dependency array.
1 participant
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores