From 63d01cb4615e5e49844f43226f070d2a769aa780 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 22:18:57 +0000 Subject: [PATCH 01/14] =?UTF-8?q?fix:=20a=20read-only=20data=20view=20no?= =?UTF-8?q?=20longer=20becomes=20editable=20on=20describe=20=E2=86=92=20ex?= =?UTF-8?q?ec?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A data view stores its editability as `Editability` (Mendix 10.7+), where input widgets store `Editable`. Three layers lost it: - the writer took a data view's Editability from DataView.ReadOnly, a field nothing sets, so `Editable: Never` (and a condition) was written as Always; - describe's DataView case read neither key, so a stored Never printed as nothing; - `alter page … set (Editable: …)` only knew the pre-10.7 boolean and refused every current data view. Fixes ako/mxcli#1676 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DRueZhZoBML5iWuJTvUnjL --- ...24-dataview-editability-never-dropped.json | 1 + .../1676-dataview-editable-never.mdl | 44 +++++++++++ mdl/backend/modelsdk/widget_write.go | 14 ++-- ...et_write_dataview_editability_1676_test.go | 49 ++++++++++++ mdl/backend/pagemutator/editable_test.go | 43 +++++++++++ mdl/backend/pagemutator/mutator.go | 28 +++++-- ...describe_dataview_editability_1676_test.go | 74 +++++++++++++++++++ mdl/executor/cmd_pages_describe_parse.go | 4 + 8 files changed, 246 insertions(+), 11 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-24-dataview-editability-never-dropped.json create mode 100644 mdl-examples/bug-tests/1676-dataview-editable-never.mdl create mode 100644 mdl/backend/modelsdk/widget_write_dataview_editability_1676_test.go create mode 100644 mdl/executor/cmd_pages_describe_dataview_editability_1676_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-24-dataview-editability-never-dropped.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-24-dataview-editability-never-dropped.json new file mode 100644 index 0000000000..b8f05bfe10 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-24-dataview-editability-never-dropped.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-10-24", "symptom": "describe → exec turned a read-only data view editable: `describe page` printed `dataview dataView1 (DataSource: microflow …, ReadOnlyStyle: Text)` without Editable, and `Editable: Never` written by hand was ignored (exec: \"Unchanged page\", stored Forms$DataView.Editability stayed Always); `alter page … set (Editable: Never) on dataView1` was refused as \"a DataView has no Editable property\"", "cause": "A data view stores the enum under `Editability` (Mendix 10.7+), where input widgets store `Editable`. The page builder put the authored value on the BaseWidget like every widget, but the writer took the data view's Editability from DataView.ReadOnly, a field nothing sets; the describe parser's DataView case read neither key; the page mutator only knew the pre-10.7 boolean `Editable`", "file": "mdl/backend/modelsdk/widget_write.go (dataViewEditability); mdl/executor/cmd_pages_describe_parse.go (Forms$DataView case); mdl/backend/pagemutator/mutator.go (editabilityKey)", "fix": "Write pages.WidgetEditability(&x.BaseWidget) for a data view (so Conditional is written beside its settings element too); describe reads `Editability`; the mutator writes `Editability` when the widget stores it", "insight": "The builder was right all along, which is why `check` and the builder-level tests were green: the value was dropped one layer down by a writer reading a dead field. Grep the semantic struct's field for writers (`\\.ReadOnly =`): none in the tree is the tell. The key name is version-dependent — Model SDK version.go has Forms$DataView `editable` Deleted 10.7.0 and `editability` Introduced 10.7.0, and the mutator's own test faked a DataView with a boolean Editable, so it asserted the pre-10.7 shape only. Survey a Studio Pro fixture before trusting a test's hand-built document: PedApp has 41 of 41 data views with Editability and none with Editable", "refs": ["ako/mxcli#1676", "ako/mxcli#1163"], "test": "TestWriteDataView_Editability (mdl/backend/modelsdk/widget_write_dataview_editability_1676_test.go); TestDescribeDataView_EmitsEditability, TestBuildDataView_EditableNeverReachesTheWidget (mdl/executor/cmd_pages_describe_dataview_editability_1676_test.go); TestSetWidgetProperty_Editable_DataViewEditability (mdl/backend/pagemutator/editable_test.go); mdl-examples/bug-tests/1676-dataview-editable-never.mdl"} diff --git a/mdl-examples/bug-tests/1676-dataview-editable-never.mdl b/mdl-examples/bug-tests/1676-dataview-editable-never.mdl new file mode 100644 index 0000000000..e7bd9eb38a --- /dev/null +++ b/mdl-examples/bug-tests/1676-dataview-editable-never.mdl @@ -0,0 +1,44 @@ +mdl 1; +-- ============================================================================ +-- ako/mxcli#1676 — a read-only data view became editable on describe → exec. +-- `describe` printed `dataview dataView1 (DataSource: microflow …, +-- ReadOnlyStyle: Text)` without `Editable`, and `Editable: Never` written by +-- hand was ignored: exec reported "Unchanged page" and the stored +-- `Forms$DataView.Editability` stayed `Always`. +-- +-- A data view stores the enum as `Editability` (Mendix 10.7+); input widgets +-- store `Editable`. The builder put the value on the widget, the writer took it +-- from a field nothing sets, and describe never read the key. `alter page … +-- set (Editable: …) on ` was refused as "a DataView has no Editable +-- property". +-- +-- Measured on 11.13.0, after a BSON edit setting dvReadOnly's Editability to +-- Never as Studio Pro stores it: +-- +-- before the fix: describe prints no Editable; exec → Editability Always +-- after: describe prints `Editable: Never`; exec → "Unchanged +-- page", Editability Never; mx check 0 errors +-- ============================================================================ + +create module BugDataViewEditable; + +create persistent entity BugDataViewEditable.Boat (Name: String(100)); + +create microflow BugDataViewEditable.DS_Boat () returns BugDataViewEditable.Boat as $Boat +begin + $Boat = create BugDataViewEditable.Boat (Name = 'Boat'); + return $Boat; +end; + +create page BugDataViewEditable.HealthPage (Title: 'Health', Layout: Atlas_Core.Atlas_Default) { + dataview dvReadOnly (DataSource: microflow BugDataViewEditable.DS_Boat, ReadOnlyStyle: Text, Editable: Never) { + textbox tbName (Label: 'Name', Attribute: Name) + } + dataview dvConditional (DataSource: microflow BugDataViewEditable.DS_Boat, Editable: $currentObject/Name != 'Locked') { + textbox tbName2 (Label: 'Name', Attribute: Name) + } +}; + +alter page BugDataViewEditable.HealthPage { + set (Editable: Always) on dvReadOnly +}; diff --git a/mdl/backend/modelsdk/widget_write.go b/mdl/backend/modelsdk/widget_write.go index 2fb10492b9..b50c80e9e9 100644 --- a/mdl/backend/modelsdk/widget_write.go +++ b/mdl/backend/modelsdk/widget_write.go @@ -493,7 +493,7 @@ func widgetToGen(w pages.Widget) (element.Element, error) { return nil, err } g.SetDataSource(ds) - g.SetEditability(editability(x.ReadOnly)) + g.SetEditability(dataViewEditability(x)) // Control is the DataView default, measured 47 of 56 across ako/TestApp's // 67 pages with not one Inherit — so it is NOT the "Inherit" every other // input widget uses. An authored or carried value wins (ako/mxcli#550). @@ -1603,12 +1603,16 @@ func widgetValidationToGenWith(expression, message string) element.Element { return v } -// editability maps a read-only flag to the Forms editability enum. -func editability(readOnly bool) string { - if readOnly { +// dataViewEditability is the Forms editability enum a data view stores. +// +// The builder puts `Editable:` on the BaseWidget, as it does for every widget. +// Reading only ReadOnly, which nothing sets, wrote Always whatever was authored, +// so a read-only data view became editable on describe → exec (ako/mxcli#1676). +func dataViewEditability(x *pages.DataView) string { + if x.ReadOnly { return "Never" } - return "Always" + return pages.WidgetEditability(&x.BaseWidget) } // dataViewSourceToGen builds a DataView's data source. The page-context source diff --git a/mdl/backend/modelsdk/widget_write_dataview_editability_1676_test.go b/mdl/backend/modelsdk/widget_write_dataview_editability_1676_test.go new file mode 100644 index 0000000000..3d85976be8 --- /dev/null +++ b/mdl/backend/modelsdk/widget_write_dataview_editability_1676_test.go @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: Apache-2.0 + +// ako/mxcli#1676, the writer half: "a data view's `Editability: Never` becomes +// `Always`, so a read-only data view's inputs become editable", and "adding +// `Editable: Never` by hand is also ignored: `exec` reports 'Unchanged page' and +// the value stays `Always`". +// +// The page builder puts a data view's `Editable:` on the BaseWidget, as it does +// for every widget. The writer took the data view's Editability from +// DataView.ReadOnly instead, a field nothing sets, so it wrote Always whatever +// was authored. +package modelsdkbackend + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/backend/bsonnav" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +func TestWriteDataView_Editability(t *testing.T) { + for _, tc := range []struct { + name string + bw pages.BaseWidget + want string + }{ + // Control: an unauthored data view keeps Mendix's default. + {"unset", pages.BaseWidget{}, "Always"}, + {"never", pages.BaseWidget{Editable: "Never"}, "Never"}, + {"always", pages.BaseWidget{Editable: "Always"}, "Always"}, + // The conditional settings element is what makes it Conditional; writing + // Always beside it contradicts the element. + {"conditional", pages.BaseWidget{ConditionalEditability: &pages.ConditionalEditabilitySettings{ + BaseElement: model.BaseElement{TypeName: "Forms$ConditionalEditabilitySettings"}, + Expression: "$currentObject/Locked = false", + }}, "Conditional"}, + } { + t.Run(tc.name, func(t *testing.T) { + bw := tc.bw + bw.BaseElement = model.BaseElement{ID: "dv"} + bw.Name = "dataView1" + d := encodeWidget(t, &pages.DataView{BaseWidget: bw}) + if got := bsonnav.DGetString(d, "Editability"); got != tc.want { + t.Errorf("Editability = %q, want %q", got, tc.want) + } + }) + } +} diff --git a/mdl/backend/pagemutator/editable_test.go b/mdl/backend/pagemutator/editable_test.go index 3640156aee..dbab42c76f 100644 --- a/mdl/backend/pagemutator/editable_test.go +++ b/mdl/backend/pagemutator/editable_test.go @@ -105,3 +105,46 @@ func TestSetWidgetProperty_Editable_NoSuchProperty(t *testing.T) { t.Fatal("set Editable on a container succeeded; it has no Editable property") } } + +// ako/mxcli#1676: from Mendix 10.7 a data view stores the Always/Never/ +// Conditional enum as `Editability` and has no `Editable` at all (Model SDK: +// Forms$DataView `editable` Deleted 10.7.0, `editability` Introduced 10.7.0; +// all 41 data views in the Studio Pro-authored PedApp fixture). The setter only +// looked at `Editable`, so `set Editable = Never on dataView1` was refused as +// "a data view has no Editable property". +func TestSetWidgetProperty_Editable_DataViewEditability(t *testing.T) { + w := bson.D{ + {Key: "$Type", Value: "Forms$DataView"}, + {Key: "Name", Value: "dataView1"}, + {Key: "Editability", Value: "Always"}, + {Key: "ConditionalEditabilitySettings", Value: nil}, + } + rawData := makeRawPage(w) + m := &Mutator{rawData: rawData, widgetFinder: findBsonWidget} + stored := func() bson.D { return findBsonWidget(rawData, "dataView1").widget } + + if err := m.SetWidgetProperty("dataView1", "Editable", "Never"); err != nil { + t.Fatalf("set Editable = Never on a data view: %v", err) + } + if got := bsonnav.DGet(stored(), "Editability"); got != "Never" { + t.Fatalf("Editability = %#v, want \"Never\"", got) + } + if got := bsonnav.DGet(stored(), "Editable"); got != nil { + t.Fatalf("the setter added an `Editable` key (%#v) that a 10.7+ data view does not declare", got) + } + if err := m.SetWidgetProperty("dataView1", "EditableIf", "$currentObject/Open"); err != nil { + t.Fatalf("set EditableIf: %v", err) + } + if got := bsonnav.DGet(stored(), "Editability"); got != "Conditional" { + t.Fatalf("Editability = %#v after set Editable = [expr], want \"Conditional\"", got) + } + if err := m.SetWidgetProperty("dataView1", "Editable", "Always"); err != nil { + t.Fatalf("set Editable = Always: %v", err) + } + if got := bsonnav.DGet(stored(), "Editability"); got != "Always" { + t.Fatalf("Editability = %#v, want \"Always\"", got) + } + if s := bsonnav.DGet(stored(), "ConditionalEditabilitySettings"); s != nil { + t.Fatalf("ConditionalEditabilitySettings = %#v after set Editable = Always, want null", s) + } +} diff --git a/mdl/backend/pagemutator/mutator.go b/mdl/backend/pagemutator/mutator.go index ac5ed48364..1c8b15ca72 100644 --- a/mdl/backend/pagemutator/mutator.go +++ b/mdl/backend/pagemutator/mutator.go @@ -3078,8 +3078,10 @@ func storedVisibleRoles(widget bson.D) bson.A { // different things under the same key, so the stored value's type decides what // the statement may say: // -// - a list view or data view stores a BOOLEAN — `true` / `false`; -// - an input widget stores the Always / Never / Conditional enum. +// - a list view, or a data view before Mendix 10.7, stores a BOOLEAN — +// `true` / `false`; +// - an input widget stores the Always / Never / Conditional enum, and so does +// a data view from 10.7, under `Editability` (editabilityKey). // // Anything else is refused. Before mendixlabs/mxcli#1214 only a string was // written, so `set Editable = true on lvRows` — the boolean every list view @@ -3087,7 +3089,8 @@ func storedVisibleRoles(widget bson.D) bson.A { // stored, leaving the list's inputs read-only at runtime. func setWidgetEditableMut(widget bson.D, value any) error { typ := bsonnav.DGetString(widget, "$Type") - switch bsonnav.DGet(widget, "Editable").(type) { + key := editabilityKey(widget) + switch bsonnav.DGet(widget, key).(type) { case bool: b, ok := editableBool(value) if !ok { @@ -3107,7 +3110,7 @@ func setWidgetEditableMut(widget bson.D, value any) error { // expression; the bracketed form writes both. return fmt.Errorf("`set Editable = Conditional` needs the condition itself: write `set Editable = [expression]`") } - bsonnav.DSet(widget, "Editable", canon) + bsonnav.DSet(widget, key, canon) // A plain value replaces a stored condition rather than contradicting it. bsonnav.DSet(widget, "ConditionalEditabilitySettings", nil) return nil @@ -3116,6 +3119,18 @@ func setWidgetEditableMut(widget bson.D, value any) error { } } +// editabilityKey is the key a widget stores its editability under. A data view +// moved from a boolean `Editable` to the `Editability` enum in Mendix 10.7 (Model +// SDK: Forms$DataView `editable` Deleted 10.7.0, `editability` Introduced +// 10.7.0); every input widget keeps `Editable`. Reading only `Editable` refused +// `set Editable = Never` on every current data view (ako/mxcli#1676). +func editabilityKey(widget bson.D) string { + if _, ok := bsonnav.DGet(widget, "Editability").(string); ok { + return "Editability" + } + return "Editable" +} + // editableBool reads a list/data view's boolean Editable from MDL, which hands // it over as a bool (or, quoted, as the string "true"/"false"). func editableBool(value any) (bool, bool) { @@ -3241,8 +3256,9 @@ func setRawWidgetPropertyMut(widget bson.D, propName string, value any) error { // The element only applies under the enum that says so — CREATE writes // both (pages.WidgetEditability); writing the element alone left the // stored enum saying Always or Never beside it. - if _, isEnum := bsonnav.DGet(widget, "Editable").(string); isEnum { - bsonnav.DSet(widget, "Editable", "Conditional") + key := editabilityKey(widget) + if _, isEnum := bsonnav.DGet(widget, key).(string); isEnum { + bsonnav.DSet(widget, key, "Conditional") } return nil case "name": diff --git a/mdl/executor/cmd_pages_describe_dataview_editability_1676_test.go b/mdl/executor/cmd_pages_describe_dataview_editability_1676_test.go new file mode 100644 index 0000000000..10e6b3d264 --- /dev/null +++ b/mdl/executor/cmd_pages_describe_dataview_editability_1676_test.go @@ -0,0 +1,74 @@ +// SPDX-License-Identifier: Apache-2.0 + +// ako/mxcli#1676, the describe half: "`describe` prints `dataview dataView1 +// (DataSource: microflow …, ShowFooter: true, ReadOnlyStyle: Text)`, without +// `Editable`". +// +// A data view stores its editability as `Editability`; input widgets store it as +// `Editable`. The data view case never read either, so a read-only data view +// described as an editable one. +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/visitor" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +func describeStoredDataView(t *testing.T, stored map[string]any) string { + t.Helper() + ctx, buf := newMockCtx(t) + for _, rw := range parseRawWidget(ctx, stored) { + outputWidgetMDLV3(ctx, rw, 0) + } + return buf.String() +} + +func TestDescribeDataView_EmitsEditability(t *testing.T) { + out := describeStoredDataView(t, map[string]any{ + "$Type": "Forms$DataView", "Name": "dataView1", + "Editability": "Never", "ReadOnlyStyle": "Text", + }) + header := strings.SplitN(out, "\n", 2)[0] + if !strings.Contains(header, "Editable: Never") { + t.Errorf("describe dropped the data view's Editability: Never, so re-executing it "+ + "makes the data view editable. got:\n%s", out) + } +} + +// Control: Always is the default, so printing it would invent a clause the +// author never wrote. +func TestDescribeDataView_OmitsDefaultEditability(t *testing.T) { + out := describeStoredDataView(t, map[string]any{ + "$Type": "Forms$DataView", "Name": "dataView1", "Editability": "Always", + }) + if strings.Contains(out, "Editable") { + t.Errorf("describe printed a default Editability: %s", out) + } +} + +// The MDL describe prints has to build back into a data view that carries the +// value, or the round trip still loses it at the next layer. +func TestBuildDataView_EditableNeverReachesTheWidget(t *testing.T) { + prog, errs := visitor.Build("create page Mod.P (Title: 'T') {\n dataview dataView1 (Editable: Never) {\n }\n}") + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + page := prog.Statements[0].(*ast.CreatePageStmtV3) + pb := &pageBuilder{widgetScope: map[string]model.ID{}} + widget, err := pb.buildWidgetV3(page.Widgets[0]) + if err != nil { + t.Fatalf("build: %v", err) + } + dv, ok := widget.(*pages.DataView) + if !ok { + t.Fatalf("built %T, want *pages.DataView", widget) + } + if got := pages.WidgetEditability(&dv.BaseWidget); got != "Never" { + t.Errorf("WidgetEditability = %q, want Never", got) + } +} diff --git a/mdl/executor/cmd_pages_describe_parse.go b/mdl/executor/cmd_pages_describe_parse.go index 4d200f3609..340e99055d 100644 --- a/mdl/executor/cmd_pages_describe_parse.go +++ b/mdl/executor/cmd_pages_describe_parse.go @@ -392,6 +392,10 @@ func parseRawWidget(ctx *ExecContext, w map[string]any, parentEntityContext ...s widget.EntityContext = inheritedCtx } widget.LabelWidth = extractDataViewLabelWidth(w) + // A data view stores `Editability`, where input widgets store `Editable`. + // Unread, a read-only data view described as an editable one and + // re-executing the description widened it (ako/mxcli#1676). + widget.Editable = extractString(w["Editability"]) widget.ReadOnlyStyle = extractReadOnlyStyle(ctx, w) widget.ShowFooter, _ = w["ShowFooter"].(bool) widget.Children = withQualifiedAttrs(ctx, widget.DataSource, func() []rawWidget { From ecf663d6a50bec988fae4c4662e650530e612dce Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 22:32:13 +0000 Subject: [PATCH 02/14] =?UTF-8?q?fix:=20a=20native=20dynamic=20text's=20Na?= =?UTF-8?q?tiveTextStyle=20survives=20describe=20=E2=86=92=20exec?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Forms$DynamicText.NativeTextStyle` (Text, Heading1–6; Mendix 8.0+) had no MDL spelling: describe did not read it, the builder had nowhere to put it, and both writers hard-coded Text, so a heading on a native page became body text on every rewrite. dynamictext now takes `NativeTextStyle: Heading3` (case-insensitive, stored canonically, an unknown value refused with the list), describe prints it when it is not Text, both backends write it, `alter page … set` accepts it, and check no longer warns MDL-WIDGET07 that it is dropped. Fixes ako/mxcli#1677 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DRueZhZoBML5iWuJTvUnjL --- .../2026-10-24-native-text-style-dropped.json | 1 + cmd/mxcli/syntax/features_page.go | 4 +- docs/01-project/MDL_QUICK_REFERENCE.md | 1 + .../bug-tests/1677-native-text-style.mdl | 26 +++++ mdl/backend/mcp/page_test.go | 16 +++ mdl/backend/mcp/page_widgets.go | 6 +- mdl/backend/modelsdk/widget_write.go | 2 +- ...idget_write_native_text_style_1677_test.go | 32 ++++++ mdl/backend/pagemutator/editable_test.go | 23 ++++ mdl/backend/pagemutator/mutator.go | 26 +++++ mdl/executor/cmd_pages_builder_v3_widgets.go | 15 +++ mdl/executor/cmd_pages_describe.go | 7 +- mdl/executor/cmd_pages_describe_output.go | 4 + mdl/executor/cmd_pages_describe_parse.go | 1 + ...dynamictext_native_text_style_1677_test.go | 104 ++++++++++++++++++ mdl/executor/validate_widgets.go | 2 + sdk/pages/pages_widgets_display.go | 36 ++++++ 17 files changed, 300 insertions(+), 6 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-24-native-text-style-dropped.json create mode 100644 mdl-examples/bug-tests/1677-native-text-style.mdl create mode 100644 mdl/backend/modelsdk/widget_write_native_text_style_1677_test.go create mode 100644 mdl/executor/dynamictext_native_text_style_1677_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-24-native-text-style-dropped.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-24-native-text-style-dropped.json new file mode 100644 index 0000000000..6947ab96ac --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-24-native-text-style-dropped.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-10-24", "symptom": "On a native page, describe → exec turned a heading into body text: Forms$DynamicText.NativeTextStyle Heading3 came back as Text. Describe printed `dynamictext text4 (Content: 'JTS boat log')`; `NativeTextStyle: Heading3` written by hand passed parsing with an MDL-WIDGET07 'will be silently dropped' warning", "cause": "MDL had no spelling for NativeTextStyle: no model field, the builder read nothing, describe read nothing, and both writers (modelsdk widget_write.go and the MCP backend's page_widgets.go) hard-coded \"Text\"", "file": "sdk/pages/pages_widgets_display.go (NativeTextStyle, CanonicalNativeTextStyle); mdl/executor/cmd_pages_builder_v3_widgets.go (buildDynamicTextV3); mdl/executor/cmd_pages_describe_parse.go, cmd_pages_describe_output.go; mdl/backend/modelsdk/widget_write.go; mdl/backend/mcp/page_widgets.go; mdl/backend/pagemutator/mutator.go (setDynamicTextNativeTextStyleMut); mdl/executor/validate_widgets.go", "fix": "Add `NativeTextStyle: Text | Heading1..Heading6` to dynamictext (case-insensitive, canonical on write, unknown refused with the list), print it when not Text, write it in both backends, accept it in `alter page … set`, and add it to the built-in widget vocabulary", "insight": "A hard-coded literal in a writer is the cheapest tell for this class: `grep -n 'Set\\w*(\"' widget_write.go` lists every property the writer can never vary, and the generated enum (modelsdk/gen/pages/enums.go) says whether the literal is the only member. Copy fixes to every writer: the MCP backend had the same literal. A fixture survey shows why no round-trip test caught it: PedApp stores Text on all 284 dynamic texts, so a Studio Pro fixture without native headings cannot fail; the repro needs a BSON edit", "refs": ["ako/mxcli#1677", "mendixlabs/mxcli#1228"], "test": "TestDescribeDynamicText_EmitsNativeTextStyle, TestBuildDynamicText_NativeTextStyle, TestBuildDynamicText_NativeTextStyleRejectsUnknown, TestValidateDynamicText_NativeTextStyleIsKnown (mdl/executor/dynamictext_native_text_style_1677_test.go); TestWriteDynamicText_NativeTextStyle (mdl/backend/modelsdk); TestSetWidgetProperty_NativeTextStyle (mdl/backend/pagemutator); TestMapPageWidget_DynamicTextNativeTextStyle (mdl/backend/mcp); mdl-examples/bug-tests/1677-native-text-style.mdl"} diff --git a/cmd/mxcli/syntax/features_page.go b/cmd/mxcli/syntax/features_page.go index efa8c65014..f60495a7c0 100644 --- a/cmd/mxcli/syntax/features_page.go +++ b/cmd/mxcli/syntax/features_page.go @@ -172,7 +172,7 @@ LIST IMPACT OF htmlelement; "-- The bracketed Visible: [IsActive] (attributes rooted for you) is the deprecated\n" + "-- spelling, MDL-DEPR081.\n\n" + "-- Actions\nACTIONBUTTON name (Caption: 'C', Action: SAVE CHANGES, ButtonStyle: Primary)\nLINKBUTTON name (Caption: 'C', Action: ...)\n\n" + - "-- Display\nDYNAMICTEXT name (Content: 'Hello, {1}!', ContentParams: ({1} = Name))\nTITLE name (Content: 'Heading')\nIMAGE name (Image: 'Module.Collection.ImageName')\nIMAGE name (ImageType: imageUrl, ImageUrl: 'https://…')\n" + + "-- Display\nDYNAMICTEXT name (Content: 'Hello, {1}!', ContentParams: ({1} = Name))\nDYNAMICTEXT name (Content: 'Welcome', NativeTextStyle: Heading2) -- native page: Text | Heading1..Heading6\nTITLE name (Content: 'Heading')\nIMAGE name (Image: 'Module.Collection.ImageName')\nIMAGE name (ImageType: imageUrl, ImageUrl: 'https://…')\n" + "-- IMAGE needs a source. Its default, `ImageType: image`, shows an entry from an\n" + "-- image collection, named as three parts: Module.Collection.ImageName.\n" + "-- `list image collections` lists the collections, `describe image collection\n" + @@ -314,7 +314,7 @@ LIST IMPACT OF htmlelement; "drop template", "insert template", "list view template", "add parameter", "drop parameter", "page parameter", "add parameters", }, - Syntax: "ALTER PAGE Module.Name { -- the generic ALTER: set / insert / replace / drop\n SET (property: value) ON widgetName; -- widget property names: any casing\n SET ('Row size': 'Small') ON lvOrders; -- an Atlas DESIGN property of that widget's\n -- type; quoted and case-sensitive.\n -- `list design properties for ` lists\n -- them. ON/OFF for a toggle, where OFF\n -- REMOVES the entry.\n -- A multi-select ('Hide on') or compound\n -- ('Spacing') one needs the inline\n -- DesignProperties: (...) form, because a\n -- SET assignment carries one value.\n SET (Action: CALL MICROFLOW Module.MF) ON btnSave; -- any CREATE PAGE action form\n SET ('createFileAction': CALL MICROFLOW Module.MF) ON fileUploader1;\n -- a pluggable widget's NAMED action slot,\n -- by the widget's own key; refused on a\n -- key that is not action-typed\n SET (DataSource: $Param) ON dvOrder; -- parameter/microflow/nanoflow/selection;\n -- DATABASE and association are REPLACE-only,\n -- and a data view takes no database source\n SET (RenderMode: H2) ON txtTitle; -- dynamic text: Text | Paragraph | H1..H6\n SET (prop1: val1, prop2: val2) ON widgetName;\n SET (Title: 'New Title'); -- page-level (case-sensitive): no ON\n SET (Documentation: 'What this page is for.');\n SET (Class: 'css-class'); -- page-level CSS class / style\n SET (Style: 'css: rule');\n SET (PopupWidth: 800, PopupHeight: 480, PopupResizable: true); -- page-level pop-up\n INSERT AFTER widgetName { };\n INSERT BEFORE widgetName { };\n INSERT INTO containerName { };\n DROP name1, name2;\n DROP TEMPLATE FOR Module.Specialization IN listViewName;\n REPLACE widgetName WITH { };\n ADD PARAMETERS $Order: Module.Order; -- or a primitive: $Count: Integer.\n -- A page with a Url needs a {Order}\n -- segment: SET (Url: '…/{Order}') in the\n -- same statement (CE5601 otherwise).\n -- A snippet takes entities only (MDL087).\n DROP PARAMETERS $Order; -- refused while a data source or an\n -- expression on the page still uses it\n ADD VARIABLES $show: Boolean = true;\n DROP VARIABLES $show;\n};\n-- A target is a widget name, or a DataGrid 2 column by what it shows:\n-- grid column(Attr), grid column('Caption'), @n when two match; or a\n-- navigation list item: nav item(widgetInIt), nav item('Text'), nav item@n. The old spellings `SET p = v`,\n-- `SET p: v` (no parentheses) and `DROP WIDGET a` still run and warn\n-- (MDL-DEPR101..103).\n\n-- The BULK form: one design property on every widget of a TYPE.\nALTER PAGES [IN Module]\n SET 'Compact' = ON, 'Striped' = ON\n WHERE WIDGETTYPE = datagrid -- the MDL keyword, which resolves to\n -- exactly one widget id. A full id in\n -- quotes works too. NOT a name: a widget\n -- name is unique only within its page.\n [DRY RUN]; -- run this FIRST. It reports the matches\n -- against a discardable copy and writes\n -- nothing.", + Syntax: "ALTER PAGE Module.Name { -- the generic ALTER: set / insert / replace / drop\n SET (property: value) ON widgetName; -- widget property names: any casing\n SET ('Row size': 'Small') ON lvOrders; -- an Atlas DESIGN property of that widget's\n -- type; quoted and case-sensitive.\n -- `list design properties for ` lists\n -- them. ON/OFF for a toggle, where OFF\n -- REMOVES the entry.\n -- A multi-select ('Hide on') or compound\n -- ('Spacing') one needs the inline\n -- DesignProperties: (...) form, because a\n -- SET assignment carries one value.\n SET (Action: CALL MICROFLOW Module.MF) ON btnSave; -- any CREATE PAGE action form\n SET ('createFileAction': CALL MICROFLOW Module.MF) ON fileUploader1;\n -- a pluggable widget's NAMED action slot,\n -- by the widget's own key; refused on a\n -- key that is not action-typed\n SET (DataSource: $Param) ON dvOrder; -- parameter/microflow/nanoflow/selection;\n -- DATABASE and association are REPLACE-only,\n -- and a data view takes no database source\n SET (RenderMode: H2) ON txtTitle; -- dynamic text: Text | Paragraph | H1..H6\n SET (NativeTextStyle: Heading2) ON txtTitle; -- dynamic text on a native page:\n -- Text | Heading1..Heading6\n SET (prop1: val1, prop2: val2) ON widgetName;\n SET (Title: 'New Title'); -- page-level (case-sensitive): no ON\n SET (Documentation: 'What this page is for.');\n SET (Class: 'css-class'); -- page-level CSS class / style\n SET (Style: 'css: rule');\n SET (PopupWidth: 800, PopupHeight: 480, PopupResizable: true); -- page-level pop-up\n INSERT AFTER widgetName { };\n INSERT BEFORE widgetName { };\n INSERT INTO containerName { };\n DROP name1, name2;\n DROP TEMPLATE FOR Module.Specialization IN listViewName;\n REPLACE widgetName WITH { };\n ADD PARAMETERS $Order: Module.Order; -- or a primitive: $Count: Integer.\n -- A page with a Url needs a {Order}\n -- segment: SET (Url: '…/{Order}') in the\n -- same statement (CE5601 otherwise).\n -- A snippet takes entities only (MDL087).\n DROP PARAMETERS $Order; -- refused while a data source or an\n -- expression on the page still uses it\n ADD VARIABLES $show: Boolean = true;\n DROP VARIABLES $show;\n};\n-- A target is a widget name, or a DataGrid 2 column by what it shows:\n-- grid column(Attr), grid column('Caption'), @n when two match; or a\n-- navigation list item: nav item(widgetInIt), nav item('Text'), nav item@n. The old spellings `SET p = v`,\n-- `SET p: v` (no parentheses) and `DROP WIDGET a` still run and warn\n-- (MDL-DEPR101..103).\n\n-- The BULK form: one design property on every widget of a TYPE.\nALTER PAGES [IN Module]\n SET 'Compact' = ON, 'Striped' = ON\n WHERE WIDGETTYPE = datagrid -- the MDL keyword, which resolves to\n -- exactly one widget id. A full id in\n -- quotes works too. NOT a name: a widget\n -- name is unique only within its page.\n [DRY RUN]; -- run this FIRST. It reports the matches\n -- against a discardable copy and writes\n -- nothing.", Example: "ALTER PAGE Module.EditPage {\n SET (Caption: 'Save & Close', ButtonStyle: Success) ON btnSave;\n INSERT AFTER txtName {\n TEXTBOX txtMiddleName (Label: 'Middle Name', Attribute: MiddleName)\n };\n DROP txtUnused;\n};", SeeAlso: []string{"page.create", "page.show", "snippet.alter"}, }) diff --git a/docs/01-project/MDL_QUICK_REFERENCE.md b/docs/01-project/MDL_QUICK_REFERENCE.md index c38d3c393b..0e5f0b2eda 100644 --- a/docs/01-project/MDL_QUICK_REFERENCE.md +++ b/docs/01-project/MDL_QUICK_REFERENCE.md @@ -1732,6 +1732,7 @@ This is the generic ALTER — `alter Module.Name { set (Key: value) on 0 { props = append(props, fmt.Sprintf("ContentParams: (%s)", strings.Join(formatParametersV3(w.Parameters), ", "))) } diff --git a/mdl/executor/cmd_pages_describe_parse.go b/mdl/executor/cmd_pages_describe_parse.go index 340e99055d..821dee9e53 100644 --- a/mdl/executor/cmd_pages_describe_parse.go +++ b/mdl/executor/cmd_pages_describe_parse.go @@ -329,6 +329,7 @@ func parseRawWidget(ctx *ExecContext, w map[string]any, parentEntityContext ...s if rm, ok := w["RenderMode"].(string); ok { widget.RenderMode = rm } + widget.NativeTextStyle = extractString(w["NativeTextStyle"]) return []rawWidget{widget} case "Forms$ActionButton", "Pages$ActionButton": diff --git a/mdl/executor/dynamictext_native_text_style_1677_test.go b/mdl/executor/dynamictext_native_text_style_1677_test.go new file mode 100644 index 0000000000..cc8b99161f --- /dev/null +++ b/mdl/executor/dynamictext_native_text_style_1677_test.go @@ -0,0 +1,104 @@ +// SPDX-License-Identifier: Apache-2.0 + +// ako/mxcli#1677: "`Forms$DynamicText.NativeTextStyle: Heading3` becomes `Text`. +// The heading on the native home and login pages becomes body text." Describe +// printed `dynamictext text4 (Content: 'JTS boat log')`, and MDL had no spelling +// for the property: the writer stored Text on every dynamic text. +// +// NativeTextStyle is the native counterpart of RenderMode (Model SDK: +// Forms$DynamicText `nativeTextStyle` Introduced 8.0.0, enum Text and +// Heading1–Heading6). +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/visitor" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +func describeStoredDynamicText(t *testing.T, style string) string { + t.Helper() + ctx, buf := newMockCtx(t) + stored := map[string]any{"$Type": "Forms$DynamicText", "Name": "text4", "RenderMode": "Text"} + if style != "" { + stored["NativeTextStyle"] = style + } + for _, rw := range parseRawWidget(ctx, stored) { + outputWidgetMDLV3(ctx, rw, 0) + } + return buf.String() +} + +func TestDescribeDynamicText_EmitsNativeTextStyle(t *testing.T) { + if out := describeStoredDynamicText(t, "Heading3"); !strings.Contains(out, "NativeTextStyle: Heading3") { + t.Errorf("describe dropped NativeTextStyle Heading3, so re-executing it turns the heading into body text. got:\n%s", out) + } +} + +// Control: Text is what every dynamic text stores (284 of 284 in PedApp), so +// printing it would put a clause in every description. +func TestDescribeDynamicText_OmitsDefaultNativeTextStyle(t *testing.T) { + for _, style := range []string{"", "Text"} { + if out := describeStoredDynamicText(t, style); strings.Contains(out, "NativeTextStyle") { + t.Errorf("describe printed NativeTextStyle for %q: %s", style, out) + } + } +} + +func buildDynamicTextFromMDL(t *testing.T, props string) (*pages.DynamicText, error) { + t.Helper() + prog, errs := visitor.Build("create page Mod.P (Title: 'T') {\n dynamictext text4 (" + props + ")\n}") + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + page := prog.Statements[0].(*ast.CreatePageStmtV3) + pb := &pageBuilder{widgetScope: map[string]model.ID{}} + w, err := pb.buildWidgetV3(page.Widgets[0]) + if err != nil { + return nil, err + } + return w.(*pages.DynamicText), nil +} + +func TestBuildDynamicText_NativeTextStyle(t *testing.T) { + for _, tc := range []struct{ written, want string }{ + {"Heading3", "Heading3"}, + {"heading6", "Heading6"}, // matched case-insensitively, stored canonically + {"Text", "Text"}, + } { + dt, err := buildDynamicTextFromMDL(t, "Content: 'JTS boat log', NativeTextStyle: "+tc.written) + if err != nil { + t.Fatalf("NativeTextStyle: %s: %v", tc.written, err) + } + if string(dt.NativeTextStyle) != tc.want { + t.Errorf("NativeTextStyle: %s built %q, want %q", tc.written, dt.NativeTextStyle, tc.want) + } + } + // Control: unwritten stays unauthored, so the writer keeps its Text default. + dt, err := buildDynamicTextFromMDL(t, "Content: 'x'") + if err != nil { + t.Fatal(err) + } + if dt.NativeTextStyle != "" { + t.Errorf("NativeTextStyle = %q for a text that declared none", dt.NativeTextStyle) + } +} + +// An unknown member is a document Studio Pro cannot open; refuse it. +func TestBuildDynamicText_NativeTextStyleRejectsUnknown(t *testing.T) { + _, err := buildDynamicTextFromMDL(t, "Content: 'x', NativeTextStyle: H3") + if err == nil || !strings.Contains(err.Error(), "Heading3") { + t.Errorf("NativeTextStyle: H3 = %v, want an error listing the valid styles", err) + } +} + +// check must not warn that the property it now writes "will be silently dropped". +func TestValidateDynamicText_NativeTextStyleIsKnown(t *testing.T) { + if !isKnownStaticWidgetProp("NativeTextStyle") { + t.Error("NativeTextStyle is not in the built-in widget vocabulary, so check warns MDL-WIDGET07 that it is dropped") + } +} diff --git a/mdl/executor/validate_widgets.go b/mdl/executor/validate_widgets.go index 070a96078e..606ad735ba 100644 --- a/mdl/executor/validate_widgets.go +++ b/mdl/executor/validate_widgets.go @@ -1189,6 +1189,8 @@ var staticWidgetKnownProps = func() map[string]bool { // a scroll-container region's toggle mode, which a sidebartoggle opens // (mendixlabs/mxcli#1220). "ToggleMode", + // a dynamic text's style on a native page (ako/mxcli#1677). + "NativeTextStyle", // a layout-grid row's and column's alignment, read by // buildLayoutGridRowV3 / buildLayoutGridColumnV3 and emitted by describe. "VerticalAlignment", "HorizontalAlignment", "SpacingBetweenColumns", diff --git a/sdk/pages/pages_widgets_display.go b/sdk/pages/pages_widgets_display.go index f94155c8ac..02e8e89847 100644 --- a/sdk/pages/pages_widgets_display.go +++ b/sdk/pages/pages_widgets_display.go @@ -3,6 +3,8 @@ package pages import ( + "strings" + "github.com/mendixlabs/mxcli/model" ) @@ -29,6 +31,37 @@ const ( TextRenderModeParagraph TextRenderMode = "Paragraph" ) +// NativeTextStyle is a dynamic text's style on a native page (Mendix 8.0+), the +// native counterpart of TextRenderMode. +type NativeTextStyle string + +const ( + NativeTextStyleText NativeTextStyle = "Text" + NativeTextStyleHeading1 NativeTextStyle = "Heading1" + NativeTextStyleHeading2 NativeTextStyle = "Heading2" + NativeTextStyleHeading3 NativeTextStyle = "Heading3" + NativeTextStyleHeading4 NativeTextStyle = "Heading4" + NativeTextStyleHeading5 NativeTextStyle = "Heading5" + NativeTextStyleHeading6 NativeTextStyle = "Heading6" +) + +// NativeTextStyles lists every NativeTextStyle in the metamodel's order. +var NativeTextStyles = []NativeTextStyle{ + NativeTextStyleText, NativeTextStyleHeading1, NativeTextStyleHeading2, NativeTextStyleHeading3, + NativeTextStyleHeading4, NativeTextStyleHeading5, NativeTextStyleHeading6, +} + +// CanonicalNativeTextStyle matches s case-insensitively against the +// NativeTextStyle enumeration and returns its canonical spelling. +func CanonicalNativeTextStyle(s string) (NativeTextStyle, bool) { + for _, v := range NativeTextStyles { + if strings.EqualFold(strings.TrimSpace(s), string(v)) { + return v, true + } + } + return "", false +} + // ClientTemplate represents a text template with parameters. // Used for dynamic text content in widgets like DynamicText and ActionButton captions. type ClientTemplate struct { @@ -70,6 +103,9 @@ type DynamicText struct { AttributePath string `json:"attributePath,omitempty"` Content *ClientTemplate `json:"content,omitempty"` RenderMode TextRenderMode `json:"renderMode,omitempty"` + // NativeTextStyle is the text's style on a native page, where RenderMode + // does not apply. Empty means not authored; the writer stores Text. + NativeTextStyle NativeTextStyle `json:"nativeTextStyle,omitempty"` } // Label represents a label widget. From 2a94dd23cbca9ed30c96c9ab3b81621081bea520 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 22:53:31 +0000 Subject: [PATCH 03/14] fix: a native page's design properties are typed against the native theme MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The theme registry read only themesource/*/web/design-properties.json, so a native page's widgets were typed against the web definitions. Atlas Core declares DynamicText `Color` as a web ColorPicker (swatch `Brand Primary`) and a native Dropdown (option `Brand primary`), so describe → exec turned the stored option into a custom colour. mx check does not flag it. The registry now loads the native files too and a document selects by its platform: a page by its layout, a layout by its layout type, a snippet by its stored Type. This applies to create, alter page set, alter pages styling, alter styling, and check -p, which also stops warning MDL-WIDGET12 on native values such as 'Smallest'. Fixes ako/mxcli#1679 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DRueZhZoBML5iWuJTvUnjL --- ...4-native-design-property-typed-as-web.json | 1 + .../1679-native-design-property-kind.mdl | 33 +++ mdl/backend/pagemutator/probe.go | 14 ++ mdl/executor/cmd_alter_page.go | 8 +- mdl/executor/cmd_alter_pages_styling.go | 2 +- mdl/executor/cmd_pages_builder.go | 14 ++ mdl/executor/cmd_pages_builder_v3.go | 7 + mdl/executor/cmd_pages_create_v3.go | 6 + mdl/executor/cmd_pages_layout_v3.go | 3 + mdl/executor/cmd_styling.go | 2 +- mdl/executor/design_property_routing.go | 55 +++++ mdl/executor/design_props_native_1679_test.go | 216 ++++++++++++++++++ mdl/executor/theme_reader.go | 56 +++-- mdl/executor/validate_design_properties.go | 60 ++++- mdl/executor/validate_program.go | 2 +- .../validate_snippetcall_appearance.go | 2 +- 16 files changed, 453 insertions(+), 28 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-24-native-design-property-typed-as-web.json create mode 100644 mdl-examples/bug-tests/1679-native-design-property-kind.mdl create mode 100644 mdl/executor/design_props_native_1679_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-24-native-design-property-typed-as-web.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-24-native-design-property-typed-as-web.json new file mode 100644 index 0000000000..1a6ccea1d6 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-24-native-design-property-typed-as-web.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-10-24", "symptom": "On a native page, describe → exec turned `'Color': 'Brand primary'` from Forms$OptionDesignPropertyValue (Option) into Forms$CustomDesignPropertyValue (Value); `Size` on the same widget stayed an option. mx check 0 errors either way. check -p warned MDL-WIDGET12 that the native values 'Brand primary' and 'Smallest' were not allowed", "cause": "loadThemeRegistry read only themesource/*/web/design-properties.json, so every widget was typed against the web definitions. Atlas Core 4.1.3 declares DynamicText Color as a web ColorPicker (swatch 'Brand Primary') and a native Dropdown (option 'Brand primary'); a value off the web list on a ColorPicker resolves to custom", "file": "mdl/executor/theme_reader.go (NativeWidgetProperties, ForPlatform); cmd_pages_builder_v3.go (buildPageV3: layoutIsNative); cmd_pages_layout_v3.go; cmd_pages_create_v3.go (storedSnippetIsNative); design_property_routing.go (documentTheme, used by alter page, alter pages styling, alter styling, the snippet-call check); mdl/backend/pagemutator/probe.go (DocumentPlatform); validate_design_properties.go (validateDesignProperties, nativeLayoutResolver)", "fix": "Load the native design-properties.json files beside the web ones and select by the document's platform: a page by its layout's Native flag (FormCall.Form for a stored page), a layout by its layout type, a snippet by its stored Type. check -p resolves the layout from the script or the project", "insight": "When one property changes kind and its siblings do not, diff the two platforms' theme files for that key before reading writer code: `jq '.DynamicText[] | select(.name==\"Color\")'` on web/ and native/ showed ColorPicker vs Dropdown and a one-letter case difference in the option, which is the whole bug. The value's spelling matching the native option exactly, and missing the web one by case, is what made it custom. mx check does not flag a custom value on a native Dropdown, so the write layer is the only place to test it; the full run needs a BSON edit to put Studio Pro's Option shape back first, because mxcli had written Custom at create time", "refs": ["ako/mxcli#1679", "ako/mxcli#511"], "test": "TestThemeRegistry_LoadsNativeDefinitions, TestNativePageDesignPropertyKeepsOptionKind, TestNativePageAlterSetDesignPropertyKeepsOptionKind, TestValidateDesignProperties_NativePageUsesNativeDefinitions (mdl/executor/design_props_native_1679_test.go); mdl-examples/bug-tests/1679-native-design-property-kind.mdl"} diff --git a/mdl-examples/bug-tests/1679-native-design-property-kind.mdl b/mdl-examples/bug-tests/1679-native-design-property-kind.mdl new file mode 100644 index 0000000000..41cd7049fe --- /dev/null +++ b/mdl-examples/bug-tests/1679-native-design-property-kind.mdl @@ -0,0 +1,33 @@ +mdl 1; +-- ============================================================================ +-- ako/mxcli#1679 — on a native page, describe → exec turned `'Color': 'Brand +-- primary'` from a Forms$OptionDesignPropertyValue into a +-- Forms$CustomDesignPropertyValue. `Size` on the same widget stayed an option. +-- +-- The theme registry read only themesource/*/web/design-properties.json. Atlas +-- Core 4.1.3 declares DynamicText `Color` as a web ColorPicker whose swatch is +-- spelled `Brand Primary`, and as a native Dropdown with the option `Brand +-- primary`; off the web list, on a ColorPicker, the value was a custom colour. +-- A native page, layout or snippet now types its widgets against the native +-- definitions — in create, alter page set, alter styling and check -p. +-- +-- Measured on 11.13.0 (Atlas Core 4.1.3), after a BSON edit storing text6's +-- Color as an option, the way Studio Pro does: +-- +-- before the fix: exec → Color Forms$CustomDesignPropertyValue; mx check 0 +-- errors (silent); check -p warns MDL-WIDGET12 on the native +-- values 'Brand primary' and 'Smallest' +-- after: exec → "Unchanged page", Color stays an option; check -p +-- quiet on the native page and still warns on a web one +-- ============================================================================ + +create module BugNativeDesignProps; + +create page BugNativeDesignProps.Login_Native (Title: 'Login', Layout: Atlas_Core.NativePhone_Default) { + dynamictext text6 (Content: 'Sign in', DesignProperties: ('Color': 'Brand primary', 'Size': 'Small')) + dynamictext text8 (Content: 'Forgot password?') +}; + +alter page BugNativeDesignProps.Login_Native { + set ('Color': 'Brand primary') on text8 +}; diff --git a/mdl/backend/pagemutator/probe.go b/mdl/backend/pagemutator/probe.go index e0fdf0fbd7..1e6e923351 100644 --- a/mdl/backend/pagemutator/probe.go +++ b/mdl/backend/pagemutator/probe.go @@ -130,3 +130,17 @@ func (m *Mutator) WidgetStorageType(widgetRef string) (bsonType, widgetID string } return bsonType, widgetID } + +// DocumentPlatform reports whether the document is a native one, so its widgets +// take the native design-property definitions (ako/mxcli#1679). A snippet says +// so in its Type and a layout in its content's $Type; a page is native when its +// layout is, so it names that layout (FormCall.Form) for the caller to resolve. +func (m *Mutator) DocumentPlatform() (native bool, layout string) { + switch m.containerType { + case backend.ContainerSnippet: + return bsonnav.DGetString(m.rawData, "Type") == "Native", "" + case backend.ContainerLayout: + return bsonnav.DGetString(bsonnav.DGetDoc(m.rawData, "Content"), "$Type") == "Forms$NativeLayoutContent", "" + } + return false, bsonnav.DGetString(bsonnav.DGetDoc(m.rawData, "FormCall"), "Form") +} diff --git a/mdl/executor/cmd_alter_page.go b/mdl/executor/cmd_alter_page.go index fe90dd1e8e..35c41fbfa8 100644 --- a/mdl/executor/cmd_alter_page.go +++ b/mdl/executor/cmd_alter_page.go @@ -457,7 +457,7 @@ func applySetProperties(ctx *ExecContext, mutator backend.PageMutator, op *ast.S return mdlerrors.NewBackend("set "+propName+" on "+op.Target.Name(), err) } } else if p := designPropertyForStoredWidget( - ctx.GetThemeRegistry(), mutator, op.Target.Widget, propName); p != nil { + documentTheme(ctx, mutator), mutator, op.Target.Widget, propName); p != nil { // An Atlas design property of THIS stored widget. It lives in // Appearance.DesignProperties, which SetWidgetProperty does not // reach, so before ako/mxcli#515 this dead-ended and ALTER STYLING @@ -576,7 +576,7 @@ func convertASTAction(ctx *ExecContext, mutator backend.PageMutator, value any, paramEntityNames: paramEntityNames, execCache: ctx.Cache, fragments: ctx.Fragments, - themeRegistry: ctx.GetThemeRegistry(), + themeRegistry: documentTheme(ctx, mutator), widgetBackend: ctx.Backend, localVariables: storedPageVariables(mutator), isDocumentWidget: alterDocumentWidgets(mutator, nil), @@ -1120,7 +1120,7 @@ func buildColumnSpecsFromAST(ctx *ExecContext, widgets []*ast.WidgetV3, moduleNa paramEntityNames: paramEntityNames, execCache: ctx.Cache, fragments: ctx.Fragments, - themeRegistry: ctx.GetThemeRegistry(), + themeRegistry: documentTheme(ctx, mutator), widgetBackend: ctx.Backend, localVariables: storedPageVariables(mutator), isDocumentWidget: alterDocumentWidgets(mutator, widgets), @@ -1325,7 +1325,7 @@ func newAlterPageBuilder(ctx *ExecContext, moduleName string, moduleID model.ID, paramEntityNames: paramEntityNames, execCache: ctx.Cache, fragments: ctx.Fragments, - themeRegistry: ctx.GetThemeRegistry(), + themeRegistry: documentTheme(ctx, mutator), widgetBackend: ctx.Backend, localVariables: storedPageVariables(mutator), isDocumentWidget: alterDocumentWidgets(mutator, nil), diff --git a/mdl/executor/cmd_alter_pages_styling.go b/mdl/executor/cmd_alter_pages_styling.go index 13eb6bc810..6a5b479e67 100644 --- a/mdl/executor/cmd_alter_pages_styling.go +++ b/mdl/executor/cmd_alter_pages_styling.go @@ -139,7 +139,7 @@ func styleWidgetsInContainer(ctx *ExecContext, containerID string, refs []widget } } - theme := ctx.GetThemeRegistry() + theme := documentTheme(ctx, target) for _, ref := range refs { landed := 0 for _, a := range s.Assignments { diff --git a/mdl/executor/cmd_pages_builder.go b/mdl/executor/cmd_pages_builder.go index e578a55bb3..76f0422d57 100644 --- a/mdl/executor/cmd_pages_builder.go +++ b/mdl/executor/cmd_pages_builder.go @@ -283,6 +283,20 @@ func (pb *pageBuilder) resolveLayout(layoutName string) (model.ID, error) { return "", mdlerrors.NewNotFound("layout", layoutName) } +// layoutIsNative reports whether the layout with this ID is a native one. +func (pb *pageBuilder) layoutIsNative(id model.ID) bool { + layouts, err := pb.getLayouts() + if err != nil { + return false + } + for _, l := range layouts { + if l.ID == id { + return l.Native + } + } + return false +} + // resolveEntity finds an entity by qualified name. func (pb *pageBuilder) resolveEntity(entityRef ast.QualifiedName) (model.ID, error) { // Get domain models which contain entities diff --git a/mdl/executor/cmd_pages_builder_v3.go b/mdl/executor/cmd_pages_builder_v3.go index 8efb7fc0b6..70c37b5500 100644 --- a/mdl/executor/cmd_pages_builder_v3.go +++ b/mdl/executor/cmd_pages_builder_v3.go @@ -90,6 +90,13 @@ func (pb *pageBuilder) buildPageV3(s *ast.CreatePageStmtV3) (*pages.Page, error) log.Printf("warning: layout %s not found", s.Layout) } else { page.LayoutID = layoutID + // A native page's widgets take the native design-property + // definitions; typed against the web ones, Atlas's native `'Color': + // 'Brand primary'` option was stored as a custom colour + // (ako/mxcli#1679). The layout decides, as it does in Studio Pro. + if pb.layoutIsNative(layoutID) { + pb.themeRegistry = pb.themeRegistry.ForPlatform(true) + } // Create LayoutCall with arguments for placeholders page.LayoutCall = &pages.LayoutCall{ diff --git a/mdl/executor/cmd_pages_create_v3.go b/mdl/executor/cmd_pages_create_v3.go index 75bfdb0eb3..c0d2fa1574 100644 --- a/mdl/executor/cmd_pages_create_v3.go +++ b/mdl/executor/cmd_pages_create_v3.go @@ -294,6 +294,12 @@ func execCreateSnippetV3(ctx *ExecContext, s *ast.CreateSnippetStmtV3) error { return describeSnippet(ctx, s.Name) }) } + // A rewrite keeps the stored snippet's Type (the header carry), so a native + // snippet's widgets take the native design-property definitions + // (ako/mxcli#1679). + if len(snippetsToDelete) > 0 && storedSnippetIsNative(ctx, snippetsToDelete[0]) { + pb.themeRegistry = pb.themeRegistry.ForPlatform(true) + } snippet, err := pb.buildSnippetV3(s) if err != nil { diff --git a/mdl/executor/cmd_pages_layout_v3.go b/mdl/executor/cmd_pages_layout_v3.go index 32e27a16c3..4943f4c8a3 100644 --- a/mdl/executor/cmd_pages_layout_v3.go +++ b/mdl/executor/cmd_pages_layout_v3.go @@ -49,6 +49,9 @@ func (pb *pageBuilder) buildLayoutV3(s *ast.CreateLayoutStmt) (*pages.Layout, er "layout %s: unknown layouttype %q (want Responsive, Phone, Tablet, ModalPopup, Default or Popup)", s.Name.String(), layoutType)) } + // A native layout's widgets take the native design-property definitions + // (ako/mxcli#1679). + pb.themeRegistry = pb.themeRegistry.ForPlatform(native) layout := &pages.Layout{ BaseElement: model.BaseElement{ diff --git a/mdl/executor/cmd_styling.go b/mdl/executor/cmd_styling.go index ccf75f2d47..c0dc13698c 100644 --- a/mdl/executor/cmd_styling.go +++ b/mdl/executor/cmd_styling.go @@ -309,7 +309,7 @@ func execAlterStyling(ctx *ExecContext, s *ast.AlterStylingStmt) error { fmt.Sprintf("widget %q not found in %s %s", s.WidgetName, containerType, s.ContainerName.String())) } - if err := applyStylingMutator(mutator, s, ctx.GetThemeRegistry()); err != nil { + if err := applyStylingMutator(mutator, s, documentTheme(ctx, mutator)); err != nil { return mdlerrors.NewBackend("alter styling", err) } diff --git a/mdl/executor/design_property_routing.go b/mdl/executor/design_property_routing.go index ad4f2c405c..e79cc9b946 100644 --- a/mdl/executor/design_property_routing.go +++ b/mdl/executor/design_property_routing.go @@ -6,8 +6,11 @@ import ( "fmt" "strings" + "go.mongodb.org/mongo-driver/v2/bson" + "github.com/mendixlabs/mxcli/mdl/ast" "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/model" ) // Routing a design property written by `ALTER PAGE … SET` (ako/mxcli#515). @@ -71,6 +74,58 @@ func storedWidgetDesignPropsKey(mutator backend.PageMutator, widgetRef string) s return "" } +// documentPlatformReporter is the optional half of a PageMutator that knows the +// platform of the document it edits. +type documentPlatformReporter interface { + DocumentPlatform() (native bool, layout string) +} + +// documentTheme is the theme registry the design properties of a mutator's +// document resolve against: the native definitions for a native page, snippet +// or layout, the web ones otherwise (ako/mxcli#1679). A page is native when its +// layout is. +func documentTheme(ctx *ExecContext, mutator backend.PageMutator) *ThemeRegistry { + reg := ctx.GetThemeRegistry() + r, ok := mutator.(documentPlatformReporter) + if reg == nil || !ok { + return reg + } + native, layout := r.DocumentPlatform() + if !native && layout != "" { + native = layoutNamedIsNative(ctx, layout) + } + return reg.ForPlatform(native) +} + +// storedSnippetIsNative reports whether the stored snippet's Type is Native. +func storedSnippetIsNative(ctx *ExecContext, id model.ID) bool { + raw, err := ctx.Backend.GetRawUnitBytes(id) + if err != nil { + return false + } + t, ok := bson.Raw(raw).Lookup("Type").StringValueOK() + return ok && t == "Native" +} + +// layoutNamedIsNative reports whether the layout with this qualified name is a +// native one; false when there is none. +func layoutNamedIsNative(ctx *ExecContext, qualifiedName string) bool { + layouts, err := ctx.Backend.ListLayouts() + if err != nil { + return false + } + h, err := getHierarchy(ctx) + if err != nil || h == nil { + return false + } + for _, l := range layouts { + if h.GetModuleName(h.FindModuleID(l.ContainerID))+"."+l.Name == qualifiedName { + return l.Native + } + } + return false +} + // designPropertyForStoredWidget returns the theme's declaration of key for the // stored widget, or nil when this is not a design property of it. // diff --git a/mdl/executor/design_props_native_1679_test.go b/mdl/executor/design_props_native_1679_test.go new file mode 100644 index 0000000000..7fe3f2610c --- /dev/null +++ b/mdl/executor/design_props_native_1679_test.go @@ -0,0 +1,216 @@ +// SPDX-License-Identifier: Apache-2.0 + +// ako/mxcli#1679: "on `Login_Native` `text6` and `text8`, `'Color': 'Brand +// primary'` is stored before as `Forms$OptionDesignPropertyValue` (Option `Brand +// primary`), and after as `Forms$CustomDesignPropertyValue` (Value `Brand +// primary`). Other properties on the same widgets (`Size`, `Spacing bottom`) +// stay options." +// +// The theme registry read only themesource/*/web/design-properties.json, so a +// native page's widgets were typed against the web definitions. Atlas Core 4.1.3 +// declares DynamicText `Color` as a web ColorPicker whose swatch is spelled +// `Brand Primary`, and as a native Dropdown with the option `Brand primary`: off +// the web list, on a ColorPicker, the value was written as a custom colour. +package executor + +import ( + "context" + "os" + "path/filepath" + "testing" + + "go.mongodb.org/mongo-driver/v2/bson" + + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// atlasColorTheme is the shape of Atlas Core 4.1.3's two DynamicText `Color` +// definitions (themesource/atlas_core/{web,native}/design-properties.json in a +// blank 11.13.0 app), cut down to the options that matter. +var atlasColorTheme = map[string]string{ + "web": `{"DynamicText": [ + {"name": "Color", "type": "ColorPicker", "options": [{"name": "Brand Primary", "class": "text-primary"}]}, + {"name": "Size", "type": "ToggleButtonGroup", "options": [{"name": "Small", "class": "text-small"}]}]}`, + "native": `{"DynamicText": [ + {"name": "Color", "type": "Dropdown", "options": [{"name": "Brand primary", "class": "textPrimary"}]}, + {"name": "Size", "type": "Dropdown", "options": [{"name": "Small", "class": "textSmall"}]}]}`, +} + +func writeAtlasColorTheme(t *testing.T, projectDir string) { + t.Helper() + for platform, body := range atlasColorTheme { + dir := filepath.Join(projectDir, "themesource", "atlas_core", platform) + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "design-properties.json"), []byte(body), 0o644); err != nil { + t.Fatal(err) + } + } +} + +func TestThemeRegistry_LoadsNativeDefinitions(t *testing.T) { + dir := t.TempDir() + writeAtlasColorTheme(t, dir) + reg, err := loadThemeRegistry(dir) + if err != nil { + t.Fatal(err) + } + for _, tc := range []struct { + native bool + want string + }{{false, "ColorPicker"}, {true, "Dropdown"}} { + props := reg.ForPlatform(tc.native).GetPropertiesForWidget("DynamicText") + if tp := findThemeProp(props, "Color"); tp == nil || tp.Type != tc.want { + t.Errorf("native=%v: Color = %+v, want type %s", tc.native, tp, tc.want) + } + } +} + +// storedDesignPropertyTypes maps each design-property key on the named widget of +// a stored page to the $Type of its value. +func storedDesignPropertyTypes(t *testing.T, exec *Executor, page, widget string) map[string]string { + t.Helper() + ctx := exec.newExecContext(context.Background()) + h, err := getHierarchy(ctx) + if err != nil { + t.Fatal(err) + } + all, err := ctx.Backend.ListPages() + if err != nil { + t.Fatal(err) + } + for _, p := range all { + if p.Name != page || h.GetModuleName(h.FindModuleID(p.ContainerID)) != "MyFirstModule" { + continue + } + raw, err := ctx.Backend.GetRawUnitBytes(p.ID) + if err != nil { + t.Fatal(err) + } + var doc bson.D + if err := bson.Unmarshal(raw, &doc); err != nil { + t.Fatal(err) + } + out := map[string]string{} + walkBSON(doc, func(d bson.D) { + if afGet(d, "Name") != widget { + return + } + app, _ := afGet(d, "Appearance").(bson.D) + for _, el := range afList(afGet(app, "DesignProperties")) { + dp := el.(bson.D) + v, _ := afGet(dp, "Value").(bson.D) + out[afGet(dp, "Key").(string)] = afGet(v, "$Type").(string) + } + }) + return out + } + t.Fatalf("page MyFirstModule.%s not found", page) + return nil +} + +func walkBSON(d bson.D, fn func(bson.D)) { + fn(d) + for _, e := range d { + switch v := e.Value.(type) { + case bson.D: + walkBSON(v, fn) + case bson.A: + for _, x := range v { + if xd, ok := x.(bson.D); ok { + walkBSON(xd, fn) + } + } + } + } +} + +func TestNativePageDesignPropertyKeepsOptionKind(t *testing.T) { + exec, _ := openPedAppFixture(t) + writeAtlasColorTheme(t, filepath.Dir(exec.newExecContext(context.Background()).MprPath)) + + for _, tc := range []struct { + page, layout string + wantColor string + }{ + // The reported case: a native page types against the native definitions. + {"Login_Native", "Atlas_Core.NativePhone_Default", "Forms$OptionDesignPropertyValue"}, + // Control: on a web page the same value is off the ColorPicker's list, + // so it is a custom colour, as before. + {"Login_Web", "Atlas_Core.Atlas_Default", "Forms$CustomDesignPropertyValue"}, + } { + t.Run(tc.page, func(t *testing.T) { + src := "create page MyFirstModule." + tc.page + " (Title: 'Login', Layout: " + tc.layout + ") {\n" + + " dynamictext text6 (Content: 'Sign in', DesignProperties: ('Color': 'Brand primary', 'Size': 'Small'))\n};" + if err := afRun(t, exec, src); err != nil { + t.Fatalf("exec: %v", err) + } + got := storedDesignPropertyTypes(t, exec, tc.page, "text6") + if got["Color"] != tc.wantColor { + t.Errorf("Color stored as %q, want %q (all: %v)", got["Color"], tc.wantColor, got) + } + if got["Size"] != "Forms$OptionDesignPropertyValue" { + t.Errorf("Size stored as %q, want an option", got["Size"]) + } + }) + } +} + +// The same typing on `alter page … set`, which resolves the stored page's +// platform from its layout rather than from a statement. +func TestNativePageAlterSetDesignPropertyKeepsOptionKind(t *testing.T) { + exec, _ := openPedAppFixture(t) + writeAtlasColorTheme(t, filepath.Dir(exec.newExecContext(context.Background()).MprPath)) + + for _, tc := range []struct { + page, layout string + wantColor string + }{ + {"Alter_Native", "Atlas_Core.NativePhone_Default", "Forms$OptionDesignPropertyValue"}, + {"Alter_Web", "Atlas_Core.Atlas_Default", "Forms$CustomDesignPropertyValue"}, // control + } { + t.Run(tc.page, func(t *testing.T) { + src := "create page MyFirstModule." + tc.page + " (Title: 'Login', Layout: " + tc.layout + ") {\n" + + " dynamictext text8 (Content: 'Forgot password?')\n};\n" + + "alter page MyFirstModule." + tc.page + " {\n set ('Color': 'Brand primary') on text8\n};" + if err := afRun(t, exec, src); err != nil { + t.Fatalf("exec: %v", err) + } + if got := storedDesignPropertyTypes(t, exec, tc.page, "text8")["Color"]; got != tc.wantColor { + t.Errorf("Color stored as %q, want %q", got, tc.wantColor) + } + }) + } +} + +// check -p typed a native page's design properties against the web definitions +// too, so the values Atlas's native theme declares were reported as not allowed +// (MDL-WIDGET12). A layout the script creates is resolved from the script. +func TestValidateDesignProperties_NativePageUsesNativeDefinitions(t *testing.T) { + dir := t.TempDir() + writeAtlasColorTheme(t, dir) + mpr := filepath.Join(dir, "App.mpr") + for _, tc := range []struct { + layoutType string + wantWarn bool + }{{"Default", false}, {"Responsive", true}} { // Responsive is the web control + t.Run(tc.layoutType, func(t *testing.T) { + prog, errs := visitor.Build("create layout M.L (LayoutType: " + tc.layoutType + ") {\n placeholder Main\n};\n" + + "create page M.P (Title: 'P', Layout: M.L) {\n" + + " dynamictext t (Content: 'x', DesignProperties: ('Color': 'Brand primary', 'Size': 'Small'))\n};") + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + var warned []string + for _, v := range ValidateDesignProperties(prog, mpr, nil) { + if v.RuleID == "MDL-WIDGET12" { + warned = append(warned, v.Message) + } + } + if got := len(warned) > 0; got != tc.wantWarn { + t.Errorf("layout type %s: MDL-WIDGET12 = %v, want warned=%v", tc.layoutType, warned, tc.wantWarn) + } + }) + } +} diff --git a/mdl/executor/theme_reader.go b/mdl/executor/theme_reader.go index 62293700ec..fe5ef43f5c 100644 --- a/mdl/executor/theme_reader.go +++ b/mdl/executor/theme_reader.go @@ -76,13 +76,32 @@ type ThemeRegistry struct { // WidgetProperties maps design-properties.json widget type key to its properties. // Keys: "Widget", "DivContainer", "Button", "DataGrid", pluggable widget IDs, etc. WidgetProperties map[string][]ThemeProperty + // NativeWidgetProperties are the same groups from the native + // design-properties.json files. A native page's widgets resolve against + // these (ForPlatform): the two platforms declare different types and options + // for the same key, so typing a native widget against the web definitions + // stored Atlas's native `'Color': 'Brand primary'` option as a custom colour + // (ako/mxcli#1679). + NativeWidgetProperties map[string][]ThemeProperty +} + +// ForPlatform returns the registry a document of that platform resolves its +// design properties against: the receiver for a web document, a view over the +// native definitions for a native one. Nil stays nil. +func (r *ThemeRegistry) ForPlatform(native bool) *ThemeRegistry { + if r == nil || !native { + return r + } + return &ThemeRegistry{WidgetProperties: r.NativeWidgetProperties, NativeWidgetProperties: r.NativeWidgetProperties} } // loadThemeRegistry reads and merges all design-properties.json files from the project's -// themesource directories (themesource/*/web/design-properties.json). +// themesource directories (themesource/*/web/design-properties.json, and the +// native ones into NativeWidgetProperties). func loadThemeRegistry(projectDir string) (*ThemeRegistry, error) { registry := &ThemeRegistry{ - WidgetProperties: make(map[string][]ThemeProperty), + WidgetProperties: make(map[string][]ThemeProperty), + NativeWidgetProperties: make(map[string][]ThemeProperty), } themesourceDir := filepath.Join(projectDir, "themesource") @@ -100,24 +119,25 @@ func loadThemeRegistry(projectDir string) (*ThemeRegistry, error) { if !entry.IsDir() { continue } - dpPath := filepath.Join(themesourceDir, entry.Name(), "web", "design-properties.json") - if _, err := os.Stat(dpPath); os.IsNotExist(err) { - continue - } - - data, err := os.ReadFile(dpPath) - if err != nil { - continue // Skip unreadable files - } + for platform, into := range map[string]map[string][]ThemeProperty{ + "web": registry.WidgetProperties, + "native": registry.NativeWidgetProperties, + } { + dpPath := filepath.Join(themesourceDir, entry.Name(), platform, "design-properties.json") + data, err := os.ReadFile(dpPath) + if err != nil { + continue // Absent or unreadable + } - fileProps, err := parseDesignPropertiesJSON(data) - if err != nil { - continue // Skip malformed files - } + fileProps, err := parseDesignPropertiesJSON(data) + if err != nil { + continue // Skip malformed files + } - // Merge into registry - for widgetType, props := range fileProps { - registry.WidgetProperties[widgetType] = append(registry.WidgetProperties[widgetType], props...) + // Merge into registry + for widgetType, props := range fileProps { + into[widgetType] = append(into[widgetType], props...) + } } } diff --git a/mdl/executor/validate_design_properties.go b/mdl/executor/validate_design_properties.go index 54610a8f05..8f06481569 100644 --- a/mdl/executor/validate_design_properties.go +++ b/mdl/executor/validate_design_properties.go @@ -9,7 +9,9 @@ import ( "strings" "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend" "github.com/mendixlabs/mxcli/mdl/linter" + "github.com/mendixlabs/mxcli/sdk/pages" ) // LoadThemeRegistryForProject loads the design-property registry for a project @@ -42,18 +44,72 @@ func LoadThemeRegistryForProject(mprPath string) *ThemeRegistry { // properties; otherwise there is no metadata to validate against and it is a // no-op. Compound (nested) entries are skipped — the registry does not model // their sub-properties, so validating them would produce false positives. -func ValidateDesignProperties(prog *ast.Program, projectPath string) []linter.Violation { +// +// A page on a native layout is checked against the native definitions; checked +// against the web ones, Atlas's native `'Color': 'Brand primary'` and `'Size': +// 'Smallest'` were reported as not allowed (ako/mxcli#1679). project opens the +// project to resolve a layout the script does not create; it may be nil or +// return nil, and a page whose layout cannot be resolved is checked as web. +func ValidateDesignProperties(prog *ast.Program, projectPath string, project func() backend.FullBackend) []linter.Violation { reg := LoadThemeRegistryForProject(projectPath) if reg == nil { return nil } + native := nativeLayoutResolver(prog, project) var out []linter.Violation for _, stmt := range prog.Statements { - out = append(out, ValidateDesignPropertiesForStatement(stmt, reg)...) + r := reg + if s, ok := stmt.(*ast.CreatePageStmtV3); ok && s.Layout != "" && native(s.Layout) { + r = reg.ForPlatform(true) + } + out = append(out, ValidateDesignPropertiesForStatement(stmt, r)...) } return out } +// nativeLayoutResolver answers whether a layout, by qualified name, is native: +// one the script creates with a native layout type, or one the project holds. +// The project is read once, on the first question. +func nativeLayoutResolver(prog *ast.Program, project func() backend.FullBackend) func(string) bool { + native := map[string]bool{} + for _, stmt := range prog.Statements { + if s, ok := stmt.(*ast.CreateLayoutStmt); ok { + if n, ok := platformForLayoutType(pages.LayoutType((&ast.WidgetV3{Properties: s.Properties}).GetStringProp("LayoutType"))); ok { + native[s.Name.String()] = n + } + } + } + loaded := project == nil + return func(qn string) bool { + if n, ok := native[qn]; ok { + return n + } + if loaded { + return false + } + loaded = true + b := project() + if b == nil { + return false + } + layouts, err := b.ListLayouts() + if err != nil { + return false + } + h, err := NewContainerHierarchyFromBackend(b) + if err != nil { + return false + } + for _, l := range layouts { + name := h.GetModuleName(h.FindModuleID(l.ContainerID)) + "." + l.Name + if _, scripted := native[name]; !scripted { + native[name] = l.Native + } + } + return native[qn] + } +} + // ValidateDesignPropertiesForStatement validates one statement's widget design // properties against a pre-loaded registry (LSP entry point). func ValidateDesignPropertiesForStatement(stmt ast.Statement, reg *ThemeRegistry) []linter.Violation { diff --git a/mdl/executor/validate_program.go b/mdl/executor/validate_program.go index 6b269effe8..320917b0df 100644 --- a/mdl/executor/validate_program.go +++ b/mdl/executor/validate_program.go @@ -182,7 +182,7 @@ func ValidateProgram(prog *ast.Program, projectPath string) []linter.Violation { // Validate design properties against the project's theme registry // (themesource design-properties.json) — flags unknown keys and invalid // option values, listing the allowed values. Only runs with --project. - violations = append(violations, ValidateDesignProperties(prog, projectPath)...) + violations = append(violations, ValidateDesignProperties(prog, projectPath, project)...) // The same question for ALTER STYLING, which that pass never looked at — the // one statement whose entire job is writing design properties, and the one diff --git a/mdl/executor/validate_snippetcall_appearance.go b/mdl/executor/validate_snippetcall_appearance.go index 54461ec0fd..861afb7655 100644 --- a/mdl/executor/validate_snippetcall_appearance.go +++ b/mdl/executor/validate_snippetcall_appearance.go @@ -198,5 +198,5 @@ func setsSnippetCallAppearance(ctx *ExecContext, m backend.PageMutator, widget, s, ok := value.(string) return !ok || strings.TrimSpace(s) != "" } - return designPropertyForStoredWidget(ctx.GetThemeRegistry(), m, widget, name) != nil + return designPropertyForStoredWidget(documentTheme(ctx, m), m, widget, name) != nil } From e41f6c159cd9c9dcf1d473f858335255891c699e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 23:16:00 +0000 Subject: [PATCH 04/14] =?UTF-8?q?fix:=20a=20page's=20own=20design=20proper?= =?UTF-8?q?ties=20survive=20describe=20=E2=86=92=20exec?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The design properties stored on a page itself (Forms$Page Appearance.DesignProperties) had no MDL spelling: describe printed none in the header and the writer stored an empty list, so a native page lost 'Spacing top', 'Spacing bottom' and 'Background color' on every rewrite. A page-level `alter page … set` refused them. The page header now takes `DesignProperties: ( … )`, typed against the theme's Document and Page groups (where Atlas Core declares them; a page does not take Widget's — mxbuild refuses Widget's 'Small' for a page's 'Spacing left' with CE6085). Describe prints it, a page-level `set ('Key': value)` writes it, and check -p validates it. Fixes ako/mxcli#1678 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DRueZhZoBML5iWuJTvUnjL --- ...-10-24-page-design-properties-dropped.json | 1 + cmd/mxcli/syntax/features_page.go | 4 +- docs/01-project/MDL_QUICK_REFERENCE.md | 2 + .../bug-tests/1678-page-design-properties.mdl | 36 ++++ mdl/ast/ast_page_v3.go | 9 +- .../page_design_properties_1678_test.go | 43 +++++ mdl/backend/modelsdk/page_write.go | 4 +- mdl/backend/pagemutator/mutator.go | 5 + mdl/executor/cmd_pages_builder_v3.go | 11 ++ mdl/executor/cmd_pages_describe.go | 5 + mdl/executor/design_property_routing.go | 5 + .../page_design_properties_1678_test.go | 177 ++++++++++++++++++ mdl/executor/theme_reader.go | 27 ++- mdl/executor/validate_design_properties.go | 13 +- mdl/grammar/domains/MDLPage.g4 | 1 + mdl/visitor/visitor_page_class_test.go | 18 ++ mdl/visitor/visitor_page_v3.go | 5 +- sdk/pages/pages.go | 11 +- 18 files changed, 362 insertions(+), 15 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-24-page-design-properties-dropped.json create mode 100644 mdl-examples/bug-tests/1678-page-design-properties.mdl create mode 100644 mdl/backend/modelsdk/page_design_properties_1678_test.go create mode 100644 mdl/executor/page_design_properties_1678_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-24-page-design-properties-dropped.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-24-page-design-properties-dropped.json new file mode 100644 index 0000000000..204f554f08 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-24-page-design-properties-dropped.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-10-24", "symptom": "describe → exec dropped the design properties stored on a page itself (Forms$Page Appearance.DesignProperties): native pages lost 'Spacing top': 'Inner large', 'Spacing bottom': 'Inner large', 'Background color': 'Background Primary'. describe printed none in the header, exec wrote an empty list, and a page-level `alter page … set ('Background color': …)` was refused as 'unsupported page-level property'", "cause": "MDL had no page-level spelling: the page header grammar had no DesignProperties, pages.Page no field, pageToGen passed nil to newAppearance, describe's header read only Class and Style, and page-level SET routed every key to applyPageLevelSetMut", "file": "mdl/grammar/domains/MDLPage.g4 (pageHeaderPropertyV3); mdl/visitor/visitor_page_v3.go; mdl/executor/cmd_pages_builder_v3.go (buildPageV3), cmd_pages_describe.go (page header); mdl/backend/modelsdk/page_write.go (pageToGen); mdl/executor/theme_reader.go (pageThemeKey: Document + Page groups); design_property_routing.go + mdl/backend/pagemutator/mutator.go (findStyleableWidget(\"\") is the page); validate_design_properties.go", "fix": "Add `DesignProperties: ( … )` to the page header, typed against the theme's Document and Page groups after the layout fixes the platform; describe prints it; page-level `alter page … set ('Key': value)` writes the page's own Appearance; check -p validates the header", "insight": "The values looked like a custom theme until every design-properties.json was grepped for the key across ALL groups: Atlas Core's native file has a 'Document' group no widget type is named after, and it holds exactly the reported values. Grep by property name, not by the group you expect. Which groups a page takes was settled by mxbuild, not by reading: a page-level 'Spacing left': 'Small' (valid in Widget, not in Document) is CE6085 'Unknown option Small', so a page takes Document and not Widget — including Widget made check warn on the reporter's own values", "refs": ["ako/mxcli#1678", "ako/mxcli#714", "ako/mxcli#1679"], "test": "TestCreatePage_DesignProperties (mdl/visitor/visitor_page_class_test.go); TestPageToGen_WritesPageDesignProperties (mdl/backend/modelsdk); TestPageDesignProperties_RoundTrip, TestPageDesignProperties_AlterPageLevelSet, TestValidateDesignProperties_PageHeader (mdl/executor/page_design_properties_1678_test.go); mdl-examples/bug-tests/1678-page-design-properties.mdl"} diff --git a/cmd/mxcli/syntax/features_page.go b/cmd/mxcli/syntax/features_page.go index f60495a7c0..b056202834 100644 --- a/cmd/mxcli/syntax/features_page.go +++ b/cmd/mxcli/syntax/features_page.go @@ -12,7 +12,7 @@ func init() { "page", "pages", "form", "UI", "user interface", "widget", "layout", "screen", }, - Syntax: "CREATE PAGE Module.Name [FOLDER 'FolderPath']\n (\n Title: 'Page Title',\n Layout: Module.LayoutName\n [, Params: ( $Param: Module.Entity )]\n [, Url: 'page-url']\n [, Variables: ( $var: Boolean = true )] -- the default is an expression, written bare\n [, PopupWidth: 800, PopupHeight: 480, PopupResizable: true]\n [, PopupCloseAction: cancelButton1]\n [, Class: 'css-class', Style: 'css: rule']\n )\n {\n -- widgets\n }", + Syntax: "CREATE PAGE Module.Name [FOLDER 'FolderPath']\n (\n Title: 'Page Title',\n Layout: Module.LayoutName\n [, Params: ( $Param: Module.Entity )]\n [, Url: 'page-url']\n [, Variables: ( $var: Boolean = true )] -- the default is an expression, written bare\n [, PopupWidth: 800, PopupHeight: 480, PopupResizable: true]\n [, PopupCloseAction: cancelButton1]\n [, Class: 'css-class', Style: 'css: rule']\n [, DesignProperties: ('Spacing top': 'Inner large')] -- the page's own; Atlas\n -- declares them under Document and Page\n )\n {\n -- widgets\n }", Example: "CREATE PAGE MyModule.EditCustomer\n (\n Params: ( $Customer: MyModule.Customer ),\n Title: 'Edit Customer',\n Layout: Atlas_Core.PopupLayout,\n Class: 'container-fluid'\n )\n {\n DATAVIEW dvCustomer (DataSource: $Customer) {\n TEXTBOX txtName (Label: 'Name', Attribute: Name)\n FOOTER {\n ACTIONBUTTON btnSave (Caption: 'Save', Action: SAVE CHANGES, ButtonStyle: Primary)\n ACTIONBUTTON btnCancel (Caption: 'Cancel', Action: CANCEL CHANGES)\n }\n }\n };", SeeAlso: []string{"page.create", "page.widgets", "page.alter", "snippet", "rename"}, }) @@ -314,7 +314,7 @@ LIST IMPACT OF htmlelement; "drop template", "insert template", "list view template", "add parameter", "drop parameter", "page parameter", "add parameters", }, - Syntax: "ALTER PAGE Module.Name { -- the generic ALTER: set / insert / replace / drop\n SET (property: value) ON widgetName; -- widget property names: any casing\n SET ('Row size': 'Small') ON lvOrders; -- an Atlas DESIGN property of that widget's\n -- type; quoted and case-sensitive.\n -- `list design properties for ` lists\n -- them. ON/OFF for a toggle, where OFF\n -- REMOVES the entry.\n -- A multi-select ('Hide on') or compound\n -- ('Spacing') one needs the inline\n -- DesignProperties: (...) form, because a\n -- SET assignment carries one value.\n SET (Action: CALL MICROFLOW Module.MF) ON btnSave; -- any CREATE PAGE action form\n SET ('createFileAction': CALL MICROFLOW Module.MF) ON fileUploader1;\n -- a pluggable widget's NAMED action slot,\n -- by the widget's own key; refused on a\n -- key that is not action-typed\n SET (DataSource: $Param) ON dvOrder; -- parameter/microflow/nanoflow/selection;\n -- DATABASE and association are REPLACE-only,\n -- and a data view takes no database source\n SET (RenderMode: H2) ON txtTitle; -- dynamic text: Text | Paragraph | H1..H6\n SET (NativeTextStyle: Heading2) ON txtTitle; -- dynamic text on a native page:\n -- Text | Heading1..Heading6\n SET (prop1: val1, prop2: val2) ON widgetName;\n SET (Title: 'New Title'); -- page-level (case-sensitive): no ON\n SET (Documentation: 'What this page is for.');\n SET (Class: 'css-class'); -- page-level CSS class / style\n SET (Style: 'css: rule');\n SET (PopupWidth: 800, PopupHeight: 480, PopupResizable: true); -- page-level pop-up\n INSERT AFTER widgetName { };\n INSERT BEFORE widgetName { };\n INSERT INTO containerName { };\n DROP name1, name2;\n DROP TEMPLATE FOR Module.Specialization IN listViewName;\n REPLACE widgetName WITH { };\n ADD PARAMETERS $Order: Module.Order; -- or a primitive: $Count: Integer.\n -- A page with a Url needs a {Order}\n -- segment: SET (Url: '…/{Order}') in the\n -- same statement (CE5601 otherwise).\n -- A snippet takes entities only (MDL087).\n DROP PARAMETERS $Order; -- refused while a data source or an\n -- expression on the page still uses it\n ADD VARIABLES $show: Boolean = true;\n DROP VARIABLES $show;\n};\n-- A target is a widget name, or a DataGrid 2 column by what it shows:\n-- grid column(Attr), grid column('Caption'), @n when two match; or a\n-- navigation list item: nav item(widgetInIt), nav item('Text'), nav item@n. The old spellings `SET p = v`,\n-- `SET p: v` (no parentheses) and `DROP WIDGET a` still run and warn\n-- (MDL-DEPR101..103).\n\n-- The BULK form: one design property on every widget of a TYPE.\nALTER PAGES [IN Module]\n SET 'Compact' = ON, 'Striped' = ON\n WHERE WIDGETTYPE = datagrid -- the MDL keyword, which resolves to\n -- exactly one widget id. A full id in\n -- quotes works too. NOT a name: a widget\n -- name is unique only within its page.\n [DRY RUN]; -- run this FIRST. It reports the matches\n -- against a discardable copy and writes\n -- nothing.", + Syntax: "ALTER PAGE Module.Name { -- the generic ALTER: set / insert / replace / drop\n SET (property: value) ON widgetName; -- widget property names: any casing\n SET ('Row size': 'Small') ON lvOrders; -- an Atlas DESIGN property of that widget's\n -- type; quoted and case-sensitive.\n -- `list design properties for ` lists\n -- them. ON/OFF for a toggle, where OFF\n -- REMOVES the entry.\n -- A multi-select ('Hide on') or compound\n -- ('Spacing') one needs the inline\n -- DesignProperties: (...) form, because a\n -- SET assignment carries one value.\n SET (Action: CALL MICROFLOW Module.MF) ON btnSave; -- any CREATE PAGE action form\n SET ('createFileAction': CALL MICROFLOW Module.MF) ON fileUploader1;\n -- a pluggable widget's NAMED action slot,\n -- by the widget's own key; refused on a\n -- key that is not action-typed\n SET (DataSource: $Param) ON dvOrder; -- parameter/microflow/nanoflow/selection;\n -- DATABASE and association are REPLACE-only,\n -- and a data view takes no database source\n SET (RenderMode: H2) ON txtTitle; -- dynamic text: Text | Paragraph | H1..H6\n SET (NativeTextStyle: Heading2) ON txtTitle; -- dynamic text on a native page:\n -- Text | Heading1..Heading6\n SET (prop1: val1, prop2: val2) ON widgetName;\n SET (Title: 'New Title'); -- page-level (case-sensitive): no ON\n SET (Documentation: 'What this page is for.');\n SET (Class: 'css-class'); -- page-level CSS class / style\n SET (Style: 'css: rule');\n SET (PopupWidth: 800, PopupHeight: 480, PopupResizable: true); -- page-level pop-up\n SET ('Background color': 'Background Primary'); -- the page's own design\n -- property: no ON; OFF removes a toggle\n INSERT AFTER widgetName { };\n INSERT BEFORE widgetName { };\n INSERT INTO containerName { };\n DROP name1, name2;\n DROP TEMPLATE FOR Module.Specialization IN listViewName;\n REPLACE widgetName WITH { };\n ADD PARAMETERS $Order: Module.Order; -- or a primitive: $Count: Integer.\n -- A page with a Url needs a {Order}\n -- segment: SET (Url: '…/{Order}') in the\n -- same statement (CE5601 otherwise).\n -- A snippet takes entities only (MDL087).\n DROP PARAMETERS $Order; -- refused while a data source or an\n -- expression on the page still uses it\n ADD VARIABLES $show: Boolean = true;\n DROP VARIABLES $show;\n};\n-- A target is a widget name, or a DataGrid 2 column by what it shows:\n-- grid column(Attr), grid column('Caption'), @n when two match; or a\n-- navigation list item: nav item(widgetInIt), nav item('Text'), nav item@n. The old spellings `SET p = v`,\n-- `SET p: v` (no parentheses) and `DROP WIDGET a` still run and warn\n-- (MDL-DEPR101..103).\n\n-- The BULK form: one design property on every widget of a TYPE.\nALTER PAGES [IN Module]\n SET 'Compact' = ON, 'Striped' = ON\n WHERE WIDGETTYPE = datagrid -- the MDL keyword, which resolves to\n -- exactly one widget id. A full id in\n -- quotes works too. NOT a name: a widget\n -- name is unique only within its page.\n [DRY RUN]; -- run this FIRST. It reports the matches\n -- against a discardable copy and writes\n -- nothing.", Example: "ALTER PAGE Module.EditPage {\n SET (Caption: 'Save & Close', ButtonStyle: Success) ON btnSave;\n INSERT AFTER txtName {\n TEXTBOX txtMiddleName (Label: 'Middle Name', Attribute: MiddleName)\n };\n DROP txtUnused;\n};", SeeAlso: []string{"page.create", "page.show", "snippet.alter"}, }) diff --git a/docs/01-project/MDL_QUICK_REFERENCE.md b/docs/01-project/MDL_QUICK_REFERENCE.md index 0e5f0b2eda..6dc0dd121a 100644 --- a/docs/01-project/MDL_QUICK_REFERENCE.md +++ b/docs/01-project/MDL_QUICK_REFERENCE.md @@ -1489,6 +1489,7 @@ MDL uses explicit property declarations for pages: |---------|-----------|---------| | Page properties | `(key: value, ...)` | `(title: 'Edit', layout: Atlas_Core.Atlas_Default)` | | Pop-up dimensions | `PopupWidth: n, PopupHeight: n, PopupResizable: bool` | `(Layout: Atlas_Core.PopupLayout, PopupWidth: 800, PopupHeight: 480, PopupResizable: true)` — case-sensitive; default 600×600 | +| Page design properties | `DesignProperties: ( 'Key': 'Value', … )` | `(Layout: Atlas_Core.NativePhone_Default, DesignProperties: ('Spacing top': 'Inner large'))` — the page's own, from the theme's Document and Page groups | | Pop-up close button | `PopupCloseAction: ` | `(Layout: Atlas_Core.PopupLayout, PopupCloseAction: cancelButton1)` — names a widget on this page. Not carried from the stored document on a rewrite: the statement rebuilds the widget tree, so a carried name could dangle | | DataView read-only style | `ReadOnlyStyle: Inherit\|Control\|Text` | `dataview dv (datasource: $O, ReadOnlyStyle: Text)` — a DataView's own, distinct from a checkbox's. **Control** is Studio Pro's default here, not Inherit | | Page CSS class / style | `Class: 'css-class', Style: 'css: rule'` | `(Title: 'Home', Class: 'container-fluid bg-light', Style: 'min-height: 100vh')` — the page's Appearance | @@ -1731,6 +1732,7 @@ This is the generic ALTER — `alter Module.Name { set (Key: value) on { … }` blocks (issue #532). `Widgets` above is the // bare-body content, which binds to the Main placeholder. A `placeholder diff --git a/mdl/backend/modelsdk/page_design_properties_1678_test.go b/mdl/backend/modelsdk/page_design_properties_1678_test.go new file mode 100644 index 0000000000..1a71ae48f2 --- /dev/null +++ b/mdl/backend/modelsdk/page_design_properties_1678_test.go @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: Apache-2.0 + +// ako/mxcli#1678, the writer half: "exec writes an empty list" for the page's +// own Appearance.DesignProperties — pageToGen passed nil to newAppearance. +package modelsdkbackend + +import ( + "testing" + + "go.mongodb.org/mongo-driver/v2/bson" + + "github.com/mendixlabs/mxcli/mdl/backend/bsonnav" + "github.com/mendixlabs/mxcli/modelsdk/codec" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +func TestPageToGen_WritesPageDesignProperties(t *testing.T) { + page := &pages.Page{Name: "Home_Native", DesignProperties: []pages.DesignPropertyValue{ + {Key: "Background color", ValueType: "option", Option: "Background Primary"}, + {Key: "Hide header bottom border", ValueType: "toggle"}, + }} + g, err := pageToGen(page, nil) + if err != nil { + t.Fatal(err) + } + out, err := (&codec.Encoder{}).Encode(g) + if err != nil { + t.Fatal(err) + } + var d bson.D + if err := bson.Unmarshal(out, &d); err != nil { + t.Fatal(err) + } + var keys []string + for _, el := range bsonnav.DGetArrayElements(bsonnav.DGet(bsonnav.DGetDoc(d, "Appearance"), "DesignProperties")) { + if dp, ok := el.(bson.D); ok { + keys = append(keys, bsonnav.DGetString(dp, "Key")) + } + } + if len(keys) != 2 || keys[0] != "Background color" || keys[1] != "Hide header bottom border" { + t.Errorf("page Appearance.DesignProperties keys = %v, want the two authored", keys) + } +} diff --git a/mdl/backend/modelsdk/page_write.go b/mdl/backend/modelsdk/page_write.go index 5fd5ef1b46..6816fbaa75 100644 --- a/mdl/backend/modelsdk/page_write.go +++ b/mdl/backend/modelsdk/page_write.go @@ -306,7 +306,9 @@ func pageToGen(page *pages.Page, pv *types.ProjectVersion) (*genPg.Page, error) out.SetPopupResizable(page.PopupResizable) out.SetAllowedRolesQualifiedNames(moduleRoleNames(page.AllowedRoles)) out.SetTitle(captionToGen(page.Title)) - out.SetAppearance(newAppearance(page.Class, page.Style, "", nil)) + // The page's own design properties; nil here wrote an empty list over every + // stored page's (ako/mxcli#1678). + out.SetAppearance(newAppearance(page.Class, page.Style, "", page.DesignProperties)) if page.LayoutCall != nil { lc, err := layoutCallToGen(page.LayoutCall) diff --git a/mdl/backend/pagemutator/mutator.go b/mdl/backend/pagemutator/mutator.go index e1308756c0..e449a1a3fd 100644 --- a/mdl/backend/pagemutator/mutator.go +++ b/mdl/backend/pagemutator/mutator.go @@ -423,6 +423,11 @@ func (m *Mutator) ClearDesignProperties(widgetRef string) error { // findStyleableWidget locates a widget by name for design-property operations. func (m *Mutator) findStyleableWidget(widgetRef string) (bson.D, error) { + // No widget is the page itself: a page's own Appearance carries design + // properties too (ako/mxcli#1678). + if widgetRef == "" && m.containerType == backend.ContainerPage { + return m.rawData, nil + } result := m.widgetFinder(m.rawData, widgetRef) if result == nil { return nil, fmt.Errorf("widget %q not found", widgetRef) diff --git a/mdl/executor/cmd_pages_builder_v3.go b/mdl/executor/cmd_pages_builder_v3.go index 70c37b5500..8712ec244e 100644 --- a/mdl/executor/cmd_pages_builder_v3.go +++ b/mdl/executor/cmd_pages_builder_v3.go @@ -110,6 +110,17 @@ func (pb *pageBuilder) buildPageV3(s *ast.CreatePageStmtV3) (*pages.Page, error) } } + // The page's own design properties, typed against the theme's "Page" group — + // after the layout, which decides the platform (ako/mxcli#1678). + if len(s.DesignProperties) > 0 { + dps, err := designPropertyValuesV3(&ast.WidgetV3{Name: s.Name.Name, + Properties: map[string]any{"DesignProperties": s.DesignProperties}}, pageThemeKey, pb.themeRegistry) + if err != nil { + return nil, fmt.Errorf("page %s: %w", s.Name.String(), err) + } + page.DesignProperties = dps + } + // Build parameters for _, param := range s.Parameters { pageParam := &pages.PageParameter{ diff --git a/mdl/executor/cmd_pages_describe.go b/mdl/executor/cmd_pages_describe.go index 66ee4bba7e..dbdb81edbc 100644 --- a/mdl/executor/cmd_pages_describe.go +++ b/mdl/executor/cmd_pages_describe.go @@ -127,6 +127,11 @@ func describePage(ctx *ExecContext, name ast.QualifiedName) error { if st, _ := ap["Style"].(string); st != "" { props = append(props, fmt.Sprintf("Style: %s", mdlQuote(ctx, st))) } + // The page's own design properties; unprinted, a rewrite dropped + // them (ako/mxcli#1678). + if dps := extractDesignProperties(ap); len(dps) > 0 { + props = append(props, formatDesignPropertiesMDL(ctx, dps)) + } } } if len(foundPage.Parameters) > 0 { diff --git a/mdl/executor/design_property_routing.go b/mdl/executor/design_property_routing.go index e79cc9b946..cfd1082492 100644 --- a/mdl/executor/design_property_routing.go +++ b/mdl/executor/design_property_routing.go @@ -140,6 +140,11 @@ func designPropertyForStoredWidget(theme *ThemeRegistry, mutator backend.PageMut return nil } dpKey := storedWidgetDesignPropsKey(mutator, widgetRef) + if widgetRef == "" && mutator != nil && mutator.ContainerType() == backend.ContainerPage { + // A page-level SET: the page's own design properties, which the theme + // declares under "Page" (ako/mxcli#1678). + dpKey = pageThemeKey + } if dpKey == "" { return nil } diff --git a/mdl/executor/page_design_properties_1678_test.go b/mdl/executor/page_design_properties_1678_test.go new file mode 100644 index 0000000000..a4d1eb0903 --- /dev/null +++ b/mdl/executor/page_design_properties_1678_test.go @@ -0,0 +1,177 @@ +// SPDX-License-Identifier: Apache-2.0 + +// ako/mxcli#1678: "the design properties stored on the page itself (`Forms$Page` +// `Appearance.DesignProperties`)" — `'Spacing top': 'Inner large'` on +// Login_Native, `'Background color': 'Background Primary'` on Home_Native — were +// dropped: "`describe page` prints none of them in the page header, and exec +// writes an empty list." +package executor + +import ( + "bytes" + "context" + "os" + "path/filepath" + "strings" + "testing" + + "go.mongodb.org/mongo-driver/v2/bson" + + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// The groups a page's own design properties come from, cut down from Atlas Core +// 4.1.3's native design-properties.json: "Document" holds the reported 'Spacing +// top' and 'Background color', "Page" a toggle only pages take. "Widget" has a +// 'Spacing top' too, with other options — a page does not take it. +const pageDesignPropsTheme = `{ + "Document": [ + {"name": "Spacing top", "type": "Dropdown", "options": [{"name": "Inner large", "class": "spacingInnerTopLarge"}]}, + {"name": "Background color", "type": "Dropdown", "options": [{"name": "Background Primary", "class": "backgroundPrimary"}]}], + "Page": [{"name": "Hide header bottom border", "type": "Toggle", "class": "pageHeaderBorderNone"}], + "Widget": [{"name": "Spacing top", "type": "Dropdown", "options": [{"name": "Small", "class": "spacingOuterTopSmall"}]}]}` + +func storedPageDesignProperties(t *testing.T, exec *Executor, page string) map[string]string { + t.Helper() + ctx := exec.newExecContext(context.Background()) + h, err := getHierarchy(ctx) + if err != nil { + t.Fatal(err) + } + all, err := ctx.Backend.ListPages() + if err != nil { + t.Fatal(err) + } + for _, p := range all { + if p.Name != page || h.GetModuleName(h.FindModuleID(p.ContainerID)) != "MyFirstModule" { + continue + } + raw, err := ctx.Backend.GetRawUnitBytes(p.ID) + if err != nil { + t.Fatal(err) + } + var doc bson.D + if err := bson.Unmarshal(raw, &doc); err != nil { + t.Fatal(err) + } + out := map[string]string{} + app, _ := afGet(doc, "Appearance").(bson.D) + for _, el := range afList(afGet(app, "DesignProperties")) { + dp := el.(bson.D) + v, _ := afGet(dp, "Value").(bson.D) + out[afGet(dp, "Key").(string)] = afGet(v, "$Type").(string) + } + return out + } + t.Fatalf("page MyFirstModule.%s not found", page) + return nil +} + +func TestPageDesignProperties_RoundTrip(t *testing.T) { + exec, out := openPedAppFixture(t) + dir := filepath.Join(filepath.Dir(exec.newExecContext(context.Background()).MprPath), "themesource", "myfirstmodule", "native") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "design-properties.json"), []byte(pageDesignPropsTheme), 0o644); err != nil { + t.Fatal(err) + } + + src := "create page MyFirstModule.Home_Native (Title: 'Home', Layout: Atlas_Core.NativePhone_Default,\n" + + " DesignProperties: ('Background color': 'Background Primary', 'Hide header bottom border': on)) {\n" + + " dynamictext text4 (Content: 'JTS boat log')\n};" + if err := afRun(t, exec, src); err != nil { + t.Fatalf("exec: %v", err) + } + got := storedPageDesignProperties(t, exec, "Home_Native") + if got["Background color"] != "Forms$OptionDesignPropertyValue" || got["Hide header bottom border"] != "Forms$ToggleDesignPropertyValue" { + t.Fatalf("stored page design properties = %v, want an option and a toggle", got) + } + + out.Reset() + if err := afRun(t, exec, "describe page MyFirstModule.Home_Native;"); err != nil { + t.Fatal(err) + } + described := out.String() + header := described[:strings.Index(described, "{")] + if !strings.Contains(strings.ToLower(header), "designproperties: ('background color': 'background primary', 'hide header bottom border': on)") { + t.Fatalf("describe page header does not carry the page's design properties:\n%s", described) + } + + // GetPut: the description, executed unchanged, writes nothing. + out.Reset() + if err := afRun(t, exec, described); err != nil { + t.Fatalf("re-exec: %v", err) + } + if !bytes.Contains(out.Bytes(), []byte("Unchanged page")) { + t.Errorf("re-executing the description was not a no-op:\n%s", out.String()) + } + if got := storedPageDesignProperties(t, exec, "Home_Native"); len(got) != 2 { + t.Errorf("after the round trip the page has %v", got) + } +} + +// check -p reads the page's own design properties against the theme's "Page" +// group, as it reads a widget's against its type's. +func TestValidateDesignProperties_PageHeader(t *testing.T) { + dir := t.TempDir() + themeDir := filepath.Join(dir, "themesource", "atlas_core", "native") + if err := os.MkdirAll(themeDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(themeDir, "design-properties.json"), []byte(pageDesignPropsTheme), 0o644); err != nil { + t.Fatal(err) + } + for _, tc := range []struct { + props, want string + }{ + {"'Background color': 'Background Primary'", ""}, // control: declared key and value + {"'Spacing top': 'Inner large'", ""}, // Document's options, not Widget's + {"'Spacing top': 'Small'", "MDL-WIDGET12"}, // Widget's option: CE6085 in mxbuild + {"'Backgroundcolor': 'Background Primary'", "MDL-WIDGET11"}, + {"'Background color': 'Background primary'", "MDL-WIDGET12"}, + } { + prog, errs := visitor.Build("create layout M.L (LayoutType: Default) {\n placeholder Main\n};\n" + + "create page M.P (Title: 'P', Layout: M.L, DesignProperties: (" + tc.props + ")) {\n dynamictext t (Content: 'x')\n};") + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + var rules []string + for _, v := range ValidateDesignProperties(prog, filepath.Join(dir, "App.mpr"), nil) { + rules = append(rules, v.RuleID) + } + if got := strings.Join(rules, ","); got != tc.want { + t.Errorf("%s: rules = %q, want %q", tc.props, got, tc.want) + } + } +} + +// `alter page … set ('Key': value)` with no widget writes the page's own +// design properties. It was refused as "unsupported page-level property". +func TestPageDesignProperties_AlterPageLevelSet(t *testing.T) { + exec, _ := openPedAppFixture(t) + dir := filepath.Join(filepath.Dir(exec.newExecContext(context.Background()).MprPath), "themesource", "myfirstmodule", "native") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "design-properties.json"), []byte(pageDesignPropsTheme), 0o644); err != nil { + t.Fatal(err) + } + src := "create page MyFirstModule.Home_Native (Title: 'Home', Layout: Atlas_Core.NativePhone_Default) {\n" + + " dynamictext text4 (Content: 'JTS boat log')\n};\n" + + "alter page MyFirstModule.Home_Native {\n set ('Background color': 'Background Primary', 'Hide header bottom border': on)\n};" + if err := afRun(t, exec, src); err != nil { + t.Fatalf("exec: %v", err) + } + got := storedPageDesignProperties(t, exec, "Home_Native") + if got["Background color"] != "Forms$OptionDesignPropertyValue" || got["Hide header bottom border"] != "Forms$ToggleDesignPropertyValue" { + t.Fatalf("after the page-level set: %v", got) + } + // OFF removes a toggle, as on a widget. + if err := afRun(t, exec, "alter page MyFirstModule.Home_Native {\n set ('Hide header bottom border': off)\n};"); err != nil { + t.Fatal(err) + } + if got := storedPageDesignProperties(t, exec, "Home_Native"); len(got) != 1 { + t.Errorf("after OFF: %v, want only Background color", got) + } +} diff --git a/mdl/executor/theme_reader.go b/mdl/executor/theme_reader.go index fe5ef43f5c..9e4bc1003f 100644 --- a/mdl/executor/theme_reader.go +++ b/mdl/executor/theme_reader.go @@ -155,10 +155,22 @@ func parseDesignPropertiesJSON(data []byte) (map[string][]ThemeProperty, error) return fileProps, nil } +// pageThemeKey is the group a page's own design properties are declared in. A +// page does not inherit "Widget" but "Document": Atlas Core 4.1.3's native theme +// declares 'Spacing top' (Inner small/medium/large) and 'Background color' there, +// and mxbuild types a page's 'Spacing left' against Document's options, refusing +// Widget's 'Small' with CE6085 "Unknown option Small" (ako/mxcli#1678). +const pageThemeKey = "Page" + // GetPropertiesForWidget returns properties applicable to a widget type, -// including inherited "Widget" properties that apply to all widget types. +// including inherited "Widget" properties that apply to all widget types. For +// pageThemeKey it returns the page's: "Document", then "Page". func (r *ThemeRegistry) GetPropertiesForWidget(widgetTypeKey string) []ThemeProperty { var result []ThemeProperty + if widgetTypeKey == pageThemeKey { + result = append(result, r.WidgetProperties["Document"]...) + return append(result, r.WidgetProperties[pageThemeKey]...) + } // Add inherited "Widget" properties first (apply to all widgets) if widgetProps, ok := r.WidgetProperties["Widget"]; ok { @@ -175,6 +187,19 @@ func (r *ThemeRegistry) GetPropertiesForWidget(widgetTypeKey string) []ThemeProp return result } +// declaresGroup reports whether the theme defines design properties for key, +// so a validator knows the full property set rather than guessing. +func (r *ThemeRegistry) declaresGroup(key string) bool { + if _, ok := r.WidgetProperties[key]; ok { + return true + } + if key == pageThemeKey { + _, ok := r.WidgetProperties["Document"] + return ok + } + return false +} + // mdlKeywordStorageType maps each MDL keyword that builds a NATIVE widget to the // $Type the builder writes for it. The theme key is then read off that $Type // (bsonTypeToDesignPropsKey), so an inline widget and a stored one resolve diff --git a/mdl/executor/validate_design_properties.go b/mdl/executor/validate_design_properties.go index 8f06481569..5d3537f48f 100644 --- a/mdl/executor/validate_design_properties.go +++ b/mdl/executor/validate_design_properties.go @@ -24,7 +24,7 @@ func LoadThemeRegistryForProject(mprPath string) *ThemeRegistry { return nil } reg, err := loadThemeRegistry(filepath.Dir(mprPath)) - if err != nil || reg == nil || len(reg.WidgetProperties) == 0 { + if err != nil || reg == nil || (len(reg.WidgetProperties) == 0 && len(reg.NativeWidgetProperties) == 0) { return nil } return reg @@ -117,7 +117,14 @@ func ValidateDesignPropertiesForStatement(stmt ast.Statement, reg *ThemeRegistry return nil } if label, widgets, ok := documentWidgets(stmt); ok { - return validateDesignPropsTree(widgets, reg, label) + out := validateDesignPropsTree(widgets, reg, label) + // The page's own, typed against the theme's "Page" group (ako/mxcli#1678). + if s, isPage := stmt.(*ast.CreatePageStmtV3); isPage && len(s.DesignProperties) > 0 { + page := &ast.WidgetV3{Type: "page", Name: s.Name.Name, + Properties: map[string]any{"DesignProperties": s.DesignProperties}} + out = append(out, validateDesignPropsAs(page, pageThemeKey, reg, label)...) + } + return out } switch s := stmt.(type) { case *ast.AlterPageStmt: @@ -258,7 +265,7 @@ func validateDesignPropsAs(w *ast.WidgetV3, key string, reg *ThemeRegistry, loca // Only validate widgets we have type-specific metadata for. For an unknown // widget type (e.g. a pluggable widget not in the theme registry) we don't // know the full property set, so we must not flag its keys as unknown. - if _, ok := reg.WidgetProperties[key]; !ok { + if !reg.declaresGroup(key) { return nil } props := reg.GetPropertiesForWidget(key) diff --git a/mdl/grammar/domains/MDLPage.g4 b/mdl/grammar/domains/MDLPage.g4 index e48964f586..a88356c02e 100644 --- a/mdl/grammar/domains/MDLPage.g4 +++ b/mdl/grammar/domains/MDLPage.g4 @@ -259,6 +259,7 @@ pageHeaderPropertyV3 | FOLDER COLON /* @alias MDL-DEPR105 */ STRING_LITERAL // Folder: 'Pages/Admin' | CLASS COLON STRING_LITERAL // Class: 'my-page bg-primary' | STYLE COLON STRING_LITERAL // Style: 'padding: 10px' + | DESIGNPROPERTIES COLON designPropertyListV3 // DesignProperties: ( 'Spacing top': 'Inner large' ) — the page's own (ako/mxcli#1678) | IDENTIFIER COLON propertyValueV3 // Generic page property: PopupWidth: 800, PopupResizable: true ; diff --git a/mdl/visitor/visitor_page_class_test.go b/mdl/visitor/visitor_page_class_test.go index fde4364589..cd00afed1c 100644 --- a/mdl/visitor/visitor_page_class_test.go +++ b/mdl/visitor/visitor_page_class_test.go @@ -39,3 +39,21 @@ func TestCreatePage_ClassOmitted(t *testing.T) { t.Errorf("expected empty Class/Style, got %q/%q", stmt.Class, stmt.Style) } } + +// ako/mxcli#1678 — the page's own design properties, written in the header with +// the same list a widget takes. +func TestCreatePage_DesignProperties(t *testing.T) { + prog, errs := Build(`create page M.P ( + Title: 'x', + Layout: Atlas_Core.NativePhone_Default, + DesignProperties: ('Spacing top': 'Inner large', 'Hide header bottom border': on) + ) { dynamictext t (Content: 'x') };`) + if len(errs) > 0 { + t.Fatalf("unexpected parse errors: %v", errs) + } + got := prog.Statements[0].(*ast.CreatePageStmtV3).DesignProperties + if len(got) != 2 || got[0].Key != "Spacing top" || got[0].Value != "Inner large" || + got[1].Key != "Hide header bottom border" || got[1].Value != "on" { + t.Errorf("DesignProperties = %+v", got) + } +} diff --git a/mdl/visitor/visitor_page_v3.go b/mdl/visitor/visitor_page_v3.go index 9ddaf93ec9..d10ccc5cbd 100644 --- a/mdl/visitor/visitor_page_v3.go +++ b/mdl/visitor/visitor_page_v3.go @@ -144,6 +144,9 @@ func (b *Builder) parsePageHeaderV3(ctx parser.IPageHeaderV3Context, stmt *ast.C if str := prop.STRING_LITERAL(); str != nil { stmt.Style = unquoteStringLit(str) } + } else if dp := prop.DesignPropertyListV3(); dp != nil { + // DesignProperties: ( … ) — the page's own (ako/mxcli#1678) + stmt.DesignProperties = buildDesignPropertyListV3(dp) } else if id := prop.IDENTIFIER(); id != nil { // Generic page header property (e.g. PopupWidth: 800). Only the // pop-up dimensions are recognized; anything else is a typo/unsupported. @@ -183,7 +186,7 @@ func (b *Builder) applyGenericPageHeaderProp(stmt *ast.CreatePageStmtV3, name st stmt.PopupCloseAction = strings.Trim(fmt.Sprintf("%v", val), `"'`) default: b.addError(fmt.Errorf("line %d:%d: unknown page property %q "+ - "(supported: Title, Layout, Url, Folder, Params, Variables, PopupWidth, PopupHeight, PopupResizable, PopupCloseAction, Class, Style)", + "(supported: Title, Layout, Url, Folder, Params, Variables, PopupWidth, PopupHeight, PopupResizable, PopupCloseAction, Class, Style, DesignProperties)", tok.GetLine(), tok.GetColumn(), name)) } } diff --git a/sdk/pages/pages.go b/sdk/pages/pages.go index 00606e8c6c..3353e94488 100644 --- a/sdk/pages/pages.go +++ b/sdk/pages/pages.go @@ -28,10 +28,13 @@ type Page struct { PopupCloseAction string `json:"popupCloseAction,omitempty"` // Class / Style are the page's Forms$Appearance CSS class and inline style // (issue #714). - Class string `json:"class,omitempty"` - Style string `json:"style,omitempty"` - MarkAsUsed bool `json:"markAsUsed"` - Excluded bool `json:"excluded"` + Class string `json:"class,omitempty"` + Style string `json:"style,omitempty"` + // DesignProperties are the page's own Appearance.DesignProperties — Atlas + // declares them under "Page" (ako/mxcli#1678). + DesignProperties []DesignPropertyValue `json:"designProperties,omitempty"` + MarkAsUsed bool `json:"markAsUsed"` + Excluded bool `json:"excluded"` } // GetName returns the page's name. From 207909d82a891ab5f7b01eaf11a60fa49740f629 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 23:21:42 +0000 Subject: [PATCH 05/14] test: pin a Data grid 2 column's association-path template parameter ako/mxcli#1680 reported, on v0.28.0, that describe printed a Data grid 2 column's text-template parameter without its association step (`{1} = Hour` for a Stormglass_Weer/Hour binding), so exec resolved the attribute against the grid's entity: CE1613 at build. The mendixlabs/mxcli#1494 change after v0.28.0 fixed the one template- parameter reader, but its test covers only the path helper. This test runs the reported case on PedApp: a Studio Pro-shaped parameter (AttributeRef + IndirectEntityRef), described, re-executed ("Unchanged page"), with the EntityRef still in place. With the path reader disabled it fails with the reported `{1} = Hour` line; that description, executed on 11.13.0, is the reported CE1613. Closes ako/mxcli#1680 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DRueZhZoBML5iWuJTvUnjL --- ...grid2-column-template-association-path.mdl | 32 ++++++ ...g2_column_template_assoc_path_1680_test.go | 105 ++++++++++++++++++ 2 files changed, 137 insertions(+) create mode 100644 mdl-examples/bug-tests/1680-datagrid2-column-template-association-path.mdl create mode 100644 mdl/executor/dg2_column_template_assoc_path_1680_test.go diff --git a/mdl-examples/bug-tests/1680-datagrid2-column-template-association-path.mdl b/mdl-examples/bug-tests/1680-datagrid2-column-template-association-path.mdl new file mode 100644 index 0000000000..a906fff653 --- /dev/null +++ b/mdl-examples/bug-tests/1680-datagrid2-column-template-association-path.mdl @@ -0,0 +1,32 @@ +mdl 1; +-- ============================================================================ +-- ako/mxcli#1680 (reported on v0.28.0) — a Data grid 2 column's text-template +-- parameter lost its association path on describe → exec. describe printed the +-- column's `Attribute: Stormglass_Weer/Hour` with the path but the template as +-- `ContentParams: ({1} = Hour format (dateFormat: DateTime))`, so exec resolved +-- Hour against the grid's entity and mxbuild reported CE1613 "The selected +-- attribute '….Weer.Hour' no longer exists". +-- +-- Fixed after v0.28.0 by the mendixlabs/mxcli#1494 change to the one +-- template-parameter reader. Measured on 11.13.0: +-- +-- the v0.28.0 description, executed → CE1613 at the column +-- this page, described and re-executed → describe prints +-- `{1} = MyFirstModule.Stormglass_Weer/Hour`; +-- "Unchanged page"; mx check 0 errors +-- ============================================================================ + +create module BugGridTemplatePath; + +create persistent entity BugGridTemplatePath.Stormglass (Hour: DateTime); +create persistent entity BugGridTemplatePath.Weer (Plaats: String(100)); +create association BugGridTemplatePath.Stormglass_Weer + from BugGridTemplatePath.Weer to BugGridTemplatePath.Stormglass; + +create page BugGridTemplatePath.Weer_Overview (Title: 'Weer', Layout: Atlas_Core.Atlas_Default) { + datagrid grid1 (DataSource: database BugGridTemplatePath.Weer) { + column (Attribute: Plaats, Caption: 'Plaats') + column (Attribute: Stormglass_Weer/Hour, Caption: 'Hour', ShowContentAs: dynamicText, Content: '{1}', + ContentParams: ({1} = Stormglass_Weer/Hour format (dateFormat: DateTime))) + } +}; diff --git a/mdl/executor/dg2_column_template_assoc_path_1680_test.go b/mdl/executor/dg2_column_template_assoc_path_1680_test.go new file mode 100644 index 0000000000..137ad8c504 --- /dev/null +++ b/mdl/executor/dg2_column_template_assoc_path_1680_test.go @@ -0,0 +1,105 @@ +// SPDX-License-Identifier: Apache-2.0 + +// ako/mxcli#1680, reported against v0.28.0: "`describe` prints the column's own +// `Attribute: Stormglass_Weer/Hour` with the path, but the template as +// `ContentParams: ({1} = Hour format (dateFormat: DateTime))` without it. `exec` +// resolves `Hour` against the grid's entity" — 13 × CE1613 "The selected +// attribute 'JTSBootLogboek.Weer.Hour' no longer exists". +// +// Fixed after v0.28.0 by the mendixlabs/mxcli#1494 change, which taught the one +// template-parameter reader (extractTextTemplateParameters) the association +// path. Its test covers the path helper alone; this pins the Data grid 2 column +// the report was about, through describe and a re-exec, on a Studio Pro-shaped +// parameter (AttributeRef + IndirectEntityRef). +package executor + +import ( + "context" + "strings" + "testing" + + "go.mongodb.org/mongo-driver/v2/bson" +) + +// storedColumnTemplateRefs lists, for every template parameter on the page, +// its AttributeRef.Attribute and the $Type of its EntityRef ("" when null). +func storedColumnTemplateRefs(t *testing.T, exec *Executor, page string) [][2]string { + t.Helper() + ctx := exec.newExecContext(context.Background()) + h, err := getHierarchy(ctx) + if err != nil { + t.Fatal(err) + } + all, err := ctx.Backend.ListPages() + if err != nil { + t.Fatal(err) + } + for _, p := range all { + if p.Name != page || h.GetModuleName(h.FindModuleID(p.ContainerID)) != "MyFirstModule" { + continue + } + raw, err := ctx.Backend.GetRawUnitBytes(p.ID) + if err != nil { + t.Fatal(err) + } + var doc bson.D + if err := bson.Unmarshal(raw, &doc); err != nil { + t.Fatal(err) + } + var out [][2]string + walkBSON(doc, func(d bson.D) { + if afGet(d, "$Type") != "Forms$ClientTemplateParameter" { + return + } + ref, _ := afGet(d, "AttributeRef").(bson.D) + attr, _ := afGet(ref, "Attribute").(string) + entityRef, _ := afGet(ref, "EntityRef").(bson.D) + typ, _ := afGet(entityRef, "$Type").(string) + out = append(out, [2]string{attr, typ}) + }) + return out + } + t.Fatalf("page MyFirstModule.%s not found", page) + return nil +} + +func TestDataGrid2ColumnTemplateParamKeepsAssociationPath(t *testing.T) { + exec, out := openPedAppWithWidgets(t) + src := `create persistent entity MyFirstModule.Stormglass (Hour: DateTime); +create persistent entity MyFirstModule.Weer (Plaats: String(100)); +create association MyFirstModule.Stormglass_Weer from MyFirstModule.Weer to MyFirstModule.Stormglass; +create page MyFirstModule.Weer_Overview (Title: 'Weer', Layout: Atlas_Core.Atlas_Default) { + datagrid grid1 (DataSource: database MyFirstModule.Weer) { + column (Attribute: Stormglass_Weer/Hour, Caption: 'Hour', ShowContentAs: dynamicText, Content: '{1}', + ContentParams: ({1} = Stormglass_Weer/Hour format (dateFormat: DateTime))) + } +};` + if err := afRun(t, exec, src); err != nil { + t.Fatalf("exec: %v", err) + } + want := [2]string{"MyFirstModule.Stormglass.Hour", "DomainModels$IndirectEntityRef"} + if got := storedColumnTemplateRefs(t, exec, "Weer_Overview"); len(got) != 1 || got[0] != want { + t.Fatalf("stored template parameter = %v, want %v (the Studio Pro shape)", got, want) + } + + out.Reset() + if err := afRun(t, exec, "describe page MyFirstModule.Weer_Overview;"); err != nil { + t.Fatal(err) + } + described := out.String() + if !strings.Contains(described, "{1} = MyFirstModule.Stormglass_Weer/Hour format (dateFormat: DateTime)") { + t.Fatalf("describe dropped the template parameter's association path, so exec resolves Hour "+ + "against the grid's entity (CE1613):\n%s", described) + } + + out.Reset() + if err := afRun(t, exec, described); err != nil { + t.Fatalf("re-exec: %v", err) + } + if !strings.Contains(out.String(), "Unchanged page") { + t.Errorf("re-executing the description was not a no-op:\n%s", out.String()) + } + if got := storedColumnTemplateRefs(t, exec, "Weer_Overview"); len(got) != 1 || got[0] != want { + t.Errorf("after the round trip the parameter is %v, want %v", got, want) + } +} From 1f451cc148d16077b81ecdb579182b62de752671 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 23:49:02 +0000 Subject: [PATCH 06/14] fix: an import mapping call's input object and commit mode are expressible An import-with-mapping activity's ImportMappingCall.ParameterVariableName (the mapping's input object) and Commit had no MDL: the writer stored "" and YesWithoutEvents on every call, the reader read neither, and describe printed `import from mapping M.IMM($json) all;` without the object. mxcli could not call a mapping with an input object at all (mxbuild CE0244), and lint CONV020 reported the passed object as never committed. The activity takes an ADR-0013 settings list after its operand: `import from mapping M.IMM($Json) (Parameter: $Obj, Commit: No) all;`. It is checked in the visitor, written, read and described; exec refuses a missing parameter on a mapping that declares one (CE0244) and a parameter on one that does not (CE0246). The splice refused every statement over such a flow as "no result entity, which Mendix cannot load". An unbound result stored as a VoidType, or with no type, loads at 0 errors on mxbuild 11.13.0; only an object or list type naming no entity (mendixlabs/mxcli#1418) is refused now. Part of ako/mxcli#1681 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DRueZhZoBML5iWuJTvUnjL --- ...4-import-mapping-parameter-and-commit.json | 1 + .../json-structures-and-mappings/SKILL.md | 17 ++ cmd/mxcli/syntax/features_microflow.go | 11 +- docs/01-project/MDL_QUICK_REFERENCE.md | 2 +- ...81-import-mapping-parameter-and-commit.mdl | 68 ++++++ mdl/ast/ast_microflow.go | 8 + ...microflow_import_mapping_call_1681_test.go | 52 +++++ .../modelsdk/microflow_read_actions.go | 3 + mdl/backend/modelsdk/microflow_write.go | 7 +- mdl/executor/cmd_flow_modify.go | 10 +- mdl/executor/cmd_microflows_builder_calls.go | 14 ++ mdl/executor/cmd_microflows_format_action.go | 23 +++ .../import_mapping_parameter_1681_test.go | 193 ++++++++++++++++++ mdl/grammar/domains/MDLMicroflow.g4 | 17 +- .../conv_created_never_persisted_test.go | 7 +- mdl/visitor/visitor_import_export_mapping.go | 5 + .../visitor_import_mapping_settings.go | 104 ++++++++++ ...sitor_import_mapping_settings_1681_test.go | 75 +++++++ sdk/microflows/microflows_actions.go | 13 ++ 19 files changed, 621 insertions(+), 9 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-24-import-mapping-parameter-and-commit.json create mode 100644 mdl-examples/bug-tests/1681-import-mapping-parameter-and-commit.mdl create mode 100644 mdl/backend/modelsdk/microflow_import_mapping_call_1681_test.go create mode 100644 mdl/executor/import_mapping_parameter_1681_test.go create mode 100644 mdl/visitor/visitor_import_mapping_settings.go create mode 100644 mdl/visitor/visitor_import_mapping_settings_1681_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-24-import-mapping-parameter-and-commit.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-24-import-mapping-parameter-and-commit.json new file mode 100644 index 0000000000..4dd5b5a78c --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-24-import-mapping-parameter-and-commit.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-10-24", "symptom": "An import-with-mapping activity that passes the mapping's input object (ImportMappingCall.ParameterVariableName) described as `import from mapping M.IMM($json) all;` without it; v0.28.0 refused every rewrite of the flow as \"its stored `import from mapping …` activity has no result entity, which Mendix cannot load\"; lint CONV020 reported the passed object as never committed; and mxcli itself could not call such a mapping — it wrote an empty ParameterVariableName, which mxbuild refuses with CE0244", "cause": "The writer hard-coded Commit YesWithoutEvents and ParameterVariableName \"\", the reader read neither, so nothing above them could carry the object (CONV020's reflective scan never saw the variable name). storedImportWithoutEntity treated any empty result entity as the #1418 unloadable shape, including an unbound result stored as DataTypes$VoidType or with no VariableType, both of which mxbuild loads at 0 errors", "file": "mdl/grammar/domains/MDLMicroflow.g4 (importMappingSettings); mdl/visitor/visitor_import_mapping_settings.go; mdl/executor/cmd_microflows_builder_calls.go (addImportFromMappingAction: CE0244/CE0246 refusals), cmd_microflows_format_action.go (formatImportMappingSettings), cmd_flow_modify.go (storedImportWithoutEntity); mdl/backend/modelsdk/microflow_write.go, microflow_read_actions.go (readMappingCall); sdk/microflows ResultHandlingMapping.ParameterVariable/Commit/ResultType", "fix": "`import from mapping M.IMM($Json) (Parameter: $Obj, Commit: Yes|YesWithoutEvents|No)` — an ADR-0013 settings list after the operand, checked in the visitor; written, read and described; exec refuses a missing parameter on a mapping that declares one and a parameter on one that does not; the splice refuses only an object or list result type naming no entity", "insight": "No fixture had the shape (PedApp and TestApp hold no import activity with a parameter), so mxbuild was the oracle: build the activity with mxcli, then BSON-edit one property at a time and run mx check — missing parameter CE0244, unexpected one CE0246, undefined variable CE0109, unbound result as VoidType or with no VariableType 0 errors. That last measurement is what showed the 'no result entity' refusal was false: the #1418 guard keyed on the symptom (empty entity) rather than the shape that breaks loading (an Object/List type naming no entity). A lint false positive from a rule that scans every string is a READER bug: the variable name was never in the model", "refs": ["ako/mxcli#1681", "mendixlabs/mxcli#1418", "mendixlabs/mxcli#1518", "ako/mxcli#265"], "test": "TestImportFromMapping_Settings, TestImportFromMapping_SettingsRejectsBadInput (mdl/visitor); TestImportXmlActionToGen_ParameterAndCommit (mdl/backend/modelsdk); TestImportMappingParameterAndCommitRoundTrip, TestImportMappingParameterMismatchRefused, TestImportMappingVoidResultIsNotRefused (mdl/executor/import_mapping_parameter_1681_test.go); TestCreatedNeverPersisted 'import mapping parameter' (mdl/linter/rules); mdl-examples/bug-tests/1681-import-mapping-parameter-and-commit.mdl"} diff --git a/.claude/skills/mendix/json-structures-and-mappings/SKILL.md b/.claude/skills/mendix/json-structures-and-mappings/SKILL.md index 877b345d24..3bee336d6f 100644 --- a/.claude/skills/mendix/json-structures-and-mappings/SKILL.md +++ b/.claude/skills/mendix/json-structures-and-mappings/SKILL.md @@ -386,6 +386,9 @@ Using `parameter` without declaring one is refused — the build reports it as CE0279. The declared entity must match the microflow's parameter type, which the build checks as CE0282. +The **activity** that calls such a mapping passes the object in its settings +list, `(Parameter: $Obj)` — see "Settings" under Import from Mapping below. + --- ## Export Mappings @@ -516,6 +519,20 @@ $PetResponse = import from mapping Module.IMM_Pet($JsonContent); import from mapping Module.IMM_Pet($JsonContent); ``` +#### Settings — the input object and the commit mode + +A settings list after the source variable holds the activity's dialog: + +```sql +import from mapping Module.IMM_Embed($Json) (Parameter: $Collection); +$Pets = import from mapping Module.IMM_Pets($Json) (Commit: No) all; +``` + +- `Parameter: $Obj` — the mapping's input object. **Required** for a mapping + declared with `parameter Module.Entity` (without it mxbuild reports CE0244), + and refused on a mapping without one (CE0246); `exec` refuses both. +- `Commit: Yes | YesWithoutEvents | No` — omitted means YesWithoutEvents. + #### Range — how much of the result to bind Optional trailing clause, matching Studio Pro's **All / First / Custom** setting diff --git a/cmd/mxcli/syntax/features_microflow.go b/cmd/mxcli/syntax/features_microflow.go index 15a6c29015..be5bad0604 100644 --- a/cmd/mxcli/syntax/features_microflow.go +++ b/cmd/mxcli/syntax/features_microflow.go @@ -923,9 +923,17 @@ func init() { Keywords: []string{ "import from mapping", "export to mapping", "import mapping activity", "range", "all", "first", "limit", "offset", "single object", + "mapping parameter", "input object", "commit", }, - Syntax: "[$Var =] IMPORT FROM MAPPING Module.IMM ($SourceVar) [];\n" + + Syntax: "[$Var =] IMPORT FROM MAPPING Module.IMM ($SourceVar) [( )] [];\n" + "$Var = EXPORT TO MAPPING Module.EMM ($EntityVar);\n\n" + + " — the activity's dialog, any of:\n" + + " Parameter: $Obj the mapping's input object, for a mapping\n" + + " declared with `parameter Module.Entity`.\n" + + " Required there (mxbuild CE0244 without it),\n" + + " refused on any other mapping (CE0246)\n" + + " Commit: Yes | YesWithoutEvents | No\n" + + " omitted = YesWithoutEvents\n\n" + " — Studio Pro's Range on the import activity:\n" + " ALL bind the whole result\n" + " FIRST bind ONE object (not a one-element list)\n" + @@ -941,6 +949,7 @@ func init() { Example: "$Pets = import from mapping Shop.IMM_Pets($Json) all;\n" + "$Pet = import from mapping Shop.IMM_Pets($Json) first;\n" + "$Page = import from mapping Shop.IMM_Pets($Json) limit 10 offset 5;\n" + + "import from mapping Shop.IMM_Embed($Json) (Parameter: $Collection, Commit: Yes);\n" + "$Json2 = export to mapping Shop.EMM_Pet($Pet);", SeeAlso: []string{"import-mapping", "export-mapping", "json-structure"}, }) diff --git a/docs/01-project/MDL_QUICK_REFERENCE.md b/docs/01-project/MDL_QUICK_REFERENCE.md index 6dc0dd121a..6484cbc44f 100644 --- a/docs/01-project/MDL_QUICK_REFERENCE.md +++ b/docs/01-project/MDL_QUICK_REFERENCE.md @@ -625,7 +625,7 @@ it is for pages. | Return | `return $value;` | Required at end of every flow path, and never inside a loop (MDL062) | | List range (paging) | `$Page = range($List, , );` | **Offset first, then amount.** `range($L, 0, $N)` = first N; `range($L, $Off)` = skip and take the rest. At least one bound is required — a bare `range($L)` is CE6520 / MDL068. Stored as a `CustomRange` nested inside the `ListRange` | | Execute DB query | `$Result = execute database query Module.Conn.Query;` | 3-part name; supports DYNAMIC, params, CONNECTION override | -| Import mapping | `[$Var =] import from mapping Module.IMM($SourceVar) [all\|first\|limit [offset ]];` | Apply import mapping to string variable. Trailing clause is Studio Pro's Range; omitted = infer from the mapping's root. `first` binds one OBJECT (`limit 1` is a one-element LIST). Mendix rejects `offset` on a non-list mapping (CE6100), and `first` on an object-rooted one throws at runtime — both MDL-MAP04 | +| Import mapping | `[$Var =] import from mapping Module.IMM($SourceVar) [(Parameter: $Obj, Commit: Yes\|YesWithoutEvents\|No)] [all\|first\|limit [offset ]];` | Apply import mapping to string variable. Trailing clause is Studio Pro's Range; omitted = infer from the mapping's root. `first` binds one OBJECT (`limit 1` is a one-element LIST). Mendix rejects `offset` on a non-list mapping (CE6100), and `first` on an object-rooted one throws at runtime — both MDL-MAP04. The settings list is the activity's dialog: `Parameter` is the mapping's input object (required for a mapping declared with `parameter Module.Entity`, mxbuild CE0244; refused on any other, CE0246), `Commit` defaults to YesWithoutEvents. | | Export mapping | `$Var = export to mapping Module.EMM($EntityVar);` | Apply export mapping to entity, returns string | | Error handling | `... on error continue\|rollback\|[without rollback] begin handler end error;` | Goes on the activity that may fail — including `declare`, `set`, `change`, `log`, `show page`, `close page`, `show message` and `validation feedback`, which gained it in mendixlabs/mxcli#1078 so a Studio Pro handler survives DESCRIBE. `on error continue` is refused (MDL076) where Mendix raises CE6035: create, change, commit, log, show page, close page, show message, validation feedback — a custom `begin handler end error` is accepted on all of them. List operations, aggregates, `create list of`, `add`/`remove`, `cast` and `show home page` take a custom handler and `on error rollback` but not `continue` (MDL076); `rollback` takes every form (mendixlabs/mxcli#1252, measured on 11.12.3). Not supported on EXECUTE DATABASE QUERY. **In a nanoflow** `change`, `log`, `show page`, `close page`, `show message` and `validation feedback` are CE6035 in every form, and are refused; create, commit, the calls, the list activities and `cast` take only `on error without rollback begin … end error`. A handler that does not end in `return`/`raise error` merges back into the main flow, so a later variable is out of scope on the error path (CE0108) | | Re-raise the error | `raise error;` | **Inside an `on error begin … end error` handler only.** The error event re-raises the error being handled, so Mendix needs one in scope; Studio Pro will not draw the shape and mxbuild rejects it with **CE0710** "The main flow cannot join an error flow or end in an error event". On the main flow — at any nesting depth, and in a rule too — it is **MDL084**. Mendix has no main-flow "throw": call a Java action that throws | diff --git a/mdl-examples/bug-tests/1681-import-mapping-parameter-and-commit.mdl b/mdl-examples/bug-tests/1681-import-mapping-parameter-and-commit.mdl new file mode 100644 index 0000000000..80e9606db1 --- /dev/null +++ b/mdl-examples/bug-tests/1681-import-mapping-parameter-and-commit.mdl @@ -0,0 +1,68 @@ +mdl 1; +-- ============================================================================ +-- ako/mxcli#1681 — an import-with-mapping activity's input object and commit +-- mode had no MDL. JTSBootLogboek.GetWeer passes NewStormglass and +-- NewZeeweerActueelWeer to its mappings (ImportMappingCall.ParameterVariableName); +-- describe printed `import from mapping …($stormglass) all;` without them, and +-- v0.28.0 refused every rewrite of the flow, naming `create or modify` for a +-- `create or replace` and giving the reason as "no result entity". Lint CONV020 +-- reported both objects as never committed. +-- +-- Measured on mxbuild 11.13.0 against the mapping below: +-- +-- the call without a parameter CE0244 "The import mapping requires a +-- parameter but none is specified in the action." +-- a parameter on IMM_NoParam CE0246 "… does not accept a parameter …" +-- (Parameter: $Coll), bound or not 0 errors +-- an unbound result stored as a 0 errors — so "no result entity" was a +-- VoidType, or with no type at all false refusal; only an object or list type +-- naming no entity is unloadable (#1418) +-- +-- After: the settings list is written and described, describe → exec is +-- "Unchanged", exec refuses the CE0244 / CE0246 shapes, CONV020 is quiet. +-- ============================================================================ + +create module BugImportParam; + +create persistent entity BugImportParam.ChunkCollection (Name: String(100)); +create persistent entity BugImportParam.Chunk (Text: String(200)); +create association BugImportParam.Chunk_ChunkCollection + from BugImportParam.Chunk to BugImportParam.ChunkCollection type Reference; + +create json structure BugImportParam.JSON_Embed + sample $${"id": "x", "embeddings": [{"text": "t", "idx": 1}]}$$; + +create microflow BugImportParam.MF_FindChunk ($Collection: BugImportParam.ChunkCollection, $Index: Integer) +returns BugImportParam.Chunk +begin + return empty; +end; + +create import mapping BugImportParam.IMM_Embed + with json structure BugImportParam.JSON_Embed + parameter BugImportParam.ChunkCollection +{ + create BugImportParam.ChunkCollection { + Name = id, + find BugImportParam.Chunk_ChunkCollection/BugImportParam.Chunk + by BugImportParam.MF_FindChunk (Collection: parameter, Index: idx) + = embeddings { Text = text } + } +}; + +-- The reported shape: a new object handed to the mapping, the result unbound. +create microflow BugImportParam.GetWeer ($Json: String) returns Boolean as $Ok +begin + $NewColl = create BugImportParam.ChunkCollection (Name = 'weather'); + import from mapping BugImportParam.IMM_Embed($Json) (Parameter: $NewColl); + declare $Ok Boolean = true; + return $Ok; +end; + +-- A bound result and a non-default commit mode. +create microflow BugImportParam.ImportCommitted ($Json: String, $Coll: BugImportParam.ChunkCollection) +returns BugImportParam.ChunkCollection as $Result +begin + $Result = import from mapping BugImportParam.IMM_Embed($Json) (Parameter: $Coll, Commit: Yes); + return $Result; +end; diff --git a/mdl/ast/ast_microflow.go b/mdl/ast/ast_microflow.go index 5a4b9cece1..7a7729c82b 100644 --- a/mdl/ast/ast_microflow.go +++ b/mdl/ast/ast_microflow.go @@ -1409,6 +1409,14 @@ type ImportFromMappingStmt struct { First bool // FIRST — bind ONE object rather than a list LimitExpr Expression // LIMIT — Custom range OffsetExpr Expression // OFFSET — Custom range + + // The activity's settings list, `( Parameter: $Obj, Commit: No )` (ADR-0013, + // ako/mxcli#1681). Parameter is the variable holding the mapping's input + // object (ImportMappingCall.ParameterVariableName, without $); Commit is + // Yes, YesWithoutEvents or No. Empty means not authored: no parameter, and + // the writer's YesWithoutEvents. + Parameter string + Commit string } func (s *ImportFromMappingStmt) isMicroflowStatement() {} diff --git a/mdl/backend/modelsdk/microflow_import_mapping_call_1681_test.go b/mdl/backend/modelsdk/microflow_import_mapping_call_1681_test.go new file mode 100644 index 0000000000..90141b4c70 --- /dev/null +++ b/mdl/backend/modelsdk/microflow_import_mapping_call_1681_test.go @@ -0,0 +1,52 @@ +// SPDX-License-Identifier: Apache-2.0 + +// ako/mxcli#1681, the writer half: importXmlActionToGen wrote Commit +// YesWithoutEvents and an empty ParameterVariableName on every call, so a +// mapping with an input object could not be called (mxbuild CE0244 "The import +// mapping requires a parameter but none is specified in the action."). +package modelsdkbackend + +import ( + "testing" + + "go.mongodb.org/mongo-driver/v2/bson" + + "github.com/mendixlabs/mxcli/mdl/backend/bsonnav" + "github.com/mendixlabs/mxcli/modelsdk/codec" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +func encodeImportMappingCall(t *testing.T, rh *microflows.ResultHandlingMapping) bson.D { + t.Helper() + g := importXmlActionToGen(µflows.ImportXmlAction{XmlDocumentVariable: "Json", ResultHandling: rh}) + out, err := (&codec.Encoder{}).Encode(g) + if err != nil { + t.Fatal(err) + } + var d bson.D + if err := bson.Unmarshal(out, &d); err != nil { + t.Fatal(err) + } + return bsonnav.DGetDoc(bsonnav.DGetDoc(d, "ResultHandling"), "ImportMappingCall") +} + +func TestImportXmlActionToGen_ParameterAndCommit(t *testing.T) { + for _, tc := range []struct { + param, commit string + wantParam, wantCommit string + }{ + {"", "", "", "YesWithoutEvents"}, // control: today's defaults + {"Coll", "No", "Coll", "No"}, + {"NewStormglass", "Yes", "NewStormglass", "Yes"}, + } { + imc := encodeImportMappingCall(t, µflows.ResultHandlingMapping{ + MappingID: "M.IMM", ResultEntityID: "M.E", ParameterVariable: tc.param, Commit: tc.commit, + }) + if got := bsonnav.DGetString(imc, "ParameterVariableName"); got != tc.wantParam { + t.Errorf("param %q: ParameterVariableName = %q, want %q", tc.param, got, tc.wantParam) + } + if got := bsonnav.DGetString(imc, "Commit"); got != tc.wantCommit { + t.Errorf("commit %q: Commit = %q, want %q", tc.commit, got, tc.wantCommit) + } + } +} diff --git a/mdl/backend/modelsdk/microflow_read_actions.go b/mdl/backend/modelsdk/microflow_read_actions.go index a8b2d9f9e1..e7eddf3e7f 100644 --- a/mdl/backend/modelsdk/microflow_read_actions.go +++ b/mdl/backend/modelsdk/microflow_read_actions.go @@ -946,6 +946,8 @@ func readMappingCall(doc, imc bson.Raw) (h *microflows.ResultHandlingMapping, fo } h.MappingID = model.ID(mapping) h.ContentType = rawStr(imc, "ContentType") + h.ParameterVariable = rawStr(imc, "ParameterVariableName") + h.Commit = rawStr(imc, "Commit") force, _ = imc.Lookup("ForceSingleOccurrence").BooleanOK() h.ForceSingleOccurrence = &force // The Range is polymorphic: a ConstantRange carries SingleObject (All/First) @@ -969,6 +971,7 @@ func readMappingCall(doc, imc bson.Raw) (h *microflows.ResultHandlingMapping, fo h.ResultEntityID = model.ID(rawStr(vt, "Entity")) vtType = rawStr(vt, "$Type") } + h.ResultType = vtType return h, force, vtType } diff --git a/mdl/backend/modelsdk/microflow_write.go b/mdl/backend/modelsdk/microflow_write.go index e52e1c4437..b24e203d31 100644 --- a/mdl/backend/modelsdk/microflow_write.go +++ b/mdl/backend/modelsdk/microflow_write.go @@ -1035,11 +1035,14 @@ func importXmlActionToGen(a *microflows.ImportXmlAction) element.Element { } imc := newElem("Microflows$ImportMappingCall", "") - addStr(imc, "Commit", "YesWithoutEvents") + // Both were constants, so a mapping with an input object could not be + // called (mxbuild CE0244) and a non-default commit mode was unwritable + // (ako/mxcli#1681). + addStr(imc, "Commit", orDefault(rh.Commit, "YesWithoutEvents")) addStr(imc, "ContentType", orDefault(rh.ContentType, "Json")) addBool(imc, "ForceSingleOccurrence", forceSingle) addStr(imc, "ObjectHandlingBackup", "Create") - addStr(imc, "ParameterVariableName", "") + addStr(imc, "ParameterVariableName", rh.ParameterVariable) addPart(imc, "Range", importMappingRangeToGen(rh)) addStr(imc, "ReturnValueMapping", string(rh.MappingID)) diff --git a/mdl/executor/cmd_flow_modify.go b/mdl/executor/cmd_flow_modify.go index 4bbb04f721..c1689af6e0 100644 --- a/mdl/executor/cmd_flow_modify.go +++ b/mdl/executor/cmd_flow_modify.go @@ -414,7 +414,12 @@ func fallBack(ctx *ExecContext, d *flowDecl, v flowVerdict) (bool, error) { } // storedImportWithoutEntity returns the mapping of the first import-from-mapping -// activity in oc (loop bodies included) whose result entity is empty, or "". +// activity in oc (loop bodies included) whose result is an object or list type +// naming no entity, or "". That is the shape Mendix cannot load +// (mendixlabs/mxcli#1418). An activity that stores no result type, or a +// DataTypes$VoidType, binds no result and loads at 0 errors (measured on +// mxbuild 11.13.0); refusing it as "no result entity" refused every statement +// over a flow importing into a parameter object (ako/mxcli#1681). func storedImportWithoutEntity(oc *microflows.MicroflowObjectCollection) string { if oc == nil { return "" @@ -426,7 +431,8 @@ func storedImportWithoutEntity(oc *microflows.MicroflowObjectCollection) string if ix.ResultHandling == nil { return "(none)" } - if ix.ResultHandling.ResultEntityID == "" { + rt := ix.ResultHandling.ResultType + if ix.ResultHandling.ResultEntityID == "" && (rt == "DataTypes$ObjectType" || rt == "DataTypes$ListType") { return string(ix.ResultHandling.MappingID) } } diff --git a/mdl/executor/cmd_microflows_builder_calls.go b/mdl/executor/cmd_microflows_builder_calls.go index d4b1e65ed0..cf0b3cd390 100644 --- a/mdl/executor/cmd_microflows_builder_calls.go +++ b/mdl/executor/cmd_microflows_builder_calls.go @@ -2073,6 +2073,9 @@ func (fb *flowBuilder) addImportFromMappingAction(s *ast.ImportFromMappingStmt) MappingID: model.ID(s.Mapping.String()), ResultVariable: s.OutputVariable, SingleObject: true, + // The settings list (ako/mxcli#1681). + ParameterVariable: s.Parameter, + Commit: s.Commit, } // The RANGE and the RESULT VARIABLE's type are two independent axes, and @@ -2146,6 +2149,17 @@ func (fb *flowBuilder) addImportFromMappingAction(s *ast.ImportFromMappingStmt) case err != nil || im == nil: fb.addError("import from mapping %s: import mapping not found in the project (create it above this microflow; an activity without its result entity makes the project unloadable)", s.Mapping.String()) default: + // The call's input object must match the mapping's declaration: + // mxbuild refuses either mismatch, and check cannot see it without + // the mapping (ako/mxcli#1681). + switch { + case im.ParameterEntity != "" && s.Parameter == "": + fb.addError("import from mapping %s: the mapping takes an input object of %s; pass it with `(Parameter: $Obj)` (mxbuild refuses the call without one: CE0244)", + s.Mapping.String(), im.ParameterEntity) + case im.ParameterEntity == "" && s.Parameter != "": + fb.addError("import from mapping %s: the mapping declares no input object, so `Parameter: $%s` has nothing to bind (mxbuild CE0246)", + s.Mapping.String(), s.Parameter) + } if !rangeAuthored && fb.mappingRootIsList(im) { resultHandling.SingleObject = false } diff --git a/mdl/executor/cmd_microflows_format_action.go b/mdl/executor/cmd_microflows_format_action.go index 36cdbd362c..920d3e0ced 100644 --- a/mdl/executor/cmd_microflows_format_action.go +++ b/mdl/executor/cmd_microflows_format_action.go @@ -1796,12 +1796,35 @@ func formatImportXmlAction(ctx *ExecContext, a *microflows.ImportXmlAction, enti sb.WriteString("($") sb.WriteString(a.XmlDocumentVariable) sb.WriteString(")") + sb.WriteString(formatImportMappingSettings(a.ResultHandling)) sb.WriteString(formatImportMappingRange(a.ResultHandling)) sb.WriteString(";") return sb.String() } +// formatImportMappingSettings renders the activity's settings list, ` +// (Parameter: $Obj, Commit: No)`, or "" when it has neither. Unprinted, a +// rewrite dropped the input object and the commit mode (ako/mxcli#1681). +// YesWithoutEvents is what the writer stores for an unwritten Commit, so it is +// not printed. +func formatImportMappingSettings(h *microflows.ResultHandlingMapping) string { + if h == nil { + return "" + } + var parts []string + if h.ParameterVariable != "" { + parts = append(parts, "Parameter: $"+h.ParameterVariable) + } + if h.Commit != "" && h.Commit != "YesWithoutEvents" { + parts = append(parts, "Commit: "+h.Commit) + } + if len(parts) == 0 { + return "" + } + return " (" + strings.Join(parts, ", ") + ")" +} + // formatImportMappingRange renders the activity's Range — Studio Pro's // All / First / Custom setting. // diff --git a/mdl/executor/import_mapping_parameter_1681_test.go b/mdl/executor/import_mapping_parameter_1681_test.go new file mode 100644 index 0000000000..627469ccdf --- /dev/null +++ b/mdl/executor/import_mapping_parameter_1681_test.go @@ -0,0 +1,193 @@ +// SPDX-License-Identifier: Apache-2.0 + +// ako/mxcli#1681: an import-with-mapping activity's input object +// (ImportMappingCall.ParameterVariableName) and commit mode had no MDL. describe +// printed `import from mapping JTSBootLogboek.stormglass_import_mapping($stormglass) +// all;`, dropping the object, and v0.28.0 refused the statement under both +// `create or modify` and `create or replace` — naming `create or modify` either +// way, and giving the reason as "no result entity" for an activity that binds no +// result. +// +// Measured on mxbuild 11.13.0 with the #265 mapping (an input parameter): +// +// no parameter on the call CE0244 "The import mapping requires a +// parameter but none is specified in the action." +// a parameter on a mapping without one CE0246 "The import mapping does not +// accept a parameter but it is specified in the action." +// ParameterVariableName set 0 errors, bound or not +// unbound, VariableType DataTypes$VoidType 0 errors +package executor + +import ( + "context" + "strings" + "testing" + + "go.mongodb.org/mongo-driver/v2/bson" +) + +const imm1681Prelude = `create persistent entity MyFirstModule.ChunkCollection (Name: String(100)); +create persistent entity MyFirstModule.Chunk (Text: String(200)); +create association MyFirstModule.Chunk_ChunkCollection from MyFirstModule.Chunk to MyFirstModule.ChunkCollection type Reference; +create json structure MyFirstModule.JSON_Embed sample $${"id": "x", "embeddings": [{"text": "t", "idx": 1}]}$$; +create microflow MyFirstModule.MF_FindChunk ($Collection: MyFirstModule.ChunkCollection, $Index: Integer) +returns MyFirstModule.Chunk +begin + return empty; +end; +create import mapping MyFirstModule.IMM_Embed with json structure MyFirstModule.JSON_Embed + parameter MyFirstModule.ChunkCollection +{ + create MyFirstModule.ChunkCollection { + Name = id, + find MyFirstModule.Chunk_ChunkCollection/MyFirstModule.Chunk + by MyFirstModule.MF_FindChunk (Collection: parameter, Index: idx) + = embeddings { Text = text } + } +}; +create import mapping MyFirstModule.IMM_NoParam with json structure MyFirstModule.JSON_Embed +{ create MyFirstModule.ChunkCollection { Name = id } }; +` + +func imm1681Flow(name, importLine string) string { + return "create or modify microflow MyFirstModule." + name + + " ($Json: String, $Coll: MyFirstModule.ChunkCollection)\nbegin\n " + importLine + "\nend;\n" +} + +// storedImportCall returns the stored ImportMappingCall and result +// VariableType of a microflow's first import activity. +func storedImportCall(t *testing.T, exec *Executor, name string) (imc, varType bson.D) { + t.Helper() + _, raw := mfUnitByName(t, exec, "MyFirstModule", name) + if raw == nil { + t.Fatalf("microflow MyFirstModule.%s not found", name) + } + var d bson.D + if err := bson.Unmarshal(raw, &d); err != nil { + t.Fatal(err) + } + walkBSON(d, func(x bson.D) { + if afGet(x, "$Type") == "Microflows$ResultHandling" && imc == nil { + imc, _ = afGet(x, "ImportMappingCall").(bson.D) + varType, _ = afGet(x, "VariableType").(bson.D) + } + }) + return imc, varType +} + +func TestImportMappingParameterAndCommitRoundTrip(t *testing.T) { + exec, out := openPedAppFixture(t) + src := "mdl 1;\n" + imm1681Prelude + + imm1681Flow("MF_Bound", "$R = import from mapping MyFirstModule.IMM_Embed($Json) (Parameter: $Coll, Commit: No) all;") + + imm1681Flow("MF_Unbound", "import from mapping MyFirstModule.IMM_Embed($Json) (Parameter: $Coll);") + if err := agreeExec(t, exec, src); err != nil { + t.Fatalf("exec: %v", err) + } + for _, tc := range []struct{ flow, commit string }{{"MF_Bound", "No"}, {"MF_Unbound", "YesWithoutEvents"}} { + imc, _ := storedImportCall(t, exec, tc.flow) + if got := afGet(imc, "ParameterVariableName"); got != "Coll" { + t.Errorf("%s: ParameterVariableName = %v, want Coll (CE0244 without it)", tc.flow, got) + } + if got := afGet(imc, "Commit"); got != tc.commit { + t.Errorf("%s: Commit = %v, want %s", tc.flow, got, tc.commit) + } + + out.Reset() + if err := afRun(t, exec, "describe microflow MyFirstModule."+tc.flow+";"); err != nil { + t.Fatal(err) + } + described := out.String() + want := "import from mapping MyFirstModule.IMM_Embed($Json) (Parameter: $Coll" + if tc.commit == "No" { + want += ", Commit: No)" + } else { + want += ")" + } + if !strings.Contains(described, want) { + t.Fatalf("%s: describe does not carry the settings (%q):\n%s", tc.flow, want, described) + } + out.Reset() + if err := agreeExec(t, exec, described); err != nil { + t.Fatalf("%s: re-exec of the description: %v", tc.flow, err) + } + if !strings.Contains(out.String(), "Unchanged microflow") { + t.Errorf("%s: re-executing the description was not a no-op:\n%s", tc.flow, out.String()) + } + } +} + +// The two calls mxbuild refuses are refused at exec, naming what to write. +func TestImportMappingParameterMismatchRefused(t *testing.T) { + exec, _ := openPedAppFixture(t) + if err := agreeExec(t, exec, "mdl 1;\n"+imm1681Prelude); err != nil { + t.Fatal(err) + } + for _, tc := range []struct{ line, want string }{ + {"$R = import from mapping MyFirstModule.IMM_Embed($Json) all;", "CE0244"}, + {"$R = import from mapping MyFirstModule.IMM_NoParam($Json) (Parameter: $Coll) all;", "CE0246"}, + } { + err := agreeExec(t, exec, "mdl 1;\n"+imm1681Flow("MF_Bad", tc.line)) + if err == nil || !strings.Contains(err.Error(), tc.want) { + t.Errorf("%s: err = %v, want a refusal naming %s", tc.line, err, tc.want) + } + if id, _ := mfUnitByName(t, exec, "MyFirstModule", "MF_Bad"); id != "" { + t.Fatalf("%s: the refused microflow was written", tc.line) + } + } +} + +// An activity that binds no result may store no entity at all — a +// DataTypes$VoidType result, which mxbuild loads at 0 errors. v0.28.0 refused +// every statement over such a flow as "has no result entity, which Mendix +// cannot load". The #1418 shape that cannot be loaded — an object type naming +// no entity — is still refused (TestImportMappingEmptyStoredEntityIsNotUnchanged1418). +func TestImportMappingVoidResultIsNotRefused(t *testing.T) { + exec, out := openPedAppFixture(t) + flow := imm1681Flow("MF_Void", "import from mapping MyFirstModule.IMM_Embed($Json) (Parameter: $Coll);") + if err := agreeExec(t, exec, "mdl 1;\n"+imm1681Prelude+flow); err != nil { + t.Fatal(err) + } + id, raw := mfUnitByName(t, exec, "MyFirstModule", "MF_Void") + var d bson.D + if err := bson.Unmarshal(raw, &d); err != nil { + t.Fatal(err) + } + walkBSON(d, func(x bson.D) { + if afGet(x, "$Type") != "Microflows$ResultHandling" { + return + } + for i := range x { + if x[i].Key == "VariableType" { + vt := x[i].Value.(bson.D) + x[i].Value = bson.D{{Key: "$ID", Value: afGet(vt, "$ID")}, {Key: "$Type", Value: "DataTypes$VoidType"}} + } + } + }) + b, err := bson.Marshal(d) + if err != nil { + t.Fatal(err) + } + if err := exec.newExecContext(context.Background()).Backend.UpdateRawUnit(string(id), b); err != nil { + t.Fatal(err) + } + + // describe → exec, the reported round trip. + out.Reset() + if err := afRun(t, exec, "describe microflow MyFirstModule.MF_Void;"); err != nil { + t.Fatal(err) + } + described := out.String() + if !strings.Contains(described, "(Parameter: $Coll)") { + t.Fatalf("describe dropped the input object:\n%s", described) + } + out.Reset() + if err := agreeExec(t, exec, described); err != nil { + t.Fatalf("re-exec over a stored VoidType result: %v", err) + } + if !strings.Contains(out.String(), "Unchanged microflow") { + t.Errorf("want Unchanged, got:\n%s", out.String()) + } + if _, vt := storedImportCall(t, exec, "MF_Void"); afGet(vt, "$Type") != "DataTypes$VoidType" { + t.Errorf("the stored result type was rewritten to %v", afGet(vt, "$Type")) + } +} diff --git a/mdl/grammar/domains/MDLMicroflow.g4 b/mdl/grammar/domains/MDLMicroflow.g4 index c99f5ad332..866dcfdcc4 100644 --- a/mdl/grammar/domains/MDLMicroflow.g4 +++ b/mdl/grammar/domains/MDLMicroflow.g4 @@ -1004,14 +1004,29 @@ sendRestRequestBodyClause ; /** - * Import from mapping: [$Var =] IMPORT FROM MAPPING Module.IMM($SourceVar); + * Import from mapping: [$Var =] IMPORT FROM MAPPING Module.IMM($SourceVar) + * [( Parameter: $Obj, Commit: Yes | YesWithoutEvents | No )] [range]; */ importFromMappingStatement : (VARIABLE EQUALS)? IMPORT FROM MAPPING qualifiedName LPAREN VARIABLE RPAREN + importMappingSettings? importMappingRange? onErrorClause? ; +/** + * The activity's dialog settings, one ( Key: value, … ) list after the main + * operand (ADR-0013, ako/mxcli#1681): the mapping's input object and the + * commit mode. Keys and value shapes are checked by the visitor. + */ +importMappingSettings + : LPAREN importMappingSetting (COMMA importMappingSetting)* COMMA? RPAREN + ; + +importMappingSetting + : identifierOrKeyword COLON (VARIABLE | identifierOrKeyword) + ; + /** * How much of the mapping's result to bind — Mendix's "Range" on the import * activity, stored as Microflows$ConstantRange{SingleObject} or diff --git a/mdl/linter/rules/conv_created_never_persisted_test.go b/mdl/linter/rules/conv_created_never_persisted_test.go index c6bac0e040..9a2cf733eb 100644 --- a/mdl/linter/rules/conv_created_never_persisted_test.go +++ b/mdl/linter/rules/conv_created_never_persisted_test.go @@ -111,8 +111,11 @@ func TestCreatedNeverPersisted_EscapesAreNotFlagged(t *testing.T) { "javascript action argument": act(µflows.JavaScriptActionCallAction{JavaScriptAction: "Tmp.JSA_Save", ParameterMappings: []*microflows.JavaScriptActionParameterMapping{{Parameter: "Tmp.JSA_Save.Item", Value: µflows.BasicCodeActionParameterValue{Argument: "$o"}}}}), - "workflow context": act(µflows.WorkflowCallAction{Workflow: "Tmp.WF", WorkflowContextVariable: "o"}), - "export mapping": act(µflows.ExportMappingCallAction{MappingID: "m1", SourceVariable: "o", OutputVariable: "json"}), + "workflow context": act(µflows.WorkflowCallAction{Workflow: "Tmp.WF", WorkflowContextVariable: "o"}), + "export mapping": act(µflows.ExportMappingCallAction{MappingID: "m1", SourceVariable: "o", OutputVariable: "json"}), + // The mapping fills and commits its input object (ako/mxcli#1681). + "import mapping parameter": act(µflows.ImportXmlAction{XmlDocumentVariable: "json", + ResultHandling: µflows.ResultHandlingMapping{MappingID: "m1", ParameterVariable: "o"}}), "show page argument": act(µflows.ShowPageAction{PageName: "Tmp.Item_Edit", PageParameterMappings: []*microflows.PageParameterMapping{{Parameter: "Tmp.Item_Edit.Item", Argument: "$o"}}}), "show page passed obj": act(µflows.ShowPageAction{PageName: "Tmp.Item_Edit", PassedObject: "o"}), "added to a list": act(µflows.ChangeListAction{ChangeVariable: "Items", Type: microflows.ChangeListTypeAdd, Value: "$o"}), diff --git a/mdl/visitor/visitor_import_export_mapping.go b/mdl/visitor/visitor_import_export_mapping.go index 7c4e52220d..7eb4e3a01b 100644 --- a/mdl/visitor/visitor_import_export_mapping.go +++ b/mdl/visitor/visitor_import_export_mapping.go @@ -294,6 +294,11 @@ func buildImportFromMappingStatement(ctx antlr.ParserRuleContext) ast.MicroflowS stmt.SourceVariable = strings.TrimPrefix(vars[0].GetText(), "$") } + // The settings list, ( Parameter: $Obj, Commit: … ) (ako/mxcli#1681). + if settings := c.ImportMappingSettings(); settings != nil { + importMappingSettingsInto(stmt, settings) + } + // Range: FIRST, or LIMIT/OFFSET. Absent means "All", which leaves the builder // inferring cardinality from the mapping's root shape as it always has. (#881) if r := c.ImportMappingRange(); r != nil { diff --git a/mdl/visitor/visitor_import_mapping_settings.go b/mdl/visitor/visitor_import_mapping_settings.go new file mode 100644 index 0000000000..17af5eaacc --- /dev/null +++ b/mdl/visitor/visitor_import_mapping_settings.go @@ -0,0 +1,104 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "fmt" + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/grammar/parser" +) + +// ADR-0013: the import-with-mapping activity's dialog settings are one ( Key: +// value, … ) list after its main operand, `import from mapping M.IMM($Json) +// ( … )`. MDL could not say either before ako/mxcli#1681, so a mapping with an +// input object could not be called at all (mxbuild CE0244) and a describe of +// one dropped the object. New syntax, so an unknown key, a repeated key or a +// value of the wrong shape is an error under every language version (R11). +// +// Keys are the metamodel's (Microflows$ImportMappingCall): Parameter for +// ParameterVariableName, whose value is the variable holding the input object, +// and Commit, whose values are the CommitEnum members. +var importMappingSettingKeys = []string{"Parameter", "Commit"} + +var importMappingCommitValues = []string{"Yes", "YesWithoutEvents", "No"} + +// importMappingSettingsInto reads the list into stmt. It is lenient — a key or +// value it does not recognise is skipped — because the statement builders have +// no error channel; ExitImportFromMappingStatement reports it. +func importMappingSettingsInto(stmt *ast.ImportFromMappingStmt, settings parser.IImportMappingSettingsContext) { + sc, ok := settings.(*parser.ImportMappingSettingsContext) + if !ok || sc == nil { + return + } + for _, s := range sc.AllImportMappingSetting() { + key, variable, word := importMappingSettingParts(s) + switch canonicalKey(key, importMappingSettingKeys) { + case "Parameter": + if variable != "" { + stmt.Parameter = strings.TrimPrefix(variable, "$") + } + case "Commit": + stmt.Commit = canonicalKey(word, importMappingCommitValues) + } + } +} + +// importMappingSettingParts returns a setting's key and its value, as a +// variable ("$Obj") or as a word ("No"); the other is "". +func importMappingSettingParts(s parser.IImportMappingSettingContext) (key, variable, word string) { + c, ok := s.(*parser.ImportMappingSettingContext) + if !ok || c == nil { + return "", "", "" + } + keys := c.AllIdentifierOrKeyword() + if len(keys) == 0 { + return "", "", "" + } + key = identifierOrKeywordText(keys[0]) + if v := c.VARIABLE(); v != nil { + variable = v.GetText() + } else if len(keys) > 1 { + word = identifierOrKeywordText(keys[1]) + } + return key, variable, word +} + +func (b *Builder) ExitImportFromMappingStatement(ctx *parser.ImportFromMappingStatementContext) { + settings, ok := ctx.ImportMappingSettings().(*parser.ImportMappingSettingsContext) + if !ok || settings == nil { + return + } + const what = "import from mapping" + seen := map[string]bool{} + for _, s := range settings.AllImportMappingSetting() { + key, variable, word := importMappingSettingParts(s) + line := s.GetStart().GetLine() + canonical := canonicalKey(key, importMappingSettingKeys) + if canonical == "" { + b.addError(unknownKeyError(line, what, key, importMappingSettingKeys)) + continue + } + if seen[canonical] { + b.addError(fmt.Errorf("line %d: %s sets %s twice", line, what, canonical)) + continue + } + seen[canonical] = true + switch canonical { + case "Parameter": + if variable == "" { + b.addError(fmt.Errorf("line %d: %s: Parameter is a variable holding the mapping's input object, like $Obj, not %s", + line, what, word)) + } + case "Commit": + if canonicalKey(word, importMappingCommitValues) == "" { + shown := word + if variable != "" { + shown = variable + } + b.addError(fmt.Errorf("line %d: %s: Commit is Yes, YesWithoutEvents or No, not %s", line, what, shown)) + } + } + } +} diff --git a/mdl/visitor/visitor_import_mapping_settings_1681_test.go b/mdl/visitor/visitor_import_mapping_settings_1681_test.go new file mode 100644 index 0000000000..9470b02052 --- /dev/null +++ b/mdl/visitor/visitor_import_mapping_settings_1681_test.go @@ -0,0 +1,75 @@ +// SPDX-License-Identifier: Apache-2.0 + +// ako/mxcli#1681: "`Microflows$ImportMappingCall.ParameterVariableName` +// (`NewStormglass`, `NewZeeweerActueelWeer`) and `Commit: YesWithoutEvents` … +// MDL has no syntax for the parameter or the commit mode." +// +// The activity's dialog settings are one ( Key: value, … ) list after the main +// operand (ADR-0013), like send email's: `import from mapping M.IMM($Json) +// (Parameter: $Obj, Commit: No) all;`. New syntax, so an unknown key, a repeated +// key or a value of the wrong shape is an error under every language version. +package visitor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +func importStmtOf(t *testing.T, body string) (*ast.ImportFromMappingStmt, []error) { + t.Helper() + prog, errs := Build("create microflow M.F ($Json: String, $Obj: M.E) begin\n " + body + "\nend;") + if prog == nil || len(prog.Statements) == 0 { + return nil, errs + } + mf, ok := prog.Statements[0].(*ast.CreateMicroflowStmt) + if !ok || len(mf.Body) == 0 { + return nil, errs + } + s, _ := mf.Body[0].(*ast.ImportFromMappingStmt) + return s, errs +} + +func TestImportFromMapping_Settings(t *testing.T) { + for _, tc := range []struct { + body string + param, commit string + all, first, bound bool + }{ + {"$R = import from mapping M.IMM($Json) (Parameter: $Obj, Commit: No) all;", "Obj", "No", true, false, true}, + {"import from mapping M.IMM($Json) (Parameter: $Obj);", "Obj", "", false, false, false}, + {"import from mapping M.IMM($Json) (commit: yeswithoutevents) first;", "", "YesWithoutEvents", false, true, false}, + {"$R = import from mapping M.IMM($Json) (Commit: Yes,) all;", "", "Yes", true, false, true}, + // Control: no list leaves both unset, so the writer keeps its defaults. + {"$R = import from mapping M.IMM($Json) all;", "", "", true, false, true}, + } { + s, errs := importStmtOf(t, tc.body) + if len(errs) > 0 || s == nil { + t.Fatalf("%s: parse errors %v", tc.body, errs) + } + if s.Parameter != tc.param || s.Commit != tc.commit || s.All != tc.all || s.First != tc.first || + (s.OutputVariable != "") != tc.bound || s.SourceVariable != "Json" { + t.Errorf("%s: got Parameter=%q Commit=%q All=%v First=%v Output=%q Source=%q", + tc.body, s.Parameter, s.Commit, s.All, s.First, s.OutputVariable, s.SourceVariable) + } + } +} + +func TestImportFromMapping_SettingsRejectsBadInput(t *testing.T) { + for _, tc := range []struct{ body, want string }{ + {"import from mapping M.IMM($Json) (Parametr: $Obj);", "did you mean 'Parameter'"}, + {"import from mapping M.IMM($Json) (Commit: No, Commit: Yes);", "sets Commit twice"}, + {"import from mapping M.IMM($Json) (Commit: Maybe);", "Yes, YesWithoutEvents or No"}, + {"import from mapping M.IMM($Json) (Parameter: Obj);", "a variable"}, + } { + _, errs := importStmtOf(t, tc.body) + var msgs []string + for _, e := range errs { + msgs = append(msgs, e.Error()) + } + if !strings.Contains(strings.Join(msgs, "\n"), tc.want) { + t.Errorf("%s: errors %q, want one containing %q", tc.body, msgs, tc.want) + } + } +} diff --git a/sdk/microflows/microflows_actions.go b/sdk/microflows/microflows_actions.go index 96476662ca..b833362ff2 100644 --- a/sdk/microflows/microflows_actions.go +++ b/sdk/microflows/microflows_actions.go @@ -1171,6 +1171,19 @@ type ResultHandlingMapping struct { // selected import mapping cannot handle content type JSON" // (mendixlabs/mxcli#1232). Empty writes "Json". ContentType string `json:"contentType,omitempty"` + + // ParameterVariable is the variable holding the mapping's input object, + // ImportMappingCall.ParameterVariableName; empty for a mapping without one. + // Commit is the call's Yes / YesWithoutEvents / No; empty writes + // YesWithoutEvents. MDL could say neither before ako/mxcli#1681. + ParameterVariable string `json:"parameterVariable,omitempty"` + Commit string `json:"commit,omitempty"` + + // ResultType is the stored result VariableType's $Type, "" when none is + // stored. Read only: it tells a stored activity Mendix cannot load (an + // object or list type naming no entity, mendixlabs/mxcli#1418) from one that + // stores no result at all. + ResultType string `json:"resultType,omitempty"` } func (ResultHandlingMapping) isResultHandling() {} From 8daaa3571ff46511a8888bafc0b06ca59716eacd Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 25 Oct 2026 00:03:10 +0000 Subject: [PATCH 07/14] fix: a splice refusal names the statement as written MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `create or replace microflow` splices like `create or modify`, and the visitor folded the two into one flag, so a refusal said "create or modify microflow M.F: this change cannot be spliced …" for a statement spelled `create or replace`. The spelling is now recorded beside the flag for microflows and nanoflows and the refusal prints it. Closes ako/mxcli#1681 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DRueZhZoBML5iWuJTvUnjL --- ...lice-refusal-names-modify-for-replace.json | 1 + mdl/ast/ast_microflow.go | 7 ++++- mdl/executor/cmd_flow_modify.go | 23 +++++++++++---- mdl/executor/flow_verdict.go | 8 +++--- mdl/executor/flow_verdict_test.go | 28 +++++++++++++++++++ mdl/visitor/visitor_deprecations_test.go | 13 +++++++-- mdl/visitor/visitor_microflow.go | 2 ++ 7 files changed, 69 insertions(+), 13 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-24-splice-refusal-names-modify-for-replace.json diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-24-splice-refusal-names-modify-for-replace.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-24-splice-refusal-names-modify-for-replace.json new file mode 100644 index 0000000000..71acd2a792 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-24-splice-refusal-names-modify-for-replace.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-10-24", "symptom": "`create or replace microflow M.F …` refused by the splice said \"create or modify microflow M.F: this change cannot be spliced into the stored flow …\", naming a statement the author never wrote; same for nanoflows, and the same text in check's 'exec would refuse this statement'", "cause": "The visitor folds `or replace` into CreateOrModify (the two splice alike) and kept no record of the spelling, so flowRefusal had only a literal 'create or modify' to print", "file": "mdl/visitor/visitor_microflow.go (CreateOrReplace); mdl/ast/ast_microflow.go; mdl/executor/cmd_flow_modify.go (flowDecl.replace, verb); mdl/executor/flow_verdict.go (flowRefusal)", "fix": "Record CreateOrReplace beside CreateOrModify for microflows and nanoflows, carry it on flowDecl, and print the verb as written", "insight": "When one AST flag stands for two spellings, every message keyed on the flag names one of them wrongly; keep the spelling beside the meaning. A user who wrote `create or replace` and read 'create or modify' reasonably concluded that their statement was silently re-interpreted, which is the opposite of what the refusal is for", "refs": ["ako/mxcli#1681"], "test": "TestFlowRefusalNamesTheStatementAsWritten (mdl/executor/flow_verdict_test.go)"} diff --git a/mdl/ast/ast_microflow.go b/mdl/ast/ast_microflow.go index 7a7729c82b..9cb92ee9d0 100644 --- a/mdl/ast/ast_microflow.go +++ b/mdl/ast/ast_microflow.go @@ -65,7 +65,11 @@ type CreateMicroflowStmt struct { DocumentationSet bool Folder string // Folder path within module (e.g., "Resources/Images") CreateOrModify bool - Excluded bool // @excluded — document excluded from project + // CreateOrReplace records that CreateOrModify was spelled `create or + // replace`, which splices the same way, so a message names the statement + // as written (ako/mxcli#1681). + CreateOrReplace bool + Excluded bool // @excluded — document excluded from project // ApplyEntityAccess is Studio Pro's "apply entity access" checkbox, set by // `@applyentityaccess` / `@applyentityaccess(false)`. // @@ -163,6 +167,7 @@ type CreateNanoflowStmt struct { DocumentationSet bool // see mendixlabs/mxcli#1018: absent preserves, empty clears Folder string // Folder path within module CreateOrModify bool + CreateOrReplace bool // spelled `create or replace` (see CreateMicroflowStmt) Excluded bool // @excluded — document excluded from project // No ApplyEntityAccess: a nanoflow runs in the CLIENT and Mendix stores no // such property on Nanoflows$Nanoflow (modelsdk/gen declares the accessor diff --git a/mdl/executor/cmd_flow_modify.go b/mdl/executor/cmd_flow_modify.go index c1689af6e0..1625392db9 100644 --- a/mdl/executor/cmd_flow_modify.go +++ b/mdl/executor/cmd_flow_modify.go @@ -74,9 +74,12 @@ var flowRebuildRefused = langver.Change{ // NANOFLOW statement. type flowDecl struct { nanoflow bool - name ast.QualifiedName - body []ast.MicroflowStatement - folder string + // replace is a statement spelled `create or replace`, which splices as + // `create or modify` does; messages name it as written (ako/mxcli#1681). + replace bool + name ast.QualifiedName + body []ast.MicroflowStatement + folder string // returnVar is the `returns … as $Var` variable, which the end a body // falls through to returns; "" when there is none. returnVar string @@ -112,6 +115,14 @@ func (d *flowDecl) buildOnce(ctx *ExecContext) func() (any, []*microflows.Microf } } +// verb is the statement as written: `create or replace` or `create or modify`. +func (d *flowDecl) verb() string { + if d.replace { + return "create or replace" + } + return "create or modify" +} + func (d *flowDecl) kind() string { if d.nanoflow { return "nanoflow" @@ -652,7 +663,8 @@ func storedFolderOf(st ast.Statement) string { // microflowDecl adapts a CREATE MICROFLOW statement. func microflowDecl(s *ast.CreateMicroflowStmt) *flowDecl { return &flowDecl{ - name: s.Name, body: s.Body, folder: s.Folder, returnVar: returnVariable(s.ReturnType), params: s.Parameters, + replace: s.CreateOrReplace, + name: s.Name, body: s.Body, folder: s.Folder, returnVar: returnVariable(s.ReturnType), params: s.Parameters, build: func(ctx *ExecContext) (any, []*microflows.MicroflowParameter, map[string]string, error) { built, err := buildMicroflowFromStmt(ctx, s, buildFlowOpts{Quiet: true}) if err != nil { @@ -712,7 +724,8 @@ func returnVariable(rt *ast.MicroflowReturnType) string { // nanoflowDecl adapts a CREATE NANOFLOW statement. func nanoflowDecl(s *ast.CreateNanoflowStmt) *flowDecl { return &flowDecl{ - nanoflow: true, name: s.Name, body: s.Body, folder: s.Folder, returnVar: returnVariable(s.ReturnType), params: s.Parameters, + nanoflow: true, replace: s.CreateOrReplace, + name: s.Name, body: s.Body, folder: s.Folder, returnVar: returnVariable(s.ReturnType), params: s.Parameters, build: func(ctx *ExecContext) (any, []*microflows.MicroflowParameter, map[string]string, error) { built, err := buildNanoflowFromStmt(ctx, s, buildFlowOpts{Quiet: true}) if err != nil { diff --git a/mdl/executor/flow_verdict.go b/mdl/executor/flow_verdict.go index dc77a5925a..2d7226bc66 100644 --- a/mdl/executor/flow_verdict.go +++ b/mdl/executor/flow_verdict.go @@ -66,17 +66,17 @@ func flowRefusal(d *flowDecl, why error) error { // alter that makes the change. // The deliberate rebuild is a route here too (mendixlabs/mxcli#1517). return mdlerrors.NewValidation(fmt.Sprintf( - "create or modify %s %s: this change cannot be spliced into the stored flow: %v. "+ + "%s %s %s: this change cannot be spliced into the stored flow: %v. "+ "Nothing was written: rebuilding the whole flow instead would reset what Studio Pro drew "+ "(curves, merges, element IDs). %s. Or, %s", - d.kind(), d.name, why, strings.TrimSuffix(replaceLoopAdvice(d.kind(), d.name.String(), ns.loopHandle), "."), + d.verb(), d.kind(), d.name, why, strings.TrimSuffix(replaceLoopAdvice(d.kind(), d.name.String(), ns.loopHandle), "."), deliberateRebuildAdvice(d.kind()))) } return mdlerrors.NewValidation(fmt.Sprintf( - "create or modify %s %s: this change cannot be spliced into the stored flow: %v. "+ + "%s %s %s: this change cannot be spliced into the stored flow: %v. "+ "Nothing was written: rebuilding the whole flow instead would reset what Studio Pro drew "+ "(curves, merges, element IDs). Change activities with `alter %s %s { … }`; %s", - d.kind(), d.name, why, d.kind(), d.name, deliberateRebuildAdvice(d.kind()))) + d.verb(), d.kind(), d.name, why, d.kind(), d.name, deliberateRebuildAdvice(d.kind()))) } // deliberateRebuildAdvice is the route every splice refusal names for a diff --git a/mdl/executor/flow_verdict_test.go b/mdl/executor/flow_verdict_test.go index a86bc3b62e..b8bc180543 100644 --- a/mdl/executor/flow_verdict_test.go +++ b/mdl/executor/flow_verdict_test.go @@ -8,6 +8,7 @@ import ( "github.com/mendixlabs/mxcli/mdl/ast" "github.com/mendixlabs/mxcli/mdl/langver" + "github.com/mendixlabs/mxcli/mdl/visitor" ) // check reports the flow refusal under the rule ID exec's warning carries, so @@ -51,3 +52,30 @@ func TestFlowRefusalNamesTheFlowAndTheReason(t *testing.T) { t.Errorf("rebuild warning: %q", w) } } + +// ako/mxcli#1681: "The refusal names `create or modify` even for `create or +// replace`". The two splice alike, so the refusal must name the statement the +// author wrote, not the one it is treated as. +func TestFlowRefusalNamesTheStatementAsWritten(t *testing.T) { + why := cannotSplice("the Loop at (1, 2) changes inside its body") + for _, tc := range []struct{ src, want string }{ + {"create or replace microflow M.F () begin end;", "create or replace microflow M.F:"}, + {"create or modify microflow M.F () begin end;", "create or modify microflow M.F:"}, + {"create or replace nanoflow M.N () begin end;", "create or replace nanoflow M.N:"}, + } { + prog, errs := visitor.Build(tc.src) + if len(errs) > 0 { + t.Fatalf("parse %q: %v", tc.src, errs) + } + var d *flowDecl + switch s := prog.Statements[0].(type) { + case *ast.CreateMicroflowStmt: + d = microflowDecl(s) + case *ast.CreateNanoflowStmt: + d = nanoflowDecl(s) + } + if msg := flowRefusal(d, why).Error(); !strings.HasPrefix(msg, tc.want) { + t.Errorf("%s: refusal %q, want it to start with %q", tc.src, msg, tc.want) + } + } +} diff --git a/mdl/visitor/visitor_deprecations_test.go b/mdl/visitor/visitor_deprecations_test.go index c1b1290cd9..4a31373d53 100644 --- a/mdl/visitor/visitor_deprecations_test.go +++ b/mdl/visitor/visitor_deprecations_test.go @@ -116,9 +116,10 @@ var createOrReplaceCases = map[string]string{ "translations": "translations in Administration for nl_NL ('Save' as 'Opslaan');", } -// foldPageModeFlags erases the one AST difference between `or replace` and -// `or modify` that means nothing: pages, snippets and layouts keep both flags, -// and every reader in the executor tests `IsModify || IsReplace`. +// foldPageModeFlags erases the AST differences between `or replace` and `or +// modify` that mean nothing: pages, snippets and layouts keep both flags, and +// every reader in the executor tests `IsModify || IsReplace`; microflows and +// nanoflows keep the spelling, which only messages read. func foldPageModeFlags(stmts []ast.Statement) { for _, s := range stmts { switch n := s.(type) { @@ -128,6 +129,12 @@ func foldPageModeFlags(stmts []ast.Statement) { n.IsModify, n.IsReplace = n.IsModify || n.IsReplace, false case *ast.CreateLayoutStmt: n.IsModify, n.IsReplace = n.IsModify || n.IsReplace, false + case *ast.CreateMicroflowStmt: + // The spelling only, kept so a refusal names the statement as + // written (ako/mxcli#1681). + n.CreateOrReplace = false + case *ast.CreateNanoflowStmt: + n.CreateOrReplace = false } } } diff --git a/mdl/visitor/visitor_microflow.go b/mdl/visitor/visitor_microflow.go index dba92874f6..c498882cad 100644 --- a/mdl/visitor/visitor_microflow.go +++ b/mdl/visitor/visitor_microflow.go @@ -129,6 +129,7 @@ func (b *Builder) ExitCreateMicroflowStatement(ctx *parser.CreateMicroflowStatem if createStmt != nil { if createStmt.OR() != nil && (createStmt.MODIFY() != nil || createStmt.REPLACE() != nil) { stmt.CreateOrModify = true + stmt.CreateOrReplace = createStmt.REPLACE() != nil } for _, ann := range createStmt.AllAnnotation() { annCtx := ann.(*parser.AnnotationContext) @@ -189,6 +190,7 @@ func (b *Builder) ExitCreateNanoflowStatement(ctx *parser.CreateNanoflowStatemen if createStmt != nil { if createStmt.OR() != nil && (createStmt.MODIFY() != nil || createStmt.REPLACE() != nil) { stmt.CreateOrModify = true + stmt.CreateOrReplace = createStmt.REPLACE() != nil } for _, ann := range createStmt.AllAnnotation() { annCtx := ann.(*parser.AnnotationContext) From 516f9079e2f525b22f86011e84d94bdc33d1da3c Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 25 Oct 2026 00:04:15 +0000 Subject: [PATCH 08/14] docs(skills): refresh the mapping skill's section index The import-mapping settings section added for ako/mxcli#1681 moved the line ranges the generated index points at (scripts/skill-index.py). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DRueZhZoBML5iWuJTvUnjL --- .../json-structures-and-mappings/SKILL.md | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/.claude/skills/mendix/json-structures-and-mappings/SKILL.md b/.claude/skills/mendix/json-structures-and-mappings/SKILL.md index 3bee336d6f..a494b11f49 100644 --- a/.claude/skills/mendix/json-structures-and-mappings/SKILL.md +++ b/.claude/skills/mendix/json-structures-and-mappings/SKILL.md @@ -14,16 +14,16 @@ description: "Create and manage JSON structures, import mappings and export mapp | 111-122 | Export Mappings | | 123-187 | JSON Structures | | 188-229 | Message Definitions | -| 230-390 | Import Mappings | -| 391-488 | Export Mappings | -| 489-506 | Starting a Mapping Below the Payload Root | -| 507-576 | Microflow Actions | -| 577-589 | Browse | -| 590-637 | Export Workflow: PE → NPE → JSON | -| 638-644 | Realistic Example: Countries REST API | -| 645-668 | Placing Documents in Folders | -| 669-684 | Documentation | -| 685-699 | Common Mistakes | +| 230-393 | Import Mappings | +| 394-491 | Export Mappings | +| 492-509 | Starting a Mapping Below the Payload Root | +| 510-593 | Microflow Actions | +| 594-606 | Browse | +| 607-654 | Export Workflow: PE → NPE → JSON | +| 655-661 | Realistic Example: Countries REST API | +| 662-685 | Placing Documents in Folders | +| 686-701 | Documentation | +| 702-716 | Common Mistakes | # JSON Structures, Import Mappings & Export Mappings From b3141898eed86285fcb112dfa79cd5bcbbb51710 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 25 Oct 2026 00:05:43 +0000 Subject: [PATCH 09/14] docs(skills): refresh the demo-data skill's section index The generated index (scripts/skill-index.py) was stale on main; make build regenerates it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DRueZhZoBML5iWuJTvUnjL --- .claude/skills/mendix/demo-data/SKILL.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.claude/skills/mendix/demo-data/SKILL.md b/.claude/skills/mendix/demo-data/SKILL.md index a18614a9e9..2ccc94a4da 100644 --- a/.claude/skills/mendix/demo-data/SKILL.md +++ b/.claude/skills/mendix/demo-data/SKILL.md @@ -15,12 +15,12 @@ description: "Seed a Mendix app's PostgreSQL database directly, bypassing the ru | 43-76 | Step 1: Get Database Settings | | 77-103 | Step 2: Connect to the Database | | 104-165 | Step 3: Understand the Mendix ID System | -| 166-253 | Step 4: Check Association Storage and Optimistic Locking | -| 254-355 | Step 5: Insert Demo Data | -| 356-401 | Important Caveats | -| 402-447 | Quick Reference | -| 448-494 | Automated Alternative: IMPORT FROM | -| 495-499 | Related Skills | +| 166-260 | Step 4: Check Association Storage and Optimistic Locking | +| 261-362 | Step 5: Insert Demo Data | +| 363-408 | Important Caveats | +| 409-454 | Quick Reference | +| 455-501 | Automated Alternative: IMPORT FROM | +| 502-506 | Related Skills | # Skill: Connect to Application Database and Generate Demo Data From 3cafbec0c7cdbe6e69d4a523e673f60d8964407a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 25 Oct 2026 00:16:11 +0000 Subject: [PATCH 10/14] test: fold the create-or-replace spelling in the upgrade equivalence test fmt --upgrade rewrites `create or replace microflow` to `create or modify` and asserts the statement builds the same. The spelling flag recorded for ako/mxcli#1681 (only messages read it) differs by design, so it is folded like the page, snippet and layout mode flags. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DRueZhZoBML5iWuJTvUnjL --- mdl/upgrade/examples_test.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/mdl/upgrade/examples_test.go b/mdl/upgrade/examples_test.go index d646996c94..40d49c6bf7 100644 --- a/mdl/upgrade/examples_test.go +++ b/mdl/upgrade/examples_test.go @@ -89,10 +89,11 @@ func TestFormatterKeepsMeaning(t *testing.T) { } } -// foldModeFlags erases the one AST difference between `or replace` and -// `or modify` that means nothing: pages, snippets and layouts keep both flags, -// and every reader in the executor tests `IsModify || IsReplace` (pinned by -// mdl/visitor's TestCreateOrReplaceMatchesModifyExceptExemptKinds). +// foldModeFlags erases the AST differences between `or replace` and `or +// modify` that mean nothing: pages, snippets and layouts keep both flags, and +// every reader in the executor tests `IsModify || IsReplace`; microflows and +// nanoflows keep the spelling, which only messages read (ako/mxcli#1681). Pinned +// by mdl/visitor's TestCreateOrReplaceMatchesModifyExceptExemptKinds. func foldModeFlags(stmts []ast.Statement) { for _, s := range stmts { switch n := s.(type) { @@ -102,6 +103,10 @@ func foldModeFlags(stmts []ast.Statement) { n.IsModify, n.IsReplace = n.IsModify || n.IsReplace, false case *ast.CreateLayoutStmt: n.IsModify, n.IsReplace = n.IsModify || n.IsReplace, false + case *ast.CreateMicroflowStmt: + n.CreateOrReplace = false + case *ast.CreateNanoflowStmt: + n.CreateOrReplace = false } } } From 59e08c3e7acfeb5f5bbe4ca9134fb3035f489b8c Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 25 Oct 2026 00:16:24 +0000 Subject: [PATCH 11/14] style: gofmt splice_handler_test.go make lint's fmt-check failed on it on main as well. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DRueZhZoBML5iWuJTvUnjL --- mdl/backend/mfmutator/splice_handler_test.go | 24 ++++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/mdl/backend/mfmutator/splice_handler_test.go b/mdl/backend/mfmutator/splice_handler_test.go index 5f0df41dde..c4dbbee4c7 100644 --- a/mdl/backend/mfmutator/splice_handler_test.go +++ b/mdl/backend/mfmutator/splice_handler_test.go @@ -45,18 +45,18 @@ func handlerFlow() ([]bson.D, []bson.D) { a := withEH(obj("a", "Microflows$ActionActivity", 250, 200), microflows.ErrorHandlingTypeCustomWithoutRollback) dSet(a, "Size", "160;60") // resized in Studio Pro: kept return []bson.D{ - obj("start", "Microflows$StartEvent", 100, 200), - a, - obj("h", "Microflows$ActionActivity", 250, 330), - obj("m", "Microflows$ExclusiveMerge", 400, 200), - obj("end", "Microflows$EndEvent", 550, 200), - }, []bson.D{ - flow("f1", "start", "a", 1, 3, false), - flow("f2", "a", "m", 1, 3, false), - flow("err", "a", "h", 2, 0, true), - flow("fh", "h", "m", 1, 2, false), - flow("f3", "m", "end", 1, 3, false), - } + obj("start", "Microflows$StartEvent", 100, 200), + a, + obj("h", "Microflows$ActionActivity", 250, 330), + obj("m", "Microflows$ExclusiveMerge", 400, 200), + obj("end", "Microflows$EndEvent", 550, 200), + }, []bson.D{ + flow("f1", "start", "a", 1, 3, false), + flow("f2", "a", "m", 1, 3, false), + flow("err", "a", "h", 2, 0, true), + flow("fh", "h", "m", 1, 2, false), + flow("f3", "m", "end", 1, 3, false), + } } func TestSplice_ReplaceKeepsTheErrorHandler(t *testing.T) { From 14277f6105b0204d175a252ae4fb71674c5f21e4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 25 Oct 2026 00:18:13 +0000 Subject: [PATCH 12/14] test: rename the stored-document walker so integration vet builds walkBSON in the #1679 test collided with the integration-tagged studiopro_roundtrip_test.go helper of the same name, so `go vet -tags integration` (make lint) failed to build the package. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DRueZhZoBML5iWuJTvUnjL --- mdl/executor/design_props_native_1679_test.go | 8 ++++---- mdl/executor/dg2_column_template_assoc_path_1680_test.go | 2 +- mdl/executor/import_mapping_parameter_1681_test.go | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/mdl/executor/design_props_native_1679_test.go b/mdl/executor/design_props_native_1679_test.go index 7fe3f2610c..5bd3ae7a5a 100644 --- a/mdl/executor/design_props_native_1679_test.go +++ b/mdl/executor/design_props_native_1679_test.go @@ -93,7 +93,7 @@ func storedDesignPropertyTypes(t *testing.T, exec *Executor, page, widget string t.Fatal(err) } out := map[string]string{} - walkBSON(doc, func(d bson.D) { + walkStoredDocs(doc, func(d bson.D) { if afGet(d, "Name") != widget { return } @@ -110,16 +110,16 @@ func storedDesignPropertyTypes(t *testing.T, exec *Executor, page, widget string return nil } -func walkBSON(d bson.D, fn func(bson.D)) { +func walkStoredDocs(d bson.D, fn func(bson.D)) { fn(d) for _, e := range d { switch v := e.Value.(type) { case bson.D: - walkBSON(v, fn) + walkStoredDocs(v, fn) case bson.A: for _, x := range v { if xd, ok := x.(bson.D); ok { - walkBSON(xd, fn) + walkStoredDocs(xd, fn) } } } diff --git a/mdl/executor/dg2_column_template_assoc_path_1680_test.go b/mdl/executor/dg2_column_template_assoc_path_1680_test.go index 137ad8c504..fdc85e55a9 100644 --- a/mdl/executor/dg2_column_template_assoc_path_1680_test.go +++ b/mdl/executor/dg2_column_template_assoc_path_1680_test.go @@ -47,7 +47,7 @@ func storedColumnTemplateRefs(t *testing.T, exec *Executor, page string) [][2]st t.Fatal(err) } var out [][2]string - walkBSON(doc, func(d bson.D) { + walkStoredDocs(doc, func(d bson.D) { if afGet(d, "$Type") != "Forms$ClientTemplateParameter" { return } diff --git a/mdl/executor/import_mapping_parameter_1681_test.go b/mdl/executor/import_mapping_parameter_1681_test.go index 627469ccdf..0ac4fc6d1b 100644 --- a/mdl/executor/import_mapping_parameter_1681_test.go +++ b/mdl/executor/import_mapping_parameter_1681_test.go @@ -66,7 +66,7 @@ func storedImportCall(t *testing.T, exec *Executor, name string) (imc, varType b if err := bson.Unmarshal(raw, &d); err != nil { t.Fatal(err) } - walkBSON(d, func(x bson.D) { + walkStoredDocs(d, func(x bson.D) { if afGet(x, "$Type") == "Microflows$ResultHandling" && imc == nil { imc, _ = afGet(x, "ImportMappingCall").(bson.D) varType, _ = afGet(x, "VariableType").(bson.D) @@ -152,7 +152,7 @@ func TestImportMappingVoidResultIsNotRefused(t *testing.T) { if err := bson.Unmarshal(raw, &d); err != nil { t.Fatal(err) } - walkBSON(d, func(x bson.D) { + walkStoredDocs(d, func(x bson.D) { if afGet(x, "$Type") != "Microflows$ResultHandling" { return } From 83316c19508032d2aaf00fd30ab3ad51ca64d689 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 25 Oct 2026 08:01:32 +0000 Subject: [PATCH 13/14] test: strike two TestApp documents the fixes made round-trip WorkflowCommons.WorkflowUserTaskView_View was listed for a page Appearance that kept 1 of its 3 design properties, which page-level design properties (#1678) now carry. Snip_Workflow_TaskDetails was never triaged into #721's classes; it passes getput on this branch too. The allowlist may only shrink. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DRueZhZoBML5iWuJTvUnjL --- mdl/roundtrip/testapp_allowlist_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/mdl/roundtrip/testapp_allowlist_test.go b/mdl/roundtrip/testapp_allowlist_test.go index 39f39400c3..539cb7a5e1 100644 --- a/mdl/roundtrip/testapp_allowlist_test.go +++ b/mdl/roundtrip/testapp_allowlist_test.go @@ -139,7 +139,6 @@ var testAppKnownFailures = map[string]knownFailure{ "page WorkflowCommons.WorkflowDefinition_View": {laws: []law{lawExec}, issue: "#721", why: "page: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "page WorkflowCommons.WorkflowEndedUserTask": {laws: []law{lawGetPut}, issue: "#721", why: "page: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "page WorkflowCommons.WorkflowSubProcess_View": {laws: []law{lawExec}, issue: "#721", why: "page: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "page WorkflowCommons.WorkflowUserTaskView_View": {laws: []law{lawGetPut}, issue: "#721 #826", why: "executes since describe prints snippet call Params (#826); the page Appearance keeps 1 of 3 DesignProperties (#721 C)"}, "page WorkflowCommons.Workflow_Dashboard": {laws: []law{lawGetPut}, issue: "#721", why: "page: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "rest client Clients.OrdersRestClient": {laws: []law{lawParse}, issue: "#721", why: "rest client: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "rule Rules.Rule1": {laws: []law{lawGetPut}, issue: "#721", why: "rule: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, @@ -188,7 +187,6 @@ var testAppKnownFailures = map[string]knownFailure{ "snippet WorkflowCommons.Snip_Workflow_NotificationArea": {laws: []law{lawGetPut, lawPutGet}, issue: "#721", why: "snippet: breaks getput and putget on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "snippet WorkflowCommons.Snip_Workflow_StateCircleOnly": {laws: []law{lawGetPut, lawPutGet}, issue: "#721", why: "snippet: breaks getput and putget on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "snippet WorkflowCommons.Snip_Workflow_SubProcessDetails": {laws: []law{lawExec}, issue: "#721", why: "snippet: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "snippet WorkflowCommons.Snip_Workflow_TaskDetails": {laws: []law{lawGetPut}, issue: "#721", why: "snippet: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, } // createOrModifyProbeKnownFailures is the allowlist of From ea353574871550382694397e7739920b2bef5be7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 25 Oct 2026 08:06:59 +0000 Subject: [PATCH 14/14] style: restore splice_handler_test.go to main's formatting The earlier "gofmt" of this file used /usr/local/go's gofmt, older than the go1.27.3 toolchain go.mod pins, which formats the composite-literal return the other way. Under the toolchain's gofmt (what CI's fmt-check runs) main's version was clean and the reformatted one is not. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01DRueZhZoBML5iWuJTvUnjL --- mdl/backend/mfmutator/splice_handler_test.go | 24 ++++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/mdl/backend/mfmutator/splice_handler_test.go b/mdl/backend/mfmutator/splice_handler_test.go index c4dbbee4c7..5f0df41dde 100644 --- a/mdl/backend/mfmutator/splice_handler_test.go +++ b/mdl/backend/mfmutator/splice_handler_test.go @@ -45,18 +45,18 @@ func handlerFlow() ([]bson.D, []bson.D) { a := withEH(obj("a", "Microflows$ActionActivity", 250, 200), microflows.ErrorHandlingTypeCustomWithoutRollback) dSet(a, "Size", "160;60") // resized in Studio Pro: kept return []bson.D{ - obj("start", "Microflows$StartEvent", 100, 200), - a, - obj("h", "Microflows$ActionActivity", 250, 330), - obj("m", "Microflows$ExclusiveMerge", 400, 200), - obj("end", "Microflows$EndEvent", 550, 200), - }, []bson.D{ - flow("f1", "start", "a", 1, 3, false), - flow("f2", "a", "m", 1, 3, false), - flow("err", "a", "h", 2, 0, true), - flow("fh", "h", "m", 1, 2, false), - flow("f3", "m", "end", 1, 3, false), - } + obj("start", "Microflows$StartEvent", 100, 200), + a, + obj("h", "Microflows$ActionActivity", 250, 330), + obj("m", "Microflows$ExclusiveMerge", 400, 200), + obj("end", "Microflows$EndEvent", 550, 200), + }, []bson.D{ + flow("f1", "start", "a", 1, 3, false), + flow("f2", "a", "m", 1, 3, false), + flow("err", "a", "h", 2, 0, true), + flow("fh", "h", "m", 1, 2, false), + flow("f3", "m", "end", 1, 3, false), + } } func TestSplice_ReplaceKeepsTheErrorHandler(t *testing.T) {