Skip to content

feat(web): manual assets W04 — third device class, add/edit modal, e2e, docs (#4622) - #5261

Merged
ToddHebebrand merged 4 commits into
mainfrom
feature/4622-manual-asset-entry/wave-5227
Sep 8, 2026
Merged

feat(web): manual assets W04 — third device class, add/edit modal, e2e, docs (#4622)#5261
ToddHebebrand merged 4 commits into
mainfrom
feature/4622-manual-asset-entry/wave-5227

Conversation

@ToddHebebrand

Copy link
Copy Markdown
Collaborator

Summary

Wave W04 of #4622 (manual asset entry) — the web-side third class of the unified Devices list, closing #5227. Depends on W01/W02 (manual_assets table + apps/api/src/routes/devices/manual.ts), already on main.

List integration (mergedListFilter / deviceClassFilter / DeviceClassSegment / DeviceList / DevicesPage)

  • DeviceClass"agent" | "network" | "manual"; DeviceStatus gains "unknown" (a manual asset has no reachability — renders an explicit Unknown chip, never "Offline").
  • mergedListFilter.ts: manualFieldValue dispatch beside networkFieldValue; summarizeHiddenNetworkDevicessummarizeHiddenNonAgentDevices, now class-aware so the hidden-rows notice says "N manual assets hidden" instead of defaulting every non-agent row to "network devices"; search now matches serial number and asset tag; VPN facet excludes manual rows same as network.
  • Fourth segment chip (Manual, Package icon); segment/notice block is gated on ENABLE_NETWORK_DEVICES_IN_LIST || manual-rows-presentthe Manual segment is independent of that flag, per spec, since the manual arm carries no feature flag of its own.
  • Three new opt-in columns: serial (manual now, agent once the API sends hardware.serialNumber — plumbing is in place, no API change in this PR), assetTag, location (manual-only). Every two-branch deviceClass === "network" ternary that would have silently folded manual into the wrong branch (sortValue, column availability, row actions) was swept to a three-way check.
  • Bulk bar: manual rows are eligible for Delete only — a new bulk-delete-manual action gated on a manual-only allowlist (mirroring the existing agent-only allowlist), with "N of M eligible" reporting on a mixed selection. Row-level Edit/Delete replace the network arm's View for a manual row (no detail page in v1, per spec).
  • Manual-asset mutations (create/update/link/unlink/delete) all carry the API's requireMfa() contract — handled via runAction's friendly mapper, matching the ArchiveOrgModal/MergeOrgModal MFA_REQUIRED precedent.

ManualAssetModal.tsx (new)

Add/edit modal: Org (fixed from page context or a fleet-view picker), Site (required, defaults to the org's only site), Name, Asset type (reuses the discovered_asset_type/DeviceRole enum + icons), Manufacturer, Model, Serial, Asset tag, Location, Assigned contact (client-filtered picker over org contacts — the route has no server-side search), Tags, Notes. The duplicate-serial warning (DUPLICATE_SERIAL) renders non-blockingly and does not prevent submission. The link control lives on the modal's primary surface, not a tab — #3451 records burying it in a tab as today's mistake to not repeat. Every mutation is wrapped in runAction; ManualAssetModal.tsx is added to no-silent-mutations.test.ts's enforced target set (not the allowlist).

Add-button composition with #5228

#5228 (manual network assets, AddNetworkAssetModal) is not yet on main. Per the wave brief, I built the split control (AddAssetMenu) with the manual item now and left an explicit, tested slot (a dropdown menu, not a bare button) for #5228's "Add network asset…" item — see the comment in AddAssetMenu in DevicesPage.tsx. Whoever lands #5228 adds one more <button> inside the existing menu.

E2E + docs

Testing

  • apps/web: full suite green — 824 files / 9288 tests passed (npx vitest run).
  • npx tsc --noEmit -p apps/web — clean.
  • npx tsc --noEmit -p apps/api — clean (confirms no shared-type break from the Device/DeviceStatus/DeviceClass changes).
  • no-silent-mutations.test.ts — passes with ManualAssetModal.tsx in the enforced set (bumped the frozen file-count assertion, not the allowlist).
  • localeParity.test.ts / translationCoverage.test.ts — all 8 locales carry the new keys; the per-locale duplicate-baseline bumps are all genuine cognates/loanwords (Manual/Site/Notes/Tags/Model/Name), each commented in place.
  • E2E: brought up a real stack via pnpm wt-stack up, ran tests/manual-assets.spec.ts against it — 1 passed. (Needed local-only stack fixes unrelated to this feature to get a working dev .env: FORCE_HTTPS=false, ENABLE_2FA=false, REMOTE_ACCESS_ADMISSION_MODE/REMOTE_WS_AUTH_MODE set, MFA_FORCE_FOR_PARTNER_ADMIN=false — none of that is part of this PR's diff.) Tore the stack down after.

No API changes, no migration — this wave is web-only, consuming the GET/POST/PATCH/DELETE /devices/manual and /devices/manual/:id/link contract W02 already shipped.

Follow-ups to file (per the plan's final section)

  1. CSV/bulk import for manual assets (spec Decision 6) — the manual_asset_source enum already ships with 'import' and createManualAssetSchema is array-friendly; needs preview/dry-run, per-row error reporting, and a stable idempotency key.
  2. Custom fields on manual assets (spec Decision 4, option b) — generalize custom_field_definitions with an entity_type and add a manual_asset_custom_field_values table; the two couplings to solve are device_custom_field_values's composite FK to devices and its devices.custom_fields projection trigger. A jsonb attributes bag is disqualified (lands in excludedOpen, vanishes from tenant export).
  3. Per-asset detail page for manual assets, blocked on Unified device list — per-asset detail pages and inline approval #1424 (unified list phase 2 owns routing/pagination unification, including where link/unlink lives per [UI] Network-asset link/unlink controls are hard to discover (Monitoring tab only; no path from the asset modal) #3451). This wave keeps a stable id and class discriminator on every DTO so Unified device list — per-asset detail pages and inline approval #1424 isn't blocked.

Also worth noting, not filed as issues: warranty alerting stays device-only; manual assets are deliberately absent from billableDeviceConds (not billable); the three-arm client-side merge in DevicesPage.tsx (fetchAllDevices + fetchAllNetworkDevices + fetchAllManualAssets, all concatenated and filtered client-side) remains #1424's debt to pay down, not deepened by this wave.

Closes #5227

🤖 Generated with Claude Code

https://claude.ai/code/session_013peXzPJo3GG3kSGvscjWBU

… modal, e2e, docs (#4622)

Surfaces manual_assets (W02/W01) as a third class in the unified Devices
list alongside agent and network rows: class-aware filter dispatch
(mergedListFilter), segment/columns/sort/bulk-bar generalization
(DeviceList/DevicesPage/deviceClassFilter/DeviceClassSegment), a new
ManualAssetModal for add/edit/link/unlink (MFA_REQUIRED handled via
runAction, matching the API's requireMfa() contract on every mutator),
a split Add-asset menu with a slot for #5228's network-asset entry
point, a Playwright e2e spec, and a docs section explaining the
discovery-vs-inventory split.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013peXzPJo3GG3kSGvscjWBU
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 8, 2026

Copy link
Copy Markdown

Deploying breeze with  Cloudflare Pages  Cloudflare Pages

Latest commit: 7e0b493
Status: ✅  Deploy successful!
Preview URL: https://04bd26a2.breeze-9te.pages.dev
Branch Preview URL: https://feature-4622-manual-asset-en-6mk4.breeze-9te.pages.dev

View logs

…silent failures, tests

- DeviceCard.tsx (grid view) never got the manual-class treatment: a
  manual asset fell into the agent branch and got the full agent kebab
  (Terminal/Run Script/Reboot/Decommission/Permanent Delete) wired to
  real onAction handlers, plus an unconditional metrics-history fetch
  on every card mount — the same #4014 failure class already fixed for
  network rows. Now mirrors DeviceList's manual row Actions cell
  (Edit + Delete only) and skips the metrics fetch.
- Widened DevicesPage.handleDeviceAction's network-only invariant guard
  to also refuse non-delete-manual actions on a manual row, so a future
  caller reopening this class of bug is caught at the funnel, not just
  by each UI surface hiding the actions.
- ManualAssetModal.tsx: runBulkDeleteManual gained a missing outer
  catch (every sibling bulk-mutation function has one); handleSubmit/
  handleLink/handleUnlink no longer risk mislabeling a post-success
  onSaved/onClose failure as a mutation failure; the contact-fetch
  best-effort catch and the per-item delete-manual catch now log
  instead of failing silently.
- Added ManualAssetModal.test.tsx (name required, site defaults to the
  org's only site, duplicate-serial warning is non-blocking, link
  control is on the primary surface) and DeviceList.test.tsx coverage
  for bulk-delete-manual gating and the manual row Actions cell —
  gaps flagged by the PR's test-coverage review.
- New devicesPage.toasts.agentOnlyActionManual locale key (8 locales).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013peXzPJo3GG3kSGvscjWBU
@ToddHebebrand

Copy link
Copy Markdown
Collaborator Author

Review run: /pr-review-toolkit:review-pr — code-reviewer, pr-test-analyzer, silent-failure-hunter, comment-analyzer (parallel, Sonnet).

Findings: 2 critical + 4 silent-failure + 1 test-coverage report → all addressed in 324e714; 0 outstanding.

  • code-reviewer: DeviceCard.tsx (grid view) never got the manual-class treatment — a manual row fell into the agent branch and rendered the full agent action kebab (Terminal/Run Script/Reboot/Decommission/Permanent Delete), each wired to a real handler that would 404 against manual_assets.id — the same [Web] Grid-view DeviceCard kebab offers agent actions for network-discovered assets — no deviceClass guard #4014 failure class already fixed for network rows, plus an unconditional metrics-history fetch on every manual card mount. Fixed: mirrors DeviceList's manual row Actions cell (Edit + Delete only), metrics fetch skipped. Also widened DevicesPage.handleDeviceAction's invariant guard to refuse non-delete-manual actions on a manual row as defense in depth. The reviewer's second claim (an "accidentally deleted" compareNeedsTwo translation key) was investigated and is a false positive: origin/main's en/devices.json had that key duplicated twice already; my locale-key script's JSON round-trip harmlessly deduped it to one instance (same value, still resolves at runtime) — no action taken.
  • silent-failure-hunter: runBulkDeleteManual was missing the outer catch every sibling bulk-mutation function has; ManualAssetModal's handleSubmit/handleLink/handleUnlink could mislabel a post-success onSaved/onClose throw as a mutation failure; the contact-fetch best-effort catch and the per-item delete-manual catch swallowed errors with zero logging. Fixed: added the missing catch, separated mutation-failure handling from post-success side-effect handling (logged, not silently swallowed), added console.warn to both previously-silent catches.
  • pr-test-analyzer: ManualAssetModal.tsx had zero unit coverage (link/unlink, duplicate-serial warning, site-default-to-only-site, name-required), and DeviceList.test.tsx didn't cover bulk-delete-manual gating or the manual row Actions cell. Fixed: added ManualAssetModal.test.tsx (8 tests) and 4 new tests in DeviceList.test.tsx; also added a DeviceCard.test.tsx regression suite (3 tests) locking in the grid-view fix.
  • comment-analyzer: no findings — spot-checked every load-bearing comment claim (requireMfa on every mutator, Manually add a network asset — hand-entered discovered_assets rows with full network-device parity #5228 status, DUPLICATE_SERIAL non-blocking, discovered_assets has no serial column) and all checked out accurate.

Tests: full apps/web suite green — 825 files / 9303 tests (npx vitest run). npx tsc --noEmit clean for both apps/web and apps/api. E2E: manual-assets.spec.ts re-verified green against a real wt-stack stack after the fixes (add → Manual segment → edit → delete), stack torn down after.

Status: review-clean, awaiting maintainer merge.

Todd Hebebrand and others added 2 commits September 8, 2026 02:31
#5213's manual network assets W02 (#5258) and W03 (#5260) landed while this
wave was open; both extend the same Devices surfaces. Resolution keeps both
features in full:

- DevicesPage: ONE Add control. main's inline split menu is folded into this
  wave's shared AddAssetMenu component, now three items (Install agent…,
  Add asset manually…, Add network asset…) in both the header and empty-state
  instances. Testids follow main's names (devices-page-add-menu*), so #5260's
  e2e spec is unchanged; this wave's Page Object was renamed to match. Side
  benefit: main's two instances shared one addMenuOpen flag (they opened
  together); AddAssetMenu holds per-instance state.
- Locale catalogs (8): structural 3-way JSON union, plus devicesPage.addAsset /
  installAgent / addManualAssetAction folded into devicesPage.addMenu.* so the
  two features share one namespace.
- DeviceList / columnVisibility: serial + assetTag + location (this wave) AND
  source (#5213) all kept.
- bulkActionGating: took main's dedicated
  deviceActions.unavailable.unknown string over this wave's notOnline reuse —
  a manual asset was never online, so "not online" is the less honest label.
- DeviceList.statusSortRank: kept this wave's ordering (decommissioned 6,
  unknown 7) — "no probe has ever run" is a different axis, not a state worse
  than offline.
- no-silent-mutations: both handlers registered, count 123 -> 125.
- translationCoverage: baselines recomputed as base + both deltas.

Verified: apps/web 828 files / 9336 tests pass; apps/web and apps/api tsc clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016tcUieLXQuupVjBUSZUn5j
Merge review finding. #5213's devicesPage.hiddenNetworkNotice{,_one,_other}
were superseded by this wave's more general devicesPage.hiddenNonAgentNotice
(which covers hidden manual assets as well as hidden network devices). The
locale union kept both; the network-only variants have zero references in
apps/web/src or e2e-tests, and neither translationCoverage.test.ts nor
keyUsage.test.ts flags unused keys, so they would have sat dead in all 8
catalogs indefinitely.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016tcUieLXQuupVjBUSZUn5j
@ToddHebebrand
ToddHebebrand force-pushed the feature/4622-manual-asset-entry/wave-5227 branch from 7e0b493 to 90333d7 Compare September 8, 2026 08:52
@ToddHebebrand

Copy link
Copy Markdown
Collaborator Author

Merge of origin/main after #5258/#5260 landed — resolution summary:

origin/main was merged in at 5c3ad58677 (no rebase; history preserved). #5213's manual-network-assets W02 (#5258) and W03 (#5260) extend the same web surfaces as this wave, so 16 files conflicted. Nothing from either side was dropped. Per file:

File From this wave (#4622 W04) From main (#5213 W02/W03)
DevicesPage.tsx the shared AddAssetMenu component; showAddManualAsset / editingManualAsset state; ManualAssetModal render; hiddenNonAgentNotice showAddNetworkAsset hash state; AddNetworkAssetModal render + its unconditional clear-on-close; the "Add network asset…" item
DeviceList.tsx unknown status doc + statusSortRank ordering; serial/assetTag/location sort accessors source sort accessor (network-only, blanks-last)
columnVisibility.ts serial, assetTag, location (ids + labels) source (id + label)
bulkActionGating.ts unknown: 'deviceActions.unavailable.unknown'
DeviceCompare.tsx, DeviceDetails.tsx comment-only hunks — merged into one comment covering both origins of unknown same
no-silent-mutations.test.ts ManualAssetModal.tsx AddNetworkAssetModal.tsx — count 123 → 125
translationCoverage.test.ts — baselines recomputed as base + both deltas (pt-BR 174, es-419 125, fr-FR/fr-CA 152, de-DE 164 + discovery 27, tr-TR 88), not by taking a side
locales/*/devices.json (all 8) serial/assetTag/location, unknown status, manual-asset modal + segment strings source.{scan,unifi,manual}, addNetworkAssetModal.*, addMenu.*
DeviceCard.tsx, mergedListFilter.ts, DevicesPage.test.tsx no textual conflict; re-read and verified three-way (agent | network | manual) branching is complete on both sides

One Add control, not two. main shipped its own inline split menu; this wave had already extracted AddAssetMenu with an explicit reserved slot for it. The resolution folds main's item into that component, so the header and the empty state each render a single three-item menu (Install agent… / Add asset manually… / Add network asset…). Testids follow main's names (devices-page-add-menu*, devices-page-empty-add-menu*) so #5260's manual-network-asset.spec.ts is untouched; this wave's own Page Object was renamed to match. Side benefit: main's two menu instances shared one addMenuOpen flag and opened together — AddAssetMenu holds per-instance state, so that's fixed.

Locale key consolidation. This wave's flat devicesPage.addAsset / installAgent / addManualAssetAction were folded into main's devicesPage.addMenu.* namespace (adding addMenu.addManualAsset) so both features share one namespace rather than leaving a duplicate set. A follow-up commit also removes main's devicesPage.hiddenNetworkNotice{,_one,_other} from all 8 catalogs — superseded by this wave's more general hiddenNonAgentNotice, and dead everywhere (neither translationCoverage nor keyUsage flags unused keys).

Two genuine semantic clashes, resolved on the merits rather than by taking a side:

  1. DeviceList.statusSortRank — kept this wave's ordering (decommissioned: 6, unknown: 7) over main's inverse. "No probe has ever run" is a different axis, not a state worse than offline; it sorts last.
  2. bulkActionGating.notOnlineTitleKeys.unknown — kept main's dedicated deviceActions.unavailable.unknown string over this wave's notOnline reuse. A hand-typed or never-probed asset was never "not online"; the dedicated string is the honest label. (Both features produce unknown, so this is now shared.)

Verification

  • apps/web full suite: 828 files / 9336 tests pass; after the locale-cleanup commit, the devices + i18n subset re-run at 100 files / 1255 pass.
  • apps/web tsc --noEmit: clean. apps/api tsc --noEmit: clean.
  • Review round on the merged head (code-reviewer + silent-failure-hunter lenses): 1 finding, low — the dead hiddenNetworkNotice* keys above, fixed in 90333d7f3b. No dropped code, no orphan Add control, no silent-failure regressions, all 8 catalogs key-identical.

E2E (worktree stack, torn down after)

🤖 Generated with Claude Code

https://claude.ai/code/session_016tcUieLXQuupVjBUSZUn5j

@ToddHebebrand
ToddHebebrand added this pull request to the merge queue Sep 8, 2026
Merged via the queue into main with commit 3a8a299 Sep 8, 2026
70 checks passed
@ToddHebebrand
ToddHebebrand deleted the feature/4622-manual-asset-entry/wave-5227 branch September 8, 2026 09:55
ToddHebebrand pushed a commit that referenced this pull request Sep 8, 2026
…ession test (#5265)

W04 of #4622 (#5261) added fetchAllManualAssets to DevicesPage's Promise.all;
the harness mocked the module without it, so the page fell into the error
branch and the device list never rendered.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pc21knHQGa6fCM7UA9YtKX
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

W04: Web third class, add/edit modal, e2e, docs

1 participant