From 40944a0d04b76d8d0d5463be195589210675f394 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 15 Aug 2026 21:33:08 -0400 Subject: [PATCH] =?UTF-8?q?feat(centralui):=20row=2075=20=E2=80=94=20Appro?= =?UTF-8?q?ve=20is=20caution=20amber,=20not=20destructive=20red?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Secured Writes page rendered Approve and Reject as IDENTICAL red buttons. Row 28 fixed the labels but left the styling, and the styling was the worse half: at the moment of a two-person-control decision the colour carried no discriminating information at all, and the affirmative half of the control wore the delete affordance. Root cause is that `danger` was a two-state flag for a three-state concept, so a consequential-but-not-destructive action had to choose between looking like a delete and looking like the safe default. Adds the missing middle rung: DialogSeverity { Normal -> btn-primary, Caution -> btn-warning, Destructive -> btn-danger } DECISION. Approve -> Caution (amber): approving executes a real write to live plant equipment, so it is not a benign primary action, but it is not destroying anything either. Reject KEEPS Destructive (red): it is terminal and irreversible for the request — the Operator must resubmit — and holding it red is what keeps the two opposite decisions visually distinct now that Approve is amber. Dropping Reject to neutral was considered and rejected: it would invert the bias, making declining look like the safe default. Colour is never the only signal — both buttons carry their verb. Only Destructive inherits the row-28 "Delete" label fallback; an unlabelled Caution reads "Confirm", since a caution action is by definition not a delete — otherwise row 28 would come back through the new door. ADDITIVE OVERLOAD, deliberately. The boolean form stays the signature for all ~34 existing call sites, which are genuinely two-state, so nothing in src/ outside SecuredWrites.razor changed. Adding a fifth optional parameter to the boolean form instead would have silently un-matched every existing four-argument NSubstitute stub — the exact row-28 gotcha (null Task, caller NREs, no compile error). The overload cannot: the only breakage was six hand-written IDialogService fakes failing to compile, which is the loud failure mode. That gotcha did land once, in SecuredWritesTests' own stub, and is now stubbed on both overloads with a comment saying why. DialogState's `bool Danger` became `DialogSeverity Severity` rather than gaining a second field — it is read only by DialogHost, and one field cannot disagree with itself. Fail-before, both halves: - Approve call site reverted to `danger: true` -> both new Approve pins fail. - Caution remapped to btn-danger in DialogHost -> the amber styling pin fails. Tests: DialogSeverityStylingTests renders the REAL DialogService behind a real DialogHost (FailoverConfirmLabelTests' harness), so it pins what the user sees, not what an argument was — all three severities, both label fallbacks, and the boolean adapter. SecuredWritesTests pins that the page asks for Caution, with a negative control that it did NOT also go through the boolean form (without which the assertion would pass on a call site left at danger:true). Reject's red is pinned too, so a later "make this page less alarming" pass cannot flatten both to one colour. No Playwright selectors needed updating — the suite has no Secured Writes coverage, and no `.btn-danger` selector elsewhere targets this page. Docs: Component-CentralUI.md gains the severity contract under Technology (next to the row-28 label contract) plus the decision on the Secured Writes section. CentralUI.Tests: 1055/1055 green. --- docs/requirements/Component-CentralUI.md | 7 +- .../Pages/Operations/SecuredWrites.razor | 14 +- .../Components/Shared/DialogHost.razor | 25 +++- .../Components/Shared/DialogService.cs | 58 ++++++++- .../Components/Shared/IDialogService.cs | 34 ++++- .../Components/SecuredWritesTests.cs | 58 ++++++++- .../Design/SchemaLibraryPageTests.cs | 6 + .../Pages/NotificationListsPageTests.cs | 6 + .../NotificationReportDetailModalTests.cs | 6 + .../Pages/NotificationReportPageTests.cs | 6 + .../Pages/QueryStringDrillInTests.cs | 6 + .../Pages/SiteCallsReportPageTests.cs | 6 + .../Shared/DialogSeverityStylingTests.cs | 123 ++++++++++++++++++ 13 files changed, 340 insertions(+), 15 deletions(-) create mode 100644 tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Shared/DialogSeverityStylingTests.cs diff --git a/docs/requirements/Component-CentralUI.md b/docs/requirements/Component-CentralUI.md index 8fe002eb..c1a65cd1 100644 --- a/docs/requirements/Component-CentralUI.md +++ b/docs/requirements/Component-CentralUI.md @@ -14,6 +14,11 @@ Central cluster only. Sites have no user interface. - Keeps the entire stack in C#/.NET, consistent with the rest of the system (Akka.NET, EF Core). - SignalR provides built-in support for real-time UI updates. - **Dialog service confirm labels.** Every confirmation goes through `IDialogService.ConfirmAsync(title, message, danger, confirmLabel)`, rendered by the single `DialogHost` in `MainLayout`. `danger` selects **styling only** (`btn-danger`); `confirmLabel` names the action. When no label is supplied the button falls back to "Delete" for a danger confirm and "Confirm" otherwise — a convenience for the ~20 genuine delete call sites, and the reason the Health dashboard's *Trigger failover* dialog used to offer a red **Delete** button (deferred-work register row 28). **Any destructive action that is not literally a delete must pass `confirmLabel`**: failover → *Fail over*, secured-write approve/reject → *Approve* / *Reject*, instance disable → *Disable*, and every parked-message / notification / cached-call discard → *Discard*. +- **Dialog confirm severity — three rungs, not two** (deferred-work register row 75). `DialogSeverity` drives the confirm button's colour: `Normal` → `btn-primary`, `Caution` → `btn-warning`, `Destructive` → `btn-danger`. Only `Destructive` inherits the "Delete" label fallback above; an unlabelled `Caution` reads *Confirm*, because a caution action is by definition not a delete. + - **Why a middle rung exists.** The boolean was a two-state flag for a three-state concept, so a consequential-but-not-destructive action had to choose between looking like a delete and looking like the safe default. The **Secured Writes** page is where that bit: *Approve* and *Reject* both rendered as identical red buttons, so at the moment of a two-person-control decision the colour carried **no discriminating information at all** — and the affirmative half of the control wore the delete affordance. + - **The decision.** *Approve* is `Caution` (amber): approving executes a real write to live plant equipment, so it is not a benign primary action, but it is not destroying anything either. *Reject* **keeps** `Destructive` (red): it is terminal and irreversible for the request — the Operator must resubmit — and holding it red is what keeps the two opposite decisions visually distinct. Dropping *Reject* to a neutral button was considered and rejected: it would simply invert the bias, making declining look like the safe default. + - Colour is never the only signal — the button always carries its verb (see the label contract above), so the mapping is an accessibility affordance, not the meaning. + - **`DialogSeverity` is an additive overload**, not a replacement: the boolean form remains the signature for the ~34 existing call sites, which are genuinely two-state. Adding a fifth optional parameter to the boolean form instead would have silently un-matched every existing four-argument NSubstitute stub — the row-28 gotcha, where a stub that stops matching returns a null `Task` and the caller NREs with no compile error. Reach for the overload only when `Caution` is what you mean. ## Failover Behavior @@ -226,7 +231,7 @@ Per-leaf alarm rendering (leaf nodes are individual conditions for native alarms - A **Secured Writes** page (`/operations/secured-writes`) drives the **two-person** authorization workflow for writes through the MxAccess Gateway: an **Operator** initiates the write, a separate **Verifier** approves it, and only an approved write reaches the site. - The **page itself is gated by `RequireSecuredWriteAccess`** — satisfied by any of `Operator` / `Verifier` / `Administrator`. Although the pending/history lists are read-only, they expose process-sensitive tag values, so the page is not open to every authenticated user; this aligns with the ManagementActor `ListSecuredWritesCommand` any-of gate (arch-review UA1). The submit and approve/reject sub-actions are further gated by `RequireOperator` / `RequireVerifier` respectively (below). - **Operator (submit)** — a submit form gated by `RequireOperator`: pick the site → an **MxGateway** connection on that site → the tag path → a typed value → an optional comment. Submission inserts a `Pending` `PendingSecuredWrite` row centrally; it does **not** write anything yet. -- **Verifier (approve / reject)** — a pending queue gated by `RequireVerifier` with **Approve** / **Reject** (+comment) actions. Approve shows a confirmation of the exact site / connection / tag / value before firing, and surfaces the **submission age** (`Submitted ago ()`) so a verifier can catch a stale setpoint before it reaches the device. The verifier's **own submissions are disabled in the UI and rejected server-side** (no self-approval). On approve, central marks the row `Approved` and relays the write to the site MxGateway (records `Executed` / `Failed`); reject moves it to `Rejected` with a reason. The pending queue is **unpaged** — it is bounded by the submission **TTL**: a `Pending` row that is neither approved nor rejected before the TTL elapses expires to the terminal **`Expired`** status and drops out of the queue into History. +- **Verifier (approve / reject)** — a pending queue gated by `RequireVerifier` with **Approve** / **Reject** (+comment) actions. Approve shows a confirmation of the exact site / connection / tag / value before firing, and surfaces the **submission age** (`Submitted ago ()`) so a verifier can catch a stale setpoint before it reaches the device. The two confirmations are deliberately styled apart: **Approve is amber (`DialogSeverity.Caution`) and Reject stays red** — see the confirm-severity contract under Technology for why (register row 75). The verifier's **own submissions are disabled in the UI and rejected server-side** (no self-approval). On approve, central marks the row `Approved` and relays the write to the site MxGateway (records `Executed` / `Failed`); reject moves it to `Rejected` with a reason. The pending queue is **unpaged** — it is bounded by the submission **TTL**: a `Pending` row that is neither approved nor rejected before the TTL elapses expires to the terminal **`Expired`** status and drops out of the queue into History. - **History** — terminal rows (Executed / Failed / Rejected / **Expired**, where `Expired` is a real TTL-driven terminal status) with the full who/when/outcome trail (operator, verifier, comments, timestamps, any execution error). Terminal rows accumulate without bound, so the History table is **offset-paged** (page-size 50, Prev/Next) driven by the server's unpaged `TotalCount` (`ListSecuredWritesCommand` `Skip`/`Take`); the pager buttons disable at the real bounds. - Every lifecycle event (submit / approve / reject / execute) is written to the central Audit Log; the rows share the `PendingSecuredWrite.Id` as `CorrelationId` so they join into one operation (see Component-ManagementService.md, Component-AuditLog.md). - **Dev caveat**: with `DisableLogin` on, the auto-login identity holds all roles, so the two-person flow cannot be exercised end-to-end by a single user via the dev UI — no-self-approval is covered by handler tests; real two-person use requires two real identities. diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Operations/SecuredWrites.razor b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Operations/SecuredWrites.razor index cbe873e3..e1ee95b4 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Operations/SecuredWrites.razor +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Operations/SecuredWrites.razor @@ -424,7 +424,13 @@ $"Value: {row.ValueJson} ({row.ValueType})\n" + $"Submitted {FormatAge(row.SubmittedAtUtc)} ago ({row.SubmittedAtUtc:u})\n" + $"Operator: {row.OperatorUser}", - danger: true, + // Caution, NOT danger (deferred-work register row 75). Approving executes a + // real write to live plant equipment, so this is not a benign primary action + // — but it is not destroying anything either, and red made it look like a + // delete while rendering it identical to the Reject button right beside it. + // Amber keeps the beat of hesitation the write deserves and restores the + // colour contrast between the two opposite decisions. + DialogSeverity.Caution, confirmLabel: "Approve"); if (!confirmed) return; @@ -461,6 +467,12 @@ var confirmed = await Dialog.ConfirmAsync( "Reject secured write", $"Reject the secured write to {row.SiteId}/{row.ConnectionName}/{row.TagPath}?", + // Reject KEEPS danger styling (register row 75). It is terminal and + // irreversible for the request — the Operator has to resubmit — and holding + // it red is what makes the pair readable: with Approve amber, the two + // opposite decisions no longer look alike. Dropping Reject to a neutral + // button would just invert the bias, making declining look like the safe + // default. danger: true, confirmLabel: "Reject"); if (!confirmed) return; diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/DialogHost.razor b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/DialogHost.razor index 22bac228..ab5bea09 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/DialogHost.razor +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/DialogHost.razor @@ -50,7 +50,7 @@
@@ -185,17 +185,32 @@ svc.Resolve(resolveValue); } - // An explicit label from the caller always wins. The kind/danger fallback below - // is a convenience for the common cases only: a bare danger confirm is assumed to - // be a delete, which is exactly the assumption that put "Delete" on the Trigger + // Bootstrap class for the confirm button, by severity (deferred-work register row 75). + // Caution is the middle rung: amber, for an action that is consequential but is not + // destroying anything — the Secured Writes "Approve" is the case it was added for, + // where red made the affirmative half of a two-person control look like a delete and + // made Approve and Reject visually indistinguishable. Colour is never the only signal: + // the button is always labelled with its verb. + private static string ConfirmButtonClass(DialogState state) => state.Severity switch + { + DialogSeverity.Destructive => "btn-danger", + DialogSeverity.Caution => "btn-warning", + _ => "btn-primary", + }; + + // An explicit label from the caller always wins. The kind/severity fallback below + // is a convenience for the common cases only: a bare destructive confirm is assumed + // to be a delete, which is exactly the assumption that put "Delete" on the Trigger // failover button (deferred-work register row 28). Callers whose destructive verb // is not "Delete" must pass ConfirmLabel — see IDialogService.ConfirmAsync. + // Note Caution deliberately does NOT inherit that fallback: an unlabelled caution + // dialog reads "Confirm", because a caution action is by definition not a delete. private static string ConfirmLabel(DialogState state) => !string.IsNullOrWhiteSpace(state.ConfirmLabel) ? state.ConfirmLabel : state.Kind switch { DialogKind.Prompt => "Save", - _ => state.Danger ? "Delete" : "Confirm", + _ => state.Severity == DialogSeverity.Destructive ? "Delete" : "Confirm", }; } diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/DialogService.cs b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/DialogService.cs index 3baa15e3..d2d457c7 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/DialogService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/DialogService.cs @@ -35,7 +35,15 @@ public class DialogService : IDialogService private TaskCompletionSource? _tcs; /// - public Task ConfirmAsync(string title, string message, bool danger = false, string? confirmLabel = null) + public Task ConfirmAsync(string title, string message, bool danger = false, string? confirmLabel = null) => + ConfirmAsync( + title, + message, + danger ? DialogSeverity.Destructive : DialogSeverity.Normal, + confirmLabel); + + /// + public Task ConfirmAsync(string title, string message, DialogSeverity severity, string? confirmLabel = null) { EnsureNoActiveDialog(); var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); @@ -44,7 +52,7 @@ public class DialogService : IDialogService title, DialogKind.Confirm, message, - danger, + severity, PromptInitial: string.Empty, Placeholder: null, ConfirmLabel: confirmLabel); @@ -58,7 +66,7 @@ public class DialogService : IDialogService EnsureNoActiveDialog(); var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); _tcs = tcs; - Current = new DialogState(title, DialogKind.Prompt, label, Danger: false, PromptInitial: initialValue, Placeholder: placeholder); + Current = new DialogState(title, DialogKind.Prompt, label, DialogSeverity.Normal, PromptInitial: initialValue, Placeholder: placeholder); OnChange?.Invoke(); return Project(tcs.Task, static r => r as string); } @@ -85,7 +93,7 @@ public class DialogService : IDialogService title, DialogKind.Custom, Body: string.Empty, - Danger: false, + DialogSeverity.Normal, PromptInitial: string.Empty, Placeholder: null, Content: content, @@ -146,7 +154,9 @@ public class DialogService : IDialogService /// Modal title text. /// Discriminates between confirm, prompt, and custom rendering. /// For confirm: the message; for prompt: the input label; ignored for custom. -/// When true, the confirm button uses danger styling. +/// Drives the confirm button's styling, and the verb the host +/// falls back to when is blank. Single source of truth +/// for both — there is deliberately no separate boolean, so the two cannot disagree. /// Initial value for prompt-kind dialogs. /// Placeholder shown when the prompt input is empty. /// For : the body fragment to @@ -160,7 +170,7 @@ public record DialogState( string Title, DialogKind Kind, string Body, - bool Danger, + DialogSeverity Severity, string PromptInitial, string? Placeholder, RenderFragment? Content = null, @@ -173,3 +183,39 @@ public enum DialogKind Prompt, Custom } + +/// +/// How consequential the confirmed action is — drives the confirm button's colour. +/// +/// +/// Deferred-work register row 75. The dialog surface used to carry a two-state +/// bool danger, which forced every consequential-but-not-destructive action to +/// choose between looking like a delete and looking like the safe default. The Secured +/// Writes page is where that bit them: Approve and Reject both rendered +/// as identical red buttons, so at the moment of a two-person-control decision the colour +/// carried no discriminating information at all — and the affirmative half of the control +/// was painted with the delete affordance. +/// +/// is the middle rung that was missing. It is for actions that are +/// consequential and worth a beat of hesitation, but are not destroying anything: the +/// canonical case is a write to live plant equipment. +/// +/// +public enum DialogSeverity +{ + /// Ordinary confirmation — renders btn-primary. + Normal, + + /// + /// Consequential but not destructive — renders btn-warning. Does NOT inherit + /// the "Delete" label fallback; a caution dialog with no explicit label reads + /// "Confirm". + /// + Caution, + + /// + /// Destructive or irreversible — renders btn-danger, and is the only severity + /// that falls back to the "Delete" verb (see DialogHost.ConfirmLabel). + /// + Destructive +} diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/IDialogService.cs b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/IDialogService.cs index 0b90d48e..6a7fd9de 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/IDialogService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/IDialogService.cs @@ -20,7 +20,10 @@ public interface IDialogService /// Body text shown to the user. /// When true, the confirm button renders in /// btn-danger styling; otherwise a primary button is shown. This - /// controls styling only — it does not name the action. + /// controls styling only — it does not name the action. For an action that + /// is consequential but NOT destructive, use the + /// overload and pass rather than reaching for + /// red (deferred-work register row 75). /// Verb rendered on the confirm button. When /// null or blank the button falls back to "Delete" for a danger /// dialog and "Confirm" otherwise. Any destructive action that is @@ -31,6 +34,35 @@ public interface IDialogService /// A task that resolves to true when the user confirms, or false when cancelled. Task ConfirmAsync(string title, string message, bool danger = false, string? confirmLabel = null); + /// + /// Shows a confirmation dialog with an explicit , for + /// actions the danger boolean cannot describe. + /// + /// + /// Deferred-work register row 75. This is an ADDITIVE overload, deliberately: the + /// boolean form stays the signature for the ~34 existing call sites, all of which are + /// genuinely two-state. Changing the boolean's type instead would have rippled into + /// pages this change has no business touching, and adding a fifth optional parameter + /// to the boolean form would have silently un-matched every existing four-argument + /// NSubstitute stub — the exact gotcha register row 28 recorded (a stub that stops + /// matching returns a null Task and the caller NREs, with no compile error). A + /// separate overload cannot do either. + /// + /// Reach for this only when is what you mean. + /// For Normal / Destructive the boolean form says the same thing more briefly. + /// + /// + /// Modal title text. + /// Body text shown to the user. + /// How consequential the action is; selects the confirm + /// button's styling. + /// Verb rendered on the confirm button. Only + /// falls back to "Delete" when this is + /// blank; the other severities fall back to "Confirm". The row-28 rule still + /// applies — name the action. + /// A task that resolves to true when the user confirms, or false when cancelled. + Task ConfirmAsync(string title, string message, DialogSeverity severity, string? confirmLabel = null); + /// /// Shows a single-line text prompt and resolves to the entered value, or /// null if the user cancels. diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Components/SecuredWritesTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Components/SecuredWritesTests.cs index d57ba929..6dcbf49a 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Components/SecuredWritesTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Components/SecuredWritesTests.cs @@ -45,8 +45,14 @@ public class SecuredWritesTests : BunitContext Services.AddSingleton(_browse); // Confirm dialogs resolve to "yes" without a DialogHost in the render scope. + // BOTH overloads must be stubbed: Reject takes the bool form, Approve takes the + // DialogSeverity form (register row 75). An unstubbed NSubstitute call returns a + // null Task, so missing one here surfaces as an NRE inside the click handler + // rather than as anything that names the dialog — the row-28 gotcha. _dialog.ConfirmAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) .Returns(Task.FromResult(true)); + _dialog.ConfirmAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(true)); // Empty list by default; individual tests seed the queue/history. _service.ListAsync( @@ -303,10 +309,60 @@ public class SecuredWritesTests : BunitContext _dialog.Received(1).ConfirmAsync( Arg.Any(), Arg.Is(m => Regex.IsMatch(m, "Submitted .* ago")), - Arg.Any(), + Arg.Any(), Arg.Any()); } + // ── (f) Approve/Reject confirm-button severity (register row 75) ── + + [Fact] + public void Approve_ConfirmDialog_IsCautionNotDestructive() + { + // Row 75. Approving executes a real write to live plant equipment, so it is not a + // benign primary action — but it is not a delete either, and red made it both + // look like one AND look identical to the Reject button beside it, leaving colour + // carrying no information at the moment of a two-person-control decision. + AddAuth("verifier-user", Roles.Verifier); + SeedSites(new Site("Plant-A", "plant-a") { Id = 1 }); + SeedList(new[] { Dto(11, "Pending", "someone-else") }); + _service.ApproveAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(SecuredWriteActionResult.Ok(Dto(11, "Executed", "someone-else", "verifier-user")))); + + var cut = RenderPage(); + cut.WaitForAssertion(() => Assert.NotNull(cut.Find("[data-test='secured-write-approve']"))); + cut.Find("[data-test='secured-write-approve']").Click(); + + _dialog.Received(1).ConfirmAsync( + Arg.Any(), Arg.Any(), DialogSeverity.Caution, "Approve"); + + // Negative control: it must not ALSO have gone through the boolean form, which is + // the one that renders red. Without this the assertion above would still pass if + // the call site had been left on danger:true and merely gained a severity call. + _dialog.DidNotReceive().ConfirmAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); + } + + [Fact] + public void Reject_ConfirmDialog_KeepsDestructiveStyling() + { + // Row 75 decided Reject KEEPS red: it is terminal and irreversible for the request + // (the Operator must resubmit), and holding it red is what keeps the two opposite + // decisions visually distinct now that Approve is amber. Pinned so a later + // "make the page less alarming" pass cannot quietly flatten both to one colour. + AddAuth("verifier-user", Roles.Verifier); + SeedSites(new Site("Plant-A", "plant-a") { Id = 1 }); + SeedList(new[] { Dto(12, "Pending", "someone-else") }); + _service.RejectAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(SecuredWriteActionResult.Ok(Dto(12, "Rejected", "someone-else", "verifier-user")))); + + var cut = RenderPage(); + cut.WaitForAssertion(() => Assert.NotNull(cut.Find("[data-test='secured-write-reject']"))); + cut.Find("[data-test='secured-write-reject']").Click(); + + _dialog.Received(1).ConfirmAsync( + Arg.Any(), Arg.Any(), danger: true, confirmLabel: "Reject"); + } + [Fact] public void Viewer_DoesNotSeeOperatorOrVerifierRegions() { diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Design/SchemaLibraryPageTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Design/SchemaLibraryPageTests.cs index c9182cf0..2d4ad654 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Design/SchemaLibraryPageTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Design/SchemaLibraryPageTests.cs @@ -213,6 +213,12 @@ public class SchemaLibraryPageTests : BunitContext public Task ConfirmAsync(string title, string message, bool danger = false, string? confirmLabel = null) => Task.FromResult(true); + // Register row 75 added a DialogSeverity overload. This stub auto-confirms + // regardless of severity — it exists to let action paths run end to end, not to + // assert anything about styling. + public Task ConfirmAsync(string title, string message, DialogSeverity severity, string? confirmLabel = null) + => Task.FromResult(true); + public Task PromptAsync( string title, string label, string initialValue = "", string? placeholder = null) => Task.FromResult(null); diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/NotificationListsPageTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/NotificationListsPageTests.cs index 196f7818..d8efa79f 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/NotificationListsPageTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/NotificationListsPageTests.cs @@ -194,6 +194,12 @@ public class NotificationListsPageTests : BunitContext public Task ConfirmAsync(string title, string message, bool danger = false, string? confirmLabel = null) => Task.FromResult(true); + // Register row 75 added a DialogSeverity overload. This stub auto-confirms + // regardless of severity — it exists to let action paths run end to end, not to + // assert anything about styling. + public Task ConfirmAsync(string title, string message, DialogSeverity severity, string? confirmLabel = null) + => Task.FromResult(true); + public Task PromptAsync( string title, string label, string initialValue = "", string? placeholder = null) => Task.FromResult(null); diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/NotificationReportDetailModalTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/NotificationReportDetailModalTests.cs index d1daade5..22947bd3 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/NotificationReportDetailModalTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/NotificationReportDetailModalTests.cs @@ -339,6 +339,12 @@ public class NotificationReportDetailModalTests : BunitContext public Task ConfirmAsync(string title, string message, bool danger = false, string? confirmLabel = null) => Task.FromResult(true); + // Register row 75 added a DialogSeverity overload. This stub auto-confirms + // regardless of severity — it exists to let action paths run end to end, not to + // assert anything about styling. + public Task ConfirmAsync(string title, string message, DialogSeverity severity, string? confirmLabel = null) + => Task.FromResult(true); + public Task PromptAsync( string title, string label, string initialValue = "", string? placeholder = null) => Task.FromResult(null); diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/NotificationReportPageTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/NotificationReportPageTests.cs index 7d9fda24..bb6a97b1 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/NotificationReportPageTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/NotificationReportPageTests.cs @@ -274,6 +274,12 @@ public class NotificationReportPageTests : BunitContext public Task ConfirmAsync(string title, string message, bool danger = false, string? confirmLabel = null) => Task.FromResult(true); + // Register row 75 added a DialogSeverity overload. This stub auto-confirms + // regardless of severity — it exists to let action paths run end to end, not to + // assert anything about styling. + public Task ConfirmAsync(string title, string message, DialogSeverity severity, string? confirmLabel = null) + => Task.FromResult(true); + public Task PromptAsync( string title, string label, string initialValue = "", string? placeholder = null) => Task.FromResult(null); diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/QueryStringDrillInTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/QueryStringDrillInTests.cs index 72521239..de1864aa 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/QueryStringDrillInTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/QueryStringDrillInTests.cs @@ -213,6 +213,12 @@ public sealed class QueryStringDrillInTests { public Task ConfirmAsync(string title, string message, bool danger = false, string? confirmLabel = null) => Task.FromResult(true); + + // Register row 75 added a DialogSeverity overload. This stub auto-confirms + // regardless of severity — it exists to let action paths run end to end, not to + // assert anything about styling. + public Task ConfirmAsync(string title, string message, DialogSeverity severity, string? confirmLabel = null) + => Task.FromResult(true); public Task PromptAsync(string title, string label, string initialValue = "", string? placeholder = null) => Task.FromResult(null); diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/SiteCallsReportPageTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/SiteCallsReportPageTests.cs index e7629476..71477834 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/SiteCallsReportPageTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/SiteCallsReportPageTests.cs @@ -682,6 +682,12 @@ public class SiteCallsReportPageTests : BunitContext public Task ConfirmAsync(string title, string message, bool danger = false, string? confirmLabel = null) => Task.FromResult(true); + // Register row 75 added a DialogSeverity overload. This stub auto-confirms + // regardless of severity — it exists to let action paths run end to end, not to + // assert anything about styling. + public Task ConfirmAsync(string title, string message, DialogSeverity severity, string? confirmLabel = null) + => Task.FromResult(true); + public Task PromptAsync( string title, string label, string initialValue = "", string? placeholder = null) => Task.FromResult(null); diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Shared/DialogSeverityStylingTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Shared/DialogSeverityStylingTests.cs new file mode 100644 index 00000000..f48bb9fb --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Shared/DialogSeverityStylingTests.cs @@ -0,0 +1,123 @@ +using Bunit; +using Microsoft.Extensions.DependencyInjection; +using ZB.MOM.WW.ScadaBridge.CentralUI.Components.Shared; + +namespace ZB.MOM.WW.ScadaBridge.CentralUI.Tests.Shared; + +/// +/// Deferred-work register row 75 — pins the confirm button's severity → Bootstrap class +/// mapping against the REAL behind a real DialogHost, +/// so these cover what the user actually sees rather than what an argument was. +/// +/// +/// Row 28 split the label from the styling but left the styling a two-state boolean, so a +/// consequential-but-not-destructive action had to choose between looking like a delete +/// and looking like the safe default. Row 75 added as +/// the middle rung. The three-way mapping is the whole of that change, and these are its +/// pins; SecuredWritesTests separately pins that the Secured Writes page asks for +/// the right one. +/// +/// +/// +/// bUnit cannot run real JS interop, so the host's sbDialog.* focus-trap calls run +/// in as no-ops (same arrangement as +/// FailoverConfirmLabelTests). +/// +/// +public class DialogSeverityStylingTests : BunitContext +{ + private readonly DialogService _dialog = new(); + + public DialogSeverityStylingTests() + { + Services.AddSingleton(_dialog); + JSInterop.Mode = JSRuntimeMode.Loose; + } + + /// + /// Opens a confirm dialog on a live host and returns its confirm button. + /// + /// + /// The ConfirmAsync task is deliberately not awaited — with the real service it + /// does not complete until someone answers the dialog, so awaiting it would deadlock + /// the test. The state is published synchronously before the task is returned. + /// + private AngleSharp.Dom.IElement OpenConfirm(Func> open) + { + var cut = Render(); + cut.InvokeAsync(() => { _ = open(_dialog); }).GetAwaiter().GetResult(); + cut.Render(); + + var footer = cut.FindAll("div.modal-footer button").ToList(); + return footer.Single(b => b.TextContent.Trim() != "Cancel"); + } + + [Fact] + public void Caution_rendersAmber_notRedAndNotPrimary() + { + // The row-75 case. Amber is the point: not red (which reads "delete"), and not + // the primary blue (which reads "safe default" — wrong for a live device write). + var confirm = OpenConfirm(d => d.ConfirmAsync("Approve thing", "Sure?", DialogSeverity.Caution, "Approve")); + + Assert.Contains("btn-warning", confirm.ClassList); + Assert.DoesNotContain("btn-danger", confirm.ClassList); + Assert.DoesNotContain("btn-primary", confirm.ClassList); + } + + [Fact] + public void Destructive_stillRendersRed() + { + // Regression guard on the ~20 genuine delete call sites: row 75 is additive and + // must not have shifted what danger looks like. + var confirm = OpenConfirm(d => d.ConfirmAsync("Delete thing", "Sure?", DialogSeverity.Destructive)); + + Assert.Contains("btn-danger", confirm.ClassList); + } + + [Fact] + public void Normal_stillRendersPrimary() + { + var confirm = OpenConfirm(d => d.ConfirmAsync("Do thing", "Sure?", DialogSeverity.Normal)); + + Assert.Contains("btn-primary", confirm.ClassList); + } + + [Theory] + [InlineData(true, "btn-danger")] + [InlineData(false, "btn-primary")] + public void BooleanForm_mapsOntoTheSameTwoSeverities(bool danger, string expectedClass) + { + // The boolean overload is now a thin adapter onto the severity one. Every existing + // call site goes through it, so this pins that the adapter did not shift any of + // them: true is still red, false is still primary. + // + // One case per fact, deliberately: DialogService permits only one open dialog at a + // time and throws on the second, so two OpenConfirm calls in one body would fail + // on the service's own nested-dialog guard rather than on anything about styling. + var confirm = OpenConfirm(d => d.ConfirmAsync("Do thing", "Sure?", danger: danger)); + + Assert.Contains(expectedClass, confirm.ClassList); + } + + [Fact] + public void UnlabelledCaution_readsConfirm_notDelete() + { + // Caution deliberately does NOT inherit the danger "Delete" fallback: a caution + // action is by definition not a delete, so falling back to that verb would + // reintroduce register row 28 through the new door. + var confirm = OpenConfirm(d => d.ConfirmAsync("Approve thing", "Sure?", DialogSeverity.Caution)); + + Assert.Equal("Confirm", confirm.TextContent.Trim()); + } + + [Fact] + public void UnlabelledDestructive_stillFallsBackToDelete() + { + // The row-28 fallback is retained deliberately (the Playwright suites click the + // literal "Delete"). Pinned here alongside its new sibling so the pair is read + // together. + var confirm = OpenConfirm(d => d.ConfirmAsync("Delete thing", "Sure?", DialogSeverity.Destructive)); + + Assert.Equal("Delete", confirm.TextContent.Trim()); + } +}