araemer/recipe-platform
1
0
Fork 0
Code Issues 13 Pull requests 3 Projects Releases Packages Wiki Activity Actions

WIP: fix(contract) Make ShoppingListItemReasonDto.addedBy optional for deleted accounts #57

Draft
araemer wants to merge 5 commits from fix-reason-deleted-user-contract into main
pull from: fix-reason-deleted-user-contract
merge into: araemer:main
Conversation 0 Commits 5 Files changed 3 +561 -26
araemer commented 2026-10-25 07:00:25 +00:00
Owner
Copy link

Contract fix for recipe-backend#35 (deleting a user leaves shopping list items without any reason).

Change (api-contract/src/shopping-lists.ts)

  • ShoppingListItemReasonDto.addedBy is now optional: absent when that user's account has been deleted; the reason itself stays. Same rule as CommentDto.createdByUser, ratings and tags.
  • ShoppingListItemDto.tickedBy: doc only. Absent also on a ticked item when the ticker's account has been deleted (the backend already behaves that way).

Base and merge order

  • Branched from 004-fill-list-from-meal-plan-contract (cf101fc), so until #004's contract PR merges this diff also shows the 004 contract. Only commit 18f9b60 belongs to this fix.
  • Order: merge 004's contract PR first, merge main into this branch (merge, never rebase; siblings pin it), merge this PR, re-run contract_pin, then merge the sibling PRs.
  • Siblings: recipe-backend fix-reason-deleted-user and a recipe-app PR both pin this branch's commit (releases need equal pins).

Breaking for siblings

  • addedBy is now optional: code reading reason.addedBy.<field> must handle its absence (compile error until it does).
  • tickedBy (runtime only, not caught by the compiler): never assume ticked ⇒ tickedBy. No non-null assertions; "ticked off by" text needs a fallback (e.g. getUserDisplayName() → "Unbekannt").

ADR

A cross-cutting ADR recording the rule and partially superseding docs/adr/2026-09-27-shopping-list-contract.md comes in a separate docs PR against main (branch fix-reason-deleted-user), together with a convention paragraph in docs/api-conventions.md.

Clean contract review (review-contract, working tree vs HEAD) and decisions

  1. [breaking] :149: addedBy changes from CompactUserDto to addedBy?: CompactUserDto; sibling code reading reason.addedBy.<field> stops compiling. Suggestion: move both siblings' contract.ref together and handle the absent case.
    Decision: rejected (no contract change). Deliberate, agreed with the product owner. Both siblings move their pin to this commit and handle the absent case; noted above.
  2. [breaking] :188-192: tickedBy's guarantee changes from "present when ticked" to "may be absent even when ticked"; the compiler does not catch it. Suggestion: call it out in the PR.
    Decision: acted. Called out in "Breaking for siblings" above.
  3. [question] :149, :192: the accepted ADR 2026-09-27-shopping-list-contract.md gives addedBy: CompactUserDto and says tickedAt/tickedBy are both present while ticked; no ADR records the change, and the deleted-author rule is not written down as a general rule.
    Decision: asked; product owner chose a new ADR. The cross-cutting ADR partially supersedes those two statements, and the rule goes into docs/api-conventions.md, in a separate docs PR.
  4. [question] :145-149: the ShoppingListItemSource display strings ("... by on ") don't cover the absent-user case.
    Decision: rejected. Agreed to leave them: the field doc states when the user is absent, and what to display then is the app's concern (as for comments and ratings).
  5. [inconsistency] :146-147: wording differs from the comments/ratings/tags pattern.
    Decision: acted. The tickedBy wording is aligned ("..., and absent then too when that user's account has been deleted."); addedBy already follows the "Absent when ...'s account has been deleted" pattern. Not re-reviewed: the only change from the findings is that one rephrased sentence.

🤖 Generated with Claude Code

Contract fix for recipe-backend#35 (deleting a user leaves shopping list items without any reason). ## Change (`api-contract/src/shopping-lists.ts`) - `ShoppingListItemReasonDto.addedBy` is now optional: absent when that user's account has been deleted; the reason itself stays. Same rule as `CommentDto.createdByUser`, ratings and tags. - `ShoppingListItemDto.tickedBy`: doc only. Absent also on a ticked item when the ticker's account has been deleted (the backend already behaves that way). ## Base and merge order - Branched from `004-fill-list-from-meal-plan-contract` (`cf101fc`), so until #004's contract PR merges this diff also shows the 004 contract. Only commit `18f9b60` belongs to this fix. - Order: merge 004's contract PR first, merge `main` into this branch (merge, never rebase; siblings pin it), merge this PR, re-run `contract_pin`, then merge the sibling PRs. - Siblings: recipe-backend `fix-reason-deleted-user` and a recipe-app PR both pin this branch's commit (releases need equal pins). ## Breaking for siblings - `addedBy` is now optional: code reading `reason.addedBy.<field>` must handle its absence (compile error until it does). - `tickedBy` (runtime only, not caught by the compiler): never assume `ticked ⇒ tickedBy`. No non-null assertions; "ticked off by" text needs a fallback (e.g. `getUserDisplayName()` → "Unbekannt"). ## ADR A cross-cutting ADR recording the rule and partially superseding `docs/adr/2026-09-27-shopping-list-contract.md` comes in a separate docs PR against `main` (branch `fix-reason-deleted-user`), together with a convention paragraph in `docs/api-conventions.md`. ## Clean contract review (`review-contract`, working tree vs HEAD) and decisions 1. **[breaking]** `:149`: `addedBy` changes from `CompactUserDto` to `addedBy?: CompactUserDto`; sibling code reading `reason.addedBy.<field>` stops compiling. Suggestion: move both siblings' `contract.ref` together and handle the absent case. **Decision: rejected (no contract change).** Deliberate, agreed with the product owner. Both siblings move their pin to this commit and handle the absent case; noted above. 2. **[breaking]** `:188-192`: `tickedBy`'s guarantee changes from "present when ticked" to "may be absent even when ticked"; the compiler does not catch it. Suggestion: call it out in the PR. **Decision: acted.** Called out in "Breaking for siblings" above. 3. **[question]** `:149`, `:192`: the accepted ADR `2026-09-27-shopping-list-contract.md` gives `addedBy: CompactUserDto` and says `tickedAt`/`tickedBy` are both present while ticked; no ADR records the change, and the deleted-author rule is not written down as a general rule. **Decision: asked; product owner chose a new ADR.** The cross-cutting ADR partially supersedes those two statements, and the rule goes into `docs/api-conventions.md`, in a separate docs PR. 4. **[question]** `:145-149`: the `ShoppingListItemSource` display strings ("... by <addedBy> on <addedAt>") don't cover the absent-user case. **Decision: rejected.** Agreed to leave them: the field doc states when the user is absent, and what to display then is the app's concern (as for comments and ratings). 5. **[inconsistency]** `:146-147`: wording differs from the comments/ratings/tags pattern. **Decision: acted.** The `tickedBy` wording is aligned ("..., and absent then too when that user's account has been deleted."); `addedBy` already follows the "Absent when ...'s account has been deleted" pattern. Not re-reviewed: the only change from the findings is that one rephrased sentence. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
feat(contract) #004 Fill a shopping list from a meal plan, and move a dish keeping its id
All checks were successful
/ typecheck-contract (pull_request) Successful in 22s
Details
fe52ccc289 
Adds POST /shopping-list/:id/fill-preview, /fill and
/remove-meal-plan-items, the MEAL_PLAN reason source with its mealPlan
block, POST /meal-plan-dish/:dishId/move, and the error codes
SOURCE_MEAL_PLAN_NOT_FOUND and SOURCE_MEAL_PLAN_ACCESS_DENIED.
See docs/adr/2026-10-24-fill-list-from-meal-plan.md. Approved at GATE A.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fix(contract) #004 Apply clearTicked only while the ticked-items question is shown
All checks were successful
/ typecheck-contract (pull_request) Successful in 21s
Details
712a4f9c7e 
clearTicked is ignored when no ticked item of the target list covers a
dish in the range, so a fill never removes ticked items the user was not
asked about, e.g. after switching the target list with decisions kept.
Found at GATE B (recipe-app keeps decisions on a list change).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fix(contract) #004 Show a meal plan reason's day as null once its dish is no longer planned
All checks were successful
/ typecheck-contract (pull_request) Successful in 20s
Details
8ab6bbee52 
ShoppingListItemMealPlanReasonDto.date becomes string | null: the dish's
current day while it is planned, null once it is removed, its day or plan
deleted, or its recipe swapped. Replaces "the last day the dish was on",
which needed a stored copy synced by the meal plan code. Decided at GATE B.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fix(contract) #004 Reject dish ids from another day on a day save, document its errors
All checks were successful
/ typecheck-contract (pull_request) Successful in 21s
Details
cf101fc4b2 
A day save may carry only ids of dishes already on that day; a dish move
goes only through POST /meal-plan-dish/:dishId/move. Closes a hole where a
day save could pull a dish out of another, possibly inaccessible, plan.
Lists the route's errors (400, 403, 404, 409). Decided at GATE B.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fix(contract) Make ShoppingListItemReasonDto.addedBy optional for deleted accounts
All checks were successful
/ typecheck-contract (pull_request) Successful in 22s
Details
18f9b60079 
A deleted user's reasons now keep the item's reason with the user unset, like comment,
rating and tag creators. Also document that tickedBy is absent when the ticker's account
has been deleted, which the backend already does.

Refs recipe-backend#35

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
All checks were successful
/ typecheck-contract (pull_request) Successful in 22s
Details
This pull request is marked as a work in progress.
This branch is out-of-date with the base branch
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin fix-reason-deleted-user-contract:fix-reason-deleted-user-contract
git switch fix-reason-deleted-user-contract

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main
git merge --no-ff fix-reason-deleted-user-contract
git switch fix-reason-deleted-user-contract
git rebase main
git switch main
git merge --ff-only fix-reason-deleted-user-contract
git switch fix-reason-deleted-user-contract
git rebase main
git switch main
git merge --no-ff fix-reason-deleted-user-contract
git switch main
git merge --squash fix-reason-deleted-user-contract
git switch main
git merge --ff-only fix-reason-deleted-user-contract
git switch main
git merge fix-reason-deleted-user-contract
git push origin main
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
araemer/recipe-platform!57
No description provided.