diff --git a/.changeset/dataset-widget-null-category-label-4500.md b/.changeset/dataset-widget-null-category-label-4500.md new file mode 100644 index 000000000..2b22ed65c --- /dev/null +++ b/.changeset/dataset-widget-null-category-label-4500.md @@ -0,0 +1,11 @@ +--- +'@object-ui/plugin-dashboard': patch +--- + +A dashboard chart's null-value bucket now reads the app's language instead of the English `(None)` + +`buildChartSeries` groups rows whose category value is `null` under a labelled bucket, so the group draws as a bar instead of vanishing off the axis (objectui#4466). The label comes from the caller: `@object-ui/core` is React-free, cannot read the locale bundle, and falls back to the English constant `(None)`. `ObjectChart` passes its resolved label and localizes; `DatasetWidget` called the same helper with no options, so a dashboard widget in a zh app labelled the bucket `(None)` while the standalone chart one panel over labelled it `(未指定)`. It now passes `chart.nullCategory` from the i18n channel, which every locale pack already carries. + +The same label goes to `findChartSeriesRow`, and that half is what keeps the bar clickable. That helper is the inverse map behind segment-click drill-through: it compares the clicked category against its own copy of the bucket label, defaulting to the same English floor. Passing the localized label to only the forward call would draw a bar reading `(未指定)` while the drill matched `(None)` — the click resolves to no row and the drawer never opens, which is a worse outcome than the untranslated word this fixes. Both calls now read one binding, so they cannot drift apart. + +Nothing else moves: non-null categories chart and drill exactly as before, an `en` app still reads `(None)` (now via its locale pack rather than the hardcoded floor), and a widget over data with no null group is untouched. diff --git a/packages/plugin-dashboard/src/DatasetWidget.tsx b/packages/plugin-dashboard/src/DatasetWidget.tsx index db06506b3..aadeb8e31 100644 --- a/packages/plugin-dashboard/src/DatasetWidget.tsx +++ b/packages/plugin-dashboard/src/DatasetWidget.tsx @@ -1344,9 +1344,18 @@ export function DatasetWidget({ widget, dataSource }: { widget: any; dataSource: // axis with its count intact (cloud#667). `chartRows` stays index-aligned with // `state.rows`/`drillRawRows`, so drill-through still maps to the raw value. const chartRows = relabelDimensions(state.rows, dimensionLabels); + // objectui#4500 — the null-category bucket's LABEL. `@object-ui/core` is + // React-free and cannot read the locale bundle, so it falls back to the + // English `NULL_CATEGORY_LABEL`; the resolved label has to come from here + // (the same division ObjectChart uses — core takes the string, the renderer + // holds the provider). ONE binding on purpose: `buildChartSeries` writes it + // into the rendered rows and `findChartSeriesRow` matches a clicked category + // against it, so the two drifting apart costs the bucket bar its drill — + // a visible bar that silently no-ops (#4498's dead-click rule). + const nullCategoryLabel = tt('chart.nullCategory', '(None)'); // ADR-0021 (#1759): shared helper — pivots a second dimension into grouped // series so multi-dimension dataset widgets match the chart-view renderer. - const { data: chartData, xAxisKey, series } = buildChartSeries(chartRows, dimensions, values, state.fields); + const { data: chartData, xAxisKey, series } = buildChartSeries(chartRows, dimensions, values, state.fields, { nullCategoryLabel }); // The author's PRESENTATION, merged onto those derived bindings — per-series // mark and axis binding, plus the axis definitions (#4229). Membership stays @@ -1409,7 +1418,7 @@ export function DatasetWidget({ widget, dataSource }: { widget: any; dataSource: // Map a clicked chart segment back to its dataset row, then drill through to // the underlying records — same governed path the table/pivot rows use. const handleChartDrill = (ev: { category?: string; series?: string; value?: number }) => { - const idx = findChartSeriesRow(chartRows, dimensions, values, ev?.category, ev?.series); + const idx = findChartSeriesRow(chartRows, dimensions, values, ev?.category, ev?.series, { nullCategoryLabel }); if (idx < 0) return; // `series` is a real (second) dimension value only when pivoted; otherwise // it's the measure name — omit it from the drawer title. diff --git a/packages/plugin-dashboard/src/__tests__/DatasetWidget.nullCategoryI18n.test.tsx b/packages/plugin-dashboard/src/__tests__/DatasetWidget.nullCategoryI18n.test.tsx new file mode 100644 index 000000000..9b17e173f --- /dev/null +++ b/packages/plugin-dashboard/src/__tests__/DatasetWidget.nullCategoryI18n.test.tsx @@ -0,0 +1,244 @@ +/** + * 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#4500 — DatasetWidget's half of #4466's null bucket. + * + * `buildChartSeries` (core) buckets a null-keyed category under + * `options.nullCategoryLabel`, falling back to the English constant + * `NULL_CATEGORY_LABEL = '(None)'`. `@object-ui/core` is React-free and cannot + * read the locale bundle, so the LABEL has to come from the renderer — which is + * exactly what makes this failure quiet: the bar draws, the count is right, and + * only the word is wrong. ObjectChart (plugin-charts) passes its resolved label + * and localizes; DatasetWidget called the same helper with no options at all, + * so a dashboard widget rendered the raw English `(None)` inside a zh app. + * + * TWO call sites, not one, and the second is the sharp edge. `findChartSeriesRow` + * is the inverse map used by the segment-click drill: it compares the CLICKED + * category string against its own `nullCategoryLabel`, defaulting to the same + * English floor. Localize only the forward call and the bucket bar is drawn as + * `(未指定)` while the drill still matches against `(None)` — the click resolves + * to `-1` and `handleChartDrill` returns without opening the drawer. That is a + * strictly WORSE outcome than the untranslated label this card is about: a + * visible bar that silently does nothing (#4498's dead-click rule). So the two + * calls are pinned as a PAIR here, not as two independent facts. + * + * DIRECTIONS, written before the reverse verification was run: + * - the zh label case and the zh drill case are RED before the change (the + * label reads `(None)`; the drill on the localized label no-ops); + * - the non-null row, the `en` case and the no-null-group case are GREEN on + * both sides. They are the acceptance boundary: this card buys a translated + * bucket label and must not move a single other byte the widget charts. + * + * Scope note: single-dimension only. The multi-dimension PIVOT arm of + * `buildChartSeries` is #4497's in-flight surface and is deliberately not + * touched here — `findChartSeriesRow`'s pivot branch matches the x-axis + * dimension through the same `xOf`, so the pair property this file pins holds + * there too, but asserting it would collide with that card. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, beforeAll, afterEach } from 'vitest'; +import { render, cleanup, waitFor } from '@testing-library/react'; +import { ComponentRegistry, NULL_CATEGORY_LABEL } from '@object-ui/core'; +import { I18nProvider } from '@object-ui/i18n'; +import { DatasetWidget } from '../DatasetWidget'; + +/** Capture what the widget hands the chart renderer (jsdom lays out no SVG). */ +let capturedChartProps: any = null; +beforeAll(() => { + ComponentRegistry.register('chart', (props: any) => { + capturedChartProps = props; + return null; + }); +}); + +/** Observe the drill filter without rendering the real drawer. */ +const drillFilters: Array> = []; +vi.mock('../DrillDownDrawer', () => ({ + DrillDownDrawer: ({ filter }: { filter: Record }) => { + drillFilters.push(filter); + return null; + }, +})); + +afterEach(() => { + cleanup(); + capturedChartProps = null; + drillFilters.length = 0; + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +/** + * The dataset's base object. `owner_id` is a plain TEXT field on purpose: this + * card is about the null BUCKET, so the fixture must carry no option list that + * could relabel a category through `relabelDimensions` and blur which channel + * produced the string under test. + */ +const OPPORTUNITY = { name: 'crm_opportunity', fields: { owner_id: { type: 'text' } } }; + +function installMetaRouter() { + vi.stubGlobal( + 'fetch', + vi.fn(async () => ({ ok: true, json: async () => ({ item: OPPORTUNITY }) })), + ); +} + +/** + * A first-boot shape: three deals are owned, three are not (`owner = NULL`). + * `drillRawRows` carries the IDENTITY keys — deliberately spelled differently + * from the display values so an index-aligned drill can be told apart from a + * lucky string match. + */ +const nullKeyedDataset = () => ({ + queryDataset: vi.fn(async () => ({ + rows: [ + { owner_name: 'Ada Lovelace', deals: 5 }, + { owner_name: null, deals: 3 }, + ], + fields: [ + { name: 'owner_name', type: 'text', label: 'Owner' }, + { name: 'deals', type: 'number', label: 'Deals' }, + ], + object: 'crm_opportunity', + dimensionFields: { owner_name: 'owner_id' }, + drillRawRows: [{ owner_name: 'user-ada' }, { owner_name: null }], + })), +}); + +/** The same widget over data with NO null group — the untouched-surface case. */ +const fullyKeyedDataset = () => ({ + queryDataset: vi.fn(async () => ({ + rows: [ + { owner_name: 'Ada Lovelace', deals: 5 }, + { owner_name: 'Grace Hopper', deals: 3 }, + ], + fields: [ + { name: 'owner_name', type: 'text', label: 'Owner' }, + { name: 'deals', type: 'number', label: 'Deals' }, + ], + object: 'crm_opportunity', + dimensionFields: { owner_name: 'owner_id' }, + drillRawRows: [{ owner_name: 'user-ada' }, { owner_name: 'user-grace' }], + })), +}); + +function renderIn(language: string, dataSource: { queryDataset: unknown }) { + return render( + + + , + ); +} + +/** The categories the widget actually handed the chart renderer. */ +const categories = () => + (capturedChartProps?.schema?.data ?? []).map((r: any) => r.owner_name); + +describe('DatasetWidget — the null-category bucket reads the locale bundle (objectui#4500)', () => { + it('labels the null group `(未指定)` under `zh`, not the English floor', async () => { + installMetaRouter(); + renderIn('zh', nullKeyedDataset()); + + await waitFor(() => expect(categories().length).toBe(2)); + expect(categories()).toContain('(未指定)'); + // The English floor must not survive beside the translation — this is the + // whole card, and `(None)` is what a zh dashboard renders today. + expect(categories()).not.toContain(NULL_CATEGORY_LABEL); + }); + + it('keeps the measure attached to the (now translated) bucket', async () => { + installMetaRouter(); + renderIn('zh', nullKeyedDataset()); + + await waitFor(() => expect(categories().length).toBe(2)); + const byCategory = Object.fromEntries( + capturedChartProps.schema.data.map((r: any) => [r.owner_name, r.deals]), + ); + expect(byCategory).toMatchObject({ '(未指定)': 3 }); + }); + + it('PAIR — a click on the localized bucket bar still drills to ITS row', async () => { + // The dead-click rule (#4498), now on the localized label. Pre-fix this is + // red in the worse direction: with only the forward call localized the bar + // reads `(未指定)`, `findChartSeriesRow` matches against `(None)`, returns + // -1, and `handleChartDrill` returns before `openDrill` — a visible bar + // that does nothing at all. + installMetaRouter(); + renderIn('zh', nullKeyedDataset()); + + await waitFor(() => expect(categories()).toContain('(未指定)')); + // Click with WHATEVER the bucket bar reads — the chart knows no other string. + const bucket = categories().find((c: string) => c !== 'Ada Lovelace'); + capturedChartProps.onSegmentClick({ category: bucket }); + + await waitFor(() => expect(drillFilters.length).toBeGreaterThan(0)); + // Index 1 of `drillRawRows`: an explicit "is empty" filter on the OBJECT + // field, never the display label and never Ada's row. + expect(drillFilters[drillFilters.length - 1]).toEqual({ owner_id: null }); + }); + + it('BOUNDARY — the non-null group is byte-identical', async () => { + // Green on both sides. A category that was never null must not acquire a + // bucket, a relabel, or a changed measure. + installMetaRouter(); + renderIn('zh', nullKeyedDataset()); + + await waitFor(() => expect(categories().length).toBe(2)); + expect(capturedChartProps.schema.data[0]).toEqual({ owner_name: 'Ada Lovelace', deals: 5 }); + }); + + it('BOUNDARY — a click on the non-null bar drills by the STORED id', async () => { + // Green on both sides: identity keys do not translate (#4263's convention), + // and the fix must not perturb the drill of a category it does not bucket. + installMetaRouter(); + renderIn('zh', nullKeyedDataset()); + + await waitFor(() => expect(categories()).toContain('Ada Lovelace')); + capturedChartProps.onSegmentClick({ category: 'Ada Lovelace' }); + + await waitFor(() => expect(drillFilters.length).toBeGreaterThan(0)); + expect(drillFilters[drillFilters.length - 1]).toEqual({ owner_id: 'user-ada' }); + }); + + it('BOUNDARY — under `en` the bucket reads `(None)`, through the same channel', async () => { + // Green on both sides, but NOT for the same reason on each: before the fix + // core's hardcoded floor produces it, after the fix the `en` pack's + // `chart.nullCategory` does. Same bytes, and that is the requirement — an + // `en` dashboard must read exactly what it reads today. + installMetaRouter(); + renderIn('en', nullKeyedDataset()); + + await waitFor(() => expect(categories().length).toBe(2)); + expect(categories()).toContain(NULL_CATEGORY_LABEL); + expect(categories()).not.toContain('(未指定)'); + }); + + it('BOUNDARY — a widget with no null group is untouched in either locale', async () => { + installMetaRouter(); + renderIn('zh', fullyKeyedDataset()); + + await waitFor(() => expect(categories().length).toBe(2)); + expect(capturedChartProps.schema.data).toEqual([ + { owner_name: 'Ada Lovelace', deals: 5 }, + { owner_name: 'Grace Hopper', deals: 3 }, + ]); + expect(categories()).not.toContain(NULL_CATEGORY_LABEL); + expect(categories()).not.toContain('(未指定)'); + }); +});