Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Discover] Show "Unsaved changes" badge on time filter changes in case time range is stored along with saved search #178659

Merged
merged 8 commits into from
Mar 20, 2024
2 changes: 2 additions & 0 deletions src/plugins/discover/public/__mocks__/saved_search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ export const savedSearchMock = {
id: 'the-saved-search-id',
title: 'A saved search',
searchSource: createSearchSourceMock({ index: dataViewMock }),
columns: ['default_column'],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ Making the mocked saved search more realistic, as these are filled in automatically when discover loads the saved search

sort: [],
} as unknown as SavedSearch;

export const savedSearchMockWithTimeField = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@ export interface DiscoverSavedSearchContainer {
* @param params
*/
update: (params: UpdateParams) => SavedSearch;
/**
* Updates the current state of the saved search with new time range and refresh interval
*/
updateTimeRange: () => void;
/**
* Passes filter manager filters to saved search filters
* @param params
Expand Down Expand Up @@ -218,6 +222,23 @@ export function getSavedSearchContainer({
return nextSavedSearch;
};

const updateTimeRange = () => {
const previousSavedSearch = getState();
if (!previousSavedSearch.timeRestore) {
return;
}
const refreshInterval = services.timefilter.getRefreshInterval();
const nextSavedSearch: SavedSearch = {
...previousSavedSearch,
timeRange: services.timefilter.getTime(),
refreshInterval: { value: refreshInterval.value, pause: refreshInterval.pause },
};

assignNextSavedSearch({ nextSavedSearch });

addLog('[savedSearch] updateWithTimeRange done', nextSavedSearch);
};

const load = async (id: string, dataView: DataView | undefined): Promise<SavedSearch> => {
addLog('[savedSearch] load', { id, dataView });

Expand Down Expand Up @@ -245,6 +266,7 @@ export function getSavedSearchContainer({
persist,
set,
update,
updateTimeRange,
updateWithFilterManagerFilters,
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
createSearchSessionRestorationDataProvider,
} from './discover_state';
import { createBrowserHistory, createMemoryHistory, History } from 'history';
import { dataPluginMock } from '@kbn/data-plugin/public/mocks';
import { createSearchSourceMock, dataPluginMock } from '@kbn/data-plugin/public/mocks';
import type { SavedSearch, SortOrder } from '@kbn/saved-search-plugin/public';
import {
savedSearchAdHoc,
Expand Down Expand Up @@ -356,6 +356,9 @@ describe('Test discover state actions', () => {
discoverServiceMock.data.query.timefilter.timefilter.getTime = jest.fn(() => {
return { from: 'now-15d', to: 'now' };
});
discoverServiceMock.data.query.timefilter.timefilter.getRefreshInterval = jest.fn(() => {
return { pause: true, value: 1000 };
});
discoverServiceMock.data.search.searchSource.create = jest
.fn()
.mockReturnValue(savedSearchMock.searchSource);
Expand Down Expand Up @@ -523,6 +526,71 @@ describe('Test discover state actions', () => {
unsubscribe();
});

test('loadSavedSearch given a URL with different time range than the stored one showing as changed', async () => {
const url = '/#_g=(time:(from:now-24h%2Fh,to:now))';
const savedSearch = {
...savedSearchMock,
searchSource: createSearchSourceMock({ index: dataViewMock, filter: [] }),
timeRestore: true,
timeRange: { from: 'now-15d', to: 'now' },
};
const { state } = await getState(url, {
savedSearch,
isEmptyUrl: false,
});
await state.actions.loadSavedSearch({ savedSearchId: savedSearchMock.id });
const unsubscribe = state.actions.initializeAndSync();
await new Promise(process.nextTick);
expect(state.savedSearchState.getHasChanged$().getValue()).toBe(true);
unsubscribe();
});

test('loadSavedSearch given a URL with different refresh interval than the stored one showing as changed', async () => {
const url = '/#_g=(time:(from:now-15d,to:now),refreshInterval:(pause:!f,value:1234))';
discoverServiceMock.data.query.timefilter.timefilter.getRefreshInterval = jest.fn(() => {
return { pause: false, value: 1234 };
});
const savedSearch = {
...savedSearchMock,
searchSource: createSearchSourceMock({ index: dataViewMock, filter: [] }),
timeRestore: true,
timeRange: { from: 'now-15d', to: 'now' },
refreshInterval: { pause: false, value: 60000 },
};
const { state } = await getState(url, {
savedSearch,
isEmptyUrl: false,
});
await state.actions.loadSavedSearch({ savedSearchId: savedSearchMock.id });
const unsubscribe = state.actions.initializeAndSync();
await new Promise(process.nextTick);
expect(state.savedSearchState.getHasChanged$().getValue()).toBe(true);
unsubscribe();
});

test('loadSavedSearch given a URL with matching time range and refresh interval not showing as changed', async () => {
const url = '/#?_g=(time:(from:now-15d,to:now),refreshInterval:(pause:!f,value:60000))';
discoverServiceMock.data.query.timefilter.timefilter.getRefreshInterval = jest.fn(() => {
return { pause: false, value: 60000 };
});
const savedSearch = {
...savedSearchMock,
searchSource: createSearchSourceMock({ index: dataViewMock, filter: [] }),
timeRestore: true,
timeRange: { from: 'now-15d', to: 'now' },
refreshInterval: { pause: false, value: 60000 },
};
const { state } = await getState(url, {
savedSearch,
isEmptyUrl: false,
});
await state.actions.loadSavedSearch({ savedSearchId: savedSearchMock.id });
const unsubscribe = state.actions.initializeAndSync();
await new Promise(process.nextTick);
expect(state.savedSearchState.getHasChanged$().getValue()).toBe(false);
unsubscribe();
});

test('loadSavedSearch ignoring hideChart in URL', async () => {
const url = '/#?_a=(hideChart:true,columns:!(message))&_g=()';
const { state } = await getState(url, { savedSearch: savedSearchMock });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,15 @@ export function getDiscoverStateContainer({
* state containers initializing and subscribing to changes triggering e.g. data fetching
*/
const initializeAndSync = () => {
// This needs to be the first thing that's wired up because initAndSync is pulling the current state from the URL which
// might change the time filter and thus needs to re-check whether the saved search has changed.
const timefilerUnsubscribe = merge(
services.timefilter.getTimeUpdate$(),
services.timefilter.getRefreshIntervalUpdate$()
).subscribe(() => {
savedSearchContainer.updateTimeRange();
});

// initialize app state container, syncing with _g and _a part of the URL
const appStateInitAndSyncUnsubscribe = appStateContainer.initAndSync(
savedSearchContainer.getState()
Expand Down Expand Up @@ -426,6 +435,7 @@ export function getDiscoverStateContainer({
appStateUnsubscribe();
appStateInitAndSyncUnsubscribe();
filterUnsubscribe.unsubscribe();
timefilerUnsubscribe.unsubscribe();
};
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ 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';
import { SavedSearch } from '@kbn/saved-search-plugin/public';

describe('updateSavedSearch', () => {
const query: Query = {
Expand Down Expand Up @@ -73,6 +74,56 @@ describe('updateSavedSearch', () => {
expect(savedSearch.searchSource.getField('filter')).toEqual([globalFilter, appFilter]);
});

it('should set time range is timeRestore is enabled', async () => {
const savedSearch: SavedSearch = {
...savedSearchMock,
searchSource: savedSearchMock.searchSource.createCopy(),
timeRestore: true,
};
(discoverServiceMock.timefilter.getTime as jest.Mock).mockReturnValue({
from: 'now-666m',
to: 'now',
});
updateSavedSearch({
savedSearch,
globalStateContainer: createGlobalStateContainer(),
services: discoverServiceMock,
state: {
query,
filters: [appFilter],
},
});
expect(savedSearch.timeRange).toEqual({
from: 'now-666m',
to: 'now',
});
});

it('should not set time range if timeRestore is not enabled', async () => {
const savedSearch: SavedSearch = {
...savedSearchMock,
searchSource: savedSearchMock.searchSource.createCopy(),
timeRestore: false,
};
(discoverServiceMock.timefilter.getTime as jest.Mock).mockReturnValue({
from: 'now-666m',
to: 'now',
});
updateSavedSearch({
savedSearch,
globalStateContainer: createGlobalStateContainer(),
services: discoverServiceMock,
state: {
query,
filters: [appFilter],
},
});
expect(savedSearch.timeRange).not.toEqual({
from: 'now-666m',
to: 'now',
});
});

it('should set query and filters from services', async () => {
const savedSearch = {
...savedSearchMock,
Expand Down