Show error message when batch diffs errors

Previously we would keep showing a loading spinner even
if the request throws a 500.
This changes that to instead show a alert saying there was
an error and allows the user to retry loading.

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/337216

app/assets/javascripts/diffs/components/app.vue

 <script>
-import { GlLoadingIcon, GlPagination, GlSprintf } from '@gitlab/ui';
+import { GlLoadingIcon, GlPagination, GlSprintf, GlAlert } from '@gitlab/ui';
 import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils';
 import Mousetrap from 'mousetrap';
 import { mapState, mapGetters, mapActions } from 'vuex';
@@ -78,6 +78,7 @@ export default {
     GlPagination,
     GlSprintf,
     MrWidgetHowToMergeModal,
+    GlAlert,
   },
   alerts: {
     ALERT_OVERFLOW_HIDDEN,
@@ -200,7 +201,6 @@ export default {
     ...mapState('diffs', [
       'showTreeList',
       'isLoading',
-      'isBatchLoading',
       'diffFiles',
       'diffViewType',
       'commit',
@@ -227,6 +227,8 @@ export default {
       'isParallelView',
       'currentDiffIndex',
       'isVirtualScrollingEnabled',
+      'isBatchLoading',
+      'isBatchLoadingError',
     ]),
     ...mapGetters('batchComments', ['draftsCount']),
     ...mapGetters(['isNotesFetched', 'getNoteableData']),
@@ -621,6 +623,9 @@ export default {
         this.subscribedToVirtualScrollingEvents = true;
       }
     },
+    reloadPage() {
+      window.location.reload();
+    },
   },
   minTreeWidth: MIN_TREE_WIDTH,
   maxTreeWidth: MAX_TREE_WIDTH,
@@ -639,17 +644,19 @@ export default {
         :diff-files-count-text="numTotalFiles"
       />
 
-      <hidden-files-warning
-        v-if="visibleWarning == $options.alerts.ALERT_OVERFLOW_HIDDEN"
-        :visible="numVisibleFiles"
-        :total="numTotalFiles"
-        :plain-diff-path="plainDiffPath"
-        :email-patch-path="emailPatchPath"
-      />
-      <collapsed-files-warning
-        v-if="visibleWarning == $options.alerts.ALERT_COLLAPSED_FILES"
-        :limited="isLimitedContainer"
-      />
+      <template v-if="!isBatchLoadingError">
+        <hidden-files-warning
+          v-if="visibleWarning == $options.alerts.ALERT_OVERFLOW_HIDDEN"
+          :visible="numVisibleFiles"
+          :total="numTotalFiles"
+          :plain-diff-path="plainDiffPath"
+          :email-patch-path="emailPatchPath"
+        />
+        <collapsed-files-warning
+          v-if="visibleWarning == $options.alerts.ALERT_COLLAPSED_FILES"
+          :limited="isLimitedContainer"
+        />
+      </template>
 
       <div
         :data-can-create-note="getNoteableData.current_user.can_create_note"
@@ -678,7 +685,18 @@ export default {
           }"
         >
           <commit-widget v-if="commit" :commit="commit" :collapsible="false" />
-          <div v-if="isBatchLoading" class="loading"><gl-loading-icon size="lg" /></div>
+          <gl-alert
+            v-if="isBatchLoadingError"
+            variant="danger"
+            :dismissible="false"
+            :primary-button-text="__('Reload page')"
+            @primaryAction="reloadPage"
+          >
+            {{ __("Error: Couldn't load some or all of the changes.") }}
+          </gl-alert>
+          <div v-if="isBatchLoading && !isBatchLoadingError" class="loading">
+            <gl-loading-icon size="lg" />
+          </div>
           <template v-else-if="renderDiffFiles">
             <dynamic-scroller
               v-if="isVirtualScrollingEnabled"
@@ -754,7 +772,10 @@ export default {
             </div>
             <gl-loading-icon v-else-if="retrievingBatches" size="lg" />
           </template>
-          <no-changes v-else :changes-empty-state-illustration="changesEmptyStateIllustration" />
+          <no-changes
+            v-else-if="!isBatchLoadingError"
+            :changes-empty-state-illustration="changesEmptyStateIllustration"
+          />
         </div>
       </div>
       <mr-widget-how-to-merge-modal

app/assets/javascripts/diffs/store/actions.js

@@ -104,7 +104,7 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => {
   let totalLoaded = 0;
   let scrolledVirtualScroller = false;
 
-  commit(types.SET_BATCH_LOADING, true);
+  commit(types.SET_BATCH_LOADING_STATE, 'loading');
   commit(types.SET_RETRIEVING_BATCHES, true);
   eventHub.$emit(EVT_PERF_MARK_DIFF_FILES_START);
 
@@ -115,7 +115,7 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => {
         totalLoaded += diff_files.length;
 
         commit(types.SET_DIFF_DATA_BATCH, { diff_files });
-        commit(types.SET_BATCH_LOADING, false);
+        commit(types.SET_BATCH_LOADING_STATE, 'loaded');
 
         if (window.gon?.features?.diffsVirtualScrolling && !scrolledVirtualScroller) {
           const index = state.diffFiles.findIndex(
@@ -130,7 +130,7 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => {
         }
 
         if (!isNoteLink && !state.currentDiffFileId) {
-          commit(types.VIEW_DIFF_FILE, diff_files[0].file_hash);
+          commit(types.VIEW_DIFF_FILE, diff_files[0]?.file_hash);
         }
 
         if (isNoteLink) {
@@ -182,11 +182,14 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => {
 
         return null;
       })
-      .catch(() => commit(types.SET_RETRIEVING_BATCHES, false));
+      .catch(() => {
+        commit(types.SET_RETRIEVING_BATCHES, false);
+        commit(types.SET_BATCH_LOADING_STATE, 'error');
+      });
 
-  return getBatch()
-    .then(() => !window.gon?.features?.diffsVirtualScrolling && handleLocationHash())
-    .catch(() => null);
+  return getBatch().then(
+    () => !window.gon?.features?.diffsVirtualScrolling && handleLocationHash(),
+  );
 };
 
 export const fetchDiffFilesMeta = ({ commit, state }) => {

app/assets/javascripts/diffs/store/getters.js

@@ -191,3 +191,6 @@ export const isVirtualScrollingEnabled = (state) => {
       getParameterValues('virtual_scrolling')[0] === 'true')
   );
 };
+
+export const isBatchLoading = (state) => state.batchLoadingState === 'loading';
+export const isBatchLoadingError = (state) => state.batchLoadingState === 'error';

app/assets/javascripts/diffs/store/modules/diff_state.js

@@ -12,7 +12,7 @@ const defaultViewType = INLINE_DIFF_VIEW_TYPE;
 export default () => ({
   isLoading: true,
   isTreeLoaded: false,
-  isBatchLoading: false,
+  batchLoadingState: null,
   retrievingBatches: false,
   addedLines: null,
   removedLines: null,

app/assets/javascripts/diffs/store/mutation_types.js

 export const SET_BASE_CONFIG = 'SET_BASE_CONFIG';
 export const SET_LOADING = 'SET_LOADING';
-export const SET_BATCH_LOADING = 'SET_BATCH_LOADING';
+export const SET_BATCH_LOADING_STATE = 'SET_BATCH_LOADING_STATE';
 export const SET_RETRIEVING_BATCHES = 'SET_RETRIEVING_BATCHES';
 
 export const SET_DIFF_METADATA = 'SET_DIFF_METADATA';

app/assets/javascripts/diffs/store/mutations.js

@@ -60,8 +60,8 @@ export default {
     Object.assign(state, { isLoading });
   },
 
-  [types.SET_BATCH_LOADING](state, isBatchLoading) {
-    Object.assign(state, { isBatchLoading });
+  [types.SET_BATCH_LOADING_STATE](state, batchLoadingState) {
+    Object.assign(state, { batchLoadingState });
   },
 
   [types.SET_RETRIEVING_BATCHES](state, retrievingBatches) {

locale/gitlab.pot

@@ -13268,6 +13268,9 @@ msgstr ""
 msgid "Error: %{error_message}"
 msgstr ""
 
+msgid "Error: Couldn't load some or all of the changes."
+msgstr ""
+
 msgid "Error: No AWS credentials were supplied"
 msgstr ""
 
@@ -27793,6 +27796,9 @@ msgstr ""
 msgid "Release|Something went wrong while saving the release details."
 msgstr ""
 
+msgid "Reload page"
+msgstr ""
+
 msgid "Remediations"
 msgstr ""
 

spec/frontend/diffs/components/app_spec.js

@@ -183,7 +183,7 @@ describe('diffs/components/app', () => {
 
   it('displays loading icon on batch loading', () => {
     createComponent({}, ({ state }) => {
-      state.diffs.isBatchLoading = true;
+      state.diffs.batchLoadingState = 'loading';
     });
 
     expect(wrapper.find(GlLoadingIcon).exists()).toBe(true);

spec/frontend/diffs/store/actions_spec.js

@@ -186,15 +186,16 @@ describe('DiffsStoreActions', () => {
         {},
         { endpointBatch, diffViewType: 'inline' },
         [
-          { type: types.SET_BATCH_LOADING, payload: true },
+          { type: types.SET_BATCH_LOADING_STATE, payload: 'loading' },
           { type: types.SET_RETRIEVING_BATCHES, payload: true },
           { type: types.SET_DIFF_DATA_BATCH, payload: { diff_files: res1.diff_files } },
-          { type: types.SET_BATCH_LOADING, payload: false },
+          { type: types.SET_BATCH_LOADING_STATE, payload: 'loaded' },
           { type: types.VIEW_DIFF_FILE, payload: 'test' },
           { type: types.SET_DIFF_DATA_BATCH, payload: { diff_files: res2.diff_files } },
-          { type: types.SET_BATCH_LOADING, payload: false },
+          { type: types.SET_BATCH_LOADING_STATE, payload: 'loaded' },
           { type: types.VIEW_DIFF_FILE, payload: 'test2' },
           { type: types.SET_RETRIEVING_BATCHES, payload: false },
+          { type: types.SET_BATCH_LOADING_STATE, payload: 'error' },
         ],
         [{ type: 'startRenderDiffsQueue' }, { type: 'startRenderDiffsQueue' }],
         done,

spec/frontend/diffs/store/mutations_spec.js

@@ -31,13 +31,13 @@ describe('DiffsStoreMutations', () => {
     });
   });
 
-  describe('SET_BATCH_LOADING', () => {
+  describe('SET_BATCH_LOADING_STATE', () => {
     it('should set loading state', () => {
       const state = {};
 
-      mutations[types.SET_BATCH_LOADING](state, false);
+      mutations[types.SET_BATCH_LOADING_STATE](state, false);
 
-      expect(state.isBatchLoading).toEqual(false);
+      expect(state.batchLoadingState).toEqual(false);
     });
   });