diff --git a/FormCraft.ForMudBlazor.UnitTests/Components/CollectionRenderCharacterisationTests.cs b/FormCraft.ForMudBlazor.UnitTests/Components/CollectionRenderCharacterisationTests.cs index 958eee1f..376ea3b1 100644 --- a/FormCraft.ForMudBlazor.UnitTests/Components/CollectionRenderCharacterisationTests.cs +++ b/FormCraft.ForMudBlazor.UnitTests/Components/CollectionRenderCharacterisationTests.cs @@ -371,34 +371,36 @@ private IRenderedComponent> RenderOrderForm( bool allowAdd = false, bool allowRemove = false) { - var config = FormBuilder - .Create() - .AddCollectionField(x => x.Items, collection => + // The label and the one-string-field item form are byte-identical to TextItemForm's, so the + // only thing this suite varies is the three flags — and configureCollection (#300) is how a + // caller reaches settings that belong to the collection rather than to a field. + // + // They stay CONDITIONAL. Applying them unconditionally would render an Add button on every + // call, and the sibling tests locate it with FindComponent(), which throws when + // it is absent and matches a newly-rendered one when it is not expected — so the flags are + // load-bearing for what those tests assert, not just for what this helper builds. + var config = TextItemForm(configureCollection: collection => + { + if (allowReorder) + { + collection.AllowReorder(); + } + + if (allowAdd) { - collection.WithLabel("Items"); - if (allowReorder) - { - collection.AllowReorder(); - } - - if (allowAdd) - { - collection.AllowAdd(); - } - - if (allowRemove) - { - collection.AllowRemove(); - } - - collection.WithItemForm(item => item - .AddField(x => x.ProductName, field => field.WithLabel("Product"))); - }) - .Build(); - - return Render>(parameters => parameters - .Add(p => p.Model, model) - .Add(p => p.Configuration, config)); + collection.AllowAdd(); + } + + if (allowRemove) + { + collection.AllowRemove(); + } + }); + + // ...and the render goes through the fixture's own helper too, which is the rest of "built + // from the fixture": this was the last hand-rolled Model/Configuration wiring in a suite + // that already calls RenderItemForm everywhere else. + return this.RenderItemForm(model, config); } /// diff --git a/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixture.cs b/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixture.cs index 6343af1e..2f6a3569 100644 --- a/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixture.cs +++ b/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixture.cs @@ -67,8 +67,24 @@ namespace FormCraft.ForMudBlazor.UnitTests.Fields; /// /// /// Models are deliberately dumb and stable — no behaviour, no validation attributes, no computed -/// members. Everything a suite wants to vary goes through the configure callback on the item-form -/// builders instead, so a change here cannot ripple into what an unrelated suite asserts. +/// members. Everything a suite wants to vary goes through a callback on the item-form builders +/// instead, so a change here cannot ripple into what an unrelated suite asserts. +/// +/// +/// There are two such callbacks on every builder, and the split is by target rather than by +/// convenience: configure (or the per-field configureText/configureNumeric/… on +/// ) reaches a field, while configureCollection reaches +/// the collection — AllowReorder, AllowAdd, WithMinItems and the rest, +/// which no field callback can express. Until #300 only had the +/// latter, so a suite needing a collection-level setting on a single-field item form hand-rolled the +/// whole configuration; the parameter is now uniform across every builder here. +/// +/// +/// Both callbacks run after the builder has applied its own label and item form, so a caller +/// can override those. That ordering is the contract, and it is pinned by +/// CollectionItemFixtureTests.Each_Item_Forms_Collection_Callback_Should_Run_After_The_Fixtures_Own_Configuration +/// — a reorder-only test cannot catch a builder that invokes the callback first, because +/// AllowReorder is an independent setter that reads the same either way. /// /// internal static class CollectionItemFixture @@ -121,70 +137,112 @@ internal static NamedOrderModel NewNamedOrder(string name = "", string itemName /// /// A collection whose item form holds one string field labelled "Product" — the text path. /// + /// Configures the item's field. Runs after its default label, so it can override it. + /// + /// Configures the collection — AllowReorder, AllowAdd, WithMinItems + /// and the rest, none of which a field callback can reach. Runs after this builder has set the + /// label and the item form, so it can override those too (#300). + /// internal static IFormConfiguration TextItemForm( - Action>? configure = null) => + Action>? configure = null, + Action>? configureCollection = null) => FormBuilder .Create() - .AddCollectionField(x => x.Items, collection => collection - .WithLabel("Items") - .WithItemForm(item => item - .AddField(x => x.ProductName, field => - { - field.WithLabel("Product"); - configure?.Invoke(field); - }))) + .AddCollectionField(x => x.Items, collection => + { + collection + .WithLabel("Items") + .WithItemForm(item => item + .AddField(x => x.ProductName, field => + { + field.WithLabel("Product"); + configure?.Invoke(field); + })); + configureCollection?.Invoke(collection); + }) .Build(); /// /// A collection whose item form holds one int field labelled "Quantity" — the numeric path. /// + /// Configures the item's field. Runs after its default label, so it can override it. + /// + /// Configures the collection rather than a field; runs last, after the label and item + /// form. See for the full contract (#300). + /// internal static IFormConfiguration NumericItemForm( - Action>? configure = null) => + Action>? configure = null, + Action>? configureCollection = null) => FormBuilder .Create() - .AddCollectionField(x => x.Lines, collection => collection - .WithLabel("Lines") - .WithItemForm(item => item - .AddField(x => x.Quantity, field => - { - field.WithLabel("Quantity"); - configure?.Invoke(field); - }))) + .AddCollectionField(x => x.Lines, collection => + { + collection + .WithLabel("Lines") + .WithItemForm(item => item + .AddField(x => x.Quantity, field => + { + field.WithLabel("Quantity"); + configure?.Invoke(field); + })); + configureCollection?.Invoke(collection); + }) .Build(); /// /// A collection whose item form holds one DateTime field labelled "When" — the date path. /// + /// Configures the item's field. Runs after its default label, so it can override it. + /// + /// Configures the collection rather than a field; runs last, after the label and item + /// form. See for the full contract (#300). + /// internal static IFormConfiguration DateItemForm( - Action>? configure = null) => + Action>? configure = null, + Action>? configureCollection = null) => FormBuilder .Create() - .AddCollectionField(x => x.Slots, collection => collection - .WithLabel("Slots") - .WithItemForm(item => item - .AddField(x => x.When, field => - { - field.WithLabel("When"); - configure?.Invoke(field); - }))) + .AddCollectionField(x => x.Slots, collection => + { + collection + .WithLabel("Slots") + .WithItemForm(item => item + .AddField(x => x.When, field => + { + field.WithLabel("When"); + configure?.Invoke(field); + })); + configureCollection?.Invoke(collection); + }) .Build(); /// /// A collection whose item form holds one bool field labelled "Gift" — the checkbox, the /// one component that binds neither adornments nor Required. /// + /// Configures the item's field. Runs after its default label, so it can override it. + /// + /// Configures the collection rather than a field; runs last, after the label and item + /// form. Distinct from 's despite both configuring + /// BasketModel.Lines — separate builders wire separate callbacks (#300). + /// internal static IFormConfiguration BooleanItemForm( - Action>? configure = null) => + Action>? configure = null, + Action>? configureCollection = null) => FormBuilder .Create() - .AddCollectionField(x => x.Lines, collection => collection - .WithLabel("Lines") - .WithItemForm(item => item - .AddField(x => x.IsGift, field => - { - field.WithLabel("Gift"); - configure?.Invoke(field); - }))) + .AddCollectionField(x => x.Lines, collection => + { + collection + .WithLabel("Lines") + .WithItemForm(item => item + .AddField(x => x.IsGift, field => + { + field.WithLabel("Gift"); + configure?.Invoke(field); + })); + configureCollection?.Invoke(collection); + }) .Build(); /// @@ -193,18 +251,28 @@ internal static IFormConfiguration BooleanItemForm( /// MudNumericField<int>, so a suite that asserts on one has said nothing about the /// other; culture-sensitive parsing in particular only shows up on the decimal one. /// + /// Configures the item's field. Runs after its default label, so it can override it. + /// + /// Configures the collection rather than a field; runs last, after the label and item + /// form. See for the full contract (#300). + /// internal static IFormConfiguration DecimalItemForm( - Action>? configure = null) => + Action>? configure = null, + Action>? configureCollection = null) => FormBuilder .Create() - .AddCollectionField(x => x.Lines, collection => collection - .WithLabel("Lines") - .WithItemForm(item => item - .AddField(x => x.Price, field => - { - field.WithLabel("Price"); - configure?.Invoke(field); - }))) + .AddCollectionField(x => x.Lines, collection => + { + collection + .WithLabel("Lines") + .WithItemForm(item => item + .AddField(x => x.Price, field => + { + field.WithLabel("Price"); + configure?.Invoke(field); + })); + configureCollection?.Invoke(collection); + }) .Build(); /// @@ -260,11 +328,12 @@ internal static MixedItemModel NewMixedItems(params MixedItem[] rows) /// those two; the others still render, which is what makes the row mixed. /// /// - /// is the one callback here that reaches the - /// collection rather than a field. The reorder test needs .AllowReorder(), which is - /// a property of the collection, and without it that suite would have to hand-roll the whole - /// configuration — and would keep its own model copy along with it, which is the duplication - /// this member exists to remove. + /// is the callback that reaches the collection + /// rather than a field: the reorder test needs .AllowReorder(), a property of the + /// collection, and without it that suite would have to hand-roll the whole configuration — and + /// would keep its own model copy along with it, the duplication this member exists to remove. + /// This member had it first (#282); #300 gave every other builder here the same parameter, so it + /// is no longer special — see the class summary for the ordering contract it shares with them. /// /// internal static IFormConfiguration MultiFieldItemForm( @@ -391,9 +460,17 @@ internal static IFormConfiguration TwoCollectionItemForm( /// labels would buy nothing and would leave a caller unable to tell the root field from the item /// field in a rendered form. /// + /// Configures the top-level field. Runs after its default label. + /// Configures the item's field. Runs after its default label. + /// + /// Configures the collection rather than either field; appended last so every existing + /// call site, positional or named, still compiles. See for the full + /// contract (#300). + /// internal static IFormConfiguration RootFieldAndItemForm( Action>? configureRoot = null, - Action>? configureItem = null) => + Action>? configureItem = null, + Action>? configureCollection = null) => FormBuilder .Create() .AddField(x => x.Name, field => @@ -401,14 +478,18 @@ internal static IFormConfiguration RootFieldAndItemForm( field.WithLabel("Name"); configureRoot?.Invoke(field); }) - .AddCollectionField(x => x.Items, collection => collection - .WithLabel("Items") - .WithItemForm(item => item - .AddField(x => x.Name, field => - { - field.WithLabel("Item name"); - configureItem?.Invoke(field); - }))) + .AddCollectionField(x => x.Items, collection => + { + collection + .WithLabel("Items") + .WithItemForm(item => item + .AddField(x => x.Name, field => + { + field.WithLabel("Item name"); + configureItem?.Invoke(field); + })); + configureCollection?.Invoke(collection); + }) .Build(); } diff --git a/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixtureTests.cs b/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixtureTests.cs index 97e1ac8f..cd82c2f3 100644 --- a/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixtureTests.cs +++ b/FormCraft.ForMudBlazor.UnitTests/Fields/CollectionItemFixtureTests.cs @@ -482,4 +482,190 @@ public void NewAppointment_Should_Default_Blank_And_Honour_Its_Seed() blank.Slots[0].When.ShouldBe(default); seeded.Slots[0].When.ShouldBe(new DateTime(2030, 12, 31)); } + + [Fact] + public void Each_Item_Form_Should_Apply_The_Callers_Collection_Configuration() + { + // Arrange & Act - AllowReorder belongs to the COLLECTION, not to a field, so no field + // callback can reach it. Until #300 only MultiFieldItemForm took a collection callback, and + // any suite needing one on a single-field item form had to hand-roll the whole + // configuration — keeping its own model copy with it, which is the duplication this fixture + // exists to remove. + // + // Every model below carries TWO rows deliberately. CollectionFieldComponent renders Move up + // with Disabled="@(index == 0)", so a one-row collection renders the control permanently + // inert: a mere "the button exists" assertion would hold for a form on which reordering + // cannot actually happen. + var text = this.RenderItemForm( + CollectionItemFixture.NewOrderWithItems("A", "B"), + CollectionItemFixture.TextItemForm( + configureCollection: collection => collection.AllowReorder())); + var numeric = this.RenderItemForm( + new BasketModel { Lines = { new BasketLine(), new BasketLine() } }, + CollectionItemFixture.NumericItemForm( + configureCollection: collection => collection.AllowReorder())); + var date = this.RenderItemForm( + new AppointmentModel { Slots = { new AppointmentSlot(), new AppointmentSlot() } }, + CollectionItemFixture.DateItemForm( + configureCollection: collection => collection.AllowReorder())); + var boolean = this.RenderItemForm( + new BasketModel { Lines = { new BasketLine(), new BasketLine() } }, + CollectionItemFixture.BooleanItemForm( + configureCollection: collection => collection.AllowReorder())); + var dec = this.RenderItemForm( + new PricedBasketModel { Lines = { new PricedLine(), new PricedLine() } }, + CollectionItemFixture.DecimalItemForm( + configureCollection: collection => collection.AllowReorder())); + + // ...and each form WITHOUT the callback, the half that makes this a test of the callback + // rather than of MudBlazor. Asserting only that the controls appear would stay green if + // reordering ever became the default, or if a regression rendered them unconditionally — + // proving nothing about the parameter the test is named for. + // + // Numeric and Boolean are both here even though they configure the SAME collection + // (BasketModel.Lines): they are separate builders wiring separate callbacks, so one passing + // says nothing about the other. + var plainText = this.RenderItemForm( + CollectionItemFixture.NewOrderWithItems("A", "B"), + CollectionItemFixture.TextItemForm()); + var plainNumeric = this.RenderItemForm( + new BasketModel { Lines = { new BasketLine(), new BasketLine() } }, + CollectionItemFixture.NumericItemForm()); + var plainDate = this.RenderItemForm( + new AppointmentModel { Slots = { new AppointmentSlot(), new AppointmentSlot() } }, + CollectionItemFixture.DateItemForm()); + var plainBoolean = this.RenderItemForm( + new BasketModel { Lines = { new BasketLine(), new BasketLine() } }, + CollectionItemFixture.BooleanItemForm()); + var plainDecimal = this.RenderItemForm( + new PricedBasketModel { Lines = { new PricedLine(), new PricedLine() } }, + CollectionItemFixture.DecimalItemForm()); + + // Assert - usable reorder controls, and only where the caller asked for them + ShouldOfferReordering(text); + ShouldOfferReordering(numeric); + ShouldOfferReordering(date); + ShouldOfferReordering(boolean); + ShouldOfferReordering(dec); + + ShouldNotOfferReordering(plainText); + ShouldNotOfferReordering(plainNumeric); + ShouldNotOfferReordering(plainDate); + ShouldNotOfferReordering(plainBoolean); + ShouldNotOfferReordering(plainDecimal); + } + + [Fact] + public void Each_Item_Forms_Collection_Callback_Should_Run_After_The_Fixtures_Own_Configuration() + { + // Arrange & Act - the invoke ORDER is half the contract, and AllowReorder cannot pin it: it + // is an independent setter that reads the same whether the caller's callback runs before or + // after the fixture's own WithLabel/WithItemForm. Overriding the label can only succeed if + // the caller runs LAST — the same property Each_Item_Form_Should_Apply_The_Callers_ + // Configuration pins for the field callback ("the callback runs after the default label, so + // it can override it"). Without this test all six builders could invoke the callback first + // and every other assertion here would stay green. + var forms = new IRenderedComponent[] + { + this.RenderItemForm( + CollectionItemFixture.NewOrder(), + CollectionItemFixture.TextItemForm( + configureCollection: collection => collection.WithLabel("Renamed"))), + this.RenderItemForm( + CollectionItemFixture.NewBasket(), + CollectionItemFixture.NumericItemForm( + configureCollection: collection => collection.WithLabel("Renamed"))), + this.RenderItemForm( + CollectionItemFixture.NewAppointment(), + CollectionItemFixture.DateItemForm( + configureCollection: collection => collection.WithLabel("Renamed"))), + this.RenderItemForm( + CollectionItemFixture.NewBasket(), + CollectionItemFixture.BooleanItemForm( + configureCollection: collection => collection.WithLabel("Renamed"))), + this.RenderItemForm( + CollectionItemFixture.NewPricedBasket(), + CollectionItemFixture.DecimalItemForm( + configureCollection: collection => collection.WithLabel("Renamed"))), + this.RenderItemForm( + CollectionItemFixture.NewNamedOrder(), + CollectionItemFixture.RootFieldAndItemForm( + configureCollection: collection => collection.WithLabel("Renamed"))), + + // MultiFieldItemForm has carried the parameter since #282; asserting it here is what + // makes the ordering uniform across all SEVEN builders rather than the six #300 touched. + this.RenderItemForm( + CollectionItemFixture.NewMixedItems(new MixedItem()), + CollectionItemFixture.MultiFieldItemForm( + configureCollection: collection => collection.WithLabel("Renamed"))), + }; + + // Assert - the caller's label won, so its callback ran after the fixture set its own. + // The collection's label renders as the header MudText (Typo.h6). + foreach (var form in forms) + { + form.Find("h6").TextContent.Trim().ShouldBe("Renamed"); + } + } + + [Fact] + public void RootFieldAndItemForm_Should_Apply_The_Callers_Collection_Configuration() + { + // Arrange & Act - three callbacks now, two of which target fields and one the collection. + // A mis-wire is easy in that shape, so this asserts the new one reaches the collection AND + // that the two existing ones still reach their own fields — a builder that routed the root + // callback into the collection would otherwise pass a test that only looked at reordering. + var reorderable = this.RenderItemForm( + new NamedOrderModel { Items = { new NamedOrderItem(), new NamedOrderItem() } }, + CollectionItemFixture.RootFieldAndItemForm( + root => root.WithLabel("Customer name"), + item => item.WithLabel("Product name"), + collection => collection.AllowReorder())); + + // ...and the same form WITHOUT the collection callback: the negative half, which is what + // makes this a test of the parameter rather than of MudBlazor's defaults. + var plain = this.RenderItemForm( + new NamedOrderModel { Items = { new NamedOrderItem(), new NamedOrderItem() } }, + CollectionItemFixture.RootFieldAndItemForm()); + + // Assert - the collection callback reaches the collection... + ShouldOfferReordering(reorderable); + ShouldNotOfferReordering(plain); + + // ...and adding a third callback did not re-route the two that were already there + var fields = reorderable.FindComponents>(); + fields[0].Instance.Label.ShouldBe("Customer name"); + fields[1].Instance.Label.ShouldBe("Product name"); + } + + /// + /// Asserts that a rendered collection offers usable reorder controls — the observable + /// effect of AllowReorder(), and therefore the proof that a builder's collection callback + /// reached the collection. + /// + /// + /// The enabled-ness check is the point. CollectionFieldComponent disables Move up on row + /// 0 and Move down on the last row, so on a one-row collection both controls render and both are + /// permanently inert — "a Move up button exists" is true of a form that cannot be reordered. + /// Callers therefore pass two-row models, and this asserts one control per row with the second + /// one live. + /// + private static void ShouldOfferReordering( + IRenderedComponent> component) + where TModel : new() + { + var moveUp = component.FindAll("button[aria-label='Move up']"); + moveUp.Count.ShouldBe(2); + moveUp[0].HasAttribute("disabled").ShouldBeTrue(); + moveUp[1].HasAttribute("disabled").ShouldBeFalse(); + } + + /// + /// The negative control for : no callback, so the + /// collection never allows reordering and the controls are absent rather than merely disabled. + /// + private static void ShouldNotOfferReordering( + IRenderedComponent> component) + where TModel : new() + => component.FindAll("button[aria-label='Move up']").ShouldBeEmpty(); }