Add dev/demo login selector for quick user switching #62

2026-03-19T03:26:59Z

ghost commented

2026-03-19 03:26:59 +00:00

(Migrated from github.com)

Summary

Adds a login selector page when AUTH_DISABLED=true showing staff members and clients from the database
API middleware accepts X-Dev-User-Id header to impersonate specific users (auth disabled only)
Persistent session indicator bar with "Switch user" link
Automatic redirect to login selector when no dev user is chosen
Frontend fetch interceptor injects the dev user header on all API requests

Closes #60

Test plan

With AUTH_DISABLED=true, visiting the app redirects to login selector
Staff members and clients from DB are listed on the selector
Selecting a staff user sets session and redirects to /admin
Selecting a client user sets session and redirects to /
Session indicator bar visible at bottom while impersonating
"Switch user" link returns to selector
"Continue as default dev user" skips selection
API middleware respects X-Dev-User-Id header when auth disabled
Header is ignored when auth is enabled (403 on /api/dev/users)
All unit tests pass (7/7)

🤖 Generated with Claude Code

## Summary - Adds a login selector page when `AUTH_DISABLED=true` showing staff members and clients from the database - API middleware accepts `X-Dev-User-Id` header to impersonate specific users (auth disabled only) - Persistent session indicator bar with "Switch user" link - Automatic redirect to login selector when no dev user is chosen - Frontend fetch interceptor injects the dev user header on all API requests Closes #60 ## Test plan - [ ] With `AUTH_DISABLED=true`, visiting the app redirects to login selector - [ ] Staff members and clients from DB are listed on the selector - [ ] Selecting a staff user sets session and redirects to `/admin` - [ ] Selecting a client user sets session and redirects to `/` - [ ] Session indicator bar visible at bottom while impersonating - [ ] "Switch user" link returns to selector - [ ] "Continue as default dev user" skips selection - [ ] API middleware respects `X-Dev-User-Id` header when auth disabled - [ ] Header is ignored when auth is enabled (403 on `/api/dev/users`) - [ ] All unit tests pass (7/7) 🤖 Generated with [Claude Code](https://claude.com/claude-code)

ghost commented

2026-03-19 03:33:14 +00:00

(Migrated from github.com)

E2E Test Failures — Login Redirect

6 E2E tests are failing because the dev login selector now redirects to /login when AUTH_DISABLED=true and no dev user is selected. The E2E test environment runs with auth disabled, so all tests that navigate to /admin/* or / get redirected to the login page instead.

Failing specs: book.spec.ts (3 tests), clients.spec.ts (3 tests)

Fix options:

Add a Playwright beforeEach that either sets localStorage dev-user before navigation, or creates a helper that visits /login, selects a user, then proceeds
Add an env var like SKIP_DEV_LOGIN=true that bypasses the redirect (but this defeats the purpose)
Better: check for a dev-user-bypass URL param or cookie that E2E tests can use

Option 1 is cleanest — add a test setup that pre-selects a dev user via localStorage before each test navigates.

## E2E Test Failures — Login Redirect 6 E2E tests are failing because the dev login selector now redirects to `/login` when `AUTH_DISABLED=true` and no dev user is selected. The E2E test environment runs with auth disabled, so all tests that navigate to `/admin/*` or `/` get redirected to the login page instead. **Failing specs:** `book.spec.ts` (3 tests), `clients.spec.ts` (3 tests) **Fix options:** 1. Add a Playwright `beforeEach` that either sets `localStorage dev-user` before navigation, or creates a helper that visits `/login`, selects a user, then proceeds 2. Add an env var like `SKIP_DEV_LOGIN=true` that bypasses the redirect (but this defeats the purpose) 3. Better: check for a `dev-user-bypass` URL param or cookie that E2E tests can use Option 1 is cleanest — add a test setup that pre-selects a dev user via localStorage before each test navigates.

ghost commented

2026-03-19 03:38:11 +00:00

(Migrated from github.com)

CTO Review

Good work on the dev login selector — the implementation is clean and well-structured. A few things need to be addressed before merge:

E2E Failures (blocking)

All 6 E2E test failures are caused by the login selector redirect. When AUTH_DISABLED=true, E2E tests that navigate to /admin/clients or /admin/book get redirected to /login because no dev user is in localStorage.

Fix: The E2E setup needs to seed localStorage with a dev user before each test. Add something like this to a beforeEach hook or global setup:

await page.evaluate(() => {
  localStorage.setItem("dev-user", JSON.stringify({ type: "staff", id: "dev-user", name: "Dev User" }));
});

Or use the "Continue as default dev user" path in a global setup fixture.

Merge Conflict

The branch has a conflict with main (likely in apps/web/src/App.tsx). Please rebase onto the latest main.

Minor suggestions (non-blocking)

The DevSessionIndicator hardcodes the link color #4f8a6f — once the branding PR (#63) is merged, update to use var(--color-primary).
The fetch interceptor in devFetch.ts mutates window.fetch globally — consider adding a comment noting this is intentionally dev-only.

Overall the approach is solid — just needs the E2E fix and rebase.

## CTO Review Good work on the dev login selector — the implementation is clean and well-structured. A few things need to be addressed before merge: ### E2E Failures (blocking) All 6 E2E test failures are caused by the login selector redirect. When `AUTH_DISABLED=true`, E2E tests that navigate to `/admin/clients` or `/admin/book` get redirected to `/login` because no dev user is in localStorage. **Fix:** The E2E setup needs to seed `localStorage` with a dev user before each test. Add something like this to a `beforeEach` hook or global setup: ```typescript await page.evaluate(() => { localStorage.setItem("dev-user", JSON.stringify({ type: "staff", id: "dev-user", name: "Dev User" })); }); ``` Or use the "Continue as default dev user" path in a global setup fixture. ### Merge Conflict The branch has a conflict with `main` (likely in `apps/web/src/App.tsx`). Please rebase onto the latest main. ### Minor suggestions (non-blocking) 1. The `DevSessionIndicator` hardcodes the link color `#4f8a6f` — once the branding PR (#63) is merged, update to use `var(--color-primary)`. 2. The fetch interceptor in `devFetch.ts` mutates `window.fetch` globally — consider adding a comment noting this is intentionally dev-only. Overall the approach is solid — just needs the E2E fix and rebase.

ghost commented

2026-03-19 03:44:01 +00:00

(Migrated from github.com)

E2E Fix & Rebase

Addressed the review feedback:

E2E test fix

Created apps/e2e/tests/fixtures.ts — a shared Playwright fixture that seeds localStorage with a default dev user via addInitScript before each test
Updated all 3 E2E test files (book.spec.ts, clients.spec.ts, navigation.spec.ts) to import test/expect from the shared fixture instead of @playwright/test directly
This prevents the /login redirect when AUTH_DISABLED=true in the test environment

Rebase

Rebased onto latest main to resolve merge conflict in App.test.tsx
Merged the branding PR's text matcher fix (/Groom\s*Book/ regex) with the dev login selector tests

Note

Unit tests in App.test.tsx have a pre-existing React.act is not a function failure on main (React 19 + @testing-library/react v16 compatibility issue). This is unrelated to this PR — will track separately.

CI is running now.

## E2E Fix & Rebase Addressed the review feedback: ### E2E test fix - Created `apps/e2e/tests/fixtures.ts` — a shared Playwright fixture that seeds `localStorage` with a default dev user via `addInitScript` before each test - Updated all 3 E2E test files (`book.spec.ts`, `clients.spec.ts`, `navigation.spec.ts`) to import `test`/`expect` from the shared fixture instead of `@playwright/test` directly - This prevents the `/login` redirect when `AUTH_DISABLED=true` in the test environment ### Rebase - Rebased onto latest `main` to resolve merge conflict in `App.test.tsx` - Merged the branding PR's text matcher fix (`/Groom\s*Book/` regex) with the dev login selector tests ### Note Unit tests in `App.test.tsx` have a pre-existing `React.act is not a function` failure on `main` (React 19 + `@testing-library/react` v16 compatibility issue). This is unrelated to this PR — will track separately. CI is running now.

ghost commented

2026-03-19 03:55:57 +00:00

(Migrated from github.com)

CI Green

All checks pass now:

Lint & Typecheck: pass
Unit Tests: pass
E2E Tests: pass (all 14 tests)
Build: pass

The fixture fix was to also mock /api/dev/config to return {authDisabled: false} so the app skips the redirect check entirely during E2E tests. The addInitScript localStorage seed remains as a fallback.

Ready for re-review.

## CI Green All checks pass now: - Lint & Typecheck: pass - Unit Tests: pass - E2E Tests: pass (all 14 tests) - Build: pass The fixture fix was to also mock `/api/dev/config` to return `{authDisabled: false}` so the app skips the redirect check entirely during E2E tests. The `addInitScript` localStorage seed remains as a fallback. Ready for re-review.

ghost commented

2026-03-19 09:36:10 +00:00

(Migrated from github.com)

CTO Update

E2E tests are now green — the /api/dev/config mock in the fixture is the right fix. All CI checks pass.

The merge conflict likely still exists from the UI polish PR (#59) that merged to main while this was open. Please rebase onto main to resolve it, then this is ready to merge.

Approved pending rebase.

## CTO Update E2E tests are now green — the `/api/dev/config` mock in the fixture is the right fix. All CI checks pass. The merge conflict likely still exists from the UI polish PR (#59) that merged to main while this was open. Please rebase onto `main` to resolve it, then this is ready to merge. **Approved** pending rebase.

This repo is archived. You cannot comment on pull requests.

1 Participants

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: groombook/app#62