[Discover] Fix documents request missing pinned filters (#160693)

## Summary

This PR fixes a bug where pinned filters were not being applied to the
Discover documents request.

Fixes #160579.

### Checklist

- [ ] ~Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)~
- [ ]
~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials~
- [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
- [ ] ~Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard
accessibility](https://webaim.org/techniques/keyboard/))~
- [ ] ~Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))~
- [ ] ~If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~
- [ ] ~This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))~
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
This commit is contained in:
Davis McPhee 2023-06-29 11:39:04 -03:00 committed by GitHub
parent 1932c7936c
commit d9744464dc
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 247 additions and 55 deletions

View file

@ -259,8 +259,6 @@ export interface AppStateUrl extends Omit<DiscoverAppState, 'sort'> {
sort?: string[][] | [string, string];
}
export const GLOBAL_STATE_URL_KEY = '_g';
export function getInitialState(
stateStorage: IKbnUrlStateStorage | undefined,
savedSearch: SavedSearch,

View file

@ -0,0 +1,26 @@
/*
* 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 type { QueryState } from '@kbn/data-plugin/common';
import type { IKbnUrlStateStorage } from '@kbn/kibana-utils-plugin/public';
export interface DiscoverGlobalStateContainer {
get: () => QueryState | null;
set: (state: QueryState) => Promise<void>;
}
const GLOBAL_STATE_URL_KEY = '_g';
export const getDiscoverGlobalStateContainer = (
stateStorage: IKbnUrlStateStorage
): DiscoverGlobalStateContainer => ({
get: () => stateStorage.get<QueryState>(GLOBAL_STATE_URL_KEY),
set: async (state: QueryState) => {
await stateStorage.set(GLOBAL_STATE_URL_KEY, state, { replace: true });
},
});

View file

@ -12,19 +12,22 @@ import { discoverServiceMock } from '../../../__mocks__/services';
import { savedSearchMock, savedSearchMockWithTimeField } from '../../../__mocks__/saved_search';
import { dataViewMock } from '../../../__mocks__/data_view';
import { dataViewComplexMock } from '../../../__mocks__/data_view_complex';
import { getDiscoverGlobalStateContainer } from './discover_global_state_container';
import { createKbnUrlStateStorage } from '@kbn/kibana-utils-plugin/public';
describe('DiscoverSavedSearchContainer', () => {
const savedSearch = savedSearchMock;
const services = discoverServiceMock;
const globalStateContainer = getDiscoverGlobalStateContainer(createKbnUrlStateStorage());
describe('getTitle', () => {
it('returns undefined for new saved searches', () => {
const container = getSavedSearchContainer({ services });
const container = getSavedSearchContainer({ services, globalStateContainer });
expect(container.getTitle()).toBe(undefined);
});
it('returns the title of a persisted saved searches', () => {
const container = getSavedSearchContainer({ services });
const container = getSavedSearchContainer({ services, globalStateContainer });
container.set(savedSearch);
expect(container.getTitle()).toBe(savedSearch.title);
});
@ -32,7 +35,7 @@ describe('DiscoverSavedSearchContainer', () => {
describe('set', () => {
it('should update the current and initial state of the saved search', () => {
const container = getSavedSearchContainer({ services });
const container = getSavedSearchContainer({ services, globalStateContainer });
const newSavedSearch: SavedSearch = { ...savedSearch, title: 'New title' };
const result = container.set(newSavedSearch);
@ -45,7 +48,7 @@ describe('DiscoverSavedSearchContainer', () => {
});
it('should reset hasChanged$ to false', () => {
const container = getSavedSearchContainer({ services });
const container = getSavedSearchContainer({ services, globalStateContainer });
const newSavedSearch: SavedSearch = { ...savedSearch, title: 'New title' };
container.set(newSavedSearch);
@ -55,7 +58,7 @@ describe('DiscoverSavedSearchContainer', () => {
describe('new', () => {
it('should create a new saved search', async () => {
const container = getSavedSearchContainer({ services });
const container = getSavedSearchContainer({ services, globalStateContainer });
const result = await container.new(dataViewMock);
expect(result.title).toBeUndefined();
@ -68,7 +71,7 @@ describe('DiscoverSavedSearchContainer', () => {
});
it('should create a new saved search with provided DataView', async () => {
const container = getSavedSearchContainer({ services });
const container = getSavedSearchContainer({ services, globalStateContainer });
const result = await container.new(dataViewMock);
expect(result.title).toBeUndefined();
expect(result.id).toBeUndefined();
@ -86,6 +89,7 @@ describe('DiscoverSavedSearchContainer', () => {
it('loads a saved search', async () => {
const savedSearchContainer = getSavedSearchContainer({
services: discoverServiceMock,
globalStateContainer,
});
await savedSearchContainer.load('the-saved-search-id');
expect(savedSearchContainer.getInitial$().getValue().id).toEqual('the-saved-search-id');
@ -100,6 +104,7 @@ describe('DiscoverSavedSearchContainer', () => {
it('calls saveSavedSearch with the given saved search and save options', async () => {
const savedSearchContainer = getSavedSearchContainer({
services: discoverServiceMock,
globalStateContainer,
});
const savedSearchToPersist = {
...savedSearchMockWithTimeField,
@ -124,6 +129,7 @@ describe('DiscoverSavedSearchContainer', () => {
const savedSearchContainer = getSavedSearchContainer({
services: discoverServiceMock,
globalStateContainer,
});
const result = await savedSearchContainer.persist(persistedSavedSearch, saveOptions);
@ -135,6 +141,7 @@ describe('DiscoverSavedSearchContainer', () => {
it('emits false to the hasChanged$ BehaviorSubject', async () => {
const savedSearchContainer = getSavedSearchContainer({
services: discoverServiceMock,
globalStateContainer,
});
const savedSearchToPersist = {
...savedSearchMockWithTimeField,
@ -153,6 +160,7 @@ describe('DiscoverSavedSearchContainer', () => {
}));
const savedSearchContainer = getSavedSearchContainer({
services: discoverServiceMock,
globalStateContainer,
});
const savedSearchToPersist = {
...savedSearchMockWithTimeField,
@ -176,6 +184,7 @@ describe('DiscoverSavedSearchContainer', () => {
const savedSearchContainer = getSavedSearchContainer({
services: discoverServiceMock,
globalStateContainer,
});
savedSearchContainer.set(savedSearch);
savedSearchContainer.update({ nextState: { hideChart: true } });
@ -196,28 +205,30 @@ describe('DiscoverSavedSearchContainer', () => {
it('updates a saved search by app state providing hideChart', async () => {
const savedSearchContainer = getSavedSearchContainer({
services: discoverServiceMock,
globalStateContainer,
});
savedSearchContainer.set(savedSearch);
const updated = await savedSearchContainer.update({ nextState: { hideChart: true } });
const updated = savedSearchContainer.update({ nextState: { hideChart: true } });
expect(savedSearchContainer.getHasChanged$().getValue()).toBe(true);
savedSearchContainer.set(updated);
expect(savedSearchContainer.getHasChanged$().getValue()).toBe(false);
await savedSearchContainer.update({ nextState: { hideChart: false } });
savedSearchContainer.update({ nextState: { hideChart: false } });
expect(savedSearchContainer.getHasChanged$().getValue()).toBe(true);
await savedSearchContainer.update({ nextState: { hideChart: true } });
savedSearchContainer.update({ nextState: { hideChart: true } });
expect(savedSearchContainer.getHasChanged$().getValue()).toBe(false);
});
it('updates a saved search by data view', async () => {
const savedSearchContainer = getSavedSearchContainer({
services: discoverServiceMock,
globalStateContainer,
});
const updated = await savedSearchContainer.update({ nextDataView: dataViewMock });
const updated = savedSearchContainer.update({ nextDataView: dataViewMock });
expect(savedSearchContainer.getHasChanged$().getValue()).toBe(true);
savedSearchContainer.set(updated);
expect(savedSearchContainer.getHasChanged$().getValue()).toBe(false);
await savedSearchContainer.update({ nextDataView: dataViewComplexMock });
savedSearchContainer.update({ nextDataView: dataViewComplexMock });
expect(savedSearchContainer.getHasChanged$().getValue()).toBe(true);
await savedSearchContainer.update({ nextDataView: dataViewMock });
savedSearchContainer.update({ nextDataView: dataViewMock });
expect(savedSearchContainer.getHasChanged$().getValue()).toBe(false);
});
});

View file

@ -18,6 +18,7 @@ import { handleSourceColumnState } from '../../../utils/state_helpers';
import { DiscoverAppState } from './discover_app_state_container';
import { DiscoverServices } from '../../../build_services';
import { getStateDefaults } from '../utils/get_state_defaults';
import type { DiscoverGlobalStateContainer } from './discover_global_state_container';
export interface UpdateParams {
/**
@ -106,8 +107,10 @@ export interface DiscoverSavedSearchContainer {
export function getSavedSearchContainer({
services,
globalStateContainer,
}: {
services: DiscoverServices;
globalStateContainer: DiscoverGlobalStateContainer;
}): DiscoverSavedSearchContainer {
const initialSavedSearch = services.savedSearch.getNew();
const savedSearchInitial$ = new BehaviorSubject(initialSavedSearch);
@ -137,6 +140,7 @@ export function getSavedSearchContainer({
savedSearch: { ...nextSavedSearch },
dataView,
state: newAppState,
globalStateContainer,
services,
});
return set(nextSavedSearchToSet);
@ -144,7 +148,12 @@ export function getSavedSearchContainer({
const persist = async (nextSavedSearch: SavedSearch, saveOptions?: SavedObjectSaveOpts) => {
addLog('[savedSearch] persist', { nextSavedSearch, saveOptions });
updateSavedSearch({ savedSearch: nextSavedSearch, services }, true);
updateSavedSearch({
savedSearch: nextSavedSearch,
globalStateContainer,
services,
useFilterAndQueryServices: true,
});
const id = await services.savedSearch.save(nextSavedSearch, saveOptions || {});
@ -161,15 +170,14 @@ export function getSavedSearchContainer({
? nextDataView
: previousSavedSearch.searchSource.getField('index')!;
const nextSavedSearch = updateSavedSearch(
{
savedSearch: { ...previousSavedSearch },
dataView,
state: nextState || {},
services,
},
useFilterAndQueryServices
);
const nextSavedSearch = updateSavedSearch({
savedSearch: { ...previousSavedSearch },
dataView,
state: nextState || {},
globalStateContainer,
services,
useFilterAndQueryServices,
});
const hasChanged = !isEqualSavedSearch(savedSearchInitial$.getValue(), nextSavedSearch);
hasChanged$.next(hasChanged);

View file

@ -16,7 +16,6 @@ import {
import {
DataPublicPluginStart,
noSearchSessionStorageCapabilityMessage,
QueryState,
SearchSessionInfoProvider,
} from '@kbn/data-plugin/public';
import { DataView, DataViewSpec, DataViewType } from '@kbn/data-views-plugin/public';
@ -38,7 +37,6 @@ import {
DiscoverAppState,
DiscoverAppStateContainer,
getDiscoverAppStateContainer,
GLOBAL_STATE_URL_KEY,
} from './discover_app_state_container';
import {
DiscoverInternalStateContainer,
@ -51,6 +49,7 @@ import {
DiscoverSavedSearchContainer,
} from './discover_saved_search_container';
import { updateFiltersReferences } from '../utils/update_filter_references';
import { getDiscoverGlobalStateContainer } from './discover_global_state_container';
interface DiscoverStateContainerParams {
/**
* Browser history
@ -192,6 +191,7 @@ export function getDiscoverStateContainer({
}: DiscoverStateContainerParams): DiscoverStateContainer {
const storeInSessionStorage = services.uiSettings.get('state:storeInSessionStorage');
const toasts = services.core.notifications.toasts;
/**
* state storage for state in the URL
*/
@ -208,11 +208,18 @@ export function getDiscoverStateContainer({
history,
session: services.data.search.session,
});
/**
* Global State Container, synced with the _g part URL
*/
const globalStateContainer = getDiscoverGlobalStateContainer(stateStorage);
/**
* Saved Search State Container, the persisted saved object of Discover
*/
const savedSearchContainer = getSavedSearchContainer({
services,
globalStateContainer,
});
/**
@ -223,6 +230,7 @@ export function getDiscoverStateContainer({
savedSearch: savedSearchContainer.getState(),
services,
});
/**
* Internal State Container, state that's not persisted and not part of the URL
*/
@ -230,13 +238,12 @@ export function getDiscoverStateContainer({
const pauseAutoRefreshInterval = async (dataView: DataView) => {
if (dataView && (!dataView.isTimeBased() || dataView.type === DataViewType.ROLLUP)) {
const state = stateStorage.get<QueryState>(GLOBAL_STATE_URL_KEY);
const state = globalStateContainer.get();
if (state?.refreshInterval && !state.refreshInterval.pause) {
await stateStorage.set(
GLOBAL_STATE_URL_KEY,
{ ...state, refreshInterval: { ...state?.refreshInterval, pause: true } },
{ replace: true }
);
await globalStateContainer.set({
...state,
refreshInterval: { ...state?.refreshInterval, pause: true },
});
}
}
};
@ -351,8 +358,8 @@ export function getDiscoverStateContainer({
const unsubscribeData = dataStateContainer.subscribe();
// updates saved search when query or filters change, triggers data fetching
const filterUnsubscribe = merge(services.filterManager.getFetches$()).subscribe(async () => {
await savedSearchContainer.update({
const filterUnsubscribe = merge(services.filterManager.getFetches$()).subscribe(() => {
savedSearchContainer.update({
nextDataView: internalStateContainer.getState().dataView,
nextState: appStateContainer.getState(),
useFilterAndQueryServices: true,
@ -424,7 +431,7 @@ export function getDiscoverStateContainer({
const undoSavedSearchChanges = async () => {
addLog('undoSavedSearchChanges');
const nextSavedSearch = savedSearchContainer.getInitial$().getValue();
await savedSearchContainer.set(nextSavedSearch);
savedSearchContainer.set(nextSavedSearch);
restoreStateFromSavedSearch({
savedSearch: nextSavedSearch,
timefilter: services.timefilter,

View file

@ -0,0 +1,96 @@
/*
* 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 { savedSearchMock } from '../../../__mocks__/saved_search';
import { discoverServiceMock } from '../../../__mocks__/services';
import { Filter, FilterStateStore, Query } from '@kbn/es-query';
import { updateSavedSearch } from './update_saved_search';
describe('updateSavedSearch', () => {
const query: Query = {
query: 'extension:jpg',
language: 'kuery',
};
const appFilter: Filter = {
meta: {},
query: {
match_phrase: {
extension: {
query: 'jpg',
type: 'phrase',
},
},
},
$state: {
store: FilterStateStore.APP_STATE,
},
};
const globalFilter: Filter = {
meta: {},
query: {
match_phrase: {
extension: {
query: 'png',
type: 'phrase',
},
},
},
$state: {
store: FilterStateStore.GLOBAL_STATE,
},
};
const createGlobalStateContainer = () => ({
get: jest.fn(() => ({ filters: [globalFilter] })),
set: jest.fn(),
});
beforeEach(() => {
jest.clearAllMocks();
});
it('should set query and filters from appState and globalState', async () => {
const savedSearch = {
...savedSearchMock,
searchSource: savedSearchMock.searchSource.createCopy(),
};
expect(savedSearch.searchSource.getField('query')).toBeUndefined();
expect(savedSearch.searchSource.getField('filter')).toBeUndefined();
updateSavedSearch({
savedSearch,
globalStateContainer: createGlobalStateContainer(),
services: discoverServiceMock,
state: {
query,
filters: [appFilter],
},
});
expect(savedSearch.searchSource.getField('query')).toEqual(query);
expect(savedSearch.searchSource.getField('filter')).toEqual([appFilter, globalFilter]);
});
it('should set query and filters from services', async () => {
const savedSearch = {
...savedSearchMock,
searchSource: savedSearchMock.searchSource.createCopy(),
};
expect(savedSearch.searchSource.getField('query')).toBeUndefined();
expect(savedSearch.searchSource.getField('filter')).toBeUndefined();
jest
.spyOn(discoverServiceMock.data.query.filterManager, 'getFilters')
.mockReturnValue([appFilter, globalFilter]);
jest.spyOn(discoverServiceMock.data.query.queryString, 'getQuery').mockReturnValue(query);
updateSavedSearch({
savedSearch,
globalStateContainer: createGlobalStateContainer(),
services: discoverServiceMock,
useFilterAndQueryServices: true,
});
expect(savedSearch.searchSource.getField('query')).toEqual(query);
expect(savedSearch.searchSource.getField('filter')).toEqual([appFilter, globalFilter]);
});
});

View file

@ -5,12 +5,13 @@
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import { SavedSearch, SortOrder } from '@kbn/saved-search-plugin/public';
import { DataView } from '@kbn/data-views-plugin/common';
import type { SavedSearch, SortOrder } from '@kbn/saved-search-plugin/public';
import type { DataView } from '@kbn/data-views-plugin/common';
import { cloneDeep } from 'lodash';
import { isTextBasedQuery } from './is_text_based_query';
import { DiscoverAppState } from '../services/discover_app_state_container';
import { DiscoverServices } from '../../../build_services';
import type { DiscoverAppState } from '../services/discover_app_state_container';
import type { DiscoverServices } from '../../../build_services';
import type { DiscoverGlobalStateContainer } from '../services/discover_global_state_container';
/**
* Updates the saved search with a given data view & Appstate
@ -22,20 +23,21 @@ import { DiscoverServices } from '../../../build_services';
* @param services
* @param useFilterAndQueryServices - when true data services are being used for updating filter + query
*/
export function updateSavedSearch(
{
savedSearch,
dataView,
state,
services,
}: {
savedSearch: SavedSearch;
dataView?: DataView;
state?: DiscoverAppState;
services: DiscoverServices;
},
useFilterAndQueryServices: boolean = false
) {
export function updateSavedSearch({
savedSearch,
dataView,
state,
globalStateContainer,
services,
useFilterAndQueryServices = false,
}: {
savedSearch: SavedSearch;
dataView?: DataView;
state?: DiscoverAppState;
globalStateContainer: DiscoverGlobalStateContainer;
services: DiscoverServices;
useFilterAndQueryServices?: boolean;
}) {
if (dataView) {
savedSearch.searchSource.setField('index', dataView);
savedSearch.usesAdHocDataView = !dataView.isPersisted();
@ -45,9 +47,12 @@ export function updateSavedSearch(
.setField('query', services.data.query.queryString.getQuery())
.setField('filter', services.data.query.filterManager.getFilters());
} else if (state) {
const appFilters = state.filters ? cloneDeep(state.filters) : [];
const globalFilters = globalStateContainer.get()?.filters ?? [];
savedSearch.searchSource
.setField('query', state.query ?? undefined)
.setField('filter', state.filters ? cloneDeep(state.filters) : []);
.setField('filter', [...appFilters, ...globalFilters]);
}
if (state) {
savedSearch.columns = state.columns || [];

View file

@ -17,7 +17,9 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
const kibanaServer = getService('kibanaServer');
const filterBar = getService('filterBar');
const security = getService('security');
const PageObjects = getPageObjects(['common', 'discover', 'timePicker']);
const browser = getService('browser');
const dataGrid = getService('dataGrid');
const PageObjects = getPageObjects(['common', 'discover', 'timePicker', 'unifiedFieldList']);
const defaultSettings = {
defaultIndex: 'logstash-*',
};
@ -124,6 +126,45 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
});
});
});
const runFilterTest = async (pinned = false) => {
await filterBar.removeAllFilters();
await PageObjects.unifiedFieldList.clickFieldListItemAdd('extension');
await retry.try(async function () {
const cell = await dataGrid.getCellElement(0, 3);
expect(await cell.getVisibleText()).to.be('jpg');
expect(await PageObjects.discover.getHitCount()).to.be('14,004');
});
await filterBar.addFilter({
field: 'extension.raw',
operation: 'is',
value: 'css',
});
if (pinned) {
await filterBar.toggleFilterPinned('extension.raw');
}
await retry.try(async function () {
expect(await filterBar.hasFilter('extension.raw', 'css', true, pinned)).to.be(true);
const cell = await dataGrid.getCellElement(0, 3);
expect(await cell.getVisibleText()).to.be('css');
expect(await PageObjects.discover.getHitCount()).to.be('2,159');
});
await browser.refresh();
await retry.try(async function () {
expect(await filterBar.hasFilter('extension.raw', 'css', true, pinned)).to.be(true);
expect(await PageObjects.discover.getHitCount()).to.be('2,159');
const cell = await dataGrid.getCellElement(0, 3);
expect(await cell.getVisibleText()).to.be('css');
});
};
it('should support app filters in histogram/total hits and data grid', async function () {
await runFilterTest();
});
it('should support pinned filters in histogram/total hits and data grid', async function () {
await runFilterTest(true);
});
});
after(async () => {