From 2216b226d6c1bf0b10317bef0d31a597ef12a5e2 Mon Sep 17 00:00:00 2001 From: Tim Sullivan Date: Thu, 19 Jul 2018 14:29:25 -0700 Subject: [PATCH] Monitoring Angular directives to use React components (#19183) * remove some webpack aliases for jest * remove status icon angular directive * fix some component import problems --- .../public/components/beats/beat/beat.js | 4 +- .../components/beats/overview/overview.js | 2 +- .../elasticsearch/indices/indices.js | 2 +- .../components/elasticsearch/nodes/nodes.js | 2 +- .../elasticsearch/overview/overview.js | 2 +- .../monitoring/public/components/index.js | 10 +- .../__snapshots__/summary_status.test.js.snap | 143 ++++++++++++++++++ .../public/components/summary_status/index.js | 62 +------- .../summary_status/summary_status.js | 67 ++++++++ .../summary_status/summary_status.test.js | 69 +++++++++ .../monitoring/public/directives/all.js | 1 - .../public/directives/status_icon/index.js | 57 ------- 12 files changed, 291 insertions(+), 130 deletions(-) create mode 100644 x-pack/plugins/monitoring/public/components/summary_status/__snapshots__/summary_status.test.js.snap create mode 100644 x-pack/plugins/monitoring/public/components/summary_status/summary_status.js create mode 100644 x-pack/plugins/monitoring/public/components/summary_status/summary_status.test.js delete mode 100644 x-pack/plugins/monitoring/public/directives/status_icon/index.js diff --git a/x-pack/plugins/monitoring/public/components/beats/beat/beat.js b/x-pack/plugins/monitoring/public/components/beats/beat/beat.js index e0fbd90eea6d..5ab967da27ac 100644 --- a/x-pack/plugins/monitoring/public/components/beats/beat/beat.js +++ b/x-pack/plugins/monitoring/public/components/beats/beat/beat.js @@ -5,8 +5,8 @@ */ import React, { Fragment } from 'react'; -import { MonitoringTimeseriesContainer } from 'plugins/monitoring/components'; -import { formatMetric } from 'plugins/monitoring/lib/format_number'; +import { MonitoringTimeseriesContainer } from '../../chart'; +import { formatMetric } from '../../../lib/format_number'; function renderTransportAddress(summary) { let output = null; diff --git a/x-pack/plugins/monitoring/public/components/beats/overview/overview.js b/x-pack/plugins/monitoring/public/components/beats/overview/overview.js index 8098effc9752..8540bd0aff51 100644 --- a/x-pack/plugins/monitoring/public/components/beats/overview/overview.js +++ b/x-pack/plugins/monitoring/public/components/beats/overview/overview.js @@ -9,7 +9,7 @@ import { LatestActive } from './latest_active'; import { LatestVersions } from './latest_versions'; import { LatestTypes } from './latest_types'; import { Stats } from '../'; -import { MonitoringTimeseriesContainer } from 'plugins/monitoring/components'; +import { MonitoringTimeseriesContainer } from '../../chart'; import { EuiCallOut } from '@elastic/eui'; function renderLatestActive(latestActive, latestTypes, latestVersions) { diff --git a/x-pack/plugins/monitoring/public/components/elasticsearch/indices/indices.js b/x-pack/plugins/monitoring/public/components/elasticsearch/indices/indices.js index 3412390984e0..bd497142ad84 100644 --- a/x-pack/plugins/monitoring/public/components/elasticsearch/indices/indices.js +++ b/x-pack/plugins/monitoring/public/components/elasticsearch/indices/indices.js @@ -11,7 +11,7 @@ import { LARGE_FLOAT, LARGE_BYTES, LARGE_ABBREVIATED } from '../../../../common/ import { formatMetric } from '../../../lib/format_number'; import { ElasticsearchStatusIcon } from '../status_icon'; import { ClusterStatus } from '../cluster_status'; -import { MonitoringTable } from '../../'; +import { MonitoringTable } from '../../table'; import { EuiLink } from '@elastic/eui'; import { KuiTableRowCell, KuiTableRow } from '@kbn/ui-framework/components'; import { SystemIndicesCheckbox } from './system_indices_checkbox'; diff --git a/x-pack/plugins/monitoring/public/components/elasticsearch/nodes/nodes.js b/x-pack/plugins/monitoring/public/components/elasticsearch/nodes/nodes.js index 1d54e432d18f..f8dd6e2bf697 100644 --- a/x-pack/plugins/monitoring/public/components/elasticsearch/nodes/nodes.js +++ b/x-pack/plugins/monitoring/public/components/elasticsearch/nodes/nodes.js @@ -10,7 +10,7 @@ import { SORT_ASCENDING } from '../../../../common/constants'; import { NodeStatusIcon } from '../node'; import { extractIp } from '../../../lib/extract_ip'; // TODO this is only used for elasticsearch nodes summary / node detail, so it should be moved to components/elasticsearch/nodes/lib import { ClusterStatus } from '../cluster_status'; -import { MonitoringTable } from '../../'; +import { MonitoringTable } from '../../table'; import { MetricCell, OfflineCell } from './cells'; import { EuiLink, EuiToolTip } from '@elastic/eui'; import { KuiTableRowCell, KuiTableRow } from '@kbn/ui-framework/components'; diff --git a/x-pack/plugins/monitoring/public/components/elasticsearch/overview/overview.js b/x-pack/plugins/monitoring/public/components/elasticsearch/overview/overview.js index e981c405b3ab..2c36a1231cb1 100644 --- a/x-pack/plugins/monitoring/public/components/elasticsearch/overview/overview.js +++ b/x-pack/plugins/monitoring/public/components/elasticsearch/overview/overview.js @@ -7,7 +7,7 @@ import React, { Fragment } from 'react'; import { ClusterStatus } from '../cluster_status'; import { ShardActivity } from '../shard_activity'; -import { MonitoringTimeseriesContainer } from 'plugins/monitoring/components'; +import { MonitoringTimeseriesContainer } from '../../chart'; export function ElasticsearchOverview({ clusterStatus, diff --git a/x-pack/plugins/monitoring/public/components/index.js b/x-pack/plugins/monitoring/public/components/index.js index 665ef7b5fce9..ffc0a738513b 100644 --- a/x-pack/plugins/monitoring/public/components/index.js +++ b/x-pack/plugins/monitoring/public/components/index.js @@ -4,13 +4,13 @@ * you may not use this file except in compliance with the Elastic License. */ -export { MonitoringTimeseriesContainer } from './chart'; -export { MonitoringTable } from './table'; -export { Tooltip } from './tooltip'; + +/* + * This file should only export page-level components for view controllers to + * mount React to the DOM + */ export { NoData } from './no_data'; export { License } from './license'; -export { StatusIcon } from './status_icon'; -export { SummaryStatus } from './summary_status'; // TODO this line can be removed, component is only used by other components export { PageLoading } from './page_loading'; export { ElasticsearchOverview, diff --git a/x-pack/plugins/monitoring/public/components/summary_status/__snapshots__/summary_status.test.js.snap b/x-pack/plugins/monitoring/public/components/summary_status/__snapshots__/summary_status.test.js.snap new file mode 100644 index 000000000000..0cf2556e3cf8 --- /dev/null +++ b/x-pack/plugins/monitoring/public/components/summary_status/__snapshots__/summary_status.test.js.snap @@ -0,0 +1,143 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Summary Status Component should allow label to be optional 1`] = ` +
+
+
+
+ + 127.0.0.1:9300 + +
+
+ Documents: + + 24.8k + +
+
+
+ Status: + + Status: yellow + + Yellow +
+
+
+
+
+`; + +exports[`Summary Status Component should allow status to be optional 1`] = ` +
+
+
+
+ Free Disk Space: + + 173.9 GB + +
+
+ Documents: + + 24.8k + +
+
+
+
+
+`; + +exports[`Summary Status Component should render metrics in a summary bar 1`] = ` +
+
+
+
+ Free Disk Space: + + 173.9 GB + +
+
+ Documents: + + 24.8k + +
+
+
+ Status: + + Status: green + + Green +
+
+
+
+
+`; diff --git a/x-pack/plugins/monitoring/public/components/summary_status/index.js b/x-pack/plugins/monitoring/public/components/summary_status/index.js index 2881d5179b71..689a769e57e2 100644 --- a/x-pack/plugins/monitoring/public/components/summary_status/index.js +++ b/x-pack/plugins/monitoring/public/components/summary_status/index.js @@ -4,64 +4,4 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { Fragment } from 'react'; -import PropTypes from 'prop-types'; -import { isEmpty, capitalize } from 'lodash'; -import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; -import { StatusIcon } from '../'; - -const wrapChild = ({ label, value, dataTestSubj }, index) => ( - - {label ? label + ': ' : null} - {value} - -); - -const DefaultIconComponent = ({ status }) => ( - - Status: {( - - )} - -); - -const StatusIndicator = ({ status, isOnline, IconComponent }) => { - if (isEmpty(status)) { - return null; - } - - return ( -
- {' '} - {capitalize(status)} -
- ); -}; - -export function SummaryStatus({ metrics, status, isOnline, IconComponent = DefaultIconComponent, ...props }) { - return ( -
-
- - {metrics.map(wrapChild)} - - - - - -
-
- ); -} - -SummaryStatus.propTypes = { - metrics: PropTypes.array.isRequired -}; +export { SummaryStatus } from './summary_status'; diff --git a/x-pack/plugins/monitoring/public/components/summary_status/summary_status.js b/x-pack/plugins/monitoring/public/components/summary_status/summary_status.js new file mode 100644 index 000000000000..f8ed4b70924d --- /dev/null +++ b/x-pack/plugins/monitoring/public/components/summary_status/summary_status.js @@ -0,0 +1,67 @@ +/* + * 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, { Fragment } from 'react'; +import PropTypes from 'prop-types'; +import { isEmpty, capitalize } from 'lodash'; +import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; +import { StatusIcon } from '../status_icon'; + +const wrapChild = ({ label, value, dataTestSubj }, index) => ( + + {label ? label + ': ' : null} + {value} + +); + +const DefaultIconComponent = ({ status }) => ( + + Status: {( + + )} + +); + +const StatusIndicator = ({ status, isOnline, IconComponent }) => { + if (isEmpty(status)) { + return null; + } + + return ( +
+ {' '} + {capitalize(status)} +
+ ); +}; + +export function SummaryStatus({ metrics, status, isOnline, IconComponent = DefaultIconComponent, ...props }) { + return ( +
+
+ + {metrics.map(wrapChild)} + + + + + +
+
+ ); +} + +SummaryStatus.propTypes = { + metrics: PropTypes.array.isRequired +}; diff --git a/x-pack/plugins/monitoring/public/components/summary_status/summary_status.test.js b/x-pack/plugins/monitoring/public/components/summary_status/summary_status.test.js new file mode 100644 index 000000000000..f142ed545a99 --- /dev/null +++ b/x-pack/plugins/monitoring/public/components/summary_status/summary_status.test.js @@ -0,0 +1,69 @@ +/* + * 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 { render } from 'enzyme'; +import { SummaryStatus } from './summary_status'; + +describe('Summary Status Component', () => { + it('should render metrics in a summary bar', () => { + const props = { + metrics: [ + { + label: 'Free Disk Space', + value: '173.9 GB', + dataTestSubj: 'freeDiskSpace' + }, + { + label: 'Documents', + value: '24.8k', + dataTestSubj: 'documentCount' + }, + ], + status: 'green' + }; + + expect(render()).toMatchSnapshot(); + }); + + it('should allow label to be optional', () => { + const props = { + metrics: [ + { + value: '127.0.0.1:9300', + dataTestSubj: 'transportAddress' + }, + { + label: 'Documents', + value: '24.8k', + dataTestSubj: 'documentCount' + }, + ], + status: 'yellow' + }; + + expect(render()).toMatchSnapshot(); + }); + + it('should allow status to be optional', () => { + const props = { + metrics: [ + { + label: 'Free Disk Space', + value: '173.9 GB', + dataTestSubj: 'freeDiskSpace' + }, + { + label: 'Documents', + value: '24.8k', + dataTestSubj: 'documentCount' + }, + ] + }; + + expect(render()).toMatchSnapshot(); + }); +}); diff --git a/x-pack/plugins/monitoring/public/directives/all.js b/x-pack/plugins/monitoring/public/directives/all.js index 4e0021e0fc45..98b3a10237ba 100644 --- a/x-pack/plugins/monitoring/public/directives/all.js +++ b/x-pack/plugins/monitoring/public/directives/all.js @@ -8,7 +8,6 @@ import './main'; import './chart'; import './sparkline'; import './alerts'; -import './status_icon'; import './cluster/overview'; import './cluster/listing'; import './elasticsearch/cluster_status'; diff --git a/x-pack/plugins/monitoring/public/directives/status_icon/index.js b/x-pack/plugins/monitoring/public/directives/status_icon/index.js deleted file mode 100644 index e330068b964f..000000000000 --- a/x-pack/plugins/monitoring/public/directives/status_icon/index.js +++ /dev/null @@ -1,57 +0,0 @@ -/* - * 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 ReactDOM from 'react-dom'; -import { uiModules } from 'ui/modules'; -import { ElasticsearchStatusIcon } from 'plugins/monitoring/components/elasticsearch/status_icon'; -import { NodeStatusIcon } from 'plugins/monitoring/components/elasticsearch/node/status_icon'; -import { KibanaStatusIcon } from 'plugins/monitoring/components/kibana/status_icon'; - -const uiModule = uiModules.get('monitoring/directives', []); - -function linkStatusIconComponent(scope, $el, StatusIconComponent) { - // The watch callback always runs the first time connecting the data to the directive - // The first time callback runs, unmountComponentAtNode does nothing - scope.$watch('status', (status) => { - ReactDOM.unmountComponentAtNode($el[0]); - ReactDOM.render(( -
- -
- ), $el[0]); - }); -} - -uiModule.directive('monitoringKibanaStatusIcon', function () { - return { - restrict: 'E', - scope: { status: '=' }, - link(scope, $el) { - linkStatusIconComponent(scope, $el, KibanaStatusIcon); - } - }; -}); - -uiModule.directive('monitoringElasticsearchStatusIcon', function () { - return { - restrict: 'E', - scope: { status: '=' }, - link(scope, $el) { - linkStatusIconComponent(scope, $el, ElasticsearchStatusIcon); - } - }; -}); - -uiModule.directive('monitoringElasticsearchNodeStatusIcon', function () { - return { - restrict: 'E', - scope: { status: '=' }, - link(scope, $el) { - linkStatusIconComponent(scope, $el, NodeStatusIcon); - } - }; -});