Repository navigation
Commit 165bc50
fix(studio): hold a report whose new Order or Blocks item, or a chart picked before its axes, is unfinished (#27824)
<!-- claude-projects-attribution: {"github_login":"os-bill"} -->
_Requested by **Bill** · [project
thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wg4i9JiKjEjwtuVzd4gKy9hB)_
## 维护者速读
- 改了什么:Studio 界面支柱里,报表属性面板「排序(Order)」或联合报表「Blocks」点「Add
item」后、以及先选了图表类型还没选 X/Y 轴时,草稿不再自动保存;画布上方出现一行中性提示,点名还缺的输入(如 “Order By on
item #1 of “Order” needs a value”、“X-Axis (dimension) on the report’s
chart needs a value”),带「Show
me」打开报表属性并定位到该列表或该轴的选择框,发布在暂缓期间拒绝;填好后下一次自动保存把期间所有改动一起发出。
- 为什么改:以前这三种半成品立刻被 PUT 出去(`order:[{}]`、`blocks:[{}]`、`chart:{type}`),草稿门
422;之后每次保存都带着它继续 422,期间改的其他内容重载后全丢。仪表盘同类问题已由 #27784 修好,这张卡把同一规则用到报表上。
- 风险与回滚:只改前端的暂缓判定,判定问 spec 自己的
`ReportSchema`,不另存必填清单;只有「所有报错都落在还没填的输入上」才暂缓,填了错值的(如排序字段不在报表里、图表类型非法)照常发送并显示红色拒绝条。最坏情况是多暂缓一次(提示行
+ Show me 可解)。回滚即 revert 本 PR。
-
先看:`ui/packages/app-shell/src/views/studio-design/metadataError.ts`(`reportHeldEdit`
/ `objectHeldInput`)、`StudioDesignSurface.tsx`(界面支柱 `leafIncomplete`
加报表分支)、`InterfacesPillar.reportHeld-27785.test.tsx`。
- 两分钟验证:`pnpm --dir ui exec vitest run
packages/app-shell/src/views/studio-design/metadataError.reportHeld-27785.test.ts
packages/app-shell/src/views/studio-design/InterfacesPillar.reportHeld-27785.test.tsx`,预期
2 个文件 23 条全部通过。
- 需要你做的:无,席位落地。
Fixes #27785
Clause-②: no
Metadata-protocol: none
## What changes
**Built on PR #27784.** This branch started from #27784's head
`7ff9cb5c` while it was in the merge queue, reusing its
`listItemHeldInput` / `heldListItem`, and merged `origin/main` after
#27784 landed (head `93747963`). The diff against `main` is this card's
7 files only.
**Measured first, on the spec this branch builds**
(`ReportSchema.safeParse`, `packages/spec/dist`), all three cases of the
card reproduce:
| Case | Body the autosave sent | `ReportSchema` refuses at |
|:--|:--|:--|
| Order → Add item | `order: [{}]` | `order.0.by` |
| joined Blocks → Add item | `blocks: [{}]` | `blocks.0.name`, then
`blocks.0.dataset` once named |
| chart type before axes | `chart: { type: 'bar' }` | `chart.xAxis`,
`chart.yAxis` |
On the pillar, the gate was `leafType === 'dashboard'`, so a report was
never held: with the new report branch removed (the ablation below) the
pillar sends the `order: [{}]` body the card saw.
**The hold.** `reportHeldEdit(draft, locale)` (in `metadataError.ts`,
beside `dashboardHeldEdit`) is the report's leaf rule, and the
Interfaces pillar's `leafIncomplete` now asks it for a `report` leaf:
- **List items** (Order, joined Blocks): `listItemHeldInput(draft,
ReportSchema)` from #27784, unchanged, with the report form
(`getReportForm()`) for the labels. The line reads "Order By on item #1
of “Order” needs a value" with the existing
`engine.studio.held.listItemNeedsInput` clause.
- **The chart, held on its own** (`objectHeldInput(body, schema, key)`,
asked for `chart`). Only issues under `chart` are this rule's; the chart
is held only when every one of them names an input directly below it
that the chart leaves unset. A chart refused for a value it holds (an
unknown `type`) is sent and its refusal shows as before.
- Why not generalise `listItemHeldInput` to every nested object: that
would change what the dashboard holds too (its `header`, `dateRange` and
other nested objects), with no measurement behind it; and the chart's
inputs live in the report inspector's own chart panel, not in the spec
form whose rows `heldListItem` reads its labels from.
- The axis is named by the chart panel's own label
(`engine.inspector.report.chartX` / `chartY`), the text "Show me" then
focuses. That table names labels only; which inputs are required is
still `ReportSchema`'s answer. One new clause,
`engine.studio.held.reportChartNeedsInput` (en, zh), because
`listItemNeedsInput` names an item number and a list a chart does not
have.
- Both are placed on the report's own properties (`target: null`), so
"Show me" goes through the pillar's existing `showLeafRefusal`, as the
dashboard's list items do. A list item is named before the chart when
both are unfinished.
The autosave block, the Publish refusal and the leave guard read the
same `leafIncomplete` / reported clause as before, unchanged.
## Tests
- New `metadataError.reportHeld-27785.test.ts` (16 cases). Every held
case is paired with `ReportSchema`'s verdict on the same report: Order's
new item (the repeater's all-`undefined` row and the stored `{}`),
joined Blocks' new item (held for Name, then for Dataset), a chart with
only its type (held for the X axis), then with only its X axis (held for
Y), zh-CN naming. Controls: Order By filled, a block named and bound,
both axes picked are accepted and not held; an Order By naming something
the report does not select, a block with an unset Dataset and a
malformed Name, and a chart with an unknown type are sent; a report with
no chart holds nothing; `objectHeldInput` ignores a refusal outside its
key.
- New `InterfacesPillar.reportHeld-27785.test.tsx` (7 cases), the real
`ReportDefaultInspector` over a recording metadata client: Order → Add
item, then a Label edit sends no PUT past the debounce, shows no red
strip, names the Order By, and reports the clause to Publish; "Show me"
focuses a control in the Order row; filling Order By sends one save
carrying the item and the Label edit, which `ReportSchema` accepts;
joined Blocks → Add item is held for the block's Name; a chart type
picked first is held for the X axis, "Show me" focuses the X-axis
picker, the X axis alone is still held for Y, and both axes send one
save with the whole chart; control: a Label edit alone saves at once.
Readings below are on the final head `93747963` (this branch merged with
`origin/main` after #27784 landed) unless a line says otherwise.
<details><summary>Ablations (ablation-replace, restored blob == HEAD,
pre-merge head dae6776)</summary>
- `StudioDesignSurface.tsx`: the pillar's new `if (leafType ===
'report') return reportHeldEdit(draft, locale);` line deleted (the gate
as it was, dashboard only) → 5 failed / 7 in
`InterfacesPillar.reportHeld-27785`: every case that needs a hold. The
first one shows the body the card saw going out: `expected [ { name:
'sales_report', … "order": [ … ] } ] to deeply equal []`. The control (a
Label edit alone saves at once) and "both axes picked: one save" stay
green. Restored: blob `064578278d42` == HEAD, `git diff HEAD` empty.
- `metadataError.ts`: in `objectHeldInput`, `if (input === undefined ||
!itemInputUnset(object[input])) return null;` replaced by `if (input ===
undefined) return null;` (a chart input holding a value no longer makes
it finished but wrong) → 1 failed / 16 in
`metadataError.reportHeld-27785`: "a chart refused for a value it holds
is finished but wrong: not held". Restored: blob `90e17158edd5` == HEAD,
`git diff HEAD` empty.
- app-shell tests import `src` directly, so no dist build sits between
the mutation and the run.
</details>
<details><summary>Affected suites: 120 files / 1631 tests passed (before
and after the merge)</summary>
Every app-shell test file naming `metadataError`, `reportHeldEdit`,
`dashboardHeldEdit`, `StudioHeld`, `ReportDefaultInspector` or
`held.listItem` (30 files, the two new ones included), plus the whole
`packages/i18n/` directory, under the verification lock: `Test Files 120
passed (120) · Tests 1631 passed | 13 skipped (1644)`, lock VERDICT
command-exit 0, on `dae67768` and again on `93747963`. The rest of the
ui tree is left to CI `Test UI`.
```
pnpm --filter '@object-ui/app-shell^...' build :: exit 0 (9374796, after the merge)
pnpm --filter @object-ui/app-shell run type-check && pnpm --filter @object-ui/i18n run type-check :: exit 0 (9374796)
node ui/scripts/check-new-cross-file-line-citations.mjs :: exit 0 (0 new)
```
</details>
## Acceptance notes
- **The chart is held by its own rule, not by generalising the list-item
rule** (the card left this to the implementer). Reasons in What changes:
a general nested-object rule would also change what the dashboard holds,
unmeasured, and the chart's inputs are named by the inspector's chart
panel, not by spec form rows.
- **A joined report with a chart type set before its axes is held for
the axes, not refused for the chart.** `ReportSchema` judges a joined
container's `chart` only once the chart parses, so it names the unset
axes first. The report inspector hides the chart panel on a joined
report and clears `chart` when the type switches to joined, so the
Studio cannot produce this body; noted, not changed.
- **An "Add item" inside a block's own `order` is not held.**
`listItemHeldInput` places an issue on the outermost list item (the
block), whose `order` input holds a value, so the block counts as
finished but wrong and is sent. Whether the block repeater's Order
column can append an empty nested sort key was not measured here; noted
as the boundary of #27784's rule, not filed.
- **"Show me" and collapsed sections.** The report form's Order row
(Dataset binding) and Blocks row (Joined blocks) sit in sections that
are not collapsed by default, and the chart panel is never collapsed, so
"Show me" reaches every input this card holds. The dashboard's residual
(its Filters section collapsed by default) is untouched: expanding a
collapsed SchemaForm section from "Show me" is not the same code.
- An Order By typed in but naming something the report does not select
is sent, not held: the refusal names the value the author wrote (`order
key "…" is not selected by this report`).
<details><summary>Derived gates (dispatch-gates --commands on this diff,
head 9374796): 34 / 34 exit 0</summary>
```
node scripts/check-adr-0087-registration.mjs --base origin/main :: exit 0
node scripts/check-adr-0087-registration.mjs --self-test :: exit 0
node scripts/check-changeset-no-major.mjs --base origin/main :: exit 0
node scripts/check-closing-keyword-parity.mjs :: exit 0
node scripts/check-closing-keyword-parity.mjs --self-test :: exit 0
node scripts/check-comment-mask-corpus.mjs :: exit 0
node scripts/check-empty-changeset.mjs --base origin/main :: exit 0
node scripts/check-empty-changeset.mjs --self-test :: exit 0
node scripts/check-sdui-manifest.mjs :: exit 0
node scripts/check-sdui-manifest.mjs --self-test :: exit 0
node scripts/pm/release-rehearsal-clone.mjs --self-test :: exit 0
node scripts/release-pending-publish.mjs --self-test :: exit 0
pnpm --dir ui exec vitest run scripts/ :: exit 0
pnpm --filter @objectstack/spec run check:duration-unit-keys :: exit 0
pnpm check:changeset-gate-self-tests :: exit 0
pnpm check:closed-ceremony-text :: exit 0
pnpm check:cross-package-test-inputs :: exit 0
pnpm check:dispatcher-error-vocabulary :: exit 0
pnpm check:driver-memory-census :: exit 0
pnpm check:engine-double-contract :: exit 0
pnpm check:gitlink-declared :: exit 0
pnpm check:nul-bytes :: exit 0
pnpm check:pm-changeset-deadline-census :: exit 0
pnpm check:published-files :: exit 0
pnpm check:query-options-erasure :: exit 0
pnpm check:refd-timer-probe :: exit 0
pnpm check:skill-identifier-liveness :: exit 0
pnpm check:test-source-alias :: exit 0
pnpm check:tier-file-adoption :: exit 0
pnpm check:type-check-coverage :: exit 0
pnpm check:type-check-debt :: exit 0
pnpm check:undeclared-dep-imports :: exit 0
pnpm check:watch-hint-literal :: exit 0
pnpm check:where-matcher :: exit 0
```
`dispatch-gates --ran`: 34 derived families accounted for, 34 run, 0
NOT-MEASURED. `node scripts/check-sdui-manifest.mjs` (the ui card gate)
is among them. The same 34 also ran 34 / 34 exit 0 on the pre-merge head
`dae67768`. CI runs the type-check lanes, the artifact-roster and
wide-population families and `Test UI`.
</details>
---
_Generated by [Claude
Code](https://claude.ai/code/session_01Utp6CaQABe2o9G1TyQ3LTx)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 9b85162 commit 165bc50
7 files changed
Lines changed: 576 additions & 6 deletions
File tree
- .changeset
- ui/packages
- app-shell/src/views/studio-design
- i18n/src/namespaces/studio
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
261 | 261 | | |
262 | 262 | | |
263 | 263 | | |
| 264 | + | |
264 | 265 | | |
265 | 266 | | |
266 | 267 | | |
| |||
4835 | 4836 | | |
4836 | 4837 | | |
4837 | 4838 | | |
| 4839 | + | |
| 4840 | + | |
| 4841 | + | |
4838 | 4842 | | |
4839 | | - | |
4840 | | - | |
4841 | | - | |
4842 | | - | |
| 4843 | + | |
| 4844 | + | |
| 4845 | + | |
| 4846 | + | |
| 4847 | + | |
| 4848 | + | |
4843 | 4849 | | |
4844 | 4850 | | |
4845 | 4851 | | |
| |||
0 commit comments