Merge branch 'mw-productivity-analytics-no-access-error-handling' into 'master'

Productivity Analytics: Add error handling for reporting on groups which have no plan

Closes #13571

See merge request gitlab-org/gitlab-ee!15291

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

@@ -36,6 +36,10 @@ export default {
       type: String,
       required: true,
     },
+    noAccessSvgPath: {
+      type: String,
+      required: true,
+    },
   },
   data() {
     return {
@@ -65,7 +69,11 @@ export default {
       'getColumnOptions',
       'columnMetricLabel',
       'isSelectedSortField',
+      'hasNoAccessError',
     ]),
+    showAppContent() {
+      return this.groupNamespace && !this.hasNoAccessError;
+    },
   },
   mounted() {
     this.setEndpoint(this.endpoint);
@@ -106,7 +114,18 @@ export default {
         )
       "
     />
-    <template v-else>
+    <gl-empty-state
+      v-if="hasNoAccessError"
+      class="js-empty-state"
+      :title="__('You don’t have acces to Productivity Analaytics in this group')"
+      :svg-path="noAccessSvgPath"
+      :description="
+        __(
+          'Only ‘Reporter’ roles and above on tiers Premium / Silver and above can see Productivity Analytics.',
+        )
+      "
+    />
+    <template v-if="showAppContent">
       <h4>{{ __('Merge Requests') }}</h4>
       <div class="qa-time-to-merge mb-4">
         <h5>{{ __('Time to merge') }}</h5>

ee/app/assets/javascripts/analytics/productivity_analytics/index.js

@@ -16,7 +16,7 @@ export default () => {
   const timeframeContainer = container.querySelector('.js-timeframe-container');
   const appContainer = container.querySelector('.js-productivity-analytics-app-container');
 
-  const { endpoint, emptyStateSvgPath } = appContainer.dataset;
+  const { endpoint, emptyStateSvgPath, noAccessSvgPath } = appContainer.dataset;
 
   let filterManager;
 
@@ -100,6 +100,7 @@ export default () => {
         props: {
           endpoint,
           emptyStateSvgPath,
+          noAccessSvgPath,
         },
       });
     },

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

@@ -26,9 +26,7 @@ export const fetchChartData = ({ dispatch, getters, rootState }, chartKey) => {
       const { data } = response;
       dispatch('receiveChartDataSuccess', { chartKey, data });
     })
-    .catch(() => {
-      dispatch('receiveChartDataError', chartKey);
-    });
+    .catch(() => dispatch('receiveChartDataError', chartKey));
 };
 
 export const receiveChartDataSuccess = ({ commit }, { chartKey, data = {} }) => {

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

@@ -3,8 +3,11 @@ import * as types from './mutation_types';
 export const setGroupNamespace = ({ commit, dispatch }, groupNamespace) => {
   commit(types.SET_GROUP_NAMESPACE, groupNamespace);
 
-  dispatch('charts/fetchAllChartData', null, { root: true });
-  dispatch('table/fetchMergeRequests', null, { root: true });
+  // let's fetch the merge requests first to see if the user has access to the selected group
+  // if there's no 403, then we fetch all chart data
+  return dispatch('table/fetchMergeRequests', null, { root: true }).then(() =>
+    dispatch('charts/fetchAllChartData', null, { root: true }),
+  );
 };
 
 export const setProjectPath = ({ commit, dispatch }, projectPath) => {

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

@@ -21,9 +21,7 @@ export const fetchMergeRequests = ({ dispatch, state, rootState, rootGetters })
       const { headers, data } = response;
       dispatch('receiveMergeRequestsSuccess', { headers, data });
     })
-    .catch(() => {
-      dispatch('receiveMergeRequestsError');
-    });
+    .catch(err => dispatch('receiveMergeRequestsError', err));
 };
 
 export const requestMergeRequests = ({ commit }) => commit(types.REQUEST_MERGE_REQUESTS);
@@ -35,7 +33,10 @@ export const receiveMergeRequestsSuccess = ({ commit }, { headers, data: mergeRe
   commit(types.RECEIVE_MERGE_REQUESTS_SUCCESS, { pageInfo, mergeRequests });
 };
 
-export const receiveMergeRequestsError = ({ commit }) => commit(types.RECEIVE_MERGE_REQUESTS_ERROR);
+export const receiveMergeRequestsError = ({ commit }, { response }) => {
+  const { status } = response;
+  commit(types.RECEIVE_MERGE_REQUESTS_ERROR, status);
+};
 
 export const setSortField = ({ commit, dispatch }, data) => {
   commit(types.SET_SORT_FIELD, data);

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

+import httpStatus from '~/lib/utils/http_status';
 import { tableSortOrder } from './../../../constants';
 
 export const sortIcon = state => tableSortOrder[state.sortOrder].icon;
@@ -18,5 +19,7 @@ export const columnMetricLabel = (state, getters) => getters.getColumnOptions[st
 
 export const isSelectedSortField = state => sortField => state.sortField === sortField;
 
+export const hasNoAccessError = state => state.hasError === httpStatus.FORBIDDEN;
+
 // prevent babel-plugin-rewire from generating an invalid default during karma tests
 export default () => {};

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

@@ -11,9 +11,9 @@ export default {
     state.pageInfo = pageInfo;
     state.mergeRequests = mergeRequests;
   },
-  [types.RECEIVE_MERGE_REQUESTS_ERROR](state) {
+  [types.RECEIVE_MERGE_REQUESTS_ERROR](state, errCode) {
     state.isLoadingTable = false;
-    state.hasError = true;
+    state.hasError = errCode;
     state.pageInfo = {};
     state.mergeRequests = [];
   },

ee/app/views/analytics/productivity_analytics/show.html.haml

@@ -6,4 +6,4 @@
     .js-search-bar.filter-container.hide
       = render 'shared/issuable/search_bar', type: :productivity_analytics
     .js-timeframe-container
-  .js-productivity-analytics-app-container{ data: { endpoint: analytics_productivity_analytics_path, empty_state_svg_path: image_path('illustrations/productivity-analytics-empty-state.svg') } }
+  .js-productivity-analytics-app-container{ data: { endpoint: analytics_productivity_analytics_path, empty_state_svg_path: image_path('illustrations/productivity-analytics-empty-state.svg'), no_access_svg_path: image_path('illustrations/analytics/no-access.svg') } }

ee/changelogs/unreleased/mw-productivity-analytics-no-access-error-handling.yml

+---
+title: 'Productivity Analytics: Add error handling for reporting on groups which have
+  no plan'
+merge_request: 15291
+author:
+type: other

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

@@ -8,29 +8,32 @@ describe('Productivity analytics filter actions', () => {
   const projectPath = 'gitlab-test';
 
   describe('setGroupNamespace', () => {
-    it('commits the SET_GROUP_NAMESPACE mutation', done =>
-      testAction(
-        actions.setGroupNamespace,
-        groupNamespace,
-        getInitialState(),
-        [
-          {
-            type: types.SET_GROUP_NAMESPACE,
-            payload: groupNamespace,
-          },
-        ],
-        [
-          {
-            type: 'charts/fetchAllChartData',
-            payload: null,
-          },
-          {
-            type: 'table/fetchMergeRequests',
-            payload: null,
-          },
-        ],
-        done,
-      ));
+    it('commits the SET_GROUP_NAMESPACE mutation', done => {
+      const store = {
+        commit: jest.fn(),
+        dispatch: jest.fn(() => Promise.resolve()),
+      };
+
+      actions
+        .setGroupNamespace(store, groupNamespace)
+        .then(() => {
+          expect(store.commit).toHaveBeenCalledWith(types.SET_GROUP_NAMESPACE, groupNamespace);
+
+          expect(store.dispatch.mock.calls[0]).toEqual([
+            'table/fetchMergeRequests',
+            jasmine.any(Object),
+            { root: true },
+          ]);
+
+          expect(store.dispatch.mock.calls[1]).toEqual([
+            'charts/fetchAllChartData',
+            jasmine.any(Object),
+            { root: true },
+          ]);
+        })
+        .then(done)
+        .catch(done.fail);
+    });
   });
 
   describe('setProjectPath', () => {

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

@@ -123,15 +123,22 @@ describe('Productivity analytics table actions', () => {
         mock.onGet(mockedState.endpoint).replyOnce(500);
       });
 
-      it('dispatches error', done =>
+      it('dispatches error', done => {
         testAction(
           actions.fetchMergeRequests,
           null,
           mockedState,
           [],
-          [{ type: 'requestMergeRequests' }, { type: 'receiveMergeRequestsError' }],
+          [
+            { type: 'requestMergeRequests' },
+            {
+              type: 'receiveMergeRequestsError',
+              payload: new Error('Request failed with status code 500'),
+            },
+          ],
           done,
-        ));
+        );
+      });
     });
   });
 
@@ -168,9 +175,9 @@ describe('Productivity analytics table actions', () => {
     it('should commit error', done =>
       testAction(
         actions.receiveMergeRequestsError,
-        null,
+        { response: { status: 500 } },
         mockedContext.state,
-        [{ type: types.RECEIVE_MERGE_REQUESTS_ERROR }],
+        [{ type: types.RECEIVE_MERGE_REQUESTS_ERROR, payload: 500 }],
         [],
         done,
       ));

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

@@ -69,4 +69,16 @@ describe('Productivity analytics table getters', () => {
       });
     });
   });
+
+  describe('hasNoAccessError', () => {
+    it('returns true if "hasError" is set to 403', () => {
+      state.hasError = 403;
+      expect(getters.hasNoAccessError(state)).toEqual(true);
+    });
+
+    it('returns false if "hasError" is not set to 403', () => {
+      state.hasError = false;
+      expect(getters.hasNoAccessError(state)).toEqual(false);
+    });
+  });
 });

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

@@ -40,11 +40,12 @@ describe('Productivity analytics table mutations', () => {
   });
 
   describe(types.RECEIVE_MERGE_REQUESTS_ERROR, () => {
-    it('sets isError and clears data', () => {
-      mutations[types.RECEIVE_MERGE_REQUESTS_ERROR](state);
+    it('sets hasError to error code and clears data', () => {
+      const errorCode = 500;
+      mutations[types.RECEIVE_MERGE_REQUESTS_ERROR](state, errorCode);
 
       expect(state.isLoadingTable).toBe(false);
-      expect(state.hasError).toBe(true);
+      expect(state.hasError).toBe(errorCode);
       expect(state.mergeRequests).toEqual([]);
       expect(state.pageInfo).toEqual({});
     });

locale/gitlab.pot

@@ -10567,6 +10567,9 @@ msgstr ""
 msgid "Only users with an email address in this domain can be added to the group.<br>Example: <code>gitlab.com</code>. Some common domains are not allowed. %{read_more_link}."
 msgstr ""
 
+msgid "Only ‘Reporter’ roles and above on tiers Premium / Silver and above can see Productivity Analytics."
+msgstr ""
+
 msgid "Oops, are you sure?"
 msgstr ""
 
@@ -17827,6 +17830,9 @@ msgstr ""
 msgid "You don't have any recent searches"
 msgstr ""
 
+msgid "You don’t have acces to Productivity Analaytics in this group"
+msgstr ""
+
 msgid "You have been granted %{access_level} access to the %{source_link} %{source_type}."
 msgstr ""