Merge branch 'ide-even-more-improved-errors' into 'master'

Improve error message across IDE store modules

Closes #47323

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

app/assets/javascripts/ide/stores/modules/commit/actions.js

@@ -1,7 +1,6 @@
 import $ from 'jquery';
 import { sprintf, __ } from '~/locale';
 import flash from '~/flash';
-import { stripHtml } from '~/lib/utils/text_utility';
 import * as rootTypes from '../../mutation_types';
 import { createCommitPayload, createNewMergeRequestUrl } from '../../utils';
 import router from '../../../ide_router';
@@ -198,11 +197,18 @@ export const commitChanges = ({ commit, state, getters, dispatch, rootState, roo
       if (err.response.status === 400) {
         $('#ide-create-branch-modal').modal('show');
       } else {
-        let errMsg = __('Error committing changes. Please try again.');
-        if (err.response.data && err.response.data.message) {
-          errMsg += ` (${stripHtml(err.response.data.message)})`;
-        }
-        flash(errMsg, 'alert', document, null, false, true);
+        dispatch(
+          'setErrorMessage',
+          {
+            text: __('An error accured whilst committing your changes.'),
+            action: () =>
+              dispatch('commitChanges').then(() =>
+                dispatch('setErrorMessage', null, { root: true }),
+              ),
+            actionText: __('Please try again'),
+          },
+          { root: true },
+        );
         window.dispatchEvent(new Event('resize'));
       }
 

app/assets/javascripts/ide/stores/modules/merge_requests/actions.js

@@ -1,6 +1,5 @@
 import { __ } from '../../../../locale';
 import Api from '../../../../api';
-import flash from '../../../../flash';
 import router from '../../../ide_router';
 import { scopes } from './constants';
 import * as types from './mutation_types';
@@ -8,8 +7,20 @@ import * as rootTypes from '../../mutation_types';
 
 export const requestMergeRequests = ({ commit }, type) =>
   commit(types.REQUEST_MERGE_REQUESTS, type);
-export const receiveMergeRequestsError = ({ commit }, type) => {
-  flash(__('Error loading merge requests.'));
+export const receiveMergeRequestsError = ({ commit, dispatch }, { type, search }) => {
+  dispatch(
+    'setErrorMessage',
+    {
+      text: __('Error loading merge requests.'),
+      action: payload =>
+        dispatch('fetchMergeRequests', payload).then(() =>
+          dispatch('setErrorMessage', null, { root: true }),
+        ),
+      actionText: __('Please try again'),
+      actionPayload: { type, search },
+    },
+    { root: true },
+  );
   commit(types.RECEIVE_MERGE_REQUESTS_ERROR, type);
 };
 export const receiveMergeRequestsSuccess = ({ commit }, { type, data }) =>
@@ -22,7 +33,7 @@ export const fetchMergeRequests = ({ dispatch, state: { state } }, { type, searc
 
   Api.mergeRequests({ scope, state, search })
     .then(({ data }) => dispatch('receiveMergeRequestsSuccess', { type, data }))
-    .catch(() => dispatch('receiveMergeRequestsError', type));
+    .catch(() => dispatch('receiveMergeRequestsError', { type, search }));
 };
 
 export const resetMergeRequests = ({ commit }, type) => commit(types.RESET_MERGE_REQUESTS, type);

app/assets/javascripts/ide/stores/modules/pipelines/actions.js

@@ -1,7 +1,7 @@
 import Visibility from 'visibilityjs';
 import axios from 'axios';
+import httpStatus from '../../../../lib/utils/http_status';
 import { __ } from '../../../../locale';
-import flash from '../../../../flash';
 import Poll from '../../../../lib/utils/poll';
 import service from '../../../services';
 import { rightSidebarViews } from '../../../constants';
@@ -18,10 +18,27 @@ export const stopPipelinePolling = () => {
 export const restartPipelinePolling = () => {
   if (eTagPoll) eTagPoll.restart();
 };
+export const forcePipelineRequest = () => {
+  if (eTagPoll) eTagPoll.makeRequest();
+};
 
 export const requestLatestPipeline = ({ commit }) => commit(types.REQUEST_LATEST_PIPELINE);
-export const receiveLatestPipelineError = ({ commit, dispatch }) => {
-  flash(__('There was an error loading latest pipeline'));
+export const receiveLatestPipelineError = ({ commit, dispatch }, err) => {
+  if (err.status !== httpStatus.NOT_FOUND) {
+    dispatch(
+      'setErrorMessage',
+      {
+        text: __('An error occured whilst fetching the latest pipline.'),
+        action: () =>
+          dispatch('forcePipelineRequest').then(() =>
+            dispatch('setErrorMessage', null, { root: true }),
+          ),
+        actionText: __('Please try again'),
+        actionPayload: null,
+      },
+      { root: true },
+    );
+  }
   commit(types.RECEIVE_LASTEST_PIPELINE_ERROR);
   dispatch('stopPipelinePolling');
 };
@@ -46,7 +63,7 @@ export const fetchLatestPipeline = ({ dispatch, rootGetters }) => {
     method: 'lastCommitPipelines',
     data: { getters: rootGetters },
     successCallback: ({ data }) => dispatch('receiveLatestPipelineSuccess', data),
-    errorCallback: () => dispatch('receiveLatestPipelineError'),
+    errorCallback: err => dispatch('receiveLatestPipelineError', err),
   });
 
   if (!Visibility.hidden()) {
@@ -63,9 +80,21 @@ export const fetchLatestPipeline = ({ dispatch, rootGetters }) => {
 };
 
 export const requestJobs = ({ commit }, id) => commit(types.REQUEST_JOBS, id);
-export const receiveJobsError = ({ commit }, id) => {
-  flash(__('There was an error loading jobs'));
-  commit(types.RECEIVE_JOBS_ERROR, id);
+export const receiveJobsError = ({ commit, dispatch }, stage) => {
+  dispatch(
+    'setErrorMessage',
+    {
+      text: __('An error occured whilst loading the pipelines jobs.'),
+      action: payload =>
+        dispatch('fetchJobs', payload).then(() =>
+          dispatch('setErrorMessage', null, { root: true }),
+        ),
+      actionText: __('Please try again'),
+      actionPayload: stage,
+    },
+    { root: true },
+  );
+  commit(types.RECEIVE_JOBS_ERROR, stage.id);
 };
 export const receiveJobsSuccess = ({ commit }, { id, data }) =>
   commit(types.RECEIVE_JOBS_SUCCESS, { id, data });
@@ -76,7 +105,7 @@ export const fetchJobs = ({ dispatch }, stage) => {
   axios
     .get(stage.dropdownPath)
     .then(({ data }) => dispatch('receiveJobsSuccess', { id: stage.id, data }))
-    .catch(() => dispatch('receiveJobsError', stage.id));
+    .catch(() => dispatch('receiveJobsError', stage));
 };
 
 export const toggleStageCollapsed = ({ commit }, stageId) =>
@@ -90,8 +119,18 @@ export const setDetailJob = ({ commit, dispatch }, job) => {
 };
 
 export const requestJobTrace = ({ commit }) => commit(types.REQUEST_JOB_TRACE);
-export const receiveJobTraceError = ({ commit }) => {
-  flash(__('Error fetching job trace'));
+export const receiveJobTraceError = ({ commit, dispatch }) => {
+  dispatch(
+    'setErrorMessage',
+    {
+      text: __('An error occured whilst fetching the job trace.'),
+      action: () =>
+        dispatch('fetchJobTrace').then(() => dispatch('setErrorMessage', null, { root: true })),
+      actionText: __('Please try again'),
+      actionPayload: null,
+    },
+    { root: true },
+  );
   commit(types.RECEIVE_JOB_TRACE_ERROR);
 };
 export const receiveJobTraceSuccess = ({ commit }, data) =>

spec/javascripts/ide/stores/modules/merge_requests/actions_spec.js

@@ -2,7 +2,7 @@ import MockAdapter from 'axios-mock-adapter';
 import axios from '~/lib/utils/axios_utils';
 import state from '~/ide/stores/modules/merge_requests/state';
 import * as types from '~/ide/stores/modules/merge_requests/mutation_types';
-import actions, {
+import {
   requestMergeRequests,
   receiveMergeRequestsError,
   receiveMergeRequestsSuccess,
@@ -41,28 +41,26 @@ describe('IDE merge requests actions', () => {
   });
 
   describe('receiveMergeRequestsError', () => {
-    let flashSpy;
-
-    beforeEach(() => {
-      flashSpy = spyOnDependency(actions, 'flash');
-    });
-
     it('should should commit error', done => {
       testAction(
         receiveMergeRequestsError,
-        'created',
+        { type: 'created', search: '' },
         mockedState,
         [{ type: types.RECEIVE_MERGE_REQUESTS_ERROR, payload: 'created' }],
-        [],
+        [
+          {
+            type: 'setErrorMessage',
+            payload: {
+              text: 'Error loading merge requests.',
+              action: jasmine.any(Function),
+              actionText: 'Please try again',
+              actionPayload: { type: 'created', search: '' },
+            },
+          },
+        ],
         done,
       );
     });
-
-    it('creates flash message', () => {
-      receiveMergeRequestsError({ commit() {} }, 'created');
-
-      expect(flashSpy).toHaveBeenCalled();
-    });
   });
 
   describe('receiveMergeRequestsSuccess', () => {

spec/javascripts/ide/stores/modules/pipelines/actions_spec.js

@@ -1,7 +1,7 @@
 import Visibility from 'visibilityjs';
 import MockAdapter from 'axios-mock-adapter';
 import axios from '~/lib/utils/axios_utils';
-import actions, {
+import {
   requestLatestPipeline,
   receiveLatestPipelineError,
   receiveLatestPipelineSuccess,
@@ -59,7 +59,7 @@ describe('IDE pipelines actions', () => {
     it('commits error', done => {
       testAction(
         receiveLatestPipelineError,
-        null,
+        { status: 404 },
         mockedState,
         [{ type: types.RECEIVE_LASTEST_PIPELINE_ERROR }],
         [{ type: 'stopPipelinePolling' }],
@@ -67,12 +67,26 @@ describe('IDE pipelines actions', () => {
       );
     });
 
-    it('creates flash message', () => {
-      const flashSpy = spyOnDependency(actions, 'flash');
-
-      receiveLatestPipelineError({ commit() {}, dispatch() {} });
-
-      expect(flashSpy).toHaveBeenCalled();
+    it('dispatches setErrorMessage is not 404', done => {
+      testAction(
+        receiveLatestPipelineError,
+        { status: 500 },
+        mockedState,
+        [{ type: types.RECEIVE_LASTEST_PIPELINE_ERROR }],
+        [
+          {
+            type: 'setErrorMessage',
+            payload: {
+              text: 'An error occured whilst fetching the latest pipline.',
+              action: jasmine.any(Function),
+              actionText: 'Please try again',
+              actionPayload: null,
+            },
+          },
+          { type: 'stopPipelinePolling' },
+        ],
+        done,
+      );
     });
   });
 
@@ -181,7 +195,10 @@ describe('IDE pipelines actions', () => {
 
         new Promise(resolve => requestAnimationFrame(resolve))
           .then(() => {
-            expect(dispatch.calls.argsFor(1)).toEqual(['receiveLatestPipelineError']);
+            expect(dispatch.calls.argsFor(1)).toEqual([
+              'receiveLatestPipelineError',
+              jasmine.anything(),
+            ]);
           })
           .then(done)
           .catch(done.fail);
@@ -199,21 +216,23 @@ describe('IDE pipelines actions', () => {
     it('commits error', done => {
       testAction(
         receiveJobsError,
-        1,
+        { id: 1 },
         mockedState,
         [{ type: types.RECEIVE_JOBS_ERROR, payload: 1 }],
-        [],
+        [
+          {
+            type: 'setErrorMessage',
+            payload: {
+              text: 'An error occured whilst loading the pipelines jobs.',
+              action: jasmine.anything(),
+              actionText: 'Please try again',
+              actionPayload: { id: 1 },
+            },
+          },
+        ],
         done,
       );
     });
-
-    it('creates flash message', () => {
-      const flashSpy = spyOnDependency(actions, 'flash');
-
-      receiveJobsError({ commit() {} }, 1);
-
-      expect(flashSpy).toHaveBeenCalled();
-    });
   });
 
   describe('receiveJobsSuccess', () => {
@@ -268,7 +287,7 @@ describe('IDE pipelines actions', () => {
           [],
           [
             { type: 'requestJobs', payload: stage.id },
-            { type: 'receiveJobsError', payload: stage.id },
+            { type: 'receiveJobsError', payload: stage },
           ],
           done,
         );
@@ -337,18 +356,20 @@ describe('IDE pipelines actions', () => {
         null,
         mockedState,
         [{ type: types.RECEIVE_JOB_TRACE_ERROR }],
-        [],
+        [
+          {
+            type: 'setErrorMessage',
+            payload: {
+              text: 'An error occured whilst fetching the job trace.',
+              action: jasmine.any(Function),
+              actionText: 'Please try again',
+              actionPayload: null,
+            },
+          },
+        ],
         done,
       );
     });
-
-    it('creates flash message', () => {
-      const flashSpy = spyOnDependency(actions, 'flash');
-
-      receiveJobTraceError({ commit() {} });
-
-      expect(flashSpy).toHaveBeenCalled();
-    });
   });
 
   describe('receiveJobTraceSuccess', () => {