From 036b1c051aa99f62e534a0858d2e1c117e240e01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Go=C5=82embiewski?= Date: Thu, 24 Sep 2026 20:31:43 +0200 Subject: [PATCH] fix(pages): keep a navigation tree's menu-document source (#1189) A navigationtree or menubar draws its items from a navigation profile or a menu document. Only the profile was read, written and described: Menu: was ignored and stored as the Responsive profile, and a tree on a menu document (Atlas_Core.Tablet_Sidebar, Phone_Sidebar) described as a bare navigationtree, so a describe -> exec copy showed the desktop menu. Menu: Module.Menu is now written as a Forms$MenuDocumentSource and described back; Menu: with Profile:, an unqualified name, or a missing menu is refused. --- .../fix-issue/findings/mdl-executor.jsonl | 1 + .claude/skills/mendix/write-layouts/SKILL.md | 4 +- CHANGELOG.md | 4 + cmd/mxcli/syntax/features_page.go | 1 + docs-site/src/reference/page/create-layout.md | 4 +- .../navigationtree-menu-document-source.mdl | 36 ++++++ .../modelsdk/widget_menu_source_write_test.go | 40 ++++++ mdl/backend/modelsdk/widget_write.go | 26 ++-- mdl/executor/cmd_pages_builder_v3.go | 39 ++++++ mdl/executor/cmd_pages_describe.go | 2 + mdl/executor/cmd_pages_describe_output.go | 4 +- mdl/executor/cmd_pages_describe_parse.go | 5 + mdl/executor/cmd_pages_menu_source_test.go | 116 ++++++++++++++++++ sdk/pages/pages_widgets_advanced.go | 11 +- 14 files changed, 278 insertions(+), 15 deletions(-) create mode 100644 mdl-examples/bug-tests/navigationtree-menu-document-source.mdl create mode 100644 mdl/backend/modelsdk/widget_menu_source_write_test.go create mode 100644 mdl/executor/cmd_pages_menu_source_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 1319382a8..b74fae009 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -686,3 +686,4 @@ {"area": "mdl/executor", "date": "2026-09-23", "symptom": "`CALL MICROFLOW M.F(…) IN QUEUE M.Q` where `F` returns **Boolean**: `mxcli check -p --references` says `Check passed!`, `mx check` says **CE7033** \"A microflow used for background execution must have a Microflow return type of 'Nothing'.\" (at Call microflow activity 'F'). Reported with the CE0142 after-startup sibling, which MDL073 had already closed.", "cause": "The --references pass resolved the call target and the queue name separately, and both resolve. Nothing compared the binding (`in queue`) against the signature of the flow it names. Added MDL088: a project-less pass (ValidateQueuedCallReturnType) for a target the script creates, and validateQueuedMicroflowTargets on the --references path for a stored target, which skips script-defined targets so the fault is not printed twice. Stored void microflows read back as ReturnType \"Void\", not \"\" — both must mean Nothing.", "file": "`mdl/executor/validate_queued_call_return.go` (queuedMicroflowCalls, checkQueuedMicroflowReturnsNothing, validateQueuedMicroflowTargets, ValidateQueuedCallReturnType), wired in `validate_program.go` and `validate.go` (validateFlowBodyReferences); examples `mdl-examples/bug-tests/1064-queued-microflow-must-return-nothing{,.fail}.mdl`", "insight": "Same class as MDL073 (\"the reference resolves\" ≠ \"the reference is usable\"): any binding that names a flow carries a constraint on that flow's signature, and a resolver checks only the name. When one such check lands, sweep for its siblings at other binding sites. The queued CALL JAVA ACTION twin (CE7038) is still unchecked and was deliberately left out of scope. Two things that cost time: (1) `mxcli exec` of a script that CREATEs a queue and then binds a call to it refuses with 'task queue not found' — validateFlowBodyReferences checks queues against the project only, not the script context — so the repro has to create the queue in a separate exec; (2) walk call statements by reflection, not by the flowRefCollector switch, which does not descend into WHILE bodies. Measured on mxbuild 11.12.0 with two projects: Boolean target → CE7033, void target → 0 errors.", "refs": ["mendixlabs/mxcli#1064"], "ce": ["CE7033"], "rules": ["MDL088"]} {"area": "mdl/executor/microflow-layout", "date": "2026-09-23", "symptom": "MPR011 fires on EVERY `while` loop mxcli writes \u2014 'first activity at (50,80) lies outside the loop box' \u2014 single-level loops included. `mx check` passes and the app runs; the flow just renders wrong in Studio Pro. Reported from a real project as 'looks like an mxcli layout issue', with 3 MPR011 warnings still in its final lint run. mxcli's own lint rule was correctly flagging mxcli's own output.", "cause": "One missing term in the WHILE builder. addWhileStatement had `innerStartX := LoopPadding` (50) where addLoopStatement has `LoopPadding + iteratorSpace + ActivityWidth/2` (210). A microflow object's Position is its CENTRE \u2014 the builder says so itself ('Position is the CENTER point (RelativeMiddlePoint in Mendix)') \u2014 so a centre at x=50 with ActivityWidth=120 puts the left edge at -10. The doc comment says the while layout 'matches addLoopStatement but without iterator icon space': dropping the iterator space (100) was right, taking ActivityWidth/2 with it was not, because that term is not iterator space, it is what converts a centre to a left edge. The very next line, `innerStartY := LoopPadding + ActivityHeight/2`, adds the half-height for exactly this reason \u2014 so the omission was accidental, not a choice. Reported (50,80) matches term for term: 50 = LoopPadding, 80 = LoopPadding + ActivityHeight/2.", "file": "`mdl/executor/cmd_microflows_builder_control.go` (addWhileStatement: `innerStartX := LoopPadding + ActivityWidth/2`), tests `mdl/executor/loop_containment_test.go` (TestWhileLoopBox_ContainsDefaultLaidOutChildren, TestWhileLoopFirstChildLeftEdgeIsInsideTheBox)", "insight": "The containment invariant WAS already tested \u2014 loop_containment_test.go exists from #884 and asserts exactly this \u2014 but every fixture in it built a FOREACH loop. There are two loop builders; one was covered and the uncovered one shipped the violation into every project that writes a `while`. An invariant is worth what its COVERAGE is, and a file named for an invariant reads as if it covers the invariant, which is how a second code path goes unexamined for months. When a rule flags the tool's own output, believe the rule first: the reporter hedged with 'looks like an mxcli layout issue' and was exactly right. Cheap tell for this class: a term present on one axis and absent on the other in adjacent lines (`+ ActivityHeight/2` on Y, nothing on X) is almost always an omission rather than a decision. Failing test written first; it reproduced the reported geometry to the pixel, x[-10,...] at 1, 2, 4 and 7 activities. Still uncovered: addManualWhileTrueStatement, the third loop builder.", "refs": ["ako/mxcli#884", "ako/mxcli#645"]} {"date": "2026-09-23", "area": "mdl-executor", "symptom": "upstream #1176: DESCRIBE prints `all` on an import activity that returns ONE object — `$objectResponse = import from mapping M.IMM($s) all;` — which reads as a list import. Reported on v0.23.0 / Studio Pro 11.12.3, after #881 was believed to have settled import ranges", "cause": "#881 made `formatImportMappingRange` always emit a range keyword, because at the time a missing keyword let the range fall back to the variable's cardinality and store First. The later runtime fix (unauthored range written as All explicitly) made bare and `all` build the same activity, but the describe side was never revisited, so `all` kept printing where it was only noise", "file": "`mdl/executor/cmd_microflows_format_action.go` (`formatImportMappingRange`: return \"\" for All against SingleObject); tests `mdl/executor/cmd_microflows_import_range_test.go` (`TestImportRange_ObjectResultDescribesWithoutAll`); example `mdl-examples/bug-tests/1176-import-mapping-object-describes-without-all.mdl`", "insight": "**This was not #881 regressing — it was #881's own workaround outliving its reason.** 'DESCRIBE must never emit nothing' was a guard against the builder's then-broken default; once the builder wrote a missing keyword as All explicitly, the guard became pure noise, and nothing linked the two sites. When a formatter emits something 'because the builder would otherwise infer X', put that reason in a test that asserts the builder equivalence (bare vs keyword build the same activity), so fixing the builder flags the formatter. Proving the omission safe needs that equivalence on a real project, not just the unit test: on 11.12.3 both spellings store byte-identical ResultHandling (ConstantRange{SingleObject:false} + ObjectType), `mx check` 0 errors, and exec'ing the described text reports 'Unchanged microflow'. Wrong turn to skip: a JSON diff of two EMPTY extractions prints 'IDENTICAL' — `bson dump` emits ordered Key/Value lists, not objects; check the extraction is non-empty before trusting a diff", "refs": ["#881", "#1176"]} +{"area": "mdl/executor", "date": "2026-09-24", "symptom": "`navigationtree nav (Menu: MyFirstModule.Side_Menu)` passed check and exec and came back from describe as `(Profile: 'Responsive')`; a describe -> exec copy of Atlas_Core.Tablet_Sidebar showed the desktop menu instead of Tablet_Menu. mx check: 0 errors either way.", "cause": "A tree's MenuSource is polymorphic (Forms$NavigationSource | Forms$MenuDocumentSource). The builder read only `Profile`, the writer always emitted a NavigationSource defaulting to Responsive, and describe read only MenuSource.NavigationProfile -- so the menu-document branch existed in the metamodel and in nine stock Atlas layouts but in none of the three code paths.", "file": "mdl/executor/cmd_pages_builder_v3.go", "insight": "When a stored property is polymorphic, grep the stock project for every $Type it takes before writing the reader and writer. Here `bson dump` over Atlas_Core's layouts showed MenuDocumentSource in nine of them; a defaulting writer (orDefaultStr) turns an unhandled variant into a silent substitution rather than an error.", "refs": "mendixlabs/mxcli#1189"} diff --git a/.claude/skills/mendix/write-layouts/SKILL.md b/.claude/skills/mendix/write-layouts/SKILL.md index 614cc1d59..1c3db3b62 100644 --- a/.claude/skills/mendix/write-layouts/SKILL.md +++ b/.claude/skills/mendix/write-layouts/SKILL.md @@ -146,8 +146,8 @@ create page MyModule.Home (title: 'Home', layout: MyModule.App_Default) { | Scroll container | `scrollcontainer name { … }` | The layout's root. Its children are **regions**, never widgets | | Region | `region top \| right \| bottom \| left \| center` | Five **named slots**, not a list. One region per slot; a repeat is refused | | Placeholder | `placeholder Main` | The hole a page's content goes into. No properties, no body | -| Navigation tree | `navigationtree name (profile: 'Responsive')` | The sidebar menu — vertical. The profile is a navigation profile name | -| Menu bar | `menubar name (profile: 'Responsive')` | The topbar menu — horizontal. Same stored shape as a navigation tree | +| Navigation tree | `navigationtree name (profile: 'Responsive')` or `(menu: Module.MenuName)` | The sidebar menu — vertical. Its items come from a navigation profile **or** a menu document (`create menu`), never both | +| Menu bar | `menubar name (profile: 'Responsive')` or `(menu: Module.MenuName)` | The topbar menu — horizontal. Same stored shape as a navigation tree | Region properties: `size` (integer), `sizemode` (`Fixed` / `Pixels` / `Auto`), `class`. Unset is Studio Pro's `200` / `Auto`. diff --git a/CHANGELOG.md b/CHANGELOG.md index cc8b6e66b..a9db3a3de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +### Fixed + +- **A navigation tree or menu bar on a menu document kept the Responsive menu instead** (mendixlabs/mxcli#1189) — `navigationtree nav (Menu: Module.Menu)` passed `check` and `exec` and was stored on the Responsive profile; `describe` printed a tree on a menu document (Atlas_Core's `Tablet_Sidebar`, `Phone_Sidebar`) as a bare `navigationtree`, so a describe → exec copy of `Tablet_Sidebar` showed the desktop menu. `Menu:` is now written as a `Forms$MenuDocumentSource` and described back; `Menu:` with `Profile:`, or a menu that does not exist, is refused. + ## [0.24.0] - 2026-09-24 Headline: **An element's storage GUID is the database's identity, and mxcli now treats it as one.** A production report of 28 attributes emptied across 607 rows by a single edit (mendixlabs/mxcli#1119) traced to five write paths that re-minted GUIDs — one of them moving 282 in a single module. They are fixed, and a new guard at the write choke point refuses any write that moves one: a class of data loss that leaves the model valid, `mx check` clean and `DESCRIBE` byte-identical, and surfaces only when the package meets a database that already holds data. Alongside it, `MOVE ENTITY` and `RENAME` stop leaving a project unbuildable, and four more scripts that passed every gate and failed the build are refused. diff --git a/cmd/mxcli/syntax/features_page.go b/cmd/mxcli/syntax/features_page.go index 3b1b8602d..b4b229f57 100644 --- a/cmd/mxcli/syntax/features_page.go +++ b/cmd/mxcli/syntax/features_page.go @@ -460,6 +460,7 @@ CREATE PAGE Sales.Detail (Title: 'Detail', Layout: Atlas_Core.Atlas_Default) { " -- widgets, plus:\n" + " NAVIGATIONTREE name (Profile: 'Responsive') -- vertical, for a sidebar\n" + " MENUBAR name (Profile: 'Responsive') -- horizontal, for a topbar\n" + + " -- or (Menu: Module.MenuName): items from a menu document, not a profile\n" + " PLACEHOLDER Main\n" + " }\n" + " }\n" + diff --git a/docs-site/src/reference/page/create-layout.md b/docs-site/src/reference/page/create-layout.md index 201a7ffa4..8308137f1 100644 --- a/docs-site/src/reference/page/create-layout.md +++ b/docs-site/src/reference/page/create-layout.md @@ -119,8 +119,8 @@ Main, not by a property) | Scroll container | `SCROLLCONTAINER name { … }` | The layout's root. Its children are **regions**, never widgets | | Region | `REGION top \| right \| bottom \| left \| center` | Five **named slots**, not a list. One region per slot | | Placeholder | `PLACEHOLDER Main` | The hole a page's content goes into. No properties, no body | -| Navigation tree | `NAVIGATIONTREE name (profile: 'Responsive')` | The sidebar menu — vertical | -| Menu bar | `MENUBAR name (profile: 'Responsive')` | The topbar menu — horizontal | +| Navigation tree | `NAVIGATIONTREE name (profile: 'Responsive')` or `(menu: Module.MenuName)` | The sidebar menu — vertical. Items from a navigation profile or a menu document, not both | +| Menu bar | `MENUBAR name (profile: 'Responsive')` or `(menu: Module.MenuName)` | The topbar menu — horizontal | ## Examples diff --git a/mdl-examples/bug-tests/navigationtree-menu-document-source.mdl b/mdl-examples/bug-tests/navigationtree-menu-document-source.mdl new file mode 100644 index 000000000..16b953dfa --- /dev/null +++ b/mdl-examples/bug-tests/navigationtree-menu-document-source.mdl @@ -0,0 +1,36 @@ +-- ============================================================================ +-- mendixlabs/mxcli#1189 — a navigation tree and a menu bar on a menu document +-- ============================================================================ +-- +-- A tree's items come from a navigation profile OR a menu document. `Menu:` +-- used to be ignored: the widget was stored on the Responsive profile, and +-- DESCRIBE printed a tree on a menu document (Atlas_Core.Tablet_Sidebar) as a +-- bare `navigationtree`, so a describe -> exec copy showed the desktop menu. +-- +-- After exec, DESCRIBE LAYOUT BugNavMenuSource.Side_Layout prints both widgets +-- with `(Menu: BugNavMenuSource.Side_Menu)`, and `mxcli bson dump --type layout` +-- shows MenuSource $Type Forms$MenuDocumentSource. Verified on 11.12.1: +-- mx check 0 errors. +-- ============================================================================ + +CREATE MODULE BugNavMenuSource; + +CREATE OR MODIFY MENU BugNavMenuSource.Side_Menu ( + MENU ITEM 'Home' ICON Atlas_Core.Atlas_Filled.home; +); + +CREATE LAYOUT BugNavMenuSource.Side_Layout ( + layouttype: 'Responsive' +) { + SCROLLCONTAINER layoutContainer { + REGION top (Size: 60, SizeMode: 'Pixels') { + MENUBAR topMenu (Menu: BugNavMenuSource.Side_Menu) + } + REGION left (Size: 232, SizeMode: 'Pixels') { + NAVIGATIONTREE sideMenu (Menu: BugNavMenuSource.Side_Menu) + } + REGION center { + PLACEHOLDER Main + } + } +} diff --git a/mdl/backend/modelsdk/widget_menu_source_write_test.go b/mdl/backend/modelsdk/widget_menu_source_write_test.go new file mode 100644 index 000000000..89ab5fcd3 --- /dev/null +++ b/mdl/backend/modelsdk/widget_menu_source_write_test.go @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "testing" + + "github.com/mendixlabs/mxcli/sdk/pages" +) + +// A tree or menu bar can draw its items from a menu document instead of a +// profile: Atlas_Core.Tablet_Sidebar's tree carries +// MenuSource → Forms$MenuDocumentSource, Menu "Atlas_Core.Tablet_Menu". +// It used to be written as the Responsive profile whatever the widget named, so +// a copy of Tablet_Sidebar showed the desktop menu (mendixlabs/mxcli#1189). +func TestMenuWidgetsToGen_WriteAMenuDocumentSource(t *testing.T) { + for _, w := range []pages.Widget{ + &pages.NavigationTree{BaseWidget: pages.BaseWidget{Name: "nav"}, MenuDocument: "Atlas_Core.Tablet_Menu"}, + &pages.MenuBar{BaseWidget: pages.BaseWidget{Name: "bar"}, MenuDocument: "Atlas_Core.Tablet_Menu"}, + } { + g, err := widgetToGen(w) + if err != nil { + t.Fatal(err) + } + doc := encodeToMap(t, g) + src, ok := doc["MenuSource"].(map[string]any) + if !ok { + t.Fatalf("%v: MenuSource missing; keys = %v", doc["$Type"], keysOf(doc)) + } + if src["$Type"] != "Forms$MenuDocumentSource" { + t.Errorf("%v: MenuSource $Type = %v, want Forms$MenuDocumentSource", doc["$Type"], src["$Type"]) + } + if src["Menu"] != "Atlas_Core.Tablet_Menu" { + t.Errorf("%v: Menu = %v, want Atlas_Core.Tablet_Menu", doc["$Type"], src["Menu"]) + } + if _, has := src["NavigationProfile"]; has { + t.Errorf("%v: a menu document source carries no NavigationProfile", doc["$Type"]) + } + } +} diff --git a/mdl/backend/modelsdk/widget_write.go b/mdl/backend/modelsdk/widget_write.go index 15fa85c3e..8d0a7fc75 100644 --- a/mdl/backend/modelsdk/widget_write.go +++ b/mdl/backend/modelsdk/widget_write.go @@ -644,10 +644,7 @@ func widgetToGen(w pages.Widget) (element.Element, error) { case *pages.NavigationTree: g := genPg.NewNavigationTree() applyWidgetBase(g, &x.BaseWidget) - src := genPg.NewNavigationSource() - assignID(src) - src.SetNavigationProfileQualifiedName(orDefaultStr(x.NavigationProfile, "Responsive")) - g.SetMenuSource(src) + g.SetMenuSource(menuSourceToGen(x.NavigationProfile, x.MenuDocument)) return g, nil case *pages.MenuBar: @@ -655,10 +652,7 @@ func widgetToGen(w pages.Widget) (element.Element, error) { // a menu bar is the horizontal navigation a topbar carries. g := genPg.NewMenuBar() applyWidgetBase(g, &x.BaseWidget) - src := genPg.NewNavigationSource() - assignID(src) - src.SetNavigationProfileQualifiedName(orDefaultStr(x.NavigationProfile, "Responsive")) - g.SetMenuSource(src) + g.SetMenuSource(menuSourceToGen(x.NavigationProfile, x.MenuDocument)) return g, nil case *pages.GroupBox: @@ -1844,6 +1838,22 @@ func clientActionToGen(a pages.ClientAction) (element.Element, error) { } // orDefaultStr returns s, or def when s is empty. +// menuSourceToGen is the MenuSource a navigation tree or menu bar carries: a +// Forms$MenuDocumentSource when the widget names a menu document, else a +// Forms$NavigationSource on the profile (Responsive when none is named). +func menuSourceToGen(profile, menuDocument string) element.Element { + if menuDocument != "" { + src := genPg.NewMenuDocumentSource() + assignID(src) + src.SetMenuQualifiedName(menuDocument) + return src + } + src := genPg.NewNavigationSource() + assignID(src) + src.SetNavigationProfileQualifiedName(orDefaultStr(profile, "Responsive")) + return src +} + func orDefaultStr(s, def string) string { if s == "" { return def diff --git a/mdl/executor/cmd_pages_builder_v3.go b/mdl/executor/cmd_pages_builder_v3.go index 10652c860..2582dec65 100644 --- a/mdl/executor/cmd_pages_builder_v3.go +++ b/mdl/executor/cmd_pages_builder_v3.go @@ -2786,6 +2786,10 @@ func (pb *pageBuilder) buildScrollContainerV3(w *ast.WidgetV3) (pages.Widget, er // buildNavigationTreeV3 builds the sidebar menu. The profile is stored inside a // Forms$NavigationSource, not on the tree — see widget_write.go. func (pb *pageBuilder) buildNavigationTreeV3(w *ast.WidgetV3) (pages.Widget, error) { + menu, err := pb.menuSourceDocument(w) + if err != nil { + return nil, err + } nt := &pages.NavigationTree{ BaseWidget: pages.BaseWidget{ BaseElement: model.BaseElement{ @@ -2795,10 +2799,40 @@ func (pb *pageBuilder) buildNavigationTreeV3(w *ast.WidgetV3) (pages.Widget, err Name: w.Name, }, NavigationProfile: w.GetStringProp("Profile"), + MenuDocument: menu, } return nt, nil } +// menuSourceDocument reads `Menu: Module.Name`, the menu document a navigation +// tree or menu bar draws its items from instead of a profile. It used to be +// ignored: the widget was stored with the Responsive profile, and a copy of +// Atlas_Core.Tablet_Sidebar showed the desktop menu (mendixlabs/mxcli#1189). +// The menu must exist when the layout is written: a dangling by-name reference +// passes mx check and draws an empty menu. +func (pb *pageBuilder) menuSourceDocument(w *ast.WidgetV3) (string, error) { + menu := strings.TrimSpace(w.GetStringProp("Menu")) + if menu == "" { + return "", nil + } + if w.GetStringProp("Profile") != "" { + return "", mdlerrors.NewValidationf( + "%s %s: Menu and Profile are two sources for the same items -- give one", strings.ToLower(w.Type), w.Name) + } + qn := parseQualifiedNameStr(menu) + if qn.Module == "" { + return "", mdlerrors.NewValidationf("%s %s: Menu needs a qualified name, Module.Menu -- got %q", + strings.ToLower(w.Type), w.Name, menu) + } + if pb.backend != nil { + if _, err := pb.backend.GetMenuDocumentByQualifiedName(qn.Module, qn.Name); err != nil { + return "", mdlerrors.NewValidationf("%s %s: menu not found: %s (create it with `create menu`)", + strings.ToLower(w.Type), w.Name, qn.String()) + } + } + return qn.String(), nil +} + // buildPlaceholderV3 declares a slot a page can bind to. // // The name is the API: a page references it as Module.Layout., so it is @@ -2822,6 +2856,10 @@ func (pb *pageBuilder) buildPlaceholderV3(w *ast.WidgetV3) (pages.Widget, error) // buildMenuBarV3 builds the horizontal navigation a topbar carries. Same shape // as a navigation tree — see widget_write.go. func (pb *pageBuilder) buildMenuBarV3(w *ast.WidgetV3) (pages.Widget, error) { + menu, err := pb.menuSourceDocument(w) + if err != nil { + return nil, err + } return &pages.MenuBar{ BaseWidget: pages.BaseWidget{ BaseElement: model.BaseElement{ @@ -2831,6 +2869,7 @@ func (pb *pageBuilder) buildMenuBarV3(w *ast.WidgetV3) (pages.Widget, error) { Name: w.Name, }, NavigationProfile: w.GetStringProp("Profile"), + MenuDocument: menu, }, nil } diff --git a/mdl/executor/cmd_pages_describe.go b/mdl/executor/cmd_pages_describe.go index 2d604adcf..3f582e649 100644 --- a/mdl/executor/cmd_pages_describe.go +++ b/mdl/executor/cmd_pages_describe.go @@ -704,6 +704,8 @@ type rawWidget struct { // NavigationProfile is a Forms$NavigationTree's profile, which the document // keeps one level down in MenuSource rather than on the tree. NavigationProfile string + // MenuDocument is the other MenuSource: a Forms$MenuDocumentSource's menu. + MenuDocument string // Specialization is the entity a List View template renders. Set only on the // synthetic wrappers parseListViewContent emits for Forms$ListViewTemplate, // which is the same shape as TabCaption above: a container with no name, whose diff --git a/mdl/executor/cmd_pages_describe_output.go b/mdl/executor/cmd_pages_describe_output.go index 8c131d324..f6f771613 100644 --- a/mdl/executor/cmd_pages_describe_output.go +++ b/mdl/executor/cmd_pages_describe_output.go @@ -311,7 +311,9 @@ func outputWidgetMDLV3(ctx *ExecContext, w rawWidget, indent int) { } header := fmt.Sprintf("%s %s", keyword, mdlIdent(w.Name)) var props []string - if w.NavigationProfile != "" { + if w.MenuDocument != "" { + props = append(props, fmt.Sprintf("Menu: %s", w.MenuDocument)) + } else if w.NavigationProfile != "" { props = append(props, fmt.Sprintf("Profile: %s", mdlQuote(w.NavigationProfile))) } props = appendAppearanceProps(props, w) diff --git a/mdl/executor/cmd_pages_describe_parse.go b/mdl/executor/cmd_pages_describe_parse.go index 98ea47608..153fafb1d 100644 --- a/mdl/executor/cmd_pages_describe_parse.go +++ b/mdl/executor/cmd_pages_describe_parse.go @@ -274,10 +274,15 @@ func parseRawWidget(ctx *ExecContext, w map[string]any, parentEntityContext ...s "Forms$MenuBar", "Pages$MenuBar": // The profile is a qualified name one level down, in a // Forms$NavigationSource, not a property of the tree. + // Or a menu document, in a Forms$MenuDocumentSource (Atlas_Core's + // Tablet_Sidebar and Phone_Sidebar); mendixlabs/mxcli#1189. if src, ok := w["MenuSource"].(map[string]any); ok { if p, ok := src["NavigationProfile"].(string); ok { widget.NavigationProfile = p } + if m, ok := src["Menu"].(string); ok { + widget.MenuDocument = m + } } return []rawWidget{widget} diff --git a/mdl/executor/cmd_pages_menu_source_test.go b/mdl/executor/cmd_pages_menu_source_test.go new file mode 100644 index 000000000..43c08e885 --- /dev/null +++ b/mdl/executor/cmd_pages_menu_source_test.go @@ -0,0 +1,116 @@ +// SPDX-License-Identifier: Apache-2.0 + +// A navigation tree or menu bar draws its items from a navigation profile or +// from a menu document. Only the profile was read, written and described: a +// tree on a menu document (Atlas_Core.Tablet_Sidebar, Phone_Sidebar) described +// as a bare `navigationtree`, exec wrote it back on the Responsive profile, and +// `Menu:` in a script was ignored without a word (mendixlabs/mxcli#1189). +package executor + +import ( + "bytes" + "context" + "fmt" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +func describeWidget(t *testing.T, raw map[string]any) string { + t.Helper() + ctx := (&Executor{}).newExecContext(context.Background()) + var out bytes.Buffer + ctx.Output = &out + for _, w := range parseRawWidget(ctx, raw) { + outputWidgetMDLV3(ctx, w, 0) + } + return out.String() +} + +func TestDescribeMenuWidgets_PrintTheMenuDocument(t *testing.T) { + for _, typ := range []string{"Forms$NavigationTree", "Forms$MenuBar"} { + got := describeWidget(t, map[string]any{ + "$Type": typ, + "Name": "nav", + "MenuSource": map[string]any{ + "$Type": "Forms$MenuDocumentSource", + "Menu": "Atlas_Core.Tablet_Menu", + }, + }) + if !strings.Contains(got, "(Menu: Atlas_Core.Tablet_Menu)") { + t.Errorf("%s: describe = %q, want the menu document", typ, got) + } + if strings.Contains(got, "Profile") { + t.Errorf("%s: describe = %q, a menu-document tree has no profile", typ, got) + } + } +} + +func TestDescribeNavigationTree_StillPrintsTheProfile(t *testing.T) { + got := describeWidget(t, map[string]any{ + "$Type": "Forms$NavigationTree", + "Name": "nav", + "MenuSource": map[string]any{ + "$Type": "Forms$NavigationSource", + "NavigationProfile": "Responsive", + }, + }) + if !strings.Contains(got, "(Profile: 'Responsive')") { + t.Errorf("describe = %q, want the profile", got) + } +} + +func menuSourceBuilder(known ...string) *pageBuilder { + mb := &mock.MockBackend{ + GetMenuDocumentByQualifiedNameFunc: func(module, name string) (*types.MenuDocument, error) { + for _, k := range known { + if k == module+"."+name { + return &types.MenuDocument{Name: name}, nil + } + } + return nil, fmt.Errorf("menu not found") + }, + } + return &pageBuilder{backend: mb} +} + +func TestBuildMenuWidgets_KeepTheMenuDocument(t *testing.T) { + pb := menuSourceBuilder("MyFirstModule.Side_Menu") + w := &ast.WidgetV3{Type: "navigationtree", Name: "nav", Properties: map[string]any{"Menu": "MyFirstModule.Side_Menu"}} + tree, err := pb.buildNavigationTreeV3(w) + if err != nil { + t.Fatal(err) + } + if got := tree.(*pages.NavigationTree).MenuDocument; got != "MyFirstModule.Side_Menu" { + t.Errorf("tree MenuDocument = %q", got) + } + w.Type = "menubar" + bar, err := pb.buildMenuBarV3(w) + if err != nil { + t.Fatal(err) + } + if got := bar.(*pages.MenuBar).MenuDocument; got != "MyFirstModule.Side_Menu" { + t.Errorf("bar MenuDocument = %q", got) + } +} + +func TestBuildNavigationTree_RejectsAMissingMenuOrTwoSources(t *testing.T) { + pb := menuSourceBuilder("MyFirstModule.Side_Menu") + for _, tc := range []struct { + props map[string]any + want string + }{ + {map[string]any{"Menu": "MyFirstModule.Nope"}, "menu not found: MyFirstModule.Nope"}, + {map[string]any{"Menu": "MyFirstModule.Side_Menu", "Profile": "Responsive"}, "give one"}, + {map[string]any{"Menu": "Side_Menu"}, "qualified name"}, + } { + _, err := pb.buildNavigationTreeV3(&ast.WidgetV3{Type: "navigationtree", Name: "nav", Properties: tc.props}) + if err == nil || !strings.Contains(err.Error(), tc.want) { + t.Errorf("%v: err = %v, want %q", tc.props, err, tc.want) + } + } +} diff --git a/sdk/pages/pages_widgets_advanced.go b/sdk/pages/pages_widgets_advanced.go index eba3af1ac..76220cd95 100644 --- a/sdk/pages/pages_widgets_advanced.go +++ b/sdk/pages/pages_widgets_advanced.go @@ -16,8 +16,13 @@ type NavigationTree struct { BaseWidget // NavigationProfile names the profile the menu is drawn from. It is stored // inside a Forms$NavigationSource under MenuSource, not on the tree itself. - NavigationProfile string `json:"navigationProfile,omitempty"` - Items []*NavigationItem `json:"items,omitempty"` + NavigationProfile string `json:"navigationProfile,omitempty"` + // MenuDocument is the other source a tree can draw from: a Menus$MenuDocument, + // stored as a Forms$MenuDocumentSource. Atlas_Core's Tablet_Sidebar and + // Phone_Sidebar trees read Tablet_Menu and Phone_Menu this way. At most one + // of MenuDocument and NavigationProfile is set. + MenuDocument string `json:"menuDocument,omitempty"` + Items []*NavigationItem `json:"items,omitempty"` } // NavigationItem represents an item in navigation. @@ -40,6 +45,8 @@ type NavigationItem struct { type MenuBar struct { BaseWidget NavigationProfile string `json:"navigationProfile,omitempty"` + // MenuDocument: see NavigationTree. + MenuDocument string `json:"menuDocument,omitempty"` // MenuSource is the older polymorphic form, kept because the type is // exported. Nothing reads or writes it.