From fbb661ee8490c0a9b66914dd4f7136159840a0da Mon Sep 17 00:00:00 2001 From: Tim Sullivan Date: Tue, 21 Sep 2021 18:46:44 -0700 Subject: [PATCH] [7.14] [Reporting/Discover/Search Sessions] Only use relative time filter when generating share data (#112588) (#112740) * [Reporting/Discover/Search Sessions] Only use relative time filter when generating share data (#112588) * only use relative time filter when generating share data * Added comment on absolute time filter. Co-authored-by: Tim Sullivan * improve discover search session relative time range test * update discover tests and types for passing in data plugin to getSharingData function * updated reporting to pass in data plugin to getSharingData, also updates jest tests Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Tim Sullivan Co-authored-by: Anton Dosov * fix bad merge * fixes * fix Co-authored-by: Jean-Louis Leysens Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Anton Dosov --- .../components/top_nav/get_top_nav_links.ts | 2 +- .../apps/main/utils/get_sharing_data.test.ts | 37 ++++++----- .../apps/main/utils/get_sharing_data.ts | 11 +++- .../apps/main/utils/update_search_source.ts | 6 +- .../get_csv_panel_action.test.ts | 62 +++++++++++-------- .../panel_actions/get_csv_panel_action.tsx | 14 +++-- x-pack/plugins/reporting/public/plugin.ts | 11 +++- .../search_sessions_management_page.ts | 1 + .../tests/apps/discover/async_search.ts | 13 +++- 9 files changed, 97 insertions(+), 60 deletions(-) diff --git a/src/plugins/discover/public/application/apps/main/components/top_nav/get_top_nav_links.ts b/src/plugins/discover/public/application/apps/main/components/top_nav/get_top_nav_links.ts index d57421aa0b7cf..639f50e1511c7 100644 --- a/src/plugins/discover/public/application/apps/main/components/top_nav/get_top_nav_links.ts +++ b/src/plugins/discover/public/application/apps/main/components/top_nav/get_top_nav_links.ts @@ -112,7 +112,7 @@ export const getTopNavLinks = ({ const sharingData = await getSharingData( searchSource, state.appStateContainer.getState(), - services.uiSettings + services ); services.share.toggleShareContextMenu({ diff --git a/src/plugins/discover/public/application/apps/main/utils/get_sharing_data.test.ts b/src/plugins/discover/public/application/apps/main/utils/get_sharing_data.test.ts index ffad3d955c1a3..8da6c05fc30eb 100644 --- a/src/plugins/discover/public/application/apps/main/utils/get_sharing_data.test.ts +++ b/src/plugins/discover/public/application/apps/main/utils/get_sharing_data.test.ts @@ -7,32 +7,37 @@ */ import { Capabilities, IUiSettingsClient } from 'kibana/public'; -import { IndexPattern } from 'src/plugins/data/public'; +import type { IndexPattern } from 'src/plugins/data/public'; +import type { DiscoverServices } from '../../../../build_services'; +import { dataPluginMock } from '../../../../../../data/public/mocks'; import { createSearchSourceMock } from '../../../../../../data/common/search/search_source/mocks'; import { DOC_HIDE_TIME_COLUMN_SETTING, SORT_DEFAULT_ORDER_SETTING } from '../../../../../common'; import { indexPatternMock } from '../../../../__mocks__/index_pattern'; import { getSharingData, showPublicUrlSwitch } from './get_sharing_data'; describe('getSharingData', () => { - let mockConfig: IUiSettingsClient; + let services: DiscoverServices; beforeEach(() => { - mockConfig = ({ - get: (key: string) => { - if (key === SORT_DEFAULT_ORDER_SETTING) { - return 'desc'; - } - if (key === DOC_HIDE_TIME_COLUMN_SETTING) { + services = { + data: dataPluginMock.createStartContract(), + uiSettings: { + get: (key: string) => { + if (key === SORT_DEFAULT_ORDER_SETTING) { + return 'desc'; + } + if (key === DOC_HIDE_TIME_COLUMN_SETTING) { + return false; + } return false; - } - return false; + }, }, - } as unknown) as IUiSettingsClient; + } as DiscoverServices; }); test('returns valid data for sharing', async () => { const searchSourceMock = createSearchSourceMock({ index: indexPatternMock }); - const result = await getSharingData(searchSourceMock, { columns: [] }, mockConfig); + const result = await getSharingData(searchSourceMock, { columns: [] }, services); expect(result).toMatchInlineSnapshot(` Object { "columns": Array [], @@ -53,7 +58,7 @@ describe('getSharingData', () => { const result = await getSharingData( searchSourceMock, { columns: ['column_a', 'column_b'] }, - mockConfig + services ); expect(result).toMatchInlineSnapshot(` Object { @@ -90,7 +95,7 @@ describe('getSharingData', () => { 'cool-field-6', ], }, - mockConfig + services ); expect(result).toMatchInlineSnapshot(` Object { @@ -116,7 +121,7 @@ describe('getSharingData', () => { }); test('fields conditionally do not have prepended timeField', async () => { - mockConfig = ({ + services.uiSettings = ({ get: (key: string) => { if (key === DOC_HIDE_TIME_COLUMN_SETTING) { return true; @@ -141,7 +146,7 @@ describe('getSharingData', () => { 'cool-field-6', ], }, - mockConfig + services ); expect(result).toMatchInlineSnapshot(` Object { diff --git a/src/plugins/discover/public/application/apps/main/utils/get_sharing_data.ts b/src/plugins/discover/public/application/apps/main/utils/get_sharing_data.ts index 00473956c57e3..38002fa0287f6 100644 --- a/src/plugins/discover/public/application/apps/main/utils/get_sharing_data.ts +++ b/src/plugins/discover/public/application/apps/main/utils/get_sharing_data.ts @@ -6,8 +6,10 @@ * Side Public License, v 1. */ -import type { Capabilities, IUiSettingsClient } from 'kibana/public'; -import { ISearchSource } from '../../../../../../data/common'; +import type { Capabilities } from 'kibana/public'; +import type { IUiSettingsClient } from 'src/core/public'; +import type { DataPublicPluginStart } from 'src/plugins/data/public'; +import type { ISearchSource } from 'src/plugins/data/common'; import { DOC_HIDE_TIME_COLUMN_SETTING, SORT_DEFAULT_ORDER_SETTING } from '../../../../../common'; import type { SavedSearch, SortOrder } from '../../../../saved_searches/types'; import { AppState } from '../services/discover_state'; @@ -19,8 +21,9 @@ import { getSortForSearchSource } from '../../../angular/doc_table'; export async function getSharingData( currentSearchSource: ISearchSource, state: AppState | SavedSearch, - config: IUiSettingsClient + services: { uiSettings: IUiSettingsClient; data: DataPublicPluginStart } ) { + const { uiSettings: config, data } = services; const searchSource = currentSearchSource.createCopy(); const index = searchSource.getField('index')!; @@ -28,6 +31,8 @@ export async function getSharingData( 'sort', getSortForSearchSource(state.sort as SortOrder[], index, config.get(SORT_DEFAULT_ORDER_SETTING)) ); + // When sharing externally we preserve relative time values + searchSource.setField('filter', data.query.timefilter.timefilter.createRelativeFilter(index)); searchSource.removeField('highlight'); searchSource.removeField('highlightAll'); searchSource.removeField('aggs'); diff --git a/src/plugins/discover/public/application/apps/main/utils/update_search_source.ts b/src/plugins/discover/public/application/apps/main/utils/update_search_source.ts index 9b99341d77cfb..50007c31ad5ee 100644 --- a/src/plugins/discover/public/application/apps/main/utils/update_search_source.ts +++ b/src/plugins/discover/public/application/apps/main/utils/update_search_source.ts @@ -57,10 +57,8 @@ export function updateSearchSource( // this is not the default index pattern, it determines that it's not of type rollup if (indexPatternsUtils.isDefault(indexPattern)) { - searchSource.setField( - 'filter', - data.query.timefilter.timefilter.createRelativeFilter(indexPattern) - ); + // Set the date range filter fields from timeFilter using the absolute format. Search sessions requires that it be converted from a relative range + searchSource.setField('filter', data.query.timefilter.timefilter.createFilter(indexPattern)); } if (useNewFieldsApi) { diff --git a/x-pack/plugins/reporting/public/panel_actions/get_csv_panel_action.test.ts b/x-pack/plugins/reporting/public/panel_actions/get_csv_panel_action.test.ts index dbd0421fdf9b0..0f527a5e99b50 100644 --- a/x-pack/plugins/reporting/public/panel_actions/get_csv_panel_action.test.ts +++ b/x-pack/plugins/reporting/public/panel_actions/get_csv_panel_action.test.ts @@ -8,7 +8,9 @@ import * as Rx from 'rxjs'; import { first } from 'rxjs/operators'; import { CoreStart } from 'src/core/public'; +import { dataPluginMock } from '../../../../../src/plugins/data/public/mocks'; import { LicensingPluginSetup } from '../../../licensing/public'; +import type { ReportingPublicPluginStartDendencies } from '../plugin'; import { ReportingCsvPanelAction } from './get_csv_panel_action'; type LicenseResults = 'valid' | 'invalid' | 'unavailable' | 'expired'; @@ -18,8 +20,8 @@ describe('GetCsvReportPanelAction', () => { let context: any; let mockLicense$: any; let mockSearchSource: any; - let mockStartServicesPayload: [CoreStart, object, unknown]; - let mockStartServices$: Rx.Subject; + let mockStartServicesPayload: [CoreStart, ReportingPublicPluginStartDendencies, unknown]; + let mockStartServices$: Rx.Observable; beforeAll(() => { if (typeof window.URL.revokeObjectURL === 'undefined') { @@ -38,15 +40,6 @@ describe('GetCsvReportPanelAction', () => { }) as unknown) as LicensingPluginSetup['license$']; }; - mockStartServices$ = new Rx.Subject<[CoreStart, object, unknown]>(); - mockStartServicesPayload = [ - ({ - application: { capabilities: { dashboard: { downloadCsv: true } } }, - } as unknown) as CoreStart, - {}, - null, - ]; - core = { http: { post: jest.fn().mockImplementation(() => Promise.resolve(true)), @@ -62,6 +55,18 @@ describe('GetCsvReportPanelAction', () => { }, } as any; + mockStartServicesPayload = [ + ({ + ...core, + application: { capabilities: { dashboard: { downloadCsv: true } } }, + } as unknown) as CoreStart, + { + data: dataPluginMock.createStartContract(), + } as ReportingPublicPluginStartDendencies, + null, + ]; + mockStartServices$ = Rx.from(Promise.resolve(mockStartServicesPayload)); + mockSearchSource = { createCopy: () => mockSearchSource, removeField: jest.fn(), @@ -97,7 +102,7 @@ describe('GetCsvReportPanelAction', () => { usesUiCapabilities: true, }); - mockStartServices$.next(mockStartServicesPayload); + await mockStartServices$.pipe(first()).toPromise(); await panel.execute(context); @@ -131,7 +136,7 @@ describe('GetCsvReportPanelAction', () => { usesUiCapabilities: true, }); - mockStartServices$.next(mockStartServicesPayload); + await mockStartServices$.pipe(first()).toPromise(); await panel.execute(context); @@ -152,7 +157,7 @@ describe('GetCsvReportPanelAction', () => { usesUiCapabilities: true, }); - mockStartServices$.next(mockStartServicesPayload); + await mockStartServices$.pipe(first()).toPromise(); await panel.execute(context); @@ -167,7 +172,7 @@ describe('GetCsvReportPanelAction', () => { usesUiCapabilities: true, }); - mockStartServices$.next(mockStartServicesPayload); + await mockStartServices$.pipe(first()).toPromise(); await panel.execute(context); @@ -189,7 +194,7 @@ describe('GetCsvReportPanelAction', () => { usesUiCapabilities: true, }); - mockStartServices$.next(mockStartServicesPayload); + await mockStartServices$.pipe(first()).toPromise(); await panel.execute(context); @@ -205,14 +210,14 @@ describe('GetCsvReportPanelAction', () => { usesUiCapabilities: true, }); - mockStartServices$.next(mockStartServicesPayload); + await mockStartServices$.pipe(first()).toPromise(); await licenseMock$.pipe(first()).toPromise(); expect(await plugin.isCompatible(context)).toEqual(false); }); - it('sets a display and icon type', () => { + it('sets a display and icon type', async () => { const panel = new ReportingCsvPanelAction({ core, license$: mockLicense$(), @@ -220,7 +225,7 @@ describe('GetCsvReportPanelAction', () => { usesUiCapabilities: true, }); - mockStartServices$.next(mockStartServicesPayload); + await mockStartServices$.pipe(first()).toPromise(); expect(panel.getIconType()).toMatchInlineSnapshot(`"document"`); expect(panel.getDisplayName()).toMatchInlineSnapshot(`"Download CSV"`); @@ -228,24 +233,27 @@ describe('GetCsvReportPanelAction', () => { describe('Application UI Capabilities', () => { it(`doesn't allow downloads when UI capability is not enabled`, async () => { + mockStartServicesPayload = [ + ({ application: { capabilities: {} } } as unknown) as CoreStart, + { + data: dataPluginMock.createStartContract(), + } as ReportingPublicPluginStartDendencies, + null, + ]; + const startServices$ = Rx.from(Promise.resolve(mockStartServicesPayload)); const plugin = new ReportingCsvPanelAction({ core, license$: mockLicense$(), - startServices$: mockStartServices$, + startServices$, usesUiCapabilities: true, }); - mockStartServices$.next([ - ({ application: { capabilities: {} } } as unknown) as CoreStart, - {}, - null, - ]); + await startServices$.pipe(first()).toPromise(); expect(await plugin.isCompatible(context)).toEqual(false); }); it(`allows downloads when license is valid and UI capability is enabled`, async () => { - mockStartServices$ = new Rx.Subject(); const plugin = new ReportingCsvPanelAction({ core, license$: mockLicense$(), @@ -253,7 +261,7 @@ describe('GetCsvReportPanelAction', () => { usesUiCapabilities: true, }); - mockStartServices$.next(mockStartServicesPayload); + await mockStartServices$.pipe(first()).toPromise(); expect(await plugin.isCompatible(context)).toEqual(true); }); diff --git a/x-pack/plugins/reporting/public/panel_actions/get_csv_panel_action.tsx b/x-pack/plugins/reporting/public/panel_actions/get_csv_panel_action.tsx index 8a863e1ceaa65..2f8efcb7d4d3c 100644 --- a/x-pack/plugins/reporting/public/panel_actions/get_csv_panel_action.tsx +++ b/x-pack/plugins/reporting/public/panel_actions/get_csv_panel_action.tsx @@ -8,6 +8,7 @@ import { i18n } from '@kbn/i18n'; import moment from 'moment-timezone'; import * as Rx from 'rxjs'; +import { first } from 'rxjs/operators'; import type { CoreSetup } from 'src/core/public'; import { CoreStart } from 'src/core/public'; import type { ISearchEmbeddable, SavedSearch } from '../../../../../src/plugins/discover/public'; @@ -23,6 +24,7 @@ import type { LicensingPluginSetup } from '../../../licensing/public'; import { API_GENERATE_IMMEDIATE, CSV_REPORTING_ACTION } from '../../common/constants'; import type { JobParamsDownloadCSV } from '../../server/export_types/csv_searchsource_immediate/types'; import { checkLicense } from '../lib/license_check'; +import type { ReportingPublicPluginStartDendencies } from '../plugin'; function isSavedSearchEmbeddable( embeddable: IEmbeddable | ISearchEmbeddable @@ -36,7 +38,7 @@ interface ActionContext { interface Params { core: CoreSetup; - startServices$: Rx.Observable<[CoreStart, object, unknown]>; + startServices$: Rx.Observable<[CoreStart, ReportingPublicPluginStartDendencies, unknown]>; license$: LicensingPluginSetup['license$']; usesUiCapabilities: boolean; } @@ -48,11 +50,14 @@ export class ReportingCsvPanelAction implements ActionDefinition private licenseHasDownloadCsv: boolean = false; private capabilityHasDownloadCsv: boolean = false; private core: CoreSetup; + private startServices$: Params['startServices$']; constructor({ core, startServices$, license$, usesUiCapabilities }: Params) { this.isDownloading = false; this.core = core; + this.startServices$ = startServices$; + license$.subscribe((license) => { const results = license.check('reporting', 'basic'); const { showLinks } = checkLicense(results); @@ -60,7 +65,7 @@ export class ReportingCsvPanelAction implements ActionDefinition }); if (usesUiCapabilities) { - startServices$.subscribe(([{ application }]) => { + this.startServices$.subscribe(([{ application }]) => { this.capabilityHasDownloadCsv = application.capabilities.dashboard?.downloadCsv === true; }); } else { @@ -79,11 +84,12 @@ export class ReportingCsvPanelAction implements ActionDefinition } public async getSearchSource(savedSearch: SavedSearch, embeddable: ISearchEmbeddable) { + const [{ uiSettings }, { data }] = await this.startServices$.pipe(first()).toPromise(); const { getSharingData } = await loadSharingDataHelpers(); return await getSharingData( savedSearch.searchSource, - savedSearch, // TODO: get unsaved state (using embeddale.searchScope): https://github.com/elastic/kibana/issues/43977 - this.core.uiSettings + savedSearch, // TODO: get unsaved state (using embeddable.searchScope): https://github.com/elastic/kibana/issues/43977 + { uiSettings, data } ); } diff --git a/x-pack/plugins/reporting/public/plugin.ts b/x-pack/plugins/reporting/public/plugin.ts index c5b40369824db..21cbcf0497fa3 100644 --- a/x-pack/plugins/reporting/public/plugin.ts +++ b/x-pack/plugins/reporting/public/plugin.ts @@ -8,6 +8,7 @@ import { i18n } from '@kbn/i18n'; import * as Rx from 'rxjs'; import { catchError, filter, map, mergeMap, takeUntil } from 'rxjs/operators'; +import type { DataPublicPluginStart } from 'src/plugins/data/public'; import { CoreSetup, CoreStart, @@ -70,6 +71,7 @@ export interface ReportingPublicPluginSetupDendencies { export interface ReportingPublicPluginStartDendencies { home: HomePublicPluginStart; + data: DataPublicPluginStart; management: ManagementStart; licensing: LicensingPluginStart; uiActions: UiActionsStart; @@ -114,8 +116,11 @@ export class ReportingPublicPlugin return this.contract; } - public setup(core: CoreSetup, setupDeps: ReportingPublicPluginSetupDendencies) { - const { http, getStartServices, uiSettings } = core; + public setup( + core: CoreSetup, + setupDeps: ReportingPublicPluginSetupDendencies + ) { + const { getStartServices, uiSettings } = core; const { home, management, @@ -127,7 +132,7 @@ export class ReportingPublicPlugin const startServices$ = Rx.from(getStartServices()); const usesUiCapabilities = !this.config.roles.enabled; - const apiClient = new ReportingAPIClient(http); + const apiClient = new ReportingAPIClient(core.http); home.featureCatalogue.register({ id: 'reporting', diff --git a/x-pack/test/functional/page_objects/search_sessions_management_page.ts b/x-pack/test/functional/page_objects/search_sessions_management_page.ts index 86391b568fdf2..15c87ea450425 100644 --- a/x-pack/test/functional/page_objects/search_sessions_management_page.ts +++ b/x-pack/test/functional/page_objects/search_sessions_management_page.ts @@ -38,6 +38,7 @@ export function SearchSessionsPageProvider({ getService, getPageObjects }: FtrPr mainUrl: $.findTestSubject('sessionManagementNameCol').text(), created: $.findTestSubject('sessionManagementCreatedCol').text(), expires: $.findTestSubject('sessionManagementExpiresCol').text(), + searchesCount: Number($.findTestSubject('sessionManagementNumSearchesCol').text()), app: $.findTestSubject('sessionManagementAppIcon').attr('data-test-app-id'), view: async () => { log.debug('management ui: view the session'); diff --git a/x-pack/test/search_sessions_integration/tests/apps/discover/async_search.ts b/x-pack/test/search_sessions_integration/tests/apps/discover/async_search.ts index 93dca78b34a82..1b4690f40bc54 100644 --- a/x-pack/test/search_sessions_integration/tests/apps/discover/async_search.ts +++ b/x-pack/test/search_sessions_integration/tests/apps/discover/async_search.ts @@ -25,6 +25,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { ]); const searchSessions = getService('searchSessions'); const retry = getService('retry'); + const toasts = getService('toasts'); describe('discover async search', () => { before(async () => { @@ -104,12 +105,20 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { // load URL to restore a saved session await PageObjects.searchSessionsManagement.goTo(); - const searchSessionList = await PageObjects.searchSessionsManagement.getList(); + const searchSessionListBeforeRestore = await PageObjects.searchSessionsManagement.getList(); + const searchesCountBeforeRestore = searchSessionListBeforeRestore[0].searchesCount; // navigate to Discover - await searchSessionList[0].view(); + await searchSessionListBeforeRestore[0].view(); await PageObjects.header.waitUntilLoadingHasFinished(); await searchSessions.expectState('restored'); expect(await PageObjects.discover.hasNoResults()).to.be(true); + expect(await toasts.getToastCount()).to.be(0); // no session restoration related warnings + + await PageObjects.searchSessionsManagement.goTo(); + const searchSessionListAfterRestore = await PageObjects.searchSessionsManagement.getList(); + const searchesCountAfterRestore = searchSessionListAfterRestore[0].searchesCount; + + expect(searchesCountBeforeRestore).to.be(searchesCountAfterRestore); // no new searches started during restore }); });