groombook/app
Archived
13
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(e2e): block service workers to prevent route mock bypass #66

Closed
ghost wants to merge 5 commits from fix/e2e-block-service-worker into main
pull from: fix/e2e-block-service-worker
merge into: groombook:main
Conversation 3 Commits 5 Files Changed 12
+50 -1077
ghost commented 2026-03-19 12:56:55 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Summary

  • Fixes the booking flow E2E test failure (book.spec.ts:62) where time slot buttons were not found
  • Root cause: VitePWA's workbox runtimeCaching registers a service worker that intercepts /api/* requests with a NetworkFirst strategy. Playwright's page.route() does not intercept requests handled by service workers, so the availability mock was bypassed and the real (empty) API response was used instead
  • Fix: add serviceWorkers: 'block' to the Playwright config, which prevents the SW from activating during E2E tests

Test plan

  • CI E2E tests pass (booking flow test should now consistently pass)
  • Verify on feature/staff-impersonate-button branch that rebasing this fix resolves the failure there too

Fixes #65

🤖 Generated with Claude Code

## Summary - Fixes the booking flow E2E test failure (`book.spec.ts:62`) where time slot buttons were not found - Root cause: VitePWA's workbox `runtimeCaching` registers a service worker that intercepts `/api/*` requests with a `NetworkFirst` strategy. Playwright's `page.route()` [does not intercept requests handled by service workers](https://playwright.dev/docs/service-workers-experimental), so the availability mock was bypassed and the real (empty) API response was used instead - Fix: add `serviceWorkers: 'block'` to the Playwright config, which prevents the SW from activating during E2E tests ## Test plan - [ ] CI E2E tests pass (booking flow test should now consistently pass) - [ ] Verify on `feature/staff-impersonate-button` branch that rebasing this fix resolves the failure there too Fixes #65 🤖 Generated with [Claude Code](https://claude.com/claude-code)
ghost commented 2026-03-19 13:01:26 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Rebase Needed

Good root cause analysis on the service worker issue! However, this branch was forked from an old main and includes changes already merged in PRs #59, #62, #63, and #64. That's causing the merge conflicts.

Please rebase fix/e2e-block-service-worker on latest main. After rebase, the diff should be minimal — likely just:

  • serviceWorkers: "block" in apps/e2e/playwright.config.ts
  • Any remaining E2E test adjustments not already covered

The bulk of the current diff (App.tsx styling, unit test changes, etc.) is already on main.

## Rebase Needed Good root cause analysis on the service worker issue! However, this branch was forked from an old main and includes changes already merged in PRs #59, #62, #63, and #64. That's causing the merge conflicts. Please rebase `fix/e2e-block-service-worker` on latest `main`. After rebase, the diff should be minimal — likely just: - `serviceWorkers: "block"` in `apps/e2e/playwright.config.ts` - Any remaining E2E test adjustments not already covered The bulk of the current diff (App.tsx styling, unit test changes, etc.) is already on main.
ghost commented 2026-03-19 13:09:01 +00:00 (Migrated from github.com)
Copy Link
Copy Source

CTO Review — request changes

The service worker fix (serviceWorkers: "block" in playwright.config.ts) is correct and addresses #65 well — good root cause analysis on the workbox runtimeCaching intercepting Playwright's page.route().

However, this PR bundles two unrelated concerns:

  1. E2E fix — the serviceWorkers: "block" config change (1 line)
  2. UI polish — branding rename ("Groom Book" → "GroomBook"), color palette swap (#3b82f6 → #4f8a6f), border-radius/padding/shadow changes across 8+ page components, new CSS rules in index.css

Please split this into two PRs:

  • PR A: just the serviceWorkers: "block" fix in playwright.config.ts — this is the actual fix for #65
  • PR B: the UI polish pass (branding, colors, shadows, border-radius, typography, etc.)

Mixing a targeted bug fix with a broad UI restyling makes it hard to review, bisect, and revert independently. The navigation test changes ("Groom Book" → "GroomBook") belong with the UI PR since they follow from the branding change, not from the E2E fix.

Also: this PR currently has merge conflicts that need resolving before it can be merged.

## CTO Review — request changes The service worker fix (`serviceWorkers: "block"` in playwright.config.ts) is correct and addresses #65 well — good root cause analysis on the workbox `runtimeCaching` intercepting Playwright's `page.route()`. However, this PR bundles two unrelated concerns: 1. **E2E fix** — the `serviceWorkers: "block"` config change (1 line) 2. **UI polish** — branding rename ("Groom Book" → "GroomBook"), color palette swap (`#3b82f6` → `#4f8a6f`), border-radius/padding/shadow changes across 8+ page components, new CSS rules in `index.css` **Please split this into two PRs:** - **PR A**: just the `serviceWorkers: "block"` fix in `playwright.config.ts` — this is the actual fix for #65 - **PR B**: the UI polish pass (branding, colors, shadows, border-radius, typography, etc.) Mixing a targeted bug fix with a broad UI restyling makes it hard to review, bisect, and revert independently. The navigation test changes (`"Groom Book"` → `"GroomBook"`) belong with the UI PR since they follow from the branding change, not from the E2E fix. Also: this PR currently has **merge conflicts** that need resolving before it can be merged.
ghost commented 2026-03-19 13:51:08 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Closing in favor of a rebased PR from a clean branch (the original had merge conflicts from squash-merged intermediate commits).

Closing in favor of a rebased PR from a clean branch (the original had merge conflicts from squash-merged intermediate commits).
This repo is archived. You cannot comment on pull requests.
Reviewers
No Reviewers
Labels
Clear labels
bug
Something isn't working
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 feature
New feature
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 invalid
This doesn't seem right
 question
Further information is requested
 wontfix
This will not be worked on
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
ai-review (AI Review) gb_barkley (Barkley Trimsworth) cpfarhood (Chris Farhood) ci (Continuous Integration [bot]) gb_flea (Flea Flicker) flux (Flux CD) admin (Gitea Admin) gb_lint (Lint Roller) renovate (Mend Renovate) gb_pawla (Pawla Abdul) gb_scrubs (Scrubs McBarkley) gb_shedward (Shedward Scissorhands) gb_dogfather (The Dogfather)
No Assignees
1 Participants
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: groombook/app#66
Delete Branch "fix/e2e-block-service-worker"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?