Skip to content

Commit 71d882c

Browse files
akshay-vizCopilot
andcommitted
fix(model-apps): dashboard chart tiles emitted ChartId, which the platform rejects
Found by a live build against a real environment. A chart tile set `parameters.ChartId`, and the platform validates dashboard FormXML against a schema that enumerates the legal children of `<parameters>`: The element 'parameters' has invalid child element 'ChartId'. List of possible elements expected: 'ViewId, IsUserView, ... VisualizationId, ...' So any spec with a chart tile failed the entire `dashboards` phase with a 400. The correct name is `VisualizationId` — which `download-model-app.js` already reads, so a dashboard could not round-trip through download -> rebuild either. Build wrote one name and download looked for the other. Pre-existing, not a regression from the SDK uptake: `parameters.ChartId` is on origin/main and dates from the original plugin commit, and this branch had not touched that function. Why 1599 tests passed while a real build failed: `sdk-build.test.js` asserted `chart.parameters.ChartId` — the suite had encoded the wrong wire name, so it agreed with the bug. A mock cannot validate FormXML against the platform schema, so the assertion now pins the NAME explicitly and also asserts that `ChartId` is not emitted, which is the half that would have caught this. Live-proven: the same spec that failed the dashboards phase now completes `build complete - 16 created, 0 failed (27 steps)`, and the deployed dashboard reads back with 3 tiles, 2 VisualizationId and 0 ChartId. Also documents that teardown deliberately leaves the publisher behind — a publisher can own other solutions, so deleting it is not this app's decision, the same fail-safe reasoning that keeps an `external` web resource. It is the one artifact that legitimately survives a clean teardown. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 42626da2-b66f-4162-acaa-b1127ef23d89
1 parent 7983f92 commit 71d882c

4 files changed

Lines changed: 32 additions & 2 deletions

File tree

plugins/model-apps/AGENTS.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -923,6 +923,10 @@ node scripts/teardown-model-app.js --env <envUrl> --spec @<dir>/app-spec.json --
923923

924924
AI features are **admin-gated** — preflight readiness with `node scripts/ai-preflight.js --env <envUrl>`.
925925
Prefer a scratch env; always tear down probes (`teardown-model-app.js --apply`) to leave 0 leftovers.
926+
Teardown needs `--allow-destructive` as well as `--apply`, and it deliberately leaves **one** thing
927+
behind: the **publisher**. A publisher can own other solutions in the environment, so deleting it is
928+
not this app's decision to make — the same fail-safe reasoning that keeps an `external` web resource.
929+
Expect a clean environment afterwards *except* for `<prefix>publisher`.
926930

927931
**After modifying the plugin also:** run `claude --debug` to confirm the plugin loads, exercise the
928932
skill (`/genpage` or `/app-builder`), and for genpage verify Playwright browser checks

plugins/model-apps/CHANGELOG.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,18 @@ and corrects a smoke-eval assertion that could never pass live.
129129
GitHub Copilot CLI or Claude Code host when a newer version is available.
130130

131131
### Fixed
132+
- **Dashboard chart tiles no longer fail the whole `dashboards` phase.** A chart tile emitted
133+
`ChartId`, which the platform's FormXML schema rejects — it is not a legal child of
134+
`<parameters>` — so the phase died with:
135+
136+
> The element 'parameters' has invalid child element 'ChartId'. List of possible elements
137+
> expected: 'ViewId, IsUserView, … VisualizationId, …'
138+
139+
The correct name is `VisualizationId`, which is what `download-model-app.js` already read, so a
140+
dashboard could not round-trip through download → rebuild either. Found by a live build; the
141+
mock-based test had asserted the wrong name, so the suite agreed with the bug. Both are corrected,
142+
and the test now also asserts that `ChartId` is *not* emitted.
143+
132144
- **Rebuilds no longer duplicate sub-grids or skip field removals** — the plugin now
133145
awaits the maker SDK's asynchronous artifact surface.
134146

plugins/model-apps/scripts/lib/sdk-build.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,8 +142,16 @@ function dashboardTileOpts(spec, tile, result) {
142142
function dashboardComponent(t, index) {
143143
const parameters = {};
144144
if (t.type === 'chart') {
145+
// `VisualizationId`, NOT `ChartId`. The platform validates dashboard FormXML against a schema
146+
// that enumerates the legal children of `<parameters>`, and `ChartId` is not one of them:
147+
// The element 'parameters' has invalid child element 'ChartId'. List of possible elements
148+
// expected: 'ViewId, IsUserView, ... VisualizationId, ...'
149+
// A chart tile therefore failed the whole dashboards phase with a 400. Caught by a live build;
150+
// the mock-based test had asserted the wrong name, so the suite agreed with the bug.
151+
// `download-model-app.js` already reads `VisualizationId`, so this also makes a dashboard
152+
// round-trip through download -> rebuild instead of losing its chart binding.
145153
parameters.TargetEntityType = t.targetEntity; parameters.ViewId = t.viewId;
146-
parameters.ChartId = t.visualizationId; parameters.ChartGridMode = 'Chart';
154+
parameters.VisualizationId = t.visualizationId; parameters.ChartGridMode = 'Chart';
147155
} else if (t.type === 'list') {
148156
parameters.TargetEntityType = t.targetEntity; parameters.ViewId = t.viewId;
149157
parameters.IsUserView = 'false'; parameters.ChartGridMode = 'Grid'; parameters.RecordsPerPage = '10';

plugins/model-apps/scripts/tests/sdk-build.test.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -967,7 +967,13 @@ test('dashboards: chart + list tiles resolve the created view/visualization ids'
967967
const chart = tileComps.find((t) => t.type === 'chart');
968968
assert.strictEqual(chart.parameters.TargetEntityType, 'new_ticket', 'entity derived from the view');
969969
assert.ok(chart.parameters.ViewId, 'view id resolved');
970-
assert.ok(chart.parameters.ChartId, 'chart visualization id resolved');
970+
// `VisualizationId`, NOT `ChartId`. This assertion previously named the wrong parameter, so the
971+
// suite agreed with a bug that failed the whole dashboards phase on a real environment with a
972+
// 400: "The element 'parameters' has invalid child element 'ChartId'". A mock cannot validate
973+
// FormXML against the platform schema, so the wire NAME has to be pinned explicitly here.
974+
assert.ok(chart.parameters.VisualizationId, 'chart visualization id resolved');
975+
assert.strictEqual(chart.parameters.ChartId, undefined,
976+
'the rejected `ChartId` spelling is not emitted (it is not a legal <parameters> child)');
971977
const list = tileComps.find((t) => t.type === 'list');
972978
assert.strictEqual(list.name, 'Recent');
973979
assert.ok(list.parameters.ViewId);

0 commit comments

Comments
 (0)