Merge branch '60162-fix-time-windows' into 'master'

Resolve Environments#additional_metrics TypeError, ensure unix format

Closes #60162

See merge request gitlab-org/gitlab-ce!27118

app/assets/javascripts/monitoring/constants.js

@@ -18,5 +18,3 @@ export const timeWindows = {
   threeDays: __('3 days'),
   oneWeek: __('1 week'),
 };
-
-export const msPerMinute = 60000;

app/assets/javascripts/monitoring/utils.js

-import { timeWindows, msPerMinute } from './constants';
+import { timeWindows } from './constants';
 /**
  * method that converts a predetermined time window to minutes
@@ -6,27 +6,26 @@ import { timeWindows, msPerMinute } from './constants';
  * @param {String} timeWindow - The time window to convert to minutes
  * @returns {number} The time window in minutes
  */
-const getTimeDifferenceMinutes = timeWindow => {
+const getTimeDifferenceSeconds = timeWindow => {
   switch (timeWindow) {
     case timeWindows.thirtyMinutes:
-      return 30;
+      return 60 * 30;
     case timeWindows.threeHours:
-      return 60 * 3;
+      return 60 * 60 * 3;
     case timeWindows.oneDay:
-      return 60 * 24 * 1;
+      return 60 * 60 * 24 * 1;
     case timeWindows.threeDays:
-      return 60 * 24 * 3;
+      return 60 * 60 * 24 * 3;
     case timeWindows.oneWeek:
-      return 60 * 24 * 7 * 1;
+      return 60 * 60 * 24 * 7 * 1;
     default:
-      return 60 * 8;
+      return 60 * 60 * 8;
   }
 };
 export const getTimeDiff = selectedTimeWindow => {
-  const end = Date.now();
-  const timeDifferenceMinutes = getTimeDifferenceMinutes(selectedTimeWindow);
-  const start = new Date(end - timeDifferenceMinutes * msPerMinute).getTime();
+  const end = Date.now() / 1000; // convert milliseconds to seconds
+  const start = end - getTimeDifferenceSeconds(selectedTimeWindow);
   return { start, end };
 };

app/controllers/projects/environments_controller.rb

@@ -193,7 +193,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController
     return unless Feature.enabled?(:metrics_time_window, project)
     return unless params[:start].present? || params[:end].present?
-    params.require([:start, :end]).values_at(:start, :end)
+    params.require([:start, :end])
   end
   def search_environment_names

spec/controllers/projects/environments_controller_spec.rb

@@ -419,6 +419,17 @@ describe Projects::EnvironmentsController do
         expect(json_response['data']).to eq({})
         expect(json_response['last_update']).to eq(42)
       end
+
+      context 'when time params are provided' do
+        it 'returns a metrics JSON document' do
+          additional_metrics(start: '1554702993.5398998', end: '1554717396.996232')
+
+          expect(response).to be_ok
+          expect(json_response['success']).to be(true)
+          expect(json_response['data']).to eq({})
+          expect(json_response['last_update']).to eq(42)
+        end
+      end
     end
     context 'when only one time param is provided' do

spec/javascripts/monitoring/utils_spec.js

+import { getTimeDiff } from '~/monitoring/utils';
+import { timeWindows } from '~/monitoring/constants';
+
+describe('getTimeDiff', () => {
+  it('defaults to an 8 hour (28800s) difference', () => {
+    const params = getTimeDiff();
+
+    expect(params.end - params.start).toEqual(28800);
+  });
+
+  it('accepts time window as an argument', () => {
+    const params = getTimeDiff(timeWindows.thirtyMinutes);
+
+    expect(params.end - params.start).not.toEqual(28800);
+  });
+
+  it('returns a value for every defined time window', () => {
+    const nonDefaultWindows = Object.keys(timeWindows).filter(window => window !== 'eightHours');
+
+    nonDefaultWindows.forEach(window => {
+      const params = getTimeDiff(timeWindows[window]);
+      const diff = params.end - params.start;
+
+      // Ensure we're not returning the default, 28800 (the # of seconds in 8 hrs)
+      expect(diff).not.toEqual(28800);
+      expect(typeof diff).toEqual('number');
+    });
+  });
+});