Merge branch 'mw-productivity-analytics-scatterplot-backup-feature-flag' into 'master'

Productivity Analytics: Show scatterplot behind feature flag

See merge request gitlab-org/gitlab!17735

ee/app/assets/javascripts/analytics/productivity_analytics/components/app.vue

@@ -9,6 +9,7 @@ import {
   GlTooltipDirective,
 } from '@gitlab/ui';
 import { GlColumnChart } from '@gitlab/ui/dist/charts';
+import featureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import Icon from '~/vue_shared/components/icon.vue';
 import MetricChart from './metric_chart.vue';
 import Scatterplot from '../../shared/components/scatterplot.vue';
@@ -31,6 +32,7 @@ export default {
   directives: {
     GlTooltip: GlTooltipDirective,
   },
+  mixins: [featureFlagsMixin()],
   props: {
     endpoint: {
       type: String,
@@ -65,6 +67,7 @@ export default {
       'getSelectedMetric',
       'scatterplotYaxisLabel',
       'hasNoAccessError',
+      'isChartEnabled',
     ]),
     ...mapGetters('table', [
       'sortFieldDropdownLabel',
@@ -89,10 +92,19 @@ export default {
   },
   mounted() {
     this.setEndpoint(this.endpoint);
+    this.setChartEnabled({
+      chartKey: chartKeys.scatterplot,
+      isEnabled: this.isScatterplotFeatureEnabled(),
+    });
   },
   methods: {
     ...mapActions(['setEndpoint']),
-    ...mapActions('charts', ['fetchChartData', 'setMetricType', 'chartItemClicked']),
+    ...mapActions('charts', [
+      'fetchChartData',
+      'setMetricType',
+      'chartItemClicked',
+      'setChartEnabled',
+    ]),
     ...mapActions('table', ['setSortField', 'setPage', 'toggleSortOrder', 'setColumnMetric']),
     onMainChartItemClicked({ params }) {
       const itemValue = params.data.value[0];
@@ -108,6 +120,9 @@ export default {
         ...this.getColumnChartDatazoomOption(chartKey),
       };
     },
+    isScatterplotFeatureEnabled() {
+      return this.glFeatures.productivityAnalyticsScatterplotEnabled;
+    },
   },
 };
 </script>
@@ -217,6 +232,7 @@ export default {
           </div>
 
           <metric-chart
+            v-if="isChartEnabled(chartKeys.scatterplot)"
             ref="scatterplot"
             class="mb-4"
             :title="s__('ProductivityAnalytics|Trendline')"

ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/actions.js

@@ -16,18 +16,21 @@ export const fetchSecondaryChartData = ({ state, dispatch }) => {
 export const requestChartData = ({ commit }, chartKey) =>
   commit(types.REQUEST_CHART_DATA, chartKey);
 
-export const fetchChartData = ({ dispatch, getters, rootState }, chartKey) => {
+export const fetchChartData = ({ dispatch, getters, state, rootState }, chartKey) => {
+  // let's fetch data for enabled charts only
+  if (state.charts[chartKey].enabled) {
     dispatch('requestChartData', chartKey);
 
     const params = getters.getFilterParams(chartKey);
 
-  return axios
+    axios
       .get(rootState.endpoint, { params })
       .then(response => {
         const { data } = response;
         dispatch('receiveChartDataSuccess', { chartKey, data });
       })
       .catch(error => dispatch('receiveChartDataError', { chartKey, error }));
+  }
 };
 
 export const receiveChartDataSuccess = ({ commit }, { chartKey, data = {} }) => {
@@ -57,5 +60,8 @@ export const chartItemClicked = ({ commit, dispatch }, { chartKey, item }) => {
   dispatch('table/setPage', 0, { root: true });
 };
 
+export const setChartEnabled = ({ commit }, { chartKey, isEnabled }) =>
+  commit(types.SET_CHART_ENABLED, { chartKey, isEnabled });
+
 // prevent babel-plugin-rewire from generating an invalid default during karma tests
 export default () => {};

ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/getters.js

@@ -169,5 +169,7 @@ export const scatterplotYaxisLabel = (_state, getters, rootState) => {
 export const hasNoAccessError = state =>
   state.charts[chartKeys.main].errorCode === httpStatus.FORBIDDEN;
 
+export const isChartEnabled = state => chartKey => state.charts[chartKey].enabled;
+
 // prevent babel-plugin-rewire from generating an invalid default during karma tests
 export default () => {};

ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/mutation_types.js

@@ -5,3 +5,4 @@ export const RECEIVE_CHART_DATA_ERROR = 'RECEIVE_CHART_DATA_ERROR';
 
 export const SET_METRIC_TYPE = 'SET_METRIC_TYPE';
 export const UPDATE_SELECTED_CHART_ITEMS = 'UPDATE_SELECTED_CHART_ITEMS';
+export const SET_CHART_ENABLED = 'SET_CHART_ENABLED';

ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/mutations.js

@@ -29,4 +29,7 @@ export default {
       state.charts[chartKey].selected.splice(idx, 1);
     }
   },
+  [types.SET_CHART_ENABLED](state, { chartKey, isEnabled }) {
+    state.charts[chartKey].enabled = isEnabled;
+  },
 };

ee/app/assets/javascripts/analytics/productivity_analytics/store/modules/charts/state.js

@@ -5,6 +5,7 @@ export default () => ({
     [chartKeys.main]: {
       isLoading: false,
       errorCode: null,
+      enabled: true,
       data: {},
       selected: [],
       params: {
@@ -14,6 +15,7 @@ export default () => ({
     [chartKeys.timeBasedHistogram]: {
       isLoading: false,
       errorCode: null,
+      enabled: true,
       data: {},
       selected: [],
       params: {
@@ -24,6 +26,7 @@ export default () => ({
     [chartKeys.commitBasedHistogram]: {
       isLoading: false,
       errorCode: null,
+      enabled: true,
       data: {},
       selected: [],
       params: {
@@ -34,6 +37,7 @@ export default () => ({
     [chartKeys.scatterplot]: {
       isLoading: false,
       errorCode: null,
+      enabled: true,
       data: {},
       selected: [],
       params: {

ee/app/controllers/analytics/productivity_analytics_controller.rb

@@ -13,6 +13,9 @@ class Analytics::ProductivityAnalyticsController < Analytics::ApplicationControl
   before_action -> {
     authorize_view_productivity_analytics!(:view_productivity_analytics)
   }
+  before_action -> {
+    push_frontend_feature_flag(:productivity_analytics_scatterplot_enabled, default_enabled: true)
+  }
 
   include IssuableCollections
 

ee/spec/frontend/analytics/productivity_analytics/components/app_spec.js

@@ -34,8 +34,7 @@ describe('ProductivityApp component', () => {
 
   const mainChartData = { 1: 2, 2: 3 };
 
-  beforeEach(() => {
-    mock = new MockAdapter(axios);
+  const createComponent = (scatterplotEnabled = true) => {
     wrapper = shallowMount(localVue.extend(ProductivityApp), {
       localVue,
       store,
@@ -44,7 +43,14 @@ describe('ProductivityApp component', () => {
       methods: {
         ...actionSpies,
       },
+      provide: {
+        glFeatures: { productivityAnalyticsScatterplotEnabled: scatterplotEnabled },
+      },
     });
+  };
+
+  beforeEach(() => {
+    mock = new MockAdapter(axios);
   });
 
   afterEach(() => {
@@ -66,6 +72,7 @@ describe('ProductivityApp component', () => {
   describe('template', () => {
     describe('without a group being selected', () => {
       it('renders the empty state illustration', () => {
+        createComponent();
         const emptyState = wrapper.find(GlEmptyState);
         expect(emptyState.exists()).toBe(true);
 
@@ -81,6 +88,7 @@ describe('ProductivityApp component', () => {
 
       describe('user has no access to the group', () => {
         beforeEach(() => {
+          createComponent();
           const error = { response: { status: 403 } };
           wrapper.vm.$store.dispatch('charts/receiveChartDataError', {
             chartKey: chartKeys.main,
@@ -106,6 +114,7 @@ describe('ProductivityApp component', () => {
 
         describe('when the main chart is loading', () => {
           beforeEach(() => {
+            createComponent();
             wrapper.vm.$store.dispatch('charts/requestChartData', chartKeys.main);
           });
 
@@ -130,6 +139,7 @@ describe('ProductivityApp component', () => {
         describe('when the main chart finished loading', () => {
           describe('and has data', () => {
             beforeEach(() => {
+              createComponent();
               wrapper.vm.$store.dispatch('charts/receiveChartDataSuccess', {
                 chartKey: chartKeys.main,
                 data: mainChartData,
@@ -252,6 +262,11 @@ describe('ProductivityApp component', () => {
             });
 
             describe('Scatterplot', () => {
+              describe('when the feature flag is enabled', () => {
+                it('isScatterplotFeatureEnabled returns true', () => {
+                  expect(wrapper.vm.isScatterplotFeatureEnabled()).toBe(true);
+                });
+
                 it('renders a metric chart component', () => {
                   expect(findScatterplotMetricChart().exists()).toBe(true);
                 });
@@ -298,6 +313,21 @@ describe('ProductivityApp component', () => {
                 });
               });
 
+              describe('when the feature flag is disabled', () => {
+                beforeEach(() => {
+                  createComponent(false);
+                });
+
+                it('isScatterplotFeatureEnabled returns false', () => {
+                  expect(wrapper.vm.isScatterplotFeatureEnabled()).toBe(false);
+                });
+
+                it("doesn't render a metric chart component", () => {
+                  expect(findScatterplotMetricChart().exists()).toBe(false);
+                });
+              });
+            });
+
             describe('MR table', () => {
               describe('when table is loading', () => {
                 beforeEach(() => {
@@ -400,6 +430,7 @@ describe('ProductivityApp component', () => {
 
           describe('and has no data', () => {
             beforeEach(() => {
+              createComponent();
               wrapper.vm.$store.dispatch('charts/receiveChartDataSuccess', {
                 chartKey: chartKeys.main,
                 data: {},

ee/spec/frontend/analytics/productivity_analytics/store/modules/charts/actions_spec.js

@@ -47,6 +47,7 @@ describe('Productivity analytics chart actions', () => {
   });
 
   describe('fetchChartData', () => {
+    describe('when chart is enabled', () => {
       describe('success', () => {
         beforeEach(() => {
           mock.onGet(mockedState.endpoint).replyOnce(200, mockHistogramData);
@@ -106,6 +107,7 @@ describe('Productivity analytics chart actions', () => {
         });
       });
     });
+  });
 
   describe('requestChartData', () => {
     it('should commit the request mutation', done => {
@@ -118,6 +120,24 @@ describe('Productivity analytics chart actions', () => {
         done,
       );
     });
+
+    describe('when chart is disabled', () => {
+      const disabledChartKey = chartKeys.scatterplot;
+      beforeEach(() => {
+        mock.onGet(mockedState.endpoint).replyOnce(200);
+        mockedState.charts[disabledChartKey].enabled = false;
+      });
+
+      it('does not dispatch the requestChartData action', done => {
+        testAction(actions.fetchChartData, disabledChartKey, mockedState, [], [], done);
+      });
+
+      it('does not call the API', () => {
+        actions.fetchChartData(mockedContext, disabledChartKey);
+        jest.spyOn(axios, 'get');
+        expect(axios.get).not.toHaveBeenCalled();
+      });
+    });
   });
 
   describe('receiveChartDataSuccess', () => {
@@ -205,4 +225,22 @@ describe('Productivity analytics chart actions', () => {
       );
     });
   });
+
+  describe('setChartEnabled', () => {
+    it('should commit enabled state', done => {
+      testAction(
+        actions.setChartEnabled,
+        { chartKey: chartKeys.scatterplot, isEnabled: false },
+        mockedContext.state,
+        [
+          {
+            type: types.SET_CHART_ENABLED,
+            payload: { chartKey: chartKeys.scatterplot, isEnabled: false },
+          },
+        ],
+        [],
+        done,
+      );
+    });
+  });
 });

ee/spec/frontend/analytics/productivity_analytics/store/modules/charts/getters_spec.js

@@ -283,4 +283,17 @@ describe('Productivity analytics chart getters', () => {
       expect(getters.hasNoAccessError(state)).toEqual(false);
     });
   });
+
+  describe('isChartEnabled', () => {
+    const chartKey = chartKeys.scatterplot;
+    it('returns true if the chart is enabled', () => {
+      state.charts[chartKey].enabled = true;
+      expect(getters.isChartEnabled(state)(chartKey)).toBe(true);
+    });
+
+    it('returns false if the chart is disabled', () => {
+      state.charts[chartKey].enabled = false;
+      expect(getters.isChartEnabled(state)(chartKey)).toBe(false);
+    });
+  });
 });

ee/spec/frontend/analytics/productivity_analytics/store/modules/charts/mutations_spec.js

@@ -85,4 +85,20 @@ describe('Productivity analytics chart mutations', () => {
       expect(state.charts[chartKey].selected).toEqual([]);
     });
   });
+
+  describe(types.SET_CHART_ENABLED, () => {
+    chartKey = chartKeys.scatterplot;
+
+    it('sets the enabled flag to true on the scatterplot chart', () => {
+      mutations[types.SET_CHART_ENABLED](state, { chartKey, isEnabled: true });
+
+      expect(state.charts[chartKey].enabled).toBe(true);
+    });
+
+    it('sets the enabled flag to false on the scatterplot chart', () => {
+      mutations[types.SET_CHART_ENABLED](state, { chartKey, isEnabled: false });
+
+      expect(state.charts[chartKey].enabled).toBe(false);
+    });
+  });
 });