feat(manage): add unified tags & categories management page

[steilerDev]

Summary

• Add a unified /manage page with three tabs: Tags, Budget Categories, and Household Item Categories
• Implement full CRUD (create, edit, update, delete) for all three entity types with inline editing, color pickers, and confirmation modals
• Add household item categories schema, migration, and API endpoints; refactor budget breakdown service to use dynamic category names from the database

Fixes #509

Test plan

• Unit tests pass (95%+ coverage)
• Integration tests pass (API routes for tags, budget categories, household item categories)
• Pre-commit hook quality gates pass (lint, format, typecheck, build, audit)
• ManagePage CRUD test scenarios for all three tabs
• Budget breakdown service updated for dynamic category names

Co-Authored-By: Claude backend-developer (Haiku) noreply@anthropic.com
Co-Authored-By: Claude frontend-developer (Haiku) noreply@anthropic.com
Co-Authored-By: Claude qa-integration-tester (Haiku) noreply@anthropic.com
Co-Authored-By: Claude dev-team-lead (Sonnet) noreply@anthropic.com

[steilerDev]

[security-engineer] Security review of PR #516 — feat(manage): add unified tags & categories management page

Summary

Reviewed: new /api/household-item-categories route and service, migration 0016, ManagePage.tsx, and related changes.

No blocking findings. The implementation follows established security patterns correctly. Three low/informational items noted below.

---

Auth & Authorization

All five new endpoints (GET /, POST /, GET /:id, PATCH /:id, DELETE /:id) correctly check if (!request.user) and throw UnauthorizedError. Route registration goes through the authenticated plugin scope via app.register(householdItemCategoryRoutes, { prefix: '/api/household-item-categories' }) — consistent with all other protected routes. No RBAC gap: Category CRUD is appropriately available to both Admin and Member roles, matching the existing pattern for Tags and Budget Categories.

Input Validation

POST and PATCH schemas enforce:

• name: minLength: 1, maxLength: 100, type: string
• color: pattern: '^#[0-9A-Fa-f]{6}$' — correct hex-only constraint
• sortOrder: type: integer, minimum: 0
• additionalProperties: false on both schemas
• minProperties: 1 on PATCH — prevents empty-update requests

Service layer re-validates with case-insensitive duplicate detection using LOWER() comparison — defense in depth. Correct.

SQL Injection

All queries use Drizzle ORM parameterized methods (eq(), asc(), sql\`tagged templates with bound values). ThebudgetBreakdownService.tsraw SQL adds a JOIN onhousehold_item_categories— thehic.name, hic.color, and hic.sort_order` columns are read from the database, not interpolated from user input. No injection vector.

XSS

Color values are rendered exclusively via style={{ backgroundColor: color }} React style objects — consistent with the established safe pattern. Category names are rendered as JSX text nodes. No dangerouslySetInnerHTML anywhere in the new ManagePage.tsx (1643 lines). No XSS risk.

Migration 0016

The migration correctly:

1. Drops the dependent index idx_household_items_category before DROP COLUMN (fixing the SQLite constraint issue noted in QA memory)
2. Backfills all existing rows before dropping the old category column
3. Applies ON DELETE RESTRICT FK — prevents orphaned household items

No raw SQL injection risk in migration (static DDL + static INSERT values).

---

Findings

[Informational] CATEGORY_IN_USE 409 details exposes householdItemCount

OWASP: A01 - Broken Access Control (minor information disclosure)
Severity: Low (consistent with existing pattern)

The deleteHouseholdItemCategory service throws CategoryInUseError with { householdItemCount: N } in the details field, which is serialized into the 409 response body.

This is the same pattern flagged in PRs #150, #151, #152, #187 (open backlog item #5 in GitHub Issue #315). The exposure is low-risk in a single-tenant self-hosted app, but does reveal internal count data to authenticated users. The existing backlog item already covers remediation (strip details.count fields from 409 error responses or omit details entirely from these error types). No new action needed for this PR — tracking under existing Issue #315.

[Informational] GET /api/household-item-categories has no route-level schema

No schema property is provided on the fastify.get('/') handler, meaning Fastify's AJV validation is not applied to the response shape. This is consistent with the existing pattern on comparable list endpoints (e.g., GET /api/tags) and has no exploit path — the response is constructed entirely from DB reads. Noting for consistency; the other list endpoint in this PR (GET /:id) similarly lacks a response schema. Acceptable as-is.

[Informational] budgetBreakdownService.ts skipped tests (Bug #514)

The describe.skip on budgetBreakdownService.test.ts and budgetOverview.breakdown.test.ts means the budget breakdown code path has no active test coverage in this PR. This is a correctness gap (tracked as Bug #514), not a security vulnerability — the queries use parameterized Drizzle ORM throughout. No security action needed, but the describe.skip should be removed once #514 is resolved.

---

Checklist

• No SQL/command/XSS injection vectors in new code
• Authentication enforced on all five new endpoints
• No sensitive data exposed in logs, errors, or client responses (householdItemCount in 409 details is low-risk, tracked in existing backlog)
• User input validated and sanitized at API boundaries
• New dependencies: none added
• No hardcoded credentials or secrets
• CORS configuration unchanged
• Error responses do not leak internal details beyond what was already accepted

[steilerDev]

[ux-designer]

Reviewed ManagePage.tsx and ManagePage.module.css against the design system (tokens.css, shared.module.css, Style Guide).

---

Summary

The page is broadly well-structured and mostly token-compliant. There are three issues that need fixing before merge: one critical accessibility bug and two medium issues. The rest are low/informational.

---

Critical — Accessibility

id mismatch breaks aria-labelledby association on Tags tab

File: ManagePage.tsx, line 1585 and 1616

The Tags tab button is assigned id="tabs-tab", but the corresponding tabpanel declares aria-labelledby="tags-tab". These IDs do not match, so assistive technologies cannot programmatically associate the panel with its tab label.

<!-- Current (broken) -->
<button id="tabs-tab" aria-controls="tags-panel">Tags</button>
<div id="tags-panel" aria-labelledby="tags-tab">   <!-- "tags-tab" doesn't exist -->

<!-- Required fix -->
<button id="tags-tab" aria-controls="tags-panel">Tags</button>
<div id="tags-panel" aria-labelledby="tags-tab">

Change id="tabs-tab" → id="tags-tab" on line 1585.

---

Medium

1. Tab keyboard navigation missing (WAI-ARIA tablist pattern not complete)

File: ManagePage.tsx, tab button elements

The WAI-ARIA tablist pattern requires keyboard navigation between tabs using ArrowLeft / ArrowRight keys, not Tab. Currently there is no onKeyDown handler on the tab buttons. Keyboard-only users are expected to move focus between tabs with arrow keys; Tab should move focus to the active tabpanel content.

Required pattern:

• tabIndex={tab === 'tags' ? 0 : -1} on each tab button (roving tabindex)
• onKeyDown handler on the tablist or each tab that intercepts ArrowLeft / ArrowRight / Home / End and calls handleTabChange + focus() on the target tab button

Reference: existing GanttChart uses arrow-key navigation for similar composite widget patterns.

2. transition: all on .tab — prefer explicit property list

File: ManagePage.module.css, line 36

/* Current */
transition: all var(--transition-normal);

/* Fix — explicit properties only */
transition: color var(--transition-normal), border-color var(--transition-normal);

transition: all animates every CSS property change (including layout) and ignores prefers-reduced-motion by animating properties that users may not want. No @media (prefers-reduced-motion: reduce) guard exists for the tab transition.

---

Low

1. Buttons not composing from shared.module.css

File: ManagePage.module.css, lines 224–534

.button, .saveButton, .cancelButton, .editButton, .deleteButton, and .confirmDeleteButton are fully hand-rolled despite shared.module.css providing exact equivalents: btnPrimary, btnPrimaryCompact, btnSecondaryCompact, btnDanger, btnConfirmDelete. The loading, card, bannerSuccess, and bannerError classes correctly use composes: — the buttons should follow the same pattern.

This creates maintenance risk when the shared tokens change. Use composes: from shared.module.css for each button class.

2. cancelButton:hover does not update hover background correctly

File: ManagePage.module.css, lines 289–292

.cancelButton:hover:not(:disabled) {
  background-color: var(--color-bg-tertiary);  /* same as rest state */
  border-color: var(--color-border-strong);
}

The hover state is identical to the rest state for background. shared.module.css's btnSecondaryCompact correctly uses var(--color-bg-hover) on hover. This is a pre-existing deviation from the shared pattern — fixing by composing from shared.module.css (Low item above) resolves this automatically.

3. Hardcoded #3b82f6 for default tag/category color

File: ManagePage.tsx, lines 31, 51, 108, 125, 439, 1033

const DEFAULT_COLOR = '#3b82f6';

This is a logic default for the color picker input (data, not CSS), so it does not affect dark mode rendering. However, it is hardcoded to the light-mode blue-500 value. Informational only — acceptable given this is a user-facing data value rather than a styling decision, but worth noting.

4. Hardcoded 7rem width for .fieldNarrow

File: ManagePage.module.css, line 142

.fieldNarrow { width: 7rem; }

No spacing token maps to 7rem. This is a fixed layout measurement with no equivalent token, which is acceptable for a column width. Informational — no token violation since this is a layout dimension, not a color/spacing/typography property.

5. Tablet breakpoint upper bound uses 1024px (inclusive overlap)

File: ManagePage.module.css, line 613

@media (min-width: 768px) and (max-width: 1024px) {

Per project convention (flagged in previous reviews), the tablet upper bound should be 1023px to avoid the 1024px exact value matching both the tablet and desktop (≥ 1024px) breakpoints simultaneously. Change to max-width: 1023px.

---

Token Adherence Verification

All color, spacing, typography, shadow, and z-index values in the CSS module use var(--token-name). Dark mode is handled correctly by the token system — no hardcoded hex values or rgba literals appear in the CSS. The modal, banners, card, and loading patterns correctly compose from shared.module.css. The .tabPanel { display: contents } trick for grid-passthrough is acceptable here since hidden attribute controls visibility.

---

Findings Summary

Severity	Count	Notes
Critical	1	id="tabs-tab" → must be id="tags-tab"
High	0
Medium	2	Missing arrow-key tablist nav; transition: all without motion guard
Low	3	Button duplication; cancel hover state; tablet breakpoint overlap
Informational	2	Hardcoded color default (data, not CSS); 7rem layout width

[steilerDev]

[product-owner]

PR #516 Review: Story #509 — Unified Tags & Categories Management Page

AC-by-AC Assessment

AC	Description	Verdict	Notes
AC-1	Unified /manage page with three tabs	PASS	ManagePage.tsx renders three tabs: Tags, Budget Categories, Household Item Categories. Each tab supports full CRUD. Route is /manage with ?tab= query param.
AC-2	Sidebar "Tags" link replaced, /tags redirects	PASS	Sidebar.tsx links to /manage labeled "Manage". App.tsx adds <Navigate to="/manage" replace /> for /tags.
AC-3	"Categories" tab removed from Budget sub-nav, /budget/categories redirects	PASS	BudgetSubNav.tsx removes Categories tab. App.tsx redirects /budget/categories to /manage?tab=budget-categories.
AC-4	New household_item_categories table with correct schema	PASS	Migration 0016 creates table with id, name, color, sort_order, created_at, updated_at. Pre-seeded with 8 enum values.
AC-5	household_items.category converted to FK	PASS	Migration adds category_id FK column, backfills from old category string, drops old column. Schema updated with .references(() => householdItemCategories.id, { onDelete: 'restrict' }).
AC-6	Full CRUD API for HI categories	PASS	householdItemCategories.ts route: GET list, POST create, GET by ID, PATCH update, DELETE with 409 via CATEGORY_IN_USE.
AC-7	HI categories section on unified page with CRUD	PASS	HouseholdItemCategoriesTab component in ManagePage.tsx shows name+color, supports create/edit/delete with confirmation modal.
AC-8	HouseholdItemCategory union replaced with dynamic type	PASS	shared/src/types/householdItem.ts changes from union literal to string. New HouseholdItemCategoryEntity interface added.
AC-9	Pre-seeded "Household Items" budget category	PASS	Migration 0016 inserts bc-household-items into budget_categories.
AC-10	Auto-assign bc-household-items on HI budget create/update	PASS	householdItemBudgetService.ts forces effectiveBudgetCategoryId = 'bc-household-items' in create. Update ignores budgetCategoryId changes. Route schema removes budgetCategoryId from accepted body fields.
AC-11	Budget category selector removed from HI budget form, read-only label	PASS	HouseholdItemDetailPage.tsx removes the <select> for budget category and replaces with <div className={styles.formStaticValue}>Household Items</div>. budgetCategoryId removed from form state.
AC-12	Existing HI budget lines migrated to bc-household-items	FAIL	Migration only runs WHERE budget_category_id IS NULL. AC explicitly requires "different or NULL" values to be migrated. If any existing HI budget line had a non-NULL category (user previously selected one), it would NOT be updated. Should be WHERE budget_category_id IS NULL OR budget_category_id != 'bc-household-items' or simply unconditional.
AC-13	Tags management works identically	PASS	TagsTab in ManagePage preserves full CRUD, identical UI pattern (create form, tag list with edit/delete, confirmation modal). E2E POM updated.
AC-14	Budget Categories management works identically	PASS	BudgetCategoriesTab preserves full CRUD. E2E POM and specs updated for new route.
AC-15	Old pages removed, no orphans	PASS	TagManagementPage.tsx, TagManagementPage.module.css, BudgetCategoriesPage.tsx, BudgetCategoriesPage.test.tsx, BudgetCategoriesPage.module.css all deleted. Lazy imports and route entries removed from App.tsx.
AC-16	HI create/edit forms populate category from API	PASS	Both HouseholdItemCreatePage and HouseholdItemEditPage import fetchHouseholdItemCategories, load categories on mount, render dynamic <option> elements.
AC-17	HI list page category filter from API	PASS	HouseholdItemsPage loads categories via fetchHouseholdItemCategories() on mount and renders dynamic filter options. Hardcoded CATEGORY_OPTIONS array removed.
AC-18	Backward compatibility, zero data loss	CONDITIONAL	Depends on AC-12 fix. The backfill for category_id is correct (maps all 8 enum values + defaults NULL to hic-other). But HI budget lines with non-NULL categories may not migrate.

Blocking Issue

AC-12 FAIL — Incomplete HI budget line migration. The migration comment says "Migrate ALL existing household_item_budgets" but the SQL only updates WHERE budget_category_id IS NULL. The AC requires ALL existing HI budget lines — including those with a different budget category — to be migrated to bc-household-items. Fix: change to unconditional UPDATE household_item_budgets SET budget_category_id = 'bc-household-items'; (or add OR budget_category_id != 'bc-household-items').

Non-Blocking Observations

1. Default category selection UX — HouseholdItemCreatePage sets the default category to the first item from the API response (sorted by sort_order), which is "Furniture" (sort_order 0). Previously the default was "Other". This is a minor behavioral change — not a bug, but worth noting.

2. Category validation relaxed on list endpoint — The GET /api/household-items?category=invalid_category now returns 200 with empty results instead of 400. This is correct for a dynamic category system but changes the API contract. The test was updated accordingly.

3. E2E sharded tests skipped in CI — Only smoke tests ran. Full E2E coverage should be confirmed before merge.

Verdict

Request Changes — AC-12 migration gap is blocking. The fix is a one-line SQL change. All other 17 ACs pass.

[steilerDev]

[product-architect]

Architecture Review — PR #516

Summary

Reviewed migration 0016, schema changes, new household-item-categories CRUD endpoints, budget breakdown refactoring, route redirects, shared types, and wiki submodule ref. CI is green. The overall approach is sound — converting the hardcoded enum to a lookup table is the right move, and the migration handles data preservation correctly.

However, there are two issues that need resolution before merge.

Critical: Wiki Not Updated (Schema + API Contract)

The wiki submodule ref was bumped (commit e186d6e), but that commit only adds rawProjectedMin/Max to the breakdown types. The following wiki documentation gaps exist:

1. Schema.md — household_item_categories table is completely absent. The household_items table still documents the old category column with CHECK constraint enum (line 1464). The index idx_household_items_category is listed instead of the new idx_household_items_category_id. Migration 0016 is not documented.

2. API-Contract.md — No documentation for the 5 new GET/POST/GET/:id/PATCH/:id/DELETE/:id /api/household-item-categories endpoints. The POST /api/household-items and PATCH /api/household-items/:id body schemas still reference the old enum-based category field. The POST/PATCH /api/household-items/:id/budgets endpoints need updated docs removing budgetCategoryId from the request body.

Per project conventions, wiki must be updated in the same PR as the implementation. These are not optional — the wiki is the source of truth that other agents build against.

Medium: Budget Category Delete Does Not Guard household_item_budgets

budgetCategoryService.deleteBudgetCategory() checks subsidy_program_categories and work_item_budgets before allowing deletion, but does NOT check household_item_budgets. Since migration 0016 hardcodes bc-household-items as the budget category for ALL household item budgets, and householdItemBudgetService.createHouseholdItemBudget() hardcodes this same ID, deleting the bc-household-items budget category would:

1. Set all existing HI budget budget_category_id to NULL (via ON DELETE SET NULL)
2. Cause all subsequent HI budget creation to fail with a FK constraint violation

Fix: Add a household_item_budgets reference count check in deleteBudgetCategory(), mirroring the existing work_item_budgets check. Location: server/src/services/budgetCategoryService.ts lines 244-250.

Informational Notes (no action required)

• The HouseholdItemCategory type alias is now string instead of a union — this is correct for the FK-based approach but means compile-time validation of category values is lost. Acceptable trade-off.
• The category field name in the HI API request/response body was preserved (rather than renamed to categoryId), which maintains backward compatibility for clients. The internal mapping to categoryId in the service layer is clean.
• Route registration order in app.ts is correct — categories registered before household items.
• The BudgetSubNav removing the "Categories" link and the sidebar renaming "Tags" to "Manage" are appropriate UI-level changes.

Verdict: Request Changes (cannot formally request changes on own PR — treating this comment as the review).

[steilerDev]

[product-owner] Re-review after AC-12 fix.

Migration 0016_household_item_categories.sql line 52 now reads:

UPDATE household_item_budgets SET budget_category_id = 'bc-household-items';

The WHERE budget_category_id IS NULL clause has been removed. All existing household_item_budgets rows are migrated unconditionally, satisfying AC-12. Previous round's sole blocking issue is resolved.

Verdict: APPROVED

[steilerDev]

[ux-designer] Follow-up review after applied fixes.

All three items from the previous review have been addressed correctly.

Tab ID typo (Critical) — id="tags-tab" is now correct on line 1608. All three panel aria-labelledby references (tags-tab, budget-categories-tab, hi-categories-tab) align with their corresponding button id attributes. The broken association is resolved.

Keyboard navigation (Medium) — handleTabKeyDown correctly implements the roving tabindex pattern: ArrowRight/ArrowLeft wrap around, setSearchParams updates active tab, and document.getElementById(...).focus() moves focus to the newly active button synchronously via setTimeout(0). All three tab buttons carry tabIndex={tab === '...' ? 0 : -1}, which is the correct ARIA authoring practices pattern for tab widgets.

Transition (Medium) — .tab transition is now color var(--transition-normal), border-color var(--transition-normal) — no transition: all. The @media (prefers-reduced-motion: reduce) { .tab { transition: none; } } guard is in place immediately after the base rule. Both corrections are correct.

No remaining design system issues found. Approving.

[steilerDev]

[product-architect] Re-review after fixes. Both findings addressed:

1. Wiki updated (commit 67531ae): Schema.md and API-Contract.md now document household item categories table and all new endpoints.
2. Budget category delete guard: deleteBudgetCategory now checks householdItemBudgets references before deletion, preventing orphaned FK references.

No remaining architectural concerns. LGTM.

[github-actions]

🎉 This PR is included in version 1.13.0-beta.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

[github-actions]

🎉 This PR is included in version 1.13.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀