From b87d8772705619931dcf0a8820dcd3c582b9508a Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 9 Jun 2026 08:18:15 -0400 Subject: [PATCH] refactor(uns): modal-polish nits on the global UNS page (task #137) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../Components/Pages/Uns/GlobalUns.razor | 227 +++++++++++------- .../Shared/Uns/VirtualTagModal.razor | 13 +- 2 files changed, 147 insertions(+), 93 deletions(-) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Uns/GlobalUns.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Uns/GlobalUns.razor index 940e9cba..666bc43a 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Uns/GlobalUns.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Uns/GlobalUns.razor @@ -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 @@ /// 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; } } /// Opens the create modal for a new virtual tag scoped to the clicked equipment. 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; + } } /// @@ -307,60 +329,69 @@ /// 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 @@ /// /// 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). /// 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; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/VirtualTagModal.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/VirtualTagModal.razor index 4b8408b7..a1767c05 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/VirtualTagModal.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/VirtualTagModal.razor @@ -38,8 +38,12 @@
- + + @foreach (var dt in DataTypes) + { + + } +
@@ -101,6 +105,11 @@ } @code { + /// The OPC UA data types offered for a virtual tag — the same set the TagModal uses. + private static readonly string[] DataTypes = + ["Boolean", "SByte", "Byte", "Int16", "UInt16", "Int32", "UInt32", + "Int64", "UInt64", "Float", "Double", "String", "DateTime", "Guid", "ByteString"]; + /// Whether the modal is shown. The host owns this flag. [Parameter] public bool Visible { get; set; }