diff --git a/.changeset/chart-null-category-bucket-4466.md b/.changeset/chart-null-category-bucket-4466.md new file mode 100644 index 000000000..bdef23cad --- /dev/null +++ b/.changeset/chart-null-category-bucket-4466.md @@ -0,0 +1,17 @@ +--- +'@object-ui/core': minor +'@object-ui/plugin-charts': patch +'@object-ui/i18n': patch +--- + +A null-keyed group renders as an explicit bucket instead of silently vanishing from a chart (objectui#4466) + +`buildChartSeries`' single-dimension branch passed rows through verbatim, so a row whose category VALUE is `null` reached recharts with a null category and drew no mark. The visible outcome was not an empty chart but a quietly wrong one: rows `[{user_id: null, event_count: 51}, {user_id: 'Dev Admin', event_count: 2}]` drew exactly ONE bar — the dominant group, 51 of 53 events, dropped while the y-axis scale still accommodated it, so the chart understated its own data and the axis proved the data had been there. With every group null it drew axes, gridlines and an axis title with zero marks and no empty state, which is the shipped first-boot state of the built-in System Overview board's "Events by User" (every seeded `sys_audit_log` row is written with `user_id = NULL`). + +The mapping lives in the shared series layer, so dashboard widgets and standalone `ObjectChart` get one answer rather than a per-chart patch in the recharts wrapper. It resolves the two-answers disagreement the card names as well: an empty result set keeps the designed empty state, a non-empty result always draws bars — the null bucket included. + +`@object-ui/core` gains `NULL_CATEGORY_LABEL` and `ChartSeriesOptions`; `buildChartSeries` and `findChartSeriesRow` each take an optional trailing `options`. Both additive — every existing call site compiles and behaves identically, and a result with no null category is still returned by array identity. The two helpers are a pair on purpose: the caller matches a clicked segment against rows that still carry the raw `null`, so `findChartSeriesRow` reads the bucket label back to that row and the newly-visible bar keeps its drill-through instead of resolving to `-1`. + +The label goes through the i18n channel (`chart.nullCategory`, en `(None)` / zh `(未指定)`, all ten packs), passed down by the renderer: `@object-ui/core` is React-free and cannot read the locale bundle, so it takes the resolved string the same way `dimensionOptionTranslator` takes a resolver. Its English constant is the floor for a provider-less host, not the mechanism. + +`hasNoCategoryKey` (framework#4033) is untouched and now documented against this: a row that does not carry the category key AT ALL is a different defect — a dimension grouped by but never projected — and keeps its explanatory placeholder. The bucket deliberately never ADDS the key to such a row, which is what keeps that guard's signal alive. Key absent → the placeholder; key present with a null value → the bucket. diff --git a/packages/core/src/utils/chart-series.nullCategory.test.ts b/packages/core/src/utils/chart-series.nullCategory.test.ts new file mode 100644 index 000000000..dc8cf1c4b --- /dev/null +++ b/packages/core/src/utils/chart-series.nullCategory.test.ts @@ -0,0 +1,143 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#4466 — a NULL-keyed group must render as an explicit bucket, never + * vanish. + * + * The single-dimension branch of `buildChartSeries` passed rows through + * verbatim, so a row whose category VALUE is `null` reached recharts with a + * null category and drew no mark. The sharpest measured case is the partial + * one: `[{user_id: null, event_count: 51}, {user_id: 'Dev Admin', + * event_count: 2}]` drew exactly ONE bar — the DOMINANT group (51 of 53 + * events) silently dropped, while the y-axis scale still accommodated it. The + * chart was not merely empty, it understated its own data without saying so. + * + * This is the shipped first-boot state of the built-in System Overview board: + * every seeded `sys_audit_log` row is written with `user_id = NULL`, so "Events + * by User" groups to exactly one row and drew nothing at all. + * + * The division of labour with the framework#4033 guard is pinned here too, + * because the fix could easily erase it: a row that does not carry the category + * key AT ALL is NOT bucketed — that shape belongs to `hasNoCategoryKey` + * (plugin-charts' `AdvancedChartImpl`), which explains itself instead of + * drawing an empty axis. Key absent → that path; key present, value null → + * this bucket. + */ +import { describe, it, expect } from 'vitest'; +import { buildChartSeries, findChartSeriesRow, NULL_CATEGORY_LABEL } from './chart-series'; + +/** The card's case 3, verbatim — the dominant group is the null-keyed one. */ +const PARTIAL = [ + { user_id: null, event_count: 51 }, + { user_id: 'Dev Admin', event_count: 2 }, +]; + +/** The card's case 1/2 — the organic first-boot seed state. */ +const ALL_NULL = [{ user_id: null, event_count: 50 }]; + +describe('buildChartSeries — null-keyed category bucket (objectui#4466)', () => { + it('labels the null group instead of dropping it (the partial case)', () => { + const r = buildChartSeries(PARTIAL, ['user_id'], ['event_count']); + + expect(r.xAxisKey).toBe('user_id'); + // BOTH groups survive, and the null one keeps its 51 events. + expect(r.data).toEqual([ + { user_id: NULL_CATEGORY_LABEL, event_count: 51 }, + { user_id: 'Dev Admin', event_count: 2 }, + ]); + }); + + it('labels the all-null result rather than drawing an axis with no marks', () => { + const r = buildChartSeries(ALL_NULL, ['user_id'], ['event_count']); + expect(r.data).toEqual([{ user_id: NULL_CATEGORY_LABEL, event_count: 50 }]); + }); + + it('buckets an undefined category value the same way', () => { + const r = buildChartSeries([{ user_id: undefined, event_count: 7 }], ['user_id'], ['event_count']); + expect(r.data).toEqual([{ user_id: NULL_CATEGORY_LABEL, event_count: 7 }]); + }); + + it('uses the caller-supplied (localized) label when one is given', () => { + const r = buildChartSeries(ALL_NULL, ['user_id'], ['event_count'], null, { + nullCategoryLabel: '(未指定)', + }); + expect(r.data).toEqual([{ user_id: '(未指定)', event_count: 50 }]); + }); + + it('never mutates the caller rows — drill-through reads the raw null', () => { + const rows = [{ user_id: null, event_count: 50 }]; + buildChartSeries(rows, ['user_id'], ['event_count']); + expect(rows[0].user_id).toBeNull(); + }); + + it('leaves a row that lacks the category key ENTIRELY to the #4033 guard', () => { + // Adding the key here would erase `hasNoCategoryKey`'s whole signal: the + // renderer would draw an "(None)" axis instead of naming the unprojected + // dimension. Key absent is a different defect with a different answer. + const unreadable = [{ count: 2 }, { count: 8 }]; + const r = buildChartSeries(unreadable, ['issued'], ['count']); + expect(r.data).toBe(unreadable); + expect(r.data.every((row) => !('issued' in row))).toBe(true); + }); +}); + +describe('buildChartSeries — must-not-change (objectui#4466)', () => { + it('returns non-null rows BY IDENTITY, unchanged', () => { + const rows = [ + { status: 'Backlog', est_hours: 5 }, + { status: 'Done', est_hours: 24 }, + ]; + const r = buildChartSeries(rows, ['status'], ['est_hours']); + expect(r.data).toBe(rows); + expect(r.data).toEqual(rows); + }); + + it('keeps an empty result set empty — the designed empty state is untouched', () => { + const r = buildChartSeries([], ['user_id'], ['event_count']); + expect(r.data).toEqual([]); + }); + + it('leaves the multi-dimension pivot branch exactly as it was', () => { + 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 }, + ]); + }); +}); + +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 + // "(None)" bar resolves to index -1 and the drill silently no-ops. + expect(findChartSeriesRow(PARTIAL, ['user_id'], ['event_count'], NULL_CATEGORY_LABEL)).toBe(0); + expect(findChartSeriesRow(PARTIAL, ['user_id'], ['event_count'], 'Dev Admin')).toBe(1); + }); + + it('matches a caller-supplied bucket label the same way', () => { + expect( + findChartSeriesRow(ALL_NULL, ['user_id'], ['event_count'], '(未指定)', undefined, { + nullCategoryLabel: '(未指定)', + }), + ).toBe(0); + }); + + it('still resolves the empty-string category to a null row (unchanged)', () => { + // The pre-existing `String(r[xDim] ?? '')` behaviour, kept: the pivot/drill + // layer already spells "no group value" as '' (see computeDrillFilter). + expect(findChartSeriesRow(ALL_NULL, ['user_id'], ['event_count'], '')).toBe(0); + }); +}); diff --git a/packages/core/src/utils/chart-series.ts b/packages/core/src/utils/chart-series.ts index 1965e7313..90a0a786d 100644 --- a/packages/core/src/utils/chart-series.ts +++ b/packages/core/src/utils/chart-series.ts @@ -20,7 +20,10 @@ * 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). + * 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). */ export interface ChartResultField { @@ -52,11 +55,71 @@ export interface ChartSeriesResult { series: ChartSeriesBinding[]; } +/** + * The bucket a NULL/undefined category VALUE renders under, when the caller + * supplies no localized label of its own (objectui#4466). + * + * English by construction and deliberately so: this package is React-free and + * i18n-free (see {@link OptionLabelTranslator} for the same reason stated one + * layer down), so the locale bundle cannot be read here. Every renderer that + * has an i18n provider passes its own resolved label through + * {@link ChartSeriesOptions.nullCategoryLabel} — this constant is the floor + * that keeps a provider-less host (or a caller that has not been wired yet) + * drawing the group rather than dropping it. + * + * Exported so a consumer can recognise the bucket it will be handed — + * {@link findChartSeriesRow} uses the same default, which is what makes a + * clicked bucket bar resolve back to its (raw null) dataset row. + */ +export const NULL_CATEGORY_LABEL = '(None)'; + +/** Per-call knobs shared by {@link buildChartSeries} / {@link findChartSeriesRow}. */ +export interface ChartSeriesOptions { + /** + * Display label for a category whose value is `null`/`undefined`. Defaults to + * {@link NULL_CATEGORY_LABEL}. + * + * Pass the SAME value to both helpers: `buildChartSeries` writes it into the + * rendered rows and `findChartSeriesRow` matches a clicked category against + * it, so a mismatch costs the null bucket its drill-through. + */ + nullCategoryLabel?: string; +} + +/** + * 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. + * + * 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 + * drill through by index into the RAW rows, which must keep their null. + */ +function bucketNullCategories( + rows: Array>, + key: string, + label: string, +): Array> { + let changed = false; + const next = rows.map((row) => { + if (!row || typeof row !== 'object' || !(key in row) || row[key] != null) return row; + changed = true; + return { ...row, [key]: label }; + }); + return changed ? next : rows; +} + export function buildChartSeries( rows: Array> | null | undefined, dimensions: string[] | null | undefined, values: string[] | null | undefined, fields?: ChartResultField[] | null, + options?: ChartSeriesOptions, ): ChartSeriesResult { const dims = (dimensions ?? []).filter(Boolean); const vals = (values ?? []).filter(Boolean); @@ -91,9 +154,24 @@ export function buildChartSeries( } // Default: first dimension on the x-axis, one series per measure. + // + // A row whose category VALUE is null renders under an explicit bucket rather + // than reaching the renderer with a null category, which draws no mark at all + // (objectui#4466). The measured cost of passing it through was not an empty + // chart but a quietly WRONG one: `[{user_id: null, event_count: 51}, + // {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. + const xKey = dims[0]; return { - data: safeRows, - xAxisKey: dims[0], + data: xKey + ? bucketNullCategories(safeRows, xKey, options?.nullCategoryLabel ?? NULL_CATEGORY_LABEL) + : safeRows, + xAxisKey: xKey, series: vals.map((v) => ({ dataKey: v, label: labelOf(v) })), }; } @@ -159,6 +237,14 @@ export function relabelDimensions( * * Comparison is string-wise on the rows' display values (which is what the chart * surfaces as `category` / series key). Returns `-1` when nothing matches. + * + * The NULL-category bucket {@link buildChartSeries} renders (objectui#4466) is + * matched back to its row here, and it has to be: the caller passes the rows it + * charted FROM (still carrying the raw null) while the click event carries the + * bucket LABEL, so without this the one bar the fix made visible would resolve + * 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. */ export function findChartSeriesRow( rows: Array> | null | undefined, @@ -166,6 +252,7 @@ export function findChartSeriesRow( values: string[] | null | undefined, category: string | undefined, seriesKey?: string, + options?: ChartSeriesOptions, ): number { const dims = (dimensions ?? []).filter(Boolean); const vals = (values ?? []).filter(Boolean); @@ -173,12 +260,17 @@ export function findChartSeriesRow( const xDim = dims[0]; if (!xDim) return -1; const c = String(category ?? ''); + const nullLabel = options?.nullCategoryLabel ?? NULL_CATEGORY_LABEL; + // A null x reads as BOTH its rendered bucket label and the legacy '' — the + // two spellings of the same fact, neither of which a stored value can be. + const xOf = (r: Record): string => + r[xDim] == null ? (c === nullLabel ? nullLabel : '') : String(r[xDim]); if (dims.length >= 2 && vals.length === 1) { const gDim = dims[1]; const s = String(seriesKey ?? ''); - return safeRows.findIndex((r) => String(r[xDim] ?? '') === c && String(r[gDim] ?? '') === s); + return safeRows.findIndex((r) => xOf(r) === c && String(r[gDim] ?? '') === s); } - return safeRows.findIndex((r) => String(r[xDim] ?? '') === c); + return safeRows.findIndex((r) => xOf(r) === c); } /** diff --git a/packages/i18n/src/locales/ar.ts b/packages/i18n/src/locales/ar.ts index 7d295ffd5..ba6afd512 100644 --- a/packages/i18n/src/locales/ar.ts +++ b/packages/i18n/src/locales/ar.ts @@ -969,6 +969,7 @@ const ar = { chart: { noData: "لا تتوفر بيانات للرسم البياني", loading: "جاري تحميل الرسم البياني…", + nullCategory: "(غير محدد)", }, map: { searchLocations: "البحث عن المواقع…", diff --git a/packages/i18n/src/locales/de.ts b/packages/i18n/src/locales/de.ts index 075b36b2e..5b4b6d379 100644 --- a/packages/i18n/src/locales/de.ts +++ b/packages/i18n/src/locales/de.ts @@ -962,6 +962,7 @@ const de = { chart: { noData: "Keine Diagrammdaten verfügbar", loading: "Diagramm wird geladen…", + nullCategory: "(Ohne Angabe)", }, map: { searchLocations: "Orte suchen…", diff --git a/packages/i18n/src/locales/en.ts b/packages/i18n/src/locales/en.ts index 324e6c9e4..b566d954f 100644 --- a/packages/i18n/src/locales/en.ts +++ b/packages/i18n/src/locales/en.ts @@ -1045,6 +1045,7 @@ const en = { chart: { noData: 'No chart data available', loading: 'Loading chart…', + nullCategory: '(None)', }, report: { total: 'Total', diff --git a/packages/i18n/src/locales/es.ts b/packages/i18n/src/locales/es.ts index a0a5439b2..dc646ab92 100644 --- a/packages/i18n/src/locales/es.ts +++ b/packages/i18n/src/locales/es.ts @@ -966,6 +966,7 @@ const es = { chart: { noData: "No hay datos de gráfico disponibles", loading: "Cargando gráfico…", + nullCategory: "(Sin especificar)", }, map: { searchLocations: "Buscar ubicaciones…", diff --git a/packages/i18n/src/locales/fr.ts b/packages/i18n/src/locales/fr.ts index 78a3a4939..493f386ae 100644 --- a/packages/i18n/src/locales/fr.ts +++ b/packages/i18n/src/locales/fr.ts @@ -964,6 +964,7 @@ const fr = { chart: { noData: "Aucune donnée de graphique disponible", loading: "Chargement du graphique…", + nullCategory: "(Non défini)", }, map: { searchLocations: "Rechercher des lieux…", diff --git a/packages/i18n/src/locales/ja.ts b/packages/i18n/src/locales/ja.ts index 025e1c895..b264df508 100644 --- a/packages/i18n/src/locales/ja.ts +++ b/packages/i18n/src/locales/ja.ts @@ -962,6 +962,7 @@ const ja = { chart: { noData: "チャートデータがありません", loading: "チャート読み込み中…", + nullCategory: "(未設定)", }, map: { searchLocations: "場所を検索…", diff --git a/packages/i18n/src/locales/ko.ts b/packages/i18n/src/locales/ko.ts index a8fd36f0d..9c46ccb05 100644 --- a/packages/i18n/src/locales/ko.ts +++ b/packages/i18n/src/locales/ko.ts @@ -962,6 +962,7 @@ const ko = { chart: { noData: "차트 데이터가 없습니다", loading: "차트 로딩 중…", + nullCategory: "(미지정)", }, map: { searchLocations: "위치 검색…", diff --git a/packages/i18n/src/locales/pt.ts b/packages/i18n/src/locales/pt.ts index 89d149f98..5ba3c5004 100644 --- a/packages/i18n/src/locales/pt.ts +++ b/packages/i18n/src/locales/pt.ts @@ -961,6 +961,7 @@ const pt = { chart: { noData: "Nenhum dado de gráfico disponível", loading: "Carregando gráfico…", + nullCategory: "(Não especificado)", }, map: { searchLocations: "Pesquisar locais…", diff --git a/packages/i18n/src/locales/ru.ts b/packages/i18n/src/locales/ru.ts index b0f77d08c..7f00b78fe 100644 --- a/packages/i18n/src/locales/ru.ts +++ b/packages/i18n/src/locales/ru.ts @@ -972,6 +972,7 @@ const ru = { chart: { noData: "Нет данных для графика", loading: "Загрузка графика…", + nullCategory: "(Не указано)", }, map: { searchLocations: "Поиск местоположений…", diff --git a/packages/i18n/src/locales/zh.ts b/packages/i18n/src/locales/zh.ts index 3534c1468..8b9b70289 100644 --- a/packages/i18n/src/locales/zh.ts +++ b/packages/i18n/src/locales/zh.ts @@ -998,6 +998,7 @@ const zh = { chart: { noData: '暂无图表数据', loading: '图表加载中…', + nullCategory: '(未指定)', }, report: { total: '总计', diff --git a/packages/plugin-charts/src/AdvancedChartImpl.nullCategoryBucket.test.tsx b/packages/plugin-charts/src/AdvancedChartImpl.nullCategoryBucket.test.tsx new file mode 100644 index 000000000..1e382cd82 --- /dev/null +++ b/packages/plugin-charts/src/AdvancedChartImpl.nullCategoryBucket.test.tsx @@ -0,0 +1,136 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * objectui#4466 — a NULL-keyed group renders as an explicit bucket, at the DOM. + * + * The defect was proven at two levels, so it is pinned at two levels. The unit + * half lives in `packages/core/src/utils/chart-series.nullCategory.test.ts`; + * this half renders the SAME composition both consumers perform — the shared + * `buildChartSeries` transform feeding `AdvancedChartImpl` — and counts the + * marks recharts actually draws. + * + * Measured on the shipped first-boot state of the built-in System Overview + * board ("Events by User", every seeded `sys_audit_log` row written with + * `user_id = NULL`): `.recharts-bar` = 1, `.recharts-bar-rectangle` = 0 — axes, + * gridlines and the axis label with no marks and no empty state. The partial + * case is sharper still: two groups in, ONE bar out, the dominant (null-keyed) + * group silently dropped while the y-axis scale still accommodated it. + * + * The framework#4033 guard's half of the division is pinned here too: rows that + * lack the category key entirely still reach the explanatory placeholder. Key + * absent → that path; key present with a null value → this bucket. + */ + +import React from 'react'; +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { render, cleanup } from '@testing-library/react'; +import { buildChartSeries, 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); + +/** The card's case 3 — the sharpest: the dropped group is the DOMINANT one. */ +const PARTIAL = [ + { user_id: null, event_count: 51 }, + { user_id: 'Dev Admin', event_count: 2 }, +]; + +/** The card's case 1/2 — the organic first-boot seed state. */ +const ALL_NULL = [{ user_id: null, event_count: 50 }]; + +/** Render exactly what a dataset-bound chart renders: the shared transform's output. */ +const renderDataset = (rows: Array>) => { + const { data, xAxisKey, series } = buildChartSeries(rows, ['user_id'], ['event_count']); + 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 (measured — it is what `AdvancedChartImpl.specConfig.test.tsx` + * works around too). + */ +const chartTexts = (container: HTMLElement) => + Array.from(container.querySelectorAll('text')).map((t) => t.textContent ?? ''); + +describe('AdvancedChartImpl — null-keyed group renders as a bucket (objectui#4466)', () => { + it('draws BOTH bars for the partial case, the null one labelled and counted', () => { + const { container } = renderDataset(PARTIAL); + // Pre-fix this was 1: the 51-event group vanished while the axis kept its + // scale, so the chart understated its own data without saying so. + expect(barCount(container)).toBe(2); + expect(chartTexts(container)).toContain(NULL_CATEGORY_LABEL); + expect(chartTexts(container)).toContain('Dev Admin'); + }); + + it('draws one labelled bar for the all-null result instead of an empty axis', () => { + const { container } = renderDataset(ALL_NULL); + // Pre-fix: axes and gridlines with ZERO bar rectangles and no empty state. + expect(barCount(container)).toBe(1); + expect(chartTexts(container)).toContain(NULL_CATEGORY_LABEL); + }); + + it('renders the caller-supplied localized label on the axis', () => { + const { data, xAxisKey, series } = buildChartSeries(ALL_NULL, ['user_id'], ['event_count'], null, { + nullCategoryLabel: '(未指定)', + }); + const { container } = render( + , + ); + expect(chartTexts(container)).toContain('(未指定)'); + }); +}); + +describe('AdvancedChartImpl — must-not-change (objectui#4466)', () => { + it('draws non-null groups exactly as before', () => { + const { container } = renderDataset([ + { user_id: 'Dev Admin', event_count: 2 }, + { user_id: 'Ops', event_count: 3 }, + ]); + expect(barCount(container)).toBe(2); + expect(chartTexts(container)).toEqual(expect.arrayContaining(['Dev Admin', 'Ops'])); + expect(chartTexts(container)).not.toContain(NULL_CATEGORY_LABEL); + }); + + it('leaves the key-absent shape to the #4033 placeholder — not to the bucket', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const { data, xAxisKey, series } = buildChartSeries([{ count: 2 }, { count: 8 }], ['issued'], ['count']); + const { container } = render( + , + ); + expect(container.querySelector('[data-chart-error="missing-category-key"]')).not.toBeNull(); + expect(container.querySelector('svg')).toBeNull(); + warn.mockRestore(); + }); + + it('keeps a genuinely empty result set empty — no phantom bucket bar', () => { + const { container } = renderDataset([]); + expect(barCount(container)).toBe(0); + expect(chartTexts(container)).not.toContain(NULL_CATEGORY_LABEL); + }); +}); diff --git a/packages/plugin-charts/src/AdvancedChartImpl.tsx b/packages/plugin-charts/src/AdvancedChartImpl.tsx index eb7cd1286..4aeb25d0c 100644 --- a/packages/plugin-charts/src/AdvancedChartImpl.tsx +++ b/packages/plugin-charts/src/AdvancedChartImpl.tsx @@ -1115,6 +1115,24 @@ function AdvancedChartImplInner({ /** * Detect the framework#4033 shape from props alone: rows are present, the chart * plots a category axis, and NOT ONE row carries the key it was told to plot. + * + * **Key ABSENT is what this asks, and only that** — `key in row` — which is the + * line dividing it from objectui#4466's null bucket. The two failures look + * identical on screen (an axis frame with no marks) and have different causes + * and different answers: + * + * - **key absent** — the dataset query grouped by a dimension it never + * PROJECTED, so no value for it exists anywhere. Nothing can be plotted and + * nothing can be labelled; this guard says so, naming the missing key. + * - **key present, value null** — a real group whose key happens to be NULL + * (`{user_id: null, event_count: 50}`, the shipped first-boot state of + * System Overview's "Events by User"). The data IS there, so it is drawn: + * `buildChartSeries` maps it to an explicit bucket label upstream of this + * component, and that guard never fires because `'user_id' in row` is true. + * + * That upstream mapping deliberately does not ADD the key to a row that lacks + * it (see `bucketNullCategories` in `@object-ui/core`), which is what keeps this + * predicate meaning what it says. */ function hasNoCategoryKey(props: AdvancedChartImplProps): boolean { const chartType = props.chartType === 'column' ? 'bar' : (props.chartType ?? 'bar'); diff --git a/packages/plugin-charts/src/ObjectChart.nullCategoryBucket.test.tsx b/packages/plugin-charts/src/ObjectChart.nullCategoryBucket.test.tsx new file mode 100644 index 000000000..94519667b --- /dev/null +++ b/packages/plugin-charts/src/ObjectChart.nullCategoryBucket.test.tsx @@ -0,0 +1,113 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * objectui#4466 — the null bucket reaches a REAL dataset-bound `ObjectChart`, + * with its label taken from the i18n channel. + * + * `AdvancedChartImpl.nullCategoryBucket.test.tsx` pins the transform → renderer + * seam; this pins the wiring on the consumer side, which is the half a shared + * fix can silently miss: `@object-ui/core` is React-free and cannot read the + * locale bundle, so it applies its English floor unless the renderer passes the + * resolved label down. Nothing else fails when that argument goes missing — the + * chart still draws a bar, just never a translated one. + */ +import React from 'react'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { render, cleanup, waitFor } from '@testing-library/react'; +import { NULL_CATEGORY_LABEL } from '@object-ui/core'; + +vi.mock('recharts', async () => { + const actual = await vi.importActual('recharts'); + return { + ...actual, + ResponsiveContainer: ({ children }: any) => + React.cloneElement(children, { width: 480, height: 320 }), + }; +}); + +import { ObjectChart } from './ObjectChart'; + +/** + * A dataset-bound chart probes `GET /api/v1/meta/dataset/` off the GLOBAL + * fetch before it can resolve option colours. `{}` answers it without a live + * request — see the same double, and the reasoning behind it, in + * `ObjectChart.datasetMeasureLabel.test.tsx` (objectui#4106). + */ +beforeEach(() => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => ({ ok: true, json: async () => ({}) })), + ); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + cleanup(); +}); + +/** The card's own first-boot shape: audit events written with `user_id = NULL`. */ +const makeDS = (rows: Array>) => ({ + queryDataset: vi.fn().mockResolvedValue({ + rows, + fields: [ + { name: 'user_id', label: 'User' }, + { name: 'event_count', label: 'Events' }, + ], + }), +}); + +const renderChart = (ds: ReturnType) => + render( + , + ); + +describe('ObjectChart — dataset-bound null-keyed group (objectui#4466)', () => { + it('draws the null group as a labelled bucket instead of an empty axis', async () => { + const ds = makeDS([{ user_id: null, event_count: 50 }]); + const { container } = renderChart(ds); + + await waitFor(() => expect(ds.queryDataset).toHaveBeenCalled()); + // Pre-fix: `.recharts-bar` = 1 with `.recharts-bar-rectangle` = 0 — an axis + // frame, gridlines and no mark, for a server response of 50 real events. + await waitFor(() => + expect(container.querySelectorAll('.recharts-bar-rectangle').length).toBe(1), + ); + expect(container.textContent).toContain(NULL_CATEGORY_LABEL); + }); + + it('keeps the dominant null group when a named group is present too', async () => { + const ds = makeDS([ + { user_id: null, event_count: 51 }, + { user_id: 'Dev Admin', event_count: 2 }, + ]); + const { container } = renderChart(ds); + + await waitFor(() => expect(ds.queryDataset).toHaveBeenCalled()); + // Pre-fix this was ONE bar: 51 of 53 events silently dropped. + await waitFor(() => + expect(container.querySelectorAll('.recharts-bar-rectangle').length).toBe(2), + ); + expect(container.textContent).toContain(NULL_CATEGORY_LABEL); + expect(container.textContent).toContain('Dev Admin'); + }); + + it('renders no bucket bar for a genuinely empty result set', async () => { + const ds = makeDS([]); + const { container } = renderChart(ds); + + await waitFor(() => expect(ds.queryDataset).toHaveBeenCalled()); + expect(container.querySelectorAll('.recharts-bar-rectangle').length).toBe(0); + expect(container.textContent).not.toContain(NULL_CATEGORY_LABEL); + }); +}); diff --git a/packages/plugin-charts/src/ObjectChart.tsx b/packages/plugin-charts/src/ObjectChart.tsx index 8616b4f9f..ddd849fd0 100644 --- a/packages/plugin-charts/src/ObjectChart.tsx +++ b/packages/plugin-charts/src/ObjectChart.tsx @@ -827,8 +827,20 @@ export const ObjectChart = (props: any) => { // ADR-0021 (#1759): when the chart binds to a dataset, derive data/xAxisKey/ // series from its dimensions/measures via the shared buildChartSeries helper — // this pivots a second dimension into grouped series, matching DatasetWidget. + // + // `nullCategoryLabel` is this layer's half of objectui#4466: core maps a null + // category value to a bucket so the group renders at all, and the LABEL comes + // from here because `@object-ui/core` is React-free and cannot read the locale + // bundle (same division as `dimensionOptionTranslator` above — core takes the + // resolver, the renderer holds the provider). const datasetChart = schema.dataset - ? buildChartSeries(relabelDimensions(finalData, dimensionLabels), schema.dimensions, schema.values, datasetFields) + ? buildChartSeries( + relabelDimensions(finalData, dimensionLabels), + schema.dimensions, + schema.values, + datasetFields, + { nullCategoryLabel: tt('chart.nullCategory', '(None)') }, + ) : null; const finalSchema = datasetChart