Repository navigation
Add admin tables' missing columns and fix QA round 3 UI nits (T19: DB-01, bug 7) - #225
Merged
Merged
Conversation
…: DB-01, bug 7) Production's rate_limits has no updated_at, so the admin function's Set limit returned 500. A new additive migration adds it (backfilled from created_at only when missing), the same for global_prompts, and the unique keys both admin upserts infer, skipping a key with a warning when duplicates would block it. No Edge Function changes. Sparring Start shows a spinner and what it is waiting for, sends one quota check however often it is pressed, and never stays spinning. The setup says the whole round costs N AI uses. The radar chart translates its axis. Ask's user bubble takes the bubble's color instead of prose gray (5.1:1 light, 6.7:1 dark) and its header wraps at 375 px in Hungarian. The model picker field shows only the model name; an Auto description sits below. Promo Pro in Settings says where Pro came from and that billing is optional until it ends. Sign out on Settings no longer makes Settings the next sign-in's landing page. Hungarian History says Beszélgetések, not Chatek. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e shifts nothing Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedEnable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Next included review available in 59 minutes. View limit details
Note 🎁 Summarized by CodeRabbit Free
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Batch T19: DB-01 (admin Set limit 500 on production) and the QA round 3 UI nits (bug 7). One PR, scoped to this batch.
To apply and deploy
supabase/migrations/20261023120000_reconcile_admin_table_drift.sql. It's additive and safe to rerun. Not applied by me.admin/index.tsis untouched, so nothing to deploy. The migration makes the existing function's writes work.DB-01: rate_limits.updated_at
Production's
public.rate_limitspredates20260204000000_admin_improvements.sql, whoseCREATE TABLE IF NOT EXISTSskipped it, so it has noupdated_at. The admin function writesupdated_aton Set limit and orders global limits by it.20261022180000had moved the readers tocreated_atand chose not to add the column; this batch asked for the column, so the new migration adds it instead of changing the function.The migration checks the catalog before each step:
rate_limits.updated_atandglobal_prompts.updated_at: added only when missing, then backfilled fromcreated_at. Existing values on local and staging are left alone.global_prompts.is_activeandcreated_by(Set prompt writes both):ADD COLUMN IF NOT EXISTS.rate_limits (user_id, limit_type)andglobal_prompts (feature): created only when no unique index on exactly those columns exists. If duplicate rows would block a key, it raises a warning and skips it; nothing is deleted.BEFORE UPDATEtrigger onrate_limitsusing the existingpublic.set_updated_at().Drift survey of every table
admin/index.tstouches:rate_limits: missingupdated_aton production (known). Fixed here.global_prompts: created by the same skipped migration, so it may be legacy too; Set prompt writesupdated_at,is_activeandcreated_byand upserts onfeature. Guarded here. Production's actual shape is unverified (no production access).user_roles: same drift, already fixed by20260907155821_reconcile_user_roles_updated_at.sql.demo_access_codes: written from a production dump (20260820070000); every column the admin reads is there.admin_announcements,promo_codes,subscriptions: created by plainCREATE TABLEin repo migrations, so the shape matches.ai_requestsanduser_settings: their admin-read columns are used by every function on production.admin_grants:CREATE TABLE IF NOT EXISTS, but the admin only inserts columns the migration defines, and a failed audit insert is only a warning.Proof:
scripts/test-admin-table-drift-migration.sh(now part ofnpm run test:db): rebuilds the legacy shape in a rolled-back transaction, runs the migration with a duplicate pair (columns land, key skipped, no row deleted), resolves the duplicate, reruns (key created), runs the exact Set limit (per-user upsert, global update) and Set prompt SQL, then reruns again and checks no row changed.POST admin?action=set-limitreturned 500. After applying the migration it returned 200.e2e/full-stack/admin.spec.ts› "an admin sets a per-user and a global rate limit through the admin function": both Set limit paths twice, one row each, values updated../.superset/db-gate.sh --db-only: pgTAPResult: PASS, and everytest:dbscript passed, the new one included.Bug 7 UI items
aria-busy, and a status line ("Checking your AI uses for today…") for the quota check, which can take up to 8 s. A ref guard means a second click or Enter sends no second check. try/finally means the button can't stay spinning, and a failed check is treated as unknown (the round opens, as on a timeout). If opening the round throws, a visible error replaces the silent no-op. Tests:sparring-quota-start.test.tsx(pending state, one check for repeated presses, failed check) andrecipes/practice.spec.ts›spar-start(the quota check held in the real app; one check, round opens without a reload).PricingModalis a static import inside the Settings chunk, andinstallStaleChunkReloadplusRouteErrorBoundaryalready reload once and then offer a Reload button. So I didn't add a second reload prompt. Addedaria-haspopup="dialog"to the button. Promo copy: Pro without a Stripe customer now says "Your Pro access comes from a promo code and runs until {date}. Billing is optional until then; set it up only to keep Pro after that date." (or "…so billing is optional." for a lifetime promo), in EN, ES and HU. Covered bySubscriptionStatus.test.tsxand a newpromo-prostate inrecipes/billing-states.spec.ts(EN and HU, light and dark, 360, 375, 768 and 1280 px).proseclass painted its own gray text over the bubble's colour. The user's message, its bold text, and its time line now take the bubble's foreground. Measured in the real app: 5.08:1 light, 6.72:1 dark (AA needs 4.5). By my token math it was about 1.9:1 light and 1.4:1 dark before. The HU placeholder was already shortened by T13 (Unify translations, plurals and number/date formats (T13: IN-02, IN-05, IN-06, IN-08, IN-09, IN-10, IN-12, IN-13, IN-15) #223) to "Mit kérdeznél?", and the recipe confirms it fits at 375 px. The drive also found the chat header overflowing by 48 px at 375 px in HU; it now wraps. Recipe:recipes/ai-tools.spec.ts›ask-bubble.spar-verdict. History "Chatek" and related chat wording is now "Beszélgetések". Already fixed on main by T13 (Unify translations, plurals and number/date formats (T13: IN-02, IN-05, IN-06, IN-08, IN-09, IN-10, IN-12, IN-13, IN-15) #223): the format description "bevezető, R1, R2 és záró", "Az írásstílust", Kíméletlen everywhere (it's the glossary term), and the Load/Loading split (Megnyitás / Betöltés). Left as is: "Cáfolat 1/2" in the Timer vs "R1/R2" in Sparring is the glossary rule T13 added ("the Timer spells out R1 and R2"; EN does the same). ES Introductor/Conclusor are the Spanish role names, per the glossary.recipes/model-defaults.spec.ts›model-picker-narrow(HU, 375 px, nothing clipped).RequireAuthstampedfrom: /app/settingswith the same user id, so that account signing back in returned to Settings. A deliberate sign-out now sets a flag (src/lib/supabase/sign-out-intent.ts) so the redirect carries no return path, and LoginPage clears the flag on arrival. An expired session still returns the account to its page, and explicit deep links still work. Tests:account-switch-route.test.tsx(deliberate sign-out →/app; self-ended session keeps Settings; a later deep link keeps its page) andrecipes/auth.spec.ts›auth-same-account-after-sign-out(email code through Mailpit, lands on/app).Verify
Full
npm run verifyrun locally, once, atb2e70927(exit 0): docs:check, lint and typecheck passed;npm test88 files, 640 tests passed; test:collaboration 30 passed; test:edge 457 passed, 0 failed; build passed; test:sw passed (all offline checks true); e2e smoke (desktop Chromium,@smoke) 47 passed.Other checks at this head:
npm run lint,npm run typecheck,npm test,npm run test:edge(457 passed),npm run test:collaboration(30 passed), and./.superset/db-gate.sh --db-only(pgTAP PASS plus alltest:dbscripts).verify-debatelab recipes driven on desktop-chromium and mobile-chromium (fake mode):
practice(spar-),ai-tools(ask-),model-defaults(all),auth(all, 12/12),billing-states(all 14, one desktop test rerun after a Vite dev-server stale-module error), ande2e/full-stack/admin. GitHub Actions minutes are used up, so this is a local pass, not CI.Screenshots are in
.verify/artifacts/locally (not attached). Covered: Sparring setup and Start pending (EN and HU, 375 and 1280 px), the Ask bubble light and dark, HU chart labels, the Generate picker in HU at 375 px, promo-Pro Settings (EN and HU, four widths), and the post-sign-in landing.Review
One fresh-context review round (Claude Opus 5.5, full diff against main): no High or Medium findings. Per the deadline, no further rounds. The Low findings were not fixed and are listed for follow-up:
(user_id, limit_type)rate_limits rows or duplicatefeatureglobal_prompts rows, the migration only warns and skips the unique key, and per-user Set limit or Set prompt would still 500 (no constraint matching ON CONFLICT). Before applying, run read-only on production:SELECT user_id, limit_type, count(*) FROM public.rate_limits WHERE user_id IS NOT NULL GROUP BY 1, 2 HAVING count(*) > 1;andSELECT feature, count(*) FROM public.global_prompts GROUP BY 1 HAVING count(*) > 1;. After applying, confirm both unique indexes exist. Also suggested: requireindisvalidin the key check, and addSET lock_timeout.sparring-quota-start.test.tsx, the "one check however often Start is pressed" test also passes withoutstartingRef: the disabled button already blocks the second press. The "check fails outright" test is a real regression test.e2e/full-stack/admin.spec.tsSet limit test passes on main too, because the local schema is already correct. DB-01 is proven byscripts/test-admin-table-drift-migration.shand by the manual 500 → 200 reproduction above, not by that test.global_prompts.is_activeandcreated_by, so thoseADD COLUMNpaths are not exercised against the legacy shape.supabase.auth.signOutreturns{ error }without throwing, the sign-out flag stays set while the user stays signed in. A later expiry would then drop the return path. Suggested: check the returned error, or mark the intent only after sign-out succeeds.Not done or open
global_promptsshape is unverified. The migration handles both shapes.🤖 Generated with Claude Code