Skip to content

Address post-merge review feedback for recipe E2E coverage - #1278

Merged
dgee2 merged 4 commits into
mainfrom
codex/1277-follow-up
Sep 9, 2026
Merged

dgee2 merged 4 commits into
mainfrom
codex/1277-follow-up

Conversation

@dgee2

@dgee2 dgee2 commented Sep 9, 2026

Copy link
Copy Markdown
Owner

Summary

Follow-up to merged PR #1277, incorporating the remaining review feedback about Storybook coverage and reused Aspire stacks.

Related to #1277

Changes

  • Register Quasar Notify in the Storybook preview and cover the recipe delete Undo/restore flow with an MSW handler and interaction assertion.
  • Add an authenticated E2E cleanup status check alongside the test-only cleanup route from Implement UUIDv7 recipe ids and soft delete #1277.
  • Have each mutating E2E spec verify cleanup support before creating data, so a stale reused AppHost fails early with a restart instruction instead of leaking fixtures.

Verification

  • dotnet build MenuApi.sln --configuration Release --no-restore — passed with 0 warnings and 0 errors.
  • pnpm build — passed.
  • pnpm lint — passed with 0 errors and 16 existing warnings.
  • Prettier check for changed E2E files — passed.
  • Local Storybook execution is blocked by Windows Playwright spawn EPERM; no Storybook assertion result is claimed locally.

dgee2 and others added 2 commits September 9, 2026 08:44
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
Copilot AI lite review requested due to automatic review settings September 9, 2026 07:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

There are a few correctness/contract issues (TypeScript narrowing in E2E specs, MSW restore response shape completeness, and missing 401 OpenAPI documentation for the new status endpoint) that should be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR follows up on #1277 by strengthening recipe delete/restore coverage in Storybook and making Playwright E2E runs safer when reusing an existing Aspire stack by verifying cleanup support up-front.

Changes:

  • Extends the RecipeDetail Storybook story to cover the delete “Undo” (restore) flow, including MSW wiring and an interaction assertion.
  • Adds a /status endpoint to the E2E cleanup API and introduces a Playwright helper + per-spec preflight check to fail early on stale/reused Aspire stacks.
  • Registers Quasar Notify in Storybook preview so notification-driven interactions can be asserted.
File summaries
File Description
ui/menu-website/src/pages/RecipeDetail.stories.ts Adds restore handler usage + interaction assertions for delete/Undo notifications.
ui/menu-website/e2e/test-helpers.ts Adds cleanup status preflight helper and centralizes cleanup base URL usage.
ui/menu-website/e2e/recipe-detail.spec.ts Adds cleanup-availability preflight before seeding/creating data.
ui/menu-website/e2e/edit-delete-recipe.spec.ts Adds cleanup-availability preflight before creating fixtures.
ui/menu-website/e2e/create-recipe.spec.ts Adds cleanup-availability preflight before creating fixtures.
ui/menu-website/.storybook/preview.ts Registers Quasar Notify plugin for Storybook runtime parity.
ui/menu-website/.storybook/msw-handlers.ts Adds MSW restore handler for the delete/Undo Storybook scenario.
backend/MenuApi/Recipes/E2eTestCleanupApi.cs Adds /status endpoint for E2E cleanup capability checks.
Review details
  • Files reviewed: 8/8 changed files
  • Comments generated: 6
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread ui/menu-website/.storybook/msw-handlers.ts
Comment thread ui/menu-website/e2e/create-recipe.spec.ts Outdated
Comment thread ui/menu-website/e2e/edit-delete-recipe.spec.ts Outdated
Comment thread ui/menu-website/e2e/test-helpers.ts
Comment thread backend/MenuApi/Recipes/E2eTestCleanupApi.cs Outdated
Comment thread ui/menu-website/e2e/test-helpers.ts Outdated
Co-authored-by: Codex <codex@openai.com>
@dgee2
dgee2 marked this pull request as ready for review September 9, 2026 20:37
@dgee2
dgee2 requested a lite review from Copilot September 9, 2026 20:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The changes align with the stated review-feedback goals and only leave a minor wording nit in a new E2E helper error message.

Review details
  • Files reviewed: 8/8 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread ui/menu-website/e2e/test-helpers.ts
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@dgee2
dgee2 enabled auto-merge (squash) September 9, 2026 20:47
@sonarqubecloud

sonarqubecloud Bot commented Sep 9, 2026

Copy link
Copy Markdown

@dgee2
dgee2 merged commit 9ceae47 into main Sep 9, 2026
18 checks passed
@dgee2
dgee2 deleted the codex/1277-follow-up branch September 9, 2026 20:51
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.

2 participants