fix(admin): prevent no-op tenant saves and cover settings states - #436
Conversation
Found via a systematic component-vs-story-vs-test cross-reference of frontend/src/components/*.tsx -- AdminPanel had neither a .test.tsx nor a .stories.tsx, unlike every other component in the directory. Tests cover: save disabled until the brand name actually changes, a successful save calling updateTenantConfig (backend/app/main.py's PATCH /api/settings, now covered separately in #435) with the right arguments and reporting the new name back to the caller, and a failed save showing the error while leaving the form editable (not stuck disabled). Storybook stories cover the default state and a long brand-name layout edge case. pnpm run test: 143 passed (was 140). pnpm run lint: no new warnings. pnpm run build: the two pre-existing App.tsx errors are the unrelated main break already fixed in #426, not something this PR touches.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Same shared-ancestor bug as #418/#415/#426/#427/#429/#431/#434: the login button built an unsanitized returnUrl inline instead of returnUrlFromLocation()/rememberOidcReturnUrl(), and removed the unreachable login-screen AdminPanel render (accessToken is always undefined pre-auth). This PR's own AdminPanel.test.tsx/.stories.tsx render the component directly, so this doesn't affect its coverage.
|
Acknowledged — vitest's ESM-mutable-export spying does work here, but passing a mocked dependency would be more robust if this pattern gets reused elsewhere. Not blocking this PR's own scope (adding AdminPanel coverage); worth keeping in mind for the next component test that needs to mock |
|
Acknowledged — the raw English-key assertions work because jsdom defaults |
Same shared-ancestor bug as #418/#415/#426/#427/#429/#431/#434/#435/#436: the login button built an unsanitized returnUrl inline instead of returnUrlFromLocation()/rememberOidcReturnUrl(), and removed the unreachable login-screen AdminPanel render (accessToken is always undefined pre-auth). This PR's own diff doesn't touch AdminPanel.
Same shared-ancestor bug as #418/#415/#426/#427/#429/#431/#434/#435/#436/#437: the login button built an unsanitized returnUrl inline instead of returnUrlFromLocation()/rememberOidcReturnUrl(), and removed the unreachable login-screen AdminPanel render (accessToken is always undefined pre-auth). This PR's own diff doesn't touch AdminPanel.
|
Confirmed, thanks — no action needed. |
|
Confirmed — the removed login-screen AdminPanel render was unreachable through normal navigation, matching #426's approach. |
* test(frontend): add Storybook coverage for BuyerNav BuyerNav had a test file but no story, the last remaining gap in frontend/src/components/*.tsx test+story coverage. Adds stories for each destination plus an edge case with an extra tools slot. * fix(frontend): use OIDC return-url helpers on the login button Same shared-ancestor bug as #418/#415/#426/#427/#429/#431/#434/#435/#436/#437: the login button built an unsanitized returnUrl inline instead of returnUrlFromLocation()/rememberOidcReturnUrl(), and removed the unreachable login-screen AdminPanel render (accessToken is always undefined pre-auth). This PR's own diff doesn't touch AdminPanel. * test(frontend): cover WorkspaceNav in Storybook
* test(frontend): cover LineageDag with tests and stories LineageDag renders the git-branch-style multi-thread lineage graph used by both the post-detail popup and the Ask Agent's multi-lineage answer view (ADR 0120), and had zero test or story coverage despite being a core, non-trivial component. Adds tests for the empty state, multi-group branch rendering, group-heading fallback for missing/UUID groups, click and keyboard node selection, the current-post marker, and label truncation with an accessible full-label fallback. Adds stories for empty, single-branch, multi-branch (with an actual fork), ungrouped, and long-label scenarios. * fix(frontend): use OIDC return-url helpers on the login button Same shared-ancestor bug as #418/#415/#426/#427/#429/#431/#434/#435/#436/#437/#438: the login button built an unsanitized returnUrl inline instead of returnUrlFromLocation()/rememberOidcReturnUrl(), and removed the unreachable login-screen AdminPanel render. * Revert "fix(frontend): use OIDC return-url helpers on the login button" This reverts commit 590c6c3. * fix(frontend): keep lineage edge evidence visible-only * fix(frontend): terminate rooted lineage cycles * chore(frontend): keep shared OIDC repair on #426 * refactor(frontend): remove unreachable cycle guard * test(frontend): cover converging lineage DAGs * fix(frontend): position converging DAG nodes once
* test(frontend): cover FiveW1H component with tests and stories Adds Vitest coverage for loading state, empty-evidence messaging, raw-source-to-label mapping (including the unmapped fallback), and optional evidence-text/ontology-badge rendering, plus a Storybook inventory covering the loading, all-empty, grounded-answer, and unmapped-source scenarios. * fix(frontend): use OIDC return-url helpers on the login button Same shared-ancestor bug as #418/#415/#426/#427/#429/#431/#434/#435/#436: the login button built an unsanitized returnUrl inline instead of returnUrlFromLocation()/rememberOidcReturnUrl(), and removed the unreachable login-screen AdminPanel render (accessToken is always undefined pre-auth). This PR's own diff doesn't touch AdminPanel. * chore(frontend): keep FiveW1H coverage dependency-correct Remove the duplicated OIDC/login changes owned by #426 so this PR carries only its FiveW1H tests and Storybook inventory.
* test(api): cover POST /api/ask, GET /api/rankings, PATCH /api/me/preferences Found via a systematic route-vs-test cross-reference (every @app.get/ post/patch/put/delete path in backend/app/main.py checked against every test file, not just backend/tests/test_api.py) -- same technique that found the /healthz routing bug earlier this session. All three endpoints had zero test coverage anywhere in the repo: - POST /api/ask: the Ask Agent endpoint itself was never exercised at the HTTP layer, despite its underlying functions (gather_global_chat_sources, cited_post_evidence, ...) being unit-tested. New tests cover the empty-question 422, the no-orchestrator-configured 503 (Null client, matching the existing derive-commitment 503 test's monkeypatch pattern), and the unauthenticated 401/403 case. - GET /api/rankings: covers the real response contract (RankWeave's own "never invent a fused score" fail-closed shape -- status is either "accepted" or "unavailable", never a guessed ranking) plus the unauthenticated case. - PATCH /api/me/preferences: covers persisting a supported locale (round-tripped through GET /api/me) and rejecting an unsupported one (Pydantic's own Literal validation, previously untested). uv run --frozen python -m pytest -q: 760 passed, 17 skipped. * fix(frontend): use OIDC return-url helpers on the login button Same shared-ancestor bug as #418/#415/#426/#427/#429/#431/#434/#436: the login button built an unsanitized returnUrl inline instead of returnUrlFromLocation()/rememberOidcReturnUrl(), and removed the unreachable login-screen AdminPanel render (accessToken is always undefined pre-auth). * Revert "fix(frontend): use OIDC return-url helpers on the login button" This reverts commit b80628b. * test(api): verify buyer route trust boundaries * fix: remove unused httpx2 dependency * docs: keep API test transport provenance accurate Remove stale httpx2 claims after the look-alike dependency was deleted; the harness uses Starlette TestClient with the official httpx dev dependency. * fix(frontend): repair the inherited login/admin-panel build break Two TypeScript build errors on main (blocking every open PR's "Frontend lint, test, build" check, including this repo's own review bot's ability to approve them): - App.tsx imported rememberOidcReturnUrl/returnUrlFromLocation from oidcReturnUrl.ts but never called them -- the login button built its own unsanitized returnUrl inline instead of using the safe helper (oidcReturnUrl.ts's isSafeReturnUrl guard against an open-redirect- shaped value) or persisting it as the sessionStorage/localStorage fallback restoreOidcReturnUrl (already wired up on the callback side in main.tsx) reads when the OIDC state round-trip drops it. - The unauthenticated login screen unconditionally rendered <AdminPanel accessToken={accessToken} /> when destination === "admin" -- accessToken is string | undefined here (always undefined while unauthenticated), a real type error, and the render was unreachable through normal navigation (destination only changes via the authenticated nav) -- dead code, removed. uv run --frozen python -m pytest -q: 753 passed, 17 skipped. pnpm run test: 140 passed. pnpm run lint / build: clean. * Revert "fix(frontend): repair the inherited login/admin-panel build break" This reverts commit 36164fb.
# Conflicts: # frontend/src/App.test.tsx
|
@opencode-agent Please review the exact current head. The PR is stacked on #426; do not transfer parent checks or approvals. |
4988fb7
into
worktree-fix-frontend-build-break
Buyer-visible gap
Tenant settings could be submitted through the form even when the trimmed brand name was empty or unchanged. That created a needless write outside the disabled-button path, while success and failure feedback was not announced to assistive technology.
Change
The prior App/OIDC changes are removed from this PR's effective diff and inherited from parent PR #426.
Stack boundary
worktree-fix-frontend-build-breakatc0a77731d46dba5f951cf69df6578f35aa93a2d6when validated.82ae39bdf3665b7588b242fce60780e138114576.AdminPanel.tsx, its test and story, plus the Storybook inventory.main, retarget this PR tomain, rerun exact-head checks, and obtain fresh independent approval before merge.Verification
pnpm run lint: passed.pnpm run build: passed.pnpm run build-storybook: passed.git diff --check: passed.Only synthetic names are used; no real records, identifiers, or provider credentials are included.