From cdf1762d7783300e31907819f5fdb71dc8661d98 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Tue, 14 Sep 2021 14:40:06 +0200 Subject: [PATCH] [Discover/Reporting] Fix export that does not contain relative time filter (#110459) * version 1 of fix: we set the time range on the search source at CSV generation time * updated jest tests and updated API for getSharingData * make time range optional for getSharingData * pivot to updating "getTime" functionality by introducing a new flag * update jest snapshots * update comment * refactored coerceToAbsoluteTime -> coerceRelativeTimeToAbsoluteTime and updated behaviour to be more specific * fix jest test * do not change createFilter API, rather create new createRelativeFilter API, also only use this in one place in discover * update jest tests * update mock * update jest test mock Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../common/query/timefilter/get_time.test.ts | 88 +++++++++++++++++-- .../data/common/query/timefilter/get_time.ts | 71 +++++++++++---- .../public/query/timefilter/timefilter.ts | 24 ++++- .../timefilter/timefilter_service.mock.ts | 1 + .../components/top_nav/get_top_nav_links.ts | 1 + .../apps/main/utils/update_search_source.ts | 5 +- ...ble_anomaly_charts_container.test.tsx.snap | 1 + .../generate_csv/generate_csv.ts | 2 +- 8 files changed, 168 insertions(+), 25 deletions(-) diff --git a/src/plugins/data/common/query/timefilter/get_time.test.ts b/src/plugins/data/common/query/timefilter/get_time.test.ts index 5389eb71a10bb..70f6f418cc739 100644 --- a/src/plugins/data/common/query/timefilter/get_time.test.ts +++ b/src/plugins/data/common/query/timefilter/get_time.test.ts @@ -7,9 +7,10 @@ */ import { RangeFilter } from '@kbn/es-query'; +import type { IIndexPattern } from '../..'; import moment from 'moment'; import sinon from 'sinon'; -import { getTime, getAbsoluteTimeRange } from './get_time'; +import { getTime, getRelativeTime, getAbsoluteTimeRange } from './get_time'; describe('get_time', () => { describe('getTime', () => { @@ -17,7 +18,7 @@ describe('get_time', () => { const clock = sinon.useFakeTimers(moment.utc([2000, 1, 1, 0, 0, 0, 0]).valueOf()); const filter = getTime( - { + ({ id: 'test', title: 'test', timeFieldName: 'date', @@ -31,7 +32,7 @@ describe('get_time', () => { filterable: true, }, ], - } as any, + } as unknown) as IIndexPattern, { from: 'now-60y', to: 'now' } ) as RangeFilter; expect(filter.range.date).toEqual({ @@ -46,7 +47,7 @@ describe('get_time', () => { const clock = sinon.useFakeTimers(moment.utc([2000, 1, 1, 0, 0, 0, 0]).valueOf()); const filter = getTime( - { + ({ id: 'test', title: 'test', timeFieldName: 'date', @@ -68,7 +69,7 @@ describe('get_time', () => { filterable: true, }, ], - } as any, + } as unknown) as IIndexPattern, { from: 'now-60y', to: 'now' }, { fieldName: 'myCustomDate' } ) as RangeFilter; @@ -80,6 +81,83 @@ describe('get_time', () => { clock.restore(); }); }); + describe('getRelativeTime', () => { + test('do not coerce relative time to absolute time when given flag', () => { + const filter = getRelativeTime( + ({ + id: 'test', + title: 'test', + timeFieldName: 'date', + fields: [ + { + name: 'date', + type: 'date', + esTypes: ['date'], + aggregatable: true, + searchable: true, + filterable: true, + }, + { + name: 'myCustomDate', + type: 'date', + esTypes: ['date'], + aggregatable: true, + searchable: true, + filterable: true, + }, + ], + } as unknown) as IIndexPattern, + { from: 'now-60y', to: 'now' }, + { fieldName: 'myCustomDate' } + ) as RangeFilter; + + expect(filter.range.myCustomDate).toEqual({ + gte: 'now-60y', + lte: 'now', + format: 'strict_date_optional_time', + }); + }); + test('do not coerce relative time to absolute time when given flag - with mixed from and to times', () => { + const clock = sinon.useFakeTimers(moment.utc().valueOf()); + const filter = getRelativeTime( + ({ + id: 'test', + title: 'test', + timeFieldName: 'date', + fields: [ + { + name: 'date', + type: 'date', + esTypes: ['date'], + aggregatable: true, + searchable: true, + filterable: true, + }, + { + name: 'myCustomDate', + type: 'date', + esTypes: ['date'], + aggregatable: true, + searchable: true, + filterable: true, + }, + ], + } as unknown) as IIndexPattern, + { + from: '2020-09-01T08:30:00.000Z', + to: 'now', + }, + { fieldName: 'myCustomDate' } + ) as RangeFilter; + + expect(filter.range.myCustomDate).toEqual({ + gte: '2020-09-01T08:30:00.000Z', + lte: 'now', + format: 'strict_date_optional_time', + }); + clock.restore(); + }); + }); describe('getAbsoluteTimeRange', () => { test('should forward absolute timerange as is', () => { const from = '2000-01-01T00:00:00.000Z'; diff --git a/src/plugins/data/common/query/timefilter/get_time.ts b/src/plugins/data/common/query/timefilter/get_time.ts index 4c20e49f53315..fd21b2251ea3a 100644 --- a/src/plugins/data/common/query/timefilter/get_time.ts +++ b/src/plugins/data/common/query/timefilter/get_time.ts @@ -7,20 +7,30 @@ */ import dateMath from '@elastic/datemath'; +import { omitBy } from 'lodash'; import { buildRangeFilter } from '@kbn/es-query'; -import type { IIndexPattern, TimeRange, TimeRangeBounds } from '../..'; +import type { Moment } from 'moment'; +import type { IIndexPattern, TimeRange, TimeRangeBounds, RangeFilterParams } from '../..'; interface CalculateBoundsOptions { forceNow?: Date; } +const calculateLowerBound = (from: string, forceNow?: Date): undefined | Moment => + dateMath.parse(from, { forceNow }); + +const calculateUpperBound = (to: string, forceNow?: Date): undefined | Moment => + dateMath.parse(to, { roundUp: true, forceNow }); + +const isRelativeTime = (value: string): boolean => value.includes('now'); + export function calculateBounds( timeRange: TimeRange, options: CalculateBoundsOptions = {} ): TimeRangeBounds { return { - min: dateMath.parse(timeRange.from, { forceNow: options.forceNow }), - max: dateMath.parse(timeRange.to, { roundUp: true, forceNow: options.forceNow }), + min: calculateLowerBound(timeRange.from, options.forceNow), + max: calculateUpperBound(timeRange.to, options.forceNow), }; } @@ -44,7 +54,22 @@ export function getTime( indexPattern, timeRange, options?.fieldName || indexPattern?.timeFieldName, - options?.forceNow + options?.forceNow, + true + ); +} + +export function getRelativeTime( + indexPattern: IIndexPattern | undefined, + timeRange: TimeRange, + options?: { forceNow?: Date; fieldName?: string } +) { + return createTimeRangeFilter( + indexPattern, + timeRange, + options?.fieldName || indexPattern?.timeFieldName, + options?.forceNow, + false ); } @@ -52,7 +77,8 @@ function createTimeRangeFilter( indexPattern: IIndexPattern | undefined, timeRange: TimeRange, fieldName?: string, - forceNow?: Date + forceNow?: Date, + coerceRelativeTimeToAbsoluteTime: boolean = true ) { if (!indexPattern) { return; @@ -64,17 +90,28 @@ function createTimeRangeFilter( return; } - const bounds = calculateBounds(timeRange, { forceNow }); - if (!bounds) { - return; + let rangeFilterParams: RangeFilterParams = { + format: 'strict_date_optional_time', + }; + + if (coerceRelativeTimeToAbsoluteTime) { + const bounds = calculateBounds(timeRange, { forceNow }); + if (!bounds) { + return; + } + rangeFilterParams.gte = bounds.min?.toISOString(); + rangeFilterParams.lte = bounds.max?.toISOString(); + } else { + rangeFilterParams.gte = isRelativeTime(timeRange.from) + ? timeRange.from + : calculateLowerBound(timeRange.from, forceNow)?.toISOString(); + + rangeFilterParams.lte = isRelativeTime(timeRange.to) + ? timeRange.to + : calculateUpperBound(timeRange.to, forceNow)?.toISOString(); } - return buildRangeFilter( - field, - { - ...(bounds.min && { gte: bounds.min.toISOString() }), - ...(bounds.max && { lte: bounds.max.toISOString() }), - format: 'strict_date_optional_time', - }, - indexPattern - ); + + rangeFilterParams = omitBy(rangeFilterParams, (v) => v == null); + + return buildRangeFilter(field, rangeFilterParams, indexPattern); } diff --git a/src/plugins/data/public/query/timefilter/timefilter.ts b/src/plugins/data/public/query/timefilter/timefilter.ts index 9894010601d2b..3b537562586a7 100644 --- a/src/plugins/data/public/query/timefilter/timefilter.ts +++ b/src/plugins/data/public/query/timefilter/timefilter.ts @@ -17,6 +17,7 @@ import { calculateBounds, getAbsoluteTimeRange, getTime, + getRelativeTime, IIndexPattern, RefreshInterval, TimeRange, @@ -27,7 +28,6 @@ import { createAutoRefreshLoop, AutoRefreshDoneFn } from './lib/auto_refresh_loo export { AutoRefreshDoneFn }; // TODO: remove! - export class Timefilter { // Fired when isTimeRangeSelectorEnabled \ isAutoRefreshSelectorEnabled are toggled private enabledUpdated$ = new BehaviorSubject(false); @@ -178,12 +178,34 @@ export class Timefilter { } }; + /** + * Create a time filter that coerces all time values to absolute time. + * + * This is useful for creating a filter that ensures all ES queries will fetch the exact same data + * and leverages ES query cache for performance improvement. + * + * One use case is keeping different elements embedded in the same UI in sync. + */ public createFilter = (indexPattern: IIndexPattern, timeRange?: TimeRange) => { return getTime(indexPattern, timeRange ? timeRange : this._time, { forceNow: this.nowProvider.get(), }); }; + /** + * Create a time filter that converts only absolute time to ISO strings, it leaves relative time + * values unchanged (e.g. "now-1"). + * + * This is useful for sending datemath values to ES endpoints to generate reports over time. + * + * @note Consumers of this function need to ensure that the ES endpoint supports datemath. + */ + public createRelativeFilter = (indexPattern: IIndexPattern, timeRange?: TimeRange) => { + return getRelativeTime(indexPattern, timeRange ? timeRange : this._time, { + forceNow: this.nowProvider.get(), + }); + }; + public getBounds(): TimeRangeBounds { return this.calculateBounds(this._time); } diff --git a/src/plugins/data/public/query/timefilter/timefilter_service.mock.ts b/src/plugins/data/public/query/timefilter/timefilter_service.mock.ts index f1f02a010e98c..2b6a65e6c9bd7 100644 --- a/src/plugins/data/public/query/timefilter/timefilter_service.mock.ts +++ b/src/plugins/data/public/query/timefilter/timefilter_service.mock.ts @@ -34,6 +34,7 @@ const createSetupContractMock = () => { getBounds: jest.fn(), calculateBounds: jest.fn(), createFilter: jest.fn(), + createRelativeFilter: jest.fn(), getRefreshIntervalDefaults: jest.fn(), getTimeDefaults: jest.fn(), getAbsoluteTime: jest.fn(), 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 a692dacd5e9f4..58a7242974ba2 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 @@ -114,6 +114,7 @@ export const getTopNavLinks = ({ state.appStateContainer.getState(), services.uiSettings ); + services.share.toggleShareContextMenu({ anchorElement, allowEmbed: false, 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 b4a1dab41a096..74e63c399743f 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 @@ -54,7 +54,10 @@ 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.createFilter(indexPattern)); + searchSource.setField( + 'filter', + data.query.timefilter.timefilter.createRelativeFilter(indexPattern) + ); } if (useNewFieldsApi) { diff --git a/x-pack/plugins/ml/public/embeddables/anomaly_charts/__snapshots__/embeddable_anomaly_charts_container.test.tsx.snap b/x-pack/plugins/ml/public/embeddables/anomaly_charts/__snapshots__/embeddable_anomaly_charts_container.test.tsx.snap index 52318c80d5d08..703a3778ebfb9 100644 --- a/x-pack/plugins/ml/public/embeddables/anomaly_charts/__snapshots__/embeddable_anomaly_charts_container.test.tsx.snap +++ b/x-pack/plugins/ml/public/embeddables/anomaly_charts/__snapshots__/embeddable_anomaly_charts_container.test.tsx.snap @@ -33,6 +33,7 @@ Object { "timefilter": Object { "calculateBounds": [MockFunction], "createFilter": [MockFunction], + "createRelativeFilter": [MockFunction], "disableAutoRefreshSelector": [MockFunction], "disableTimeRangeSelector": [MockFunction], "enableAutoRefreshSelector": [MockFunction], diff --git a/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.ts b/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.ts index 61dc9881f5bcc..dc5c560d29546 100644 --- a/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.ts +++ b/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.ts @@ -291,7 +291,7 @@ export class CsvGenerator { const index = searchSource.getField('index'); if (!index) { - throw new Error(`The search must have a revference to an index pattern!`); + throw new Error(`The search must have a reference to an index pattern!`); } const { maxSizeBytes, bom, escapeFormulaValues, scroll: scrollSettings } = settings;