#62 — Tech Debt: `TodoListBroadcastDto` Construction Is Manually Repeated at 8 Call Sites #62

New issue

Closed

opened 2026-08-18 13:13:23 +02:00 by lena · 0 comments

lena commented

2026-08-18 13:13:23 +02:00

(Migrated from git.butzei.de)

Tech Debt: `TodoListBroadcastDto` Construction Is Manually Repeated at 8 Call Sites

Reported by: Backend Engineer, spotted during #23's code review, 2026-07-21

Symptom

Every handler that mutates a TodoListEntity and broadcasts the change constructs
new TodoListBroadcastDto(dto.Id, dto.Title, dto.Description, dto.IsArchived, dto.Color, dto.Icon)
by hand from a freshly-fetched TodoListDto — currently 8 call sites across
CreateTodoListCommandHandler, RenameTodoListCommandHandler, ArchiveTodoListCommandHandler,
UnarchiveTodoListCommandHandler, TransferTodoListOwnershipCommandHandler,
AcceptListInvitationCommandHandler, RemoveTodoListMemberCommandHandler, and
SetTodoListAppearanceCommandHandler.

This has already caused two real omissions found only by code review, not the compiler (both
trailing constructor params have defaults, so a forgotten field compiles silently):

RemoveTodoListMemberCommandHandler shipped without forwarding IsArchived at all (pre-#23),
silently broadcasting isArchived: false for archived lists whenever a member was removed from
one.
The same handler was missed for Color/Icon during #23's first pass — caught and fixed in
that cycle's code review (see docs/features/done/23_list_color_icon.md), but only because a
reviewer happened to check every call site by hand.

Expected behaviour

A single dto.ToBroadcastDto() (or equivalent Mapperly-based) mapping from TodoListDto to
TodoListBroadcastDto, called at all 8 sites, so adding a new field to TodoListDto only requires
updating one mapping definition — a compile error (missing mapped member) rather than a silent
default if a field is forgotten.

Acceptance criteria

One shared mapping from TodoListDto to TodoListBroadcastDto (Mapperly [Mapper] static
method, matching the existing TodoListMappings precedent, or a plain extension method) replaces
all 8 hand-written positional/named new TodoListBroadcastDto(...) calls
Adding a field to TodoListDto without updating the mapping fails the build (Mapperly's
RequiredMappingStrategy.Source, or equivalent), not just silently defaults on the wire
No behavior change to any existing broadcast — same fields, same values, at every call site
Existing handler tests continue to pass unmodified where they assert on broadcast dto shape

Implementation notes

Added TodoListMappings.ToBroadcastDto(this TodoListDto dto) — a Mapperly [Mapper] extension
method with RequiredMappingStrategy.Source (fails the build if a new TodoListDto field isn't
either mapped or explicitly ignored) and [MapperIgnoreSource(nameof(TodoListDto.CurrentUserRole))]
for the one field that has no TodoListBroadcastDto counterpart. All 8 call sites
(CreateTodoListCommandHandler, RenameTodoListCommandHandler, ArchiveTodoListCommandHandler,
UnarchiveTodoListCommandHandler, TransferTodoListOwnershipCommandHandler,
AcceptListInvitationCommandHandler, RemoveTodoListMemberCommandHandler,
SetTodoListAppearanceCommandHandler) now call dto.ToBroadcastDto().

ArchiveTodoListCommandHandler/UnarchiveTodoListCommandHandler/CreateTodoListCommandHandler
previously passed a hardcoded true/false/false for IsArchived instead of dto.IsArchived —
confirmed behaviorally identical before replacing: GetTodoListQueryHandler opens a fresh
DbContext and queries the database directly (not the EF change tracker), so by the time dto is
fetched after the preceding ExecuteUpdateAsync/SaveChangesAsync, dto.IsArchived already
reflects the just-written value.

No existing test references TodoListBroadcastDto directly (confirmed via grep across
CqsTodo.Tests), and neither TodoListMappings nor TodoMappings have direct unit tests for their
generated [Mapper] methods — correctness is enforced by the compiler via
RequiredMappingStrategy, matching the existing precedent. dotnet build succeeds with 0 errors.

Notes for whoever picks this up

Out of scope for this story: the parallel observation (same review) that TodoListColor and
LabelColor duplicate the same 10-value palette across two C# enums and two TS unions. That's a
smaller, lower-confidence finding (the two concepts — list accent color and label color — were
deliberately kept independent so they can diverge later, see
docs/features/done/23_list_color_icon_design.md) and not backed by a repeated-bug track record
the way this mapping gap is — don't fold it into this story without separately re-justifying it.

Agents involved

Backend Engineer — owns the mapping helper and updating the 8 call sites

Blockers

None.

# Tech Debt: `TodoListBroadcastDto` Construction Is Manually Repeated at 8 Call Sites **Reported by:** Backend Engineer, spotted during `#23`'s code review, 2026-07-21 ## Symptom Every handler that mutates a `TodoListEntity` and broadcasts the change constructs `new TodoListBroadcastDto(dto.Id, dto.Title, dto.Description, dto.IsArchived, dto.Color, dto.Icon)` by hand from a freshly-fetched `TodoListDto` — currently 8 call sites across `CreateTodoListCommandHandler`, `RenameTodoListCommandHandler`, `ArchiveTodoListCommandHandler`, `UnarchiveTodoListCommandHandler`, `TransferTodoListOwnershipCommandHandler`, `AcceptListInvitationCommandHandler`, `RemoveTodoListMemberCommandHandler`, and `SetTodoListAppearanceCommandHandler`. This has already caused two real omissions found only by code review, not the compiler (both trailing constructor params have defaults, so a forgotten field compiles silently): - `RemoveTodoListMemberCommandHandler` shipped without forwarding `IsArchived` at all (pre-`#23`), silently broadcasting `isArchived: false` for archived lists whenever a member was removed from one. - The same handler was missed for `Color`/`Icon` during `#23`'s first pass — caught and fixed in that cycle's code review (see `docs/features/done/23_list_color_icon.md`), but only because a reviewer happened to check every call site by hand. ## Expected behaviour A single `dto.ToBroadcastDto()` (or equivalent Mapperly-based) mapping from `TodoListDto` to `TodoListBroadcastDto`, called at all 8 sites, so adding a new field to `TodoListDto` only requires updating one mapping definition — a compile error (missing mapped member) rather than a silent default if a field is forgotten. ## Acceptance criteria - [x] One shared mapping from `TodoListDto` to `TodoListBroadcastDto` (Mapperly `[Mapper]` static method, matching the existing `TodoListMappings` precedent, or a plain extension method) replaces all 8 hand-written positional/named `new TodoListBroadcastDto(...)` calls - [x] Adding a field to `TodoListDto` without updating the mapping fails the build (Mapperly's `RequiredMappingStrategy.Source`, or equivalent), not just silently defaults on the wire - [x] No behavior change to any existing broadcast — same fields, same values, at every call site - [x] Existing handler tests continue to pass unmodified where they assert on broadcast dto shape ## Implementation notes Added `TodoListMappings.ToBroadcastDto(this TodoListDto dto)` — a Mapperly `[Mapper]` extension method with `RequiredMappingStrategy.Source` (fails the build if a new `TodoListDto` field isn't either mapped or explicitly ignored) and `[MapperIgnoreSource(nameof(TodoListDto.CurrentUserRole))]` for the one field that has no `TodoListBroadcastDto` counterpart. All 8 call sites (`CreateTodoListCommandHandler`, `RenameTodoListCommandHandler`, `ArchiveTodoListCommandHandler`, `UnarchiveTodoListCommandHandler`, `TransferTodoListOwnershipCommandHandler`, `AcceptListInvitationCommandHandler`, `RemoveTodoListMemberCommandHandler`, `SetTodoListAppearanceCommandHandler`) now call `dto.ToBroadcastDto()`. `ArchiveTodoListCommandHandler`/`UnarchiveTodoListCommandHandler`/`CreateTodoListCommandHandler` previously passed a hardcoded `true`/`false`/`false` for `IsArchived` instead of `dto.IsArchived` — confirmed behaviorally identical before replacing: `GetTodoListQueryHandler` opens a fresh `DbContext` and queries the database directly (not the EF change tracker), so by the time `dto` is fetched after the preceding `ExecuteUpdateAsync`/`SaveChangesAsync`, `dto.IsArchived` already reflects the just-written value. No existing test references `TodoListBroadcastDto` directly (confirmed via grep across `CqsTodo.Tests`), and neither `TodoListMappings` nor `TodoMappings` have direct unit tests for their generated `[Mapper]` methods — correctness is enforced by the compiler via `RequiredMappingStrategy`, matching the existing precedent. `dotnet build` succeeds with 0 errors. ## Notes for whoever picks this up Out of scope for this story: the parallel observation (same review) that `TodoListColor` and `LabelColor` duplicate the same 10-value palette across two C# enums and two TS unions. That's a smaller, lower-confidence finding (the two concepts — list accent color and label color — were deliberately kept independent so they can diverge later, see `docs/features/done/23_list_color_icon_design.md`) and not backed by a repeated-bug track record the way this mapping gap is — don't fold it into this story without separately re-justifying it. ## Agents involved - **Backend Engineer** — owns the mapping helper and updating the 8 call sites ## Blockers None.