diff --git a/src/FluentCMS.Admin/Components/Pages/ChangeSetView.razor b/src/FluentCMS.Admin/Components/Pages/ChangeSetView.razor index d50826d0d..7d95ac1b6 100644 --- a/src/FluentCMS.Admin/Components/Pages/ChangeSetView.razor +++ b/src/FluentCMS.Admin/Components/Pages/ChangeSetView.razor @@ -6,6 +6,7 @@ @using FluentCMS.Core.Desk @inject IChangeSetDirectory Directory @inject IDeskQueues Queues +@inject FluentCMS.Core.Checks.IChangeSetChecks Checks @inject IAuditReader Audit @inject NavigationManager Navigation @inject IStringLocalizer L @@ -90,6 +91,14 @@ } + @if (set.State == ChangeSetState.Open) + { +
+ + + + + } @if (set.State == ChangeSetState.Approved) {
@@ -132,6 +141,10 @@ } @* A form posted from a page shown before the set moved on must still find its handler, which then says the set moved on. *@ + @if (set.State != ChangeSetState.Open) + { +
+ } @if (set.State != ChangeSetState.AwaitingReview) { @@ -188,17 +201,74 @@

@L["Checks"]

 - @if (set.State == ChangeSetState.ChecksFailed) + @if (_checks?.Run is { } run) + { + + @CheckLabel(run.Status) + @L["Run {0}, {1}", run.Number, run.Mode == FluentCMS.Core.Checks.CheckRunMode.Inline ? L["checked on submit"] : L["checked in the background"]] + @Time(run.FinishedAt) + @if (run.Cost > 0) + { + @run.Cost.ToString("0.00####", CultureInfo.InvariantCulture) USD + } + + } + else if (_checks?.Job is { } job) { - @L["The checks failed for this set."] + @L["The checks are running: {0} of {1} items done.", job.Progress.Done, job.Progress.Total?.ToString(CultureInfo.CurrentCulture) ?? "?"] } else { - @L["No check results are stored for this set."] + @L["The checks run when the set is submitted."] } - @L["Results per check appear here once checks run on every change set."] + @if (_checks?.Run is { } results && (results.Problems.Count > 0 || results.Items.Any(item => item.Results.Count > 0))) + { + + @if (results.Problems.Count > 0) + { +
    + @foreach (var problem in results.Problems) + { +
  • @problem
    • + } +
+ } + @foreach (var item in results.Items.Where(item => item.Results.Count > 0)) + { +
+

@item.ContentType @item.ItemId.ToString("N")[..8]

+
    + @foreach (var result in item.Results.OrderBy(result => CheckOrder(result.Status))) + { +
  • + + @CheckLabel(result.Status) + @result.Check + @(result.Severity == FluentCMS.Core.Checks.CheckSeverity.Block ? L["blocks"] : L["warns"]) + + @foreach (var finding in result.Findings) + { + + @finding.Message + @if (finding.Pointer.Length > 0) + { + @finding.Pointer + } + @if (finding.Excerpt is { } excerpt) + { + @excerpt + } + + } +
    • + } +
+
+ } + + } @@ -260,9 +330,13 @@ [SupplyParameterFromQuery(Name = "done")] private string? Done { get; set; } + private FluentCMS.Core.Checks.ChangeSetChecksState? _checks; private DeskQueue? _queue; private Guid? _next; + [SupplyParameterFromForm(FormName = "submit")] + private ReviewForm? Submit { get; set; } + [SupplyParameterFromForm(FormName = "approve")] private ReviewForm? Approve { get; set; } @@ -300,6 +374,7 @@ [ new(["J", "K"], L["Move to the next or previous item"]), new(["Enter"], L["Fold or unfold the item that has focus"]), + new(["S"], L["Submit an open set for checks"]), new(["A"], L["Approve the set and move to the next one"]), new(["R"], L["Reject the set with a reason"]), new(["P"], L["Publish an approved set"]), @@ -378,9 +453,22 @@ _stateSince = lastMove?.Timestamp; _seen = lastMove?.Id.ToString("N"); _next = await NextAsync(); + var checks = await Checks.GetAsync(Id); + _checks = checks.IsSuccess ? checks.Value : null; return true; } + private async Task SubmitAsync() + { + await ActAsync(Submit, async () => + { + var submitted = await Checks.SubmitAsync(Id); + return submitted.IsSuccess + ? FluentCMS.Core.Results.Result.Success(submitted.Value.ChangeSet) + : FluentCMS.Core.Results.Result.Failure(submitted.Errors); + }, L["Submitted. The checks run on the set before it is reviewed."]); + } + private async Task ApproveAsync() { if (await ActAsync(Approve, () => Directory.TransitionAsync(Id, ChangeSetState.Approved), L["Approved. Publish the set to make its items live."]) is not null) @@ -456,13 +544,42 @@ return result.IsSuccess ? result.Value : null; } + private static string CheckSlug(FluentCMS.Core.Checks.CheckStatus status) => status switch + { + FluentCMS.Core.Checks.CheckStatus.NeedsReview => "needs-review", + _ => status.ToString().ToLowerInvariant(), + }; + + private static int CheckOrder(FluentCMS.Core.Checks.CheckStatus status) => status switch + { + FluentCMS.Core.Checks.CheckStatus.Fail => 0, + FluentCMS.Core.Checks.CheckStatus.NeedsReview => 1, + FluentCMS.Core.Checks.CheckStatus.Warn => 2, + _ => 3, + }; + + private static string CheckVariant(FluentCMS.Core.Checks.CheckStatus status) => status switch + { + FluentCMS.Core.Checks.CheckStatus.Fail => "destructive", + FluentCMS.Core.Checks.CheckStatus.Pass => "secondary", + _ => "outline", + }; + + private string CheckLabel(FluentCMS.Core.Checks.CheckStatus status) => status switch + { + FluentCMS.Core.Checks.CheckStatus.Pass => L["Passed"], + FluentCMS.Core.Checks.CheckStatus.Warn => L["Warning"], + FluentCMS.Core.Checks.CheckStatus.Fail => L["Failed"], + _ => L["Needs review"], + }; + private string ReviewHint(ChangeSetState state) => state switch { ChangeSetState.AwaitingReview => L["Approve the set, or reject it with a reason."], ChangeSetState.ChecksFailed => L["The checks failed. Whoever opened the set can fix and resubmit it, or you can reject it with a reason."], ChangeSetState.Approved => L["The set is approved. Publishing it makes all its items live together."], ChangeSetState.Published => L["The set is live. Reverting it opens a set that undoes it, which is reviewed like any other."], - ChangeSetState.Open => L["The set is still open, so it cannot be reviewed yet."], + ChangeSetState.Open => L["The set is still open. Submit it when it holds every change; the checks run, then a person reviews it."], ChangeSetState.ChecksRunning => L["The checks are running, so it cannot be reviewed yet."], ChangeSetState.Rejected => L["The set was rejected. Nothing more can be done with it."], _ => L["The set was reverted. Nothing more can be done with it."], diff --git a/tests/FluentCMS.IntegrationTests/Desk/ReviewActionTests.cs b/tests/FluentCMS.IntegrationTests/Desk/ReviewActionTests.cs index af09226a7..578c4909d 100644 --- a/tests/FluentCMS.IntegrationTests/Desk/ReviewActionTests.cs +++ b/tests/FluentCMS.IntegrationTests/Desk/ReviewActionTests.cs @@ -1,4 +1,5 @@ using System.Net; +using System.Text.Json; using System.Text.RegularExpressions; using FluentCMS.Core.Audit; using FluentCMS.Core.ChangeSets; @@ -56,6 +57,48 @@ public async Task Reviewing_from_a_queue_moves_on_to_its_next_set_and_back_to_th Assert.Contains("data-testid=\"desk-done\"", await host.Client.GetStringAsync(host.LocationOf(rejected)!)); } + [Fact] + public async Task Submitting_an_open_set_runs_its_checks_and_the_view_shows_each_result_with_its_evidence() + { + using var install = new TemporaryInstall(); + await using var host = await AdminHost.StartAsync(install); + var ownerId = await host.CreateFirstAdministratorAsync(); + await host.AsUserAsync(ownerId, async types => (await types.CreateAsync(new NewContentType( + "Product page", "product-page", new ContentTypeDefinition([new FieldDefinition("headline", "text", required: true)])))).Value); + await host.AsUserAsync(ownerId, async checks => + { + var definition = new FluentCMS.Core.Checks.CheckDefinition( + "terms", + "No superlatives.", + JsonDocument.Parse("""{"terms": ["best in class"]}""").RootElement, + [ + new FluentCMS.Core.Checks.CheckExample("passes", FluentCMS.Core.Checks.CheckVerdict.Pass, JsonDocument.Parse("""{"headline": "A kettle"}""").RootElement, contentType: "product-page"), + new FluentCMS.Core.Checks.CheckExample("fails", FluentCMS.Core.Checks.CheckVerdict.Fail, JsonDocument.Parse("""{"headline": "Best in class"}""").RootElement, contentType: "product-page"), + ], + scope: new FluentCMS.Core.Checks.CheckScope(["product-page"])); + Assert.True((await checks.CreateAsync(new FluentCMS.Core.Checks.NewCheck("no-superlatives", definition))).IsSuccess); + return (await checks.ActivateAsync("no-superlatives", 1)).IsSuccess; + }); + var entry = await host.AsUserAsync(ownerId, async entries => (await entries.CreateAsync( + new FluentCMS.Core.Content.NewEntry("product-page", JsonDocument.Parse("""{"headline": "The best in class kettle"}""").RootElement))).Value); + var set = await host.AsUserAsync(ownerId, async directory => + { + var opened = (await directory.OpenAsync(new NewChangeSet("Launch the kettle.", []))).Value; + return (await directory.AddOperationAsync(opened.Id, ChangeOperation.Create(OperationKind.Create, ItemKind.Entry, entry.Id, null, 1).Value)).Value; + }); + await host.SignInAsync(AdminHost.Owner); + Assert.Equal(["submit"], Actions(await host.Client.GetStringAsync($"/sets/{set.Id}"))); + + var submitted = await (await ActAsync(host, set.Id, "submit")).Content.ReadAsStringAsync(); + + Assert.Contains("data-state=\"checks-failed\"", submitted); + Assert.Contains("data-testid=\"check-run-status\" data-status=\"fail\"", submitted); + var result = submitted[submitted.IndexOf("data-check=\"no-superlatives\"", StringComparison.Ordinal)..]; + Assert.StartsWith("data-check=\"no-superlatives\" data-status=\"fail\"", result); + Assert.Contains("/headline", result); + Assert.Contains("best in class", result); + } + [Fact] public async Task A_rejection_needs_a_reason_which_the_view_then_shows() { diff --git a/tests/FluentCMS.Tests/Admin/ChangeSetViewTests.cs b/tests/FluentCMS.Tests/Admin/ChangeSetViewTests.cs index 5f85b0f4c..1422f9913 100644 --- a/tests/FluentCMS.Tests/Admin/ChangeSetViewTests.cs +++ b/tests/FluentCMS.Tests/Admin/ChangeSetViewTests.cs @@ -3,6 +3,7 @@ using FluentCMS.Admin.Components.Review; using FluentCMS.Core.Audit; using FluentCMS.Core.ChangeSets; +using FluentCMS.Core.Checks; using FluentCMS.Core.Desk; using FluentCMS.Core.Entities; using FluentCMS.Core.Identity; @@ -97,23 +98,41 @@ public void A_revert_set_links_to_the_set_it_reverts_and_a_rejected_set_shows_it } [Fact] - public void Says_where_check_results_and_the_routing_decision_will_appear() + public void The_checks_card_shows_the_latest_run_with_failing_results_first_and_their_evidence() { var sets = new FakeSets { State = ChangeSetState.ChecksFailed }; using var context = Context(sets); var cut = context.Render(page => page.Add(p => p.Id, sets.Set.Id)); - Assert.Contains("The checks failed for this set.", cut.Find("[data-testid=change-set-checks]").TextContent); + var card = cut.Find("[data-testid=change-set-checks]"); + Assert.Equal("fail", card.QuerySelector("[data-testid=check-run-status]")!.GetAttribute("data-status")); + Assert.Equal(["no-superlatives", "short-headline"], card.QuerySelectorAll("[data-check]").Select(check => check.GetAttribute("data-check"))); + var finding = card.QuerySelector("[data-check=no-superlatives] [data-finding]")!; + Assert.Contains("Remove 'best in class'.", finding.TextContent); + Assert.Equal("/headline", finding.QuerySelector("code")!.TextContent); + Assert.Equal("Best in class", finding.QuerySelector("q")!.TextContent); + Assert.Contains("The engine could not read one item.", card.QuerySelector("[data-testid=check-problems]")!.TextContent); Assert.Contains("Routing has not decided on this set.", cut.Find("[data-testid=change-set-routing]").TextContent); } + [Fact] + public void A_set_still_being_checked_shows_the_job_progress() + { + var sets = new FakeSets { State = ChangeSetState.ChecksRunning }; + using var context = Context(sets); + + var cut = context.Render(page => page.Add(p => p.Id, sets.Set.Id)); + + Assert.Contains("3 of 10 items done", cut.Find("[data-testid=change-set-checks]").TextContent); + } + [Theory] [InlineData(ChangeSetState.AwaitingReview, new[] { "approve", "reject" })] [InlineData(ChangeSetState.ChecksFailed, new[] { "reject" })] [InlineData(ChangeSetState.Approved, new[] { "publish" })] [InlineData(ChangeSetState.Published, new[] { "revert" })] - [InlineData(ChangeSetState.Open, new string[0])] + [InlineData(ChangeSetState.Open, new[] { "submit" })] [InlineData(ChangeSetState.ChecksRunning, new string[0])] [InlineData(ChangeSetState.Rejected, new string[0])] [InlineData(ChangeSetState.Reverted, new string[0])] @@ -129,8 +148,8 @@ public void The_review_panel_offers_only_the_actions_the_state_allows(ChangeSetS Assert.NotEmpty(review.QuerySelector("[data-slot=card-description]")!.TextContent.Trim()); // Every action's form is on the page, so a post from an older page still reaches its handler; only the offered ones show. - Assert.Equal(4, review.QuerySelectorAll("form").Length); - Assert.Equal(4 - actions.Length, review.QuerySelectorAll("form[hidden]").Length); + Assert.Equal(5, review.QuerySelectorAll("form").Length); + Assert.Equal(5 - actions.Length, review.QuerySelectorAll("form[hidden]").Length); Assert.All( review.QuerySelectorAll("input[name$='.Seen']"), seen => Assert.Matches("^[0-9a-f]{32}$", seen.GetAttribute("value"))); @@ -153,7 +172,7 @@ public void The_actions_have_their_keys_and_a_set_opened_from_a_queue_links_back Assert.Equal($"sets/{next}?queue=awaiting-anyone", cut.Find("[data-testid=next-in-queue]").GetAttribute("href")); Assert.Equal("N", cut.Find("[data-testid=next-in-queue]").GetAttribute("aria-keyshortcuts")); Assert.Equal("The previous set was approved.", cut.Find("[data-testid=review-done]").TextContent); - Assert.Equal(8, cut.FindAll("[data-testid=shortcut-list] dt").Count); + Assert.Equal(9, cut.FindAll("[data-testid=shortcut-list] dt").Count); } [Fact] @@ -240,6 +259,7 @@ private static AdminTestContext Context(FakeSets sets) context.Services.AddSingleton(sets); context.Services.AddSingleton(new FakeHistory(sets)); context.Services.AddSingleton(new QueueAfter(sets)); + context.Services.AddSingleton(new FakeChecks(sets)); context.Renderer.SetRendererInfo(new RendererInfo("Static", isInteractive: false)); return context; } @@ -331,6 +351,45 @@ public Task> ListChangeSetsAsync(DeskQueue queue, int offs public Task> ListStaleFactsAsync(int offset, int limit, CancellationToken cancellationToken = default) => throw new NotSupportedException(); } + private sealed class FakeChecks(FakeSets sets) : IChangeSetChecks + { + public Task> GetAsync(Guid changeSetId, CancellationToken cancellationToken = default) + { + var job = new FluentCMS.Core.Jobs.Job + { + Id = Guid.NewGuid(), + Type = "checks.run", + Payload = default, + State = FluentCMS.Core.Jobs.JobState.Running, + Attempts = 1, + MaxAttempts = 6, + CreatedAt = Opened, + ScheduledAt = Opened, + Progress = new FluentCMS.Core.Jobs.JobProgress(3, 10, null), + }; + if (sets.State is not ChangeSetState.ChecksFailed) + { + return Task.FromResult(Result.Success(new ChangeSetChecksState(sets.Set, null, sets.State == ChangeSetState.ChecksRunning ? job : null))); + } + + CheckResult Result(string check, CheckStatus status, params CheckFinding[] findings) => + new(Guid.NewGuid(), check, 1, "terms", CheckKind.Deterministic, CheckSeverity.Block, "A check.", status, findings, null); + var item = new ItemChecks(ItemKind.Entry, Guid.NewGuid(), 1, "product-page", + [ + Result("short-headline", CheckStatus.Pass), + Result("no-superlatives", CheckStatus.Fail, new CheckFinding("Remove 'best in class'.", "/headline", "Best in class")), + ]); + var run = new ChangeSetCheckRun(Guid.NewGuid(), sets.Set.Id, job.Id, 1, CheckRunMode.Inline, Opened, Opened, CheckStatus.Fail, 0m, [item], ["The engine could not read one item."]); + return Task.FromResult(Result.Success(new ChangeSetChecksState(sets.Set, run, job))); + } + + public Task> SubmitAsync(Guid changeSetId, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + + public Task> RunAsync(Guid changeSetId, Guid jobId, CheckRunMode mode, Func>? progress = null, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + + public Task> AbandonAsync(Guid changeSetId, Guid jobId, CheckRunMode mode, CancellationToken cancellationToken = default) => throw new NotSupportedException(); + } + private sealed class FakeHistory(FakeSets sets) : IAuditReader { public Result> Read(AuditQuery query)