Skip to content

qa: Playwright suite Phase 1+2, Svelte 5 fixes, Trello sync - #13

Open
jeremie0342 wants to merge 19 commits into
masterfrom
qa/playwright-suite-and-trello-sync
Open

qa: Playwright suite Phase 1+2, Svelte 5 fixes, Trello sync#13
jeremie0342 wants to merge 19 commits into
masterfrom
qa/playwright-suite-and-trello-sync

Conversation

@jeremie0342

Copy link
Copy Markdown
Collaborator

Summary

  • fix(admin) — Svelte 5 hydration race (deep-link redirected authed admins to /auth/login on 7 pages) + is_banned field-name mismatch on /users + preventive refetch pattern on challenges lifecycle
  • test(e2e) — new Playwright admin project 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)
  • chore(qa)qa/ folder holds bugs/todos markdown + push-to-trello.py mirroring them into a shared board so the backend team sees P0/P1 issues alongside admin

Bugs surfaced by the new suite (all filed in qa/BUGS_BACK.md + Trello)

  • [P1] Routes admin hors admin_gate middleware (digest, github/sync, accounting export)
  • [P1] GET /admin/users/{id} omits totp_enabled (breaks reset-2FA UI)
  • [P1] GET /admin/sso/sessions returns {data:{sessions:[]}} instead of {data:[]} (SSO list always empty in UI)
  • [P1] POST /admin/community/{id}/approve 500s when the community challenge has no is_training/project_id (DB check constraint)
  • [P1] Seasons /status vs /activate endpoint mismatch
  • [P2] Projects DELETE vs POST /archive mismatch, GET /admin/users/{id} also omits email_2fa_enabled

Trello board

https://trello.com/b/DgCwxpV7/skilluv-qa-bugs-admin (21 cards synced)

Test plan

  • npm run check — 0 errors
  • npm test — 76/76 vitest
  • node e2e/setup/bootstrap-admin.mjs (one-off, needs backend on :3001)
  • npx playwright test --project=public — 21 tests
  • npx playwright test --project=admin — 20 tests (needs backend + fresh DB)
  • python qa/push-to-trello.py — idempotent, shows = for all existing cards

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant