Skip to content

Commit

Permalink
[Search Sessions][Discover] Send to background integration improvemen…
Browse files Browse the repository at this point in the history
…ts and fixes (#87311)
  • Loading branch information
Dosant authored Jan 12, 2021
1 parent 78dea4f commit 86d53b6
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ function discoverController($element, $route, $scope, $timeout, Promise, uiCapab
createSearchSessionRestorationDataProvider({
appStateContainer,
data,
getSavedSearchId: () => savedSearch.id,
getSavedSearch: () => savedSearch,
})
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,14 @@
* under the License.
*/

import { getState, GetStateReturn } from './discover_state';
import {
getState,
GetStateReturn,
createSearchSessionRestorationDataProvider,
} from './discover_state';
import { createBrowserHistory, History } from 'history';
import { dataPluginMock } from '../../../../data/public/mocks';
import { SavedSearch } from '../../saved_searches';

let history: History;
let state: GetStateReturn;
Expand Down Expand Up @@ -103,3 +109,30 @@ describe('Test discover state with legacy migration', () => {
`);
});
});

describe('createSearchSessionRestorationDataProvider', () => {
let mockSavedSearch: SavedSearch = ({} as unknown) as SavedSearch;
const searchSessionInfoProvider = createSearchSessionRestorationDataProvider({
data: dataPluginMock.createStartContract(),
appStateContainer: getState({
history: createBrowserHistory(),
}).appStateContainer,
getSavedSearch: () => mockSavedSearch,
});

describe('session name', () => {
test('No saved search returns default name', async () => {
expect(await searchSessionInfoProvider.getName()).toBe('Discover');
});

test('Saved Search with a title returns saved search title', async () => {
mockSavedSearch = ({ id: 'id', title: 'Name' } as unknown) as SavedSearch;
expect(await searchSessionInfoProvider.getName()).toBe('Name');
});

test('Saved Search without a title returns default name', async () => {
mockSavedSearch = ({ id: 'id', title: undefined } as unknown) as SavedSearch;
expect(await searchSessionInfoProvider.getName()).toBe('Discover');
});
});
});
27 changes: 23 additions & 4 deletions src/plugins/discover/public/application/angular/discover_state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* under the License.
*/
import { isEqual } from 'lodash';
import { i18n } from '@kbn/i18n';
import { History } from 'history';
import { NotificationsStart } from 'kibana/public';
import {
Expand All @@ -38,6 +39,7 @@ import {
import { migrateLegacyQuery } from '../helpers/migrate_legacy_query';
import { DiscoverGridSettings } from '../components/discover_grid/types';
import { DISCOVER_APP_URL_GENERATOR, DiscoverUrlGeneratorState } from '../../url_generator';
import { SavedSearch } from '../../saved_searches';

export interface AppState {
/**
Expand Down Expand Up @@ -264,15 +266,32 @@ export function isEqualState(stateA: AppState, stateB: AppState) {
export function createSearchSessionRestorationDataProvider(deps: {
appStateContainer: StateContainer<AppState>;
data: DataPublicPluginStart;
getSavedSearchId: () => string | undefined;
getSavedSearch: () => SavedSearch;
}): SearchSessionInfoProvider {
const getSavedSearchId = () => deps.getSavedSearch().id;
return {
getName: async () => 'Discover',
getName: async () => {
const savedSearch = deps.getSavedSearch();
return (
(savedSearch.id && savedSearch.title) ||
i18n.translate('discover.discoverDefaultSearchSessionName', {
defaultMessage: 'Discover',
})
);
},
getUrlGeneratorData: async () => {
return {
urlGeneratorId: DISCOVER_APP_URL_GENERATOR,
initialState: createUrlGeneratorState({ ...deps, forceAbsoluteTime: false }),
restoreState: createUrlGeneratorState({ ...deps, forceAbsoluteTime: true }),
initialState: createUrlGeneratorState({
...deps,
getSavedSearchId,
forceAbsoluteTime: false,
}),
restoreState: createUrlGeneratorState({
...deps,
getSavedSearchId,
forceAbsoluteTime: true,
}),
};
},
};
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/discover/public/url_generator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ describe('Discover url generator', () => {
const url = await generator.createUrl({ savedSearchId });
const { _a, _g } = getStatesFromKbnUrl(url, ['_a', '_g']);

expect(url.startsWith(`${appBasePath}#/${savedSearchId}`)).toBe(true);
expect(url.startsWith(`${appBasePath}#/view/${savedSearchId}`)).toBe(true);
expect(_a).toEqual({});
expect(_g).toEqual({});
});
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/discover/public/url_generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ export class DiscoverUrlGenerator
sort,
interval,
}: DiscoverUrlGeneratorState): Promise<string> => {
const savedSearchPath = savedSearchId ? encodeURIComponent(savedSearchId) : '';
const savedSearchPath = savedSearchId ? `view/${encodeURIComponent(savedSearchId)}` : '';
const appState: {
query?: Query;
filters?: Filter[];
Expand Down
1 change: 0 additions & 1 deletion x-pack/test/functional/apps/discover/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,5 @@ export default function ({ loadTestFile }: FtrProviderContext) {
loadTestFile(require.resolve('./error_handling'));
loadTestFile(require.resolve('./visualize_field'));
loadTestFile(require.resolve('./value_suggestions'));
loadTestFile(require.resolve('./async_search'));
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/
import expect from '@kbn/expect';
import { FtrProviderContext } from '../../ftr_provider_context';
import { FtrProviderContext } from '../../../ftr_provider_context';

export default function ({ getPageObjects, getService }: FtrProviderContext) {
const esArchiver = getService('esArchiver');
Expand All @@ -31,14 +31,13 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
expect(searchSessionId2).not.to.be(searchSessionId1);
});

// NOTE: this test will be revised when
// `searchSessionId` functionality actually works
it('search session id should be picked up from the URL', async () => {
it('search session id should be picked up from the URL, non existing session id errors out', async () => {
const url = await browser.getCurrentUrl();
const fakeSearchSessionId = '__test__';
const savedSessionURL = url + `&searchSessionId=${fakeSearchSessionId}`;
await browser.navigateTo(savedSessionURL);
await PageObjects.header.waitUntilLoadingHasFinished();
await testSubjects.existOrFail('discoverNoResultsError'); // expect error because of fake searchSessionId
const searchSessionId1 = await getSearchSessionId();
expect(searchSessionId1).to.be(fakeSearchSessionId);
await queryBar.clickQuerySubmitButton();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export default function ({ loadTestFile, getService }: FtrProviderContext) {
await kibanaServer.uiSettings.replace({ defaultIndex: 'logstash-*' });
});

loadTestFile(require.resolve('./async_search'));
loadTestFile(require.resolve('./sessions_in_space'));
});
}

0 comments on commit 86d53b6

Please sign in to comment.