From c49fccbe0ca740bfd7e45942220c1ae303b52e1d Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 29 May 2026 08:45:50 -0400 Subject: [PATCH] docs(adminui): design for completing deferred follow-ups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Driver collection editors (modal-per-row shared shell), resilience typed form, editable DB-backed LDAP->role map (global roles, live on next sign-in), and stale-comment/note cleanup. Roles intentionally global — no per-cluster permissions. --- .../2026-05-29-adminui-followups-design.md | 132 ++++++++++++++++++ 1 file changed, 132 insertions(+) create mode 100644 docs/plans/2026-05-29-adminui-followups-design.md diff --git a/docs/plans/2026-05-29-adminui-followups-design.md b/docs/plans/2026-05-29-adminui-followups-design.md new file mode 100644 index 00000000..ef010318 --- /dev/null +++ b/docs/plans/2026-05-29-adminui-followups-design.md @@ -0,0 +1,132 @@ +# Design — Complete AdminUI deferred follow-ups + +**Date:** 2026-05-29 +**Status:** Approved (design); implementation plan to follow +**Author:** Joseph Doherty (with Claude Code) + +## Background + +The AdminUI carried a family of "deferred / Phase C.2 follow-up" notes. A prior +change stripped the stale *rendered roadmap banners* from the cluster list pages. +Three remaining note groups were investigated to decide what real work they hide: + +- **Group 1 — driver-page inline notes** ("list-editor coming in a follow-up + phase" for tags/devices/endpoints; "typed-form-ifying Polly is a follow-up"). + → **Real pending UI work.** +- **Group 2 — RoleGrants** ("UI-driven editing of the mapping is deferred — it + implies a config-reload mechanism that doesn't exist yet"). → **Real work; half + the infra already exists.** +- **Group 3 — source comments** (F15 Razor migration, F16 FleetStatusHub bridge, + "Phase 4" identity section, `TODO(3.3/3.4)` route collision). → **~90% stale**; + the referenced work already shipped (the F16 bridge is wired; the legacy + `DriverEdit.razor` no longer exists). Only the Polly typed form is real, and it + is already counted in Group 1. + +### Key facts established during exploration + +- **Driver-embedded tag/device lists in `DriverConfig` JSON are the runtime source + of truth.** Driver factories deserialize them and poll exactly those rows; the + canonical `Tag` table is orthogonal (OPC UA browse-tree only, never read by + drivers). So inline editors are meaningful, not redundant — editing them changes + what the driver polls on the next publish/reinitialize. +- **Resilience** already has a strongly-typed model: `DriverResilienceOptions` + (`BulkheadMaxConcurrent`, `BulkheadMaxQueue`, `RecycleIntervalSeconds`, + `CapabilityPolicies: {DriverCapability → (TimeoutSeconds, RetryCount, + BreakerFailureThreshold)}`) with tier A/B/C defaults via `GetTierDefaults(tier)` + and a `DriverResilienceOptionsParser`. The stored JSON is an *override* shape; + null/absent keys fall back to tier defaults. +- **LDAP role map**: the `LdapGroupRoleMapping` entity + migration + + `ILdapGroupRoleMappingService` (CRUD) already exist but are **not wired** into + login. `LdapAuthService` still reads the static appsettings `GroupToRole` + (`Dictionary`). `RoleGrants.razor` is read-only. +- **Testing**: no bUnit. Established pattern = test `FromOptions`/`ToOptions` + round-trips (xUnit + Shouldly in `AdminUI.Tests`) and services with in-memory EF + (`Configuration.Tests`). + +## Decisions + +- **Scope:** full build — all real follow-ups in Groups 1 & 2, plus Group 3 + comment cleanup. +- **List-editor UX:** modal-per-row with a shared shell component. +- **LDAP reload semantics:** DB-backed, **live on the user's next sign-in** + (per-login DB query; no restart, no new infra). appsettings `GroupToRole` becomes + a bootstrap **fallback** layer. +- **Roles are GLOBAL.** No cluster-level permissions / no per-cluster enforcement + (explicitly chosen for simplicity, reversing an earlier cluster-scoping answer). + Every `LdapGroupRoleMapping` row is `IsSystemWide=true`, `ClusterId=null`. + +## Workstreams + +### WS1 — Driver collection editors (modal-per-row + shared shell) + +- New generic `CollectionEditor` component in `Components/Shared/Drivers/`: + compact read-only table + `[+ Add]` / per-row `Edit` / `Delete`, and a Bootstrap + modal editing a **working copy** of a row (commit on modal-Save, discard on + Cancel). Parameters: `List Items` (bound), header fragment, read-only-cells + fragment, modal-body fragment, `NewRow` factory, optional `Validate` delegate. +- Each driver page swaps its read-only `
` for a `CollectionEditor` supplying
+  its own columns + modal fields. Edits mutate the in-memory `List` already in
+  the page's `FormModel`; the page's existing **Save** serializes it into
+  `DriverConfig` — no new persistence path.
+- Coverage: tags (Modbus, AbCip, AbLegacy, TwinCAT, S7, FOCAS); devices (AbCip,
+  AbLegacy, TwinCAT, FOCAS); endpoints (OpcUaClient).
+- **Errors/validation:** required fields, duplicate Name within list,
+  driver-specific address format; delete confirm; list mutates only on valid commit.
+- **Testing:** per-driver `NewRow` factories + `Validate` methods unit-tested
+  directly; existing `*FormSerializationTests` extended for add/remove via the form
+  model. Modal interaction verified manually via `/run`.
+
+### WS2 — Resilience typed form
+
+- Replace the textarea in `DriverResilienceSection.razor` with a typed form bound to
+  a new mutable `ResilienceFormModel` (all fields nullable; null = tier default):
+  bulkhead concurrent/queue, recycle interval, and an 8-capability grid (Read,
+  Write, Discover, Subscribe, Probe, AlarmSubscribe, AlarmAcknowledge, HistoryRead)
+  of (timeout / retry / breaker-threshold).
+- `FromJson`/`ToJson` emit only non-null overrides (blank → `null`, preserving the
+  current "null = tier defaults" contract). The section gains a `DriverTier`
+  parameter; each driver page passes its known tier so `GetTierDefaults(tier)`
+  renders as placeholders. A collapsible "raw JSON" view remains as escape hatch.
+- **Errors:** non-negative / sane-range numeric validation; emitted JSON must
+  re-parse cleanly through `DriverResilienceOptionsParser`.
+- **Testing:** `ResilienceFormModel` round-trip tests in `AdminUI.Tests` —
+  blank→null, partial-override-preserved, emit→parse-back compatibility.
+
+### WS3 — Editable LDAP→role map (DB-backed, global, live on next sign-in)
+
+- `RoleGrants.razor` → full CRUD over `LdapGroupRoleMapping` via the existing
+  `ILdapGroupRoleMappingService`. **Global only**: `IsSystemWide=true`,
+  `ClusterId=null`; no cluster UI. Fields: LDAP group, `AdminRole`
+  (ConfigViewer/ConfigEditor/FleetAdmin), notes. A group may carry several roles
+  (multiple rows). Edit page gated to **FleetAdmin** (add a minimal FleetAdmin
+  authorization policy; confirm existing role-policy plumbing during plan-writing).
+- Wire the service into `LdapAuthService`: at login → resolve groups →
+  `GetByGroupsAsync` (indexed) → map roles → **merge appsettings `GroupToRole` as a
+  fallback layer** (used when no DB row covers a group). Edits take effect on the
+  user's next sign-in. DB rows authoritative + editable; appsettings entries shown
+  read-only as "fallback."
+- **Errors:** DB unreachable at login → catch, log, fall back to appsettings;
+  login never blocks. CRUD: no duplicate `(LdapGroup, Role)`; group/role required.
+- **Testing:** extend `LdapGroupRoleMappingServiceTests` (in-memory EF) for CRUD +
+  dedupe; new `RoleMapper` overload `Map(groups, dbRows, fallbackDict)` unit-tested
+  for merge + fallback precedence + DB-error fallback.
+
+### WS4 — Cleanup (runs last, after the features exist)
+
+- **Delete stale comments:** `FleetStatusHub.cs` ("passive channel / until the
+  bridge lands"), `EndpointRouteBuilderExtensions.cs` (F15), `DriverIdentitySection.razor`
+  ("Phase 4 / generic DriverEdit"), `DriverEditRouter.razor` + `DriverTypePicker.razor`
+  (`TODO(3.3/3.4)` + the "falls back to legacy DriverEdit" path — verify & clean,
+  legacy file is gone), and update `DriverResilienceSection.razor`'s comment.
+- **Strip rendered notes** now true: per-driver "list-editor coming in a follow-up
+  phase" notes, the OpcUaClient endpoint note, the resilience "typed-form-ifying
+  Polly is a follow-up" note, and the RoleGrants "UI-driven editing is deferred" note.
+
+## Cross-cutting
+
+- **No DB schema change** — `LdapGroupRoleMapping` migration already applied;
+  `DriverConfig`/`ResilienceConfig` columns unchanged.
+- **Definition of done:** build clean + `dotnet test` green + a `/run` pass
+  exercising the modal editors and role-map CRUD.
+- **Suggested sequence:** WS1 shared shell + Modbus tags as proof → remaining
+  drivers → WS2 → WS3 → WS4.