From c2e03fcfdf06582d4a747bacaad12bbe1e79b539 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 22:36:16 +0000 Subject: [PATCH 1/2] fix(charts): the pivot branch buckets null first-dimension values instead of dropping the bar (#4497) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `buildChartSeries`' multi-dimension pivot branch bucketed rows by `String(xRaw ?? '')` but wrote the RAW value into the emitted row, so a null first-dimension value produced `{status: null, Low: 3}` and reached recharts with a null category — which draws no mark. Measured at the DOM: a two-group pivot drew ONE bar; an all-null pivot drew zero bar rectangles under a full axis. Same mechanism as #4466 (PR #4498), one branch over. The pivot now maps a null/undefined first-dimension VALUE to the same bucket label the single-dimension branch uses, through the same `ChartSeriesOptions.nullCategoryLabel` / `NULL_CATEGORY_LABEL` default. One doctrine, one predicate (`isNullCategory`, extracted from the existing `bucketNullCategories` so the two branches cannot drift), two call sites. No new export: the emitted `.d.ts` declarations are byte-identical. The bucket KEY is untouched, so which rows share a bar is byte-identical and only the display value changes. Rows lacking the category key entirely are still not bucketed (framework#4033's division). Drill-through needed no change, and that was measured rather than assumed: the pivot AGGREGATES, so its emitted rows are not index-aligned with `drillRawRows`, and `DatasetWidget.handleChartDrill` already drills by searching the raw rows through `findChartSeriesRow` — whose `xOf` (from #4498) already covers the multi-dimension arm. Pinned at both levels. #4498's deliberate pin of the pivot's null-drop ("leaves the multi-dimension pivot branch exactly as it was") is updated in this same commit, citing #4497, which is what that pin existed for. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 --- .changeset/chart-pivot-null-bucket-4497.md | 14 + .../utils/chart-series.nullCategory.test.ts | 257 +++++++++++++++++- packages/core/src/utils/chart-series.ts | 83 ++++-- ...AdvancedChartImpl.pivotNullBucket.test.tsx | 170 ++++++++++++ 4 files changed, 501 insertions(+), 23 deletions(-) create mode 100644 .changeset/chart-pivot-null-bucket-4497.md create mode 100644 packages/plugin-charts/src/AdvancedChartImpl.pivotNullBucket.test.tsx diff --git a/.changeset/chart-pivot-null-bucket-4497.md b/.changeset/chart-pivot-null-bucket-4497.md new file mode 100644 index 000000000..514567fe1 --- /dev/null +++ b/.changeset/chart-pivot-null-bucket-4497.md @@ -0,0 +1,14 @@ +--- +'@object-ui/core': patch +'@object-ui/plugin-charts': patch +--- + +The multi-dimension pivot branch buckets a null first-dimension value instead of dropping its bar (objectui#4497) + +`buildChartSeries`' pivot branch (2+ dimensions, single measure) bucketed rows by `String(xRaw ?? '')` but wrote the RAW value into the emitted row, so a null first-dimension value produced `{status: null, Low: 3}` and reached recharts with a null category — which draws no mark. Measured at the DOM: a two-group pivot drew ONE bar, and an all-null pivot drew axes and gridlines with zero bar rectangles and no empty state. That is the same mechanism objectui#4466 fixed one branch below, on the branch that card deliberately left pinned as-is until the pivot's own bucketing had been measured. + +The pivot now maps a null/undefined first-dimension VALUE to the same bucket label the single-dimension branch uses — `ChartSeriesOptions.nullCategoryLabel`, defaulting to `NULL_CATEGORY_LABEL`. One doctrine, one predicate, two call sites; no new export, and every existing call site compiles and behaves identically. + +The bucket KEY is untouched, which is what keeps this a display fix: `String(xRaw ?? '')` still decides which rows share a bar, so every existing grouping is byte-identical and only the label the bucket carries changes. Rows that lack the category key entirely are still not bucketed — that shape is a dimension grouped by but never projected (framework#4033), a different defect with a different answer. + +Drill-through needed no change, which was measured rather than assumed: the pivot's emitted rows are AGGREGATED, so they are not index-aligned with `drillRawRows` and the one production caller (`DatasetWidget.handleChartDrill`) already drills by SEARCHING the raw rows through `findChartSeriesRow`. Those raw rows still carry their null, and objectui#4466's label-matching covers the multi-dimension arm as well as the single-dimension one, so the newly-visible bar resolves to the right record. Pinned at both levels so a regression in either half surfaces as the dead click it would be. diff --git a/packages/core/src/utils/chart-series.nullCategory.test.ts b/packages/core/src/utils/chart-series.nullCategory.test.ts index dc8cf1c4b..c041391a0 100644 --- a/packages/core/src/utils/chart-series.nullCategory.test.ts +++ b/packages/core/src/utils/chart-series.nullCategory.test.ts @@ -28,6 +28,15 @@ * (plugin-charts' `AdvancedChartImpl`), which explains itself instead of * drawing an empty axis. Key absent → that path; key present, value null → * this bucket. + * + * objectui#4497 extends the same doctrine to the MULTI-dimension pivot branch, + * which #4466 deliberately left pinned as-is (that pin is updated below, in the + * commit that changed it). The pivot's mechanism differs in exactly one way, + * which is why it needed its own card: it buckets rows by a map KEY + * (`String(xRaw ?? '')`) and writes a separate DISPLAY value into the emitted + * row, so a null x was bucketed correctly and STILL reached recharts raw. The + * key is untouched; only the display value is labelled. The drill half is + * pinned at the bottom of this file, with the measurement behind it. */ import { describe, it, expect } from 'vitest'; import { buildChartSeries, findChartSeriesRow, NULL_CATEGORY_LABEL } from './chart-series'; @@ -103,22 +112,152 @@ describe('buildChartSeries — must-not-change (objectui#4466)', () => { expect(r.data).toEqual([]); }); - it('leaves the multi-dimension pivot branch exactly as it was', () => { + /** + * THE DELIBERATE-PIN UPDATE (objectui#4497). + * + * This case used to read "leaves the multi-dimension pivot branch exactly as + * it was" and asserted `{status: null, Low: 3}` — #4466's pivot behaviour + * pinned AS-IS, not as correct, precisely so that changing it would have to + * be a deliberate act with a card behind it. #4497 is that card: it ruled the + * pivot inherits this branch's answer, so the pin moves here, in the same + * commit as the fix. + */ + it('buckets the pivot branch the SAME way, as of objectui#4497', () => { const rows = [ { status: 'Backlog', priority: 'High', est_hours: 5 }, { status: null, priority: 'Low', est_hours: 3 }, ]; const r = buildChartSeries(rows, ['status', 'priority'], ['est_hours']); - // Pre-existing pivot behaviour: a null x collapses to the '' bucket and the - // row keeps its raw null. Pinned as-is — this branch is out of #4466's - // ruled scope, and the pin makes any future change to it deliberate. expect(r.data).toEqual([ { status: 'Backlog', High: 5 }, - { status: null, Low: 3 }, + { status: NULL_CATEGORY_LABEL, Low: 3 }, ]); }); }); +/** + * objectui#4497 — the pivot branch, whose mechanism differs from the branch + * above in one way that had to be measured before it could be changed: it + * buckets by a map KEY (`String(xRaw ?? '')`) and writes a separate DISPLAY + * value into the emitted row. The fix moves the display value only; the key is + * untouched, so WHICH rows share a bar is byte-identical to before. + */ +describe('buildChartSeries — the pivot branch buckets null too (objectui#4497)', () => { + it('labels the null first-dimension bucket instead of emitting a raw null', () => { + const r = buildChartSeries( + [ + { status: 'Backlog', priority: 'High', est_hours: 5 }, + { status: null, priority: 'Low', est_hours: 3 }, + ], + ['status', 'priority'], + ['est_hours'], + ); + expect(r.xAxisKey).toBe('status'); + expect(r.series).toEqual([ + { dataKey: 'High', label: 'High' }, + { dataKey: 'Low', label: 'Low' }, + ]); + // Pre-fix: `{status: null, Low: 3}` — a null category reaching recharts, + // which draws no mark (the #4466 mechanism, one branch over). + expect(r.data).toEqual([ + { status: 'Backlog', High: 5 }, + { status: NULL_CATEGORY_LABEL, Low: 3 }, + ]); + }); + + it('buckets an undefined first-dimension value the same way', () => { + const r = buildChartSeries( + [{ status: undefined, priority: 'Low', est_hours: 3 }], + ['status', 'priority'], + ['est_hours'], + ); + expect(r.data).toEqual([{ status: NULL_CATEGORY_LABEL, Low: 3 }]); + }); + + it('uses the caller-supplied (localized) label, from the same option', () => { + const r = buildChartSeries( + [{ status: null, priority: 'Low', est_hours: 3 }], + ['status', 'priority'], + ['est_hours'], + null, + { nullCategoryLabel: '(未指定)' }, + ); + expect(r.data).toEqual([{ status: '(未指定)', Low: 3 }]); + }); + + it('keeps the null bucket MERGED with every other null row, as before', () => { + // The bucket key is unchanged, so two null-x rows still share one bar and + // contribute a column each — the label lands on the bucket, not per row. + const r = buildChartSeries( + [ + { status: null, priority: 'Low', est_hours: 3 }, + { status: null, priority: 'High', est_hours: 8 }, + ], + ['status', 'priority'], + ['est_hours'], + ); + expect(r.data).toEqual([{ status: NULL_CATEGORY_LABEL, Low: 3, High: 8 }]); + }); + + it('never mutates the caller rows — drill-through reads the raw null', () => { + const rows = [{ status: null, priority: 'Low', est_hours: 3 }]; + buildChartSeries(rows, ['status', 'priority'], ['est_hours']); + expect(rows[0].status).toBeNull(); + }); +}); + +describe('buildChartSeries — pivot must-not-change (objectui#4497)', () => { + it('leaves non-null pivot groups byte-identical', () => { + const rows = [ + { status: 'Backlog', priority: 'High', est_hours: 5 }, + { status: 'Backlog', priority: 'Low', est_hours: 3 }, + { status: 'Done', priority: 'High', est_hours: 24 }, + ]; + const r = buildChartSeries(rows, ['status', 'priority'], ['est_hours']); + expect(r.data).toEqual([ + { status: 'Backlog', High: 5, Low: 3 }, + { status: 'Done', High: 24 }, + ]); + expect(r.data.some((row) => row.status === NULL_CATEGORY_LABEL)).toBe(false); + }); + + it('does NOT relabel a genuine empty-string group — it is a value, not a null', () => { + // `''` and null share the bucket KEY (pre-existing), but only null is + // absent data. A stored empty string keeps its own (empty) display value. + const r = buildChartSeries( + [{ status: '', priority: 'High', est_hours: 5 }], + ['status', 'priority'], + ['est_hours'], + ); + expect(r.data).toEqual([{ status: '', High: 5 }]); + }); + + it('keeps an empty pivot result empty — no phantom bucket row', () => { + const r = buildChartSeries([], ['status', 'priority'], ['est_hours']); + expect(r.data).toEqual([]); + expect(r.series).toEqual([]); + }); + + it('does NOT bucket a row that lacks the category key ENTIRELY', () => { + // Same division as the single-dimension branch: key absent is a different + // defect (a dimension grouped by but never projected) and must not be + // relabelled "(None)", which would say the records have no value. + // + // MEASURED LIMIT, deliberately pinned: the pivot writes `[xKey]` onto every + // bucket it creates, so this shape reaches the renderer WITH the key and + // `hasNoCategoryKey` (framework#4033) cannot see it — that was already true + // before #4497 and is unchanged by it. Filed separately rather than widened + // into this card. + const r = buildChartSeries( + [{ priority: 'Low', est_hours: 3 }], + ['status', 'priority'], + ['est_hours'], + ); + expect(r.data).toEqual([{ status: undefined, Low: 3 }]); + expect(r.data[0].status).not.toBe(NULL_CATEGORY_LABEL); + }); +}); + describe('findChartSeriesRow — the bucket label maps back to its null row (objectui#4466)', () => { it('matches the bucket label against the raw null category', () => { // Symmetry with buildChartSeries: without it, clicking the rendered @@ -141,3 +280,111 @@ describe('findChartSeriesRow — the bucket label maps back to its null row (obj expect(findChartSeriesRow(ALL_NULL, ['user_id'], ['event_count'], '')).toBe(0); }); }); + +/** + * objectui#4497's DRILL half — the newly-visible pivot bar keeps its click. + * + * The measurement the card asked for, pinned rather than described. The pivot's + * emitted rows are AGGREGATED (`byX` collapses N raw rows into one bucket per + * first-dimension value), so — unlike every table/pivot surface — they are NOT + * index-aligned with `drillRawRows` and a caller cannot drill by the emitted + * row's position. `DatasetWidget.handleChartDrill`, the one production caller, + * therefore SEARCHES the raw rows through `findChartSeriesRow` and indexes + * `drillRawRows` with what it returns. + * + * That is why the pivot's split between map key and emitted display value never + * reached the drill, and why #4497 changed `buildChartSeries` alone: the raw + * rows searched here still carry their null, and `xOf` (objectui#4466) already + * reads a bucket label back to them in the MULTI-dimension arm as well as the + * single-dimension one. These cases pin that the two halves agree — a + * regression in either would be a dead click, which is what #4466 named. + */ +describe('findChartSeriesRow — the pivot bucket bar keeps its drill (objectui#4497)', () => { + /** Raw dataset rows, the shape `drillRawRows` is index-aligned with. */ + const PIVOT_RAW = [ + { status: 'Backlog', priority: 'High', est_hours: 5 }, + { status: null, priority: 'Low', est_hours: 3 }, + { status: null, priority: 'High', est_hours: 8 }, + ]; + + it('resolves the rendered bucket label to the RAW row, per series', () => { + const idxLow = findChartSeriesRow(PIVOT_RAW, ['status', 'priority'], ['est_hours'], NULL_CATEGORY_LABEL, 'Low'); + const idxHigh = findChartSeriesRow(PIVOT_RAW, ['status', 'priority'], ['est_hours'], NULL_CATEGORY_LABEL, 'High'); + expect(idxLow).toBe(1); + expect(idxHigh).toBe(2); + // The index is into the RAW rows — the drill filter is built from + // `drillRawRows[idx]`, which still carries the null the bar was drawn for. + expect(PIVOT_RAW[idxLow]).toEqual({ status: null, priority: 'Low', est_hours: 3 }); + expect(PIVOT_RAW[idxHigh]).toEqual({ status: null, priority: 'High', est_hours: 8 }); + }); + + it('is NOT the emitted row index — the pivot aggregates, so alignment cannot exist', () => { + const emitted = buildChartSeries(PIVOT_RAW, ['status', 'priority'], ['est_hours']).data; + // 3 raw rows → 2 bars. Any caller drilling by emitted position would read + // the wrong record; this helper is what makes the click correct instead. + expect(emitted).toHaveLength(2); + expect(PIVOT_RAW).toHaveLength(3); + expect(emitted[1]).toEqual({ status: NULL_CATEGORY_LABEL, Low: 3, High: 8 }); + expect(findChartSeriesRow(PIVOT_RAW, ['status', 'priority'], ['est_hours'], NULL_CATEGORY_LABEL, 'High')).toBe(2); + }); + + it('matches a caller-supplied localized bucket label the same way', () => { + expect( + findChartSeriesRow(PIVOT_RAW, ['status', 'priority'], ['est_hours'], '(未指定)', 'Low', { + nullCategoryLabel: '(未指定)', + }), + ).toBe(1); + }); + + it('keeps the legacy empty-string category spelling working (unchanged)', () => { + expect(findChartSeriesRow(PIVOT_RAW, ['status', 'priority'], ['est_hours'], '', 'Low')).toBe(1); + }); + + it('leaves non-null pivot drills exactly as they were', () => { + expect(findChartSeriesRow(PIVOT_RAW, ['status', 'priority'], ['est_hours'], 'Backlog', 'High')).toBe(0); + expect(findChartSeriesRow(PIVOT_RAW, ['status', 'priority'], ['est_hours'], 'Backlog', 'Low')).toBe(-1); + }); +}); + +/** + * The two MEASURED AMBIGUITIES of the bucket label, pinned as the limits they + * are (objectui#4497). Neither is created by that card and neither is fixed by + * it — both are properties of the #4466 doctrine itself, filed rather than + * widened into this surface. They are pinned so the next change to either + * branch has to face them explicitly. + */ +describe('findChartSeriesRow — the measured limits of the bucket label (objectui#4497)', () => { + it('DEAD, not wrong: an empty-string row sharing the null bucket has no drill of its own', () => { + // The bucket KEY merges null and '' (`String(xRaw ?? '')`), so these two + // raw rows draw ONE bar carrying both series. The bar is labelled from the + // row that created the bucket, and the `''` row's segment then matches no + // category: -1, which `handleChartDrill` returns on — a no-op click, never + // a drill into the wrong records. Pre-#4497 the whole bar was invisible, so + // this is strictly more affordance than before, not a regression. + const merged = [ + { status: null, priority: 'Low', n: 3 }, + { status: '', priority: 'High', n: 5 }, + ]; + expect(buildChartSeries(merged, ['status', 'priority'], ['n']).data).toEqual([ + { status: NULL_CATEGORY_LABEL, Low: 3, High: 5 }, + ]); + expect(findChartSeriesRow(merged, ['status', 'priority'], ['n'], NULL_CATEGORY_LABEL, 'Low')).toBe(0); + expect(findChartSeriesRow(merged, ['status', 'priority'], ['n'], NULL_CATEGORY_LABEL, 'High')).toBe(-1); + }); + + it('first match wins when a STORED value spells the bucket label literally', () => { + // A row whose stored category IS the label string keeps its own bucket (the + // key is `'(None)'`, not `''`), so two bars carry the same axis text and the + // click resolves to the first. Inherited from the single-dimension branch, + // where #4466 shipped exactly this trade — the pivot does not add to it. + const literal = [ + { status: NULL_CATEGORY_LABEL, priority: 'High', n: 1 }, + { status: null, priority: 'High', n: 2 }, + ]; + expect(buildChartSeries(literal, ['status', 'priority'], ['n']).data).toEqual([ + { status: NULL_CATEGORY_LABEL, High: 1 }, + { status: NULL_CATEGORY_LABEL, High: 2 }, + ]); + expect(findChartSeriesRow(literal, ['status', 'priority'], ['n'], NULL_CATEGORY_LABEL, 'High')).toBe(0); + }); +}); diff --git a/packages/core/src/utils/chart-series.ts b/packages/core/src/utils/chart-series.ts index 90a0a786d..14606d3ef 100644 --- a/packages/core/src/utils/chart-series.ts +++ b/packages/core/src/utils/chart-series.ts @@ -20,10 +20,12 @@ * second-dimension value holding the measure. This makes the second dimension * visible instead of just repeating the x-axis label. * - **otherwise** (single dimension, or multiple measures) → first dimension is - * the x-axis and each measure is its own series (long format passes through), - * with a NULL category value mapped to an explicit bucket label - * ({@link NULL_CATEGORY_LABEL}) so the group renders instead of vanishing - * (objectui#4466). + * the x-axis and each measure is its own series (long format passes through). + * + * BOTH branches map a NULL first-dimension VALUE to an explicit bucket label + * ({@link NULL_CATEGORY_LABEL}) so the group renders instead of vanishing — + * objectui#4466 for the single-dimension branch, objectui#4497 for the pivot. + * One doctrine, one predicate (`isNullCategory`), two call sites. */ export interface ChartResultField { @@ -87,14 +89,33 @@ export interface ChartSeriesOptions { } /** - * Map a null/undefined category VALUE to its bucket label (objectui#4466). - * - * The key must be PRESENT on the row: a row that does not carry the category - * key at all is a different defect with a different answer — the dimension was - * grouped by but never projected — and it belongs to `hasNoCategoryKey` in - * plugin-charts' `AdvancedChartImpl`, which names the unprojected key instead - * of drawing an axis (framework#4033). Bucketing that shape here would ADD the - * key and silently erase that guard's only signal. + * Does this row carry `key` with a NULL/undefined value — the shape that gets + * the bucket label (objectui#4466 / objectui#4497)? + * + * ONE predicate for both of `buildChartSeries`' branches, so the doctrine + * cannot drift between them: the pivot inherited the single-dimension answer in + * objectui#4497 and this is the whole of what "the same answer" means. + * + * The key must be PRESENT: a row that does not carry the category key at all is + * a different defect with a different answer — the dimension was grouped by but + * never projected — and it belongs to `hasNoCategoryKey` in plugin-charts' + * `AdvancedChartImpl`, which names the unprojected key instead of drawing an + * axis (framework#4033). Bucketing that shape would relabel "this dimension was + * never projected" as "these records have no value", which is a different + * sentence and a false one. + */ +function isNullCategory(row: unknown, key: string): boolean { + return ( + !!row && + typeof row === 'object' && + key in row && + (row as Record)[key] == null + ); +} + +/** + * Map a null/undefined category VALUE to its bucket label, for the + * single-dimension branch (objectui#4466). * * Returns the input array itself when nothing was null, and copies only the * rows it rewrites, so the caller's rows are never mutated — dataset surfaces @@ -107,7 +128,7 @@ function bucketNullCategories( ): Array> { let changed = false; const next = rows.map((row) => { - if (!row || typeof row !== 'object' || !(key in row) || row[key] != null) return row; + if (!isNullCategory(row, key)) return row; changed = true; return { ...row, [key]: label }; }); @@ -132,13 +153,26 @@ export function buildChartSeries( const xKey = dims[0]; const groupKey = dims[1]; const measure = vals[0]; + const nullLabel = options?.nullCategoryLabel ?? NULL_CATEGORY_LABEL; const seriesKeys: string[] = []; const byX = new Map>(); for (const row of safeRows) { const xRaw = row[xKey]; const xId = String(xRaw ?? ''); - if (!byX.has(xId)) byX.set(xId, { [xKey]: xRaw }); + // The bucket KEY is untouched — `String(xRaw ?? '')` still decides WHICH + // rows share a bar, so every existing grouping is byte-identical. What + // changes is the DISPLAY value the bucket carries: a null first-dimension + // value used to be written into the emitted row raw, so the bar reached + // recharts with a null category and drew no mark — the same defect + // objectui#4466 fixed one branch below, and the reason key and row value + // were two different things here (objectui#4497). + // + // The bucket takes its label from the row that CREATED it, exactly as it + // took its raw value before. + if (!byX.has(xId)) { + byX.set(xId, { [xKey]: isNullCategory(row, xKey) ? nullLabel : xRaw }); + } const gId = String(row[groupKey] ?? ''); if (gId !== '' && !seriesKeys.includes(gId)) seriesKeys.push(gId); byX.get(xId)![gId] = row[measure]; @@ -162,10 +196,11 @@ export function buildChartSeries( // {user_id: 'Dev Admin', event_count: 2}]` drew one bar, dropping the // dominant group while the y-axis scale still accommodated it. // - // The multi-dimension branch above is deliberately NOT changed here: its x - // buckets are keyed `String(xRaw ?? '')` and it carries the raw value into - // the pivoted row, so it needs its own answer (and its own pin) rather than - // this one applied on the way past. + // The pivot branch above answers this the SAME way as of objectui#4497 — it + // was left alone here only until its own bucketing had been measured, which + // is what that card did. The two branches differ in WHERE the label lands + // (there: the bucket's display value, the map key untouched; here: the row + // itself), never in what a null category means. const xKey = dims[0]; return { data: xKey @@ -245,6 +280,18 @@ export function relabelDimensions( * to `-1` and its drill-through would silently no-op. Pass the same * `nullCategoryLabel` both helpers were given. The pre-existing `''` spelling * of "no group value" still matches too — `computeDrillFilter` writes that one. + * + * **`xOf` covers BOTH arms, which is what let objectui#4497 bucket the pivot + * branch without touching this function.** Worth stating because the reverse + * would be silent: the pivot's emitted rows are AGGREGATED (`byX` collapses N + * raw rows into one bucket per first-dimension value), so they are NOT + * index-aligned with anything, and a caller cannot drill by the emitted row's + * position. It drills by SEARCHING the raw rows here and indexing + * `drillRawRows` with what this returns — see `DatasetWidget.handleChartDrill`, + * the one production caller. So the pivot's map key and its emitted display + * value being two different things never reached the drill: the raw rows this + * searches still carry their null either way, and `xOf` reads the label back to + * them in the multi-dimension arm exactly as in the single-dimension one. */ export function findChartSeriesRow( rows: Array> | null | undefined, diff --git a/packages/plugin-charts/src/AdvancedChartImpl.pivotNullBucket.test.tsx b/packages/plugin-charts/src/AdvancedChartImpl.pivotNullBucket.test.tsx new file mode 100644 index 000000000..953a0aa7a --- /dev/null +++ b/packages/plugin-charts/src/AdvancedChartImpl.pivotNullBucket.test.tsx @@ -0,0 +1,170 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * objectui#4497 — the MULTI-DIMENSION PIVOT branch's null bucket, at the DOM. + * + * The sibling of `AdvancedChartImpl.nullCategoryBucket.test.tsx` (objectui#4466), + * which pins the same property for the single-dimension branch. Both render the + * composition the two consumers actually perform — the shared `buildChartSeries` + * transform feeding `AdvancedChartImpl` — and count the marks recharts draws, + * because the defect was never visible in the transform's return value alone: a + * `{status: null, Low: 3}` row looks like data right up until recharts is asked + * to place it on a category axis and draws nothing. + * + * Measured on this file's own fixture, with the fix reverted: 1 bar rectangle + * for a two-group pivot, the null-keyed group's bar missing while its series + * still occupied the legend. Post-fix: 2, the second labelled with the bucket. + * + * The drill half is pinned here too, at the seam where a dead click would come + * from: the axis label a user sees IS what recharts hands back as `activeLabel` + * (see `handleCartesianClick`), so the string read off the DOM below is fed to + * `findChartSeriesRow` exactly as `DatasetWidget.handleChartDrill` feeds the + * click event to it. A real SVG segment click is deliberately not simulated — + * in jsdom that tests recharts' hit-test geometry rather than this branch, the + * same reason `ObjectChart.drillNavigate.test.tsx` mocks its click surface. + */ + +import React from 'react'; +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { render, cleanup } from '@testing-library/react'; +import { buildChartSeries, findChartSeriesRow, NULL_CATEGORY_LABEL } from '@object-ui/core'; + +// Recharts' ResponsiveContainer measures via ResizeObserver, which reports 0×0 +// under the headless DOM, so nothing paints. Fix its size. +vi.mock('recharts', async () => { + const actual = await vi.importActual('recharts'); + return { + ...actual, + ResponsiveContainer: ({ children }: any) => + React.cloneElement(children, { width: 480, height: 320 }), + }; +}); + +import AdvancedChartImpl from './AdvancedChartImpl'; + +afterEach(cleanup); + +/** + * A pivot-shaped result: 2 dimensions + 1 measure, one group's first-dimension + * value NULL. The dataset shape behind it is ordinary — any `GROUP BY status, + * priority` over records whose `status` was never set. + */ +const PIVOT_RAW = [ + { status: 'Backlog', priority: 'High', est_hours: 5 }, + { status: null, priority: 'Low', est_hours: 3 }, +]; + +const DIMS = ['status', 'priority']; +const VALS = ['est_hours']; + +/** Render exactly what a dataset-bound pivot chart renders. */ +const renderPivot = ( + rows: Array>, + options?: { nullCategoryLabel?: string }, +) => { + const { data, xAxisKey, series } = buildChartSeries(rows, DIMS, VALS, null, options); + return render( + , + ); +}; + +const barCount = (container: HTMLElement) => + container.querySelectorAll('.recharts-bar-rectangle').length; + +/** + * Every string recharts painted. Deliberately NOT `.recharts-xAxis text`: + * recharts 3 draws tick labels in their own z-index layer rather than inside the + * axis group, so that selector reports `[]` for a chart with a perfectly good + * axis (the same workaround `AdvancedChartImpl.nullCategoryBucket.test.tsx` and + * `AdvancedChartImpl.specConfig.test.tsx` use). + */ +const chartTexts = (container: HTMLElement) => + Array.from(container.querySelectorAll('text')).map((t) => t.textContent ?? ''); + +describe('AdvancedChartImpl — the pivot null bucket renders (objectui#4497)', () => { + it('draws BOTH pivot groups, the null-keyed one labelled', () => { + const { container } = renderPivot(PIVOT_RAW); + // Pre-fix this was 1: the null group's bar never drew, because its emitted + // row carried the RAW null into the category axis. + expect(barCount(container)).toBe(2); + expect(chartTexts(container)).toContain(NULL_CATEGORY_LABEL); + expect(chartTexts(container)).toContain('Backlog'); + }); + + it('renders the caller-supplied localized label on the pivot axis', () => { + const { container } = renderPivot(PIVOT_RAW, { nullCategoryLabel: '(未指定)' }); + expect(barCount(container)).toBe(2); + expect(chartTexts(container)).toContain('(未指定)'); + expect(chartTexts(container)).not.toContain(NULL_CATEGORY_LABEL); + }); + + it('draws the all-null pivot instead of an axis with no marks', () => { + const { container } = renderPivot([ + { status: null, priority: 'Low', est_hours: 3 }, + { status: null, priority: 'High', est_hours: 8 }, + ]); + // One bucket, one bar per series — pre-fix: zero marks under a drawn axis. + expect(barCount(container)).toBe(2); + expect(chartTexts(container)).toContain(NULL_CATEGORY_LABEL); + }); +}); + +describe('AdvancedChartImpl — the pivot bucket bar keeps its drill (objectui#4497)', () => { + it('resolves the label AS RENDERED back to the raw row it was drawn from', () => { + const { container } = renderPivot(PIVOT_RAW); + + // Read the bucket's category off the DOM rather than asserting a constant: + // the string the user sees is the one recharts reports as `activeLabel`, + // and a drill is dead precisely when those two drift apart. + const rendered = chartTexts(container).find((t) => t === NULL_CATEGORY_LABEL); + expect(rendered).toBeDefined(); + + // The pivot AGGREGATES, so the emitted rows are not index-aligned with the + // raw ones; `DatasetWidget` drills by searching the raw rows with exactly + // this call and indexing `drillRawRows` with the result. + const idx = findChartSeriesRow(PIVOT_RAW, DIMS, VALS, rendered, 'Low'); + expect(idx).toBe(1); + expect(PIVOT_RAW[idx]).toEqual({ status: null, priority: 'Low', est_hours: 3 }); + }); + + it('resolves a localized bucket label the same way, when both halves get it', () => { + const { container } = renderPivot(PIVOT_RAW, { nullCategoryLabel: '(未指定)' }); + const rendered = chartTexts(container).find((t) => t === '(未指定)'); + expect(rendered).toBeDefined(); + expect( + findChartSeriesRow(PIVOT_RAW, DIMS, VALS, rendered, 'Low', { nullCategoryLabel: '(未指定)' }), + ).toBe(1); + }); + + it('still drills the non-null pivot group exactly as before', () => { + const { container } = renderPivot(PIVOT_RAW); + const rendered = chartTexts(container).find((t) => t === 'Backlog'); + expect(rendered).toBeDefined(); + expect(findChartSeriesRow(PIVOT_RAW, DIMS, VALS, rendered, 'High')).toBe(0); + }); +}); + +describe('AdvancedChartImpl — pivot must-not-change (objectui#4497)', () => { + it('draws non-null pivot groups exactly as before', () => { + const { container } = renderPivot([ + { status: 'Backlog', priority: 'High', est_hours: 5 }, + { status: 'Done', priority: 'High', est_hours: 24 }, + ]); + expect(barCount(container)).toBe(2); + expect(chartTexts(container)).toEqual(expect.arrayContaining(['Backlog', 'Done'])); + expect(chartTexts(container)).not.toContain(NULL_CATEGORY_LABEL); + }); + + it('keeps a genuinely empty pivot result empty — no phantom bucket bar', () => { + const { container } = renderPivot([]); + expect(barCount(container)).toBe(0); + expect(chartTexts(container)).not.toContain(NULL_CATEGORY_LABEL); + }); +}); From b3188e2302ebe7fadd2896cc1e0ef86c7d1e1e86 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 02:09:21 +0000 Subject: [PATCH 2/2] test(charts): pin the merged null/'' bucket in BOTH row orders (#4497) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The card's STOP condition — "two raw groups mapping to one label" — was measured in only one of its two row orders. The suite pinned [null first, '' second], where the bucket renders `(None)` and the `''` row's segment drills to -1 (a dead click, filed as #4508). The reverse order was left unmeasured, and it does not behave alike: with the `''` row creating the bucket the label is the raw empty string, `isNullCategory` never fires, and BOTH segments resolve correctly. Only one of the two orders costs a drill. Green on both sides of the fix, deliberately: this order is byte-identical before and after (verified against origin/main's chart-series.ts), so it is a must-not-change control rather than a red-first case. It also pins one asymmetry the writer cannot show: `findChartSeriesRow` accepts BOTH spellings of "no value" unconditionally, so it resolves the bucket label even in the order where no bar carries it. That is #4466's stated design and is harmless — a renderer only ever hands back a category recharts painted — but it is invisible from `buildChartSeries` alone, and it is the slack that keeps the two callers' labels from having to agree. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 --- .../utils/chart-series.nullCategory.test.ts | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/packages/core/src/utils/chart-series.nullCategory.test.ts b/packages/core/src/utils/chart-series.nullCategory.test.ts index c041391a0..fac3db8dc 100644 --- a/packages/core/src/utils/chart-series.nullCategory.test.ts +++ b/packages/core/src/utils/chart-series.nullCategory.test.ts @@ -372,6 +372,41 @@ describe('findChartSeriesRow — the measured limits of the bucket label (object expect(findChartSeriesRow(merged, ['status', 'priority'], ['n'], NULL_CATEGORY_LABEL, 'High')).toBe(-1); }); + it('and the SAME two groups keep both drills when the empty-string row comes first', () => { + // The other row order of the case above, pinned because the two do NOT + // behave alike and only one of them loses anything. The bucket takes its + // label from whichever row CREATED it: with the `''` row first the label is + // the raw empty string, `isNullCategory` never fires, and `xOf` reads a null + // row back as `''` whenever the clicked category is not the bucket label — + // so BOTH segments resolve, and neither resolves to the wrong record. + // + // This is the ruling's STOP condition measured to its end (objectui#4497): + // "two raw groups mapping to one label" is real, but it costs a drill in + // exactly one of the two orders. Widening `findChartSeriesRow` to close that + // one would make a bar labelled `(None)` drill to a row whose stored value + // is `''` — a wrong click traded for a dead one — so the limit is filed + // (objectui#4508) rather than fixed here. + const merged = [ + { status: '', priority: 'High', n: 5 }, + { status: null, priority: 'Low', n: 3 }, + ]; + expect(buildChartSeries(merged, ['status', 'priority'], ['n']).data).toEqual([ + { status: '', High: 5, Low: 3 }, + ]); + expect(findChartSeriesRow(merged, ['status', 'priority'], ['n'], '', 'High')).toBe(0); + expect(findChartSeriesRow(merged, ['status', 'priority'], ['n'], '', 'Low')).toBe(1); + + // MEASURED, and deliberately not what was predicted: the reader is looser + // than the writer. `xOf` accepts BOTH spellings of "no value" (the bucket + // label and the legacy `''`) unconditionally — #4466's stated design — so + // it resolves the label even in this order, where NO bar was labelled with + // it. Harmless in the real flow, because the only category a renderer ever + // hands back is one recharts actually painted (here: `''`), and it is the + // slack that makes the two callers' label agreement a non-issue. Pinned + // because the asymmetry is invisible from `buildChartSeries` alone. + expect(findChartSeriesRow(merged, ['status', 'priority'], ['n'], NULL_CATEGORY_LABEL, 'Low')).toBe(1); + }); + it('first match wins when a STORED value spells the bucket label literally', () => { // A row whose stored category IS the label string keeps its own bucket (the // key is `'(None)'`, not `''`), so two bars carry the same axis text and the