diff --git a/app/assets/javascripts/mr_notes/stores/actions.js b/app/assets/javascripts/mr_notes/stores/actions.js
index bc66d1dd68fb8d9afd1cc2f08fb93eff135ba3a0..0200a8aefc87ed9b47f7b9f904b0c7106ab47029 100644
--- a/app/assets/javascripts/mr_notes/stores/actions.js
+++ b/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);
+ }
}
}
diff --git a/app/assets/javascripts/mr_notes/stores/modules/index.js b/app/assets/javascripts/mr_notes/stores/modules/index.js
index 52e12ba664c73bed5f7cea485d341436b2e3ce74..75b2b2f4dc69a3cf29bf7a769d16f2bd4e9d3fbc 100644
--- a/app/assets/javascripts/mr_notes/stores/modules/index.js
+++ b/app/assets/javascripts/mr_notes/stores/modules/index.js
@@ -7,6 +7,7 @@ export default () => ({
endpoints: {},
activeTab: null,
mrMetadata: {},
+ failedToLoadMetadata: false,
},
actions,
getters,
diff --git a/app/assets/javascripts/mr_notes/stores/mutation_types.js b/app/assets/javascripts/mr_notes/stores/mutation_types.js
index 88cf6e48988b5efdd8390c585df8376643cc29ca..91d75e77a60358181e930d3c984bf7a5eed09a10 100644
--- a/app/assets/javascripts/mr_notes/stores/mutation_types.js
+++ b/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',
};
diff --git a/app/assets/javascripts/mr_notes/stores/mutations.js b/app/assets/javascripts/mr_notes/stores/mutations.js
index 6af6adb4e188c053967d1b011cd2f2a60c4c191c..8b17f63cfb15074a6bdb510ee7c4938e738b7f83 100644
--- a/app/assets/javascripts/mr_notes/stores/mutations.js
+++ b/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 });
+ },
};
diff --git a/app/assets/javascripts/notes/components/note_body.vue b/app/assets/javascripts/notes/components/note_body.vue
index 60da8644af94403df77fd0049482f4772e1657f7..fe17a061c0a5fca885d14654af7dfe5f45c7ffc7 100644
--- a/app/assets/javascripts/notes/components/note_body.vue
+++ b/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"
diff --git a/app/assets/javascripts/vue_shared/components/markdown/apply_suggestion.vue b/app/assets/javascripts/vue_shared/components/markdown/apply_suggestion.vue
index 709d3592828761cdd6de61efa2cd11cf326b6a0a..926034efd10a22b478a5e15d8148bb61faf88ea9 100644
--- a/app/assets/javascripts/vue_shared/components/markdown/apply_suggestion.vue
+++ b/app/assets/javascripts/vue_shared/components/markdown/apply_suggestion.vue
@@ -1,9 +1,9 @@
<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"
diff --git a/app/assets/javascripts/vue_shared/components/markdown/suggestion_diff.vue b/app/assets/javascripts/vue_shared/components/markdown/suggestion_diff.vue
index 7d8d8c0b90ea381d8f474596e4698cdd08935642..4d10c3f0a518b31e194c867fdca8c11a2138d04b 100644
--- a/app/assets/javascripts/vue_shared/components/markdown/suggestion_diff.vue
+++ b/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"
diff --git a/app/assets/javascripts/vue_shared/components/markdown/suggestion_diff_header.vue b/app/assets/javascripts/vue_shared/components/markdown/suggestion_diff_header.vue
index 648e9c9462f19a542a9d6250486bd3dc31bb210f..8a1b8363f19a23a45e6d74a09eafe9dcffd04193 100644
--- a/app/assets/javascripts/vue_shared/components/markdown/suggestion_diff_header.vue
+++ b/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"
/>
diff --git a/app/assets/javascripts/vue_shared/components/markdown/suggestions.vue b/app/assets/javascripts/vue_shared/components/markdown/suggestions.vue
index 2f6776f835eb7a1d399f9c80afa3b253e181afbd..de3eda6b04f7d42b4dd16832fd3b334f84036db1 100644
--- a/app/assets/javascripts/vue_shared/components/markdown/suggestions.vue
+++ b/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,
},
});
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 6c90e54d2c3346893b99343131e0a671cb0bc75b..a2684eaf1c676f0beb25e70e3bad0dbae01b31ee 100644
--- a/locale/gitlab.pot
+++ b/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 ""
diff --git a/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb b/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb
index beb658bb7a0b4495ee585ba2641fd4b74f4ebc00..f77a42ee5063a6339b38b26ed1b6efd62506c428 100644
--- a/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb
+++ b/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb
@@ -379,4 +379,41 @@ def hash(path)
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
diff --git a/spec/frontend/mr_notes/stores/actions_spec.js b/spec/frontend/mr_notes/stores/actions_spec.js
index c6578453d853e8ce5926babb62058983e7d9ec2f..568c1b930c99dc06a55ab4cde699a0101ffc4327 100644
--- a/spec/frontend/mr_notes/stores/actions_spec.js
+++ b/spec/frontend/mr_notes/stores/actions_spec.js
@@ -1,64 +1,37 @@
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);
});
});
});
diff --git a/spec/frontend/vue_shared/components/markdown/apply_suggestion_spec.js b/spec/frontend/vue_shared/components/markdown/apply_suggestion_spec.js
index c56628fcbcde0d5ac880d0e7bf987012cba9b56b..ecb2b37c3a563180cab18545e1418a8f4eccbfc3 100644
--- a/spec/frontend/vue_shared/components/markdown/apply_suggestion_spec.js
+++ b/spec/frontend/vue_shared/components/markdown/apply_suggestion_spec.js
@@ -1,4 +1,4 @@
-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);