Skip to content

Commit

Permalink
[Discover] Fix navigation to a new from saved search and saved quer…
Browse files Browse the repository at this point in the history
…y, fix `discover:searchOnPageLoad` (#112262)

* [Discover] fix saved search become active

* [Discover] add another fix to be consistent with data fetching code

* [Discover] simplify solution

* [Discover] add functionals

* [Discover] fix saved query bug, add functionals

* [Discover] fix functionals

* [Discover] fix functional test

* [Discover] split saved query tests

* [Discover] preselect logstash index pattern

* [Discover] remove saved query after test complete

* [Discover] change query fill order

* [Discover] try to fix unrelated functional test

* [Discover] one more fix

* [Discover] try to fix one more problem

* [Discover] fix commonly used time range test

* [Discover] revert uisettings init in before statement, do small adjustments

* [Discover] fix unit test

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
dimaanj and kibanamachine authored Oct 5, 2021
1 parent 35e9f6a commit 158b396
Show file tree
Hide file tree
Showing 14 changed files with 248 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export const DiscoverUninitialized = ({ onRefresh }: Props) => {
</p>
}
actions={
<EuiButton color="primary" fill onClick={onRefresh}>
<EuiButton color="primary" fill onClick={onRefresh} data-test-subj="refreshDataButton">
<FormattedMessage
id="discover.uninitializedRefreshButtonText"
defaultMessage="Refresh data"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,14 @@ export interface DiscoverMainProps {
}

export function DiscoverMainApp(props: DiscoverMainProps) {
const { services, history, indexPatternList } = props;
const { savedSearch, services, history, indexPatternList } = props;
const { chrome, docLinks, uiSettings: config, data } = services;
const navigateTo = useCallback(
(path: string) => {
history.push(path);
},
[history]
);
const savedSearch = props.savedSearch;

/**
* State related logic
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,6 @@ export function DiscoverMainRoute({ services, history }: DiscoverMainProps) {

async function loadSavedSearch() {
try {
// force a refresh if a given saved search without id was saved
setSavedSearch(undefined);
const loadedSavedSearch = await services.getSavedSearchById(savedSearchId);
const loadedIndexPattern = await loadDefaultOrCurrentIndexPattern(loadedSavedSearch);
if (loadedSavedSearch && !loadedSavedSearch?.searchSource.getField('index')) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ export function useDiscoverState({

useEffect(() => {
const stopSync = stateContainer.initializeAndSync(indexPattern, filterManager, data);

return () => stopSync();
}, [stateContainer, filterManager, data, indexPattern]);

Expand Down Expand Up @@ -209,16 +210,13 @@ export function useDiscoverState({
}, [config, data, savedSearch, reset, stateContainer]);

/**
* Initial data fetching, also triggered when index pattern changes
* Trigger data fetching on indexPattern or savedSearch changes
*/
useEffect(() => {
if (!indexPattern) {
return;
}
if (initialFetchStatus === FetchStatus.LOADING) {
if (indexPattern) {
refetch$.next();
}
}, [initialFetchStatus, refetch$, indexPattern]);
}, [initialFetchStatus, refetch$, indexPattern, savedSearch.id]);

return {
data$,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ export const useSavedSearch = ({
refetch$,
searchSessionManager,
searchSource,
initialFetchStatus,
});

const subscription = fetch$.subscribe((val) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/
import { merge } from 'rxjs';
import { debounceTime, filter, tap } from 'rxjs/operators';
import { debounceTime, filter, skip, tap } from 'rxjs/operators';

import { FetchStatus } from '../../../types';
import type {
Expand All @@ -26,17 +26,19 @@ export function getFetch$({
main$,
refetch$,
searchSessionManager,
initialFetchStatus,
}: {
setAutoRefreshDone: (val: AutoRefreshDoneFn | undefined) => void;
data: DataPublicPluginStart;
main$: DataMain$;
refetch$: DataRefetch$;
searchSessionManager: DiscoverSearchSessionManager;
searchSource: SearchSource;
initialFetchStatus: FetchStatus;
}) {
const { timefilter } = data.query.timefilter;
const { filterManager } = data.query;
return merge(
let fetch$ = merge(
refetch$,
filterManager.getFetches$(),
timefilter.getFetch$(),
Expand All @@ -58,4 +60,13 @@ export function getFetch$({
data.query.queryString.getUpdates$(),
searchSessionManager.newSearchSessionIdFromURL$.pipe(filter((sessionId) => !!sessionId))
).pipe(debounceTime(100));

/**
* Skip initial fetch when discover:searchOnPageLoad is disabled.
*/
if (initialFetchStatus === FetchStatus.UNINITIALIZED) {
fetch$ = fetch$.pipe(skip(1));
}

return fetch$;
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ describe('getFetchObservable', () => {
data: createDataMock(new Subject(), new Subject(), new Subject(), new Subject()),
searchSessionManager: searchSessionManagerMock.searchSessionManager,
searchSource: savedSearchMock.searchSource,
initialFetchStatus: FetchStatus.LOADING,
});

fetch$.subscribe(() => {
Expand All @@ -81,6 +82,7 @@ describe('getFetchObservable', () => {
data: dataMock,
searchSessionManager: searchSessionManagerMock.searchSessionManager,
searchSource: savedSearchMockWithTimeField.searchSource,
initialFetchStatus: FetchStatus.LOADING,
});

const fetchfnMock = jest.fn();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ describe('getStateDefaults', () => {
"index": "index-pattern-with-timefield-id",
"interval": "auto",
"query": undefined,
"savedQuery": undefined,
"sort": Array [
Array [
"timestamp",
Expand Down Expand Up @@ -59,6 +60,7 @@ describe('getStateDefaults', () => {
"index": "the-index-pattern-id",
"interval": "auto",
"query": undefined,
"savedQuery": undefined,
"sort": Array [],
}
`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export function getStateDefaults({
interval: 'auto',
filters: cloneDeep(searchSource.getOwnField('filter')),
hideChart: undefined,
savedQuery: undefined,
} as AppState;
if (savedSearch.grid) {
defaultState.grid = savedSearch.grid;
Expand Down
28 changes: 4 additions & 24 deletions test/functional/apps/discover/_discover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -233,11 +233,11 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {

describe('time zone switch', () => {
it('should show bars in the correct time zone after switching', async function () {
await kibanaServer.uiSettings.replace({ 'dateFormat:tz': 'America/Phoenix' });
await kibanaServer.uiSettings.update({ 'dateFormat:tz': 'America/Phoenix' });
await PageObjects.common.navigateToApp('discover');
await PageObjects.header.awaitKibanaChrome();
await queryBar.clearQuery();
await PageObjects.timePicker.setDefaultAbsoluteRange();
await queryBar.clearQuery();

log.debug(
'check that the newest doc timestamp is now -7 hours from the UTC time in the first test'
Expand All @@ -246,36 +246,16 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
expect(rowData.startsWith('Sep 22, 2015 @ 16:50:13.253')).to.be.ok();
});
});
describe('usage of discover:searchOnPageLoad', () => {
it('should not fetch data from ES initially when discover:searchOnPageLoad is false', async function () {
await kibanaServer.uiSettings.replace({ 'discover:searchOnPageLoad': false });
await PageObjects.common.navigateToApp('discover');
await PageObjects.header.awaitKibanaChrome();

expect(await PageObjects.discover.getNrOfFetches()).to.be(0);
});

it('should fetch data from ES initially when discover:searchOnPageLoad is true', async function () {
await kibanaServer.uiSettings.replace({ 'discover:searchOnPageLoad': true });
await PageObjects.common.navigateToApp('discover');
await PageObjects.header.awaitKibanaChrome();
await retry.waitFor('number of fetches to be 1', async () => {
const nrOfFetches = await PageObjects.discover.getNrOfFetches();
return nrOfFetches === 1;
});
});
});

describe('invalid time range in URL', function () {
it('should get the default timerange', async function () {
const prevTime = await PageObjects.timePicker.getTimeConfig();
await PageObjects.common.navigateToUrl('discover', '#/?_g=(time:(from:now-15m,to:null))', {
useActualUrl: true,
});
await PageObjects.header.awaitKibanaChrome();
const time = await PageObjects.timePicker.getTimeConfig();
expect(time.start).to.be(prevTime.start);
expect(time.end).to.be(prevTime.end);
expect(time.start).to.be('~ 15 minutes ago');
expect(time.end).to.be('now');
});
});

Expand Down
74 changes: 59 additions & 15 deletions test/functional/apps/discover/_saved_queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,39 +17,83 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
const kibanaServer = getService('kibanaServer');
const PageObjects = getPageObjects(['common', 'discover', 'timePicker']);
const browser = getService('browser');

const defaultSettings = {
defaultIndex: 'logstash-*',
};
const filterBar = getService('filterBar');
const queryBar = getService('queryBar');
const savedQueryManagementComponent = getService('savedQueryManagementComponent');
const testSubjects = getService('testSubjects');
const defaultSettings = {
defaultIndex: 'logstash-*',
};

const setUpQueriesWithFilters = async () => {
// set up a query with filters and a time filter
log.debug('set up a query with filters to save');
const fromTime = 'Sep 20, 2015 @ 08:00:00.000';
const toTime = 'Sep 21, 2015 @ 08:00:00.000';
await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime);
await filterBar.addFilter('extension.raw', 'is one of', 'jpg');
await queryBar.setQuery('response:200');
};

describe('saved queries saved objects', function describeIndexTests() {
before(async function () {
log.debug('load kibana index with default index pattern');
await kibanaServer.savedObjects.clean({ types: ['search', 'index-pattern'] });

await kibanaServer.importExport.load('test/functional/fixtures/kbn_archiver/discover.json');
await esArchiver.load('test/functional/fixtures/es_archiver/date_nested');
await esArchiver.load('test/functional/fixtures/es_archiver/logstash_functional');

// and load a set of makelogs data
await esArchiver.loadIfNeeded('test/functional/fixtures/es_archiver/logstash_functional');
await kibanaServer.uiSettings.replace(defaultSettings);
log.debug('discover');
await PageObjects.common.navigateToApp('discover');
await PageObjects.timePicker.setDefaultAbsoluteRange();
});

describe('saved query management component functionality', function () {
before(async function () {
// set up a query with filters and a time filter
log.debug('set up a query with filters to save');
await queryBar.setQuery('response:200');
await filterBar.addFilter('extension.raw', 'is one of', 'jpg');
const fromTime = 'Sep 20, 2015 @ 08:00:00.000';
const toTime = 'Sep 21, 2015 @ 08:00:00.000';
await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime);
after(async () => {
await kibanaServer.importExport.unload('test/functional/fixtures/kbn_archiver/discover');
await esArchiver.unload('test/functional/fixtures/es_archiver/date_nested');
await esArchiver.unload('test/functional/fixtures/es_archiver/logstash_functional');
});

describe('saved query selection', () => {
before(async () => await setUpQueriesWithFilters());

it(`should unselect saved query when navigating to a 'new'`, async function () {
await savedQueryManagementComponent.saveNewQuery(
'test-unselect-saved-query',
'mock',
true,
true
);

await queryBar.submitQuery();

expect(await filterBar.hasFilter('extension.raw', 'jpg')).to.be(true);
expect(await queryBar.getQueryString()).to.eql('response:200');

await PageObjects.discover.clickNewSearchButton();

expect(await filterBar.hasFilter('extension.raw', 'jpg')).to.be(false);
expect(await queryBar.getQueryString()).to.eql('');

await PageObjects.discover.selectIndexPattern('date-nested');

expect(await filterBar.hasFilter('extension.raw', 'jpg')).to.be(false);
expect(await queryBar.getQueryString()).to.eql('');

await PageObjects.discover.selectIndexPattern('logstash-*');

expect(await filterBar.hasFilter('extension.raw', 'jpg')).to.be(false);
expect(await queryBar.getQueryString()).to.eql('');

// reset state
await savedQueryManagementComponent.deleteSavedQuery('test-unselect-saved-query');
});
});

describe('saved query management component functionality', function () {
before(async () => await setUpQueriesWithFilters());

it('should show the saved query management component when there are no saved queries', async () => {
await savedQueryManagementComponent.openSavedQueryManagementComponent();
Expand Down
Loading

0 comments on commit 158b396

Please sign in to comment.