From bb53d262cfb8713543cb6709f43474db2d36f588 Mon Sep 17 00:00:00 2001
From: Miguel Rincon <mrincon@gitlab.com>
Date: Tue, 10 Sep 2019 10:39:29 +0200
Subject: Fix code based on reviews:

- Add extra checks for null in getEarliestDatapoint
- Simplify validator setup for graphData prop
- Remove showBorder
- Add specs for new utils
- Add documentation
---
 .../monitoring/components/charts/anomaly.vue       | 100 +++++++++------------
 .../monitoring/components/panel_type.vue           |  47 ----------
 app/assets/javascripts/monitoring/utils.js         |  43 +++++----
 spec/frontend/monitoring/mock_data.js              |   4 +-
 spec/javascripts/monitoring/mock_data.js           |   4 +
 spec/javascripts/monitoring/utils_spec.js          |  26 +++++-
 6 files changed, 101 insertions(+), 123 deletions(-)

diff --git a/app/assets/javascripts/monitoring/components/charts/anomaly.vue b/app/assets/javascripts/monitoring/components/charts/anomaly.vue
index 72f5412d456..20c8450f423 100644
--- a/app/assets/javascripts/monitoring/components/charts/anomaly.vue
+++ b/app/assets/javascripts/monitoring/components/charts/anomaly.vue
@@ -35,7 +35,7 @@ export default {
     graphData: {
       type: Object,
       required: true,
-      validator: graphDataValidatorForAnomalyValues.bind(null, false),
+      validator: graphDataValidatorForAnomalyValues,
     },
     containerWidth: {
       type: Number,
@@ -51,11 +51,6 @@ export default {
       required: false,
       default: '',
     },
-    showBorder: {
-      type: Boolean,
-      required: false,
-      default: false,
-    },
     singleEmbed: {
       type: Boolean,
       required: false,
@@ -213,12 +208,10 @@ export default {
       const handleIcon = this.svgs['scroll-handle'];
       return handleIcon ? { handleIcon } : {};
     },
-    isMultiSeries() {
-      return this.tooltip.content.length > 1;
-    },
     recentDeployments() {
       return this.deploymentData.reduce((acc, deployment) => {
-        if (deployment.created_at >= getEarliestDatapoint(this.chartData)) {
+        const earliestDatapoint = getEarliestDatapoint(this.chartData);
+        if (earliestDatapoint && deployment.created_at >= getEarliestDatapoint(this.chartData)) {
           const { id, created_at, sha, ref, tag } = deployment;
           acc.push({
             id,
@@ -333,54 +326,49 @@ export default {
 </script>
 
 <template>
-  <div
-    class="prometheus-graph col-12"
-    :class="[showBorder ? 'p-2' : 'p-0', { 'col-lg-6': !singleEmbed }]"
-  >
-    <div :class="{ 'prometheus-graph-embed w-100 p-3': showBorder }">
-      <div class="prometheus-graph-header">
-        <h5 class="prometheus-graph-title js-graph-title">{{ graphData.title }}</h5>
-        <div class="prometheus-graph-widgets js-graph-widgets">
-          <slot></slot>
-        </div>
+  <div class="prometheus-graph col-12" :class="{ 'col-lg-6': !singleEmbed }">
+    <div class="prometheus-graph-header">
+      <h5 class="prometheus-graph-title js-graph-title">{{ graphData.title }}</h5>
+      <div class="prometheus-graph-widgets js-graph-widgets">
+        <slot></slot>
       </div>
-      <gl-line-chart
-        ref="chart"
-        v-bind="$attrs"
-        :data="chartData"
-        :option="chartOptions"
-        :include-legend-avg-max="false"
-        :format-tooltip-text="formatTooltipText"
-        :thresholds="thresholds"
-        :width="width"
-        :height="height"
-        @updated="onChartUpdated"
-      >
-        <template v-if="tooltip.isDeployment">
-          <template slot="tooltipTitle">{{ __('Deployed') }}</template>
-          <div slot="tooltipContent" class="d-flex align-items-center">
-            <icon name="commit" class="mr-2" />
-            <gl-link :href="tooltip.commitUrl">{{ tooltip.sha }}</gl-link>
-          </div>
+    </div>
+    <gl-line-chart
+      ref="chart"
+      v-bind="$attrs"
+      :data="chartData"
+      :option="chartOptions"
+      :include-legend-avg-max="false"
+      :format-tooltip-text="formatTooltipText"
+      :thresholds="thresholds"
+      :width="width"
+      :height="height"
+      @updated="onChartUpdated"
+    >
+      <template v-if="tooltip.isDeployment">
+        <template slot="tooltipTitle">{{ __('Deployed') }}</template>
+        <div slot="tooltipContent" class="d-flex align-items-center">
+          <icon name="commit" class="mr-2" />
+          <gl-link :href="tooltip.commitUrl">{{ tooltip.sha }}</gl-link>
+        </div>
+      </template>
+      <template v-else>
+        <template slot="tooltipTitle">
+          <div class="text-nowrap">{{ tooltip.title }}</div>
         </template>
-        <template v-else>
-          <template slot="tooltipTitle">
-            <div class="text-nowrap">{{ tooltip.title }}</div>
-          </template>
-          <template slot="tooltipContent">
-            <div
-              v-for="(content, key) in tooltip.content"
-              :key="key"
-              class="d-flex justify-content-between"
-            >
-              <gl-chart-series-label :color="isMultiSeries ? content.color : ''">{{
-                content.name
-              }}</gl-chart-series-label>
-              <div class="prepend-left-32">{{ content.value }}</div>
-            </div>
-          </template>
+        <template slot="tooltipContent">
+          <div
+            v-for="(seriesLabel, key) in tooltip.content"
+            :key="key"
+            class="d-flex justify-content-between"
+          >
+            <gl-chart-series-label :color="tooltip.content.length > 1 ? seriesLabel.color : ''">{{
+              seriesLabel.name
+            }}</gl-chart-series-label>
+            <div class="prepend-left-32">{{ seriesLabel.value }}</div>
+          </div>
         </template>
-      </gl-line-chart>
-    </div>
+      </template>
+    </gl-line-chart>
   </div>
 </template>
diff --git a/app/assets/javascripts/monitoring/components/panel_type.vue b/app/assets/javascripts/monitoring/components/panel_type.vue
index c56bcfb8db2..73ff651d510 100644
--- a/app/assets/javascripts/monitoring/components/panel_type.vue
+++ b/app/assets/javascripts/monitoring/components/panel_type.vue
@@ -11,7 +11,6 @@ import {
 } from '@gitlab/ui';
 import Icon from '~/vue_shared/components/icon.vue';
 import MonitorTimeSeriesChart from './charts/time_series.vue';
-import MonitorAnomalyChart from './charts/anomaly.vue';
 import MonitorSingleStatChart from './charts/single_stat.vue';
 import MonitorEmptyChart from './charts/empty_chart.vue';
 
@@ -19,7 +18,6 @@ export default {
   components: {
     MonitorSingleStatChart,
     MonitorTimeSeriesChart,
-    MonitorAnomalyChart,
     MonitorEmptyChart,
     Icon,
     GlDropdown,
@@ -94,51 +92,6 @@ export default {
     v-if="isPanelType('single-stat') && graphDataHasMetrics"
     :graph-data="graphData"
   />
-  <monitor-anomaly-chart
-    v-else-if="isPanelType('anomaly-chart') && graphDataHasMetrics"
-    :graph-data="graphData"
-    :deployment-data="deploymentData"
-    :project-path="projectPath"
-    :thresholds="getGraphAlertValues(graphData.queries)"
-    :container-width="dashboardWidth"
-    group-id="monitor-anomaly-chart"
-  >
-    <div class="d-flex align-items-center">
-      <alert-widget
-        v-if="alertWidgetAvailable && graphData"
-        :modal-id="`alert-modal-${index}`"
-        :alerts-endpoint="alertsEndpoint"
-        :relevant-queries="graphData.queries"
-        :alerts-to-manage="getGraphAlerts(graphData.queries)"
-        @setAlerts="setAlerts"
-      />
-      <gl-dropdown
-        v-gl-tooltip
-        class="mx-2"
-        toggle-class="btn btn-transparent border-0"
-        :right="true"
-        :no-caret="true"
-        :title="__('More actions')"
-      >
-        <template slot="button-content">
-          <icon name="ellipsis_v" class="text-secondary" />
-        </template>
-        <gl-dropdown-item :href="downloadCsv" download="chart_metrics.csv">
-          {{ __('Download CSV') }}
-        </gl-dropdown-item>
-        <gl-dropdown-item
-          class="js-chart-link"
-          :data-clipboard-text="clipboardText"
-          @click="showToast"
-        >
-          {{ __('Generate link to chart') }}
-        </gl-dropdown-item>
-        <gl-dropdown-item v-if="alertWidgetAvailable" v-gl-modal="`alert-modal-${index}`">
-          {{ __('Alerts') }}
-        </gl-dropdown-item>
-      </gl-dropdown>
-    </div>
-  </monitor-anomaly-chart>
   <monitor-time-series-chart
     v-else-if="graphDataHasMetrics"
     :graph-data="graphData"
diff --git a/app/assets/javascripts/monitoring/utils.js b/app/assets/javascripts/monitoring/utils.js
index 3df9387b4f2..0f6f4ad1fd6 100644
--- a/app/assets/javascripts/monitoring/utils.js
+++ b/app/assets/javascripts/monitoring/utils.js
@@ -45,15 +45,30 @@ export const graphDataValidatorForValues = (isValues, graphData) => {
   );
 };
 
-export const graphDataValidatorForAnomalyValues = (isValues, graphData) => {
+/**
+ * This function validates the graph data contains exactly 3 queries plus
+ * value validations from graphDataValidatorForValues.
+ * @param {Object} isValues
+ * @param {Object} graphData  the graph data response from a prometheus request
+ * @returns {boolean} true if the data is valid
+ */
+export const graphDataValidatorForAnomalyValues = graphData => {
   const anomalySeriesCount = 3; // metric, upper, lower
   return (
     graphData.queries &&
     graphData.queries.length === anomalySeriesCount &&
-    graphDataValidatorForValues(isValues, graphData)
+    graphDataValidatorForValues(false, graphData)
   );
 };
 
+/**
+ * This function returns the earliest time value in all series of a chart.
+ * @param {Object} chartData  chart data with data to populate a timeseries.
+ * data should be an array of data points [t, y] where t is a ISO formatted date,
+ * and is sorted by t (time).
+ * @returns {(String|null)} earliest x value from all series, or null when the
+ * chart series data is empty.
+ */
 export const getEarliestDatapoint = chartData =>
   chartData.reduce((acc, series) => {
     const { data } = series;
@@ -69,18 +84,16 @@ export const getEarliestDatapoint = chartData =>
     return seriesEarliest < acc || acc === null ? seriesEarliest : acc;
   }, null);
 
-export const makeTimeAxis = config => {
-  const defaults = {
-    name: __('Time'),
-    type: 'time',
-    axisLabel: {
-      formatter: date => dateFormat(date, dateFormats.timeOfDay),
-    },
-    axisPointer: {
-      snap: true,
-    },
-  };
-  return { ...defaults, ...config };
-};
+export const makeTimeAxis = config => ({
+  name: __('Time'),
+  type: 'time',
+  axisLabel: {
+    formatter: date => dateFormat(date, dateFormats.timeOfDay),
+  },
+  axisPointer: {
+    snap: true,
+  },
+  ...config,
+});
 
 export default {};
diff --git a/spec/frontend/monitoring/mock_data.js b/spec/frontend/monitoring/mock_data.js
index 38afe2b9e3b..88d56b3e382 100644
--- a/spec/frontend/monitoring/mock_data.js
+++ b/spec/frontend/monitoring/mock_data.js
@@ -1,6 +1,4 @@
-import * as mockData from '../../javascripts/monitoring/mock_data';
-
-export const { mockProjectPath } = mockData;
+export const mockProjectPath = '/frontend-fixtures/environments-project';
 
 export const anomalyDeploymentData = [
   {
diff --git a/spec/javascripts/monitoring/mock_data.js b/spec/javascripts/monitoring/mock_data.js
index 17e7314e214..e10108b6026 100644
--- a/spec/javascripts/monitoring/mock_data.js
+++ b/spec/javascripts/monitoring/mock_data.js
@@ -1,3 +1,7 @@
+import { anomalyMockGraphData as importedAnomalyMockGraphData } from '../../frontend/monitoring/mock_data';
+
+export const anomalyMockGraphData = importedAnomalyMockGraphData;
+
 export const mockApiEndpoint = `${gl.TEST_HOST}/monitoring/mock`;
 
 export const mockProjectPath = '/frontend-fixtures/environments-project';
diff --git a/spec/javascripts/monitoring/utils_spec.js b/spec/javascripts/monitoring/utils_spec.js
index e22e8cdc03d..1cd3cc27e88 100644
--- a/spec/javascripts/monitoring/utils_spec.js
+++ b/spec/javascripts/monitoring/utils_spec.js
@@ -1,6 +1,14 @@
-import { getTimeDiff, graphDataValidatorForValues } from '~/monitoring/utils';
+import {
+  getTimeDiff,
+  graphDataValidatorForValues,
+  graphDataValidatorForAnomalyValues,
+} from '~/monitoring/utils';
 import { timeWindows } from '~/monitoring/constants';
-import { graphDataPrometheusQuery, graphDataPrometheusQueryRange } from './mock_data';
+import {
+  graphDataPrometheusQuery,
+  graphDataPrometheusQueryRange,
+  anomalyMockGraphData,
+} from './mock_data';
 
 describe('getTimeDiff', () => {
   function secondsBetween({ start, end }) {
@@ -62,3 +70,17 @@ describe('graphDataValidatorForValues', () => {
     expect(validGraphData).toBe(true);
   });
 });
+
+describe('graphDataValidatorForAnomalyValues', () => {
+  /*
+   * Anomaly charts can accept results for exactly 3 queries,
+   * Anomaly validation also require graphDataValidatorForValues to have `values`
+   */
+  it('validates data with the query format', () => {
+    const validAnomalyData = graphDataValidatorForAnomalyValues(anomalyMockGraphData);
+    const validGraphData = graphDataValidatorForValues(false, anomalyMockGraphData);
+
+    expect(validAnomalyData).toBe(true);
+    expect(validGraphData).toBe(true);
+  });
+});
-- 
cgit v1.2.3