diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index 5269d52e3..3f0fc3183 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -44,6 +44,7 @@ to the symptom table below, so the next similar issue costs fewer reads. | Studio Pro shows a dropdown / property as its default value even though MDL set it explicitly (e.g. CREATE ODATA CLIENT with `ConfigurationMicroflow:` set, but the "Configuration source" dropdown reads "Constants only") | The BSON key mxcli writes isn't what Studio Pro reads — either the key name is wrong, or multiple dropdown options actually share a single field discriminated by something other than the key name (return type of a referenced microflow, sibling property, etc.) | The `serializeXxx` function in `sdk/mpr/writer_*.go` for the affected document type | (1) Ask the user to duplicate the offending object in Studio Pro and **explicitly pick each dropdown option** on the duplicate(s). An unconfigured duplicate just looks like "Constants only" and tells you nothing about which field the option uses. (2) Re-dump the duplicates from `mprcontents/**/*.mxunit` **after every Studio Pro change** — cached `/tmp/svc-*.json` files go stale the instant the user edits the project (see [[feedback-refresh-bson-dumps]]). (3) Diff against mxcli's output to find the renamed key. (4) Don't assume one-state-per-key. The OData "Configuration microflow" / "Headers microflow" case stores BOTH options in the single `ConfigurationMicroflow` BSON field — Studio Pro picks the dropdown label from the referenced microflow's return type. When a discriminator like that exists, have both MDL keywords write to the same model field. Issues #573, #587 and 2026-05-27 unify-config-microflow fix | | `DESCRIBE` of a document type omits a property the user set in the CREATE (e.g. enum-level `/** doc */` vanishes after a roundtrip; `CREATE OR REPLACE` silently runs as plain CREATE for some types) — even though the AST struct has the field, the model has the field, the writer serialises it, and DESCRIBE prints it | Visitor never copied the parsed value into the AST struct — every layer below the visitor is wired but the visitor's `ExitCreateXxxStatement` is missing the assignment | `mdl/visitor/visitor_.go` → `ExitCreateXxxStatement` | Diff the visitor against a known-good sibling (e.g. enumeration vs constant). Standard wiring: `stmt.Documentation = findDocCommentText(ctx)` for doc-comments; `if createStmt.OR() != nil && (createStmt.MODIFY() != nil \|\| createStmt.REPLACE() != nil) { stmt.CreateOrModify = true }` for OR MODIFY/REPLACE. When adding a new CREATE statement, grep `mdl/visitor/visitor_constant.go` and copy these two blocks verbatim. Issue #393 | | `create [or modify] association ... to System.X` passes `mxcli check --references` and `mxcli diff` but fails at `mxcli exec` with `child entity not found: System.X` | Two divergent entity resolvers: the write path's `findEntity` resolved the owning module via `h.FindModuleID(dm.ID)`, but the virtual System domain model is not a real unit, so the hierarchy walk yielded an empty module name and System entities never matched. The validation path (`buildEntityQualifiedNames`) keyed on `dm.ContainerID` and worked, hence the check-passes/exec-fails split | `mdl/executor/oql_type_inference.go` → `findEntity` | Resolve the module from `dm.ContainerID` (the module ID `BuildSystemDomainModel` sets), not by walking up from the DM's own unit ID. When a symptom is "passes check/diff but fails exec," suspect two resolvers and make the write-path one match the validation-path one; add an `exec`-level test, not just a `check` test. Issue #610 | +| `ALTER STYLING ON PAGE/SNIPPET ... SET ...` fails with `unsupported container type: PAGE` (and `DESCRIBE STYLING` silently shows "No widgets found"); even past that, design-property writes never reached builder-created pages | Two layered bugs: (1) the visitor emits uppercase `ContainerType` `"PAGE"`/`"SNIPPET"` but `execAlterStyling`/`execDescribeStyling` compared lowercase, so it fell through to the unsupported-container error; (2) ALTER STYLING used the reflection walker `walkPageWidgets` (legacy `ListPages`/`UpdatePage`), which can't locate widgets in MDL-builder pages and violates the mutator-pattern rule | `mdl/executor/cmd_styling.go` (`execAlterStyling`, `execDescribeStyling`) + `mdl/backend/mpr/page_mutator.go` | Normalise container type with `strings.ToLower`. Route ALTER STYLING through `ctx.Backend.OpenPageForMutation(unitID)` like ALTER PAGE; check `mutator.FindWidget`. Add `SetDesignProperty`/`RemoveDesignProperty`/`ClearDesignProperties` to the `PageMutator` interface, writing the widget's `Appearance.DesignProperties` BSON array (`Forms$DesignPropertyValue` → `Toggle`/`Option`/`Custom` value), preserving an existing custom kind on option updates. When an ALTER uses a container-type discriminator, mirror the casing fix already done for ALTER PAGE (#402). Issue #631 | | `declare $x list of T = empty;` (or any list-typed `declare`) passes `mxcli check` but Studio Pro rejects with CE0053 ("type not allowed") + CE0038 ("value required") | `declare` maps to a Create Variable activity, which Mendix forbids from producing a list — but the validator only flagged an empty list *used as a loop source* (MDL002), never the declaration itself | `mdl/executor/validate_microflow.go` → `walkBody` `*ast.DeclareStmt` case | Emit `MDL040` (SeverityError) for any `stmt.Type.Kind == ast.TypeListOf`, regardless of initializer. Lists must come from a microflow parameter, a `retrieve`, or `$x = create list of T;`. Also fix the synced skills that present declare-list as valid (`write-microflows.md`, `cheatsheet-variables.md`, `check-syntax.md`, `patterns-*`). Issue #607 | --- diff --git a/docs/01-project/MDL_FEATURE_MATRIX.md b/docs/01-project/MDL_FEATURE_MATRIX.md index 2404eff68..b333980a4 100644 --- a/docs/01-project/MDL_FEATURE_MATRIX.md +++ b/docs/01-project/MDL_FEATURE_MATRIX.md @@ -187,7 +187,7 @@ Document types that exist in Mendix but have no MDL support: |---------|------|----------|--------|-----------|------|-------|----------|-------|---------|------|-----|--------|------|-----|------|--------|----------|-------| | **Microflow activities** | - | - | P | - | - | - | 02 | Y | P | P | P | Y | Y | - | - | - | P | 60+ activities supported; some edge cases missing | | **Building blocks** | N | N | N | N | N | N | N | N | N | N | N | N | N | N | N | N | N | Reusable page building blocks | -| **Styling** | P | P | P | N | N | N | N | N | N | N | N | P | N | P | N | N | N | Class/Style/DesignProperties on widgets; full theme system not yet | +| **Styling** | P | Y | P | N | N | Y | Y | Y | N | N | N | P | N | P | N | N | N | Class/Style/DesignProperties on widgets via ALTER STYLING (#631); full theme system not yet | | **Extensions** | N | N | N | N | N | N | N | N | N | N | N | N | N | N | N | N | N | Mendix extensions / add-ons | | **Custom JS actions** | N | N | N | N | N | N | N | N | N | N | N | N | N | N | N | N | N | JavaScript actions for nanoflows | | **Custom widgets** | N | N | N | N | N | N | N | N | N | N | N | N | N | N | N | N | N | Pluggable widget packages | diff --git a/mdl-examples/bug-tests/631-alter-styling-design-properties.mdl b/mdl-examples/bug-tests/631-alter-styling-design-properties.mdl new file mode 100644 index 000000000..4e63d5f6e --- /dev/null +++ b/mdl-examples/bug-tests/631-alter-styling-design-properties.mdl @@ -0,0 +1,67 @@ +-- ============================================================================ +-- Bug #631: ALTER STYLING cannot set design properties on widgets in pages +-- and snippets +-- ============================================================================ +-- +-- Symptom (before fix): +-- `ALTER STYLING ON PAGE ... WIDGET ... SET ...` always failed with +-- "unsupported container type: PAGE". The visitor stored ContainerType as +-- uppercase "PAGE"/"SNIPPET" but the executor compared against lowercase, so +-- it fell through to the unsupported-container error. The same casing bug +-- silently broke DESCRIBE STYLING. Even past the casing issue, ALTER STYLING +-- used a reflection walker (walkPageWidgets) that could not locate widgets in +-- pages created by the MDL builder, so there was no working write path for +-- design properties. +-- +-- After fix: +-- ALTER STYLING normalises the container type and goes through the page +-- mutator (OpenPageForMutation), which works uniformly on pages/snippets +-- created in Studio Pro and by the MDL builder. The mutator gained +-- SetDesignProperty / RemoveDesignProperty / ClearDesignProperties writing the +-- widget's Appearance.DesignProperties BSON array. +-- +-- Usage: +-- mxcli exec mdl-examples/bug-tests/631-alter-styling-design-properties.mdl -p app.mpr +-- Open in Studio Pro — the container shows the css class, inline style, and +-- Atlas design properties (Spacing bottom = Large, Full width toggle). +-- ============================================================================ + +create module BugTest631; + +-- A builder-created page (the case the old reflection walker could not handle). +create page BugTest631.P_Styling +( + title: 'Styling Target', + layout: Atlas_Core.Atlas_Default +) +{ + container ctnTarget (class: 'old-class') { + dynamictext txtHello (content: 'Hello') + } +} + +-- Class + inline style +ALTER STYLING ON PAGE BugTest631.P_Styling WIDGET ctnTarget + SET Class = 'card card-bordered', + Style = 'padding: 24px;'; + +-- Option (dropdown) design property +ALTER STYLING ON PAGE BugTest631.P_Styling WIDGET ctnTarget + SET 'Spacing bottom' = 'Large'; + +-- Toggle design property ON +ALTER STYLING ON PAGE BugTest631.P_Styling WIDGET ctnTarget + SET 'Full width' = ON; + +-- Read back — should report the class, style, and both design properties +DESCRIBE STYLING ON PAGE BugTest631.P_Styling WIDGET ctnTarget; + +-- Toggle OFF removes the property again +ALTER STYLING ON PAGE BugTest631.P_Styling WIDGET ctnTarget + SET 'Full width' = OFF; + +-- Clear all design properties (Class/Style remain) +ALTER STYLING ON PAGE BugTest631.P_Styling WIDGET ctnTarget + CLEAR DESIGN PROPERTIES; + +DESCRIBE STYLING ON PAGE BugTest631.P_Styling WIDGET ctnTarget; diff --git a/mdl-examples/doctype-tests/12-styling-examples.mdl b/mdl-examples/doctype-tests/12-styling-examples.mdl index 852ddb7ae..c6a281e24 100644 --- a/mdl-examples/doctype-tests/12-styling-examples.mdl +++ b/mdl-examples/doctype-tests/12-styling-examples.mdl @@ -572,26 +572,38 @@ describe styling on snippet StyleTest.MySnippet widget ctnSnippet; -- Change Class, Style, or DesignProperties on a single widget without -- rewriting the entire page. This is a targeted, in-place update. --- TODO: ALTER STYLING cannot find widgets in pages created by the MDL page builder --- because walkPageWidgets traverses LayoutCall.Arguments but the page parser doesn't --- fully reconstruct the widget tree when re-reading builder-created pages. --- These commands work on pages originally created in Studio Pro. - --- ALTER STYLING ON PAGE StyleTest.P006_Roundtrip WIDGET ctnHeader --- SET Class = 'card card-bordered'; --- ALTER STYLING ON PAGE StyleTest.P006_Roundtrip WIDGET ctnHeader --- SET Style = 'padding: 24px; border-radius: 12px;'; --- ALTER STYLING ON PAGE StyleTest.P006_Roundtrip WIDGET ctnHeader --- SET 'Spacing bottom' = 'Large'; --- ALTER STYLING ON PAGE StyleTest.P006_Roundtrip WIDGET ctnHeader --- SET 'Full width' = ON; --- ALTER STYLING ON PAGE StyleTest.P006_Roundtrip WIDGET ctnHeader --- SET 'Full width' = OFF; --- ALTER STYLING ON PAGE StyleTest.P006_Roundtrip WIDGET ctnHeader --- SET Class = 'card card-bordered', --- Style = 'padding: 24px;', --- 'Spacing bottom' = 'Large', --- 'Full width' = ON; --- ALTER STYLING ON PAGE StyleTest.P006_Roundtrip WIDGET ctnHeader --- CLEAR DESIGN PROPERTIES; --- DESCRIBE STYLING ON PAGE StyleTest.P006_Roundtrip WIDGET ctnHeader; +-- ALTER STYLING goes through the page mutator (issue #631), so it works on +-- pages created by the MDL builder as well as those authored in Studio Pro. + +-- Set a CSS class +ALTER STYLING ON PAGE StyleTest.P006_Roundtrip WIDGET ctnHeader + SET Class = 'card card-bordered'; + +-- Set an inline style +ALTER STYLING ON PAGE StyleTest.P006_Roundtrip WIDGET ctnHeader + SET Style = 'padding: 24px; border-radius: 12px;'; + +-- Set an option (dropdown) design property +ALTER STYLING ON PAGE StyleTest.P006_Roundtrip WIDGET ctnHeader + SET 'Spacing bottom' = 'Large'; + +-- Turn a toggle design property ON +ALTER STYLING ON PAGE StyleTest.P006_Roundtrip WIDGET ctnHeader + SET 'Full width' = ON; + +-- Turn a toggle design property OFF (removes it) +ALTER STYLING ON PAGE StyleTest.P006_Roundtrip WIDGET ctnHeader + SET 'Full width' = OFF; + +-- Combine Class, Style, and design properties in one statement +ALTER STYLING ON PAGE StyleTest.P006_Roundtrip WIDGET ctnHeader + SET Class = 'card card-bordered', + Style = 'padding: 24px;', + 'Spacing bottom' = 'Large', + 'Full width' = ON; + +-- Remove all design properties from a widget +ALTER STYLING ON PAGE StyleTest.P006_Roundtrip WIDGET ctnHeader + CLEAR DESIGN PROPERTIES; + +DESCRIBE STYLING ON PAGE StyleTest.P006_Roundtrip WIDGET ctnHeader; diff --git a/mdl/ast/ast_styling.go b/mdl/ast/ast_styling.go index 8f24098df..60a139ddd 100644 --- a/mdl/ast/ast_styling.go +++ b/mdl/ast/ast_styling.go @@ -37,6 +37,7 @@ func (s *AlterStylingStmt) isStatement() {} type StylingAssignment struct { Property string // "Class", "Style", or design property key (e.g., "Spacing top") Value string // Value string (CSS class, style, or option name) + IsCSS bool // true when set via the CLASS/STYLE keyword (appearance), false for a quoted design-property key IsToggle bool // true for ON/OFF values ToggleOn bool // true for ON, false for OFF (only meaningful when IsToggle is true) } diff --git a/mdl/backend/mock/mock_page_mutator.go b/mdl/backend/mock/mock_page_mutator.go index 82c205edd..f94fc2f9e 100644 --- a/mdl/backend/mock/mock_page_mutator.go +++ b/mdl/backend/mock/mock_page_mutator.go @@ -17,25 +17,28 @@ var _ backend.PageMutator = (*MockPageMutator)(nil) // nil error (never panics). ContainerType defaults to ContainerPage when unset; // all other methods return zero values. type MockPageMutator struct { - ContainerTypeFunc func() backend.ContainerKind - SetWidgetPropertyFunc func(widgetRef string, prop string, value any) error - SetWidgetDataSourceFunc func(widgetRef string, ds pages.DataSource) error - SetColumnPropertyFunc func(gridRef string, columnRef string, prop string, value any) error - InsertWidgetFunc func(widgetRef string, columnRef string, position backend.InsertPosition, widgets []pages.Widget) error - DropWidgetFunc func(refs []backend.WidgetRef) error - ReplaceWidgetFunc func(widgetRef string, columnRef string, widgets []pages.Widget) error - InsertColumnsFunc func(gridRef, afterColumnRef string, position backend.InsertPosition, columns []*backend.DataGridColumnSpec) error - ReplaceColumnFunc func(gridRef, columnRef string, columns []*backend.DataGridColumnSpec) error - FindWidgetFunc func(name string) bool - AddVariableFunc func(name, dataType, defaultValue string) error - DropVariableFunc func(name string) error - SetLayoutFunc func(newLayout string, paramMappings map[string]string) error - SetPluggablePropertyFunc func(widgetRef string, propKey string, op backend.PluggablePropertyOp, ctx backend.PluggablePropertyContext) error - EnclosingEntityFunc func(widgetRef string) string + ContainerTypeFunc func() backend.ContainerKind + SetWidgetPropertyFunc func(widgetRef string, prop string, value any) error + SetWidgetDataSourceFunc func(widgetRef string, ds pages.DataSource) error + SetColumnPropertyFunc func(gridRef string, columnRef string, prop string, value any) error + SetDesignPropertyFunc func(widgetRef string, key string, valueType string, option string) error + RemoveDesignPropertyFunc func(widgetRef string, key string) error + ClearDesignPropertiesFunc func(widgetRef string) error + InsertWidgetFunc func(widgetRef string, columnRef string, position backend.InsertPosition, widgets []pages.Widget) error + DropWidgetFunc func(refs []backend.WidgetRef) error + ReplaceWidgetFunc func(widgetRef string, columnRef string, widgets []pages.Widget) error + InsertColumnsFunc func(gridRef, afterColumnRef string, position backend.InsertPosition, columns []*backend.DataGridColumnSpec) error + ReplaceColumnFunc func(gridRef, columnRef string, columns []*backend.DataGridColumnSpec) error + FindWidgetFunc func(name string) bool + AddVariableFunc func(name, dataType, defaultValue string) error + DropVariableFunc func(name string) error + SetLayoutFunc func(newLayout string, paramMappings map[string]string) error + SetPluggablePropertyFunc func(widgetRef string, propKey string, op backend.PluggablePropertyOp, ctx backend.PluggablePropertyContext) error + EnclosingEntityFunc func(widgetRef string) string EnclosingEntityForChildrenFunc func(widgetRef string) string - WidgetScopeFunc func() map[string]model.ID - ParamScopeFunc func() (map[string]model.ID, map[string]string) - SaveFunc func() error + WidgetScopeFunc func() map[string]model.ID + ParamScopeFunc func() (map[string]model.ID, map[string]string) + SaveFunc func() error } func (m *MockPageMutator) ContainerType() backend.ContainerKind { @@ -66,6 +69,27 @@ func (m *MockPageMutator) SetColumnProperty(gridRef string, columnRef string, pr return nil } +func (m *MockPageMutator) SetDesignProperty(widgetRef string, key string, valueType string, option string) error { + if m.SetDesignPropertyFunc != nil { + return m.SetDesignPropertyFunc(widgetRef, key, valueType, option) + } + return nil +} + +func (m *MockPageMutator) RemoveDesignProperty(widgetRef string, key string) error { + if m.RemoveDesignPropertyFunc != nil { + return m.RemoveDesignPropertyFunc(widgetRef, key) + } + return nil +} + +func (m *MockPageMutator) ClearDesignProperties(widgetRef string) error { + if m.ClearDesignPropertiesFunc != nil { + return m.ClearDesignPropertiesFunc(widgetRef) + } + return nil +} + func (m *MockPageMutator) InsertWidget(widgetRef string, columnRef string, position backend.InsertPosition, widgets []pages.Widget) error { if m.InsertWidgetFunc != nil { return m.InsertWidgetFunc(widgetRef, columnRef, position, widgets) diff --git a/mdl/backend/mpr/design_property_mut_test.go b/mdl/backend/mpr/design_property_mut_test.go new file mode 100644 index 000000000..e5bb26754 --- /dev/null +++ b/mdl/backend/mpr/design_property_mut_test.go @@ -0,0 +1,227 @@ +// SPDX-License-Identifier: Apache-2.0 + +package mprbackend + +import ( + "testing" + + "go.mongodb.org/mongo-driver/bson" +) + +// makeStyleableWidget builds a widget with a Forms$Appearance sub-document and +// an empty (marker-only) DesignProperties array, matching serializeAppearance. +func makeStyleableWidget(name string) bson.D { + return bson.D{ + {Key: "$Type", Value: "Pages$DivContainer"}, + {Key: "Name", Value: name}, + {Key: "Appearance", Value: bson.D{ + {Key: "$Type", Value: "Forms$Appearance"}, + {Key: "Class", Value: ""}, + {Key: "DesignProperties", Value: bson.A{int32(3)}}, + {Key: "DynamicClasses", Value: ""}, + {Key: "Style", Value: ""}, + }}, + } +} + +// designPropEntries returns the DesignPropertyValue entries (marker stripped) for a widget. +func designPropEntries(t *testing.T, rawData bson.D, widgetName string) []any { + t.Helper() + result := findBsonWidget(rawData, widgetName) + if result == nil { + t.Fatalf("widget %q not found", widgetName) + } + app := dGetDoc(result.widget, "Appearance") + if app == nil { + t.Fatalf("widget %q has no Appearance", widgetName) + } + return dGetArrayElements(dGet(app, "DesignProperties")) +} + +// findEntry returns the DesignPropertyValue entry with the given Key, or nil. +func findEntry(entries []any, key string) bson.D { + for _, el := range entries { + if entry, ok := el.(bson.D); ok && dGetString(entry, "Key") == key { + return entry + } + } + return nil +} + +func TestSetDesignProperty_ToggleOn_Appends(t *testing.T) { + rawData := makeRawPage(makeStyleableWidget("ctn1")) + m := &mprPageMutator{rawData: rawData, widgetFinder: findBsonWidget} + + if err := m.SetDesignProperty("ctn1", "Full width", "toggle", ""); err != nil { + t.Fatalf("SetDesignProperty failed: %v", err) + } + + entries := designPropEntries(t, rawData, "ctn1") + if len(entries) != 1 { + t.Fatalf("expected 1 design property, got %d", len(entries)) + } + entry := findEntry(entries, "Full width") + if entry == nil { + t.Fatal("expected entry for 'Full width'") + } + if dGetString(entry, "$Type") != designPropertyEntryType { + t.Errorf("expected entry $Type=%q, got %q", designPropertyEntryType, dGetString(entry, "$Type")) + } + val := dGetDoc(entry, "Value") + if val == nil || dGetString(val, "$Type") != toggleDesignPropertyType { + t.Errorf("expected toggle value, got %#v", dGet(entry, "Value")) + } +} + +func TestSetDesignProperty_Option_Appends(t *testing.T) { + rawData := makeRawPage(makeStyleableWidget("ctn1")) + m := &mprPageMutator{rawData: rawData, widgetFinder: findBsonWidget} + + if err := m.SetDesignProperty("ctn1", "Spacing bottom", "option", "Large"); err != nil { + t.Fatalf("SetDesignProperty failed: %v", err) + } + + entry := findEntry(designPropEntries(t, rawData, "ctn1"), "Spacing bottom") + if entry == nil { + t.Fatal("expected entry for 'Spacing bottom'") + } + val := dGetDoc(entry, "Value") + if val == nil || dGetString(val, "$Type") != optionDesignPropertyType { + t.Fatalf("expected option value, got %#v", dGet(entry, "Value")) + } + if dGetString(val, "Option") != "Large" { + t.Errorf("expected Option='Large', got %q", dGetString(val, "Option")) + } +} + +func TestSetDesignProperty_Custom_Appends(t *testing.T) { + rawData := makeRawPage(makeStyleableWidget("ctn1")) + m := &mprPageMutator{rawData: rawData, widgetFinder: findBsonWidget} + + // ToggleButtonGroup/ColorPicker values use the "custom" value-type. + if err := m.SetDesignProperty("ctn1", "Flex container", "custom", "Horizontal (row)"); err != nil { + t.Fatalf("SetDesignProperty failed: %v", err) + } + + val := dGetDoc(findEntry(designPropEntries(t, rawData, "ctn1"), "Flex container"), "Value") + if val == nil || dGetString(val, "$Type") != customDesignPropertyType { + t.Fatalf("expected custom value type, got %#v", dGet(findEntry(designPropEntries(t, rawData, "ctn1"), "Flex container"), "Value")) + } + if dGetString(val, "Value") != "Horizontal (row)" { + t.Errorf("expected custom Value='Horizontal (row)', got %q", dGetString(val, "Value")) + } +} + +func TestSetDesignProperty_UpdatesExistingKeyInPlace(t *testing.T) { + rawData := makeRawPage(makeStyleableWidget("ctn1")) + m := &mprPageMutator{rawData: rawData, widgetFinder: findBsonWidget} + + // First set as toggle, then re-set the same key as an option. + if err := m.SetDesignProperty("ctn1", "Mode", "toggle", ""); err != nil { + t.Fatalf("first set failed: %v", err) + } + if err := m.SetDesignProperty("ctn1", "Mode", "option", "Compact"); err != nil { + t.Fatalf("second set failed: %v", err) + } + + entries := designPropEntries(t, rawData, "ctn1") + if len(entries) != 1 { + t.Fatalf("expected key updated in place (1 entry), got %d", len(entries)) + } + val := dGetDoc(findEntry(entries, "Mode"), "Value") + if dGetString(val, "$Type") != optionDesignPropertyType || dGetString(val, "Option") != "Compact" { + t.Errorf("expected option 'Compact', got %#v", val) + } +} + +// An option-typed set must overwrite a stale Custom value with an +// OptionDesignPropertyValue. ToggleButtonGroup values are Option, not Custom; +// writing Custom triggers Studio Pro CE6084, so re-applying must repair it. +func TestSetDesignProperty_OptionOverwritesCustom(t *testing.T) { + w := makeStyleableWidget("ctn1") + app := dGetDoc(w, "Appearance") + dSetArray(app, "DesignProperties", []any{ + bson.D{ + {Key: "$Type", Value: designPropertyEntryType}, + {Key: "Key", Value: "Flex container"}, + {Key: "Value", Value: bson.D{ + {Key: "$Type", Value: customDesignPropertyType}, + {Key: "Value", Value: "Horizontal (row)"}, + }}, + }, + }) + rawData := makeRawPage(w) + m := &mprPageMutator{rawData: rawData, widgetFinder: findBsonWidget} + + if err := m.SetDesignProperty("ctn1", "Flex container", "option", "Horizontal (row)"); err != nil { + t.Fatalf("SetDesignProperty failed: %v", err) + } + + val := dGetDoc(findEntry(designPropEntries(t, rawData, "ctn1"), "Flex container"), "Value") + if dGetString(val, "$Type") != optionDesignPropertyType { + t.Errorf("expected Custom overwritten with Option, got %q", dGetString(val, "$Type")) + } + if dGetString(val, "Option") != "Horizontal (row)" { + t.Errorf("expected Option='Horizontal (row)', got %q", dGetString(val, "Option")) + } +} + +func TestRemoveDesignProperty(t *testing.T) { + rawData := makeRawPage(makeStyleableWidget("ctn1")) + m := &mprPageMutator{rawData: rawData, widgetFinder: findBsonWidget} + _ = m.SetDesignProperty("ctn1", "A", "toggle", "") + _ = m.SetDesignProperty("ctn1", "B", "toggle", "") + + if err := m.RemoveDesignProperty("ctn1", "A"); err != nil { + t.Fatalf("RemoveDesignProperty failed: %v", err) + } + + entries := designPropEntries(t, rawData, "ctn1") + if len(entries) != 1 || findEntry(entries, "A") != nil || findEntry(entries, "B") == nil { + t.Fatalf("expected only 'B' to remain, got %d entries", len(entries)) + } +} + +func TestClearDesignProperties(t *testing.T) { + rawData := makeRawPage(makeStyleableWidget("ctn1")) + m := &mprPageMutator{rawData: rawData, widgetFinder: findBsonWidget} + _ = m.SetDesignProperty("ctn1", "A", "toggle", "") + _ = m.SetDesignProperty("ctn1", "B", "option", "X") + + if err := m.ClearDesignProperties("ctn1"); err != nil { + t.Fatalf("ClearDesignProperties failed: %v", err) + } + + if entries := designPropEntries(t, rawData, "ctn1"); len(entries) != 0 { + t.Fatalf("expected all design properties cleared, got %d", len(entries)) + } + // Marker must be preserved so the array still serializes correctly. + result := findBsonWidget(rawData, "ctn1") + arr := toBsonA(dGet(dGetDoc(result.widget, "Appearance"), "DesignProperties")) + if len(arr) != 1 { + t.Fatalf("expected marker-only array, got %d elements", len(arr)) + } + if _, ok := arr[0].(int32); !ok { + t.Errorf("expected int32 marker preserved, got %T", arr[0]) + } +} + +func TestSetDesignProperty_WidgetNotFound(t *testing.T) { + rawData := makeRawPage(makeStyleableWidget("ctn1")) + m := &mprPageMutator{rawData: rawData, widgetFinder: findBsonWidget} + if err := m.SetDesignProperty("nope", "A", "toggle", ""); err == nil { + t.Fatal("expected error for nonexistent widget") + } +} + +func TestSetDesignProperty_NoAppearance(t *testing.T) { + w := bson.D{ + {Key: "$Type", Value: "Pages$DivContainer"}, + {Key: "Name", Value: "ctn1"}, + } + rawData := makeRawPage(w) + m := &mprPageMutator{rawData: rawData, widgetFinder: findBsonWidget} + if err := m.SetDesignProperty("ctn1", "A", "toggle", ""); err == nil { + t.Fatal("expected error when widget has no Appearance") + } +} diff --git a/mdl/backend/mpr/page_mutator.go b/mdl/backend/mpr/page_mutator.go index be4fd140d..6041e6f55 100644 --- a/mdl/backend/mpr/page_mutator.go +++ b/mdl/backend/mpr/page_mutator.go @@ -113,6 +113,39 @@ func (m *mprPageMutator) SetColumnProperty(gridRef string, columnRef string, pro return setColumnPropertyMut(result.widget, result.colPropKeys, prop, value) } +func (m *mprPageMutator) SetDesignProperty(widgetRef, key, valueType, option string) error { + widget, err := m.findStyleableWidget(widgetRef) + if err != nil { + return err + } + return setDesignPropertyMut(widget, key, valueType, option) +} + +func (m *mprPageMutator) RemoveDesignProperty(widgetRef, key string) error { + widget, err := m.findStyleableWidget(widgetRef) + if err != nil { + return err + } + return removeDesignPropertyMut(widget, key) +} + +func (m *mprPageMutator) ClearDesignProperties(widgetRef string) error { + widget, err := m.findStyleableWidget(widgetRef) + if err != nil { + return err + } + return clearDesignPropertiesMut(widget) +} + +// findStyleableWidget locates a widget by name for design-property operations. +func (m *mprPageMutator) findStyleableWidget(widgetRef string) (bson.D, error) { + result := m.widgetFinder(m.rawData, widgetRef) + if result == nil { + return nil, fmt.Errorf("widget %q not found", widgetRef) + } + return result.widget, nil +} + func (m *mprPageMutator) InsertWidget(widgetRef string, columnRef string, position backend.InsertPosition, widgets []pages.Widget) error { var result *bsonWidgetResult if columnRef != "" { @@ -1711,6 +1744,107 @@ func setRawWidgetPropertyMut(widget bson.D, propName string, value any) error { } } +// --------------------------------------------------------------------------- +// Design property (Atlas styling) mutation +// --------------------------------------------------------------------------- + +const ( + designPropertyEntryType = "Forms$DesignPropertyValue" + toggleDesignPropertyType = "Forms$ToggleDesignPropertyValue" + optionDesignPropertyType = "Forms$OptionDesignPropertyValue" + customDesignPropertyType = "Forms$CustomDesignPropertyValue" +) + +// setDesignPropertyMut sets or updates a single design property in the widget's +// Appearance.DesignProperties array. valueType is "toggle" (no value) or "option" +// (carries option). An existing entry's Value is fully rewritten to the new +// valueType — so an option-type set on a stale "custom" value +// (ToggleButtonGroup/ColorPicker) overwrites it with an OptionDesignPropertyValue, +// repairing the CE6084 that a Custom encoding triggers (see +// buildDesignPropertyValueDoc and TestSetDesignProperty_OptionOverwritesCustom). +func setDesignPropertyMut(widget bson.D, key, valueType, option string) error { + appearance := dGetDoc(widget, "Appearance") + if appearance == nil { + return fmt.Errorf("widget has no Appearance; cannot set design property %q", key) + } + elements := dGetArrayElements(dGet(appearance, "DesignProperties")) + + for _, el := range elements { + entry, ok := el.(bson.D) + if !ok || dGetString(entry, "Key") != key { + continue + } + dSet(entry, "Value", buildDesignPropertyValueDoc(valueType, option)) + dSetArray(appearance, "DesignProperties", elements) + return nil + } + + entry := bson.D{ + {Key: "$ID", Value: bsonutil.NewIDBsonBinary()}, + {Key: "$Type", Value: designPropertyEntryType}, + {Key: "Key", Value: key}, + {Key: "Value", Value: buildDesignPropertyValueDoc(valueType, option)}, + } + dSetArray(appearance, "DesignProperties", append(elements, entry)) + return nil +} + +// removeDesignPropertyMut removes a single design property by key. +func removeDesignPropertyMut(widget bson.D, key string) error { + appearance := dGetDoc(widget, "Appearance") + if appearance == nil { + return nil + } + elements := dGetArrayElements(dGet(appearance, "DesignProperties")) + kept := make([]any, 0, len(elements)) + for _, el := range elements { + if entry, ok := el.(bson.D); ok && dGetString(entry, "Key") == key { + continue + } + kept = append(kept, el) + } + dSetArray(appearance, "DesignProperties", kept) + return nil +} + +// clearDesignPropertiesMut removes all design properties from the widget, +// leaving an empty (marker-only) array. +func clearDesignPropertiesMut(widget bson.D) error { + appearance := dGetDoc(widget, "Appearance") + if appearance == nil { + return nil + } + dSetArray(appearance, "DesignProperties", nil) + return nil +} + +// buildDesignPropertyValueDoc builds the typed Value sub-document for a design +// property entry. valueType is "toggle", "option", or "custom". Single-selection +// design properties (Dropdown AND ToggleButtonGroup) use "option" +// (Forms$OptionDesignPropertyValue) — verified against Studio Pro-authored +// widgets; encoding a ToggleButtonGroup value as "custom" triggers CE6084. +func buildDesignPropertyValueDoc(valueType, option string) bson.D { + switch valueType { + case "toggle": + return bson.D{ + {Key: "$ID", Value: bsonutil.NewIDBsonBinary()}, + {Key: "$Type", Value: toggleDesignPropertyType}, + } + case "custom": + return bson.D{ + {Key: "$ID", Value: bsonutil.NewIDBsonBinary()}, + {Key: "$Type", Value: customDesignPropertyType}, + {Key: "Value", Value: option}, + } + default: + return bson.D{ + {Key: "$ID", Value: bsonutil.NewIDBsonBinary()}, + {Key: "$Type", Value: optionDesignPropertyType}, + {Key: "Option", Value: option}, + } + } +} + func setWidgetCaptionMut(widget bson.D, value any) error { caption := dGetDoc(widget, "Caption") if caption == nil { diff --git a/mdl/backend/mutation.go b/mdl/backend/mutation.go index 4032f9bb0..11589b7cf 100644 --- a/mdl/backend/mutation.go +++ b/mdl/backend/mutation.go @@ -83,6 +83,23 @@ type PageMutator interface { // SetColumnProperty sets a property on a column within a grid widget. SetColumnProperty(gridRef string, columnRef string, prop string, value any) error + // --- Design property (Atlas styling) operations --- + + // SetDesignProperty sets or updates an Atlas design property on the named + // widget's Appearance. valueType is "toggle" (no value) or "option" (carries + // option). An existing entry's value is fully rewritten to the new valueType: + // an option-type set on a stale "custom" value (ToggleButtonGroup/ColorPicker) + // overwrites it with an option value, repairing the CE6084 a Custom encoding + // triggers. + SetDesignProperty(widgetRef string, key string, valueType string, option string) error + + // RemoveDesignProperty removes a single design property by key from the named + // widget (e.g. a toggle set to OFF). + RemoveDesignProperty(widgetRef string, key string) error + + // ClearDesignProperties removes all design properties from the named widget. + ClearDesignProperties(widgetRef string) error + // --- Widget tree operations --- // InsertWidget inserts serialized widgets at the given position diff --git a/mdl/executor/cmd_styling.go b/mdl/executor/cmd_styling.go index aa25e57b7..e7bcfcaa7 100644 --- a/mdl/executor/cmd_styling.go +++ b/mdl/executor/cmd_styling.go @@ -5,11 +5,11 @@ package executor import ( "fmt" "path/filepath" - "reflect" "sort" "strings" "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend" mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" "github.com/mendixlabs/mxcli/model" "github.com/mendixlabs/mxcli/sdk/pages" @@ -127,7 +127,11 @@ func execDescribeStyling(ctx *ExecContext, s *ast.DescribeStylingStmt) error { var rawWidgets []rawWidget - if s.ContainerType == "page" { + // ContainerType is stored uppercase ("PAGE"/"SNIPPET") by the visitor; + // normalize so comparisons are case-insensitive. + containerType := strings.ToLower(s.ContainerType) + + if containerType == "page" { // Find page allPages, err := ctx.Backend.ListPages() if err != nil { @@ -147,7 +151,7 @@ func execDescribeStyling(ctx *ExecContext, s *ast.DescribeStylingStmt) error { return mdlerrors.NewNotFound("page", s.ContainerName.String()) } rawWidgets = getPageWidgetsFromRaw(ctx, foundPage.ID) - } else if s.ContainerType == "snippet" { + } else if containerType == "snippet" { // Find snippet allSnippets, err := ctx.Backend.ListSnippets() if err != nil { @@ -170,7 +174,7 @@ func execDescribeStyling(ctx *ExecContext, s *ast.DescribeStylingStmt) error { } if len(rawWidgets) == 0 { - fmt.Fprintf(ctx.Output, "No widgets found in %s %s\n", s.ContainerType, s.ContainerName.String()) + fmt.Fprintf(ctx.Output, "No widgets found in %s %s\n", containerType, s.ContainerName.String()) return nil } @@ -179,9 +183,9 @@ func execDescribeStyling(ctx *ExecContext, s *ast.DescribeStylingStmt) error { if len(styledWidgets) == 0 { if s.WidgetName != "" { - return mdlerrors.NewNotFoundMsg("widget", s.WidgetName, fmt.Sprintf("widget %q not found in %s %s", s.WidgetName, s.ContainerType, s.ContainerName.String())) + return mdlerrors.NewNotFoundMsg("widget", s.WidgetName, fmt.Sprintf("widget %q not found in %s %s", s.WidgetName, containerType, s.ContainerName.String())) } - fmt.Fprintf(ctx.Output, "No styled widgets found in %s %s\n", s.ContainerType, s.ContainerName.String()) + fmt.Fprintf(ctx.Output, "No styled widgets found in %s %s\n", containerType, s.ContainerName.String()) return nil } @@ -271,185 +275,86 @@ func execAlterStyling(ctx *ExecContext, s *ast.AlterStylingStmt) error { return mdlerrors.NewBackend("build hierarchy", err) } - if s.ContainerType == "page" { - return alterStylingOnPage(ctx, s, h) - } else if s.ContainerType == "snippet" { - return alterStylingOnSnippet(ctx, s, h) - } - - return mdlerrors.NewUnsupported("unsupported container type: " + s.ContainerType) -} - -func alterStylingOnPage(ctx *ExecContext, s *ast.AlterStylingStmt, h *ContainerHierarchy) error { + // ContainerType is stored uppercase ("PAGE"/"SNIPPET") by the visitor; + // normalize so comparisons are case-insensitive. + containerType := strings.ToLower(s.ContainerType) - // Find page - allPages, err := ctx.Backend.ListPages() - if err != nil { - return mdlerrors.NewBackend("list pages", err) - } - - var page *pages.Page - for _, p := range allPages { - modID := h.FindModuleID(p.ContainerID) - modName := h.GetModuleName(modID) - if p.Name == s.ContainerName.Name && (s.ContainerName.Module == "" || modName == s.ContainerName.Module) { - page = p - break + var unitID model.ID + switch containerType { + case "page": + page, err := findPageByName(ctx, s.ContainerName, h) + if err != nil { + return err } - } - if page == nil { - return mdlerrors.NewNotFound("page", s.ContainerName.String()) - } - - // Walk the page to find the widget by name - found := false - err = walkPageWidgets(page, func(widget any) error { - name := getWidgetName(widget) - if name != s.WidgetName { - return nil + unitID = page.ID + case "snippet": + snippet, _, err := findSnippetByName(ctx, s.ContainerName, h) + if err != nil { + return err } - found = true - return applyStylingAssignments(widget, s.Assignments, s.ClearDesignProps) - }) - if err != nil { - return err - } - - if !found { - return mdlerrors.NewNotFoundMsg("widget", s.WidgetName, fmt.Sprintf("widget %q not found in page %s", s.WidgetName, s.ContainerName.String())) - } - - // Save the page - if err := ctx.Backend.UpdatePage(page); err != nil { - return mdlerrors.NewBackend("save page", err) + unitID = snippet.ID + default: + return mdlerrors.NewUnsupported("unsupported container type: " + s.ContainerType) } - fmt.Fprintf(ctx.Output, "Updated styling on widget %q in page %s\n", s.WidgetName, s.ContainerName.String()) - return nil -} - -func alterStylingOnSnippet(ctx *ExecContext, s *ast.AlterStylingStmt, h *ContainerHierarchy) error { - - // Find snippet - allSnippets, err := ctx.Backend.ListSnippets() + // Open the unit for mutation via the backend (mutator pattern). This works + // uniformly on pages/snippets created in Studio Pro and by the MDL builder. + mutator, err := ctx.Backend.OpenPageForMutation(unitID) if err != nil { - return mdlerrors.NewBackend("list snippets", err) - } - - var snippet *pages.Snippet - for _, sn := range allSnippets { - modID := h.FindModuleID(sn.ContainerID) - modName := h.GetModuleName(modID) - if sn.Name == s.ContainerName.Name && (s.ContainerName.Module == "" || modName == s.ContainerName.Module) { - snippet = sn - break - } - } - if snippet == nil { - return mdlerrors.NewNotFound("snippet", s.ContainerName.String()) + return mdlerrors.NewBackend("open "+containerType+" for mutation", err) } - // Walk the snippet to find the widget by name - found := false - err = walkSnippetWidgets(snippet, func(widget any) error { - name := getWidgetName(widget) - if name != s.WidgetName { - return nil - } - found = true - return applyStylingAssignments(widget, s.Assignments, s.ClearDesignProps) - }) - if err != nil { - return err + if !mutator.FindWidget(s.WidgetName) { + return mdlerrors.NewNotFoundMsg("widget", s.WidgetName, + fmt.Sprintf("widget %q not found in %s %s", s.WidgetName, containerType, s.ContainerName.String())) } - if !found { - return mdlerrors.NewNotFoundMsg("widget", s.WidgetName, fmt.Sprintf("widget %q not found in snippet %s", s.WidgetName, s.ContainerName.String())) + if err := applyStylingMutator(mutator, s); err != nil { + return mdlerrors.NewBackend("alter styling", err) } - // Save the snippet - if err := ctx.Backend.UpdateSnippet(snippet); err != nil { - return mdlerrors.NewBackend("save snippet", err) + if err := mutator.Save(); err != nil { + return mdlerrors.NewBackend("save "+containerType, err) } - fmt.Fprintf(ctx.Output, "Updated styling on widget %q in snippet %s\n", s.WidgetName, s.ContainerName.String()) + fmt.Fprintf(ctx.Output, "Updated styling on widget %q in %s %s\n", s.WidgetName, containerType, s.ContainerName.String()) return nil } -// getWidgetName extracts the Name from a widget using reflection. -func getWidgetName(widget any) string { - if widget == nil { - return "" - } - - // Try Widget interface first - if w, ok := widget.(pages.Widget); ok { - return w.GetName() - } - - // Fall back to reflection - v := reflect.ValueOf(widget) - if v.Kind() == reflect.Pointer { - v = v.Elem() - } - if v.Kind() != reflect.Struct { - return "" - } - - // Try BaseWidget.Name - if baseWidget := v.FieldByName("BaseWidget"); baseWidget.IsValid() { - if nameField := baseWidget.FieldByName("Name"); nameField.IsValid() { - return nameField.String() +// applyStylingMutator applies the ALTER STYLING assignments through the page +// mutator. CLEAR DESIGN PROPERTIES is applied first, then each assignment in order. +func applyStylingMutator(mutator backend.PageMutator, s *ast.AlterStylingStmt) error { + if s.ClearDesignProps { + if err := mutator.ClearDesignProperties(s.WidgetName); err != nil { + return err } } - // Direct Name field - if nameField := v.FieldByName("Name"); nameField.IsValid() { - return nameField.String() - } - - return "" -} - -// applyStylingAssignments applies styling changes to a widget. -func applyStylingAssignments(widget any, assignments []ast.StylingAssignment, clearDesignProps bool) error { - v := reflect.ValueOf(widget) - if v.Kind() == reflect.Pointer { - v = v.Elem() - } - if v.Kind() != reflect.Struct { - return mdlerrors.NewValidation("widget is not a struct") - } - - // Get BaseWidget - baseWidget := v.FieldByName("BaseWidget") - if !baseWidget.IsValid() { - return mdlerrors.NewValidation("widget has no BaseWidget field") - } - - // Clear design properties if requested - if clearDesignProps { - dpField := baseWidget.FieldByName("DesignProperties") - if dpField.IsValid() && dpField.CanSet() { - dpField.Set(reflect.Zero(dpField.Type())) + for _, a := range s.Assignments { + // CLASS/STYLE keywords set the widget's CSS appearance — not a design + // property (a quoted key named 'Class'/'Style' is a design property). + if a.IsCSS { + if err := mutator.SetWidgetProperty(s.WidgetName, a.Property, a.Value); err != nil { + return err + } + continue } - } - for _, a := range assignments { - switch a.Property { - case "Class": - classField := baseWidget.FieldByName("Class") - if classField.IsValid() && classField.CanSet() { - classField.SetString(a.Value) + switch { + case a.IsToggle && a.ToggleOn: + if err := mutator.SetDesignProperty(s.WidgetName, a.Property, "toggle", ""); err != nil { + return err } - case "Style": - styleField := baseWidget.FieldByName("Style") - if styleField.IsValid() && styleField.CanSet() { - styleField.SetString(a.Value) + case a.IsToggle && !a.ToggleOn: + if err := mutator.RemoveDesignProperty(s.WidgetName, a.Property); err != nil { + return err } default: - // Design property assignment - if err := setDesignProperty(baseWidget, a); err != nil { + // A single-selection design property (Dropdown or ToggleButtonGroup) + // is stored as Forms$OptionDesignPropertyValue — verified against + // Studio Pro-authored widgets. (The SDK's "custom" classification for + // ToggleButtonGroup is incorrect and triggers CE6084.) + if err := mutator.SetDesignProperty(s.WidgetName, a.Property, "option", a.Value); err != nil { return err } } @@ -458,64 +363,6 @@ func applyStylingAssignments(widget any, assignments []ast.StylingAssignment, cl return nil } -// setDesignProperty sets or updates a design property on the widget's BaseWidget. -func setDesignProperty(baseWidget reflect.Value, a ast.StylingAssignment) error { - dpField := baseWidget.FieldByName("DesignProperties") - if !dpField.IsValid() || !dpField.CanSet() { - return mdlerrors.NewUnsupported("widget does not support design properties") - } - - // Get existing design properties - var existing []pages.DesignPropertyValue - if !dpField.IsNil() { - existing = dpField.Interface().([]pages.DesignPropertyValue) - } - - if a.IsToggle && !a.ToggleOn { - // OFF: remove the design property - var updated []pages.DesignPropertyValue - for _, dp := range existing { - if dp.Key != a.Property { - updated = append(updated, dp) - } - } - dpField.Set(reflect.ValueOf(updated)) - return nil - } - - // Update existing or append new - found := false - for i, dp := range existing { - if dp.Key == a.Property { - if a.IsToggle { - existing[i].ValueType = "toggle" - existing[i].Option = "" - } else { - existing[i].ValueType = "option" - existing[i].Option = a.Value - } - found = true - break - } - } - - if !found { - newProp := pages.DesignPropertyValue{ - Key: a.Property, - } - if a.IsToggle { - newProp.ValueType = "toggle" - } else { - newProp.ValueType = "option" - newProp.Option = a.Value - } - existing = append(existing, newProp) - } - - dpField.Set(reflect.ValueOf(existing)) - return nil -} - // findPageByName looks up a page by qualified name. func findPageByName(ctx *ExecContext, name ast.QualifiedName, h *ContainerHierarchy) (*pages.Page, error) { diff --git a/mdl/executor/cmd_styling_mock_test.go b/mdl/executor/cmd_styling_mock_test.go index e614c6ec6..bafc0959d 100644 --- a/mdl/executor/cmd_styling_mock_test.go +++ b/mdl/executor/cmd_styling_mock_test.go @@ -6,7 +6,11 @@ import ( "testing" "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend" "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/pages" ) // --------------------------------------------------------------------------- @@ -78,8 +82,184 @@ func TestAlterStyling_NotConnected(t *testing.T) { assertContainsStr(t, err.Error(), "not connected") } -// NOTE: execAlterStyling happy path uses walkPageWidgets / walkSnippetWidgets -// (reflection-based applyStylingAssignments on real Page/Snippet structs) + -// ListPages/UpdatePage. The reflection walker needs real page struct data. -// ConnectedForWrite delegates to Connected — cannot differentiate in mock. -// Tracked separately. +// stylingMutatorBackend wires a mock backend whose OpenPageForMutation returns +// the supplied mutator, with a page "MyModule.Home" resolvable. +func stylingMutatorBackend(t *testing.T, mut *mock.MockPageMutator) (*ExecContext, *mock.MockBackend, func() string) { + t.Helper() + mod := mkModule("MyModule") + pg := mkPage(mod.ID, "Home") + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListFoldersFunc: func() ([]*types.FolderInfo, error) { return nil, nil }, + ListPagesFunc: func() ([]*pages.Page, error) { return []*pages.Page{pg}, nil }, + OpenPageForMutationFunc: func(unitID model.ID) (backend.PageMutator, error) { + return mut, nil + }, + } + h := mkHierarchy(mod) + withContainer(h, pg.ContainerID, mod.ID) + ctx, buf := newMockCtx(t, withBackend(mb), withHierarchy(h)) + return ctx, mb, buf.String +} + +// Issue #631 — visitor sets ContainerType to uppercase "PAGE"; the executor must +// normalise before comparing, then go through the page mutator (not the old +// reflection walker) so it works on builder-created pages too. +func TestAlterStyling_UppercaseContainerType_SetClass_Issue631(t *testing.T) { + saved := false + var gotProp, gotValue string + mut := &mock.MockPageMutator{ + FindWidgetFunc: func(name string) bool { return name == "ctnHeader" }, + SetWidgetPropertyFunc: func(widgetRef, prop string, value any) error { + gotProp = prop + gotValue, _ = value.(string) + return nil + }, + SaveFunc: func() error { saved = true; return nil }, + } + ctx, _, out := stylingMutatorBackend(t, mut) + + assertNoError(t, execAlterStyling(ctx, &ast.AlterStylingStmt{ + ContainerType: "PAGE", // uppercase, as produced by the visitor + ContainerName: ast.QualifiedName{Module: "MyModule", Name: "Home"}, + WidgetName: "ctnHeader", + Assignments: []ast.StylingAssignment{ + {Property: "Class", Value: "card card-bordered", IsCSS: true}, + }, + })) + + if gotProp != "Class" || gotValue != "card card-bordered" { + t.Errorf("expected Class='card card-bordered', got %s=%q", gotProp, gotValue) + } + if !saved { + t.Error("expected Save to be called") + } + assertContainsStr(t, out(), "Updated styling on widget \"ctnHeader\"") +} + +func TestAlterStyling_DesignProperties_ToggleAndOption(t *testing.T) { + type call struct { + key, valueType, option string + } + var calls []call + mut := &mock.MockPageMutator{ + FindWidgetFunc: func(name string) bool { return true }, + SetDesignPropertyFunc: func(widgetRef, key, valueType, option string) error { + calls = append(calls, call{key, valueType, option}) + return nil + }, + SaveFunc: func() error { return nil }, + } + ctx, _, _ := stylingMutatorBackend(t, mut) + + assertNoError(t, execAlterStyling(ctx, &ast.AlterStylingStmt{ + ContainerType: "PAGE", + ContainerName: ast.QualifiedName{Module: "MyModule", Name: "Home"}, + WidgetName: "ctnHeader", + Assignments: []ast.StylingAssignment{ + {Property: "Full width", IsToggle: true, ToggleOn: true}, + {Property: "Spacing bottom", Value: "Large"}, + }, + })) + + if len(calls) != 2 { + t.Fatalf("expected 2 SetDesignProperty calls, got %d", len(calls)) + } + if calls[0] != (call{"Full width", "toggle", ""}) { + t.Errorf("toggle call wrong: %+v", calls[0]) + } + if calls[1] != (call{"Spacing bottom", "option", "Large"}) { + t.Errorf("option call wrong: %+v", calls[1]) + } +} + +func TestAlterStyling_ToggleOff_RemovesDesignProperty(t *testing.T) { + var removed string + mut := &mock.MockPageMutator{ + FindWidgetFunc: func(name string) bool { return true }, + RemoveDesignPropertyFunc: func(widgetRef, key string) error { removed = key; return nil }, + SaveFunc: func() error { return nil }, + } + ctx, _, _ := stylingMutatorBackend(t, mut) + + assertNoError(t, execAlterStyling(ctx, &ast.AlterStylingStmt{ + ContainerType: "PAGE", + ContainerName: ast.QualifiedName{Module: "MyModule", Name: "Home"}, + WidgetName: "ctnHeader", + Assignments: []ast.StylingAssignment{ + {Property: "Full width", IsToggle: true, ToggleOn: false}, + }, + })) + + if removed != "Full width" { + t.Errorf("expected RemoveDesignProperty('Full width'), got %q", removed) + } +} + +func TestAlterStyling_ClearDesignProperties(t *testing.T) { + cleared := false + mut := &mock.MockPageMutator{ + FindWidgetFunc: func(name string) bool { return true }, + ClearDesignPropertiesFunc: func(widgetRef string) error { cleared = true; return nil }, + SaveFunc: func() error { return nil }, + } + ctx, _, _ := stylingMutatorBackend(t, mut) + + assertNoError(t, execAlterStyling(ctx, &ast.AlterStylingStmt{ + ContainerType: "PAGE", + ContainerName: ast.QualifiedName{Module: "MyModule", Name: "Home"}, + WidgetName: "ctnHeader", + ClearDesignProps: true, + })) + + if !cleared { + t.Error("expected ClearDesignProperties to be called") + } +} + +// Issue #631 follow-up: a quoted design-property key named 'Style' must be +// treated as a design property (e.g. Button Style = 'Icon button'), NOT routed +// to the CSS Style widget property. IsCSS is false for quoted keys. +func TestAlterStyling_QuotedStyleIsDesignProperty(t *testing.T) { + var dpKey, dpVal string + cssCalled := false + mut := &mock.MockPageMutator{ + FindWidgetFunc: func(name string) bool { return true }, + SetWidgetPropertyFunc: func(widgetRef, prop string, value any) error { cssCalled = true; return nil }, + SetDesignPropertyFunc: func(widgetRef, key, valueType, option string) error { dpKey = key; dpVal = option; return nil }, + SaveFunc: func() error { return nil }, + } + ctx, _, _ := stylingMutatorBackend(t, mut) + + assertNoError(t, execAlterStyling(ctx, &ast.AlterStylingStmt{ + ContainerType: "PAGE", + ContainerName: ast.QualifiedName{Module: "MyModule", Name: "Home"}, + WidgetName: "btnSave", + Assignments: []ast.StylingAssignment{ + {Property: "Style", Value: "Icon button", IsCSS: false}, // quoted 'Style' design property + }, + })) + + if cssCalled { + t.Error("quoted 'Style' must NOT be routed to the CSS Style property") + } + if dpKey != "Style" || dpVal != "Icon button" { + t.Errorf("expected design property Style='Icon button', got %s=%q", dpKey, dpVal) + } +} + +func TestAlterStyling_WidgetNotFound(t *testing.T) { + mut := &mock.MockPageMutator{ + FindWidgetFunc: func(name string) bool { return false }, + } + ctx, _, _ := stylingMutatorBackend(t, mut) + + err := execAlterStyling(ctx, &ast.AlterStylingStmt{ + ContainerType: "PAGE", + ContainerName: ast.QualifiedName{Module: "MyModule", Name: "Home"}, + WidgetName: "ghost", + }) + assertError(t, err) + assertContainsStr(t, err.Error(), "not found") +} diff --git a/mdl/executor/widget_property.go b/mdl/executor/widget_property.go deleted file mode 100644 index 75bdafc43..000000000 --- a/mdl/executor/widget_property.go +++ /dev/null @@ -1,125 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 - -// Package executor - Widget tree walking for styling and introspection commands. -package executor - -import ( - "github.com/mendixlabs/mxcli/sdk/pages" -) - -// walkPageWidgets walks all widgets in a page and calls the visitor function. -func walkPageWidgets(page *pages.Page, visitor func(widget any) error) error { - if page == nil || page.LayoutCall == nil { - return nil - } - - // Walk through layout call arguments (each argument has a single widget) - for _, arg := range page.LayoutCall.Arguments { - if arg.Widget != nil { - if err := walkWidget(arg.Widget, visitor); err != nil { - return err - } - } - } - - return nil -} - -// walkSnippetWidgets walks all widgets in a snippet and calls the visitor function. -func walkSnippetWidgets(snippet *pages.Snippet, visitor func(widget any) error) error { - if snippet == nil { - return nil - } - - for _, widget := range snippet.Widgets { - if err := walkWidget(widget, visitor); err != nil { - return err - } - } - - return nil -} - -// walkWidget recursively walks a widget and its children. -func walkWidget(widget pages.Widget, visitor func(widget any) error) error { - if widget == nil { - return nil - } - - // Visit this widget - if err := visitor(widget); err != nil { - return err - } - - // Recursively walk children based on widget type - switch w := widget.(type) { - case *pages.LayoutGrid: - for _, row := range w.Rows { - for _, col := range row.Columns { - for _, child := range col.Widgets { - if err := walkWidget(child, visitor); err != nil { - return err - } - } - } - } - case *pages.DataView: - for _, child := range w.Widgets { - if err := walkWidget(child, visitor); err != nil { - return err - } - } - for _, child := range w.FooterWidgets { - if err := walkWidget(child, visitor); err != nil { - return err - } - } - case *pages.ListView: - for _, child := range w.Widgets { - if err := walkWidget(child, visitor); err != nil { - return err - } - } - case *pages.Container: - for _, child := range w.Widgets { - if err := walkWidget(child, visitor); err != nil { - return err - } - } - case *pages.GroupBox: - for _, child := range w.Widgets { - if err := walkWidget(child, visitor); err != nil { - return err - } - } - case *pages.TabContainer: - for _, pg := range w.TabPages { - for _, child := range pg.Widgets { - if err := walkWidget(child, visitor); err != nil { - return err - } - } - } - case *pages.ScrollContainer: - for _, child := range w.Widgets { - if err := walkWidget(child, visitor); err != nil { - return err - } - } - case *pages.CustomWidget: - // Custom widgets may have nested widgets in their value properties - if w.WidgetObject != nil { - for _, prop := range w.WidgetObject.Properties { - if prop.Value != nil { - for _, child := range prop.Value.Widgets { - if err := walkWidget(child, visitor); err != nil { - return err - } - } - } - } - } - } - - return nil -} diff --git a/mdl/visitor/visitor_styling.go b/mdl/visitor/visitor_styling.go index 10910c6d0..d667e075b 100644 --- a/mdl/visitor/visitor_styling.go +++ b/mdl/visitor/visitor_styling.go @@ -54,19 +54,22 @@ func parseStylingAssignment(ctx *parser.AlterStylingAssignmentContext) ast.Styli assignment := ast.StylingAssignment{} if ctx.CLASS() != nil { - // CLASS = 'value' + // CLASS = 'value' (CSS appearance, not a design property) assignment.Property = "Class" + assignment.IsCSS = true if sl := ctx.STRING_LITERAL(0); sl != nil { assignment.Value = unquoteString(sl.GetText()) } } else if ctx.STYLE() != nil { - // STYLE = 'value' + // STYLE = 'value' (CSS appearance, not a design property) assignment.Property = "Style" + assignment.IsCSS = true if sl := ctx.STRING_LITERAL(0); sl != nil { assignment.Value = unquoteString(sl.GetText()) } } else { - // STRING_LITERAL = STRING_LITERAL | ON | OFF (design property) + // STRING_LITERAL = STRING_LITERAL | ON | OFF (design property; the key + // may itself be 'Class' or 'Style' — that is a design property, not CSS) literals := ctx.AllSTRING_LITERAL() if len(literals) > 0 { assignment.Property = unquoteString(literals[0].GetText())