From 7678ba19f5a3c75ffdcddf5893d3010865471140 Mon Sep 17 00:00:00 2001 From: Amir Pournasserian Date: Mon, 19 Oct 2026 06:44:53 -0400 Subject: [PATCH 1/5] Keep the entry form's base version and the values a person didn't touch Closes #2630. Closes #2633. --- .../Components/Forms/PickerField.razor | 2 +- .../Components/Pages/EntryEditor.razor | 70 +++++++++-- src/FluentCMS.Admin/Forms/FactValues.cs | 40 +++++- src/FluentCMS.Admin/Forms/FieldValues.cs | 20 ++- src/FluentCMS.Core/Content/EntryEdits.cs | 11 ++ .../Browser/AccessibilityTests.cs | 35 +++++- .../Forms/EntityFormScreenTests.cs | 99 ++++++++++++++- .../Forms/EntryFormScreenTests.cs | 118 ++++++++++++++++++ .../FluentCMS.Tests/Admin/FactValuesTests.cs | 36 ++++++ .../Admin/GeneratedFormTests.cs | 39 +++++- .../FluentCMS.Tests/Admin/PickerFieldTests.cs | 1 + 11 files changed, 449 insertions(+), 22 deletions(-) diff --git a/src/FluentCMS.Admin/Components/Forms/PickerField.razor b/src/FluentCMS.Admin/Components/Forms/PickerField.razor index 1acd12c78..333be49fc 100644 --- a/src/FluentCMS.Admin/Components/Forms/PickerField.razor +++ b/src/FluentCMS.Admin/Components/Forms/PickerField.razor @@ -41,7 +41,7 @@ @oninput="SearchAsync" @onkeydown="OnKeyDown" placeholder="@(Kind == MediaKind ? L["Search by file name or folder"] : L["Search by name"])" - aria-controls="@ListId" + aria-controls="@(_hits.Count > 0 ? ListId : null)" aria-autocomplete="list" data-ui="rove-input" data-field="@Field" /> diff --git a/src/FluentCMS.Admin/Components/Pages/EntryEditor.razor b/src/FluentCMS.Admin/Components/Pages/EntryEditor.razor index 80c089855..d483b1995 100644 --- a/src/FluentCMS.Admin/Components/Pages/EntryEditor.razor +++ b/src/FluentCMS.Admin/Components/Pages/EntryEditor.razor @@ -34,9 +34,26 @@ {

@L["An entity type: every field is a fact with its source, owner and validity."]

} + @if (_entry is { PublishedVersion: { } published } entry && entry.LatestVersion != published && entry.State != EntryState.InReview) + { +

+ @if (FromPublished) + { + @L["The form shows the published version {0}. The latest draft, version {1}, is not published; saving proposes this version's values with your changes.", published, entry.LatestVersion] + @L["Show the latest draft"] + } + else + { + @L["The form shows the latest draft, version {0}, which is not published; a rejected or reverted change can leave such a draft. Version {1} is published.", entry.LatestVersion, published] + @L["Start from the published version"] + } +

+ }
- + @* The version the shown values were made from: after a refused save it is still the one the person started from, so a change + someone saved meanwhile is refused as a conflict rather than overwritten. *@ + @if (_formError is not null) { @@ -84,6 +101,7 @@ private Dictionary> _factProblems = new(StringComparer.Ordinal); private HashSet _unsourced = new(StringComparer.Ordinal); private IReadOnlyList _owners = []; + private HashSet _ownerIds = []; private string? _rationaleError; private string? _formError; @@ -94,6 +112,15 @@ [SupplyParameterFromForm(FormName = "entry")] private EntryFormModel? Form { get; set; } + // Start from the published version instead of a latest draft no set is reviewing, such as a rejected or reverted one. + [SupplyParameterFromQuery(Name = "from")] + private string? From { get; set; } + + private bool FromPublished => From == "published" && _entry?.PublishedVersion is not null; + + // The stored values the form was filled from, by field, so a value the person left as it was is saved exactly as stored. + private Dictionary _stored = new(StringComparer.Ordinal); + // What the form shows: what was posted after a refused save, the stored values otherwise. private EntryFormModel Shown { get; set; } = new(); @@ -131,11 +158,25 @@ _entry = entry.Value; } + var shownFields = _entry?.Latest.Fields; + if (FromPublished) + { + var published = await Entries.GetVersionAsync(_entry!.Id, _entry.PublishedVersion!.Value); + shownFields = published.IsSuccess ? published.Value.Fields : shownFields; + } + + // An update is always made from the latest version, so the version check holds whichever version the values come from. Shown = new EntryFormModel { BaseVersion = _entry?.LatestVersion ?? 0 }; _unsourced.Clear(); + _stored.Clear(); foreach (var field in _type.Definition.Fields) { - JsonElement? stored = _entry is not null && _entry.Latest.Fields.TryGetProperty(field.Name, out var value) ? value : null; + JsonElement? stored = shownFields is { } document && document.TryGetProperty(field.Name, out var value) ? value.Clone() : null; + if (stored is { } kept) + { + _stored[field.Name] = kept; + } + if (_type.Definition.IsEntity) { if (FactValues.IsUnsourced(stored)) @@ -157,7 +198,10 @@ { if (_type!.Definition.IsEntity) { - _owners = [.. (await Users.ListAsync()).Where(user => !user.IsDisabled).Select(user => new FieldOption(user.Id.ToString(), user.DisplayName))]; + // Every user, so a fact owned by someone since disabled keeps its owner when it is saved again. + var users = await Users.ListAsync(); + _owners = [.. users.Select(user => new FieldOption(user.Id.ToString(), user.IsDisabled ? L["{0} (disabled)", user.DisplayName] : user.DisplayName))]; + _ownerIds = [.. users.Select(user => user.Id)]; } foreach (var field in _type!.Definition.Fields) @@ -186,7 +230,15 @@ var fields = new JsonObject(); foreach (var field in _type.Definition.Fields) { - var value = FieldValues.Read(field, Form.Values.GetValueOrDefault(field.Name), out var problem); + var stored = _stored.TryGetValue(field.Name, out var storedValue) ? storedValue : (JsonElement?)null; + var storedValueOnly = _type.Definition.IsEntity ? FactValues.Show(field.Name, stored, new Dictionary()) : stored; + var value = FieldValues.Read(field, Form.Values.GetValueOrDefault(field.Name), out var problem, storedValueOnly); + if (value is null && field.Type == "boolean" && _type.Definition.IsEntity && FactValues.HasDetails(field.Name, Form.Facts)) + { + // A sourced fact that a box is unchecked is the fact that it is false. + value = JsonValue.Create(false); + } + if (problem is not null) { _fieldErrors[field.Name] = Problem(problem); @@ -195,7 +247,11 @@ if (value is null) { - if (field.Required) + if (_type.Definition.IsEntity && FactValues.HasDetails(field.Name, Form.Facts)) + { + _fieldErrors[field.Name] = L["Give the value too, or clear its fact details."]; + } + else if (field.Required) { _fieldErrors[field.Name] = L["Fill in {0}.", field.Name]; } @@ -218,7 +274,7 @@ } var problems = new Dictionary(); - var fact = FactValues.Read(field.Name, value, Form.Facts, problems); + var fact = FactValues.Read(field.Name, value, Form.Facts, problems, stored, _ownerIds); if (fact is null) { _factProblems[field.Name] = problems; @@ -263,7 +319,7 @@ Shown = Form; foreach (var error in saved.Errors) { - if (FieldOf(error.Message) is { } field && _type.Definition.HasField(field)) + if (FieldOf(error.Message) is { } field && _type.Definition.HasField(field) && !_fieldErrors.ContainsKey(field)) { _fieldErrors[field] = error.Message[(error.Message.IndexOf(':') + 1)..].Trim(); } diff --git a/src/FluentCMS.Admin/Forms/FactValues.cs b/src/FluentCMS.Admin/Forms/FactValues.cs index 2799cf595..3727d37eb 100644 --- a/src/FluentCMS.Admin/Forms/FactValues.cs +++ b/src/FluentCMS.Admin/Forms/FactValues.cs @@ -18,8 +18,19 @@ public static class FactValues public static string Key(string field, string part) => $"{field}.{part}"; // The fact for a value, or null with the problems of each part by part name. - public static JsonObject? Read(string field, JsonNode value, IReadOnlyDictionary posted, Dictionary problems) + // A time that shows the same as the stored fact's keeps its stored text, so its seconds are not lost to the widget. + // An owner must be one of the known users when they are given. + public static JsonObject? Read( + string field, + JsonNode value, + IReadOnlyDictionary posted, + Dictionary problems, + JsonElement? stored = null, + IReadOnlySet? owners = null) { + var shown = new Dictionary(); + Show(field, stored, shown); + string? Part(string part) => posted.GetValueOrDefault(Key(field, part)) is { } text && text.Trim().Length > 0 ? text.Trim() : null; var kind = Part(Kind); @@ -33,12 +44,16 @@ public static class FactValues problems[kind is null ? Kind : Reference] = "source"; } - Guid? owner = null; + string? owner = null; if (Part(Owner) is { } ownerText) { - if (Guid.TryParse(ownerText, out var ownerId)) + if (Guid.TryParse(ownerText, out var ownerId) && (owners is null || owners.Contains(ownerId))) { - owner = ownerId; + // The stored text is kept when it names the same user, whatever its case. + owner = stored is { } fact && fact.TryGetProperty(FactMetadata.Owner, out var storedOwner) && storedOwner.ValueKind == JsonValueKind.String + && Guid.TryParse(storedOwner.GetString(), out var storedId) && storedId == ownerId + ? storedOwner.GetString() + : ownerId.ToString(); } else { @@ -55,6 +70,12 @@ public static class FactValues continue; } + if (text == shown.GetValueOrDefault(Key(field, name)) && stored is { } fact && fact.GetProperty(name).GetString() is { } kept) + { + times[name] = kept; + continue; + } + if (DateTime.TryParseExact(text, [FieldValues.DateTimeFormat, FieldValues.DateTimeFormat + ":ss"], CultureInfo.InvariantCulture, DateTimeStyles.AssumeUniversal | DateTimeStyles.AdjustToUniversal, out var time)) { times[name] = time.ToString("yyyy-MM-ddTHH:mm:ssZ", CultureInfo.InvariantCulture); @@ -74,7 +95,7 @@ public static class FactValues { [FactMetadata.Value] = value.DeepClone(), [FactMetadata.Source] = kind is null ? null : new JsonObject { [FactMetadata.SourceKind] = kind, [FactMetadata.SourceReference] = reference }, - [FactMetadata.Owner] = owner?.ToString(), + [FactMetadata.Owner] = owner, [FactMetadata.ValidFrom] = times[FactMetadata.ValidFrom], [FactMetadata.ValidUntil] = times[FactMetadata.ValidUntil], [FactMetadata.LastVerified] = times[FactMetadata.LastVerified], @@ -95,9 +116,10 @@ public static class FactValues facts[Key(field, Reference)] = source.GetProperty(FactMetadata.SourceReference).GetString(); } + // Compared as a GUID, so an id stored in another case still selects its user. if (fact.TryGetProperty(FactMetadata.Owner, out var owner) && owner.ValueKind == JsonValueKind.String) { - facts[Key(field, Owner)] = owner.GetString(); + facts[Key(field, Owner)] = Guid.TryParse(owner.GetString(), out var ownerId) ? ownerId.ToString() : owner.GetString(); } foreach (var name in Times) @@ -112,6 +134,12 @@ public static class FactValues return fact.TryGetProperty(FactMetadata.Value, out var value) ? value : null; } + // Whether any part of the field's fact was given, such as a source typed for a value left empty. + public static bool HasDetails(string field, IReadOnlyDictionary posted) + { + return new[] { Kind, Reference, Owner }.Concat(Times).Any(part => !string.IsNullOrWhiteSpace(posted.GetValueOrDefault(Key(field, part)))); + } + public static bool IsUnsourced(JsonElement? stored) { return stored is { ValueKind: JsonValueKind.Object } fact diff --git a/src/FluentCMS.Admin/Forms/FieldValues.cs b/src/FluentCMS.Admin/Forms/FieldValues.cs index 2855ffb85..6dc92054b 100644 --- a/src/FluentCMS.Admin/Forms/FieldValues.cs +++ b/src/FluentCMS.Admin/Forms/FieldValues.cs @@ -36,15 +36,29 @@ public static IReadOnlyList Strings(FieldDefinition field, string name) : []; } - // Null when the field was left empty. A problem is set when the text cannot be the field's value at all. - public static JsonNode? Read(FieldDefinition field, string? posted, out string? problem) + // Null when the field was left empty. A problem is set when the text cannot be the field's value at all. A text that shows the + // same as the stored value keeps the stored value, so a field the person did not touch is not rewritten by the widget's precision, + // such as the seconds a date-time input drops. + public static JsonNode? Read(FieldDefinition field, string? posted, out string? problem, JsonElement? stored = null) { problem = null; + + // Browsers send a textarea's line breaks as CRLF; stored text keeps LF. + posted = posted?.ReplaceLineEndings("\n"); + if (stored is { ValueKind: not JsonValueKind.Null } kept && posted is not null && posted == Show(field, kept)?.ReplaceLineEndings("\n")) + { + return JsonNode.Parse(kept.GetRawText()); + } + var text = posted?.Trim() ?? string.Empty; + // An unchecked box posts nothing: it is false for a stored value or a required field, and leaves an optional field that had + // no value empty. if (field.Type == "boolean") { - return JsonValue.Create(text == "true"); + return text == "true" ? JsonValue.Create(true) + : field.Required || stored is { ValueKind: JsonValueKind.True or JsonValueKind.False } ? JsonValue.Create(false) + : null; } if (text.Length == 0) diff --git a/src/FluentCMS.Core/Content/EntryEdits.cs b/src/FluentCMS.Core/Content/EntryEdits.cs index e241e2a90..b8d804286 100644 --- a/src/FluentCMS.Core/Content/EntryEdits.cs +++ b/src/FluentCMS.Core/Content/EntryEdits.cs @@ -22,6 +22,17 @@ public async Task> SaveAsync(EntryEdit edit, Cancellation return Result.Failure(ContentErrorCodes.VersionConflict, "An update names the version it was made from."); } + // The set's own expected failures are found before anything is written, as the directories find theirs. + if (string.IsNullOrWhiteSpace(edit.ChangeSet.Rationale)) + { + return Result.Failure(ChangeSetErrorCodes.RationaleMissing, "A change set needs a rationale that says why the changes are made."); + } + + if (edit.ChangeSet.Sources.Any(string.IsNullOrWhiteSpace)) + { + return Result.Failure(ChangeSetErrorCodes.SourceBlank, "A source is blank. A source names where a change came from, such as a URL, a file, an export or a person."); + } + await using var scope = await unitOfWork.BeginAsync(cancellationToken); var entry = edit.EntryId is { } id diff --git a/tests/FluentCMS.IntegrationTests/Browser/AccessibilityTests.cs b/tests/FluentCMS.IntegrationTests/Browser/AccessibilityTests.cs index 0d3f11b9c..87597a0b3 100644 --- a/tests/FluentCMS.IntegrationTests/Browser/AccessibilityTests.cs +++ b/tests/FluentCMS.IntegrationTests/Browser/AccessibilityTests.cs @@ -27,7 +27,7 @@ public async Task The_showcase_desk_and_set_view_have_no_wcag_21_aa_violations() AssetMetadata.Create("drill.jpg", "products", ["hero"], null, null).Value))).Value.Id); await session.SignInWithKeyboardAsync(); - string[] paths = ["/ui", "/", "/?queue=stale-facts", $"/sets/{set.Id}?queue=awaiting-me", "/security", "/media", $"/media/{asset}", "/content", "/content/product", "/content/product/new", "/content/supplier/new"]; + string[] paths = ["/ui", "/", "/?queue=stale-facts", $"/sets/{set.Id}?queue=awaiting-me", "/security", "/media", $"/media/{asset}", "/content", "/content/product", "/content/product/new", "/content/supplier/new", "/content/kitchen-sink/new"]; var violations = new List(); foreach (var theme in new[] { "light", "dark" }) { @@ -47,15 +47,48 @@ public async Task The_showcase_desk_and_set_view_have_no_wcag_21_aa_violations() } } + // A refused submit, with its problems shown next to their fields. + await session.Page.GotoAsync($"{session.BaseUrl}/content/kitchen-sink/new"); + await session.Page.Locator("input[name='Form.Values[title]']").FillAsync("Drill"); + await session.Page.Locator("textarea[name='Form.Values[extra]']").FillAsync("{"); + await session.Page.Locator("textarea[name='Form.Rationale']").FillAsync("Add the drill."); + await session.Page.Locator("[data-action=save]").ClickAsync(); + await Assertions.Expect(session.Page.Locator("[data-slot=form-message]")).ToHaveCountAsync(1); + var refused = await session.Page.RunAxe(Wcag21AA); + violations.AddRange(refused.Violations.Select(violation => $"refused form: {violation.Id} ({violation.Impact}) {violation.Help}: " + + string.Join(" | ", violation.Nodes.Take(3).Select(node => node.Target + " " + node.Html)))); + Assert.True(violations.Count == 0, string.Join(Environment.NewLine, violations)); } + private static System.Text.Json.JsonElement Json(string json) + { + return System.Text.Json.JsonDocument.Parse(json).RootElement; + } + private static async Task AwaitingReviewAsync(BrowserSession session) { var type = await session.Host.AsUserAsync(session.OwnerId, async directory => (await directory.CreateAsync(new NewContentType("Product", "product", new ContentTypeDefinition([new FieldDefinition("title", "text")])))).Value); await session.Host.AsUserAsync(session.OwnerId, async directory => (await directory.CreateAsync(new NewContentType("Supplier", "supplier", new ContentTypeDefinition([new FieldDefinition("name", "text")], isEntity: true)))).Value); + await session.Host.AsUserAsync(session.OwnerId, async directory => (await directory.CreateAsync(new NewContentType( + "Kitchen sink", + "kitchen-sink", + new ContentTypeDefinition( + [ + new FieldDefinition("title", "text", required: true), + new FieldDefinition("body", "richText"), + new FieldDefinition("price", "number"), + new FieldDefinition("active", "boolean"), + new FieldDefinition("launch", "dateTime"), + new FieldDefinition("day", "dateTime", settings: Json("""{"dateOnly": true}""")), + new FieldDefinition("colour", "enum", settings: Json("""{"values": ["red", "blue"]}""")), + new FieldDefinition("product", "reference", settings: Json("""{"target": "product"}""")), + new FieldDefinition("hero", "media"), + new FieldDefinition("tags", "list", settings: Json("""{"itemType": "text"}""")), + new FieldDefinition("extra", "json"), + ])))).Value); return await session.Host.AsUserAsync(session.OwnerId, async directory => { diff --git a/tests/FluentCMS.IntegrationTests/Forms/EntityFormScreenTests.cs b/tests/FluentCMS.IntegrationTests/Forms/EntityFormScreenTests.cs index 0edeb35d6..de7e23cac 100644 --- a/tests/FluentCMS.IntegrationTests/Forms/EntityFormScreenTests.cs +++ b/tests/FluentCMS.IntegrationTests/Forms/EntityFormScreenTests.cs @@ -3,6 +3,7 @@ using FluentCMS.Core.ChangeSets; using FluentCMS.Core.Content; using FluentCMS.Core.Entities; +using FluentCMS.Core.Identity; using FluentCMS.Core.Schema; using Microsoft.Extensions.DependencyInjection; @@ -73,23 +74,115 @@ public async Task A_half_given_source_and_a_bad_time_are_reported_and_nothing_is ["Form.Values[price]"] = "10", ["Form.Facts[name.kind]"] = "url", ["Form.Facts[price.validUntil]"] = "next year", + ["Form.Facts[price.owner]"] = Guid.NewGuid().ToString(), ["Form.Rationale"] = "Add the drill.", ["Form.BaseVersion"] = "0", }); var page = await refused.Content.ReadAsStringAsync(); Assert.Equal(HttpStatusCode.OK, refused.StatusCode); - Assert.Contains("Give both the kind of source and the source, or neither.", page); - Assert.Contains("Enter a date and a time.", page); + Assert.Contains("Give both the kind of source and the source, or neither.", Fieldset(page, "name")); + Assert.Contains("Enter a date and a time.", Fieldset(page, "price")); + Assert.Contains("Choose an owner from the list.", Fieldset(page, "price")); Assert.Contains("value=\"next year\"", page); Assert.Empty(await host.AsUserAsync>(ownerId, async entries => (await entries.ListAsync("product")).Value)); } + [Fact] + public async Task Fact_details_for_an_empty_value_are_reported_next_to_the_field() + { + using var install = new TemporaryInstall(); + await using var host = await AdminHost.StartAsync(install); + var ownerId = await host.CreateFirstAdministratorAsync(); + await ProductTypeAsync(host, ownerId); + await host.SignInAsync(AdminHost.Owner); + + var refused = await host.SubmitAsync("/content/product/new", "entry", new Dictionary + { + ["Form.Values[name]"] = "Drill", + ["Form.Facts[price.kind]"] = "file", + ["Form.Facts[price.reference]"] = "prices.csv", + ["Form.Rationale"] = "Add the drill.", + ["Form.BaseVersion"] = "0", + }); + + Assert.Contains("Give the value too, or clear its fact details.", await refused.Content.ReadAsStringAsync()); + Assert.Empty(await host.AsUserAsync>(ownerId, async entries => (await entries.ListAsync("product")).Value)); + } + + [Fact] + public async Task A_name_only_edit_changes_only_the_name_fact_and_keeps_every_other_fact_as_stored() + { + using var install = new TemporaryInstall(); + await using var host = await AdminHost.StartAsync(install); + var ownerId = await host.CreateFirstAdministratorAsync(); + await ProductTypeAsync(host, ownerId); + var leaverId = await host.AsUserAsync(ownerId, async users => + { + var id = (await users.CreateAsync(new NewUser("leaver@example.com", "Leaver", "An0ther!Passw0rd"))).Value; + await users.DisableAsync(id); + return id; + }); + var owner = leaverId.ToString().ToUpperInvariant(); + var entity = await host.AsUserAsync(ownerId, async entries => (await entries.CreateAsync(new NewEntry("product", Json($$""" + { + "name": {"value": "Drill", "source": null, "owner": null, "validFrom": null, "validUntil": null, "lastVerified": null}, + "price": {"value": 4.50, "source": {"kind": "file", "reference": "prices.csv"}, "owner": "{{owner}}", "validFrom": null, "validUntil": null, "lastVerified": "2026-10-18T09:00:45.5+02:00"} + } + """)))).Value); + await host.SignInAsync(AdminHost.Owner); + + // The disabled owner stays selected, and the time shows to the minute in UTC. + var edit = await host.Client.GetStringAsync($"/content/product/{entity.Id}"); + Assert.Contains($"", edit); + Assert.Contains("value=\"2026-10-18T07:00\"", Fieldset(edit, "price")); + Assert.Contains("data-testid=\"fact-unsourced\"", Fieldset(edit, "name")); + Assert.DoesNotContain("data-testid=\"fact-unsourced\"", Fieldset(edit, "price")); + + // What the browser posts back: the widgets' text for every untouched part, and nothing for the unchecked box. + var saved = await host.SubmitAsync($"/content/product/{entity.Id}", "entry", new Dictionary + { + ["Form.Values[name]"] = "Cordless drill", + ["Form.Values[price]"] = "4.50", + ["Form.Facts[price.kind]"] = "file", + ["Form.Facts[price.reference]"] = "prices.csv", + ["Form.Facts[price.owner]"] = leaverId.ToString(), + ["Form.Facts[price.lastVerified]"] = "2026-10-18T07:00", + ["Form.Rationale"] = "Rename.", + ["Form.BaseVersion"] = "1", + }); + + Assert.StartsWith("/sets/", host.LocationOf(saved)); + var fields = (await host.AsUserAsync(ownerId, async entries => (await entries.GetAsync(entity.Id)).Value)).Latest.Fields; + Assert.Equal("Cordless drill", fields.GetProperty("name").GetProperty("value").GetString()); + var price = fields.GetProperty("price"); + Assert.Equal("4.50", price.GetProperty("value").GetRawText()); + Assert.Equal(owner, price.GetProperty("owner").GetString()); + Assert.Equal(DateTimeOffset.Parse("2026-10-18T09:00:45.5+02:00", System.Globalization.CultureInfo.InvariantCulture), price.GetProperty("lastVerified").GetDateTimeOffset()); + Assert.Equal("prices.csv", price.GetProperty("source").GetProperty("reference").GetString()); + Assert.False(fields.TryGetProperty("inStock", out _)); + } + + private static JsonElement Json(string json) + { + return JsonSerializer.Deserialize(json); + } + + // The fact details fieldset of one field, where its problems and its unsourced marker are shown. + private static string Fieldset(string page, string field) + { + var start = page.IndexOf($"data-fact=\"{field}\"", StringComparison.Ordinal); + Assert.True(start >= 0, $"No fact details for {field}."); + return page[start..page.IndexOf("", start, StringComparison.Ordinal)]; + } + private static Task ProductTypeAsync(AdminHost host, Guid ownerId) { return host.AsUserAsync(ownerId, async types => (await types.CreateAsync(new NewContentType( "Product", "product", - new ContentTypeDefinition([new FieldDefinition("name", "text", required: true), new FieldDefinition("price", "number")], isEntity: true)))).Value); + new ContentTypeDefinition( + [new FieldDefinition("name", "text", required: true), new FieldDefinition("price", "number"), new FieldDefinition("inStock", "boolean")], + isEntity: true)))).Value); } } diff --git a/tests/FluentCMS.IntegrationTests/Forms/EntryFormScreenTests.cs b/tests/FluentCMS.IntegrationTests/Forms/EntryFormScreenTests.cs index 1768adb8a..04b5783ff 100644 --- a/tests/FluentCMS.IntegrationTests/Forms/EntryFormScreenTests.cs +++ b/tests/FluentCMS.IntegrationTests/Forms/EntryFormScreenTests.cs @@ -103,6 +103,7 @@ public async Task Problems_show_next_to_their_fields_and_nothing_is_stored() Assert.Contains("Enter a number", page); Assert.Contains("Enter a date and a time.", page); Assert.Contains("Enter valid JSON", page); + Assert.Contains("The value must be one of: red, blue.", page); Assert.Contains("Say why you are making this change.", page); Assert.Contains("value=\"cheap\"", page); Assert.True(System.Text.RegularExpressions.Regex.Count(page, "aria-invalid=\"true\"") >= 5); @@ -135,6 +136,123 @@ await host.AsUserAsync(ownerId, async entries => Assert.Contains("data-version=\"2\"", page); } + // A refused save comes back with the version the person started from, so their next save cannot overwrite a change made meanwhile. + [Fact] + public async Task A_form_refused_for_its_own_problems_keeps_its_base_version_and_its_next_save_is_a_conflict() + { + using var install = new TemporaryInstall(); + await using var host = await AdminHost.StartAsync(install); + var ownerId = await host.CreateFirstAdministratorAsync(); + await KitchenSinkAsync(host, ownerId); + var entry = await host.AsUserAsync(ownerId, async entries => + (await entries.CreateAsync(new NewEntry("kitchen-sink", Json("""{"title": "First"}""")))).Value); + await host.SignInAsync(AdminHost.Owner); + var form = await host.Client.GetStringAsync($"/content/kitchen-sink/{entry.Id}"); + Assert.Contains("name=\"Form.BaseVersion\" value=\"1\"", form); + await host.AsUserAsync(ownerId, async entries => + (await entries.UpdateAsync(entry.Id, 1, Json("""{"title": "Theirs"}"""))).Value); + + var refused = await host.SubmitAsync($"/content/kitchen-sink/{entry.Id}", "entry", new Dictionary + { + ["Form.Values[title]"] = "Mine", + ["Form.Values[extra]"] = "{", + ["Form.Rationale"] = "Retitle.", + ["Form.BaseVersion"] = "1", + }); + Assert.Contains("name=\"Form.BaseVersion\" value=\"1\"", await refused.Content.ReadAsStringAsync()); + + var again = await host.SubmitAsync($"/content/kitchen-sink/{entry.Id}", "entry", new Dictionary + { + ["Form.Values[title]"] = "Mine", + ["Form.Rationale"] = "Retitle.", + ["Form.BaseVersion"] = "1", + }); + + Assert.Contains("data-testid=\"entry-form-error\"", await again.Content.ReadAsStringAsync()); + var stored = await host.AsUserAsync(ownerId, async entries => (await entries.GetAsync(entry.Id)).Value); + Assert.Equal((2, "Theirs"), (stored.LatestVersion, stored.Latest.Fields.GetProperty("title").GetString())); + } + + [Fact] + public async Task An_edit_of_one_field_leaves_every_other_stored_value_exactly_as_it_was() + { + using var install = new TemporaryInstall(); + await using var host = await AdminHost.StartAsync(install); + var ownerId = await host.CreateFirstAdministratorAsync(); + await KitchenSinkAsync(host, ownerId); + var entry = await host.AsUserAsync(ownerId, async entries => (await entries.CreateAsync(new NewEntry( + "kitchen-sink", + Json("""{"title": "Drill", "body": "First line.\nSecond line.", "launch": "2026-11-01T09:30:45.5+02:00", "price": 4.50}""")))).Value); + await host.SignInAsync(AdminHost.Owner); + + // What a browser posts back for the untouched fields: the widgets' text, with CRLF line breaks and minutes only. + var saved = await host.SubmitAsync($"/content/kitchen-sink/{entry.Id}", "entry", new Dictionary + { + ["Form.Values[title]"] = "Cordless drill", + ["Form.Values[body]"] = "First line.\r\nSecond line.", + ["Form.Values[launch]"] = "2026-11-01T07:30", + ["Form.Values[price]"] = "4.50", + ["Form.Rationale"] = "Retitle.", + ["Form.BaseVersion"] = "1", + }); + + var set = await host.AsUserAsync(ownerId, async sets => (await sets.GetAsync(Guid.Parse(host.LocationOf(saved)![6..42]))).Value); + var fields = (await host.AsUserAsync(ownerId, async entries => (await entries.GetAsync(entry.Id)).Value)).Latest.Fields; + Assert.Equal("Cordless drill", fields.GetProperty("title").GetString()); + Assert.Equal("First line.\nSecond line.", fields.GetProperty("body").GetString()); + Assert.Equal(DateTimeOffset.Parse("2026-11-01T09:30:45.5+02:00", System.Globalization.CultureInfo.InvariantCulture), fields.GetProperty("launch").GetDateTimeOffset()); + Assert.Equal("4.50", fields.GetProperty("price").GetRawText()); + Assert.False(fields.TryGetProperty("active", out _)); + Assert.Equal(2, set.Operations[0].ProposedVersion); + } + + [Fact] + public async Task Editing_a_published_entry_proposes_an_update_from_the_published_version_and_a_stray_draft_is_named() + { + using var install = new TemporaryInstall(); + await using var host = await AdminHost.StartAsync(install); + var ownerId = await host.CreateFirstAdministratorAsync(); + await KitchenSinkAsync(host, ownerId); + var entry = await host.AsUserAsync(ownerId, async entries => + (await entries.CreateAsync(new NewEntry("kitchen-sink", Json("""{"title": "Published"}""")))).Value); + await host.AsUserAsync(ownerId, async sets => + { + var set = (await sets.OpenAsync(new NewChangeSet("Publish it.", []))).Value; + await sets.AddOperationAsync(set.Id, ChangeOperation.Create(OperationKind.Create, ItemKind.Entry, entry.Id, null, 1).Value); + await sets.TransitionAsync(set.Id, ChangeSetState.ChecksRunning); + await sets.TransitionAsync(set.Id, ChangeSetState.Approved); + return (await sets.PublishAsync(set.Id)).Value; + }); + + // A draft no set reviews, as a rejected change leaves one. + await host.AsUserAsync(ownerId, async entries => (await entries.UpdateAsync(entry.Id, 1, Json("""{"title": "Stray draft"}"""))).Value); + await host.SignInAsync(AdminHost.Owner); + + var latest = await host.Client.GetStringAsync($"/content/kitchen-sink/{entry.Id}"); + Assert.Contains("data-testid=\"entry-draft-note\"", latest); + Assert.Contains("value=\"Stray draft\"", latest); + var published = await host.Client.GetStringAsync($"/content/kitchen-sink/{entry.Id}?from=published"); + Assert.Contains("value=\"Published\"", published); + Assert.Contains("name=\"Form.BaseVersion\" value=\"2\"", published); + + var saved = await host.SubmitAsync($"/content/kitchen-sink/{entry.Id}?from=published", "entry", new Dictionary + { + ["Form.Values[title]"] = "Published, retitled", + ["Form.Rationale"] = "Retitle.", + ["Form.BaseVersion"] = "2", + }); + + var set = await host.AsUserAsync(ownerId, async sets => (await sets.GetAsync(Guid.Parse(host.LocationOf(saved)![6..42]))).Value); + var operation = Assert.Single(set.Operations); + Assert.Equal((OperationKind.Update, (int?)1, (int?)3), (operation.Kind, operation.BaseVersion, operation.ProposedVersion)); + + // The saved set goes through the checks from the set view like any other. + var page = await host.Client.GetStringAsync($"/sets/{set.Id}"); + var seen = System.Text.RegularExpressions.Regex.Match(page, "name=\"Submit.Seen\" value=\"([0-9a-f]+)\"").Groups[1].Value; + var submitted = await host.SubmitAsync($"/sets/{set.Id}", "submit", new Dictionary { ["Submit.Seen"] = seen }); + Assert.Contains("data-state=\"awaiting-review\"", await submitted.Content.ReadAsStringAsync()); + } + [Fact] public async Task The_content_pages_list_types_and_entries_and_need_a_signed_in_person() { diff --git a/tests/FluentCMS.Tests/Admin/FactValuesTests.cs b/tests/FluentCMS.Tests/Admin/FactValuesTests.cs index 98e25955f..7704f8d0b 100644 --- a/tests/FluentCMS.Tests/Admin/FactValuesTests.cs +++ b/tests/FluentCMS.Tests/Admin/FactValuesTests.cs @@ -1,6 +1,7 @@ using System.Text.Json; using System.Text.Json.Nodes; using FluentCMS.Admin.Forms; +using FluentCMS.Core.Schema; namespace FluentCMS.Tests.Admin; @@ -61,6 +62,41 @@ public void A_stored_fact_shows_its_value_and_parts() Assert.False(FactValues.IsUnsourced(stored)); } + [Fact] + public void Parts_that_show_as_the_stored_fact_keep_its_text() + { + var stored = JsonDocument.Parse($$"""{"value": 10, "source": null, "owner": "{{Owner.ToString().ToUpperInvariant()}}", "validFrom": null, "validUntil": null, "lastVerified": "2026-03-04T05:06:07.89+01:00"}""").RootElement; + var shown = new Dictionary(); + FactValues.Show("price", stored, shown); + var problems = new Dictionary(); + + var fact = FactValues.Read("price", JsonValue.Create(10), shown, problems, stored, new HashSet { Owner }); + + Assert.Empty(problems); + Assert.Equal(Owner.ToString(), shown["price.owner"]); + Assert.Equal(Owner.ToString().ToUpperInvariant(), fact![FactMetadata.Owner]!.GetValue()); + Assert.Equal("2026-03-04T05:06:07.89+01:00", fact[FactMetadata.LastVerified]!.GetValue()); + } + + [Fact] + public void An_owner_who_is_not_a_known_user_is_a_problem() + { + var problems = new Dictionary(); + + Assert.Null(FactValues.Read("price", JsonValue.Create(10m), Posted(("price.owner", Guid.NewGuid().ToString())), problems, owners: new HashSet { Owner })); + Assert.Equal("owner", problems["owner"]); + } + + [Theory] + [InlineData("price.reference", "prices.csv", true)] + [InlineData("price.validFrom", "2026-01-01T00:00", true)] + [InlineData("other.kind", "url", false)] + [InlineData("price.owner", " ", false)] + public void Details_are_given_when_any_part_of_the_field_has_text(string key, string posted, bool expected) + { + Assert.Equal(expected, FactValues.HasDetails("price", Posted((key, posted)))); + } + private static Dictionary Posted(params (string Key, string Value)[] parts) { return parts.ToDictionary(part => part.Key, part => (string?)part.Value); diff --git a/tests/FluentCMS.Tests/Admin/GeneratedFormTests.cs b/tests/FluentCMS.Tests/Admin/GeneratedFormTests.cs index a6fc7da7c..4d58c6270 100644 --- a/tests/FluentCMS.Tests/Admin/GeneratedFormTests.cs +++ b/tests/FluentCMS.Tests/Admin/GeneratedFormTests.cs @@ -14,7 +14,6 @@ public sealed class GeneratedFormTests [InlineData("number", "{}", "129.5", "129.5")] [InlineData("number", "{}", "-3", "-3")] [InlineData("boolean", "{}", "true", "true")] - [InlineData("boolean", "{}", null, "false")] [InlineData("dateTime", "{}", "2026-11-01T09:30", "\"2026-11-01T09:30:00Z\"")] [InlineData("dateTime", """{"dateOnly": true}""", "2026-11-02", "\"2026-11-02\"")] [InlineData("enum", """{"values": ["red"]}""", "red", "\"red\"")] @@ -50,6 +49,42 @@ public void An_empty_field_has_no_value(string type) Assert.Null(problem); } + [Theory] + [InlineData("dateTime", "{}", "\"2026-11-01T09:30:45.5+02:00\"", "2026-11-01T07:30")] + [InlineData("number", "{}", "4.50", "4.50")] + [InlineData("text", "{}", "\"one\\ntwo\"", "one\r\ntwo")] + [InlineData("json", "{}", """{"k":[1,2]}""", "{\r\n \"k\": [\r\n 1,\r\n 2\r\n ]\r\n}")] + [InlineData("boolean", "{}", "true", "true")] + public void A_posted_text_that_shows_as_the_stored_value_keeps_it_exactly(string type, string settings, string stored, string posted) + { + var value = FieldValues.Read(Field(type, settings), posted, out var problem, JsonDocument.Parse(stored).RootElement); + + Assert.Null(problem); + Assert.Equal(stored, value!.ToJsonString(Unescaped)); + } + + [Fact] + public void A_changed_text_replaces_the_stored_value() + { + var value = FieldValues.Read(Field("dateTime", "{}"), "2026-11-01T07:31", out _, JsonDocument.Parse("\"2026-11-01T09:30:45+02:00\"").RootElement); + + Assert.Equal("\"2026-11-01T07:31:00Z\"", value!.ToJsonString()); + } + + [Theory] + [InlineData(null, false, null)] + [InlineData("true", false, "false")] + [InlineData(null, true, "false")] + public void An_unchecked_box_is_false_only_when_the_field_is_required_or_had_a_value(string? stored, bool required, string? expected) + { + var storedValue = stored is null ? (JsonElement?)null : JsonDocument.Parse(stored).RootElement; + + var value = FieldValues.Read(Field("boolean", "{}", required), null, out var problem, storedValue); + + Assert.Null(problem); + Assert.Equal(expected, value?.ToJsonString()); + } + [Theory] [InlineData("dateTime", "{}", "\"2026-11-01T11:30:00+02:00\"", "2026-11-01T09:30")] [InlineData("boolean", "{}", "true", "true")] @@ -103,6 +138,8 @@ public void An_enum_offers_its_values_and_marks_the_stored_one() Assert.True(cut.Find("option[value=blue]").HasAttribute("selected")); } + private static readonly JsonSerializerOptions Unescaped = new() { Encoder = System.Text.Encodings.Web.JavaScriptEncoder.UnsafeRelaxedJsonEscaping }; + private static FieldDefinition Field(string type, string settings, bool required = false) { return new FieldDefinition("field", type, required, JsonDocument.Parse(settings).RootElement); diff --git a/tests/FluentCMS.Tests/Admin/PickerFieldTests.cs b/tests/FluentCMS.Tests/Admin/PickerFieldTests.cs index d9fb82d69..9c42de4b9 100644 --- a/tests/FluentCMS.Tests/Admin/PickerFieldTests.cs +++ b/tests/FluentCMS.Tests/Admin/PickerFieldTests.cs @@ -36,6 +36,7 @@ public void An_entry_picker_searches_the_target_type_and_posts_the_chosen_id() var cut = context.Render(EntryPicker(chosen: false)); Assert.Equal("", cut.Find("input[type=hidden]").GetAttribute("value") ?? ""); + Assert.False(cut.Find("input[type=search]").HasAttribute("aria-controls")); cut.Find("input[type=search]").Input("dri"); Assert.Equal(("dri", "product"), (search.Queries[0].Text, search.Queries[0].ContentTypes!.Single())); From d2e27e0c2d3ada28da156610e93d6d0074cd4a67 Mon Sep 17 00:00:00 2001 From: Amir Pournasserian Date: Mon, 19 Oct 2026 07:52:44 -0400 Subject: [PATCH 2/5] Keep untouched facts and unknown owners, and refuse a form whose published version moved --- .../Components/Pages/EntryEditor.razor | 24 ++++++- src/FluentCMS.Admin/Forms/EntryFormModel.cs | 3 + src/FluentCMS.Admin/Forms/FactValues.cs | 41 +++++++++--- .../Forms/EntityFormScreenTests.cs | 67 ++++++++++++++----- .../Forms/EntryFormScreenTests.cs | 58 +++++++++++++++- .../FluentCMS.Tests/Admin/FactValuesTests.cs | 39 +++++++++++ 6 files changed, 201 insertions(+), 31 deletions(-) diff --git a/src/FluentCMS.Admin/Components/Pages/EntryEditor.razor b/src/FluentCMS.Admin/Components/Pages/EntryEditor.razor index d483b1995..4da51688e 100644 --- a/src/FluentCMS.Admin/Components/Pages/EntryEditor.razor +++ b/src/FluentCMS.Admin/Components/Pages/EntryEditor.razor @@ -54,6 +54,10 @@ @* The version the shown values were made from: after a refused save it is still the one the person started from, so a change someone saved meanwhile is refused as a conflict rather than overwritten. *@ + @if (Shown.FromPublished is { } fromPublished) + { + + } @if (_formError is not null) { @@ -166,7 +170,7 @@ } // An update is always made from the latest version, so the version check holds whichever version the values come from. - Shown = new EntryFormModel { BaseVersion = _entry?.LatestVersion ?? 0 }; + Shown = new EntryFormModel { BaseVersion = _entry?.LatestVersion ?? 0, FromPublished = FromPublished ? _entry!.PublishedVersion : null }; _unsourced.Clear(); _stored.Clear(); foreach (var field in _type.Definition.Fields) @@ -198,10 +202,16 @@ { if (_type!.Definition.IsEntity) { - // Every user, so a fact owned by someone since disabled keeps its owner when it is saved again. + // Every user, so a fact owned by someone since disabled keeps its owner when it is saved again, and any stored owner who is + // not a user, such as one an agent set, so it stays selected rather than being cleared. var users = await Users.ListAsync(); - _owners = [.. users.Select(user => new FieldOption(user.Id.ToString(), user.IsDisabled ? L["{0} (disabled)", user.DisplayName] : user.DisplayName))]; _ownerIds = [.. users.Select(user => user.Id)]; + var unknown = _stored.Values.Select(stored => FactValues.OwnerOf(stored)).OfType().Where(id => !_ownerIds.Contains(id)).Distinct(); + _owners = + [ + .. users.Select(user => new FieldOption(user.Id.ToString(), user.IsDisabled ? L["{0} (disabled)", user.DisplayName] : user.DisplayName)), + .. unknown.Select(id => new FieldOption(id.ToString(), L["Unknown user {0}", id.ToString()[..8]])), + ]; } foreach (var field in _type!.Definition.Fields) @@ -227,6 +237,14 @@ return; } + // The values came from a published version that has since changed, so saving them would undo what was published meanwhile. + if (Form.FromPublished is { } fromPublished && _entry?.PublishedVersion != fromPublished) + { + await LoadAsync(); + _formError = L["Someone saved this entry since the form was shown. Here it is as it is now; make your change again."]; + return; + } + var fields = new JsonObject(); foreach (var field in _type.Definition.Fields) { diff --git a/src/FluentCMS.Admin/Forms/EntryFormModel.cs b/src/FluentCMS.Admin/Forms/EntryFormModel.cs index 87b163a85..32b37905b 100644 --- a/src/FluentCMS.Admin/Forms/EntryFormModel.cs +++ b/src/FluentCMS.Admin/Forms/EntryFormModel.cs @@ -15,6 +15,9 @@ public sealed class EntryFormModel public int BaseVersion { get; set; } + // The published version the values came from, when the form started from it rather than from the latest draft. + public int? FromPublished { get; set; } + public IReadOnlyList SourceList() { return (Sources ?? string.Empty).Split('\n', StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries); diff --git a/src/FluentCMS.Admin/Forms/FactValues.cs b/src/FluentCMS.Admin/Forms/FactValues.cs index 3727d37eb..2d657e3fe 100644 --- a/src/FluentCMS.Admin/Forms/FactValues.cs +++ b/src/FluentCMS.Admin/Forms/FactValues.cs @@ -15,11 +15,21 @@ public static class FactValues public static IReadOnlyList Times { get; } = [FactMetadata.ValidFrom, FactMetadata.ValidUntil, FactMetadata.LastVerified]; + private static IReadOnlyList Parts { get; } = [Kind, Reference, Owner, .. Times]; + public static string Key(string field, string part) => $"{field}.{part}"; + // The owner a stored fact names, so an owner who is no longer a user still shows and is kept. + public static Guid? OwnerOf(JsonElement? stored) + { + return stored is { ValueKind: JsonValueKind.Object } fact && fact.TryGetProperty(FactMetadata.Owner, out var owner) + && owner.ValueKind == JsonValueKind.String && Guid.TryParse(owner.GetString(), out var id) ? id : null; + } + // The fact for a value, or null with the problems of each part by part name. - // A time that shows the same as the stored fact's keeps its stored text, so its seconds are not lost to the widget. - // An owner must be one of the known users when they are given. + // A fact whose value and parts all show as stored is kept exactly as stored. Otherwise a time that shows the same as the stored + // fact's keeps its stored text, so its seconds are not lost to the widget. + // An owner must be one of the known users, or the owner the fact already had. public static JsonObject? Read( string field, JsonNode value, @@ -28,8 +38,19 @@ public static class FactValues JsonElement? stored = null, IReadOnlySet? owners = null) { + var fact = stored is { ValueKind: JsonValueKind.Object } storedFact ? storedFact : (JsonElement?)null; var shown = new Dictionary(); - Show(field, stored, shown); + var storedValue = Show(field, fact, shown); + if (fact is { } untouched && storedValue is { } kept && JsonNode.DeepEquals(value, JsonNode.Parse(kept.GetRawText())) + && Parts.All(part => posted.GetValueOrDefault(Key(field, part)) is var text + && (string.IsNullOrEmpty(text) ? string.IsNullOrEmpty(shown.GetValueOrDefault(Key(field, part))) : text == shown.GetValueOrDefault(Key(field, part))))) + { + return JsonNode.Parse(untouched.GetRawText())!.AsObject(); + } + + var storedOwner = fact is { } withOwner && withOwner.TryGetProperty(FactMetadata.Owner, out var ownerElement) + && ownerElement.ValueKind == JsonValueKind.String ? ownerElement.GetString() : null; + Guid? storedOwnerId = Guid.TryParse(storedOwner, out var parsedOwner) ? parsedOwner : null; string? Part(string part) => posted.GetValueOrDefault(Key(field, part)) is { } text && text.Trim().Length > 0 ? text.Trim() : null; @@ -47,13 +68,10 @@ public static class FactValues string? owner = null; if (Part(Owner) is { } ownerText) { - if (Guid.TryParse(ownerText, out var ownerId) && (owners is null || owners.Contains(ownerId))) + if (Guid.TryParse(ownerText, out var ownerId) && (owners is null || owners.Contains(ownerId) || ownerId == storedOwnerId)) { // The stored text is kept when it names the same user, whatever its case. - owner = stored is { } fact && fact.TryGetProperty(FactMetadata.Owner, out var storedOwner) && storedOwner.ValueKind == JsonValueKind.String - && Guid.TryParse(storedOwner.GetString(), out var storedId) && storedId == ownerId - ? storedOwner.GetString() - : ownerId.ToString(); + owner = ownerId == storedOwnerId ? storedOwner : ownerId.ToString(); } else { @@ -70,9 +88,10 @@ public static class FactValues continue; } - if (text == shown.GetValueOrDefault(Key(field, name)) && stored is { } fact && fact.GetProperty(name).GetString() is { } kept) + if (text == shown.GetValueOrDefault(Key(field, name)) && fact is { } timed && timed.TryGetProperty(name, out var keptTime) + && keptTime.GetString() is { } keptText) { - times[name] = kept; + times[name] = keptText; continue; } @@ -137,7 +156,7 @@ public static class FactValues // Whether any part of the field's fact was given, such as a source typed for a value left empty. public static bool HasDetails(string field, IReadOnlyDictionary posted) { - return new[] { Kind, Reference, Owner }.Concat(Times).Any(part => !string.IsNullOrWhiteSpace(posted.GetValueOrDefault(Key(field, part)))); + return Parts.Any(part => !string.IsNullOrWhiteSpace(posted.GetValueOrDefault(Key(field, part)))); } public static bool IsUnsourced(JsonElement? stored) diff --git a/tests/FluentCMS.IntegrationTests/Forms/EntityFormScreenTests.cs b/tests/FluentCMS.IntegrationTests/Forms/EntityFormScreenTests.cs index a969f1375..cbd117c24 100644 --- a/tests/FluentCMS.IntegrationTests/Forms/EntityFormScreenTests.cs +++ b/tests/FluentCMS.IntegrationTests/Forms/EntityFormScreenTests.cs @@ -123,46 +123,81 @@ public async Task A_name_only_edit_changes_only_the_name_fact_and_keeps_every_ot await users.DisableAsync(id); return id; }); - var owner = leaverId.ToString().ToUpperInvariant(); - var entity = await host.AsUserAsync(ownerId, async entries => (await entries.CreateAsync(new NewEntry("product", Json($$""" + + // As an agent may store them: an owner who is no user, a source with its properties in another order and a trailing space, + // an upper-case owner id, and times with seconds and offsets. + var stranger = Guid.NewGuid(); + var created = await host.AsUserAsync(ownerId, async entries => (await entries.CreateAsync(new NewEntry("product", Json($$""" { - "name": {"value": "Drill", "source": null, "owner": null, "validFrom": null, "validUntil": null, "lastVerified": null}, - "price": {"value": 4.50, "source": {"kind": "file", "reference": "prices.csv"}, "owner": "{{owner}}", "validFrom": null, "validUntil": null, "lastVerified": "2026-10-18T09:00:45.5+02:00"} + "name": {"value": "Drill", "source": null, "owner": "{{stranger}}", "validFrom": null, "validUntil": null, "lastVerified": null}, + "price": {"value": 4.50, "source": {"reference": "prices.csv ", "kind": "file"}, "owner": "{{leaverId.ToString().ToUpperInvariant()}}", "validFrom": "2026-01-01T00:00:00Z", "validUntil": "2026-12-31T23:59:59+01:00", "lastVerified": "2026-10-18T09:00:45.5+02:00"} } """)))).Value); + var before = (await host.AsUserAsync(ownerId, async entries => (await entries.GetAsync(created.Id)).Value)).Latest.Fields; await host.SignInAsync(AdminHost.Owner); - // The disabled owner stays selected, and the time shows to the minute in UTC. - var edit = await host.Client.GetStringAsync($"/content/product/{entity.Id}"); - Assert.Contains($"", edit); + // The disabled owner and the owner who is no user stay selected, and the times show to the minute in UTC. + var edit = await host.Client.GetStringAsync($"/content/product/{created.Id}"); + Assert.Contains($"", Fieldset(edit, "price")); + Assert.Contains($"", Fieldset(edit, "name")); Assert.Contains("value=\"2026-10-18T07:00\"", Fieldset(edit, "price")); + Assert.Contains("value=\"2026-12-31T22:59\"", Fieldset(edit, "price")); Assert.Contains("data-testid=\"fact-unsourced\"", Fieldset(edit, "name")); Assert.DoesNotContain("data-testid=\"fact-unsourced\"", Fieldset(edit, "price")); // What the browser posts back: the widgets' text for every untouched part, and nothing for the unchecked box. - var saved = await host.SubmitAsync($"/content/product/{entity.Id}", "entry", new Dictionary + var saved = await host.SubmitAsync($"/content/product/{created.Id}", "entry", new Dictionary { ["Form.Values[name]"] = "Cordless drill", + ["Form.Facts[name.owner]"] = stranger.ToString(), ["Form.Values[price]"] = "4.50", ["Form.Facts[price.kind]"] = "file", - ["Form.Facts[price.reference]"] = "prices.csv", + ["Form.Facts[price.reference]"] = "prices.csv ", ["Form.Facts[price.owner]"] = leaverId.ToString(), + ["Form.Facts[price.validFrom]"] = "2026-01-01T00:00", + ["Form.Facts[price.validUntil]"] = "2026-12-31T22:59", ["Form.Facts[price.lastVerified]"] = "2026-10-18T07:00", ["Form.Rationale"] = "Rename.", ["Form.BaseVersion"] = "1", }); Assert.StartsWith("/sets/", host.LocationOf(saved)); - var fields = (await host.AsUserAsync(ownerId, async entries => (await entries.GetAsync(entity.Id)).Value)).Latest.Fields; - Assert.Equal("Cordless drill", fields.GetProperty("name").GetProperty("value").GetString()); - var price = fields.GetProperty("price"); - Assert.Equal("4.50", price.GetProperty("value").GetRawText()); - Assert.Equal(owner, price.GetProperty("owner").GetString()); - Assert.Equal(DateTimeOffset.Parse("2026-10-18T09:00:45.5+02:00", System.Globalization.CultureInfo.InvariantCulture), price.GetProperty("lastVerified").GetDateTimeOffset()); - Assert.Equal("prices.csv", price.GetProperty("source").GetProperty("reference").GetString()); + var fields = (await host.AsUserAsync(ownerId, async entries => (await entries.GetAsync(created.Id)).Value)).Latest.Fields; + Assert.Equal(("Cordless drill", stranger.ToString()), (fields.GetProperty("name").GetProperty("value").GetString(), fields.GetProperty("name").GetProperty("owner").GetString())); + Assert.Equal(JsonSerializer.Serialize(before.GetProperty("price")), JsonSerializer.Serialize(fields.GetProperty("price"))); Assert.False(fields.TryGetProperty("inStock", out _)); } + // A type made an entity type after entries were written keeps their plain values until they are edited. + [Fact] + public async Task An_edit_of_an_entry_written_before_its_type_held_facts_saves_its_facts() + { + using var install = new TemporaryInstall(); + await using var host = await AdminHost.StartAsync(install); + var ownerId = await host.CreateFirstAdministratorAsync(); + IReadOnlyList fields = [new FieldDefinition("name", "text", required: true), new FieldDefinition("price", "number")]; + await host.AsUserAsync(ownerId, async types => (await types.CreateAsync( + new NewContentType("Product", "product", new ContentTypeDefinition(fields)), TestChangeSetTargets.ContentModel)).Value); + var entry = await host.AsUserAsync(ownerId, async entries => + (await entries.CreateAsync(new NewEntry("product", Json("""{"name": "Drill", "price": 4.5}""")))).Value); + await host.AsUserAsync(ownerId, async types => (await types.ReviseAsync( + "product", 1, new ContentTypeDefinition(fields, isEntity: true), TestChangeSetTargets.ContentModel)).Value); + await host.SignInAsync(AdminHost.Owner); + + var saved = await host.SubmitAsync($"/content/product/{entry.Id}", "entry", new Dictionary + { + ["Form.Values[name]"] = "Cordless drill", + ["Form.Facts[name.owner]"] = ownerId.ToString(), + ["Form.Values[price]"] = "4.5", + ["Form.Rationale"] = "Record the facts.", + ["Form.BaseVersion"] = "1", + }); + + Assert.StartsWith("/sets/", host.LocationOf(saved)); + var name = (await host.AsUserAsync(ownerId, async entries => (await entries.GetAsync(entry.Id)).Value)).Latest.Fields.GetProperty("name"); + Assert.Equal(("Cordless drill", ownerId.ToString()), (name.GetProperty("value").GetString(), name.GetProperty("owner").GetString())); + } + private static JsonElement Json(string json) { return JsonSerializer.Deserialize(json); diff --git a/tests/FluentCMS.IntegrationTests/Forms/EntryFormScreenTests.cs b/tests/FluentCMS.IntegrationTests/Forms/EntryFormScreenTests.cs index e0151e2eb..22553d01b 100644 --- a/tests/FluentCMS.IntegrationTests/Forms/EntryFormScreenTests.cs +++ b/tests/FluentCMS.IntegrationTests/Forms/EntryFormScreenTests.cs @@ -168,7 +168,7 @@ await host.AsUserAsync(ownerId, async entries => ["Form.BaseVersion"] = "1", }); - Assert.Contains("data-testid=\"entry-form-error\"", await again.Content.ReadAsStringAsync()); + Assert.Contains("Someone saved this entry since the form was shown.", await again.Content.ReadAsStringAsync()); var stored = await host.AsUserAsync(ownerId, async entries => (await entries.GetAsync(entry.Id)).Value); Assert.Equal((2, "Theirs"), (stored.LatestVersion, stored.Latest.Fields.GetProperty("title").GetString())); } @@ -253,6 +253,45 @@ await host.AsUserAsync(ownerId, async sets => Assert.Contains("data-state=\"awaiting-review\"", await submitted.Content.ReadAsStringAsync()); } + [Fact] + public async Task After_a_revert_the_form_names_the_reverted_draft_and_a_form_from_a_published_version_since_replaced_is_refused() + { + using var install = new TemporaryInstall(); + await using var host = await AdminHost.StartAsync(install); + var ownerId = await host.CreateFirstAdministratorAsync(); + await KitchenSinkAsync(host, ownerId); + var entry = await host.AsUserAsync(ownerId, async entries => + (await entries.CreateAsync(new NewEntry("kitchen-sink", Json("""{"title": "First"}""")))).Value); + await PublishAsync(host, ownerId, sets => sets.OpenAsync(new NewChangeSet("Publish it.", [])), ChangeOperation.Create(OperationKind.Create, ItemKind.Entry, entry.Id, null, 1).Value); + await host.AsUserAsync(ownerId, async entries => (await entries.UpdateAsync(entry.Id, 1, Json("""{"title": "Wrong"}"""))).Value); + var wrong = await PublishAsync(host, ownerId, sets => sets.OpenAsync(new NewChangeSet("Retitle.", [])), ChangeOperation.Create(OperationKind.Update, ItemKind.Entry, entry.Id, 1, 2).Value); + await PublishAsync(host, ownerId, sets => sets.RevertAsync(wrong, "The title was wrong.")); + await host.SignInAsync(AdminHost.Owner); + + var reverted = await host.AsUserAsync(ownerId, async entries => (await entries.GetAsync(entry.Id)).Value); + Assert.Equal((EntryState.Reverted, (int?)1, 2), (reverted.State, reverted.PublishedVersion, reverted.LatestVersion)); + var latest = await host.Client.GetStringAsync($"/content/kitchen-sink/{entry.Id}"); + Assert.Contains("data-testid=\"entry-draft-note\"", latest); + Assert.Contains("value=\"Wrong\"", latest); + var published = await host.Client.GetStringAsync($"/content/kitchen-sink/{entry.Id}?from=published"); + Assert.Contains("value=\"First\"", published); + Assert.Contains("name=\"Form.FromPublished\" value=\"1\"", published); + + // The published version moves on while the form is open: its values would undo that, so the save is refused. + await host.AsUserAsync(ownerId, async entries => (await entries.UpdateAsync(entry.Id, 2, Json("""{"title": "Second"}"""))).Value); + await PublishAsync(host, ownerId, sets => sets.OpenAsync(new NewChangeSet("Retitle again.", [])), ChangeOperation.Create(OperationKind.Update, ItemKind.Entry, entry.Id, 1, 3).Value); + var refused = await host.SubmitAsync($"/content/kitchen-sink/{entry.Id}?from=published", "entry", new Dictionary + { + ["Form.Values[title]"] = "First, retitled", + ["Form.Rationale"] = "Retitle.", + ["Form.BaseVersion"] = "3", + ["Form.FromPublished"] = "1", + }); + + Assert.Contains("Someone saved this entry since the form was shown.", await refused.Content.ReadAsStringAsync()); + Assert.Equal(3, (await host.AsUserAsync(ownerId, async entries => (await entries.GetAsync(entry.Id)).Value)).LatestVersion); + } + [Fact] public async Task The_content_pages_list_types_and_entries_and_need_a_signed_in_person() { @@ -271,6 +310,23 @@ public async Task The_content_pages_list_types_and_entries_and_need_a_signed_in_ Assert.Equal(HttpStatusCode.NotFound, (await host.Client.GetAsync($"/content/kitchen-sink/{Guid.NewGuid()}")).StatusCode); } + // Opens a set, adds the operations and publishes it; a revert opens its own set with its operations. + private static Task PublishAsync(AdminHost host, Guid ownerId, Func>> open, params ChangeOperation[] operations) + { + return host.AsUserAsync(ownerId, async sets => + { + var set = (await open(sets)).Value; + foreach (var operation in operations) + { + await sets.AddOperationAsync(set.Id, operation); + } + + await sets.TransitionAsync(set.Id, ChangeSetState.ChecksRunning); + await sets.TransitionAsync(set.Id, ChangeSetState.Approved); + return (await sets.PublishAsync(set.Id)).Value.Id; + }); + } + // A type with one field of every built-in type, a category entry to reference and an asset to show. private static async Task<(Guid Category, Guid Asset)> KitchenSinkAsync(AdminHost host, Guid ownerId) { diff --git a/tests/FluentCMS.Tests/Admin/FactValuesTests.cs b/tests/FluentCMS.Tests/Admin/FactValuesTests.cs index 7704f8d0b..95e2119b5 100644 --- a/tests/FluentCMS.Tests/Admin/FactValuesTests.cs +++ b/tests/FluentCMS.Tests/Admin/FactValuesTests.cs @@ -78,6 +78,45 @@ public void Parts_that_show_as_the_stored_fact_keep_its_text() Assert.Equal("2026-03-04T05:06:07.89+01:00", fact[FactMetadata.LastVerified]!.GetValue()); } + [Fact] + public void A_fact_whose_value_and_parts_all_show_as_stored_is_kept_exactly() + { + var stored = JsonDocument.Parse($$"""{"value": 4.50, "source": {"reference": "prices.csv ", "kind": "file"}, "owner": "{{Owner.ToString().ToUpperInvariant()}}", "validFrom": null, "validUntil": null, "lastVerified": "2026-03-04T05:06:07+01:00", "note": "kept"}""").RootElement; + var shown = new Dictionary(); + FactValues.Show("price", stored, shown); + + var fact = FactValues.Read("price", JsonNode.Parse("4.50")!, shown, [], stored, new HashSet { Owner }); + + var unescaped = new JsonSerializerOptions { Encoder = System.Text.Encodings.Web.JavaScriptEncoder.UnsafeRelaxedJsonEscaping }; + Assert.Equal(stored.GetRawText().Replace(" ", "", StringComparison.Ordinal), fact!.ToJsonString(unescaped).Replace(" ", "", StringComparison.Ordinal)); + } + + [Fact] + public void A_stored_plain_value_is_read_as_no_fact_yet() + { + var problems = new Dictionary(); + + var fact = FactValues.Read("price", JsonValue.Create(10m), Posted(("price.owner", Owner.ToString())), problems, JsonDocument.Parse("10").RootElement, new HashSet { Owner }); + + Assert.Empty(problems); + Assert.Equal(Owner.ToString(), fact![FactMetadata.Owner]!.GetValue()); + Assert.Null(FactValues.OwnerOf(JsonDocument.Parse("10").RootElement)); + } + + [Fact] + public void The_owner_a_fact_already_has_is_kept_though_no_user_has_that_id() + { + var stranger = Guid.NewGuid(); + var stored = JsonDocument.Parse($$"""{"value": 10, "source": null, "owner": "{{stranger}}", "validFrom": null, "validUntil": null, "lastVerified": null}""").RootElement; + var problems = new Dictionary(); + + var fact = FactValues.Read("price", JsonValue.Create(11m), Posted(("price.owner", stranger.ToString())), problems, stored, new HashSet { Owner }); + + Assert.Empty(problems); + Assert.Equal(stranger.ToString(), fact![FactMetadata.Owner]!.GetValue()); + Assert.Equal(stranger, FactValues.OwnerOf(stored)); + } + [Fact] public void An_owner_who_is_not_a_known_user_is_a_problem() { From 676871cf79f09c511c013bee5c6e27ffb610d179 Mon Sep 17 00:00:00 2001 From: Amir Pournasserian Date: Mon, 19 Oct 2026 08:33:33 -0400 Subject: [PATCH 3/5] Keep multi-line texts and plain entity values, and check the published base in the unit of work --- .../Components/Forms/FieldWidget.razor | 17 +++++-- .../Components/Pages/EntryEditor.razor | 38 +++++++++------- src/FluentCMS.Admin/Forms/FactValues.cs | 10 +++-- src/FluentCMS.Core/Content/EntryEdits.cs | 8 ++++ src/FluentCMS.Core/Content/IEntryEdits.cs | 7 ++- .../Browser/EntryFormBrowserTests.cs | 44 +++++++++++++++++++ .../Forms/EntityFormScreenTests.cs | 10 ++++- .../Forms/EntryEditsTests.cs | 21 +++++++++ .../Forms/EntryFormScreenTests.cs | 25 +++++++++-- 9 files changed, 150 insertions(+), 30 deletions(-) create mode 100644 tests/FluentCMS.IntegrationTests/Browser/EntryFormBrowserTests.cs diff --git a/src/FluentCMS.Admin/Components/Forms/FieldWidget.razor b/src/FluentCMS.Admin/Components/Forms/FieldWidget.razor index 74d2db178..11f413a88 100644 --- a/src/FluentCMS.Admin/Components/Forms/FieldWidget.razor +++ b/src/FluentCMS.Admin/Components/Forms/FieldWidget.razor @@ -26,11 +26,11 @@ break; case "richText": - ", form); + Assert.Matches("]*name=\"Form.Values\\[body\\]\"[^>]*> First line.", form); + // What a browser posts back for the untouched fields: the widgets' text, with CRLF line breaks and minutes only. var saved = await host.SubmitAsync($"/content/kitchen-sink/{entry.Id}", "entry", new Dictionary { ["Form.Values[title]"] = "Cordless drill", - ["Form.Values[body]"] = "First line.\r\nSecond line.", + ["Form.Values[body]"] = "\r\nFirst line.\r\nSecond line.", + ["Form.Values[subtitle]"] = "Line one\r\nLine two", ["Form.Values[launch]"] = "2026-11-01T07:30", ["Form.Values[price]"] = "4.50", ["Form.Rationale"] = "Retitle.", @@ -199,8 +205,9 @@ public async Task An_edit_of_one_field_leaves_every_other_stored_value_exactly_a var set = await host.AsUserAsync(ownerId, async sets => (await sets.GetAsync(Guid.Parse(host.LocationOf(saved)![6..42]))).Value); var fields = (await host.AsUserAsync(ownerId, async entries => (await entries.GetAsync(entry.Id)).Value)).Latest.Fields; Assert.Equal("Cordless drill", fields.GetProperty("title").GetString()); - Assert.Equal("First line.\nSecond line.", fields.GetProperty("body").GetString()); - Assert.Equal(DateTimeOffset.Parse("2026-11-01T09:30:45.5+02:00", System.Globalization.CultureInfo.InvariantCulture), fields.GetProperty("launch").GetDateTimeOffset()); + Assert.Equal("\nFirst line.\nSecond line.", fields.GetProperty("body").GetString()); + Assert.Equal("Line one\nLine two", fields.GetProperty("subtitle").GetString()); + Assert.Equal("2026-11-01T09:30:45.5+02:00", fields.GetProperty("launch").GetString()); Assert.Equal("4.50", fields.GetProperty("price").GetRawText()); Assert.False(fields.TryGetProperty("active", out _)); Assert.Equal(2, set.Operations[0].ProposedVersion); @@ -290,6 +297,15 @@ public async Task After_a_revert_the_form_names_the_reverted_draft_and_a_form_fr Assert.Contains("Someone saved this entry since the form was shown.", await refused.Content.ReadAsStringAsync()); Assert.Equal(3, (await host.AsUserAsync(ownerId, async entries => (await entries.GetAsync(entry.Id)).Value)).LatestVersion); + + // A reverted create leaves nothing published, and the form says whose values it shows. + var created = await host.AsUserAsync(ownerId, async entries => + (await entries.CreateAsync(new NewEntry("kitchen-sink", Json("""{"title": "Never meant"}""")))).Value); + var creation = await PublishAsync(host, ownerId, sets => sets.OpenAsync(new NewChangeSet("Add it.", [])), ChangeOperation.Create(OperationKind.Create, ItemKind.Entry, created.Id, null, 1).Value); + await PublishAsync(host, ownerId, sets => sets.RevertAsync(creation, "It was never meant.")); + var revertedCreate = await host.Client.GetStringAsync($"/content/kitchen-sink/{created.Id}"); + Assert.Contains("whose creation was reverted", revertedCreate); + Assert.DoesNotContain("data-testid=\"from-published\"", revertedCreate); } [Fact] @@ -339,6 +355,7 @@ await host.AsUserAsync(ownerId, async types [ new FieldDefinition("title", "text", required: true, settings: Json("""{"maxLength": 80}""")), new FieldDefinition("body", "richText"), + new FieldDefinition("subtitle", "text"), new FieldDefinition("price", "number"), new FieldDefinition("active", "boolean"), new FieldDefinition("launch", "dateTime"), From 336986b61f2a9cc7f53a4eeb68491f61ced4e981 Mon Sep 17 00:00:00 2001 From: Amir Pournasserian Date: Mon, 19 Oct 2026 18:15:03 -0400 Subject: [PATCH 4/5] Keep values a changed field type no longer takes and plain JSON objects, and catch a revert under an open form --- .../Components/Pages/EntryEditor.razor | 15 ++++++- src/FluentCMS.Admin/Forms/EntryFormModel.cs | 2 +- src/FluentCMS.Admin/Forms/FactValues.cs | 18 +++++--- src/FluentCMS.Admin/Forms/FieldValues.cs | 8 ++-- src/FluentCMS.Core/Content/EntryEdits.cs | 20 +++++++-- src/FluentCMS.Core/Content/IEntryEdits.cs | 4 +- .../Forms/EntityFormScreenTests.cs | 8 +++- .../Forms/EntryEditsTests.cs | 3 ++ .../Forms/EntryFormScreenTests.cs | 42 ++++++++++++++++++- .../FluentCMS.Tests/Admin/FactValuesTests.cs | 14 +++++++ .../Admin/GeneratedFormTests.cs | 8 ++++ 11 files changed, 123 insertions(+), 19 deletions(-) diff --git a/src/FluentCMS.Admin/Components/Pages/EntryEditor.razor b/src/FluentCMS.Admin/Components/Pages/EntryEditor.razor index 5e4f6f318..efbce8d25 100644 --- a/src/FluentCMS.Admin/Components/Pages/EntryEditor.razor +++ b/src/FluentCMS.Admin/Components/Pages/EntryEditor.razor @@ -184,7 +184,13 @@ } // An update is always made from the latest version, so the version check holds whichever version the values come from. - Shown = new EntryFormModel { BaseVersion = _entry?.LatestVersion ?? 0, FromPublished = FromPublished ? _entry!.PublishedVersion : null }; + // The published version the values came from: the one asked for, or the latest when it is the published one, so a revert or a + // publish while the form is open is caught when it is saved. + Shown = new EntryFormModel + { + BaseVersion = _entry?.LatestVersion ?? 0, + FromPublished = FromPublished || _entry?.LatestVersion == _entry?.PublishedVersion ? _entry?.PublishedVersion : null, + }; _unsourced.Clear(); _stored.Clear(); foreach (var field in _type.Definition.Fields) @@ -251,6 +257,13 @@ { var stored = _stored.TryGetValue(field.Name, out var storedValue) ? storedValue : (JsonElement?)null; var storedValueOnly = _type.Definition.IsEntity ? FactValues.Show(field.Name, stored, new Dictionary()) : stored; + + // A stored value its field's type no longer takes, as after a number field became text, is read from what was posted. + if (storedValueOnly is { } keptValue && FieldTypes.ValidateValue(field.Type, field.Settings, keptValue).Count > 0) + { + storedValueOnly = null; + } + var value = FieldValues.Read(field, Form.Values.GetValueOrDefault(field.Name), out var problem, storedValueOnly); if (value is null && field.Type == "boolean" && _type.Definition.IsEntity && FactValues.HasDetails(field.Name, Form.Facts)) { diff --git a/src/FluentCMS.Admin/Forms/EntryFormModel.cs b/src/FluentCMS.Admin/Forms/EntryFormModel.cs index 32b37905b..6cc342113 100644 --- a/src/FluentCMS.Admin/Forms/EntryFormModel.cs +++ b/src/FluentCMS.Admin/Forms/EntryFormModel.cs @@ -15,7 +15,7 @@ public sealed class EntryFormModel public int BaseVersion { get; set; } - // The published version the values came from, when the form started from it rather than from the latest draft. + // The published version the values came from, when they are the published version's. public int? FromPublished { get; set; } public IReadOnlyList SourceList() diff --git a/src/FluentCMS.Admin/Forms/FactValues.cs b/src/FluentCMS.Admin/Forms/FactValues.cs index 1dd431e67..692896518 100644 --- a/src/FluentCMS.Admin/Forms/FactValues.cs +++ b/src/FluentCMS.Admin/Forms/FactValues.cs @@ -19,10 +19,17 @@ public static class FactValues public static string Key(string field, string part) => $"{field}.{part}"; + // A stored fact is an object with a value. Anything else, such as a JSON object written before its type held facts, is a plain + // value with no fact around it yet. + public static bool IsFact(JsonElement? stored) + { + return stored is { ValueKind: JsonValueKind.Object } fact && fact.TryGetProperty(FactMetadata.Value, out _); + } + // The owner a stored fact names, so an owner who is no longer a user still shows and is kept. public static Guid? OwnerOf(JsonElement? stored) { - return stored is { ValueKind: JsonValueKind.Object } fact && fact.TryGetProperty(FactMetadata.Owner, out var owner) + return IsFact(stored) && stored!.Value.TryGetProperty(FactMetadata.Owner, out var owner) && owner.ValueKind == JsonValueKind.String && Guid.TryParse(owner.GetString(), out var id) ? id : null; } @@ -38,7 +45,7 @@ public static class FactValues JsonElement? stored = null, IReadOnlySet? owners = null) { - var fact = stored is { ValueKind: JsonValueKind.Object } storedFact ? storedFact : (JsonElement?)null; + var fact = IsFact(stored) ? stored : null; var shown = new Dictionary(); var storedValue = Show(field, fact, shown); if (fact is { } untouched && storedValue is { } kept && JsonNode.DeepEquals(value, JsonNode.Parse(kept.GetRawText())) @@ -125,11 +132,13 @@ public static class FactValues // held facts, is the value of a fact with no details yet. public static JsonElement? Show(string field, JsonElement? stored, IDictionary facts) { - if (stored is not { ValueKind: JsonValueKind.Object } fact) + if (!IsFact(stored)) { return stored is { ValueKind: not (JsonValueKind.Null or JsonValueKind.Undefined) } plain ? plain : null; } + var fact = stored!.Value; + if (fact.TryGetProperty(FactMetadata.Source, out var source) && source.ValueKind == JsonValueKind.Object) { facts[Key(field, Kind)] = source.GetProperty(FactMetadata.SourceKind).GetString(); @@ -163,7 +172,6 @@ public static bool HasDetails(string field, IReadOnlyDictionary public static bool IsUnsourced(JsonElement? stored) { return stored is { ValueKind: not (JsonValueKind.Null or JsonValueKind.Undefined) } value - && (value.ValueKind != JsonValueKind.Object - || !value.TryGetProperty(FactMetadata.Source, out var source) || source.ValueKind != JsonValueKind.Object); + && (!IsFact(value) || !value.TryGetProperty(FactMetadata.Source, out var source) || source.ValueKind != JsonValueKind.Object); } } diff --git a/src/FluentCMS.Admin/Forms/FieldValues.cs b/src/FluentCMS.Admin/Forms/FieldValues.cs index 6dc92054b..246d2d2b2 100644 --- a/src/FluentCMS.Admin/Forms/FieldValues.cs +++ b/src/FluentCMS.Admin/Forms/FieldValues.cs @@ -43,9 +43,9 @@ public static IReadOnlyList Strings(FieldDefinition field, string name) { problem = null; - // Browsers send a textarea's line breaks as CRLF; stored text keeps LF. - posted = posted?.ReplaceLineEndings("\n"); - if (stored is { ValueKind: not JsonValueKind.Null } kept && posted is not null && posted == Show(field, kept)?.ReplaceLineEndings("\n")) + // Browsers send a textarea's line breaks as CRLF; stored text keeps LF. Other line separators are text and stay as typed. + posted = Lf(posted); + if (stored is { ValueKind: not JsonValueKind.Null } kept && posted is not null && posted == Lf(Show(field, kept))) { return JsonNode.Parse(kept.GetRawText()); } @@ -114,6 +114,8 @@ public static IReadOnlyList Strings(FieldDefinition field, string name) } } + private static string? Lf(string? text) => text?.Replace("\r\n", "\n", StringComparison.Ordinal).Replace('\r', '\n'); + // What the widget shows for a stored value. public static string? Show(FieldDefinition field, JsonElement? value) { diff --git a/src/FluentCMS.Core/Content/EntryEdits.cs b/src/FluentCMS.Core/Content/EntryEdits.cs index 11b3aa578..8097d2b89 100644 --- a/src/FluentCMS.Core/Content/EntryEdits.cs +++ b/src/FluentCMS.Core/Content/EntryEdits.cs @@ -22,6 +22,13 @@ public async Task> SaveAsync(EntryEdit edit, Cancellation return Result.Failure(ContentErrorCodes.VersionConflict, "An update names the version it was made from."); } + // Publishing that moved on since the values were read is found before anything is written, as the directories find theirs. + if (edit.PublishedBase is { } expected && edit.EntryId is { } entryId + && await entries.GetAsync(entryId, cancellationToken) is { IsSuccess: true } current && current.Value.PublishedVersion != expected) + { + return PublishedMoved(expected, current.Value.PublishedVersion); + } + // The set's own expected failures are found before anything is written, as the directories find theirs. if (string.IsNullOrWhiteSpace(edit.ChangeSet.Rationale)) { @@ -43,12 +50,10 @@ public async Task> SaveAsync(EntryEdit edit, Cancellation return Result.Failure(entry.Errors); } - // Checked against the version the operation is based on below, read in this unit of work, so no publish slips in between. + // Checked again against the published version the operation is based on below, for a publish since the check above. if (edit.PublishedBase is { } publishedBase && entry.Value.PublishedVersion != publishedBase) { - return Result.Failure( - ContentErrorCodes.VersionConflict, - $"The values came from published version {publishedBase}, and version {entry.Value.PublishedVersion?.ToString(System.Globalization.CultureInfo.InvariantCulture) ?? "none"} is published now."); + return PublishedMoved(publishedBase, entry.Value.PublishedVersion); } var type = await contentTypes.GetAsync(entry.Value.ContentType, cancellationToken); @@ -82,4 +87,11 @@ public async Task> SaveAsync(EntryEdit edit, Cancellation await scope.CommitAsync(cancellationToken); return Result.Success(new SavedEntryEdit(entry.Value, set.Value)); } + + private static Result PublishedMoved(int expected, int? published) + { + return Result.Failure( + ContentErrorCodes.VersionConflict, + $"The values came from published version {expected}, and {(published is { } version ? $"version {version}" : "no version")} is published now."); + } } diff --git a/src/FluentCMS.Core/Content/IEntryEdits.cs b/src/FluentCMS.Core/Content/IEntryEdits.cs index deb7e1b57..889b0ac88 100644 --- a/src/FluentCMS.Core/Content/IEntryEdits.cs +++ b/src/FluentCMS.Core/Content/IEntryEdits.cs @@ -14,8 +14,8 @@ public interface IEntryEdits // EntryId and BaseVersion name the entry and the version the form read for an update, and are null for a create. public sealed record EntryEdit(string ContentType, Guid? EntryId, int? BaseVersion, JsonElement Fields, NewChangeSet ChangeSet) { - // The published version an update's values came from, when they came from it rather than from the latest draft. If publishing - // has moved on, the save is refused, so it cannot undo what was published meanwhile. + // The published version an update's values came from, when they are the published version's. If publishing has moved on, the + // save is refused, so it cannot undo what was published meanwhile. public int? PublishedBase { get; init; } } diff --git a/tests/FluentCMS.IntegrationTests/Forms/EntityFormScreenTests.cs b/tests/FluentCMS.IntegrationTests/Forms/EntityFormScreenTests.cs index ed5e59d19..0d5737599 100644 --- a/tests/FluentCMS.IntegrationTests/Forms/EntityFormScreenTests.cs +++ b/tests/FluentCMS.IntegrationTests/Forms/EntityFormScreenTests.cs @@ -176,11 +176,11 @@ public async Task An_edit_of_an_entry_written_before_its_type_held_facts_saves_i using var install = new TemporaryInstall(); await using var host = await AdminHost.StartAsync(install); var ownerId = await host.CreateFirstAdministratorAsync(); - IReadOnlyList fields = [new FieldDefinition("name", "text", required: true), new FieldDefinition("price", "number")]; + IReadOnlyList fields = [new FieldDefinition("name", "text", required: true), new FieldDefinition("price", "number"), new FieldDefinition("specs", "json")]; await host.AsUserAsync(ownerId, async types => (await types.CreateAsync( new NewContentType("Product", "product", new ContentTypeDefinition(fields)), TestChangeSetTargets.ContentModel)).Value); var entry = await host.AsUserAsync(ownerId, async entries => - (await entries.CreateAsync(new NewEntry("product", Json("""{"name": "Drill", "price": 4.5}""")))).Value); + (await entries.CreateAsync(new NewEntry("product", Json("""{"name": "Drill", "price": 4.5, "specs": {"watts": 500}}""")))).Value); await host.AsUserAsync(ownerId, async types => (await types.ReviseAsync( "product", 1, new ContentTypeDefinition(fields, isEntity: true), TestChangeSetTargets.ContentModel)).Value); await host.SignInAsync(AdminHost.Owner); @@ -189,12 +189,15 @@ await host.AsUserAsync(ownerId, async types var form = await host.Client.GetStringAsync($"/content/product/{entry.Id}"); Assert.Contains("value=\"4.5\"", form); Assert.Contains("data-testid=\"fact-unsourced\"", Fieldset(form, "price")); + Assert.Contains(""watts": 500", form); + Assert.Contains("data-testid=\"fact-unsourced\"", Fieldset(form, "specs")); var saved = await host.SubmitAsync($"/content/product/{entry.Id}", "entry", new Dictionary { ["Form.Values[name]"] = "Cordless drill", ["Form.Facts[name.owner]"] = ownerId.ToString(), ["Form.Values[price]"] = "4.5", + ["Form.Values[specs]"] = "{\r\n \"watts\": 500\r\n}", ["Form.Rationale"] = "Record the facts.", ["Form.BaseVersion"] = "1", }); @@ -204,6 +207,7 @@ await host.AsUserAsync(ownerId, async types var name = stored.GetProperty("name"); Assert.Equal(("Cordless drill", ownerId.ToString()), (name.GetProperty("value").GetString(), name.GetProperty("owner").GetString())); Assert.Equal("4.5", stored.GetProperty("price").GetProperty("value").GetRawText()); + Assert.Equal(500, stored.GetProperty("specs").GetProperty("value").GetProperty("watts").GetInt32()); } private static JsonElement Json(string json) diff --git a/tests/FluentCMS.IntegrationTests/Forms/EntryEditsTests.cs b/tests/FluentCMS.IntegrationTests/Forms/EntryEditsTests.cs index bf401554d..7fb414683 100644 --- a/tests/FluentCMS.IntegrationTests/Forms/EntryEditsTests.cs +++ b/tests/FluentCMS.IntegrationTests/Forms/EntryEditsTests.cs @@ -71,6 +71,7 @@ public async Task An_update_from_a_published_version_that_is_not_the_published_o { await using var scenario = await CreateAsync(provider); var first = (await SaveAsync(scenario, new EntryEdit("blog-post", null, null, Json("""{"title": "Hello"}"""), new NewChangeSet("Draft.", [])))).Value; + var events = await scenario.Schema.QueryAsync(context => context.AuditEvents.CountAsync()); var refused = await SaveAsync(scenario, new EntryEdit("blog-post", first.Entry.Id, 1, Json("""{"title": "Old values"}"""), new NewChangeSet("Retitle.", [])) { @@ -79,6 +80,8 @@ public async Task An_update_from_a_published_version_that_is_not_the_published_o Assert.Equal(ContentErrorCodes.VersionConflict, Assert.Single(refused.Errors).Code); Assert.Single(await scenario.EntryRowsAsync()); + Assert.Equal(1, await scenario.Schema.QueryAsync(context => context.EntryVersions.CountAsync(row => row.EntryId == first.Entry.Id))); + Assert.Equal(events, await scenario.Schema.QueryAsync(context => context.AuditEvents.CountAsync())); Assert.Equal(1, await scenario.Schema.QueryAsync(context => context.ChangeSets.CountAsync(row => row.SessionId != TestChangeSetTargets.ContentModelSession))); } diff --git a/tests/FluentCMS.IntegrationTests/Forms/EntryFormScreenTests.cs b/tests/FluentCMS.IntegrationTests/Forms/EntryFormScreenTests.cs index 813577fce..9e5e4cb00 100644 --- a/tests/FluentCMS.IntegrationTests/Forms/EntryFormScreenTests.cs +++ b/tests/FluentCMS.IntegrationTests/Forms/EntryFormScreenTests.cs @@ -272,9 +272,20 @@ public async Task After_a_revert_the_form_names_the_reverted_draft_and_a_form_fr await PublishAsync(host, ownerId, sets => sets.OpenAsync(new NewChangeSet("Publish it.", [])), ChangeOperation.Create(OperationKind.Create, ItemKind.Entry, entry.Id, null, 1).Value); await host.AsUserAsync(ownerId, async entries => (await entries.UpdateAsync(entry.Id, 1, Json("""{"title": "Wrong"}"""))).Value); var wrong = await PublishAsync(host, ownerId, sets => sets.OpenAsync(new NewChangeSet("Retitle.", [])), ChangeOperation.Create(OperationKind.Update, ItemKind.Entry, entry.Id, 1, 2).Value); - await PublishAsync(host, ownerId, sets => sets.RevertAsync(wrong, "The title was wrong.")); await host.SignInAsync(AdminHost.Owner); + // A form open on the published version when a revert publishes an older one can't put the reverted values back. + Assert.Contains("name=\"Form.FromPublished\" value=\"2\"", await host.Client.GetStringAsync($"/content/kitchen-sink/{entry.Id}")); + await PublishAsync(host, ownerId, sets => sets.RevertAsync(wrong, "The title was wrong.")); + var stale = await host.SubmitAsync($"/content/kitchen-sink/{entry.Id}", "entry", new Dictionary + { + ["Form.Values[title]"] = "Wrong, retitled", + ["Form.Rationale"] = "Retitle.", + ["Form.BaseVersion"] = "2", + ["Form.FromPublished"] = "2", + }); + Assert.Contains("Someone saved this entry since the form was shown.", await stale.Content.ReadAsStringAsync()); + var reverted = await host.AsUserAsync(ownerId, async entries => (await entries.GetAsync(entry.Id)).Value); Assert.Equal((EntryState.Reverted, (int?)1, 2), (reverted.State, reverted.PublishedVersion, reverted.LatestVersion)); var latest = await host.Client.GetStringAsync($"/content/kitchen-sink/{entry.Id}"); @@ -308,6 +319,35 @@ public async Task After_a_revert_the_form_names_the_reverted_draft_and_a_form_fr Assert.DoesNotContain("data-testid=\"from-published\"", revertedCreate); } + // A field whose type changed keeps its stored value until it is saved; the form then saves what the new type takes. + [Fact] + public async Task An_untouched_value_its_field_type_no_longer_takes_is_saved_as_the_new_type() + { + using var install = new TemporaryInstall(); + await using var host = await AdminHost.StartAsync(install); + var ownerId = await host.CreateFirstAdministratorAsync(); + FieldDefinition[] before = [new FieldDefinition("title", "text"), new FieldDefinition("code", "number")]; + await host.AsUserAsync(ownerId, async types => (await types.CreateAsync( + new NewContentType("Part", "part", new ContentTypeDefinition(before)), TestChangeSetTargets.ContentModel)).Value); + var entry = await host.AsUserAsync(ownerId, async entries => + (await entries.CreateAsync(new NewEntry("part", Json("""{"title": "Bolt", "code": 45}""")))).Value); + await host.AsUserAsync(ownerId, async types => (await types.ReviseAsync( + "part", 1, new ContentTypeDefinition([new FieldDefinition("title", "text"), new FieldDefinition("code", "text")]), TestChangeSetTargets.ContentModel)).Value); + await host.SignInAsync(AdminHost.Owner); + + var saved = await host.SubmitAsync($"/content/part/{entry.Id}", "entry", new Dictionary + { + ["Form.Values[title]"] = "Hex bolt", + ["Form.Values[code]"] = "45", + ["Form.Rationale"] = "Retitle.", + ["Form.BaseVersion"] = "1", + }); + + Assert.StartsWith("/sets/", host.LocationOf(saved)); + var code = (await host.AsUserAsync(ownerId, async entries => (await entries.GetAsync(entry.Id)).Value)).Latest.Fields.GetProperty("code"); + Assert.Equal((JsonValueKind.String, "45"), (code.ValueKind, code.GetString())); + } + [Fact] public async Task The_content_pages_list_types_and_entries_and_need_a_signed_in_person() { diff --git a/tests/FluentCMS.Tests/Admin/FactValuesTests.cs b/tests/FluentCMS.Tests/Admin/FactValuesTests.cs index 95e2119b5..d86223791 100644 --- a/tests/FluentCMS.Tests/Admin/FactValuesTests.cs +++ b/tests/FluentCMS.Tests/Admin/FactValuesTests.cs @@ -103,6 +103,20 @@ public void A_stored_plain_value_is_read_as_no_fact_yet() Assert.Null(FactValues.OwnerOf(JsonDocument.Parse("10").RootElement)); } + [Fact] + public void A_json_object_without_a_value_is_a_plain_value_not_a_fact() + { + var facts = new Dictionary(); + var plain = JsonDocument.Parse("""{"watts": 500}""").RootElement; + + var shown = FactValues.Show("specs", plain, facts); + + Assert.Equal(500, shown!.Value.GetProperty("watts").GetInt32()); + Assert.Empty(facts); + Assert.False(FactValues.IsFact(plain)); + Assert.True(FactValues.IsUnsourced(plain)); + } + [Fact] public void The_owner_a_fact_already_has_is_kept_though_no_user_has_that_id() { diff --git a/tests/FluentCMS.Tests/Admin/GeneratedFormTests.cs b/tests/FluentCMS.Tests/Admin/GeneratedFormTests.cs index 4d58c6270..d14514d7a 100644 --- a/tests/FluentCMS.Tests/Admin/GeneratedFormTests.cs +++ b/tests/FluentCMS.Tests/Admin/GeneratedFormTests.cs @@ -63,6 +63,14 @@ public void A_posted_text_that_shows_as_the_stored_value_keeps_it_exactly(string Assert.Equal(stored, value!.ToJsonString(Unescaped)); } + [Fact] + public void Only_carriage_returns_are_line_breaks_to_compare_and_other_separators_stay_as_typed() + { + var value = FieldValues.Read(Field("richText", "{}"), "One\r\nTwo\u2028three", out _, JsonDocument.Parse("\"One\\nTwo\"").RootElement); + + Assert.Equal("One\nTwo\u2028three", value!.GetValue()); + } + [Fact] public void A_changed_text_replaces_the_stored_value() { From ad11fae821bd43ef42c7b9aa5850141432573de4 Mon Sep 17 00:00:00 2001 From: Amir Pournasserian Date: Mon, 19 Oct 2026 18:31:02 -0400 Subject: [PATCH 5/5] Show CR-only texts in a textarea and name a reverted draft with the published version on offer --- .../Components/Forms/FieldWidget.razor | 2 +- .../Components/Pages/EntryEditor.razor | 8 ++++++-- .../Forms/EntryEditsTests.cs | 2 +- .../Forms/EntryFormScreenTests.cs | 3 ++- tests/FluentCMS.Tests/Admin/GeneratedFormTests.cs | 13 +++++++++++++ 5 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/FluentCMS.Admin/Components/Forms/FieldWidget.razor b/src/FluentCMS.Admin/Components/Forms/FieldWidget.razor index 11f413a88..3747582e8 100644 --- a/src/FluentCMS.Admin/Components/Forms/FieldWidget.razor +++ b/src/FluentCMS.Admin/Components/Forms/FieldWidget.razor @@ -49,7 +49,7 @@ break; default: @* A single-line input drops line breaks, so a text that has them is shown where they survive the post. *@ - @if (Value?.Contains('\n') == true) + @if (Value?.IndexOfAny(['\n', '\r']) >= 0) {