Joseph Doherty
522 lines
36 KiB
Markdown
522 lines
36 KiB
Markdown
# 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<string>` 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 `<tr>` at **line 248** (inside `@foreach (var alarm in _overridableAlarms)`).
|
||
- Edit button at **lines 269-271** (`@onclick="() => BeginEditOverride(alarm)"`).
|
||
- Override badge `<span class="badge bg-warning text-dark me-1">●</span>` at **line 260** (rendered only when `HasOverride(alarm.Name)`).
|
||
- Row Clear button at **lines 274-276** (rendered only when `HasOverride`).
|
||
- Modal priority input `<input type="number" min="0" max="1000" ... @bind="_editingPriorityText" ...>` at **lines 320-322**.
|
||
- Modal Save button `<button class="btn btn-success btn-sm" @onclick="SaveOverrideFromModal" ...>Save Override</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<InstanceConfigureFixture>`, 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;
|
||
|
||
/// <summary>
|
||
/// 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).
|
||
/// </summary>
|
||
internal static class AlarmTriggerConfigJson
|
||
{
|
||
/// <summary>
|
||
/// Builds the trigger-config JSON for <paramref name="triggerType"/> 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.
|
||
/// </summary>
|
||
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<string?>("--attribute") { Description = "Monitored attribute name (not for Expression)" };
|
||
var matchValueOption = new Option<string?>("--match-value") { Description = "ValueMatch: value to match" };
|
||
var notEqualsOption = new Option<bool>("--not-equals") { Description = "ValueMatch: invert the match" };
|
||
notEqualsOption.DefaultValueFactory = _ => false;
|
||
var minOption = new Option<double?>("--min") { Description = "RangeViolation: minimum" };
|
||
var maxOption = new Option<double?>("--max") { Description = "RangeViolation: maximum" };
|
||
var thresholdOption = new Option<double?>("--threshold-per-second") { Description = "RateOfChange: threshold per second" };
|
||
var windowOption = new Option<double?>("--window-seconds") { Description = "RateOfChange: window seconds" };
|
||
var directionOption = new Option<string?>("--direction") { Description = "RateOfChange: rising|falling|either" };
|
||
var loLoOption = new Option<double?>("--lolo") { Description = "HiLo: low-low setpoint" };
|
||
var loOption = new Option<double?>("--lo") { Description = "HiLo: low setpoint" };
|
||
var hiOption = new Option<double?>("--hi") { Description = "HiLo: high setpoint" };
|
||
var hiHiOption = new Option<double?>("--hihi") { Description = "HiLo: high-high setpoint" };
|
||
var expressionOption = new Option<string?>("--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
|
||
/// <summary>
|
||
/// Adds an alarm to a template via <c>template alarm add</c> (using the typed setpoint
|
||
/// flags) and returns its new <c>id</c>. Throws on failure.
|
||
/// </summary>
|
||
public static async Task<int> 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<string>
|
||
{
|
||
"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
|
||
/// <summary>Best-effort delete of an instance alarm override (teardown). Never throws.</summary>
|
||
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: `<tr data-test="alarm-override-row-@alarm.Name">`
|
||
- Line 260 badge: `<span class="badge bg-warning text-dark me-1" data-test="alarm-override-badge" title="Override is set">●</span>`
|
||
- Lines 269-271 Edit button: add `data-test="alarm-edit-btn"` to the `<button class="btn btn-outline-primary btn-sm me-1" ...>Edit</button>`.
|
||
- Lines 274-276 row Clear button: add `data-test="alarm-clear-btn"` to the `<button class="btn btn-outline-danger btn-sm" ...>Clear</button>`.
|
||
- Lines 320-322 priority input: add `data-test="alarm-priority-input"` to the `<input type="number" min="0" max="1000" ...>`.
|
||
- Line 342 Save button: add `data-test="alarm-save-override"` to `<button class="btn btn-success btn-sm" @onclick="SaveOverrideFromModal" ...>Save Override</button>`.
|
||
|
||
(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
|
||
/// <summary>The single non-locked alarm on the fixture template (for the override test).</summary>
|
||
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
|
||
/// <summary>
|
||
/// 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.)
|
||
/// </summary>
|
||
[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. `<div class="d-flex justify-content-between align-items-center">`); 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 `<remarks>` 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 <fixture InstanceId>` 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 <plan-base>..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.
|