feat(centralui): row 75 — Approve is caution amber, not destructive red

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.
2026-08-15 21:33:08 -04:00
parent 9d668861ad
commit 40944a0d04
13 changed files with 340 additions and 15 deletions
@@ -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<string>(), Arg.Any<string>(), Arg.Any<bool>(), Arg.Any<string?>())
            .Returns(Task.FromResult(true));
+        _dialog.ConfirmAsync(Arg.Any<string>(), Arg.Any<string>(), Arg.Any<DialogSeverity>(), Arg.Any<string?>())
+            .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<string>(),
            Arg.Is<string>(m => Regex.IsMatch(m, "Submitted .* ago")),
-            Arg.Any<bool>(),
+            Arg.Any<DialogSeverity>(),
            Arg.Any<string?>());
    }

+    // ── (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<long>(), Arg.Any<string?>(), Arg.Any<CancellationToken>())
+            .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<string>(), Arg.Any<string>(), 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<string>(), Arg.Any<string>(), Arg.Any<bool>(), Arg.Any<string?>());
+    }
+
+    [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<long>(), Arg.Any<string?>(), Arg.Any<CancellationToken>())
+            .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<string>(), Arg.Any<string>(), danger: true, confirmLabel: "Reject");
+    }
+
    [Fact]
    public void Viewer_DoesNotSeeOperatorOrVerifierRegions()
    {
@@ -213,6 +213,12 @@ public class SchemaLibraryPageTests : BunitContext
        public Task<bool> 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<bool> ConfirmAsync(string title, string message, DialogSeverity severity, string? confirmLabel = null)
+            => Task.FromResult(true);
+
        public Task<string?> PromptAsync(
            string title, string label, string initialValue = "", string? placeholder = null)
            => Task.FromResult<string?>(null);
@@ -194,6 +194,12 @@ public class NotificationListsPageTests : BunitContext
        public Task<bool> 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<bool> ConfirmAsync(string title, string message, DialogSeverity severity, string? confirmLabel = null)
+            => Task.FromResult(true);
+
        public Task<string?> PromptAsync(
            string title, string label, string initialValue = "", string? placeholder = null)
            => Task.FromResult<string?>(null);
@@ -339,6 +339,12 @@ public class NotificationReportDetailModalTests : BunitContext
        public Task<bool> 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<bool> ConfirmAsync(string title, string message, DialogSeverity severity, string? confirmLabel = null)
+            => Task.FromResult(true);
+
        public Task<string?> PromptAsync(
            string title, string label, string initialValue = "", string? placeholder = null)
            => Task.FromResult<string?>(null);
@@ -274,6 +274,12 @@ public class NotificationReportPageTests : BunitContext
        public Task<bool> 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<bool> ConfirmAsync(string title, string message, DialogSeverity severity, string? confirmLabel = null)
+            => Task.FromResult(true);
+
        public Task<string?> PromptAsync(
            string title, string label, string initialValue = "", string? placeholder = null)
            => Task.FromResult<string?>(null);
@@ -213,6 +213,12 @@ public sealed class QueryStringDrillInTests
    {
        public Task<bool> 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<bool> ConfirmAsync(string title, string message, DialogSeverity severity, string? confirmLabel = null)
+            => Task.FromResult(true);
        public Task<string?> PromptAsync(string title, string label, string initialValue = "", string? placeholder = null)
            => Task.FromResult<string?>(null);

@@ -682,6 +682,12 @@ public class SiteCallsReportPageTests : BunitContext
        public Task<bool> 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<bool> ConfirmAsync(string title, string message, DialogSeverity severity, string? confirmLabel = null)
+            => Task.FromResult(true);
+
        public Task<string?> PromptAsync(
            string title, string label, string initialValue = "", string? placeholder = null)
            => Task.FromResult<string?>(null);
@@ -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;
+
+/// <summary>
+/// Deferred-work register row 75 — pins the confirm button's severity → Bootstrap class
+/// mapping against the REAL <see cref="DialogService"/> behind a real <c>DialogHost</c>,
+/// so these cover what the user actually sees rather than what an argument was.
+///
+/// <para>
+/// 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 <see cref="DialogSeverity.Caution"/> as
+/// the middle rung. The three-way mapping is the whole of that change, and these are its
+/// pins; <c>SecuredWritesTests</c> separately pins that the Secured Writes page asks for
+/// the right one.
+/// </para>
+///
+/// <para>
+/// bUnit cannot run real JS interop, so the host's <c>sbDialog.*</c> focus-trap calls run
+/// in <see cref="JSRuntimeMode.Loose"/> as no-ops (same arrangement as
+/// <c>FailoverConfirmLabelTests</c>).
+/// </para>
+/// </summary>
+public class DialogSeverityStylingTests : BunitContext
+{
+    private readonly DialogService _dialog = new();
+
+    public DialogSeverityStylingTests()
+    {
+        Services.AddSingleton<IDialogService>(_dialog);
+        JSInterop.Mode = JSRuntimeMode.Loose;
+    }
+
+    /// <summary>
+    /// Opens a confirm dialog on a live host and returns its confirm button.
+    /// </summary>
+    /// <remarks>
+    /// The <c>ConfirmAsync</c> 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.
+    /// </remarks>
+    private AngleSharp.Dom.IElement OpenConfirm(Func<IDialogService, Task<bool>> open)
+    {
+        var cut = Render<DialogHost>();
+        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());
+    }
+}