feat(contract) #001 Shopping lists: epic E01, spec A and contract #20
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "001-shopping-lists"
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?
Epic E01 (shopping lists), its first spec (spec A), and the GATE A output for spec A. The epic, the spec and GATE A are approved.
Epic and spec
docs/epics/E01-shopping-lists.md: the idea, split into six specs (A lists, B filling from a meal plan, C ingredient catalog, D units, E shop layouts, F offline mode), with dependencies, order and the decisions that span them.docs/features/001-shopping-lists.md: spec A. Named lists with a per-user default, sharing at read-write or full access, items with name, amount, unit and comment, ticking into a done section, manual reordering, moving between lists, and "why is this here?".GATE A: contract and ADRs
Contract (
api-contract/src/)shopping-lists.ts(new): the shopping list domain. List actions live under/shopping-list/:id(GET,add-itemwith a server-side duplicate check,add-share,reorder,remove-ticked,set-default). Item actions use/shopping-list-item/:itemId/…(edit, tick, untick, move, delete), and share actions use/shopping-list-share/:shareId/….GET /shopping-list/allis the overview.shared.ts:CompactUserDto, next toUserDto.errors.ts(new):ErrorCodeandErrorResponse { error, code }.users.ts:GET /user/list-shareable, a share picker that every user can call, returningCompactUserDto.Cross-cutting ADRs
docs/adr/2026-09-27-shopping-list-contract.md: default list, item reasons (with asource, extensible for spec B), duplicate check,versioncovering content only, reorder,tickedBy, owner in the overview.docs/adr/2026-09-27-required-error-code.md: every error carries a required machine-readablecode.docs/adr/2026-09-27-compact-user-dto.md: other users appear only asCompactUserDto.docs/adr/2026-09-30-get-reads-and-shallow-nesting.md(from review): reads without a body use GET; sub-resources use shallow nesting, with at most one id per path.docs/api-conventions.mdis updated to match. It also writes down the existing naming practice: reduced views are namedCompact<Entity>Dtoand live next to their full DTO.Spec changes made during the discussion
recipe-backendandrecipe-appare both affected.Follow-up issues found along the way
UserDto.Workflow
architectandpoagents now end their assessment with a numbered list of open questions, as headlines only, and then discuss them one at a time. For the PO, the list is a living agenda that answers may change visibly. See.claude/agent-adr/architect/2026-09-27-architect-one-question-at-a-time.mdand.claude/agent-adr/po/2026-09-27-po-one-question-at-a-time.md.architectreplaces the template's instruction paragraph when it fills in Affected Repos. See.claude/agent-adr/architect/2026-09-30-architect-affected-repos-cleanup.md.review-contractskill: a read-only contract review in a clean sub-agent (context: fork), checked against api-conventions, the documentation standard and the cross-cutting ADRs. The architect runs it before GATE A and decides per finding whether to act, reject or ask.plan-featurecovers the case where the architect cannot start it, with a standing permission to post the report as a plain PR comment. See.claude/skills/review-contract/adr/2026-09-30-review-contract.md.review-contractruns on this PR led to the contract fixes inbddeaa9: 409RECIPIENT_ALREADY_HAS_ACCESS, complete error documentation, trimming ofunitandcomment, and an idempotent move.Next
Merge, then run the repo architects for
recipe-backendandrecipe-app(/plan-feature, step 3).🤖 Generated with Claude Code
WIP: docs(features) #001 Shopping lists epic E01 and spec Ato feat(contract) #001 Shopping lists: epic E01, spec A and contract@ -0,0 +53,4 @@is short.- The architect keeps a running numbered list. Decisions are summarised again in full atthe start of Phase 3, as before.- The `po` agent is unchanged. Whether it should work the same way is a separate decision.We already changed it. A reference to the PO ADR would be better than this claim
@ -66,0 +76,4 @@* Read-only: never sent in a request. Does not extend AbstractDto — it is a reference to a* user, not an editable entity, so `version` does not apply.*/export interface CompactUserDto {Why is the CompactUserDto located here instead of in its own file?
@ -66,0 +103,4 @@* never from the `error` text.* See `docs/adr/2026-09-27-required-error-code.md`.*/export const ErrorCode = {Same here. Why aren't ErrorCode and ErrorResponse separate files?
@ -0,0 +40,4 @@* Kind of a reason why an item is on a list — see `ShoppingListItemReasonDto`.* Designed to grow: spec B of epic E01 adds a meal-plan kind.*/export const ShoppingListItemReasonKind = {Wouldn't ShoppingListItemSource be better. Id don't like the name
@ -0,0 +50,4 @@/*** Result of POST /shopping-list/:id/item/add.*/export const AddShoppingListItemOutcome = {Somehow, I don't like Outcome. wouldn't status be better?
@ -0,0 +107,4 @@/** Whether the item is ticked off. Always present. Read-only — changed via the tick/untick routes. */ticked: boolean;/** ISO-8601 timestamp of the last tick. Present only when `ticked` is true. Read-only. */tickedAt?: string;Should we add tickedBy?
@ -0,0 +227,4 @@version: number;}/** Request body for POST /shopping-list/:id/item/:itemId/move. */I don't like this type of path with two ids in it. Wouldn't
/shopping-list/:id/move-item with both itemId and targetListId in the body be better?
@ -0,0 +283,4 @@},/*** POST /shopping-list/:id/loadIf this has no body, why do we use POST?
@ -0,0 +381,4 @@},/*** POST /shopping-list/:id/item/addOpen discussion? item/add or add-item. Usually, I prefer the latter as we are talking about modifying the shopping-list so why go two levels down? Is there a need for grouping items?
@ -0,0 +410,4 @@},/*** POST /shopping-list/:id/item/:itemId/editI don't like these calls with two ids. let's discuss.
@ -0,0 +434,4 @@},/*** POST /shopping-list/:id/item/:itemId/ticksee above
@ -0,0 +16,4 @@`TagDto.createdByUser`. `UserDto` carries `email` and `role`, so every reader of a comment andevery recipient of a shared meal plan receives other users' email addresses.The name for the reduced type came up too. A first proposal was `UserSummaryDto`. The contractskip this paragraph. We are following a project rule here. No need to mention it.
@ -0,0 +22,4 @@## DecisionA reduced view of an entity is named `Compact<Entity>Dto`. Any response that shows a userThe first sentence should not be part of this ADR. It's on naming of compact representations not on users.
@ -0,0 +191,4 @@## Affected ReposFilled in by the cross-cutting architect and approved at GATE A — leave as is when writingCan't we remove this part once we filled it in?
Replies to the review comments (reviews 1 and 2), in order. Forgejo's API cannot reply inside a comment thread, so everything is collected here.
Review 1
1.
architect-one-question-at-a-time.md: "A reference to the PO ADR would be better than this claim"Fixed in
514bcc7. The last consequence now points to.claude/agent-adr/po/2026-09-27-po-one-question-at-a-time.mdinstead of claiming the PO is unchanged.2.
shared.ts: "Why is the CompactUserDto located here instead of in its own file?"Kept in
shared.tson purpose: a compact DTO lives in the same file as its full DTO, asCompactMealPlanDtosits next toMealPlanDto.UserDtois inshared.tsbecauseauth.tsandmeal-plans.tsimport it. The placement rule is now written down indocs/api-conventions.md("Reduced views are namedCompact<Entity>Dto").3.
shared.ts: "Why aren't ErrorCode and ErrorResponse separate files?"Agreed. Moved to the new
api-contract/src/errors.tsin514bcc7.4.
shopping-lists.ts: "item/add or add-item?"Now
POST /shopping-list/:id/add-item(514bcc7). It is an action on the list, in the same form asreorderandremove-ticked, and the/item/grouping only served the two-id paths, which are gone.5.
shopping-lists.ts: "I don't like these calls with two ids. let's discuss."Discussed and decided. There is a new convention with an ADR: shallow nesting, at most one id per path (
docs/adr/2026-09-30-get-reads-and-shallow-nesting.md). Actions on one item use its own id:POST /shopping-list-item/:itemId/edit. Your variant with the list id in the path and the item id in the body was considered and is recorded in the ADR as not adopted. The app tells "list gone" from "item gone" by reloading the list after an item error.6.
shopping-lists.ts, tick: "see above"Same fix:
POST /shopping-list-item/:itemId/tickand/untick.7.
shopping-lists.ts: "Outcome — wouldn't status be better?"Agreed. Now
AddShoppingListItemStatusin astatusfield, consistent withRecipeImportStatus(514bcc7).8.
shopping-lists.ts: "ShoppingListItemSource would be better"Agreed. Now enum
ShoppingListItemSourcein the fieldsource(514bcc7). The DTO staysShoppingListItemReasonDto, because the whole entry answers "why is this here?". Spec B's nested block is renamed tomealPlanin the ADR, to avoid "source" twice.9.
shopping-lists.ts: "Should we add tickedBy?"Added (
514bcc7):tickedBy: CompactUserDto, present only while the item is ticked. The first tick counts, untick clears it, and a move keeps it. Spec 001 now shows "ticked off by on " in the same hover or tap as "why is this here?".10.
shopping-lists.ts, move: "/shopping-list/:id/move-item with itemId and targetListId in the body?"Resolved by the shallow-nesting decision (see 5):
POST /shopping-list-item/:itemId/move { targetListId }. The ADR records why the item id stays in the path: it is the addressed resource, and every item action would otherwise need its own…-itemverb on the list.11.
shopping-lists.ts: "If this has no body, why do we use POST?"It shouldn't. New convention (same ADR): reads without a request body use GET. Now
GET /shopping-list/:id,GET /shopping-list/allandGET /user/list-shareable. POST stays for filter queries with a typed body. Older bodyless POST reads are tracked in #23.Review 2
12.
compact-user-dto.md: "The first sentence should not be part of this ADR"Removed in
c29ee78, together with the naming alternative and the title part. The ADR now covers only how other users are exposed. TheCompact…Dtonaming rule stays indocs/api-conventions.mdwithout an ADR, since it records existing practice.13.
compact-user-dto.md: "skip this paragraph"Removed in
c29ee78.14.
001-shopping-lists.md: "Can't we remove this part once we filled it in?"Removed in
c29ee78. The architect agent now replaces that instruction paragraph whenever it fills in Affected Repos (.claude/agent-adr/architect/2026-09-30-architect-affected-repos-cleanup.md).🤖 Generated with Claude Code