spikerj/spikersoft-issues
1
0 Fork 0
Code Issues 23 Pull Requests Actions Packages Projects Releases Wiki Activity

marks-address: duplicate @for track keys for unsaved addresses + saveAddresses() is a stub #760

New Issue
Closed
opened 2026-07-21 02:59:53 +00:00 by spikerj · 2 comments
spikerj commented 2026-07-21 02:59:53 +00:00
Owner
Copy link
Copy source

Two pre-existing issues noticed while converting marks-site off ngModel (epic #722 wave-2 batch 7, PR spikersoft-angular#492 — neither introduced there):

  1. Duplicate track keys: the address list iterates @for (entry of ... ; track entry.addressId) and newCustomerAddress() creates entries with addressId: "". Add a second address before saving the first → two entries share the "" key → Angular duplicate-track-key runtime error (NG0955 class) and broken row identity. Track by $index or mint a temp id.

  2. saveAddresses() is a console.warn/console.log stub — the Addresses tab has no persistence path at all (only verifyAddress() touches the service, and only for addresses[0]). Related smell in the same area: marks-service-contract renders a large hard-coded demo contracts array, and day-scheduler declares public hours: ["12:00am", ...] as a type annotation with no initializer (the property is undefined at runtime and unused).

Worth deciding whether marks-site's address/contract tabs are demo scaffolding to finish or to prune.

Two pre-existing issues noticed while converting marks-site off ngModel (epic #722 wave-2 batch 7, PR spikersoft-angular#492 — neither introduced there): 1. **Duplicate track keys:** the address list iterates `@for (entry of ... ; track entry.addressId)` and `newCustomerAddress()` creates entries with `addressId: ""`. Add a second address before saving the first → two entries share the `""` key → Angular duplicate-track-key runtime error (NG0955 class) and broken row identity. Track by `$index` or mint a temp id. 2. **`saveAddresses()` is a `console.warn`/`console.log` stub** — the Addresses tab has no persistence path at all (only `verifyAddress()` touches the service, and only for `addresses[0]`). Related smell in the same area: `marks-service-contract` renders a large hard-coded demo `contracts` array, and `day-scheduler` declares `public hours: ["12:00am", ...]` as a *type annotation with no initializer* (the property is `undefined` at runtime and unused). Worth deciding whether marks-site's address/contract tabs are demo scaffolding to finish or to prune.
spikerj commented 2026-07-22 02:49:42 +00:00
Author
Owner
Copy link
Copy source

Board-sweep verification (2026-07-22): still live — marks-address.component.html line 30 still tracks @for by entry.addressId, which is exactly the reported duplicate-key collision for unsaved addresses (no id yet). No fix commit references this file.

Board-sweep verification (2026-07-22): still live — marks-address.component.html line 30 still tracks @for by entry.addressId, which is exactly the reported duplicate-key collision for unsaved addresses (no id yet). No fix commit references this file.
spikerj commented 2026-08-07 13:48:29 +00:00
Author
Owner
Copy link
Copy source

Migrated to spikerj/spikersoft-angular#646 as part of the umbrella-tracker breakup.

Verified 2026-08-07 against spikersoft-angular@8e5a404, and live against the deployed main-KZKZUOSX.js bundle on learn.spikersoft.com.

  • Code: Both items unchanged. projects/spikersoft/src/app/_components/marks-site/marks-address/marks-address.component.html:30 — @for (entry of _selectedCustomer?.addresses; track entry.addressId; let index = $index), i.e. still tracking by an id that is "" for unsaved rows. marks-address.component.ts:106-116 — saveAddresses(form: any) is still nothing but console.warn + two console.group/console.log pairs; no service call.
  • Live: Not user-reachable: marks-site is unrouted (rg marks over app-routing.module.ts → 0 hits, no marks* route in prod main-KZKZUOSX.js), and MarksAddressComponent's only non-spec reference in the repo is its own marks-address.component.stories.ts. So neither defect can bite a user today — which is exactly the finish-or-prune decision the ticket asks for.

Status: not done — Everything. The decision the ticket names is now the whole ticket: marks-site is dead code, so either prune the address/contract tabs or finish them (temp id or $index tracking + a real persistence path in saveAddresses(), plus the marks-service-contract hard-coded demo array and day-scheduler's uninitialised hours annotation).

Closing here. Work now lives in the repo that holds the fix, so fixes #646 in a PR will auto-close it on merge. The umbrella tracker keeps cross-repo epics only.

— Opus 5 Agent

Migrated to **spikerj/spikersoft-angular#646** as part of the umbrella-tracker breakup. Verified 2026-08-07 against `spikersoft-angular@8e5a404`, and live against the deployed `main-KZKZUOSX.js` bundle on learn.spikersoft.com. - **Code:** Both items unchanged. `projects/spikersoft/src/app/_components/marks-site/marks-address/marks-address.component.html:30` — `@for (entry of _selectedCustomer?.addresses; track entry.addressId; let index = $index)`, i.e. still tracking by an id that is `""` for unsaved rows. `marks-address.component.ts:106-116` — `saveAddresses(form: any)` is still nothing but `console.warn` + two `console.group`/`console.log` pairs; no service call. - **Live:** Not user-reachable: marks-site is unrouted (`rg marks` over `app-routing.module.ts` → 0 hits, no `marks*` route in prod `main-KZKZUOSX.js`), and `MarksAddressComponent`'s only non-spec reference in the repo is its own `marks-address.component.stories.ts`. So neither defect can bite a user today — which is exactly the finish-or-prune decision the ticket asks for. Status: **not done** — Everything. The decision the ticket names is now the whole ticket: marks-site is dead code, so either prune the address/contract tabs or finish them (temp id or `$index` tracking + a real persistence path in `saveAddresses()`, plus the `marks-service-contract` hard-coded demo array and `day-scheduler`'s uninitialised `hours` annotation). Closing here. Work now lives in the repo that holds the fix, so `fixes #646` in a PR will auto-close it on merge. The umbrella tracker keeps cross-repo epics only. — Opus 5 Agent
Sign in to join this conversation.
No Branch/Tag Specified
Labels
Clear labels
agentic
Actionable surfaced by the agentic architecture & security review
 bug
Something is not working
 duplicate
This issue or pull request already exists
 enhancement
New feature
 epic
Cross-repo epic — children live in per-repo trackers
 help wanted
Need some help
 invalid
Something is wrong
 question
More information is needed
 sonarqube
Code quality issue surfaced by SonarQube (bug / code smell)
 wontfix
This won't be fixed
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
spikerj (Joseph Spiker)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: spikerj/spikersoft-issues#760