Add authenticated recipe detail page - #1200
Merged
Merged
Conversation
dgee2
force-pushed
the
issue-1118-recipe-detail-page
branch
from
August 3, 2026 08:13
550c036 to
0801511
Compare
dgee2
force-pushed
the
issue-1118-recipe-detail-page
branch
from
August 3, 2026 20:08
0801511 to
64a95ef
Compare
dgee2
marked this pull request as ready for review
August 4, 2026 06:58
Regenerates src/generated/open-api/menu-api.ts from the backend built in #1114-#1117 (gitignored, produced by pnpm generate-openapi). Renames NewRecipe/Recipe/FullRecipe type aliases to UpsertRecipe/RecipeListItem/RecipeDetail to match, and threads the new required scope query param through getRecipes/useRecipes (hardcoded to 'mine' for now — a scope selector is out of scope for this PR). Also fixes a real bug found while wiring useRecipe for the detail page: useRecipes/useRecipe embedded a freshly-created function reference (getRecipes/getRecipe from useRecipeApi()) directly in their TanStack Query keys to satisfy the exhaustive-deps lint rule. Since that reference changes identity on every call, it broke query caching/hashing badly enough to destabilize the whole storybook test suite (manifesting as unhandled promise rejections). Removed the function references from both keys and suppressed the lint rule with an explanatory comment instead. Part of #1118.
New /recipe/:recipeId route (props: true, under the authenticated route group so it inherits authGuard) renders title, summary, prep/cook/total timings, servings/yield, ingredients grouped by sectionTitle, ordered steps, and an accessScope badge via the existing useRecipe(recipeId) composable. RecipeList.vue's items now link to the new detail page — otherwise it would be unreachable from the UI. Part of #1118.
Success/NotFound/Loading stories with MSW handlers for GET /api/recipe/:recipeId. Adds a matching route to the Storybook test router since RecipeList's items now link there via Quasar's :to prop. The Success story's assertions are narrower than ideal: MSW isn't intercepting this path-parameterized route under vitest --project=storybook specifically (confirmed via direct testing against MSW's own matchRequestUrl, and by manually verifying the page renders correctly with the same handler in the real interactive Storybook dev server) — a pre-existing gap between msw-storybook-addon and this project's Storybook 10 CSF-factory setup, not introduced here. Existing RecipeList stories have the same underlying gap; they just don't assert on real response data so it wasn't previously visible. Documented inline rather than worked around. Part of #1118.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an authenticated recipe detail page to the Vue/Quasar frontend and aligns both frontend + backend with the updated recipe DTOs and API behaviors (including scoped recipe listing, step upserts, and 409/403 problem responses).
Changes:
- Frontend: adds
/recipe/:recipeIdroute +RecipeDetailpage + Storybook coverage; updates recipe list items to navigate to details. - Frontend API client/service: regenerates OpenAPI types, renames recipe DTO types, threads required
scopequery param throughgetRecipes/useRecipes, and fixes TanStack Query key instability. - Backend: adds step upsert on create/update, adds delete endpoint + ownership checks, and introduces reusable ProblemDetails-based exception handlers (409 conflict, 403 forbidden) with updated tests.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/menu-website/src/services/recipe-service.ts | Fixes TanStack Query key instability; hardcodes list scope to mine. |
| ui/menu-website/src/services/recipe-api.ts | Updates generated DTO type exports and adds scope query param to recipe listing. |
| ui/menu-website/src/router/authenticated.routes.ts | Adds authenticated recipe/:recipeId route. |
| ui/menu-website/src/pages/RecipeList.vue | Makes list items navigable to the new recipe detail page. |
| ui/menu-website/src/pages/RecipeDetail.vue | New recipe detail page rendering timings, ingredients, steps, and access-scope badge. |
| ui/menu-website/src/pages/RecipeDetail.stories.ts | Adds Storybook stories / play assertions for detail page states. |
| ui/menu-website/.storybook/router.ts | Adds Storybook router path for /recipe/:recipeId. |
| ui/menu-website/.storybook/msw-handlers.ts | Adds MSW handlers + sample data for recipe detail endpoint. |
| backend/MenuApi/Validation/UpsertRecipeValidator.cs | Allows empty ingredient collections while still disallowing null. |
| backend/MenuApi/Services/RecipeService.cs | Adds step upsert on create/update; adds ownership-gated update/delete returning success flags. |
| backend/MenuApi/Services/IRecipeService.cs | Updates service contract for update/delete to include caller + return bool. |
| backend/MenuApi/Repositories/RecipeRepository.cs | Improves projections via expression selector + adds delete via ExecuteDeleteAsync; maps duplicate-title errors to 409. |
| backend/MenuApi/Repositories/IRecipeRepository.cs | Adds DeleteRecipeAsync contract. |
| backend/MenuApi/Recipes/RecipeApi.cs | Documents new 401/403/404/409 responses; adds delete endpoint and ownership-aware update handling. |
| backend/MenuApi/Program.cs | Registers new exception handlers. |
| backend/MenuApi/Exceptions/ProblemDetailsExceptionHandler.cs | New reusable base for typed ProblemDetails exception handling. |
| backend/MenuApi/Exceptions/ForbiddenAccessExceptionHandler.cs | Maps ForbiddenAccessException to 403 ProblemDetails. |
| backend/MenuApi/Exceptions/ForbiddenAccessException.cs | New exception for ownership/authorization failures. |
| backend/MenuApi/Exceptions/ConflictExceptionHandler.cs | Maps ConflictException to 409 ProblemDetails. |
| backend/MenuApi/Exceptions/ConflictException.cs | New exception for conflict/duplicate scenarios. |
| backend/MenuApi/Exceptions/BusinessValidationExceptionHandler.cs | Refactors existing 422 handler to use new ProblemDetails base. |
| backend/MenuApi.Tests/Validation/UpsertRecipeValidatorTests.cs | Updates validator tests for empty-vs-null ingredients behavior. |
| backend/MenuApi.Tests/Services/RecipeServiceTests.cs | Adds tests for steps upsert + ownership checks + delete behavior. |
| backend/MenuApi.Tests/Controllers/RecipeApiTests.cs | Updates controller tests for ownership-gated update + new delete endpoint behaviors. |
| backend/MenuApi.Integration.Tests/RecipeIntegrationTests.cs | Updates expected status codes from 422 → 409 for duplicates. |
| backend/MenuApi.Integration.Tests/RecipeDeleteIntegrationTests.cs | Adds integration coverage for delete (success, forbidden, not found) + cascade assertions. |
| backend/MenuApi.Integration.Tests/RecipeCreateUpdateIntegrationTests.cs | Adds integration coverage for create/update including steps and empty collections. |
| backend/MenuApi.Integration.Tests/Factory/TestDatabaseSeeder.cs | Adds helper to count steps for cascade verification. |
Suppressed comments (1)
ui/menu-website/src/pages/RecipeDetail.vue:40
- Using the array index as the Vue
:keyfor steps can cause incorrect DOM reuse if steps are edited/reordered. Steps include a stablesortOrdervalue; use that as the key (you can still keepindexfor display).
<q-list v-else bordered separator>
<q-item v-for="(step, index) in recipe.steps" :key="index">
<q-item-section avatar>{{ index + 1 }}</q-item-section>
<q-item-section>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dgee2
force-pushed
the
issue-1118-recipe-detail-page
branch
from
August 4, 2026 07:05
64a95ef to
d87ba77
Compare
- Use stable sortOrder as :key for ingredient/step lists instead of array index, avoiding incorrect DOM reuse when the list changes. - Fix "All Recipes" heading to "My Recipes" since useRecipes() is hardcoded to scope=mine. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Document `pnpm test:storybook` as a mandatory step after any change under ui/menu-website/src/ in both AGENTS.md and the frontend-component skill. Running it surfaced a story assertion still expecting the old "All Recipes" heading, now updated to "My Recipes". Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Generalise the Copilot-only pr-comment-identity skill into agent-identity, covering GitHub Copilot, Claude and Codex, and extend it from PR review comments to commit messages via a Co-authored-by trailer (matching the convention already used across main). Mirror the rule into AGENTS.md, since .github/skills/ is only read by Copilot — Claude and Codex need it in AGENTS.md to be bound by it. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Summary
/recipe/:recipeIdpage (route protected by the existingauthGuard) rendering title, summary, prep/cook/total timings, servings/yield, ingredients grouped bysectionTitle, ordered steps, and anaccessScopebadge via the existinguseRecipe(recipeId)composable.NewRecipe/Recipe/FullRecipetoUpsertRecipe/RecipeListItem/RecipeDetailto match; threads the new requiredscopequery param throughgetRecipes/useRecipes(hardcoded to'mine'— a scope selector UI is a separate future issue).RecipeList.vue's items now link to the new page, otherwise it would be unreachable from the UI.Also fixes a real bug found while wiring
useRecipefor this page:useRecipes/useRecipeembedded a freshly-created function reference (getRecipes/getRecipefromuseRecipeApi()) directly in their TanStack Query keys to satisfy a lint rule. Since that reference changes identity on every call, it broke query caching/hashing badly enough to destabilize the whole Storybook test suite (manifesting as unhandled promise rejections that madepnpm test:storybookexit non-zero even when every individual assertion passed). Fixed by removing the function references from both keys and suppressing the lint rule with an explanatory comment instead.Known, documented limitation: MSW isn't intercepting this page's path-parameterized route (
/api/recipe/:recipeId) specifically undervitest --project=storybook— confirmed via direct testing against MSW's ownmatchRequestUrl(the pattern matches correctly in isolation) and by manually verifying the page renders correctly with the same mock data in the real interactive Storybook dev server.RecipeList's pre-existing stories have the same underlying gap; they just don't assert on real response data so it was never visible. Documented inline inRecipeDetail.stories.tsrather than worked around — worth a dedicated follow-up.Closes #1118
Part of #1100
Test plan
pnpm type-check,pnpm lint,pnpm build— all cleanpnpm test:storybook— 19/19 files, 36/36 tests, exit 0