merge: small fixes (register rows 28, 35, 57) — confirm dialogs name their own action (9 mislabeled 'Delete' buttons fixed incl. secured-write Approve/Reject), DeploymentRecords composite (DeployedAt DESC, Id DESC) index, audit-config query CLI table projection

2026-08-15 17:56:09 -04:00
parent acc6209665 b2cab59cb9
commit d607011e5e
30 changed files with 2857 additions and 45 deletions
@@ -0,0 +1,153 @@
+using System.Text.Json;
+using System.Text.Json.Nodes;
+
+namespace ZB.MOM.WW.ScadaBridge.CLI.Commands;
+
+/// <summary>
+/// Compact table projection for <c>audit-config query</c> (deferred-work register row 57).
+///
+/// <para>
+/// The handler returns an <c>AuditLogQueryResult</c> — <c>{ "entries": [ … ],
+/// "totalCount": N }</c>. Handed to the generic renderer unprojected, that object takes
+/// the Property/Value branch and the whole <c>entries</c> array lands in a single cell as
+/// raw JSON, so <c>--format table</c> was unreadable. This unwraps the envelope to the
+/// row array the renderer expects and drops <c>afterStateJson</c>, the per-entry snapshot
+/// of the entire changed entity — the same "one fat field makes the table useless" problem
+/// <see cref="TemplateTableProjection"/> solves by dropping child collections.
+/// </para>
+///
+/// <para>
+/// <c>totalCount</c> would be lost by a plain unwrap, so when the page is a strict subset
+/// of the matches a single trailing row carries a <c>note</c> cell saying how many of how
+/// many are shown. Every real entry leaves that cell empty, and when the page IS the whole
+/// result set the column is not emitted at all — so the common small-query table stays
+/// exactly as narrow as its data.
+/// </para>
+///
+/// <para>
+/// JSON output is never touched (this runs only on the table path), and the command's
+/// <c>--detail</c> flag skips the projection to render the raw payload as-is — that is
+/// the way to see <c>afterStateJson</c> in table form.
+/// </para>
+/// </summary>
+internal static class AuditConfigTableProjection
+{
+    /// <summary>
+    /// Projects an <c>AuditLogQueryResult</c> envelope (or a bare entry array) to the
+    /// compact row array the table renderer consumes. Returns the input unchanged when it
+    /// is not JSON or not one of those shapes, so the renderer's own fallbacks still apply.
+    /// </summary>
+    /// <param name="json">The raw success JSON body from the management API.</param>
+    /// <returns>Compact JSON suitable for table rendering.</returns>
+    internal static string ProjectEntries(string json)
+    {
+        JsonDocument doc;
+        try
+        {
+            doc = JsonDocument.Parse(json);
+        }
+        catch (JsonException)
+        {
+            // Not JSON (e.g. a proxy error page) — let the renderer print it verbatim.
+            return json;
+        }
+
+        using (doc)
+        {
+            var root = doc.RootElement;
+
+            // A bare array is accepted so the projection still works if the payload is
+            // ever unwrapped upstream, and so the deprecated `audit-log` alias against an
+            // older server does not render worse than it does today.
+            if (root.ValueKind == JsonValueKind.Array)
+            {
+                return BuildRows(root, totalCount: null).ToJsonString();
+            }
+
+            if (root.ValueKind == JsonValueKind.Object
+                && TryGetPropertyCI(root, "entries", out var entries)
+                && entries.ValueKind == JsonValueKind.Array)
+            {
+                int? total = TryGetPropertyCI(root, "totalCount", out var tc)
+                    && tc.ValueKind == JsonValueKind.Number
+                    && tc.TryGetInt32(out var n)
+                        ? n
+                        : null;
+                return BuildRows(entries, total).ToJsonString();
+            }
+
+            // Some other object (e.g. an error envelope) — leave it to the renderer.
+            return json;
+        }
+    }
+
+    private static JsonArray BuildRows(JsonElement entries, int? totalCount)
+    {
+        var arr = new JsonArray();
+        var shown = 0;
+        foreach (var item in entries.EnumerateArray())
+        {
+            arr.Add(ProjectElement(item));
+            shown++;
+        }
+
+        // Only when the page is a strict subset — otherwise the count is just the row
+        // count the operator can already see, and the extra column would be noise.
+        if (totalCount is int total && total > shown)
+        {
+            arr.Add(new JsonObject
+            {
+                ["note"] = JsonValue.Create(
+                    $"showing {shown} of {total} — use --page / --page-size for the rest"),
+            });
+        }
+
+        return arr;
+    }
+
+    /// <summary>Projects a single audit entry to its compact row node.</summary>
+    private static JsonNode ProjectElement(JsonElement element)
+    {
+        if (element.ValueKind != JsonValueKind.Object)
+            return JsonValue.Create(element.ToString())!;
+
+        // JsonObject preserves insertion order, which fixes the table's column order.
+        // afterStateJson (the full post-change entity snapshot) and bundleImportId are
+        // deliberately omitted — `--detail`, or --format json, for those.
+        return new JsonObject
+        {
+            ["id"] = Int(element, "id"),
+            ["timestamp"] = Str(element, "timestamp"),
+            ["user"] = Str(element, "user"),
+            ["action"] = Str(element, "action"),
+            ["entityType"] = Str(element, "entityType"),
+            ["entityName"] = Str(element, "entityName"),
+            ["entityId"] = Str(element, "entityId"),
+        };
+    }
+
+    private static bool TryGetPropertyCI(JsonElement obj, string name, out JsonElement value)
+    {
+        foreach (var prop in obj.EnumerateObject())
+        {
+            if (string.Equals(prop.Name, name, StringComparison.OrdinalIgnoreCase))
+            {
+                value = prop.Value;
+                return true;
+            }
+        }
+
+        value = default;
+        return false;
+    }
+
+    private static JsonNode? Str(JsonElement obj, string name)
+        => TryGetPropertyCI(obj, name, out var v) && v.ValueKind == JsonValueKind.String
+            ? JsonValue.Create(v.GetString())
+            : null;
+
+    private static JsonNode? Int(JsonElement obj, string name)
+        => TryGetPropertyCI(obj, name, out var v) && v.ValueKind == JsonValueKind.Number && v.TryGetInt32(out var n)
+            ? JsonValue.Create(n)
+            : null;
+}
@@ -75,6 +75,13 @@ public static class AuditLogCommands
        pageOption.DefaultValueFactory = _ => 1;
        var pageSizeOption = new Option<int>("--page-size") { Description = "Page size" };
        pageSizeOption.DefaultValueFactory = _ => 50;
+        var detailOption = new Option<bool>("--detail")
+        {
+            Description = "Render the raw query payload in table output instead of the compact column projection. "
+                + "This is how to see 'afterStateJson' (the full post-change entity snapshot) in table form — it is "
+                + "omitted from the compact columns because a single entry's snapshot fills the terminal. "
+                + "No effect on JSON output."
+        };

        var cmd = new Command("query") { Description = "Query audit log entries" };
        cmd.Add(userOption);
@@ -84,6 +91,7 @@ public static class AuditLogCommands
        cmd.Add(toOption);
        cmd.Add(pageOption);
        cmd.Add(pageSizeOption);
+        cmd.Add(detailOption);
        cmd.SetAction(async (ParseResult result) =>
        {
            var user = result.GetValue(userOption);
@@ -93,9 +101,11 @@ public static class AuditLogCommands
            var to = result.GetValue(toOption);
            var page = result.GetValue(pageOption);
            var pageSize = result.GetValue(pageSizeOption);
+            var detail = result.GetValue(detailOption);
            return await CommandHelpers.ExecuteCommandAsync(
                result, urlOption, formatOption, usernameOption, passwordOption,
-                new QueryAuditLogCommand(user, entityType, action, from, to, page, pageSize));
+                new QueryAuditLogCommand(user, entityType, action, from, to, page, pageSize),
+                tableProjector: detail ? null : AuditConfigTableProjection.ProjectEntries);
        });
        return cmd;
    }
@@ -1801,6 +1801,7 @@ scadabridge --url <url> audit-config query [options]
 | `--to` | no | — | End timestamp in ISO 8601 format |
 | `--page` | no | `1` | Page number |
 | `--page-size` | no | `50` | Results per page |
+| `--detail` | no | `false` | Table output only: render the raw payload instead of the compact column projection (see below). No effect on JSON output. |

 Output shape:

@@ -1822,7 +1823,25 @@ Output shape:
 ```

 `totalCount` is the unpaged number of rows matching the filters, so a client can page
-without a second call.
+without a second call. JSON is the primary format for this command and is what scripts
+should consume — it is never altered by the table projection below.
+
+**Table output.** `--format table` renders a compact projection with one row per entry:
+
+```
+id    timestamp                    user         action   entityType      entityName     entityId
+412   2026-08-15T12:00:00+00:00    multi-role   Created  Template        PumpTemplate   7
+413   2026-08-15T12:05:00+00:00    designer     Deleted  ExternalSystem  MES            3
+```
+
+`afterStateJson` (the full post-change entity snapshot) and `bundleImportId` are omitted
+— one snapshot per row fills a terminal. Pass `--detail` to render the raw payload
+instead, or use `--format json`. When the page is a strict subset of the matches, a final
+row carries a `note` cell (`showing 50 of 137 — use --page / --page-size for the rest`);
+when the page is the whole result set that column is not emitted at all.
+
+Before this projection existed the whole `entries` array was printed as raw JSON inside a
+single `Property`/`Value` cell (deferred-work register row 57).

 ---

@@ -70,14 +70,16 @@
                + "standby takes over and its Deployment Manager singleton hands over gracefully. "
                + "In-flight work on that site node is interrupted, and the site is briefly "
                + "unavailable while the handover completes.",
-                danger: true)
+                danger: true,
+                confirmLabel: "Fail over")
            : await Dialog.ConfirmAsync(
                "Trigger central failover?",
                "The active central node will leave the cluster and restart; the standby takes over "
                + "and becomes active. Cluster singletons hand over gracefully, but in-flight work on "
                + "the active node is interrupted. This page is served by the active node, so it will "
                + "briefly disconnect and reconnect against the new active node.",
-                danger: true);
+                danger: true,
+                confirmLabel: "Fail over");

        if (!confirmed)
        {
@@ -868,7 +868,8 @@
        var confirmed = await Dialog.ConfirmAsync(
            "Disable Instance",
            $"Disable instance '{inst.UniqueName}'? The instance actor will be stopped.",
-            danger: true);
+            danger: true,
+            confirmLabel: "Disable");
        if (!confirmed) return;

        _actionInProgress = true;
@@ -668,7 +668,8 @@
        var confirmed = await Dialog.ConfirmAsync(
            "Discard parked messages",
            $"Permanently discard {ids.Count} message{(ids.Count == 1 ? "" : "s")}? This cannot be undone.",
-            danger: true);
+            danger: true,
+            confirmLabel: "Discard");
        if (!confirmed) return;

        _bulkInProgress = true;
@@ -720,7 +721,8 @@
        var confirmed = await Dialog.ConfirmAsync(
            "Discard parked message",
            $"Permanently discard message {ShortId(msg.MessageId)}? This cannot be undone.",
-            danger: true);
+            danger: true,
+            confirmLabel: "Discard");
        if (!confirmed) return false;

        _actionInProgress = true;
@@ -566,7 +566,8 @@
        var confirmed = await Dialog.ConfirmAsync(
            "Discard notification",
            $"Permanently discard notification {ShortId(n.NotificationId)} (\"{n.Subject}\")? This cannot be undone.",
-            danger: true);
+            danger: true,
+            confirmLabel: "Discard");
        if (!confirmed) return;

        _actionInProgress = true;
@@ -424,7 +424,8 @@
            $"Value: {row.ValueJson} ({row.ValueType})\n" +
            $"Submitted {FormatAge(row.SubmittedAtUtc)} ago ({row.SubmittedAtUtc:u})\n" +
            $"Operator: {row.OperatorUser}",
-            danger: true);
+            danger: true,
+            confirmLabel: "Approve");
        if (!confirmed) return;

        _actionInProgress = true;
@@ -460,7 +461,8 @@
        var confirmed = await Dialog.ConfirmAsync(
            "Reject secured write",
            $"Reject the secured write to {row.SiteId}/{row.ConnectionName}/{row.TagPath}?",
-            danger: true);
+            danger: true,
+            confirmLabel: "Reject");
        if (!confirmed) return;

        _actionInProgress = true;
@@ -343,7 +343,8 @@ public partial class SiteCallsReport
            "Discard cached call",
            $"Relay a discard of cached call {ShortId(c.TrackedOperationId)} (\"{c.Target}\") " +
            $"to site {SiteName(c.SourceSite)}? This cannot be undone.",
-            danger: true);
+            danger: true,
+            confirmLabel: "Discard");
        if (!confirmed) return;

        _actionInProgress = true;
@@ -185,9 +185,17 @@
        svc.Resolve(resolveValue);
    }

-    private static string ConfirmLabel(DialogState state) => state.Kind switch
-    {
-        DialogKind.Prompt => "Save",
-        _ => state.Danger ? "Delete" : "Confirm",
-    };
+    // 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
+    // failover button (deferred-work register row 28). Callers whose destructive verb
+    // is not "Delete" must pass ConfirmLabel — see IDialogService.ConfirmAsync.
+    private static string ConfirmLabel(DialogState state) =>
+        !string.IsNullOrWhiteSpace(state.ConfirmLabel)
+            ? state.ConfirmLabel
+            : state.Kind switch
+            {
+                DialogKind.Prompt => "Save",
+                _ => state.Danger ? "Delete" : "Confirm",
+            };
 }
@@ -35,12 +35,19 @@ public class DialogService : IDialogService
    private TaskCompletionSource<object?>? _tcs;

    /// <inheritdoc />
-    public Task<bool> ConfirmAsync(string title, string message, bool danger = false)
+    public Task<bool> ConfirmAsync(string title, string message, bool danger = false, string? confirmLabel = null)
    {
        EnsureNoActiveDialog();
        var tcs = new TaskCompletionSource<object?>(TaskCreationOptions.RunContinuationsAsynchronously);
        _tcs = tcs;
-        Current = new DialogState(title, DialogKind.Confirm, message, danger, PromptInitial: string.Empty, Placeholder: null);
+        Current = new DialogState(
+            title,
+            DialogKind.Confirm,
+            message,
+            danger,
+            PromptInitial: string.Empty,
+            Placeholder: null,
+            ConfirmLabel: confirmLabel);
        OnChange?.Invoke();
        return Project(tcs.Task, static r => r is bool b && b);
    }
@@ -146,6 +153,9 @@ public class DialogService : IDialogService
 /// render inside <c>.modal-body</c>; <c>null</c> for confirm/prompt.</param>
 /// <param name="Size">Optional Bootstrap modal-dialog size modifier class applied
 /// to the <c>modal-dialog</c> element (e.g. <c>modal-lg</c>); <c>null</c> = default.</param>
+/// <param name="ConfirmLabel">Explicit verb for the confirm button. <c>null</c> or
+/// blank falls back to the kind/danger default ("Save" for a prompt, "Delete" for a
+/// danger confirm, "Confirm" otherwise).</param>
 public record DialogState(
    string Title,
    DialogKind Kind,
@@ -154,7 +164,8 @@ public record DialogState(
    string PromptInitial,
    string? Placeholder,
    RenderFragment? Content = null,
-    string? Size = null);
+    string? Size = null,
+    string? ConfirmLabel = null);

 public enum DialogKind
 {
@@ -19,10 +19,17 @@ public interface IDialogService
    /// <param name="title">Modal title text.</param>
    /// <param name="message">Body text shown to the user.</param>
    /// <param name="danger">When <c>true</c>, the confirm button renders in
-    /// <c>btn-danger</c> styling with the label "Delete"; otherwise a primary
-    /// "Confirm" button is shown.</param>
+    /// <c>btn-danger</c> styling; otherwise a primary button is shown. This
+    /// controls <em>styling only</em> — it does not name the action.</param>
+    /// <param name="confirmLabel">Verb rendered on the confirm button. When
+    /// <c>null</c> or blank the button falls back to "Delete" for a danger
+    /// dialog and "Confirm" otherwise. <strong>Any destructive action that is
+    /// not literally a delete must pass this</strong> — otherwise the red button
+    /// on, say, a failover or a discard reads "Delete", which is simply wrong
+    /// (deferred-work register row 28). See the label table in
+    /// <c>Component-CentralUI.md</c> → Dialog Service.</param>
    /// <returns>A task that resolves to <c>true</c> when the user confirms, or <c>false</c> when cancelled.</returns>
-    Task<bool> ConfirmAsync(string title, string message, bool danger = false);
+    Task<bool> ConfirmAsync(string title, string message, bool danger = false, string? confirmLabel = null);

    /// <summary>
    /// Shows a single-line text prompt and resolves to the entered value, or
@@ -39,7 +39,26 @@ public class DeploymentRecordConfiguration : IEntityTypeConfiguration<Deployment

        builder.HasIndex(d => d.DeploymentId).IsUnique();
        builder.HasIndex(d => d.InstanceId);
-        builder.HasIndex(d => d.DeployedAt);
+
+        // Deployment-list sort key. Every list read orders DeployedAt DESC, Id DESC
+        // (DeploymentManagerRepository.QueryDeploymentListPageAsync and
+        // QueryDeploymentSummariesAsync) — Id is the deterministic tie-break, because
+        // a bulk site deploy stamps many rows in the same DeployedAt instant and a
+        // non-deterministic order would let a row repeat or vanish across pages.
+        // Declaring both key columns descending lets a forward scan feed the paged
+        // Skip/Take directly instead of the server sorting the whole filtered set per
+        // page (deferred-work register row 35).
+        //
+        // This REPLACES the old single-column HasIndex(d => d.DeployedAt): that index
+        // is strictly redundant here, not merely overlapping. DeployedAt leads both, so
+        // any predicate the old one served the composite serves too (direction does not
+        // affect seekability — SQL Server scans either way), and the leaf is the same
+        // width, since Id is the clustered PK and was therefore already sitting in the
+        // old index's leaf as the row locator. Keeping it would buy nothing and cost a
+        // second index maintained on every insert and status transition.
+        builder.HasIndex(d => new { d.DeployedAt, d.Id })
+            .IsDescending(true, true)
+            .HasDatabaseName("IX_DeploymentRecords_DeployedAt_Id");
    }
 }

@@ -0,0 +1,60 @@
+using Microsoft.EntityFrameworkCore.Migrations;
+
+#nullable disable
+
+namespace ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Migrations
+{
+    /// <summary>
+    /// Replaces the single-column <c>IX_DeploymentRecords_DeployedAt</c> with the
+    /// composite <c>IX_DeploymentRecords_DeployedAt_Id (DeployedAt DESC, Id DESC)</c>,
+    /// which is the deployment list's exact sort key (deferred-work register row 35).
+    /// </summary>
+    /// <remarks>
+    /// The list read (<c>DeploymentManagerRepository.QueryDeploymentListPageAsync</c>)
+    /// joins <c>Instance</c> to make <c>SiteId</c> filterable, then orders
+    /// <c>DeployedAt DESC, Id DESC</c> and pages with Skip/Take. With both key columns
+    /// declared descending, a forward scan of this index already delivers rows in the
+    /// requested order, so the plan can feed the Top-N page directly rather than sorting
+    /// the whole filtered set on every page request.
+    ///
+    /// The drop is safe rather than merely tolerable: <c>DeployedAt</c> leads both
+    /// indexes, so every predicate the old one served the composite serves as well
+    /// (index direction does not affect seekability — SQL Server scans either
+    /// direction), and the leaf rows are the same width, because <c>Id</c> is the
+    /// clustered primary key and was therefore already present in the old nonclustered
+    /// index's leaf as the row locator. What changes is only that <c>Id</c> is now a
+    /// declared, ordered key column instead of an implicit locator. Keeping both would
+    /// add a second index to maintain on every insert and status transition for no read
+    /// benefit. <c>SystemArtifactDeploymentRecords</c> has its own separate
+    /// <c>DeployedAt</c> index and is deliberately untouched.
+    /// </remarks>
+    public partial class AddDeploymentRecordListSortIndex : Migration
+    {
+        /// <inheritdoc />
+        protected override void Up(MigrationBuilder migrationBuilder)
+        {
+            migrationBuilder.DropIndex(
+                name: "IX_DeploymentRecords_DeployedAt",
+                table: "DeploymentRecords");
+
+            migrationBuilder.CreateIndex(
+                name: "IX_DeploymentRecords_DeployedAt_Id",
+                table: "DeploymentRecords",
+                columns: new[] { "DeployedAt", "Id" },
+                descending: new bool[0]);
+        }
+
+        /// <inheritdoc />
+        protected override void Down(MigrationBuilder migrationBuilder)
+        {
+            migrationBuilder.DropIndex(
+                name: "IX_DeploymentRecords_DeployedAt_Id",
+                table: "DeploymentRecords");
+
+            migrationBuilder.CreateIndex(
+                name: "IX_DeploymentRecords_DeployedAt",
+                table: "DeploymentRecords",
+                column: "DeployedAt");
+        }
+    }
+}
@@ -267,13 +267,15 @@ namespace ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Migrations

                    b.HasKey("Id");

-                    b.HasIndex("DeployedAt");
-
                    b.HasIndex("DeploymentId")
                        .IsUnique();

                    b.HasIndex("InstanceId");

+                    b.HasIndex("DeployedAt", "Id")
+                        .IsDescending()
+                        .HasDatabaseName("IX_DeploymentRecords_DeployedAt_Id");
+
                    b.ToTable("DeploymentRecords");
                });