Skip to content

Commit

Permalink
[Discover] Show "Unsaved changes" badge on time filter changes in cas…
Browse files Browse the repository at this point in the history
…e time range is stored along with saved search (elastic#178659)

Closes elastic#172288

There's two parts to this:
* When updating the saved search, update the time range as well using
the current state of the services.timefilter instance
* Similar to filter updates, run an update action on the state container
if the time changes

---------

Co-authored-by: Julia Rechkunova <julia.rechkunova@gmail.com>
  • Loading branch information
flash1293 and jughosta authored Mar 20, 2024
1 parent 17f46eb commit 5012949
Show file tree
Hide file tree
Showing 5 changed files with 154 additions and 1 deletion.
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'],
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

0 comments on commit 5012949

Please sign in to comment.