From ddcba23fc71b74cf6522b6978193744c6df2c423 Mon Sep 17 00:00:00 2001 From: Hamza Alqurneh Date: Sun, 20 Sep 2026 12:49:47 +0300 Subject: [PATCH] fix: six small backlog items HAM-71 log the document store reads and writes, not the key building HAM-72 name a pipeline stage for its state, not always in the past tense HAM-75 offer "+ New" for a retry policy where work group already does HAM-76 stop Back reopening a create form that was just submitted HAM-81 show a work group's queue, so two of the same name differ HAM-83 cap a run's exchange column and link the rest to Exchanges, via a new ReceiveAttemptId search filter --- .../Resources/Xchanges/BulkRetryPlanner.cs | 9 +- SW.Bitween.Api/Resources/Xchanges/Search.cs | 1 + .../Resources/Xchanges/XchangeFilters.cs | 53 ++++++++ SW.Bitween.Api/Services/XchangeService.cs | 43 +++++-- .../Tests/ReceiveAttemptFilterTests.cs | 121 ++++++++++++++++++ .../ClientApp/src/api/http/exchanges.ts | 4 + SW.Bitween.Web/ClientApp/src/api/types.ts | 6 + .../components/config/RetryPolicyDialog.tsx | 74 +++++++++++ .../ClientApp/src/lib/busMessageName.ts | 11 ++ .../pages/bus-gateways/studio/Inspector.tsx | 9 +- .../src/pages/exchanges/ExchangesPage.tsx | 3 +- .../exchanges/__tests__/journeyStages.test.ts | 65 ++++++++++ .../ClientApp/src/pages/exchanges/shared.tsx | 40 ++++-- .../global-values/GlobalValueSetsPage.tsx | 4 +- .../src/pages/notifiers/NotifiersPage.tsx | 4 +- .../retry-policies/RetryPoliciesPage.tsx | 51 +------- .../subscriptions/studio/LaneAndRetry.tsx | 47 +++++-- .../studio/ReceiveAttemptsPanel.tsx | 37 +++++- .../src/pages/work-groups/WorkGroupsPage.tsx | 10 +- 19 files changed, 495 insertions(+), 97 deletions(-) create mode 100644 SW.Bitween.IntegrationTests/Tests/ReceiveAttemptFilterTests.cs create mode 100644 SW.Bitween.Web/ClientApp/src/components/config/RetryPolicyDialog.tsx create mode 100644 SW.Bitween.Web/ClientApp/src/pages/exchanges/__tests__/journeyStages.test.ts diff --git a/SW.Bitween.Api/Resources/Xchanges/BulkRetryPlanner.cs b/SW.Bitween.Api/Resources/Xchanges/BulkRetryPlanner.cs index 7c65bd9c..9c0a6927 100644 --- a/SW.Bitween.Api/Resources/Xchanges/BulkRetryPlanner.cs +++ b/SW.Bitween.Api/Resources/Xchanges/BulkRetryPlanner.cs @@ -260,7 +260,7 @@ private async Task> ResolveSelection(XchangeBulkRetry request) // One past the limit is all it takes to know the selection is too big, and stops a // "select all" over a wide filter from reading a million ids to refuse them. - return await SelectionQuery(request) + return await (await SelectionQuery(request)) .Where(r => !exclude.Contains(r.Id)) .Select(r => r.Id) .Take(Limit + 1) @@ -270,7 +270,7 @@ private async Task> ResolveSelection(XchangeBulkRetry request) private async Task CountSelection(XchangeBulkRetry request) { var exclude = request.ExcludeIds ?? new List(); - return await SelectionQuery(request) + return await (await SelectionQuery(request)) .Where(r => !exclude.Contains(r.Id)) // Same reason the search caps its own count: counting every match has to visit every // matching row. The number is only being used to say "too many", so stopping early @@ -290,10 +290,13 @@ private async Task CountSelection(XchangeBulkRetry request) /// in C#. Filtering on a column that is not here would silently match nothing, so a new filter /// on the list needs a column here too. /// - private IQueryable SelectionQuery(XchangeBulkRetry request) + private async Task> SelectionQuery(XchangeBulkRetry request) { var searchyRequest = new SearchyRequest(request.Filter); searchyRequest.DatesToUtc(); + // Async, and inside here rather than in the two callers, so a "select all matching" over a + // run selects exactly the rows the list showed for it. + await XchangeFilters.ResolveReceiveAttemptFilterAsync(searchyRequest, _dbContext); var query = from xchange in _dbContext.Set() join result in _dbContext.Set() on xchange.Id equals result.Id into xr diff --git a/SW.Bitween.Api/Resources/Xchanges/Search.cs b/SW.Bitween.Api/Resources/Xchanges/Search.cs index 44929300..d8e352cf 100644 --- a/SW.Bitween.Api/Resources/Xchanges/Search.cs +++ b/SW.Bitween.Api/Resources/Xchanges/Search.cs @@ -34,6 +34,7 @@ public async Task Handle(SearchyRequest searchyRequest, bool lookup = fa await requestContext.EnsurePermission(dbContext, Model.Permissions.Exchanges.View, Model.Permissions.Dashboard.View); searchyRequest.DatesToUtc(); + await XchangeFilters.ResolveReceiveAttemptFilterAsync(searchyRequest, dbContext); await using var dr = await dbContext.Database.BeginTransactionAsync(IsolationLevel.ReadUncommitted); var query = from xchange in dbContext.Set() diff --git a/SW.Bitween.Api/Resources/Xchanges/XchangeFilters.cs b/SW.Bitween.Api/Resources/Xchanges/XchangeFilters.cs index c4e8fd3d..caaba804 100644 --- a/SW.Bitween.Api/Resources/Xchanges/XchangeFilters.cs +++ b/SW.Bitween.Api/Resources/Xchanges/XchangeFilters.cs @@ -1,4 +1,5 @@ using System.Linq; +using System.Threading.Tasks; using Microsoft.EntityFrameworkCore; using SW.Bitween.Domain; using SW.Bitween.Model; @@ -19,6 +20,58 @@ namespace SW.Bitween.Resources.Xchanges; /// internal static class XchangeFilters { + /// + /// Turns a ReceiveAttemptId filter into the Id filter the rest of the pipeline + /// already understands, by reading the run's own record of the exchanges it created. + /// + /// + /// Separate from , and async, because the run has to be read + /// before the query can be built. ReceiveAttempt.ExchangeIds is persisted as a single + /// separator-delimited string rather than an array (see BitweenDbContext), so no + /// subquery can reach inside it — and with three database providers in the solution there is + /// no one translation that would. + /// + /// It rewrites rather than filters here so the result stays identical to the long + /// ?ids=a,b,c URL this replaces, relatives and all — the only thing that changes is that + /// the ids no longer have to fit in a web address. + /// + internal static async Task ResolveReceiveAttemptFilterAsync(SearchyRequest searchyRequest, + BitweenDbContext dbContext) + { + var condition = searchyRequest.Conditions.FirstOrDefault(); + if (condition == null) + return; + + var attemptFilters = condition.Filters.Where(f => f.Field == "ReceiveAttemptId").ToList(); + foreach (var attemptFilter in attemptFilters) + { + if (!int.TryParse(attemptFilter.Value?.ToString(), out var attemptId)) + throw new SWValidationException("NOT_SUPPORTED", + $"'{attemptFilter.Value}' is not a receive attempt id."); + + var exchangeIds = await dbContext.Set().AsNoTracking() + .Where(a => a.Id == attemptId) + .Select(a => a.ExchangeIds) + .SingleOrDefaultAsync(); + + if (exchangeIds == null) + throw new SWValidationException("NOT_FOUND", + $"Run {attemptId} no longer exists. Its history may have been cleaned up."); + + // A run that created nothing leaves an empty array, and the Contains branch below reads + // that as "matches no row" — which is the honest answer. Dropping the filter instead + // would widen the selection to every exchange, and bulk retry acts on whatever this + // selects. + condition.Filters.Add(new SearchyFilter + { + Field = "Id", + Rule = SearchyRule.Contains, + ValueStringArray = exchangeIds, + }); + condition.Filters.Remove(attemptFilter); + } + } + /// /// Applies the special filters and removes them from , leaving /// the plain per-column ones for Searchy to handle. diff --git a/SW.Bitween.Api/Services/XchangeService.cs b/SW.Bitween.Api/Services/XchangeService.cs index 0aec3642..9194cef0 100644 --- a/SW.Bitween.Api/Services/XchangeService.cs +++ b/SW.Bitween.Api/Services/XchangeService.cs @@ -394,11 +394,22 @@ private async Task RunHandler(Xchange xchange, XchangeFile xchangeF private async Task AddFile(string xchangeId, XchangeFileType type, XchangeFile file) { - await cloudFiles.WriteTextAsync(file.Data, new WriteFileSettings + var key = GetFileKey(xchangeId, type); + try { - Public = !BitweenSettings.AreXChangeFilesPrivate, - Key = GetFileKey(xchangeId, type) - }); + await cloudFiles.WriteTextAsync(file.Data, new WriteFileSettings + { + Public = !BitweenSettings.AreXChangeFilesPrivate, + Key = key + }); + logger.LogDebug("Wrote the {FileType} file of xchange {XchangeId} to {Key}.", type, xchangeId, key); + } + catch (Exception ex) + { + logger.LogError(ex, "Could not write the {FileType} file of xchange {XchangeId} to {Key}.", + type, xchangeId, key); + throw; + } } public string GetFileUrl(string xchangeId, XchangeFileType type) @@ -415,23 +426,29 @@ public string GetFileKey(string xchangeId, int? fileSize, XchangeFileType type) { if (fileSize is null or 0) return null; - var key = $"{BitweenSettings.DocumentPrefix}/{xchangeId}/{type.ToString().ToLower()}"; - logger.LogInformation($"the file key is:'{key}'"); - return key; + return $"{BitweenSettings.DocumentPrefix}/{xchangeId}/{type.ToString().ToLower()}"; } private string GetFileKey(string xchangeId, XchangeFileType type) { - var key = $"{BitweenSettings.DocumentPrefix}/{xchangeId}/{type.ToString().ToLower()}"; - logger.LogInformation($"the file key is:'{key}'"); - return key; + return $"{BitweenSettings.DocumentPrefix}/{xchangeId}/{type.ToString().ToLower()}"; } public async Task GetFile(string xchangeId, XchangeFileType type) { - await using var cloudStream = await cloudFiles.OpenReadAsync(GetFileKey(xchangeId, type)); - using var reader = new StreamReader(cloudStream); - return await reader.ReadToEndAsync(); + var key = GetFileKey(xchangeId, type); + try + { + await using var cloudStream = await cloudFiles.OpenReadAsync(key); + using var reader = new StreamReader(cloudStream); + return await reader.ReadToEndAsync(); + } + catch (Exception ex) + { + logger.LogError(ex, "Could not read the {FileType} file of xchange {XchangeId} from {Key}.", + type, xchangeId, key); + throw; + } } private async Task Process(XchangeMessage message) diff --git a/SW.Bitween.IntegrationTests/Tests/ReceiveAttemptFilterTests.cs b/SW.Bitween.IntegrationTests/Tests/ReceiveAttemptFilterTests.cs new file mode 100644 index 00000000..992794e8 --- /dev/null +++ b/SW.Bitween.IntegrationTests/Tests/ReceiveAttemptFilterTests.cs @@ -0,0 +1,121 @@ +using System.Linq; +using System.Threading.Tasks; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using SW.Bitween.Domain; +using SW.Bitween.IntegrationTests.Fixtures; +using SW.Bitween.Model; +using SW.PrimitiveTypes; +using Xunit; + +namespace SW.Bitween.IntegrationTests.Tests; + +/// +/// The ReceiveAttemptId search filter: "the exchanges one run created". +/// +/// +/// It exists so the UI can link to a run's exchanges without naming every one of them in the URL, +/// which put a ceiling on how big a run could be linked to. The filter is resolved before the +/// query rather than as a subquery, because ReceiveAttempt.ExchangeIds is stored as one +/// separator-delimited string and no provider can see inside it. +/// +[Collection("Bitween")] +public class ReceiveAttemptFilterTests(BitweenFixture fixture) +{ + private static SearchyRequest ForAttempt(int attemptId) => + new("ReceiveAttemptId", SearchyRule.EqualsTo, attemptId.ToString()); + + /// A subscription with one exchange, and a run that claims to have created it. + private static async Task<(int AttemptId, string XchangeId)> ARunThatCreated( + BitweenDbContext db, XchangeService xs, string name) + { + var doc = new Document(null, name, DocumentFormat.Json); + db.Set().Add(doc); + await db.SaveChangesAsync(); + + var sub = new Subscription(name, doc.Id) { Inactive = false }; + db.Set().Add(sub); + await db.SaveChangesAsync(); + + var xchange = await xs.CreateXchange(sub, new XchangeFile("{}")); + await db.SaveChangesAsync(); + + var attempt = new ReceiveAttempt + { + SubscriptionId = sub.Id, + StartedOn = System.DateTime.UtcNow, + FinishedOn = System.DateTime.UtcNow, + Outcome = ReceiveOutcome.Received, + ExchangeIds = [xchange.Id], + }; + db.Set().Add(attempt); + await db.SaveChangesAsync(); + + return (attempt.Id, xchange.Id); + } + + private static async Task Search(AsyncServiceScope scope, SearchyRequest request) + { + var db = scope.ServiceProvider.GetRequiredService(); + var xs = scope.ServiceProvider.GetRequiredService(); + var response = (SearchyResponse)await new Resources.Xchanges.Search( + db, xs, scope.Superuser()).Handle(request); + return response.Result.Select(r => r.Id).ToArray(); + } + + [Fact] + public async Task Returns_the_run_s_own_exchanges_and_nobody_else_s() + { + await using var scope = fixture.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var xs = scope.ServiceProvider.GetRequiredService(); + + var mine = await ARunThatCreated(db, xs, "Attempt filter mine"); + // A second run, so "returns everything" cannot pass by accident. + var theirs = await ARunThatCreated(db, xs, "Attempt filter theirs"); + + var found = await Search(scope, ForAttempt(mine.AttemptId)); + + Assert.Equal([mine.XchangeId], found); + Assert.DoesNotContain(theirs.XchangeId, found); + } + + [Fact] + public async Task A_run_that_created_nothing_matches_nothing_rather_than_everything() + { + await using var scope = fixture.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var xs = scope.ServiceProvider.GetRequiredService(); + + // Exchanges exist; this run just is not responsible for any of them. Dropping the filter + // instead of honouring it would return all of them — and bulk retry acts on this selection. + await ARunThatCreated(db, xs, "Attempt filter bystander"); + + var empty = new ReceiveAttempt + { + SubscriptionId = 1, + StartedOn = System.DateTime.UtcNow, + FinishedOn = System.DateTime.UtcNow, + Outcome = ReceiveOutcome.NoNewData, + ExchangeIds = [], + }; + db.Set().Add(empty); + await db.SaveChangesAsync(); + + Assert.Empty(await Search(scope, ForAttempt(empty.Id))); + } + + [Fact] + public async Task A_run_that_no_longer_exists_is_refused() + { + await using var scope = fixture.CreateScope(); + + // Refused rather than ignored, for the same reason: a silently dropped filter widens the + // selection to every exchange. ReceiveAttemptCleanupJob does delete old runs, so a stale + // link is a real case, not a hypothetical one. + var ex = await Assert.ThrowsAsync( + () => Search(scope, ForAttempt(int.MaxValue))); + + Assert.Contains(ex.Validations, v => v.Key == "NOT_FOUND"); + } +} diff --git a/SW.Bitween.Web/ClientApp/src/api/http/exchanges.ts b/SW.Bitween.Web/ClientApp/src/api/http/exchanges.ts index 9084e746..c669bf45 100644 --- a/SW.Bitween.Web/ClientApp/src/api/http/exchanges.ts +++ b/SW.Bitween.Web/ClientApp/src/api/http/exchanges.ts @@ -181,6 +181,10 @@ function buildExchangeFilters(query: ExchangeQuery): URLSearchParams { const ids = query.ids.split(/[\s,|]+/).filter(Boolean); params.append("filter", `Id:4:text|${ids.join("|")}`); } + // The server turns this into the same Id filter above, from the run's own record of what it + // created — so a run of any size is one short parameter rather than every id in the URL. + if (query.receiveAttemptId !== undefined) + params.append("filter", `ReceiveAttemptId:1:${query.receiveAttemptId}`); if (query.correlationId?.trim()) params.append("filter", `CorrelationId:1:${query.correlationId.trim()}`); if (query.latest) params.append("filter", "LatestOnly:1:true"); // PromotedPropertiesRaw is stored as "key:value,key:value", so prefixing the key turns diff --git a/SW.Bitween.Web/ClientApp/src/api/types.ts b/SW.Bitween.Web/ClientApp/src/api/types.ts index 1c210fe7..5de7e457 100644 --- a/SW.Bitween.Web/ClientApp/src/api/types.ts +++ b/SW.Bitween.Web/ClientApp/src/api/types.ts @@ -1126,6 +1126,12 @@ export interface ExchangeQuery { informationTypeId?: number; /** Comma/pipe/newline separated; matches id, retryFor OR aggregationXchangeId. */ ids?: string; + /** + * The exchanges one run of a scheduled job created. The server reads the run's own list of + * them, so this says the same thing as spelling every id into `ids` — at any size, and + * without putting them all in the URL. + */ + receiveAttemptId?: number; correlationId?: string; /** * Only the newest attempt of each retry chain. A chain is one piece of work however many diff --git a/SW.Bitween.Web/ClientApp/src/components/config/RetryPolicyDialog.tsx b/SW.Bitween.Web/ClientApp/src/components/config/RetryPolicyDialog.tsx new file mode 100644 index 00000000..c8c8edc2 --- /dev/null +++ b/SW.Bitween.Web/ClientApp/src/components/config/RetryPolicyDialog.tsx @@ -0,0 +1,74 @@ +import { useState, type FormEvent } from "react"; +import { useNavigate } from "react-router"; +import { useMutation, useQueryClient } from "@tanstack/react-query"; +import { api } from "../../api"; +import { Button, FormError } from "../ui/basics"; +import { Field, TextInput } from "../ui/forms"; +import { Dialog } from "../ui/overlays"; +import { keys } from "../../api/queryKeys"; + +/** + * Creating a retry policy, as one component. + * + * A policy is born with nothing but a name — its budget, groups and alert routing are + * configured on its own page, which is too much to ask for inside a dialog. So the two + * callers want opposite things once it exists, and `onCreated` is what tells them apart: + * the policies list sends you to the new policy's page to carry on configuring it, while + * a subscription attaching one wants to stay where it is with the policy now selected. + */ +export function CreateRetryPolicyDialog({ + onClose, + /** Given, the new policy is handed back and the dialog closes. Omitted, we navigate to it. */ + onCreated, +}: { + onClose: () => void; + onCreated?: (id: number) => void; +}) { + const navigate = useNavigate(); + const queryClient = useQueryClient(); + const [name, setName] = useState(""); + + const create = useMutation({ + mutationFn: () => api.createRetryPolicy({ name }), + onSuccess: (policy) => { + void queryClient.invalidateQueries({ queryKey: keys.retryPolicies.all }); + if (onCreated) { + onCreated(policy.id); + onClose(); + return; + } + // replace, not push: the ?new=1 entry this dialog opened on is still behind us, + // and Back onto it would reopen the form that was just submitted. + navigate(`/retry-policies/${policy.id}`, { replace: true }); + }, + }); + + const submit = (e: FormEvent) => { + e.preventDefault(); + create.mutate(); + }; + + return ( + +
+ + setName(e.target.value)} + placeholder="e.g. Transient failures" + /> + + {create.error?.message} +
+ + +
+
+ + ); +} diff --git a/SW.Bitween.Web/ClientApp/src/lib/busMessageName.ts b/SW.Bitween.Web/ClientApp/src/lib/busMessageName.ts index 7c03c243..57d60c1c 100644 --- a/SW.Bitween.Web/ClientApp/src/lib/busMessageName.ts +++ b/SW.Bitween.Web/ClientApp/src/lib/busMessageName.ts @@ -20,3 +20,14 @@ export const busMessageNameProblem = (name: string): string | null => * `MyMessage` and `mymessage` are the same message on the wire. */ export const BUS_MESSAGE_NAME_PLACEHOLDER = "PurchaseOrderReceived"; + +/** + * A work group's real queue name, as `WorkGroup.GetBusMessageName()` builds it server-side: + * the id, then the bus message name. + * + * It is the only thing about a group that is guaranteed to be unique. Nothing stops two + * groups sharing both a name and a bus message name — local data has two called `test` + * whose bus name is also `test` — and the id is what actually tells such a pair apart. + */ +export const workGroupQueueName = (group: { id: number; busMessageName: string }): string => + `${group.id}${group.busMessageName}`; diff --git a/SW.Bitween.Web/ClientApp/src/pages/bus-gateways/studio/Inspector.tsx b/SW.Bitween.Web/ClientApp/src/pages/bus-gateways/studio/Inspector.tsx index 2b8b6b74..a885b71e 100644 --- a/SW.Bitween.Web/ClientApp/src/pages/bus-gateways/studio/Inspector.tsx +++ b/SW.Bitween.Web/ClientApp/src/pages/bus-gateways/studio/Inspector.tsx @@ -11,6 +11,7 @@ import { MatchExpressionEditor } from "../../../components/config/MatchExpressio import { HealthBadge } from "../../../components/config/shared"; import { ResponseFields } from "../../subscriptions/studio/ResponseFields"; import { keys } from "../../../api/queryKeys"; +import { workGroupQueueName } from "../../../lib/busMessageName"; import { BUS_NODES, type BusNodeId, @@ -281,7 +282,13 @@ export function SubscriptionBody({ disabled={disabled} onChange={(v) => onChange({ workGroupId: v === "" ? null : Number(v) })} clearLabel="Ungrouped (default lane)" - options={(workGroups.data ?? []).map((w) => ({ value: String(w.id), label: w.name }))} + options={(workGroups.data ?? []).map((w) => ({ + value: String(w.id), + label: w.name, + // Two groups may share a name AND a bus message name, so neither tells them + // apart. The queue name carries the id, which is the part that always differs. + code: workGroupQueueName(w), + }))} /> diff --git a/SW.Bitween.Web/ClientApp/src/pages/exchanges/ExchangesPage.tsx b/SW.Bitween.Web/ClientApp/src/pages/exchanges/ExchangesPage.tsx index 395cb3a0..775721cf 100644 --- a/SW.Bitween.Web/ClientApp/src/pages/exchanges/ExchangesPage.tsx +++ b/SW.Bitween.Web/ClientApp/src/pages/exchanges/ExchangesPage.tsx @@ -35,7 +35,7 @@ const REFRESH_OPTIONS = [ ]; /** Everything except paging counts as "a filter" for the Clear affordance. */ -const FILTER_KEYS = ["status", "subscriptionId", "partnerId", "informationTypeId", "ids", "correlationId", "propertyKey", "property", "from", "to", "latest"] as const; +const FILTER_KEYS = ["status", "subscriptionId", "partnerId", "informationTypeId", "ids", "receiveAttemptId", "correlationId", "propertyKey", "property", "from", "to", "latest"] as const; const readQuery = (sp: URLSearchParams): ExchangeQuery => ({ status: (sp.get("status") as ExchangeStatus | null) ?? undefined, @@ -43,6 +43,7 @@ const readQuery = (sp: URLSearchParams): ExchangeQuery => ({ partnerId: sp.get("partnerId") ? Number(sp.get("partnerId")) : undefined, informationTypeId: sp.get("informationTypeId") ? Number(sp.get("informationTypeId")) : undefined, ids: sp.get("ids") ?? undefined, + receiveAttemptId: sp.get("receiveAttemptId") ? Number(sp.get("receiveAttemptId")) : undefined, correlationId: sp.get("correlationId") ?? undefined, latest: sp.get("latest") === "1" || undefined, propertyKey: sp.get("propertyKey") ?? undefined, diff --git a/SW.Bitween.Web/ClientApp/src/pages/exchanges/__tests__/journeyStages.test.ts b/SW.Bitween.Web/ClientApp/src/pages/exchanges/__tests__/journeyStages.test.ts new file mode 100644 index 00000000..a2d8f3d5 --- /dev/null +++ b/SW.Bitween.Web/ClientApp/src/pages/exchanges/__tests__/journeyStages.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, it } from "vitest"; +import type { ExchangeFileRef, ExchangeRow, ExchangeStatus } from "../../../api"; +import { journeyStages } from "../shared"; + +/** + * A stage's name is a claim about what happened to the document. The cards used to be named + * "Mapped" and "Handled" whatever state they were in, so a Processing exchange read as though + * both stages were behind it — "Mapped" sitting beside a Running badge. + */ +const A_FILE = { name: "doc.json", size: 12 } as ExchangeFileRef; + +/** Only the three fields `journeyStages` reads; the rest of the row does not reach it. */ +const row = (status: ExchangeStatus, over: { mapperSkipped?: boolean; mapped?: boolean } = {}) => + ({ + status, + mapperSkipped: over.mapperSkipped ?? false, + files: { input: A_FILE, mapped: over.mapped ? A_FILE : null, handled: null }, + }) as ExchangeRow; + +/** The stage keys are stable; only the words shown to a reader move. */ +const labels = (x: ExchangeRow) => journeyStages(x).map((s) => `${s.label}:${s.state}`); + +describe("journeyStages labels", () => { + it("names a stage in the past tense only once it is behind us", () => { + expect(labels(row("success", { mapped: true }))).toEqual([ + "Received:done", + "Mapped:done", + "Handled:done", + ]); + }); + + it("does not claim a Processing exchange was mapped or handled", () => { + expect(labels(row("processing"))).toEqual([ + "Received:done", + "Mapping:running", + "Handling:running", + ]); + }); + + it("does not claim a skipped mapping stage was mapped", () => { + // The note already says "No mapper configured"; the title must not contradict it. + expect(labels(row("processing", { mapperSkipped: true }))).toEqual([ + "Received:done", + "Mapping:skipped", + "Handling:running", + ]); + }); + + it("does not claim a handler was reached when mapping failed first", () => { + expect(labels(row("failed"))).toEqual([ + "Received:done", + "Mapped:failed", + "Handling:notReached", + ]); + }); + + it("keeps the past tense for a delivery the response rejected", () => { + // badResponse means it was handled — the far end just did not like it. + expect(labels(row("badResponse", { mapped: true }))).toEqual([ + "Received:done", + "Mapped:done", + "Handled:bad", + ]); + }); +}); diff --git a/SW.Bitween.Web/ClientApp/src/pages/exchanges/shared.tsx b/SW.Bitween.Web/ClientApp/src/pages/exchanges/shared.tsx index bb4313f4..f2a56079 100644 --- a/SW.Bitween.Web/ClientApp/src/pages/exchanges/shared.tsx +++ b/SW.Bitween.Web/ClientApp/src/pages/exchanges/shared.tsx @@ -36,30 +36,44 @@ export interface JourneyStage { note?: string; } +/** + * Past tense only once the stage is actually behind us. "Mapped" beside a Running badge + * claims something that has not happened yet — while it is in flight, or was skipped, or + * was never reached, the stage is named for the work rather than for a result it lacks. + */ +function stageLabel(key: JourneyStage["key"], state: StageState): string { + if (key === "Input") return "Received"; + const finished = state === "done" || state === "bad" || state === "failed"; + if (key === "Mapped") return finished ? "Mapped" : "Mapping"; + return finished ? "Handled" : "Handling"; +} + /** Derives what happened at each pipeline stage from the row's fields. */ export function journeyStages(x: ExchangeRow): JourneyStage[] { - const mapped: JourneyStage = x.mapperSkipped - ? { key: "Mapped", label: "Mapped", state: "skipped", note: "No mapper configured" } + const mapped: Omit = x.mapperSkipped + ? { key: "Mapped", state: "skipped", note: "No mapper configured" } : x.files.mapped - ? { key: "Mapped", label: "Mapped", state: "done" } + ? { key: "Mapped", state: "done" } : x.status === "processing" - ? { key: "Mapped", label: "Mapped", state: "running" } + ? { key: "Mapped", state: "running" } : x.status === "failed" - ? { key: "Mapped", label: "Mapped", state: "failed", note: "Failed while mapping" } - : { key: "Mapped", label: "Mapped", state: "notReached" }; + ? { key: "Mapped", state: "failed", note: "Failed while mapping" } + : { key: "Mapped", state: "notReached" }; const handlerReached = !(mapped.state === "failed"); - const handled: JourneyStage = !handlerReached - ? { key: "Handled", label: "Handled", state: "notReached", note: "Never reached" } + const handled: Omit = !handlerReached + ? { key: "Handled", state: "notReached", note: "Never reached" } : x.status === "success" - ? { key: "Handled", label: "Handled", state: "done" } + ? { key: "Handled", state: "done" } : x.status === "badResponse" - ? { key: "Handled", label: "Handled", state: "bad", note: "Delivered, but the response reports an error" } + ? { key: "Handled", state: "bad", note: "Delivered, but the response reports an error" } : x.status === "failed" - ? { key: "Handled", label: "Handled", state: "failed", note: "Failed while handling" } - : { key: "Handled", label: "Handled", state: "running" }; + ? { key: "Handled", state: "failed", note: "Failed while handling" } + : { key: "Handled", state: "running" }; - return [{ key: "Input", label: "Received", state: "done" }, mapped, handled]; + return [{ key: "Input", state: "done" } as Omit, mapped, handled].map( + (s) => ({ ...s, label: stageLabel(s.key, s.state) }), + ); } const STRIP_COLORS: Record = { diff --git a/SW.Bitween.Web/ClientApp/src/pages/global-values/GlobalValueSetsPage.tsx b/SW.Bitween.Web/ClientApp/src/pages/global-values/GlobalValueSetsPage.tsx index f059641f..c6508da8 100644 --- a/SW.Bitween.Web/ClientApp/src/pages/global-values/GlobalValueSetsPage.tsx +++ b/SW.Bitween.Web/ClientApp/src/pages/global-values/GlobalValueSetsPage.tsx @@ -25,7 +25,9 @@ function CreateValueSetDialog({ onClose }: { onClose: () => void }) { mutationFn: () => api.createValueSet({ id: slug, name, values: {} }), onSuccess: (set) => { void queryClient.invalidateQueries({ queryKey: keys.valueSets.all }); - navigate(`/global-values/${set.id}`); + // replace, not push: the ?new=1 entry this dialog opened on is still behind us, + // and Back onto it would reopen the form that was just submitted. + navigate(`/global-values/${set.id}`, { replace: true }); }, }); diff --git a/SW.Bitween.Web/ClientApp/src/pages/notifiers/NotifiersPage.tsx b/SW.Bitween.Web/ClientApp/src/pages/notifiers/NotifiersPage.tsx index e7cfd2aa..0ed1d724 100644 --- a/SW.Bitween.Web/ClientApp/src/pages/notifiers/NotifiersPage.tsx +++ b/SW.Bitween.Web/ClientApp/src/pages/notifiers/NotifiersPage.tsx @@ -23,7 +23,9 @@ function CreateNotifierDialog({ onClose }: { onClose: () => void }) { mutationFn: () => api.createNotifier({ name }), onSuccess: (notifier) => { void queryClient.invalidateQueries({ queryKey: keys.notifiers.all }); - navigate(`/notifiers/${notifier.id}`); + // replace, not push: the ?new=1 entry this dialog opened on is still behind us, + // and Back onto it would reopen the form that was just submitted. + navigate(`/notifiers/${notifier.id}`, { replace: true }); }, }); diff --git a/SW.Bitween.Web/ClientApp/src/pages/retry-policies/RetryPoliciesPage.tsx b/SW.Bitween.Web/ClientApp/src/pages/retry-policies/RetryPoliciesPage.tsx index 54b49398..b78db440 100644 --- a/SW.Bitween.Web/ClientApp/src/pages/retry-policies/RetryPoliciesPage.tsx +++ b/SW.Bitween.Web/ClientApp/src/pages/retry-policies/RetryPoliciesPage.tsx @@ -1,61 +1,16 @@ -import { useState, type FormEvent } from "react"; import { useNavigate, useSearchParams } from "react-router"; -import { keepPreviousData, useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { keepPreviousData, useQuery } from "@tanstack/react-query"; import { Plus, RotateCcw, Search } from "lucide-react"; import { api } from "../../api"; import { Can } from "../../auth/guards"; import { PageHeader } from "../../components/layout/PageHeader"; -import { Button, EmptyState, FormError, LoadingBlock } from "../../components/ui/basics"; -import { Field, TextInput } from "../../components/ui/forms"; -import { Dialog } from "../../components/ui/overlays"; +import { Button, EmptyState, LoadingBlock } from "../../components/ui/basics"; import { Pagination } from "../../components/ui/Pagination"; import { Table } from "../../components/ui/Table"; import { UsedByCell, useSubscriptionsCache } from "../../components/config/shared"; +import { CreateRetryPolicyDialog } from "../../components/config/RetryPolicyDialog"; import { keys } from "../../api/queryKeys"; -function CreateRetryPolicyDialog({ onClose }: { onClose: () => void }) { - const navigate = useNavigate(); - const queryClient = useQueryClient(); - const [name, setName] = useState(""); - - const create = useMutation({ - mutationFn: () => api.createRetryPolicy({ name }), - onSuccess: (policy) => { - void queryClient.invalidateQueries({ queryKey: keys.retryPolicies.all }); - navigate(`/retry-policies/${policy.id}`); - }, - }); - - const submit = (e: FormEvent) => { - e.preventDefault(); - create.mutate(); - }; - - return ( - -
- - setName(e.target.value)} - placeholder="e.g. Transient failures" - /> - - {create.error?.message} -
- - -
-
- - ); -} - const PAGE_SIZE = 25; export function RetryPoliciesPage() { diff --git a/SW.Bitween.Web/ClientApp/src/pages/subscriptions/studio/LaneAndRetry.tsx b/SW.Bitween.Web/ClientApp/src/pages/subscriptions/studio/LaneAndRetry.tsx index ded02619..dbfc5472 100644 --- a/SW.Bitween.Web/ClientApp/src/pages/subscriptions/studio/LaneAndRetry.tsx +++ b/SW.Bitween.Web/ClientApp/src/pages/subscriptions/studio/LaneAndRetry.tsx @@ -5,7 +5,9 @@ import { api } from "../../../api"; import { useSessionCan } from "../../../auth/guards"; import { SearchSelect } from "../../../components/ui/SearchSelect"; import { WorkGroupDialog } from "../../../components/config/WorkGroupDialog"; +import { CreateRetryPolicyDialog } from "../../../components/config/RetryPolicyDialog"; import { keys } from "../../../api/queryKeys"; +import { workGroupQueueName } from "../../../lib/busMessageName"; import { Fact } from "./Fact"; /** @@ -36,6 +38,7 @@ export function LaneAndRetry({ idPrefix: string; }) { const canCreateWorkGroup = useSessionCan("workgroups.create"); + const canCreateRetryPolicy = useSessionCan("retry-policies.create"); const workGroups = useQuery({ queryKey: keys.workGroups.list, queryFn: () => api.listWorkGroups() }); const retryPolicies = useQuery({ queryKey: keys.retryPolicies.list, @@ -43,6 +46,7 @@ export function LaneAndRetry({ }); /** undefined = closed, null = creating, number = editing that group. */ const [groupDialog, setGroupDialog] = useState(undefined); + const [creatingPolicy, setCreatingPolicy] = useState(false); return ( <> @@ -54,7 +58,13 @@ export function LaneAndRetry({ disabled={!canEdit} onChange={(v) => onWorkGroupChange(v === "" ? null : Number(v))} clearLabel="Ungrouped (default lane)" - options={(workGroups.data ?? []).map((w) => ({ value: String(w.id), label: w.name }))} + options={(workGroups.data ?? []).map((w) => ({ + value: String(w.id), + label: w.name, + // Two groups may share a name AND a bus message name, so neither tells them + // apart. The queue name carries the id, which is the part that always differs. + code: workGroupQueueName(w), + }))} />
@@ -89,14 +99,25 @@ export function LaneAndRetry({ options={(retryPolicies.data ?? []).map((p) => ({ value: String(p.id), label: p.name }))} />
- {retryPolicyId !== null && ( - - View - - )} +
+ {retryPolicyId !== null && ( + + View + + )} + {canCreateRetryPolicy && ( + + )} +
{groupDialog !== undefined && ( )} + {creatingPolicy && ( + // Selects the new policy rather than navigating to it — the rest of the policy is + // configured behind "View", and leaving here would abandon the subscription edit. + setCreatingPolicy(false)} + onCreated={onRetryPolicyChange} + /> + )} ); } diff --git a/SW.Bitween.Web/ClientApp/src/pages/subscriptions/studio/ReceiveAttemptsPanel.tsx b/SW.Bitween.Web/ClientApp/src/pages/subscriptions/studio/ReceiveAttemptsPanel.tsx index a61f08c0..86d7ad7c 100644 --- a/SW.Bitween.Web/ClientApp/src/pages/subscriptions/studio/ReceiveAttemptsPanel.tsx +++ b/SW.Bitween.Web/ClientApp/src/pages/subscriptions/studio/ReceiveAttemptsPanel.tsx @@ -1,7 +1,7 @@ import { useState } from "react"; import { Link } from "react-router"; import { keepPreviousData, useQuery } from "@tanstack/react-query"; -import { Check, ChevronDown, ChevronRight, Copy } from "lucide-react"; +import { ArrowUpRight, Check, ChevronDown, ChevronRight, Copy } from "lucide-react"; import { api, type ReceiveAttemptRow, type ReceiveOutcome } from "../../../api"; import { Badge, EmptyState, LoadingBlock } from "../../../components/ui/basics"; import { Select } from "../../../components/ui/forms"; @@ -153,11 +153,26 @@ function AttemptResult({ attempt, kind }: { attempt: ReceiveAttemptRow; kind: At ); } -function AttemptExchanges({ exchanges }: { exchanges: ReceiveAttemptRow["exchanges"] }) { +/** + * A single poll can pick up dozens of documents, and each one is a line here. Uncapped, + * one busy run makes a row taller than the screen while its When and Result cells stay + * two short lines — so the column is capped and the rest is one click away. + */ +const EXCHANGES_SHOWN = 5; + +function AttemptExchanges({ + attemptId, + exchanges, +}: { + attemptId: number; + exchanges: ReceiveAttemptRow["exchanges"]; +}) { if (exchanges.length === 0) return —; + + const hidden = exchanges.length - EXCHANGES_SHOWN; return ( - - {exchanges.map((x) => ( + + {exchanges.slice(0, EXCHANGES_SHOWN).map((x) => ( ))} + {hidden > 0 && ( + // One parameter whatever the run picked up: the server reads the run's own list of the + // exchanges it created, so this never has to spell them into the URL. + e.stopPropagation()} + className="flex items-center gap-1 text-[11px] font-medium text-crimson-700 hover:underline" + > + View all {exchanges.length} in Exchanges + + + )} ); } @@ -253,7 +280,7 @@ export function ReceiveAttemptsPanel({ { header: w.exchangeHeader, headerTitle: w.exchangeHint, - cell: (a) => , + cell: (a) => , }, ]} /> diff --git a/SW.Bitween.Web/ClientApp/src/pages/work-groups/WorkGroupsPage.tsx b/SW.Bitween.Web/ClientApp/src/pages/work-groups/WorkGroupsPage.tsx index d92c99b5..8f33e56c 100644 --- a/SW.Bitween.Web/ClientApp/src/pages/work-groups/WorkGroupsPage.tsx +++ b/SW.Bitween.Web/ClientApp/src/pages/work-groups/WorkGroupsPage.tsx @@ -12,6 +12,7 @@ import { Table, type Column } from "../../components/ui/Table"; import { UsedByCell, queueHealthTitle, useSubscriptionsCache } from "../../components/config/shared"; import { keys } from "../../api/queryKeys"; import { useRabbitMqManagementConfigured } from "../../lib/appConfig"; +import { workGroupQueueName } from "../../lib/busMessageName"; /** * The live RabbitMQ numbers, as columns rather than a per-row drill-down. @@ -159,8 +160,13 @@ export function WorkGroupsPage() { columns={[ { header: "Name", cell: (g) => {g.name} }, { - header: "Bus message name", - cell: (g) => {g.busMessageName}, + // The queue rather than the bare bus message name: nothing stops two groups + // sharing both a name and a bus message name, and then neither column tells + // them apart. The queue carries the id, so it always does — and it is what + // these rows are called in RabbitMQ. + header: "Queue", + headerTitle: "The group's queue in RabbitMQ — its id followed by its bus message name.", + cell: (g) => {workGroupQueueName(g)}, }, { header: "Used by",