From 822b92c1837adeab303ceda520ece8af976b1ab2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Filip=20Ma=C5=88as?= Date: Fri, 7 Aug 2026 03:41:46 +0000 Subject: [PATCH] fix(forms): follow a renamed datasource into the design and the join (#164) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `eval` CI job has been red since #167 merged: `eval coverage --check` reported COVERAGE_DRIFT because #167 added the `form-clone` capability without regenerating eval/COVERAGE.md. (#165's red build-test and knowledge-audit were an Actions outage — "Failed to resolve action download info" — not code, and are already green on main.) Writing the missing eval case surfaced a real defect in the capability the stale row was hiding. `--rebind` renamed the datasource and repointed the control-level nodes, but left the design's own and naming the datasource the clone no longer has — a form that compiles and fails at runtime. The cause is that those two are first-level children of , so they are written as , and the pattern was anchored on the bare ""; the control-level ones nest under , inherit the namespace, appear bare, and so were the only ones matched. was not handled at all, so a joined datasource kept pointing at its parent's old name. The element list is grounded on the live installation rather than guessed: across 300 shipped ApplicationSuite forms, DataSource, TitleDataSource, WorkflowDataSource, PresenceDataSource and JoinSource are the only elements whose value is a datasource name — the rest of the *DataSource* family is either a container or holds something else ( DataSourceChangeGroupMode is an enum, DataSourceRelation names a relation). Matching now tolerates attributes and backreferences the closing tag, so it cannot bleed from DataSource into DataSourceLinks. The unit fixture's went straight to and so never had the nodes that break — it now carries the design properties and a joined datasource, which is what makes the four new tests fail before the fix. L2-form-clone-basic clones the two-datasource details-transaction form: 19 KB in, and the clone differs on exactly the root and the class declaration and is byte-identical everywhere else, which is the guarantee worth locking for string surgery over a document this code does not own. It is a plain clone rather than a rebind because MiniAot has only FmVehicle and FmVehicleLine: rebinding the simple form onto FmVehicleLine binds a Make that table does not have, and the richer form already uses both, so either golden would enshrine a form nobody should ship. The rebind path is covered by the unit tests instead. A fixture table would let it graduate to an eval case; left as follow-up. Verified locally against all four CI jobs: build clean, 1178 tests pass, knowledge audit clean, 52/52 eval cases with `coverage --check` green, and no skills drift. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SuTvn5ymx3HAUtGjsxBuaA --- eval/COVERAGE.md | 6 +- eval/cases/L2-form-clone-basic.json | 13 + .../ConFmVehicleServiceOrderInquiry.xml | 411 ++++++++++++++++++ src/D365FO.Core/FormPatterns/FormCloner.cs | 46 +- .../Eval/EvalListCommandTests.cs | 3 +- .../Eval/EvalCaseCatalogTests.cs | 2 +- tests/D365FO.Core.Tests/FormClonerTests.cs | 67 ++- 7 files changed, 541 insertions(+), 7 deletions(-) create mode 100644 eval/cases/L2-form-clone-basic.json create mode 100644 eval/goldens/L2-form-clone-basic/ConFmVehicleServiceOrderInquiry.xml diff --git a/eval/COVERAGE.md b/eval/COVERAGE.md index 13c0e0c..4057c70 100644 --- a/eval/COVERAGE.md +++ b/eval/COVERAGE.md @@ -14,7 +14,7 @@ Every column is derived — families from `ObjectTypeRegistry`, capabilities fro file cannot claim coverage that no longer exists. Regenerate with `d365fo eval coverage --write`; CI runs `--check`. -**42 of 70 leaves complete.** +**42 of 71 leaves complete.** ## AOT families @@ -30,7 +30,7 @@ file cannot claim coverage that no longer exists. Regenerate with | `AxEdtExtension` | edtextension | — | ✅ | ✅ | — | `L2-edt-extension` | generate extension | | `AxEnum` | enum | ✅ | ✅ | ✅ | `event-handler-authoring`, `object-extension-authoring` | `L0-enum-basic`, `L0-enum-non-extensible` | generate enum | | `AxEnumExtension` | enumextension | — | ✅ | ✅ | — | `L2-enum-extension` | generate extension | -| `AxForm` | form | ✅ | ✅ | ✅ | `coc-extension-authoring`, `data-entity-scaffolding`, `event-handler-authoring`, `form-pattern-scaffolding`, `label-translation`, `model-dependency-and-coupling`, `object-extension-authoring`, `review-and-checkpoint-workflow`, `security-hierarchy-trace`, `table-scaffolding`, `x++-class-authoring`, `xpp-best-practice-rules`, `xpp-class-and-method-rules`, `xpp-database-queries`, `xpp-statement-and-type-rules` | `L1-form-basic`, `L1-form-details-master`, `L1-form-details-transaction`, `L1-form-dialog`, `L1-form-list-page`, `L1-form-lookup`, `L1-form-simplelist-details`, `L1-form-table-of-contents`, `L1-form-workspace`, `L2-control-method-basic`, `L2-datasource-method-basic` | generate form | +| `AxForm` | form | ✅ | ✅ | ✅ | `coc-extension-authoring`, `data-entity-scaffolding`, `event-handler-authoring`, `form-pattern-scaffolding`, `label-translation`, `model-dependency-and-coupling`, `object-extension-authoring`, `review-and-checkpoint-workflow`, `security-hierarchy-trace`, `table-scaffolding`, `x++-class-authoring`, `xpp-best-practice-rules`, `xpp-class-and-method-rules`, `xpp-database-queries`, `xpp-statement-and-type-rules` | `L1-form-basic`, `L1-form-details-master`, `L1-form-details-transaction`, `L1-form-dialog`, `L1-form-list-page`, `L1-form-lookup`, `L1-form-simplelist-details`, `L1-form-table-of-contents`, `L1-form-workspace`, `L2-control-method-basic`, `L2-datasource-method-basic`, `L2-form-clone-basic` | generate form | | `AxFormExtension` | formextension | ✅ | ✅ | ✅ | `form-pattern-scaffolding`, `review-and-checkpoint-workflow` | `L2-form-extension-basic` | generate extension | | `AxFormPart` | formpart | — | — | — | — | — | — | | `AxLabelFile` | labelfile | ✅ | — | — | `label-translation` | — | — | @@ -75,6 +75,7 @@ whether it is taught and proven. | `generate form` | AxForm | ✅ | ✅ | `form-pattern-scaffolding` | `L1-form-basic`, `L1-form-details-master`, `L1-form-details-transaction`, `L1-form-dialog`, `L1-form-list-page`, `L1-form-lookup`, `L1-form-simplelist-details`, `L1-form-table-of-contents`, `L1-form-workspace` | | `generate datasource-method` | AxForm | — | ✅ | — | `L2-datasource-method-basic` | | `generate control-method` | AxForm | — | ✅ | — | `L2-control-method-basic` | +| `generate form-clone` | AxForm | — | ✅ | — | `L2-form-clone-basic` | | `generate simple-list` | deprecated alias | — | — | — | — | | `generate entity` | AxDataEntityView | ✅ | ✅ | `data-entity-scaffolding`, `integration-dmf-dualwrite`, `integration-patterns` | `L2-virtual-entity-basic` | | `generate extension` | AxTableExtension, AxFormExtension, AxEdtExtension, AxEnumExtension, AxViewExtension, AxQuerySimpleExtension, AxDataEntityViewExtension, AxSecurityDutyExtension, AxSecurityRoleExtension | ✅ | ✅ | `forms-and-navigation`, `object-extension-authoring`, `xpp-best-practice-rules` | `L2-edt-extension`, `L2-enum-extension`, `L2-form-extension-basic`, `L2-security-duty-extension`, `L2-security-role-extension`, `L2-table-extension` | @@ -106,6 +107,7 @@ cannot build at all are omitted — they are a generation gap, not a coverage ga - **control-method** (capability) — no topic names it - **datasource-method** (capability) — no topic names it +- **form-clone** (capability) — no topic names it - **map** (capability) — no topic names it - **simple-list** (capability) — no topic names it; no case with a reviewed golden - **systest** (capability) — no topic names it diff --git a/eval/cases/L2-form-clone-basic.json b/eval/cases/L2-form-clone-basic.json new file mode 100644 index 0000000..e4789c2 --- /dev/null +++ b/eval/cases/L2-form-clone-basic.json @@ -0,0 +1,13 @@ +{ + "id": "L2-form-clone-basic", + "title": "Clone a two-datasource details-transaction form under a new name", + "tier": 2, + "instruction": "Clone the existing form eval/goldens/L1-form-details-transaction/ConFmVehicleServiceOrder.xml as ConFmVehicleServiceOrderInquiry. The clone keeps the original's pattern, both datasources and their join, and its whole control tree — only the form's own self-references (root name, class declaration, formStr) take the new name. Use only the d365fo CLI (search/get/generate/validate) — do not hand-write XML.", + "canonical_args": ["generate", "form-clone", "ConFmVehicleServiceOrderInquiry", "--from", "eval/goldens/L1-form-details-transaction/ConFmVehicleServiceOrder.xml"], + "target_artifact_types": ["AxForm"], + "golden_path": "L2-form-clone-basic", + "tags": ["form-clone", "form", "metadata"], + "ignore": [], + "requires_fixture_index": false, + "golden_pending": false +} diff --git a/eval/goldens/L2-form-clone-basic/ConFmVehicleServiceOrderInquiry.xml b/eval/goldens/L2-form-clone-basic/ConFmVehicleServiceOrderInquiry.xml new file mode 100644 index 0000000..5bfe728 --- /dev/null +++ b/eval/goldens/L2-form-clone-basic/ConFmVehicleServiceOrderInquiry.xml @@ -0,0 +1,411 @@ + + + + ConFmVehicleServiceOrderInquiry + + + + classDeclaration + + + + + + + + + + FmVehicle + FmVehicle
+ + + No + + + + + + + + + + + + + + FmVehicleLineFmVehicleLine
FmVehicleActiveNo + + + + Vertical + Vehicle service order + 1 + Fixed + FmVehicle + DetailsTransaction + 1.4 + + FmVehicle + + + ActionPane + 134217727 + %1 + ActionPane + -1 + + + + ButtonGroup + ButtonGroup + + + Vertical + + + No + No + Vertical + + + + NavigationList + -1 + SizeToAvailable + Group + No + -1 + SizeToContent + + + + + NavigationListQuickFilter + SizeToAvailable + + QuickFilterControl + + + + targetControlName + String + NavigationListGrid + + + + + + NavigationListGrid + No + -1 + SizeToAvailable + Grid + -1 + SizeToContent + + + FmVehicle + No + No + + + + Vertical + 1 + Fixed + None + + + + PanelTab + -1 + SizeToAvailable + Tab + -1 + SizeToAvailable + + + + DetailsPanel + TabPage + + + + TitleGroup + -1 + SizeToContent + Group + -1 + SizeToAvailable + + + + HeaderTitle + Yes + String + SizeToAvailable + + VIN + FmVehicle + No + + + + HorizontalLeft + 1 + Fixed + FmVehicle + None + + + + HeaderLinePanels + -1 + SizeToAvailable + Tab + -1 + SizeToAvailable + + + + LinesPanel + TabPage + + + + LineViewTab + -1 + SizeToAvailable + Tab + -1 + SizeToAvailable + + + + LineViewHeader + TabPage + + + Yes + Auto + + + + + + + + + + + + LineViewLines-1SizeToAvailableTabPage + + LineViewLinesToolbar + -1 + SizeToContent + ActionPane + -1 + SizeToAvailable + + + + + + LineViewLinesGrid + -1 + SizeToAvailable + Grid + -1 + SizeToAvailable + + + Overview + FmVehicleLine + + 5 + Fixed + + YesAlwaysAuto + + LineViewLineDetails + TabPage + + + + LineDetailsTab + -1 + SizeToAvailable + Tab + -1 + SizeToAvailable + + + + + + + + + + LineDetailTabPageTabPageLine detailsAuto + + Yes + + + + Yes + Auto + + + + + + Vertical + 1 + Fixed + Yes + DetailsLine + + + HeaderPanel + TabPage + + + + HeaderDetailsTab + -1 + SizeToAvailable + Tab + -1 + SizeToAvailable + + + + + + + + + + HeaderGeneralPageTabPage + + HeaderGeneralGroup + Group + + + + Header_VIN + String + + VIN + FmVehicle + + + + Overview + FmVehicle + + GeneralAuto + + + + + 1 + Fixed + Yes + DetailsHeader + + + No + + + + Vertical + 1 + Fixed + Details + + + + GridPanel + TabPage + + + + CustomAndQuickFilterGroup + CustomAndQuickFilters + 1.1 + Group + SizeToAvailable + + + + QuickFilterControl + SizeToAvailable + + QuickFilterControl + + + + targetControlName + String + MainGrid + + + + + + HorizontalLeft + None + + Edit + + + MainGrid + Yes + -1 + SizeToAvailable + Grid + -1 + SizeToAvailable + + + FmVehicle + MainGridDefaultAction + + + + MainGridDefaultAction + CommandButton + No + + DetailsView + + + Vertical + 1 + Fixed + Grid + + + + No + + + + + + diff --git a/src/D365FO.Core/FormPatterns/FormCloner.cs b/src/D365FO.Core/FormPatterns/FormCloner.cs index 5114124..2527eef 100644 --- a/src/D365FO.Core/FormPatterns/FormCloner.cs +++ b/src/D365FO.Core/FormPatterns/FormCloner.cs @@ -141,12 +141,12 @@ private static (List Rebound, List Renamed) RebindTables( xml = Regex.Replace(xml, tableTag, $"{newTable}
"); rebound.Add($"{oldTable} -> {newTable}"); - // The datasource element that carried the table, and every control pointing at it. + // The datasource element that carried the table, and everything pointing at it. var dsName = $"{Regex.Escape(oldTable)}"; if (Regex.IsMatch(xml, dsName)) { xml = Regex.Replace(xml, dsName, $"{newTable}"); - xml = Regex.Replace(xml, $"{Regex.Escape(oldTable)}", $"{newTable}"); + xml = RenameDataSourceReferences(xml, oldTable, newTable); renamed.Add($"{oldTable} -> {newTable}"); } @@ -159,6 +159,48 @@ private static (List Rebound, List Renamed) RebindTables( return (rebound, renamed); } + /// + /// Elements whose text is the name of a datasource, so a renamed datasource has to be + /// followed into all of them. + /// + /// + /// Grounded on a live installation rather than guessed: across 300 shipped + /// ApplicationSuite\Foundation\AxForm forms these are the only elements carrying a + /// datasource name as their value. The rest of the *DataSource* family is either a + /// container (DataSources, DataSourceLinks, ReferencedDataSources) or + /// holds something else entirely (DataSourceChangeGroupMode is an enum, + /// DataSourceRelation names a relation). Datasource names reached through a nested + /// <Name> — root links, referenced and derived datasources — are already covered + /// by the <Name> rewrite above. + /// + private static readonly string[] DataSourceRefElements = + ["DataSource", "TitleDataSource", "WorkflowDataSource", "PresenceDataSource", "JoinSource"]; + + /// + /// Repoint every reference to a renamed datasource, whatever attributes the tag carries. + /// + /// + /// The attribute tolerance is the whole point. A <Design>'s own + /// DataSource/TitleDataSource are first-level children written in the empty + /// namespace, so they appear as <DataSource xmlns="">, while the control-level + /// ones nested under <Controls xmlns=""> inherit it and appear bare. Matching only + /// the bare form rebinds the controls and leaves the design pointing at a datasource the + /// clone no longer has. + /// + /// The alternation cannot bleed into a longer tag: the group must be followed immediately by + /// whitespace or >, so DataSource never matches the start of + /// DataSourceLinks. The closing tag is a backreference, so the two always agree. + /// + /// + private static string RenameDataSourceReferences(string xml, string oldName, string newName) + { + var tags = string.Join('|', DataSourceRefElements); + return Regex.Replace( + xml, + $@"<({tags})((?:\s[^>]*)?)>{Regex.Escape(oldName)}", + m => $"<{m.Groups[1].Value}{m.Groups[2].Value}>{newName}"); + } + /// /// The clone has to still be a parseable form carrying its new name. /// diff --git a/tests/D365FO.Cli.Tests/Eval/EvalListCommandTests.cs b/tests/D365FO.Cli.Tests/Eval/EvalListCommandTests.cs index c7a6d78..1e4707b 100644 --- a/tests/D365FO.Cli.Tests/Eval/EvalListCommandTests.cs +++ b/tests/D365FO.Cli.Tests/Eval/EvalListCommandTests.cs @@ -33,9 +33,10 @@ public void Lists_every_authored_case() Assert.Equal(0, exit); Assert.Contains("\"ok\":true", stdout); - Assert.Contains("\"count\":51", stdout); + Assert.Contains("\"count\":52", stdout); Assert.Contains("L0-edt-basic", stdout); Assert.Contains("L2-coc-extension", stdout); Assert.Contains("L1-form-workspace", stdout); + Assert.Contains("L2-form-clone-basic", stdout); } } diff --git a/tests/D365FO.Core.Tests/Eval/EvalCaseCatalogTests.cs b/tests/D365FO.Core.Tests/Eval/EvalCaseCatalogTests.cs index e71c1a9..8e38630 100644 --- a/tests/D365FO.Core.Tests/Eval/EvalCaseCatalogTests.cs +++ b/tests/D365FO.Core.Tests/Eval/EvalCaseCatalogTests.cs @@ -14,7 +14,7 @@ public void Loads_the_real_authored_catalog_with_no_errors() var (cases, errors) = EvalCaseCatalog.LoadAll(EvalPaths.CasesDir(RepoRoot)); Assert.Empty(errors); - Assert.Equal(51, cases.Count); + Assert.Equal(52, cases.Count); Assert.Contains(cases, c => c.Id == "L0-edt-basic"); Assert.Contains(cases, c => c.Id == "L0-enum-basic"); Assert.Contains(cases, c => c.Id == "L1-table-basic"); diff --git a/tests/D365FO.Core.Tests/FormClonerTests.cs b/tests/D365FO.Core.Tests/FormClonerTests.cs index 5ae6dbf..5f8ceab 100644 --- a/tests/D365FO.Core.Tests/FormClonerTests.cs +++ b/tests/D365FO.Core.Tests/FormClonerTests.cs @@ -11,7 +11,10 @@ namespace D365FO.Core.Tests; /// /// The fixture mirrors the shape of a real shipped form: the root name, the X++ class /// declaration, a datasource entry under <SourceCode> (where override methods live), -/// the design datasource, and a control pointing at it by name. Verified against +/// the design datasource, the design's own DataSource/TitleDataSource properties +/// (first-level children of <Design>, so they carry an explicit xmlns="" where +/// the control-level ones inherit it), and a control pointing at the datasource by name. Verified +/// against /// ApplicationSuite\Foundation\AxForm\CustGroup.xml on a live installation — a 16 KB form /// where the clone differs from the source on exactly the intended lines and is byte-identical /// everywhere else. @@ -41,8 +44,15 @@ public class FormClonerTests CustGroup CustGroup
+ + CustTrans + CustTrans
+ CustGroup + + CustGroup + CustGroup Grid_CustGroupId CustGroupId @@ -89,6 +99,61 @@ public void A_rebind_moves_the_table_the_datasource_and_every_control_that_names Assert.Single(result.RenamedDataSources); } + [Fact] + public void A_rebind_follows_the_datasource_into_the_design_properties_that_name_it() + { + // The design's own DataSource/TitleDataSource are first-level children of , so + // they are written as . A pattern anchored on the bare "" + // misses them, and the clone then names a datasource it no longer has — a form that + // compiles and breaks at runtime. Grounded on a live installation: of 300 shipped + // ApplicationSuite forms, 128 and 78 nodes carry attributes. + var result = FormCloner.Clone(Source, "ConVehicleGroup", Rebind("CustGroup", "ConVehicleGroupTable")); + + Assert.Contains("ConVehicleGroupTable", result.Xml); + Assert.Contains("ConVehicleGroupTable", result.Xml); + Assert.DoesNotContain("CustGroup", result.Xml); + Assert.DoesNotContain("CustGroup", result.Xml); + } + + [Fact] + public void A_rebind_follows_the_datasource_into_the_join_that_names_it() + { + // A joined datasource names its parent by datasource name, so renaming the parent has to + // reach the join too — otherwise the child joins to something that no longer exists. + var result = FormCloner.Clone(Source, "ConVehicleGroup", Rebind("CustGroup", "ConVehicleGroupTable")); + + Assert.Contains("ConVehicleGroupTable", result.Xml); + } + + [Fact] + public void A_tag_that_merely_starts_with_a_matched_element_name_is_not_rewritten() + { + // The alternation must not let "DataSource" bleed into "DataSourceLinks" and friends. + var withLinks = Source.Replace( + "CustGroup", + "CustGroupCustGroup"); + + var result = FormCloner.Clone(withLinks, "ConVehicleGroup", Rebind("CustGroup", "ConVehicleGroupTable")); + + Assert.Contains("CustGroup", result.Xml); + } + + [Fact] + public void A_rebind_leaves_no_control_or_design_node_naming_the_old_datasource() + { + // The whole-document guarantee behind the case above: after a rebind that renamed the + // datasource, nothing anywhere still refers to the old name. + var result = FormCloner.Clone(Source, "ConVehicleGroup", Rebind("CustGroup", "ConVehicleGroupTable")); + + var stillNamingOld = XDocument.Parse(result.Xml) + .Descendants() + .Where(e => e.Value.Trim() == "CustGroup" && !e.HasElements) + .Select(e => e.Name.LocalName) + .ToList(); + + Assert.Empty(stillNamingOld); + } + [Fact] public void The_form_is_renamed_before_the_rebind_so_the_root_never_takes_the_table_name() {