Merge branch 'slashmanov/display-error-message-when-failed-to-load-diff-metadata' into 'master'

Display an error within Apply suggestion box when failed to load diff metadata

See merge request gitlab-org/gitlab!83520

app/assets/javascripts/mr_notes/stores/actions.js

@@ -10,23 +10,14 @@ export function setEndpoints({ commit }, endpoints) {
   commit(types.SET_ENDPOINTS, endpoints);
 }
 
-export function setMrMetadata({ commit }, metadata) {
-  commit(types.SET_MR_METADATA, metadata);
-}
-
-export function fetchMrMetadata({ dispatch, state }) {
+export async function fetchMrMetadata({ state, commit }) {
   if (state.endpoints?.metadata) {
-    axios
-      .get(state.endpoints.metadata)
-      .then((response) => {
-        dispatch('setMrMetadata', response.data);
-      })
-      .catch(() => {
-        // https://gitlab.com/gitlab-org/gitlab/-/issues/324740
-        // We can't even do a simple console warning here because
-        // the pipeline will fail. However, the issue above will
-        // eventually handle errors appropriately.
-        // console.warn('Failed to load MR Metadata for the Overview tab.');
-      });
+    commit(types.SET_FAILED_TO_LOAD_METADATA, false);
+    try {
+      const { data } = await axios.get(state.endpoints.metadata);
+      commit(types.SET_MR_METADATA, data);
+    } catch (error) {
+      commit(types.SET_FAILED_TO_LOAD_METADATA, true);
+    }
   }
 }

app/assets/javascripts/mr_notes/stores/modules/index.js

@@ -7,6 +7,7 @@ export default () => ({
     endpoints: {},
     activeTab: null,
     mrMetadata: {},
+    failedToLoadMetadata: false,
   },
   actions,
   getters,

app/assets/javascripts/mr_notes/stores/mutation_types.js

@@ -2,4 +2,5 @@ export default {
   SET_ACTIVE_TAB: 'SET_ACTIVE_TAB',
   SET_ENDPOINTS: 'SET_ENDPOINTS',
   SET_MR_METADATA: 'SET_MR_METADATA',
+  SET_FAILED_TO_LOAD_METADATA: 'SET_FAILED_TO_LOAD_METADATA',
 };

app/assets/javascripts/mr_notes/stores/mutations.js

@@ -10,4 +10,7 @@ export default {
   [types.SET_MR_METADATA](state, metadata) {
     Object.assign(state, { mrMetadata: metadata });
   },
+  [types.SET_FAILED_TO_LOAD_METADATA](state, value) {
+    Object.assign(state, { failedToLoadMetadata: value });
+  },
 };

app/assets/javascripts/notes/components/note_body.vue

@@ -57,14 +57,15 @@ export default {
   computed: {
     ...mapGetters(['getDiscussion', 'suggestionsCount', 'getSuggestionsFilePaths']),
     ...mapGetters('diffs', ['suggestionCommitMessage']),
+    ...mapState({
+      batchSuggestionsInfo: (state) => state.notes.batchSuggestionsInfo,
+      failedToLoadMetadata: (state) => state.page.failedToLoadMetadata,
+    }),
     discussion() {
       if (!this.note.isDraft) return {};
 
       return this.getDiscussion(this.note.discussion_id);
     },
-    ...mapState({
-      batchSuggestionsInfo: (state) => state.notes.batchSuggestionsInfo,
-    }),
     noteBody() {
       return this.note.note;
     },
@@ -165,6 +166,7 @@ export default {
       :line-type="lineType"
       :help-page-path="helpPagePath"
       :default-commit-message="commitMessage"
+      :failed-to-load-metadata="failedToLoadMetadata"
       @apply="applySuggestion"
       @applyBatch="applySuggestionBatch"
       @addToBatch="addSuggestionToBatch"

app/assets/javascripts/vue_shared/components/markdown/apply_suggestion.vue

 <script>
-import { GlDropdown, GlDropdownForm, GlFormTextarea, GlButton } from '@gitlab/ui';
+import { GlDropdown, GlDropdownForm, GlFormTextarea, GlButton, GlAlert } from '@gitlab/ui';
 import { __, n__ } from '~/locale';
 
 export default {
-  components: { GlDropdown, GlDropdownForm, GlFormTextarea, GlButton },
+  components: { GlDropdown, GlDropdownForm, GlFormTextarea, GlButton, GlAlert },
   props: {
     disabled: {
       type: Boolean,
@@ -19,6 +19,11 @@ export default {
       required: false,
       default: 0,
     },
+    errorMessage: {
+      type: String,
+      required: false,
+      default: null,
+    },
   },
   data() {
     return {
@@ -55,6 +60,9 @@ export default {
   >
     <gl-dropdown-form class="gl-px-4! gl-m-0!">
       <label for="commit-message">{{ __('Commit message') }}</label>
+      <gl-alert v-if="errorMessage" variant="danger" :dismissible="false" class="gl-mb-4">
+        {{ errorMessage }}
+      </gl-alert>
       <gl-form-textarea
         id="commit-message"
         ref="commitMessage"

app/assets/javascripts/vue_shared/components/markdown/suggestion_diff.vue

@@ -36,6 +36,11 @@ export default {
       required: false,
       default: 0,
     },
+    failedToLoadMetadata: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
   },
   computed: {
     batchSuggestionsCount() {
@@ -80,6 +85,7 @@ export default {
       :help-page-path="helpPagePath"
       :default-commit-message="defaultCommitMessage"
       :inapplicable-reason="suggestion.inapplicable_reason"
+      :failed-to-load-metadata="failedToLoadMetadata"
       @apply="applySuggestion"
       @applyBatch="applySuggestionBatch"
       @addToBatch="addSuggestionToBatch"

app/assets/javascripts/vue_shared/components/markdown/suggestion_diff_header.vue

@@ -4,6 +4,10 @@ import { isLoggedIn } from '~/lib/utils/common_utils';
 import { __ } from '~/locale';
 import ApplySuggestion from './apply_suggestion.vue';
 
+const APPLY_SUGGESTION_ERROR_MESSAGE = __(
+  'Unable to fully load the default commit message. You can still apply this suggestion and the commit message will be correct.',
+);
+
 export default {
   components: { GlBadge, GlIcon, GlButton, GlLoadingIcon, ApplySuggestion },
   directives: { 'gl-tooltip': GlTooltipDirective },
@@ -52,6 +56,11 @@ export default {
       required: false,
       default: 0,
     },
+    failedToLoadMetadata: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
   },
   data() {
     return {
@@ -94,6 +103,9 @@ export default {
 
       return true;
     },
+    applySuggestionErrorMessage() {
+      return this.failedToLoadMetadata ? APPLY_SUGGESTION_ERROR_MESSAGE : null;
+    },
   },
   methods: {
     apply(message) {
@@ -171,6 +183,7 @@ export default {
         :disabled="isDisableButton"
         :default-commit-message="defaultCommitMessage"
         :batch-suggestions-count="batchSuggestionsCount"
+        :error-message="applySuggestionErrorMessage"
         class="gl-ml-3"
         @apply="apply"
       />

app/assets/javascripts/vue_shared/components/markdown/suggestions.vue

@@ -47,6 +47,11 @@ export default {
       required: false,
       default: 0,
     },
+    failedToLoadMetadata: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
   },
   data() {
     return {
@@ -60,6 +65,9 @@ export default {
     noteHtml() {
       this.reset();
     },
+    failedToLoadMetadata() {
+      this.reset();
+    },
   },
   mounted() {
     this.renderSuggestions();
@@ -105,6 +113,7 @@ export default {
         helpPagePath,
         defaultCommitMessage,
         suggestionsCount,
+        failedToLoadMetadata,
       } = this;
       const suggestion =
         suggestions && suggestions[suggestionIndex] ? suggestions[suggestionIndex] : {};
@@ -117,6 +126,7 @@ export default {
           helpPagePath,
           defaultCommitMessage,
           suggestionsCount,
+          failedToLoadMetadata,
         },
       });
 

locale/gitlab.pot

@@ -40102,6 +40102,9 @@ msgstr ""
 msgid "Unable to find Jira project to import data from."
 msgstr ""
 
+msgid "Unable to fully load the default commit message. You can still apply this suggestion and the commit message will be correct."
+msgstr ""
+
 msgid "Unable to generate new instance ID"
 msgstr ""
 

spec/features/merge_request/user_suggests_changes_on_diff_spec.rb

@@ -379,4 +379,41 @@ RSpec.describe 'User comments on a diff', :js do
       end
     end
   end
+
+  context 'failed to load metadata' do
+    let(:dummy_controller) do
+      Class.new(Projects::MergeRequests::DiffsController) do
+        def diffs_metadata
+          render json: '', status: :internal_server_error
+        end
+      end
+    end
+
+    before do
+      stub_const('Projects::MergeRequests::DiffsController', dummy_controller)
+
+      click_diff_line(find_by_scrolling("[id='#{sample_compare.changes[1][:line_code]}']"))
+
+      page.within('.js-discussion-note-form') do
+        fill_in('note_note', with: "```suggestion\n# change to a comment\n```")
+        click_button('Add comment now')
+      end
+
+      wait_for_requests
+
+      visit(project_merge_request_path(project, merge_request))
+
+      wait_for_requests
+    end
+
+    it 'displays an error' do
+      page.within('.discussion-notes') do
+        click_button('Apply suggestion')
+
+        wait_for_requests
+
+        expect(page).to have_content('Unable to fully load the default commit message. You can still apply this suggestion and the commit message will be correct.')
+      end
+    end
+  end
 end

spec/frontend/mr_notes/stores/actions_spec.js

 import MockAdapter from 'axios-mock-adapter';
-
-import testAction from 'helpers/vuex_action_helper';
 import axios from '~/lib/utils/axios_utils';
 
-import { setEndpoints, setMrMetadata, fetchMrMetadata } from '~/mr_notes/stores/actions';
-import mutationTypes from '~/mr_notes/stores/mutation_types';
+import { createStore } from '~/mr_notes/stores';
 
 describe('MR Notes Mutator Actions', () => {
+  let store;
+
+  beforeEach(() => {
+    store = createStore();
+  });
+
   describe('setEndpoints', () => {
-    it('should trigger the SET_ENDPOINTS state mutation', (done) => {
+    it('sets endpoints', async () => {
       const endpoints = { endpointA: 'a' };
 
-      testAction(
-        setEndpoints,
-        endpoints,
-        {},
-        [
-          {
-            type: mutationTypes.SET_ENDPOINTS,
-            payload: endpoints,
-          },
-        ],
-        [],
-        done,
-      );
-    });
-  });
+      await store.dispatch('setEndpoints', endpoints);
 
-  describe('setMrMetadata', () => {
-    it('should trigger the SET_MR_METADATA state mutation', async () => {
-      const mrMetadata = { propA: 'a', propB: 'b' };
-
-      await testAction(
-        setMrMetadata,
-        mrMetadata,
-        {},
-        [
-          {
-            type: mutationTypes.SET_MR_METADATA,
-            payload: mrMetadata,
-          },
-        ],
-        [],
-      );
+      expect(store.state.page.endpoints).toEqual(endpoints);
     });
   });
 
   describe('fetchMrMetadata', () => {
     const mrMetadata = { meta: true, data: 'foo' };
-    const state = {
-      endpoints: {
-        metadata: 'metadata',
-      },
-    };
+    const metadata = 'metadata';
+    const endpoints = { metadata };
     let mock;
 
-    beforeEach(() => {
+    beforeEach(async () => {
+      await store.dispatch('setEndpoints', endpoints);
+
       mock = new MockAdapter(axios);
 
-      mock.onGet(state.endpoints.metadata).reply(200, mrMetadata);
+      mock.onGet(metadata).reply(200, mrMetadata);
     });
 
     afterEach(() => {
@@ -66,27 +39,26 @@ describe('MR Notes Mutator Actions', () => {
     });
 
     it('should fetch the data from the API', async () => {
-      await fetchMrMetadata({ state, dispatch: () => {} });
+      await store.dispatch('fetchMrMetadata');
 
       await axios.waitForAll();
 
       expect(mock.history.get).toHaveLength(1);
-      expect(mock.history.get[0].url).toBe(state.endpoints.metadata);
+      expect(mock.history.get[0].url).toBe(metadata);
+    });
+
+    it('should set the fetched data into state', async () => {
+      await store.dispatch('fetchMrMetadata');
+
+      expect(store.state.page.mrMetadata).toEqual(mrMetadata);
     });
 
-    it('should set the fetched data into state', () => {
-      return testAction(
-        fetchMrMetadata,
-        {},
-        state,
-        [],
-        [
-          {
-            type: 'setMrMetadata',
-            payload: mrMetadata,
-          },
-        ],
-      );
+    it('should set failedToLoadMetadata flag when request fails', async () => {
+      mock.onGet(metadata).reply(500);
+
+      await store.dispatch('fetchMrMetadata');
+
+      expect(store.state.page.failedToLoadMetadata).toBe(true);
     });
   });
 });

spec/frontend/vue_shared/components/markdown/apply_suggestion_spec.js

-import { GlDropdown, GlFormTextarea, GlButton } from '@gitlab/ui';
+import { GlDropdown, GlFormTextarea, GlButton, GlAlert } from '@gitlab/ui';
 import { shallowMount } from '@vue/test-utils';
 import ApplySuggestionComponent from '~/vue_shared/components/markdown/apply_suggestion.vue';
 
@@ -10,9 +10,10 @@ describe('Apply Suggestion component', () => {
     wrapper = shallowMount(ApplySuggestionComponent, { propsData: { ...propsData, ...props } });
   };
 
-  const findDropdown = () => wrapper.find(GlDropdown);
-  const findTextArea = () => wrapper.find(GlFormTextarea);
-  const findApplyButton = () => wrapper.find(GlButton);
+  const findDropdown = () => wrapper.findComponent(GlDropdown);
+  const findTextArea = () => wrapper.findComponent(GlFormTextarea);
+  const findApplyButton = () => wrapper.findComponent(GlButton);
+  const findAlert = () => wrapper.findComponent(GlAlert);
 
   beforeEach(() => createWrapper());
 
@@ -53,6 +54,20 @@ describe('Apply Suggestion component', () => {
     });
   });
 
+  describe('error', () => {
+    it('displays an error message', () => {
+      const errorMessage = 'Error message';
+      createWrapper({ errorMessage });
+
+      const alert = findAlert();
+
+      expect(alert.exists()).toBe(true);
+      expect(alert.props('variant')).toBe('danger');
+      expect(alert.props('dismissible')).toBe(false);
+      expect(alert.text()).toBe(errorMessage);
+    });
+  });
+
   describe('apply suggestion', () => {
     it('emits an apply event with no message if no message was added', () => {
       findTextArea().vm.$emit('input', null);