Round start and end values (#89030) (#90035)

smith · web-flow · commit 3e2c484538ca · 2021-02-02T13:06:47.000-06:00
When getting the start and end times, use the d3 time scale `ticks` function to round the start and end times. Example from a query: Before: ```json { "range": { "@timestamp": { "gte": 1611262874814, "lte": 1611263774814, "format": "epoch_millis" } } }, ``` After: ```json { "range": { "@timestamp": { "gte": 1611263040000, "lte": 1611263880000, "format": "epoch_millis" } } }, ``` The `ticks` function makes it so the amount of rounding is proportional to the size of the time range, so shorter time ranges will be rounded less. Also fix a bug where invalid ranges in the query string were not handled correctly. Fixes #84530.
diff --git a/x-pack/plugins/apm/public/components/app/TraceLink/trace_link.test.tsx b/x-pack/plugins/apm/public/components/app/TraceLink/trace_link.test.tsx
@@ -60,12 +60,13 @@ describe('TraceLink', () => {
   describe('when no transaction is found', () => {
     it('renders a trace page', () => {
       jest.spyOn(urlParamsHooks, 'useUrlParams').mockReturnValue({
+        rangeId: 0,
+        refreshTimeRange: jest.fn(),
+        uiFilters: {},
         urlParams: {
           rangeFrom: 'now-24h',
           rangeTo: 'now',
         },
-        refreshTimeRange: jest.fn(),
-        uiFilters: {},
       });
       jest.spyOn(hooks, 'useFetcher').mockReturnValue({
         data: { transaction: undefined },
@@ -87,12 +88,13 @@ describe('TraceLink', () => {
   describe('transaction page', () => {
     beforeAll(() => {
       jest.spyOn(urlParamsHooks, 'useUrlParams').mockReturnValue({
+        rangeId: 0,
+        refreshTimeRange: jest.fn(),
+        uiFilters: {},
         urlParams: {
           rangeFrom: 'now-24h',
           rangeTo: 'now',
         },
-        refreshTimeRange: jest.fn(),
-        uiFilters: {},
       });
     });
 
diff --git a/x-pack/plugins/apm/public/components/shared/DatePicker/date_picker.test.tsx b/x-pack/plugins/apm/public/components/shared/DatePicker/date_picker.test.tsx
@@ -31,8 +31,9 @@ function MockUrlParamsProvider({
   return (
     <UrlParamsContext.Provider
       value={{
-        urlParams,
+        rangeId: 0,
         refreshTimeRange: mockRefreshTimeRange,
+        urlParams,
         uiFilters: useUiFilters(urlParams),
       }}
       children={children}
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
@@ -9,29 +9,53 @@ import moment from 'moment-timezone';
 import * as helpers from './helpers';
 
 describe('url_params_context helpers', () => {
-  describe('getParsedDate', () => {
-    describe('given undefined', () => {
-      it('returns undefined', () => {
-        expect(helpers.getParsedDate(undefined)).toBeUndefined();
+  describe('getDateRange', () => {
+    describe('with non-rounded dates', () => {
+      describe('one minute', () => {
+        it('rounds the values', () => {
+          expect(
+            helpers.getDateRange({
+              state: {},
+              rangeFrom: '2021-01-28T05:47:52.134Z',
+              rangeTo: '2021-01-28T05:48:55.304Z',
+            })
+          ).toEqual({
+            start: '2021-01-28T05:47:50.000Z',
+            end: '2021-01-28T05:49:00.000Z',
+          });
+        });
       });
-    });
-
-    describe('given a parsable date', () => {
-      it('returns the parsed date', () => {
-        expect(helpers.getParsedDate('1970-01-01T00:00:00.000Z')).toEqual(
-          '1970-01-01T00:00:00.000Z'
-        );
+      describe('one day', () => {
+        it('rounds the values', () => {
+          expect(
+            helpers.getDateRange({
+              state: {},
+              rangeFrom: '2021-01-27T05:46:07.377Z',
+              rangeTo: '2021-01-28T05:46:13.367Z',
+            })
+          ).toEqual({
+            start: '2021-01-27T03:00:00.000Z',
+            end: '2021-01-28T06:00:00.000Z',
+          });
+        });
       });
-    });
 
-    describe('given a non-parsable date', () => {
-      it('returns null', () => {
-        expect(helpers.getParsedDate('nope')).toEqual(null);
+      describe('one year', () => {
+        it('rounds the values', () => {
+          expect(
+            helpers.getDateRange({
+              state: {},
+              rangeFrom: '2020-01-28T05:52:36.290Z',
+              rangeTo: '2021-01-28T05:52:39.741Z',
+            })
+          ).toEqual({
+            start: '2020-01-01T00:00:00.000Z',
+            end: '2021-02-01T00:00:00.000Z',
+          });
+        });
       });
     });
-  });
 
-  describe('getDateRange', () => {
     describe('when rangeFrom and rangeTo are not changed', () => {
       it('returns the previous state', () => {
         expect(
@@ -52,6 +76,45 @@ describe('url_params_context helpers', () => {
       });
     });
 
+    describe('when rangeFrom or rangeTo are falsy', () => {
+      it('returns the previous state', () => {
+        // Disable console warning about not receiving a valid date for rangeFrom
+        jest.spyOn(console, 'warn').mockImplementationOnce(() => {});
+
+        expect(
+          helpers.getDateRange({
+            state: {
+              start: '1972-01-01T00:00:00.000Z',
+              end: '1973-01-01T00:00:00.000Z',
+            },
+            rangeFrom: '',
+            rangeTo: 'now',
+          })
+        ).toEqual({
+          start: '1972-01-01T00:00:00.000Z',
+          end: '1973-01-01T00:00:00.000Z',
+        });
+      });
+    });
+
+    describe('when the start or end are invalid', () => {
+      it('returns the previous state', () => {
+        expect(
+          helpers.getDateRange({
+            state: {
+              start: '1972-01-01T00:00:00.000Z',
+              end: '1973-01-01T00:00:00.000Z',
+            },
+            rangeFrom: 'nope',
+            rangeTo: 'now',
+          })
+        ).toEqual({
+          start: '1972-01-01T00:00:00.000Z',
+          end: '1973-01-01T00:00:00.000Z',
+        });
+      });
+    });
+
     describe('when rangeFrom or rangeTo have changed', () => {
       it('returns new state', () => {
         jest.spyOn(datemath, 'parse').mockReturnValue(moment(0).utc());
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
@@ -4,15 +4,16 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { compact, pickBy } from 'lodash';
 import datemath from '@elastic/datemath';
+import { scaleUtc } from 'd3-scale';
+import { compact, pickBy } from 'lodash';
 import { IUrlParams } from './types';
 
-export function getParsedDate(rawDate?: string, opts = {}) {
+function getParsedDate(rawDate?: string, options = {}) {
   if (rawDate) {
-    const parsed = datemath.parse(rawDate, opts);
-    if (parsed) {
-      return parsed.toISOString();
+    const parsed = datemath.parse(rawDate, options);
+    if (parsed && parsed.isValid()) {
+      return parsed.toDate();
     }
   }
 }
@@ -26,13 +27,27 @@ export function getDateRange({
   rangeFrom?: string;
   rangeTo?: string;
 }) {
+  // 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 };
   }
 
+  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 };
+  }
+
+  // Calculate ticks for the time ranges to produce nicely rounded values.
+  const ticks = scaleUtc().domain([start, end]).nice().ticks();
+
+  // Return the first and last tick values.
   return {
-    start: getParsedDate(rangeFrom),
-    end: getParsedDate(rangeTo, { roundUp: true }),
+    start: ticks[0].toISOString(),
+    end: ticks[ticks.length - 1].toISOString(),
   };
 }
 
diff --git a/x-pack/plugins/apm/public/context/url_params_context/mock_url_params_context_provider.tsx b/x-pack/plugins/apm/public/context/url_params_context/mock_url_params_context_provider.tsx
@@ -30,8 +30,9 @@ export function MockUrlParamsContextProvider({
   return (
     <UrlParamsContext.Provider
       value={{
-        urlParams,
+        rangeId: 0,
         refreshTimeRange,
+        urlParams,
         uiFilters: useUiFilters(urlParams),
       }}
       children={children}
diff --git a/x-pack/plugins/apm/public/context/url_params_context/url_params_context.test.tsx b/x-pack/plugins/apm/public/context/url_params_context/url_params_context.test.tsx
@@ -4,15 +4,14 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import * as React from 'react';
-import { UrlParamsContext, UrlParamsProvider } from './url_params_context';
+import { waitFor } from '@testing-library/react';
 import { mount } from 'enzyme';
-import { Location, History } from 'history';
-import { MemoryRouter, Router } from 'react-router-dom';
+import { History, Location } from 'history';
 import moment from 'moment-timezone';
+import * as React from 'react';
+import { MemoryRouter, Router } from 'react-router-dom';
 import { IUrlParams } from './types';
-import { getParsedDate } from './helpers';
-import { waitFor } from '@testing-library/react';
+import { UrlParamsContext, UrlParamsProvider } from './url_params_context';
 
 function mountParams(location: Location) {
   return mount(
@@ -33,11 +32,11 @@ function getDataFromOutput(wrapper: ReturnType<typeof mount>) {
 }
 
 describe('UrlParamsContext', () => {
-  beforeEach(() => {
+  beforeAll(() => {
     moment.tz.setDefault('Etc/GMT');
   });
 
-  afterEach(() => {
+  afterAll(() => {
     moment.tz.setDefault('');
   });
 
@@ -50,8 +49,11 @@ describe('UrlParamsContext', () => {
 
     const wrapper = mountParams(location);
     const params = getDataFromOutput(wrapper);
-    expect(params.start).toEqual('2010-03-15T12:00:00.000Z');
-    expect(params.end).toEqual('2010-04-10T12:00:00.000Z');
+
+    expect([params.start, params.end]).toEqual([
+      '2010-03-15T00:00:00.000Z',
+      '2010-04-11T00:00:00.000Z',
+    ]);
   });
 
   it('should update param values if location has changed', () => {
@@ -66,8 +68,11 @@ describe('UrlParamsContext', () => {
     // force an update
     wrapper.setProps({ abc: 123 });
     const params = getDataFromOutput(wrapper);
-    expect(params.start).toEqual('2009-03-15T12:00:00.000Z');
-    expect(params.end).toEqual('2009-04-10T12:00:00.000Z');
+
+    expect([params.start, params.end]).toEqual([
+      '2009-03-15T00:00:00.000Z',
+      '2009-04-11T00:00:00.000Z',
+    ]);
   });
 
   it('should parse relative time ranges on mount', () => {
@@ -76,13 +81,20 @@ describe('UrlParamsContext', () => {
       search: '?rangeFrom=now-1d%2Fd&rangeTo=now-1d%2Fd&transactionId=UPDATED',
     } as Location;
 
+    const nowSpy = jest.spyOn(Date, 'now').mockReturnValue(0);
+
     const wrapper = mountParams(location);
 
     // force an update
     wrapper.setProps({ abc: 123 });
     const params = getDataFromOutput(wrapper);
-    expect(params.start).toEqual(getParsedDate('now-1d/d'));
-    expect(params.end).toEqual(getParsedDate('now-1d/d', { roundUp: true }));
+
+    expect([params.start, params.end]).toEqual([
+      '1969-12-31T00:00:00.000Z',
+      '1970-01-01T00:00:00.000Z',
+    ]);
+
+    nowSpy.mockRestore();
   });
 
   it('should refresh the time range with new values', async () => {
@@ -130,8 +142,11 @@ describe('UrlParamsContext', () => {
     expect(calls.length).toBe(2);
 
     const params = getDataFromOutput(wrapper);
-    expect(params.start).toEqual('2005-09-20T12:00:00.000Z');
-    expect(params.end).toEqual('2005-10-21T12:00:00.000Z');
+
+    expect([params.start, params.end]).toEqual([
+      '2005-09-19T00:00:00.000Z',
+      '2005-10-23T00:00:00.000Z',
+    ]);
   });
 
   it('should refresh the time range with new values if time range is relative', async () => {
@@ -177,7 +192,10 @@ describe('UrlParamsContext', () => {
     await waitFor(() => {});
 
     const params = getDataFromOutput(wrapper);
-    expect(params.start).toEqual('2000-06-14T00:00:00.000Z');
-    expect(params.end).toEqual('2000-06-14T23:59:59.999Z');
+
+    expect([params.start, params.end]).toEqual([
+      '2000-06-14T00:00:00.000Z',
+      '2000-06-15T00:00:00.000Z',
+    ]);
   });
 });
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
diff --git a/x-pack/plugins/apm/public/hooks/use_fetcher.tsx b/x-pack/plugins/apm/public/hooks/use_fetcher.tsx
