refactor(uns): modal-polish nits on the global UNS page (task #137)

Low-severity review nits, no behaviour change to the happy path: - CloseModals() now also resets the leftover _*ModalIsNew / parent-id fields (area ClusterId, line AreaId, equipment LineId, tag/vtag) for symmetry — harmless today (always set before a modal opens) but consistent. - HandleAddChild / HandleAddVirtualTag / HandleEdit gain a _modalBusy guard (try/finally) so a rapid double-action can't race two service loads into the same modal state. The switch bodies are re-indented under the try block. - VirtualTagModal DataType is now an InputSelect over the standard OPC UA type list (the same set TagModal uses) instead of free-text InputText. - RefreshEquipmentChildrenAsync documents that callers own StateHasChanged() and the full-reload fallback is spelled out as a block with a comment. Build clean; AdminUI.Tests 216/216.
2026-06-09 08:18:15 -04:00
parent f19f2ee73b
commit b87d877270
2 changed files with 147 additions and 93 deletions
@@ -127,6 +127,10 @@
    private string? _filter;
    private bool _loading = true;

+    // Guards the async modal openers (HandleAddChild/HandleAddVirtualTag/HandleEdit) so a rapid
+    // double-action can't race two service loads into the same modal state.
+    private bool _modalBusy;
+
    // --- Area modal state ---
    private bool _areaModalVisible;
    private bool _areaModalIsNew;
@@ -250,54 +254,72 @@
    /// </summary>
    private async Task HandleAddChild(UnsNode node)
    {
-        CloseModals();
-        switch (node.Kind)
+        if (_modalBusy) { return; }
+        _modalBusy = true;
+        try
        {
-            case UnsNodeKind.Cluster:
-                _areaModalIsNew = true;
-                _areaModalExisting = null;
-                _areaModalClusterId = node.ClusterId ?? node.EntityId;
-                _areaModalVisible = true;
-                break;
+            CloseModals();
+            switch (node.Kind)
+            {
+                case UnsNodeKind.Cluster:
+                    _areaModalIsNew = true;
+                    _areaModalExisting = null;
+                    _areaModalClusterId = node.ClusterId ?? node.EntityId;
+                    _areaModalVisible = true;
+                    break;

-            case UnsNodeKind.Area:
-                _lineModalIsNew = true;
-                _lineModalExisting = null;
-                _lineModalAreaId = node.EntityId;
-                _lineModalAreaOptions = AreaOptionsForCluster(node.ClusterId);
-                _lineModalVisible = true;
-                break;
+                case UnsNodeKind.Area:
+                    _lineModalIsNew = true;
+                    _lineModalExisting = null;
+                    _lineModalAreaId = node.EntityId;
+                    _lineModalAreaOptions = AreaOptionsForCluster(node.ClusterId);
+                    _lineModalVisible = true;
+                    break;

-            case UnsNodeKind.Line:
-                _equipmentModalIsNew = true;
-                _equipmentModalExisting = null;
-                _equipmentModalLineId = node.EntityId;
-                _equipmentModalLineOptions = LinesForCluster(node.ClusterId);
-                _equipmentModalDriverOptions = await Svc.LoadDriversForClusterAsync(node.ClusterId!);
-                _equipmentModalVisible = true;
-                break;
+                case UnsNodeKind.Line:
+                    _equipmentModalIsNew = true;
+                    _equipmentModalExisting = null;
+                    _equipmentModalLineId = node.EntityId;
+                    _equipmentModalLineOptions = LinesForCluster(node.ClusterId);
+                    _equipmentModalDriverOptions = await Svc.LoadDriversForClusterAsync(node.ClusterId!);
+                    _equipmentModalVisible = true;
+                    break;

-            case UnsNodeKind.Equipment:
-                _tagModalIsNew = true;
-                _tagModalExisting = null;
-                _tagModalEquipmentId = node.EntityId;
-                _childRefreshEquipmentId = node.EntityId;
-                _tagModalDriverOptions = await Svc.LoadTagDriversForEquipmentAsync(node.EntityId!);
-                _tagModalVisible = true;
-                break;
+                case UnsNodeKind.Equipment:
+                    _tagModalIsNew = true;
+                    _tagModalExisting = null;
+                    _tagModalEquipmentId = node.EntityId;
+                    _childRefreshEquipmentId = node.EntityId;
+                    _tagModalDriverOptions = await Svc.LoadTagDriversForEquipmentAsync(node.EntityId!);
+                    _tagModalVisible = true;
+                    break;
+            }
+        }
+        finally
+        {
+            _modalBusy = false;
        }
    }

    /// <summary>Opens the create modal for a new virtual tag scoped to the clicked equipment.</summary>
    private async Task HandleAddVirtualTag(UnsNode node)
    {
-        CloseModals();
-        _vtagModalIsNew = true;
-        _vtagModalExisting = null;
-        _vtagModalEquipmentId = node.EntityId;
-        _childRefreshEquipmentId = node.EntityId;
-        _vtagModalScriptOptions = await Svc.LoadScriptsAsync();
-        _vtagModalVisible = true;
+        if (_modalBusy) { return; }
+        _modalBusy = true;
+        try
+        {
+            CloseModals();
+            _vtagModalIsNew = true;
+            _vtagModalExisting = null;
+            _vtagModalEquipmentId = node.EntityId;
+            _childRefreshEquipmentId = node.EntityId;
+            _vtagModalScriptOptions = await Svc.LoadScriptsAsync();
+            _vtagModalVisible = true;
+        }
+        finally
+        {
+            _modalBusy = false;
+        }
    }

    /// <summary>
@@ -307,60 +329,69 @@
    /// </summary>
    private async Task HandleEdit(UnsNode node)
    {
-        CloseModals();
-        switch (node.Kind)
+        if (_modalBusy) { return; }
+        _modalBusy = true;
+        try
        {
-            case UnsNodeKind.Area:
-                var area = await Svc.LoadAreaAsync(node.EntityId!);
-                if (area is null) { return; }
-                _areaModalIsNew = false;
-                _areaModalExisting = area;
-                _areaModalClusterId = area.ClusterId;
-                _areaModalVisible = true;
-                break;
+            CloseModals();
+            switch (node.Kind)
+            {
+                case UnsNodeKind.Area:
+                    var area = await Svc.LoadAreaAsync(node.EntityId!);
+                    if (area is null) { return; }
+                    _areaModalIsNew = false;
+                    _areaModalExisting = area;
+                    _areaModalClusterId = area.ClusterId;
+                    _areaModalVisible = true;
+                    break;

-            case UnsNodeKind.Line:
-                var line = await Svc.LoadLineAsync(node.EntityId!);
-                if (line is null) { return; }
-                _lineModalIsNew = false;
-                _lineModalExisting = line;
-                _lineModalAreaId = line.UnsAreaId;
-                _lineModalAreaOptions = AreaOptionsForCluster(node.ClusterId);
-                _lineModalVisible = true;
-                break;
+                case UnsNodeKind.Line:
+                    var line = await Svc.LoadLineAsync(node.EntityId!);
+                    if (line is null) { return; }
+                    _lineModalIsNew = false;
+                    _lineModalExisting = line;
+                    _lineModalAreaId = line.UnsAreaId;
+                    _lineModalAreaOptions = AreaOptionsForCluster(node.ClusterId);
+                    _lineModalVisible = true;
+                    break;

-            case UnsNodeKind.Equipment:
-                var equipment = await Svc.LoadEquipmentAsync(node.EntityId!);
-                if (equipment is null) { return; }
-                _equipmentModalIsNew = false;
-                _equipmentModalExisting = equipment;
-                _equipmentModalLineId = equipment.UnsLineId;
-                _equipmentModalLineOptions = LinesForCluster(node.ClusterId);
-                _equipmentModalDriverOptions = await Svc.LoadDriversForClusterAsync(node.ClusterId!);
-                _equipmentModalVisible = true;
-                break;
+                case UnsNodeKind.Equipment:
+                    var equipment = await Svc.LoadEquipmentAsync(node.EntityId!);
+                    if (equipment is null) { return; }
+                    _equipmentModalIsNew = false;
+                    _equipmentModalExisting = equipment;
+                    _equipmentModalLineId = equipment.UnsLineId;
+                    _equipmentModalLineOptions = LinesForCluster(node.ClusterId);
+                    _equipmentModalDriverOptions = await Svc.LoadDriversForClusterAsync(node.ClusterId!);
+                    _equipmentModalVisible = true;
+                    break;

-            case UnsNodeKind.Tag:
-                var tag = await Svc.LoadTagAsync(node.EntityId!);
-                if (tag is null) { return; }
-                _tagModalIsNew = false;
-                _tagModalExisting = tag;
-                _tagModalEquipmentId = tag.EquipmentId;
-                _childRefreshEquipmentId = tag.EquipmentId;
-                _tagModalDriverOptions = await Svc.LoadTagDriversForEquipmentAsync(tag.EquipmentId);
-                _tagModalVisible = true;
-                break;
+                case UnsNodeKind.Tag:
+                    var tag = await Svc.LoadTagAsync(node.EntityId!);
+                    if (tag is null) { return; }
+                    _tagModalIsNew = false;
+                    _tagModalExisting = tag;
+                    _tagModalEquipmentId = tag.EquipmentId;
+                    _childRefreshEquipmentId = tag.EquipmentId;
+                    _tagModalDriverOptions = await Svc.LoadTagDriversForEquipmentAsync(tag.EquipmentId);
+                    _tagModalVisible = true;
+                    break;

-            case UnsNodeKind.VirtualTag:
-                var vtag = await Svc.LoadVirtualTagAsync(node.EntityId!);
-                if (vtag is null) { return; }
-                _vtagModalIsNew = false;
-                _vtagModalExisting = vtag;
-                _vtagModalEquipmentId = vtag.EquipmentId;
-                _childRefreshEquipmentId = vtag.EquipmentId;
-                _vtagModalScriptOptions = await Svc.LoadScriptsAsync();
-                _vtagModalVisible = true;
-                break;
+                case UnsNodeKind.VirtualTag:
+                    var vtag = await Svc.LoadVirtualTagAsync(node.EntityId!);
+                    if (vtag is null) { return; }
+                    _vtagModalIsNew = false;
+                    _vtagModalExisting = vtag;
+                    _vtagModalEquipmentId = vtag.EquipmentId;
+                    _childRefreshEquipmentId = vtag.EquipmentId;
+                    _vtagModalScriptOptions = await Svc.LoadScriptsAsync();
+                    _vtagModalVisible = true;
+                    break;
+            }
+        }
+        finally
+        {
+            _modalBusy = false;
        }
    }

@@ -505,12 +536,18 @@
    /// <summary>
    /// Reloads a single equipment node's tag/virtual-tag children in place, leaving the rest of the tree
    /// (and the user's expansion) untouched. Falls back to a full structural reload only if the node
-    /// can no longer be found in the current tree.
+    /// can no longer be found in the current tree. Either branch only mutates state — the caller is
+    /// responsible for calling StateHasChanged() afterwards (every current caller does).
    /// </summary>
    private async Task RefreshEquipmentChildrenAsync(string equipmentId)
    {
        var node = FindEquipmentNode(equipmentId);
-        if (node is null) { _roots = await Svc.LoadStructureAsync(); return; }
+        if (node is null)
+        {
+            // Fallback: the equipment node is no longer in the current tree — reload the whole structure.
+            _roots = await Svc.LoadStructureAsync();
+            return;
+        }

        var kids = await Svc.LoadEquipmentChildrenAsync(equipmentId);
        node.Children.Clear();
@@ -558,21 +595,29 @@
    private void CloseModals()
    {
        _areaModalVisible = false;
+        _areaModalIsNew = false;
+        _areaModalClusterId = null;
        _areaModalExisting = null;
        _lineModalVisible = false;
+        _lineModalIsNew = false;
+        _lineModalAreaId = null;
        _lineModalExisting = null;
        _lineModalAreaOptions = Array.Empty<(string, string)>();
        _equipmentModalVisible = false;
+        _equipmentModalIsNew = false;
+        _equipmentModalLineId = null;
        _equipmentModalExisting = null;
        _equipmentModalLineOptions = Array.Empty<(string, string)>();
        _equipmentModalDriverOptions = Array.Empty<(string, string)>();
        _tagModalVisible = false;
-        _tagModalExisting = null;
+        _tagModalIsNew = false;
        _tagModalEquipmentId = null;
+        _tagModalExisting = null;
        _tagModalDriverOptions = Array.Empty<(string, string)>();
        _vtagModalVisible = false;
-        _vtagModalExisting = null;
+        _vtagModalIsNew = false;
        _vtagModalEquipmentId = null;
+        _vtagModalExisting = null;
        _vtagModalScriptOptions = Array.Empty<(string, string)>();
        _importModalVisible = false;
        _childRefreshEquipmentId = null;
@@ -38,8 +38,12 @@
                        <div class="row">
                            <div class="col-md-6 mb-3">
                                <label class="form-label" for="vtag-dtype">DataType</label>
-                                <InputText id="vtag-dtype" @bind-Value="_form.DataType" class="form-control form-control-sm mono"
-                                           placeholder="Double" />
+                                <InputSelect id="vtag-dtype" @bind-Value="_form.DataType" class="form-select form-select-sm">
+                                    @foreach (var dt in DataTypes)
+                                    {
+                                        <option value="@dt">@dt</option>
+                                    }
+                                </InputSelect>
                            </div>
                            <div class="col-md-6 mb-3">
                                <label class="form-label" for="vtag-script">Script</label>
@@ -101,6 +105,11 @@
 }

@code {
+    /// <summary>The OPC UA data types offered for a virtual tag — the same set the TagModal uses.</summary>
+    private static readonly string[] DataTypes =
+        ["Boolean", "SByte", "Byte", "Int16", "UInt16", "Int32", "UInt32",
+         "Int64", "UInt64", "Float", "Double", "String", "DateTime", "Guid", "ByteString"];
+
    /// <summary>Whether the modal is shown. The host owns this flag.</summary>
    [Parameter] public bool Visible { get; set; }