From 09a6c9de3b9b06381a0b338b35b03c596e719248 Mon Sep 17 00:00:00 2001 From: Shahzad Date: Mon, 10 Aug 2020 17:20:36 +0200 Subject: [PATCH] [7.x] [Uptime] Fix full reloads while navigating to alert/ml (#73796) (#74651) Co-authored-by: Elastic Machine Co-authored-by: Elastic Machine --- .../plugins/uptime/public/apps/uptime_app.tsx | 15 ++- .../__tests__/link_events.test.ts | 102 ++++++++++++++++++ .../__tests__/link_for_eui.test.tsx | 77 +++++++++++++ .../common/react_router_helpers/index.ts | 12 +++ .../react_router_helpers/link_events.ts | 31 ++++++ .../react_router_helpers/link_for_eui.tsx | 74 +++++++++++++ .../ml_integerations.test.tsx.snap | 5 +- .../__snapshots__/ml_manage_job.test.tsx.snap | 5 +- .../components/monitor/ml/manage_ml_job.tsx | 8 +- .../components/monitor/ml/translations.tsx | 8 ++ .../uptime/public/lib/__mocks__/index.ts | 7 ++ .../__mocks__/react_router_history.mock.ts | 25 +++++ .../uptime/public/pages/certificates.tsx | 8 +- .../uptime/public/pages/page_header.tsx | 9 +- .../plugins/uptime/public/pages/settings.tsx | 10 +- .../__test__/get_histogram_interval.test.ts | 4 +- 16 files changed, 371 insertions(+), 29 deletions(-) create mode 100644 x-pack/plugins/uptime/public/components/common/react_router_helpers/__tests__/link_events.test.ts create mode 100644 x-pack/plugins/uptime/public/components/common/react_router_helpers/__tests__/link_for_eui.test.tsx create mode 100644 x-pack/plugins/uptime/public/components/common/react_router_helpers/index.ts create mode 100644 x-pack/plugins/uptime/public/components/common/react_router_helpers/link_events.ts create mode 100644 x-pack/plugins/uptime/public/components/common/react_router_helpers/link_for_eui.tsx create mode 100644 x-pack/plugins/uptime/public/lib/__mocks__/index.ts create mode 100644 x-pack/plugins/uptime/public/lib/__mocks__/react_router_history.mock.ts diff --git a/x-pack/plugins/uptime/public/apps/uptime_app.tsx b/x-pack/plugins/uptime/public/apps/uptime_app.tsx index 41370f9fff492..1dc34b44b7c64 100644 --- a/x-pack/plugins/uptime/public/apps/uptime_app.tsx +++ b/x-pack/plugins/uptime/public/apps/uptime_app.tsx @@ -10,7 +10,10 @@ import React, { useEffect } from 'react'; import { Provider as ReduxProvider } from 'react-redux'; import { BrowserRouter as Router } from 'react-router-dom'; import { I18nStart, ChromeBreadcrumb, CoreStart } from 'kibana/public'; -import { KibanaContextProvider } from '../../../../../src/plugins/kibana_react/public'; +import { + KibanaContextProvider, + RedirectAppLinks, +} from '../../../../../src/plugins/kibana_react/public'; import { ClientPluginsSetup, ClientPluginsStart } from './plugin'; import { UMUpdateBadge } from '../lib/lib'; import { @@ -103,10 +106,12 @@ const Application = (props: UptimeAppProps) => { -
- - -
+ +
+ + +
+ diff --git a/x-pack/plugins/uptime/public/components/common/react_router_helpers/__tests__/link_events.test.ts b/x-pack/plugins/uptime/public/components/common/react_router_helpers/__tests__/link_events.test.ts new file mode 100644 index 0000000000000..3e857c7c20904 --- /dev/null +++ b/x-pack/plugins/uptime/public/components/common/react_router_helpers/__tests__/link_events.test.ts @@ -0,0 +1,102 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { letBrowserHandleEvent } from '../index'; + +describe('letBrowserHandleEvent', () => { + const event = { + defaultPrevented: false, + metaKey: false, + altKey: false, + ctrlKey: false, + shiftKey: false, + button: 0, + target: { + getAttribute: () => '_self', + }, + } as any; + + describe('the browser should handle the link when', () => { + it('default is prevented', () => { + expect(letBrowserHandleEvent({ ...event, defaultPrevented: true })).toBe(true); + }); + + it('is modified with metaKey', () => { + expect(letBrowserHandleEvent({ ...event, metaKey: true })).toBe(true); + }); + + it('is modified with altKey', () => { + expect(letBrowserHandleEvent({ ...event, altKey: true })).toBe(true); + }); + + it('is modified with ctrlKey', () => { + expect(letBrowserHandleEvent({ ...event, ctrlKey: true })).toBe(true); + }); + + it('is modified with shiftKey', () => { + expect(letBrowserHandleEvent({ ...event, shiftKey: true })).toBe(true); + }); + + it('it is not a left click event', () => { + expect(letBrowserHandleEvent({ ...event, button: 2 })).toBe(true); + }); + + it('the target is anything value other than _self', () => { + expect( + letBrowserHandleEvent({ + ...event, + target: targetValue('_blank'), + }) + ).toBe(true); + }); + }); + + describe('the browser should NOT handle the link when', () => { + it('default is not prevented', () => { + expect(letBrowserHandleEvent({ ...event, defaultPrevented: false })).toBe(false); + }); + + it('is not modified', () => { + expect( + letBrowserHandleEvent({ + ...event, + metaKey: false, + altKey: false, + ctrlKey: false, + shiftKey: false, + }) + ).toBe(false); + }); + + it('it is a left click event', () => { + expect(letBrowserHandleEvent({ ...event, button: 0 })).toBe(false); + }); + + it('the target is a value of _self', () => { + expect( + letBrowserHandleEvent({ + ...event, + target: targetValue('_self'), + }) + ).toBe(false); + }); + + it('the target has no value', () => { + expect( + letBrowserHandleEvent({ + ...event, + target: targetValue(null), + }) + ).toBe(false); + }); + }); +}); + +const targetValue = (value: string | null) => { + return { + getAttribute: () => value, + }; +}; diff --git a/x-pack/plugins/uptime/public/components/common/react_router_helpers/__tests__/link_for_eui.test.tsx b/x-pack/plugins/uptime/public/components/common/react_router_helpers/__tests__/link_for_eui.test.tsx new file mode 100644 index 0000000000000..4a681f6fa60bf --- /dev/null +++ b/x-pack/plugins/uptime/public/components/common/react_router_helpers/__tests__/link_for_eui.test.tsx @@ -0,0 +1,77 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React from 'react'; +import { shallow, mount } from 'enzyme'; +import { EuiLink, EuiButton } from '@elastic/eui'; + +import '../../../../lib/__mocks__/react_router_history.mock'; + +import { ReactRouterEuiLink, ReactRouterEuiButton } from '../link_for_eui'; +import { mockHistory } from '../../../../lib/__mocks__'; + +describe('EUI & React Router Component Helpers', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('renders', () => { + const wrapper = shallow(); + + expect(wrapper.find(EuiLink)).toHaveLength(1); + }); + + it('renders an EuiButton', () => { + const wrapper = shallow(); + + expect(wrapper.find(EuiButton)).toHaveLength(1); + }); + + it('passes down all ...rest props', () => { + const wrapper = shallow(); + const link = wrapper.find(EuiLink); + + expect(link.prop('external')).toEqual(true); + expect(link.prop('data-test-subj')).toEqual('foo'); + }); + + it('renders with the correct href and onClick props', () => { + const wrapper = mount(); + const link = wrapper.find(EuiLink); + + expect(link.prop('onClick')).toBeInstanceOf(Function); + expect(link.prop('href')).toEqual('/enterprise_search/foo/bar'); + expect(mockHistory.createHref).toHaveBeenCalled(); + }); + + describe('onClick', () => { + it('prevents default navigation and uses React Router history', () => { + const wrapper = mount(); + + const simulatedEvent = { + button: 0, + target: { getAttribute: () => '_self' }, + preventDefault: jest.fn(), + }; + wrapper.find(EuiLink).simulate('click', simulatedEvent); + + expect(simulatedEvent.preventDefault).toHaveBeenCalled(); + expect(mockHistory.push).toHaveBeenCalled(); + }); + + it('does not prevent default browser behavior on new tab/window clicks', () => { + const wrapper = mount(); + + const simulatedEvent = { + shiftKey: true, + target: { getAttribute: () => '_blank' }, + }; + wrapper.find(EuiLink).simulate('click', simulatedEvent); + + expect(mockHistory.push).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/x-pack/plugins/uptime/public/components/common/react_router_helpers/index.ts b/x-pack/plugins/uptime/public/components/common/react_router_helpers/index.ts new file mode 100644 index 0000000000000..a1885eaee4cbe --- /dev/null +++ b/x-pack/plugins/uptime/public/components/common/react_router_helpers/index.ts @@ -0,0 +1,12 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +export { letBrowserHandleEvent } from './link_events'; +export { + ReactRouterEuiLink, + ReactRouterEuiButton, + ReactRouterEuiButtonEmpty, +} from './link_for_eui'; diff --git a/x-pack/plugins/uptime/public/components/common/react_router_helpers/link_events.ts b/x-pack/plugins/uptime/public/components/common/react_router_helpers/link_events.ts new file mode 100644 index 0000000000000..93da2ab71d952 --- /dev/null +++ b/x-pack/plugins/uptime/public/components/common/react_router_helpers/link_events.ts @@ -0,0 +1,31 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { MouseEvent } from 'react'; + +/** + * Helper functions for determining which events we should + * let browsers handle natively, e.g. new tabs/windows + */ + +type THandleEvent = (event: MouseEvent) => boolean; + +export const letBrowserHandleEvent: THandleEvent = (event) => + event.defaultPrevented || + isModifiedEvent(event) || + !isLeftClickEvent(event) || + isTargetBlank(event); + +const isModifiedEvent: THandleEvent = (event) => + !!(event.metaKey || event.altKey || event.ctrlKey || event.shiftKey); + +const isLeftClickEvent: THandleEvent = (event) => event.button === 0; + +const isTargetBlank: THandleEvent = (event) => { + const element = event.target as HTMLElement; + const target = element.getAttribute('target'); + return !!target && target !== '_self'; +}; diff --git a/x-pack/plugins/uptime/public/components/common/react_router_helpers/link_for_eui.tsx b/x-pack/plugins/uptime/public/components/common/react_router_helpers/link_for_eui.tsx new file mode 100644 index 0000000000000..7adc8be4533bc --- /dev/null +++ b/x-pack/plugins/uptime/public/components/common/react_router_helpers/link_for_eui.tsx @@ -0,0 +1,74 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React from 'react'; +import { useHistory } from 'react-router-dom'; +import { + EuiLink, + EuiButton, + EuiButtonProps, + EuiButtonEmptyProps, + EuiLinkAnchorProps, + EuiButtonEmpty, +} from '@elastic/eui'; + +import { letBrowserHandleEvent } from './link_events'; + +/** + * Generates either an EuiLink or EuiButton with a React-Router-ified link + * + * Based off of EUI's recommendations for handling React Router: + * https://github.com/elastic/eui/blob/master/wiki/react-router.md#react-router-51 + */ + +interface IEuiReactRouterProps { + to: string; +} + +export const ReactRouterHelperForEui: React.FC = ({ to, children }) => { + const history = useHistory(); + + const onClick = (event: React.MouseEvent) => { + if (letBrowserHandleEvent(event)) return; + + // Prevent regular link behavior, which causes a browser refresh. + event.preventDefault(); + + // Push the route to the history. + history.push(to); + }; + + // Generate the correct link href (with basename etc. accounted for) + const href = history.createHref({ pathname: to }); + + const reactRouterProps = { href, onClick }; + return React.cloneElement(children as React.ReactElement, reactRouterProps); +}; + +type TEuiReactRouterLinkProps = EuiLinkAnchorProps & IEuiReactRouterProps; +type TEuiReactRouterButtonProps = EuiButtonProps & IEuiReactRouterProps; +type TEuiReactRouterButtonEmptyProps = EuiButtonEmptyProps & IEuiReactRouterProps; + +export const ReactRouterEuiLink: React.FC = ({ to, ...rest }) => ( + + + +); + +export const ReactRouterEuiButton: React.FC = ({ to, ...rest }) => ( + + + +); + +export const ReactRouterEuiButtonEmpty: React.FC = ({ + to, + ...rest +}) => ( + + + +); diff --git a/x-pack/plugins/uptime/public/components/monitor/ml/__tests__/__snapshots__/ml_integerations.test.tsx.snap b/x-pack/plugins/uptime/public/components/monitor/ml/__tests__/__snapshots__/ml_integerations.test.tsx.snap index 15f5c03512bf1..e7ad86f72dab6 100644 --- a/x-pack/plugins/uptime/public/components/monitor/ml/__tests__/__snapshots__/ml_integerations.test.tsx.snap +++ b/x-pack/plugins/uptime/public/components/monitor/ml/__tests__/__snapshots__/ml_integerations.test.tsx.snap @@ -8,6 +8,7 @@ exports[`ML Integrations renders without errors 1`] = ` class="euiPopover__anchor" > diff --git a/x-pack/plugins/uptime/public/components/monitor/ml/__tests__/__snapshots__/ml_manage_job.test.tsx.snap b/x-pack/plugins/uptime/public/components/monitor/ml/__tests__/__snapshots__/ml_manage_job.test.tsx.snap index fabe94763e07d..cc3417e09987e 100644 --- a/x-pack/plugins/uptime/public/components/monitor/ml/__tests__/__snapshots__/ml_manage_job.test.tsx.snap +++ b/x-pack/plugins/uptime/public/components/monitor/ml/__tests__/__snapshots__/ml_manage_job.test.tsx.snap @@ -8,6 +8,7 @@ exports[`Manage ML Job renders without errors 1`] = ` class="euiPopover__anchor" > diff --git a/x-pack/plugins/uptime/public/components/monitor/ml/manage_ml_job.tsx b/x-pack/plugins/uptime/public/components/monitor/ml/manage_ml_job.tsx index 7a2899558891d..f4382b37b3d30 100644 --- a/x-pack/plugins/uptime/public/components/monitor/ml/manage_ml_job.tsx +++ b/x-pack/plugins/uptime/public/components/monitor/ml/manage_ml_job.tsx @@ -54,6 +54,10 @@ export const ManageMLJobComponent = ({ hasMLJob, onEnableJob, onJobDelete }: Pro const deleteAnomalyAlert = () => dispatch(deleteAlertAction.get({ alertId: anomalyAlert?.id as string })); + const showLoading = isMLJobCreating || isMLJobLoading; + + const btnText = hasMLJob ? labels.ANOMALY_DETECTION : labels.ENABLE_ANOMALY_DETECTION; + const button = ( - {hasMLJob ? labels.ANOMALY_DETECTION : labels.ENABLE_ANOMALY_DETECTION} + {showLoading ? '' : btnText} ); @@ -79,7 +84,6 @@ export const ManageMLJobComponent = ({ hasMLJob, onEnableJob, onJobDelete }: Pro monitorId, dateRange: { from: dateRangeStart, to: dateRangeEnd }, }), - target: '_blank', }, { name: anomalyAlert ? labels.DISABLE_ANOMALY_ALERT : labels.ENABLE_ANOMALY_ALERT, diff --git a/x-pack/plugins/uptime/public/components/monitor/ml/translations.tsx b/x-pack/plugins/uptime/public/components/monitor/ml/translations.tsx index 90ebdf10a73f5..dfc912e6be9ee 100644 --- a/x-pack/plugins/uptime/public/components/monitor/ml/translations.tsx +++ b/x-pack/plugins/uptime/public/components/monitor/ml/translations.tsx @@ -162,3 +162,11 @@ export const START_TRAIL_DESC = i18n.translate( 'In order to access duration anomaly detection, you have to be subscribed to an Elastic Platinum license.', } ); + +export const ENABLE_MANAGE_JOB = i18n.translate( + 'xpack.uptime.ml.enableAnomalyDetectionPanel.enable_or_manage_job', + { + defaultMessage: + 'You can enable anomaly detection job or if job is already there you can manage the job or alert.', + } +); diff --git a/x-pack/plugins/uptime/public/lib/__mocks__/index.ts b/x-pack/plugins/uptime/public/lib/__mocks__/index.ts new file mode 100644 index 0000000000000..45ef5787927e1 --- /dev/null +++ b/x-pack/plugins/uptime/public/lib/__mocks__/index.ts @@ -0,0 +1,7 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +export { mockHistory } from './react_router_history.mock'; diff --git a/x-pack/plugins/uptime/public/lib/__mocks__/react_router_history.mock.ts b/x-pack/plugins/uptime/public/lib/__mocks__/react_router_history.mock.ts new file mode 100644 index 0000000000000..fd422465d87f1 --- /dev/null +++ b/x-pack/plugins/uptime/public/lib/__mocks__/react_router_history.mock.ts @@ -0,0 +1,25 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +/** + * NOTE: This variable name MUST start with 'mock*' in order for + * Jest to accept its use within a jest.mock() + */ +export const mockHistory = { + createHref: jest.fn(({ pathname }) => `/enterprise_search${pathname}`), + push: jest.fn(), + location: { + pathname: '/current-path', + }, +}; + +jest.mock('react-router-dom', () => ({ + useHistory: jest.fn(() => mockHistory), +})); + +/** + * For example usage, @see public/applications/shared/react_router_helpers/eui_link.test.tsx + */ diff --git a/x-pack/plugins/uptime/public/pages/certificates.tsx b/x-pack/plugins/uptime/public/pages/certificates.tsx index e46d228c6d21f..a524ce6ba9b71 100644 --- a/x-pack/plugins/uptime/public/pages/certificates.tsx +++ b/x-pack/plugins/uptime/public/pages/certificates.tsx @@ -29,6 +29,7 @@ import { certificatesSelector, getCertificatesAction } from '../state/certificat import { CertificateList, CertificateSearch, CertSort } from '../components/certificates'; import { ToggleAlertFlyoutButton } from '../components/overview/alerts/alerts_containers'; import { CLIENT_ALERT_TYPES } from '../../common/constants/alerts'; +import { ReactRouterEuiButtonEmpty } from '../components/common/react_router_helpers'; const DEFAULT_PAGE_SIZE = 10; const LOCAL_STORAGE_KEY = 'xpack.uptime.certList.pageSize'; @@ -79,15 +80,16 @@ export const CertificatesPage: React.FC = () => { <> - {labels.RETURN_TO_OVERVIEW} - + diff --git a/x-pack/plugins/uptime/public/pages/page_header.tsx b/x-pack/plugins/uptime/public/pages/page_header.tsx index 16279a63b5f40..325d82696d47c 100644 --- a/x-pack/plugins/uptime/public/pages/page_header.tsx +++ b/x-pack/plugins/uptime/public/pages/page_header.tsx @@ -7,12 +7,12 @@ import React from 'react'; import { EuiFlexGroup, EuiFlexItem, EuiTitle, EuiSpacer, EuiButtonEmpty } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; -import { useHistory } from 'react-router-dom'; import styled from 'styled-components'; import { UptimeDatePicker } from '../components/common/uptime_date_picker'; import { SETTINGS_ROUTE } from '../../common/constants'; import { ToggleAlertFlyoutButton } from '../components/overview/alerts/alerts_containers'; import { useKibana } from '../../../../../src/plugins/kibana_react/public'; +import { ReactRouterEuiButtonEmpty } from '../components/common/react_router_helpers'; interface PageHeaderProps { headingText: string | JSX.Element; @@ -58,7 +58,6 @@ export const PageHeader = React.memo( ) : null; const kibana = useKibana(); - const history = useHistory(); const extraLinkComponents = !extraLinks ? null : ( @@ -66,13 +65,13 @@ export const PageHeader = React.memo( - {SETTINGS_LINK_TEXT} - + { ); - const history = useHistory(); - return ( <> - {Translations.settings.returnToOverviewLinkLabel} - + diff --git a/x-pack/plugins/uptime/server/lib/helper/__test__/get_histogram_interval.test.ts b/x-pack/plugins/uptime/server/lib/helper/__test__/get_histogram_interval.test.ts index bddca1b863ce4..09b857f37e1df 100644 --- a/x-pack/plugins/uptime/server/lib/helper/__test__/get_histogram_interval.test.ts +++ b/x-pack/plugins/uptime/server/lib/helper/__test__/get_histogram_interval.test.ts @@ -10,11 +10,11 @@ import { assertCloseTo } from '../assert_close_to'; describe('getHistogramInterval', () => { it('specifies the interval necessary to divide a given timespan into equal buckets, rounded to the nearest integer, expressed in ms', () => { const interval = getHistogramInterval('now-15m', 'now', 10); - assertCloseTo(interval, 90000, 10); + assertCloseTo(interval, 90000, 20); }); it('will supply a default constant value for bucketCount when none is provided', () => { const interval = getHistogramInterval('now-15m', 'now'); - assertCloseTo(interval, 36000, 10); + assertCloseTo(interval, 36000, 20); }); });