From d73b459057a2d9e796a120a1adbe8657966e7522 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 21 May 2026 04:59:12 -0400 Subject: [PATCH] fix(centralui): single relay toast, paging/skip polish, extra Site Calls tests --- .../Pages/SiteCalls/SiteCallsReport.razor | 3 + .../Pages/SiteCalls/SiteCallsReport.razor.cs | 34 ++++--- ...ScadaLink.CentralUI.PlaywrightTests.csproj | 7 ++ .../SiteCalls/SiteCallsPageTests.cs | 92 +++++++++++++++---- .../Pages/SiteCallsReportPageTests.cs | 82 +++++++++++++++++ 5 files changed, 190 insertions(+), 28 deletions(-) diff --git a/src/ScadaLink.CentralUI/Components/Pages/SiteCalls/SiteCallsReport.razor b/src/ScadaLink.CentralUI/Components/Pages/SiteCalls/SiteCallsReport.razor index 019b316..30ece7b 100644 --- a/src/ScadaLink.CentralUI/Components/Pages/SiteCalls/SiteCallsReport.razor +++ b/src/ScadaLink.CentralUI/Components/Pages/SiteCalls/SiteCallsReport.razor @@ -201,6 +201,9 @@ backwards and the response's NextAfter* cursor to step forwards. *@
+ @* No "of N" total: keyset paging has no cheap total-count, so + the label is intentionally page-number-only. Do not "fix" + this by adding a total — that would require a COUNT(*). *@ Page @(_cursorStack.Count + 1) · @_siteCalls.Count rows
diff --git a/src/ScadaLink.CentralUI/Components/Pages/SiteCalls/SiteCallsReport.razor.cs b/src/ScadaLink.CentralUI/Components/Pages/SiteCalls/SiteCallsReport.razor.cs index 2a37606..079c009 100644 --- a/src/ScadaLink.CentralUI/Components/Pages/SiteCalls/SiteCallsReport.razor.cs +++ b/src/ScadaLink.CentralUI/Components/Pages/SiteCalls/SiteCallsReport.razor.cs @@ -224,10 +224,20 @@ public partial class SiteCallsReport } /// - /// Surface a relay outcome on the toast. The - /// case is deliberately distinct from a generic failure — the action was not - /// applied but the operator can retry once the site is back online. + /// Surface a relay outcome on the toast — exactly one toast per relay + /// response. The case is + /// deliberately distinct from a generic failure: the action was not applied + /// but the operator can retry once the site is back online. /// + /// + /// The switch is exhaustive, so it owns + /// the single toast. is a redundant + /// cross-check on the same signal (the contract sets it false only + /// for ); it is folded + /// INTO the case rather + /// than firing a second toast — an OperationFailed response that also + /// reports an unreachable site shows the unreachable wording, once. + /// private void ShowRelayOutcome( SiteCallRelayOutcome outcome, bool siteReachable, string? errorMessage, string appliedMessage) { @@ -245,19 +255,19 @@ public partial class SiteCallsReport ?? "Site unreachable — the relay did not reach the owning site. " + "Try again once the site is back online."); break; + case SiteCallRelayOutcome.OperationFailed when !siteReachable: + // An OperationFailed response that nonetheless reports the site + // unreachable: trust the reachability signal and show the + // unreachable wording instead of the generic failure message. + _toast.ShowError(errorMessage + ?? "Site unreachable — the relay did not reach the owning site. " + + "Try again once the site is back online."); + break; case SiteCallRelayOutcome.OperationFailed: default: _toast.ShowError(errorMessage ?? "The site could not apply the action."); break; } - - // Defensive: a non-Applied/non-Unreachable outcome that somehow reports an - // unreachable site still gets the unreachable wording. - if (outcome != SiteCallRelayOutcome.SiteUnreachable && !siteReachable - && outcome != SiteCallRelayOutcome.Applied) - { - _toast.ShowError("Site unreachable — the relay did not reach the owning site."); - } } private async Task ShowDetail(SiteCallSummary c) @@ -353,6 +363,8 @@ public partial class SiteCallsReport ? null : new DateTimeOffset(DateTime.SpecifyKind(value.Value, DateTimeKind.Utc)); + // A Guid's "N" format is always exactly 32 hex chars, so the [..12] slice is + // always in range — no length guard needed. private static string ShortId(Guid id) => id.ToString("N")[..12]; private static string StatusBadgeClass(string status) => status switch diff --git a/tests/ScadaLink.CentralUI.PlaywrightTests/ScadaLink.CentralUI.PlaywrightTests.csproj b/tests/ScadaLink.CentralUI.PlaywrightTests/ScadaLink.CentralUI.PlaywrightTests.csproj index dd87843..e89e456 100644 --- a/tests/ScadaLink.CentralUI.PlaywrightTests/ScadaLink.CentralUI.PlaywrightTests.csproj +++ b/tests/ScadaLink.CentralUI.PlaywrightTests/ScadaLink.CentralUI.PlaywrightTests.csproj @@ -15,6 +15,13 @@ + + diff --git a/tests/ScadaLink.CentralUI.PlaywrightTests/SiteCalls/SiteCallsPageTests.cs b/tests/ScadaLink.CentralUI.PlaywrightTests/SiteCalls/SiteCallsPageTests.cs index 6a6c6e9..242c197 100644 --- a/tests/ScadaLink.CentralUI.PlaywrightTests/SiteCalls/SiteCallsPageTests.cs +++ b/tests/ScadaLink.CentralUI.PlaywrightTests/SiteCalls/SiteCallsPageTests.cs @@ -1,4 +1,5 @@ using Microsoft.Playwright; +using Xunit; namespace ScadaLink.CentralUI.PlaywrightTests.SiteCalls; @@ -27,8 +28,16 @@ namespace ScadaLink.CentralUI.PlaywrightTests.SiteCalls; /// Audit Log pre-filtered to the call's TrackedOperationId. /// RetryDiscardVisibility — Retry/Discard appear only on Parked /// rows, never on Failed (or other) rows. +/// RetryClickThrough — clicking Retry on a Parked row confirms +/// the dialog, relays to the owning site, and surfaces an outcome toast. /// /// +/// +/// +/// The DB-seeding tests are + Skip.IfNot: +/// when the cluster / MSSQL is unreachable they report as Skipped (not Failed), +/// matching the established ScadaLink.ConfigurationDatabase.Tests idiom. +/// /// [Collection("Playwright")] public class SiteCallsPageTests @@ -55,15 +64,15 @@ public class SiteCallsPageTests await Assertions.Expect(page.Locator("[data-test='site-calls-query']")).ToBeVisibleAsync(); } - [Fact] + /// Skip reason shared by the DB-seeding tests when MSSQL is down. + private const string DbUnavailableSkipReason = + "SiteCallDataSeeder cannot reach MSSQL at localhost:1433 — bring up infra/docker-compose and docker/deploy.sh, " + + "or set SCADALINK_PLAYWRIGHT_DB to a reachable connection string."; + + [SkippableFact] public async Task FilterNarrowing_ChannelFilterShrinksGrid() { - if (!await SiteCallDataSeeder.IsAvailableAsync()) - { - throw new InvalidOperationException( - "SiteCallDataSeeder cannot reach MSSQL at localhost:1433 — bring up infra/docker-compose and docker/deploy.sh, " + - "or set SCADALINK_PLAYWRIGHT_DB to a reachable connection string."); - } + Skip.IfNot(await SiteCallDataSeeder.IsAvailableAsync(), DbUnavailableSkipReason); var runId = Guid.NewGuid().ToString("N"); var targetPrefix = $"playwright-test/sc-filter/{runId}/"; @@ -112,13 +121,10 @@ public class SiteCallsPageTests } } - [Fact] + [SkippableFact] public async Task DrillIn_ViewAuditHistory_NavigatesToPreFilteredAuditLog() { - if (!await SiteCallDataSeeder.IsAvailableAsync()) - { - throw new InvalidOperationException("MSSQL unavailable; see FilterNarrowing test for setup instructions."); - } + Skip.IfNot(await SiteCallDataSeeder.IsAvailableAsync(), DbUnavailableSkipReason); var runId = Guid.NewGuid().ToString("N"); var targetPrefix = $"playwright-test/sc-drill-in/{runId}/"; @@ -162,13 +168,10 @@ public class SiteCallsPageTests } } - [Fact] + [SkippableFact] public async Task RetryDiscard_VisibleOnlyOnParkedRows() { - if (!await SiteCallDataSeeder.IsAvailableAsync()) - { - throw new InvalidOperationException("MSSQL unavailable; see FilterNarrowing test for setup instructions."); - } + Skip.IfNot(await SiteCallDataSeeder.IsAvailableAsync(), DbUnavailableSkipReason); var runId = Guid.NewGuid().ToString("N"); var targetPrefix = $"playwright-test/sc-actions/{runId}/"; @@ -221,4 +224,59 @@ public class SiteCallsPageTests await SiteCallDataSeeder.DeleteByTargetPrefixAsync(targetPrefix); } } + + [SkippableFact] + public async Task RetryClickThrough_OnParkedRow_ConfirmsRelayAndShowsOutcomeToast() + { + Skip.IfNot(await SiteCallDataSeeder.IsAvailableAsync(), DbUnavailableSkipReason); + + var runId = Guid.NewGuid().ToString("N"); + var targetPrefix = $"playwright-test/sc-retry-click/{runId}/"; + var parkedId = Guid.NewGuid(); + var now = DateTime.UtcNow; + + try + { + // A single Parked row — the only status from which Retry/Discard can + // be relayed to the owning site. + await SiteCallDataSeeder.InsertSiteCallAsync( + trackedOperationId: parkedId, channel: "ApiOutbound", target: targetPrefix + "parked", + sourceSite: "plant-a", status: "Parked", retryCount: 3, + lastError: "HTTP 503 from ERP", httpStatus: 503, + createdAtUtc: now, updatedAtUtc: now); + + var page = await _fixture.NewAuthenticatedPageAsync(); + await page.GotoAsync($"{PlaywrightFixture.BaseUrl}{SiteCallsUrl}"); + await page.WaitForLoadStateAsync(LoadState.NetworkIdle); + + await page.Locator("#sc-search").FillAsync(targetPrefix + "parked"); + await page.Locator("[data-test='site-calls-query']").ClickAsync(); + await page.WaitForLoadStateAsync(LoadState.NetworkIdle); + + var parkedRow = page.Locator("tbody tr", new() { HasText = targetPrefix + "parked" }); + await Assertions.Expect(parkedRow).ToBeVisibleAsync(); + + // Click Retry — this opens the confirmation dialog (DialogHost modal). + await parkedRow.Locator("button:has-text('Retry')").ClickAsync(); + + // Confirm the relay in the dialog footer ("Confirm" — the non-danger + // label; Discard would render "Delete"). + var confirmButton = page.Locator(".modal-footer button:has-text('Confirm')"); + await Assertions.Expect(confirmButton).ToBeVisibleAsync(); + await confirmButton.ClickAsync(); + + // The relay outcome surfaces on a toast — Applied, NotParked or, if + // the owning site is offline in this environment, SiteUnreachable. + // We only assert that an outcome toast appears (exactly one — the + // single-toast contract), not which one, since the live cluster + // state determines the outcome. + var toast = page.Locator(".toast"); + await Assertions.Expect(toast).ToBeVisibleAsync(); + Assert.Equal(1, await toast.CountAsync()); + } + finally + { + await SiteCallDataSeeder.DeleteByTargetPrefixAsync(targetPrefix); + } + } } diff --git a/tests/ScadaLink.CentralUI.Tests/Pages/SiteCallsReportPageTests.cs b/tests/ScadaLink.CentralUI.Tests/Pages/SiteCallsReportPageTests.cs index b665cc4..e4fa93d 100644 --- a/tests/ScadaLink.CentralUI.Tests/Pages/SiteCallsReportPageTests.cs +++ b/tests/ScadaLink.CentralUI.Tests/Pages/SiteCallsReportPageTests.cs @@ -329,6 +329,88 @@ public class SiteCallsReportPageTests : BunitContext }); } + [Fact] + public void Paging_PrevButton_PopsBackStackAndRefetchesPriorCursor() + { + // The keyset back-stack is the trickiest paging path: Next pushes the + // current cursor, Prev pops it and refetches that prior page. Page 1 is + // opened with the empty (null, null) cursor, so after Next→Previous the + // follow-up query must carry (null, null) again. + var firstPage = new List(); + for (var i = 0; i < 50; i++) + { + firstPage.Add(new SiteCallSummary( + Guid.NewGuid(), "plant-a", "ApiOutbound", $"ERP.Op{i}", "Delivered", + RetryCount: 0, LastError: null, HttpStatus: 200, + CreatedAtUtc: DateTime.UtcNow.AddMinutes(-i), UpdatedAtUtc: DateTime.UtcNow.AddMinutes(-i), + TerminalAtUtc: DateTime.UtcNow.AddMinutes(-i), IsStuck: false)); + } + + var cursorCreated = new DateTime(2026, 5, 20, 12, 0, 0, DateTimeKind.Utc); + var cursorId = Guid.Parse("99999999-9999-9999-9999-999999999999"); + _queryReply = new SiteCallQueryResponse( + "q", true, null, firstPage, + NextAfterCreatedAtUtc: cursorCreated, + NextAfterId: cursorId); + + var cut = Render(); + cut.WaitForState(() => cut.Markup.Contains("ERP.Op0")); + + // Step forward — query 2 carries the keyset cursor. + var next = cut.Find("[data-test='site-calls-next']"); + next.Click(); + cut.WaitForAssertion(() => + { + Assert.Equal(2, _queryRequests.Count); + Assert.Equal(cursorCreated, _queryRequests[1].AfterCreatedAtUtc); + }); + + // Previous is now live (the back-stack has one entry); click it. + var prev = cut.Find("[data-test='site-calls-prev']"); + Assert.False(prev.HasAttribute("disabled")); + prev.Click(); + + cut.WaitForAssertion(() => + { + // Query 3 is the Previous refetch — the back-stack popped the page-1 + // cursor, which is the empty (null, null) first-page cursor. + Assert.Equal(3, _queryRequests.Count); + Assert.Null(_queryRequests[2].AfterCreatedAtUtc); + Assert.Null(_queryRequests[2].AfterId); + // Back on page 1, the back-stack is empty again so Previous re-disables. + Assert.True(cut.Find("[data-test='site-calls-prev']").HasAttribute("disabled")); + }); + } + + [Fact] + public void RetryRelay_NotParked_ShowsInfoMessage_AndExactlyOneToast() + { + // NotParked is a definitive answer from the site (nothing to do), not a + // failure — it surfaces as a single info toast, never an error. This + // also guards the single-toast contract: a non-Applied outcome must + // produce exactly one toast. + _retryReply = new RetrySiteCallResponse( + "q", SiteCallRelayOutcome.NotParked, Success: false, SiteReachable: true, + ErrorMessage: "The cached call is no longer parked."); + + var cut = Render(); + cut.WaitForState(() => cut.Markup.Contains("ERP.GetOrder")); + + var parkedRow = cut.FindAll("tbody tr") + .First(r => r.TextContent.Contains("ERP.GetOrder")); + parkedRow.QuerySelectorAll("button") + .First(b => b.TextContent.Contains("Retry")) + .Click(); + + cut.WaitForAssertion(() => + { + Assert.Contains("no longer parked", cut.Markup); + // Exactly one toast — the ShowRelayOutcome switch owns the single + // toast; no second (error) toast piggybacks on the same response. + Assert.Single(cut.FindAll(".toast")); + }); + } + protected override void Dispose(bool disposing) { if (disposing)