fix(centralui): confirm dialogs name their own action instead of always saying "Delete" (register row 28)
The Health dashboard's "Trigger failover" confirmation offered a red button
labelled *Delete*. The copy was right; the button was not. `DialogHost` derived
the confirm label solely from `danger`, so `danger: true` meant the verb was
literally "Delete" for every caller — and no caller could say otherwise, because
`IDialogService.ConfirmAsync` had no label parameter.
`danger` now selects styling only; a new optional `confirmLabel` names the action
and wins over the fallback. The kind/danger fallback is deliberately RETAINED
("Save" for a prompt, "Delete" for a bare danger confirm, "Confirm" otherwise):
around 20 call sites are genuine deletes where it is correct, and the Playwright
suites click the literal "Delete". Requiring the label everywhere would have
churned those 20 sites for no behaviour change, with the miss-detection resting
on Playwright runs that need a live rig.
Audited all 34 confirm call sites; 9 read "Delete" for something that is not a
delete and are relabelled:
Fail over CentralFailoverControl (central pair, site pair)
Approve SecuredWrites — approve (this one WRITES TO A DEVICE)
Reject SecuredWrites — reject
Disable Topology — disable instance
Discard SiteCallsReport, NotificationReport, ParkedMessages (bulk + single)
The remaining 25 are correct as they stand: 18 genuine deletes plus 7 non-danger
confirms ("Revert to base", the three Retry paths) whose button reads "Confirm".
Tests: FailoverConfirmLabelTests drives the real DialogService behind a real
DialogHost — a substitute would pin the argument but not what the operator reads
— covering both branches, that the button keeps btn-danger, and that an
unlabelled danger confirm still falls back to "Delete". Note its click is
deliberately not awaited: with a real dialog the handler parks on ConfirmAsync
until answered, so awaiting deadlocks. Existing IDialogService fakes and
NSubstitute matchers gain the 4th parameter (an optional param is baked into the
call site, so a 3-arg Arg.Any stub silently stops matching a labelled call).
This commit is contained in:
Joseph Doherty
@@ -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;
|
||||
|
||||
+4
-2
@@ -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;
|
||||
|
||||
+2
-1
@@ -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;
|
||||
|
||||
+2
-1
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user