Skip to content

Commit 06fb50a

Browse files
akoclaude
andcommitted
fix(pages): ALTER PAGE can set conditional Visible/Editable expressions (Issue 627)
ALTER PAGE had the same conditional-visibility gap as CREATE: `SET Visible = [Name != ''] ON ctn` parsed the `[...]` as a propertyValueV3 array, which the mutator's Visible handler (string/bool only) silently ignored — reporting "Altered page" while doing nothing. - Grammar: targeted `VISIBLE/EDITABLE EQUALS xpathConstraint` alternatives on alterPageAssignment (before the generic rule), so a bracketed RHS on Visible/ Editable is captured as an expression rather than an array. `Visible = false` and other properties are unaffected. - Visitor: buildAlterPageAssignment routes these to VisibleIf/EditableIf via the shared buildConditionalExpression ($currentObject rooting, idempotent). - Mutator: new VisibleIf/EditableIf cases in setRawWidgetPropertyMut build the ConditionalVisibility/EditabilitySettings node (setWidgetConditionalSettingMut) with Studio Pro's sub-field set, replacing the null slot. Editability on a non-input widget is rejected with a clear error. Shared mutator → both engines. Verified on 11.11.0: `mx check` = 0 errors on both engines for `SET Visible = [Name != '']` and `SET Editable = [Active]`; expressions written as `$currentObject/…`; DESCRIBE round-trips; misuse (Editable on a container) errors. Tests: visitor (Visible/Editable rooting), mutator (node built / non-input rejected). Bug-test extended with an ALTER section. Docs: alter-page skill, fix-issue symptom-table row. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 35b9378 commit 06fb50a

8 files changed

Lines changed: 145 additions & 3 deletions

File tree

.claude/skills/fix-issue.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ to the symptom table below, so the next similar issue costs fewer reads.
4848
| CE0463 on a `datagrid` whose column uses `ColumnWidth: manual` + `Size: N` — Studio Pro resets the column `size` to `1` | The MDL `ColumnWidth:` keyword isn't mapped to the schema `width` enum, so `width` stays at its `autoFill` default; `size` only applies when `width=manual`, so the value is inconsistent. Regression from the Stream B keyword-path consolidation (the deleted `datagrid_builder.go` did `colPropString(col.Properties, "ColumnWidth", "autoFill")`) | `mdl/executor/widget_defs.go` `itemPropertyAliases` | Add the MDL→schema alias under `[datagrid]["columns"]`: `"width": {"ColumnWidth"}`. Bump `WidgetDefGeneratorVersion` so stale `.def.json` regenerate. General rule when a column/object-list property's MDL keyword differs from the `.mpk` schema key (not just case), add it to `itemPropertyAliases`; cross-check against the pre-B3 `datagrid_builder.go` `colProp*` calls for any other dropped mappings |
4949
| Nightly `mx check` reports `CE0117 "Error(s) in expression." at Log message activity 'Log message (warning)'` on Mendix 10.24.19+ but not 10.24.16 or 11.x | Mendix 10.24.19 tightened expression validation: `toString(<string>)` is now a type error (toString expects a non-string input). An example called `toString($OrderNumber)` where `$OrderNumber` was already a string parameter | The offending `log warning ... with ({1} = toString($stringVar))` — find via `~/.mxcli/mxbuild/{ver}/modeler/mx check`, then bisect with `drop microflow ...` until CE0117 disappears | Remove the redundant `toString()` wrapper around already-string values. Only wrap non-string values (integers, decimals, dates, enums) in `toString()`. The Mendix 11.x parser is more lenient and lets this slide, but 10.24.19+ rejects it |
5050
| A page-level property can't be set — `ALTER PAGE X { SET PopupWidth = 800; }` (or any page-level prop other than Title/Url) fails with `unsupported page-level property: …` | The page-level SET handler only special-cased a couple of properties; everything else fell through to the default error | `mdl/backend/pagemutator/mutator.go` → `applyPageLevelSetMut` (shared by both engines) | Add a `case` writing the field at the top level of the Forms$Page doc via `dSetOrAppend` with the on-disk BSON type (int64 for PopupWidth/PopupHeight, bool for PopupResizable — verify against a Studio Pro page with `mxcli bson dump --format bson`). Page-level prop names are **case-sensitive**. For DESCRIBE roundtrip, emit the values back in the CREATE PAGE header. CREATE-time support: add a generic `IDENTIFIER COLON propertyValueV3` to `pageHeaderPropertyV3` (regen grammar), recognize the keys in `parsePageHeaderV3` (`applyGenericPageHeaderProp`, error on unknown), carry `*int`/`*bool` on `CreatePageStmtV3`, default to 600/600/false in `buildPageV3`, and have both writers honour `page.Popup*` (legacy `sdk/mpr/writer_pages.go` int64; codec `mdl/backend/modelsdk/page_write.go` int32 via gen — tolerated by mx check). The MCP backend has its **own** `mcpPageMutator` (pg content tree, not raw BSON) — page-level SET there reaches `SetWidgetProperty("")`; map it or reject honestly (it rejects, pending a `pg_read_page` probe of the pop-up keys). Issue #661 |
51-
| `Visible: [Attr != '']` / `Editable: […]` on a widget (CONTAINER, textbox, …) → CE0117 "Error(s) in expression." in Studio Pro on the legacy engine, and silently *ignored* on the modelsdk engine | Two bugs (NOT BSON structure): (1) the expression was emitted with a bare attribute reference (`Name != ''`) — a Mendix client expression must root attributes in the widget data context (`$currentObject/Name != ''`); (2) the modelsdk codec hardcoded `ConditionalVisibilitySettings` to null, dropping the expression entirely | `mdl/visitor/visitor_page_v3.go` → `buildConditionalExpression`/`conditionalExprToString`; `mdl/backend/modelsdk/widget_write.go` → `applyWidgetBase` + TypeDefaults | (1) For `VISIBLE`/`EDITABLE` xpath constraints, build the xpath AST and serialize via `conditionalExprToString`, which prefixes bare `IdentifierExpr` / non-variable-rooted `XPathPathExpr` with `$currentObject/` (leaves `$…`-rooted paths, literals, functions, enum qualified-names untouched). Do **not** apply this to datasource `where` clauses (those are real XPath). (2) In `applyWidgetBase`, type-assert `SetConditionalVisibilitySettings`/`SetConditionalEditabilitySettings` and emit a settings element when the model has one; register `Forms$ConditionalVisibilitySettings`/`…EditabilitySettings` TypeDefaults (`NullFields: Attribute,SourceVariable`; `MandatoryLists: Conditions[,ModuleRoles]`) so the null-when-unset slots stay null (the encoder only fills NullFields when not already emitted). Verify both engines with `mx check` = 0 errors. Issue #627 |
51+
| `Visible: [Attr != '']` / `Editable: […]` on a widget (CONTAINER, textbox, …) → CE0117 "Error(s) in expression." in Studio Pro on the legacy engine, and silently *ignored* on the modelsdk engine | Two bugs (NOT BSON structure): (1) the expression was emitted with a bare attribute reference (`Name != ''`) — a Mendix client expression must root attributes in the widget data context (`$currentObject/Name != ''`); (2) the modelsdk codec hardcoded `ConditionalVisibilitySettings` to null, dropping the expression entirely | `mdl/visitor/visitor_page_v3.go` → `buildConditionalExpression`/`conditionalExprToString`; `mdl/backend/modelsdk/widget_write.go` → `applyWidgetBase` + TypeDefaults | (1) For `VISIBLE`/`EDITABLE` xpath constraints, build the xpath AST and serialize via `conditionalExprToString`, which prefixes bare `IdentifierExpr` / non-variable-rooted `XPathPathExpr` with `$currentObject/` (leaves `$…`-rooted paths, literals, functions, enum qualified-names untouched). Do **not** apply this to datasource `where` clauses (those are real XPath). (2) In `applyWidgetBase`, type-assert `SetConditionalVisibilitySettings`/`SetConditionalEditabilitySettings` and emit a settings element when the model has one; register `Forms$ConditionalVisibilitySettings`/`…EditabilitySettings` TypeDefaults (`NullFields: Attribute,SourceVariable`; `MandatoryLists: Conditions[,ModuleRoles]`) so the null-when-unset slots stay null (the encoder only fills NullFields when not already emitted). **ALTER PAGE** had the same gap (`SET Visible = [expr]` parsed as a `propertyValueV3` array and silently no-op'd): add `VISIBLE/EDITABLE EQUALS xpathConstraint` alternatives to `alterPageAssignment` (regen grammar), route to `VisibleIf`/`EditableIf` in `buildAlterPageAssignment` (reusing `buildConditionalExpression`), and add `VisibleIf`/`EditableIf` cases to `setRawWidgetPropertyMut` (`mdl/backend/pagemutator/mutator.go`) that build the settings node via `setWidgetConditionalSettingMut` (rejecting editability on non-input widgets). The shared mutator covers both engines. Verify both engines with `mx check` = 0 errors. Issue #627 |
5252
| Mendix can't resolve the microflow named in `CREATE ODATA CLIENT (ConfigurationMicroflow: microflow X.Y)` / `ErrorHandlingMicroflow:` — error names the literal string `"MICROFLOW X.Y"` as the missing microflow | Case-mismatched prefix strip: visitor emits uppercase `"MICROFLOW "` from `odataValueText`, but `extractMicroflowRef` only trimmed lowercase `"microflow "`, so the keyword survived into BSON | `mdl/executor/cmd_odata.go``extractMicroflowRef` | Use a case-insensitive strip: `if strings.EqualFold(ref[:10], "microflow ") { return ref[10:] }`. Whenever a value goes from a visitor that emits a keyword-prefixed form to an executor that strips it, the strip must match the case the visitor produces — grep visitor files for `"MICROFLOW " +`/`"ENTITY " +`/etc. when adding a new property. Issue #573 |
5353
| `describe`/catalog reports a numeric BSON field (Length, MinOccurs, MaxOccurs, MaxLength, FractionDigits, TotalDigits, Interval, NumberOfPagesToClose, …) as `0` / `unlimited` even though Studio Pro shows a real value | BSON numeric width mismatch — Studio Pro writes the field as `int64`, but the parser asserted `raw["X"].(int32)` so the type assertion failed silently and the field defaulted to its zero value | `sdk/mpr/parser_*.go` — grep for the field name; the fix point is the narrow assertion | Replace narrow type assertions on BSON numeric fields with the existing `extractInt(raw["X"])` helper (`sdk/mpr/parser.go`). It handles int32/int64/int/float64. When a non-zero default must survive a missing field, gate with `if _, ok := raw["X"]; ok { … = extractInt(...) }`. Sweep `grep -n '\.(int32)' sdk/mpr/parser_*.go` and ignore matches whose comment says "marker" (BSON-array-prefix probes are intentional). Issues #583, #585 |
5454
| 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 |

.claude/skills/mendix/alter-page.md

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,17 @@ set PopupResizable = true
9292
| `PopupWidth` | Page-level only (case-sensitive) | Positive integer (pixels) | `set PopupWidth = 800` |
9393
| `PopupHeight` | Page-level only (case-sensitive) | Positive integer (pixels) | `set PopupHeight = 480` |
9494
| `PopupResizable` | Page-level only (case-sensitive) | Boolean | `set PopupResizable = true` |
95-
| `visible` | Any widget | Boolean or `[xpath]` | `set visible = false on txtHidden` |
96-
| `editable` | Input widgets | Never/Always or `[xpath]` | `set editable = Never on txtReadOnly` |
95+
| `Visible` (conditional) | Any widget | `[expression]` | `set Visible = [Name != ''] on ctnDetails` |
96+
| `Editable` (conditional) | Input widgets | `[expression]` | `set Editable = [Active] on txtName` |
9797
| `'quotedProp'` | Pluggable widgets | String, Boolean, Number | `set 'showLabel' = false on cbStatus` |
9898

99+
> **Conditional visibility/editability**`set Visible = [expr] on widget` (and
100+
> `Editable`) attach a per-object expression. Bare attributes are rooted in the
101+
> widget data context automatically: `[Name != '']` becomes
102+
> `$currentObject/Name != ''` (paths you write with `$currentObject/…`/`$Param/…`
103+
> pass through). Setting `Editable` on a non-input widget is rejected. This mirrors
104+
> CREATE PAGE's `visible: [...]` — see the create-page skill for enum-value rules.
105+
99106
**Pluggable widget properties** use quoted names to set values in the widget's `Object.Properties[]`. Boolean values are stored as `"yes"`/`"no"` in BSON.
100107

101108
**Column property names are case-insensitive** in MDL — `set caption = …` and `set Caption = …` both work. The internal BSON keys are dictated by the widget schema and stay case-sensitive on the storage side.

mdl-examples/bug-tests/627-container-visible-expression.mdl

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,16 @@ create page MyFirstModule.P_Vis
5050
}
5151
-- conditional editability uses the same rewrite
5252
textbox tbName (label: 'Name', attribute: Name, Editable: [Active])
53+
-- a plain container we'll make conditionally visible via ALTER below
54+
container ctnLater { dynamictext tL (content: 'shown via ALTER') }
5355
}
5456
}
5557

58+
-- ALTER PAGE sets conditional visibility/editability after the fact, with the
59+
-- same $currentObject rooting (issue #627 — previously a silent no-op).
60+
alter page MyFirstModule.P_Vis {
61+
set Visible = [Active] on ctnLater;
62+
set Editable = [Name != ''] on tbName;
63+
}
64+
5665
describe page MyFirstModule.P_Vis;

mdl/backend/pagemutator/mutator.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1674,6 +1674,29 @@ func updateTextsTextValue(textsTextDoc bson.D, text string) bool {
16741674
return true
16751675
}
16761676

1677+
// setWidgetConditionalSettingMut replaces a widget's ConditionalVisibility/
1678+
// EditabilitySettings slot (null when unset) with a node carrying the expression,
1679+
// mirroring the legacy/Studio Pro structure (null Attribute/SourceVariable, empty
1680+
// marker-3 Conditions, plus IgnoreSecurity/ModuleRoles for visibility). Returns
1681+
// false when the widget has no such slot (e.g. editability on a non-input widget).
1682+
func setWidgetConditionalSettingMut(widget bson.D, field, typeName, expression string, withSecurity bool) bool {
1683+
doc := bson.D{
1684+
{Key: "$ID", Value: bsonutil.NewIDBsonBinary()},
1685+
{Key: "$Type", Value: typeName},
1686+
{Key: "Attribute", Value: nil},
1687+
{Key: "Conditions", Value: bson.A{int32(3)}},
1688+
{Key: "Expression", Value: expression},
1689+
}
1690+
if withSecurity {
1691+
doc = append(doc,
1692+
bson.E{Key: "IgnoreSecurity", Value: false},
1693+
bson.E{Key: "ModuleRoles", Value: bson.A{int32(3)}},
1694+
)
1695+
}
1696+
doc = append(doc, bson.E{Key: "SourceVariable", Value: nil})
1697+
return bsonnav.DSet(widget, field, doc)
1698+
}
1699+
16771700
func setRawWidgetPropertyMut(widget bson.D, propName string, value any) error {
16781701
switch propName {
16791702
case "Caption":
@@ -1717,6 +1740,23 @@ func setRawWidgetPropertyMut(widget bson.D, propName string, value any) error {
17171740
}
17181741
}
17191742
return nil
1743+
case "VisibleIf":
1744+
// Conditional visibility expression (issue #627): replace the widget's
1745+
// ConditionalVisibilitySettings node (null when unset) with one carrying
1746+
// the rooted expression the visitor produced.
1747+
expr, _ := value.(string)
1748+
if !setWidgetConditionalSettingMut(widget, "ConditionalVisibilitySettings",
1749+
"Forms$ConditionalVisibilitySettings", expr, true) {
1750+
return fmt.Errorf("widget does not support conditional visibility")
1751+
}
1752+
return nil
1753+
case "EditableIf":
1754+
expr, _ := value.(string)
1755+
if !setWidgetConditionalSettingMut(widget, "ConditionalEditabilitySettings",
1756+
"Forms$ConditionalEditabilitySettings", expr, false) {
1757+
return fmt.Errorf("widget does not support conditional editability (only input widgets are editable)")
1758+
}
1759+
return nil
17201760
case "Name":
17211761
if s, ok := value.(string); ok {
17221762
bsonnav.DSet(widget, "Name", s)

mdl/backend/pagemutator/mutator_test.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1063,3 +1063,45 @@ func TestSetPageLevel_UnsupportedProperty(t *testing.T) {
10631063
t.Fatal("expected error for unsupported page-level property, got nil")
10641064
}
10651065
}
1066+
1067+
// Issue #627 — SET VisibleIf builds a ConditionalVisibilitySettings node carrying
1068+
// the expression, replacing the null slot. Editable on a widget without the slot
1069+
// is rejected.
1070+
func TestSetWidgetProperty_VisibleIf(t *testing.T) {
1071+
w := bson.D{
1072+
{Key: "$Type", Value: "Forms$DivContainer"},
1073+
{Key: "Name", Value: "ctn"},
1074+
{Key: "ConditionalVisibilitySettings", Value: nil},
1075+
}
1076+
rawData := makeRawPage(w)
1077+
m := &Mutator{rawData: rawData, widgetFinder: findBsonWidget}
1078+
1079+
if err := m.SetWidgetProperty("ctn", "VisibleIf", "$currentObject/Name != ''"); err != nil {
1080+
t.Fatalf("SetWidgetProperty VisibleIf: %v", err)
1081+
}
1082+
res := findBsonWidget(rawData, "ctn")
1083+
cvs, ok := bsonnav.DGet(res.widget, "ConditionalVisibilitySettings").(bson.D)
1084+
if !ok {
1085+
t.Fatalf("ConditionalVisibilitySettings not a doc: %T", bsonnav.DGet(res.widget, "ConditionalVisibilitySettings"))
1086+
}
1087+
if got := bsonnav.DGetString(cvs, "Expression"); got != "$currentObject/Name != ''" {
1088+
t.Errorf("Expression = %q", got)
1089+
}
1090+
if bsonnav.DGetString(cvs, "$Type") != "Forms$ConditionalVisibilitySettings" {
1091+
t.Errorf("wrong $Type: %v", bsonnav.DGet(cvs, "$Type"))
1092+
}
1093+
}
1094+
1095+
func TestSetWidgetProperty_EditableIf_Unsupported(t *testing.T) {
1096+
// A container has no ConditionalEditabilitySettings slot → reject.
1097+
w := bson.D{
1098+
{Key: "$Type", Value: "Forms$DivContainer"},
1099+
{Key: "Name", Value: "ctn"},
1100+
{Key: "ConditionalVisibilitySettings", Value: nil},
1101+
}
1102+
rawData := makeRawPage(w)
1103+
m := &Mutator{rawData: rawData, widgetFinder: findBsonWidget}
1104+
if err := m.SetWidgetProperty("ctn", "EditableIf", "$currentObject/Active"); err == nil {
1105+
t.Fatal("expected error setting EditableIf on a container, got nil")
1106+
}
1107+
}

mdl/grammar/MDLParser.g4

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,8 @@ alterLayoutMapping
245245

246246
alterPageAssignment
247247
: DATASOURCE EQUALS dataSourceExprV3 // DataSource = SELECTION widgetName
248+
| VISIBLE EQUALS xpathConstraint // Visible = [Name != ''] (conditional visibility)
249+
| EDITABLE EQUALS xpathConstraint // Editable = [Status = 'Open'] (conditional editability)
248250
| identifierOrKeyword EQUALS propertyValueV3 // Caption = 'Save'
249251
| STRING_LITERAL EQUALS propertyValueV3 // 'showLabel' = false
250252
;

mdl/visitor/visitor_alter_page.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,19 @@ func (b *Builder) buildAlterPageAssignment(ctx *parser.AlterPageAssignmentContex
111111
return "DataSource", buildDataSourceV3(dsCtx)
112112
}
113113

114+
// Visible = [expr] / Editable = [expr] — conditional visibility/editability.
115+
// Same context-rooting as CREATE PAGE (issue #627): bare attributes become
116+
// $currentObject/Attr. Routed to VisibleIf/EditableIf so the mutator builds a
117+
// ConditionalVisibilitySettings node instead of a static Visible string.
118+
if xc := ctx.XpathConstraint(); xc != nil {
119+
if ctx.VISIBLE() != nil {
120+
return "VisibleIf", buildConditionalExpression(xc)
121+
}
122+
if ctx.EDITABLE() != nil {
123+
return "EditableIf", buildConditionalExpression(xc)
124+
}
125+
}
126+
114127
var name string
115128

116129
if id := ctx.IdentifierOrKeyword(); id != nil {

mdl/visitor/visitor_alter_page_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,3 +507,32 @@ func TestAlterPageContainerType(t *testing.T) {
507507
t.Errorf("Expected ContainerType 'PAGE', got %q", stmt.ContainerType)
508508
}
509509
}
510+
511+
// Issue #627 — ALTER PAGE SET Visible/Editable = [expr] roots bare attributes in
512+
// $currentObject and routes to VisibleIf/EditableIf (so the mutator builds a
513+
// ConditionalVisibilitySettings node, not a static field).
514+
func TestAlterPageSetConditionalVisibility(t *testing.T) {
515+
input := `ALTER PAGE Module.Page {
516+
SET Visible = [Name != ''] ON ctn;
517+
SET Editable = [Active] ON tb;
518+
};`
519+
prog, errs := Build(input)
520+
if len(errs) > 0 {
521+
t.Fatalf("parse errors: %v", errs)
522+
}
523+
stmt := prog.Statements[0].(*ast.AlterPageStmt)
524+
if len(stmt.Operations) != 2 {
525+
t.Fatalf("expected 2 operations, got %d", len(stmt.Operations))
526+
}
527+
vis := stmt.Operations[0].(*ast.SetPropertyOp)
528+
if vis.Target.Widget != "ctn" {
529+
t.Errorf("visible target = %q, want ctn", vis.Target.Widget)
530+
}
531+
if v, _ := vis.Properties["VisibleIf"].(string); v != "$currentObject/Name != ''" {
532+
t.Errorf("VisibleIf = %q, want $currentObject/Name != ''", v)
533+
}
534+
edit := stmt.Operations[1].(*ast.SetPropertyOp)
535+
if v, _ := edit.Properties["EditableIf"].(string); v != "$currentObject/Active" {
536+
t.Errorf("EditableIf = %q, want $currentObject/Active", v)
537+
}
538+
}

0 commit comments

Comments
 (0)