From eb4bce3e49ca561f633d27fc8a83a724345b1311 Mon Sep 17 00:00:00 2001
From: Joseph Doherty <dohejw01@gmail.com>
Date: Wed, 17 Jun 2026 20:44:34 -0400
Subject: [PATCH] =?UTF-8?q?refactor(kpi):=20K13/K15=20trend=20review=20fix?=
 =?UTF-8?q?ups=20=E2=80=94=20per-metric=20isolation,=20disable-during-load?=
 =?UTF-8?q?=20+=20logging,=20loading-flag=20finally,=20test=20coverage?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 .../Components/Pages/Audit/AuditLogPage.razor |  4 +-
 .../Pages/Audit/AuditLogPage.razor.cs         | 52 +++++++++----
 .../Components/Pages/Monitoring/Health.razor  |  1 +
 .../Notifications/NotificationKpis.razor      | 76 ++++++++++++-------
 .../Pages/SiteCalls/SiteCallsReport.razor.cs  | 30 +++++---
 .../Pages/HealthPageTests.cs                  |  5 +-
 .../Pages/NotificationKpisPageTests.cs        | 38 ++++++++++
 7 files changed, 151 insertions(+), 55 deletions(-)

diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Audit/AuditLogPage.razor b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Audit/AuditLogPage.razor
index 8d0a0612..bbf9e444 100644
--- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Audit/AuditLogPage.razor
+++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Audit/AuditLogPage.razor
@@ -31,12 +31,12 @@
                 <div class="btn-group btn-group-sm" role="group" aria-label="Trend window">
                     <button type="button"
                             class="btn @(_windowHours == 24 ? "btn-secondary" : "btn-outline-secondary")"
-                            @onclick="() => SetWindowAsync(24)">
+                            @onclick="() => SetWindowAsync(24)" disabled="@_trendsLoading">
                         24h
                     </button>
                     <button type="button"
                             class="btn @(_windowHours == 168 ? "btn-secondary" : "btn-outline-secondary")"
-                            @onclick="() => SetWindowAsync(168)">
+                            @onclick="() => SetWindowAsync(168)" disabled="@_trendsLoading">
                         7d
                     </button>
                 </div>
diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Audit/AuditLogPage.razor.cs b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Audit/AuditLogPage.razor.cs
index dfce1c10..17e69792 100644
--- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Audit/AuditLogPage.razor.cs
+++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Audit/AuditLogPage.razor.cs
@@ -2,6 +2,7 @@ using System.Globalization;
 using Microsoft.AspNetCore.Components;
 using Microsoft.AspNetCore.Components.Routing;
 using Microsoft.AspNetCore.WebUtilities;
+using Microsoft.Extensions.Logging;
 using ZB.MOM.WW.ScadaBridge.CentralUI.Services;
 using ZB.MOM.WW.ScadaBridge.Commons.Types.Audit;
 using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums;
@@ -56,6 +57,10 @@ public partial class AuditLogPage : IDisposable
     /// </summary>
     [Inject] private IKpiHistoryQueryService KpiHistory { get; set; } = null!;
 
+    /// <summary>Logger for the best-effort Trends panel — a degraded series fetch
+    /// is logged at warning level so the silent fallback is still observable.</summary>
+    [Inject] private ILogger<AuditLogPage> Logger { get; set; } = null!;
+
     private AuditLogQueryFilter? _currentFilter;
     private AuditEventView? _selectedEvent;
     private bool _drawerOpen;
@@ -271,6 +276,13 @@ public partial class AuditLogPage : IDisposable
     /// <summary>Whether the Trends panel is expanded (open by default).</summary>
     private bool _trendsOpen = true;
 
+    /// <summary>
+    /// True while a window's series are being (re)fetched — disables the 24h/7d
+    /// window toggle buttons so a mid-flight click cannot stack overlapping loads
+    /// (mirrors the K13/K14 trend pages).
+    /// </summary>
+    private bool _trendsLoading;
+
     /// <summary>Active window in hours — 24 (default) or 168 (7 days).</summary>
     private int _windowHours = 24;
 
@@ -297,25 +309,35 @@ public partial class AuditLogPage : IDisposable
     /// </summary>
     private async Task LoadTrendsAsync()
     {
-        var toUtc = DateTime.UtcNow;
-        var fromUtc = toUtc - TimeSpan.FromHours(_windowHours);
-
-        foreach (var (metric, _, _) in TrendMetrics)
+        _trendsLoading = true;
+        try
         {
-            try
+            var toUtc = DateTime.UtcNow;
+            var fromUtc = toUtc - TimeSpan.FromHours(_windowHours);
+
+            foreach (var (metric, _, _) in TrendMetrics)
             {
-                var points = await KpiHistory.GetSeriesAsync(
-                    KpiSources.AuditLog, metric, KpiScopes.Global, scopeKey: null,
-                    fromUtc, toUtc);
-                _trendSeries[metric] = new TrendSeries(points, IsAvailable: true, ErrorMessage: null);
-            }
-            catch (Exception)
-            {
-                // Best-effort: degrade this chart only, keep the rest of the page alive.
-                _trendSeries[metric] = new TrendSeries(
-                    Points: null, IsAvailable: false, ErrorMessage: "Trend data unavailable.");
+                try
+                {
+                    var points = await KpiHistory.GetSeriesAsync(
+                        KpiSources.AuditLog, metric, KpiScopes.Global, scopeKey: null,
+                        fromUtc, toUtc);
+                    _trendSeries[metric] = new TrendSeries(points, IsAvailable: true, ErrorMessage: null);
+                }
+                catch (Exception ex)
+                {
+                    // Best-effort: degrade this chart only, keep the rest of the page
+                    // alive — but log the failure so the silent fallback is observable.
+                    Logger.LogWarning(ex, "Failed to load Audit Log KPI trend series for metric {Metric}.", metric);
+                    _trendSeries[metric] = new TrendSeries(
+                        Points: null, IsAvailable: false, ErrorMessage: "Trend data unavailable.");
+                }
             }
         }
+        finally
+        {
+            _trendsLoading = false;
+        }
     }
 
     /// <summary>Returns the rendered state for a metric, defaulting to available-empty.</summary>
diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Monitoring/Health.razor b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Monitoring/Health.razor
index 9ecc5ebd..a2381855 100644
--- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Monitoring/Health.razor
+++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Monitoring/Health.razor
@@ -92,6 +92,7 @@
                     <select class="form-select form-select-sm" style="width:auto"
                             data-test="site-health-trends-site"
                             value="@_trendSiteId"
+                            @* Blazor wraps this async handler in an EventCallback, so the returned Task IS awaited — keep the Async signature; do not change it to a void/fire-and-forget handler. *@
                             @onchange="OnTrendSiteChangedAsync">
                         @foreach (var key in _trendSiteKeys)
                         {
diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Notifications/NotificationKpis.razor b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Notifications/NotificationKpis.razor
index 65fd8d93..e2032706 100644
--- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Notifications/NotificationKpis.razor
+++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/Notifications/NotificationKpis.razor
@@ -178,20 +178,20 @@
         <div class="col-lg-4 col-md-6">
             <KpiTrendChart Title="Queue Depth"
                            Points="@_queueDepthSeries"
-                           IsAvailable="@_trendsAvailable"
-                           ErrorMessage="@_trendsError" />
+                           IsAvailable="@_queueDepthAvailable"
+                           ErrorMessage="@_queueDepthError" />
         </div>
         <div class="col-lg-4 col-md-6">
             <KpiTrendChart Title="Parked"
                            Points="@_parkedSeries"
-                           IsAvailable="@_trendsAvailable"
-                           ErrorMessage="@_trendsError" />
+                           IsAvailable="@_parkedAvailable"
+                           ErrorMessage="@_parkedError" />
         </div>
         <div class="col-lg-4 col-md-6">
             <KpiTrendChart Title="Delivered / interval"
                            Points="@_deliveredSeries"
-                           IsAvailable="@_trendsAvailable"
-                           ErrorMessage="@_trendsError" />
+                           IsAvailable="@_deliveredAvailable"
+                           ErrorMessage="@_deliveredError" />
         </div>
     </div>
 </div>
@@ -213,13 +213,24 @@
 
     // ── Trends (T11: first KPI-history consumer) ──
     // Window in hours: 24h (default) or 168h (7d). Toggling re-queries.
+    // Per-metric isolation (mirrors the K14 SiteCallsReport pattern): each metric
+    // carries its own series + availability + error, each loaded via its own
+    // try/catch so one metric's failure only blanks that one chart and the others
+    // still render their already-fetched data.
     private int _windowHours = 24;
     private bool _trendsLoading;
-    private bool _trendsAvailable = true;
-    private string? _trendsError;
+
     private IReadOnlyList<KpiSeriesPoint>? _queueDepthSeries;
+    private bool _queueDepthAvailable = true;
+    private string? _queueDepthError;
+
     private IReadOnlyList<KpiSeriesPoint>? _parkedSeries;
+    private bool _parkedAvailable = true;
+    private string? _parkedError;
+
     private IReadOnlyList<KpiSeriesPoint>? _deliveredSeries;
+    private bool _deliveredAvailable = true;
+    private string? _deliveredError;
 
     protected override async Task OnInitializedAsync()
     {
@@ -264,21 +275,15 @@
             var toUtc = DateTime.UtcNow;
             var fromUtc = toUtc - TimeSpan.FromHours(_windowHours);
 
-            // Best-effort: one query failure must NOT break the page — on any
-            // exception the charts fall back to the unavailable placeholder while
-            // the KPI tiles above stay rendered.
-            _queueDepthSeries = await GetSeries("queueDepth", fromUtc, toUtc);
-            _parkedSeries = await GetSeries("parkedCount", fromUtc, toUtc);
-            _deliveredSeries = await GetSeries("deliveredLastInterval", fromUtc, toUtc);
-
-            _trendsAvailable = true;
-            _trendsError = null;
-        }
-        catch (Exception ex)
-        {
-            _trendsAvailable = false;
-            _trendsError = "Trend data unavailable.";
-            Logger.LogWarning(ex, "Failed to load notification-outbox KPI trend series.");
+            // Per-metric isolation: each series is loaded via its own try/catch so
+            // one metric's failure only blanks that one chart while the others still
+            // render their fetched data — and a throw never breaks the KPI tiles above.
+            (_queueDepthSeries, _queueDepthAvailable, _queueDepthError) =
+                await LoadSeries("queueDepth", fromUtc, toUtc);
+            (_parkedSeries, _parkedAvailable, _parkedError) =
+                await LoadSeries("parkedCount", fromUtc, toUtc);
+            (_deliveredSeries, _deliveredAvailable, _deliveredError) =
+                await LoadSeries("deliveredLastInterval", fromUtc, toUtc);
         }
         finally
         {
@@ -286,9 +291,28 @@
         }
     }
 
-    private Task<IReadOnlyList<KpiSeriesPoint>> GetSeries(string metric, DateTime fromUtc, DateTime toUtc) =>
-        KpiHistory.GetSeriesAsync(
-            KpiSources.NotificationOutbox, metric, KpiScopes.Global, scopeKey: null, fromUtc, toUtc);
+    /// <summary>
+    /// Fetch one NotificationOutbox / Global series, swallowing any failure into the
+    /// chart's unavailable state. Returns the points (null on failure), the
+    /// availability flag, and a short error message for the chart placeholder.
+    /// </summary>
+    private async Task<(IReadOnlyList<KpiSeriesPoint>? Points, bool Available, string? Error)>
+        LoadSeries(string metric, DateTime fromUtc, DateTime toUtc)
+    {
+        try
+        {
+            var points = await KpiHistory.GetSeriesAsync(
+                KpiSources.NotificationOutbox, metric, KpiScopes.Global, scopeKey: null, fromUtc, toUtc);
+            return (points, true, null);
+        }
+        catch (Exception ex)
+        {
+            // A KPI-history hiccup must never break the page — the chart degrades to
+            // its unavailable placeholder while the KPI tiles above stay rendered.
+            Logger.LogWarning(ex, "Failed to load notification-outbox KPI trend series for metric {Metric}.", metric);
+            return (null, false, "Trend data unavailable.");
+        }
+    }
 
     private async Task LoadGlobalKpis()
     {
diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/SiteCalls/SiteCallsReport.razor.cs b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/SiteCalls/SiteCallsReport.razor.cs
index 3b1b8056..742bfde5 100644
--- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/SiteCalls/SiteCallsReport.razor.cs
+++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Pages/SiteCalls/SiteCallsReport.razor.cs
@@ -585,18 +585,26 @@ public partial class SiteCallsReport
     private async Task LoadTrendsAsync()
     {
         _trendsLoading = true;
+        try
+        {
+            var toUtc = DateTime.UtcNow;
+            var fromUtc = toUtc - TimeSpan.FromHours(_windowHours);
 
-        var toUtc = DateTime.UtcNow;
-        var fromUtc = toUtc - TimeSpan.FromHours(_windowHours);
-
-        (_bufferedSeries, _bufferedAvailable, _bufferedError) =
-            await LoadSeriesAsync("buffered", fromUtc, toUtc);
-        (_parkedSeries, _parkedAvailable, _parkedError) =
-            await LoadSeriesAsync("parked", fromUtc, toUtc);
-        (_failedSeries, _failedAvailable, _failedError) =
-            await LoadSeriesAsync("failedLastInterval", fromUtc, toUtc);
-
-        _trendsLoading = false;
+            (_bufferedSeries, _bufferedAvailable, _bufferedError) =
+                await LoadSeriesAsync("buffered", fromUtc, toUtc);
+            (_parkedSeries, _parkedAvailable, _parkedError) =
+                await LoadSeriesAsync("parked", fromUtc, toUtc);
+            (_failedSeries, _failedAvailable, _failedError) =
+                await LoadSeriesAsync("failedLastInterval", fromUtc, toUtc);
+        }
+        finally
+        {
+            // try/finally so an unexpected throw before this line can't lock the
+            // spinner/disabled state on forever. The per-metric LoadSeriesAsync
+            // try/catches already swallow GetSeriesAsync failures, so this guards
+            // only the truly unexpected.
+            _trendsLoading = false;
+        }
     }
 
     /// <summary>
diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/HealthPageTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/HealthPageTests.cs
index 8a83fa75..b07b1de3 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/HealthPageTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/HealthPageTests.cs
@@ -237,10 +237,13 @@ public class HealthPageTests : BunitContext
             Assert.Contains("data-test=\"site-health-trends\"", cut.Markup);
             Assert.Contains("data-test=\"site-health-trends-site\"", cut.Markup);
             // The four metric charts render (the shared KpiTrendChart slug hook),
-            // and the seeded non-empty series draws a polyline.
+            // and the seeded non-empty series draws a polyline. The "S&F Buffer
+            // Depth" title slugifies to "s-f-buffer-depth" (the & and the spaces
+            // each collapse to a dash) — see KpiTrendChart.Slugify.
             Assert.Contains("kpi-trend-connections-down", cut.Markup);
             Assert.Contains("kpi-trend-dead-letters", cut.Markup);
             Assert.Contains("kpi-trend-script-errors", cut.Markup);
+            Assert.Contains("kpi-trend-s-f-buffer-depth", cut.Markup);
             Assert.Contains("<polyline", cut.Markup);
         });
     }
diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/NotificationKpisPageTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/NotificationKpisPageTests.cs
index f08eb0e4..8ee1f85c 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/NotificationKpisPageTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Pages/NotificationKpisPageTests.cs
@@ -178,6 +178,44 @@ public class NotificationKpisPageTests : BunitContext
         });
     }
 
+    [Fact]
+    public void PartialTrendFailure_IsolatesToTheFailingMetric()
+    {
+        // Per-metric isolation (K13 fixup): the substitute returns a known series
+        // for two metrics but THROWS for "parkedCount". The two surviving charts
+        // must still draw their polylines while only the failing chart degrades to
+        // the unavailable placeholder — one metric's failure no longer blanks all
+        // three (and does not discard the already-fetched siblings).
+        _kpiHistory.GetSeriesAsync(
+                Arg.Any<string>(), Arg.Is<string>(m => m == "parkedCount"), Arg.Any<string>(),
+                Arg.Any<string?>(), Arg.Any<DateTime>(), Arg.Any<DateTime>(),
+                Arg.Any<int?>(), Arg.Any<CancellationToken>())
+            .Returns<Task<IReadOnlyList<KpiSeriesPoint>>>(_ => throw new InvalidOperationException("parked metric down"));
+
+        var cut = Render<NotificationKpisPage>();
+
+        cut.WaitForAssertion(() =>
+        {
+            // The page is still alive — KPI tiles render and the trends section
+            // never went fully blank.
+            Assert.Contains("Queue Depth", cut.Markup);
+            Assert.NotNull(cut.Find("[data-test=\"notification-trends\"]"));
+
+            // The two surviving metrics (queueDepth, deliveredLastInterval) drew
+            // their polylines …
+            var queueChart = cut.Find("[data-test=\"kpi-trend-queue-depth\"]");
+            Assert.Contains("<polyline", queueChart.InnerHtml);
+            var deliveredChart = cut.Find("[data-test=\"kpi-trend-delivered-interval\"]");
+            Assert.Contains("<polyline", deliveredChart.InnerHtml);
+
+            // … while only the failing "Parked" chart degraded to the unavailable
+            // placeholder (no polyline, the short error message instead).
+            var parkedChart = cut.Find("[data-test=\"kpi-trend-parked\"]");
+            Assert.DoesNotContain("<polyline", parkedChart.InnerHtml);
+            Assert.Contains("Trend data unavailable.", parkedChart.InnerHtml);
+        });
+    }
+
     [Fact]
     public void TrendQueryFailure_DoesNotBreakPage()
     {