Merge branch '35936-decouple-time-window-specs-for-the-monitoring-dashboard' into 'master'

Decouple time window specs for the monitoring dashboard

Closes #35936

See merge request gitlab-org/gitlab!21866

app/assets/javascripts/monitoring/components/dashboard.vue

@@ -22,9 +22,11 @@ import GraphGroup from './graph_group.vue';
 import EmptyState from './empty_state.vue';
 import GroupEmptyState from './group_empty_state.vue';
 import TrackEventDirective from '~/vue_shared/directives/track_event';
-import { getTimeDiff, isValidDate, getAddMetricTrackingOptions } from '../utils';
+import { getTimeDiff, getAddMetricTrackingOptions } from '../utils';
 import { metricStates } from '../constants';
 
+const defaultTimeDiff = getTimeDiff();
+
 export default {
   components: {
     VueDraggable,
@@ -168,9 +170,10 @@ export default {
     return {
       state: 'gettingStarted',
       formIsValid: null,
-      selectedTimeWindow: {},
-      isRearrangingPanels: false,
+      startDate: getParameterValues('start')[0] || defaultTimeDiff.start,
+      endDate: getParameterValues('end')[0] || defaultTimeDiff.end,
       hasValidDates: true,
+      isRearrangingPanels: false,
     };
   },
   computed: {
@@ -228,24 +231,10 @@ export default {
     if (!this.hasMetrics) {
       this.setGettingStartedEmptyState();
     } else {
-      const defaultRange = getTimeDiff();
-      const start = getParameterValues('start')[0] || defaultRange.start;
-      const end = getParameterValues('end')[0] || defaultRange.end;
-
-      const range = {
-        start,
-        end,
-      };
-
-      this.selectedTimeWindow = range;
-
-      if (!isValidDate(start) || !isValidDate(end)) {
-        this.hasValidDates = false;
-        this.showInvalidDateError();
-      } else {
-        this.hasValidDates = true;
-        this.fetchData(range);
-      }
+      this.fetchData({
+        start: this.startDate,
+        end: this.endDate,
+      });
     }
   },
   methods: {
@@ -267,9 +256,20 @@ export default {
         key,
       });
     },
-    showInvalidDateError() {
-      createFlash(s__('Metrics|Link contains an invalid time window.'));
+
+    onDateTimePickerApply(params) {
+      redirectTo(mergeUrlParams(params, window.location.href));
+    },
+    onDateTimePickerInvalid() {
+      createFlash(
+        s__(
+          'Metrics|Link contains an invalid time window, please verify the link to see the requested time range.',
+        ),
+      );
+      this.startDate = defaultTimeDiff.start;
+      this.endDate = defaultTimeDiff.end;
     },
+
     generateLink(group, title, yLabel) {
       const dashboard = this.currentDashboard || this.firstDashboard.path;
       const params = _.pick({ dashboard, group, title, y_label: yLabel }, value => value != null);
@@ -287,9 +287,6 @@ export default {
     submitCustomMetricsForm() {
       this.$refs.customMetricsForm.submit();
     },
-    onDateTimePickerApply(timeWindowUrlParams) {
-      return redirectTo(mergeUrlParams(timeWindowUrlParams, window.location.href));
-    },
     /**
      * Return a single empty state for a group.
      *
@@ -378,15 +375,16 @@ export default {
           </gl-form-group>
 
           <gl-form-group
-            v-if="hasValidDates"
             :label="s__('Metrics|Show last')"
             label-size="sm"
             label-for="monitor-time-window-dropdown"
             class="col-sm-6 col-md-6 col-lg-4"
           >
             <date-time-picker
-              :selected-time-window="selectedTimeWindow"
-              @onApply="onDateTimePickerApply"
+              :start="startDate"
+              :end="endDate"
+              @apply="onDateTimePickerApply"
+              @invalid="onDateTimePickerInvalid"
             />
           </gl-form-group>
         </template>

app/assets/javascripts/monitoring/components/date_time_picker/date_time_picker.vue

@@ -5,14 +5,21 @@ import Icon from '~/vue_shared/components/icon.vue';
 import DateTimePickerInput from './date_time_picker_input.vue';
 import {
   getTimeDiff,
+  isValidDate,
   getTimeWindow,
   stringToISODate,
   ISODateToString,
   truncateZerosInDateTime,
   isDateTimePickerInputValid,
 } from '~/monitoring/utils';
+
 import { timeWindows } from '~/monitoring/constants';
 
+const events = {
+  apply: 'apply',
+  invalid: 'invalid',
+};
+
 export default {
   components: {
     Icon,
@@ -23,77 +30,94 @@ export default {
     GlDropdownItem,
   },
   props: {
+    start: {
+      type: String,
+      required: true,
+    },
+    end: {
+      type: String,
+      required: true,
+    },
     timeWindows: {
       type: Object,
       required: false,
       default: () => timeWindows,
     },
-    selectedTimeWindow: {
-      type: Object,
-      required: false,
-      default: () => {},
-    },
   },
   data() {
     return {
-      selectedTimeWindowText: '',
-      customTime: {
-        from: null,
-        to: null,
-      },
+      startDate: this.start,
+      endDate: this.end,
     };
   },
   computed: {
-    applyEnabled() {
-      return Boolean(this.inputState.from && this.inputState.to);
+    startInputValid() {
+      return isValidDate(this.startDate);
     },
-    inputState() {
-      const { from, to } = this.customTime;
-      return {
-        from: from && isDateTimePickerInputValid(from),
-        to: to && isDateTimePickerInputValid(to),
-      };
+    endInputValid() {
+      return isValidDate(this.endDate);
     },
-  },
-  watch: {
-    selectedTimeWindow() {
-      this.verifyTimeRange();
+    isValid() {
+      return this.startInputValid && this.endInputValid;
+    },
+
+    startInput: {
+      get() {
+        return this.startInputValid ? this.formatDate(this.startDate) : this.startDate;
+      },
+      set(val) {
+        // Attempt to set a formatted date if possible
+        this.startDate = isDateTimePickerInputValid(val) ? stringToISODate(val) : val;
+      },
+    },
+    endInput: {
+      get() {
+        return this.endInputValid ? this.formatDate(this.endDate) : this.endDate;
+      },
+      set(val) {
+        // Attempt to set a formatted date if possible
+        this.endDate = isDateTimePickerInputValid(val) ? stringToISODate(val) : val;
+      },
+    },
+
+    timeWindowText() {
+      const timeWindow = getTimeWindow({ start: this.start, end: this.end });
+      if (timeWindow) {
+        return this.timeWindows[timeWindow];
+      } else if (isValidDate(this.start) && isValidDate(this.end)) {
+        return sprintf(s__('%{start} to %{end}'), {
+          start: this.formatDate(this.start),
+          end: this.formatDate(this.end),
+        });
+      }
+      return '';
     },
   },
   mounted() {
-    this.verifyTimeRange();
+    // Validate on mounted, and trigger an update if needed
+    if (!this.isValid) {
+      this.$emit(events.invalid);
+    }
   },
   methods: {
-    activeTimeWindow(key) {
-      return this.timeWindows[key] === this.selectedTimeWindowText;
+    formatDate(date) {
+      return truncateZerosInDateTime(ISODateToString(date));
     },
-    setCustomTimeWindowParameter() {
-      this.$emit('onApply', {
-        start: stringToISODate(this.customTime.from),
-        end: stringToISODate(this.customTime.to),
-      });
-    },
-    setTimeWindowParameter(key) {
+    setTimeWindow(key) {
       const { start, end } = getTimeDiff(key);
-      this.$emit('onApply', {
-        start,
-        end,
-      });
+      this.startDate = start;
+      this.endDate = end;
+
+      this.apply();
     },
     closeDropdown() {
       this.$refs.dropdown.hide();
     },
-    verifyTimeRange() {
-      const range = getTimeWindow(this.selectedTimeWindow);
-      if (range) {
-        this.selectedTimeWindowText = this.timeWindows[range];
-      } else {
-        this.customTime = {
-          from: truncateZerosInDateTime(ISODateToString(this.selectedTimeWindow.start)),
-          to: truncateZerosInDateTime(ISODateToString(this.selectedTimeWindow.end)),
-        };
-        this.selectedTimeWindowText = sprintf(s__('%{from} to %{to}'), this.customTime);
-      }
+    apply() {
+      this.$emit(events.apply, {
+        start: this.startDate,
+        end: this.endDate,
+      });
     },
   },
 };
@@ -101,7 +125,7 @@ export default {
 <template>
   <gl-dropdown
     ref="dropdown"
-    :text="selectedTimeWindowText"
+    :text="timeWindowText"
     menu-class="time-window-dropdown-menu"
     class="js-time-window-dropdown"
   >
@@ -113,24 +137,21 @@ export default {
       >
         <date-time-picker-input
           id="custom-time-from"
-          v-model="customTime.from"
+          v-model="startInput"
           :label="__('From')"
-          :state="inputState.from"
+          :state="startInputValid"
         />
         <date-time-picker-input
           id="custom-time-to"
-          v-model="customTime.to"
+          v-model="endInput"
           :label="__('To')"
-          :state="inputState.to"
+          :state="endInputValid"
         />
         <gl-form-group>
           <gl-button @click="closeDropdown">{{ __('Cancel') }}</gl-button>
-          <gl-button
-            variant="success"
-            :disabled="!applyEnabled"
-            @click="setCustomTimeWindowParameter"
-            >{{ __('Apply') }}</gl-button
-          >
+          <gl-button variant="success" :disabled="!isValid" @click="apply()">
+            {{ __('Apply') }}
+          </gl-button>
         </gl-form-group>
       </gl-form-group>
       <gl-form-group
@@ -142,14 +163,14 @@ export default {
         <gl-dropdown-item
           v-for="(value, key) in timeWindows"
           :key="key"
-          :active="activeTimeWindow(key)"
+          :active="value === timeWindowText"
           active-class="active"
-          @click="setTimeWindowParameter(key)"
+          @click="setTimeWindow(key)"
         >
           <icon
             name="mobile-issue-close"
             class="align-bottom"
-            :class="{ invisible: !activeTimeWindow(key) }"
+            :class="{ invisible: value !== timeWindowText }"
           />
           {{ value }}
         </gl-dropdown-item>

locale/gitlab.pot

@@ -269,9 +269,6 @@ msgstr ""
 msgid "%{firstLabel} +%{labelCount} more"
 msgstr ""
 
-msgid "%{from} to %{to}"
-msgstr ""
-
 msgid "%{global_id} is not a valid id for %{expected_type}."
 msgstr ""
 
@@ -370,6 +367,9 @@ msgstr ""
 msgid "%{spammable_titlecase} was submitted to Akismet successfully."
 msgstr ""
 
+msgid "%{start} to %{end}"
+msgstr ""
+
 msgid "%{state} epics"
 msgstr ""
 
@@ -11478,7 +11478,7 @@ msgstr ""
 msgid "Metrics|Legend label (optional)"
 msgstr ""
 
-msgid "Metrics|Link contains an invalid time window."
+msgid "Metrics|Link contains an invalid time window, please verify the link to see the requested time range."
 msgstr ""
 
 msgid "Metrics|Max"

spec/frontend/monitoring/components/dashboard_spec.js

@@ -10,7 +10,6 @@ import DateTimePicker from '~/monitoring/components/date_time_picker/date_time_p
 import GroupEmptyState from '~/monitoring/components/group_empty_state.vue';
 import { createStore } from '~/monitoring/stores';
 import * as types from '~/monitoring/stores/mutation_types';
-import * as monitoringUtils from '~/monitoring/utils';
 import { setupComponentStore, propsData } from '../init_utils';
 import {
   metricsGroupsAPIResponse,
@@ -137,7 +136,6 @@ describe('Dashboard', () => {
     });
 
     it('fetches the metrics data with proper time window', done => {
-      const getTimeDiffSpy = jest.spyOn(monitoringUtils, 'getTimeDiff');
       jest.spyOn(store, 'dispatch');
 
       createMountedWrapper(
@@ -154,7 +152,6 @@ describe('Dashboard', () => {
         .$nextTick()
         .then(() => {
           expect(store.dispatch).toHaveBeenCalled();
-          expect(getTimeDiffSpy).toHaveBeenCalled();
 
           done();
         })

spec/frontend/monitoring/components/dashboard_time_url_spec.js

 import { mount, createLocalVue } from '@vue/test-utils';
 import createFlash from '~/flash';
+import MockAdapter from 'axios-mock-adapter';
 import Dashboard from '~/monitoring/components/dashboard.vue';
 import { createStore } from '~/monitoring/stores';
 import { propsData } from '../init_utils';
+import axios from '~/lib/utils/axios_utils';
 
 const localVue = createLocalVue();
 
@@ -15,6 +17,7 @@ jest.mock('~/lib/utils/url_utility', () => ({
 describe('dashboard invalid url parameters', () => {
   let store;
   let wrapper;
+  let mock;
 
   const createMountedWrapper = (props = {}, options = {}) => {
     wrapper = mount(localVue.extend(Dashboard), {
@@ -28,12 +31,14 @@ describe('dashboard invalid url parameters', () => {
 
   beforeEach(() => {
     store = createStore();
+    mock = new MockAdapter(axios);
   });
 
   afterEach(() => {
     if (wrapper) {
       wrapper.destroy();
     }
+    mock.restore();
   });
 
   it('shows an error message if invalid url parameters are passed', done => {
@@ -46,7 +51,6 @@ describe('dashboard invalid url parameters', () => {
       .$nextTick()
       .then(() => {
         expect(createFlash).toHaveBeenCalled();
-
         done();
       })
       .catch(done.fail);

spec/frontend/monitoring/components/date_time_picker/date_time_picker_spec.js

@@ -3,10 +3,8 @@ import DateTimePicker from '~/monitoring/components/date_time_picker/date_time_p
 import { timeWindows } from '~/monitoring/constants';
 
 const timeWindowsCount = Object.keys(timeWindows).length;
-const selectedTimeWindow = {
-  start: '2019-10-10T07:00:00.000Z',
-  end: '2019-10-13T07:00:00.000Z',
-};
+const start = '2019-10-10T07:00:00.000Z';
+const end = '2019-10-13T07:00:00.000Z';
 const selectedTimeWindowText = `3 days`;
 
 describe('DateTimePicker', () => {
@@ -28,7 +26,8 @@ describe('DateTimePicker', () => {
     dateTimePicker = mount(DateTimePicker, {
       propsData: {
         timeWindows,
-        selectedTimeWindow,
+        start,
+        end,
         ...props,
       },
       sync: false,
@@ -66,10 +65,8 @@ describe('DateTimePicker', () => {
 
   it('renders inputs with h/m/s truncated if its all 0s', done => {
     createComponent({
-      selectedTimeWindow: {
-        start: '2019-10-10T00:00:00.000Z',
-        end: '2019-10-14T00:10:00.000Z',
-      },
+      start: '2019-10-10T00:00:00.000Z',
+      end: '2019-10-14T00:10:00.000Z',
     });
     dateTimePicker.vm.$nextTick(() => {
       expect(dateTimePicker.find('#custom-time-from').element.value).toBe('2019-10-10');
@@ -98,8 +95,10 @@ describe('DateTimePicker', () => {
     });
   });
 
-  it('renders a disabled apply button on load', () => {
-    createComponent();
+  it('renders a disabled apply button on wrong input', () => {
+    createComponent({
+      start: 'invalid-input-date',
+    });
 
     expect(applyButtonElement().getAttribute('disabled')).toBe('disabled');
   });
@@ -131,29 +130,29 @@ describe('DateTimePicker', () => {
     fillInputAndBlur('#custom-time-from', '2019-10-01')
       .then(() => fillInputAndBlur('#custom-time-to', '2019-10-19'))
       .then(() => {
-        dateTimePicker.vm.$nextTick(() => {
-          expect(applyButtonElement().getAttribute('disabled')).toBeNull();
-          done();
-        });
+        expect(applyButtonElement().getAttribute('disabled')).toBeNull();
+        done();
       })
-      .catch(done);
+      .catch(done.fail);
   });
 
-  it('returns an object when apply is clicked', done => {
+  it('emits dates in an object when apply is clicked', done => {
     createComponent();
     fillInputAndBlur('#custom-time-from', '2019-10-01')
       .then(() => fillInputAndBlur('#custom-time-to', '2019-10-19'))
       .then(() => {
-        jest.spyOn(dateTimePicker.vm, '$emit');
         applyButtonElement().click();
 
-        expect(dateTimePicker.vm.$emit).toHaveBeenCalledWith('onApply', {
-          end: '2019-10-19T00:00:00Z',
-          start: '2019-10-01T00:00:00Z',
-        });
+        expect(dateTimePicker.emitted().apply).toHaveLength(1);
+        expect(dateTimePicker.emitted().apply[0]).toEqual([
+          {
+            end: '2019-10-19T00:00:00Z',
+            start: '2019-10-01T00:00:00Z',
+          },
+        ]);
         done();
       })
-      .catch(done);
+      .catch(done.fail);
   });
 
   it('hides the popover with cancel button', done => {