mirror of
https://github.com/elastic/kibana.git
synced 2025-04-23 09:19:04 -04:00
[8.15] [Discover] View mode changes should trigger Unsaved changes badge in ES|QL (#187244) (#187687)
# Backport This will backport the following commits from `main` to `8.15`: - [[Discover] View mode changes should trigger Unsaved changes badge in ES|QL (#187244)](https://github.com/elastic/kibana/pull/187244) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Julia Rechkunova","email":"julia.rechkunova@elastic.co"},"sourceCommit":{"committedDate":"2024-07-05T14:46:00Z","message":"[Discover] View mode changes should trigger Unsaved changes badge in ES|QL (#187244)\n\n- Closes https://github.com/elastic/kibana/issues/184624\r\n\r\n## Summary\r\n\r\nThis PR updates the logic to show Unsaved changes badge when view mode\r\nchanges in ES|QL mode.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Matthias Wilhelm <matthias.wilhelm@elastic.co>","sha":"25d73466c3e1afff0c17b7f8800f92449e9480a8","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:DataDiscovery","backport:prev-minor","v8.16.0"],"title":"[Discover] View mode changes should trigger Unsaved changes badge in ES|QL","number":187244,"url":"https://github.com/elastic/kibana/pull/187244","mergeCommit":{"message":"[Discover] View mode changes should trigger Unsaved changes badge in ES|QL (#187244)\n\n- Closes https://github.com/elastic/kibana/issues/184624\r\n\r\n## Summary\r\n\r\nThis PR updates the logic to show Unsaved changes badge when view mode\r\nchanges in ES|QL mode.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Matthias Wilhelm <matthias.wilhelm@elastic.co>","sha":"25d73466c3e1afff0c17b7f8800f92449e9480a8"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/187244","number":187244,"mergeCommit":{"message":"[Discover] View mode changes should trigger Unsaved changes badge in ES|QL (#187244)\n\n- Closes https://github.com/elastic/kibana/issues/184624\r\n\r\n## Summary\r\n\r\nThis PR updates the logic to show Unsaved changes badge when view mode\r\nchanges in ES|QL mode.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Matthias Wilhelm <matthias.wilhelm@elastic.co>","sha":"25d73466c3e1afff0c17b7f8800f92449e9480a8"}}]}] BACKPORT--> Co-authored-by: Julia Rechkunova <julia.rechkunova@elastic.co>
This commit is contained in:
Kibana Machine
GitHub
parent
9d6491dbed
commit
9c5f961c6b
4 changed files with 87 additions and 13 deletions
|
|
@ -14,6 +14,8 @@ import { dataViewMock } from '@kbn/discover-utils/src/__mocks__';
|
|||
import { dataViewComplexMock } from '../../../__mocks__/data_view_complex';
|
||||
import { getDiscoverGlobalStateContainer } from './discover_global_state_container';
|
||||
import { createKbnUrlStateStorage } from '@kbn/kibana-utils-plugin/public';
|
||||
import { VIEW_MODE } from '../../../../common/constants';
|
||||
import { createSearchSourceMock } from '@kbn/data-plugin/common/search/search_source/mocks';
|
||||
|
||||
describe('DiscoverSavedSearchContainer', () => {
|
||||
const savedSearch = savedSearchMock;
|
||||
|
|
@ -232,4 +234,57 @@ describe('DiscoverSavedSearchContainer', () => {
|
|||
expect(savedSearchContainer.getHasChanged$().getValue()).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isEqualSavedSearch', () => {
|
||||
it('should return true for equal saved searches', () => {
|
||||
const savedSearch1 = savedSearchMock;
|
||||
const savedSearch2 = { ...savedSearchMock };
|
||||
expect(isEqualSavedSearch(savedSearch1, savedSearch2)).toBe(true);
|
||||
});
|
||||
|
||||
it('should return false for different saved searches', () => {
|
||||
const savedSearch1 = savedSearchMock;
|
||||
const savedSearch2 = { ...savedSearchMock, title: 'New title' };
|
||||
expect(isEqualSavedSearch(savedSearch1, savedSearch2)).toBe(false);
|
||||
});
|
||||
|
||||
it('should return true for saved searches with equal default values of viewMode and false otherwise', () => {
|
||||
const savedSearch1 = { ...savedSearchMock, viewMode: undefined };
|
||||
const savedSearch2 = { ...savedSearchMock, viewMode: VIEW_MODE.DOCUMENT_LEVEL };
|
||||
const savedSearch3 = { ...savedSearchMock, viewMode: VIEW_MODE.AGGREGATED_LEVEL };
|
||||
expect(isEqualSavedSearch(savedSearch1, savedSearch2)).toBe(true);
|
||||
expect(isEqualSavedSearch(savedSearch2, savedSearch1)).toBe(true);
|
||||
expect(isEqualSavedSearch(savedSearch1, savedSearch3)).toBe(false);
|
||||
expect(isEqualSavedSearch(savedSearch2, savedSearch3)).toBe(false);
|
||||
});
|
||||
|
||||
it('should return true for saved searches with equal default values of breakdownField and false otherwise', () => {
|
||||
const savedSearch1 = { ...savedSearchMock, breakdownField: undefined };
|
||||
const savedSearch2 = { ...savedSearchMock, breakdownField: '' };
|
||||
const savedSearch3 = { ...savedSearchMock, breakdownField: 'test' };
|
||||
expect(isEqualSavedSearch(savedSearch1, savedSearch2)).toBe(true);
|
||||
expect(isEqualSavedSearch(savedSearch2, savedSearch1)).toBe(true);
|
||||
expect(isEqualSavedSearch(savedSearch1, savedSearch3)).toBe(false);
|
||||
expect(isEqualSavedSearch(savedSearch2, savedSearch3)).toBe(false);
|
||||
});
|
||||
|
||||
it('should check searchSource fields', () => {
|
||||
const savedSearch1 = {
|
||||
...savedSearchMock,
|
||||
searchSource: createSearchSourceMock({
|
||||
index: savedSearchMock.searchSource.getField('index'),
|
||||
}),
|
||||
};
|
||||
const savedSearch2 = {
|
||||
...savedSearchMock,
|
||||
searchSource: createSearchSourceMock({
|
||||
index: savedSearchMock.searchSource.getField('index'),
|
||||
}),
|
||||
};
|
||||
expect(isEqualSavedSearch(savedSearch1, savedSearch1)).toBe(true);
|
||||
expect(isEqualSavedSearch(savedSearch1, savedSearch2)).toBe(true);
|
||||
savedSearch2.searchSource.setField('query', { language: 'lucene', query: 'test' });
|
||||
expect(isEqualSavedSearch(savedSearch1, savedSearch2)).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -341,12 +341,7 @@ export function isEqualSavedSearch(savedSearchPrev: SavedSearch, savedSearchNext
|
|||
const prevValue = getSavedSearchFieldForComparison(prevSavedSearch, key);
|
||||
const nextValue = getSavedSearchFieldForComparison(nextSavedSearchWithoutSearchSource, key);
|
||||
|
||||
const isSame =
|
||||
isEqual(prevValue, nextValue) ||
|
||||
// By default, viewMode: undefined is equivalent to documents view
|
||||
// So they should be treated as same
|
||||
(key === 'viewMode' &&
|
||||
(prevValue ?? VIEW_MODE.DOCUMENT_LEVEL) === (nextValue ?? VIEW_MODE.DOCUMENT_LEVEL));
|
||||
const isSame = isEqual(prevValue, nextValue);
|
||||
|
||||
if (!isSame) {
|
||||
addLog('[savedSearch] difference between initial and changed version', {
|
||||
|
|
@ -412,6 +407,12 @@ function getSavedSearchFieldForComparison(
|
|||
return savedSearch.breakdownField || ''; // ignore the difference between an empty string and undefined
|
||||
}
|
||||
|
||||
if (fieldName === 'viewMode') {
|
||||
// By default, viewMode: undefined is equivalent to documents view
|
||||
// So they should be treated as same
|
||||
return savedSearch.viewMode ?? VIEW_MODE.DOCUMENT_LEVEL;
|
||||
}
|
||||
|
||||
return savedSearch[fieldName];
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -11,7 +11,8 @@ import { FetchStatus } from '../../../types';
|
|||
import { dataViewComplexMock } from '../../../../__mocks__/data_view_complex';
|
||||
import { getDiscoverStateMock } from '../../../../__mocks__/discover_state.mock';
|
||||
import { discoverServiceMock } from '../../../../__mocks__/services';
|
||||
import { createDataViewDataSource } from '../../../../../common/data_sources';
|
||||
import { createDataViewDataSource, DataSourceType } from '../../../../../common/data_sources';
|
||||
import { VIEW_MODE } from '@kbn/saved-search-plugin/common';
|
||||
|
||||
describe('buildStateSubscribe', () => {
|
||||
const savedSearch = savedSearchMock;
|
||||
|
|
@ -19,6 +20,7 @@ describe('buildStateSubscribe', () => {
|
|||
stateContainer.dataState.refetch$.next = jest.fn();
|
||||
stateContainer.dataState.reset = jest.fn();
|
||||
stateContainer.actions.setDataView = jest.fn();
|
||||
stateContainer.savedSearchState.update = jest.fn();
|
||||
|
||||
const getSubscribeFn = () => {
|
||||
return buildStateSubscribe({
|
||||
|
|
@ -51,6 +53,20 @@ describe('buildStateSubscribe', () => {
|
|||
expect(stateContainer.dataState.refetch$.next).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not call refetch$ if viewMode changes', async () => {
|
||||
await getSubscribeFn()({
|
||||
...stateContainer.appState.getState(),
|
||||
dataSource: {
|
||||
type: DataSourceType.Esql,
|
||||
},
|
||||
viewMode: VIEW_MODE.AGGREGATED_LEVEL,
|
||||
});
|
||||
|
||||
expect(stateContainer.dataState.refetch$.next).not.toHaveBeenCalled();
|
||||
expect(stateContainer.dataState.reset).not.toHaveBeenCalled();
|
||||
expect(stateContainer.savedSearchState.update).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should call refetch$ if the chart is hidden', async () => {
|
||||
await getSubscribeFn()({ hideChart: true });
|
||||
|
||||
|
|
|
|||
|
|
@ -55,12 +55,14 @@ export const buildStateSubscribe =
|
|||
const isEsqlMode = isDataSourceType(nextState.dataSource, DataSourceType.Esql);
|
||||
const queryChanged = !isEqual(nextQuery, prevQuery) || !isEqual(nextQuery, prevState.query);
|
||||
|
||||
if (
|
||||
isEsqlMode &&
|
||||
isEqualState(prevState, nextState, ['dataSource', 'viewMode']) &&
|
||||
!queryChanged
|
||||
) {
|
||||
// When there's a switch from data view to es|ql, this just leads to a cleanup of index and viewMode
|
||||
if (isEsqlMode && prevState.viewMode !== nextState.viewMode && !queryChanged) {
|
||||
savedSearchState.update({ nextState });
|
||||
addLog('[appstate] subscribe $fetch ignored for es|ql', { prevState, nextState });
|
||||
return;
|
||||
}
|
||||
|
||||
if (isEsqlMode && isEqualState(prevState, nextState, ['dataSource']) && !queryChanged) {
|
||||
// When there's a switch from data view to es|ql, this just leads to a cleanup of index
|
||||
// And there's no subsequent action in this function required
|
||||
addLog('[appstate] subscribe update ignored for es|ql', { prevState, nextState });
|
||||
return;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue