From 25614e5c20b292e64d77a2d0f46e998f9d3b7c13 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Mon, 22 Feb 2021 09:03:11 +0100 Subject: [PATCH 1/4] Improve sorting by _score in Discover --- .../application/angular/doc_table/actions/columns.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/plugins/discover/public/application/angular/doc_table/actions/columns.ts b/src/plugins/discover/public/application/angular/doc_table/actions/columns.ts index 53ced59b17c5d..8028aa6c08634 100644 --- a/src/plugins/discover/public/application/angular/doc_table/actions/columns.ts +++ b/src/plugins/discover/public/application/angular/doc_table/actions/columns.ts @@ -5,10 +5,11 @@ * in compliance with, at your election, the Elastic License 2.0 or the Server * Side Public License, v 1. */ -import { Capabilities } from 'kibana/public'; +import { Capabilities, IUiSettingsClient } from 'kibana/public'; import { popularizeField } from '../../../helpers/popularize_field'; import { IndexPattern, IndexPatternsContract } from '../../../../kibana_services'; import { AppState } from '../../discover_state'; +import { SORT_DEFAULT_ORDER_SETTING } from '../../../../../common'; /** * Helper function to provide a fallback to a single _source column if the given array of columns @@ -54,6 +55,7 @@ export function moveColumn(columns: string[], columnName: string, newIndex: numb export function getStateColumnActions({ capabilities, + config, indexPattern, indexPatterns, useNewFieldsApi, @@ -61,6 +63,7 @@ export function getStateColumnActions({ state, }: { capabilities: Capabilities; + config: IUiSettingsClient; indexPattern: IndexPattern; indexPatterns: IndexPatternsContract; useNewFieldsApi: boolean; @@ -72,7 +75,10 @@ export function getStateColumnActions({ popularizeField(indexPattern, columnName, indexPatterns); } const columns = addColumn(state.columns || [], columnName, useNewFieldsApi); - setAppState({ columns }); + const defaultOrder = config.get(SORT_DEFAULT_ORDER_SETTING); + const sort = + columnName === '_score' && !state.sort?.length ? [['_score', defaultOrder]] : state.sort; + setAppState({ columns, sort }); } function onRemoveColumn(columnName: string) { From 6b421cfbd7ac3c46b4d4ebc941ef0f1050f44f4c Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Mon, 22 Feb 2021 13:03:10 +0100 Subject: [PATCH 2/4] Fix missing config param --- .../discover/public/application/components/discover.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/plugins/discover/public/application/components/discover.tsx b/src/plugins/discover/public/application/components/discover.tsx index 1d183aa75cf3a..efb1802d95edc 100644 --- a/src/plugins/discover/public/application/components/discover.tsx +++ b/src/plugins/discover/public/application/components/discover.tsx @@ -105,13 +105,14 @@ export function Discover({ () => getStateColumnActions({ capabilities, + config, indexPattern, indexPatterns, setAppState, state, useNewFieldsApi, }), - [capabilities, indexPattern, indexPatterns, setAppState, state, useNewFieldsApi] + [capabilities, config, indexPattern, indexPatterns, setAppState, state, useNewFieldsApi] ); const onOpenInspector = useCallback(() => { From d013354b1553ad532e76f2f7495114cae90865dd Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Mon, 22 Feb 2021 13:43:12 +0100 Subject: [PATCH 3/4] Add tests --- .../discover/public/__mocks__/config.ts | 5 +- .../angular/doc_table/actions/columns.test.ts | 60 +++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 src/plugins/discover/public/application/angular/doc_table/actions/columns.test.ts diff --git a/src/plugins/discover/public/__mocks__/config.ts b/src/plugins/discover/public/__mocks__/config.ts index a79c56aa77f3b..977dc5699a57a 100644 --- a/src/plugins/discover/public/__mocks__/config.ts +++ b/src/plugins/discover/public/__mocks__/config.ts @@ -6,12 +6,15 @@ * Side Public License, v 1. */ -import { IUiSettingsClient } from '../../../../core/public'; +import { IUiSettingsClient } from 'kibana/public'; +import { SORT_DEFAULT_ORDER_SETTING } from '../../common'; export const configMock = ({ get: (key: string) => { if (key === 'defaultIndex') { return 'the-index-pattern-id'; + } else if (key === SORT_DEFAULT_ORDER_SETTING) { + return 'desc'; } return ''; diff --git a/src/plugins/discover/public/application/angular/doc_table/actions/columns.test.ts b/src/plugins/discover/public/application/angular/doc_table/actions/columns.test.ts new file mode 100644 index 0000000000000..abc8ff616130f --- /dev/null +++ b/src/plugins/discover/public/application/angular/doc_table/actions/columns.test.ts @@ -0,0 +1,60 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { getStateColumnActions } from './columns'; +import { configMock } from '../../../../__mocks__/config'; +import { indexPatternMock } from '../../../../__mocks__/index_pattern'; +import { indexPatternsMock } from '../../../../__mocks__/index_patterns'; +import { Capabilities } from '../../../../../../../core/types'; +import { AppState } from '../../discover_state'; + +function getStateColumnAction(state: {}, setAppState: (state: Partial) => void) { + return getStateColumnActions({ + capabilities: ({ + discover: { + save: false, + }, + } as unknown) as Capabilities, + config: configMock, + indexPattern: indexPatternMock, + indexPatterns: indexPatternsMock, + useNewFieldsApi: true, + setAppState, + state, + }); +} + +describe('Test column actions', () => { + test('getStateColumnActions with empty state', () => { + const setAppState = jest.fn(); + const actions = getStateColumnAction({}, setAppState); + + actions.onAddColumn('_score'); + expect(setAppState).toHaveBeenCalledWith({ columns: ['_score'], sort: [['_score', 'desc']] }); + actions.onAddColumn('test'); + expect(setAppState).toHaveBeenCalledWith({ columns: ['test'] }); + }); + test('getStateColumnActions with columns and sort in state', () => { + const setAppState = jest.fn(); + const actions = getStateColumnAction( + { columns: ['first'], sort: [['first', 'desc']] }, + setAppState + ); + + actions.onAddColumn('_score'); + expect(setAppState).toHaveBeenCalledWith({ + columns: ['first', '_score'], + sort: [['first', 'desc']], + }); + actions.onAddColumn('test'); + expect(setAppState).toHaveBeenCalledWith({ + columns: ['first', '_score'], + sort: [['first', 'desc']], + }); + }); +}); From 208b625f200abf5523473f73d794965de0463f72 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Mon, 22 Feb 2021 16:01:49 +0100 Subject: [PATCH 4/4] Improve tests --- .../angular/doc_table/actions/columns.test.ts | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/src/plugins/discover/public/application/angular/doc_table/actions/columns.test.ts b/src/plugins/discover/public/application/angular/doc_table/actions/columns.test.ts index abc8ff616130f..43044d08f6ac0 100644 --- a/src/plugins/discover/public/application/angular/doc_table/actions/columns.test.ts +++ b/src/plugins/discover/public/application/angular/doc_table/actions/columns.test.ts @@ -42,19 +42,37 @@ describe('Test column actions', () => { test('getStateColumnActions with columns and sort in state', () => { const setAppState = jest.fn(); const actions = getStateColumnAction( - { columns: ['first'], sort: [['first', 'desc']] }, + { columns: ['first', 'second'], sort: [['first', 'desc']] }, setAppState ); actions.onAddColumn('_score'); expect(setAppState).toHaveBeenCalledWith({ - columns: ['first', '_score'], + columns: ['first', 'second', '_score'], sort: [['first', 'desc']], }); - actions.onAddColumn('test'); + setAppState.mockClear(); + actions.onAddColumn('third'); expect(setAppState).toHaveBeenCalledWith({ - columns: ['first', '_score'], + columns: ['first', 'second', 'third'], sort: [['first', 'desc']], }); + setAppState.mockClear(); + actions.onRemoveColumn('first'); + expect(setAppState).toHaveBeenCalledWith({ + columns: ['second'], + sort: [], + }); + setAppState.mockClear(); + actions.onSetColumns(['first', 'second', 'third']); + expect(setAppState).toHaveBeenCalledWith({ + columns: ['first', 'second', 'third'], + }); + setAppState.mockClear(); + + actions.onMoveColumn('second', 0); + expect(setAppState).toHaveBeenCalledWith({ + columns: ['second', 'first'], + }); }); });