From 606475bee4a0d1e49f9391e939895a3f69987e14 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Fri, 4 Jun 2021 14:08:31 -0400 Subject: [PATCH 1/5] fixing time comparison types --- .../shared/time_comparison/index.test.tsx | 374 ++++++++++-------- .../shared/time_comparison/index.tsx | 143 ++++--- .../url_params_context/helpers.test.ts | 50 +++ .../context/url_params_context/helpers.ts | 41 +- .../url_params_context/resolve_url_params.ts | 2 +- .../context/url_params_context/types.ts | 2 + .../url_params_context/url_params_context.tsx | 13 +- 7 files changed, 402 insertions(+), 223 deletions(-) diff --git a/x-pack/plugins/apm/public/components/shared/time_comparison/index.test.tsx b/x-pack/plugins/apm/public/components/shared/time_comparison/index.test.tsx index a4f44290fe777f..dd87c23908cbc5 100644 --- a/x-pack/plugins/apm/public/components/shared/time_comparison/index.test.tsx +++ b/x-pack/plugins/apm/public/components/shared/time_comparison/index.test.tsx @@ -15,7 +15,7 @@ import { expectTextsInDocument, expectTextsNotInDocument, } from '../../../utils/testHelpers'; -import { TimeComparison } from './'; +import { getComparisonTypes, getSelectOptions, TimeComparison } from './'; import * as urlHelpers from '../../shared/Links/url_helpers'; import moment from 'moment'; import { TimeRangeComparisonType } from './get_time_range_comparison'; @@ -37,188 +37,248 @@ describe('TimeComparison', () => { moment.tz.setDefault('Europe/Amsterdam'); }); afterAll(() => moment.tz.setDefault('')); - const spy = jest.spyOn(urlHelpers, 'replace'); - beforeEach(() => { - jest.resetAllMocks(); - }); - describe('Time range is between 0 - 24 hours', () => { - it('sets default values', () => { - const Wrapper = getWrapper({ - start: '2021-01-28T14:45:00.000Z', - end: '2021-01-28T15:00:00.000Z', - rangeTo: 'now', - }); - render(, { - wrapper: Wrapper, - }); - expect(spy).toHaveBeenCalledWith(expect.anything(), { - query: { - comparisonEnabled: 'true', - comparisonType: TimeRangeComparisonType.DayBefore, - }, - }); + + describe('getComparisonTypes', () => { + it('shows week and day before when 15 minutes is selected', () => { + expect( + getComparisonTypes({ + start: '2021-06-04T16:17:02.335Z', + end: '2021-06-04T16:32:02.335Z', + }) + ).toEqual([ + TimeRangeComparisonType.DayBefore.valueOf(), + TimeRangeComparisonType.WeekBefore.valueOf(), + ]); }); - it('selects day before and enables comparison', () => { - const Wrapper = getWrapper({ - start: '2021-01-28T14:45:00.000Z', - end: '2021-01-28T15:00:00.000Z', - comparisonEnabled: true, - comparisonType: TimeRangeComparisonType.DayBefore, - rangeTo: 'now', - }); - const component = render(, { - wrapper: Wrapper, - }); - expectTextsInDocument(component, ['Day before', 'Week before']); + + it('shows week and day before when Today is selected', () => { expect( - (component.getByTestId('comparisonSelect') as HTMLSelectElement) - .selectedIndex - ).toEqual(0); + getComparisonTypes({ + start: '2021-06-04T04:00:00.000Z', + end: '2021-06-05T03:59:59.999Z', + }) + ).toEqual([ + TimeRangeComparisonType.DayBefore.valueOf(), + TimeRangeComparisonType.WeekBefore.valueOf(), + ]); }); - it('enables yesterday option when date difference is equal to 24 hours', () => { - const Wrapper = getWrapper({ - start: '2021-01-28T10:00:00.000Z', - end: '2021-01-29T10:00:00.000Z', - comparisonEnabled: true, - comparisonType: TimeRangeComparisonType.DayBefore, - rangeTo: 'now', - }); - const component = render(, { - wrapper: Wrapper, - }); - expectTextsInDocument(component, ['Day before', 'Week before']); + it('shows week and day before when 24 hours is selected', () => { + expect( + getComparisonTypes({ + start: '2021-06-03T16:31:35.748Z', + end: '2021-06-04T16:31:35.748Z', + }) + ).toEqual([ + TimeRangeComparisonType.DayBefore.valueOf(), + TimeRangeComparisonType.WeekBefore.valueOf(), + ]); + }); + it('shows week before when 25 hours is selected', () => { expect( - (component.getByTestId('comparisonSelect') as HTMLSelectElement) - .selectedIndex - ).toEqual(0); + getComparisonTypes({ + start: '2021-06-02T12:32:00.000Z', + end: '2021-06-03T13:32:09.079Z', + }) + ).toEqual([TimeRangeComparisonType.WeekBefore.valueOf()]); }); - it('selects previous period when rangeTo is different than now', () => { - const Wrapper = getWrapper({ - start: '2021-01-28T10:00:00.000Z', - end: '2021-01-29T10:00:00.000Z', - comparisonEnabled: true, - comparisonType: TimeRangeComparisonType.PeriodBefore, - rangeTo: 'now-15m', - }); - const component = render(, { - wrapper: Wrapper, - }); - expectTextsInDocument(component, ['27/01 11:00 - 28/01 11:00']); + it('shows week before when 7 days is selected', () => { + expect( + getComparisonTypes({ + start: '2021-05-28T16:32:17.520Z', + end: '2021-06-04T16:32:17.520Z', + }) + ).toEqual([TimeRangeComparisonType.WeekBefore.valueOf()]); + }); + it('shows period before when 8 days is selected', () => { expect( - (component.getByTestId('comparisonSelect') as HTMLSelectElement) - .selectedIndex - ).toEqual(0); + getComparisonTypes({ + start: '2021-05-27T16:32:46.747Z', + end: '2021-06-04T16:32:46.747Z', + }) + ).toEqual([TimeRangeComparisonType.PeriodBefore.valueOf()]); }); }); - describe('Time range is between 24 hours - 1 week', () => { - it("doesn't show yesterday option when date difference is greater than 24 hours", () => { - const Wrapper = getWrapper({ - start: '2021-01-28T10:00:00.000Z', - end: '2021-01-29T11:00:00.000Z', - comparisonEnabled: true, - comparisonType: TimeRangeComparisonType.WeekBefore, - rangeTo: 'now', - }); - const component = render(, { - wrapper: Wrapper, - }); - expectTextsNotInDocument(component, ['Day before']); - expectTextsInDocument(component, ['Week before']); - }); - it('sets default values', () => { - const Wrapper = getWrapper({ - start: '2021-01-26T15:00:00.000Z', - end: '2021-01-28T15:00:00.000Z', - rangeTo: 'now', - }); - render(, { - wrapper: Wrapper, - }); - expect(spy).toHaveBeenCalledWith(expect.anything(), { - query: { - comparisonEnabled: 'true', - comparisonType: TimeRangeComparisonType.WeekBefore, + describe('getSelectOptions', () => { + it('returns formatted text based on comparison type', () => { + expect( + getSelectOptions({ + comparisonTypes: [ + TimeRangeComparisonType.DayBefore, + TimeRangeComparisonType.WeekBefore, + TimeRangeComparisonType.PeriodBefore, + ], + start: '2021-05-27T16:32:46.747Z', + end: '2021-06-04T16:32:46.747Z', + }) + ).toEqual([ + { + value: TimeRangeComparisonType.DayBefore.valueOf(), + text: 'Day before', }, - }); + { + value: TimeRangeComparisonType.WeekBefore.valueOf(), + text: 'Week before', + }, + { + value: TimeRangeComparisonType.PeriodBefore.valueOf(), + text: '19/05 18:32 - 27/05 18:32', + }, + ]); }); - it('selects week and enables comparison', () => { - const Wrapper = getWrapper({ - start: '2021-01-26T15:00:00.000Z', - end: '2021-01-28T15:00:00.000Z', - comparisonEnabled: true, - comparisonType: TimeRangeComparisonType.WeekBefore, - rangeTo: 'now', - }); - const component = render(, { - wrapper: Wrapper, - }); - expectTextsNotInDocument(component, ['Day before']); - expectTextsInDocument(component, ['Week before']); + + it('formats period before as DD/MM/YY HH:mm when range years are different', () => { expect( - (component.getByTestId('comparisonSelect') as HTMLSelectElement) - .selectedIndex - ).toEqual(0); + getSelectOptions({ + comparisonTypes: [TimeRangeComparisonType.PeriodBefore], + start: '2020-05-27T16:32:46.747Z', + end: '2021-06-04T16:32:46.747Z', + }) + ).toEqual([ + { + value: TimeRangeComparisonType.PeriodBefore.valueOf(), + text: '20/05/19 18:32 - 27/05/20 18:32', + }, + ]); }); + }); - it('selects previous period when rangeTo is different than now', () => { - const Wrapper = getWrapper({ - start: '2021-01-26T15:00:00.000Z', - end: '2021-01-28T15:00:00.000Z', - comparisonEnabled: true, - comparisonType: TimeRangeComparisonType.PeriodBefore, - rangeTo: '2021-01-28T15:00:00.000Z', + describe('TimeComparison component', () => { + const spy = jest.spyOn(urlHelpers, 'replace'); + beforeEach(() => { + jest.resetAllMocks(); + }); + describe('Time range is between 0 - 24 hours', () => { + it('sets default values', () => { + const Wrapper = getWrapper({ + exactStart: '2021-06-04T16:17:02.335Z', + exactEnd: '2021-06-04T16:32:02.335Z', + }); + render(, { wrapper: Wrapper }); + expect(spy).toHaveBeenCalledWith(expect.anything(), { + query: { + comparisonEnabled: 'true', + comparisonType: TimeRangeComparisonType.DayBefore, + }, + }); }); - const component = render(, { - wrapper: Wrapper, + it('selects day before and enables comparison', () => { + const Wrapper = getWrapper({ + exactStart: '2021-06-04T16:17:02.335Z', + exactEnd: '2021-06-04T16:32:02.335Z', + comparisonEnabled: true, + comparisonType: TimeRangeComparisonType.DayBefore, + }); + const component = render(, { wrapper: Wrapper }); + expectTextsInDocument(component, ['Day before', 'Week before']); + expect( + (component.getByTestId('comparisonSelect') as HTMLSelectElement) + .selectedIndex + ).toEqual(0); + }); + + it('enables day before option when date difference is equal to 24 hours', () => { + const Wrapper = getWrapper({ + exactStart: '2021-06-03T16:31:35.748Z', + exactEnd: '2021-06-04T16:31:35.748Z', + comparisonEnabled: true, + comparisonType: TimeRangeComparisonType.DayBefore, + }); + const component = render(, { wrapper: Wrapper }); + expectTextsInDocument(component, ['Day before', 'Week before']); + expect( + (component.getByTestId('comparisonSelect') as HTMLSelectElement) + .selectedIndex + ).toEqual(0); }); - expectTextsInDocument(component, ['24/01 16:00 - 26/01 16:00']); - expect( - (component.getByTestId('comparisonSelect') as HTMLSelectElement) - .selectedIndex - ).toEqual(0); }); - }); - describe('Time range is greater than 7 days', () => { - it('Shows absolute times without year when within the same year', () => { - const Wrapper = getWrapper({ - start: '2021-01-20T15:00:00.000Z', - end: '2021-01-28T15:00:00.000Z', - comparisonEnabled: true, - comparisonType: TimeRangeComparisonType.PeriodBefore, - rangeTo: 'now', + describe('Time range is between 24 hours - 1 week', () => { + it("doesn't show day before option when date difference is greater than 24 hours", () => { + const Wrapper = getWrapper({ + exactStart: '2021-06-02T12:32:00.000Z', + exactEnd: '2021-06-03T13:32:09.079Z', + comparisonEnabled: true, + comparisonType: TimeRangeComparisonType.WeekBefore, + }); + const component = render(, { + wrapper: Wrapper, + }); + expectTextsNotInDocument(component, ['Day before']); + expectTextsInDocument(component, ['Week before']); }); - const component = render(, { - wrapper: Wrapper, + it('sets default values', () => { + const Wrapper = getWrapper({ + exactStart: '2021-06-02T12:32:00.000Z', + exactEnd: '2021-06-03T13:32:09.079Z', + }); + render(, { + wrapper: Wrapper, + }); + expect(spy).toHaveBeenCalledWith(expect.anything(), { + query: { + comparisonEnabled: 'true', + comparisonType: TimeRangeComparisonType.WeekBefore, + }, + }); + }); + it('selects week before and enables comparison', () => { + const Wrapper = getWrapper({ + exactStart: '2021-06-02T12:32:00.000Z', + exactEnd: '2021-06-03T13:32:09.079Z', + comparisonEnabled: true, + comparisonType: TimeRangeComparisonType.WeekBefore, + }); + const component = render(, { + wrapper: Wrapper, + }); + expectTextsNotInDocument(component, ['Day before']); + expectTextsInDocument(component, ['Week before']); + expect( + (component.getByTestId('comparisonSelect') as HTMLSelectElement) + .selectedIndex + ).toEqual(0); }); - expect(spy).not.toHaveBeenCalled(); - expectTextsInDocument(component, ['12/01 16:00 - 20/01 16:00']); - expect( - (component.getByTestId('comparisonSelect') as HTMLSelectElement) - .selectedIndex - ).toEqual(0); }); - it('Shows absolute times with year when on different year', () => { - const Wrapper = getWrapper({ - start: '2020-12-20T15:00:00.000Z', - end: '2021-01-28T15:00:00.000Z', - comparisonEnabled: true, - comparisonType: TimeRangeComparisonType.PeriodBefore, - rangeTo: 'now', + describe('Time range is greater than 7 days', () => { + it('Shows absolute times without year when within the same year', () => { + const Wrapper = getWrapper({ + exactStart: '2021-05-27T16:32:46.747Z', + exactEnd: '2021-06-04T16:32:46.747Z', + comparisonEnabled: true, + comparisonType: TimeRangeComparisonType.PeriodBefore, + }); + const component = render(, { + wrapper: Wrapper, + }); + expect(spy).not.toHaveBeenCalled(); + expectTextsInDocument(component, ['19/05 18:32 - 27/05 18:32']); + expect( + (component.getByTestId('comparisonSelect') as HTMLSelectElement) + .selectedIndex + ).toEqual(0); }); - const component = render(, { - wrapper: Wrapper, + + it('Shows absolute times with year when on different year', () => { + const Wrapper = getWrapper({ + exactStart: '2020-05-27T16:32:46.747Z', + exactEnd: '2021-06-04T16:32:46.747Z', + comparisonEnabled: true, + comparisonType: TimeRangeComparisonType.PeriodBefore, + }); + const component = render(, { + wrapper: Wrapper, + }); + expect(spy).not.toHaveBeenCalled(); + expectTextsInDocument(component, ['20/05/19 18:32 - 27/05/20 18:32']); + expect( + (component.getByTestId('comparisonSelect') as HTMLSelectElement) + .selectedIndex + ).toEqual(0); }); - expect(spy).not.toHaveBeenCalled(); - expectTextsInDocument(component, ['11/11/20 16:00 - 20/12/20 16:00']); - expect( - (component.getByTestId('comparisonSelect') as HTMLSelectElement) - .selectedIndex - ).toEqual(0); }); }); }); diff --git a/x-pack/plugins/apm/public/components/shared/time_comparison/index.tsx b/x-pack/plugins/apm/public/components/shared/time_comparison/index.tsx index 98fbd4f399d980..57f60e9fd83bb4 100644 --- a/x-pack/plugins/apm/public/components/shared/time_comparison/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/time_comparison/index.tsx @@ -59,80 +59,94 @@ function formatDate({ return `${momentStart.format(dateFormat)} - ${momentEnd.format(dateFormat)}`; } -function getSelectOptions({ +export function getComparisonTypes({ start, end, - rangeTo, - comparisonEnabled, }: { start?: string; end?: string; - rangeTo?: string; - comparisonEnabled?: boolean; }) { const momentStart = moment(start); const momentEnd = moment(end); - const dayBeforeOption = { - value: TimeRangeComparisonType.DayBefore, - text: i18n.translate('xpack.apm.timeComparison.select.dayBefore', { - defaultMessage: 'Day before', - }), - }; - - const weekBeforeOption = { - value: TimeRangeComparisonType.WeekBefore, - text: i18n.translate('xpack.apm.timeComparison.select.weekBefore', { - defaultMessage: 'Week before', - }), - }; - const dateDiff = Number( getDateDifference({ start: momentStart, end: momentEnd, unitOfTime: 'days', precise: true, - }).toFixed(2) + }) ); - const isRangeToNow = rangeTo === 'now'; - - if (isRangeToNow) { - // Less than or equals to one day - if (dateDiff <= 1) { - return [dayBeforeOption, weekBeforeOption]; - } + // Less than or equals to one day + if (dateDiff <= 1) { + return [ + TimeRangeComparisonType.DayBefore, + TimeRangeComparisonType.WeekBefore, + ]; + } - // Less than or equals to one week - if (dateDiff <= 7) { - return [weekBeforeOption]; - } + // Less than or equals to one week + if (dateDiff <= 7) { + return [TimeRangeComparisonType.WeekBefore]; } + // } - const { comparisonStart, comparisonEnd } = getTimeRangeComparison({ - comparisonType: TimeRangeComparisonType.PeriodBefore, - start, - end, - comparisonEnabled, - }); + // above one week or when rangeTo is not "now" + return [TimeRangeComparisonType.PeriodBefore]; +} - const dateFormat = getDateFormat({ - previousPeriodStart: comparisonStart, - currentPeriodEnd: end, - }); +export function getSelectOptions({ + comparisonTypes, + start, + end, +}: { + comparisonTypes: TimeRangeComparisonType[]; + start?: string; + end?: string; +}) { + return comparisonTypes.map((value) => { + switch (value) { + case TimeRangeComparisonType.DayBefore: { + return { + value, + text: i18n.translate('xpack.apm.timeComparison.select.dayBefore', { + defaultMessage: 'Day before', + }), + }; + } + case TimeRangeComparisonType.WeekBefore: { + return { + value, + text: i18n.translate('xpack.apm.timeComparison.select.weekBefore', { + defaultMessage: 'Week before', + }), + }; + } + case TimeRangeComparisonType.PeriodBefore: { + const { comparisonStart, comparisonEnd } = getTimeRangeComparison({ + comparisonType: TimeRangeComparisonType.PeriodBefore, + start, + end, + comparisonEnabled: true, + }); - const prevPeriodOption = { - value: TimeRangeComparisonType.PeriodBefore, - text: formatDate({ - dateFormat, - previousPeriodStart: comparisonStart, - previousPeriodEnd: comparisonEnd, - }), - }; + const dateFormat = getDateFormat({ + previousPeriodStart: comparisonStart, + currentPeriodEnd: end, + }); - // above one week or when rangeTo is not "now" - return [prevPeriodOption]; + return { + value, + text: formatDate({ + dateFormat, + previousPeriodStart: comparisonStart, + previousPeriodEnd: comparisonEnd, + }), + }; + } + } + }); } export function TimeComparison() { @@ -140,14 +154,19 @@ export function TimeComparison() { const history = useHistory(); const { isMedium, isLarge } = useBreakPoints(); const { - urlParams: { start, end, comparisonEnabled, comparisonType, rangeTo }, + urlParams: { + start, + end, + comparisonEnabled, + comparisonType, + exactStart, + exactEnd, + }, } = useUrlParams(); - const selectOptions = getSelectOptions({ - start, - end, - rangeTo, - comparisonEnabled, + const comparisonTypes = getComparisonTypes({ + start: exactStart, + end: exactEnd, }); // Sets default values @@ -155,14 +174,18 @@ export function TimeComparison() { urlHelpers.replace(history, { query: { comparisonEnabled: comparisonEnabled === false ? 'false' : 'true', - comparisonType: comparisonType - ? comparisonType - : selectOptions[0].value, + comparisonType: comparisonType ? comparisonType : comparisonTypes[0], }, }); return null; } + const selectOptions = getSelectOptions({ + comparisonTypes, + start: exactStart, + end: exactEnd, + }); + const isSelectedComparisonTypeAvailable = selectOptions.some( ({ value }) => value === comparisonType ); diff --git a/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts b/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts index 4de68a5bc20362..9cdf36e0bd655e 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts +++ b/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts @@ -10,6 +10,9 @@ import moment from 'moment-timezone'; import * as helpers from './helpers'; describe('url_params_context helpers', () => { + beforeEach(() => { + jest.restoreAllMocks(); + }); describe('getDateRange', () => { describe('with non-rounded dates', () => { describe('one minute', () => { @@ -22,7 +25,9 @@ describe('url_params_context helpers', () => { }) ).toEqual({ start: '2021-01-28T05:47:00.000Z', + exactStart: '2021-01-28T05:47:00.000Z', end: '2021-01-28T05:48:55.304Z', + exactEnd: '2021-01-28T05:48:55.304Z', }); }); }); @@ -36,7 +41,9 @@ describe('url_params_context helpers', () => { }) ).toEqual({ start: '2021-01-27T05:46:00.000Z', + exactStart: '2021-01-27T05:46:00.000Z', end: '2021-01-28T05:46:13.367Z', + exactEnd: '2021-01-28T05:46:13.367Z', }); }); }); @@ -51,7 +58,9 @@ describe('url_params_context helpers', () => { }) ).toEqual({ start: '2020-01-28T05:52:00.000Z', + exactStart: '2020-01-28T05:52:00.000Z', end: '2021-01-28T05:52:39.741Z', + exactEnd: '2021-01-28T05:52:39.741Z', }); }); }); @@ -66,13 +75,17 @@ describe('url_params_context helpers', () => { rangeTo: 'now', start: '1970-01-01T00:00:00.000Z', end: '1971-01-01T00:00:00.000Z', + exactStart: '1970-01-01T00:00:00.000Z', + exactEnd: '1971-01-01T00:00:00.000Z', }, rangeFrom: 'now-1m', rangeTo: 'now', }) ).toEqual({ start: '1970-01-01T00:00:00.000Z', + exactStart: '1970-01-01T00:00:00.000Z', end: '1971-01-01T00:00:00.000Z', + exactEnd: '1971-01-01T00:00:00.000Z', }); }); }); @@ -93,25 +106,35 @@ describe('url_params_context helpers', () => { }) ).toEqual({ start: '1972-01-01T00:00:00.000Z', + exactStart: '1972-01-01T00:00:00.000Z', end: '1973-01-01T00:00:00.000Z', + exactEnd: '1973-01-01T00:00:00.000Z', }); }); }); describe('when the start or end are invalid', () => { it('returns the previous state', () => { + jest + .spyOn(datemath, 'parse') + .mockReturnValueOnce(undefined) + .mockReturnValueOnce(moment('2021-06-04T18:03:24.211Z')); expect( helpers.getDateRange({ state: { start: '1972-01-01T00:00:00.000Z', end: '1973-01-01T00:00:00.000Z', + exactStart: '1972-01-01T00:00:00.000Z', + exactEnd: '1973-01-01T00:00:00.000Z', }, rangeFrom: 'nope', rangeTo: 'now', }) ).toEqual({ start: '1972-01-01T00:00:00.000Z', + exactStart: '1972-01-01T00:00:00.000Z', end: '1973-01-01T00:00:00.000Z', + exactEnd: '1973-01-01T00:00:00.000Z', }); }); }); @@ -134,8 +157,35 @@ describe('url_params_context helpers', () => { ).toEqual({ start: '1970-01-01T00:00:00.000Z', end: '1970-01-01T00:00:00.000Z', + exactStart: '1970-01-01T00:00:00.000Z', + exactEnd: '1970-01-01T00:00:00.000Z', }); }); }); }); + + describe('getExactDate', () => { + it('returns undefined when date in not provided', () => { + expect(helpers.getExactDate()).toBeUndefined(); + }); + it('returns undefined when date in not relative', () => { + expect(helpers.getExactDate('2021-01-28T05:47:52.134Z')).toBeUndefined(); + }); + it('returns exact date', () => { + jest + .spyOn(datemath, 'parse') + .mockReturnValue(moment('2021-06-02T17:56:49.260Z').utc()); + expect(helpers.getExactDate('now-24h/h')?.toISOString()).toEqual( + '2021-06-02T17:56:49.260Z' + ); + }); + it('returns original date when now/d is passed', () => { + jest + .spyOn(datemath, 'parse') + .mockReturnValue(moment('2021-06-02T17:00:00.000Z').utc()); + expect(helpers.getExactDate('now/d')?.toISOString()).toEqual( + '2021-06-02T17:00:00.000Z' + ); + }); + }); }); diff --git a/x-pack/plugins/apm/public/context/url_params_context/helpers.ts b/x-pack/plugins/apm/public/context/url_params_context/helpers.ts index eae9eba8b3ddad..584332542f5128 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/helpers.ts +++ b/x-pack/plugins/apm/public/context/url_params_context/helpers.ts @@ -19,6 +19,27 @@ function getParsedDate(rawDate?: string, options = {}) { } } +export function getExactDate(rawDate?: string, options = {}) { + if (rawDate) { + const isRelativeDate = rawDate.substring(0, 3) === 'now'; + if (isRelativeDate) { + const isSubtractingDate = rawDate.indexOf('-') > 0; + const isRoundingDate = rawDate.indexOf('/') > 0; + + const rawDateWithouRounding = + // When relative time is subtracting a period and rounding the result (e.g. now-24h/h) + // removed the rounding part in order to get the exact time. + // This is needed because of of "Today"(now/d) and "This week"(now/w) options, it rounds the values up and down + // so the exact time is the rounded value. + isSubtractingDate && isRoundingDate + ? rawDate.substring(0, rawDate.indexOf('/')) + : rawDate; + + return getParsedDate(rawDateWithouRounding, options); + } + } +} + export function getDateRange({ state, rangeFrom, @@ -30,16 +51,25 @@ export function getDateRange({ }) { // If the previous state had the same range, just return that instead of calculating a new range. if (state.rangeFrom === rangeFrom && state.rangeTo === rangeTo) { - return { start: state.start, end: state.end }; + return { + start: state.start, + end: state.end, + exactStart: state.exactStart, + exactEnd: state.exactEnd, + }; } - const start = getParsedDate(rangeFrom); const end = getParsedDate(rangeTo, { roundUp: true }); // `getParsedDate` will return undefined for invalid or empty dates. We return // the previous state if either date is undefined. if (!start || !end) { - return { start: state.start, end: state.end }; + return { + start: state.start, + exactStart: state.start, + end: state.end, + exactEnd: state.end, + }; } // rounds down start to minute @@ -47,7 +77,12 @@ export function getDateRange({ return { start: roundedStart.toISOString(), + exactStart: + getExactDate(rangeFrom)?.toISOString() || roundedStart.toISOString(), end: end.toISOString(), + exactEnd: + getExactDate(rangeTo, { roundUp: true })?.toISOString() || + end.toISOString(), }; } diff --git a/x-pack/plugins/apm/public/context/url_params_context/resolve_url_params.ts b/x-pack/plugins/apm/public/context/url_params_context/resolve_url_params.ts index b6e7330be30cbd..134f65bbf0f405 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/resolve_url_params.ts +++ b/x-pack/plugins/apm/public/context/url_params_context/resolve_url_params.ts @@ -24,7 +24,7 @@ import { IUrlParams } from './types'; type TimeUrlParams = Pick< IUrlParams, - 'start' | 'end' | 'rangeFrom' | 'rangeTo' + 'start' | 'end' | 'rangeFrom' | 'rangeTo' | 'exactStart' | 'exactEnd' >; export function resolveUrlParams(location: Location, state: TimeUrlParams) { diff --git a/x-pack/plugins/apm/public/context/url_params_context/types.ts b/x-pack/plugins/apm/public/context/url_params_context/types.ts index 4332019d1a1c9e..5e9e4bd257b87b 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/types.ts +++ b/x-pack/plugins/apm/public/context/url_params_context/types.ts @@ -17,6 +17,8 @@ export type IUrlParams = { environment?: string; rangeFrom?: string; rangeTo?: string; + exactStart?: string; + exactEnd?: string; refreshInterval?: number; refreshPaused?: boolean; sortDirection?: string; diff --git a/x-pack/plugins/apm/public/context/url_params_context/url_params_context.tsx b/x-pack/plugins/apm/public/context/url_params_context/url_params_context.tsx index 1da57ac10a20c8..f3969745b6ea76 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/url_params_context.tsx +++ b/x-pack/plugins/apm/public/context/url_params_context/url_params_context.tsx @@ -54,7 +54,14 @@ const UrlParamsProvider: React.ComponentClass<{}> = withRouter( ({ location, children }) => { const refUrlParams = useRef(resolveUrlParams(location, {})); - const { start, end, rangeFrom, rangeTo } = refUrlParams.current; + const { + start, + end, + rangeFrom, + rangeTo, + exactStart, + exactEnd, + } = refUrlParams.current; // Counter to force an update in useFetcher when the refresh button is clicked. const [rangeId, setRangeId] = useState(0); @@ -66,8 +73,10 @@ const UrlParamsProvider: React.ComponentClass<{}> = withRouter( end, rangeFrom, rangeTo, + exactStart, + exactEnd, }), - [location, start, end, rangeFrom, rangeTo] + [location, start, end, rangeFrom, rangeTo, exactStart, exactEnd] ); refUrlParams.current = urlParams; From f33f4f4826b6e3af0ecd68ab9a85a982e1c4ff18 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Fri, 4 Jun 2021 16:15:54 -0400 Subject: [PATCH 2/5] fixing ts issues --- .../public/components/shared/time_comparison/index.tsx | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/x-pack/plugins/apm/public/components/shared/time_comparison/index.tsx b/x-pack/plugins/apm/public/components/shared/time_comparison/index.tsx index 57f60e9fd83bb4..4ac7ee1a2171ab 100644 --- a/x-pack/plugins/apm/public/components/shared/time_comparison/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/time_comparison/index.tsx @@ -154,14 +154,7 @@ export function TimeComparison() { const history = useHistory(); const { isMedium, isLarge } = useBreakPoints(); const { - urlParams: { - start, - end, - comparisonEnabled, - comparisonType, - exactStart, - exactEnd, - }, + urlParams: { comparisonEnabled, comparisonType, exactStart, exactEnd }, } = useUrlParams(); const comparisonTypes = getComparisonTypes({ From 2f3442d7f86c22d1b5fb6b7b46fc97786e2ec96d Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Mon, 14 Jun 2021 09:56:31 -0400 Subject: [PATCH 3/5] addressing PR comments --- .../url_params_context/helpers.test.ts | 22 ++++++------------ .../context/url_params_context/helpers.ts | 23 +++++-------------- 2 files changed, 13 insertions(+), 32 deletions(-) diff --git a/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts b/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts index 9cdf36e0bd655e..2ef4e4e519bce6 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts +++ b/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts @@ -171,21 +171,13 @@ describe('url_params_context helpers', () => { it('returns undefined when date in not relative', () => { expect(helpers.getExactDate('2021-01-28T05:47:52.134Z')).toBeUndefined(); }); - it('returns exact date', () => { - jest - .spyOn(datemath, 'parse') - .mockReturnValue(moment('2021-06-02T17:56:49.260Z').utc()); - expect(helpers.getExactDate('now-24h/h')?.toISOString()).toEqual( - '2021-06-02T17:56:49.260Z' - ); - }); - it('returns original date when now/d is passed', () => { - jest - .spyOn(datemath, 'parse') - .mockReturnValue(moment('2021-06-02T17:00:00.000Z').utc()); - expect(helpers.getExactDate('now/d')?.toISOString()).toEqual( - '2021-06-02T17:00:00.000Z' - ); + + it('removes rounding option from relative time', () => { + const spy = jest.spyOn(datemath, 'parse'); + helpers.getExactDate('now/d'); + expect(spy).toHaveBeenCalledWith('now', {}); + helpers.getExactDate('now-24h/h'); + expect(spy).toHaveBeenCalledWith('now-24h', {}); }); }); }); diff --git a/x-pack/plugins/apm/public/context/url_params_context/helpers.ts b/x-pack/plugins/apm/public/context/url_params_context/helpers.ts index 584332542f5128..7960d9ab25553a 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/helpers.ts +++ b/x-pack/plugins/apm/public/context/url_params_context/helpers.ts @@ -23,18 +23,7 @@ export function getExactDate(rawDate?: string, options = {}) { if (rawDate) { const isRelativeDate = rawDate.substring(0, 3) === 'now'; if (isRelativeDate) { - const isSubtractingDate = rawDate.indexOf('-') > 0; - const isRoundingDate = rawDate.indexOf('/') > 0; - - const rawDateWithouRounding = - // When relative time is subtracting a period and rounding the result (e.g. now-24h/h) - // removed the rounding part in order to get the exact time. - // This is needed because of of "Today"(now/d) and "This week"(now/w) options, it rounds the values up and down - // so the exact time is the rounded value. - isSubtractingDate && isRoundingDate - ? rawDate.substring(0, rawDate.indexOf('/')) - : rawDate; - + const rawDateWithouRounding = rawDate.replace(/\/(\w)$/, ''); return getParsedDate(rawDateWithouRounding, options); } } @@ -74,15 +63,15 @@ export function getDateRange({ // rounds down start to minute const roundedStart = moment(start).startOf('minute'); + // rounds down to minute + const exactStart = getExactDate(rangeFrom) || roundedStart; + const exactEnd = getExactDate(rangeTo, { roundUp: true }) || end; return { start: roundedStart.toISOString(), - exactStart: - getExactDate(rangeFrom)?.toISOString() || roundedStart.toISOString(), + exactStart: exactStart.toISOString(), end: end.toISOString(), - exactEnd: - getExactDate(rangeTo, { roundUp: true })?.toISOString() || - end.toISOString(), + exactEnd: exactEnd.toISOString(), }; } From e9231957b971439700de57a51cd10f99898a74b1 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Mon, 14 Jun 2021 11:25:06 -0400 Subject: [PATCH 4/5] addressing PR comments --- .../shared/time_comparison/index.tsx | 14 ++++++-------- .../context/url_params_context/helpers.test.ts | 18 +++++++++++++++--- .../context/url_params_context/helpers.ts | 6 +++--- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/x-pack/plugins/apm/public/components/shared/time_comparison/index.tsx b/x-pack/plugins/apm/public/components/shared/time_comparison/index.tsx index 4ac7ee1a2171ab..cbe7b81486a64d 100644 --- a/x-pack/plugins/apm/public/components/shared/time_comparison/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/time_comparison/index.tsx @@ -69,14 +69,12 @@ export function getComparisonTypes({ const momentStart = moment(start); const momentEnd = moment(end); - const dateDiff = Number( - getDateDifference({ - start: momentStart, - end: momentEnd, - unitOfTime: 'days', - precise: true, - }) - ); + const dateDiff = getDateDifference({ + start: momentStart, + end: momentEnd, + unitOfTime: 'days', + precise: true, + }); // Less than or equals to one day if (dateDiff <= 1) { diff --git a/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts b/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts index 2ef4e4e519bce6..54ec68947607da 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts +++ b/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts @@ -172,12 +172,24 @@ describe('url_params_context helpers', () => { expect(helpers.getExactDate('2021-01-28T05:47:52.134Z')).toBeUndefined(); }); - it('removes rounding option from relative time', () => { + ['s', 'm', 'h', 'd', 'w'].map((roundingOption) => + it(`removes /${roundingOption} rounding option from relative time`, () => { + const spy = jest.spyOn(datemath, 'parse'); + helpers.getExactDate(`now/${roundingOption}`); + expect(spy).toHaveBeenCalledWith('now', {}); + }) + ); + + it('removes rounding option but keeps subtracting time', () => { const spy = jest.spyOn(datemath, 'parse'); - helpers.getExactDate('now/d'); - expect(spy).toHaveBeenCalledWith('now', {}); helpers.getExactDate('now-24h/h'); expect(spy).toHaveBeenCalledWith('now-24h', {}); }); + + it('removes rounding option but keeps adding time', () => { + const spy = jest.spyOn(datemath, 'parse'); + helpers.getExactDate('now+15m/h'); + expect(spy).toHaveBeenCalledWith('now+15m', {}); + }); }); }); diff --git a/x-pack/plugins/apm/public/context/url_params_context/helpers.ts b/x-pack/plugins/apm/public/context/url_params_context/helpers.ts index 7960d9ab25553a..795ffe8796a70c 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/helpers.ts +++ b/x-pack/plugins/apm/public/context/url_params_context/helpers.ts @@ -23,7 +23,8 @@ export function getExactDate(rawDate?: string, options = {}) { if (rawDate) { const isRelativeDate = rawDate.substring(0, 3) === 'now'; if (isRelativeDate) { - const rawDateWithouRounding = rawDate.replace(/\/(\w)$/, ''); + // remove rounding from relative dates "Today" (now/d) and "This week" (now/w) + const rawDateWithouRounding = rawDate.replace(/\/([smhdw])$/, ''); return getParsedDate(rawDateWithouRounding, options); } } @@ -63,9 +64,8 @@ export function getDateRange({ // rounds down start to minute const roundedStart = moment(start).startOf('minute'); - // rounds down to minute const exactStart = getExactDate(rangeFrom) || roundedStart; - const exactEnd = getExactDate(rangeTo, { roundUp: true }) || end; + const exactEnd = getExactDate(rangeTo) || end; return { start: roundedStart.toISOString(), From c30cfdf590c7630423e03f783e6d93c9d447ae8d Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Wed, 16 Jun 2021 09:24:12 -0400 Subject: [PATCH 5/5] addressing PR comments --- .../url_params_context/helpers.test.ts | 26 +++++++++-------- .../context/url_params_context/helpers.ts | 28 +++++++++---------- 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts b/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts index 54ec68947607da..784b10b3f3ee1e 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts +++ b/x-pack/plugins/apm/public/context/url_params_context/helpers.test.ts @@ -25,8 +25,8 @@ describe('url_params_context helpers', () => { }) ).toEqual({ start: '2021-01-28T05:47:00.000Z', - exactStart: '2021-01-28T05:47:00.000Z', end: '2021-01-28T05:48:55.304Z', + exactStart: '2021-01-28T05:47:52.134Z', exactEnd: '2021-01-28T05:48:55.304Z', }); }); @@ -41,8 +41,8 @@ describe('url_params_context helpers', () => { }) ).toEqual({ start: '2021-01-27T05:46:00.000Z', - exactStart: '2021-01-27T05:46:00.000Z', end: '2021-01-28T05:46:13.367Z', + exactStart: '2021-01-27T05:46:07.377Z', exactEnd: '2021-01-28T05:46:13.367Z', }); }); @@ -58,8 +58,8 @@ describe('url_params_context helpers', () => { }) ).toEqual({ start: '2020-01-28T05:52:00.000Z', - exactStart: '2020-01-28T05:52:00.000Z', end: '2021-01-28T05:52:39.741Z', + exactStart: '2020-01-28T05:52:36.290Z', exactEnd: '2021-01-28T05:52:39.741Z', }); }); @@ -83,8 +83,8 @@ describe('url_params_context helpers', () => { }) ).toEqual({ start: '1970-01-01T00:00:00.000Z', - exactStart: '1970-01-01T00:00:00.000Z', end: '1971-01-01T00:00:00.000Z', + exactStart: '1970-01-01T00:00:00.000Z', exactEnd: '1971-01-01T00:00:00.000Z', }); }); @@ -106,19 +106,22 @@ describe('url_params_context helpers', () => { }) ).toEqual({ start: '1972-01-01T00:00:00.000Z', - exactStart: '1972-01-01T00:00:00.000Z', end: '1973-01-01T00:00:00.000Z', - exactEnd: '1973-01-01T00:00:00.000Z', + exactStart: undefined, + exactEnd: undefined, }); }); }); describe('when the start or end are invalid', () => { it('returns the previous state', () => { + const endDate = moment('2021-06-04T18:03:24.211Z'); jest .spyOn(datemath, 'parse') .mockReturnValueOnce(undefined) - .mockReturnValueOnce(moment('2021-06-04T18:03:24.211Z')); + .mockReturnValueOnce(endDate) + .mockReturnValueOnce(undefined) + .mockReturnValueOnce(endDate); expect( helpers.getDateRange({ state: { @@ -165,11 +168,10 @@ describe('url_params_context helpers', () => { }); describe('getExactDate', () => { - it('returns undefined when date in not provided', () => { - expect(helpers.getExactDate()).toBeUndefined(); - }); - it('returns undefined when date in not relative', () => { - expect(helpers.getExactDate('2021-01-28T05:47:52.134Z')).toBeUndefined(); + it('returns date when it is not not relative', () => { + expect(helpers.getExactDate('2021-01-28T05:47:52.134Z')).toEqual( + new Date('2021-01-28T05:47:52.134Z') + ); }); ['s', 'm', 'h', 'd', 'w'].map((roundingOption) => diff --git a/x-pack/plugins/apm/public/context/url_params_context/helpers.ts b/x-pack/plugins/apm/public/context/url_params_context/helpers.ts index 795ffe8796a70c..902456bf4ebc07 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/helpers.ts +++ b/x-pack/plugins/apm/public/context/url_params_context/helpers.ts @@ -19,15 +19,14 @@ function getParsedDate(rawDate?: string, options = {}) { } } -export function getExactDate(rawDate?: string, options = {}) { - if (rawDate) { - const isRelativeDate = rawDate.substring(0, 3) === 'now'; - if (isRelativeDate) { - // remove rounding from relative dates "Today" (now/d) and "This week" (now/w) - const rawDateWithouRounding = rawDate.replace(/\/([smhdw])$/, ''); - return getParsedDate(rawDateWithouRounding, options); - } +export function getExactDate(rawDate: string) { + const isRelativeDate = rawDate.startsWith('now'); + if (isRelativeDate) { + // remove rounding from relative dates "Today" (now/d) and "This week" (now/w) + const rawDateWithouRounding = rawDate.replace(/\/([smhdw])$/, ''); + return getParsedDate(rawDateWithouRounding); } + return getParsedDate(rawDate); } export function getDateRange({ @@ -51,27 +50,28 @@ export function getDateRange({ const start = getParsedDate(rangeFrom); const end = getParsedDate(rangeTo, { roundUp: true }); + const exactStart = rangeFrom ? getExactDate(rangeFrom) : undefined; + const exactEnd = rangeTo ? getExactDate(rangeTo) : undefined; + // `getParsedDate` will return undefined for invalid or empty dates. We return // the previous state if either date is undefined. if (!start || !end) { return { start: state.start, - exactStart: state.start, end: state.end, - exactEnd: state.end, + exactStart: state.exactStart, + exactEnd: state.exactEnd, }; } // rounds down start to minute const roundedStart = moment(start).startOf('minute'); - const exactStart = getExactDate(rangeFrom) || roundedStart; - const exactEnd = getExactDate(rangeTo) || end; return { start: roundedStart.toISOString(), - exactStart: exactStart.toISOString(), end: end.toISOString(), - exactEnd: exactEnd.toISOString(), + exactStart: exactStart?.toISOString(), + exactEnd: exactEnd?.toISOString(), }; }