diff --git a/frontend/src/components/data-table/DataTable.jsx b/frontend/src/components/data-table/DataTable.jsx index ddf6b17fad..da80224549 100644 --- a/frontend/src/components/data-table/DataTable.jsx +++ b/frontend/src/components/data-table/DataTable.jsx @@ -39,6 +39,24 @@ import { cn } from "@/lib/utils"; * import it rather than build their own. */ +/** + * One cell's value, read the way antd reads it. + * + * `dataIndex` is either a key or a PATH: `["product", "name"]` is antd's + * documented nested form and means `record.product.name`. The flat lookup this + * replaces indexed the record with the array itself, and JavaScript stringifies + * that to the property name `"product,name"` — so the value was always + * undefined, and silently so, because a column with no `render` hands it + * straight to the cell. LLMWhisperer's API Keys table declares its Plan column + * exactly that way and lost the whole column to a blank strip. + */ +function cellValue(record, dataIndex) { + if (Array.isArray(dataIndex)) { + return dataIndex.reduce((v, k) => (v == null ? undefined : v[k]), record); + } + return record?.[dataIndex]; +} + /** * One antd column → one TanStack column def. * @@ -53,6 +71,14 @@ import { cn } from "@/lib/utils"; * version used would collide across levels. */ function toColumn(c, path) { + /* + * Three places spell this identity, and they must agree: here, `columnKey` + * (ColumnFilter.jsx) and `toSorterInfo` below, which matches on it to answer + * antd's `onChange`. A PATH `dataIndex` with no `key` coerces to + * `"product,name"` in all three, so they still agree — which is why this is + * deliberately NOT normalised to `dataIndex.join(".")`. Tidying it here alone + * would silently stop sorting and filtering reporting for a nested column. + */ const id = String(c.key ?? c.dataIndex ?? path); // `column` rides along so the header can render antd's filter affordance, // which is declared on the antd def and has no TanStack equivalent. @@ -76,7 +102,31 @@ function toColumn(c, path) { return { id, - accessorKey: c.dataIndex, + /* + * A path needs an accessor FUNCTION: TanStack's `accessorKey` is a single + * key, so handing it the array repeats the same stringified-`"product,name"` + * lookup one layer down. + * + * This is NOT inert. TanStack defaults every column to `sortUndefined: 1`, + * and `getSortedRowModel` reads `row.getValue()` to apply it BEFORE it ever + * consults `sortingFn` — so undefined-valued rows sort to the end even + * though `sortingFn` is `() => 0`. A nested column was accidentally exempt + * while every one of its values was undefined; with a real accessor it now + * behaves exactly as an equivalent string column already does (verified: + * both reorder identically on a sparse column). That quirk is pre-existing + * and cross-cutting, not introduced here, and no nested column declares a + * `sorter` today. + * + * The string case keeps `accessorKey` because it is unchanged, not because + * its behaviour is right: TanStack deep-reads a DOTTED string while the + * cell below reads it as a literal key, which is antd's own reading. Those + * two disagree for `dataIndex: "a.b"`. Pre-existing on both halves, no + * literal dotted `dataIndex` exists in either repo, and reconciling it is a + * behaviour change beyond this fix. + */ + ...(Array.isArray(c.dataIndex) + ? { accessorFn: (record) => cellValue(record, c.dataIndex) } + : { accessorKey: c.dataIndex }), header: c.title, enableSorting: Boolean(c.sorter), /* @@ -95,7 +145,9 @@ function toColumn(c, path) { : () => 0, meta, cell: ({ row }) => { - const value = c.dataIndex ? row.original?.[c.dataIndex] : undefined; + const value = c.dataIndex + ? cellValue(row.original, c.dataIndex) + : undefined; // antd's render(value, record, index) contract. return c.render ? c.render(value, row.original, row.index) : value; }, diff --git a/frontend/src/components/data-table/DataTable.test.jsx b/frontend/src/components/data-table/DataTable.test.jsx index d305594ffc..f2faccdbbd 100644 --- a/frontend/src/components/data-table/DataTable.test.jsx +++ b/frontend/src/components/data-table/DataTable.test.jsx @@ -1521,3 +1521,128 @@ describe("DataTable controlled filters", () => { }); }); }); + +/** + * antd's `dataIndex` is either a key or a PATH — `["product", "name"]` reads + * `record.product.name`. The flat lookup this guards indexed the record with + * the array itself, which JavaScript stringifies to the property name + * `"product,name"`, so the value was always undefined and — because a column + * with no `render` hands it straight to the cell — silently blank. + * + * LLMWhisperer's API Keys table declares its Plan column exactly this way and + * lost the whole column to an empty strip after the migration. + */ +describe("DataTable nested dataIndex", () => { + const nested = [ + { id: 1, product: { id: "free", name: "LLM Whisperer Free" } }, + ]; + + it("resolves an array dataIndex as a path into the record", () => { + render( + , + ); + expect(screen.getByText("LLM Whisperer Free")).toBeInTheDocument(); + }); + + it("renders an empty cell rather than throwing on a missing segment", () => { + expect(() => + render( + , + ), + ).not.toThrow(); + // Two rows, both with an EMPTY Plan cell — not the empty state, and not a + // stand-in like "undefined" or "N/A" that a laxer resolver would print. + const cells = document.querySelectorAll("tbody td"); + expect(cells).toHaveLength(2); + for (const cell of cells) { + expect(cell).toHaveTextContent(""); + } + }); + + it("walks a path of any depth, including an array index", () => { + render( + , + ); + expect(screen.getByText("a@example.com")).toBeInTheDocument(); + }); + + /* + * The identity a nested column reports back through antd's `onChange`. + * + * With no `key`, the column id and `toSorterInfo`'s lookup both coerce the + * array to `"product,name"` — agreeing only because NEITHER normalises it. + * Normalising the id alone (to `"product.name"`, say) is exactly the tidy-up + * a later reader would attempt, and it would silently stop a nested column + * reporting its sort, with every other test here still green. + */ + it("reports a keyless nested column through onChange when sorted", async () => { + const onChange = vi.fn(); + render( + , + ); + await userEvent.click(screen.getByText("Plan")); + expect(onChange).toHaveBeenCalled(); + const sorter = onChange.mock.calls.at(-1)[2]; + expect(sorter.field).toEqual(["product", "name"]); + expect(sorter.order).toBe("ascend"); + }); + + it("hands the resolved nested value to render, as antd does", () => { + const renderCell = vi.fn((value) => `plan: ${value}`); + render( + , + ); + expect(renderCell).toHaveBeenCalledWith("LLM Whisperer Free", nested[0], 0); + expect(screen.getByText("plan: LLM Whisperer Free")).toBeInTheDocument(); + }); +});