diff --git a/docs/plans/2026-06-07-template-alarm-cli-and-override-coverage.md b/docs/plans/2026-06-07-template-alarm-cli-and-override-coverage.md new file mode 100644 index 00000000..76349530 --- /dev/null +++ b/docs/plans/2026-06-07-template-alarm-cli-and-override-coverage.md @@ -0,0 +1,521 @@ +# Template-Alarm CLI Ergonomics + Alarm-Override Coverage — Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:executing-plans (or subagent-driven-development) to implement this plan task-by-task. + +**Goal:** Add ergonomic typed setpoint flags to the existing `template alarm add` CLI verb, then close the deferred `InstanceConfigure` alarm-override UI coverage gap (gated on additive `data-test` hooks), then apply the cheap Wave-4 hygiene items. + +**Architecture:** Part A is **CLI-only** (host-side `dotnet build`) — a new CLI-side `AlarmTriggerConfigJson` serializer + typed options on `BuildAlarm` that ride the *existing* `AddTemplateAlarmCommand` DTO (no server change). Part B adds 6 non-functional `data-test` hooks to `InstanceConfigure.razor` (**one `bash docker/deploy.sh` rebuild**), gives `InstanceConfigureFixture` an alarm, and adds one override round-trip test. Part C is test-only hygiene. + +**Tech Stack:** C# / System.CommandLine (CLI), Blazor Server (CentralUI), xUnit + `Xunit.SkippableFact` + Microsoft.Playwright, the `scadabridge` CLI. TFM `net10.0`, `Nullable=enable`, `TreatWarningsAsErrors=true`. + +--- + +## Grounding facts (verified — do NOT re-derive) + +**The premise was stale.** `template alarm add | update | delete` already exist and work end-to-end (CLI → `POST /management` → `ManagementActor.HandleAddAlarm` → `TemplateService.AddAlarmAsync` → repo + audit). The only gaps: no typed setpoint flags (must hand-write `--trigger-config` JSON), no `CliRunner.AddAlarmAsync` helper, and the alarm-override UI is hook-poor + untested. + +**Codec key names** (the contract — from `src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/AlarmTriggerConfigCodec.cs`, `internal`, do NOT move it): +- `ValueMatch` → `{ attributeName, matchValue }` (matchValue `"!=X"` ⇒ not-equals) +- `RangeViolation` → `{ attributeName, min, max }` +- `RateOfChange` → `{ attributeName, thresholdPerSecond, windowSeconds, direction }` (direction normalized to `rising` | `falling` | `either`) +- `HiLo` → `{ attributeName, loLo, lo, hi, hiHi, ... }` (only the setpoints present are emitted) +- `Expression` → `{ expression }` (no attributeName) +The codec's `Serialize` writes `attributeName` for every type except `Expression`, and writes only numeric keys whose value `HasValue`. + +**CLI `BuildAlarm`** (`src/ZB.MOM.WW.ScadaBridge.CLI/Commands/TemplateCommands.cs:216-296`): the `add` subcommand registers `--template-id --name --trigger-type --priority --description --trigger-config --locked` and its `SetAction` builds `new AddTemplateAlarmCommand(templateId, name, triggerType, priority, description, triggerConfig, locked)` via `CommandHelpers.ExecuteCommandAsync`. DTO: `AddTemplateAlarmCommand(int TemplateId, string Name, string TriggerType, int PriorityLevel, string? Description, string? TriggerConfiguration, bool IsLocked)` (`Commons/Messages/Management/TemplateCommands.cs:14`). The CLI is host-side (`OutputType=Exe`, references only `Commons`); a CLI change needs `dotnet build src/ZB.MOM.WW.ScadaBridge.CLI`. + +**Test-helper patterns** (`tests/.../Cluster/CliRunner.Helpers.cs`): `CreateTemplateAsync` (line 50, `RunJsonAsync` + `RequireId(doc, "template create")`), `AddAttributeAsync` (line 82, builds a `List` of args + `RunAsync` — returns void, uses `--data-source` only when set), `DeleteTemplateAsync` (line 210, `BestEffortAsync("template","delete",id)`), `GetInstanceDocumentAsync(int id)` (line 408), `RequireId(JsonDocument, string command)` (line 689). Three-token best-effort deletes are written inline (try/catch swallow) like `DeleteAreaAsync`/`DeleteRoleMappingAsync` (the shared `BestEffortAsync` only models two-token groups). + +**InstanceConfigure alarm-override markup** (`src/.../CentralUI/Components/Pages/Deployment/InstanceConfigure.razor`), the 6 hook sites: +- Row `` at **line 248** (inside `@foreach (var alarm in _overridableAlarms)`). +- Edit button at **lines 269-271** (`@onclick="() => BeginEditOverride(alarm)"`). +- Override badge `●` at **line 260** (rendered only when `HasOverride(alarm.Name)`). +- Row Clear button at **lines 274-276** (rendered only when `HasOverride`). +- Modal priority input `` at **lines 320-322**. +- Modal Save button `` at **line 342**. +The section only renders rows for `_overridableAlarms = alarms.Where(a => !a.IsLocked)` — so the fixture template MUST carry a non-locked alarm, else the card shows `"No overridable (non-locked) alarms on this template."` The modal opens on Edit (`@if (_editingAlarm != null)` → `.modal.show.d-block`). **Save with no config-diff AND empty priority DELETES the override** — so the test's reliable "create override" delta is **setting the priority field**. Clear is **immediate, no confirm dialog** (toast `"Cleared override on '{name}'."`). + +**Fixture** (`tests/.../Deployment/InstanceConfigureFixture.cs`): `InitializeAsync` does (on `site-a`, instance NOT deployed): `CreateTemplateAsync(UniqueName("cfgtmpl"))` → `AddAttributeAsync(TemplateId, "Value", "Double", dataSourceReference:"Value")` → `CreateDataConnectionAsync` → `CreateAreaAsync` → `CreateInstanceAsync`. Public: `SiteAId/TemplateId/ConnectionId/AreaId/InstanceId` (int), `AttributeName => "Value"`, `ConnectionName`, `Available`. `SafeCleanupAsync` deletes instance→connection→area→template (template delete cascades children). + +**Test idioms** (`tests/.../Deployment/InstanceConfigureTests.cs`): `[Collection("Playwright")]` + `IClassFixture`, ctor injects `PlaywrightFixture _fixture` + `InstanceConfigureFixture _cfg`; `Skip.IfNot(_cfg.Available, ClusterAvailability.SkipReason)`; `_fixture.NewAuthenticatedPageAsync("multi-role","password")`; navigate `{PlaywrightFixture.BaseUrl}/deployment/instances/{_cfg.InstanceId}/configure`; toast `Assertions.Expect(page.Locator(".toast")).ToHaveCountAsync(1, new(){Timeout=15_000})`; persistence read-back e.g. `using var doc = await CliRunner.GetInstanceDocumentAsync(_cfg.InstanceId); var overrides = doc.RootElement.GetProperty("attributeOverrides"); ...o.GetProperty("attributeName").GetString()...`. The stale TODO is at **line 169**. Alarm overrides are eager-loaded on the instance document → expect a sibling `alarmOverrides` array (each element: `alarmCanonicalName`, `priorityLevelOverride`, `triggerConfigurationOverride`) — confirm the exact camelCase key by reading what the document serializer emits, falling back to `instance alarm-override list` if absent. + +**Part C targets** (verified line numbers): +- `plant-a` seeds in `SiteCalls/SiteCallsPageTests.cs`: lines **125, 129, 179, 229, 234** (seed calls). Line **285** is a *comment* in a relay test that deliberately discusses the cosmetic `plant-a`/unknown-site behavior — **verify each occurrence's intent; only change seeds whose rows must appear in the filtered grid; leave any that intentionally test unknown-site/relay behavior.** +- Pager indicator `var ... = page.Locator("span.text-muted.small")`: `NotificationActionTests.cs:418`, `SiteCallsPageTests.cs:552`. +- Pagination back-leg (re-assert next): `SiteCallsPageTests.cs:572` (after the back-to-page-1 `prev` disabled, add `next` enabled); `NotificationActionTests.cs:437` (same). +- `DeleteRoleMappingAsync` doc note: `CliRunner.Helpers.cs` (the three-token delete helper added in Wave 4). + +**Build map:** A = `dotnet build src/ZB.MOM.WW.ScadaBridge.CLI` (host) + test-project build; **B Task 2 = `bash docker/deploy.sh` (the only docker rebuild)**; B Tasks 3-4 + C = test-project build. **Cadence:** one shared cluster + browser + build → serialize every cluster-running/rebuild task; the docker rebuild in Task 2 must run with NO cluster test in flight (it recreates the containers). + +**Native tasks:** #125–132 (plan Task 0–7). + +--- + +## Part A — CLI ergonomic alarm flags + test helper + +### Task 0: AlarmTriggerConfigJson serializer + typed flags on `template alarm add` + +**Classification:** standard +**Estimated implement time:** ~5 min +**Parallelizable with:** none + +**Files:** +- Create: `src/ZB.MOM.WW.ScadaBridge.CLI/AlarmTriggerConfigJson.cs` +- Modify: `src/ZB.MOM.WW.ScadaBridge.CLI/Commands/TemplateCommands.cs:216-250` (the `add` subcommand inside `BuildAlarm`) + +**Step 1 — create `AlarmTriggerConfigJson`** (CLI-side serializer mirroring the codec's `Serialize`; emits the exact keys; returns `null` when no typed flags are supplied so a bare `alarm add` keeps working): + +```csharp +using System.Text; +using System.Text.Json; + +namespace ZB.MOM.WW.ScadaBridge.CLI; + +/// +/// Serializes typed alarm-setpoint CLI flags into the trigger-config JSON the +/// server expects. Key names MUST stay in lockstep with the canonical codec at +/// src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/AlarmTriggerConfigCodec.cs +/// (that codec is internal to CentralUI, so this is a deliberate CLI-side mirror; +/// the round-trip test verifies the JSON against the live server — the real contract). +/// +internal static class AlarmTriggerConfigJson +{ + /// + /// Builds the trigger-config JSON for from the typed + /// flags, or returns null when none are supplied (so the alarm is created without a + /// trigger config). Unknown/blank trigger types yield null. + /// + internal static string? Build( + string triggerType, string? attribute, + string? matchValue, bool notEquals, + double? min, double? max, + double? thresholdPerSecond, double? windowSeconds, string? direction, + double? loLo, double? lo, double? hi, double? hiHi, + string? expression) + { + var type = triggerType?.Trim(); + // Nothing typed → no config (caller may still pass --trigger-config raw JSON). + var anyTyped = attribute is not null || matchValue is not null || notEquals + || min.HasValue || max.HasValue || thresholdPerSecond.HasValue || windowSeconds.HasValue + || direction is not null || loLo.HasValue || lo.HasValue || hi.HasValue || hiHi.HasValue + || expression is not null; + if (!anyTyped) return null; + + using var stream = new MemoryStream(); + using (var w = new Utf8JsonWriter(stream)) + { + w.WriteStartObject(); + if (!string.Equals(type, "Expression", StringComparison.OrdinalIgnoreCase)) + w.WriteString("attributeName", attribute ?? ""); + + switch (type?.ToLowerInvariant()) + { + case "valuematch": + var mv = matchValue ?? ""; + if (notEquals) mv = "!=" + mv; + w.WriteString("matchValue", mv); + break; + case "rangeviolation": + if (min.HasValue) w.WriteNumber("min", min.Value); + if (max.HasValue) w.WriteNumber("max", max.Value); + break; + case "rateofchange": + if (thresholdPerSecond.HasValue) w.WriteNumber("thresholdPerSecond", thresholdPerSecond.Value); + if (windowSeconds.HasValue) w.WriteNumber("windowSeconds", windowSeconds.Value); + w.WriteString("direction", NormalizeDirection(direction)); + break; + case "hilo": + if (loLo.HasValue) w.WriteNumber("loLo", loLo.Value); + if (lo.HasValue) w.WriteNumber("lo", lo.Value); + if (hi.HasValue) w.WriteNumber("hi", hi.Value); + if (hiHi.HasValue) w.WriteNumber("hiHi", hiHi.Value); + break; + case "expression": + w.WriteString("expression", expression ?? ""); + break; + } + w.WriteEndObject(); + } + return Encoding.UTF8.GetString(stream.ToArray()); + } + + // Mirrors AlarmTriggerConfigCodec.NormalizeDirection. + private static string NormalizeDirection(string? raw) => raw?.ToLowerInvariant() switch + { + "rising" or "up" or "positive" => "rising", + "falling" or "down" or "negative" => "falling", + _ => "either", + }; +} +``` + +**Step 2 — add typed options to the `add` subcommand** in `BuildAlarm` (after the existing `lockedOption`, before `addCmd.SetAction`). Add the `Option<...>` declarations and `addCmd.Add(...)` for each, then thread them into the JSON build: + +```csharp +var attributeOption = new Option("--attribute") { Description = "Monitored attribute name (not for Expression)" }; +var matchValueOption = new Option("--match-value") { Description = "ValueMatch: value to match" }; +var notEqualsOption = new Option("--not-equals") { Description = "ValueMatch: invert the match" }; +notEqualsOption.DefaultValueFactory = _ => false; +var minOption = new Option("--min") { Description = "RangeViolation: minimum" }; +var maxOption = new Option("--max") { Description = "RangeViolation: maximum" }; +var thresholdOption = new Option("--threshold-per-second") { Description = "RateOfChange: threshold per second" }; +var windowOption = new Option("--window-seconds") { Description = "RateOfChange: window seconds" }; +var directionOption = new Option("--direction") { Description = "RateOfChange: rising|falling|either" }; +var loLoOption = new Option("--lolo") { Description = "HiLo: low-low setpoint" }; +var loOption = new Option("--lo") { Description = "HiLo: low setpoint" }; +var hiOption = new Option("--hi") { Description = "HiLo: high setpoint" }; +var hiHiOption = new Option("--hihi") { Description = "HiLo: high-high setpoint" }; +var expressionOption = new Option("--expression") { Description = "Expression: boolean expression" }; +// addCmd.Add(...) for each of the above. +``` + +Then in `addCmd.SetAction`, derive the trigger config — **raw `--trigger-config` wins; otherwise build from typed flags**: + +```csharp +addCmd.SetAction(async (ParseResult result) => +{ + var triggerType = result.GetValue(triggerTypeOption)!; + var rawConfig = result.GetValue(triggerConfigOption); + var triggerConfig = rawConfig ?? AlarmTriggerConfigJson.Build( + triggerType, + result.GetValue(attributeOption), + result.GetValue(matchValueOption), result.GetValue(notEqualsOption), + result.GetValue(minOption), result.GetValue(maxOption), + result.GetValue(thresholdOption), result.GetValue(windowOption), result.GetValue(directionOption), + result.GetValue(loLoOption), result.GetValue(loOption), result.GetValue(hiOption), result.GetValue(hiHiOption), + result.GetValue(expressionOption)); + + return await CommandHelpers.ExecuteCommandAsync( + result, urlOption, formatOption, usernameOption, passwordOption, + new AddTemplateAlarmCommand( + result.GetValue(templateIdOption), result.GetValue(nameOption)!, + triggerType, result.GetValue(priorityOption)!, + result.GetValue(descOption), triggerConfig, result.GetValue(lockedOption))); +}); +``` + +Leave `update`/`delete` untouched (YAGNI). No new DTO, no server change. + +**Step 3 — build the CLI:** `cd /Users/dohertj2/Desktop/ScadaBridge && dotnet build src/ZB.MOM.WW.ScadaBridge.CLI` — clean under `TreatWarningsAsErrors`. (This rebuilds `bin/Debug/net10.0/scadabridge.dll`, which the tests shell.) Smoke: `dotnet build` succeeds; optionally `dotnet src/ZB.MOM.WW.ScadaBridge.CLI/bin/Debug/net10.0/scadabridge.dll template alarm add --help` lists the new options. + +**Step 4 — commit:** `git add src/ZB.MOM.WW.ScadaBridge.CLI/`; commit `feat(cli): typed setpoint flags for template alarm add (serializes trigger-config JSON)`. + +**Acceptance:** CLI builds warning-clean; `template alarm add` accepts the typed flags; raw `--trigger-config` still takes precedence; a bare add (no typed flags) still produces `null` config. No server/`Commons` change (`git diff --stat` shows only `src/...CLI/`). + +--- + +### Task 1: CliRunner.AddAlarmAsync + DeleteInstanceAlarmOverrideAsync helpers + round-trip test + +**Classification:** standard +**Estimated implement time:** ~4 min +**Parallelizable with:** none +**Blocked by:** Task 0 (the CLI must be rebuilt with the typed flags before the round-trip exercises them). + +**Files:** +- Modify: `tests/ZB.MOM.WW.ScadaBridge.CentralUI.PlaywrightTests/Cluster/CliRunner.Helpers.cs` +- Modify: `tests/ZB.MOM.WW.ScadaBridge.CentralUI.PlaywrightTests/Cluster/CliRunnerHelpersTests.cs` + +**Step 1 — add `AddAlarmAsync`** (returns the new alarm id via `RunJsonAsync` + `RequireId`, mirroring `CreateTemplateAsync`; passes typed flags through): + +```csharp +/// +/// Adds an alarm to a template via template alarm add (using the typed setpoint +/// flags) and returns its new id. Throws on failure. +/// +public static async Task AddAlarmAsync( + int templateId, string name, string triggerType = "HiLo", int priority = 500, + string? attribute = null, double? hi = null, double? hiHi = null, + double? lo = null, double? loLo = null) +{ + var inv = System.Globalization.CultureInfo.InvariantCulture; + var args = new List + { + "template", "alarm", "add", + "--template-id", templateId.ToString(inv), + "--name", name, + "--trigger-type", triggerType, + "--priority", priority.ToString(inv), + }; + if (attribute is not null) { args.Add("--attribute"); args.Add(attribute); } + if (hi.HasValue) { args.Add("--hi"); args.Add(hi.Value.ToString(inv)); } + if (hiHi.HasValue) { args.Add("--hihi"); args.Add(hiHi.Value.ToString(inv)); } + if (lo.HasValue) { args.Add("--lo"); args.Add(lo.Value.ToString(inv)); } + if (loLo.HasValue) { args.Add("--lolo"); args.Add(loLo.Value.ToString(inv)); } + + using var doc = await RunJsonAsync([.. args]); + return RequireId(doc, "template alarm add"); +} +``` + +**Step 2 — add `DeleteInstanceAlarmOverrideAsync`** (best-effort teardown for Part B; three-token group → inline try/catch, like `DeleteAreaAsync`): + +```csharp +/// Best-effort delete of an instance alarm override (teardown). Never throws. +public static async Task DeleteInstanceAlarmOverrideAsync(int instanceId, string alarmCanonicalName) +{ + var inv = System.Globalization.CultureInfo.InvariantCulture; + try + { + await RunAsync("instance", "alarm-override", "delete", + "--instance-id", instanceId.ToString(inv), "--alarm", alarmCanonicalName); + } + catch { /* best-effort teardown — never mask the test's own failure. */ } +} +``` + +**Step 3 — round-trip test** in `CliRunnerHelpersTests.cs` (mirror `CreateThenDeleteRoleMapping_RoundTrips`; exercises the typed HiLo flags): + +```csharp +[SkippableFact] +public async Task AddAlarmWithTypedFlags_RoundTrips() +{ + Skip.IfNot(await ClusterAvailability.IsAvailableAsync(), ClusterAvailability.SkipReason); + var id = await CliRunner.CreateTemplateAsync(CliRunner.UniqueName("tmpl")); + try + { + await CliRunner.AddAttributeAsync(id, "Value", "Double"); + var alarmId = await CliRunner.AddAlarmAsync(id, "HiHi", "HiLo", 500, attribute: "Value", hi: 80, hiHi: 95); + Assert.True(alarmId > 0); // server accepted the serialized trigger-config JSON + } + finally { await CliRunner.DeleteTemplateAsync(id); } +} +``` + +**Step 4 — run:** `dotnet test tests/ZB.MOM.WW.ScadaBridge.CentralUI.PlaywrightTests --filter AddAlarmWithTypedFlags_RoundTrips`. Expected PASS (or SKIP if cluster down). **The CLI must be built (Task 0) first** — the test shells the rebuilt dll. + +**Step 5 — commit:** `git add` the two files; commit `test(playwright): CliRunner AddAlarm + alarm-override-delete helpers + round-trip (typed flags)`. + +**Acceptance:** helpers compile warning-clean; round-trip passes against the live cluster (proves the typed flags serialize to JSON the server accepts); zero `zztest-tmpl-*` residue. + +--- + +## Part B — alarm-override coverage (app change → ONE docker rebuild) + +### Task 2: data-test hooks on the InstanceConfigure alarm section + docker rebuild + +**Classification:** standard +**Estimated implement time:** ~4 min (edit) + the `docker/deploy.sh` rebuild wall-time +**Parallelizable with:** none +**Blocked by:** Task 1 (keeps Part A's cluster round-trip ahead of the cluster-recreating rebuild). + +**Files:** +- Modify: `src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/InstanceConfigure.razor` (lines 248, 260, 269-271, 274-276, 320-322, 342) + +**Step 1 — add the 6 additive, non-functional `data-test` attributes** (do NOT change any behavior/markup beyond adding the attribute): +- Line 248 row: `` +- Line 260 badge: `●` +- Lines 269-271 Edit button: add `data-test="alarm-edit-btn"` to the ``. +- Lines 274-276 row Clear button: add `data-test="alarm-clear-btn"` to the ``. +- Lines 320-322 priority input: add `data-test="alarm-priority-input"` to the ``. +- Line 342 Save button: add `data-test="alarm-save-override"` to ``. + +(The row badge/Clear are scoped per row; the Edit button is unique per row too. Since the fixture has exactly one alarm, page-level `[data-test='alarm-edit-btn']` etc. resolve uniquely — but the row uses `alarm-override-row-@alarm.Name` for explicit scoping if ever needed.) + +**Step 2 — rebuild the cluster image so the live UI exposes the hooks:** `cd /Users/dohertj2/Desktop/ScadaBridge && bash docker/deploy.sh`. This is the **only** docker rebuild in this plan; it recreates the 8-node cluster, so **no other cluster test may run during it**. Wait for it to finish and the cluster to be healthy (the next task's `ClusterAvailability` gate will confirm). + +**Step 3 — verify the hooks rendered (optional sanity):** after the rebuild, the alarm section in the running UI exposes the 6 hooks (Task 4 will exercise them). Confirm the build itself was clean (deploy.sh builds the image under the same warnings-as-errors). + +**Step 4 — commit:** `git add src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/InstanceConfigure.razor`; commit `feat(ui): add data-test hooks to InstanceConfigure alarm-override section`. + +**Acceptance:** the 6 hooks are present and additive only (no rendered-behavior change); image rebuilt and cluster healthy. `git diff --stat` for this task shows only `InstanceConfigure.razor`. + +--- + +### Task 3: fixture alarm + +**Classification:** small +**Estimated implement time:** ~3 min +**Parallelizable with:** none +**Blocked by:** Task 1 (needs `AddAlarmAsync`), Task 2 (cluster rebuilt — avoids the rebuild disrupting this task's cluster re-run). + +**Files:** +- Modify: `tests/ZB.MOM.WW.ScadaBridge.CentralUI.PlaywrightTests/Deployment/InstanceConfigureFixture.cs` + +**Step 1 — expose `AlarmName`** (next to `AttributeName`): +```csharp +/// The single non-locked alarm on the fixture template (for the override test). +public string AlarmName => "HiHi"; +``` + +**Step 2 — provision the alarm** in `InitializeAsync`, right after the `AddAttributeAsync` call (line 60). Dogfoods the Part-A typed flags (HiLo on the `Value` Double): +```csharp +await CliRunner.AddAlarmAsync(TemplateId, AlarmName, "HiLo", priority: 500, + attribute: AttributeName, hi: 80, hiHi: 95); +``` +No teardown change needed — `DeleteTemplateAsync` cascades the alarm. + +**Step 3 — re-run the existing InstanceConfigure suite** to confirm the added alarm doesn't perturb the bindings/override/area tests: `dotnet test tests/ZB.MOM.WW.ScadaBridge.CentralUI.PlaywrightTests --filter "FullyQualifiedName~InstanceConfigureTests"`. Expected: all existing tests still PASS (the alarm is just another template child; the existing tests don't touch it). + +**Step 4 — commit:** `git add tests/.../Deployment/InstanceConfigureFixture.cs`; commit `test(playwright): provision a HiLo alarm in InstanceConfigureFixture (via typed CLI flags)`. + +**Acceptance:** fixture builds + provisions the alarm; existing InstanceConfigure tests still green; zero residue (cascade). + +--- + +### Task 4: AlarmOverride_SetPriority_ThenClear_RoundTrips test + +**Classification:** standard +**Estimated implement time:** ~5 min +**Parallelizable with:** none +**Blocked by:** Task 2 (hooks live in the running cluster), Task 3 (fixture alarm). + +**Files:** +- Modify: `tests/ZB.MOM.WW.ScadaBridge.CentralUI.PlaywrightTests/Deployment/InstanceConfigureTests.cs` (add the test; remove the stale TODO at line 169) + +**Step 1 — add the test** (priority override is the reliable delta; assert toast + badge + CLI read-back; Clear; assert badge gone + read-back empty; `finally` cleans the shared instance): + +```csharp +/// +/// Alarm-override round-trip on InstanceConfigure: Edit the fixture's HiLo alarm, set a +/// priority override, Save → toast + "overridden" badge, verify the InstanceAlarmOverride +/// persisted via CLI read-back, then Clear → badge gone + override removed. The fixture +/// instance is SHARED, so the test clears its own override (+ finally) to leave it as found. +/// (Setting only the priority is the reliable "create override" delta — a no-delta Save +/// deletes the override instead. See InstanceConfigure.razor SaveOverrideFromModal.) +/// +[SkippableFact] +public async Task AlarmOverride_SetPriority_ThenClear_RoundTrips() +{ + Skip.IfNot(_cfg.Available, ClusterAvailability.SkipReason); + var page = await _fixture.NewAuthenticatedPageAsync("multi-role", "password"); + try + { + await page.GotoAsync($"{PlaywrightFixture.BaseUrl}/deployment/instances/{_cfg.InstanceId}/configure"); + await page.WaitForLoadStateAsync(LoadState.NetworkIdle); + + var row = page.Locator($"[data-test='alarm-override-row-{_cfg.AlarmName}']"); + await Assertions.Expect(row).ToBeVisibleAsync(new() { Timeout = 15_000 }); + // No override yet. + await Assertions.Expect(row.Locator("[data-test='alarm-override-badge']")).ToHaveCountAsync(0); + + // Edit → modal → set a priority override → Save. + await row.Locator("[data-test='alarm-edit-btn']").ClickAsync(); + var priorityInput = page.Locator("[data-test='alarm-priority-input']"); + await Assertions.Expect(priorityInput).ToBeVisibleAsync(); + await priorityInput.FillAsync("750"); + await page.Locator("[data-test='alarm-save-override']").ClickAsync(); + + // Toast + the badge appears in the row (modal closed, re-rendered). + await Assertions.Expect(page.Locator(".toast")).ToHaveCountAsync(1, new() { Timeout = 15_000 }); + await Assertions.Expect(row.Locator("[data-test='alarm-override-badge']")).ToBeVisibleAsync(new() { Timeout = 15_000 }); + + // Persistence via CLI read-back. + using (var doc = await CliRunner.GetInstanceDocumentAsync(_cfg.InstanceId)) + { + var overrides = doc.RootElement.GetProperty("alarmOverrides"); + Assert.Contains(overrides.EnumerateArray(), o => + o.GetProperty("alarmCanonicalName").GetString() == _cfg.AlarmName + && o.GetProperty("priorityLevelOverride").GetInt32() == 750); + } + + // Clear (immediate, no confirm) → badge gone + override removed. + await row.Locator("[data-test='alarm-clear-btn']").ClickAsync(); + await Assertions.Expect(row.Locator("[data-test='alarm-override-badge']")).ToHaveCountAsync(0, new() { Timeout = 15_000 }); + using (var doc = await CliRunner.GetInstanceDocumentAsync(_cfg.InstanceId)) + { + var overrides = doc.RootElement.GetProperty("alarmOverrides"); + Assert.DoesNotContain(overrides.EnumerateArray(), o => + o.GetProperty("alarmCanonicalName").GetString() == _cfg.AlarmName); + } + } + finally + { + // Shared fixture instance — guarantee no override leaks even if the test failed mid-way. + await CliRunner.DeleteInstanceAlarmOverrideAsync(_cfg.InstanceId, _cfg.AlarmName); + } +} +``` + +Remove the stale `// TODO(wave-N): alarm-override UI coverage ...` line (169). + +**⚠ VALIDATION-BEHAVIOR PROTOCOL:** before finalizing, confirm the instance document's alarm-override key/shape. Read what `GetInstanceDocumentAsync` (`instance get` → the document serializer) actually emits: the array is expected to be `alarmOverrides` with elements `{ alarmCanonicalName, priorityLevelOverride, triggerConfigurationOverride }` (camelCase). If the key differs, assert the real key; if the document does NOT surface overrides, read back via `instance alarm-override list` instead (a `RunJsonAsync("instance","alarm-override","list","--instance-id", id)` call) and assert there. Also confirm the priority field name (`priorityLevelOverride`). Assert reality; note any deviation in a code comment. + +**Step 2 — run:** `dotnet test tests/ZB.MOM.WW.ScadaBridge.CentralUI.PlaywrightTests --filter AlarmOverride_SetPriority_ThenClear_RoundTrips`. Expected PASS. (Requires Task 2's rebuilt cluster so the hooks exist.) + +**Step 3 — commit:** `git add tests/.../Deployment/InstanceConfigureTests.cs`; commit `test(playwright): InstanceConfigure alarm-override set-priority/clear round-trip; drop stale TODO`. + +**Acceptance:** test passes; asserts persistence via CLI read-back both ways; the shared fixture instance is left override-free (badge gone + read-back empty + `finally`). + +--- + +## Part C — cheap hygiene + +### Task 5: SiteCalls hygiene (plant-a→site-a surgical + pager-indicator scope + next re-assert) + +**Classification:** standard +**Estimated implement time:** ~4 min +**Parallelizable with:** Task 6 (disjoint files) +**Blocked by:** none (independent of A/B; may run any time, but cluster-serialized). + +**Files:** +- Modify: `tests/ZB.MOM.WW.ScadaBridge.CentralUI.PlaywrightTests/SiteCalls/SiteCallsPageTests.cs` (lines ~125,129,179,229,234,285,552,572) + +**Step 1 — `plant-a` → `site-a` (SURGICAL).** For EACH `sourceSite: "plant-a"` seed (lines 125,129,179,229,234), read the surrounding test: if the seeded row must appear in the (permitted) grid for the assertion to hold — i.e. FilterNarrowing / DrillIn / RetryDiscard-visibility — change `"plant-a"` → `"site-a"` (removes the system-wide-user fragility). **Do NOT change** any occurrence whose test *intends* an unknown/cosmetic site (the comment at line 285 discusses this deliberately) — leave those and the comment as-is, or refine the comment if a seed it refers to changed. When unsure for a given line, keep `plant-a` and add a one-line comment explaining why it's intentional. + +**Step 2 — scope the pager indicator** (line 552): change `page.Locator("span.text-muted.small")` to a pager-container-scoped locator. Read the `SiteCallsReport.razor` pager markup for the exact wrapper (e.g. `
`); use e.g. `page.Locator(".d-flex.justify-content-between span.text-muted.small")` (or the real wrapper class). Future-proofs against another `span.text-muted.small` being added. + +**Step 3 — re-assert `next` on the back-leg** (after line 572, where the test returns to page 1 and asserts `prev` disabled): add `await Assertions.Expect(page.Locator("[data-test='site-calls-next']")).ToBeEnabledAsync();` so a regression that broke Next after a page-back is caught. + +**Step 4 — run the SiteCalls suite:** `dotnet test tests/ZB.MOM.WW.ScadaBridge.CentralUI.PlaywrightTests --filter "FullyQualifiedName~SiteCallsPageTests"`. Expected: all PASS. + +**Step 5 — commit:** `git add tests/.../SiteCalls/SiteCallsPageTests.cs`; commit `test(playwright): SiteCalls hygiene — site-a seeds, scoped pager locator, next-enabled re-assert`. + +**Acceptance:** SiteCalls suite green; only the intended `plant-a` (if any) remains, documented; pager locator scoped; back-leg asserts `next` enabled. + +--- + +### Task 6: Notification hygiene (pager-indicator scope + next re-assert) + DeleteRoleMappingAsync doc note + +**Classification:** small +**Estimated implement time:** ~3 min +**Parallelizable with:** Task 5 (disjoint files) +**Blocked by:** none. + +**Files:** +- Modify: `tests/ZB.MOM.WW.ScadaBridge.CentralUI.PlaywrightTests/Notifications/NotificationActionTests.cs` (lines ~418, 437) +- Modify: `tests/ZB.MOM.WW.ScadaBridge.CentralUI.PlaywrightTests/Cluster/CliRunner.Helpers.cs` (the `DeleteRoleMappingAsync` doc comment) + +**Step 1 — scope the pager indicator** (line 418): same as Task 5 Step 2 but for `NotificationReport.razor`'s pager wrapper — change `page.Locator("span.text-muted.small")` to the pager-container-scoped locator (read the page for the exact wrapper class). + +**Step 2 — re-assert `next` on the back-leg** (after line 437, the return-to-page-1 `prev` disabled): add `await Assertions.Expect(next).ToBeEnabledAsync();` (`next` is the existing pager-next locator in that test). + +**Step 3 — add the maintenance note to `DeleteRoleMappingAsync`** (CliRunner.Helpers.cs): append to its XML `` the same cross-reference the sibling three-token deletes carry, e.g. *"Three-token CLI group (`security role-mapping`) so it can't use the two-token `BestEffortAsync`; if the teardown idiom changes, update all inline three-token deletes (`DeleteAreaAsync`, `DeleteInstanceAlarmOverrideAsync`, this) together."* + +**Step 4 — run the Notification suite:** `dotnet test tests/ZB.MOM.WW.ScadaBridge.CentralUI.PlaywrightTests --filter "FullyQualifiedName~NotificationActionTests"`. Expected: all PASS. + +**Step 5 — commit:** `git add tests/.../Notifications/NotificationActionTests.cs tests/.../Cluster/CliRunner.Helpers.cs`; commit `test(playwright): Notification hygiene — scoped pager locator, next-enabled re-assert, role-mapping-delete doc note`. + +**Acceptance:** Notification suite green; pager locator scoped; back-leg asserts `next` enabled; the doc note is present. + +--- + +### Task 7: Verification + residue check + +**Classification:** standard +**Estimated implement time:** ~5 min +**Parallelizable with:** none +**Blocked by:** Tasks 0–6. + +**Files:** none (verification; may touch the plan's `.tasks.json`). + +**Step 1 — full build:** `dotnet build src/ZB.MOM.WW.ScadaBridge.CLI` AND `dotnet build tests/ZB.MOM.WW.ScadaBridge.CentralUI.PlaywrightTests` — clean under `TreatWarningsAsErrors`. + +**Step 2 — full suite vs live cluster:** run the entire Playwright project. Expected: **0 failed**; the new round-trip + alarm-override facts pass; skips logged (the known SmtpEdit no-op). Capture the tally. + +**Step 3 — residue scan (must be zero):** `template list` → no `zztest-tmpl-*`; `instance list` → no `zztest-*`; `instance alarm-override list --instance-id ` is N/A post-suite (fixture torn down), but confirm no orphaned override by checking the suite left the fixture instance clean during the run; direct-SQL markers (`AuditLog`/`SiteCalls`/`Notifications` `playwright-test/`/`zztest-notif-*`) → 0; `site-a` left as found. + +**Step 4 — app-diff guard (this effort DOES change `src/`):** `git diff --name-only ..HEAD | grep '^src/'` must list **only** `src/ZB.MOM.WW.ScadaBridge.CLI/...` (Part A) and `src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Deployment/InstanceConfigure.razor` (Part B). Any other `src/` file changed is a defect — investigate. Everything else is under `tests/` or `docs/plans/`. + +**Step 5 — mark complete:** update `…2026-06-07-template-alarm-cli-and-override-coverage.md.tasks.json` → `completed`; commit `docs(plans): mark template-alarm/override plan tasks complete`. + +**Acceptance:** full suite 0-failed; zero residue; clean build; `src/` changes limited to the CLI + the one InstanceConfigure.razor hook addition; `site-a` + fixture left as found. + +--- + +## Scope guard (YAGNI) + +No new server command/DTO (Part A rides the existing `AddTemplateAlarmCommand`). No HiLo per-level priority/deadband/message flags (raw `--trigger-config` remains the escape hatch). The codec is NOT moved (CLI-side mirror + round-trip verification). The `data-test` hooks are additive-only. No new fixtures (extend `InstanceConfigureFixture`). Part C is surgical — no suite-wide `WaitForLoadState` refactor, no blind `plant-a` rewrite. + +## Success criteria + +`template alarm add` accepts typed setpoint flags (round-trip-verified); `InstanceConfigure`'s alarm-override set-priority→badge→clear flow has functional coverage asserting persistence via CLI read-back; the cheap hygiene items are applied; full suite green with logged skips; zero residue; `src/` touched only in the CLI + the single InstanceConfigure hook commit; `site-a` and the shared fixture instance left as found. diff --git a/docs/plans/2026-06-07-template-alarm-cli-and-override-coverage.md.tasks.json b/docs/plans/2026-06-07-template-alarm-cli-and-override-coverage.md.tasks.json new file mode 100644 index 00000000..e025a789 --- /dev/null +++ b/docs/plans/2026-06-07-template-alarm-cli-and-override-coverage.md.tasks.json @@ -0,0 +1,16 @@ +{ + "planPath": "docs/plans/2026-06-07-template-alarm-cli-and-override-coverage.md", + "lastUpdated": "2026-06-07T00:00:00Z", + "nativeTaskIdBase": 125, + "status": "pending", + "tasks": [ + {"id": 0, "nativeId": 125, "subject": "Task 0: CLI AlarmTriggerConfigJson + typed flags on template alarm add", "status": "pending"}, + {"id": 1, "nativeId": 126, "subject": "Task 1: CliRunner.AddAlarmAsync + DeleteInstanceAlarmOverrideAsync + round-trip test", "status": "pending", "blockedBy": [0]}, + {"id": 2, "nativeId": 127, "subject": "Task 2: data-test hooks on InstanceConfigure alarm section + docker rebuild", "status": "pending", "blockedBy": [1]}, + {"id": 3, "nativeId": 128, "subject": "Task 3: fixture alarm in InstanceConfigureFixture", "status": "pending", "blockedBy": [1, 2]}, + {"id": 4, "nativeId": 129, "subject": "Task 4: AlarmOverride_SetPriority_ThenClear_RoundTrips test", "status": "pending", "blockedBy": [2, 3]}, + {"id": 5, "nativeId": 130, "subject": "Task 5: SiteCalls hygiene (plant-a surgical + pager scope + next re-assert)", "status": "pending"}, + {"id": 6, "nativeId": 131, "subject": "Task 6: Notification hygiene + DeleteRoleMappingAsync doc note", "status": "pending"}, + {"id": 7, "nativeId": 132, "subject": "Task 7: Verification + residue check", "status": "pending", "blockedBy": [0, 1, 2, 3, 4, 5, 6]} + ] +}