From 2bfb697ee5a9b8ea693a227b29a80f74ea0833bd Mon Sep 17 00:00:00 2001 From: stagknee <241821830+stagknee@users.noreply.github.com> Date: Sat, 17 Oct 2026 09:00:36 -0500 Subject: [PATCH 1/2] fix(executor): store a quoted Editable expression as a conditional editability `Editable: '$currentObject/Locked = false'` on an input widget passed check, exec said "Created page", and the widget was stored `Editable: Always` with no ConditionalEditabilitySettings, so a field meant to be locked stayed editable. `Visible: ''` was stored as a conditional visibility: its consumer (pages.StaticVisibleExpression) returns any other string as the condition, while Editable's (pages.CanonicalEditability) knew only Always|Never|Conditional and the builder skipped every other string without a word. New pages.StaticEditability is the Editable counterpart: an enum word, a boolean (false is Never, true is Always) or, for any other string, the condition. A list view's and a grid column's Editable stays their own plain bool. check now refuses what still cannot be stored (a misspelt word, a quoted text that is not an expression, `Conditional` with no condition) as MDL-WIDGET19 naming the accepted spellings, and a quoted condition gets the checks the bare one gets (MDL-WIDGET13, MDL-BUTTON02, MDL-WIDGET41, MDL-WIDGET20). describe prints the stored condition bare, so describe -> exec is stable. ALTER PAGE set (Editable: '') keeps its loud refusal. Control: same script on the pre-fix binary stores Always/null for the quoted widgets; with the fix they are byte-equal (ids stripped) to the bare spelling and mx check reports 0 errors on Mendix 11.12.5. Co-Authored-By: Claude Sonnet 5.5 --- ...17-quoted-editable-expression-dropped.json | 1 + CHANGELOG.md | 4 + cmd/mxcli/syntax/features_page.go | 3 + docs-site/src/language/alter-page.md | 2 +- docs-site/src/language/pages.md | 4 +- docs-site/src/language/widget-types.md | 4 +- docs-site/src/reference/page/create-page.md | 2 +- .../quoted-editable-expression.fail.mdl | 28 ++ .../bug-tests/quoted-editable-expression.mdl | 55 +++ mdl/executor/cmd_pages_builder_v3.go | 21 +- mdl/executor/pluggable_system_props.go | 4 +- mdl/executor/validate_page_button_context.go | 6 + mdl/executor/validate_widget_editability.go | 6 +- mdl/executor/validate_widgets.go | 28 +- .../widget_editable_quoted_expression_test.go | 360 ++++++++++++++++++ mdl/executor/widget_editable_static.go | 104 +++++ mdl/visitor/visitor_argument_binding.go | 11 + sdk/pages/pages_widgets.go | 43 +++ sdk/pages/pages_widgets_editable_test.go | 51 +++ 19 files changed, 718 insertions(+), 19 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-17-quoted-editable-expression-dropped.json create mode 100644 mdl-examples/bug-tests/quoted-editable-expression.fail.mdl create mode 100644 mdl-examples/bug-tests/quoted-editable-expression.mdl create mode 100644 mdl/executor/widget_editable_quoted_expression_test.go create mode 100644 mdl/executor/widget_editable_static.go create mode 100644 sdk/pages/pages_widgets_editable_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-17-quoted-editable-expression-dropped.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-17-quoted-editable-expression-dropped.json new file mode 100644 index 000000000..87add4d0c --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-17-quoted-editable-expression-dropped.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-10-17","symptom":"A quoted `Editable: '$currentObject/Locked = false'` on a checkbox or text box passes check and exec (\"Created page\") and is stored `Editable: Always` with no ConditionalEditabilitySettings, so a field meant to be locked stays editable; `describe page` prints the widget without any Editable. The quoted `Visible: ''` beside it is stored as a conditional visibility, and the unquoted `Editable: $currentObject/Locked = false` works. `Editable: false` on an input was dropped the same way","cause":"`applyConditionalSettings` (cmd_pages_builder_v3.go) consulted only `pages.CanonicalEditability`, which accepts exactly Always|Never|Conditional and answers (\"\", false) for any other string, so the `else if` did not fire and the default Always stayed. `Visible` goes through `pages.StaticVisibleExpression`, whose default branch hands any other non-empty string back as the expression. MDL-WIDGET19 let every bool/string through for both keys (\"a static form StaticVisibleExpression consumes\"), true for Visible and false for Editable; a bool never reached the builder at all because it read the value with GetStringProp, which is \"\" for a bool","file":"`sdk/pages/pages_widgets.go` (new `StaticEditability`), `mdl/executor/widget_editable_static.go` (new: `staticEditability`, `editableCondition`, `editableValueProblem`), `cmd_pages_builder_v3.go` (`applyConditionalSettings`), `pluggable_system_props.go` (`widgetAsksEditability`), `validate_widgets.go` (MDL-WIDGET19, MDL-WIDGET13), `validate_page_button_context.go` (MDL-BUTTON02), `validate_widget_editability.go` (MDL-WIDGET20)","insight":"Two sibling properties with the same grammar slot but different consumers: one takes any string as an expression, the other only an enum word and skips the rest silently. A skipped write must be a refusal, so the fix is one rule for both keys (a string that is not the property's plain value is its condition) PLUS a check that names what still cannot be stored (a lone word such as `Neever`, text that is not an expression, `Conditional` with no condition). Every reader of `EditableIf` is a second consumer of the same value and has to see the quoted spelling too: MDL-WIDGET13 (association step, CE0117), MDL-BUTTON02 (own container name), MDL-WIDGET41 (pluggable Editability gate) and MDL-WIDGET20; fixing only the builder leaves them blind. Not every widget stores the enum: a list view and a grid column have a plain `Editable bool` (ako/mxcli#510), so a boolean there must NOT be read as Never/Always (`plainEditableWidgetTypes`). A quoted string reaches the validators unparsed, so a malformed quoted condition needs `visitor.ParsesAsExpression`; the bare spelling fails in the grammar. Quoted Visible still gets none of those checks (WIDGET13, BUTTON02, a syntax check) — left as is, same family. ALTER PAGE `set (Editable: '')` keeps its loud refusal (pagemutator setWidgetEditableMut); accepting it is about five lines (settings element plus the Conditional enum), not one. Control: same script on the pre-fix binary stores Always/null for the quoted widgets; the fix stores Conditional + expression, byte-equal (ids stripped) to the unquoted spelling, and describe -> exec reports `Unchanged page`","refs":[],"rules":["MDL-WIDGET19","MDL-WIDGET13","MDL-BUTTON02","MDL-WIDGET20","MDL-WIDGET41"]} diff --git a/CHANGELOG.md b/CHANGELOG.md index 0798c1db7..39d4cc5dc 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 quoted `Editable: ''` is stored as a conditional editability instead of being dropped** (issue number to be added) — `CHECKBOX cb (Attribute: Flag, Editable: '$currentObject/Locked = false')` passed `check`, `exec` said "Created page", and the widget was stored `Editable: Always` with no condition, so a field meant to be locked stayed editable (`mx check` also reported 0 errors). `Visible: ''` beside it was stored as a conditional visibility, and the unquoted `Editable: $currentObject/Locked = false` worked: the page builder read `Editable` only as one of `Always`/`Never`/`Conditional` and skipped every other string without a word. A string that is not one of those is now the condition, exactly as for `Visible`, and `describe` prints it back bare, so a describe → exec round trip is stable. `Editable: false` / `true` were dropped the same way and now store `Never` / `Always` (on a list view and a grid column they stay the plain boolean they always were). `check` refuses what still cannot be stored — a misspelt `Editable: Neever`, a quoted text that is not an expression, `Editable: Conditional` with no condition — as MDL-WIDGET19 naming the accepted spellings, and a quoted condition now gets the checks the bare one gets (MDL-WIDGET13 association steps, MDL-BUTTON02, MDL-WIDGET41 on a pluggable widget that declares no Editability, MDL-WIDGET20). `alter page … set (Editable: '')` still refuses a quoted condition with its existing message. + ## [0.27.0] - 2026-10-15 Headline: **`exec` is all or nothing: when a statement fails at write time, the whole run is rolled back, so a script never leaves a project half-changed.** Alongside it: a wave of `describe` → `check` → `exec` fixes from round-trip runs over real apps (the Siemens FSM template, the GenAI showcase, Marketplace modules), Mendix 9 projects can take microflows, nanoflows and pluggable widgets again, `check -p` resolves many more of the references `exec` and the build would refuse, the classic `filemanager` widget and role-based `Visible: roles (…)` are in MDL, and the companion draws a module's domain model. The MongoDB driver is upgraded for the October 2026 BSON advisories. diff --git a/cmd/mxcli/syntax/features_page.go b/cmd/mxcli/syntax/features_page.go index 2ad1840c1..4c353b80a 100644 --- a/cmd/mxcli/syntax/features_page.go +++ b/cmd/mxcli/syntax/features_page.go @@ -166,6 +166,9 @@ LIST IMPACT OF htmlelement; "-- written (name attributes as $currentObject/Attr). A plain value is static.\n" + "TEXTBOX name (Attribute: Attr, Visible: $currentObject/IsActive, Editable: $currentObject/Status != 'Closed')\n" + "TEXTBOX name (Attribute: Attr, Visible: false, Editable: Never)\n" + + "-- A condition may also be a quoted string holding the same expression; Editable:\n" + + "-- false / true mean Never / Always.\n" + + "TEXTBOX name (Attribute: Attr, Editable: '$currentObject/Status != ''Closed''')\n" + "-- The bracketed Visible: [IsActive] (attributes rooted for you) is the deprecated\n" + "-- spelling, MDL-DEPR081.\n\n" + "-- Actions\nACTIONBUTTON name (Caption: 'C', Action: SAVE CHANGES, ButtonStyle: Primary)\nLINKBUTTON name (Caption: 'C', Action: ...)\n\n" + diff --git a/docs-site/src/language/alter-page.md b/docs-site/src/language/alter-page.md index 73a818739..c8b532d60 100644 --- a/docs-site/src/language/alter-page.md +++ b/docs-site/src/language/alter-page.md @@ -50,7 +50,7 @@ ALTER PAGE Module.EditPage { | `Class` | CSS class names | `SET (Class: 'card p-3') ON cMain` | | `Style` | Inline CSS | `SET (Style: 'margin: 8px;') ON cBox` | | `DynamicClasses` | Runtime-computed CSS classes | `SET (DynamicClasses: if $currentObject/IsActive then 'is-active' else '') ON cMain` | -| `Editable` | Editability mode | `SET (Editable: ReadOnly) ON txtEmail` | +| `Editable` | Editability: `Always`, `Never`, or a bare condition expression (the quoted form is refused in `SET`) | `SET (Editable: Never) ON txtEmail` | | `Visible` | Visibility expression | `SET (Visible: '$showField') ON txtPhone` | | `Name` | Widget name | `SET (Name: 'txtFullName') ON txtName` | diff --git a/docs-site/src/language/pages.md b/docs-site/src/language/pages.md index db83a9221..8faeac003 100644 --- a/docs-site/src/language/pages.md +++ b/docs-site/src/language/pages.md @@ -144,7 +144,9 @@ Input widgets can be conditionally editable: TEXTBOX txtStatus (Label: 'Status', Attribute: Status, Editable: $currentObject/Status != 'Closed') ``` -Static values: `Editable: Never`, `Editable: Always`. +Static values: `Editable: Never`, `Editable: Always`; `Editable: false` and `Editable: true` mean `Never` and `Always`. + +The condition may also be written as a quoted string, exactly as for `Visible:` — `Editable: '$currentObject/Status != ''Closed'''` stores the same condition as the bare form above (a quote inside the string is doubled). Any other value, such as a misspelt `Neever`, is refused by `mxcli check` (`MDL-WIDGET19`) instead of leaving the field editable. ### Pluggable Widgets diff --git a/docs-site/src/language/widget-types.md b/docs-site/src/language/widget-types.md index 0dd3e3f3e..146621426 100644 --- a/docs-site/src/language/widget-types.md +++ b/docs-site/src/language/widget-types.md @@ -259,7 +259,7 @@ All input widgets share common properties: |----------|-------------|---------| | `Label` | Field label text | `Label: 'Customer Name'` | | `Attribute` | Entity attribute to bind to | `Attribute: Name` | -| `Editable` | Editability mode | `Editable: ReadOnly` | +| `Editable` | Editability: `Always`, `Never`, `true`/`false`, or a condition expression (bare or quoted) | `Editable: Never` | | `Visible` | Visibility expression | `Visible: '$showField'` | Each input widget takes only some attribute types. Measured with mxbuild 11.14.0, @@ -641,7 +641,7 @@ These properties are shared across many widget types: | `DynamicClasses` | Runtime-computed CSS classes (expression; stacks on `Class`) | `DynamicClasses: if $currentObject/IsActive then 'is-active' else ''` | | `DesignProperties` | Atlas design properties | `DesignProperties: ('Spacing top': 'Large', 'Full width': ON)` | | `Visible` | Visibility expression | `Visible: '$showSection'` | -| `Editable` | Editability mode | `Editable: ReadOnly` | +| `Editable` | Editability: `Always`, `Never`, `true`/`false`, or a condition expression (bare or quoted) | `Editable: Never` | ## See Also diff --git a/docs-site/src/reference/page/create-page.md b/docs-site/src/reference/page/create-page.md index dd74a5ed2..3b1c0d20f 100644 --- a/docs-site/src/reference/page/create-page.md +++ b/docs-site/src/reference/page/create-page.md @@ -218,7 +218,7 @@ These properties are available on most widget types: | `Class` | CSS class names | `Class: 'card mx-spacing-top-large'` | | `Style` | Inline CSS | `Style: 'padding: 16px;'` | | `DynamicClasses` | Runtime-computed CSS classes (expression; stacks on `Class`) | `DynamicClasses: if $currentObject/IsActive then 'is-active' else ''` | -| `Editable` | Edit control | `Editable: NEVER` or `Editable: ALWAYS` | +| `Editable` | Edit control: `Always`, `Never`, or a condition expression (bare or quoted) | `Editable: NEVER` or `Editable: $currentObject/Locked = false` | | `Visible` | Visibility expression | `Visible: '$showField'` | | `DesignProperties` | Atlas design properties | `DesignProperties: ('Spacing top': 'Large')` | diff --git a/mdl-examples/bug-tests/quoted-editable-expression.fail.mdl b/mdl-examples/bug-tests/quoted-editable-expression.fail.mdl new file mode 100644 index 000000000..29dcfd515 --- /dev/null +++ b/mdl-examples/bug-tests/quoted-editable-expression.fail.mdl @@ -0,0 +1,28 @@ +mdl 1; +-- Negative companion of quoted-editable-expression.mdl: `check` must refuse every +-- Editable value that cannot be stored, naming the accepted spellings, instead of +-- letting the widget stay editable. Each statement below is refused on its own. +-- The issue number is to be added. + +create entity if not exists QEdit.Item (Code: String(50), Locked: Boolean default false); + +-- A misspelt enum word (MDL-WIDGET19). +create or modify page QEdit.Bad_Word (Title: 'Bad', Layout: Atlas_Core.Atlas_Default, Params: ( $Item: QEdit.Item )) { + layoutgrid lg { row { column (DesktopWidth: AutoFill) { + dataview dv (Datasource: $Item) { textbox txtA (Attribute: Code, Editable: 'Neever') } + } } } +}; + +-- A quoted expression that is not an expression (MDL-WIDGET19). +create or modify page QEdit.Bad_Syntax (Title: 'Bad', Layout: Atlas_Core.Atlas_Default, Params: ( $Item: QEdit.Item )) { + layoutgrid lg { row { column (DesktopWidth: AutoFill) { + dataview dv (Datasource: $Item) { textbox txtB (Attribute: Code, Editable: '$currentObject/Locked = = false') } + } } } +}; + +-- A quoted expression that follows an association (MDL-WIDGET13, CE0117). +create or modify page QEdit.Bad_Assoc (Title: 'Bad', Layout: Atlas_Core.Atlas_Default, Params: ( $Item: QEdit.Item )) { + layoutgrid lg { row { column (DesktopWidth: AutoFill) { + dataview dv (Datasource: $Item) { textbox txtC (Attribute: Code, Editable: '$currentObject/QEdit.Item_Item/Code = ''x''') } + } } } +}; diff --git a/mdl-examples/bug-tests/quoted-editable-expression.mdl b/mdl-examples/bug-tests/quoted-editable-expression.mdl new file mode 100644 index 000000000..2839fe019 --- /dev/null +++ b/mdl-examples/bug-tests/quoted-editable-expression.mdl @@ -0,0 +1,55 @@ +mdl 1; +-- A quoted `Editable: ''` was dropped on write (issue number: to be added). +-- +-- `Visible: '$currentObject/Locked = false'` was stored as a conditional +-- visibility, because pages.StaticVisibleExpression returns any string that is not +-- true/false as the condition. Editable shares the grammar slot but not the +-- consumer: pages.CanonicalEditability knew only Always | Never | Conditional and +-- the builder skipped every other string without a word. The widget was stored +-- `Editable: Always` with no ConditionalEditabilitySettings, so a field meant to +-- be locked stayed editable -- `check` clean, `exec` reporting "Created page", +-- `mx check` at 0 errors. +-- +-- Now the quoted spelling is the bare expression's twin: both store +-- `Editable: Conditional` plus the condition, and `describe page` prints either +-- as the bare form. `Editable: false` / `true` (also dropped before) store +-- Never / Always. +-- +-- Verify: exec this script, then `describe page QEdit.Item_Edit` -- the first four +-- inputs print their condition bare (`Editable: $currentObject/Locked = false`, +-- the `!= 'x'` form, ...), txtNever and txtFalse print `Editable: Never`, and the +-- stored page passes `mx check`. Values that cannot be stored are refused by +-- `check` instead: see quoted-editable-expression.fail.mdl. + +create entity if not exists QEdit.Item ( + Code: String(50), + Note: String(200), + Locked: Boolean default false +); + +create or modify page QEdit.Item_Edit ( + Title: 'Item', + Layout: Atlas_Core.Atlas_Default, + Params: ( $Item: QEdit.Item ) +) { + layoutgrid lg { + row { + column (DesktopWidth: AutoFill) { + dataview dv (Datasource: $Item) { + -- Quoted and bare spellings of the same condition store the same thing. + textbox txtQuoted (Attribute: Code, Editable: '$currentObject/Locked = false') + textbox txtBare (Attribute: Code, Editable: $currentObject/Locked = false) + -- A string literal inside the quoted expression has its quotes doubled. + textarea txtLiteral (Attribute: Note, Editable: '$currentObject/Code != ''x''') + checkbox cbLocked (Attribute: Locked, Editable: '$currentObject/Code = empty') + -- The enum words keep their meaning, quoted or not. + textbox txtNever (Attribute: Code, Editable: 'Never') + textbox txtFalse (Attribute: Code, Editable: false) + textbox txtAlways (Attribute: Code, Editable: 'Always') + } + } + } + } +}; + +describe page QEdit.Item_Edit; diff --git a/mdl/executor/cmd_pages_builder_v3.go b/mdl/executor/cmd_pages_builder_v3.go index 07a57466f..ec86e6e5d 100644 --- a/mdl/executor/cmd_pages_builder_v3.go +++ b/mdl/executor/cmd_pages_builder_v3.go @@ -589,22 +589,25 @@ func applyConditionalSettings(widget pages.Widget, w *ast.WidgetV3) { } if editableIf := w.GetStringProp("EditableIf"); editableIf != "" { - bw.ConditionalEditability = &pages.ConditionalEditabilitySettings{ - BaseElement: model.BaseElement{ - ID: model.ID(types.GenerateID()), - TypeName: "Forms$ConditionalEditabilitySettings", - }, - Expression: editableIf, - } - } else if editable, ok := pages.CanonicalEditability(w.GetStringProp("Editable")); ok { + bw.ConditionalEditability = newConditionalEditability(editableIf) + } else if enum, expr, ok := staticEditability(w); ok { // `Editable: Never` — parsed and validated (MDL-WIDGET20 checks the widget // TYPE) and then dropped, because nothing carried it to the writers, which // hardcoded "Always". Same shape as the `Visible: false` case above. // + // A string that is not an enum word is the condition, exactly as for + // Visible: `Editable: '$currentObject/Locked = false'` is the quoted + // spelling of the bare expression. It used to be skipped here, so the + // widget stayed Always. + // // EDITABLE IF wins when both are given: the conditional settings element is // what makes the enum "Conditional", so honouring a plain `Editable` too // would write an enum contradicting the element beside it. - bw.Editable = editable + if expr != "" { + bw.ConditionalEditability = newConditionalEditability(expr) + } else { + bw.Editable = enum + } } } diff --git a/mdl/executor/pluggable_system_props.go b/mdl/executor/pluggable_system_props.go index 8891a56f8..e8ca68bfe 100644 --- a/mdl/executor/pluggable_system_props.go +++ b/mdl/executor/pluggable_system_props.go @@ -64,8 +64,8 @@ func widgetAsksEditability(w *ast.WidgetV3) bool { if w.GetStringProp("EditableIf") != "" { return true } - e, ok := pages.CanonicalEditability(w.GetStringProp("Editable")) - return ok && e != "Always" + enum, expr, ok := staticEditability(w) + return ok && (expr != "" || enum != "Always") } // systemPropsOfType returns the system properties a CustomWidgetType declares, diff --git a/mdl/executor/validate_page_button_context.go b/mdl/executor/validate_page_button_context.go index 63482465d..2ec23c6bd 100644 --- a/mdl/executor/validate_page_button_context.go +++ b/mdl/executor/validate_page_button_context.go @@ -132,6 +132,12 @@ func checkOwnContainerName(w *ast.WidgetV3, nearest, locationPrefix string) []li flag(p.written+" expression", v, "CE0117") } } + // A quoted `Editable: ''` is the same condition as the bare one. + if _, routed := w.Properties["EditableIf"]; !routed { + if expr, ok := editableCondition(w); ok { + flag("Editable expression", expr, "CE0117") + } + } return out } diff --git a/mdl/executor/validate_widget_editability.go b/mdl/executor/validate_widget_editability.go index 4ed64f614..472e97d64 100644 --- a/mdl/executor/validate_widget_editability.go +++ b/mdl/executor/validate_widget_editability.go @@ -101,7 +101,11 @@ func validateWidgetEditability(w *ast.WidgetV3, locationPrefix string) []linter. // (lowered to EditableIf, and riding on ConditionalEditabilitySettings these // types do not have) is not. Only the second is worth reporting. if plainEditableWidgetTypes[typ] && !strings.EqualFold(key, "editableif") { - return nil + // A quoted condition is the bracket form's twin: it is the part of + // `Editable:` these types have no place for. + if _, expr, ok := staticEditability(w); !ok || expr == "" { + return nil + } } return []linter.Violation{{ RuleID: "MDL-WIDGET20", diff --git a/mdl/executor/validate_widgets.go b/mdl/executor/validate_widgets.go index f63dd3b0d..cc07920d2 100644 --- a/mdl/executor/validate_widgets.go +++ b/mdl/executor/validate_widgets.go @@ -1196,7 +1196,24 @@ func validateConsumableConditional(w *ast.WidgetV3, locationPrefix string) []lin } switch v.(type) { case bool, string: - continue // a static form StaticVisibleExpression consumes + // A static form StaticVisibleExpression / StaticEditability consumes. + // Visible takes any string as its condition; Editable takes an enum + // word, a boolean or a condition, and refuses what is none of them + // here, because the writer has nowhere to put it. + if p.plain == "Editable" { + if problem := editableValueProblem(w); problem != "" { + out = append(out, linter.Violation{ + RuleID: "MDL-WIDGET19", + Severity: linter.SeverityError, + Message: fmt.Sprintf( + "%s: %s: %s — Editable takes Always, Never (or true / false), or a condition "+ + "expression such as `Editable: $currentObject/Locked = false`, bare or in quotes; "+ + "anything else cannot be stored and would leave the widget editable", + locationPrefix, widgetLabel(w.Name, w.Type), problem), + }) + } + } + continue } out = append(out, linter.Violation{ RuleID: "MDL-WIDGET19", @@ -1312,6 +1329,13 @@ func validateWidgetExpressionAssociations(w *ast.WidgetV3, locationPrefix string var out []linter.Violation for _, key := range expressionWidgetProps { expr, ok := lookupWidgetProp(w, key) + shown := key + if key == "EditableIf" && (!ok || expr == "") { + // A quoted `Editable: ''` is the same condition. + if e, quoted := editableCondition(w); quoted { + expr, ok, shown = e, true, "Editable" + } + } if !ok || expr == "" { continue } @@ -1321,7 +1345,7 @@ func validateWidgetExpressionAssociations(w *ast.WidgetV3, locationPrefix string Severity: linter.SeverityError, Message: fmt.Sprintf( "%s: widget `%s` property `%s` expression traverses association `%s` — Mendix expressions cannot follow associations (CE0117). Bind the value via a data binding (contentparams/attribute) or precompute it onto the bound entity (e.g. a calculated attribute).", - locationPrefix, w.Name, key, m[1], + locationPrefix, w.Name, shown, m[1], ), }) } diff --git a/mdl/executor/widget_editable_quoted_expression_test.go b/mdl/executor/widget_editable_quoted_expression_test.go new file mode 100644 index 000000000..b643d8f6e --- /dev/null +++ b/mdl/executor/widget_editable_quoted_expression_test.go @@ -0,0 +1,360 @@ +// SPDX-License-Identifier: Apache-2.0 + +// A quoted `Editable: ''` on an input widget. +// +// `Visible: '$currentObject/Locked = false'` is stored as a conditional +// visibility: pages.StaticVisibleExpression hands any string that is not true or +// false back as the expression. `Editable` took the same grammar slot but a +// different consumer, pages.CanonicalEditability, which knows only the enum words +// Always | Never | Conditional and answers "not one of mine" for the rest. The +// builder's `else if` then simply did not fire, so the widget kept the default +// Always: check clean, exec "Created page", and a field the author meant to lock +// stayed editable. +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/visitor" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +const lockedExpr = "$currentObject/Locked = false" + +// buildInputFromMDL runs the real parser and page builder over one widget. +func buildInputFromMDL(t *testing.T, widget string) pages.Widget { + t.Helper() + src := "create page Mod.P (Title: 'T') {\n " + widget + "\n}" + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse %q: %v", widget, errs) + } + page := prog.Statements[0].(*ast.CreatePageStmtV3) + pb := &pageBuilder{widgetScope: map[string]model.ID{}} + w, err := pb.buildWidgetV3(page.Widgets[0]) + if err != nil { + t.Fatalf("building %q: %v", widget, err) + } + return w +} + +func baseWidgetOf(t *testing.T, w pages.Widget) *pages.BaseWidget { + t.Helper() + g, ok := w.(interface{ GetBaseWidget() *pages.BaseWidget }) + if !ok { + t.Fatalf("%T has no BaseWidget", w) + } + return g.GetBaseWidget() +} + +// storedEditability is what the writers store for a built widget: the enum and +// the condition beside it. +func storedEditability(t *testing.T, w pages.Widget) (enum, expr string) { + t.Helper() + bw := baseWidgetOf(t, w) + if bw.ConditionalEditability != nil { + expr = bw.ConditionalEditability.Expression + } + return pages.WidgetEditability(bw), expr +} + +func storedVisibility(t *testing.T, w pages.Widget) string { + t.Helper() + bw := baseWidgetOf(t, w) + if bw.ConditionalVisibility == nil { + return "" + } + return bw.ConditionalVisibility.Expression +} + +var editableInputs = []struct{ name, tmpl string }{ + {"checkbox", `CHECKBOX w (Label: 'A', Attribute: Mod.E.Flag, %s)`}, + {"textbox", `TEXTBOX w (Label: 'C', Attribute: Mod.E.Code, %s)`}, +} + +func inputWith(tmpl, prop string) string { return strings.Replace(tmpl, "%s", prop, 1) } + +// The quoted spelling stores exactly what the unquoted one stores. +func TestQuotedEditableExpression_EqualsUnquotedSpelling(t *testing.T) { + for _, in := range editableInputs { + t.Run(in.name, func(t *testing.T) { + quoted := buildInputFromMDL(t, inputWith(in.tmpl, `Editable: '`+lockedExpr+`'`)) + unquoted := buildInputFromMDL(t, inputWith(in.tmpl, `Editable: `+lockedExpr)) + + wantEnum, wantExpr := storedEditability(t, unquoted) + if wantEnum != "Conditional" || wantExpr != lockedExpr { + t.Fatalf("control: the unquoted spelling stored (%q, %q), want (Conditional, %q)", + wantEnum, wantExpr, lockedExpr) + } + gotEnum, gotExpr := storedEditability(t, quoted) + if gotEnum != wantEnum || gotExpr != wantExpr { + t.Errorf("quoted Editable stored (%q, %q), want what the unquoted spelling stores (%q, %q) — "+ + "the quoted expression was dropped and the field stays editable", + gotEnum, gotExpr, wantEnum, wantExpr) + } + }) + } +} + +// A string literal inside the expression is written with its quotes doubled in +// the quoted spelling and must come out as the unquoted spelling's text. +func TestQuotedEditableExpression_EscapedQuotes(t *testing.T) { + quoted := buildInputFromMDL(t, `TEXTBOX w (Attribute: Mod.E.Code, Editable: '$currentObject/Code != ''x''')`) + unquoted := buildInputFromMDL(t, `TEXTBOX w (Attribute: Mod.E.Code, Editable: $currentObject/Code != 'x')`) + _, want := storedEditability(t, unquoted) + enum, got := storedEditability(t, quoted) + if want != "$currentObject/Code != 'x'" { + t.Fatalf("control: unquoted stored %q", want) + } + if enum != "Conditional" || got != want { + t.Errorf("quoted stored (%q, %q), want (Conditional, %q)", enum, got, want) + } +} + +// describe prints a stored condition as `Editable: `; that has to rebuild +// the same condition, and so does the quoted spelling of it. +func TestQuotedEditableExpression_DescribeRoundTrip(t *testing.T) { + w := buildInputFromMDL(t, `CHECKBOX w (Attribute: Mod.E.Flag, Editable: '`+lockedExpr+`')`) + _, expr := storedEditability(t, w) + if expr == "" { + t.Fatal("the quoted expression was not stored, so there is nothing for describe to print") + } + printed := widgetConditionMDL("Editable", expr) + again := buildInputFromMDL(t, `CHECKBOX w (Attribute: Mod.E.Flag, `+printed+`)`) + enum, got := storedEditability(t, again) + if enum != "Conditional" || got != expr { + t.Errorf("describe line %q rebuilt as (%q, %q), want (Conditional, %q)", printed, enum, got, expr) + } +} + +// Controls: the enum words keep mapping to the enum, in any case, quoted or not. +func TestQuotedEditable_EnumWordsStillMapToTheEnum(t *testing.T) { + cases := []struct{ prop, want string }{ + {`Editable: 'Never'`, "Never"}, + {`Editable: 'never'`, "Never"}, + {`Editable: Never`, "Never"}, + {`Editable: 'Always'`, "Always"}, + {`Editable: 'ALWAYS'`, "Always"}, + {`Editable: Always`, "Always"}, + } + for _, c := range cases { + t.Run(c.prop, func(t *testing.T) { + enum, expr := storedEditability(t, buildInputFromMDL(t, `CHECKBOX w (Attribute: Mod.E.Flag, `+c.prop+`)`)) + if enum != c.want || expr != "" { + t.Errorf("%s stored (%q, %q), want (%q, no condition)", c.prop, enum, expr, c.want) + } + }) + } +} + +// A boolean said what it meant and was dropped too: GetStringProp answers "" +// for a bool, so `Editable: false` on a text box left it editable. false is the +// enum's Never, true its Always. +func TestEditableBoolean_IsNotDropped(t *testing.T) { + cases := []struct{ prop, want string }{ + {`Editable: false`, "Never"}, + {`Editable: 'false'`, "Never"}, + {`Editable: true`, "Always"}, + {`Editable: 'TRUE'`, "Always"}, + } + for _, in := range editableInputs { + for _, c := range cases { + t.Run(in.name+"/"+c.prop, func(t *testing.T) { + enum, expr := storedEditability(t, buildInputFromMDL(t, inputWith(in.tmpl, c.prop))) + if enum != c.want || expr != "" { + t.Errorf("%s stored (%q, %q), want (%q, no condition)", c.prop, enum, expr, c.want) + } + }) + } + } +} + +// Control: nothing authored, nothing stored. +func TestQuotedEditable_NothingAuthoredStoresNothing(t *testing.T) { + enum, expr := storedEditability(t, buildInputFromMDL(t, `CHECKBOX w (Attribute: Mod.E.Flag)`)) + if enum != "Always" || expr != "" { + t.Errorf("no Editable stored (%q, %q), want (Always, no condition)", enum, expr) + } +} + +// Visible is unchanged: its quoted expression is a conditional visibility, false +// is the constant condition "false", true stores nothing. +func TestQuotedVisible_Unchanged(t *testing.T) { + cases := []struct{ prop, want string }{ + {`Visible: '` + lockedExpr + `'`, lockedExpr}, + {`Visible: ` + lockedExpr, lockedExpr}, + {`Visible: false`, "false"}, + {`Visible: 'false'`, "false"}, + {`Visible: true`, ""}, + } + for _, c := range cases { + t.Run(c.prop, func(t *testing.T) { + w := buildInputFromMDL(t, `CHECKBOX w (Attribute: Mod.E.Flag, `+c.prop+`)`) + if got := storedVisibility(t, w); got != c.want { + t.Errorf("%s stored visibility %q, want %q", c.prop, got, c.want) + } + if enum, expr := storedEditability(t, w); enum != "Always" || expr != "" { + t.Errorf("%s changed editability to (%q, %q)", c.prop, enum, expr) + } + }) + } +} + +// A list view's and a grid column's Editable is a plain bool built from +// GetBoolProp (ako/mxcli#510) — a different property from the input widgets' +// editability enum. A bool there must not also be read as Never/Always on the +// widget's BaseWidget. +func TestEditableBoolean_PlainBoolTypesKeepTheirOwnProperty(t *testing.T) { + for _, typ := range []string{"listview", "column"} { + for _, v := range []any{true, false, "false"} { + w := &ast.WidgetV3{Type: typ, Name: "w", Properties: map[string]any{"Editable": v}} + lv := &pages.ListView{} + applyConditionalSettings(lv, w) + if got := lv.BaseWidget.Editable; got != "" { + t.Errorf("%s with Editable: %v set the editability enum to %q", typ, v, got) + } + if lv.ConditionalEditability != nil { + t.Errorf("%s with Editable: %v stored a condition %q", typ, v, lv.ConditionalEditability.Expression) + } + } + } +} + +// ---- check ---------------------------------------------------------------- + +func checkMessages(t *testing.T, widget, rule string) []string { + t.Helper() + src := "mdl 1;\ncreate page Mod.P (Title: 'T', Layout: Atlas_Core.Atlas_Default, Params: ($E: Mod.E)) {\n" + + " dataview dvE (DataSource: $E) {\n " + widget + "\n }\n};" + prog := parseMDL(t, src) + var out []string + for _, v := range ValidateWidgetProperties(prog, "") { + if v.RuleID == rule { + out = append(out, v.Message+" | "+v.Suggestion) + } + } + for _, v := range ValidatePageButtonContext(prog) { + if v.RuleID == rule { + out = append(out, v.Message+" | "+v.Suggestion) + } + } + return out +} + +// check refuses what exec cannot store, naming the accepted spellings, and +// stays quiet on what it can. +func TestMDLWIDGET19_EditableValues(t *testing.T) { + cases := []struct { + name string + prop string + refuse bool + }{ + {"quoted expression", `Editable: '` + lockedExpr + `'`, false}, + {"quoted expression with a literal", `Editable: '$currentObject/Code != ''x'''`, false}, + {"unquoted expression", `Editable: ` + lockedExpr, false}, + {"Never", `Editable: Never`, false}, + {"quoted Always", `Editable: 'Always'`, false}, + {"false", `Editable: false`, false}, + // Strings that are neither an enum word nor an expression the grammar reads. + {"misspelt enum word", `Editable: Neever`, true}, + {"misspelt enum word, quoted", `Editable: 'ReadOnly'`, true}, + {"malformed quoted expression", `Editable: '$currentObject/Locked = = false'`, true}, + {"Conditional without a condition", `Editable: Conditional`, true}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := checkMessages(t, `CHECKBOX w (Attribute: Mod.E.Flag, `+c.prop+`)`, "MDL-WIDGET19") + if !c.refuse { + if len(got) != 0 { + t.Fatalf("MDL-WIDGET19 refused a value exec stores: %v", got) + } + return + } + if len(got) != 1 { + t.Fatalf("want one MDL-WIDGET19, got %v", got) + } + for _, want := range []string{"Always", "Never"} { + if !strings.Contains(got[0], want) { + t.Errorf("message should name the accepted spellings (%s missing): %s", want, got[0]) + } + } + }) + } +} + +// A quoted Editable expression gets the expression checks the unquoted one gets. +func TestQuotedEditableExpression_GetsTheExpressionChecks(t *testing.T) { + const assoc = `$currentObject/Mod.E_E/Code = 'x'` + unquoted := checkMessages(t, `CHECKBOX w (Attribute: Mod.E.Flag, Editable: `+assoc+`)`, "MDL-WIDGET13") + if len(unquoted) != 1 { + t.Fatalf("control: the unquoted spelling should earn one MDL-WIDGET13, got %v", unquoted) + } + quoted := checkMessages(t, `CHECKBOX w (Attribute: Mod.E.Flag, Editable: '$currentObject/Mod.E_E/Code = ''x''')`, "MDL-WIDGET13") + if len(quoted) != 1 { + t.Errorf("the quoted spelling of the same expression earned %d MDL-WIDGET13, want 1: %v", len(quoted), quoted) + } + + // The widget's own data container is not a variable in its own context. + unquoted = checkMessages(t, `CHECKBOX w (Attribute: Mod.E.Flag, Editable: $dvE/Locked = false)`, "MDL-BUTTON02") + if len(unquoted) != 1 { + t.Fatalf("control: the unquoted spelling should earn one MDL-BUTTON02, got %v", unquoted) + } + quoted = checkMessages(t, `CHECKBOX w (Attribute: Mod.E.Flag, Editable: '$dvE/Locked = false')`, "MDL-BUTTON02") + if len(quoted) != 1 { + t.Errorf("the quoted spelling of the same expression earned %d MDL-BUTTON02, want 1: %v", len(quoted), quoted) + } +} + +// A button has no editability, quoted or not (MDL-WIDGET20, #928). +func TestMDLWIDGET20_QuotedExpressionOnAButton(t *testing.T) { + got := widgetViolations(t, buttonPage(`editable: '`+lockedExpr+`'`), "MDL-WIDGET20") + if len(got) != 1 { + t.Fatalf("got %d MDL-WIDGET20 violations for a quoted expression on a button, want 1", len(got)) + } +} + +// On a list view and a grid column `Editable: true` is the plain property, and +// a quoted expression is the part that is genuinely dropped. +func TestMDLWIDGET20_QuotedExpressionOnPlainEditableTypes(t *testing.T) { + src := `create page W.P ( Title: 'P' ) +{ + LISTVIEW lv (DataSource: DATABASE W.Product, Editable: '` + lockedExpr + `') { + DYNAMICTEXT dt ( Content: 'x' ) + } +}` + if got := widgetViolations(t, src, "MDL-WIDGET20"); len(got) != 1 { + t.Fatalf("got %d MDL-WIDGET20 violations for a quoted expression on a list view, want 1", len(got)) + } +} + +// A pluggable widget that does not declare Editability has no place to store a +// condition; the quoted spelling is refused like the unquoted one. +func TestApplyPluggableSystemSettings_QuotedExpression(t *testing.T) { + cases := []struct { + name string + declared []string + wantErr bool + }{ + {"declared", []string{"Editability"}, false}, + {"undeclared", []string{"Visibility"}, true}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + cw := &pages.CustomWidget{RawType: typeDeclaring(c.declared...), Editable: "Always"} + w := &ast.WidgetV3{Type: "switch", Name: "w", Properties: map[string]any{"Editable": lockedExpr}} + err := applyPluggableSystemSettings(cw, w) + if c.wantErr != (err != nil) { + t.Fatalf("err = %v, want error: %v", err, c.wantErr) + } + if !c.wantErr { + if cw.Editable != "Conditional" || cw.ConditionalEditability == nil || + cw.ConditionalEditability.Expression != lockedExpr { + t.Errorf("stored (%q, %v), want (Conditional, %q)", cw.Editable, cw.ConditionalEditability, lockedExpr) + } + } + }) + } +} diff --git a/mdl/executor/widget_editable_static.go b/mdl/executor/widget_editable_static.go new file mode 100644 index 000000000..472800091 --- /dev/null +++ b/mdl/executor/widget_editable_static.go @@ -0,0 +1,104 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "regexp" + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/mdl/visitor" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +// The static (non-bracket, non-bare-expression) reading of `Editable:`. +// +// `Visible` and `Editable` share a grammar slot: a string literal lands in the +// widget's plain `Visible` / `Editable` property, and what it means is the +// consumer's business. Visible's consumer (pages.StaticVisibleExpression) reads +// any string that is not true or false as the condition. Editable's used to read +// only the three enum words and skip every other string without a word, so +// `Editable: '$currentObject/Locked = false'` was stored as Always — a field +// meant to be locked stayed editable, with check clean and exec reporting +// success. One rule now serves both: a string that is not the property's plain +// value is its condition (pages.StaticEditability). + +// staticEditability resolves the plain `Editable:` value of w, reporting whether +// the author wrote one. +// +// A list view and a grid column are the exception to "a boolean is Always or +// Never": their Editable is a plain bool of their own (ako/mxcli#510), read by +// their builders from GetBoolProp, and not the editability enum. A boolean there +// is that property's value, not a request for the enum. +func staticEditability(w *ast.WidgetV3) (enum, expr string, ok bool) { + v, present := lookupPropCI(w, "Editable") + if !present { + return "", "", false + } + if plainEditableWidgetTypes[strings.ToLower(w.Type)] && isBooleanValue(v) { + return "", "", false + } + return pages.StaticEditability(v) +} + +// isBooleanValue reports whether v is a boolean or the string spelling of one. +func isBooleanValue(v any) bool { + switch t := v.(type) { + case bool: + return true + case string: + s := strings.ToLower(strings.TrimSpace(t)) + return s == "true" || s == "false" + } + return false +} + +// editableCondition is the client expression of w's conditional editability +// however it was written: the bare or bracketed form (EditableIf), or a quoted +// `Editable: ''`. Every check that reads EditableIf asks this, so the +// quoted spelling gets the checks the unquoted one gets. +func editableCondition(w *ast.WidgetV3) (string, bool) { + if expr := w.GetStringProp("EditableIf"); expr != "" { + return expr, true + } + _, expr, ok := staticEditability(w) + return expr, ok && expr != "" +} + +// newConditionalEditability is the settings element of a conditional editability. +func newConditionalEditability(expr string) *pages.ConditionalEditabilitySettings { + return &pages.ConditionalEditabilitySettings{ + BaseElement: model.BaseElement{ + ID: model.ID(types.GenerateID()), + TypeName: "Forms$ConditionalEditabilitySettings", + }, + Expression: expr, + } +} + +var loneWordRe = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`) + +// editableValueProblem reports why the plain `Editable:` value of w cannot be +// stored, or "" when it can. exec stores every other value, so this is the +// whole list of what check must refuse. +func editableValueProblem(w *ast.WidgetV3) string { + enum, expr, ok := staticEditability(w) + if !ok { + return "" + } + switch { + case enum == "Conditional": + return "`Editable: Conditional` names no condition" + case enum != "": + return "" + case loneWordRe.MatchString(expr): + // A client expression has no bare identifiers, so a lone word is a + // misspelt enum word (`Neever`, `ReadOnly`) and not a condition. + return "`Editable: " + expr + "` is not an Editable value" + case !visitor.ParsesAsExpression(expr): + return "`Editable: " + expr + "` is not a valid expression" + } + return "" +} diff --git a/mdl/visitor/visitor_argument_binding.go b/mdl/visitor/visitor_argument_binding.go index 9bef6b924..847696f9a 100644 --- a/mdl/visitor/visitor_argument_binding.go +++ b/mdl/visitor/visitor_argument_binding.go @@ -197,6 +197,17 @@ func BareExpression(expr string) bool { return bareArgumentText(ctx.(parser.IExpressionContext)) == expr } +// ParsesAsExpression reports whether text is one MDL expression, read with the +// same grammar rule as a bare `Visible: ` / `Editable: `. check asks +// it of a quoted condition, whose text the parser otherwise never sees. +func ParsesAsExpression(text string) bool { + if strings.TrimSpace(text) == "" { + return false + } + _, ok := parseRule(text, func(p *parser.MDLParser) antlr.ParserRuleContext { return p.Expression() }) + return ok +} + // bareArgumentText is the expression a bare workflow argument stores: its // source text, as the Mendix expression editor would hold it. Unlike a // microflow expression stored as written (buildSourceExpression), a string in diff --git a/sdk/pages/pages_widgets.go b/sdk/pages/pages_widgets.go index fcf80d3b3..c99c8f5c8 100644 --- a/sdk/pages/pages_widgets.go +++ b/sdk/pages/pages_widgets.go @@ -183,6 +183,49 @@ func CanonicalEditability(v string) (string, bool) { } } +// StaticEditability resolves a static `Editable:` value, the counterpart of +// StaticVisibleExpression for the property that shares its grammar slot. A +// value is exactly one of: +// +// - an enum word (Always | Never | Conditional, any case), returned canonical +// as enum; +// - a boolean, or the string "true"/"false": `true` is Always and `false` is +// Never. Visible has no plain form and stores a constant condition for +// `false`; editability has one, which is what Studio Pro writes for "Never"; +// - anything else non-empty, which is the client expression of a conditional +// editability, verbatim (the caller roots it in $currentObject if it wants +// that, as for Visible). +// +// ok is false for an empty value and for a type that is none of these. A caller +// must treat "not ok" as "nothing authored", never as "something unreadable": +// every non-empty string and boolean lands in one of the three. +func StaticEditability(v any) (enum, expr string, ok bool) { + switch t := v.(type) { + case bool: + if t { + return "Always", "", true + } + return "Never", "", true + case string: + s := strings.TrimSpace(t) + if s == "" { + return "", "", false + } + if canon, isEnum := CanonicalEditability(s); isEnum { + return canon, "", true + } + switch strings.ToLower(s) { + case "true": + return "Always", "", true + case "false": + return "Never", "", true + } + return "", s, true + default: + return "", "", false + } +} + // WidgetEditability is the value a writer should store for a widget: the // conditional settings element wins, then an explicit `editable:`, then Mendix's // own default. diff --git a/sdk/pages/pages_widgets_editable_test.go b/sdk/pages/pages_widgets_editable_test.go new file mode 100644 index 000000000..ad0315521 --- /dev/null +++ b/sdk/pages/pages_widgets_editable_test.go @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: Apache-2.0 + +package pages + +import "testing" + +// Editable and Visible share a grammar slot, so a value must mean the same +// thing to both consumers' helpers: a string that is not the property's plain +// value is its condition. +func TestStaticEditability(t *testing.T) { + cases := []struct { + in any + enum string + expr string + ok bool + }{ + {"Always", "Always", "", true}, + {"never", "Never", "", true}, + {" NEVER ", "Never", "", true}, + {"Conditional", "Conditional", "", true}, + {true, "Always", "", true}, + {false, "Never", "", true}, + {"true", "Always", "", true}, + {"False", "Never", "", true}, + {"$currentObject/Locked = false", "", "$currentObject/Locked = false", true}, + {" $currentObject/Code != 'x' ", "", "$currentObject/Code != 'x'", true}, + // Not an enum word: an expression, and the caller's check decides if it is a sensible one. + {"Neever", "", "Neever", true}, + {"", "", "", false}, + {" ", "", "", false}, + {nil, "", "", false}, + {42, "", "", false}, + {[]string{"x"}, "", "", false}, + } + for _, c := range cases { + enum, expr, ok := StaticEditability(c.in) + if enum != c.enum || expr != c.expr || ok != c.ok { + t.Errorf("StaticEditability(%#v) = (%q, %q, %v), want (%q, %q, %v)", c.in, enum, expr, ok, c.enum, c.expr, c.ok) + } + } +} + +// Visible and Editable agree on what a quoted expression is: the string, verbatim. +func TestStaticEditability_ExpressionMatchesStaticVisibleExpression(t *testing.T) { + const e = "$currentObject/Locked = false" + ve, vok := StaticVisibleExpression(e) + _, ee, eok := StaticEditability(e) + if ve != ee || !vok || !eok { + t.Errorf("Visible reads %q (%v), Editable reads %q (%v)", ve, vok, ee, eok) + } +} From f23390fa7c2cf85cd871161164860be5cf4b01a0 Mon Sep 17 00:00:00 2001 From: stagknee <241821830+stagknee@users.noreply.github.com> Date: Sat, 17 Oct 2026 09:14:29 -0500 Subject: [PATCH 2/2] fix(executor): treat `editable: false` as Never in MDL-WIDGET31, cover pluggable and data view Follow-up to the quoted Editable expression fix. MDL-WIDGET31 still tested the raw string for "Never", so an input marked `editable: false` (now Never) in a read-only list view kept earning the warning that tells it to mark itself Never; it reads the same resolved value as the builder now. Tests added for the shapes a review asked about: a pluggable widget that declares Editability is refused the same unstorable values by MDL-WIDGET19, and a data view carries the quoted condition exactly as the bare one (its static Never/Always is stored from its own ReadOnly flag, which this change does not touch). CHANGELOG, finding record updated to match. Co-Authored-By: Claude Sonnet 5.5 --- ...17-quoted-editable-expression-dropped.json | 2 +- CHANGELOG.md | 2 +- ... 1567-quoted-editable-expression.fail.mdl} | 4 +- ...dl => 1567-quoted-editable-expression.mdl} | 4 +- .../validate_listview_editable_inputs.go | 4 +- .../validate_listview_editable_inputs_test.go | 22 ++++++++++ .../widget_editable_quoted_expression_test.go | 41 +++++++++++++++++++ 7 files changed, 72 insertions(+), 7 deletions(-) rename mdl-examples/bug-tests/{quoted-editable-expression.fail.mdl => 1567-quoted-editable-expression.fail.mdl} (91%) rename mdl-examples/bug-tests/{quoted-editable-expression.mdl => 1567-quoted-editable-expression.mdl} (94%) diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-17-quoted-editable-expression-dropped.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-17-quoted-editable-expression-dropped.json index 87add4d0c..3ebb73137 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-17-quoted-editable-expression-dropped.json +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-17-quoted-editable-expression-dropped.json @@ -1 +1 @@ -{"area":"mdl/executor","date":"2026-10-17","symptom":"A quoted `Editable: '$currentObject/Locked = false'` on a checkbox or text box passes check and exec (\"Created page\") and is stored `Editable: Always` with no ConditionalEditabilitySettings, so a field meant to be locked stays editable; `describe page` prints the widget without any Editable. The quoted `Visible: ''` beside it is stored as a conditional visibility, and the unquoted `Editable: $currentObject/Locked = false` works. `Editable: false` on an input was dropped the same way","cause":"`applyConditionalSettings` (cmd_pages_builder_v3.go) consulted only `pages.CanonicalEditability`, which accepts exactly Always|Never|Conditional and answers (\"\", false) for any other string, so the `else if` did not fire and the default Always stayed. `Visible` goes through `pages.StaticVisibleExpression`, whose default branch hands any other non-empty string back as the expression. MDL-WIDGET19 let every bool/string through for both keys (\"a static form StaticVisibleExpression consumes\"), true for Visible and false for Editable; a bool never reached the builder at all because it read the value with GetStringProp, which is \"\" for a bool","file":"`sdk/pages/pages_widgets.go` (new `StaticEditability`), `mdl/executor/widget_editable_static.go` (new: `staticEditability`, `editableCondition`, `editableValueProblem`), `cmd_pages_builder_v3.go` (`applyConditionalSettings`), `pluggable_system_props.go` (`widgetAsksEditability`), `validate_widgets.go` (MDL-WIDGET19, MDL-WIDGET13), `validate_page_button_context.go` (MDL-BUTTON02), `validate_widget_editability.go` (MDL-WIDGET20)","insight":"Two sibling properties with the same grammar slot but different consumers: one takes any string as an expression, the other only an enum word and skips the rest silently. A skipped write must be a refusal, so the fix is one rule for both keys (a string that is not the property's plain value is its condition) PLUS a check that names what still cannot be stored (a lone word such as `Neever`, text that is not an expression, `Conditional` with no condition). Every reader of `EditableIf` is a second consumer of the same value and has to see the quoted spelling too: MDL-WIDGET13 (association step, CE0117), MDL-BUTTON02 (own container name), MDL-WIDGET41 (pluggable Editability gate) and MDL-WIDGET20; fixing only the builder leaves them blind. Not every widget stores the enum: a list view and a grid column have a plain `Editable bool` (ako/mxcli#510), so a boolean there must NOT be read as Never/Always (`plainEditableWidgetTypes`). A quoted string reaches the validators unparsed, so a malformed quoted condition needs `visitor.ParsesAsExpression`; the bare spelling fails in the grammar. Quoted Visible still gets none of those checks (WIDGET13, BUTTON02, a syntax check) — left as is, same family. ALTER PAGE `set (Editable: '')` keeps its loud refusal (pagemutator setWidgetEditableMut); accepting it is about five lines (settings element plus the Conditional enum), not one. Control: same script on the pre-fix binary stores Always/null for the quoted widgets; the fix stores Conditional + expression, byte-equal (ids stripped) to the unquoted spelling, and describe -> exec reports `Unchanged page`","refs":[],"rules":["MDL-WIDGET19","MDL-WIDGET13","MDL-BUTTON02","MDL-WIDGET20","MDL-WIDGET41"]} +{"area":"mdl/executor","date":"2026-10-17","symptom":"A quoted `Editable: '$currentObject/Locked = false'` on a checkbox or text box passes check and exec (\"Created page\") and is stored `Editable: Always` with no ConditionalEditabilitySettings, so a field meant to be locked stays editable; `describe page` prints the widget without any Editable. The quoted `Visible: ''` beside it is stored as a conditional visibility, and the unquoted `Editable: $currentObject/Locked = false` works. `Editable: false` on an input was dropped the same way","cause":"`applyConditionalSettings` (cmd_pages_builder_v3.go) consulted only `pages.CanonicalEditability`, which accepts exactly Always|Never|Conditional and answers (\"\", false) for any other string, so the `else if` did not fire and the default Always stayed. `Visible` goes through `pages.StaticVisibleExpression`, whose default branch hands any other non-empty string back as the expression. MDL-WIDGET19 let every bool/string through for both keys (\"a static form StaticVisibleExpression consumes\"), true for Visible and false for Editable; a bool never reached the builder at all because it read the value with GetStringProp, which is \"\" for a bool","file":"`sdk/pages/pages_widgets.go` (new `StaticEditability`), `mdl/executor/widget_editable_static.go` (new: `staticEditability`, `editableCondition`, `editableValueProblem`), `cmd_pages_builder_v3.go` (`applyConditionalSettings`), `pluggable_system_props.go` (`widgetAsksEditability`), `validate_widgets.go` (MDL-WIDGET19, MDL-WIDGET13), `validate_page_button_context.go` (MDL-BUTTON02), `validate_widget_editability.go` (MDL-WIDGET20), `validate_listview_editable_inputs.go` (MDL-WIDGET31)","insight":"Two sibling properties with the same grammar slot but different consumers: one takes any string as an expression, the other only an enum word and skips the rest silently. A skipped write must be a refusal, so the fix is one rule for both keys (a string that is not the property's plain value is its condition) PLUS a check that names what still cannot be stored (a lone word such as `Neever`, text that is not an expression, `Conditional` with no condition). Every reader of `EditableIf` is a second consumer of the same value and has to see the quoted spelling too: MDL-WIDGET13 (association step, CE0117), MDL-BUTTON02 (own container name), MDL-WIDGET41 (pluggable Editability gate) and MDL-WIDGET20; fixing only the builder leaves them blind. Not every widget stores the enum: a list view and a grid column have a plain `Editable bool` (ako/mxcli#510), so a boolean there must NOT be read as Never/Always (`plainEditableWidgetTypes`). A quoted string reaches the validators unparsed, so a malformed quoted condition needs `visitor.ParsesAsExpression`; the bare spelling fails in the grammar. Quoted Visible still gets none of those checks (WIDGET13, BUTTON02, a syntax check) — left as is, same family. ALTER PAGE `set (Editable: '')` keeps its loud refusal (pagemutator setWidgetEditableMut); accepting it is about five lines (settings element plus the Conditional enum), not one. Control: same script on the pre-fix binary stores Always/null for the quoted widgets; the fix stores Conditional + expression, byte-equal (ids stripped) to the unquoted spelling, and describe -> exec reports `Unchanged page`. Side find, not fixed: a data view stores its static editability from its own ReadOnly flag (Forms$DataView.Editability), which the builder never sets from `Editable:`, so `DATAVIEW (Editable: Never)` is still stored Always (measured: bson dump shows Editability Always for Never, false and true alike); only the condition (ConditionalEditabilitySettings) is carried, and the quoted spelling now matches the bare one there","refs":[],"rules":["MDL-WIDGET19","MDL-WIDGET13","MDL-BUTTON02","MDL-WIDGET20","MDL-WIDGET41"], "refs": ["mendixlabs/mxcli#1567"]} diff --git a/CHANGELOG.md b/CHANGELOG.md index 39d4cc5dc..3fee1bc49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed -- **A quoted `Editable: ''` is stored as a conditional editability instead of being dropped** (issue number to be added) — `CHECKBOX cb (Attribute: Flag, Editable: '$currentObject/Locked = false')` passed `check`, `exec` said "Created page", and the widget was stored `Editable: Always` with no condition, so a field meant to be locked stayed editable (`mx check` also reported 0 errors). `Visible: ''` beside it was stored as a conditional visibility, and the unquoted `Editable: $currentObject/Locked = false` worked: the page builder read `Editable` only as one of `Always`/`Never`/`Conditional` and skipped every other string without a word. A string that is not one of those is now the condition, exactly as for `Visible`, and `describe` prints it back bare, so a describe → exec round trip is stable. `Editable: false` / `true` were dropped the same way and now store `Never` / `Always` (on a list view and a grid column they stay the plain boolean they always were). `check` refuses what still cannot be stored — a misspelt `Editable: Neever`, a quoted text that is not an expression, `Editable: Conditional` with no condition — as MDL-WIDGET19 naming the accepted spellings, and a quoted condition now gets the checks the bare one gets (MDL-WIDGET13 association steps, MDL-BUTTON02, MDL-WIDGET41 on a pluggable widget that declares no Editability, MDL-WIDGET20). `alter page … set (Editable: '')` still refuses a quoted condition with its existing message. +- **A quoted `Editable: ''` is stored as a conditional editability instead of being dropped** (mendixlabs/mxcli#1567) — `CHECKBOX cb (Attribute: Flag, Editable: '$currentObject/Locked = false')` passed `check`, `exec` said "Created page", and the widget was stored `Editable: Always` with no condition, so a field meant to be locked stayed editable (`mx check` also reported 0 errors). `Visible: ''` beside it was stored as a conditional visibility, and the unquoted `Editable: $currentObject/Locked = false` worked: the page builder read `Editable` only as one of `Always`/`Never`/`Conditional` and skipped every other string without a word. A string that is not one of those is now the condition, exactly as for `Visible`, and `describe` prints it back bare, so a describe → exec round trip is stable. `Editable: false` / `true` on an input widget were dropped the same way and now store `Never` / `Always` (a list view and a grid column keep their own plain boolean, and a data view keeps its existing handling). `check` refuses what still cannot be stored — a misspelt `Editable: Neever`, a quoted text that is not an expression, `Editable: Conditional` with no condition — as MDL-WIDGET19 naming the accepted spellings, and a quoted condition now gets the checks the bare one gets (MDL-WIDGET13 association steps, MDL-BUTTON02, MDL-WIDGET41 on a pluggable widget that declares no Editability, MDL-WIDGET20, and MDL-WIDGET31 no longer warns about an input marked `editable: false`). `alter page … set (Editable: '')` still refuses a quoted condition with its existing message. ## [0.27.0] - 2026-10-15 diff --git a/mdl-examples/bug-tests/quoted-editable-expression.fail.mdl b/mdl-examples/bug-tests/1567-quoted-editable-expression.fail.mdl similarity index 91% rename from mdl-examples/bug-tests/quoted-editable-expression.fail.mdl rename to mdl-examples/bug-tests/1567-quoted-editable-expression.fail.mdl index 29dcfd515..6e1d15c1e 100644 --- a/mdl-examples/bug-tests/quoted-editable-expression.fail.mdl +++ b/mdl-examples/bug-tests/1567-quoted-editable-expression.fail.mdl @@ -1,8 +1,8 @@ mdl 1; --- Negative companion of quoted-editable-expression.mdl: `check` must refuse every +-- Negative companion of 1567-quoted-editable-expression.mdl: `check` must refuse every -- Editable value that cannot be stored, naming the accepted spellings, instead of -- letting the widget stay editable. Each statement below is refused on its own. --- The issue number is to be added. +-- mendixlabs/mxcli#1567. create entity if not exists QEdit.Item (Code: String(50), Locked: Boolean default false); diff --git a/mdl-examples/bug-tests/quoted-editable-expression.mdl b/mdl-examples/bug-tests/1567-quoted-editable-expression.mdl similarity index 94% rename from mdl-examples/bug-tests/quoted-editable-expression.mdl rename to mdl-examples/bug-tests/1567-quoted-editable-expression.mdl index 2839fe019..0da89bb47 100644 --- a/mdl-examples/bug-tests/quoted-editable-expression.mdl +++ b/mdl-examples/bug-tests/1567-quoted-editable-expression.mdl @@ -1,5 +1,5 @@ mdl 1; --- A quoted `Editable: ''` was dropped on write (issue number: to be added). +-- A quoted `Editable: ''` was dropped on write (mendixlabs/mxcli#1567). -- -- `Visible: '$currentObject/Locked = false'` was stored as a conditional -- visibility, because pages.StaticVisibleExpression returns any string that is not @@ -19,7 +19,7 @@ mdl 1; -- inputs print their condition bare (`Editable: $currentObject/Locked = false`, -- the `!= 'x'` form, ...), txtNever and txtFalse print `Editable: Never`, and the -- stored page passes `mx check`. Values that cannot be stored are refused by --- `check` instead: see quoted-editable-expression.fail.mdl. +-- `check` instead: see 1567-quoted-editable-expression.fail.mdl. create entity if not exists QEdit.Item ( Code: String(50), diff --git a/mdl/executor/validate_listview_editable_inputs.go b/mdl/executor/validate_listview_editable_inputs.go index da27d95e3..87f5bf92a 100644 --- a/mdl/executor/validate_listview_editable_inputs.go +++ b/mdl/executor/validate_listview_editable_inputs.go @@ -76,7 +76,9 @@ func firstEditableInput(widgets []*ast.WidgetV3, registry *WidgetRegistry) *ast. if typ == "listview" { continue } - never := strings.EqualFold(c.GetStringProp("Editable"), "Never") + // `editable: false` is Never too, quoted or not (pages.StaticEditability). + enum, _, hasStatic := staticEditability(c) + never := hasStatic && enum == "Never" if editableWidgetTypes[typ] && typ != "dataview" && !never { return c } diff --git a/mdl/executor/validate_listview_editable_inputs_test.go b/mdl/executor/validate_listview_editable_inputs_test.go index 22c7fbe26..48afdcdad 100644 --- a/mdl/executor/validate_listview_editable_inputs_test.go +++ b/mdl/executor/validate_listview_editable_inputs_test.go @@ -98,6 +98,28 @@ func TestMDLWIDGET31_ListViewInputsNotEditable(t *testing.T) { }`, want: 0, }, + { + // `editable: false` and the quoted spellings are Never as well: the + // author already made these inputs read-only, so there is nothing to warn about. + name: "control: inputs marked editable false or 'Never'", + src: `create page M.P (title: 'P', layout: Atlas_Core.Atlas_Default) { + listview lv (datasource: database M.Thing) { + textbox t (label: 'N', attribute: Name, editable: false) + textbox u (label: 'M', attribute: Name, editable: 'Never') + } +}`, + want: 0, + }, + { + // A condition is not Never: the input can still be editable, so the list view still decides. + name: "an input with a quoted condition is still warned about", + src: `create page M.P (title: 'P', layout: Atlas_Core.Atlas_Default) { + listview lv (datasource: database M.Thing) { + textbox t (label: 'N', attribute: Name, editable: '$currentObject/Name = empty') + } +}`, + want: 1, + }, { // A nested list view's inputs are governed by the NESTED list // view's Editable, so the outer one is not the one to report. diff --git a/mdl/executor/widget_editable_quoted_expression_test.go b/mdl/executor/widget_editable_quoted_expression_test.go index b643d8f6e..9750ed392 100644 --- a/mdl/executor/widget_editable_quoted_expression_test.go +++ b/mdl/executor/widget_editable_quoted_expression_test.go @@ -358,3 +358,44 @@ func TestApplyPluggableSystemSettings_QuotedExpression(t *testing.T) { }) } } + +// A pluggable widget that declares Editability takes the same values, and is +// refused the same way for the ones that cannot be stored. +func TestMDLWIDGET19_PluggableWidgetEditableValues(t *testing.T) { + cases := []struct { + prop string + refuse bool + }{ + {`Editable: '` + lockedExpr + `'`, false}, + {`Editable: Never`, false}, + {`Editable: false`, false}, + {`Editable: 'Neever'`, true}, + {`Editable: '$currentObject/Locked = = false'`, true}, + {`Editable: Conditional`, true}, + } + for _, c := range cases { + t.Run(c.prop, func(t *testing.T) { + got := checkMessages(t, `combobox c (Attribute: Status, `+c.prop+`)`, "MDL-WIDGET19") + if c.refuse != (len(got) == 1) || len(got) > 1 { + t.Errorf("%s: want refused=%v, got %v", c.prop, c.refuse, got) + } + }) + } +} + +// A data view stores its static editability from its own ReadOnly flag, not from +// BaseWidget.Editable, so only the condition is carried: the quoted spelling must +// store the same ConditionalEditabilitySettings the bare one does. +func TestQuotedEditableExpression_DataViewMatchesBare(t *testing.T) { + build := func(props map[string]any) *pages.DataView { + dv := &pages.DataView{} + applyConditionalSettings(dv, &ast.WidgetV3{Type: "dataview", Name: "dv", Properties: props}) + return dv + } + bare := build(map[string]any{"EditableIf": lockedExpr}) + quoted := build(map[string]any{"Editable": lockedExpr}) + if bare.ConditionalEditability == nil || quoted.ConditionalEditability == nil || + bare.ConditionalEditability.Expression != quoted.ConditionalEditability.Expression { + t.Errorf("bare stored %v, quoted stored %v", bare.ConditionalEditability, quoted.ConditionalEditability) + } +}