Skip to content

feat(household-items): Work item linking for installation coordination (EPIC-04 Story 4.7)#402

Merged
steilerDev merged 12 commits intobetafrom
feat/393-work-item-linking
Mar 3, 2026
Merged

feat(household-items): Work item linking for installation coordination (EPIC-04 Story 4.7)#402
steilerDev merged 12 commits intobetafrom
feat/393-work-item-linking

Conversation

@steilerDev
Copy link
Copy Markdown
Owner

Summary

Implement backend for work item ↔ household item bidirectional linking to support installation coordination workflows. Add startDate/endDate to household item work item summaries for scheduling context.

Implementation Details

New Types

  • HouseholdItemWorkItemSummary extended with startDate and endDate fields for scheduling context
  • WorkItemLinkedHouseholdItemSummary new interface for reverse direction (work item → household items)

Services

  • householdItemWorkItemService.ts (new): Pure functions following the junction table pattern with individual link semantics
    • listLinkedWorkItems() — list work items for a household item
    • linkWorkItemToHouseholdItem() — create link (409 on duplicate)
    • unlinkWorkItemFromHouseholdItem() — delete link (404 if not exists)
    • listLinkedHouseholdItemsForWorkItem() — list household items for a work item

API Routes

  • householdItemWorkItems.ts (new): GET/POST/DELETE routes under /api/household-items/:householdItemId/work-items
    • GET — 200 with { workItems: [...] }
    • POST — 201 with { workItem: {...} }
    • DELETE — 204
  • workItemHouseholdItems.ts (new): GET route under /api/work-items/:workItemId/household-items
    • GET — 200 with { householdItems: [...] }

All endpoints:

  • Return 401 without auth
  • Return 404 for missing entities
  • Return 409 for duplicate links (POST only)

Updated Files

  • householdItemService.ts: getHouseholdItemWorkItems() now includes startDate and endDate
  • app.ts: Registered both route modules at correct prefixes
  • shared/src/index.ts: Exported WorkItemLinkedHouseholdItemSummary
  • Test fixtures: Updated in householdItem.test.ts and HouseholdItemDetailPage.test.tsx for new fields

Verification Checklist

  • TypeScript typecheck passes
  • Build passes (shared → client → server)
  • Pre-commit hook validates (lint, format, tests)
  • All endpoints follow established patterns (route + service)
  • Error handling: 401 (unauth), 404 (not found), 409 (conflict)
  • Junction table FK constraints: CASCADE deletes on both sides (pre-existing)
  • Test fixtures updated for new required fields

Files Changed

New Files:

  • server/src/services/householdItemWorkItemService.ts
  • server/src/routes/householdItemWorkItems.ts
  • server/src/routes/workItemHouseholdItems.ts

Modified Files:

  • shared/src/types/householdItem.ts (types extended)
  • shared/src/index.ts (export added)
  • server/src/services/householdItemService.ts (function updated)
  • server/src/app.ts (routes registered)
  • shared/src/types/householdItem.test.ts (fixtures updated)
  • client/src/pages/HouseholdItemDetailPage/HouseholdItemDetailPage.test.tsx (fixtures updated)

Fixes #393

claude added 12 commits March 3, 2026 08:02
…n (EPIC-04 Story 4.7)

Implement backend for work item ↔ household item bidirectional linking to support
installation coordination workflows. Add startDate/endDate to household item work
item summaries for scheduling context.

Changes:
- Add startDate/endDate fields to HouseholdItemWorkItemSummary interface
- Add WorkItemLinkedHouseholdItemSummary interface for reverse direction
- Create householdItemWorkItemService with full CRUD link management
- Add GET/POST/DELETE routes for /api/household-items/:id/work-items
- Add GET route for /api/work-items/:id/household-items (reverse direction)
- Update getHouseholdItemWorkItems() to include date fields
- Register both route modules in app.ts
- Update test fixtures for both shared and client

HTTP Endpoints:
- GET /api/household-items/:householdItemId/work-items — list linked work items
- POST /api/household-items/:householdItemId/work-items — link work item
- DELETE /api/household-items/:householdItemId/work-items/:workItemId — unlink
- GET /api/work-items/:workItemId/household-items — list linked household items

All endpoints return 401 without auth, 404 for missing entities, 409 for duplicate
links. Service layer follows junction table pattern with individual link semantics.

Co-Authored-By: Claude backend-developer (Haiku 4.5) <noreply@anthropic.com>
…ation (Story 4.7)

Implements interactive linking UI for coordination between household items and work items.

New API client module:
- client/src/lib/householdItemWorkItemsApi.ts: Functions for linking/unlinking work items

HouseholdItemDetailPage changes:
- Interactive 'Linked Work Items' section with search and dropdown
- Inline unlink confirmation pattern
- Shows work item title, status badge, and start/end dates
- Responsive layout for mobile/tablet

WorkItemDetailPage changes:
- Read-only 'Linked Household Items' section
- Shows household item name, category badge, status badge, and expected delivery date
- Placed before Documents section

CSS updates:
- Touch-friendly targets (44px minimum on tablet/mobile)
- Responsive flex layout for mobile
- Reduced motion support
- Status-based badge coloring

Co-Authored-By: Claude frontend-developer (Haiku 4.5) <noreply@anthropic.com>
Complete test suite for household item and work item linking features:

- server/src/services/householdItemWorkItemService.test.ts (15 tests, 100% coverage)
  - Service unit tests for all linking/unlinking operations
  - Tests for listing linked items, error cases (404, 409 conflicts)

- server/src/routes/householdItemWorkItems.test.ts (15 tests)
  - Integration tests for GET /api/household-items/:id/work-items
  - Integration tests for POST /api/household-items/:id/work-items
  - Integration tests for DELETE /api/household-items/:id/work-items/:workItemId
  - Authentication, validation, and error path coverage

- server/src/routes/workItemHouseholdItems.test.ts (5 tests)
  - Integration tests for GET /api/work-items/:id/household-items
  - Tests for multiple linked items and delivery date fields
  - 404 error cases

- client/src/lib/householdItemWorkItemsApi.test.ts (22 tests)
  - API client unit tests for all four functions
  - Mock fetch patterns and error handling
  - Tests for linked items with dates and statuses

All tests follow established patterns and pass pre-commit validation.

Fixes: Story 4.7 acceptance criteria for work item linking test coverage.

Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) <noreply@anthropic.com>
…s section

The expectedDeliveryDate in the "Linked Household Items" section of WorkItemDetailPage was displaying raw ISO date strings instead of human-readable dates. Applied formatDate() helper to match other date displays throughout the page.

Co-Authored-By: Claude frontend-developer (Haiku 4.5) <noreply@anthropic.com>
…king responses

Add assignedUser to HouseholdItemWorkItemSummary type to include assigned user
details (id, displayName, email) on all work items linked to household items.
Updates both listLinkedWorkItems and linkWorkItemToHouseholdItem service
functions to fetch the assigned user via LEFT JOIN on users table.

Fixes Story 4.7 API contract compliance:
- GET /api/household-items/:id/work-items now includes assignedUser on each item
- POST /api/household-items/:id/work-items response now includes assignedUser

Co-Authored-By: Claude backend-developer (Haiku 4.5) <noreply@anthropic.com>
… tests

Add householdItemWorkItemsApi and workItemsApi mocks to HouseholdItemDetailPage
and WorkItemDetailPage tests to prevent fetch errors after Story 4.7 integration.
Update empty state text assertion to match new UI copy.

Co-Authored-By: Claude qa-integration-tester (Haiku) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Worktree node_modules symlinks were accidentally committed in 8338fef,
breaking the Docker build (COPY server/ server/ conflicts with symlink).
Remove them from tracking and add gitignore pattern for symlinks.

Also fix HouseholdItemDetailPage tests with missing mock setups for
budget/subsidy APIs added by Story 4.6 integration.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The previous commit incorrectly removed the default mock setup for
fetchLinkedWorkItems, causing all tests to crash with "Cannot read
properties of undefined (reading 'length')" when the component renders.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Format ISO date strings with formatDate() helper on linked work items in HouseholdItemDetailPage
- Replace invalid `--spacing-xs` and `--spacing-sm` tokens with correct design system tokens
  - `--spacing-xs` → `--spacing-2` (8px)
  - `--spacing-sm` → `--spacing-3` (12px)
  - Update householdItemLinkList, householdItemLinkRow, and related badge styles
- Replace undefined `--color-warning-bg` with proper household item status tokens
  - ordered → `--color-hi-status-ordered-{bg,text}`
  - in_transit → `--color-hi-status-in-transit-{bg,text}`
- Replace hardcoded values with design tokens in new linked items styles
  - `font-weight: 600` → `var(--font-weight-semibold)`
  - `border-radius: 1rem` → `var(--radius-full)`
  - `padding: 0.125rem` → `var(--spacing-0-5)`
  - `padding: 0.375rem` → `var(--spacing-1-5)`
- Fix undefined CSS class: `sectionHeading` → `sectionTitle` in WorkItemDetailPage
- Add descriptive aria-label to 'Add Link' button in HouseholdItemDetailPage

Fixes #402

Co-Authored-By: Claude frontend-developer (Haiku 4.5) <noreply@anthropic.com>
Update wiki with missing reverse-direction endpoint documentation
and update agent memory with review outcomes.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Owner Author

@steilerDev steilerDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[security-engineer] Security review for PR #402 — Story 4.7: Work Item Linking for Installation Coordination (EPIC-04)

Review Summary

No blocking security issues found. The linking feature follows the established patterns verified across all previous EPIC-04 PRs: Drizzle ORM parameterized queries throughout, proper auth checks on all endpoints, no XSS vectors, and correct 404/409 error semantics.

Checklist

  • No SQL/command/XSS injection vectors in new code
  • Authentication enforced on all new endpoints (request.user check on GET, POST, DELETE)
  • No sensitive data in logs or client responses beyond the established UserSummary pattern
  • User input validated at API boundaries (AJV schema on body workItemId, params householdItemId / workItemId)
  • New dependencies: none
  • No hardcoded credentials or secrets
  • CORS configuration unchanged
  • Error responses consistent with existing patterns; no internal details exposed

Detailed Analysis

A01 — Broken Access Control

All three new endpoints (GET /api/household-items/:id/work-items, POST /api/household-items/:id/work-items, DELETE /api/household-items/:id/work-items/:workItemId) and the reverse GET /api/work-items/:id/household-items perform the session check if (!request.user) throw new UnauthorizedError() before any service call. This is consistent with all EPIC-04 routes reviewed previously.

No horizontal privilege escalation risk: the householdItemWorkItemService validates existence of both the household item and work item via assertHouseholdItemExists / assertWorkItemExists before operating. A non-existent ID results in 404, not a successful mutation on another user's data.

No IDOR risk: in this single-tenant model, any authenticated user can see and link/unlink all items, consistent with the established authorization model for the application.

A03 — Injection

All database queries use Drizzle ORM with eq() / and() operators — parameterized at the driver level. No raw SQL strings or template literals. Consistent with the baseline established across all prior epics.

A04 / A07 — Auth & Session

The new routes use the standard Fastify request.user guard, which is populated by the session plugin and performs a fresh DB lookup per request. No route uses requireRole() since linking is available to both Admin and Member roles, which is the correct design for a collaborative home-building tool.

Frontend XSS Review

HouseholdItemDetailPage.tsx: Work item titles and household item names are rendered as JSX text nodes inside <Link> and plain <span> elements. No dangerouslySetInnerHTML. The WORK_ITEM_STATUS_LABELS dictionary lookup (WORK_ITEM_STATUS_LABELS[wi.status] || wi.status) falls back to the raw status string as a JSX text node — safe. No CSS injection risk.

WorkItemDetailPage.tsx: Household item names rendered as JSX text inside <Link>. Category and status labels from HOUSEHOLD_ITEM_CATEGORY_LABELS / HOUSEHOLD_ITEM_STATUS_LABELS dictionaries — both exhaustively typed against the server-validated enum values. The data-status={hi.status} attribute on the badge span is set from a server-validated enum — no script execution path via data attributes.

Informational: workItemId body field lacks minLength: 1

Severity: Informational
File: server/src/routes/householdItemWorkItems.ts:21

The POST body schema validates workItemId as { type: 'string' } with no minLength: 1 constraint. An empty string "" would pass schema validation and reach the service layer, where assertWorkItemExists queries the database for id = "" and correctly returns a NotFoundError — so there is no exploit path. This is consistent with the existing pattern across all other routes in the codebase (none specify minLength on ID fields). Noting for completeness as item #18 in the open recommendations list.

Risk if unaddressed: None — the service-layer existence check provides a correct defense. The 404 response for "" is semantically accurate.


No blocking or high-severity findings. This PR is clear to merge from a security perspective.

Copy link
Copy Markdown
Owner Author

@steilerDev steilerDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[product-owner] PR #402 Review — Story 4.7: Work Item Linking for Installation Coordination

Verdict: REQUEST CHANGES

Acceptance Criteria Evaluation

AC #1: API endpoints for managing household item <-> work item links — PASS

  • POST /api/household-items/:id/work-items (accepts workItemId, returns 201) — implemented in householdItemWorkItems.ts
  • DELETE /api/household-items/:id/work-items/:workItemId (returns 204) — implemented
  • GET /api/household-items/:id/work-items (returns linked work items with id, title, status) — implemented, also includes startDate, endDate, assignedUser (superset of AC)
  • GET /api/work-items/:workItemId/household-items (reverse direction) — implemented in workItemHouseholdItems.ts

AC #2: Household item detail page "Linked Work Items" section with "Add Link" button and searchable dropdown — PASS (minor deviation)

  • "Linked Work Items" section present with search input, select dropdown, and "Add Link" button
  • Minor deviation: AC describes a button that "opens" a searchable dropdown/modal. Implementation shows search + select + button always visible in an inline row. Functionally equivalent. Non-blocking.

AC #3: Each linked work item displays title, status badge, start/end dates; clicking navigates to detail page — FAIL (dates not formatted)

  • Title: displayed as a <Link> that navigates to /work-items/:id. PASS.
  • Status badge: <StatusBadge status={workItem.status} /> rendered. PASS.
  • Start/end dates: displayed but raw ISO date strings are rendered (line 734: {workItem.startDate || '—'} — {workItem.endDate || '—'}). The formatDate() helper is imported and used elsewhere on this page but is NOT applied to these dates. This will display something like "2026-04-01T00:00:00.000Z" to the user instead of a human-readable format. BLOCKING.
  • Clicking navigates: <Link to={/work-items/${workItem.id}}>. PASS.

AC #4: Each linked work item has an "Unlink" action with confirmation — PASS

  • Unlink button (x) displayed for each linked work item
  • Inline confirmation pattern: clicking Unlink shows "Confirm" / "Cancel" buttons
  • handleUnlinkWorkItem() calls unlinkWorkItemFromHouseholdItem() on confirm

AC #5: Work item detail page displays "Linked Household Items" section — PASS

  • Section added to WorkItemDetailPage.tsx showing name, category badge, status badge, expected delivery date
  • Each item is a <Link> to /household-items/:id

AC #6: Expected delivery date shown as contextual information on work item detail page — PASS

  • expectedDeliveryDate displayed with formatDate() on WorkItemDetailPage (line 2377)
  • Conditionally rendered only when date is present

AC #7: Duplicate link prevention (409) — PASS

  • Service layer checks for existing link and throws ConflictError
  • Route integration test verifies 409 response
  • Frontend handles 409 with "This work item is already linked" error message

AC #8: Cascade deletion via FK — PASS

  • PR description confirms FK CASCADE already exists in schema (Story 4.1 migration)
  • No new migration needed; junction table household_item_work_items has ON DELETE CASCADE on both FK columns

AC #9: Search/select dropdown filters by title with status badge — PASS (with caveat)

  • Search input filters allWorkItems by title (case-insensitive .toLowerCase().includes())
  • Already-linked items are excluded from the dropdown
  • Status shown as text in <option>: {wi.title} ({WORK_ITEM_STATUS_LABELS[wi.status] || wi.status})
  • Native HTML <option> elements cannot render rich badges; text status label is an acceptable implementation within browser constraints. Non-blocking.

AC #10: Empty state messages in both directions — PASS

  • Household item detail: "No work items linked. Use the form below to add a link."
  • Work item detail: "No household items linked. Link household items from their detail pages."

Summary

AC Verdict
1 PASS
2 PASS (minor deviation)
3 FAIL — raw ISO dates, formatDate() not applied
4 PASS
5 PASS
6 PASS
7 PASS
8 PASS
9 PASS (caveat noted)
10 PASS

Blocking Issue

AC #3 — Start/end dates displayed as raw ISO strings on HouseholdItemDetailPage. In HouseholdItemDetailPage.tsx line 734, the dates are rendered without formatDate():

{workItem.startDate || '—'}  {workItem.endDate || '—'}

This should be:

{workItem.startDate ? formatDate(workItem.startDate) : '—'}  {workItem.endDate ? formatDate(workItem.endDate) : '—'}

The formatDate function is already imported on this page. The WorkItemDetailPage correctly uses formatDate(hi.expectedDeliveryDate) for the reverse direction, so this is an inconsistency within the same PR.

Non-Blocking Observations

  1. Test authorship: Tests authored by qa-integration-tester (Sonnet 4.5) — PASS.
  2. CI status: Quality Gates and Docker pass. E2E sharded tests skipping (expected at story level; will run during epic-close).
  3. Agent memory updates: All relevant agent memory files updated — acceptable.
  4. Unused entityCounter: householdItemWorkItems.test.ts line 1623 declares let entityCounter = 0 but only uses it inside createTestWorkItem. Minor; non-blocking.

Action Required

Fix the date formatting on HouseholdItemDetailPage.tsx line 734 to use formatDate() for workItem.startDate and workItem.endDate, then push the fix. All other ACs pass.

Copy link
Copy Markdown
Owner Author

@steilerDev steilerDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[product-architect] Architecture review for PR #402 — Story 4.7: Work Item Linking for Installation Coordination (EPIC-04)

Review Summary

The implementation correctly follows established patterns for junction table management (matching work item subsidies, household item subsidies, etc.), uses proper error handling conventions, and the shared types are well-designed. The service layer properly validates entity existence before operating, returning the correct HTTP status codes (201/204/404/409). The bidirectional linking architecture is sound.

However, there is one medium-severity finding that requires attention before merge: a new endpoint has been added that is not documented in the API Contract wiki page.

Architecture Compliance

Service layer — Correct pattern: householdItemWorkItemService.ts uses pure functions with explicit DbType parameter, NotFoundError/ConflictError error classes, and the standard assert*Exists guard pattern. The code in householdItemService.ts was also correctly updated to add startDate, endDate, and assignedUser to the getHouseholdItemWorkItems helper. Both the service and the inline helper now join to the users table for assignedUser — query structure is consistent.

Route layer — Standard Fastify route pattern with JSON Schema validation, UnauthorizedError guard, correct HTTP status codes (200/201/204). The additionalProperties: false on the POST body schema correctly strips unknown fields (tested).

Shared typesHouseholdItemWorkItemSummary extended with startDate, endDate, assignedUser to match the API Contract. New WorkItemLinkedHouseholdItemSummary with id, name, category, status, expectedDeliveryDate is a clean summary type for the reverse direction. Both types are exported from shared/src/index.ts.

Frontend integration — The API client (householdItemWorkItemsApi.ts) uses the standard get/post/del helpers from apiClient.ts. The HouseholdItemDetailPage correctly loads linked work items and provides inline link/unlink functionality. The WorkItemDetailPage adds a read-only display section for linked household items with category badge, status badge, and delivery date.

app.ts registration — Both route modules registered at correct prefixes with appropriate comments.

Findings

MEDIUM: Undocumented endpoint GET /api/work-items/:workItemId/household-items

File: server/src/routes/workItemHouseholdItems.ts

The API Contract wiki page documents only 3 endpoints under the "Household Item Work Item Link Endpoints" section:

  • GET /api/household-items/:householdItemId/work-items
  • POST /api/household-items/:householdItemId/work-items
  • DELETE /api/household-items/:householdItemId/work-items/:workItemId

The PR adds GET /api/work-items/:workItemId/household-items (reverse-direction lookup) which is not documented in the API Contract. This endpoint exists in the implementation and is consumed by the WorkItemDetailPage, so it is needed. However, per project conventions, the wiki API Contract must document every endpoint. The product-architect should add this endpoint to the API Contract wiki page under the Work Items section (or as a sub-section of the existing Household Item Work Item Link Endpoints section).

Impact: The API Contract is the source of truth for all agents. Undocumented endpoints create discrepancy between contract and implementation.

Action needed: The API Contract wiki page should be updated to include:

#### `GET /api/work-items/:workItemId/household-items`

Returns all household items linked to a work item.

Auth required: Yes

Response (200 OK):
{
  "householdItems": [
    {
      "id": "hi-001",
      "name": "Kitchen Cabinets",
      "category": "fixtures",
      "status": "ordered",
      "expectedDeliveryDate": "2026-05-15"
    }
  ]
}

Error responses:
| 401 | UNAUTHORIZED | No valid session |
| 404 | NOT_FOUND    | Work item ID not found |

This wiki update should be part of this PR (per CLAUDE.md: "New endpoint -> API-Contract.md").

LOW: Redundant .gitignore entry

File: .gitignore:3

The PR adds node_modules (without trailing slash) on line 3, but node_modules/ (with trailing slash) already exists on line 2. Git's .gitignore treats node_modules/ as matching any directory named node_modules at any depth, which already covers the same cases as node_modules without the trailing slash. The extra line is redundant and should be removed to keep the file clean.

INFORMATIONAL: as any type annotations in test mocks

File: client/src/pages/HouseholdItemDetailPage/HouseholdItemDetailPage.test.tsx:128-134

Several mock functions use jest.fn() as any for budget/subsidy API mocks:

const mockFetchHouseholdItemBudgets = jest.fn() as any;
const mockFetchBudgetCategories = jest.fn() as any;

While this is a test file and the any type does not affect production code, the project conventions discourage unjustified any types. These could use the typed jest.fn<typeof ...>() pattern that the same file already uses for other mocks (e.g., mockFetchLinkedWorkItems, mockListWorkItems). This is informational only — test files have more flexibility.

INFORMATIONAL: Work item dates displayed as raw strings

File: client/src/pages/HouseholdItemDetailPage/HouseholdItemDetailPage.tsx, lines around the workItemDates span

The linked work item dates are displayed as raw strings (workItem.startDate || '—'). The page imports formatDate (used elsewhere for budget dates), but the work item date display does not use it. For consistency with the rest of the page, consider using formatDate(workItem.startDate) to format the dates in the user's locale.

API Contract Adherence

Endpoint Contract Implementation Match
GET /api/household-items/:id/work-items 200 { workItems: [...] } Matches Yes
POST /api/household-items/:id/work-items 201 { workItem: {...} } Matches Yes
DELETE /api/household-items/:id/work-items/:workItemId 204 Matches Yes
GET /api/work-items/:id/household-items Not documented 200 { householdItems: [...] } No

Response shapes for GET/POST match the contract (including the extended startDate, endDate, assignedUser fields). Error codes (401/404/409) match the documented error table.

Schema Consistency

The implementation uses the existing household_item_work_items junction table (created in migration 0010 per Story 4.1). No new migrations needed. The Drizzle schema reference (schema.householdItemWorkItems) correctly maps the composite PK and FK constraints documented in Schema.md.

Test Coverage

  • 15 service unit tests covering all 4 functions + error cases
  • 20 route integration tests (GET/POST/DELETE + auth/404/409/validation)
  • 22 API client tests
  • Shared type tests extended for new interfaces
  • Page tests updated with proper mocking

Test coverage appears comprehensive for the new code.

Verdict

The implementation is architecturally sound and follows all established patterns. The one medium-severity finding (undocumented reverse-direction endpoint) should be addressed by updating the API Contract wiki page before merge. The .gitignore duplication is a minor cleanup item.

Copy link
Copy Markdown
Owner Author

@steilerDev steilerDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[ux-designer]

Design Review — PR #402: Work Item Linking for Installation Coordination

Reviewed against wiki/Style-Guide.md and client/src/styles/tokens.css. The structural approach is sound — reusing existing class patterns (.subsidyItem, .unlinkButton, .addWorkItemRow) on the HI side is correct and consistent. The WI side introduces new classes for the read-only list. However, there are two high-severity issues that will silently break layout in the WorkItemDetailPage CSS, plus several medium and low findings.


HIGH — Invalid spacing tokens in WorkItemDetailPage.module.css

File: client/src/pages/WorkItemDetailPage/WorkItemDetailPage.module.css

The new "Linked Household Items" section uses var(--spacing-xs) and var(--spacing-sm) in 7 places. These tokens do not exist in tokens.css. CSS custom properties that are unset resolve to an empty string — effectively 0 — so all gap, padding, and margin on these elements will be zero.

/* WRONG — tokens do not exist */
.householdItemLinkList {
  margin: var(--spacing-sm) 0 0;   /* undefined */
  gap: var(--spacing-xs);           /* undefined */
}
.householdItemLinkRow {
  gap: var(--spacing-xs);           /* undefined */
  padding: var(--spacing-xs) var(--spacing-sm); /* both undefined */
}
.countBadge {
  margin-left: var(--spacing-xs);   /* undefined */
}
.emptyText {
  margin: var(--spacing-sm) 0 0;   /* undefined */
}

Correct mapping (from tokens.css):

Intent (approx.) Correct token
--spacing-xs (8px?) var(--spacing-2) (0.5rem / 8px)
--spacing-sm (12px?) var(--spacing-3) (0.75rem / 12px)

Use the named tokens from the scale: --spacing-1 through --spacing-16. There is no aliased size shorthand (xs, sm, lg) in this system.


HIGH — Undefined token --color-warning-bg in WorkItemDetailPage.module.css

File: client/src/pages/WorkItemDetailPage/WorkItemDetailPage.module.css, lines 1520–1525

--color-warning-bg does not exist in tokens.css. Only --color-warning exists (mapped to --color-orange-400). The background on ordered/in_transit status badges will resolve to transparent.

/* WRONG — --color-warning-bg is not a defined token */
.householdItemStatusBadge[data-status='ordered'],
.householdItemStatusBadge[data-status='in_transit'] {
  background: var(--color-warning-bg);  /* undefined → transparent */
  color: var(--color-warning);
  border-color: var(--color-warning);
}

Additionally: custom status badge styling was built from scratch here when the spec called for reusing the existing <HouseholdItemStatusBadge> component (which already handles all four statuses with the correct --color-hi-status-* token family, including dark mode overrides). The correct fix is to use <HouseholdItemStatusBadge> in the TSX rather than a new custom badge implementation.

If a custom badge is necessary, correct tokens for each state are:

Status Background Text
ordered var(--color-hi-status-ordered-bg) var(--color-hi-status-ordered-text)
in_transit var(--color-hi-status-in-transit-bg) var(--color-hi-status-in-transit-text)
delivered var(--color-hi-status-delivered-bg) var(--color-hi-status-delivered-text)
not_ordered var(--color-hi-status-not-ordered-bg) var(--color-hi-status-not-ordered-text)

MEDIUM — sectionHeading class used in TSX but not defined in CSS

File: client/src/pages/WorkItemDetailPage/WorkItemDetailPage.tsx, line 2352

<h2 className={styles.sectionHeading}>
  Linked Household Items
  {linkedHouseholdItems.length > 0 && (
    <span className={styles.countBadge}>{linkedHouseholdItems.length}</span>
  )}
</h2>

sectionHeading is not defined in WorkItemDetailPage.module.css. The file defines sectionTitle (line 212). CSS Modules will return undefined for an unknown class and apply no styles — the heading will render with no font-size, weight, or margin. Use the existing sectionTitle class:

<h2 className={styles.sectionTitle}>

MEDIUM — Hardcoded values in new WorkItemDetailPage.module.css classes

Several hardcoded literal values appear in the new section that should use design tokens:

Location Hardcoded Correct token
.householdItemLinkName font-weight: 500 var(--font-weight-medium)
.householdItemLinkName:focus-visible outline: 2px solid var(--color-primary) box-shadow: var(--shadow-focus) (standard pattern)
.householdItemCategoryBadge padding: 0.125rem ... var(--spacing-0-5)
.householdItemCategoryBadge border-radius: 1rem var(--radius-full)
.householdItemStatusBadge padding: 0.125rem ... var(--spacing-0-5)
.householdItemStatusBadge border-radius: 1rem var(--radius-full)
.countBadge min-width: 1.25rem; height: 1.25rem No exact token — acceptable, but note
.countBadge padding: 0 0.375rem var(--spacing-1-5)
.countBadge border-radius: 1rem var(--radius-full)
.countBadge font-weight: 600 var(--font-weight-semibold)

The focus ring pattern is important: the project standard is box-shadow: var(--shadow-focus) (not outline), which respects dark mode and matches every other interactive element.


LOW — householdItemLinkRow hover background inconsistency

File: client/src/pages/WorkItemDetailPage/WorkItemDetailPage.module.css, lines 1471–1473

/* Current — background switches from primary to secondary on hover */
.householdItemLinkRow {
  background: var(--color-bg-primary);
}
.householdItemLinkRow:hover {
  background: var(--color-bg-secondary);
}

The corresponding pattern on the HI page uses var(--color-bg-secondary) as the base (matching .subsidyItem) and var(--color-bg-tertiary) on hover. For a row that already has a border, starting from --color-bg-secondary is the established pattern. Either is acceptable, but starting from --color-bg-primary with a hover to --color-bg-secondary makes the row visually indistinguishable from the page background at rest — the border is the only visual anchor.


LOW — HouseholdItemDetailPage: Missing aria-label on "Add Link" button

File: client/src/pages/HouseholdItemDetailPage/HouseholdItemDetailPage.tsx

The "Add Link" button has no aria-label:

<button
  type="button"
  className={styles.button}
  onClick={() => void handleLinkWorkItem()}
  disabled={!selectedWorkItemId || isLinkingWorkItem}
>
  {isLinkingWorkItem ? 'Linking...' : 'Add Link'}
</button>

The text "Add Link" is generic. The button is paired with a select input, so a screen reader user may not have the context for what is being linked. A more descriptive label such as aria-label="Add selected work item link" would help.


INFORMATIONAL — srAnnouncement live region absent on HI page

The spec (Issue #393, per memory notes) requires a visually-hidden aria-live="polite" region announcing "Work item linked/unlinked: {title}" when operations complete. Neither the TSX nor CSS changes for HouseholdItemDetailPage include this live region. Screen reader users who are not focused on the list won't know the operation succeeded.


Checklist Summary

Check Result
Token adherence (no hardcoded hex) Pass (no raw hex values)
All spacing via tokens FAIL--spacing-xs/--spacing-sm undefined
All color via tokens FAIL--color-warning-bg undefined; custom status badge bypasses --color-hi-status-*
Dark mode correctness FAIL--color-warning-bg resolves to transparent in all modes; --color-hi-status-* tokens have correct dark overrides that are bypassed
Responsive behavior Pass (breakpoint guards present)
prefers-reduced-motion Pass (guard present on WorkItemDetailPage)
Keyboard navigation / ARIA Partial — missing aria-label on Add button, missing live region
Shared pattern reuse Partial — HI page reuses existing patterns well; WI page builds custom status badge instead of using <HouseholdItemStatusBadge>
Class name defined before use FAILsectionHeading used in TSX but only sectionTitle exists in CSS

@steilerDev steilerDev merged commit b079fe1 into beta Mar 3, 2026
9 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 3, 2026

🎉 This PR is included in version 1.12.0-beta.8 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 7, 2026

🎉 This PR is included in version 1.12.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants