Skip to content

Commit 9f4d647

Browse files
wylieconlonwylieconlon
committed
Simplify logic and add test for output ID
1 parent d222796 commit 9f4d647

6 files changed

Lines changed: 40 additions & 116 deletions

File tree

‎src/plugins/expressions/common/expression_functions/specs/map_column.ts‎

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ export const mapColumn: ExpressionFunctionDefinition<
4444
types: ['string', 'null'],
4545
help: i18n.translate('expressions.functions.mapColumn.args.idHelpText', {
4646
defaultMessage:
47-
'An optional id of the resulting column. When `null` the name/column argument is used as id.',
47+
'An optional id of the resulting column. When no id is provided, the id will be looked up from the existing column and default to the name.',
4848
}),
4949
required: false,
5050
default: null,
@@ -86,9 +86,18 @@ export const mapColumn: ExpressionFunctionDefinition<
8686
.expression?.(...params)
8787
.pipe(take(1))
8888
.toPromise() ?? Promise.resolve(null);
89-
const columnId = args.id != null ? args.id : args.name;
9089

9190
const columns = [...input.columns];
91+
const existingColumnIndex = columns.findIndex(({ id, name }) => {
92+
if (args.id) {
93+
return id === args.id;
94+
}
95+
return name === args.name;
96+
});
97+
const columnLength = columns.length;
98+
const columnId =
99+
existingColumnIndex === -1 ? args.id ?? args.name : columns[existingColumnIndex].id;
100+
92101
const rowPromises = input.rows.map((row) => {
93102
return expression({
94103
type: 'datatable',
@@ -101,21 +110,6 @@ export const mapColumn: ExpressionFunctionDefinition<
101110
});
102111

103112
return Promise.all(rowPromises).then((rows) => {
104-
const existingColumnIndex = columns.findIndex(({ id, name }) => {
105-
if (args.id) {
106-
if (!id) {
107-
return name === args.id;
108-
}
109-
// Columns that have IDs are allowed to have duplicate names, for example esaggs
110-
return id === args.id;
111-
}
112-
// If the column has an ID, but there is no ID argument to mapColumn
113-
if (id) {
114-
return id === args.name;
115-
}
116-
// Columns without ID use name as the unique key. For example, SQL output does not have IDs
117-
return name === args.name;
118-
});
119113
const type = rows.length ? getType(rows[0][columnId]) : 'null';
120114
const newColumn = {
121115
id: columnId,
@@ -128,7 +122,7 @@ export const mapColumn: ExpressionFunctionDefinition<
128122
}
129123

130124
if (existingColumnIndex === -1) {
131-
columns.push(newColumn);
125+
columns[columnLength] = newColumn;
132126
} else {
133127
columns[existingColumnIndex] = newColumn;
134128
}

‎src/plugins/expressions/common/expression_functions/specs/tests/map_column.test.ts‎

Lines changed: 25 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import { of } from 'rxjs';
1010
import { Datatable } from '../../../expression_types';
1111
import { mapColumn, MapColumnArguments } from '../map_column';
12-
import { emptyTable, functionWrapper, testTable, sqlTable } from './utils';
12+
import { emptyTable, functionWrapper, testTable } from './utils';
1313

1414
const pricePlusTwo = (datatable: Datatable) => of(datatable.rows[0].price + 2);
1515

@@ -35,66 +35,35 @@ describe('mapColumn', () => {
3535
expect(result.rows[arbitraryRowIndex]).toHaveProperty('pricePlusTwo');
3636
});
3737

38-
describe('when the table columns have id', () => {
39-
it('does not require the id arg by using the name arg as column id', async () => {
40-
const result = await runFn(testTable, { name: 'name', expression: pricePlusTwo });
41-
const nameColumnIndex = result.columns.findIndex(({ name }) => name === 'name');
42-
const arbitraryRowIndex = 4;
43-
44-
expect(result.type).toBe('datatable');
45-
expect(result.columns).toHaveLength(sqlTable.columns.length);
46-
expect(result.columns[nameColumnIndex]).toHaveProperty('name', 'name');
47-
expect(result.columns[nameColumnIndex].meta).toHaveProperty('type', 'number');
48-
expect(result.rows[arbitraryRowIndex]).toHaveProperty('name', 202);
49-
});
38+
it('allows the id arg to be optional, looking up by name instead', async () => {
39+
const result = await runFn(testTable, { name: 'name label', expression: pricePlusTwo });
40+
const nameColumnIndex = result.columns.findIndex(({ name }) => name === 'name label');
41+
const arbitraryRowIndex = 4;
5042

51-
it('allows a duplicate name when the ids are different', async () => {
52-
const result = await runFn(testTable, {
53-
id: 'new',
54-
name: 'name label',
55-
expression: pricePlusTwo,
56-
});
57-
const nameColumnIndex = result.columns.findIndex(({ id }) => id === 'new');
58-
const arbitraryRowIndex = 4;
59-
60-
expect(result.type).toBe('datatable');
61-
expect(result.columns).toHaveLength(testTable.columns.length + 1);
62-
expect(result.columns[nameColumnIndex]).toHaveProperty('id', 'new');
63-
expect(result.columns[nameColumnIndex]).toHaveProperty('name', 'name label');
64-
expect(result.columns[nameColumnIndex].meta).toHaveProperty('type', 'number');
65-
expect(result.rows[arbitraryRowIndex]).toHaveProperty('new', 202);
66-
});
43+
expect(result.type).toBe('datatable');
44+
expect(result.columns).toHaveLength(testTable.columns.length);
45+
expect(result.columns[nameColumnIndex]).toHaveProperty('id', 'name');
46+
expect(result.columns[nameColumnIndex]).toHaveProperty('name', 'name label');
47+
expect(result.columns[nameColumnIndex].meta).toHaveProperty('type', 'number');
48+
expect(result.rows[arbitraryRowIndex]).toHaveProperty('name', 202);
49+
expect(result.rows[arbitraryRowIndex]).not.toHaveProperty('name label');
6750
});
6851

69-
describe('when the table columns do not have id', () => {
70-
it('uses name as unique key when id arg is also missing', async () => {
71-
const result = await runFn(sqlTable, { name: 'name', expression: pricePlusTwo });
72-
const nameColumnIndex = result.columns.findIndex(({ name }) => name === 'name');
73-
const arbitraryRowIndex = 4;
74-
75-
expect(result.type).toBe('datatable');
76-
expect(result.columns).toHaveLength(sqlTable.columns.length);
77-
expect(result.columns[nameColumnIndex]).toHaveProperty('name', 'name');
78-
expect(result.columns[nameColumnIndex].meta).toHaveProperty('type', 'number');
79-
expect(result.rows[arbitraryRowIndex]).toHaveProperty('name', 202);
52+
it('allows a duplicate name when the ids are different', async () => {
53+
const result = await runFn(testTable, {
54+
id: 'new',
55+
name: 'name label',
56+
expression: pricePlusTwo,
8057
});
58+
const nameColumnIndex = result.columns.findIndex(({ id }) => id === 'new');
59+
const arbitraryRowIndex = 4;
8160

82-
it('overwrites columns matching id === name when the column is missing an id', async () => {
83-
const result = await runFn(sqlTable, {
84-
id: 'name',
85-
name: 'name is ignored',
86-
expression: pricePlusTwo,
87-
});
88-
const nameColumnIndex = result.columns.findIndex(({ name }) => name === 'name is ignored');
89-
const arbitraryRowIndex = 4;
90-
91-
expect(result.type).toBe('datatable');
92-
expect(result.columns).toHaveLength(sqlTable.columns.length);
93-
expect(result.columns[nameColumnIndex]).toHaveProperty('id', 'name');
94-
expect(result.columns[nameColumnIndex]).toHaveProperty('name', 'name is ignored');
95-
expect(result.columns[nameColumnIndex].meta).toHaveProperty('type', 'number');
96-
expect(result.rows[arbitraryRowIndex]).toHaveProperty('name', 202);
97-
});
61+
expect(result.type).toBe('datatable');
62+
expect(result.columns).toHaveLength(testTable.columns.length + 1);
63+
expect(result.columns[nameColumnIndex]).toHaveProperty('id', 'new');
64+
expect(result.columns[nameColumnIndex]).toHaveProperty('name', 'name label');
65+
expect(result.columns[nameColumnIndex].meta).toHaveProperty('type', 'number');
66+
expect(result.rows[arbitraryRowIndex]).toHaveProperty('new', 202);
9867
});
9968

10069
it('adds a column to empty tables', async () => {

‎src/plugins/expressions/common/expression_functions/specs/tests/math.test.ts‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
import { errors, math } from '../math';
10-
import { emptyTable, functionWrapper, testTable, sqlTable } from './utils';
10+
import { emptyTable, functionWrapper, testTable } from './utils';
1111

1212
describe('math', () => {
1313
const fn = functionWrapper<unknown>(math);
@@ -36,15 +36,6 @@ describe('math', () => {
3636
expect(fn(testTable, { expression: 'max(price)' })).toBe(605);
3737
});
3838

39-
it('evaluates math expressions with references to columns in a datatable', () => {
40-
expect(fn(sqlTable, { expression: 'unique(in_stock)' })).toBe(2);
41-
expect(fn(sqlTable, { expression: 'sum(quantity)' })).toBe(2508);
42-
expect(fn(sqlTable, { expression: 'mean(price)' })).toBe(320);
43-
expect(fn(sqlTable, { expression: 'min(price)' })).toBe(67);
44-
expect(fn(sqlTable, { expression: 'median(quantity)' })).toBe(256);
45-
expect(fn(sqlTable, { expression: 'max(price)' })).toBe(605);
46-
});
47-
4839
describe('args', () => {
4940
describe('expression', () => {
5041
it('sets the math expression to be evaluted', () => {

‎src/plugins/expressions/common/expression_functions/specs/tests/utils.ts‎

Lines changed: 2 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import { mapValues } from 'lodash';
1010
import { AnyExpressionFunctionDefinition } from '../../types';
1111
import { ExecutionContext } from '../../../execution/types';
12-
import { Datatable, DatatableColumn } from '../../../expression_types';
12+
import { Datatable } from '../../../expression_types';
1313

1414
/**
1515
* Takes a function spec and passes in default args,
@@ -224,32 +224,4 @@ const stringTable: Datatable = {
224224
],
225225
};
226226

227-
// Emulates a SQL table that doesn't have any IDs
228-
const sqlTable: Datatable = {
229-
type: 'datatable',
230-
columns: [
231-
({
232-
name: 'name',
233-
meta: { type: 'string' },
234-
} as unknown) as DatatableColumn,
235-
({
236-
name: 'time',
237-
meta: { type: 'date' },
238-
} as unknown) as DatatableColumn,
239-
({
240-
name: 'price',
241-
meta: { type: 'number' },
242-
} as unknown) as DatatableColumn,
243-
({
244-
name: 'quantity',
245-
meta: { type: 'number' },
246-
} as unknown) as DatatableColumn,
247-
({
248-
name: 'in_stock',
249-
meta: { type: 'boolean' },
250-
} as unknown) as DatatableColumn,
251-
],
252-
rows: [...testTable.rows],
253-
};
254-
255-
export { emptyTable, testTable, stringTable, sqlTable };
227+
export { emptyTable, testTable, stringTable };

‎x-pack/plugins/translations/translations/ja-JP.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2021,7 +2021,6 @@
20212021
"expressions.functions.fontHelpText": "フォントスタイルを作成します。",
20222022
"expressions.functions.mapColumn.args.copyMetaFromHelpText": "設定されている場合、指定した列IDのメタオブジェクトが指定したターゲット列にコピーされます。列が存在しない場合は失敗し、エラーは表示されません。",
20232023
"expressions.functions.mapColumn.args.expressionHelpText": "すべての行で実行される式。単一行の{DATATABLE}と一緒に指定され、セル値を返します。",
2024-
"expressions.functions.mapColumn.args.idHelpText": "結果列の任意のID。「null」の場合、name/column引数がIDとして使用されます。",
20252024
"expressions.functions.mapColumnHelpText": "他の列の結果として計算された列を追加します。引数が指定された場合のみ変更が加えられます。{alterColumnFn}と{staticColumnFn}もご参照ください。",
20262025
"expressions.functions.math.args.expressionHelpText": "評価された {TINYMATH} 表現です。{TINYMATH_URL} をご覧ください。",
20272026
"expressions.functions.math.args.onErrorHelpText": "{TINYMATH}評価が失敗するか、NaNが返される場合、戻り値はonErrorで指定されます。「'throw'」の場合、例外が発生し、式の実行が終了します (デフォルト) 。",

‎x-pack/plugins/translations/translations/zh-CN.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2033,7 +2033,6 @@
20332033
"expressions.functions.fontHelpText": "创建字体样式。",
20342034
"expressions.functions.mapColumn.args.copyMetaFromHelpText": "如果设置，指定列 ID 的元对象将复制到指定目标列。如果列不存在，复制将无提示失败。",
20352035
"expressions.functions.mapColumn.args.expressionHelpText": "在每行上执行的表达式，为其提供了单行 {DATATABLE} 上下文，其将返回单元格值。",
2036-
"expressions.functions.mapColumn.args.idHelpText": "结果列的可选 ID。如果为 `null`，名称/列参数将用作 ID。",
20372036
"expressions.functions.mapColumnHelpText": "添加计算为其他列的结果的列。只有提供参数时，才会执行更改。另请参见 {alterColumnFn} 和 {staticColumnFn}。",
20382037
"expressions.functions.math.args.expressionHelpText": "已计算的 {TINYMATH} 表达式。请参阅 {TINYMATH_URL}。",
20392038
"expressions.functions.math.args.onErrorHelpText": "如果 {TINYMATH} 评估失败或返回 NaN，返回值将由 onError 指定。为 `'throw'` 时，其将引发异常，从而终止表达式执行 (默认) 。",

0 commit comments

Comments
 (0)