Skip to content

Commit ef105c5

Browse files
nickofthymedej611
andauthored
[Lens][Table] Fix csv export column sort order (#236673)
## Summary When a table from a dashboard using the **Download CSV** action, the column sort order visible in the table on the dashboard are preserved in the csv output. https://github.com/user-attachments/assets/7454a511-150b-45a2-86c5-1cd61bc0191a Fixes #236550 ## Details The `datatable_fn` is used to set the table of data to the `adapters`, which is the data used in the csv export action. This data comes in out of order with additional `Part of X` columns for formula columns. However, we do not sort these in before passing to the adapters. Normally, with formulas, this is not a problem as the columns are in the correct order but the [`tabify`](https://github.com/elastic/kibana/blob/8d4b0956586284c35db97de2baea49b58476ee2a/src/platform/plugins/shared/data/common/search/tabify/tabify.ts#L24) logic mixes up the column order for formulas. The fix is to simply sort the columns in the table before passing it to the `adapters.` ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels. ## Release Note Fixes a bug in the Lens table in which exporting the table from a dashboard, which containg formula columns, can result in a different column order than shown on the dashboard. Co-authored-by: Marco Liberati <[email protected]>
1 parent 5603673 commit ef105c5

File tree

2 files changed

+136
-7
lines changed

2 files changed

+136
-7
lines changed
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
import { fieldFormatsServiceMock } from '@kbn/field-formats-plugin/public/mocks';
9+
import type {
10+
Datatable,
11+
DefaultInspectorAdapters,
12+
ExecutionContext,
13+
} from '@kbn/expressions-plugin/common';
14+
import type { DatatableArgs } from '../..';
15+
import { datatableFn } from './datatable_fn';
16+
import { shuffle } from 'lodash';
17+
18+
const context = {
19+
variables: { embeddableTitle: 'title' },
20+
} as unknown as ExecutionContext<DefaultInspectorAdapters>;
21+
22+
const mockFormatFactory = fieldFormatsServiceMock.createStartContract().deserialize;
23+
24+
describe('datatableFn', () => {
25+
function buildTable(): Datatable {
26+
return {
27+
type: 'datatable',
28+
columns: [
29+
{ id: 'bucket1', name: 'bucket1', meta: { type: 'string' } },
30+
{ id: 'bucket2', name: 'bucket2', meta: { type: 'string' } },
31+
{ id: 'bucket3', name: 'bucket3', meta: { type: 'string' } },
32+
{ id: 'metric1', name: 'metric1', meta: { type: 'number' } },
33+
{ id: 'metric2', name: 'metric2', meta: { type: 'number' } },
34+
],
35+
rows: [
36+
{ bucket1: 'A', bucket2: 'D', bucket3: 'X', metric1: 1, metric2: 2 },
37+
{ bucket1: 'A', bucket2: 'D', bucket3: 'Y', metric1: 3, metric2: 4 },
38+
{ bucket1: 'A', bucket2: 'D', bucket3: 'Z', metric1: 5, metric2: 6 },
39+
{ bucket1: 'A', bucket2: 'E', bucket3: 'X', metric1: 7, metric2: 8 },
40+
{ bucket1: 'A', bucket2: 'E', bucket3: 'Y', metric1: 9, metric2: 10 },
41+
{ bucket1: 'A', bucket2: 'E', bucket3: 'Z', metric1: 11, metric2: 12 },
42+
{ bucket1: 'A', bucket2: 'F', bucket3: 'X', metric1: 13, metric2: 14 },
43+
{ bucket1: 'A', bucket2: 'F', bucket3: 'Y', metric1: 15, metric2: 16 },
44+
{ bucket1: 'A', bucket2: 'F', bucket3: 'Z', metric1: 17, metric2: 18 },
45+
{ bucket1: 'B', bucket2: 'D', bucket3: 'X', metric1: 19, metric2: 20 },
46+
{ bucket1: 'B', bucket2: 'D', bucket3: 'Y', metric1: 21, metric2: 22 },
47+
{ bucket1: 'B', bucket2: 'D', bucket3: 'Z', metric1: 23, metric2: 24 },
48+
{ bucket1: 'B', bucket2: 'E', bucket3: 'X', metric1: 25, metric2: 26 },
49+
{ bucket1: 'B', bucket2: 'E', bucket3: 'Y', metric1: 27, metric2: 28 },
50+
{ bucket1: 'B', bucket2: 'E', bucket3: 'Z', metric1: 29, metric2: 30 },
51+
{ bucket1: 'B', bucket2: 'F', bucket3: 'X', metric1: 31, metric2: 32 },
52+
{ bucket1: 'B', bucket2: 'F', bucket3: 'Y', metric1: 33, metric2: 34 },
53+
{ bucket1: 'B', bucket2: 'F', bucket3: 'Z', metric1: 35, metric2: 36 },
54+
{ bucket1: 'C', bucket2: 'D', bucket3: 'X', metric1: 37, metric2: 38 },
55+
{ bucket1: 'C', bucket2: 'D', bucket3: 'Y', metric1: 39, metric2: 40 },
56+
{ bucket1: 'C', bucket2: 'D', bucket3: 'Z', metric1: 41, metric2: 42 },
57+
{ bucket1: 'C', bucket2: 'E', bucket3: 'X', metric1: 43, metric2: 44 },
58+
{ bucket1: 'C', bucket2: 'E', bucket3: 'Y', metric1: 45, metric2: 46 },
59+
{ bucket1: 'C', bucket2: 'E', bucket3: 'Z', metric1: 47, metric2: 48 },
60+
{ bucket1: 'C', bucket2: 'F', bucket3: 'X', metric1: 49, metric2: 50 },
61+
{ bucket1: 'C', bucket2: 'F', bucket3: 'Y', metric1: 51, metric2: 52 },
62+
{ bucket1: 'C', bucket2: 'F', bucket3: 'Z', metric1: 53, metric2: 54 },
63+
],
64+
};
65+
}
66+
67+
function buildArgs(): DatatableArgs {
68+
return {
69+
title: 'Table',
70+
sortingColumnId: undefined,
71+
sortingDirection: 'none',
72+
columns: [
73+
{
74+
type: 'lens_datatable_column',
75+
columnId: 'bucket1',
76+
isTransposed: false,
77+
transposable: false,
78+
},
79+
{
80+
type: 'lens_datatable_column',
81+
columnId: 'bucket2',
82+
isTransposed: false,
83+
transposable: false,
84+
},
85+
{
86+
type: 'lens_datatable_column',
87+
columnId: 'bucket3',
88+
isTransposed: false,
89+
transposable: false,
90+
},
91+
{
92+
type: 'lens_datatable_column',
93+
columnId: 'metric1',
94+
isTransposed: false,
95+
transposable: true,
96+
},
97+
{
98+
type: 'lens_datatable_column',
99+
columnId: 'metric2',
100+
isTransposed: false,
101+
transposable: true,
102+
},
103+
],
104+
};
105+
}
106+
107+
it('should correctly sort columns in table by order of args.columns', async () => {
108+
const table = buildTable();
109+
const shuffledTable: Datatable = {
110+
...table,
111+
columns: shuffle(table.columns),
112+
};
113+
const args = buildArgs();
114+
const result = await datatableFn(() => mockFormatFactory)(shuffledTable, args, context);
115+
116+
const resultColumnIds = result.value.data.columns.map((c) => c.id);
117+
const expectedColumnIds = args.columns.map((c) => c.columnId);
118+
119+
expect(resultColumnIds).toEqual(expectedColumnIds);
120+
expect(resultColumnIds).toEqual(['bucket1', 'bucket2', 'bucket3', 'metric1', 'metric2']);
121+
});
122+
});

x-pack/platform/plugins/shared/lens/common/expressions/impl/datatable/datatable_fn.ts

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,19 @@ export const datatableFn =
2020
getFormatFactory: (context: ExecutionContext) => FormatFactory | Promise<FormatFactory>
2121
): DatatableExpressionFunction['fn'] =>
2222
async (table, args, context) => {
23+
const columnSortMap = args.columns.reduce((acc, c, i) => acc.set(c.columnId, i), new Map());
24+
const getColumnSort = (id: string) => columnSortMap.get(id) ?? -1;
25+
const sortedTable: Datatable = {
26+
...table,
27+
columns: table.columns.slice().sort((a, b) => getColumnSort(a.id) - getColumnSort(b.id)),
28+
};
29+
2330
if (context?.inspectorAdapters?.tables) {
2431
context.inspectorAdapters.tables.reset();
2532
context.inspectorAdapters.tables.allowCsvExport = true;
2633

2734
const logTable = prepareLogTable(
28-
table,
35+
sortedTable,
2936
[
3037
[
3138
args.columns.map((column) => column.columnId),
@@ -45,20 +52,20 @@ export const datatableFn =
4552
const formatters: Record<string, ReturnType<FormatFactory>> = {};
4653
const formatFactory = await getFormatFactory(context);
4754

48-
table.columns.forEach((column) => {
55+
sortedTable.columns.forEach((column) => {
4956
formatters[column.id] = formatFactory(column.meta?.params);
5057
});
5158

5259
const hasTransposedColumns = args.columns.some((c) => c.isTransposed);
5360
if (hasTransposedColumns) {
5461
// store original shape of data separately
55-
untransposedData = cloneDeep(table);
62+
untransposedData = cloneDeep(sortedTable);
5663
// transposes table and args in-place
57-
transposeTable(args, table, formatters);
64+
transposeTable(args, sortedTable, formatters);
5865

5966
if (context?.inspectorAdapters?.tables) {
6067
const logTransposedTable = prepareLogTable(
61-
table,
68+
sortedTable,
6269
[
6370
[
6471
args.columns.map((column) => column.columnId),
@@ -82,7 +89,7 @@ export const datatableFn =
8289
for (const column of columnsWithSummary) {
8390
column.summaryRowValue = computeSummaryRowForColumn(
8491
column,
85-
table,
92+
sortedTable,
8693
formatters,
8794
formatFactory({ id: 'number' })
8895
);
@@ -92,7 +99,7 @@ export const datatableFn =
9299
type: 'render',
93100
as: 'lens_datatable_renderer',
94101
value: {
95-
data: table,
102+
data: sortedTable,
96103
untransposedData,
97104
syncColors: context.isSyncColorsEnabled?.() ?? false,
98105
args: {

0 commit comments

Comments
 (0)