Move removeTimeSeriesNoData logic to the storejivl-add-empty-state-graphs-null-values

7 files changed, 111 insertions, 134 deletions

diff --git a/app/assets/javascripts/monitoring/components/graph.vue b/app/assets/javascripts/monitoring/components/graph.vue
index f5a5001689d..64a1df80a8e 100644
--- a/app/assets/javascripts/monitoring/components/graph.vue
+++ b/app/assets/javascripts/monitoring/components/graph.vue
@@ -13,7 +13,7 @@ import MonitoringMixin from '../mixins/monitoring_mixins';
 import eventHub from '../event_hub';
 import measurements from '../utils/measurements';
 import { bisectDate, timeScaleFormat } from '../utils/date_time_formatters';
-import createTimeSeries, { removeTimeSeriesNoData } from '../utils/multiple_time_series';
+import createTimeSeries from '../utils/multiple_time_series';
 import bp from '../../breakpoints';
 
 const d3 = { scaleLinear, scaleTime, axisLeft, axisBottom, max, extent, select };
@@ -85,7 +85,6 @@ export default {
       graphDrawData: {},
       realPixelRatio: 1,
       seriesUnderMouse: [],
-      noDataToDisplay: false,
     };
   },
   computed: {
@@ -106,8 +105,8 @@ export default {
     deploymentFlagData() {
       return this.reducedDeploymentData.find(deployment => deployment.showDeploymentFlag);
     },
-    noDataToDisplayMsg() {
-      return s__('Metrics|No data to display');
+    shouldRenderData() {
+      return this.graphData.queries.filter(s => s.result.length > 0).length > 0;
     },
   },
   watch: {
@@ -143,19 +142,14 @@ export default {
       // pixel offsets inside the svg and outside are not 1:1
       this.realPixelRatio = svgWidth / this.baseGraphWidth;
 
-      // verify the data
-      this.graphData.queries = removeTimeSeriesNoData(this.graphData.queries);
-      const queryLengthOfData = this.graphData.queries.filter(s => s.result.length > 0).length;
-
+      // set the legends on the axes
       const [query] = this.graphData.queries;
       this.legendTitle = query ? query.label : 'Average';
       this.unitOfDisplay = query ? query.unit : '';
 
-      if (queryLengthOfData > 0) {
+      if (this.shouldRenderData) {
         this.renderAxesPaths();
         this.formatDeployments();
-      } else {
-        this.noDataToDisplay = true;
       }
     },
     handleMouseOverGraph(e) {
@@ -282,16 +276,8 @@ export default {
           :y-axis-label="yAxisLabel"
           :unit-of-display="unitOfDisplay"
         />
-        <svg
-          v-if="!noDataToDisplay"
-          ref="graphData"
-          :viewBox="innerViewBox"
-          class="graph-data"
-        >
-          <slot
-            name="additionalSvgContent"
-            :graphDrawData="graphDrawData"
-          />
+        <svg v-if="shouldRenderData" ref="graphData" :viewBox="innerViewBox" class="graph-data">
+          <slot name="additionalSvgContent" :graphDrawData="graphDrawData" />
           <graph-path
             v-for="(path, index) in timeSeries"
             :key="index"
@@ -317,23 +303,14 @@ export default {
             @mousemove="handleMouseOverGraph($event);"
           />
         </svg>
-        <svg
-          v-else
-          :viewBox="innerViewBox"
-          class="js-no-data-to-display"
-        >
-          <text
-            x="50%"
-            y="50%"
-            alignment-baseline="middle"
-            text-anchor="middle"
-          >
+        <svg v-else :viewBox="innerViewBox" class="js-no-data-to-display">
+          <text x="50%" y="50%" alignment-baseline="middle" text-anchor="middle">
             {{ s__('Metrics|No data to display') }}
           </text>
         </svg>
       </svg>
       <graph-flag
-        v-if="!noDataToDisplay"
+        v-if="shouldRenderData"
         :real-pixel-ratio="realPixelRatio"
         :current-x-coordinate="currentXCoordinate"
         :current-data="currentData"
diff --git a/app/assets/javascripts/monitoring/monitoring_bundle.js b/app/assets/javascripts/monitoring/monitoring_bundle.js
index 5a64d0b9f30..41270e015d4 100644
--- a/app/assets/javascripts/monitoring/monitoring_bundle.js
+++ b/app/assets/javascripts/monitoring/monitoring_bundle.js
@@ -1,10 +1,7 @@
 import Vue from 'vue';
 import { convertPermissionToBoolean } from '~/lib/utils/common_utils';
-import Translate from '~/vue_shared/translate';
 import Dashboard from './components/dashboard.vue';
 
-Vue.use(Translate);
-
 export default () => {
   const el = document.getElementById('prometheus-graphs');
 
diff --git a/app/assets/javascripts/monitoring/stores/monitoring_store.js b/app/assets/javascripts/monitoring/stores/monitoring_store.js
index 176f7d9eef2..fc9d4837727 100644
--- a/app/assets/javascripts/monitoring/stores/monitoring_store.js
+++ b/app/assets/javascripts/monitoring/stores/monitoring_store.js
@@ -7,10 +7,34 @@ function sortMetrics(metrics) {
     .value();
 }
 
+function checkQueryEmptyData(query) {
+  return {
+    ...query,
+    result: query.result.filter(timeSeries => {
+      const newTimeSeries = timeSeries;
+      const hasValue = series =>
+        !Number.isNaN(series.value) && (series.value !== null || series.value !== undefined);
+      const hasNonNullValue = timeSeries.values.find(hasValue);
+
+      newTimeSeries.values = hasNonNullValue ? newTimeSeries.values : [];
+
+      return newTimeSeries.values.length > 0;
+    }),
+  };
+}
+
+function removeTimeSeriesNoData(queries) {
+  const timeSeries = queries.reduce(
+    (series, query) => series.concat(checkQueryEmptyData(query)),
+    [],
+  );
+
+  return timeSeries;
+}
+
 function normalizeMetrics(metrics) {
-  return metrics.map(metric => ({
-    ...metric,
-    queries: metric.queries.map(query => ({
+  return metrics.map(metric => {
+    const queries = metric.queries.map(query => ({
       ...query,
       result: query.result.map(result => ({
         ...result,
@@ -19,8 +43,13 @@ function normalizeMetrics(metrics) {
           value: Number(value),
         })),
       })),
-    })),
-  }));
+    }));
+
+    return {
+      ...metric,
+      queries: removeTimeSeriesNoData(queries),
+    };
+  });
 }
 
 export default class MonitoringStore {
diff --git a/app/assets/javascripts/monitoring/utils/multiple_time_series.js b/app/assets/javascripts/monitoring/utils/multiple_time_series.js
index 72cf91afc7c..bb24a1acdb3 100644
--- a/app/assets/javascripts/monitoring/utils/multiple_time_series.js
+++ b/app/assets/javascripts/monitoring/utils/multiple_time_series.js
@@ -217,35 +217,3 @@ export default function createTimeSeries(queries, graphWidth, graphHeight, graph
     graphDrawData,
   };
 }
-
-function checkQueryEmptyData(query) {
-  return {
-    ...query,
-    result: query.result.filter(timeSeries => {
-      const newTimeSeries = timeSeries;
-      const emptyValues = timeSeries.values.filter(
-        val => Number.isNaN(val.value) || val.value === null || val.value === undefined,
-      );
-
-      if (emptyValues.length === timeSeries.values.length) {
-        newTimeSeries.values = [];
-      }
-
-      return newTimeSeries;
-    }),
-  };
-}
-
-export function removeTimeSeriesNoData(queries) {
-  const timeSeries = queries.reduce((series, query) => {
-    let checkedQuery = checkQueryEmptyData(query);
-    checkedQuery = {
-      ...checkedQuery,
-      result: checkedQuery.result.filter(c => c.values.length > 0),
-    };
-
-    return series.concat(checkedQuery);
-  }, []);
-
-  return timeSeries;
-}
diff --git a/spec/javascripts/monitoring/graph_spec.js b/spec/javascripts/monitoring/graph_spec.js
index 7d07ef09caf..59d6d4f3a7f 100644
--- a/spec/javascripts/monitoring/graph_spec.js
+++ b/spec/javascripts/monitoring/graph_spec.js
@@ -19,7 +19,6 @@ const createComponent = propsData => {
 };
 
 const convertedMetrics = convertDatesMultipleSeries(singleRowMetricsMultipleSeries);
-const [convertedQueryWithoutData] = convertDatesMultipleSeries(queryWithoutData);
 
 describe('Graph', () => {
   beforeEach(() => {
@@ -110,14 +109,12 @@ describe('Graph', () => {
   describe('Without data to display', () => {
     it('shows a "no data to display" empty state on a graph', done => {
       const component = createComponent({
-        graphData: convertedQueryWithoutData,
+        graphData: queryWithoutData,
         deploymentData,
         tagsPath,
         projectPath,
       });
 
-      expect(component.noDataToDisplay).toEqual(true);
-
       Vue.nextTick(() => {
         expect(
           component.$el.querySelector('.js-no-data-to-display text').textContent.trim(),
diff --git a/spec/javascripts/monitoring/mock_data.js b/spec/javascripts/monitoring/mock_data.js
index c0dab1bcbaf..18ad9843d22 100644
--- a/spec/javascripts/monitoring/mock_data.js
+++ b/spec/javascripts/monitoring/mock_data.js
@@ -642,6 +642,39 @@ export const metricsGroupsAPIResponse = {
         },
       ],
     },
+    {
+      group: 'NGINX',
+      priority: 2,
+      metrics: [
+        {
+          id: 100,
+          title: 'Http Error Rate',
+          weight: 100,
+          queries: [
+            {
+              query_range:
+                'sum(rate(nginx_upstream_responses_total{status_code="5xx", upstream=~"nginx-test-8691397-production-.*"}[2m])) / sum(rate(nginx_upstream_responses_total{upstream=~"nginx-test-8691397-production-.*"}[2m])) * 100',
+              label: '5xx errors',
+              unit: '%',
+              result: [
+                {
+                  metric: {},
+                  values: [
+                    [1495700554.925, NaN],
+                    [1495700614.925, NaN],
+                    [1495700674.925, NaN],
+                    [1495700734.925, NaN],
+                    [1495700794.925, NaN],
+                    [1495700854.925, NaN],
+                    [1495700914.925, NaN],
+                  ],
+                },
+              ],
+            },
+          ],
+        },
+      ],
+    },
   ],
   last_update: '2017-05-25T13:18:34.949Z',
 };
@@ -6529,48 +6562,20 @@ export const singleRowMetricsMultipleSeries = [
   },
 ];
 
-export const queryWithoutData = [
-  {
-    title: 'HTTP Error rate',
-    weight: 10,
-    y_label: 'Http Error Rate',
-    queries: [
-      {
-        query_range:
-          'sum(rate(nginx_upstream_responses_total{status_code="5xx", upstream=~"nginx-test-8691397-production-.*"}[2m])) / sum(rate(nginx_upstream_responses_total{upstream=~"nginx-test-8691397-production-.*"}[2m])) * 100',
-        label: '5xx errors',
-        unit: '%',
-        result: [
-          {
-            metric: {},
-            values: [
-              {
-                time: '2017-08-27T11:01:51.462Z',
-                value: NaN,
-              },
-              {
-                time: '2017-08-27T11:02:51.462Z',
-                value: NaN,
-              },
-              {
-                time: '2017-08-27T11:03:51.462Z',
-                value: NaN,
-              },
-              {
-                time: '2017-08-27T11:04:51.462Z',
-                value: NaN,
-              },
-              {
-                time: '2017-08-27T11:05:51.462Z',
-                value: NaN,
-              },
-            ]
-          }
-        ]
-      }
-    ]
-  },
-];
+export const queryWithoutData = {
+  title: 'HTTP Error rate',
+  weight: 10,
+  y_label: 'Http Error Rate',
+  queries: [
+    {
+      query_range:
+        'sum(rate(nginx_upstream_responses_total{status_code="5xx", upstream=~"nginx-test-8691397-production-.*"}[2m])) / sum(rate(nginx_upstream_responses_total{upstream=~"nginx-test-8691397-production-.*"}[2m])) * 100',
+      label: '5xx errors',
+      unit: '%',
+      result: [],
+    },
+  ],
+};
 
 export function convertDatesMultipleSeries(multipleSeries) {
   const convertedMultiple = multipleSeries;
diff --git a/spec/javascripts/monitoring/monitoring_store_spec.js b/spec/javascripts/monitoring/monitoring_store_spec.js
index bf68c911549..d8a980c874d 100644
--- a/spec/javascripts/monitoring/monitoring_store_spec.js
+++ b/spec/javascripts/monitoring/monitoring_store_spec.js
@@ -1,31 +1,35 @@
 import MonitoringStore from '~/monitoring/stores/monitoring_store';
 import MonitoringMock, { deploymentData, environmentData } from './mock_data';
 
-describe('MonitoringStore', function() {
-  this.store = new MonitoringStore();
-  this.store.storeMetrics(MonitoringMock.data);
-
-  it('contains one group that contains two queries sorted by priority', () => {
-    expect(this.store.groups).toBeDefined();
-    expect(this.store.groups.length).toEqual(1);
-    expect(this.store.groups[0].metrics.length).toEqual(2);
+describe('MonitoringStore', () => {
+  const store = new MonitoringStore();
+  store.storeMetrics(MonitoringMock.data);
+
+  it('contains two groups that contains, one of which has two queries sorted by priority', () => {
+    expect(store.groups).toBeDefined();
+    expect(store.groups.length).toEqual(2);
+    expect(store.groups[0].metrics.length).toEqual(2);
   });
 
   it('gets the metrics count for every group', () => {
-    expect(this.store.getMetricsCount()).toEqual(2);
+    expect(store.getMetricsCount()).toEqual(3);
   });
 
   it('contains deployment data', () => {
-    this.store.storeDeploymentData(deploymentData);
+    store.storeDeploymentData(deploymentData);
 
-    expect(this.store.deploymentData).toBeDefined();
-    expect(this.store.deploymentData.length).toEqual(3);
-    expect(typeof this.store.deploymentData[0]).toEqual('object');
+    expect(store.deploymentData).toBeDefined();
+    expect(store.deploymentData.length).toEqual(3);
+    expect(typeof store.deploymentData[0]).toEqual('object');
   });
 
   it('only stores environment data that contains deployments', () => {
-    this.store.storeEnvironmentsData(environmentData);
+    store.storeEnvironmentsData(environmentData);
+
+    expect(store.environmentsData.length).toEqual(2);
+  });
 
-    expect(this.store.environmentsData.length).toEqual(2);
+  it('removes the data if all the values from a query are not defined', () => {
+    expect(store.groups[1].metrics[0].queries[0].result.length).toEqual(0);
   });
 });