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

(cherry picked from commit 7a7808e3)

89dd5efe Remove unnecessary parameter transformation
fedc5e4e Pass time params in seconds per unix spec
c0e342c1 Add unit tests for js time window utils
a0ac8c25 Fix static analysis and false positive test

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');
+    });
+  });
+});