diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index 484519e74..f75d4e0bc 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -336,6 +336,10 @@ cases for these three BSON types — they fell to `default: return nil`. | `DESCRIBE MICROFLOW` emits **`on error rollback`** on activities authored with no error-handling clause at all, growing the diff on every round-trip. No checker flags it — `"Rollback"` is structurally valid, so `mx check` and every mxcli validator pass | `Rollback` is what `convertErrorHandlingType(nil)` stores for an activity with no clause **and** what the parser falls back to when `ErrorHandlingType` is absent from the BSON. The stored value therefore cannot distinguish an authored clause from the default, and read-back guessed "authored" | `mdl/executor/cmd_microflows_show_helpers.go` (`formatErrorHandlingSuffix`) | Drop the `Rollback` case so it falls through to no suffix. **The asymmetry is the whole argument**: omitting it is lossless (re-executing stores `Rollback` again, so the model is unchanged), while emitting it is lossy in the direction that matters — it puts a clause in the user's script that they never wrote. `Continue` / `Custom` / `CustomWithoutRollback` are never defaults, so they still round-trip. **Generalisable — the shape to look for**: when a formatter renders an enum whose zero/fallback value is also a legal authored value, read-back cannot invert the write; render only the values that are *never* defaults. Ask "what does the parser fall back to?" before trusting a stored enum to mean the author chose it. Repro `mdl-examples/bug-tests/840-describe-invents-on-error-rollback.mdl`; verified end-to-end (describe → exec → describe byte-identical, `mx check` 11.13.0 0 errors). Issue #840 | + +| `mxcli check` **segfaults** (`SIGSEGV` in `visitor.(*Builder).ExitSqlDisconnect`) on the one-line script `SQL DISCONNECT source;` — which is the exact line in mxcli's own `mxcli syntax sql` example. Other aliases (`mydb`, `src`) are fine | Two defects. **Grammar**: every `sqlStatement` alternative took a bare `IDENTIFIER` for the alias/driver/table, but `source` lexes as `SOURCE_KW` (`MDLLexer.g4:317`), so no alternative matched. `IMPORT FROM identifierOrKeyword` in the same file already accepted a keyword alias, so `import from source …` worked while `sql source …` did not. **Robustness**: ANTLR error-recovers and still walks the tree, so the listener received a context whose `IDENTIFIER()` was nil and `.GetText()` crashed the process — sibling handlers guarded their children (`if len(ids) < 2 { return }`), this one did not | `mdl/grammar/domains/MDLSettings.g4` (`sqlStatement` → `identifierOrKeyword`), `mdl/visitor/visitor_sql.go` (`sqlWords` helper + guards at all five handlers) | Use `identifierOrKeyword` for every user-chosen word, and read children through a helper that tolerates a missing node. **Generalisable — the shape to look for**: a listener that dereferences `ctx.X()` without a nil check is a crash waiting for the first input that fails to parse *that rule* — ANTLR does not stop the walk on a syntax error. Grep for `ctx\.[A-Z][A-Za-z]*()\.GetText()` and treat each as unguarded. **Second shape**: when a bare `IDENTIFIER` names something a user picks (alias, table, column, prefix), it silently forbids every keyword; `identifierOrKeyword` is nearly always what was meant, and the inconsistency shows up as "works in one statement, not the neighbouring one". **Watch the index shift** when converting: the alias joins the `AllIdentifierOrKeyword()` list, so downstream `[0]`/`[1:]` offsets move (this bit `ExitSqlGenerateConnector`, where getting it wrong would generate a connector into a module named after the connection). **How it was found — and the trap it hid**: `TestExamplesParse` (new) feeds every `mxcli syntax` example through the parser; the panic **aborted the test binary**, masking ten further failures in entries sorted alphabetically after `sql`. A crash in a table-driven test is not one failure, it is an unknown number. Repro `mdl-examples/bug-tests/sql-keyword-alias-crash.mdl` | + +| `mxcli syntax ` teaches MDL that does not parse — `Binds:` (removed in favour of `Attribute:`, and hard-rejected by the parser), workflow decision outcomes without the `->` arrow, `ALTER WORKFLOW … SET DUE DATE = '…'` (no `=`), `INSERT AFTER ` (operands reversed), `IMAGE 'name'` (identifier, not string), a `BEFORE` that does not exist. Nothing failed, because nothing checked | The registry (`cmd/mxcli/syntax/features_*.go`) is hand-maintained while the grammar moves underneath it, and every existing test checked *structure* — fields populated, aliases resolve, see-also targets exist — never whether the documented MDL is real. 26 of 120 entries had a non-parsing example | `cmd/mxcli/syntax/example_parses_test.go` (new guard), plus corrections across `features_page.go`, `features_workflow.go`, `features_integration.go`, `features_microflow.go`, `features_misc.go` | Parse every `Example` and fail the build if it does not. Examples come in several legitimate shapes, so each blank-line-separated **block** is tried as a statement, a microflow activity, a page widget, a workflow activity, and a retrieve clause — a failure means it parses as none of them. Only `Example` is checked; `Syntax` carries metasyntax (`[OR MODIFY]`, ``) by design. **Generalisable — the shape to look for**: documentation that is *data in the binary* can be executed against the real parser, which turns "the docs drifted" from a review problem into a test failure. The same trick applies to any embedded example corpus (skills, `--help` text, README snippets extracted at build time). **Why it matters more than it looks**: the registry is the first surface an agent consults, so a gap there does not read as "undocumented", it reads as "unsupported" — a contact-management app built with mxcli worked around three features that already existed, including replacing a `SAVE_CHANGES CLOSE_PAGE` button with a bespoke microflow because only the two halves were listed separately. **Close the opt-out** — a guard that skips empty input is a guard you can silence by emptying the field; `TestFeatureFieldsPopulated` already rejects a blank `Example`, and the new guard additionally fails an example that is all comments | | SCSS written to **`themesource//web/main.scss` never reaches the app** — no error, no warning, the build succeeds and the rules are simply absent from `theme-cache/web/theme.compiled.css`. Looks exactly like an SCSS cache problem, so the usual reflex (`rm -rf theme-cache/`) wastes the session | A theme source folder is only compiled when `` matches a **real module in the model**. mxbuild walks the model's modules and pulls each one's `themesource//web/main.scss`; it never globs the `themesource/` directory, so an invented folder (`themesource/my_theme/`) is silently skipped. Verified on 11.13: a probe rule in `themesource/myfirstmodule/` compiled, the identical rule in `themesource/mxcli_theme/` did not | `cmd/mxcli/theme/theme.go` (package doc records the compile order); the target paths live in `cmd/mxcli/theme/assets//files/` | Put app-level styling in **`theme/web/`**, not in an invented theme source folder: `theme/web/main.scss` is compiled **last** — after Atlas Core *and* after every module theme source — so a partial imported from it overrides any Atlas rule without `!important`. Use a module's theme source only when the styling genuinely belongs to that module (it exports with the `.mpk`). **Generalisable — the shape to look for**: when CSS "doesn't apply", first prove the file is *compiled at all* (grep a unique probe selector in `theme-cache/web/theme.compiled.css`) before debugging specificity or caches — absent and overridden look identical in the browser. Note also that `theme/web/custom-variables.scss` is imported once **per module**, so it must hold declarations only; a rule there is emitted N times | | A **`CREATE JAVASCRIPT ACTION`** succeeds, `mxcli check` passes and the build is clean, but calling the action in the running app throws **`JavaScript action was not implemented`** and the nanoflow aborts | mxcli wrote the source to `javascriptsource//actions/` using the module's own casing. Mendix reads a **lowercased** directory — a blank Mendix 11 app ships `javascriptsource/nanoflowcommons/`, `/datawidgets/`, `/webactions/` for modules named `NanoflowCommons`, `DataWidgets`, `WebActions`. Finding no source at the path it reads, mxbuild generates a stub whose body is `throw new Error("JavaScript action was not implemented")` and bundles that. Only reproduces on a **case-sensitive filesystem**, which is why it survived: on macOS and Windows the two spellings are the same directory | `mdl/backend/modelsdk/javascript_write.go` and `sdk/mpr/writer_javascriptactions.go` (`jsActionSourceDir`) | `strings.ToLower(moduleName)` in both writers — the comment in each previously asserted the opposite ("unlike javasource, which is lowercased"), so the belief was documented, not tested. **Generalisable — the shape to look for**: when generated *source files* pair with model units, the model unit is not evidence the file is found; the filesystem path is a separate contract, and a case-only mismatch is invisible on the developer's own machine. Check against the directories a blank project already ships rather than against what the code says. Nothing short of running the app catches it: parse, check and build all pass. Test `mdl/backend/modelsdk/javascript_write_dir_test.go`; verified end-to-end (button click flips the theme instead of throwing) | @@ -377,6 +381,9 @@ cases for these three BSON types — they fell to `default: return nil`. | Wiring a microflow that takes parameters to a **BEFORE CREATE** event handler passes `mxcli check` (and `--references`), and the build then fails `[error] [CE7247] "Microflow should not have parameters" at Event handler of entity …` | Mendix passes no object to a before-create handler — the object does not exist yet — so the handler is called with no arguments. Nothing compared the handler's moment against the microflow's signature; the pairing is only invalid for this one moment/event combination | `mdl/executor/cmd_entities.go` (`checkBeforeCreateHandlerHasNoParameters`, called from `buildEventHandlers`) | Guard where the two paths converge — `buildEventHandlers` is shared by `CREATE ENTITY`'s inline handlers and `ALTER ENTITY ADD EVENT HANDLER`, so one check covers both. It refuses **before the model is written**, and the message carries the build code plus the way out (AFTER CREATE, which does receive the object). **A microflow created earlier in the same script is not readable back yet, so an unreadable microflow is skipped rather than refused** — mxbuild still catches the real case, and failing on the read would break legitimate scripts. Note this is an exec-time guard: `mxcli check` without a project cannot see the microflow's signature at all. A/B on 11.12.1: pre-fix binary writes it and mxbuild reports CE7247; fixed binary refuses, and both AFTER CREATE and a no-parameter BEFORE CREATE still work. Tests `mdl/executor/cmd_entities_before_create_test.go`. mxcli-todo #14a | | "Contrast is low and not everything uses the dark theme" — and switching theme does not help, because `signal`, `ledger` and `console` all render the same defects. The worst of it is the **login page**: Atlas's stock photograph fills half the viewport on a dark app, and the Sign in button is green while the app's primary button is the brand colour | The login page is served from `theme/web/login.html`, which loads the SAME compiled theme CSS (`{{themecss}}`) — so it IS themeable, but nothing themed it. Two Atlas rules do the damage: `.loginpage-image` layers a brand-tinted gradient over `url("./resources/work-do-more.jpeg")`, and the submit button is `.btn-success`, so it follows the **success** colour rather than the brand. Separately, `--link-color` was mapped straight to the brand, and console's light-variant teal is 3.74:1 on white — under AA for body text | `cmd/mxcli/theme/assets/*/files/theme/web/_mxcli-atlas-map.scss` (login block + `--mxt-link` indirection, all three copies stay byte-identical), `cmd/mxcli/theme/assets/console/.../_mxcli-console.scss` (light `--mxt-link`) | Replace the photo with a token-built gradient and point the login button at the brand; give link TEXT its own `--mxt-link` token defaulting to the brand, because a link needs 4.5:1 as text while a brand used as a button FILL only needs 3:1 plus contrast against its own ink — darkening the brand everywhere would have been the wrong lever. Console light gets `#0f766e` (5.47 / 5.10 / 4.92 against surface / ground / surface-alt). **The project's own `theme/web/logo.png` is deliberately left alone** — it is the app's asset to replace, and hiding it would strip a real logo from apps that have one. **Verification boundary, stated plainly**: verified at the compiled-CSS layer (the overriding `.loginpage-image` is last and carries no photo; the button and link declarations resolve), NOT in a browser — raising the scratch project's security level to serve a login page surfaced pre-existing model errors there that block deploy. The `#999` empty-state label could not be reproduced: every `#999` in the compiled CSS is a Bootstrap default (popover arrows, modal border, print styles), so that one needs the reporter's app. mxcli-todo #19 | +| A wrong `icon:` reference passes `mxcli check` (including `--references`) and first surfaces as a **build** error: `[error] [CE1613] "The selected custom icon 'Atlas_Core.Atlas_Filled.no-such-icon' no longer exists." at Action button 'btnBad'` | Nothing resolved the reference — the name was written straight through to BSON. Icon-collection lookup existed only for `show`/`describe` (`cmd_iconcollections.go`), never in a validation path | `mdl/executor/validate_icon_refs.go` (`validateIconRefs`), wired into `validateProgram` | Index the project's icon collections once per run and resolve every reference in the program. **Put it in the `--references` pass, not the no-project one** — the collections are documents *in the project* (a blank 11.13 app ships three, ~770 icons), so unlike the #836 grant check there is genuinely nothing to resolve against without `-p`. **Report the two failures differently**: an unknown *icon* in a known collection gets near-match suggestions plus `describe icon collection `; an unknown *collection* gets the list of collections that exist, because that is where the typo usually is. **Generalisable — the shape to look for**: a string property that names a model element but is stored as a plain string is invisible to every reference checker; grep for properties whose value is a qualified name yet whose type is `string`. **Test the silence, not just the noise**: the risk in a new check rule is false positives, so sweep the repo's own examples (all 9 `mdl-examples` scripts using icons) before shipping — a rule that fires on valid input is worse than the gap it closes. **Repro cannot be a `.fail.mdl`**: `make check-mdl` runs `mxcli check` with no `-p`, so a bad-icon script would pass there and be reported as a negative test unexpectedly passing; the repro carries valid icons and the rejection cases live in unit tests. Repro `mdl-examples/bug-tests/icon-reference-validation.mdl`; verified end-to-end (bad reference reported before any write; `mx check` 11.13.0 0 errors on the valid script) | +| `ALTER PAGE … SET Action = microflow M.F ON btn` does not parse — `extraneous input 'M' expecting {DROP, ADD, SET, INSERT, REPLACE, '}'}`. The documented workaround is `REPLACE`, which works but silently drops every property the statement does not restate (ButtonStyle, Class, design properties, tooltip) | `alterPageAssignment` special-cases `DATASOURCE`, `VISIBLE` and `EDITABLE`, then falls through to `identifierOrKeyword EQUALS propertyValueV3` — and `propertyValueV3` has no `microflow ` form, so the value position could not hold an action at all. `CREATE PAGE` has had `ACTION COLON actionExprV3` all along | `mdl/grammar/MDLParser.g4` (`alterPageAssignment` gains `ACTION EQUALS actionExprV3`), `mdl/visitor/visitor_alter_page.go` (`buildAlterPageAssignment`), `mdl/executor/cmd_alter_page.go` (`convertASTAction` + routing), `mdl/backend/mutation.go` + `pagemutator/mutator.go` + `mock/` + `mcp/` (`SetWidgetAction`) | Reuse `actionExprV3` — the same rule CREATE PAGE uses — and build through the **CREATE PAGE builder** (`pb.buildClientActionV3`) rather than a second switch. **Generalisable — the shape to look for**: when `SET` and `REPLACE` (or any narrow/wide pair) can express different vocabularies for the same property, the narrow one is a whitelist that will be extended one bug report at a time — #855 was the identical bug for `DataSource`, filed separately. Delegate instead of enumerating, and the pair cannot drift. **Guard-don't-drop**: refuse `SET Action` on a widget with no `Action` property rather than writing it — Studio Pro resolves every stored property against the type's property list and throws, while mxbuild tolerates the unknown key, so a silent write **builds clean and fails to open**; the build is not a safety net. **Measurement trap**: reverting only the grammar rule does not produce a failing test, it produces a *compile* error (the visitor references `ctx.ActionExprV3()`), so prove causation by reverting grammar+visitor together and re-running the original statement through the CLI. **Do not be misled by CE1571 on a `SHOW_PAGE` action** — authoring the same action through CREATE PAGE reproduces it exactly, so it is the builder's `$currentObject` auto-binding outside a dataview, not this change; always author the control before blaming the new path. Note `OPEN_LINK` still refuses on the modelsdk engine (`LinkClientAction` is unsupported by the codec) — also pre-existing, and CREATE PAGE refuses it identically. Repro `mdl-examples/bug-tests/alter-page-set-action.mdl`; verified end-to-end (Mendix 11.13.0, `mx check` 0 errors, `Class` survives four retargets without being restated) | + **Key insight:** `microflows$ListRange` stores offset/limit inside a nested `CustomRange` map — must cast `raw["CustomRange"].(map[string]any)` before extracting `OffsetExpression`/`LimitExpression`. @@ -460,3 +467,8 @@ extracting `OffsetExpression`/`LimitExpression`. | Re-running an MDL script against a project that is already in sync rewrites the `.mxunit` files anyway — `git status` is dirty, Studio Pro shows version-control changes, and three consecutive runs of one script give three different tree hashes across 143 documents while nothing semantic moves | No write path compared against what was stored. `create or replace` rebuilds the document from the MDL and overwrites unconditionally, and every sub-element in that rebuild is a new object whose `$ID` comes from `GenerateID()` — a random UUID. The output was a function of the script **and a random source**, so byte stability was unreachable by any amount of care in the builders | `modelsdk/canon/` (`Digest`, `Equal`, `Reconcile`), wired at the two storage choke points: `modelsdk/mpr/writer_core.go` (`reconcileWithStored`, covering `updateUnit` and `WriteTransaction.WriteUnit`) and `sdk/mpr/writer_units.go` (`updateUnit`) | **Compare a canonical form, not bytes** — the rebuilt bytes always differ, so a byte comparison skips nothing. Canonicalisation replaces every element `$ID` with its index in a deterministic containment walk; you do **not** need to know which properties hold references, because the set of element IDs comes from the walk and any occurrence of one of those IDs anywhere is a reference by definition. **Bias every failure path toward writing**: a false "different" costs a redundant write (yesterday's behaviour), a false "equal" silently discards the user's intent. **Skipping is safer than writing, not just cheaper** — the stored IDs are the ones every pointer inside that unit already agrees with, which is precisely what PR #125 broke by rewriting them. **Wire every choke point, both engines**: `codec.Store.SaveUnit`/`FlushUnits` reach storage through `WriteTransaction.WriteUnit`, not `UpdateRawUnit`, and which engine ran is an `--engine` flag users must not be able to see in a diff. **A control is mandatory**: assert that with `MXCLI_ALWAYS_WRITE=1` the same write *does* churn, or the test passes against a build that never had the fix. Tests `mdl/backend/modelsdk/microflow_idempotence_test.go`, `sdk/mpr/writer_elision_test.go`, `modelsdk/canon/canon_test.go`. ADR-0008 | | A microflow's `StableId` is a different GUID after every mxcli write, so a microflow always differs from itself and no amount of no-op elision can settle it. Downstream: every `callMicroflow` entry in `deployment/model/operations.json` gets a new `operationId` on each build | `microflow_write.go` registered `StableId` as a `codec` `FreshGUIDField`, minting a fresh GUID per write. It is the one field on a microflow whose *stated purpose* is not to change: Mendix declares it `ModelPropertyAttribute("StableId", RetentionType.DesignTime)` with `IsIdentifier = true`, seeds it once via the one-time `MicroflowStableIdConversion`, and transplants it across a marketplace module update in `PackageUtils.RescueStableIDs` | `modelsdk/canon/identity.go` (`identityFields`, `CarryIdentity`) — the stored value is carried onto the rebuilt document before the comparison, so an otherwise-unchanged microflow compares equal and is elided | **Establish that a property is an identity before adding a row to `identityFields`** — do not assume it from the name. The method that settled this one: strict-boundary `strings` scan of every runtime jar (a substring match gives false positives — `newPersistableIds` contains `stableId`); `monodis` the modeler assembly and read the `ModelPropertyAttribute` blob; look for an `IOneTimeConversion` named after the field; search the packaging assembly for a get→set pair; then build with `mxbuild --target=deploy` and reproduce the derivation against `deployment/`. That last step is what proved the value escapes the model: `operationId == base64(uuid5(projectId, StableId).bytes_le)`, 10 of 10 exact. **Carry only a key both documents already have** — never invent one, and dispatch on the stored `$Type` (CLAUDE.md, overlay writes). **Identity preservation is not gated by `MXCLI_ALWAYS_WRITE`**: that flag turns off eliding a write, not preserving what the document is. Tests `modelsdk/canon/identity_test.go`, `sdk/mpr/writer_elision_test.go`. ADR-0008 | | `call workflow`, `get workflow data/workflows/activity records`, `open`/`lock`/`unlock workflow` and `workflow operation` all DESCRIBE as a placeholder, so a describe→edit→exec cycle deletes them — while authoring and building them works fine | The modelsdk reader (the DEFAULT engine) had no case for any of the eight; each read back with a nil Action. The formatters and grammar were already there, so only `actionFromGen` was missing | New `mdl/backend/modelsdk/microflow_workflow_read.go` (+ dispatch in `microflow_read_actions.go`), `mdl/grammar/domains/MDLMicroflow.g4` + `mdl/visitor/visitor_microflow_workflow.go` (positional `call workflow` form), `mdl/executor/cmd_microflows_builder_workflow.go` (abort reason) | **Read against the WRITER's keys, not gen's** — gen binds every "get" action's result as `VariableName` while the model stores `OutputVariableName`, so an accessor-based reader silently drops the variable; round-trip tests (not reader-only tests on hand-written BSON) are what catch it. **`WorkflowSelection` has two variants and the object form uses `WorkflowDefinitionVariable`**, not `WorkflowVariable`; its ABSENCE is meaningful (the all-workflows case), so do not synthesise one. **Dispatch the operation on `$Type` before reading fields** — only Abort carries a Reason. **Finishing the reader exposes what could not be seen while nothing rendered**: `call workflow` described to a positional form the grammar rejected (the model never stores the parameter NAME, so DESCRIBE cannot emit the named form), and the abort reason stored the expression *including its quotes* into a StringTemplate Text, so Mendix rendered the quotes at runtime and every round trip doubled them. **Still open**: `lock/unlock workflow all` writes an activity mxbuild rejects with CE1825 — a lock always needs a specific definition, and supplying an empty selection does not satisfy it. Tests `microflow_workflow_read_test.go`, example `mdl-examples/bug-tests/workflow-actions-describe.mdl` | +| `transform $In with Module.Transformer` reports "Created microflow" and the build then fails `[CE0008] "No action defined."` — the activity is in the model with no action inside it | The modelsdk writer (the DEFAULT engine) had no `TransformJsonAction` case, so it fell through to `default: return nil`. The grammar, builder, DESCRIBE formatter and the LEGACY writer all handled it, which is why it looked supported. The reader was missing too, so even a correctly written action described as a placeholder | `mdl/backend/modelsdk/microflow_write.go` (writer case), `mdl/backend/modelsdk/microflow_read_actions.go` (reader cases for `TransformJsonAction`, `CallExternalAction`, `RestOperationCallAction`) | **A missing WRITER case and a missing READER case present identically in a coverage audit** — grepping for reader cases found `transform` "authorable but unreadable" when it was actually unwritable, a strictly worse bug. Check both directions before sizing the work. **Mirror the legacy serializer for keys** (`InputVariableName`/`OutputVariableName`/`Transformation`); for the other two, note `CallExternalAction` stores its result under `VariableName` (not `ResultVariableName`) and REST's two mapping lists are NOT symmetric — the query list keys its name as `QueryParameter`. **Do not reconstruct `CallExternalAction.ResultDataType`**: it is resolved from the consumed service's cached `$metadata` at write time, so reading it back would let a stale value round-trip as if authored. **CE0008 turning into CE1613 is progress, not a new bug** — the action now exists and the build has moved on to validating its reference. Tests `microflow_integration_actions_test.go`, example `mdl-examples/bug-tests/transform-json-write-and-describe.mdl` | +| `mxcli check` reports `[MDL044] … calls 'currentDeviceType()', which is not a Mendix expression function`, `mxcli exec` writes the microflow anyway, and the build then fails `[error] [CE0117] "Error(s) in expression." at Create variable activity` | MDL044 was not in `execEnforcedMicroflowRules`, so it fired only on the `check` path — the #833 shape, one rule at a time. Promoting it was blocked by a false positive of its own: exprcheck's `funcTable` is MDL044's sole allow-list and was missing three REAL built-ins (`isNew`, `isSynced`, `isSyncing`), so a write barrier would have refused valid MDL | `mdl/executor/validate.go` (`execEnforcedMicroflowRules`), `mdl/exprcheck/func_checker.go` (`funcTable`), `mdl/exprcheck/unknown_funcs.go` (`roundingFuncs`) | **Sweep for false positives BEFORE promoting a check-only rule** — a rule that merely reports may be wrong for years without anyone noticing, and promotion converts every one of those into a refusal. Diff the rule's allow-list against the full published function list, then **build each candidate**: `isNew`/`isSynced`/`isSyncing` came back 0 errors (genuine gaps), while `trunc` — which *looked* like a sibling of round/floor/ceil and was even listed in `roundingFuncs` — came back CE0117, so it is correctly flagged. Adding names on resemblance is how a write barrier stops catching anything. **`funcTable` also contains unverified entries** (the `dayOfYear`/`hour`/`minute` extraction family, flagged in its own comment); those are false NEGATIVES and promotion does not worsen them, but do not add more. **Prove the promotion is the cause**: build a control binary with the rule removed from the map, exec the bad script, and confirm mxbuild reports CE0117 — `check` alone rejecting it proves nothing about the exec path. Tests `validate_microflow_rules_exec_test.go`, `validate_microflow_expr_test.go`, examples `mdl-examples/bug-tests/828-unknown-expression-function.fail.mdl` + `828-object-state-functions-ok.mdl`. upstream #828 | +| A datagrid column bound to an association — `column c (attribute: Order_Customer)` — reports success from `mxcli exec` and then fails the build with `[error] [CE1613] "The selected attribute 'Mod.Order.Order_Customer' no longer exists." at Columns (1/1) of data grid 2`. Separately, there is no MDL spelling for the drop-down filter's association mode: `mxcli check` says `[MDL-WIDGET01] has no property \`refEntity\`` | Two unrelated defects behind one report. (1) The reference is **not representable**: `CustomWidgets$WidgetValue.AttributeRef` is typed `AttributeRef`, not the polymorphic `MemberRef`, so the association was qualified like an attribute and written as a dangling `AttributeRef`. (2) `dropdownfilter.def.json` mapped only `attrChoice`/`attributes`/`defaultFilter`, so every `baseType: 'ref'` property was unmapped and dropped | `mdl/executor/cmd_pages_builder_input.go` (`rejectAssociationAsAttribute`, `entityInChain`) wired into `mdl/executor/widget_engine.go` (the objectlist `attribute` case and the `Attribute` source); `sdk/widgets/definitions/dropdownfilter.def.json` (association mode); `mdl/executor/cmd_pages_describe_parse.go` + `_pluggable.go` + `_output.go` (round-trip) | **Establish that a shape is unrepresentable before designing a fix for it** — hand-patch the BSON and run `mx check`. A `DomainModels$AssociationRef` in that slot makes the project **UNLOADABLE** (`ArgumentException: Object of type 'AssociationRef' cannot be converted to type 'AttributeRef'`), and the assembly defining the type (`Mendix.Modeler.WebUI.dll`) has no `AssociationRef` member at all — so the only correct outcome is a refusal carrying both working forms. **`` on an ATTRIBUTE-typed widget property is permission to TRAVERSE a reference, not to bind one** — `attribute: Assoc/Attr` already worked and is what the XML is advertising; the DataGrid column is the only shipped widget where the two are easy to confuse. **A def.json `mode` is the whole feature** for an unauthorable widget mode — the engine already had the `association` operation and the `hasDataSource` condition, so the second half was a data change plus its DESCRIBE reader (without which describe→edit→exec silently reverts the filter to attribute mode). **0 errors from `mx check` does not prove the properties landed** — an unmapped property is silently dropped and the build is just as green; read them back with `mx dump-mpr`. Tests `cmd_pages_builder_assoc_as_attribute_test.go`, `widget_dropdownfilter_assoc_test.go`, example `mdl-examples/bug-tests/830-datagrid-association-filter.mdl`. upstream #830 | +| An association's line anchors — where the connector attaches to the entity boxes in the domain model editor — are absent from `DESCRIBE ASSOCIATION`, and manual adjustments made in Studio Pro do not survive an mxcli round trip | `DomainModels$Association.ParentConnection`/`ChildConnection` (the string `"x;y"`) were **hardcoded** to `"0;50"`/`"100;50"` in BOTH writers and never read by either parser. Because every association write rebuilds the whole element, this was not an omission but active destruction: a documentation-only `alter association … set comment` reset them | `sdk/domainmodel/connection.go` (new: `ParseConnectionPoint`/`FormatConnectionPoint`, `Default*Connection`), `sdk/domainmodel/domainmodel.go` (fields → `*model.Point`), `sdk/mpr/parser_domainmodel.go` + `sdk/mpr/writer_domainmodel.go`, `mdl/backend/modelsdk/domainmodel.go` + `domainmodel_write.go`, `mdl/executor/cmd_associations.go` (`describeConnectionPoints`) | **A feature request that says "X is not exposed" may be hiding "X is destroyed"** — check the write path before scoping the read path. The A/B that settled it: a blank 11.13 app's own `Administration.AccountPasswordData_Account` stores `0;54/100;54`, so a Studio-Pro-authored association is a free fixture for "did mxcli overwrite this?" — no Studio Pro needed. **Learn the value's constraints from the LOADER, not from the shape**: hand-patch and run `mx check` — `"0.5;50"` dies with `StorageLoadException` (integers required) while `"0;500"` and `"-20;50"` load with 0 errors (no range check), so out-of-range values must round-trip untouched. **A zero value is not an absent value** — `{0,0}` is a real anchor (top-left), which forces the field to be a POINTER; a plain `model.Point` cannot distinguish "unset" from "top-left" and would silently rewrite it. **Fix both engines**: they share the semantic model, and a fix in one is invisible to a user on the other. **Emit unauthorable data as a COMMENT** — DESCRIBE output must stay re-executable, and inventing syntax (`@anchor(parent: bottom-left, …)`) would bake in a vocabulary the storage does not have: the pair is CONTINUOUS, not 8 named anchors (observed x values 0 9 11 17 18 47 49 50 65 77 78 84 87 100). **The marketplace is the sample** when you need to know what Studio Pro actually writes: `mxcli marketplace download ` gives real Mendix-authored models, and a module .mpk holds either a raw BSON `project.mpr` or an MPR v1 SQLite one — 88 coordinate pairs from three modules turned "looks like percentages" into a measurement (all 0..100; 85 of 88 pin one coordinate to exactly 0 or 100). **Rule a unit out from the model, not the values**: pixels is impossible because `DomainModels$EntityImpl` stores only `Location` and NO size — the box is sized by the editor from the name and attribute list, so a pixel anchor would have nothing to measure against. Not applicable to `CrossAssociation`, which has no connection properties and crashes Studio Pro if given them (#50). Tests `sdk/domainmodel/connection_test.go`, `mdl/backend/modelsdk/association_connection_test.go`, `sdk/mpr/writer_domainmodel_test.go`, example `mdl-examples/bug-tests/872-association-line-anchors.mdl`. upstream #872 | +| An association's line anchors can be preserved but not AUTHORED — a scripted domain model cannot lay out its own connector lines, so `@Position(x, y)` gets you boxes and nothing gets you the lines between them | Feature gap, not a defect. `DomainModels$Association.ParentConnection`/`ChildConnection` had no MDL surface | `mdl/grammar/domains/MDLDomainModel.g4` (`SET ANCHOR`/`anchorPoint` — the ONLY grammar change), `mdl/visitor/visitor_association.go` (`anchorAnnotation`, `annotationParenPoint`, `anchorCoord`), `mdl/ast/ast_association.go` (`FromAnchor`/`ToAnchor` on both create and alter), `mdl/executor/cmd_associations.go` (`applyAnchors`, `describeConnectionPoints`) | **Look for an existing annotation before inventing one** — `@anchor(from:, to:)` already existed for microflow sequence flows, asking the same question (where does the connector attach), and `annotationParamName` already admitted FROM and TO, and `(x, y)` was already `annotationParenValue`: CREATE needed **zero** grammar. The two forms cannot be confused because the microflow one names its inner params (`(from: right, to: left)`) while a coordinate pair is positional. **Let the storage pick the value type**: the measured pair is continuous (x takes 14 distinct values across 88 samples), so named anchors were never an option — see the preservation row above for how that was established. **Silence must mean "preserve", not "default"** — naming one end sets it and omitting one keeps what is stored, which is what stops a `create or modify association` about the delete behaviour from flattening a hand-tuned line; the AST carries POINTERS so "not mentioned" and "mentioned as (0, 0)" stay distinguishable. **Reject what the LOADER rejects, at check time**: a fractional coordinate must error, not be truncated to 0 — Mendix refuses to open such a project, and a silently-wrong value in a file that still loads is the worse failure. **Prove DESCRIBE round-trips by parsing its own output** — asserting on a string literal passes against a formatter emitting something nothing can read. Tests `mdl/visitor/visitor_association_anchor_test.go`, `mdl/executor/cmd_associations_anchor_test.go`, example `mdl-examples/bug-tests/872-association-line-anchors.mdl`. upstream #872 | diff --git a/.claude/skills/mendix/README.md b/.claude/skills/mendix/README.md index f15f4eee4..54cf5c71d 100644 --- a/.claude/skills/mendix/README.md +++ b/.claude/skills/mendix/README.md @@ -62,6 +62,7 @@ Page-specific patterns: | Skill | Purpose | Use When | |-------|---------|----------| +| [bootstrap-app.md](bootstrap-app.md) | Provision a new Mendix app in an empty repo | Starting from nothing: interview, `mxcli new`, hook + brief, commit, boot | | [generate-domain-model.md](generate-domain-model.md) | Complete domain model generation | Generating full domain models | | [create-custom-widget.md](create-custom-widget.md) | Custom pluggable widget AIGC | Creating custom React widgets from scratch | | [migrate-design-prototype.md](migrate-design-prototype.md) | Turn a Claude Design prototype into a themed Mendix app | Reproducing a design handoff/prototype as an SCSS theme + styled pages | @@ -78,6 +79,7 @@ Load skills based on the task: | User Request | Load These Skills | |--------------|-------------------| +| "Set this empty repo up as a Mendix app" | `bootstrap-app.md` | | "Create entity/domain model" | `mdl-entities.md` | | "Write microflow" | `write-microflows.md`, `cheatsheet-variables.md` | | "Create validation" | `validation-microflows.md`, `patterns-crud.md` | diff --git a/.claude/skills/mendix/alter-page.md b/.claude/skills/mendix/alter-page.md index 3e449f28c..9a9b8dacc 100644 --- a/.claude/skills/mendix/alter-page.md +++ b/.claude/skills/mendix/alter-page.md @@ -72,12 +72,33 @@ set Title = 'New Page Title' set PopupWidth = 800 set PopupHeight = 480 set PopupResizable = true + +-- Retarget a button's on-click action. Any form `create page` accepts works +-- here, including the combined ones. +set Action = microflow Module.ACT_Other on btnSave +set Action = SAVE_CHANGES CLOSE_PAGE on btnSave +set Action = SHOW_PAGE Module.DetailPage on btnEdit + +-- Rebind a data-bound widget +set DataSource = $OrderParam on dvOrder +set DataSource = DATABASE Module.Order on dgOrders ``` +**Prefer `set Action` over `replace` when only the action changes.** `replace` +rebuilds the widget from what the statement says, so any property you do not +restate — `ButtonStyle`, `Class`, design properties, tooltip — is dropped. `set` +edits the one property and leaves the rest of the widget alone. + +`set Action` is refused on a widget that has no action (a plain container, say), +rather than writing a property the widget type does not define — Studio Pro +refuses to open a document with an unknown property while MxBuild tolerates it, +so a silent write would build cleanly and then fail to open. + **Supported SET properties:** | Property | Widget Types | Value Type | Example | |----------|-------------|------------|---------| +| `Action` | Widgets with an on-click action (ACTIONBUTTON, LINKBUTTON, clickable containers) | Any `create page` action expression | `set Action = microflow M.ACT_Go on btnSave` | | `caption` | ACTIONBUTTON, LINKBUTTON | String | `set caption = 'Submit' on btnSave` | | `content` | DYNAMICTEXT | String | `set content = 'New Heading' on txtTitle` | | `label` | TEXTBOX, TEXTAREA, DATEPICKER, COMBOBOX, CHECKBOX, RADIOBUTTONS | String | `set label = 'full Name' on txtName` | diff --git a/.claude/skills/mendix/bootstrap-app.md b/.claude/skills/mendix/bootstrap-app.md new file mode 100644 index 000000000..3f8a5326c --- /dev/null +++ b/.claude/skills/mendix/bootstrap-app.md @@ -0,0 +1,231 @@ +# Bootstrap a Mendix App in an Empty Repo + +## When to Use This Skill + +Use this when a repo has **no Mendix project yet** and you have been asked to +provision one with mxcli — typically from the empty-repo seed prompt, which does +nothing but install mxcli, run `mxcli init --sync-skills`, and send you here. + +Everything the seed prompt used to spell out lives here instead, so the prompt stays +short enough to paste from a phone and this procedure can be fixed by shipping a new +mxcli rather than by re-pasting a longer prompt. + +If an `.mpr` already exists, this is the wrong skill: run `mxcli init` in the app +folder and go straight to the work. + +Related skills: `run-local.md` (the warm dev loop this ends in), `mdl-entities.md` and +`create-page.md` (building the model you propose at the end), +`migrate-design-prototype.md` (when a design was handed to you). + +--- + +## Step 0 — interview, and WAIT for the answers + +Ask **all** of these in ONE message, numbered, each with the default you would pick, +so the user can reply "defaults" or answer only what they care about. **Do not +provision anything until they reply.** + +The interview comes first for a reason: the app name becomes the `.mpr` file name, the +Studio Pro app name and the path baked into the SessionStart hook, so it is far +cheaper to ask than to rename afterwards. The rest of the answers are the brief — they +get written into the repo, so the session that resumes after an idle reap knows what +it is building. + +1. **One app, or a solution of several?** One Mendix app is the default. "Solution" + means several apps in one repo — e.g. a backend that owns the data and publishes + OData/REST, and a frontend that consumes it. If so, ask for each app's name and one + line on what it owns, and follow "If this is a solution" below. +2. **App name.** Becomes the `.mpr` file name, the app name in Studio Pro, and the + path in the session hook, so it is awkward to change later. One PascalCase word, + letters and digits only — `OrderPortal`, `FieldService`, `ClubAdmin`. Propose one + derived from the answer to Q3. +3. **What is the app for?** One or two sentences: who uses it, and what it lets them + do. If the answer is vague ("a tool for work"), ask one follow-up — everything + below is derived from this. +4. **What does it keep track of?** Three to six nouns that will become entities, and a + word on how they relate (e.g. "a Job has many Visits; each Visit has Photos"). For + a solution, also ask which app owns each noun. +5. **Who logs in?** The user roles, and roughly what each may do (e.g. "Requester + creates and sees their own; Approver sees everything and approves"). +6. **Look and feel.** One of the bundled themes: `signal` (light, high contrast), + `ledger` (light, dense, data-heavy), `console` (dark), or `none` for stock Atlas. + Default `signal`. +7. **Mendix version.** Default `11.13.0`. + +If the user says "defaults" or ignores a question, choose something sensible for it, +say what you chose in one line, and keep going — **do not block on them twice**. + +### Checking the Mendix version default + +Everything mxcli does starts with downloading MxBuild, so "supported" means "on the +CDN". If asked for a version newer than the default, verify both tarballs answer +`200` before using it — `run --local` needs the runtime as well as MxBuild: + +```bash +curl -sI -o /dev/null -w '%{http_code}\n' https://cdn.mendix.com/runtime/mxbuild-11.13.0.tar.gz +curl -sI -o /dev/null -w '%{http_code}\n' https://cdn.mendix.com/runtime/mendix-11.13.0.tar.gz +``` + +In a solution, give every app the **same** version: they share the `~/.mxcli/mxbuild` +cache, and a mismatch means a second multi-hundred-MB download and two runtimes to +keep straight. + +--- + +## Provision + +Substitute the answers for ``, `` and `` throughout. For a +solution, do steps 1–3 once per app and read "If this is a solution" first. The +commands below say `./mxcli` because that is where the seed prompt puts the binary — +drop the `./` if it came pre-installed on `PATH`. + +1. **Create the app at the repo root** — that is where `.claude/` and the `./mxcli` + binary have to live for future sessions to self-bootstrap. `mxcli new` refuses to + write into a directory that is not empty, and a git repo always has `.git`, so + create it in a subfolder and move it up: + + ```bash + ./mxcli new --version --theme + rm -f /mxcli # a hardlink to the ./mxcli you just ran; mv would + # refuse it as "the same file" + shopt -s dotglob && mv /* . && rmdir + ``` + + `mxcli new` also runs `mxcli init`, which writes `.claude/settings.json` with a + SessionStart hook plus the `.claude/bootstrap-mxcli.sh` it runs — **check that the + `.mpr` named in the script is right after the move.** +2. **Confirm the Claude tooling:** `./mxcli init --tool claude`. Idempotent — it is + what step 1 already ran, and re-running it is the cheapest way to be sure the hook, + skills and commands are in place. +3. **Bring prerequisites up:** `./mxcli run --local --setup --ensure-db -p .mpr` + (caches MxBuild + runtime, starts Postgres, creates the app database). +4. **Write the brief to `README.md`** at the repo root: the app name(s), the answers + to Q3–Q5 **in the user's words**, and the theme and Mendix version used. For a + solution, say which app owns what and how they talk to each other. This is what + tells the next session — after an idle reap, with none of this conversation — what + it is building. Keep it short enough that it stays true. +5. **Start a `FINDINGS.md`** at the repo root and keep appending to it as you work. + Log anything surprising or broken: an mxcli command that errored, a workaround you + applied, a `mxcli check` that passed but a real `mx check` later flagged. Note the + Mendix + mxcli versions and how each finding was verified. This is durable context + for the next session, and the most useful thing to share back to improve mxcli. +6. **COMMIT everything now** — `.mpr`, `.devcontainer/`, `.claude/` (the + SessionStart hook **and** `.claude/bootstrap-mxcli.sh`), `README.md` and + `FINDINGS.md`. This step is mandatory, not housekeeping: the seed prompt is a + *one-time* seed, and committing its output is what makes every later session + bootstrap from files instead of from a re-paste. The `mxcli` binary itself stays + git-ignored (~85 MB); the bootstrap script is what fetches it back into a fresh + clone, so committing the script is what makes the hook survive a reap. +7. **Boot and verify:** `./mxcli run --local -p .mpr` in the background, then + confirm the app answers HTTP 200 at http://localhost:8080/ and report. +8. **(Optional) browser preview from a cloud session:** + `./mxcli run --hub https://hub.mxcli.org -p .mpr`, and report the preview + URL it prints. Needs `MXCLI_HUB_KEY` on the environment; without it, continue as a + normal local run. + +--- + +## If this is a solution (several apps in one repo) + +Each app is a full Mendix project — one `.mpr`, one runtime, one database. Same steps, +with these deltas: + +- **Layout.** One subfolder per app, nothing at the repo root but `README.md`, + `FINDINGS.md` and `.claude/`. Run `mxcli new --version --theme + ` once per app and leave each where it lands; do not move anything up. +- **Ports.** Every app defaults to 8080/8090/6543 and they will collide. Give the + first app the defaults and the second `--app-port 8180 --admin-port 8190 + --serve-port 6643`. Avoid 8081/8091/6544 — `mxcli test --local` uses those. +- **Give each app its own hostname**, not just its own port. Cookies are keyed on + host name and **ignore the port**, so two apps on `localhost:8080` and + `localhost:8180` share one cookie jar: logging into one can silently replace the + other's `XASSESSIONID`. Two hostnames give two jars, and the differing ports do no + harm. Add them to `/etc/hosts` — + + ``` + 127.0.0.1 backend.local frontend.local + ``` + + — and browse `http://backend.local:8080/` and `http://frontend.local:8180/`. The + runtime binds `127.0.0.1` and serves any `Host` you send it, and the client uses + relative URLs, so it works under any name that resolves to loopback. (`*.nip.io` + works too if you would rather not touch `/etc/hosts`; prefer `/etc/hosts` in a + locked-down container, where public wildcard DNS may not resolve — `localtest.me` + resolves to `::1` in some of them.) + + Then record the name in each app's own configuration, so the runtime knows the URL + it is reached at and generates absolute URLs — OIDC/SAML redirect URIs, deep links — + against the host name rather than the listen address: + + ```sql + alter settings configuration 'Default' + ApplicationRootUrl = 'http://backend.local:8080/'; + ``` + + `run --local` picks that up at boot and prints which configuration it came from. + A blank app ships `http://localhost:8080/` there, and that stock loopback value is + deliberately ignored — otherwise every project would start advertising a URL, and + the wrong port under `--app-port`. Only a real host name is passed through. +- **Databases** need no action: the name is derived from the `.mpr` file name, so + differently-named apps get different databases. +- **The session hook.** `mxcli init` writes `.claude/settings.json` inside each app + folder, but Claude Code reads the one at the **repo root** — and it will not add a + second entry for you (it dedupes on the command, not on the project). Write the root + one yourself, one line per app, e.g. + `test -x backend/mxcli && (cd backend && ./mxcli run --local --setup --ensure-db -p Backend.mpr) || true`. + Verify it by checking that a fresh shell can boot each app. +- **Previews.** Pass `--hub-solution ` to every `run --hub` so the apps + appear grouped in the hub overview instead of as unrelated previews. + +**Wire the integration in dependency order — the producer must be running first.** +`CREATE ODATA CLIENT` fetches the `$metadata` at the moment you create it and caches +it in the model; if the URL is unreachable it warns and leaves the client unvalidated, +with no external entities to import. So: publish on the producer +(`CREATE ODATA SERVICE … publish entity …`), boot it (`run --local`), and only then, +on the consumer, `CREATE ODATA CLIENT … MetadataUrl: 'http://backend.local:8080/odata/…/$metadata'` +followed by `CREATE EXTERNAL ENTITIES FROM …`. Use the hostname here too, so the +cached contract and the constant below agree with what the browser sees. Point +`ServiceUrl` at a **constant** (`ServiceUrl: @Module.SvcUrl`) so the address can be +changed per environment without touching the model — it will not stay `localhost`. +`mxcli syntax odata.publish` and `mxcli syntax odata.consume` have the full syntax; +business events (`mxcli syntax business-events`) are the alternative when the link +should be asynchronous. + +--- + +## Then propose the model — do not build it yet + +The blank template ships a `MyFirstModule`; the app's own work belongs in a module +named after it. From the brief, propose in chat: + +- a module name, and the entities from Q4 with their attributes and associations +- the user roles from Q5 and what each may read/write +- the handful of pages that make it usable +- for a solution: which app owns each entity, and what crosses the boundary — publish + only what the other app actually needs + +Show it as **MDL the user can read**, and wait for their go-ahead before executing it. +If a design was handed to you, it is the source of truth for the model and the pages — +see `migrate-design-prototype.md`. + +--- + +## After bootstrap — the inner loop + +```bash +./mxcli run --local -p .mpr --watch --screenshot # warm dev loop + screenshots +./mxcli exec change.mdl -p .mpr # edit the model; the loop hot-applies +``` + +In a solution, run one loop per app from its own folder, with the second app on the +alternate ports, and start the producer first so the consumer's external entities +resolve: + +```bash +(cd backend && ./mxcli run --local -p Backend.mpr --watch) +(cd frontend && ./mxcli run --local -p Frontend.mpr --watch \ + --app-port 8180 --admin-port 8190 --serve-port 6643) +``` + +See `run-local.md` for the warm loop, `--watch`, `--ensure-db`, and the screenshot +flags. diff --git a/.claude/skills/mendix/create-page.md b/.claude/skills/mendix/create-page.md index 16c1c43c4..651e59b41 100644 --- a/.claude/skills/mendix/create-page.md +++ b/.claude/skills/mendix/create-page.md @@ -277,6 +277,21 @@ Create a button with action binding: actionbutton widgetName (caption: 'Caption', action: ACTION_TYPE [, buttonstyle: style] [, icon: 'Module.IconCollection.IconName']) ``` +`icon:` names an icon inside an icon collection — `Module.Collection.IconName`, +e.g. `'Atlas_Core.Atlas_Filled.pencil'`. Browse what a project has with +`show icon collection` and `describe icon collection Atlas_Core.Atlas_Filled`. + +A wrong icon name is a **build error** (CE1613, *"The selected custom icon … no +longer exists"*), so check it before building: + +```bash +mxcli check script.mdl -p app.mpr --references +``` + +That resolves every icon reference against the project's collections and +suggests near matches for a typo. It needs `-p` — the collections are documents +in the project, so a plain `mxcli check` cannot see them. + Use `linkbutton` instead of `actionbutton` for a button rendered as a link (same properties). Both accept an `icon:` — an **icon-collection** reference, e.g. `icon: 'Atlas_Core.Atlas_Filled.pencil'` (the modern Atlas icon set). The name @@ -715,6 +730,26 @@ datefilter datefilter (attributes: [Module.Entity.CreateDate]) dropdownfilter statusFilter (attributes: [Module.Entity.Status]) ``` +Filter by an **association** instead of an attribute — the options are the +associated objects. Giving the filter a `datasource:` (the OPTION list) selects +this mode; all three parts are required: + +```sql +column colCustomer (attribute: Order_Customer/Name, caption: 'Customer') { + dropdownfilter ddfCustomer ( + Association: Sales.Order_Customer, -- the reference on the GRID entity + datasource: database Sales.Customer, -- the option list (associated entity) + CaptionAttribute: Name -- what each option shows + ) +} +``` + +> **A column cannot bind the association itself.** `column c (attribute: Order_Customer)` +> is refused — Mendix has nowhere to store a reference in an attribute-typed widget +> property, and writing one anyway fails the build with CE1613 *"The selected attribute +> … no longer exists"*. To **show** a value from the associated object, traverse the +> reference (`attribute: Order_Customer/Name`); to **filter** by it, use the mode above. + ### NAVIGATIONLIST Widget Create a menu with action items: diff --git a/.claude/skills/mendix/generate-domain-model.md b/.claude/skills/mendix/generate-domain-model.md index 633d04961..b2907492b 100644 --- a/.claude/skills/mendix/generate-domain-model.md +++ b/.claude/skills/mendix/generate-domain-model.md @@ -113,6 +113,34 @@ The `@position(x, y)` annotation specifies where the entity appears in the domai - Organize related entities in logical groups - Example layout: Categories at y=100, Transactions at y=300, Reports at y=500 +**Association line anchors** — where the connector attaches to each entity box — +are set with `@anchor`, as a **percentage of the box** (0..100, whole numbers): + +```sql +@anchor(from: (0, 54), to: (100, 54)) +create association Sales.Order_Customer + from Sales.Order to Sales.Customer; +``` + +`from` is the anchor on the FROM entity's box, `to` the anchor on the TO +entity's. `(0, 50)` is the middle of the left edge, `(100, 50)` the middle of the +right, `(50, 100)` the bottom centre. + +Retune a line without restating the association: + +```sql +alter association Sales.Order_Customer set anchor from (50, 100) to (50, 0); +``` + +**Naming an end sets it; not naming one preserves what is stored.** An +association written without `@anchor` keeps whatever the line was dragged to in +Studio Pro, so a `create or modify association` about the delete behaviour never +flattens someone's layout. `describe association` re-emits a non-default pair as +the same `@anchor(...)` annotation, so describe → edit → exec round-trips. + +Cross-module associations have no anchors at all — Mendix stores none, and +`set anchor` on one is refused. + #### Persistent Entity ```sql diff --git a/.claude/skills/mendix/write-microflows.md b/.claude/skills/mendix/write-microflows.md index 326486134..ef50abce4 100644 --- a/.claude/skills/mendix/write-microflows.md +++ b/.claude/skills/mendix/write-microflows.md @@ -949,6 +949,14 @@ toString($value) -- Convert to string > `mxcli check` now flags an unknown expression function like `randomInt` as > **MDL044** (with a "did you mean random()?" hint), and a Decimal assigned to an > integer target as **MDL041** — before the build does. +> +> **MDL044 also blocks `mxcli exec`**, not just `check`: a call to a name Mendix +> has no built-in for is CE0117 at build time, so exec refuses to write the +> microflow rather than leaving you to find out from mxbuild. Two names that +> look plausible and are not real: `currentDeviceType()` and `trunc()` (use +> `round`/`floor`/`ceil`). If exec rejects a function you believe IS a Mendix +> built-in, build it once and — if mxbuild accepts it — add it to `funcTable` in +> `mdl/exprcheck/func_checker.go`; that table is the rule's only allow-list. ## Complete Example diff --git a/cmd/mxcli/syntax/example_parses_test.go b/cmd/mxcli/syntax/example_parses_test.go new file mode 100644 index 000000000..416658904 --- /dev/null +++ b/cmd/mxcli/syntax/example_parses_test.go @@ -0,0 +1,217 @@ +// SPDX-License-Identifier: Apache-2.0 + +package syntax_test + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/cmd/mxcli/syntax" + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// TestExamplesParse is the anti-drift guard for `mxcli syntax`. +// +// The registry is hand-maintained while the grammar moves underneath it, and +// nothing tied the two together: the other tests here check structure (fields +// populated, aliases resolve, see-also targets exist) rather than whether the +// documented MDL is real. So entries went stale silently — and the registry is +// the first place an agent looks, so a gap here becomes a wrong workaround in a +// generated app rather than a puzzled user. +// +// Only Example is checked. Syntax fields carry metasyntax (`GET|POST`, +// `[OR MODIFY]`, ``) and are not meant to parse. +// +// Examples come in several legitimate shapes, so each block is tried in turn as +// a whole statement, a microflow activity, a page widget, a workflow activity, +// and a retrieve clause. A failure means the block parses as none of them — not +// that it failed the first attempt. +func TestExamplesParse(t *testing.T) { + for _, f := range syntax.All() { + f := f + t.Run(f.Path, func(t *testing.T) { + // Examples routinely show several independent snippets separated by + // blank lines — a statement, then a variant, then a related clause. + // Checking each block on its own keeps the docs readable while still + // holding every snippet to "this really parses". + blocks := mdlBlocks(f.Example) + // TestFeatureFieldsPopulated already rejects an empty Example, but an + // example made entirely of comments would yield no blocks and quietly + // opt out of this guard. Keep that a failure everywhere except the + // handful of prose topics below, so the opt-out cannot spread by + // accident. + if len(blocks) == 0 { + if nonStatementTopics[f.Path] { + t.Skipf("example is deliberately not MDL statements — see nonStatementTopics") + } + t.Fatalf("Example contains no MDL to check:\n%s", f.Example) + } + for _, block := range blocks { + if ctx, ok := parsesInSomeContext(block); ok { + t.Logf("block parses as %s", ctx) + continue + } + _, errs := visitor.Build(block) + var first any = "unknown" + if len(errs) > 0 { + first = errs[0] + } + t.Errorf("example block parses as no known construct "+ + "(statement / microflow activity / page widget / workflow activity).\n"+ + "top-level error: %v\n--- block ---\n%s", first, block) + } + }) + } +} + +// nonStatementTopics are the entries whose Example is legitimately not made of +// MDL statements — the troubleshooting topics pair an error message with its fix +// as prose, and `oql` documents a CLI invocation whose quoted argument is OQL, +// not MDL. +// +// Keep this list short and justified: an entry added here stops being checked at +// all, which is the one way a stale example could still slip through. +var nonStatementTopics = map[string]bool{ + "errors": true, // error/fix prose + "errors.execution": true, // error/fix prose + "errors.reference": true, // error/fix prose + "errors.syntax": true, // error/fix prose + "oql": true, // `mxcli oql ""` — a shell command, and OQL is not MDL +} + +// mdlBlocks splits an Example into blank-line-separated blocks, dropping shell +// commands, comment-only blocks, and leading prose. +func mdlBlocks(example string) []string { + var blocks []string + for _, raw := range strings.Split(stripNonMDL(example), "\n\n") { + block := strings.TrimSpace(stripNonMDL(raw)) + if block == "" { + continue + } + blocks = append(blocks, block) + } + return blocks +} + +// parsesInSomeContext reports the first context the example parses in. +func parsesInSomeContext(example string) (string, bool) { + for _, c := range []struct { + name string + wrap func(string) string + }{ + {"statement", func(s string) string { return s }}, + {"microflow activity", func(s string) string { + return "CREATE MICROFLOW SyntaxDoc.Probe ()\nBEGIN\n" + s + "\nEND;" + }}, + {"page widget", func(s string) string { + return "CREATE PAGE SyntaxDoc.Probe (Title: 'Probe', Layout: 'Atlas_Core.Atlas_Default') {\n" + s + "\n};" + }}, + {"workflow activity", func(s string) string { + return "CREATE WORKFLOW SyntaxDoc.Probe PARAMETER $Ctx: SyntaxDoc.Ctx\nBEGIN\n" + s + "\nEND WORKFLOW;" + }}, + // XPath topics illustrate a constraint on its own (`WHERE [...]`), which + // is a clause rather than a statement. Hanging it off a RETRIEVE checks + // the constraint itself without forcing the docs to repeat a full + // statement around every example. + {"retrieve clause", func(s string) string { + return "CREATE MICROFLOW SyntaxDoc.Probe ()\nBEGIN\n" + + "RETRIEVE $Probe FROM SyntaxDoc.Entity\n" + s + ";\nEND;" + }}, + } { + if _, errs := visitor.Build(c.wrap(example)); len(errs) == 0 { + return c.name, true + } + } + // A block may list several alternative clauses one per line (the xpath + // function reference does this). Accept it when every line is a valid clause + // on its own — the block is a menu, not one statement. + if lines := nonEmptyLines(example); len(lines) > 1 { + all := true + for _, line := range lines { + if _, ok := parsesInSomeContext(line); !ok { + all = false + break + } + } + if all { + return "per-line clauses", true + } + } + return "", false +} + +// stripTestAnnotations removes `/** … @test … */` blocks. +// +// DOC_COMMENT is a real token in MDL, not a hidden one, and the grammar admits +// it only ahead of a handful of declarations. The `@test`/`@expect` annotations +// in a .test.mdl file sit ahead of ordinary microflow statements instead, and +// are consumed by the test runner's own front end before the MDL is parsed. +// Removing them here keeps the statements underneath under test rather than +// exempting the whole entry. +func stripTestAnnotations(s string) string { + for { + start := strings.Index(s, "/**") + if start < 0 { + break + } + end := strings.Index(s[start:], "*/") + if end < 0 { + break + } + end += start + len("*/") + if !strings.Contains(s[start:end], "@test") && !strings.Contains(s[start:end], "@expect") { + break + } + s = s[:start] + s[end:] + } + return s +} + +// nonEmptyLines returns the block's lines with blanks and comments dropped. +func nonEmptyLines(s string) []string { + var out []string + for _, l := range strings.Split(s, "\n") { + t := strings.TrimSpace(l) + if t == "" || strings.HasPrefix(t, "--") { + continue + } + out = append(out, t) + } + return out +} + +// stripNonMDL removes leading prose and any shell-command lines. Some entries +// deliberately show the `mxcli …` equivalent alongside the MDL; those lines are +// documentation, not statements to validate. +func stripNonMDL(s string) string { + s = stripTestAnnotations(s) + var kept []string + for _, line := range strings.Split(s, "\n") { + t := strings.TrimSpace(line) + if strings.HasPrefix(t, "mxcli ") { + continue + } + // A lone `/` is MDL's statement separator (used between blocks in + // .test.mdl files). It carries no syntax to validate and would otherwise + // land inside a wrapper's body. + if t == "/" { + continue + } + kept = append(kept, line) + } + out := strings.Join(kept, "\n") + + // Drop a leading run of comment/blank lines so a prose preamble does not + // decide the parse context. + lines := strings.Split(out, "\n") + i := 0 + for i < len(lines) { + t := strings.TrimSpace(lines[i]) + if t == "" || strings.HasPrefix(t, "--") { + i++ + continue + } + break + } + return strings.Join(lines[i:], "\n") +} diff --git a/cmd/mxcli/syntax/features_domain_model.go b/cmd/mxcli/syntax/features_domain_model.go index e3e0ad134..ce3eaeca3 100644 --- a/cmd/mxcli/syntax/features_domain_model.go +++ b/cmd/mxcli/syntax/features_domain_model.go @@ -92,9 +92,9 @@ func init() { "many-to-one", "many-to-many", "foreign key", "owner", "delete behavior", }, - Syntax: "CREATE [OR MODIFY] ASSOCIATION Module.Name\n FROM Module.FromEntity TO Module.ToEntity\n TYPE Reference|ReferenceSet\n [OWNER Default|Both]\n [DELETE_BEHAVIOR behavior]\n [COMMENT 'text'];\nDROP ASSOCIATION Module.Name;\n\nOR MODIFY: updates type/owner/delete behavior in-place, preserves UUID.", - Example: "-- Many-to-one\nCREATE ASSOCIATION Shop.Order_Customer\n FROM Shop.Order TO Shop.Customer\n TYPE Reference\n OWNER Default\n DELETE_BEHAVIOR DELETE_BUT_KEEP_REFERENCES;\n\n-- Many-to-many\nCREATE ASSOCIATION Shop.Product_Tag\n FROM Shop.Product TO Shop.Tag\n TYPE ReferenceSet\n OWNER Both;", - SeeAlso: []string{"domain-model.association.create", "domain-model.association.delete-behavior"}, + Syntax: "[@anchor(from: (x, y), to: (x, y))]\nCREATE [OR MODIFY] ASSOCIATION Module.Name\n FROM Module.FromEntity TO Module.ToEntity\n TYPE Reference|ReferenceSet\n [OWNER Default|Both]\n [DELETE_BEHAVIOR behavior]\n [COMMENT 'text'];\nALTER ASSOCIATION Module.Name SET ANCHOR FROM (x, y) TO (x, y);\nDROP ASSOCIATION Module.Name;\n\nOR MODIFY: updates type/owner/delete behavior in-place, preserves UUID.\n\n@anchor sets the LINE ANCHORS — where the connector attaches to each entity box\nin the domain model editor — as a PERCENTAGE of the box (0..100, whole numbers).\n`from` is the FROM entity's box, `to` the TO entity's: (0, 50) is the middle of\nthe left edge, (100, 50) the right, (50, 100) the bottom centre. Omitting an end\nPRESERVES what is stored, so a CREATE OR MODIFY about something else never\nflattens a hand-tuned line. Cross-module associations have no anchors.", + Example: "-- Many-to-one\nCREATE ASSOCIATION Shop.Order_Customer\n FROM Shop.Order TO Shop.Customer\n TYPE Reference\n OWNER Default\n DELETE_BEHAVIOR DELETE_BUT_KEEP_REFERENCES;\n\n-- Many-to-many\nCREATE ASSOCIATION Shop.Product_Tag\n FROM Shop.Product TO Shop.Tag\n TYPE ReferenceSet\n OWNER Both;\n\n-- Line leaving the bottom of Order and entering the top of Customer\n@anchor(from: (50, 100), to: (50, 0))\nCREATE ASSOCIATION Shop.Order_Customer\n FROM Shop.Order TO Shop.Customer;\n\n-- Retune the line without restating the association\nALTER ASSOCIATION Shop.Order_Customer SET ANCHOR FROM (0, 54) TO (100, 54);", + SeeAlso: []string{"domain-model.association.create", "domain-model.association.anchor", "domain-model.association.delete-behavior"}, }) Register(SyntaxFeature{ @@ -110,6 +110,32 @@ func init() { SeeAlso: []string{"domain-model.association.delete-behavior", "domain-model.entity.create"}, }) + Register(SyntaxFeature{ + Path: "domain-model.association.anchor", + Summary: "Line anchors: where an association's connector attaches to each entity box", + Keywords: []string{ + "anchor", "line anchor", "connection point", "connector", + "diagram layout", "domain model layout", "set anchor", + }, + Syntax: "@anchor(from: (x, y), to: (x, y))\nCREATE ASSOCIATION Module.Name FROM Module.From TO Module.To;\n\nALTER ASSOCIATION Module.Name SET ANCHOR FROM (x, y) TO (x, y);\n\n" + + "x and y are a PERCENTAGE of the entity box, 0..100, whole numbers:\n" + + " (0, 50) middle of the LEFT edge\n" + + " (100, 50) middle of the RIGHT edge\n" + + " (50, 0) TOP centre\n" + + " (50, 100) BOTTOM centre\n\n" + + "`from` is the anchor on the FROM entity's box, `to` on the TO entity's.\n" + + "Any point on the box is valid — the pair is continuous, not four sides.\n\n" + + "Omitting an end (or the whole annotation) PRESERVES what is stored, so a\n" + + "CREATE OR MODIFY about the delete behaviour never flattens a hand-tuned\n" + + "line. DESCRIBE ASSOCIATION re-emits a non-default pair as the same\n" + + "@anchor(...), so describe -> edit -> exec round-trips.\n\n" + + "A fractional coordinate is refused: Mendix stores two integers and its\n" + + "loader will not OPEN a project whose anchor is fractional.\n" + + "Cross-module associations have no anchors — Mendix stores none for them.", + Example: "-- Line leaving the bottom of Order, entering the top of Customer\n@anchor(from: (50, 100), to: (50, 0))\nCREATE ASSOCIATION Shop.Order_Customer\n FROM Shop.Order TO Shop.Customer\n TYPE Reference;\n\n-- Retune the line without restating the association\nALTER ASSOCIATION Shop.Order_Customer SET ANCHOR FROM (0, 54) TO (100, 54);\n\n-- Says nothing about anchors: whatever the line was dragged to survives\nCREATE OR MODIFY ASSOCIATION Shop.Order_Customer\n FROM Shop.Order TO Shop.Customer\n DELETE_BEHAVIOR CASCADE;", + SeeAlso: []string{"domain-model.association.create", "domain-model.entity"}, + }) + Register(SyntaxFeature{ Path: "domain-model.association.delete-behavior", Summary: "Delete behavior options for associations", diff --git a/cmd/mxcli/syntax/features_integration.go b/cmd/mxcli/syntax/features_integration.go index 54832775a..45149caba 100644 --- a/cmd/mxcli/syntax/features_integration.go +++ b/cmd/mxcli/syntax/features_integration.go @@ -409,8 +409,8 @@ func init() { "image collection", "create image collection", "drop image collection", "export level", "image", "icon", "logo", }, - Syntax: "SHOW IMAGE COLLECTION [IN Module];\nDESCRIBE IMAGE COLLECTION Module.Name;\nCREATE IMAGE COLLECTION Module.Name\n [EXPORT LEVEL 'Hidden'|'Public']\n [COMMENT 'text']\n [(IMAGE 'name' FROM FILE 'path', ...)];\nCREATE OR MODIFY IMAGE COLLECTION Module.Name [...];\nDROP IMAGE COLLECTION Module.Name;", - Example: "CREATE OR MODIFY IMAGE COLLECTION MyModule.AppIcons\n EXPORT LEVEL 'Public'\n COMMENT 'Application icons' (\n IMAGE 'logo' FROM FILE 'assets/logo.png',\n IMAGE 'favicon' FROM FILE 'assets/favicon.ico'\n);\n\nDESCRIBE IMAGE COLLECTION MyModule.AppIcons;", + Syntax: "SHOW IMAGE COLLECTION [IN Module];\nDESCRIBE IMAGE COLLECTION Module.Name;\nCREATE IMAGE COLLECTION Module.Name\n [EXPORT LEVEL 'Hidden'|'Public']\n [COMMENT 'text']\n [(IMAGE name FROM FILE 'path', ...)];\nCREATE OR MODIFY IMAGE COLLECTION Module.Name [...];\nDROP IMAGE COLLECTION Module.Name;", + Example: "CREATE OR MODIFY IMAGE COLLECTION MyModule.AppIcons\n EXPORT LEVEL 'Public'\n COMMENT 'Application icons' (\n IMAGE logo FROM FILE 'assets/logo.png',\n IMAGE \"favicon\" FROM FILE 'assets/favicon.ico'\n);\n\nDESCRIBE IMAGE COLLECTION MyModule.AppIcons;", SeeAlso: []string{"integration", "icon-collection"}, }) diff --git a/cmd/mxcli/syntax/features_microflow.go b/cmd/mxcli/syntax/features_microflow.go index 65aed9f6b..c151f94a7 100644 --- a/cmd/mxcli/syntax/features_microflow.go +++ b/cmd/mxcli/syntax/features_microflow.go @@ -10,9 +10,9 @@ func init() { "microflow", "nanoflow", "logic", "automation", "action", "activity", "flow", }, - Syntax: "CREATE MICROFLOW Module.Name ($Param: Type) RETURNS Type AS $Result\nBEGIN\n \nEND;", - Example: "CREATE MICROFLOW MyModule.ACT_CreateOrder ($Code: String)\nRETURNS MyModule.Order AS $NewOrder\nBEGIN\n $NewOrder = CREATE MyModule.Order (OrderNumber = $Code);\n COMMIT $NewOrder;\n RETURN $NewOrder;\nEND;", - SeeAlso: []string{"microflow.create", "microflow.variables", "microflow.control-flow"}, + Syntax: "CREATE [OR REPLACE | OR MODIFY] MICROFLOW Module.Name ($Param: Type) RETURNS Type AS $Result\nBEGIN\n \nEND;", + Example: "CREATE MICROFLOW MyModule.ACT_CreateOrder ($Code: String)\nRETURNS MyModule.Order AS $NewOrder\nBEGIN\n $NewOrder = CREATE MyModule.Order (OrderNumber = $Code);\n COMMIT $NewOrder;\n RETURN $NewOrder;\nEND;\n\n-- Re-runnable: replaces the microflow if it already exists\nCREATE OR REPLACE MICROFLOW MyModule.ACT_CreateOrder ($Code: String)\nRETURNS MyModule.Order AS $NewOrder\nBEGIN\n $NewOrder = CREATE MyModule.Order (OrderNumber = $Code);\n RETURN $NewOrder;\nEND;", + SeeAlso: []string{"microflow.create", "microflow.variables", "microflow.control-flow", "create-modifiers"}, }) Register(SyntaxFeature{ @@ -62,8 +62,10 @@ func init() { "retrieve", "query", "database", "where", "sort", "limit", "offset", "find", "fetch", }, - Syntax: "RETRIEVE $Var FROM Module.Entity\n [WHERE condition]\n [SORT BY attr ASC|DESC]\n [LIMIT n] [OFFSET n];", - Example: "RETRIEVE $Customer FROM MyModule.Customer\n WHERE Code = $CustomerCode\n LIMIT 1;\n\nRETRIEVE $Orders FROM MyModule.Order\n WHERE Status = 'Pending'\n SORT BY CreateDate DESC\n LIMIT 10 OFFSET 0;", + // Retrieve-by-association was missing here, so it read as unsupported + // even though it works and the write-microflows skill documents it. + Syntax: "-- From the database\nRETRIEVE $Var FROM Module.Entity\n [WHERE condition]\n [SORT BY attr ASC|DESC]\n [LIMIT n] [OFFSET n];\n\n-- Over an association, from an object you already have\nRETRIEVE $Var FROM $Object/Module.Association;", + Example: "RETRIEVE $Customer FROM MyModule.Customer\n WHERE Code = $CustomerCode\n LIMIT 1;\n\nRETRIEVE $Orders FROM MyModule.Order\n WHERE Status = 'Pending'\n SORT BY CreateDate DESC\n LIMIT 10 OFFSET 0;\n\n-- Follow an association rather than querying the database\nRETRIEVE $Orders FROM $Customer/MyModule.Order_Customer;\nRETRIEVE $Customer FROM $Order/MyModule.Order_Customer;", SeeAlso: []string{"microflow.object-operations", "xpath"}, }) diff --git a/cmd/mxcli/syntax/features_misc.go b/cmd/mxcli/syntax/features_misc.go index 856578409..70371f13b 100644 --- a/cmd/mxcli/syntax/features_misc.go +++ b/cmd/mxcli/syntax/features_misc.go @@ -3,6 +3,32 @@ package syntax func init() { + // ── CREATE modifiers ──────────────────────────────────────────────── + + // OR MODIFY / OR REPLACE sit on the top-level createStatement rule, so they + // apply to every CREATE uniformly. Individual entries documented them + // unevenly — most said nothing, none mentioned OR REPLACE — which read as + // "not supported here". Documented once, and referenced from the entries + // where the question comes up, rather than repeated across all 27. + Register(SyntaxFeature{ + Path: "create-modifiers", + Summary: "OR REPLACE / OR MODIFY — re-running a CREATE without dropping first", + Keywords: []string{ + "or replace", "or modify", "create or replace", "create or modify", + "idempotent", "upsert", "re-run", "already exists", "overwrite", + }, + Syntax: "CREATE [OR REPLACE | OR MODIFY] ;\n\n" + + "-- Applies to every CREATE statement — entity, microflow, page, workflow,\n" + + "-- REST client, security role, and the rest. Without a modifier, creating\n" + + "-- something that already exists is an error.\n" + + "-- OR REPLACE discard the existing document and write a fresh one\n" + + "-- OR MODIFY update the existing document in place\n" + + "-- Both reuse the existing element's ID, so references from other\n" + + "-- documents survive.", + Example: "CREATE OR REPLACE MICROFLOW MyModule.ACT_Recalculate ()\nBEGIN\n RETURN;\nEND;\n\nCREATE OR MODIFY PERSISTENT ENTITY MyModule.Customer (\n Name: String(200)\n);", + SeeAlso: []string{"microflow", "domain-model.entity", "page"}, + }) + // ── Connection ────────────────────────────────────────────────────── Register(SyntaxFeature{ diff --git a/cmd/mxcli/syntax/features_page.go b/cmd/mxcli/syntax/features_page.go index 197d1f503..31641fb42 100644 --- a/cmd/mxcli/syntax/features_page.go +++ b/cmd/mxcli/syntax/features_page.go @@ -52,7 +52,7 @@ func init() { "selection", "variable", "binding", "binds", "association", "data from context", }, Syntax: "DataSource: $Variable -- Parameter/variable binding\nDataSource: DATABASE Module.Entity -- Database query\nDataSource: MICROFLOW Module.MF -- Microflow datasource (no parens when it takes no arguments)\nDataSource: SELECTION widgetName -- Selection from another widget\nDataSource: $currentObject/Module.Assoc -- Over an association (\"data from context\")\n -- list widget → to-many collection\n -- nested DATAVIEW → the to-one referenced object\nAttribute: AttributeName -- Attribute binding (inputs)", - Example: "-- Database datasource with grid\nDATAGRID grid (DataSource: DATABASE Module.Customer) {\n COLUMN colName (Attribute: Name, Caption: 'Name')\n}\n\n-- Microflow datasource\nDATAVIEW dv (DataSource: MICROFLOW Module.GetData()) { ... }\n\n-- Over an association: a nested DataView shows the referenced (to-one) object\nDATAVIEW dvOrder (DataSource: $Order) {\n DATAVIEW dvCustomer (DataSource: $currentObject/Order_Customer) {\n TEXTBOX (Label: 'Name', Attribute: Name)\n }\n}\n\n-- Over an association: a list widget shows the (to-many) collection\nLISTVIEW lvLines (DataSource: $currentObject/Order_OrderLine) { ... }", + Example: "-- Database datasource with grid\nDATAGRID grid (DataSource: DATABASE Module.Customer) {\n COLUMN colName (Attribute: Name, Caption: 'Name')\n}\n\n-- Microflow datasource\nDATAVIEW dv (DataSource: MICROFLOW Module.GetData) {\n TEXTBOX txtName (Label: 'Name', Attribute: Name)\n}\n\n-- Over an association: a nested DataView shows the referenced (to-one) object\nDATAVIEW dvOrder (DataSource: $Order) {\n DATAVIEW dvCustomer (DataSource: $currentObject/Order_Customer) {\n TEXTBOX txtCustName (Label: 'Name', Attribute: Name)\n }\n}\n\n-- Over an association: a list widget shows the (to-many) collection\nLISTVIEW lvLines (DataSource: $currentObject/Order_OrderLine) {\n DYNAMICTEXT dtLine (Content: 'Line')\n}", SeeAlso: []string{"page.widgets", "page.create"}, }) @@ -65,7 +65,7 @@ func init() { "button style", "primary", "danger", "success", "icon", "linkbutton", "link button", }, - Syntax: "Action: SAVE_CHANGES\nAction: CANCEL_CHANGES\nAction: CLOSE_PAGE\nAction: DELETE\nAction: SHOW_PAGE Module.Page\nAction: SHOW_PAGE Module.Page(Param: $val)\nAction: MICROFLOW Module.MF\nAction: MICROFLOW Module.MF(Param: $val)\nAction: CREATE_OBJECT Module.Entity THEN SHOW_PAGE Module.Page\n\nButton styles: Default, Primary, Success, Info, Warning, Danger\nIcon: 'Module.IconCollection.IconName' -- e.g. 'Atlas_Core.Atlas_Filled.pencil'\nUse `linkbutton` instead of `actionbutton` for link render mode (same properties).", + Syntax: "Action: SAVE_CHANGES\nAction: SAVE_CHANGES CLOSE_PAGE -- save, then close the pop-up\nAction: CANCEL_CHANGES\nAction: CANCEL_CHANGES CLOSE_PAGE\nAction: CLOSE_PAGE\nAction: DELETE\nAction: DELETE CLOSE_PAGE\nAction: DELETE_OBJECT\nAction: NANOFLOW Module.NF\nAction: OPEN_LINK 'https://example.com'\nAction: SIGN_OUT\nAction: COMPLETE_TASK 'OutcomeName'\nAction: SHOW_PAGE Module.Page\nAction: SHOW_PAGE Module.Page(Param: $val)\nAction: MICROFLOW Module.MF\nAction: MICROFLOW Module.MF(Param: $val)\nAction: CREATE_OBJECT Module.Entity THEN SHOW_PAGE Module.Page\n\nButton styles: Default, Primary, Success, Info, Warning, Danger\nIcon: 'Module.IconCollection.IconName' -- e.g. 'Atlas_Core.Atlas_Filled.pencil'\nUse `linkbutton` instead of `actionbutton` for link render mode (same properties).", Example: "ACTIONBUTTON btnSave (Caption: 'Save', Action: SAVE_CHANGES, ButtonStyle: Primary)\nACTIONBUTTON btnEdit (Caption: 'Edit',\n Action: SHOW_PAGE Module.EditPage(Item: $currentObject))\nLINKBUTTON btnDelete (Caption: 'Delete', Action: DELETE,\n Icon: 'Atlas_Core.Atlas_Filled.pencil')", SeeAlso: []string{"page.widgets"}, }) @@ -89,7 +89,7 @@ func init() { "set property", "insert widget", "drop widget", "replace widget", "popup width", "popup height", "popup resizable", }, - Syntax: "ALTER PAGE Module.Name {\n SET property = value ON widgetName;\n SET (prop1 = val1, prop2 = val2) ON widgetName;\n SET Title = 'New Title'; -- page-level (case-sensitive)\n SET Class = 'css-class'; -- page-level CSS class / style\n SET Style = 'css: rule';\n SET PopupWidth = 800; -- page-level pop-up dimensions\n SET PopupHeight = 480;\n SET PopupResizable = true;\n INSERT AFTER widgetName { };\n INSERT BEFORE widgetName { };\n INSERT INTO containerName { };\n DROP WIDGET name1, name2;\n REPLACE widgetName WITH { };\n};", + Syntax: "ALTER PAGE Module.Name {\n SET property = value ON widgetName;\n SET Action = MICROFLOW Module.MF ON btnSave; -- any CREATE PAGE action form\n SET DataSource = $Param ON dvOrder;\n SET (prop1 = val1, prop2 = val2) ON widgetName;\n SET Title = 'New Title'; -- page-level (case-sensitive)\n SET Class = 'css-class'; -- page-level CSS class / style\n SET Style = 'css: rule';\n SET PopupWidth = 800; -- page-level pop-up dimensions\n SET PopupHeight = 480;\n SET PopupResizable = true;\n INSERT AFTER widgetName { };\n INSERT BEFORE widgetName { };\n INSERT INTO containerName { };\n DROP WIDGET name1, name2;\n REPLACE widgetName WITH { };\n};", Example: "ALTER PAGE Module.EditPage {\n SET (Caption = 'Save & Close', ButtonStyle = Success) ON btnSave;\n INSERT AFTER txtName {\n TEXTBOX txtMiddleName (Label: 'Middle Name', Attribute: MiddleName)\n };\n DROP WIDGET txtUnused;\n};", SeeAlso: []string{"page.create", "page.show", "snippet.alter"}, }) @@ -192,7 +192,7 @@ func init() { "use fragment", "template", "script scope", }, Syntax: "DEFINE FRAGMENT Name AS { };\nDEFINE FRAGMENT Name AS { SLOT [name] };\nDEFINE FRAGMENT Name ($d: datasource, $a: action) AS { };\nUSE FRAGMENT Name [(args)] [AS prefix_];\nUSE FRAGMENT Name [(args)] [AS prefix_] { };\nSHOW FRAGMENTS;\nDESCRIBE FRAGMENT Name;\nDESCRIBE FRAGMENT FROM PAGE Module.Page WIDGET widgetName;", - Example: "DEFINE FRAGMENT SaveCancelFooter AS {\n FOOTER footer1 {\n ACTIONBUTTON btnSave (Caption: 'Save', Action: SAVE_CHANGES, ButtonStyle: Primary)\n ACTIONBUTTON btnCancel (Caption: 'Cancel', Action: CANCEL_CHANGES)\n }\n};\n\nCREATE PAGE Module.EditPage (...) {\n DATAVIEW dv (DataSource: $Param) {\n TEXTBOX txtName (Label: 'Name', Attribute: Name)\n USE FRAGMENT SaveCancelFooter\n }\n};", + Example: "DEFINE FRAGMENT SaveCancelFooter AS {\n FOOTER footer1 {\n ACTIONBUTTON btnSave (Caption: 'Save', Action: SAVE_CHANGES, ButtonStyle: Primary)\n ACTIONBUTTON btnCancel (Caption: 'Cancel', Action: CANCEL_CHANGES)\n }\n};\n\nCREATE PAGE Module.EditPage (Params: { $Param: Module.Customer }, Title: 'Edit', Layout: 'Atlas_Core.Atlas_Default') {\n DATAVIEW dv (DataSource: $Param) {\n TEXTBOX txtName (Label: 'Name', Attribute: Name)\n USE FRAGMENT SaveCancelFooter\n }\n};", SeeAlso: []string{"fragment.define", "fragment.use", "fragment.slot", "fragment.params", "snippet"}, }) @@ -215,7 +215,7 @@ func init() { "prefix", "name conflict", }, Syntax: "USE FRAGMENT Name\nUSE FRAGMENT Name AS prefix_\nUSE FRAGMENT Name [AS prefix_] { }", - Example: "-- Basic usage\nCREATE PAGE Module.Page (...) {\n DATAVIEW dv (DataSource: $Param) {\n USE FRAGMENT FormFields\n USE FRAGMENT SaveCancelFooter\n }\n};\n\n-- With prefix to avoid name conflicts\nUSE FRAGMENT SaveCancelFooter AS order_\n-- Creates: order_footer1, order_btnSave, order_btnCancel\n\n-- Fill a fragment's content slot (see fragment.slot)\nUSE FRAGMENT Card {\n DYNAMICTEXT cardHeading (Content: 'Welcome', RenderMode: H2)\n DYNAMICTEXT cardText (Content: 'Wrapped content')\n}", + Example: "-- Basic usage\nCREATE PAGE Module.Page (Params: { $Param: Module.Customer }, Title: 'Page', Layout: 'Atlas_Core.Atlas_Default') {\n DATAVIEW dv (DataSource: $Param) {\n USE FRAGMENT FormFields\n USE FRAGMENT SaveCancelFooter\n }\n};\n\n-- With prefix to avoid name conflicts\nUSE FRAGMENT SaveCancelFooter AS order_\n-- Creates: order_footer1, order_btnSave, order_btnCancel\n\n-- Fill a fragment's content slot (see fragment.slot)\nUSE FRAGMENT Card {\n DYNAMICTEXT cardHeading (Content: 'Welcome', RenderMode: H2)\n DYNAMICTEXT cardText (Content: 'Wrapped content')\n}", SeeAlso: []string{"fragment", "fragment.define", "fragment.slot"}, }) @@ -227,7 +227,7 @@ func init() { "binding", "rebind", "building block override", "reusable component", }, Syntax: "-- Declare typed params, reference with $name in a datasource/action slot:\nDEFINE FRAGMENT Name ($data: datasource, $onEdit: action) AS { … };\n-- Supply values at the use site:\nUSE FRAGMENT Name ($data: , $onEdit: ) [{ payload }]\n-- Building blocks: rebind the outermost datasource / first button:\nUSE BUILDING BLOCK Module.Block (datasource: , action: ) [AS prefix_]", - Example: "DEFINE FRAGMENT DataPanel ($data: datasource, $onEdit: action) AS {\n CONTAINER panel (Class: 'card') {\n LISTVIEW lv (DataSource: $data) {\n SLOT content\n ACTIONBUTTON edit (Caption: 'Edit', Action: $onEdit, ButtonStyle: Primary)\n }\n }\n};\n\nCREATE PAGE Module.Orders (Title: 'Orders', Layout: Atlas_Core.Atlas_Default) {\n USE FRAGMENT DataPanel ($data: DATABASE Sales.Order, $onEdit: MICROFLOW Sales.Edit) {\n DYNAMICTEXT heading (Content: 'Orders', RenderMode: H4)\n }\n};\n\n-- Rebind a building block's datasource and primary button:\nUSE BUILDING BLOCK Atlas_Web_Content.List_Cards (datasource: DATABASE Sales.Order, action: MICROFLOW Sales.Open) AS orders_;\n\n-- Notes:\n-- * Param kinds: datasource | action. Every declared param must be supplied.\n-- * A microflow value parses as a datasource and is reinterpreted for an action param.\n-- * BB binding-point rule: datasource → first datasource widget; action → first button.", + Example: "DEFINE FRAGMENT DataPanel ($data: datasource, $onEdit: action) AS {\n CONTAINER panel (Class: 'card') {\n LISTVIEW lv (DataSource: $data) {\n SLOT content\n ACTIONBUTTON edit (Caption: 'Edit', Action: $onEdit, ButtonStyle: Primary)\n }\n }\n};\n\nCREATE PAGE Module.Orders (Title: 'Orders', Layout: Atlas_Core.Atlas_Default) {\n USE FRAGMENT DataPanel ($data: DATABASE Sales.Order, $onEdit: MICROFLOW Sales.Edit) {\n DYNAMICTEXT heading (Content: 'Orders', RenderMode: H4)\n }\n};\n\n-- Rebind a building block's datasource and primary button:\nUSE BUILDING BLOCK Atlas_Web_Content.List_Cards (datasource: DATABASE Sales.Order, action: MICROFLOW Sales.Open) AS orders_\n\n-- Notes:\n-- * Param kinds: datasource | action. Every declared param must be supplied.\n-- * A microflow value parses as a datasource and is reinterpreted for an action param.\n-- * BB binding-point rule: datasource → first datasource widget; action → first button.", SeeAlso: []string{"fragment", "fragment.slot", "fragment.use"}, }) diff --git a/cmd/mxcli/syntax/features_workflow.go b/cmd/mxcli/syntax/features_workflow.go index 55591bbf7..ddc003d31 100644 --- a/cmd/mxcli/syntax/features_workflow.go +++ b/cmd/mxcli/syntax/features_workflow.go @@ -70,8 +70,12 @@ func init() { "decision", "conditional", "branch", "if", "condition", "exclusive gateway", "XOR", }, - Syntax: "DECISION [''] [COMMENT '']\n OUTCOMES '' { } ...;", - Example: "DECISION 'Check amount'\n OUTCOMES 'Under 1000' { } 'Over 1000' {\n USER TASK ManagerApproval 'Manager must approve'\n OUTCOMES 'OK' { };\n };", + // A decision outcome needs the arrow ('Under 1000' -> { }); a USER TASK + // outcome does not ('OK' { }). The two read alike but are separate + // grammar rules, so the arrow is easy to drop — this entry did, and + // taught the broken form until TestExamplesParse started checking it. + Syntax: "DECISION [''] [COMMENT '']\n OUTCOMES '' -> { } ...;", + Example: "DECISION 'Check amount'\n OUTCOMES\n 'Under 1000' -> { }\n 'Over 1000' -> {\n USER TASK ManagerApproval 'Manager must approve'\n OUTCOMES 'OK' { };\n };", SeeAlso: []string{"workflow.create", "workflow.parallel-split"}, }) @@ -153,8 +157,16 @@ func init() { "alter workflow", "modify workflow", "update workflow", "add activity", "drop activity", "replace activity", }, - Syntax: "ALTER WORKFLOW Module.Name SET = ;\nALTER WORKFLOW Module.Name INSERT [BEFORE|AFTER ];\nALTER WORKFLOW Module.Name DROP ;\nALTER WORKFLOW Module.Name REPLACE WITH ;", - Example: "ALTER WORKFLOW HR.LeaveApproval SET DUE DATE = 'addDays([%CurrentDateTime%], 7)';\nALTER WORKFLOW HR.LeaveApproval INSERT\n CALL MICROFLOW HR.NotifyHR\n AFTER ReviewTask;", + // SET properties are keyword-led phrases, not `name = value` assignments: + // `SET DUE DATE ''`, `SET DISPLAY ''`, `SET OVERVIEW PAGE + // Module.Page`. The `= ` this entry used to show does not parse. + // INSERT names the anchor first and the activity second — INSERT AFTER + // — and there is no BEFORE. DROP and REPLACE take the + // ACTIVITY keyword. This entry previously showed the operand order + // reversed, advertised a BEFORE that does not exist, and omitted + // ACTIVITY, so none of it parsed. + Syntax: "ALTER WORKFLOW Module.Name SET DISPLAY '';\nALTER WORKFLOW Module.Name SET DUE DATE '';\nALTER WORKFLOW Module.Name SET OVERVIEW PAGE Module.Page;\nALTER WORKFLOW Module.Name SET ACTIVITY ;\nALTER WORKFLOW Module.Name INSERT AFTER ;\nALTER WORKFLOW Module.Name DROP ACTIVITY ;\nALTER WORKFLOW Module.Name REPLACE ACTIVITY WITH ;\nALTER WORKFLOW Module.Name INSERT OUTCOME '' ON { };\nALTER WORKFLOW Module.Name DROP OUTCOME '' ON ;", + Example: "ALTER WORKFLOW HR.LeaveApproval SET DUE DATE 'addDays([%CurrentDateTime%], 7)';\nALTER WORKFLOW HR.LeaveApproval INSERT AFTER ReviewTask\n CALL MICROFLOW HR.NotifyHR;\nALTER WORKFLOW HR.LeaveApproval DROP ACTIVITY ObsoleteStep;", SeeAlso: []string{"workflow.create", "workflow.drop"}, }) } diff --git a/docs-site/src/tools/bootstrap-prompt.md b/docs-site/src/tools/bootstrap-prompt.md index 738cfac3a..a7bdc48e0 100644 --- a/docs-site/src/tools/bootstrap-prompt.md +++ b/docs-site/src/tools/bootstrap-prompt.md @@ -6,11 +6,10 @@ repo** in Claude Code Web and paste the prompt below; the agent asks you what th is, then provisions everything and commits the result so future sessions self-bootstrap. -The interview comes first for a reason: the app name becomes the `.mpr` file name, the -Studio Pro app name and the path baked into the SessionStart hook, so it is far cheaper -to ask than to rename afterwards. The rest of the answers are the brief — they get -written into the repo, so the session that resumes after an idle reap knows what it is -building. +The prompt itself is deliberately tiny — install mxcli, unpack its skills, hand over to +the `bootstrap-app` skill. Everything with detail in it lives in that skill, which +ships inside the binary, so the paste stays phone-sized and the procedure is versioned +with mxcli instead of with whatever text someone copied months ago. Why a prompt instead of a GitHub template repo: the mobile "New repository" template dropdown shows only a small subset of templates, and a template repo needs per-Mendix- @@ -20,46 +19,10 @@ can seed the model from a design prototype in the same session — nothing to ma ## The prompt ````text -This is an empty repo. You are going to provision it as a Mendix app developed with -mxcli — but first find out what the app is. +This is an empty repo. Provision it as a Mendix app developed with mxcli. -## Step 0 — interview me, and WAIT for my answers before running anything - -Ask all of these in ONE message, numbered, each with the default you would pick, so I -can reply "defaults" or answer only the ones I care about. Do not start provisioning -until I have replied. - -1. **One app, or a solution of several?** One Mendix app is the default. Say - "solution" if this is several apps in one repo — e.g. a backend that owns the data - and publishes OData/REST, and a frontend that consumes it. If so, ask for each - app's name and one line on what it owns, and follow the multi-app deltas below. -2. **App name.** Becomes the `.mpr` file name, the app name in Studio Pro, and the - path in the session hook, so it is awkward to change later. One PascalCase word, - letters and digits only — `OrderPortal`, `FieldService`, `ClubAdmin`. Propose one - from my answer to Q3. -3. **What is the app for?** One or two sentences: who uses it, and what it lets them - do. If my answer is vague ("a tool for work"), ask one follow-up — everything below - is derived from this. -4. **What does it keep track of?** Three to six nouns that will become entities, and a - word on how they relate (e.g. "a Job has many Visits; each Visit has Photos"). For - a solution, also ask which app owns each noun. -5. **Who logs in?** The user roles, and roughly what each may do (e.g. "Requester - creates and sees their own; Approver sees everything and approves"). -6. **Look and feel.** One of the bundled themes: `signal` (light, high contrast), - `ledger` (light, dense, data-heavy), `console` (dark), or `none` for stock Atlas. - Default `signal`. -7. **Mendix version.** Default `11.13.0`. - -If I say "defaults" or ignore a question, choose something sensible for it, tell me -what you chose in one line, and keep going — do not block on me twice. - -## Then provision - -Substitute my answers for ``, `` and `` throughout. For a -solution, do steps 2–4 once per app and read "If this is a solution" first. - -1. Ensure `mxcli` is available. It should be pre-installed by the environment; if - not, download a prebuilt binary for your OS/arch and put it at `./mxcli`, e.g.: +1. Make sure `mxcli` is available. The environment may already have it; if not, + download the prebuilt binary for your OS/arch and put it at `./mxcli`: ```bash curl -fsSL -o ./mxcli \ @@ -67,138 +30,50 @@ solution, do steps 2–4 once per app and read "If this is a solution" first. chmod +x ./mxcli ``` - Use the **`nightly`** build: this is fast-moving alpha software and the warm-loop - commands used below (`run --local`, `--setup`, `--ensure-db`) land in `nightly` - before they appear in a tagged release. For a reproducible setup, pin a specific - release instead (`.../releases/download/vX.Y.Z/mxcli--`). Note: `go - install …@latest` does **not** work — the generated ANTLR parser isn't committed, - so use the prebuilt binary (a from-source build needs `make grammar`). -2. Create the app, and put it at the **repo root** — that is where `.claude/` and the - `./mxcli` binary have to live for future sessions to self-bootstrap. `mxcli new` - refuses to write into a directory that is not empty, and a git repo always has - `.git`, so create it in a subfolder and move it up: +2. Unpack the skills that ship inside it — this needs no project, so it works in an + empty repo: ```bash - ./mxcli new --version --theme - rm -f /mxcli # a hardlink to the ./mxcli you just ran; mv would - # refuse it as "the same file" - shopt -s dotglob && mv /* . && rmdir + ./mxcli init --sync-skills ``` - (Use `mxcli init` instead if an `.mpr` already exists.) `mxcli new` also runs - `mxcli init`, which writes `.claude/settings.json` with a SessionStart hook plus - the `.claude/bootstrap-mxcli.sh` it runs — check that the `.mpr` named in the - script is right after the move. -3. Confirm the Claude tooling: `./mxcli init --tool claude` (idempotent — it is what - step 2 already ran, and re-running it is the cheapest way to be sure the hook, - skills and commands are in place). -4. Bring prerequisites up: `./mxcli run --local --setup --ensure-db -p .mpr` - (caches MxBuild + runtime, starts Postgres, creates the app database). -5. Write the brief to `README.md` at the repo root: the app name(s), my answers to - Q3–Q5 in my words, and the theme and Mendix version you used. For a solution, say - which app owns what and how they talk to each other. This is what tells the next - session — after an idle reap, with none of this conversation — what it is building. - Keep it short enough that it stays true. -6. Create a `FINDINGS.md` at the repo root and keep appending to it as you work. - Log anything surprising or broken: an mxcli command that errored, a workaround you - applied, a `mxcli check` that passed but a real `mx check` later flagged. Note the - Mendix + mxcli versions and how each finding was verified. This is durable context - for the next session, and the most useful thing to share back to improve mxcli. -7. COMMIT everything now — `.mpr`, `.devcontainer/`, `.claude/` (the - SessionStart hook **and** `.claude/bootstrap-mxcli.sh`), `README.md` and - `FINDINGS.md` — so that after idle reaping the next session bootstraps from files, - not from re-running this prompt. The `mxcli` binary itself stays git-ignored (~85 - MB); the bootstrap script is what fetches it back into a fresh clone, so committing - the script is what makes the hook survive a reap. -8. Boot and verify: `./mxcli run --local -p .mpr` in the background, then - confirm the app answers HTTP 200 at http://localhost:8080/ and report. -9. (Optional) For a browser preview from this cloud session, run - `./mxcli run --hub https://hub.mxcli.org -p .mpr` and report the preview - URL it prints. This needs `MXCLI_HUB_KEY` set on the environment (see the workflow - page); without it, continue as a normal local run. - -## If this is a solution (several apps in one repo) - -Each app is a full Mendix project — one `.mpr`, one runtime, one database. Same steps, -with these deltas: - -- **Layout.** One subfolder per app, nothing at the repo root but `README.md`, - `FINDINGS.md` and `.claude/`. Run `mxcli new --version --theme - ` once per app and leave each where it lands; do not move anything up. -- **Ports.** Every app defaults to 8080/8090/6543 and they will collide. Give the - first app the defaults and the second `--app-port 8180 --admin-port 8190 - --serve-port 6643`. Avoid 8081/8091/6544 — `mxcli test --local` uses those. -- **Give each app its own hostname**, not just its own port. Cookies are keyed on - host name and **ignore the port**, so two apps on `localhost:8080` and - `localhost:8180` share one cookie jar: logging into one can silently replace the - other's `XASSESSIONID`. Two hostnames give two jars, and the differing ports do no - harm. Add them to `/etc/hosts` — - - ``` - 127.0.0.1 backend.local frontend.local - ``` +3. Read `.ai-context/skills/bootstrap-app.md` and follow it end to end. It begins by + interviewing me about the app, so ask me those questions and wait for my answers + before running anything else. - — and browse `http://backend.local:8080/` and `http://frontend.local:8180/`. The - runtime binds `127.0.0.1` and serves any `Host` you send it, and the client uses - relative URLs, so it works under any name that resolves to loopback. (`*.nip.io` - works too if you would rather not touch `/etc/hosts`; prefer `/etc/hosts` in a - locked-down container, where public wildcard DNS may not resolve — `localtest.me` - resolves to `::1` in some of them.) - - Then record the name in each app's own configuration, so the runtime knows the URL - it is reached at and generates absolute URLs — OIDC/SAML redirect URIs, deep links - — against the host name rather than the listen address: - - ```sql - alter settings configuration 'Default' - ApplicationRootUrl = 'http://backend.local:8080/'; - ``` - - `run --local` picks that up at boot and prints which configuration it came from. - A blank app ships `http://localhost:8080/` there, and that stock loopback value is - deliberately ignored — otherwise every project would start advertising a URL, and - the wrong port under `--app-port`. Only a real host name is passed through. -- **Databases** need no action: the name is derived from the `.mpr` file name, so - differently-named apps get different databases. -- **The session hook.** `mxcli init` writes `.claude/settings.json` inside each app - folder, but Claude Code reads the one at the **repo root** — and it will not add a - second entry for you (it dedupes on the command, not on the project). Write the root - one yourself, one line per app, e.g. - `test -x backend/mxcli && (cd backend && ./mxcli run --local --setup --ensure-db -p Backend.mpr) || true`. - Verify it by checking that a fresh shell can boot each app. -- **Previews.** Pass `--hub-solution ` to every `run --hub` so the apps - appear grouped in the hub overview instead of as unrelated previews. - -**Wire the integration in dependency order — the producer must be running first.** -`CREATE ODATA CLIENT` fetches the `$metadata` at the moment you create it and caches -it in the model; if the URL is unreachable it warns and leaves the client unvalidated, -with no external entities to import. So: publish on the producer -(`CREATE ODATA SERVICE … publish entity …`), boot it (`run --local`), and only then, -on the consumer, `CREATE ODATA CLIENT … MetadataUrl: 'http://backend.local:8080/odata/…/$metadata'` -followed by `CREATE EXTERNAL ENTITIES FROM …`. Use the hostname here too, so the -cached contract and the constant below agree with what the browser sees. Point `ServiceUrl` at a **constant** -(`ServiceUrl: @Module.SvcUrl`) so the address can be changed per environment without -touching the model — it will not stay `localhost`. `mxcli syntax odata.publish` and -`mxcli syntax odata.consume` have the full syntax; business events -(`mxcli syntax business-events`) are the alternative when the link should be -asynchronous. - -## Then propose the model — do not build it yet - -The blank template ships a `MyFirstModule`; the app's own work belongs in a module -named after it. From the brief, propose in chat: - -- a module name, and the entities from Q4 with their attributes and associations -- the user roles from Q5 and what each may read/write -- the handful of pages that make it usable -- for a solution: which app owns each entity, and what crosses the boundary — publish - only what the other app actually needs - -Show me that as MDL I can read, and wait for my go-ahead before executing it. If I -gave you a design to work from, use it as the source of truth for the model and the -pages: . +If I gave you a design to work from, use it as the source of truth for the model and +the pages: . ```` +That is the whole prompt. The procedure it used to spell out — the interview, the +provisioning steps, the multi-app deltas, the model proposal — now lives in the +**`bootstrap-app` skill**, which is embedded in the mxcli binary and unpacked by +step 2. Two things follow: the prompt is short enough to paste from a phone, and the +procedure is fixed by shipping a new mxcli rather than by asking everyone to re-paste +a longer prompt. + +## What the skill does once it takes over + +1. **Interviews you** — one app or a solution, app name, what the app is for, what it + keeps track of, who logs in, theme, Mendix version. The app name comes first + because it becomes the `.mpr` file name, the Studio Pro app name and the path baked + into the SessionStart hook. +2. **Provisions** — `mxcli new` into a subfolder and moves it to the repo root (the + root is where `.claude/` and `./mxcli` must live), `mxcli init --tool claude`, then + `run --local --setup --ensure-db` to cache MxBuild + runtime and create the + database. +3. **Writes the brief** — `README.md` (what is being built, in your words) and + `FINDINGS.md` (anything surprising or broken, appended as work proceeds). These are + what an idle-reaped session reads to know what it is working on. +4. **Commits, then boots and verifies** — HTTP 200 at `http://localhost:8080/`, plus + an optional `run --hub` preview URL. +5. **Proposes the model in MDL and waits** — module, entities, roles, pages — before + building anything. + +For a solution repo it also covers the parts that bite: per-app ports, a hostname per +app so the two apps do not share one cookie jar, the root SessionStart hook that +`mxcli init` will not write for you, and wiring OData in dependency order. + ## Which mxcli version gets installed Prebuilt binaries are the working install path. CI publishes them on every `vX.Y.Z` @@ -227,9 +102,10 @@ tag (latest is v0.16.0) **and** as a rolling `nightly` pre-release, with assets ## Which Mendix version to ask for -The prompt defaults to the newest version that has a published MxBuild — everything -mxcli does starts with downloading it, so "supported" means "on the CDN". Check before -bumping the default: +The skill defaults to the newest version that has a published MxBuild — everything +mxcli does starts with downloading it, so "supported" means "on the CDN". It runs this +check itself when asked for a newer version, and it is the check to run before bumping +the default: ```bash curl -sI -o /dev/null -w '%{http_code}\n' https://cdn.mendix.com/runtime/mxbuild-11.13.0.tar.gz # 200 @@ -243,13 +119,16 @@ two runtimes to keep straight. ## Two rules that make this robust -- **Committing the config (step 7) is mandatory.** The prompt is a *one-time seed*. - Its output — `.mpr` + `.devcontainer/` + `.claude/` with the SessionStart hook and - `bootstrap-mxcli.sh` — must be committed so the steady state is file-driven and - deterministic. After that, every new session runs the hook automatically; you never - re-paste the prompt. Miss the script and the hook has nothing to run after a reap. -- **mxcli delivery is an environment concern, not the prompt's.** Step 1 is the fragile - part in a gated web session (a GitHub release `curl` may be blocked). The robust fix +- **Committing the config is mandatory** (the skill's provisioning step 6). The prompt + is a *one-time seed*. Its output — `.mpr` + `.devcontainer/` + `.claude/` with the + SessionStart hook and `bootstrap-mxcli.sh` — must be committed so the steady state is + file-driven and deterministic. After that, every new session runs the hook + automatically; you never re-paste the prompt. Miss the script and the hook has + nothing to run after a reap. +- **mxcli delivery is an environment concern, not the prompt's.** The download in + step 1 is the fragile part in a gated web session (a GitHub release `curl` may be + blocked), and it is the one thing that cannot move into the skill — the skill is + inside the binary. The robust fix is for the Claude Code Web **environment image / setup script to pre-install mxcli** (and pre-cache MxBuild + runtime); `go install` via `proxy.golang.org` is the fallback and needs mxcli published as a public Go module. diff --git a/docs-site/src/tutorial/claude-code-web.md b/docs-site/src/tutorial/claude-code-web.md index 0cb20e496..73261c2cb 100644 --- a/docs-site/src/tutorial/claude-code-web.md +++ b/docs-site/src/tutorial/claude-code-web.md @@ -99,7 +99,11 @@ so the next session self-bootstraps. paste. In short, the agent will: - ensure `mxcli` is available (pre-installed, or download the `nightly` binary); -- `mxcli new App --version ` (or `mxcli init` if an `.mpr` already exists); +- `mxcli init --sync-skills` — unpack the skills embedded in the binary, then follow + the **`bootstrap-app`** skill, which carries the rest of this list (the paste itself + is only those two steps plus "read the skill"); +- interview you about the app, then `mxcli new App --version ` (or `mxcli init` + if an `.mpr` already exists); - `mxcli init --tool claude` — adds a **SessionStart hook** so future sessions come back up automatically; - `mxcli run --local --setup --ensure-db` — cache MxBuild + runtime, start diff --git a/docs/01-project/MDL_QUICK_REFERENCE.md b/docs/01-project/MDL_QUICK_REFERENCE.md index 54e200701..d4cb8d818 100644 --- a/docs/01-project/MDL_QUICK_REFERENCE.md +++ b/docs/01-project/MDL_QUICK_REFERENCE.md @@ -76,6 +76,8 @@ create persistent entity Module.Photo ( | Drop enumeration | `drop enumeration Module.Name;` | | | Create association | `create [or modify] association Module.Name from Parent to Child type reference\|ReferenceSet [owner default\|both] [delete_behavior ...];` | OR MODIFY updates existing association in-place | | Drop association | `drop association Module.Name;` | | +| Association line anchors | `@anchor(from: (0, 54), to: (100, 54))` above `create association …` | Where the connector attaches to each entity box, as a **percentage** of the box (0..100, whole numbers). `from` = the FROM entity's box, `to` = the TO entity's. Omitting an end preserves what is stored, so a `create or modify` about something else never flattens a hand-tuned line. Cross-module associations have no anchors — Mendix stores none | +| Retune anchors in place | `alter association Module.Name set anchor from (50, 100) to (50, 0);` | `(0, 50)` left-middle, `(100, 50)` right-middle, `(50, 100)` bottom-centre. `describe association` re-emits a non-default pair as the same `@anchor(...)`, so describe → edit → exec round-trips | ## ALTER ENTITY @@ -1020,6 +1022,18 @@ create page MyModule.Customer_Edit - Actions: `actionbutton`, `linkbutton`, `navigationlist` - Structure: `dataview`, `header`, `footer`, `controlbar`, `snippetcall` +**Drop-down filter, association mode** — filter a datagrid by a reference instead of an attribute. Giving the filter a `datasource:` (the OPTION list) selects the mode; all three parts are required: +```sql +column colCustomer (attribute: Order_Customer/Name, caption: 'Customer') { + dropdownfilter ddfCustomer ( + Association: Sales.Order_Customer, -- the reference on the GRID entity + datasource: database Sales.Customer, -- the option list (associated entity) + CaptionAttribute: Name -- what each option shows + ) +} +``` +A column cannot bind the association itself: `column c (attribute: Order_Customer)` is refused, because Mendix has nowhere to store a reference in an attribute-typed widget property (the build fails CE1613). Traverse it (`attribute: Assoc/Attr`) to show a value; use the mode above to filter by it. + **DynamicText parameter formatting** — append a `format (…)` block to a content parameter (the `format` keyword is required): ```sql dynamictext amt (content: '{1}', contentparams: [{1} = Amount format (decimalPrecision: 2, groupDigits: true)]) diff --git a/mdl-examples/bug-tests/828-object-state-functions-ok.mdl b/mdl-examples/bug-tests/828-object-state-functions-ok.mdl new file mode 100644 index 000000000..4092392a7 --- /dev/null +++ b/mdl-examples/bug-tests/828-object-state-functions-ok.mdl @@ -0,0 +1,47 @@ +-- ============================================================================ +-- Issue #828 — the accepted counterpart: object-state predicates are real +-- ============================================================================ +-- +-- POSITIVE TEST: `mxcli check` MUST accept this file. +-- +-- MDL044's allow-list is exprcheck's funcTable, and the table was missing the +-- three "special check" built-ins below, so `check` reported them as +-- hallucinated functions. That was a nuisance while MDL044 was check-only; +-- once the rule became an exec barrier (#828) it would have made `exec` REFUSE +-- valid MDL — the same shape as the MDL009 false positive that kept the #833 +-- promotion narrow. +-- +-- Each was built before being added to the table. Verified against mxbuild +-- 11.13.0: this file exec'd into a blank 11.13 app checks with 0 errors. +-- +-- `isSynced` / `isSyncing` are offline-sync predicates and are nanoflow-only; +-- that is a context restriction, not an unknown name, so they live in the +-- nanoflow here. +-- +-- The rejected counterpart is 828-unknown-expression-function.fail.mdl. +-- ============================================================================ + +create persistent entity MyFirstModule.Thing ( + Name: string(100) +); +/ + +create microflow MyFirstModule.ACT_IsNew ( + $Obj: MyFirstModule.Thing +) +returns boolean as $Flag +begin + declare $Flag boolean = isNew($Obj); + return $Flag; +end; +/ + +create nanoflow MyFirstModule.NF_SyncState ( + $Obj: MyFirstModule.Thing +) +returns boolean as $Flag +begin + declare $Flag boolean = isSynced($Obj) and isSyncing($Obj); + return $Flag; +end; +/ diff --git a/mdl-examples/bug-tests/828-unknown-expression-function.fail.mdl b/mdl-examples/bug-tests/828-unknown-expression-function.fail.mdl new file mode 100644 index 000000000..3112e9b42 --- /dev/null +++ b/mdl-examples/bug-tests/828-unknown-expression-function.fail.mdl @@ -0,0 +1,31 @@ +-- ============================================================================ +-- Issue #828 — `mxcli exec` wrote a microflow calling an unknown function +-- ============================================================================ +-- +-- NEGATIVE TEST (.fail.mdl): `mxcli check` MUST reject this file. An +-- unexpected pass means MDL044 has regressed. +-- +-- `currentDeviceType()` is not a Mendix expression function. `check` reported +-- MDL044 for it, but the rule was not on the exec allowlist, so `exec` wrote +-- the microflow anyway and the failure surfaced only at build time: +-- +-- [error] [CE0117] "Error(s) in expression." +-- at Create variable activity 'Create Boolean variable' +-- +-- Verified against mxbuild 11.13.0: exec'd with the rule unenforced, the +-- project checks with 1 error; with the rule enforced, exec refuses and the +-- project stays at 0 errors. +-- +-- The accepted counterpart is 828-object-state-functions-ok.mdl — three REAL +-- built-ins that funcTable was missing, so MDL044 flagged them too. Promoting +-- MDL044 to an exec barrier without adding them would have refused valid MDL, +-- which is the MDL009 trap from #833. Both files must be kept in step. +-- ============================================================================ + +create microflow MyFirstModule.ACT_UnknownFunction () +returns boolean as $Result +begin + declare $Result boolean = currentDeviceType() = 'Phone'; + return $Result; +end; +/ diff --git a/mdl-examples/bug-tests/830-datagrid-association-filter.mdl b/mdl-examples/bug-tests/830-datagrid-association-filter.mdl new file mode 100644 index 000000000..14d2a7217 --- /dev/null +++ b/mdl-examples/bug-tests/830-datagrid-association-filter.mdl @@ -0,0 +1,71 @@ +-- ============================================================================ +-- Issue #830 — filtering a datagrid by an association +-- ============================================================================ +-- +-- Reported: `column colCustomer (attribute: Order_Customer) { dropdownfilter … }` +-- failed the build with +-- [error] [CE1613] "The selected attribute 'ZKT38.Order.Order_Customer' no +-- longer exists." at Columns (1/1) of data grid 2 'dgOrders' +-- and there was no MDL way to filter a grid by a reference at all. +-- +-- Two separate things, and only one of them was fixable: +-- +-- 1. `attribute: ` IS NOT REPRESENTABLE, so mxcli now REFUSES it at +-- exec time (with both working forms in the message) instead of writing a +-- dangling AttributeRef and letting mxbuild find out. Verified on 11.13.0: +-- * `CustomWidgets$WidgetValue.AttributeRef` is typed `AttributeRef`, not the +-- polymorphic `MemberRef`. Hand-patching a `DomainModels$AssociationRef` +-- into it makes the project UNLOADABLE — `mx check` dies before validation +-- with "Object of type 'AssociationRef' cannot be converted to type +-- 'AttributeRef'". +-- * The WidgetValue has no association-valued property at all: +-- Mendix.Modeler.WebUI.dll, which defines the type, contains no +-- `AssociationRef` member. +-- What made it look supported is the DataGrid column property declaring +-- `Reference/ReferenceSet`. That is +-- permission for the attribute PATH to TRAVERSE a reference, not to bind one. +-- The refusal needs the domain model to tell an association from an attribute, +-- so it fires on `exec`, not on a project-less `mxcli check`. +-- +-- 2. The drop-down filter DOES have an association mode (`baseType: 'ref'`), and +-- mxcli could not author it — every ref-mode property was unmapped, so +-- `mxcli check` reported MDL-WIDGET01 "has no property `refEntity`" and exec +-- dropped them. It is now a def.json mode, entered by giving the filter a +-- `datasource:` — mirroring the ComboBox's association mode. +-- +-- Both supported forms are below. Verified: `mx check` → 0 errors on 11.13.0, +-- and `mx dump-mpr` shows baseType="ref", refEntity=IndirectEntityRef{steps: +-- [Order_Customer → Customer]}, refOptions=XPathSource{Customer}, +-- refCaption=AttributeRef{ZKT38.Customer.Name}. +-- ============================================================================ + +create module ZKT38; +/ +create entity ZKT38.Customer ( Name: String ); +create entity ZKT38.Order ( Number: String ); +create association ZKT38.Order_Customer from ZKT38.Order to ZKT38.Customer; +/ + +create or replace page ZKT38.OrderList +( Title: 'Orders', Layout: Atlas_Core.Atlas_Default ) +{ + datagrid dgOrders (datasource: database ZKT38.Order) { + column colNumber (attribute: Number, caption: 'Order') + + -- (1) SHOW a value from the associated object: traverse the reference. + -- `attribute: Order_Customer` (the reference itself) is refused. + -- (2) FILTER by the reference: association mode. The filter's `datasource:` + -- is the OPTION list (Customer), not the grid's own datasource; the + -- filter still lives in the column it filters. + column colCustomer (attribute: Order_Customer/Name, caption: 'Customer') { + dropdownfilter ddfCustomer ( + Association: ZKT38.Order_Customer, + datasource: database ZKT38.Customer, + CaptionAttribute: Name + ) + } + } +} +/ + +describe page ZKT38.OrderList; diff --git a/mdl-examples/bug-tests/872-association-line-anchors.mdl b/mdl-examples/bug-tests/872-association-line-anchors.mdl new file mode 100644 index 000000000..2033992b9 --- /dev/null +++ b/mdl-examples/bug-tests/872-association-line-anchors.mdl @@ -0,0 +1,105 @@ +-- ============================================================================ +-- Issue #872 — association line anchors survive an mxcli write +-- ============================================================================ +-- +-- Reported as "line geometry is not exposed in DESCRIBE/CREATE ASSOCIATION". +-- Investigating it turned up something worse than the missing feature: mxcli was +-- not merely omitting the anchors, it was DESTROYING them. +-- +-- `DomainModels$Association` carries `ParentConnection` / `ChildConnection` — +-- where the connector attaches to the FROM and TO entity boxes — as the string +-- "x;y". Both engines hardcoded "0;50" / "100;50" on every write and the parser +-- never read them back, so because the writers rebuild the whole element, ANY +-- association write reset them. Including a documentation-only one. +-- +-- Measured on a blank Mendix 11.13 app, whose Studio-Pro-authored +-- Administration.AccountPasswordData_Account stores 0;54 / 100;54: +-- +-- statement before after +-- alter entity … add attribute (unrelated) 0;54/100;54 0;54/100;54 (was fine) +-- alter association … set comment 0;54/100;54 0;50/100;50 (destroyed) +-- create or modify association … 0;54/100;54 0;50/100;50 (destroyed) +-- …same, MXCLI_ENGINE=legacy 0;54/100;54 0;50/100;50 (destroyed) +-- +-- Now: read → write back verbatim (guard-don't-drop, ADR-0005), AUTHORABLE from +-- MDL, and reported by DESCRIBE as the very syntax that authors them: +-- +-- @anchor(from: (0, 54), to: (100, 54)) +-- create association Administration.AccountPasswordData_Account +-- … +-- +-- The syntax reuses the EXISTING `@anchor` annotation and its `from:`/`to:` +-- parameters — the same annotation the microflow sequence-flow anchor uses, +-- asking the same question, differing only in the value type. It needed no +-- grammar rule: `annotation*` is generic across every CREATE statement, +-- `annotationParamName` already admits FROM and TO, and `(x, y)` is the existing +-- `annotationParenValue`. ALTER gets a clause mirroring +-- `alter entity … set position (x, y)`. +-- +-- Naming an end SETS it; not naming one PRESERVES what is stored. That +-- asymmetry is the whole point: a `create or modify association` about the +-- delete behaviour must not flatten a hand-tuned line. +-- +-- The units are PERCENTAGES of the entity box, 0..100. Measured across 88 +-- coordinate pairs in four Studio-Pro-authored sources — a blank 11.13 app plus +-- the Advanced Audit Trail Core 4.0.0, Email Connector 6.4.2 and Workflow +-- Commons 4.11.0 marketplace modules: +-- +-- x range 0..100, y range 0..100, nothing outside it +-- 85 of 88 pairs pin one coordinate to exactly 0 or 100 while the other +-- varies — "which edge, and how far along it". The 3 exceptions are all +-- y=99, a hair off the bottom edge. +-- distinct x: 0 9 11 17 18 47 49 50 65 77 78 84 87 100 +-- distinct y: 0 16 19 33 38 51 52 53 54 59 60 67 69 76 85 99 100 +-- +-- Pixels is ruled out by the model itself: `DomainModels$EntityImpl` stores only +-- `Location` (canvas pixels, e.g. "910;450") and NO size — the box's dimensions +-- are computed by the editor from the name and attribute list. A pixel anchor +-- would have nothing to measure against and would drift as attributes are added. +-- +-- That is why the syntax takes NUMBERS. The pair is continuous, so the issue's +-- proposed `@anchor(parent: bottom-left, child: top-right)` could not express +-- the observed values; mxcli's own default 0;50 already differs from Studio +-- Pro's 0;54 by four points. A non-integer coordinate is refused at check time, +-- because Mendix's loader refuses to OPEN a project whose anchor is fractional. +-- +-- Two more properties of the format, verified by hand-patching a project and +-- running `mx check` on 11.13.0: +-- * both components must be INTEGERS — "0.5;50" is rejected at LOAD with +-- StorageLoadException, before any validation runs; +-- * the LOADER does not range-check — "0;500" and "-20;50" load with 0 errors. +-- Nothing Mendix writes is out of range, but a value that is must round-trip +-- untouched rather than be clamped to something "sensible". +-- +-- Not applicable to cross-module associations: `DomainModels$CrossAssociation` +-- has no connection properties at all, and writing them there crashes Studio Pro +-- (issue #50). +-- +-- Verified on mxbuild 11.13.0: exec'd into a blank app this checks with 0 +-- errors, the stored pair is 11;99 / 9;0, `describe association` re-emits the +-- annotation, and re-executing that describe output into a fresh project +-- reproduces the same stored pair. +-- ============================================================================ + +@position(100, 100) +create entity Anchors.Customer ( Name: String ); +@position(500, 300) +create entity Anchors.Order ( Number: String ); +-- Authored anchors. These are real Studio Pro values (Workflow Commons +-- Settings_Configuration): neither coordinate sits on an edge, which is exactly +-- what a named-anchor vocabulary could not have expressed. +@anchor(from: (11, 99), to: (9, 0)) +create association Anchors.Order_Customer + from Anchors.Order to Anchors.Customer; +/ + +-- The write that used to destroy the anchors. Says nothing about them, so they +-- survive. +alter association Anchors.Order_Customer set comment 'touched'; +/ + +-- Retuning just the line, without restating the association. +alter association Anchors.Order_Customer set anchor from (0, 54) to (100, 54); +/ + +describe association Anchors.Order_Customer; diff --git a/mdl-examples/bug-tests/alter-page-set-action.mdl b/mdl-examples/bug-tests/alter-page-set-action.mdl new file mode 100644 index 000000000..4033f9744 --- /dev/null +++ b/mdl-examples/bug-tests/alter-page-set-action.mdl @@ -0,0 +1,77 @@ +-- ALTER PAGE … SET Action = ON +-- +-- Reported from a contact-management app built with mxcli: "SET Action = +-- MICROFLOW … fails in ALTER PAGE; use REPLACE instead". +-- +-- It failed at the *parser*: `alterPageAssignment` special-cased DataSource, +-- Visible and Editable and then fell through to `propertyValueV3`, which has no +-- `microflow ` form. So the statement did not parse at all: +-- +-- line 2:25 extraneous input 'M' expecting {DROP, ADD, SET, INSERT, REPLACE, '}'} +-- +-- REPLACE is a poor substitute. It rebuilds the widget from what the statement +-- says, so every property the author does not restate — ButtonStyle, Class, +-- design properties, tooltip — is silently dropped. Retargeting one button +-- should not require restating the button. +-- +-- The rule now reuses actionExprV3, the same action grammar CREATE PAGE uses, so +-- every action form works here on the day it works there rather than being a +-- subset extended one bug report at a time. That is the lesson of #855, where +-- `SET DataSource` carried a narrower vocabulary than REPLACE for the same +-- reason. +-- +-- Verified on Mendix 11.13.0: `mx check` reports 0 errors, and the sibling +-- properties survive the retarget. + +create module PA; +create module role PA.User; + +@position(100, 100) +create persistent entity PA.Order ( Number: String(50) ); + +create microflow PA.ACT_Save () begin end; +create microflow PA.ACT_Other () begin end; + +create page PA.OrderPage ( Title: 'Order', Layout: 'Atlas_Core.Atlas_Default' ) { + container c1 { + actionbutton btnGo ( + Caption: 'Go', + Action: MICROFLOW PA.ACT_Save, + ButtonStyle: Primary, + Class: 'my-css' + ) + } +}; + +-- Retarget the action. ButtonStyle and Class must still be there afterwards — +-- that is the difference from REPLACE. +alter page PA.OrderPage { + set Action = microflow PA.ACT_Other on btnGo; +}; + +-- Every actionExprV3 form is available, including the combined ones. The close +-- is a flag on the save action, not a separate action. +alter page PA.OrderPage { + set Action = SAVE_CHANGES CLOSE_PAGE on btnGo; +}; + +alter page PA.OrderPage { + set Action = CANCEL_CHANGES CLOSE_PAGE on btnGo; +}; + +-- Not every action form is available on the default engine: OPEN_LINK maps to +-- LinkClientAction, which the modelsdk codec does not serialize yet. That is a +-- pre-existing engine gap, not a SET one — CREATE PAGE refuses it identically — +-- and SET inherits the refusal rather than writing something broken: +-- +-- Error: failed to set: failed to set Action on btnGo: +-- unsupported action type *pages.LinkClientAction +-- +-- Inheriting CREATE PAGE's capability, refusal included, is the point of +-- delegating instead of maintaining a second switch. + +-- Settable alongside ordinary properties in one statement. +alter page PA.OrderPage { + set (Caption = 'Save & Close', ButtonStyle = Success) on btnGo; + set Action = SAVE_CHANGES CLOSE_PAGE on btnGo; +}; diff --git a/mdl-examples/bug-tests/icon-reference-validation.mdl b/mdl-examples/bug-tests/icon-reference-validation.mdl new file mode 100644 index 000000000..184a9ba01 --- /dev/null +++ b/mdl-examples/bug-tests/icon-reference-validation.mdl @@ -0,0 +1,63 @@ +-- Icon references were never resolved: a typo passed `mxcli check` and first +-- surfaced as a build error. +-- +-- [error] [CE1613] "The selected custom icon +-- 'Atlas_Core.Atlas_Filled.no-such-icon' no longer exists." at Action button 'btnBad' +-- +-- Reported as part of "`mxcli check` is insufficient validation — two +-- build-blocking issues passed check cleanly and only surfaced at runtime". +-- +-- The icon name was written straight through to BSON with nothing resolving it. +-- `mxcli check … -p --references` now resolves every icon reference +-- against the project's icon collections, and distinguishes the two failures +-- because they need different fixes: +-- +-- * unknown icon in a known collection — names near matches, and points at +-- `describe icon collection ` for the full list +-- * unknown collection — lists the collections the project actually has, +-- since the typo is usually there rather than in the icon +-- +-- NOTE ON THIS FILE. The check needs `-p`: icon collections are documents in +-- the project (a blank 11.13 app ships three, ~770 icons), so there is nothing +-- to resolve against without one. `make check-mdl` runs `mxcli check` with no +-- project, so this cannot be a `.fail.mdl` — a bad icon here would pass that +-- run and be reported as a negative test unexpectedly passing. The icons below +-- are therefore all valid; the rejection cases are covered by the unit tests in +-- mdl/executor/validate_icon_refs_test.go. +-- +-- To see the check reject a bad reference: +-- +-- mxcli check \ +-- -p app.mpr --references +-- +-- Verified on Mendix 11.13.0: the bad reference is reported by `mxcli check` +-- before any write, and the valid references below produce 0 errors under +-- `mx check`. + +create module ICONREF; +create module role ICONREF.User; + +create microflow ICONREF.ACT_Go () begin end; + +create page ICONREF.IconPage ( Title: 'Icons', Layout: 'Atlas_Core.Atlas_Default' ) { + container c1 { + -- Icon references resolve as Module.Collection.IconName. + actionbutton btnEdit (Caption: 'Edit', Action: MICROFLOW ICONREF.ACT_Go, Icon: 'Atlas_Core.Atlas_Filled.pencil') + actionbutton btnHome (Caption: 'Home', Action: MICROFLOW ICONREF.ACT_Go, Icon: 'Atlas_Core.Atlas.home') + -- Hyphenated Atlas names work too; only the navigation form needs quoting. + linkbutton lnkAdd (Caption: 'Add', Action: MICROFLOW ICONREF.ACT_Go, Icon: 'Atlas_Core.Atlas.add') + -- No icon at all is fine — the check only fires on a reference that is there. + actionbutton btnPlain (Caption: 'Plain', Action: MICROFLOW ICONREF.ACT_Go) + } +}; + +-- Icons in a navigation menu are resolved the same way, including sub-items. +-- (Hyphenated names are double-quoted in this position: Atlas_Core.Atlas."shopping-cart") +create or replace navigation Responsive + HOME PAGE ICONREF.IconPage + MENU ( + MENU ITEM 'Home' PAGE ICONREF.IconPage ICON Atlas_Core.Atlas.home; + MENU 'Group' ICON Atlas_Core.Atlas."shopping-cart" ( + MENU ITEM 'Edit' PAGE ICONREF.IconPage ICON Atlas_Core.Atlas_Filled.pencil; + ); + ); diff --git a/mdl-examples/bug-tests/sql-keyword-alias-crash.mdl b/mdl-examples/bug-tests/sql-keyword-alias-crash.mdl new file mode 100644 index 000000000..d42faf4dc --- /dev/null +++ b/mdl-examples/bug-tests/sql-keyword-alias-crash.mdl @@ -0,0 +1,40 @@ +-- `mxcli check` segfaulted on a one-line script — and the line came from +-- mxcli's own documentation (`mxcli syntax sql` used `AS source`). +-- +-- $ mxcli check script.mdl +-- panic: runtime error: invalid memory address or nil pointer dereference +-- [signal SIGSEGV: segmentation violation] +-- ... visitor.(*Builder).ExitSqlDisconnect +-- +-- Two defects: +-- +-- 1. GRAMMAR. Every sqlStatement rule took a bare IDENTIFIER for the connection +-- alias, driver and table name. `source` lexes as SOURCE_KW, so +-- `SQL DISCONNECT source` matched no alternative. IMPORT FROM already used +-- identifierOrKeyword for the very same alias, so `import from source ...` +-- worked while `sql source select ...` did not. +-- +-- 2. ROBUSTNESS. ANTLR error-recovers and keeps walking the tree, so the +-- listener ran against a context whose IDENTIFIER() was nil, and +-- `.GetText()` on it crashed the process. A malformed statement must be a +-- reported error, never a crash — the sibling handlers guarded their +-- children, this one did not. +-- +-- Found by TestExamplesParse, the guard that checks every `mxcli syntax` +-- example actually parses. The panic aborted the test binary, which had been +-- masking ten further failures in entries sorted after "sql". + +SQL CONNECT postgres 'postgres://user:pass@localhost:5432/mydb' AS source; +SQL source SHOW TABLES; +SQL source DESCRIBE users; +SQL source SELECT * FROM users WHERE active = true LIMIT 10; +SQL CONNECTIONS; +SQL DISCONNECT source; + +-- Other keywords that are natural connection/table names and were equally +-- unusable before the fix. +SQL DISCONNECT table; +SQL DISCONNECT query; +SQL DISCONNECT view; +SQL DISCONNECT index; +SQL DISCONNECT key; diff --git a/mdl-examples/bug-tests/transform-json-write-and-describe.mdl b/mdl-examples/bug-tests/transform-json-write-and-describe.mdl new file mode 100644 index 000000000..56dab15e9 --- /dev/null +++ b/mdl-examples/bug-tests/transform-json-write-and-describe.mdl @@ -0,0 +1,39 @@ +-- ============================================================================ +-- TRANSFORM JSON was dropped at write time on the default engine (CE0008) +-- ============================================================================ +-- +-- `transform $In with Module.Transformer` is authorable in MDL, has a DESCRIBE +-- formatter, and the LEGACY writer serialized it — but the modelsdk writer (the +-- default engine) had no case for TransformJsonAction, so it fell through to +-- `default: return nil` and the enclosing ActionActivity was written with NO +-- action at all: +-- +-- mxcli exec → "Created microflow: T.MF_Transform" +-- mx check → [error] [CE0008] "No action defined." at Action activity +-- +-- Same shape as #850 (DOWNLOAD FILE): exec reports success, and only the build +-- notices. The reader was missing too, so even once written it described as a +-- placeholder. +-- +-- Fixed on both sides. Storage keys mirror sdk/mpr.serializeTransformJsonAction: +-- ErrorHandlingType / InputVariableName / OutputVariableName / Transformation. +-- +-- Note the data transformer must exist: naming a missing one is CE1613 "The +-- selected data transformer … no longer exists", which is how this fixture first +-- failed after the write was fixed. +-- +-- Verified on Mendix 11.13.0: 0 errors, and describe→exec→describe is stable. +-- ============================================================================ + +create module T; + +create data transformer T.MyTransform + source json '{"name": "example"}' +{ + jslt '{"label": .name}' +} + +create microflow T.MF_Transform ($In: String) +begin + $Out = transform $In with T.MyTransform; +end; diff --git a/mdl/ast/ast_association.go b/mdl/ast/ast_association.go index 99dd9e26a..24e3eab66 100644 --- a/mdl/ast/ast_association.go +++ b/mdl/ast/ast_association.go @@ -109,6 +109,15 @@ type CreateAssociationStmt struct { Documentation string Comment string CreateOrModify bool // true for CREATE OR MODIFY / CREATE OR REPLACE + + // Line anchors from `@anchor(from: (x, y), to: (x, y))` — where the + // connector attaches to the FROM and TO entity boxes, as a PERCENTAGE of the + // box (0..100). nil means the statement said nothing, which preserves an + // existing anchor rather than resetting it. Reuses ast.Position because the + // shape is the same; the unit is not (an entity's @position is canvas + // pixels). (issue #872) + FromAnchor *Position + ToAnchor *Position } func (s *CreateAssociationStmt) isStatement() {} @@ -128,6 +137,7 @@ const ( AlterAssociationSetOwner AlterAssociationSetComment AlterAssociationSetStorage + AlterAssociationSetAnchor ) // AlterAssociationStmt represents: ALTER ASSOCIATION Module.Name SET ... @@ -138,6 +148,11 @@ type AlterAssociationStmt struct { Owner OwnerType Storage StorageType Comment string + + // SET ANCHOR FROM (x, y) TO (x, y) — both ends are always given together, + // because the pair is one visual decision. + FromAnchor *Position + ToAnchor *Position } func (s *AlterAssociationStmt) isStatement() {} diff --git a/mdl/backend/mcp/page_mutator.go b/mdl/backend/mcp/page_mutator.go index d468a3f0a..f06cb1de2 100644 --- a/mdl/backend/mcp/page_mutator.go +++ b/mdl/backend/mcp/page_mutator.go @@ -236,6 +236,15 @@ func (m *mcpPageMutator) EnclosingEntity(widgetRef string) string { // --- structural mutations --- +// SetWidgetAction is refused rather than approximated: the pg LightPage does not +// expose a widget's on-click action, so there is nothing here to write it to. +// Guard-don't-drop (ADR-0005) — an op the storage cannot express is an error, +// not a silent no-op that reports success. +func (m *mcpPageMutator) SetWidgetAction(widgetRef string, action pages.ClientAction) error { + return fmt.Errorf("setting a widget action is not supported by the MCP backend "+ + "(the pg LightPage does not expose widget actions) — widget %q", widgetRef) +} + func (m *mcpPageMutator) SetWidgetDataSource(widgetRef string, ds pages.DataSource) error { _, _, _, w, ok := findWidget(m.content, widgetRef) if !ok { diff --git a/mdl/backend/mock/mock_page_mutator.go b/mdl/backend/mock/mock_page_mutator.go index 512098cfd..fefd01693 100644 --- a/mdl/backend/mock/mock_page_mutator.go +++ b/mdl/backend/mock/mock_page_mutator.go @@ -20,6 +20,7 @@ type MockPageMutator struct { ContainerTypeFunc func() backend.ContainerKind SetWidgetPropertyFunc func(widgetRef string, prop string, value any) error SetWidgetDataSourceFunc func(widgetRef string, ds pages.DataSource) error + SetWidgetActionFunc func(widgetRef string, action pages.ClientAction) 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 @@ -63,6 +64,13 @@ func (m *MockPageMutator) SetWidgetDataSource(widgetRef string, ds pages.DataSou return nil } +func (m *MockPageMutator) SetWidgetAction(widgetRef string, action pages.ClientAction) error { + if m.SetWidgetActionFunc != nil { + return m.SetWidgetActionFunc(widgetRef, action) + } + return nil +} + func (m *MockPageMutator) SetColumnProperty(gridRef string, columnRef string, prop string, value any) error { if m.SetColumnPropertyFunc != nil { return m.SetColumnPropertyFunc(gridRef, columnRef, prop, value) diff --git a/mdl/backend/modelsdk/association_connection_test.go b/mdl/backend/modelsdk/association_connection_test.go new file mode 100644 index 000000000..eb258630b --- /dev/null +++ b/mdl/backend/modelsdk/association_connection_test.go @@ -0,0 +1,158 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "testing" + + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/domainmodel" +) + +// upstream #872. An association's line anchors (ParentConnection / +// ChildConnection — where the connector attaches to the entity boxes in the +// domain model editor) were hardcoded to mxcli's own "0;50" / "100;50" on every +// write, and never read back. Because assocToGen rebuilds the whole element, any +// association write destroyed them — including a documentation-only +// `alter association … set comment`. +// +// Measured on a blank Mendix 11.13 app, whose Studio-Pro-authored +// Administration.AccountPasswordData_Account stores 0;54 / 100;54: +// +// build after `alter association … set comment` +// pre-fix 0;50 / 100;50 (destroyed) +// fixed 0;54 / 100;54 (preserved) +// +// This test drives the same read → re-persist → reopen cycle that +// CREATE OR MODIFY ASSOCIATION and ALTER ASSOCIATION both take. +func TestUpdateDomainModel_PreservesConnectionPoints(t *testing.T) { + proj := copyFixture(t) + b := New() + if err := b.Connect(proj); err != nil { + t.Fatalf("connect: %v", err) + } + t.Cleanup(func() { _ = b.Disconnect() }) + + mod, err := b.GetModuleByName("MyFirstModule") + if err != nil || mod == nil { + t.Fatalf("GetModuleByName: %v", err) + } + dm, err := b.GetDomainModel(mod.ID) + if err != nil { + t.Fatalf("GetDomainModel: %v", err) + } + parent := &domainmodel.Entity{Name: "ZzAnchorParent", Persistable: true} + child := &domainmodel.Entity{Name: "ZzAnchorChild", Persistable: true} + if err := b.CreateEntity(dm.ID, parent); err != nil { + t.Fatalf("CreateEntity parent: %v", err) + } + if err := b.CreateEntity(dm.ID, child); err != nil { + t.Fatalf("CreateEntity child: %v", err) + } + // Anchors the developer dragged in Studio Pro: bottom-centre of the FROM box + // to top-centre of the TO box. Deliberately nothing like mxcli's defaults. + tuned := &domainmodel.Association{ + Name: "ZzAnchorChild_ZzAnchorParent", ParentID: child.ID, ChildID: parent.ID, + Type: "Reference", Owner: "Default", + ParentConnection: &model.Point{X: 50, Y: 100}, + ChildConnection: &model.Point{X: 50, Y: 0}, + } + if err := b.CreateAssociation(dm.ID, tuned); err != nil { + t.Fatalf("CreateAssociation: %v", err) + } + + find := func(t *testing.T, backend *Backend, why string) *domainmodel.Association { + t.Helper() + got, err := backend.GetDomainModel(mod.ID) + if err != nil { + t.Fatalf("GetDomainModel (%s): %v", why, err) + } + for _, a := range got.Associations { + if a.Name == "ZzAnchorChild_ZzAnchorParent" { + return a + } + } + t.Fatalf("association missing %s", why) + return nil + } + assertTuned := func(t *testing.T, a *domainmodel.Association, why string) { + t.Helper() + if a.ParentConnection == nil || a.ChildConnection == nil { + t.Fatalf("%s: anchors read back nil (%v, %v) — a field the reader drops is "+ + "a field the next write destroys", why, a.ParentConnection, a.ChildConnection) + } + if *a.ParentConnection != (model.Point{X: 50, Y: 100}) || *a.ChildConnection != (model.Point{X: 50, Y: 0}) { + t.Fatalf("%s: anchors = %+v / %+v, want {50 100} / {50 0} — reset to mxcli's defaults", + why, *a.ParentConnection, *a.ChildConnection) + } + } + + assertTuned(t, find(t, b, "on read"), "on read") + + // Re-persist the whole domain model unchanged — what ALTER ASSOCIATION and + // CREATE OR MODIFY ASSOCIATION both do. + dm2, err := b.GetDomainModel(mod.ID) + if err != nil { + t.Fatalf("GetDomainModel(2): %v", err) + } + if err := b.UpdateDomainModel(dm2); err != nil { + t.Fatalf("UpdateDomainModel: %v", err) + } + + b3 := New() + if err := b3.Connect(proj); err != nil { + t.Fatalf("reconnect: %v", err) + } + t.Cleanup(func() { _ = b3.Disconnect() }) + assertTuned(t, find(t, b3, "after UpdateDomainModel"), "after UpdateDomainModel") +} + +// An association created without anchors gets mxcli's defaults — the writer must +// not emit an empty or zero pair, which would move every new connector to the +// entity box's top-left corner. +func TestCreateAssociation_DefaultsConnectionPoints(t *testing.T) { + proj := copyFixture(t) + b := New() + if err := b.Connect(proj); err != nil { + t.Fatalf("connect: %v", err) + } + t.Cleanup(func() { _ = b.Disconnect() }) + + mod, _ := b.GetModuleByName("MyFirstModule") + dm, err := b.GetDomainModel(mod.ID) + if err != nil { + t.Fatalf("GetDomainModel: %v", err) + } + parent := &domainmodel.Entity{Name: "ZzPlainParent", Persistable: true} + child := &domainmodel.Entity{Name: "ZzPlainChild", Persistable: true} + if err := b.CreateEntity(dm.ID, parent); err != nil { + t.Fatalf("CreateEntity parent: %v", err) + } + if err := b.CreateEntity(dm.ID, child); err != nil { + t.Fatalf("CreateEntity child: %v", err) + } + if err := b.CreateAssociation(dm.ID, &domainmodel.Association{ + Name: "ZzPlainChild_ZzPlainParent", ParentID: child.ID, ChildID: parent.ID, + Type: "Reference", Owner: "Default", + }); err != nil { + t.Fatalf("CreateAssociation: %v", err) + } + + got, err := b.GetDomainModel(mod.ID) + if err != nil { + t.Fatalf("GetDomainModel: %v", err) + } + for _, a := range got.Associations { + if a.Name != "ZzPlainChild_ZzPlainParent" { + continue + } + p := domainmodel.FormatConnectionPoint(a.ParentConnection, "") + c := domainmodel.FormatConnectionPoint(a.ChildConnection, "") + if p != domainmodel.DefaultParentConnection || c != domainmodel.DefaultChildConnection { + t.Fatalf("new association anchors = %q / %q, want %q / %q", p, c, + domainmodel.DefaultParentConnection, domainmodel.DefaultChildConnection) + } + return + } + t.Fatal("association missing after create") +} diff --git a/mdl/backend/modelsdk/domainmodel.go b/mdl/backend/modelsdk/domainmodel.go index c247975d4..35009379c 100644 --- a/mdl/backend/modelsdk/domainmodel.go +++ b/mdl/backend/modelsdk/domainmodel.go @@ -466,6 +466,10 @@ func assocFromGen(a *genDm.Association) *domainmodel.Association { Type: domainmodel.AssociationType(a.Type()), Owner: domainmodel.AssociationOwner(a.Owner()), StorageFormat: domainmodel.AssociationStorageFormat(a.StorageFormat()), + // Line anchors, for the same reason: assocToGen rebuilds the element from + // this struct, so anything not read here is destroyed on the next write. + ParentConnection: domainmodel.ParseConnectionPoint(a.ParentConnection()), + ChildConnection: domainmodel.ParseConnectionPoint(a.ChildConnection()), } out.ID = model.ID(a.ID()) if db, ok := a.DeleteBehavior().(*genDm.AssociationDeleteBehavior); ok && db != nil { diff --git a/mdl/backend/modelsdk/domainmodel_write.go b/mdl/backend/modelsdk/domainmodel_write.go index 13e75deab..23631d6be 100644 --- a/mdl/backend/modelsdk/domainmodel_write.go +++ b/mdl/backend/modelsdk/domainmodel_write.go @@ -99,8 +99,11 @@ func assocToGen(a *domainmodel.Association) *genDm.Association { sf = "Column" } out.SetStorageFormat(sf) - out.SetParentConnection("0;50") - out.SetChildConnection("100;50") + // Carry the stored line anchors rather than resetting them — assocToGen runs + // on every association write, so hardcoding here discarded whatever the + // developer had dragged the connector to in Studio Pro (issue #872). + out.SetParentConnection(domainmodel.FormatConnectionPoint(a.ParentConnection, domainmodel.DefaultParentConnection)) + out.SetChildConnection(domainmodel.FormatConnectionPoint(a.ChildConnection, domainmodel.DefaultChildConnection)) db := genDm.NewAssociationDeleteBehavior() parentDB, childDB := "DeleteMeButKeepReferences", "DeleteMeButKeepReferences" diff --git a/mdl/backend/modelsdk/microflow_integration_actions_test.go b/mdl/backend/modelsdk/microflow_integration_actions_test.go new file mode 100644 index 000000000..c77bf384a --- /dev/null +++ b/mdl/backend/modelsdk/microflow_integration_actions_test.go @@ -0,0 +1,163 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "testing" + + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// The last three authorable-but-unreadable action types. Two were describe-only +// gaps; TransformJsonAction was worse — the modelsdk writer had no case for it +// either, so the action fell through to `default: return nil` and the enclosing +// ActionActivity was written with NO action at all. `mxcli exec` reported +// success and mxbuild then failed CE0008 "No action defined." — the #850 shape. +// `transform` was simply unusable on the default engine while the legacy writer +// handled it fine. +// +// Round trips rather than reader-only tests: the writers build these elements +// directly with explicit keys, and in two of the three a key diverges from what +// gen would suggest (`VariableName` for an external action's result; +// `QueryParameter` vs `Parameter` for the two REST mapping lists). A reader +// written against hand-authored BSON would assert against my guess instead of +// the writer's actual output. +func TestMicroflowRoundTrip_IntegrationActions(t *testing.T) { + tests := []struct { + name string + action microflows.MicroflowAction + verify func(t *testing.T, got microflows.MicroflowAction) + }{ + { + name: "TransformJsonAction", + action: µflows.TransformJsonAction{ + ErrorHandlingType: microflows.ErrorHandlingTypeRollback, + InputVariableName: "RawJson", + OutputVariableName: "Shaped", + Transformation: "T.MyTransform", + }, + verify: func(t *testing.T, got microflows.MicroflowAction) { + a, ok := got.(*microflows.TransformJsonAction) + if !ok { + t.Fatalf("got %T, want *microflows.TransformJsonAction", got) + } + if a.InputVariableName != "RawJson" || a.OutputVariableName != "Shaped" { + t.Errorf("variables = {%q, %q}, want {RawJson, Shaped}", a.InputVariableName, a.OutputVariableName) + } + if a.Transformation != "T.MyTransform" { + t.Errorf("Transformation = %q, want T.MyTransform", a.Transformation) + } + }, + }, + { + name: "CallExternalAction", + action: µflows.CallExternalAction{ + ErrorHandlingType: microflows.ErrorHandlingTypeRollback, + ConsumedODataService: "X.OrdersService", + Name: "PlaceOrder", + ResultVariableName: "Result", + UseReturnVariable: true, + ParameterMappings: []*microflows.ExternalActionParameterMapping{ + {ParameterName: "orderId", Argument: "$Id", CanBeEmpty: true}, + }, + }, + verify: func(t *testing.T, got microflows.MicroflowAction) { + a, ok := got.(*microflows.CallExternalAction) + if !ok { + t.Fatalf("got %T, want *microflows.CallExternalAction", got) + } + // The result variable is stored under `VariableName`, not + // `ResultVariableName` — reading the model's own field name loses it. + if a.ResultVariableName != "Result" { + t.Errorf("ResultVariableName = %q, want Result (stored under the key VariableName)", a.ResultVariableName) + } + if a.ConsumedODataService != "X.OrdersService" || a.Name != "PlaceOrder" { + t.Errorf("got service=%q name=%q, want X.OrdersService/PlaceOrder", a.ConsumedODataService, a.Name) + } + if len(a.ParameterMappings) != 1 { + t.Fatalf("ParameterMappings = %+v, want exactly one", a.ParameterMappings) + } + pm := a.ParameterMappings[0] + if pm.ParameterName != "orderId" || pm.Argument != "$Id" || !pm.CanBeEmpty { + t.Errorf("mapping = %+v, want {orderId, $Id, CanBeEmpty}", pm) + } + }, + }, + { + name: "RestOperationCallAction", + action: µflows.RestOperationCallAction{ + Operation: "X.Client.GetOrder", + OutputVariable: µflows.RestOutputVar{VariableName: "Response"}, + BodyVariable: µflows.RestBodyVar{VariableName: "Payload"}, + ParameterMappings: []*microflows.RestParameterMapping{ + {Parameter: "X.Client.GetOrder.id", Value: "$Id"}, + }, + QueryParameterMappings: []*microflows.RestQueryParameterMapping{ + {Parameter: "X.Client.GetOrder.expand", Value: "'lines'", Included: "Always"}, + }, + }, + verify: func(t *testing.T, got microflows.MicroflowAction) { + a, ok := got.(*microflows.RestOperationCallAction) + if !ok { + t.Fatalf("got %T, want *microflows.RestOperationCallAction", got) + } + if a.Operation != "X.Client.GetOrder" { + t.Errorf("Operation = %q, want X.Client.GetOrder", a.Operation) + } + // Output and body are single-child documents, not scalars. + if a.OutputVariable == nil || a.OutputVariable.VariableName != "Response" { + t.Errorf("OutputVariable = %+v, want VariableName Response", a.OutputVariable) + } + if a.BodyVariable == nil || a.BodyVariable.VariableName != "Payload" { + t.Errorf("BodyVariable = %+v, want VariableName Payload", a.BodyVariable) + } + if len(a.ParameterMappings) != 1 || a.ParameterMappings[0].Parameter != "X.Client.GetOrder.id" { + t.Errorf("ParameterMappings = %+v, want the path parameter", a.ParameterMappings) + } + // The query list stores its name under `QueryParameter`, not + // `Parameter` — the two lists are NOT symmetric. + if len(a.QueryParameterMappings) != 1 { + t.Fatalf("QueryParameterMappings = %+v, want exactly one", a.QueryParameterMappings) + } + qm := a.QueryParameterMappings[0] + if qm.Parameter != "X.Client.GetOrder.expand" || qm.Included != "Always" { + t.Errorf("query mapping = %+v, want {…expand, Always} (key is QueryParameter, not Parameter)", qm) + } + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + activity := µflows.ActionActivity{Action: tc.action} + activity.ID = model.ID("act-1") + mf := µflows.Microflow{ + Name: "ACT_Integration", + ObjectCollection: µflows.MicroflowObjectCollection{ + Objects: []microflows.MicroflowObject{activity}, + }, + } + mf.ID = model.ID("mf-1") + + got := roundTripMicroflow(t, mf) + + var found microflows.MicroflowAction + for _, obj := range got.ObjectCollection.Objects { + aa, ok := obj.(*microflows.ActionActivity) + if !ok { + continue + } + if aa.Action == nil { + t.Fatal("ActionActivity round-tripped with a nil Action — " + + "the CE0008 \"No action defined.\" shape") + } + found = aa.Action + } + if found == nil { + t.Fatal("no action survived the round trip") + } + tc.verify(t, found) + }) + } +} diff --git a/mdl/backend/modelsdk/microflow_read_actions.go b/mdl/backend/modelsdk/microflow_read_actions.go index 0ca2589ab..af13ca73d 100644 --- a/mdl/backend/modelsdk/microflow_read_actions.go +++ b/mdl/backend/modelsdk/microflow_read_actions.go @@ -498,6 +498,78 @@ func actionFromGen(el element.Element) microflows.MicroflowAction { } return out + case *genMf.TransformJsonAction: + // TRANSFORM $In WITH Module.Transformer. Read against the keys the writer + // builds (mirroring the legacy serializer) rather than the gen accessors. + raw := a.Raw() + out := µflows.TransformJsonAction{ + ErrorHandlingType: microflows.ErrorHandlingType(rawStr(raw, "ErrorHandlingType")), + InputVariableName: rawStr(raw, "InputVariableName"), + OutputVariableName: rawStr(raw, "OutputVariableName"), + Transformation: rawStr(raw, "Transformation"), + } + out.ID = model.ID(a.ID()) + return out + + case *genMf.CallExternalAction: + // CALL EXTERNAL ACTION. `VariableName` holds the result variable here — + // the model's own key, not gen's — and ResultDataType is deliberately NOT + // reconstructed: it is resolved from the consumed service's cached + // $metadata at write time, so inferring it from the stored + // VariableDataType would let a stale value round-trip as if authored. + raw := a.Raw() + out := µflows.CallExternalAction{ + ErrorHandlingType: microflows.ErrorHandlingType(rawStr(raw, "ErrorHandlingType")), + ConsumedODataService: rawStr(raw, "ConsumedODataService"), + Name: rawStr(raw, "Name"), + ResultVariableName: rawStr(raw, "VariableName"), + } + out.UseReturnVariable = out.ResultVariableName != "" + for _, md := range rawDocElements(raw, "ParameterMappings") { + pm := µflows.ExternalActionParameterMapping{ + ParameterName: rawStr(md, "ParameterName"), + Argument: rawStr(md, "Argument"), + } + if b, ok := md.Lookup("CanBeEmpty").BooleanOK(); ok { + pm.CanBeEmpty = b + } + out.ParameterMappings = append(out.ParameterMappings, pm) + } + out.ID = model.ID(a.ID()) + return out + + case *genMf.RestOperationCallAction: + // CALL REST OPERATION. The output and body variables are single-child + // documents rather than scalars, and the two mapping lists use different + // key names for the same idea (`Parameter` vs `QueryParameter`) — mirror + // the writer rather than assuming symmetry. + raw := a.Raw() + out := µflows.RestOperationCallAction{ + ErrorHandlingType: microflows.ErrorHandlingType(rawStr(raw, "ErrorHandlingType")), + Operation: rawStr(raw, "Operation"), + } + if ov, ok := raw.Lookup("OutputVariable").DocumentOK(); ok { + out.OutputVariable = µflows.RestOutputVar{VariableName: rawStr(ov, "VariableName")} + } + if bv, ok := raw.Lookup("BodyVariable").DocumentOK(); ok { + out.BodyVariable = µflows.RestBodyVar{VariableName: rawStr(bv, "VariableName")} + } + for _, md := range rawDocElements(raw, "ParameterMappings") { + out.ParameterMappings = append(out.ParameterMappings, µflows.RestParameterMapping{ + Parameter: rawStr(md, "Parameter"), + Value: rawStr(md, "Value"), + }) + } + for _, md := range rawDocElements(raw, "QueryParameterMappings") { + out.QueryParameterMappings = append(out.QueryParameterMappings, µflows.RestQueryParameterMapping{ + Parameter: rawStr(md, "QueryParameter"), + Value: rawStr(md, "Value"), + Included: rawStr(md, "Included"), + }) + } + out.ID = model.ID(a.ID()) + return out + case *genMf.WorkflowCallAction, *genMf.GetWorkflowDataAction, *genMf.GetWorkflowsAction, diff --git a/mdl/backend/modelsdk/microflow_write.go b/mdl/backend/modelsdk/microflow_write.go index 5507b104b..03c866ef3 100644 --- a/mdl/backend/modelsdk/microflow_write.go +++ b/mdl/backend/modelsdk/microflow_write.go @@ -787,6 +787,20 @@ func microflowActionToGen(action microflows.MicroflowAction) element.Element { // set task outcome, workflow operation pause/continue/abort/…, etc.). // Mirrors sdk/mpr/writer_microflow_workflow.go field-for-field. return workflowMicroflowActionToGen(a) + case *microflows.TransformJsonAction: + // "transform $In with Module.Transformer". Without this case the action + // fell through to `default: return nil` and the enclosing ActionActivity + // was written with NO action at all — `mxcli exec` reported success and + // mxbuild then failed CE0008 "No action defined." This is the #850 shape, + // and it means `transform` was unusable on the default engine while the + // legacy writer handled it. Keys mirror sdk/mpr.serializeTransformJsonAction. + g := newElem("Microflows$TransformJsonAction", string(a.ID)) + addStr(g, "ErrorHandlingType", orDefault(string(a.ErrorHandlingType), "Rollback")) + addStr(g, "InputVariableName", a.InputVariableName) + addStr(g, "OutputVariableName", a.OutputVariableName) + addStr(g, "Transformation", a.Transformation) + return g + case *microflows.RestOperationCallAction: // "call rest operation" — Microflows$RestOperationCallAction. Mirrors // serializeRestOperationCallAction. diff --git a/mdl/backend/mutation.go b/mdl/backend/mutation.go index 319149924..875a3e0a8 100644 --- a/mdl/backend/mutation.go +++ b/mdl/backend/mutation.go @@ -80,6 +80,10 @@ type PageMutator interface { // SetWidgetDataSource sets the DataSource on the named widget. SetWidgetDataSource(widgetRef string, ds pages.DataSource) error + // SetWidgetAction retargets the on-click action of the named widget. + // Refuses a widget that has no Action property. + SetWidgetAction(widgetRef string, action pages.ClientAction) error + // SetColumnProperty sets a property on a column within a grid widget. SetColumnProperty(gridRef string, columnRef string, prop string, value any) error diff --git a/mdl/backend/pagemutator/mutator.go b/mdl/backend/pagemutator/mutator.go index 1b7a425fb..a55f83df0 100644 --- a/mdl/backend/pagemutator/mutator.go +++ b/mdl/backend/pagemutator/mutator.go @@ -141,6 +141,43 @@ func (m *Mutator) SetWidgetDataSource(widgetRef string, ds pages.DataSource) err return nil } +// SetWidgetAction retargets the on-click action of an existing widget. +// +// The action is serialized through the same engine hook CREATE PAGE uses, so +// every action form is available — not a subset maintained here. Before this, +// changing a button's action meant REPLACEing the whole widget, which silently +// drops any property the author did not restate. +func (m *Mutator) SetWidgetAction(widgetRef string, action pages.ClientAction) error { + result := m.widgetFinder(m.rawData, widgetRef) + if result == nil { + return m.widgetNotFoundError(widgetRef) + } + // Refuse rather than write an Action onto something that has no such + // property: Studio Pro resolves every stored property against the type's + // property list and throws on one it does not know, while mxbuild's + // deserializer tolerates it — so a silent write here builds clean and fails + // to open. + if bsonnav.DGet(result.widget, "Action") == nil { + return fmt.Errorf("widget %q (%s) has no Action property — Action can only be set on a widget that "+ + "performs an on-click action, such as a button or a clickable container", + widgetRef, widgetTypeName(result.widget)) + } + serialized := m.deps.SerializeClientAction(action) + if serialized == nil { + return fmt.Errorf("unsupported action type %T", action) + } + bsonnav.DSet(result.widget, "Action", serialized) + return nil +} + +// widgetTypeName reports a widget's $Type for error messages, or "unknown type". +func widgetTypeName(widget bson.D) string { + if t, ok := bsonnav.DGet(widget, "$Type").(string); ok && t != "" { + return t + } + return "unknown type" +} + func (m *Mutator) SetColumnProperty(gridRef string, columnRef string, prop string, value any) error { result, err := findBsonColumn(m.rawData, gridRef, columnRef, m.widgetFinder) if err != nil { diff --git a/mdl/backend/pagemutator/mutator_action_test.go b/mdl/backend/pagemutator/mutator_action_test.go new file mode 100644 index 000000000..1876d2c23 --- /dev/null +++ b/mdl/backend/pagemutator/mutator_action_test.go @@ -0,0 +1,120 @@ +// SPDX-License-Identifier: Apache-2.0 + +package pagemutator + +import ( + "strings" + "testing" + + "go.mongodb.org/mongo-driver/bson" + + "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/mdl/backend/bsonnav" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +// stubActionDeps serializes a client action to a marker document so the mutator +// test can assert what was written without pulling in an engine. +type stubActionDeps struct { + Deps + serialized bson.D +} + +func (d *stubActionDeps) SerializeClientAction(a pages.ClientAction) bson.D { + return d.serialized +} + +// TestSetWidgetAction_ReplacesAction covers the FINDINGS gap: retargeting a +// button's action required REPLACEing the whole widget, which silently drops +// every property the author did not restate. Setting the action in place has to +// leave the rest of the widget alone. +func TestSetWidgetAction_ReplacesAction(t *testing.T) { + btn := bson.D{ + {Key: "$Type", Value: "Forms$ActionButton"}, + {Key: "Name", Value: "btnGo"}, + {Key: "Action", Value: bson.D{ + {Key: "$Type", Value: "Forms$MicroflowClientAction"}, + {Key: "Microflow", Value: "M.ACT_Save"}, + }}, + // The properties REPLACE would lose. + {Key: "ButtonStyle", Value: "Primary"}, + {Key: "CaptionTemplate", Value: "Go"}, + } + raw := makeRawPage(btn) + newAction := bson.D{ + {Key: "$Type", Value: "Forms$MicroflowClientAction"}, + {Key: "Microflow", Value: "M.ACT_Other"}, + } + m := New(raw, model.ID("unit-1"), &stubActionDeps{serialized: newAction}) + + if err := m.SetWidgetAction("btnGo", &pages.MicroflowClientAction{MicroflowName: "M.ACT_Other"}); err != nil { + t.Fatalf("SetWidgetAction: %v", err) + } + + widget := m.widgetFinder(m.rawData, "btnGo") + if widget == nil { + t.Fatal("btnGo not found after mutation") + } + action := bsonnav.DGetDoc(widget.widget, "Action") + if action == nil { + t.Fatal("Action missing after mutation") + } + if got := bsonnav.DGet(action, "Microflow"); got != "M.ACT_Other" { + t.Errorf("Microflow = %v, want M.ACT_Other", got) + } + // The whole point: everything else survives. + if got := bsonnav.DGet(widget.widget, "ButtonStyle"); got != "Primary" { + t.Errorf("ButtonStyle = %v, want Primary — a set must not disturb sibling properties", got) + } + if got := bsonnav.DGet(widget.widget, "CaptionTemplate"); got != "Go" { + t.Errorf("CaptionTemplate = %v, want Go", got) + } +} + +// TestSetWidgetAction_RefusesWidgetWithoutAction is the guard-don't-drop half. +// +// Studio Pro resolves every stored property against the type's property list and +// throws on one it does not know, while mxbuild's deserializer tolerates it — so +// writing an Action onto a container would build clean and then fail to open. +// Refusing is the only safe answer, and the build is not a safety net here. +func TestSetWidgetAction_RefusesWidgetWithoutAction(t *testing.T) { + container := bson.D{ + {Key: "$Type", Value: "Forms$DivContainer"}, + {Key: "Name", Value: "c1"}, + } + raw := makeRawPage(container) + m := New(raw, model.ID("unit-1"), &stubActionDeps{serialized: bson.D{{Key: "$Type", Value: "Forms$MicroflowClientAction"}}}) + + err := m.SetWidgetAction("c1", &pages.MicroflowClientAction{MicroflowName: "M.ACT_Save"}) + if err == nil { + t.Fatal("expected an error setting Action on a container") + } + for _, want := range []string{"c1", "Forms$DivContainer", "no Action property"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not mention %q", err.Error(), want) + } + } + // And nothing was written. + widget := m.widgetFinder(m.rawData, "c1") + if bsonnav.DGet(widget.widget, "Action") != nil { + t.Error("Action was written to a widget that has no such property") + } +} + +// TestSetWidgetAction_UnknownWidget keeps the not-found path a clear error. +func TestSetWidgetAction_UnknownWidget(t *testing.T) { + raw := makeRawPage() + m := New(raw, model.ID("unit-1"), &stubActionDeps{}) + err := m.SetWidgetAction("nope", &pages.MicroflowClientAction{MicroflowName: "M.ACT_Save"}) + if err == nil { + t.Fatal("expected an error for an unknown widget") + } + if !strings.Contains(err.Error(), "nope") { + t.Errorf("error %q does not name the widget", err.Error()) + } +} + +// Compile-time reminder that the Mutator still satisfies the backend interface +// the new method was added to. +var _ backend.PageMutator = (*Mutator)(nil) diff --git a/mdl/executor/cmd_alter_page.go b/mdl/executor/cmd_alter_page.go index 4eca7919c..6adc34e7e 100644 --- a/mdl/executor/cmd_alter_page.go +++ b/mdl/executor/cmd_alter_page.go @@ -63,7 +63,7 @@ func execAlterPage(ctx *ExecContext, s *ast.AlterPageStmt) error { for _, op := range s.Operations { switch o := op.(type) { case *ast.SetPropertyOp: - if err := applySetPropertyMutator(mutator, o); err != nil { + if err := applySetPropertyMutator(ctx, mutator, o, modName, containerID); err != nil { return mdlerrors.NewBackend("set", err) } case *ast.InsertWidgetOp: @@ -112,7 +112,7 @@ func execAlterPage(ctx *ExecContext, s *ast.AlterPageStmt) error { // SET property via mutator // ============================================================================ -func applySetPropertyMutator(mutator backend.PageMutator, op *ast.SetPropertyOp) error { +func applySetPropertyMutator(ctx *ExecContext, mutator backend.PageMutator, op *ast.SetPropertyOp, moduleName string, moduleID model.ID) error { // Sort property names for deterministic application order. propNames := make([]string, 0, len(op.Properties)) for k := range op.Properties { @@ -135,6 +135,16 @@ func applySetPropertyMutator(mutator backend.PageMutator, op *ast.SetPropertyOp) if err := mutator.SetWidgetDataSource(op.Target.Widget, ds); err != nil { return mdlerrors.NewBackend("set DataSource on "+op.Target.Name(), err) } + } else if propName == "Action" { + // Action is a polymorphic node, not a scalar — it goes through the + // same builder CREATE PAGE uses rather than being written as a value. + action, err := convertASTAction(ctx, value, moduleName, moduleID) + if err != nil { + return err + } + if err := mutator.SetWidgetAction(op.Target.Widget, action); err != nil { + return mdlerrors.NewBackend("set Action on "+op.Target.Name(), err) + } } else { if err := mutator.SetWidgetProperty(op.Target.Widget, propName, value); err != nil { return mdlerrors.NewBackend("set "+propName+" on "+op.Target.Name(), err) @@ -177,6 +187,32 @@ func convertASTDataSource(value interface{}) (pages.DataSource, error) { } } +// convertASTAction converts an AST action value to a pages.ClientAction. +// +// It delegates to the CREATE PAGE builder rather than reimplementing the switch, +// so every action form is supported here the day it is supported there — the +// alternative is the failure mode #855 documents for DataSource, where SET +// carried a narrower vocabulary than REPLACE and each missing case surfaced as +// its own bug report. +func convertASTAction(ctx *ExecContext, value any, moduleName string, moduleID model.ID) (pages.ClientAction, error) { + action, ok := value.(*ast.ActionV3) + if !ok { + return nil, mdlerrors.NewValidation("Action value must be an action expression, " + + "for example `set Action = microflow Module.MF on btnSave`") + } + pb := &pageBuilder{ + ctx: ctx, + backend: ctx.Backend, + moduleID: moduleID, + moduleName: moduleName, + execCache: ctx.Cache, + fragments: ctx.Fragments, + themeRegistry: ctx.GetThemeRegistry(), + widgetBackend: ctx.Backend, + } + return pb.buildClientActionV3(action) +} + // ============================================================================ // INSERT widget via mutator // ============================================================================ diff --git a/mdl/executor/cmd_alter_page_action_test.go b/mdl/executor/cmd_alter_page_action_test.go new file mode 100644 index 000000000..93b0a76ad --- /dev/null +++ b/mdl/executor/cmd_alter_page_action_test.go @@ -0,0 +1,173 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +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/microflows" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +// alterPageActionCtx builds the mock plumbing shared by the tests below and +// returns the statement runner plus a pointer to the action the mutator saw. +func alterPageActionCtx(t *testing.T, mutErr error) (func(*ast.ActionV3) error, *pages.ClientAction) { + t.Helper() + mod := mkModule("MyModule") + pg := mkPage(mod.ID, "TestPage") + // The CREATE PAGE builder resolves a microflow action against the backend, + // so the mock has to hold one — that resolution is the delegation working. + mf := mkMicroflow(mod.ID, "ACT_Other") + var got pages.ClientAction + + 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 }, + ListMicroflowsFunc: func() ([]*microflows.Microflow, error) { return []*microflows.Microflow{mf}, nil }, + OpenPageForMutationFunc: func(unitID model.ID) (backend.PageMutator, error) { + return &mock.MockPageMutator{ + SetWidgetActionFunc: func(widgetRef string, action pages.ClientAction) error { + if widgetRef != "btnSave" { + t.Errorf("widgetRef = %q, want btnSave", widgetRef) + } + got = action + return mutErr + }, + SaveFunc: func() error { return nil }, + }, nil + }, + } + h := mkHierarchy(mod) + withContainer(h, pg.ContainerID, mod.ID) + withContainer(h, mf.ContainerID, mod.ID) + ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(h)) + + return func(action *ast.ActionV3) error { + return execAlterPage(ctx, &ast.AlterPageStmt{ + PageName: ast.QualifiedName{Module: "MyModule", Name: "TestPage"}, + Operations: []ast.AlterPageOperation{ + &ast.SetPropertyOp{ + Target: ast.WidgetRef{Widget: "btnSave"}, + Properties: map[string]any{"Action": action}, + }, + }, + }) + }, &got +} + +// TestAlterPage_SetAction_RoutesToMutator pins that `SET Action` reaches +// SetWidgetAction rather than being written through SetWidgetProperty as a +// scalar. An action is a polymorphic node; storing it as a value would produce a +// document Studio Pro cannot open. +func TestAlterPage_SetAction_RoutesToMutator(t *testing.T) { + run, got := alterPageActionCtx(t, nil) + assertNoError(t, run(&ast.ActionV3{Type: "microflow", Target: "MyModule.ACT_Other"})) + + if *got == nil { + t.Fatal("SetWidgetAction was not called") + } + mf, ok := (*got).(*pages.MicroflowClientAction) + if !ok { + t.Fatalf("action type = %T, want *pages.MicroflowClientAction", *got) + } + if mf.MicroflowName != "MyModule.ACT_Other" { + t.Errorf("Microflow = %q, want MyModule.ACT_Other", mf.MicroflowName) + } +} + +// TestAlterPage_SetAction_DelegatesToCreatePageBuilder covers the point of the +// change: SET builds through the CREATE PAGE builder, so every action form works +// here without a second switch to keep in sync. `SAVE_CHANGES CLOSE_PAGE` is the +// interesting one — the close is a flag on the action, not a separate action. +func TestAlterPage_SetAction_DelegatesToCreatePageBuilder(t *testing.T) { + tests := []struct { + name string + action *ast.ActionV3 + verify func(t *testing.T, a pages.ClientAction) + }{ + { + name: "save and close", + action: &ast.ActionV3{Type: "save", ClosePage: true}, + verify: func(t *testing.T, a pages.ClientAction) { + s, ok := a.(*pages.SaveChangesClientAction) + if !ok { + t.Fatalf("type = %T", a) + } + if !s.ClosePage { + t.Error("ClosePage = false, want true") + } + }, + }, + { + name: "save without close", + action: &ast.ActionV3{Type: "save"}, + verify: func(t *testing.T, a pages.ClientAction) { + s, ok := a.(*pages.SaveChangesClientAction) + if !ok { + t.Fatalf("type = %T", a) + } + if s.ClosePage { + t.Error("ClosePage = true, want false") + } + }, + }, + { + name: "close page", + action: &ast.ActionV3{Type: "close"}, + verify: func(t *testing.T, a pages.ClientAction) { + if _, ok := a.(*pages.ClosePageClientAction); !ok { + t.Fatalf("type = %T, want *pages.ClosePageClientAction", a) + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + run, got := alterPageActionCtx(t, nil) + assertNoError(t, run(tt.action)) + if *got == nil { + t.Fatal("SetWidgetAction was not called") + } + tt.verify(t, *got) + }) + } +} + +// TestAlterPage_SetAction_RejectsNonAction guards the type assertion: a scalar +// where an action expression belongs must be a clear error, not a panic. +func TestAlterPage_SetAction_RejectsNonAction(t *testing.T) { + mod := mkModule("MyModule") + pg := mkPage(mod.ID, "TestPage") + 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 &mock.MockPageMutator{SaveFunc: func() error { return nil }}, nil + }, + } + h := mkHierarchy(mod) + withContainer(h, pg.ContainerID, mod.ID) + ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(h)) + + err := execAlterPage(ctx, &ast.AlterPageStmt{ + PageName: ast.QualifiedName{Module: "MyModule", Name: "TestPage"}, + Operations: []ast.AlterPageOperation{ + &ast.SetPropertyOp{ + Target: ast.WidgetRef{Widget: "btnSave"}, + Properties: map[string]any{"Action": "not-an-action"}, + }, + }, + }) + assertError(t, err) + assertContainsStr(t, err.Error(), "must be an action expression") +} diff --git a/mdl/executor/cmd_associations.go b/mdl/executor/cmd_associations.go index eb0e87028..55e7b2834 100644 --- a/mdl/executor/cmd_associations.go +++ b/mdl/executor/cmd_associations.go @@ -104,6 +104,10 @@ func execCreateAssociation(ctx *ExecContext, s *ast.CreateAssociationStmt) error if s.Comment != "" { assoc.Documentation = s.Comment } + // Anchors are applied only when the statement names them — + // silence preserves what is stored, so a `create or modify` + // that is not about layout does not flatten a hand-tuned line. + applyAnchors(assoc, s.FromAnchor, s.ToAnchor) if err := ctx.Backend.UpdateDomainModel(dm); err != nil { return mdlerrors.NewBackend("update association", err) } @@ -197,6 +201,7 @@ func execCreateAssociation(ctx *ExecContext, s *ast.CreateAssociationStmt) error Type: deleteBehavior, }, } + applyAnchors(assoc, s.FromAnchor, s.ToAnchor) if err := ctx.Backend.CreateAssociation(dm.ID, assoc); err != nil { return mdlerrors.NewBackend("create association", err) } @@ -249,6 +254,8 @@ func execAlterAssociation(ctx *ExecContext, s *ast.AlterAssociationStmt) error { assoc.StorageFormat = domainmodel.AssociationStorageFormat(s.Storage.String()) case ast.AlterAssociationSetComment: assoc.Documentation = s.Comment + case ast.AlterAssociationSetAnchor: + applyAnchors(assoc, s.FromAnchor, s.ToAnchor) } if err := ctx.Backend.UpdateDomainModel(dm); err != nil { return mdlerrors.NewBackend("update association", err) @@ -272,6 +279,13 @@ func execAlterAssociation(ctx *ExecContext, s *ast.AlterAssociationStmt) error { ca.StorageFormat = domainmodel.AssociationStorageFormat(s.Storage.String()) case ast.AlterAssociationSetComment: ca.Documentation = s.Comment + case ast.AlterAssociationSetAnchor: + // DomainModels$CrossAssociation has no connection properties at + // all, and writing them there crashes Studio Pro (#50) — so this + // is refused rather than silently ignored. + return mdlerrors.NewValidationf( + "association %s is cross-module, and Mendix stores no line anchors for those — "+ + "the connector is routed automatically", s.Name.String()) } if err := ctx.Backend.UpdateDomainModel(dm); err != nil { return mdlerrors.NewBackend("update cross-module association", err) @@ -519,6 +533,7 @@ func describeAssociation(ctx *ExecContext, name ast.QualifiedName) error { fmt.Fprintf(ctx.Output, "/**\n * %s\n */\n", assoc.Documentation) } + describeConnectionPoints(ctx, assoc) fmt.Fprintf(ctx.Output, "create association %s.%s\n", module.Name, assoc.Name) fmt.Fprintf(ctx.Output, "from %s to %s\n", fromEntity, toEntity) formatAssocDetails(assoc.Type, assoc.Owner, assoc.StorageFormat, assoc.ChildDeleteBehavior) @@ -548,4 +563,60 @@ func describeAssociation(ctx *ExecContext, name ast.QualifiedName) error { return mdlerrors.NewNotFound("association", name.String()) } +// applyAnchors copies authored `@anchor(from: …, to: …)` / `SET ANCHOR` values +// onto the association. +// +// An unnamed end is left alone rather than defaulted. That is what keeps a +// hand-tuned line safe through a `create or modify association` whose subject is +// the delete behaviour, and it is why the AST carries pointers: "not mentioned" +// and "mentioned as (0, 0)" are different instructions, and (0, 0) is a real +// anchor (the box's top-left). (issue #872) +func applyAnchors(assoc *domainmodel.Association, from, to *ast.Position) { + if assoc == nil { + return + } + if from != nil { + assoc.ParentConnection = &model.Point{X: from.X, Y: from.Y} + } + if to != nil { + assoc.ChildConnection = &model.Point{X: to.X, Y: to.Y} + } +} + +// describeConnectionPoints emits the association's line anchors — where the +// connector attaches to the FROM and TO entity boxes in the domain model editor +// — as the `@anchor(from: (x, y), to: (x, y))` annotation that authors them, so +// a describe → edit → exec cycle round-trips the layout. +// +// The units are PERCENTAGES of the entity box, 0..100 — measured across 88 +// coordinate pairs in four Studio-Pro-authored sources (a blank 11.13 app plus +// the Advanced Audit Trail Core, Email Connector and Workflow Commons modules): +// nothing falls outside 0..100, and 85 of the 88 pin one coordinate to exactly 0 +// or 100 while the other varies, i.e. "which edge, and how far along it". Pixels +// is ruled out by the model itself: `DomainModels$EntityImpl` stores only +// `Location` and NO size, so the box's dimensions are computed from the name and +// attribute list — a pixel anchor would have nothing to measure against and +// would drift every time an attribute is added. +// +// The pair is CONTINUOUS, which is why the syntax takes numbers rather than the +// eight named anchors the issue proposed: the observed x values are 0, 9, 11, +// 17, 18, 47, 49, 50, 65, 77, 78, 84, 87, 100, and mxcli's own default 0;50 +// differs from Studio Pro's 0;54 by four points. (issue #872) +// +// Only non-default anchors print, so the common case — an association mxcli +// created itself — describes exactly as before. +func describeConnectionPoints(ctx *ExecContext, assoc *domainmodel.Association) { + parent := domainmodel.FormatConnectionPoint(assoc.ParentConnection, domainmodel.DefaultParentConnection) + child := domainmodel.FormatConnectionPoint(assoc.ChildConnection, domainmodel.DefaultChildConnection) + if parent == domainmodel.DefaultParentConnection && child == domainmodel.DefaultChildConnection { + return + } + from := domainmodel.ParseConnectionPoint(parent) + to := domainmodel.ParseConnectionPoint(child) + if from == nil || to == nil { + return + } + fmt.Fprintf(ctx.Output, "@anchor(from: (%d, %d), to: (%d, %d))\n", from.X, from.Y, to.X, to.Y) +} + // --- Executor method wrappers for callers not yet migrated --- diff --git a/mdl/executor/cmd_associations_anchor_test.go b/mdl/executor/cmd_associations_anchor_test.go new file mode 100644 index 000000000..8bf98bfcb --- /dev/null +++ b/mdl/executor/cmd_associations_anchor_test.go @@ -0,0 +1,112 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "bytes" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/visitor" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/domainmodel" +) + +// upstream #872. `applyAnchors` is the single point where authored anchors reach +// the model, on the CREATE, CREATE OR MODIFY and ALTER paths alike. +// +// The property that matters is asymmetric: naming an end SETS it, and NOT naming +// one PRESERVES what is stored. Without that, a `create or modify association` +// whose subject is the delete behaviour would flatten a hand-tuned line — which +// is the bug the preservation slice of #872 fixed, reintroduced through the new +// syntax. +func TestApplyAnchors(t *testing.T) { + stored := func() *domainmodel.Association { + return &domainmodel.Association{ + ParentConnection: &model.Point{X: 11, Y: 99}, + ChildConnection: &model.Point{X: 9, Y: 0}, + } + } + + t.Run("neither end named preserves both", func(t *testing.T) { + a := stored() + applyAnchors(a, nil, nil) + if *a.ParentConnection != (model.Point{X: 11, Y: 99}) || *a.ChildConnection != (model.Point{X: 9, Y: 0}) { + t.Fatalf("a statement that says nothing about anchors must not touch them, got %+v / %+v", + *a.ParentConnection, *a.ChildConnection) + } + }) + + t.Run("one end named leaves the other alone", func(t *testing.T) { + a := stored() + applyAnchors(a, &ast.Position{X: 50, Y: 100}, nil) + if *a.ParentConnection != (model.Point{X: 50, Y: 100}) { + t.Errorf("from anchor = %+v, want {50 100}", *a.ParentConnection) + } + if *a.ChildConnection != (model.Point{X: 9, Y: 0}) { + t.Errorf("to anchor = %+v, want the stored {9 0} — it was not named", *a.ChildConnection) + } + }) + + t.Run("the zero point is a value, not an absence", func(t *testing.T) { + a := stored() + applyAnchors(a, &ast.Position{X: 0, Y: 0}, &ast.Position{X: 0, Y: 0}) + if *a.ParentConnection != (model.Point{}) || *a.ChildConnection != (model.Point{}) { + t.Fatalf("(0, 0) is the box's top-left and must be written, got %+v / %+v", + *a.ParentConnection, *a.ChildConnection) + } + }) + + t.Run("an association with nothing stored takes both", func(t *testing.T) { + a := &domainmodel.Association{} + applyAnchors(a, &ast.Position{X: 0, Y: 54}, &ast.Position{X: 100, Y: 54}) + if a.ParentConnection == nil || a.ChildConnection == nil { + t.Fatal("authored anchors were dropped") + } + }) +} + +// DESCRIBE must emit anchors as the syntax that AUTHORS them, or a +// describe → edit → exec cycle loses the layout — the round-trip requirement the +// issue asked for. Feeding the emitted line straight back through the parser is +// the only check that proves the two sides agree; asserting on a string literal +// would pass against a formatter that emits something nothing can read. +func TestDescribeConnectionPoints_RoundTripsThroughTheParser(t *testing.T) { + emit := func(t *testing.T, parent, child *model.Point) string { + t.Helper() + var buf bytes.Buffer + describeConnectionPoints(&ExecContext{Output: &buf}, + &domainmodel.Association{ParentConnection: parent, ChildConnection: child}) + return buf.String() + } + + // An association mxcli created itself carries the defaults and must describe + // exactly as it did before anchors existed. + if out := emit(t, &model.Point{X: 0, Y: 50}, &model.Point{X: 100, Y: 50}); out != "" { + t.Errorf("default anchors must not be printed, got %q", out) + } + if out := emit(t, nil, nil); out != "" { + t.Errorf("absent anchors must not be printed, got %q", out) + } + + line := emit(t, &model.Point{X: 11, Y: 99}, &model.Point{X: 9, Y: 0}) + if !strings.HasPrefix(line, "@anchor(") { + t.Fatalf("expected an @anchor annotation, got %q", line) + } + + prog, errs := visitor.Build(line + "create association M.Child_Parent from M.Child to M.Parent;") + if len(errs) > 0 { + t.Fatalf("DESCRIBE emitted MDL the parser rejects (%q): %v", line, errs) + } + stmt, ok := prog.Statements[0].(*ast.CreateAssociationStmt) + if !ok { + t.Fatalf("statement is %T, want *ast.CreateAssociationStmt", prog.Statements[0]) + } + if stmt.FromAnchor == nil || *stmt.FromAnchor != (ast.Position{X: 11, Y: 99}) { + t.Errorf("from anchor did not survive the round trip: %+v", stmt.FromAnchor) + } + if stmt.ToAnchor == nil || *stmt.ToAnchor != (ast.Position{X: 9, Y: 0}) { + t.Errorf("to anchor did not survive the round trip: %+v", stmt.ToAnchor) + } +} diff --git a/mdl/executor/cmd_pages_builder_assoc_as_attribute_test.go b/mdl/executor/cmd_pages_builder_assoc_as_attribute_test.go new file mode 100644 index 000000000..09ed91096 --- /dev/null +++ b/mdl/executor/cmd_pages_builder_assoc_as_attribute_test.go @@ -0,0 +1,136 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/domainmodel" +) + +// upstream #830: `column colCustomer (attribute: Order_Customer)` — an +// ASSOCIATION given to an attribute-typed widget property — was qualified like +// an attribute and written as +// +// DomainModels$AttributeRef{Attribute: "ZKT38.Order.Order_Customer"} +// +// so mxbuild failed CE1613 "The selected attribute … no longer exists". +// +// This is a refusal rather than a fix because the reference is NOT +// representable, established against mxbuild 11.13.0: +// - CustomWidgets$WidgetValue.AttributeRef is typed `AttributeRef`, not the +// polymorphic `MemberRef`. Hand-patching a DomainModels$AssociationRef into +// it makes the project UNLOADABLE — `mx check` dies before validation with +// "Object of type 'AssociationRef' cannot be converted to type +// 'AttributeRef'". +// - The WidgetValue carries no association-valued property at all: +// Mendix.Modeler.WebUI.dll, which defines the type, has no `AssociationRef` +// member. +// +// The DataGrid column property's Reference/ReferenceSet is +// what makes this look supported. It permits the attribute PATH to TRAVERSE a +// reference (`Order_Customer/Name`), which mxcli already writes correctly — so +// the accepted cases below matter as much as the refused one. +func TestRejectAssociationAsAttribute(t *testing.T) { + const ( + modID = model.ID("mod-zkt") + orderID = model.ID("e-order") + customerID = model.ID("e-customer") + baseID = model.ID("e-base") + ) + + newPB := func() *pageBuilder { + return &pageBuilder{ + entityContext: "ZKT38.Order", + execCache: &executorCache{ + hierarchy: &ContainerHierarchy{moduleNames: map[model.ID]string{modID: "ZKT38"}}, + domainModels: []*domainmodel.DomainModel{ + { + ContainerID: modID, + Entities: []*domainmodel.Entity{ + { + BaseElement: model.BaseElement{ID: orderID}, + Name: "Order", + GeneralizationRef: "ZKT38.Base", + Attributes: []*domainmodel.Attribute{ + {Name: "Number"}, + }, + }, + { + BaseElement: model.BaseElement{ID: customerID}, + Name: "Customer", + Attributes: []*domainmodel.Attribute{{Name: "Name"}}, + }, + { + BaseElement: model.BaseElement{ID: baseID}, + Name: "Base", + Attributes: []*domainmodel.Attribute{{Name: "Code"}}, + }, + }, + Associations: []*domainmodel.Association{ + {Name: "Order_Customer", ParentID: orderID, ChildID: customerID, Type: domainmodel.AssociationTypeReference}, + // Declared on the GENERALIZATION: reachable from Order, + // and just as unrepresentable there. + {Name: "Base_Customer", ParentID: baseID, ChildID: customerID, Type: domainmodel.AssociationTypeReference}, + }, + }, + }, + }, + } + } + + cases := []struct { + name string + binding string + wantRefs bool + }{ + {"the reported form: a reference bound as an attribute", "Order_Customer", true}, + {"an association declared on the generalization", "Base_Customer", true}, + + // Everything below must still be accepted — a refusal here would break + // working pages. + {"a plain attribute", "Number", false}, + {"an inherited attribute", "Code", false}, + {"the SUPPORTED traversal form", "Order_Customer/Name", false}, + {"an explicit three-part attribute", "ZKT38.Order.Number", false}, + {"a variable binding", "$Order", false}, + {"an empty binding", "", false}, + {"a name that is neither", "NoSuchMember", false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + pb := newPB() + err := pb.rejectAssociationAsAttribute(tc.binding, pb.entityContext, "column `c` property `attribute`") + if !tc.wantRefs { + if err != nil { + t.Fatalf("binding %q must be accepted, got: %v", tc.binding, err) + } + return + } + if err == nil { + t.Fatalf("binding %q is an association and must be refused — "+ + "writing it produces CE1613 and there is no correct BSON to write instead", tc.binding) + } + // The message has to carry both escape routes: the traversal form for + // showing a value, and the filter's association mode for filtering. + for _, want := range []string{"is an association", "CE1613", tc.binding + "/", "dropdownfilter"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error should mention %q, got: %v", want, err) + } + } + }) + } +} + +// Without a model there is no way to tell an association from an attribute, and +// guessing would reject valid MDL. Widget builders that run in isolation (unit +// tests, and any caller with no backend and nothing cached) must pass through. +func TestRejectAssociationAsAttribute_NoModelPassesThrough(t *testing.T) { + pb := &pageBuilder{entityContext: "ZKT38.Order"} + if err := pb.rejectAssociationAsAttribute("Order_Customer", "ZKT38.Order", "column `c`"); err != nil { + t.Fatalf("with no model available the binding must pass through, got: %v", err) + } +} diff --git a/mdl/executor/cmd_pages_builder_input.go b/mdl/executor/cmd_pages_builder_input.go index 73da8ba11..7d6fc7b2a 100644 --- a/mdl/executor/cmd_pages_builder_input.go +++ b/mdl/executor/cmd_pages_builder_input.go @@ -123,6 +123,92 @@ func (pb *pageBuilder) entityAttributeOwners() (owners map[string]map[string]boo return owners, parents, nil } +// rejectAssociationAsAttribute refuses a widget property that is attribute-typed +// but was given the name of an ASSOCIATION — `column c (attribute: Order_Customer)`. +// +// The reference cannot be represented. mxcli qualified the name like an attribute +// and wrote `DomainModels$AttributeRef{Attribute: "Mod.Order.Order_Customer"}`, so +// the build failed CE1613 "The selected attribute … no longer exists" — and there +// is no correct BSON to write instead: `CustomWidgets$WidgetValue.AttributeRef` is +// typed `AttributeRef`, not the polymorphic `MemberRef`. Storing an +// `AssociationRef` there makes the project UNLOADABLE ("Object of type +// 'AssociationRef' cannot be converted to type 'AttributeRef'"), and the WidgetValue +// has no association-valued property at all — `Mendix.Modeler.WebUI.dll`, which +// defines the type, carries no `AssociationRef` member. +// +// The DataGrid column's `Reference/ReferenceSet` +// is what makes this look supported. It is not permission to bind the reference; it +// is permission for the attribute PATH to traverse one — `attribute: Assoc/Attr`, +// which mxcli already writes as an AttributeRef with association steps. (issue #830) +// +// where names the binding for the message (e.g. "column `colCustomer`"). +func (pb *pageBuilder) rejectAssociationAsAttribute(name, entityContext, where string) error { + // A path (`Assoc/Attr`) is the SUPPORTED form; a `$var` reference and an + // empty binding are not ours to judge here. + if name == "" || strings.ContainsAny(name, "/$") || entityContext == "" { + return nil + } + // Deciding attribute-vs-association needs the model; without one (no backend + // and nothing cached — a unit test building widgets in isolation) let the + // binding through rather than guessing, exactly as declaringEntityFor does. + if pb.backend == nil && (pb.execCache == nil || pb.execCache.domainModels == nil) { + return nil + } + name = storedSystemMemberName(name) + // A three-part name is already an explicit Module.Entity.Attribute. + if strings.Count(name, ".") >= 2 { + return nil + } + // An attribute wins: entity members share one namespace, so a name that + // resolves as an attribute anywhere in the generalization chain is not an + // association. + if _, ok := pb.declaringEntityFor(entityContext, name); ok { + return nil + } + assocQN := pb.resolveAssociationPathIn(name, entityContext) + fromEntity, toEntity, ok := pb.associationEndpoints(assocQN) + if !ok || !pb.entityInChain(entityContext, fromEntity) { + return nil + } + leaf := toEntity + if idx := strings.LastIndex(leaf, "."); idx >= 0 { + leaf = leaf[idx+1:] + } + return mdlerrors.NewValidationf( + "%s binds `%s`, which is an association (%s → %s), not an attribute — "+ + "Mendix cannot store a reference in an attribute-typed widget property and the build fails "+ + "CE1613 \"The selected attribute '%s.%s' no longer exists\". "+ + "To SHOW a value from the associated object, traverse the reference: `attribute: %s/` "+ + "(e.g. `%s/Name`). To FILTER the grid by the reference, use the drop-down filter's association mode: "+ + "`dropdownfilter f (Association: %s, datasource: database %s, CaptionAttribute: )`", + where, name, fromEntity, toEntity, entityContext, name, + name, name, assocQN, toEntity) +} + +// entityInChain reports whether want is entityQN or one of its ancestors, so a +// member declared on a generalization still counts as reachable from the +// specialization the widget is bound to. +func (pb *pageBuilder) entityInChain(entityQN, want string) bool { + if want == "" { + return false + } + if entityQN == want { + return true + } + _, parents, err := pb.entityAttributeOwners() + if err != nil { + return false + } + seen := map[string]bool{} + for cur := entityQN; cur != "" && !seen[cur]; cur = parents[cur] { + seen[cur] = true + if cur == want { + return true + } + } + return false +} + // systemMemberBindingNames maps the name an audit member is DECLARED under to // the name Mendix actually stores it as. // diff --git a/mdl/executor/cmd_pages_describe_output.go b/mdl/executor/cmd_pages_describe_output.go index 3cbff7d17..14ccd060b 100644 --- a/mdl/executor/cmd_pages_describe_output.go +++ b/mdl/executor/cmd_pages_describe_output.go @@ -673,8 +673,12 @@ func outputWidgetMDLV3(ctx *ExecContext, w rawWidget, indent int) { if w.Content != "" { props = append(props, fmt.Sprintf("Attribute: %s", w.Content)) } - // Show DataSource and CaptionAttribute for ComboBox association mode - if w.DataSource != nil && widgetType == "combobox" { + // Show DataSource and CaptionAttribute for the association modes. + // The drop-down filter's ref mode has the same three parts as the + // ComboBox's (reference + option list + caption), so it re-emits + // through the same branch — without it the filter described back as a + // bare `dropdownfilter name` and the mode was lost on re-exec (#830). + if w.DataSource != nil && (widgetType == "combobox" || widgetType == "dropdownfilter") { switch w.DataSource.Type { case "database": props = append(props, fmt.Sprintf("DataSource: database from %s", w.DataSource.Reference)) diff --git a/mdl/executor/cmd_pages_describe_parse.go b/mdl/executor/cmd_pages_describe_parse.go index e3e390d4a..ca42ddb25 100644 --- a/mdl/executor/cmd_pages_describe_parse.go +++ b/mdl/executor/cmd_pages_describe_parse.go @@ -304,6 +304,17 @@ func parseRawWidget(ctx *ExecContext, w map[string]any, parentEntityContext ...s widget.CaptionAttribute = extractCustomWidgetPropertyAttributeRef(ctx, w, "optionsSourceAssociationCaptionAttribute") } } + // The drop-down filter's association mode is the same shape as the + // ComboBox's, on differently-named properties: `baseType` selects it and + // the reference is stored as an EntityRef, not an AttributeRef. Without + // this the filter described back as a bare `dropdownfilter name` and a + // describe→edit→exec cycle silently reverted it to attribute mode. (#830) + if widget.RenderMode == "dropdownfilter" && + extractCustomWidgetPropertyString(ctx, w, "baseType") == "ref" { + widget.DataSource = extractCustomWidgetPropertyDataSource(ctx, w, "refOptions") + widget.Content = extractCustomWidgetPropertyAssociation(ctx, w, "refEntity") + widget.CaptionAttribute = extractCustomWidgetPropertyAttributeRef(ctx, w, "refCaption") + } // For DataGrid2, also extract datasource, columns, CONTROLBAR widgets, paging, and selection if widget.RenderMode == "datagrid2" { widget.DataSource = extractDataGrid2DataSource(ctx, w) diff --git a/mdl/executor/cmd_pages_describe_pluggable.go b/mdl/executor/cmd_pages_describe_pluggable.go index 0c0742c8d..ac8ac2dae 100644 --- a/mdl/executor/cmd_pages_describe_pluggable.go +++ b/mdl/executor/cmd_pages_describe_pluggable.go @@ -101,8 +101,15 @@ func extractComboBoxDataSource(ctx *ExecContext, w map[string]any) *rawDataSourc if sourceType != "association" { return nil } + return extractCustomWidgetPropertyDataSource(ctx, w, "optionsSourceAssociationDataSource") +} - // Extract datasource from optionsSourceAssociationDataSource property +// extractCustomWidgetPropertyDataSource reads the DataSource held by a named +// pluggable-widget property. Used by the association modes, where the option +// list lives on a mode-specific property rather than the widget's own +// `datasource` (ComboBox `optionsSourceAssociationDataSource`, drop-down filter +// `refOptions`). +func extractCustomWidgetPropertyDataSource(ctx *ExecContext, w map[string]any, wantKey string) *rawDataSource { obj, ok := w["Object"].(map[string]any) if !ok { return nil @@ -110,7 +117,6 @@ func extractComboBoxDataSource(ctx *ExecContext, w map[string]any) *rawDataSourc propTypeKeyMap := buildPropertyTypeKeyMap(w, false) - // Search through properties for optionsSourceAssociationDataSource props := getBsonArrayElements(obj["Properties"]) for _, prop := range props { propMap, ok := prop.(map[string]any) @@ -118,8 +124,7 @@ func extractComboBoxDataSource(ctx *ExecContext, w map[string]any) *rawDataSourc continue } typePointerID := extractBinaryID(propMap["TypePointer"]) - propKey := propTypeKeyMap[typePointerID] - if propKey != "optionsSourceAssociationDataSource" { + if propTypeKeyMap[typePointerID] != wantKey { continue } value, ok := propMap["Value"].(map[string]any) diff --git a/mdl/executor/validate.go b/mdl/executor/validate.go index 871485cdb..09b030932 100644 --- a/mdl/executor/validate.go +++ b/mdl/executor/validate.go @@ -195,6 +195,10 @@ func validateProgram(ctx *ExecContext, prog *ast.Program) []error { } } errors = append(errors, validateForwardPageRefs(ctx, prog)...) + // Resolve icon-collection references. Needs the project (the collections + // are documents in it), so it belongs here rather than in the no-project + // pass — MxBuild otherwise reports the typo as CE1613. + errors = append(errors, validateIconRefs(ctx, prog)...) return errors } @@ -988,6 +992,14 @@ var execEnforcedMicroflowRules = map[string]bool{ // mxbuild 11.13.0 — the same class of "check caught it, exec did not" gap // that #833 was about. "MDL057": true, + // MDL044: a call to a name that is not a Mendix expression function is + // CE0117 "Error(s) in expression." at build time, verified on mxbuild + // 11.13.0 with `currentDeviceType()` (issue #828). Promoting this rule means + // exprcheck's funcTable is now a write barrier, so a name missing from it + // blocks valid MDL rather than merely warning about it: three genuine + // built-ins (isNew/isSynced/isSyncing) were found missing and added — each + // built at 0 errors — before this line was added. + "MDL044": true, } // validateMicroflowRules runs the MDL0xx microflow rule set (ValidateMicroflow) diff --git a/mdl/executor/validate_icon_refs.go b/mdl/executor/validate_icon_refs.go new file mode 100644 index 000000000..e02632d1f --- /dev/null +++ b/mdl/executor/validate_icon_refs.go @@ -0,0 +1,247 @@ +// SPDX-License-Identifier: Apache-2.0 + +// Reference validation for icon-collection references. +// +// `icon: 'Atlas_Core.Atlas_Filled.pencil'` names an icon inside an icon +// collection document. Nothing resolved it: the name was written through to +// BSON verbatim, `mxcli check` passed, and the first sign of a typo was MxBuild: +// +// [error] [CE1613] "The selected custom icon +// 'Atlas_Core.Atlas_Filled.no-such-icon' no longer exists." at Action button 'btnBad' +// +// The collections live in the project (Atlas_Core ships three, ~770 icons), so +// this needs -p — it runs in the --references pass alongside the other +// project-resolved references. +package executor + +import ( + "fmt" + "sort" + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" + mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" + "github.com/mendixlabs/mxcli/mdl/types" +) + +// iconIndex holds the project's icon collections, keyed by qualified collection +// name (Module.Collection) → set of icon names. +type iconIndex struct { + collections map[string]map[string]bool + // order preserves a stable listing for error messages. + order []string +} + +// buildIconIndex reads the project's icon collections once per validation run. +// Returns nil when the project exposes none, which disables the check rather +// than reporting every icon as unknown. +func buildIconIndex(ctx *ExecContext) *iconIndex { + cols, err := ctx.Backend.ListIconCollections() + if err != nil || len(cols) == 0 { + return nil + } + h, err := getHierarchy(ctx) + if err != nil { + return nil + } + idx := &iconIndex{collections: make(map[string]map[string]bool, len(cols))} + for _, c := range cols { + qn := iconCollectionQualifiedName(h, c) + if qn == "" { + continue + } + names := make(map[string]bool, len(c.Icons)) + for _, ic := range c.Icons { + names[ic.Name] = true + } + idx.collections[qn] = names + idx.order = append(idx.order, qn) + } + sort.Strings(idx.order) + if len(idx.collections) == 0 { + return nil + } + return idx +} + +func iconCollectionQualifiedName(h *ContainerHierarchy, c *types.IconCollection) string { + mod := h.GetModuleName(h.FindModuleID(c.ContainerID)) + if mod == "" || c.Name == "" { + return "" + } + return mod + "." + c.Name +} + +// validateIconRefs resolves every icon reference in the program against the +// project's icon collections. +func validateIconRefs(ctx *ExecContext, prog *ast.Program) []error { + if !ctx.Connected() { + return nil + } + idx := buildIconIndex(ctx) + if idx == nil { + return nil + } + + var errs []error + for i, stmt := range prog.Statements { + for _, ref := range iconRefsInStatement(stmt) { + if err := idx.check(ref); err != nil { + errs = append(errs, fmt.Errorf("statement %d: %w", i+1, err)) + } + } + } + return errs +} + +// iconRef is one icon reference and where it was written, for the message. +type iconRef struct { + value string // as authored, e.g. Atlas_Core.Atlas_Filled.pencil + where string // e.g. `button "btnSave"` or `menu item 'Home'` +} + +// iconRefsInStatement collects icon references from the statements that can +// carry one: page/snippet widget trees and navigation menus. +func iconRefsInStatement(stmt ast.Statement) []iconRef { + var out []iconRef + switch s := stmt.(type) { + case *ast.CreatePageStmtV3: + for _, w := range s.Widgets { + out = append(out, iconRefsInWidget(w)...) + } + case *ast.AlterPageStmt: + for _, op := range s.Operations { + switch o := op.(type) { + case *ast.SetPropertyOp: + if v := iconPropValue(o.Properties); v != "" { + out = append(out, iconRef{value: v, where: "widget " + quoteName(o.Target.Name())}) + } + case *ast.InsertWidgetOp: + for _, w := range o.Widgets { + out = append(out, iconRefsInWidget(w)...) + } + case *ast.ReplaceWidgetOp: + for _, w := range o.NewWidgets { + out = append(out, iconRefsInWidget(w)...) + } + } + } + case *ast.AlterNavigationStmt: + for _, item := range s.MenuItems { + out = append(out, iconRefsInMenu(item)...) + } + } + return out +} + +func iconRefsInWidget(w *ast.WidgetV3) []iconRef { + if w == nil { + return nil + } + var out []iconRef + if v := iconPropValue(w.Properties); v != "" { + out = append(out, iconRef{value: v, where: strings.ToLower(w.Type) + " " + quoteName(w.Name)}) + } + for _, c := range w.Children { + out = append(out, iconRefsInWidget(c)...) + } + return out +} + +func iconRefsInMenu(item ast.NavMenuItemDef) []iconRef { + var out []iconRef + if ref := normalizeIconRef(item.Icon); ref != "" { + out = append(out, iconRef{value: ref, where: "menu item " + quoteName(item.Caption)}) + } + for _, sub := range item.Items { + out = append(out, iconRefsInMenu(sub)...) + } + return out +} + +// iconPropValue pulls the icon property out of a widget property map. MDL +// property keys are case-insensitive, so both `Icon:` and `icon:` are accepted. +func iconPropValue(props map[string]any) string { + if props == nil { + return "" + } + for k, v := range props { + if !strings.EqualFold(k, "icon") { + continue + } + s, ok := v.(string) + if !ok { + continue + } + return normalizeIconRef(s) + } + return "" +} + +// normalizeIconRef strips the quoting MDL allows around an icon reference. +func normalizeIconRef(s string) string { + return strings.Trim(strings.TrimSpace(s), "'\"") +} + +func quoteName(s string) string { + if s == "" { + return "(unnamed)" + } + return "'" + s + "'" +} + +// check resolves one reference, distinguishing an unknown collection from an +// unknown icon within a known one — the two need different fixes. +func (idx *iconIndex) check(ref iconRef) error { + // Module.Collection.IconName — the icon name is the last segment, and the + // collection is everything before it. Splitting from the right keeps working + // if a module name ever contains a dot. + dot := strings.LastIndex(ref.value, ".") + if dot <= 0 || dot == len(ref.value)-1 { + return mdlerrors.NewValidation(fmt.Sprintf( + "%s: icon %q is not a qualified icon reference — write Module.Collection.IconName, "+ + "for example 'Atlas_Core.Atlas_Filled.pencil'.\n Collections in this project: %s", + ref.where, ref.value, strings.Join(idx.order, ", "))) + } + collection, icon := ref.value[:dot], ref.value[dot+1:] + + icons, known := idx.collections[collection] + if !known { + return mdlerrors.NewValidation(fmt.Sprintf( + "%s: unknown icon collection %q in icon reference %q.\n Collections in this project: %s", + ref.where, collection, ref.value, strings.Join(idx.order, ", "))) + } + if icons[icon] { + return nil + } + + msg := fmt.Sprintf("%s: icon %q does not exist in collection %q (MxBuild reports this as CE1613)", + ref.where, icon, collection) + if near := nearestIcons(icons, icon); len(near) > 0 { + msg += ".\n Did you mean: " + strings.Join(near, ", ") + } + msg += fmt.Sprintf(".\n List the collection's icons with: describe icon collection %s", collection) + return mdlerrors.NewValidation(msg) +} + +// nearestIcons suggests up to five icons whose names are close to the one +// written. Substring matching in both directions covers the common typos +// (a truncated name, an extra qualifier) without needing an edit-distance table. +func nearestIcons(icons map[string]bool, want string) []string { + lower := strings.ToLower(want) + var hits []string + for name := range icons { + l := strings.ToLower(name) + if l == lower { + continue + } + if strings.Contains(l, lower) || strings.Contains(lower, l) { + hits = append(hits, name) + } + } + sort.Strings(hits) + if len(hits) > 5 { + hits = hits[:5] + } + return hits +} diff --git a/mdl/executor/validate_icon_refs_test.go b/mdl/executor/validate_icon_refs_test.go new file mode 100644 index 000000000..feca88861 --- /dev/null +++ b/mdl/executor/validate_icon_refs_test.go @@ -0,0 +1,203 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +func testIconIndex() *iconIndex { + return &iconIndex{ + collections: map[string]map[string]bool{ + "Atlas_Core.Atlas": {"home": true, "pencil": true, "pencil-write-paper": true}, + "Atlas_Core.Atlas_Filled": {"home": true, "pencil": true}, + }, + order: []string{"Atlas_Core.Atlas", "Atlas_Core.Atlas_Filled"}, + } +} + +// TestIconIndexCheck covers the resolution rules. An icon reference was written +// straight through to BSON with nothing resolving it, so a typo first surfaced +// as CE1613 from MxBuild — long after `mxcli check` had passed. +func TestIconIndexCheck(t *testing.T) { + idx := testIconIndex() + tests := []struct { + name string + value string + wantErr bool + wantParts []string + }{ + { + name: "valid reference", + value: "Atlas_Core.Atlas_Filled.pencil", + }, + { + name: "valid reference in the other collection", + value: "Atlas_Core.Atlas.home", + }, + { + name: "unknown icon in a known collection", + value: "Atlas_Core.Atlas_Filled.no-such-icon", + wantErr: true, + wantParts: []string{"no-such-icon", "Atlas_Core.Atlas_Filled", "CE1613", "describe icon collection"}, + }, + { + name: "unknown collection names the ones that exist", + value: "Atlas_Core.Nope.pencil", + wantErr: true, + // Listing the real collections is the actionable half: the typo is + // usually in the collection, not the icon. + wantParts: []string{"unknown icon collection", "Atlas_Core.Nope", "Atlas_Core.Atlas_Filled"}, + }, + { + name: "not a qualified reference", + value: "pencil", + wantErr: true, + wantParts: []string{"not a qualified icon reference", "Module.Collection.IconName"}, + }, + { + name: "trailing dot", + value: "Atlas_Core.Atlas.", + wantErr: true, + wantParts: []string{"not a qualified icon reference"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := idx.check(iconRef{value: tt.value, where: "actionbutton 'btn'"}) + if !tt.wantErr { + if err != nil { + t.Fatalf("unexpected error for %q: %v", tt.value, err) + } + return + } + if err == nil { + t.Fatalf("expected an error for %q", tt.value) + } + for _, want := range tt.wantParts { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not mention %q", err.Error(), want) + } + } + }) + } +} + +// TestNearestIcons pins the suggestion behaviour: a near miss gets a hint, a +// name with nothing in common does not get a misleading one. +func TestNearestIcons(t *testing.T) { + icons := map[string]bool{"pencil": true, "pencil-write-paper": true, "home": true} + + got := nearestIcons(icons, "penci") + if len(got) != 2 || got[0] != "pencil" || got[1] != "pencil-write-paper" { + t.Errorf("nearestIcons(penci) = %v, want [pencil pencil-write-paper]", got) + } + if got := nearestIcons(icons, "zzzzz"); len(got) != 0 { + t.Errorf("nearestIcons(zzzzz) = %v, want none — a wrong hint is worse than no hint", got) + } + // An exact match is not a suggestion for itself. + if got := nearestIcons(icons, "home"); len(got) != 0 { + t.Errorf("nearestIcons(home) = %v, want none", got) + } +} + +// TestIconRefsInStatement covers collection from every statement shape that can +// carry an icon — a reference the walker misses is a reference nothing checks. +func TestIconRefsInStatement(t *testing.T) { + btn := func(name, icon string) *ast.WidgetV3 { + return &ast.WidgetV3{Type: "ACTIONBUTTON", Name: name, Properties: map[string]any{"Icon": icon}} + } + + tests := []struct { + name string + stmt ast.Statement + want []string + }{ + { + name: "create page, nested widgets", + stmt: &ast.CreatePageStmtV3{Widgets: []*ast.WidgetV3{ + {Type: "CONTAINER", Name: "c1", Children: []*ast.WidgetV3{ + btn("btnA", "Atlas_Core.Atlas.home"), + btn("btnB", "Atlas_Core.Atlas.pencil"), + }}, + }}, + want: []string{"Atlas_Core.Atlas.home", "Atlas_Core.Atlas.pencil"}, + }, + { + name: "alter page set", + stmt: &ast.AlterPageStmt{Operations: []ast.AlterPageOperation{ + &ast.SetPropertyOp{ + Target: ast.WidgetRef{Widget: "btnSave"}, + Properties: map[string]any{"Icon": "Atlas_Core.Atlas.home"}, + }, + }}, + want: []string{"Atlas_Core.Atlas.home"}, + }, + { + name: "alter page insert and replace", + stmt: &ast.AlterPageStmt{Operations: []ast.AlterPageOperation{ + &ast.InsertWidgetOp{Widgets: []*ast.WidgetV3{btn("btnI", "Atlas_Core.Atlas.a")}}, + &ast.ReplaceWidgetOp{NewWidgets: []*ast.WidgetV3{btn("btnR", "Atlas_Core.Atlas.b")}}, + }}, + want: []string{"Atlas_Core.Atlas.a", "Atlas_Core.Atlas.b"}, + }, + { + name: "navigation menu, including sub-items", + stmt: &ast.AlterNavigationStmt{MenuItems: []ast.NavMenuItemDef{ + {Caption: "Home", Icon: "Atlas_Core.Atlas.home", Items: []ast.NavMenuItemDef{ + {Caption: "Nested", Icon: "Atlas_Core.Atlas.pencil"}, + }}, + }}, + want: []string{"Atlas_Core.Atlas.home", "Atlas_Core.Atlas.pencil"}, + }, + { + name: "no icons at all", + stmt: &ast.CreatePageStmtV3{Widgets: []*ast.WidgetV3{ + {Type: "CONTAINER", Name: "c1"}, + }}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + refs := iconRefsInStatement(tt.stmt) + if len(refs) != len(tt.want) { + t.Fatalf("got %d refs %v, want %d %v", len(refs), refs, len(tt.want), tt.want) + } + for i, w := range tt.want { + if refs[i].value != w { + t.Errorf("ref[%d] = %q, want %q", i, refs[i].value, w) + } + if refs[i].where == "" { + t.Errorf("ref[%d] has no location for the message", i) + } + } + }) + } +} + +// TestIconPropValue covers the quoting and casing MDL allows — `Icon:` and +// `icon:` are the same property, and the value may arrive quoted. +func TestIconPropValue(t *testing.T) { + tests := []struct { + props map[string]any + want string + }{ + {map[string]any{"Icon": "Atlas_Core.Atlas.home"}, "Atlas_Core.Atlas.home"}, + {map[string]any{"icon": "Atlas_Core.Atlas.home"}, "Atlas_Core.Atlas.home"}, + {map[string]any{"ICON": "'Atlas_Core.Atlas.home'"}, "Atlas_Core.Atlas.home"}, + {map[string]any{"Icon": " Atlas_Core.Atlas.home "}, "Atlas_Core.Atlas.home"}, + {map[string]any{"Caption": "not an icon"}, ""}, + {map[string]any{"Icon": 42}, ""}, // non-string must not panic + {nil, ""}, + } + for _, tt := range tests { + if got := iconPropValue(tt.props); got != tt.want { + t.Errorf("iconPropValue(%v) = %q, want %q", tt.props, got, tt.want) + } + } +} diff --git a/mdl/executor/validate_microflow_expr_test.go b/mdl/executor/validate_microflow_expr_test.go index 517e8bced..68c235df5 100644 --- a/mdl/executor/validate_microflow_expr_test.go +++ b/mdl/executor/validate_microflow_expr_test.go @@ -50,6 +50,19 @@ func TestValidateMicroflow_UnknownFunction(t *testing.T) { // flagged — previously only return/if/declare/set expressions were checked. {"aggregate in create attr", `$r = create "M"."E" (Total = formatDecimal(sum($x), '0.00'));`, true, "aggregate activity"}, {"known func in create attr", `$r = create "M"."E" (Total = trim($x));`, false, ""}, + // #828: the object-state predicates were missing from funcTable, so MDL044 + // reported three real built-ins as hallucinated. Each builds at 0 errors on + // mxbuild 11.13.0. This matters more since MDL044 became exec-enforced — + // a false positive here refuses valid MDL rather than merely warning. + {"isNew is a real built-in", "declare $b Boolean = isNew($x);", false, ""}, + {"isSynced is a real built-in", "declare $b Boolean = isSynced($x);", false, ""}, + {"isSyncing is a real built-in", "declare $b Boolean = isSyncing($x);", false, ""}, + // The control for the above: `trunc` looks like a sibling of round/floor/ceil + // and was even listed in roundingFuncs, but Mendix has no such function — + // `trunc($D)` is CE0117 on 11.13.0. Adding names to funcTable on resemblance + // is how a write barrier stops catching anything. + {"trunc is not a Mendix built-in", "declare $d Decimal = trunc($x);", true, ""}, + {"currentDeviceType is not a Mendix built-in", "declare $b Boolean = currentDeviceType() = 'Phone';", true, ""}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/mdl/executor/validate_microflow_rules_exec_test.go b/mdl/executor/validate_microflow_rules_exec_test.go index 8ee753cd6..14c1ddf38 100644 --- a/mdl/executor/validate_microflow_rules_exec_test.go +++ b/mdl/executor/validate_microflow_rules_exec_test.go @@ -57,6 +57,31 @@ end;`, begin retrieve $L from M.Category where [Name = $P/Code]; return $L; +end;`, + }, + { + // MDL044 (#828): a call to a name Mendix has no built-in for. + // `currentDeviceType()` is CE0117 "Error(s) in expression." on + // mxbuild 11.13.0; before this rule was promoted, exec wrote the + // microflow and the failure surfaced only at build time. + name: "unknown expression function is rejected", + src: `create microflow M.ACT () returns Boolean +begin + declare $Result boolean = currentDeviceType() = 'Phone'; + return $Result; +end;`, + wantErr: "MDL044", + }, + { + // The counterpart that keeps the promotion honest: `isNew` is a real + // Mendix built-in (0 errors on 11.13.0) that funcTable was missing, so + // MDL044 flagged it. As a check-only rule that was a nuisance; as an + // exec barrier it would have refused valid MDL. + name: "isNew is a real built-in and is accepted", + src: `create microflow M.ACT ($Obj: M.Thing) returns Boolean +begin + declare $Flag boolean = isNew($Obj); + return $Flag; end;`, }, } diff --git a/mdl/executor/widget_dropdownfilter_assoc_test.go b/mdl/executor/widget_dropdownfilter_assoc_test.go new file mode 100644 index 000000000..8602c6ca9 --- /dev/null +++ b/mdl/executor/widget_dropdownfilter_assoc_test.go @@ -0,0 +1,92 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// upstream #830, second half: the drop-down filter's ASSOCIATION mode +// (`baseType: 'ref'`) was unauthorable. Every ref-mode property was unmapped in +// dropdownfilter.def.json, so `mxcli check` reported MDL-WIDGET01 "has no +// property `refEntity`" and exec dropped the lot — leaving no MDL way to filter +// a grid by a reference at all, which is what the issue asked for. +// +// It is now a def.json mode entered by giving the filter a `datasource:` (the +// OPTION list), mirroring the ComboBox's association mode. The four properties +// pinned here are the ones mxbuild reads; verified on 11.13.0 via `mx dump-mpr`: +// baseType="ref", refEntity=IndirectEntityRef{steps:[Order_Customer → Customer]}, +// refOptions=XPathSource{Customer}, refCaption=AttributeRef{ZKT38.Customer.Name}. +func TestDropdownFilter_AssociationModeMappings(t *testing.T) { + reg := LoadWidgetRegistry("") + if reg == nil { + t.Fatal("built-in widget registry not available") + } + def, ok := reg.Get("DROPDOWNFILTER") + if !ok { + t.Fatal("no DROPDOWNFILTER definition in the built-in registry") + } + engine := &PluggableWidgetEngine{} + + assoc := &ast.WidgetV3{ + Name: "ddf", + Type: "dropdownfilter", + Properties: map[string]any{ + "Association": "ZKT38.Order_Customer", + "CaptionAttribute": "Name", + "DataSource": &ast.DataSourceV3{Type: "database", Reference: "ZKT38.Customer"}, + }, + } + mappings, _, err := engine.selectMappings(def, assoc) + if err != nil { + t.Fatalf("selectMappings: %v", err) + } + got := map[string]PropertyMapping{} + for _, m := range mappings { + got[m.PropertyKey] = m + } + // The reference is stored on refEntity via the `association` operation — the + // WidgetValue has no association-valued field, so this writes an EntityRef + // with association steps, exactly like the ComboBox. + for key, wantOp := range map[string]string{ + "baseType": "primitive", + "refOptions": "datasource", + "refEntity": "association", + "refCaption": "attribute", + } { + m, ok := got[key] + if !ok { + t.Errorf("association mode does not map %q — the property is unwritable without it", key) + continue + } + if m.Operation != wantOp { + t.Errorf("%q operation = %q, want %q", key, m.Operation, wantOp) + } + } + if got["baseType"].Value != "ref" { + t.Errorf("baseType = %q, want the literal \"ref\" — anything else leaves the widget in attribute mode", + got["baseType"].Value) + } + + // A filter with no datasource is an ordinary column filter and must keep the + // attribute-mode mappings; the mode split must not disturb it. + plain := &ast.WidgetV3{Name: "ddf", Type: "dropdownfilter", Properties: map[string]any{}} + plainMappings, _, err := engine.selectMappings(def, plain) + if err != nil { + t.Fatalf("selectMappings (attribute mode): %v", err) + } + sawAttrChoice := false + for _, m := range plainMappings { + if m.PropertyKey == "baseType" { + t.Error("a datasource-less filter must stay in attribute mode, but baseType is being written") + } + if m.PropertyKey == "attrChoice" { + sawAttrChoice = true + } + } + if !sawAttrChoice { + t.Error("attribute mode lost its attrChoice mapping") + } +} diff --git a/mdl/executor/widget_engine.go b/mdl/executor/widget_engine.go index c69c302c8..27e3da195 100644 --- a/mdl/executor/widget_engine.go +++ b/mdl/executor/widget_engine.go @@ -659,6 +659,14 @@ func (e *PluggableWidgetEngine) resolveMapping(mapping PropertyMapping, w *ast.W attr = w.GetAttribute() } if attr != "" { + // An association named where an attribute belongs cannot be stored; + // refuse it here rather than writing a dangling AttributeRef (#830). + if err := e.pageBuilder.rejectAssociationAsAttribute( + attr, e.pageBuilder.entityContext, + fmt.Sprintf("widget `%s` property `%s`", w.Name, mapping.PropertyKey), + ); err != nil { + return nil, err + } ctx.AttributePath = e.pageBuilder.resolveAttributePath(attr) } @@ -1073,6 +1081,14 @@ func (e *PluggableWidgetEngine) buildObjectListItem(mapping *ObjectListMapping, prop.AttributePath = finalQN prop.AttributeRefSteps = steps } else if e.pageBuilder.entityContext != "" { + // A bare ASSOCIATION name here is not representable — it would be + // written as an AttributeRef and fail CE1613 (issue #830). + if err := e.pageBuilder.rejectAssociationAsAttribute( + strVal, e.pageBuilder.entityContext, + fmt.Sprintf("%s `%s` property `%s`", mapping.MDLContainer, child.Name, ip.PropertyKey), + ); err != nil { + return spec, err + } prop.AttributePath = e.pageBuilder.resolveAttributePath(strVal) } else { prop.AttributePath = strVal diff --git a/mdl/exprcheck/func_checker.go b/mdl/exprcheck/func_checker.go index 46e7aa41e..33826e14b 100644 --- a/mdl/exprcheck/func_checker.go +++ b/mdl/exprcheck/func_checker.go @@ -73,6 +73,17 @@ var funcTable = map[string]funcSig{ "getCaption": {args: []TypeKind{KindAny}, ret: KindString}, "getKey": {args: []TypeKind{KindAny}, ret: KindString}, + // Special checks — the object-state predicates. Each takes an object and + // returns Boolean; `isSynced`/`isSyncing` are offline-sync predicates and are + // nanoflow-only, but that is a context restriction, not an unknown name. + // All three were missing from this table, so MDL044 flagged them as + // hallucinated; each was built against mxbuild 11.13.0 at 0 errors before + // being added here (the table is the sole allow-list MDL044 consults, and + // MDL044 is now enforced on the exec path). + "isNew": {args: []TypeKind{KindAny}, ret: KindBoolean}, + "isSynced": {args: []TypeKind{KindAny}, ret: KindBoolean}, + "isSyncing": {args: []TypeKind{KindAny}, ret: KindBoolean}, + // DateTime — construction "currentDateTime": {args: []TypeKind{}, ret: KindDateTime}, // dateTime/dateTimeUTC(year, month, day [, hour, minute, second]) — 3 or 6 args diff --git a/mdl/exprcheck/unknown_funcs.go b/mdl/exprcheck/unknown_funcs.go index b28f13b98..faed5e82e 100644 --- a/mdl/exprcheck/unknown_funcs.go +++ b/mdl/exprcheck/unknown_funcs.go @@ -140,8 +140,13 @@ func levenshtein(a, b string) int { // roundingFuncs are the Decimal-returning built-ins whose result Mendix DOES // accept in an Integer/Long target (they yield a whole number). They must not be // flagged by SourceRejectedForIntegerTarget. +// +// `trunc` used to be listed here. Mendix has no such built-in — `trunc($D)` +// fails the build with CE0117 on 11.13.0 — and listing it invited "fix" the +// MDL044 report by adding it to funcTable, which would let the bad expression +// through. MDL044 flags it, correctly. var roundingFuncs = map[string]bool{ - "round": true, "floor": true, "ceil": true, "trunc": true, + "round": true, "floor": true, "ceil": true, } // SourceRejectedForIntegerTarget reports whether assigning src to an Integer/Long diff --git a/mdl/grammar/MDLParser.g4 b/mdl/grammar/MDLParser.g4 index 7b7d9e63f..cebf49dd0 100644 --- a/mdl/grammar/MDLParser.g4 +++ b/mdl/grammar/MDLParser.g4 @@ -246,6 +246,7 @@ alterLayoutMapping alterPageAssignment : DATASOURCE EQUALS dataSourceExprV3 // DataSource = SELECTION widgetName + | ACTION EQUALS actionExprV3 // Action = MICROFLOW Module.MF | SHOW_PAGE Module.Page | SAVE_CHANGES CLOSE_PAGE | VISIBLE EQUALS xpathConstraint // Visible = [Name != ''] (conditional visibility) | EDITABLE EQUALS xpathConstraint // Editable = [Status = 'Open'] (conditional editability) | identifierOrKeyword EQUALS propertyValueV3 // Caption = 'Save' diff --git a/mdl/grammar/domains/MDLDomainModel.g4 b/mdl/grammar/domains/MDLDomainModel.g4 index b35a9f78a..e7e31d312 100644 --- a/mdl/grammar/domains/MDLDomainModel.g4 +++ b/mdl/grammar/domains/MDLDomainModel.g4 @@ -234,6 +234,15 @@ alterAssociationAction | SET OWNER (DEFAULT | BOTH) | SET STORAGE (COLUMN | TABLE) | SET COMMENT STRING_LITERAL + // Line anchors: where the connector attaches to each entity box, as a + // PERCENTAGE of the box (0..100). Both ends together — the pair is one + // visual decision, and `from`/`to` are the association's own words for its + // two ends. Mirrors `alter entity ... set position (x, y)`. (issue #872) + | SET ANCHOR FROM anchorPoint TO anchorPoint + ; + +anchorPoint + : LPAREN NUMBER_LITERAL COMMA NUMBER_LITERAL RPAREN ; alterEnumerationAction diff --git a/mdl/grammar/domains/MDLSettings.g4 b/mdl/grammar/domains/MDLSettings.g4 index b65ca2fa4..9db3b5d0f 100644 --- a/mdl/grammar/domains/MDLSettings.g4 +++ b/mdl/grammar/domains/MDLSettings.g4 @@ -170,17 +170,25 @@ debugStatement /** * SQL statements for external database connectivity. */ +/* + * Connection aliases, driver names and table names are user-chosen words, so + * they take identifierOrKeyword rather than bare IDENTIFIER — `source` lexes as + * SOURCE_KW, and it is both the most natural alias and the one used in mxcli's + * own documentation. IMPORT FROM (below) already accepts an alias this way; the + * SQL statements were the odd ones out, and a bare IDENTIFIER there meant + * `SQL DISCONNECT source` did not parse at all. + */ sqlStatement - : SQL CONNECT IDENTIFIER STRING_LITERAL AS IDENTIFIER # sqlConnect - | SQL DISCONNECT IDENTIFIER # sqlDisconnect + : SQL CONNECT identifierOrKeyword STRING_LITERAL AS identifierOrKeyword # sqlConnect + | SQL DISCONNECT identifierOrKeyword # sqlDisconnect | SQL CONNECTIONS # sqlConnections - | SQL IDENTIFIER SHOW IDENTIFIER # sqlShowTables - | SQL IDENTIFIER DESCRIBE IDENTIFIER # sqlDescribeTable - | SQL IDENTIFIER GENERATE CONNECTOR INTO identifierOrKeyword + | SQL identifierOrKeyword SHOW identifierOrKeyword # sqlShowTables + | SQL identifierOrKeyword DESCRIBE identifierOrKeyword # sqlDescribeTable + | SQL identifierOrKeyword GENERATE CONNECTOR INTO identifierOrKeyword (TABLES LPAREN identifierOrKeyword (COMMA identifierOrKeyword)* RPAREN)? (VIEWS LPAREN identifierOrKeyword (COMMA identifierOrKeyword)* RPAREN)? EXEC? # sqlGenerateConnector - | SQL IDENTIFIER sqlPassthrough # sqlQuery + | SQL identifierOrKeyword sqlPassthrough # sqlQuery ; sqlPassthrough diff --git a/mdl/visitor/visitor_alter_page.go b/mdl/visitor/visitor_alter_page.go index 82c800034..a2b79abd8 100644 --- a/mdl/visitor/visitor_alter_page.go +++ b/mdl/visitor/visitor_alter_page.go @@ -111,6 +111,15 @@ func (b *Builder) buildAlterPageAssignment(ctx *parser.AlterPageAssignmentContex return "DataSource", buildDataSourceV3(dsCtx) } + // Action = actionExprV3 — the same action grammar CREATE PAGE uses, so every + // form is available here (MICROFLOW/NANOFLOW with arguments, SHOW_PAGE, + // SAVE_CHANGES CLOSE_PAGE, CREATE_OBJECT … THEN …). Retargeting a button was + // previously only possible by REPLACEing the whole widget, which silently + // drops any property the author did not restate. + if acCtx := ctx.ActionExprV3(); acCtx != nil { + return "Action", buildActionV3(acCtx) + } + // Visible = [expr] / Editable = [expr] — conditional visibility/editability. // Same context-rooting as CREATE PAGE (issue #627): bare attributes become // $currentObject/Attr. Routed to VisibleIf/EditableIf so the mutator builds a diff --git a/mdl/visitor/visitor_alter_page_action_test.go b/mdl/visitor/visitor_alter_page_action_test.go new file mode 100644 index 000000000..69f2459e7 --- /dev/null +++ b/mdl/visitor/visitor_alter_page_action_test.go @@ -0,0 +1,142 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// TestAlterPage_SetAction_Parses covers the grammar half of the fix. +// +// `alterPageAssignment` special-cased DataSource, Visible and Editable and then +// fell through to `propertyValueV3`, which has no `microflow ` form — so +// `SET Action = microflow M.F ON btn` did not parse at all. Retargeting a button +// meant REPLACEing the whole widget, which silently drops any property the +// author did not restate. +// +// The rule now reuses actionExprV3, the same action grammar CREATE PAGE uses, so +// every form is available rather than a subset that has to be extended one bug +// report at a time. +func TestAlterPage_SetAction_Parses(t *testing.T) { + tests := []struct { + name string + src string + verify func(t *testing.T, a *ast.ActionV3) + }{ + { + name: "microflow", + src: "alter page M.P { set Action = microflow M.ACT_Other on btnGo; };", + verify: func(t *testing.T, a *ast.ActionV3) { + if a.Type != "microflow" { + t.Errorf("Type = %q, want microflow", a.Type) + } + if a.Target != "M.ACT_Other" { + t.Errorf("Target = %q, want M.ACT_Other", a.Target) + } + }, + }, + { + name: "nanoflow", + src: "alter page M.P { set Action = nanoflow M.NF_Other on btnGo; };", + verify: func(t *testing.T, a *ast.ActionV3) { + if a.Type != "nanoflow" { + t.Errorf("Type = %q, want nanoflow", a.Type) + } + }, + }, + { + name: "save changes with close", + src: "alter page M.P { set Action = SAVE_CHANGES CLOSE_PAGE on btnGo; };", + verify: func(t *testing.T, a *ast.ActionV3) { + if a.Type != "save" { + t.Errorf("Type = %q, want save", a.Type) + } + if !a.ClosePage { + t.Error("ClosePage = false, want true — the close is a flag on the action") + } + }, + }, + { + name: "save changes without close", + src: "alter page M.P { set Action = SAVE_CHANGES on btnGo; };", + verify: func(t *testing.T, a *ast.ActionV3) { + if a.Type != "save" || a.ClosePage { + t.Errorf("Type = %q ClosePage = %v, want save/false", a.Type, a.ClosePage) + } + }, + }, + { + name: "show page", + src: "alter page M.P { set Action = SHOW_PAGE M.Other on btnGo; };", + verify: func(t *testing.T, a *ast.ActionV3) { + if a.Type != "showPage" { + t.Errorf("Type = %q, want showPage", a.Type) + } + if a.Target != "M.Other" { + t.Errorf("Target = %q, want M.Other", a.Target) + } + }, + }, + { + name: "open link", + src: "alter page M.P { set Action = OPEN_LINK 'https://example.com' on btnGo; };", + verify: func(t *testing.T, a *ast.ActionV3) { + if a.Type == "" { + t.Error("Type is empty") + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + prog, errs := Build(tt.src) + if len(errs) > 0 { + t.Fatalf("Build(%q): %v", tt.src, errs) + } + if len(prog.Statements) != 1 { + t.Fatalf("statements = %d, want 1", len(prog.Statements)) + } + stmt, ok := prog.Statements[0].(*ast.AlterPageStmt) + if !ok { + t.Fatalf("type = %T, want *ast.AlterPageStmt", prog.Statements[0]) + } + if len(stmt.Operations) != 1 { + t.Fatalf("operations = %d, want 1", len(stmt.Operations)) + } + op, ok := stmt.Operations[0].(*ast.SetPropertyOp) + if !ok { + t.Fatalf("op type = %T, want *ast.SetPropertyOp", stmt.Operations[0]) + } + if op.Target.Widget != "btnGo" { + t.Errorf("target widget = %q, want btnGo", op.Target.Widget) + } + raw, present := op.Properties["Action"] + if !present { + t.Fatalf("no Action property; got %v", op.Properties) + } + action, ok := raw.(*ast.ActionV3) + if !ok { + t.Fatalf("Action value type = %T, want *ast.ActionV3 — a scalar here would be "+ + "written as a plain property and produce an unopenable document", raw) + } + tt.verify(t, action) + }) + } +} + +// TestAlterPage_SetAction_DoesNotShadowOtherProperties makes sure adding the +// ACTION alternative did not capture assignments that should still go through +// the generic identifier path — `ActionName` is not `Action`. +func TestAlterPage_SetAction_DoesNotShadowOtherProperties(t *testing.T) { + prog, errs := Build("alter page M.P { set Caption = 'Save' on btnGo; };") + if len(errs) > 0 { + t.Fatalf("Build: %v", errs) + } + op := prog.Statements[0].(*ast.AlterPageStmt).Operations[0].(*ast.SetPropertyOp) + if got, ok := op.Properties["Caption"]; !ok || got != "Save" { + t.Errorf("Caption = %v (present=%v), want Save", got, ok) + } +} diff --git a/mdl/visitor/visitor_association.go b/mdl/visitor/visitor_association.go index 876ddcdcb..9860847cb 100644 --- a/mdl/visitor/visitor_association.go +++ b/mdl/visitor/visitor_association.go @@ -3,6 +3,10 @@ package visitor import ( + "fmt" + "strconv" + "strings" + "github.com/mendixlabs/mxcli/mdl/ast" "github.com/mendixlabs/mxcli/mdl/grammar/parser" ) @@ -69,10 +73,104 @@ func (b *Builder) ExitCreateAssociationStatement(ctx *parser.CreateAssociationSt if createStmt.OR() != nil && (createStmt.MODIFY() != nil || createStmt.REPLACE() != nil) { stmt.CreateOrModify = true } + stmt.FromAnchor, stmt.ToAnchor = b.anchorAnnotation(createStmt) } b.statements = append(b.statements, stmt) } +// anchorAnnotation reads `@anchor(from: (x, y), to: (x, y))` off a CREATE +// ASSOCIATION statement — the association's line anchors, as a percentage of +// each entity box (0..100). +// +// No grammar rule was needed: `annotation*` is generic across every CREATE +// statement, `annotationParamName` already admits FROM and TO, and a +// parenthesised value is the existing `annotationParenValue` (which the +// microflow `@anchor(true: (from: right, to: left), …)` form uses). This is the +// same annotation name and the same two parameter names as the microflow-flow +// anchor, asking the same question — where does the connector attach — and +// differing only in the value type, because an association's endpoint is a +// continuous point on the box rather than one of four sides. +// +// Either end may be omitted; a nil result means "say nothing", which preserves +// whatever is stored rather than resetting it. (issue #872) +func (b *Builder) anchorAnnotation(createStmt *parser.CreateStatementContext) (from, to *ast.Position) { + for _, annCtx := range createStmt.AllAnnotation() { + ann := annCtx.(*parser.AnnotationContext) + if !strings.EqualFold(ann.AnnotationName().GetText(), "anchor") { + continue + } + params := ann.AnnotationParams() + if params == nil { + continue + } + for _, p := range params.(*parser.AnnotationParamsContext).AllAnnotationParam() { + paramCtx := p.(*parser.AnnotationParamContext) + nameCtx := paramCtx.AnnotationParamName() + if nameCtx == nil { + continue + } + pt := b.annotationParenPoint(paramCtx) + if pt == nil { + continue + } + switch strings.ToLower(nameCtx.GetText()) { + case "from": + from = pt + case "to": + to = pt + } + } + } + return from, to +} + +// annotationParenPoint reads a `(x, y)` parenthesised annotation value into a +// Position. Returns nil for any other shape — including the microflow anchor's +// `(from: right, to: left)`, whose params are named rather than positional, so +// the two `@anchor` forms cannot be confused for one another. +func (b *Builder) annotationParenPoint(paramCtx *parser.AnnotationParamContext) *ast.Position { + paren := paramCtx.AnnotationParenValue() + if paren == nil { + return nil + } + inner := paren.(*parser.AnnotationParenValueContext).AnnotationParams() + if inner == nil { + return nil + } + coords := inner.(*parser.AnnotationParamsContext).AllAnnotationParam() + if len(coords) != 2 { + return nil + } + for _, c := range coords { + if c.(*parser.AnnotationParamContext).AnnotationParamName() != nil { + return nil // named, not a coordinate pair + } + } + x, okX := anchorCoord(coords[0].GetText()) + y, okY := anchorCoord(coords[1].GetText()) + if !okX || !okY { + b.addErrorWithExample( + fmt.Sprintf("anchor coordinate (%s, %s) is not a whole number — Mendix stores line anchors as two integers, "+ + "a percentage of the entity box (0..100), and refuses to LOAD a project whose anchor is anything else", + strings.TrimSpace(coords[0].GetText()), strings.TrimSpace(coords[1].GetText())), + "@anchor(from: (0, 54), to: (100, 54))") + return nil + } + return &ast.Position{X: x, Y: y} +} + +// anchorCoord parses one anchor coordinate. Mendix stores the pair as two +// INTEGERS and its loader rejects anything else outright — a hand-patched +// "0.5;50" fails with StorageLoadException before validation even runs — so a +// non-integer must be refused here rather than silently truncated to 0. +func anchorCoord(text string) (int, bool) { + v, err := strconv.Atoi(strings.TrimSpace(text)) + if err != nil { + return 0, false + } + return v, true +} + // ExitAlterAssociationAction handles ALTER ASSOCIATION ... SET ... actions. func (b *Builder) ExitAlterAssociationAction(ctx *parser.AlterAssociationActionContext) { // Walk up to the parent AlterStatement to get the association's qualified name @@ -128,6 +226,20 @@ func (b *Builder) ExitAlterAssociationAction(ctx *parser.AlterAssociationActionC return } + // SET ANCHOR FROM (x, y) TO (x, y) + if ctx.ANCHOR() != nil { + pts := ctx.AllAnchorPoint() + if len(pts) == 2 { + b.statements = append(b.statements, &ast.AlterAssociationStmt{ + Name: name, + Operation: ast.AlterAssociationSetAnchor, + FromAnchor: b.buildAnchorPoint(pts[0]), + ToAnchor: b.buildAnchorPoint(pts[1]), + }) + } + return + } + // SET COMMENT if ctx.COMMENT() != nil && ctx.STRING_LITERAL() != nil { b.statements = append(b.statements, &ast.AlterAssociationStmt{ @@ -149,3 +261,25 @@ func (b *Builder) ExitAlterAssociationAction(ctx *parser.AlterAssociationActionC // ---------------------------------------------------------------------------- // ExitShowStatement handles SHOW MODULES/ENTITIES/ASSOCIATIONS/etc. + +// buildAnchorPoint reads the `(x, y)` of a SET ANCHOR clause. +func (b *Builder) buildAnchorPoint(ctx parser.IAnchorPointContext) *ast.Position { + if ctx == nil { + return nil + } + nums := ctx.(*parser.AnchorPointContext).AllNUMBER_LITERAL() + if len(nums) != 2 { + return nil + } + x, okX := anchorCoord(nums[0].GetText()) + y, okY := anchorCoord(nums[1].GetText()) + if !okX || !okY { + b.addErrorWithExample( + fmt.Sprintf("anchor coordinate (%s, %s) is not a whole number — Mendix stores line anchors as two integers, "+ + "a percentage of the entity box (0..100), and refuses to LOAD a project whose anchor is anything else", + nums[0].GetText(), nums[1].GetText()), + "alter association Module.A_B set anchor from (0, 54) to (100, 54);") + return nil + } + return &ast.Position{X: x, Y: y} +} diff --git a/mdl/visitor/visitor_association_anchor_test.go b/mdl/visitor/visitor_association_anchor_test.go new file mode 100644 index 000000000..52b5b8d06 --- /dev/null +++ b/mdl/visitor/visitor_association_anchor_test.go @@ -0,0 +1,144 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// upstream #872, third slice: authoring an association's line anchors. +// +// The syntax reuses the EXISTING `@anchor` annotation and its `from:`/`to:` +// parameter names — the same annotation the microflow sequence-flow anchor uses, +// asking the same question (where does the connector attach), differing only in +// the value type. An association endpoint is a continuous point on the entity +// box (a percentage, 0..100), not one of four sides, so it takes a coordinate +// pair. No grammar rule was needed: `annotation*` is generic across every CREATE +// statement, `annotationParamName` already admits FROM and TO, and `(x, y)` is +// the existing `annotationParenValue` with two positional values. +func TestCreateAssociation_AnchorAnnotation(t *testing.T) { + cases := []struct { + name string + src string + wantFrom *ast.Position + wantTo *ast.Position + }{ + { + name: "both ends", + src: `@anchor(from: (0, 54), to: (100, 54)) +create association M.Child_Parent from M.Child to M.Parent;`, + wantFrom: &ast.Position{X: 0, Y: 54}, + wantTo: &ast.Position{X: 100, Y: 54}, + }, + { + // Real Studio Pro values from Workflow Commons — neither coordinate + // is on an edge, which is exactly what a named-anchor vocabulary + // could not have expressed. + name: "an off-edge pair", + src: `@anchor(from: (11, 99), to: (9, 0)) +create association M.Child_Parent from M.Child to M.Parent;`, + wantFrom: &ast.Position{X: 11, Y: 99}, + wantTo: &ast.Position{X: 9, Y: 0}, + }, + { + // (0,0) is the box's top-left, a real anchor. It must survive as a + // value rather than being read as "nothing was said". + name: "the zero point is authorable", + src: `@anchor(from: (0, 0), to: (0, 0)) +create association M.Child_Parent from M.Child to M.Parent;`, + wantFrom: &ast.Position{X: 0, Y: 0}, + wantTo: &ast.Position{X: 0, Y: 0}, + }, + { + name: "no annotation leaves both nil, which preserves what is stored", + src: `create association M.Child_Parent from M.Child to M.Parent;`, + wantFrom: nil, + wantTo: nil, + }, + { + name: "one end only", + src: `@anchor(from: (25, 100)) +create association M.Child_Parent from M.Child to M.Parent;`, + wantFrom: &ast.Position{X: 25, Y: 100}, + wantTo: nil, + }, + { + // The microflow-flow spelling of @anchor names its inner params, so + // it must not be misread as a coordinate pair on an association. + name: "the microflow side-named form is not a coordinate pair", + src: "@anchor(from: right, to: left)\ncreate association M.Child_Parent from M.Child to M.Parent;", + wantFrom: nil, + wantTo: nil, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + prog, errs := Build(tc.src) + if len(errs) > 0 { + t.Fatalf("parse errors: %v", errs) + } + stmt, ok := prog.Statements[0].(*ast.CreateAssociationStmt) + if !ok { + t.Fatalf("statement is %T, want *ast.CreateAssociationStmt", prog.Statements[0]) + } + assertAnchor(t, "from", stmt.FromAnchor, tc.wantFrom) + assertAnchor(t, "to", stmt.ToAnchor, tc.wantTo) + }) + } +} + +func TestAlterAssociation_SetAnchor(t *testing.T) { + prog, errs := Build(`alter association M.Child_Parent set anchor from (50, 100) to (50, 0);`) + if len(errs) > 0 { + t.Fatalf("parse errors: %v", errs) + } + stmt, ok := prog.Statements[0].(*ast.AlterAssociationStmt) + if !ok { + t.Fatalf("statement is %T, want *ast.AlterAssociationStmt", prog.Statements[0]) + } + if stmt.Operation != ast.AlterAssociationSetAnchor { + t.Errorf("Operation = %v, want AlterAssociationSetAnchor", stmt.Operation) + } + assertAnchor(t, "from", stmt.FromAnchor, &ast.Position{X: 50, Y: 100}) + assertAnchor(t, "to", stmt.ToAnchor, &ast.Position{X: 50, Y: 0}) +} + +// A non-integer coordinate must be REPORTED, not silently truncated. Mendix +// stores the pair as two integers and its loader refuses to open a project whose +// anchor is anything else (StorageLoadException, before validation runs) — so +// accepting "0.5" and writing 0 would produce a value the author never asked for +// in a file that at least still loads, which is the worse of the two failures. +func TestAssociationAnchor_NonIntegerIsRejected(t *testing.T) { + for _, src := range []string{ + "@anchor(from: (0.5, 54), to: (100, 54))\ncreate association M.Child_Parent from M.Child to M.Parent;", + "alter association M.Child_Parent set anchor from (0.5, 54) to (100, 54);", + } { + _, errs := Build(src) + if len(errs) == 0 { + t.Fatalf("expected an error for a fractional anchor coordinate: %s", src) + } + joined := "" + for _, e := range errs { + joined += e.Error() + "\n" + } + if !strings.Contains(joined, "whole number") { + t.Errorf("error should explain the integer requirement, got: %s", joined) + } + } +} + +func assertAnchor(t *testing.T, side string, got, want *ast.Position) { + t.Helper() + switch { + case want == nil && got != nil: + t.Errorf("%s anchor = %+v, want nil (nothing said ⇒ preserve what is stored)", side, *got) + case want != nil && got == nil: + t.Errorf("%s anchor = nil, want %+v", side, *want) + case want != nil && *got != *want: + t.Errorf("%s anchor = %+v, want %+v", side, *got, *want) + } +} diff --git a/mdl/visitor/visitor_sql.go b/mdl/visitor/visitor_sql.go index 53c316587..160f9298d 100644 --- a/mdl/visitor/visitor_sql.go +++ b/mdl/visitor/visitor_sql.go @@ -14,28 +14,47 @@ import ( // SQL Statements (external database connectivity) // ---------------------------------------------------------------------------- +// sqlWords returns the identifierOrKeyword texts of a SQL statement context. +// +// ANTLR still walks the tree after a syntax error, so a listener can be handed a +// context whose children are missing. Reading them through this helper (and +// checking the count) keeps a malformed statement an error rather than a crash: +// `SQL DISCONNECT source` used to segfault `mxcli check` on ctx.IDENTIFIER() +// returning nil, because `source` lexes as SOURCE_KW and the rule wanted a bare +// IDENTIFIER. +func sqlWords(all []parser.IIdentifierOrKeywordContext) []string { + out := make([]string, 0, len(all)) + for _, c := range all { + iok, ok := c.(*parser.IdentifierOrKeywordContext) + if !ok || iok == nil { + continue + } + out = append(out, identifierOrKeywordText(iok)) + } + return out +} + // ExitSqlConnect handles SQL CONNECT '' AS func (b *Builder) ExitSqlConnect(ctx *parser.SqlConnectContext) { - ids := ctx.AllIDENTIFIER() - if len(ids) < 2 { + words := sqlWords(ctx.AllIdentifierOrKeyword()) + if len(words) < 2 || ctx.STRING_LITERAL() == nil { return } - driver := ids[0].GetText() - dsn := unquoteString(ctx.STRING_LITERAL().GetText()) - alias := ids[1].GetText() - b.statements = append(b.statements, &ast.SQLConnectStmt{ - Driver: driver, - DSN: dsn, - Alias: alias, + Driver: words[0], + DSN: unquoteString(ctx.STRING_LITERAL().GetText()), + Alias: words[1], }) } // ExitSqlDisconnect handles SQL DISCONNECT func (b *Builder) ExitSqlDisconnect(ctx *parser.SqlDisconnectContext) { - alias := ctx.IDENTIFIER().GetText() + words := sqlWords([]parser.IIdentifierOrKeywordContext{ctx.IdentifierOrKeyword()}) + if len(words) < 1 { + return + } b.statements = append(b.statements, &ast.SQLDisconnectStmt{ - Alias: alias, + Alias: words[0], }) } @@ -46,12 +65,12 @@ func (b *Builder) ExitSqlConnections(ctx *parser.SqlConnectionsContext) { // ExitSqlShowTables handles SQL SHOW TABLES|VIEWS|FUNCTIONS func (b *Builder) ExitSqlShowTables(ctx *parser.SqlShowTablesContext) { - ids := ctx.AllIDENTIFIER() - if len(ids) < 2 { + words := sqlWords(ctx.AllIdentifierOrKeyword()) + if len(words) < 2 { return } - alias := ids[0].GetText() - target := strings.ToUpper(ids[1].GetText()) + alias := words[0] + target := strings.ToUpper(words[1]) switch target { case "VIEWS": @@ -66,12 +85,12 @@ func (b *Builder) ExitSqlShowTables(ctx *parser.SqlShowTablesContext) { // ExitSqlDescribeTable handles SQL DESCRIBE func (b *Builder) ExitSqlDescribeTable(ctx *parser.SqlDescribeTableContext) { - ids := ctx.AllIDENTIFIER() - if len(ids) < 2 { + words := sqlWords(ctx.AllIdentifierOrKeyword()) + if len(words) < 2 { return } - alias := ids[0].GetText() - table := ids[1].GetText() + alias := words[0] + table := words[1] b.statements = append(b.statements, &ast.SQLDescribeTableStmt{ Alias: alias, Table: table, @@ -161,14 +180,16 @@ func (b *Builder) ExitImportFromQuery(ctx *parser.ImportFromQueryContext) { // ExitSqlGenerateConnector handles SQL GENERATE CONNECTOR INTO [TABLES (...)] [VIEWS (...)] [EXEC] func (b *Builder) ExitSqlGenerateConnector(ctx *parser.SqlGenerateConnectorContext) { - alias := ctx.IDENTIFIER().GetText() - + // The alias is now the first identifierOrKeyword of the rule (it used to be a + // separate IDENTIFIER token), so the module is [1] and the table/view lists + // start at [2]. allIOK := ctx.AllIdentifierOrKeyword() - if len(allIOK) == 0 { + if len(allIOK) < 2 { return } - module := identifierOrKeywordText(allIOK[0]) - rest := allIOK[1:] + alias := identifierOrKeywordText(allIOK[0]) + module := identifierOrKeywordText(allIOK[1]) + rest := allIOK[2:] hasTables := ctx.TABLES() != nil hasViews := ctx.VIEWS() != nil @@ -214,11 +235,12 @@ func (b *Builder) ExitSqlGenerateConnector(ctx *parser.SqlGenerateConnectorConte // ExitSqlQuery handles SQL func (b *Builder) ExitSqlQuery(ctx *parser.SqlQueryContext) { - alias := ctx.IDENTIFIER().GetText() + words := sqlWords([]parser.IIdentifierOrKeywordContext{ctx.IdentifierOrKeyword()}) passthrough := ctx.SqlPassthrough() - if passthrough == nil { + if len(words) < 1 || passthrough == nil { return } + alias := words[0] query := getSpacedText(passthrough) b.statements = append(b.statements, &ast.SQLQueryStmt{ Alias: alias, diff --git a/mdl/visitor/visitor_sql_alias_test.go b/mdl/visitor/visitor_sql_alias_test.go new file mode 100644 index 000000000..70c642db9 --- /dev/null +++ b/mdl/visitor/visitor_sql_alias_test.go @@ -0,0 +1,175 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// TestSQLAlias_KeywordNames covers the crash found by the syntax-registry +// example guard: `SQL DISCONNECT source` segfaulted `mxcli check`. +// +// `source` lexes as SOURCE_KW, so the rule's bare IDENTIFIER did not match. +// ANTLR error-recovered and still walked the tree, so the listener ran against a +// context whose IDENTIFIER() was nil and `.GetText()` panicked. Two things were +// wrong: the alias should accept a keyword (IMPORT FROM already did), and no +// listener should dereference a child without checking it. +// +// `source` is not a cherry-picked case — it is the alias in mxcli's own +// documented example for `mxcli syntax sql`. +func TestSQLAlias_KeywordNames(t *testing.T) { + // Words that are MDL keywords and plausible connection/table names. + for _, alias := range []string{"source", "table", "query", "view", "index", "key", "mydb"} { + t.Run(alias, func(t *testing.T) { + prog, errs := Build("SQL DISCONNECT " + alias + ";") + if len(errs) > 0 { + t.Fatalf("SQL DISCONNECT %s: %v", alias, errs) + } + if len(prog.Statements) != 1 { + t.Fatalf("statements = %d, want 1", len(prog.Statements)) + } + stmt, ok := prog.Statements[0].(*ast.SQLDisconnectStmt) + if !ok { + t.Fatalf("statement type = %T, want *ast.SQLDisconnectStmt", prog.Statements[0]) + } + if stmt.Alias != alias { + t.Errorf("Alias = %q, want %q", stmt.Alias, alias) + } + }) + } +} + +// TestSQLStatements_KeywordAliasRoundTrip walks the whole SQL surface with a +// keyword alias, so a future rule that reverts to bare IDENTIFIER is caught on +// the statement it breaks rather than on DISCONNECT alone. +func TestSQLStatements_KeywordAliasRoundTrip(t *testing.T) { + tests := []struct { + name string + src string + check func(t *testing.T, stmt ast.Statement) + }{ + { + name: "connect", + src: "SQL CONNECT postgres 'postgres://u:p@localhost:5432/db' AS source;", + check: func(t *testing.T, stmt ast.Statement) { + s, ok := stmt.(*ast.SQLConnectStmt) + if !ok { + t.Fatalf("type = %T", stmt) + } + if s.Alias != "source" || s.Driver != "postgres" { + t.Errorf("got driver=%q alias=%q, want postgres/source", s.Driver, s.Alias) + } + if !strings.Contains(s.DSN, "localhost:5432") { + t.Errorf("DSN = %q", s.DSN) + } + }, + }, + { + name: "show tables", + src: "SQL source SHOW TABLES;", + check: func(t *testing.T, stmt ast.Statement) { + s, ok := stmt.(*ast.SQLShowTablesStmt) + if !ok { + t.Fatalf("type = %T", stmt) + } + if s.Alias != "source" { + t.Errorf("Alias = %q, want source", s.Alias) + } + }, + }, + { + name: "describe table", + src: "SQL source DESCRIBE users;", + check: func(t *testing.T, stmt ast.Statement) { + s, ok := stmt.(*ast.SQLDescribeTableStmt) + if !ok { + t.Fatalf("type = %T", stmt) + } + if s.Alias != "source" || s.Table != "users" { + t.Errorf("got alias=%q table=%q, want source/users", s.Alias, s.Table) + } + }, + }, + { + name: "query", + src: "SQL source SELECT * FROM users LIMIT 10;", + check: func(t *testing.T, stmt ast.Statement) { + s, ok := stmt.(*ast.SQLQueryStmt) + if !ok { + t.Fatalf("type = %T", stmt) + } + if s.Alias != "source" { + t.Errorf("Alias = %q, want source", s.Alias) + } + if !strings.Contains(strings.ToUpper(s.Query), "SELECT") { + t.Errorf("Query = %q", s.Query) + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + prog, errs := Build(tt.src) + if len(errs) > 0 { + t.Fatalf("Build(%q): %v", tt.src, errs) + } + if len(prog.Statements) != 1 { + t.Fatalf("statements = %d, want 1", len(prog.Statements)) + } + tt.check(t, prog.Statements[0]) + }) + } +} + +// TestSQLGenerateConnector_KeywordAlias pins the index shift: the alias joined +// the identifierOrKeyword list, so the module moved from [0] to [1] and the +// table/view names from [1:] to [2:]. Getting that wrong would silently generate +// a connector into a module named after the connection. +func TestSQLGenerateConnector_KeywordAlias(t *testing.T) { + prog, errs := Build("SQL source GENERATE CONNECTOR INTO MyModule TABLES (users, orders);") + if len(errs) > 0 { + t.Fatalf("Build: %v", errs) + } + if len(prog.Statements) != 1 { + t.Fatalf("statements = %d, want 1", len(prog.Statements)) + } + s, ok := prog.Statements[0].(*ast.SQLGenerateConnectorStmt) + if !ok { + t.Fatalf("type = %T", prog.Statements[0]) + } + if s.Alias != "source" { + t.Errorf("Alias = %q, want source", s.Alias) + } + if s.Module != "MyModule" { + t.Errorf("Module = %q, want MyModule", s.Module) + } + if len(s.Tables) != 2 || s.Tables[0] != "users" || s.Tables[1] != "orders" { + t.Errorf("Tables = %v, want [users orders]", s.Tables) + } +} + +// TestSQLDisconnect_MalformedDoesNotPanic is the robustness half. ANTLR keeps +// walking after a syntax error, so a listener must tolerate missing children — +// an unparseable statement is an error to report, never a crash. +func TestSQLDisconnect_MalformedDoesNotPanic(t *testing.T) { + for _, src := range []string{ + "SQL DISCONNECT;", + "SQL DISCONNECT 'quoted';", + "SQL DISCONNECT 123;", + "SQL CONNECT postgres AS;", + "SQL DESCRIBE;", + } { + t.Run(src, func(t *testing.T) { + defer func() { + if r := recover(); r != nil { + t.Fatalf("Build(%q) panicked: %v", src, r) + } + }() + _, _ = Build(src) + }) + } +} diff --git a/sdk/domainmodel/connection.go b/sdk/domainmodel/connection.go new file mode 100644 index 000000000..6aefe0b27 --- /dev/null +++ b/sdk/domainmodel/connection.go @@ -0,0 +1,69 @@ +// SPDX-License-Identifier: Apache-2.0 + +package domainmodel + +import ( + "fmt" + "strconv" + "strings" + + "github.com/mendixlabs/mxcli/model" +) + +// An association's line anchors — where the connector attaches to the FROM +// (ParentConnection) and TO (ChildConnection) entity boxes in the domain model +// editor. Mendix stores each as the string "x;y", a PERCENTAGE of the entity box +// in 0..100 (measured across 88 pairs in four Studio-Pro-authored sources; and +// it cannot be pixels, because DomainModels$EntityImpl stores no size — the box +// is sized by the editor from the name and attribute list). +// +// These are the values mxcli writes for a NEW association. They are not +// Mendix's: a blank 11.13 app's own `Administration.AccountPasswordData_Account` +// stores 0;54 / 100;54. Both engines hardcoded these two strings on every write, +// so any association write — including a documentation-only +// `alter association … set comment` — silently discarded whatever the developer +// had dragged the line to in Studio Pro. Read the stored value and write it back +// (guard-don't-drop, ADR-0005); these apply only when there is nothing stored. +// +// DomainModels$CrossAssociation has no connection properties at all, and writing +// them there crashes Studio Pro (issue #50) — so a cross-module association has +// no anchors to preserve. (issue #872) +const ( + DefaultParentConnection = "0;50" + DefaultChildConnection = "100;50" +) + +// ParseConnectionPoint reads a stored "x;y" anchor. Returns nil when the value +// is absent or not two integers, so an unreadable anchor falls back to the +// default rather than being written back as 0;0 (a legitimate anchor, which is +// why the field is a pointer: the zero Point is a real position, not "unset"). +// +// Both components are integers by necessity, not by convention: Mendix's loader +// rejects a non-integer component outright — a hand-patched "0.5;50" fails with +// StorageLoadException "One or more invalid values were detected while loading +// the project", verified on 11.13.0. It does NOT range-check, so negatives and +// values past 100 load fine and must round-trip untouched. +func ParseConnectionPoint(s string) *model.Point { + x, y, ok := strings.Cut(s, ";") + if !ok { + return nil + } + xi, err := strconv.Atoi(strings.TrimSpace(x)) + if err != nil { + return nil + } + yi, err := strconv.Atoi(strings.TrimSpace(y)) + if err != nil { + return nil + } + return &model.Point{X: xi, Y: yi} +} + +// FormatConnectionPoint renders an anchor back into Mendix's "x;y" form, +// falling back to the given default when nothing was read. +func FormatConnectionPoint(p *model.Point, fallback string) string { + if p == nil { + return fallback + } + return fmt.Sprintf("%d;%d", p.X, p.Y) +} diff --git a/sdk/domainmodel/connection_test.go b/sdk/domainmodel/connection_test.go new file mode 100644 index 000000000..c180ef551 --- /dev/null +++ b/sdk/domainmodel/connection_test.go @@ -0,0 +1,70 @@ +// SPDX-License-Identifier: Apache-2.0 + +package domainmodel + +import ( + "testing" + + "github.com/mendixlabs/mxcli/model" +) + +// upstream #872. The anchors are stored as the string "x;y" and were never read, +// so every association write reset them to mxcli's own defaults. Round-tripping +// them safely turns on two properties of the format, both established against +// mxbuild 11.13.0 by hand-patching a project and running `mx check`: +// +// - Both components must be INTEGERS. "0.5;50" is rejected at load with +// StorageLoadException ("One or more invalid values were detected while +// loading the project"), so a value that does not parse as two ints is not +// a value we should be writing back. +// - There is NO range validation. "0;500" and "-20;50" both load with 0 +// errors, so a value outside 0..100 must round-trip untouched rather than +// being clamped to something "sensible". +func TestConnectionPointRoundTrip(t *testing.T) { + cases := []struct { + name string + stored string + want *model.Point + reFormat string // what FormatConnectionPoint must produce; "" = the fallback + }{ + {"the value mxcli writes", "0;50", &model.Point{X: 0, Y: 50}, "0;50"}, + {"the value Studio Pro writes in a blank 11.13 app", "0;54", &model.Point{X: 0, Y: 54}, "0;54"}, + {"a hand-dragged anchor", "50;100", &model.Point{X: 50, Y: 100}, "50;100"}, + // The zero point is a REAL anchor (top-left), which is why the field is a + // pointer — treating it as "unset" would silently rewrite it to the default. + {"the zero point is a value, not an absence", "0;0", &model.Point{X: 0, Y: 0}, "0;0"}, + // mxbuild accepts both of these, so neither may be normalised away. + {"out of 0..100 range", "0;500", &model.Point{X: 0, Y: 500}, "0;500"}, + {"negative", "-20;50", &model.Point{X: -20, Y: 50}, "-20;50"}, + + // Unreadable ⇒ nil ⇒ the writer's default. Anything Mendix itself would + // refuse to load is not worth preserving. + {"absent", "", nil, ""}, + {"non-integer (rejected by Mendix's own loader)", "0.5;50", nil, ""}, + {"non-numeric", "abc;50", nil, ""}, + {"missing separator", "050", nil, ""}, + } + + const fallback = "0;50" + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := ParseConnectionPoint(tc.stored) + switch { + case tc.want == nil && got != nil: + t.Fatalf("ParseConnectionPoint(%q) = %+v, want nil so the default applies", tc.stored, got) + case tc.want != nil && got == nil: + t.Fatalf("ParseConnectionPoint(%q) = nil, want %+v — a stored anchor was dropped", tc.stored, tc.want) + case tc.want != nil && *got != *tc.want: + t.Fatalf("ParseConnectionPoint(%q) = %+v, want %+v", tc.stored, *got, *tc.want) + } + + want := tc.reFormat + if want == "" { + want = fallback + } + if out := FormatConnectionPoint(got, fallback); out != want { + t.Errorf("FormatConnectionPoint = %q, want %q — the stored anchor must go back verbatim", out, want) + } + }) + } +} diff --git a/sdk/domainmodel/domainmodel.go b/sdk/domainmodel/domainmodel.go index c45330df9..ceaa5d341 100644 --- a/sdk/domainmodel/domainmodel.go +++ b/sdk/domainmodel/domainmodel.go @@ -328,16 +328,20 @@ type AttributeValue struct { // Association represents an association between entities. type Association struct { model.BaseElement - ContainerID model.ID `json:"containerId"` - Name string `json:"name"` - Documentation string `json:"documentation,omitempty"` - ParentID model.ID `json:"parentId"` - ChildID model.ID `json:"childId"` - Type AssociationType `json:"type"` - Owner AssociationOwner `json:"owner"` - StorageFormat AssociationStorageFormat `json:"storageFormat,omitempty"` - ParentConnection model.Point `json:"parentConnection,omitempty"` - ChildConnection model.Point `json:"childConnection,omitempty"` + ContainerID model.ID `json:"containerId"` + Name string `json:"name"` + Documentation string `json:"documentation,omitempty"` + ParentID model.ID `json:"parentId"` + ChildID model.ID `json:"childId"` + Type AssociationType `json:"type"` + Owner AssociationOwner `json:"owner"` + StorageFormat AssociationStorageFormat `json:"storageFormat,omitempty"` + // Line anchors in the domain model editor. Pointers because the zero Point + // is a legitimate anchor (top-left corner), so it cannot double as "unset"; + // nil means nothing was stored and the writer uses DefaultParentConnection / + // DefaultChildConnection. See connection.go. (issue #872) + ParentConnection *model.Point `json:"parentConnection,omitempty"` + ChildConnection *model.Point `json:"childConnection,omitempty"` // Delete behavior ParentDeleteBehavior *DeleteBehavior `json:"parentDeleteBehavior,omitempty"` diff --git a/sdk/mpr/parser_domainmodel.go b/sdk/mpr/parser_domainmodel.go index d43cc1541..dd25a00eb 100644 --- a/sdk/mpr/parser_domainmodel.go +++ b/sdk/mpr/parser_domainmodel.go @@ -437,6 +437,11 @@ func parseAssociation(raw map[string]any) *domainmodel.Association { } else { assoc.StorageFormat = domainmodel.StorageFormatTable } + // The line anchors are read so the writer can put them back unchanged; every + // association write rebuilds the whole element, so a field not read here is + // a field destroyed on the next `alter association`. (issue #872) + assoc.ParentConnection = domainmodel.ParseConnectionPoint(extractString(raw["ParentConnection"])) + assoc.ChildConnection = domainmodel.ParseConnectionPoint(extractString(raw["ChildConnection"])) // Parse delete behavior if deleteBehaviorRaw, ok := raw["DeleteBehavior"].(map[string]any); ok { diff --git a/sdk/mpr/writer_domainmodel.go b/sdk/mpr/writer_domainmodel.go index 4fac522f7..eeb489c92 100644 --- a/sdk/mpr/writer_domainmodel.go +++ b/sdk/mpr/writer_domainmodel.go @@ -1186,8 +1186,8 @@ func serializeAssociation(a *domainmodel.Association) bson.D { {Key: "ChildPointer", Value: idToBsonBinary(string(a.ChildID))}, {Key: "Type", Value: string(a.Type)}, {Key: "Owner", Value: string(a.Owner)}, - {Key: "ParentConnection", Value: "0;50"}, - {Key: "ChildConnection", Value: "100;50"}, + {Key: "ParentConnection", Value: domainmodel.FormatConnectionPoint(a.ParentConnection, domainmodel.DefaultParentConnection)}, + {Key: "ChildConnection", Value: domainmodel.FormatConnectionPoint(a.ChildConnection, domainmodel.DefaultChildConnection)}, {Key: "StorageFormat", Value: storageFormat}, {Key: "DeleteBehavior", Value: serializeDeleteBehavior(a.ParentDeleteBehavior, a.ChildDeleteBehavior)}, {Key: "Source", Value: source}, diff --git a/sdk/mpr/writer_domainmodel_test.go b/sdk/mpr/writer_domainmodel_test.go index 9d563cfbe..2ddad30d7 100644 --- a/sdk/mpr/writer_domainmodel_test.go +++ b/sdk/mpr/writer_domainmodel_test.go @@ -202,3 +202,45 @@ func TestSerializeAssociation_HasConnectionFields(t *testing.T) { t.Error("serializeAssociation must include ChildConnection") } } + +// upstream #872: the legacy writer hardcoded the association's line anchors, so +// running any association write on the legacy engine destroyed whatever the +// developer had dragged the connector to in Studio Pro — exactly as the modelsdk +// engine did. Both engines share the semantic model, so both had to change; a +// fix in one is invisible to a user on the other (`--engine`/`MXCLI_ENGINE`). +func TestSerializeAssociation_PreservesConnectionPoints(t *testing.T) { + base := func() *domainmodel.Association { + a := &domainmodel.Association{ + Name: "Child_Parent", + ParentID: "parent-entity-id", + ChildID: "child-entity-id", + Type: domainmodel.AssociationTypeReference, + Owner: domainmodel.AssociationOwnerDefault, + } + a.ID = "test-assoc-id" + return a + } + + // Nothing stored → mxcli's defaults, so a brand-new association still gets a + // sensible connector rather than one pinned to the box's top-left corner. + plain := dToM(serializeAssociation(base())) + if got := plain["ParentConnection"]; got != domainmodel.DefaultParentConnection { + t.Errorf("ParentConnection = %v, want the default %q", got, domainmodel.DefaultParentConnection) + } + if got := plain["ChildConnection"]; got != domainmodel.DefaultChildConnection { + t.Errorf("ChildConnection = %v, want the default %q", got, domainmodel.DefaultChildConnection) + } + + // A read anchor goes back verbatim. {0,0} is included deliberately: it is a + // real anchor (top-left) and must not be mistaken for "unset". + tuned := base() + tuned.ParentConnection = &model.Point{X: 50, Y: 100} + tuned.ChildConnection = &model.Point{X: 0, Y: 0} + got := dToM(serializeAssociation(tuned)) + if got["ParentConnection"] != "50;100" { + t.Errorf("ParentConnection = %v, want \"50;100\" — a hand-tuned anchor was reset", got["ParentConnection"]) + } + if got["ChildConnection"] != "0;0" { + t.Errorf("ChildConnection = %v, want \"0;0\" — the zero point is a value, not an absence", got["ChildConnection"]) + } +} diff --git a/sdk/widgets/definitions/dropdownfilter.def.json b/sdk/widgets/definitions/dropdownfilter.def.json index 263a3ce4c..9c292afa3 100644 --- a/sdk/widgets/definitions/dropdownfilter.def.json +++ b/sdk/widgets/definitions/dropdownfilter.def.json @@ -3,9 +3,31 @@ "mdlName": "DROPDOWNFILTER", "templateFile": "datagrid-dropdown-filter.json", "defaultEditable": "Always", - "propertyMappings": [ - {"propertyKey": "attrChoice", "value": "auto", "operation": "primitive"}, - {"propertyKey": "attributes", "source": "Attributes", "operation": "attributeObjects"}, - {"propertyKey": "defaultFilter", "source": "FilterType", "operation": "primitive"} + "knownProperties": [ + "refCaptionSource", + "refCaptionExp", + "refSearchAttr" + ], + "modes": [ + { + "name": "association", + "condition": "hasDataSource", + "description": "Association mode — filter the grid by a reference (baseType 'ref'). The widget's `datasource:` is the OPTION list (the associated entity), not the grid's own datasource; `Association:` names the reference on the grid entity and `CaptionAttribute:` is what each option shows.", + "propertyMappings": [ + {"propertyKey": "baseType", "value": "ref", "operation": "primitive"}, + {"propertyKey": "refOptions", "source": "DataSource", "operation": "datasource"}, + {"propertyKey": "refEntity", "source": "Association", "operation": "association"}, + {"propertyKey": "refCaption", "source": "CaptionAttribute", "operation": "attribute"} + ] + }, + { + "name": "default", + "description": "Attribute mode — filter on the column's own attribute.", + "propertyMappings": [ + {"propertyKey": "attrChoice", "value": "auto", "operation": "primitive"}, + {"propertyKey": "attributes", "source": "Attributes", "operation": "attributeObjects"}, + {"propertyKey": "defaultFilter", "source": "FilterType", "operation": "primitive"} + ] + } ] }