- Moved constants from metrics to own file so that it's more re-usable

- Removed unnecessary styling - Fixed typos - Updated comments - i18n
jloleysens · Aug 14, 2019 · 0505101 · 0505101
1 parent c428a8a
commit 0505101
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 58 deletions.
diff --git a/x-pack/legacy/plugins/rollup/__jest__/client_integration/job_create_metrics.test.js b/x-pack/legacy/plugins/rollup/__jest__/client_integration/job_create_metrics.test.js
@@ -325,6 +325,9 @@ describe('Create Rollup Job, step 5: Metrics', () => {
 
       it('should deselect all of the metric types across rows (column-wise)',  () => {
         const selectAllAvgCheckbox = find('rollupJobMetricsSelectAllCheckbox-avg');
+
+        // Select first, which adds metrics column-wise, and then de-select column-wise to ensure
+        // that we correctly remove/undo our adding action.
         selectAllAvgCheckbox.last().simulate('change', { checked: true });
         selectAllAvgCheckbox.last().simulate('change', { checked: false });
 
@@ -352,7 +355,9 @@ describe('Create Rollup Job, step 5: Metrics', () => {
          *
          * 1. Select all avg column-wise
          * 2. Select all max column-wise
-         * 3. Select and deselect row-wise the first numeric metric row
+         * 3. Select and deselect row-wise the first numeric metric row. Because some items will
+         *    have been selected by the previous column-wise selection we want to test that row-wise
+         *    select all followed by de-select can effectively undo the column-wise selections.
          * 4. Expect the avg and max select all checkboxes to be unchecked
          * 5. Select all on the last date metric row-wise
          * 6. Select then deselect all max column-wise
@@ -369,9 +374,9 @@ describe('Create Rollup Job, step 5: Metrics', () => {
         const selectAllCheckbox = getSelectAllInputForRow(0);
 
         // 3.
-        // Select All
+        // Select all (which should check all checkboxes)
         selectAllCheckbox.simulate('change', { checked: true });
-        // Deselect All
+        // Deselect all (which should deselect all checkboxes)
         selectAllCheckbox.simulate('change', { checked: false });
 
         // 4.

diff --git a/x-pack/legacy/plugins/rollup/public/crud_app/constants/index.js b/x-pack/legacy/plugins/rollup/public/crud_app/constants/index.js
@@ -7,3 +7,7 @@
 export {
   CRUD_APP_BASE_PATH,
 } from './paths';
+
+export {
+  METRICS_CONFIG
+} from './metrics_config';
diff --git a/x-pack/legacy/plugins/rollup/public/crud_app/constants/metrics_config.js b/x-pack/legacy/plugins/rollup/public/crud_app/constants/metrics_config.js
@@ -0,0 +1,45 @@
+/*
+ * 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 { i18n } from '@kbn/i18n';
+
+export const METRICS_CONFIG = [
+  {
+    type: 'avg',
+    label: i18n.translate(
+      'xpack.rollupJobs.create.stepMetrics.checkboxAverageLabel',
+      { defaultMessage: 'Average' },
+    ),
+  },
+  {
+    type: 'max',
+    label: i18n.translate(
+      'xpack.rollupJobs.create.stepMetrics.checkboxMaxLabel',
+      { defaultMessage: 'Maximum' },
+    ),
+  },
+  {
+    type: 'min',
+    label: i18n.translate(
+      'xpack.rollupJobs.create.stepMetrics.checkboxMinLabel',
+      { defaultMessage: 'Minimum' },
+    ),
+  },
+  {
+    type: 'sum',
+    label: i18n.translate(
+      'xpack.rollupJobs.create.stepMetrics.checkboxSumLabel',
+      { defaultMessage: 'Sum' },
+    ),
+  },
+  {
+    type: 'value_count',
+    label: i18n.translate(
+      'xpack.rollupJobs.create.stepMetrics.checkboxValueCountLabel',
+      { defaultMessage: 'Value count' },
+    ),
+  },
+];
diff --git a/x-pack/legacy/plugins/rollup/public/crud_app/sections/job_create/steps/step_metrics.js b/x-pack/legacy/plugins/rollup/public/crud_app/sections/job_create/steps/step_metrics.js
@@ -26,6 +26,7 @@ import {
 import { metricsDetailsUrl } from '../../../services';
 import { FieldList } from '../../components';
 import { FieldChooser, StepError } from './components';
+import { METRICS_CONFIG } from '../../../constants';
 
 const whiteListedMetricByFieldType = {
   numeric: {
@@ -52,53 +53,7 @@ const checkWhiteListedMetricByFieldType = (fieldType, metricType) => {
 // objects should have a fieldTypes: { date: true, numeric: true }
 // like object.
 const metricTypesConfig = (function () {
-  return [
-    {
-      type: 'avg',
-      label: (
-        <FormattedMessage
-          id="xpack.rollupJobs.create.stepMetrics.checkboxAverageLabel"
-          defaultMessage="Average"
-        />
-      ),
-    },
-    {
-      type: 'max',
-      label: (
-        <FormattedMessage
-          id="xpack.rollupJobs.create.stepMetrics.checkboxMaxLabel"
-          defaultMessage="Maximum"
-        />
-      ),
-    },
-    {
-      type: 'min',
-      label: (
-        <FormattedMessage
-          id="xpack.rollupJobs.create.stepMetrics.checkboxMinLabel"
-          defaultMessage="Minimum"
-        />
-      ),
-    },
-    {
-      type: 'sum',
-      label: (
-        <FormattedMessage
-          id="xpack.rollupJobs.create.stepMetrics.checkboxSumLabel"
-          defaultMessage="Sum"
-        />
-      ),
-    },
-    {
-      type: 'value_count',
-      label: (
-        <FormattedMessage
-          id="xpack.rollupJobs.create.stepMetrics.checkboxValueCountLabel"
-          defaultMessage="Value count"
-        />
-      ),
-    },
-  ].map(config => {
+  return METRICS_CONFIG.map(config => {
     const fieldTypes = {};
     for (const [fieldType, metrics] of Object.entries(whiteListedMetricByFieldType)) {
       fieldTypes[fieldType] = !!metrics[config.type];
@@ -151,9 +106,9 @@ export class StepMetricsUi extends Component {
 
     /**
      * Look at all the metric configs and include the special "All" checkbox which adds the ability
-     * to select a all the checkboxes across columns and rows.
+     * to select all the checkboxes across columns and rows.
      */
-    const jsxElements = [{
+    const checkboxElements = [{
       label: i18n.translate('xpack.rollupJobs.create.stepMetrics.allCheckbox', { defaultMessage: 'All' }),
       type: 'all',
     }].concat(metricTypesConfig).map(({ label, type: metricType, fieldTypes }, idx) => {
@@ -202,7 +157,7 @@ export class StepMetricsUi extends Component {
           id={`${idx}-select-all-checkbox`}
           data-test-subj={`rollupJobMetricsSelectAllCheckbox-${metricType}`}
           disabled={isDisabled}
-          label={<span style={{ whiteSpace: 'nowrap' }}>{label}</span>}
+          label={label}
           checked={!isDisabled && isChecked}
           onChange={() => {
             if (isAllMetricTypes) {
@@ -218,14 +173,14 @@ export class StepMetricsUi extends Component {
     });
 
     return {
-      jsxElements,
-      allCheckboxesDisabled: jsxElements.length === disabledCheckboxesCount,
+      checkboxElements,
+      allCheckboxesDisabled: checkboxElements.length === disabledCheckboxesCount,
     };
   }
 
   getMetricsSelectAllMenu() {
     const {
-      jsxElements: checkboxElements,
+      checkboxElements,
       allCheckboxesDisabled
     } = this.renderMetricsSelectAllCheckboxes();
 
@@ -510,13 +465,19 @@ export class StepMetricsUi extends Component {
   static chooserColumns = [
     {
       field: 'name',
-      name: 'Field',
+      name: i18n.translate(
+        'xpack.rollupJobs.create.stepMetrics.fieldColumnLabel',
+        { defaultMessage: 'Field' }
+      ),
       sortable: true,
       width: '240px',
     },
     {
       field: 'type',
-      name: 'Type',
+      name: i18n.translate(
+        'xpack.rollupJobs.create.stepMetrics.typeColumnLabel',
+        { defaultMessage: 'Type' }
+      ),
       truncateText: true,
       sortable: true,
       width: '100px',