qa: Playwright suite Phase 1+2, Svelte 5 fixes, Trello sync - #13
Open
jeremie0342 wants to merge 19 commits into
Open
qa: Playwright suite Phase 1+2, Svelte 5 fixes, Trello sync#13jeremie0342 wants to merge 19 commits into
jeremie0342 wants to merge 19 commits into
Conversation
Two related races made deep-links + moderation flows unreliable:
1. Auth-check race on direct navigation. `onMount(() => { if (!auth.isAuthenticated) goto('/auth/login') })` fires before +layout.svelte's `$effect` migrates `data.user` into the auth store — the store starts `null`, so any deep-link (bookmark, email link, refresh) kicked authenticated admins back to login. `hooks.server.ts` already 303-redirects unauthenticated users, so the client check is dead code — removed from +layout.svelte and 7 pages (users/[id], tenants, tenants/[id], enterprise-kyc, operations, sponsored-challenges, tournaments).
2. Field-name mismatch on /users. The list card read `user.banned` while the backend returns `is_banned` — the ban badge and "Débannir" button never appeared post-ban. Renamed `UserSummary.banned` → `is_banned` in the API client type and updated all usages. As part of the same fix, replaced the in-place `user.banned = true` mutation in `confirmBan`/`unban` with `await loadUsers()`; the mutation didn't reliably re-render the `{#if user.banned}` action-button block in Svelte 5.
3. Applied the same refetch-instead-of-mutate pattern preventively to challenges (publish/archive) — same shape of bug waiting to happen.
…cal paths Split the Playwright suite into two projects: - `public` — anonymous specs (auth-redirect, auth-pages, admin-back-e2e) - `admin` — authenticated specs that reuse a storageState built by global-setup global-setup logs in via the API (POST /auth/login with a TOTP code computed from the persisted admin secret) rather than driving the UI — faster, more stable, and unaffected by Svelte hydration timing quirks. First run requires `node e2e/setup/bootstrap-admin.mjs` to register the admin, elevate it via SQL, and enable 2FA. Phase 1 (nav-smoke) covers all 17 admin routes with a shared data-driven test. Phase 2 adds 9 critical flows: - login-2fa (UI end-to-end with 2FA challenge) - user ban + unban (moderation) - challenge create → publish → archive (lifecycle) - reset-2fa (UI regression guard + API E2E — UI is blocked pending backend fix) - reports resolve + dismiss - community approve + reject - sponsored decide (approve/reject via modal) - kyc approve + reject - sso revoke (regression guard + API E2E — list UI blocked pending backend fix) - fraud mark-valid + revoke New deps: `otpauth` (TOTP code generation, zero runtime deps), `pg` already present. Test data is seeded via direct SQL through a shared `e2e/setup/db.ts` helper to bypass the 5/h auth/register rate limit and avoid Trello-like side-effects on staging.
qa/ holds the source-of-truth for cross-team QA work: - AUDIT_ADMIN.md / AUDIT_BACKEND.md / AUDIT_MAPPING.md — one-shot audit of the front's API surface vs backend routes - AUDIT_COVERAGE.md — running checklist of what Playwright covers - BUGS_FRONT.md / BUGS_BACK.md — bug tracker per team (open + fixed) - TODO_ADMIN.md / TODO_BACKEND.md — planned implementations per team - README.md — how to use, board URL, workflow push-to-trello.py mirrors these markdown files into a shared Trello board so the backend team sees their bugs/todos alongside ours without leaving their tooling. Idempotent (match by title, update desc + labels + list), auto-loads qa/.trello.env (gitignored), supports P0–P9 priorities and team/type labels (backend/frontend/admin × bug/implementation/other). Also ignoring e2e/setup/*.png so debug screenshots from local Playwright runs stay out of the repo.
Adds a second Playwright job that runs the `admin` project against a real backend service. Pulls `ghcr.io/skilluv/skilluv-backend:master` (published by skilluv-backend PR #33) instead of rebuilding Rust in every PR — target runtime ~2 min pull + 30s bootstrap + Playwright. Services (GHA): postgres 18 (with PGDATA subdir for the 18+ mount check), redis, mailpit. MinIO started via `docker run --network host` because GHA services don't accept a command and the minio image requires `server /data` as an arg. Backend also runs `--network host` so it reaches postgres/redis/minio/ mailpit at `localhost:<port>` and gets discovered by the runner's Node scripts at `localhost:3001`. `ADMIN_ORIGINS=http://localhost:5174` is set so the admin_gate middleware accepts the test's Origin header. Also splits the existing `e2e` job to run only `--project=public` (its implicit scope was already public smoke tests). This job will stay red until the backend PR merges + publishes the image. That's intentional — we prefer red-but-honest to skip-and-hide.
Fixes the 1 low-severity dependabot alert on master. `cookie` is a transitive dep of `@sveltejs/kit@2.70.1` (still pinning `^0.6.0` in its own manifest as of 2.70.1 latest), so we override at the workspace level to force the patched 0.7.x range. Verified: `npm ls cookie` shows 0.7.2, `npm audit` reports 0 vulnerabilities, `npm run check` + `npm test` green.
Every page that formats dates/numbers had a copy-pasted
function intlLocale() {
return i18n.locale === 'ar' ? 'ar' : i18n.locale === 'fr' ? 'fr-FR' : 'en-US';
}
7 routes + 5 admin components declared it identically. Extracted to
`src/lib/i18n/index.svelte.ts` and re-exported from `$lib/i18n`, so a
future locale bump only touches one place. Zero behavior change.
Note: the audit also surfaced ~37 real translation strings still using
`i18n.locale === 'fr' ? 'FR text' : 'EN text'` inline (sso-sessions x18,
auth/login x10, 4 shared UI components) — those bypass `ar.ts` entirely
and are tracked as a follow-up in qa/TODO_ADMIN.md (P2).
Backend-independent safety net for the shared components that E2E specs lean on the most. Complements the existing ConfirmDangerousDialog spec. Input (6 tests): label/id association, password type default, "show password" toggle presence + aria-label flip + type switch, error alert with aria-describedby, hint hidden when error is set. Modal (7 tests): open=false renders nothing, open=true exposes role=dialog + aria-modal + aria-label from title, close (X) button triggers onclose, Escape triggers onclose, backdrop click closes but inner content click doesn't, no header when title omitted, actions snippet rendered in footer. Select (7 tests): current value label in trigger, placeholder fallback, opens listbox on click with aria-selected on current, onchange fires with new value, searchable filter narrows options, disabled blocks open, Escape closes listbox.
…18n.t The `grep -c "i18n.locale ===" src/` count went from 37 to 0. All strings that were previously hardcoded fr/en pairs now flow through the standard `i18n.t()` machinery so ar.ts is populated. New sub-namespaces under `admin.*` (mirrored in fr.ts + en.ts + ar.ts + types.ts): - admin.sso (18 keys) — sso-sessions page - admin.loginPage (10 keys) — auth/login page - admin.levelUp / admin.multiSelect / admin.replayPlayer / admin.shareButton — the 4 shared UI components that still had inline ternaries User-visible impact for arabic locale: SSO sessions page, admin login, share menu, multiselect chips, replay player timings and level-up modal now actually render in Arabic instead of falling back to English.
`createApiClient` now catches network-level fetch failures (backend down, DNS, CORS preflight) and flips a global `backendStatus.isDown` flag. HTTP responses (4xx/5xx) still surface via SkilluError as before — this new branch only handles the truly-offline case. `<BackendStatusBanner>` in the root layout subscribes to the flag and: - Shows a red top banner with a countdown until the next probe - Polls `/api/health` with exponential backoff (3→5→10→20→30→60s) - On success, fires a "reconnected" toast and disappears - Exposes a "retry now" button so the user can bypass the wait Before: a backend outage produced a stream of opaque "erreur inattendue" toasts, one per failed request. After: single persistent banner + auto- recovery. UX defensive win for prod incidents. Unit tests: 5 cases on the store (markDown/markUp idempotence, backoff schedule caps at 60s).
…lper
Every spec used to duplicate the same `new pg.Client() / connect / try /
finally / end` boilerplate and a copy of the uniq() timestamp+random.
Extracted to `withDb(fn)`, `uniq()`, and a `seedUser({ prefix, role, totpEnabled })`
helper already living at `e2e/setup/db.ts`.
Nets ~120 lines removed across 9 spec files with zero behavior change.
Future specs get a one-liner user seed instead of 15 lines of setup, and
the pg connection lifecycle is centralized.
`sponsored-challenges/+page.svelte` was 489 lines, dominated by the
decision modal (approve/reject/negotiate form). Extracted into
`src/lib/components/admin/SponsoredDecideModal.svelte` (130 lines,
self-contained) — the page drops to 421 lines and no longer owns the
form state (`action`, `adminNotes`, `showDecide` internals).
Pattern documented in qa/TODO_ADMIN.md for the 6 other pages that need
the same treatment (projects, tournaments, operations, skills, fraud,
challenges — all still > 400 lines). Notable Svelte 5 gotcha captured:
initializing local state from a prop only captures the initial value —
use a `$effect(() => { if (open) local = prop })` to re-sync on
(re-)open.
… projects)
Follow-up to the sponsored-challenges extraction. Same pattern applied:
each big form-in-a-modal lives in `src/lib/components/admin/` with its
own state and prop-driven mode selection; the parent page keeps only
`open` / `editing` / `submitting` and a submit callback.
Line counts (page shrinkage after extraction):
- skills: 559 → 277 (SkillFormModal 277 — unified create+edit
via discriminated union `mode`)
- challenges: 411 → 185 (ChallengeFormModal 253)
- projects: 602 → 332 (ProjectFormModal 315)
- sponsored-challenges: (already done in the prior commit)
Net: ~800 lines removed from the 3 pages, ~845 lines added as 3 focused
components — no behavior change, contract-preserving (null vs undefined
in optional fields kept exactly as the pre-refactor code sent).
Notable Svelte 5 idioms:
- SkillFormModal uses `mode: {kind:'create'} | {kind:'edit', target}` so
every property that differs between the two flows (slug immutability,
clearParent checkbox) is expressed via one discriminated union rather
than parallel boolean props.
- All three re-seed local `$state` fields inside `$effect(() => { if (open) … })`
because Svelte 5's state initializers only read a prop's initial value.
Not extracted intentionally: tournaments / operations / fraud — those
only have `<ConfirmDangerousDialog>` (already a shared component); their
line count comes from tabbed sections + business logic, a different
refactor pattern documented as a follow-up.
Both backend routes existed since Phase-B (variant is IA-C.1, deep-scan is
IA-B) but had no UI trigger — admin had to curl them. Now surfaced:
- `<ChallengeVariantDialog>` : new "Générer variante IA" button on any
published challenge row. Modal picks harder|easier + optional prompt
hint. On submit hits `POST /admin/challenges/{id}/variant`.
- Deep-scan card in `/fraud` under the eval tab, next to LLM-evaluate.
Reuses the existing deliverable-id input + threshold/window sliders.
Renders similarity_score + verdict + comparison_pool_size in a dl.
Added `adminApi.generateChallengeVariant()` + `adminApi.deepScanDeliverable()`
in `$lib/api/admin.ts`. New i18n keys under `admin.variant.*` and
`admin.deepScan.*` in fr/en/ar (typed via `types.ts`).
…ct changes
Backend shipped breaking payload changes (see
skilluv-backend/.trello-push-front.md). Admin doesn't currently call any
of these 4 methods (users manage their own 2FA on the public frontend),
but the auth client is imported here and the wrong signatures would
silently rot until someone tried to expose an admin self-settings page.
Alignment now:
- `totpDisable(code)` → `totpDisable(password, code)` — BE-P0-02
requires both to prevent stolen-session 2FA drop.
- `enableEmail2fa()` → `enableEmail2fa(password)` — BE-P0-03 mirrors the
disable flow.
- `disableEmail2fa(currentPassword)` — was posting the old
`ChangePasswordRequest` shape with a `new_password` filler; new
backend struct is `PasswordConfirmRequest { password }`.
- `deleteAccount(password, totpCode?, reason?)` — BE-P0-01 response is
now `{ account_deleted, scheduled_for, message }` (was
`MessageResponse`). Signature grew a `reason` for the audit trail.
All four have jsdoc pointers to the corresponding BE-P0-XX cards.
Zero admin callers today so no consumer code needs touching.
Backend is live at https://api.skill-uv.com. Wire vite.config.ts to read the proxy target from `VITE_API_PROXY_TARGET` (defaults to the prod host when the env var is missing) so a fresh clone talks to real staging out of the box. Devs running the Rust backend locally just set VITE_API_PROXY_TARGET=http://localhost:3001 in their `.env`. `.env.example` documents both modes side-by-side. `.env` itself stays gitignored — a local copy pointing at prod ships alongside this commit for the developer machine.
Prefixing "✅ (fait)" onto the entry titles broke the by-title match in push-to-trello.py — every done item created a zombie card in Backlog while the original stayed there. Statut line alone is enough to move the card to Fait; the checkmark now lives in the body instead.
…it 4e857ad)
GET /admin/users/{id} now exposes totp_enabled, email_2fa_enabled, and
webauthn_credentials_count. Split the single '2FA' badge into three
distinct badges and derive targetHasStrongFactor from TOTP OR passkey
so the reset-2FA button reflects the backend rule accurately (admin_gate
accepts either strong factor).
Backend PR fix/dockerfile-seeds-and-admin-bugs shipped:
- aa5e79b: /admin/sso/sessions returns standard {data: T[]} envelope
- 4e857ad: /admin/users/{id} exposes totp_enabled + webauthn_credentials_count
- d96bdb8: /admin/community/{id}/approve pre-checks business rule, returns 400
reset-2fa.spec: was a disabled-button regression guard, now drives the
full UI happy path (TOTP badge, dialog, reason ≥ 8 chars, DB verifies
totp_secret nulled) + keeps a no-strong-factor guard.
sso-revoke.spec: was an empty-tbody regression guard, now seeds an SSO
session, revokes via UI, asserts revoked_at flips.
community-review.spec: adds a 400 regression guard so we notice if the
handler ever regresses to bubbling the DB check-constraint 500.
Backend team shipped fix/dockerfile-seeds-and-admin-bugs. Seven of my BUGS_BACK entries are now fixed (with commit SHAs), plus the P2 TODO for user-detail 2FA field enrichment. The two remaining P3 TODOs (list-payload convention audit, exhaustive utoipa annotation) are marked deferred with rationale for future backend follow-up.
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.
Summary
is_bannedfield-name mismatch on /users + preventive refetch pattern on challenges lifecycleadminproject with a shared authenticated storageState (API-driven login in global-setup). 10 spec files covering Phase 1 nav-smoke (17 routes) and Phase 2 critical flows (login-2fa, user ban/unban, challenge lifecycle, reset-2fa, reports, community, sponsored, kyc, sso, fraud)qa/folder holds bugs/todos markdown +push-to-trello.pymirroring them into a shared board so the backend team sees P0/P1 issues alongside adminBugs surfaced by the new suite (all filed in qa/BUGS_BACK.md + Trello)
admin_gatemiddleware (digest, github/sync, accounting export)GET /admin/users/{id}omitstotp_enabled(breaks reset-2FA UI)GET /admin/sso/sessionsreturns{data:{sessions:[]}}instead of{data:[]}(SSO list always empty in UI)POST /admin/community/{id}/approve500s when the community challenge has nois_training/project_id(DB check constraint)/statusvs/activateendpoint mismatchDELETEvsPOST /archivemismatch,GET /admin/users/{id}also omitsemail_2fa_enabledTrello board
https://trello.com/b/DgCwxpV7/skilluv-qa-bugs-admin (21 cards synced)
Test plan
npm run check— 0 errorsnpm test— 76/76 vitestnode e2e/setup/bootstrap-admin.mjs(one-off, needs backend on :3001)npx playwright test --project=public— 21 testsnpx playwright test --project=admin— 20 tests (needs backend + fresh DB)python qa/push-to-trello.py— idempotent, shows=for all existing cards