From 1f6a808598066a70e1d131aba682894b4e064f26 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Tue, 12 Oct 2021 15:49:17 +0200 Subject: [PATCH] [Reporting] Fix missing force now behaviour for v2 reports (#114516) * fix missing force now behaviour for v2 reports * added jest test * updated jest test snapshot to match removal of forceNow injection from locator params Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../server/export_types/common/index.ts | 1 - .../export_types/common/set_force_now.ts | 24 ---------- .../v2/get_full_redirect_app_url.test.ts | 45 +++++++++++++++++++ .../common/v2/get_full_redirect_app_url.ts | 7 ++- .../export_types/png_v2/execute_job.test.ts | 6 +-- .../server/export_types/png_v2/execute_job.ts | 5 +-- .../printable_pdf_v2/execute_job.ts | 3 +- .../printable_pdf_v2/lib/generate_pdf.ts | 5 ++- 8 files changed, 60 insertions(+), 36 deletions(-) delete mode 100644 x-pack/plugins/reporting/server/export_types/common/set_force_now.ts create mode 100644 x-pack/plugins/reporting/server/export_types/common/v2/get_full_redirect_app_url.test.ts diff --git a/x-pack/plugins/reporting/server/export_types/common/index.ts b/x-pack/plugins/reporting/server/export_types/common/index.ts index 09d3236fa7b54..c35dcb5344e21 100644 --- a/x-pack/plugins/reporting/server/export_types/common/index.ts +++ b/x-pack/plugins/reporting/server/export_types/common/index.ts @@ -12,7 +12,6 @@ export { omitBlockedHeaders } from './omit_blocked_headers'; export { validateUrls } from './validate_urls'; export { generatePngObservableFactory } from './generate_png'; export { getCustomLogo } from './get_custom_logo'; -export { setForceNow } from './set_force_now'; export interface TimeRangeParams { min?: Date | string | number | null; diff --git a/x-pack/plugins/reporting/server/export_types/common/set_force_now.ts b/x-pack/plugins/reporting/server/export_types/common/set_force_now.ts deleted file mode 100644 index b4f4b1b0ace05..0000000000000 --- a/x-pack/plugins/reporting/server/export_types/common/set_force_now.ts +++ /dev/null @@ -1,24 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import type { LocatorParams } from '../../../common/types'; - -/** - * Add `forceNow` to {@link LocatorParams['params']} to enable clients to set the time appropriately when - * reporting navigates to the page in Chromium. - */ -export const setForceNow = - (forceNow: string) => - (locator: LocatorParams): LocatorParams => { - return { - ...locator, - params: { - ...locator.params, - forceNow, - }, - }; - }; diff --git a/x-pack/plugins/reporting/server/export_types/common/v2/get_full_redirect_app_url.test.ts b/x-pack/plugins/reporting/server/export_types/common/v2/get_full_redirect_app_url.test.ts new file mode 100644 index 0000000000000..7a2ec5b83e7f4 --- /dev/null +++ b/x-pack/plugins/reporting/server/export_types/common/v2/get_full_redirect_app_url.test.ts @@ -0,0 +1,45 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { get } from 'lodash'; +import { ReportingConfig } from '../../../config/config'; +import { getFullRedirectAppUrl } from './get_full_redirect_app_url'; + +describe('getFullRedirectAppUrl', () => { + let config: ReportingConfig; + + beforeEach(() => { + const values = { + server: { + basePath: 'test', + }, + kibanaServer: { + protocol: 'http', + hostname: 'localhost', + port: '1234', + }, + }; + config = { + get: jest.fn((...args: string[]) => get(values, args)), + kbnConfig: { + get: jest.fn((...args: string[]) => get(values, args)), + }, + }; + }); + + test('smoke test', () => { + expect(getFullRedirectAppUrl(config, 'test', undefined)).toBe( + 'http://localhost:1234/test/s/test/app/management/insightsAndAlerting/reporting/r' + ); + }); + + test('adding forceNow', () => { + expect(getFullRedirectAppUrl(config, 'test', 'TEST with a space')).toBe( + 'http://localhost:1234/test/s/test/app/management/insightsAndAlerting/reporting/r?forceNow=TEST%20with%20a%20space' + ); + }); +}); diff --git a/x-pack/plugins/reporting/server/export_types/common/v2/get_full_redirect_app_url.ts b/x-pack/plugins/reporting/server/export_types/common/v2/get_full_redirect_app_url.ts index bb640eff667e9..9c329db64fa1a 100644 --- a/x-pack/plugins/reporting/server/export_types/common/v2/get_full_redirect_app_url.ts +++ b/x-pack/plugins/reporting/server/export_types/common/v2/get_full_redirect_app_url.ts @@ -10,7 +10,11 @@ import { ReportingConfig } from '../../..'; import { getRedirectAppPath } from '../../../../common/constants'; import { buildKibanaPath } from '../../../../common/build_kibana_path'; -export function getFullRedirectAppUrl(config: ReportingConfig, spaceId?: string) { +export function getFullRedirectAppUrl( + config: ReportingConfig, + spaceId?: string, + forceNow?: string +) { const [basePath, protocol, hostname, port] = [ config.kbnConfig.get('server', 'basePath'), config.get('kibanaServer', 'protocol'), @@ -29,5 +33,6 @@ export function getFullRedirectAppUrl(config: ReportingConfig, spaceId?: string) hostname, port, pathname: path, + query: forceNow ? { forceNow } : undefined, }); } diff --git a/x-pack/plugins/reporting/server/export_types/png_v2/execute_job.test.ts b/x-pack/plugins/reporting/server/export_types/png_v2/execute_job.test.ts index e57eab382468c..3cf3c057e7b9c 100644 --- a/x-pack/plugins/reporting/server/export_types/png_v2/execute_job.test.ts +++ b/x-pack/plugins/reporting/server/export_types/png_v2/execute_job.test.ts @@ -102,12 +102,10 @@ test(`passes browserTimezone to generatePng`, async () => { "warning": [Function], }, Array [ - "localhost:80undefined/app/management/insightsAndAlerting/reporting/r", + "localhost:80undefined/app/management/insightsAndAlerting/reporting/r?forceNow=test", Object { "id": "test", - "params": Object { - "forceNow": "test", - }, + "params": Object {}, "version": "test", }, ], diff --git a/x-pack/plugins/reporting/server/export_types/png_v2/execute_job.ts b/x-pack/plugins/reporting/server/export_types/png_v2/execute_job.ts index 5c2cc66d3d3aa..a7478de1cc96e 100644 --- a/x-pack/plugins/reporting/server/export_types/png_v2/execute_job.ts +++ b/x-pack/plugins/reporting/server/export_types/png_v2/execute_job.ts @@ -16,7 +16,6 @@ import { getConditionalHeaders, omitBlockedHeaders, generatePngObservableFactory, - setForceNow, } from '../common'; import { getFullRedirectAppUrl } from '../common/v2/get_full_redirect_app_url'; import { TaskPayloadPNGV2 } from './types'; @@ -38,8 +37,8 @@ export const runTaskFnFactory: RunTaskFnFactory> = map((decryptedHeaders) => omitBlockedHeaders(decryptedHeaders)), map((filteredHeaders) => getConditionalHeaders(config, filteredHeaders)), mergeMap((conditionalHeaders) => { - const url = getFullRedirectAppUrl(config, job.spaceId); - const [locatorParams] = job.locatorParams.map(setForceNow(job.forceNow)); + const url = getFullRedirectAppUrl(config, job.spaceId, job.forceNow); + const [locatorParams] = job.locatorParams; apmGetAssets?.end(); diff --git a/x-pack/plugins/reporting/server/export_types/printable_pdf_v2/execute_job.ts b/x-pack/plugins/reporting/server/export_types/printable_pdf_v2/execute_job.ts index e44f5e98fa4fe..2c553295aa840 100644 --- a/x-pack/plugins/reporting/server/export_types/printable_pdf_v2/execute_job.ts +++ b/x-pack/plugins/reporting/server/export_types/printable_pdf_v2/execute_job.ts @@ -16,7 +16,6 @@ import { getConditionalHeaders, omitBlockedHeaders, getCustomLogo, - setForceNow, } from '../common'; import { generatePdfObservableFactory } from './lib/generate_pdf'; import { TaskPayloadPDFV2 } from './types'; @@ -50,7 +49,7 @@ export const runTaskFnFactory: RunTaskFnFactory> = jobLogger, job, title, - locatorParams.map(setForceNow(job.forceNow)), + locatorParams, browserTimezone, conditionalHeaders, layout, diff --git a/x-pack/plugins/reporting/server/export_types/printable_pdf_v2/lib/generate_pdf.ts b/x-pack/plugins/reporting/server/export_types/printable_pdf_v2/lib/generate_pdf.ts index 424a347876a1d..9fb31a1104279 100644 --- a/x-pack/plugins/reporting/server/export_types/printable_pdf_v2/lib/generate_pdf.ts +++ b/x-pack/plugins/reporting/server/export_types/printable_pdf_v2/lib/generate_pdf.ts @@ -56,7 +56,10 @@ export async function generatePdfObservableFactory(reporting: ReportingCore) { /** * For each locator we get the relative URL to the redirect app */ - const urls = locatorParams.map(() => getFullRedirectAppUrl(reporting.getConfig(), job.spaceId)); + const urls = locatorParams.map(() => + getFullRedirectAppUrl(reporting.getConfig(), job.spaceId, job.forceNow) + ); + const screenshots$ = getScreenshots$(captureConfig, browserDriverFactory, { logger, urlsOrUrlLocatorTuples: zip(urls, locatorParams) as UrlOrUrlLocatorTuple[],