diff --git a/app/views/shared/issuable/form/_branch_chooser.html.haml b/app/views/shared/issuable/form/_branch_chooser.html.haml index 8d9e5ddf065b4f084b61d08f258666a0846313fd..8e695a0adb47570a63a8c09052a308ea279c1dc6 100644 --- a/app/views/shared/issuable/form/_branch_chooser.html.haml +++ b/app/views/shared/issuable/form/_branch_chooser.html.haml @@ -22,4 +22,5 @@ = form.hidden_field(:target_branch, { class: 'target_branch js-target-branch-select ref-name mw-xl', data: { placeholder: _('Select branch'), endpoint: refs_project_path(@project, sort: 'updated_desc', find: 'branches') }}) +#js-target-branch-alert %hr diff --git a/ee/app/assets/javascripts/approvals/components/mr_edit/mr_rules.vue b/ee/app/assets/javascripts/approvals/components/mr_edit/mr_rules.vue index 0aecd5e731ba03d35484e7e087794947ff409c40..15a45df4748808a6a09e82d43ee4580b43267c59 100644 --- a/ee/app/assets/javascripts/approvals/components/mr_edit/mr_rules.vue +++ b/ee/app/assets/javascripts/approvals/components/mr_edit/mr_rules.vue @@ -27,6 +27,7 @@ export default { ...mapState({ rules: state => state.approvals.rules, }), + ...mapState('targetBranchAlertModule', ['showTargetBranchAlert', 'confirmTargetBranchAlert']), hasNamedRule() { if (this.settings.allowMultiRule) { return this.rules.some(rule => rule.ruleType !== RULE_TYPE_ANY_APPROVER); @@ -85,13 +86,16 @@ export default { }, methods: { ...mapActions(['setEmptyRule', 'addEmptyRule', 'fetchRules']), + ...mapActions('targetBranchAlertModule', ['toggleDisplayTargetBranchAlert', 'setTargetBranch']), onTargetBranchMutation() { const selectedTargetBranchValue = this.mergeRequestTargetBranchElement.value; if (this.targetBranch !== selectedTargetBranchValue) { this.targetBranch = selectedTargetBranchValue; - this.fetchRules(this.targetBranch); + this.setTargetBranch(selectedTargetBranchValue); + this.toggleDisplayTargetBranchAlert(true); } + return false; }, }, }; diff --git a/ee/app/assets/javascripts/approvals/components/target_branch_alert/app.vue b/ee/app/assets/javascripts/approvals/components/target_branch_alert/app.vue new file mode 100644 index 0000000000000000000000000000000000000000..aec10e55e4ca98c3ab869ddfb93596b99ecbb5f3 --- /dev/null +++ b/ee/app/assets/javascripts/approvals/components/target_branch_alert/app.vue @@ -0,0 +1,40 @@ + + + diff --git a/ee/app/assets/javascripts/approvals/mount_mr_edit.js b/ee/app/assets/javascripts/approvals/mount_mr_edit.js index 62e533ed660ea91779166bac6d64c87c6dd54860..ae6b4cf398b15b325bf220a5976c6812247330c9 100644 --- a/ee/app/assets/javascripts/approvals/mount_mr_edit.js +++ b/ee/app/assets/javascripts/approvals/mount_mr_edit.js @@ -4,10 +4,11 @@ import { parseBoolean } from '~/lib/utils/common_utils'; import createStore from './stores'; import mrEditModule from './stores/modules/mr_edit'; import MrEditApp from './components/mr_edit/app.vue'; +import TargetBranchAlertApp from './components/target_branch_alert/app.vue'; Vue.use(Vuex); -export default function mountApprovalInput(el) { +export default function mountMrEdit(el) { if (!el) { return null; } @@ -19,11 +20,28 @@ export default function mountApprovalInput(el) { allowMultiRule: parseBoolean(el.dataset.allowMultiRule), }); - return new Vue({ - el, - store, - render(h) { - return h(MrEditApp); - }, - }); + const targetBranchAlertElement = document.getElementById('js-target-branch-alert'); + + const mountTargetBranchAlert = () => + new Vue({ + el: targetBranchAlertElement, + store, + render(h) { + return h(TargetBranchAlertApp); + }, + }); + + const mountApprovalInput = () => + new Vue({ + el, + store, + render(h) { + return h(MrEditApp); + }, + }); + + return { + mountApprovalInput, + mountTargetBranchAlert, + }; } diff --git a/ee/app/assets/javascripts/approvals/stores/index.js b/ee/app/assets/javascripts/approvals/stores/index.js index b17514eab74678f0cb7ba9b621973d04299ca357..d0f3ca4c40397aba061de75a2f11b17ddec68acb 100644 --- a/ee/app/assets/javascripts/approvals/stores/index.js +++ b/ee/app/assets/javascripts/approvals/stores/index.js @@ -1,5 +1,6 @@ import Vuex from 'vuex'; import modalModule from '~/vuex_shared/modules/modal'; +import targetBranchAlertModule from './modules/target_branch_alert'; import state from './state'; export const createStoreOptions = (approvalsModule, settings) => ({ @@ -8,6 +9,7 @@ export const createStoreOptions = (approvalsModule, settings) => ({ ...(approvalsModule ? { approvals: approvalsModule } : {}), createModal: modalModule(), deleteModal: modalModule(), + targetBranchAlertModule: targetBranchAlertModule(), }, }); diff --git a/ee/app/assets/javascripts/approvals/stores/modules/target_branch_alert/actions.js b/ee/app/assets/javascripts/approvals/stores/modules/target_branch_alert/actions.js new file mode 100644 index 0000000000000000000000000000000000000000..5c37e9ea5fe25ad685e38dc5d898447d4473a110 --- /dev/null +++ b/ee/app/assets/javascripts/approvals/stores/modules/target_branch_alert/actions.js @@ -0,0 +1,17 @@ +import * as types from './mutations_types'; + +export const displayTargetBranchAlert = ({ commit }, isShown) => + commit(types.TOGGLE_DISPLAY_TARGET_BRANCH_ALERT, isShown); + +export const toggleDisplayTargetBranchAlert = ({ dispatch }, isShown) => { + dispatch('displayTargetBranchAlert', isShown); +}; + +export const selectTargetBranch = ({ commit }, targetBranch) => + commit(types.SET_TARGET_BRANCH, targetBranch); + +export const setTargetBranch = ({ dispatch }, targetBranch) => { + dispatch('selectTargetBranch', targetBranch); +}; + +export default () => {}; diff --git a/ee/app/assets/javascripts/approvals/stores/modules/target_branch_alert/index.js b/ee/app/assets/javascripts/approvals/stores/modules/target_branch_alert/index.js new file mode 100644 index 0000000000000000000000000000000000000000..b88f09d74f77e06fe1e15dcf5da6119c3100d62d --- /dev/null +++ b/ee/app/assets/javascripts/approvals/stores/modules/target_branch_alert/index.js @@ -0,0 +1,10 @@ +import createState from './state'; +import * as actions from './actions'; +import mutations from './mutations'; + +export default () => ({ + namespaced: true, + state: createState(), + actions, + mutations, +}); diff --git a/ee/app/assets/javascripts/approvals/stores/modules/target_branch_alert/mutations.js b/ee/app/assets/javascripts/approvals/stores/modules/target_branch_alert/mutations.js new file mode 100644 index 0000000000000000000000000000000000000000..1862eb9508c947efcb4858c38d86c653904e94a7 --- /dev/null +++ b/ee/app/assets/javascripts/approvals/stores/modules/target_branch_alert/mutations.js @@ -0,0 +1,10 @@ +import * as types from './mutations_types'; + +export default { + [types.TOGGLE_DISPLAY_TARGET_BRANCH_ALERT](state, isShow) { + state.showTargetBranchAlert = isShow; + }, + [types.SET_TARGET_BRANCH](state, targetBranch) { + state.targetBranch = targetBranch; + }, +}; diff --git a/ee/app/assets/javascripts/approvals/stores/modules/target_branch_alert/mutations_types.js b/ee/app/assets/javascripts/approvals/stores/modules/target_branch_alert/mutations_types.js new file mode 100644 index 0000000000000000000000000000000000000000..fdbd2c5f8bcc3a5aa0533fc77cf490be62dcd42c --- /dev/null +++ b/ee/app/assets/javascripts/approvals/stores/modules/target_branch_alert/mutations_types.js @@ -0,0 +1,4 @@ +export const TOGGLE_DISPLAY_TARGET_BRANCH_ALERT = 'TOGGLE_DISPLAY_TARGET_BRANCH_ALERT'; +export const SET_TARGET_BRANCH = 'SET_TARGET_BRANCH'; + +export default () => {}; diff --git a/ee/app/assets/javascripts/approvals/stores/modules/target_branch_alert/state.js b/ee/app/assets/javascripts/approvals/stores/modules/target_branch_alert/state.js new file mode 100644 index 0000000000000000000000000000000000000000..9aba388507962464c16e7c6a567be83c6d248ab6 --- /dev/null +++ b/ee/app/assets/javascripts/approvals/stores/modules/target_branch_alert/state.js @@ -0,0 +1,4 @@ +export default () => ({ + showTargetBranchAlert: false, + targetBranch: '', +}); diff --git a/ee/app/assets/javascripts/pages/projects/merge_requests/shared/init_form.js b/ee/app/assets/javascripts/pages/projects/merge_requests/shared/init_form.js index 755746f74ba9a65c803d866a9a95819b7a5d8d98..7cb36ec7b53247e890eaa742d530316d73190772 100644 --- a/ee/app/assets/javascripts/pages/projects/merge_requests/shared/init_form.js +++ b/ee/app/assets/javascripts/pages/projects/merge_requests/shared/init_form.js @@ -1,7 +1,11 @@ -import mountApprovals from 'ee/approvals/mount_mr_edit'; +import mountMrEdit from 'ee/approvals/mount_mr_edit'; import mountBlockingMergeRequestsInput from 'ee/projects/merge_requests/blocking_mr_input'; export default () => { - mountApprovals(document.getElementById('js-mr-approvals-input')); + const { mountApprovalInput, mountTargetBranchAlert } = mountMrEdit( + document.getElementById('js-mr-approvals-input'), + ); + mountApprovalInput(); + mountTargetBranchAlert(); mountBlockingMergeRequestsInput(document.getElementById('js-blocking-merge-requests-input')); }; diff --git a/ee/changelogs/unreleased/199795-warning-message.yml b/ee/changelogs/unreleased/199795-warning-message.yml new file mode 100644 index 0000000000000000000000000000000000000000..0484b3925f52efc7b517279d1bd54fe614a7dac0 --- /dev/null +++ b/ee/changelogs/unreleased/199795-warning-message.yml @@ -0,0 +1,5 @@ +--- +title: Add warning message when changing target branch in MR edit +merge_request: 27738 +author: +type: added diff --git a/ee/spec/frontend/approvals/components/mr_edit/mr_rules_spec.js b/ee/spec/frontend/approvals/components/mr_edit/mr_rules_spec.js index 1959a14c5853aab0e9d9dfe887d83b5706d53beb..29e98b8744fc4052d4d15389c3a4e7c14746cbcd 100644 --- a/ee/spec/frontend/approvals/components/mr_edit/mr_rules_spec.js +++ b/ee/spec/frontend/approvals/components/mr_edit/mr_rules_spec.js @@ -83,6 +83,12 @@ describe('EE Approvals MRRules', () => { addEmptyRule: jest.fn(), setEmptyRule: jest.fn(), }; + + store.modules.targetBranchAlertModule.actions = { + setTargetBranch: jest.fn(), + toggleDisplayTargetBranchAlert: jest.fn(), + }; + store.state.settings.mrSettingsPath = 'some/path'; store.state.settings.eligibleApproversDocsPath = 'some/path'; store.state.settings.allowMultiRule = true; @@ -106,11 +112,25 @@ describe('EE Approvals MRRules', () => { expect(wrapper.vm.targetBranch).toBe(newValue); }); - it('re-fetches rules when target branch has changed', () => { + it('trigger alert when target branch has changed', () => { + factory(); + const newValue = setTargetBranchInputValue(); + setTargetBranchInputValue(); + callTargetBranchHandler(MutationObserverSpy); + expect(store.modules.targetBranchAlertModule.actions.setTargetBranch).toHaveBeenCalledWith( + expect.anything(), + newValue, + undefined, + ); + }); + + it('passes target branch when target branch has changed', () => { factory(); setTargetBranchInputValue(); callTargetBranchHandler(MutationObserverSpy); - expect(store.modules.approvals.actions.fetchRules).toHaveBeenCalled(); + expect( + store.modules.targetBranchAlertModule.actions.toggleDisplayTargetBranchAlert, + ).toHaveBeenCalledWith(expect.anything(), true, undefined); }); it('disconnects MutationObserver when component gets destroyed', () => { diff --git a/ee/spec/frontend/approvals/components/target_branch_alert/app_spec.js b/ee/spec/frontend/approvals/components/target_branch_alert/app_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..2ea568788b59c6ca0e1dea74b8c97f4f02b2962c --- /dev/null +++ b/ee/spec/frontend/approvals/components/target_branch_alert/app_spec.js @@ -0,0 +1,78 @@ +import { shallowMount, createLocalVue } from '@vue/test-utils'; +import Vuex from 'vuex'; +import { GlAlert } from '@gitlab/ui'; +import App from 'ee/approvals/components/target_branch_alert/app.vue'; +import { createStoreOptions } from 'ee/approvals/stores'; +import mrEditModule from 'ee/approvals/stores/modules/mr_edit'; + +const localVue = createLocalVue(); + +localVue.use(Vuex); + +describe('Target Branch App', () => { + let store; + let wrapper; + + const findAlert = () => wrapper.find(GlAlert); + + const createComponent = () => { + wrapper = shallowMount(localVue.extend(App), { + localVue, + store: new Vuex.Store(store), + }); + }; + + beforeEach(() => { + store = createStoreOptions(mrEditModule()); + store.modules.targetBranchAlertModule.state.showTargetBranchAlert = true; + store.modules.targetBranchAlertModule.actions.toggleDisplayTargetBranchAlert = jest.fn(); + store.modules.approvals.actions.fetchRules = jest.fn(); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + describe('GlAlert component', () => { + it('renders component', () => { + createComponent(); + expect(findAlert().exists()).toBe(true); + }); + + it('close alert popup when secondary action is clicked', () => { + createComponent(); + findAlert().vm.$emit('secondaryAction'); + + expect( + store.modules.targetBranchAlertModule.actions.toggleDisplayTargetBranchAlert, + ).toHaveBeenCalledWith(expect.anything(), false, undefined); + }); + + it('fetch rules and close alert popup when primary action is clicked', () => { + const targetBranch = 'some-branch'; + store.modules.targetBranchAlertModule.state.targetBranch = targetBranch; + createComponent(); + findAlert().vm.$emit('primaryAction'); + + expect(store.modules.approvals.actions.fetchRules).toHaveBeenCalledWith( + expect.anything(), + targetBranch, + undefined, + ); + expect( + store.modules.targetBranchAlertModule.actions.toggleDisplayTargetBranchAlert, + ).toHaveBeenCalledWith(expect.anything(), false, undefined); + }); + + it.each` + prop | value | desc + ${'primaryButtonText'} | ${'Charge target branch'} | ${'has a primary text'} + ${'secondaryButtonText'} | ${'Cancel'} | ${'has a secondary text'} + ${'variant'} | ${'warning'} | ${'has the warning variant'} + `('$desc', ({ prop, value }) => { + createComponent(); + + expect(findAlert().props(prop)).toContain(value); + }); + }); +}); diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d1fd25c6a863502ffbe325d867fdc1690892eded..67d0790403edf963a8521a6543dc4d8d96b48235 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3438,6 +3438,9 @@ msgstr "" msgid "Changing group path can have unintended side effects." msgstr "" +msgid "Changing target branch will reset the approval rules. Any changes you have made will be lost." +msgstr "" + msgid "Charts can't be displayed as the request for data has timed out. %{documentationLink}" msgstr ""