From 547259369b15f9bdfd2c5c16b3b9a6dc4d59d084 Mon Sep 17 00:00:00 2001 From: AnxForever <130662349+AnxForever@users.noreply.github.com> Date: Sun, 20 Sep 2026 14:54:45 +0800 Subject: [PATCH 1/2] fix(ratings): stop naming a user_id column production never had Rating writes have been failing with 503 since 1c3a4c42 (2026-09-04), which collapsed the two insert shapes into one. The legacy arm then sent `user_id: null` even after its probe found the column missing, and PostgREST rejects an insert that names an unknown column rather than ignoring it. The route's own legacy fallback could never have worked. Production's style_ratings table never received migration 003's user_id half, so every rating lives under the `session_id = "user:"` identity. The insert now branches again, with an explicit payload type so both arms stay assignable without the union that motivated the collapse. Migration 041 applies the missing column and folds the existing rows onto it. The client also stops discarding the route's already user-safe error message, which is what kept a silent 16-day outage looking like a generic failure. --- .../[slug]/rate/__tests__/route.test.ts | 75 +++++++++++++++++++ app/api/styles/[slug]/rate/route.ts | 45 ++++++++--- components/styles/style-rating.tsx | 10 ++- .../041_style_ratings_user_binding.sql | 43 +++++++++++ 4 files changed, 159 insertions(+), 14 deletions(-) create mode 100644 lib/supabase/migrations/041_style_ratings_user_binding.sql diff --git a/app/api/styles/[slug]/rate/__tests__/route.test.ts b/app/api/styles/[slug]/rate/__tests__/route.test.ts index 82102b542..2ddabeee0 100644 --- a/app/api/styles/[slug]/rate/__tests__/route.test.ts +++ b/app/api/styles/[slug]/rate/__tests__/route.test.ts @@ -159,6 +159,81 @@ describe("styles rating route", () => { }); }); + it("POST omits user_id when the column is missing and writes the legacy session identity", async () => { + mockedVerifyTrustedOrigin.mockReturnValue({ ok: true }); + mockedGetServerUser.mockResolvedValue({ id: "user-legacy" } as never); + mockedGetRequestClientKey.mockReturnValue("ip:legacy"); + mockedCheckRateLimit.mockReturnValue({ + allowed: true, + limit: 80, + remaining: 79, + resetAt: Date.now() + 1_000, + retryAfterSec: 0, + }); + mockedParseJsonBodyWithLimit.mockResolvedValue({ + ok: true, + data: { rating: 4 }, + }); + mockedIsSupabaseConfigured.mockReturnValue(true); + + // The user_id probe fails the way Postgres reports an unknown column, which + // is what selects the legacy session identity for this write. + const userProbeMaybeSingle = vi.fn().mockResolvedValue({ + data: null, + error: { code: "42703", message: "column style_ratings.user_id does not exist" }, + }); + const userProbeSelect = { + eq: vi.fn().mockReturnValue({ + eq: vi.fn().mockReturnValue({ + maybeSingle: userProbeMaybeSingle, + }), + }), + }; + const legacyProbeMaybeSingle = vi.fn().mockResolvedValue({ data: null, error: null }); + const legacyProbeSelect = { + eq: vi.fn().mockReturnValue({ + in: vi.fn().mockReturnValue({ + maybeSingle: legacyProbeMaybeSingle, + }), + }), + }; + const insert = vi.fn().mockResolvedValue({ error: null }); + const summaryMaybeSingle = vi.fn().mockResolvedValue({ + data: { average_rating: 4, total_ratings: 1 }, + error: null, + }); + const summarySelect = { + eq: vi.fn().mockReturnValue({ + maybeSingle: summaryMaybeSingle, + }), + }; + + const from = vi + .fn() + .mockReturnValueOnce({ select: vi.fn().mockReturnValue(userProbeSelect) }) + .mockReturnValueOnce({ select: vi.fn().mockReturnValue(legacyProbeSelect) }) + .mockReturnValueOnce({ insert }) + .mockReturnValueOnce({ select: vi.fn().mockReturnValue(summarySelect) }); + mockedCreateClient.mockReturnValue({ from } as never); + + const response = await POST( + new Request("https://stylekit.top/api/styles/neo-brutalist/rate", { method: "POST" }), + { params: params("neo-brutalist") }, + ); + + expect(response.status).toBe(200); + const payload = insert.mock.calls[0][0]; + // Naming a column the database does not have makes PostgREST reject the + // whole insert, so the legacy identity must not so much as mention it. + expect(payload).not.toHaveProperty("user_id"); + expect(payload).toEqual({ + style_slug: "neo-brutalist", + rating: 4, + session_id: "user:user-legacy", + ip_address: null, + }); + }); + it("POST returns DB_SCHEMA_MISMATCH when legacy session_id not-null constraint blocks writes", async () => { mockedVerifyTrustedOrigin.mockReturnValue({ ok: true }); mockedGetServerUser.mockResolvedValue({ id: "user-3" } as never); diff --git a/app/api/styles/[slug]/rate/route.ts b/app/api/styles/[slug]/rate/route.ts index 2a759c500..bca6140a5 100644 --- a/app/api/styles/[slug]/rate/route.ts +++ b/app/api/styles/[slug]/rate/route.ts @@ -34,6 +34,20 @@ interface UserRatingRow { created_at?: string | null; } +/** + * An insert that names a column the database does not have is rejected whole, + * so `user_id` is optional here: the legacy identity arm leaves it out rather + * than sending null. Declaring the shape keeps both arms assignable to one + * payload type without collapsing them into a single object literal. + */ +interface RatingInsertPayload { + style_slug: string; + rating: number; + session_id: string | null; + ip_address: string | null; + user_id?: string; +} + interface UserRatingQueryResult { data: unknown[] | null; error: DbErrorLike | null; @@ -310,18 +324,25 @@ export async function POST( ); } } else { - // Insert new rating - // One row shape for both identities. Branching the object literal made - // the two arms structurally different, and newer supabase-js typings - // reject the resulting union at the insert call. - const insertResult = await sb.from("style_ratings").insert({ - style_slug: slugParsed.data, - rating: parsed.data.rating, - session_id: useLegacySessionIdentity ? legacySessionId : null, - user_id: useLegacySessionIdentity ? null : user.id, - ip_address: ip, - }); - const { error } = insertResult; + // Insert new rating. The legacy arm writes the session identity alone: + // a database still on the pre-003 schema has no user_id column, and + // PostgREST rejects an insert that so much as names a column it does not + // have, null or not. + const payload: RatingInsertPayload = useLegacySessionIdentity + ? { + style_slug: slugParsed.data, + rating: parsed.data.rating, + session_id: legacySessionId, + ip_address: ip, + } + : { + style_slug: slugParsed.data, + rating: parsed.data.rating, + session_id: null, + user_id: user.id, + ip_address: ip, + }; + const { error } = await sb.from("style_ratings").insert(payload); if (error) { const classified = classifyDbError(error as DbErrorLike); diff --git a/components/styles/style-rating.tsx b/components/styles/style-rating.tsx index 08ac33323..0f78d9d7b 100644 --- a/components/styles/style-rating.tsx +++ b/components/styles/style-rating.tsx @@ -58,8 +58,14 @@ export function StyleRating({ slug }: StyleRatingProps) { body: JSON.stringify({ rating }), }); if (!res.ok) { - const body = await res.json().catch(() => null); - throw new Error(body?.error ?? "Failed to submit rating"); + const body = (await res.json().catch(() => null)) as { error?: string } | null; + // The route already answers with a user-safe message. Surfacing it + // keeps a rejected write diagnosable instead of looking identical to + // every other failure, which is how a schema mismatch stayed hidden. + setUserRating(0); + setError(body?.error ?? "Failed to submit rating. Please try again."); + await mutate(); + return; } await mutate(); } catch { diff --git a/lib/supabase/migrations/041_style_ratings_user_binding.sql b/lib/supabase/migrations/041_style_ratings_user_binding.sql new file mode 100644 index 000000000..d427047eb --- /dev/null +++ b/lib/supabase/migrations/041_style_ratings_user_binding.sql @@ -0,0 +1,43 @@ +-- Migration 041: bind style_ratings to user accounts +-- +-- Migration 003 declared this column, but production only ever received its +-- style_comments and submissions halves. style_ratings kept writing the legacy +-- `session_id = 'user:'` identity instead, and a database without the +-- column rejects any insert that so much as names user_id -- null or not. +-- This applies the missing half and folds the legacy rows onto the real +-- identity, so the column stops being a schema the code only pretends to have. + +ALTER TABLE public.style_ratings + ADD COLUMN IF NOT EXISTS user_id UUID REFERENCES auth.users(id); + +-- Fold the legacy `user:` session identities onto user_id. The regex +-- guard keeps a malformed suffix from aborting the cast, and the auth.users +-- check keeps a deleted account from tripping the foreign key. Rows that fail +-- either guard stay on their session identity and keep working through the +-- legacy read path. +UPDATE public.style_ratings AS r +SET user_id = (substring(r.session_id FROM 6))::uuid +WHERE r.session_id LIKE 'user:%' + AND r.user_id IS NULL + AND substring(r.session_id FROM 6) + ~ '^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$' + AND EXISTS ( + SELECT 1 FROM auth.users AS u + WHERE u.id = (substring(r.session_id FROM 6))::uuid + ); + +-- A writer holding two ratings for one style would make the unique index below +-- fail. Only the newest row per (style, user) survives, which is also the row +-- the rating read path already reports. +DELETE FROM public.style_ratings AS dup +USING public.style_ratings AS keep +WHERE dup.user_id IS NOT NULL + AND dup.user_id = keep.user_id + AND dup.style_slug = keep.style_slug + AND (dup.created_at, dup.id) < (keep.created_at, keep.id); + +CREATE UNIQUE INDEX IF NOT EXISTS style_ratings_user_slug + ON public.style_ratings(style_slug, user_id) WHERE user_id IS NOT NULL; + +CREATE INDEX IF NOT EXISTS idx_ratings_user + ON public.style_ratings(user_id) WHERE user_id IS NOT NULL; From 8e07f3282863257c3cd21bf9c74dbd7bd8712ef4 Mon Sep 17 00:00:00 2001 From: AnxForever <130662349+AnxForever@users.noreply.github.com> Date: Sun, 20 Sep 2026 14:54:47 +0800 Subject: [PATCH 2/2] fix(favorites): apply user_favorites' missing user binding Migration 034 left 003's user_favorites.user_id column unapplied on the grounds that no code referenced it. The code has referenced it since 2026-02-21 -- both the favorites API and the merge path try the user_id arm first and fall back to the session identity -- so production has served every signed-in write through the fallback. Migration 042 applies the column and folds the 370 legacy rows onto it, preserving their session_id so migration 034's RLS policies keep protecting them unchanged. Two details deliberately differ from 003 as written: the session_id NOT NULL drop is load-bearing, since the modern arm never names that column, and the unique index is not partial, because Postgres refuses to infer a partial index as the arbiter for the merge path's onConflict "user_id,style_slug" and fails it with 42P10. --- .../042_user_favorites_user_binding.sql | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 lib/supabase/migrations/042_user_favorites_user_binding.sql diff --git a/lib/supabase/migrations/042_user_favorites_user_binding.sql b/lib/supabase/migrations/042_user_favorites_user_binding.sql new file mode 100644 index 000000000..27807d24d --- /dev/null +++ b/lib/supabase/migrations/042_user_favorites_user_binding.sql @@ -0,0 +1,63 @@ +-- Migration 042: bind user_favorites to user accounts +-- +-- Migration 034 deliberately left 003's user_favorites.user_id column +-- unapplied, noting that no code referenced it. The code does reference it: +-- both the favorites API and the merge path have shipped a user_id arm with a +-- session_id fallback since 2026-02-21, so production has served every +-- signed-in favorite through the legacy `user:` identity. This applies +-- the column and folds those rows onto the real one. +-- +-- Two details deliberately differ from 003 as written: +-- +-- * The session_id NOT NULL drop is load-bearing, not cosmetic. The modern +-- arm inserts {user_id, style_slug} and never names session_id, so a +-- NOT NULL there fails the write with 23502. +-- +-- * The unique index is not partial. Postgres refuses to infer a partial +-- index as the arbiter for `onConflict: "user_id,style_slug"` and fails +-- the merge with 42P10, which no error classifier in the route treats as +-- a missing column. Dropping the predicate costs nothing here: Postgres +-- treats NULL user_id values as distinct, so anonymous rows stay +-- unconstrained either way. +-- +-- RLS is left alone on purpose. Every favorites read and write goes through +-- the service role, and 034's policies key off session_id, which the backfill +-- preserves unchanged. Rows the modern arm writes carry a NULL session_id, +-- which those policies deny to anon callers -- fail-closed rather than open. + +ALTER TABLE public.user_favorites + ADD COLUMN IF NOT EXISTS user_id UUID REFERENCES auth.users(id); + +ALTER TABLE public.user_favorites + ALTER COLUMN session_id DROP NOT NULL; + +-- Fold the legacy `user:` session identities onto user_id. The regex +-- guard keeps a malformed suffix from aborting the cast, and the auth.users +-- check keeps a deleted account from tripping the foreign key. Rows that fail +-- either guard keep working through the legacy read path. +UPDATE public.user_favorites AS f +SET user_id = (substring(f.session_id FROM 6))::uuid +WHERE f.session_id LIKE 'user:%' + AND f.user_id IS NULL + AND substring(f.session_id FROM 6) + ~ '^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$' + AND EXISTS ( + SELECT 1 FROM auth.users AS u + WHERE u.id = (substring(f.session_id FROM 6))::uuid + ); + +-- A writer holding the same style twice would make the unique index below +-- fail. Anonymous rows are untouched: they carry a NULL user_id, which never +-- participates in the comparison. +DELETE FROM public.user_favorites AS dup +USING public.user_favorites AS keep +WHERE dup.user_id IS NOT NULL + AND dup.user_id = keep.user_id + AND dup.style_slug = keep.style_slug + AND (dup.created_at, dup.id) < (keep.created_at, keep.id); + +CREATE UNIQUE INDEX IF NOT EXISTS user_favorites_user_slug + ON public.user_favorites(user_id, style_slug); + +CREATE INDEX IF NOT EXISTS idx_favorites_user + ON public.user_favorites(user_id);