From 9ab16bbd582020a7852d1898a72d186ee475eb52 Mon Sep 17 00:00:00 2001 From: Dima Arnautov Date: Thu, 8 Oct 2020 16:39:26 +0200 Subject: [PATCH] [ML] Fix job selection flyout (#79850) (#79994) * [ML] fix job selection flyout * [ML] hide time range column * [ML] show callout when no AD jobs presented * [ML] close job selector flyout on navigating away from the dashboard * [ML] add Create job button * [ML] fix mocks * [ML] add unit test for callout --- .../components/job_selector/job_selector.tsx | 30 ++- .../job_selector/job_selector_flyout.tsx | 255 +++++++++--------- .../job_selector_table/job_selector_table.js | 64 ++++- .../job_selector_table.test.js | 20 +- .../contexts/kibana/__mocks__/index.ts | 7 + .../kibana/__mocks__/kibana_context.ts | 15 ++ .../anomaly_swimlane_setup_flyout.tsx | 22 +- 7 files changed, 260 insertions(+), 153 deletions(-) create mode 100644 x-pack/plugins/ml/public/application/contexts/kibana/__mocks__/index.ts create mode 100644 x-pack/plugins/ml/public/application/contexts/kibana/__mocks__/kibana_context.ts diff --git a/x-pack/plugins/ml/public/application/components/job_selector/job_selector.tsx b/x-pack/plugins/ml/public/application/components/job_selector/job_selector.tsx index 6bdf2fdb7caa2..a00284860d668 100644 --- a/x-pack/plugins/ml/public/application/components/job_selector/job_selector.tsx +++ b/x-pack/plugins/ml/public/application/components/job_selector/job_selector.tsx @@ -6,14 +6,18 @@ import React, { useState, useEffect } from 'react'; -import { EuiButtonEmpty, EuiFlexItem, EuiFlexGroup } from '@elastic/eui'; +import { EuiButtonEmpty, EuiFlexItem, EuiFlexGroup, EuiFlyout } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { Dictionary } from '../../../../common/types/common'; import { useUrlState } from '../../util/url_state'; // @ts-ignore import { IdBadges } from './id_badges/index'; -import { BADGE_LIMIT, JobSelectorFlyout, JobSelectorFlyoutProps } from './job_selector_flyout'; +import { + BADGE_LIMIT, + JobSelectorFlyoutContent, + JobSelectorFlyoutProps, +} from './job_selector_flyout'; import { MlJobWithTimeRange } from '../../../../common/types/anomaly_detection_jobs'; interface GroupObj { @@ -163,16 +167,18 @@ export function JobSelector({ dateFormatTz, singleSelection, timeseriesOnly }: J function renderFlyout() { if (isFlyoutVisible) { return ( - + + + ); } } diff --git a/x-pack/plugins/ml/public/application/components/job_selector/job_selector_flyout.tsx b/x-pack/plugins/ml/public/application/components/job_selector/job_selector_flyout.tsx index 6c57b3d08180d..1e8ac4c15fd15 100644 --- a/x-pack/plugins/ml/public/application/components/job_selector/job_selector_flyout.tsx +++ b/x-pack/plugins/ml/public/application/components/job_selector/job_selector_flyout.tsx @@ -11,12 +11,13 @@ import { EuiButtonEmpty, EuiFlexItem, EuiFlexGroup, - EuiFlyout, EuiFlyoutBody, EuiFlyoutFooter, EuiFlyoutHeader, EuiSwitch, EuiTitle, + EuiResizeObserver, + EuiProgress, } from '@elastic/eui'; import { NewSelectionIdBadges } from './new_selection_id_badges'; // @ts-ignore @@ -39,7 +40,6 @@ export interface JobSelectorFlyoutProps { newSelection?: string[]; onFlyoutClose: () => void; onJobsFetched?: (maps: JobSelectionMaps) => void; - onSelectionChange?: (newSelection: string[]) => void; onSelectionConfirmed: (payload: { newSelection: string[]; jobIds: string[]; @@ -52,13 +52,12 @@ export interface JobSelectorFlyoutProps { withTimeRangeSelector?: boolean; } -export const JobSelectorFlyout: FC = ({ +export const JobSelectorFlyoutContent: FC = ({ dateFormatTz, selectedIds = [], singleSelection, timeseriesOnly, onJobsFetched, - onSelectionChange, onSelectionConfirmed, onFlyoutClose, maps, @@ -73,6 +72,7 @@ export const JobSelectorFlyout: FC = ({ const [newSelection, setNewSelection] = useState(selectedIds); + const [isLoading, setIsLoading] = useState(true); const [showAllBadges, setShowAllBadges] = useState(false); const [applyTimeRange, setApplyTimeRange] = useState(true); const [jobs, setJobs] = useState([]); @@ -80,7 +80,7 @@ export const JobSelectorFlyout: FC = ({ const [ganttBarWidth, setGanttBarWidth] = useState(DEFAULT_GANTT_BAR_WIDTH); const [jobGroupsMaps, setJobGroupsMaps] = useState(maps); - const flyoutEl = useRef<{ flyout: HTMLElement }>(null); + const flyoutEl = useRef(null); function applySelection() { // allNewSelection will be a list of all job ids (including those from groups) selected from the table @@ -131,19 +131,19 @@ export const JobSelectorFlyout: FC = ({ // Wrap handleResize in useCallback as it is a dependency for useEffect on line 131 below. // Not wrapping it would cause this dependency to change on every render const handleResize = useCallback(() => { - if (jobs.length > 0 && flyoutEl && flyoutEl.current && flyoutEl.current.flyout) { - // get all cols in flyout table - const tableHeaderCols: NodeListOf = flyoutEl.current.flyout.querySelectorAll( - 'table thead th' - ); - // get the width of the last col - const derivedWidth = tableHeaderCols[tableHeaderCols.length - 1].offsetWidth - 16; - const normalizedJobs = normalizeTimes(jobs, dateFormatTz, derivedWidth); - setJobs(normalizedJobs); - const { groups: updatedGroups } = getGroupsFromJobs(normalizedJobs); - setGroups(updatedGroups); - setGanttBarWidth(derivedWidth); - } + if (jobs.length === 0 || !flyoutEl.current) return; + + // get all cols in flyout table + const tableHeaderCols: NodeListOf = flyoutEl.current.querySelectorAll( + 'table thead th' + ); + // get the width of the last col + const derivedWidth = tableHeaderCols[tableHeaderCols.length - 1].offsetWidth - 16; + const normalizedJobs = normalizeTimes(jobs, dateFormatTz, derivedWidth); + setJobs(normalizedJobs); + const { groups: updatedGroups } = getGroupsFromJobs(normalizedJobs); + setGroups(updatedGroups); + setGanttBarWidth(derivedWidth); }, [dateFormatTz, jobs]); // Fetch jobs list on flyout open @@ -172,119 +172,124 @@ export const JobSelectorFlyout: FC = ({ }), }); } + setIsLoading(false); } - useEffect(() => { - // Ensure ganttBar width gets calculated on resize - window.addEventListener('resize', handleResize); - - return () => { - window.removeEventListener('resize', handleResize); - }; - }, [handleResize]); - - useEffect(() => { - handleResize(); - }, [handleResize, jobs]); - return ( - - - -

- {i18n.translate('xpack.ml.jobSelector.flyoutTitle', { - defaultMessage: 'Job selection', - })} -

- - - - - - - setShowAllBadges(!showAllBadges)} - showAllBadges={showAllBadges} - /> - - - - + + {(resizeRef) => ( + { + flyoutEl.current = e; + resizeRef(e); + }} + aria-labelledby="jobSelectorFlyout" + data-test-subj="mlFlyoutJobSelector" + > + + +

+ {i18n.translate('xpack.ml.jobSelector.flyoutTitle', { + defaultMessage: 'Job selection', + })} +

+ + + + {isLoading ? ( + + ) : ( + <> + + + + setShowAllBadges(!showAllBadges)} + showAllBadges={showAllBadges} + /> + + + + + + {!singleSelection && newSelection.length > 0 && ( + + {i18n.translate('xpack.ml.jobSelector.clearAllFlyoutButton', { + defaultMessage: 'Clear all', + })} + + )} + + {withTimeRangeSelector && ( + + + + )} + + + + + + )} + + + - {!singleSelection && newSelection.length > 0 && ( - - {i18n.translate('xpack.ml.jobSelector.clearAllFlyoutButton', { - defaultMessage: 'Clear all', - })} - - )} + + {i18n.translate('xpack.ml.jobSelector.applyFlyoutButton', { + defaultMessage: 'Apply', + })} + + + + + {i18n.translate('xpack.ml.jobSelector.closeFlyoutButton', { + defaultMessage: 'Close', + })} + - {withTimeRangeSelector && ( - - - - )} - - - - - - - - - {i18n.translate('xpack.ml.jobSelector.applyFlyoutButton', { - defaultMessage: 'Apply', - })} - - - - - {i18n.translate('xpack.ml.jobSelector.closeFlyoutButton', { - defaultMessage: 'Close', - })} - - + - - + )} + ); }; diff --git a/x-pack/plugins/ml/public/application/components/job_selector/job_selector_table/job_selector_table.js b/x-pack/plugins/ml/public/application/components/job_selector/job_selector_table/job_selector_table.js index 1136487485f17..809c1e7df361a 100644 --- a/x-pack/plugins/ml/public/application/components/job_selector/job_selector_table/job_selector_table.js +++ b/x-pack/plugins/ml/public/application/components/job_selector/job_selector_table/job_selector_table.js @@ -9,11 +9,22 @@ import { PropTypes } from 'prop-types'; import { CustomSelectionTable } from '../../custom_selection_table'; import { JobSelectorBadge } from '../job_selector_badge'; import { TimeRangeBar } from '../timerange_bar'; +import { FormattedMessage } from '@kbn/i18n/react'; -import { EuiFlexGroup, EuiFlexItem, EuiLoadingSpinner, EuiTabbedContent } from '@elastic/eui'; +import { + EuiFlexGroup, + EuiFlexItem, + EuiTabbedContent, + EuiCallOut, + EuiButton, + EuiText, +} from '@elastic/eui'; import { LEFT_ALIGNMENT, CENTER_ALIGNMENT, SortableProperties } from '@elastic/eui/lib/services'; import { i18n } from '@kbn/i18n'; +import { useMlKibana } from '../../../contexts/kibana'; +import { ML_PAGES } from '../../../../../common/constants/ml_url_generator'; +import { PLUGIN_ID } from '../../../../../common/constants/app'; const JOB_FILTER_FIELDS = ['job_id', 'groups']; const GROUP_FILTER_FIELDS = ['id']; @@ -26,10 +37,17 @@ export function JobSelectorTable({ selectedIds, singleSelection, timeseriesOnly, + withTimeRangeSelector, }) { const [sortableProperties, setSortableProperties] = useState(); const [currentTab, setCurrentTab] = useState('Jobs'); + const { + services: { + application: { navigateToApp }, + }, + } = useMlKibana(); + useEffect(() => { let sortablePropertyItems = []; let defaultSortProperty = 'job_id'; @@ -125,15 +143,18 @@ export function JobSelectorTable({ )), }, - { + ]; + + if (withTimeRangeSelector) { + columns.push({ label: 'time range', id: 'timerange', alignment: LEFT_ALIGNMENT, render: ({ timeRange = {}, isRunning }) => ( ), - }, - ]; + }); + } const filters = [ { @@ -190,15 +211,18 @@ export function JobSelectorTable({ alignment: CENTER_ALIGNMENT, render: ({ jobIds = [] }) => jobIds.length, }, - { + ]; + + if (withTimeRangeSelector) { + groupColumns.push({ label: 'time range', id: 'timerange', alignment: LEFT_ALIGNMENT, render: ({ timeRange = {} }) => ( ), - }, - ]; + }); + } return ( { + await navigateToApp(PLUGIN_ID, { path: ML_PAGES.ANOMALY_DETECTION_CREATE_JOB }); + }; + return ( - {jobs.length === 0 && } + {jobs.length === 0 && ( + + } + iconType="iInCircle" + > + + + + + + + )} {jobs.length !== 0 && singleSelection === true && renderJobsTable()} {jobs.length !== 0 && !singleSelection && renderTabs()} @@ -242,4 +289,5 @@ JobSelectorTable.propTypes = { selectedIds: PropTypes.array.isRequired, singleSelection: PropTypes.bool, timeseriesOnly: PropTypes.bool, + withTimeRangeSelector: PropTypes.bool, }; diff --git a/x-pack/plugins/ml/public/application/components/job_selector/job_selector_table/job_selector_table.test.js b/x-pack/plugins/ml/public/application/components/job_selector/job_selector_table/job_selector_table.test.js index 41e510459fcea..7e466096fd882 100644 --- a/x-pack/plugins/ml/public/application/components/job_selector/job_selector_table/job_selector_table.test.js +++ b/x-pack/plugins/ml/public/application/components/job_selector/job_selector_table/job_selector_table.test.js @@ -5,14 +5,11 @@ */ import React from 'react'; +import { I18nProvider } from '@kbn/i18n/react'; import { fireEvent, render } from '@testing-library/react'; // eslint-disable-line import/no-extraneous-dependencies import { JobSelectorTable } from './job_selector_table'; -jest.mock('../../../services/job_service', () => ({ - mlJobService: { - getJob: jest.fn(), - }, -})); +jest.mock('../../../contexts/kibana'); const props = { ganttBarWidth: 299, @@ -124,6 +121,19 @@ describe('JobSelectorTable', () => { }); describe('Not Single Selection', () => { + test('renders callout when no jobs provided', () => { + const propsEmptyJobs = { ...props, jobs: [], groupsList: [] }; + const { getByText } = render( + + + + ); + const calloutMessage = getByText('No anomaly detection jobs found'); + const createJobButton = getByText('Create job'); + expect(createJobButton).toBeDefined(); + expect(calloutMessage).toBeDefined(); + }); + test('renders tabs when not singleSelection', () => { const { getAllByRole } = render(); const tabs = getAllByRole('tab'); diff --git a/x-pack/plugins/ml/public/application/contexts/kibana/__mocks__/index.ts b/x-pack/plugins/ml/public/application/contexts/kibana/__mocks__/index.ts new file mode 100644 index 0000000000000..7051abe6dc34e --- /dev/null +++ b/x-pack/plugins/ml/public/application/contexts/kibana/__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 { useMlKibana } from './kibana_context'; diff --git a/x-pack/plugins/ml/public/application/contexts/kibana/__mocks__/kibana_context.ts b/x-pack/plugins/ml/public/application/contexts/kibana/__mocks__/kibana_context.ts new file mode 100644 index 0000000000000..9120585f525cc --- /dev/null +++ b/x-pack/plugins/ml/public/application/contexts/kibana/__mocks__/kibana_context.ts @@ -0,0 +1,15 @@ +/* + * 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 const useMlKibana = jest.fn(() => { + return { + services: { + application: { + navigateToApp: jest.fn(), + }, + }, + }; +}); diff --git a/x-pack/plugins/ml/public/embeddables/anomaly_swimlane/anomaly_swimlane_setup_flyout.tsx b/x-pack/plugins/ml/public/embeddables/anomaly_swimlane/anomaly_swimlane_setup_flyout.tsx index 3a3597a7fa927..8e591d8bdbcb2 100644 --- a/x-pack/plugins/ml/public/embeddables/anomaly_swimlane/anomaly_swimlane_setup_flyout.tsx +++ b/x-pack/plugins/ml/public/embeddables/anomaly_swimlane/anomaly_swimlane_setup_flyout.tsx @@ -7,25 +7,33 @@ import React from 'react'; import { CoreStart } from 'kibana/public'; import moment from 'moment'; +import { takeUntil } from 'rxjs/operators'; +import { from } from 'rxjs'; import { VIEW_BY_JOB_LABEL } from '../../application/explorer/explorer_constants'; import { KibanaContextProvider, toMountPoint, } from '../../../../../../src/plugins/kibana_react/public'; import { AnomalySwimlaneInitializer } from './anomaly_swimlane_initializer'; -import { JobSelectorFlyout } from '../../application/components/job_selector/job_selector_flyout'; +import { JobSelectorFlyoutContent } from '../../application/components/job_selector/job_selector_flyout'; import { AnomalyDetectorService } from '../../application/services/anomaly_detector_service'; import { getInitialGroupsMap } from '../../application/components/job_selector/job_selector'; import { getDefaultPanelTitle } from './anomaly_swimlane_embeddable'; import { getMlGlobalServices } from '../../application/app'; import { HttpService } from '../../application/services/http_service'; +import { DashboardConstants } from '../../../../../../src/plugins/dashboard/public'; import { AnomalySwimlaneEmbeddableInput } from '..'; export async function resolveAnomalySwimlaneUserInput( coreStart: CoreStart, input?: AnomalySwimlaneEmbeddableInput ): Promise> { - const { http, uiSettings, overlays } = coreStart; + const { + http, + uiSettings, + overlays, + application: { currentAppId$ }, + } = coreStart; const anomalyDetectorService = new AnomalyDetectorService(new HttpService(http)); @@ -43,7 +51,7 @@ export async function resolveAnomalySwimlaneUserInput( const flyoutSession = coreStart.overlays.openFlyout( toMountPoint( - { + if (appId !== DashboardConstants.DASHBOARDS_ID) { + flyoutSession.close(); + } + }); }); }