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 53a97a6b2a5aaeb46e8287bb88e2305703d3ab3c..0aecd5e731ba03d35484e7e087794947ff409c40 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 @@ -7,6 +7,8 @@ import RuleControls from '../rule_controls.vue'; import EmptyRule from './empty_rule.vue'; import RuleInput from './rule_input.vue'; +let targetBranchMutationObserver; + export default { components: { UserAvatarList, @@ -15,6 +17,11 @@ export default { EmptyRule, RuleInput, }, + data() { + return { + targetBranch: '', + }; + }, computed: { ...mapState(['settings']), ...mapState({ @@ -33,6 +40,9 @@ export default { canEdit() { return this.settings.canEdit; }, + isEditPath() { + return this.settings.mrSettingsPath; + }, }, watch: { rules: { @@ -50,8 +60,39 @@ export default { immediate: true, }, }, + mounted() { + if (this.isEditPath) { + this.mergeRequestTargetBranchElement = document.querySelector('#merge_request_target_branch'); + + this.targetBranch = this.mergeRequestTargetBranchElement?.value; + + if (this.targetBranch) { + targetBranchMutationObserver = new MutationObserver(this.onTargetBranchMutation); + targetBranchMutationObserver.observe(this.mergeRequestTargetBranchElement, { + attributes: true, + childList: false, + subtree: false, + attributeFilter: ['value'], + }); + } + } + }, + beforeDestroy() { + if (this.isEditPath && targetBranchMutationObserver) { + targetBranchMutationObserver.disconnect(); + targetBranchMutationObserver = null; + } + }, methods: { - ...mapActions(['setEmptyRule', 'addEmptyRule']), + ...mapActions(['setEmptyRule', 'addEmptyRule', 'fetchRules']), + onTargetBranchMutation() { + const selectedTargetBranchValue = this.mergeRequestTargetBranchElement.value; + + if (this.targetBranch !== selectedTargetBranchValue) { + this.targetBranch = selectedTargetBranchValue; + this.fetchRules(this.targetBranch); + } + }, }, }; diff --git a/ee/app/assets/javascripts/approvals/stores/modules/mr_edit/actions.js b/ee/app/assets/javascripts/approvals/stores/modules/mr_edit/actions.js index 21b940a325842921b22570cbd908f79224436bf1..8eca144fa322d561af495a9bf8879026aa3c3111 100644 --- a/ee/app/assets/javascripts/approvals/stores/modules/mr_edit/actions.js +++ b/ee/app/assets/javascripts/approvals/stores/modules/mr_edit/actions.js @@ -61,14 +61,22 @@ export const receiveRulesError = () => { createFlash(__('An error occurred fetching the approval rules.')); }; -export const fetchRules = ({ rootState, dispatch }) => { +export const fetchRules = ({ rootState, dispatch }, targetBranch = '') => { dispatch('requestRules'); const { mrSettingsPath, projectSettingsPath } = rootState.settings; const path = mrSettingsPath || projectSettingsPath; + const params = targetBranch + ? { + params: { + target_branch: targetBranch, + }, + } + : null; + return axios - .get(path) + .get(path, params) .then(response => mapMRApprovalSettingsResponse(response.data)) .then(settings => ({ ...settings, diff --git a/ee/changelogs/unreleased/199790-incorrect-approval-rule-when-update-target-branch.yml b/ee/changelogs/unreleased/199790-incorrect-approval-rule-when-update-target-branch.yml new file mode 100644 index 0000000000000000000000000000000000000000..7527b5d9bf1a19a99ffedeb1054da0a4e512e5c3 --- /dev/null +++ b/ee/changelogs/unreleased/199790-incorrect-approval-rule-when-update-target-branch.yml @@ -0,0 +1,5 @@ +--- +title: Display correct approval rules based on target branch in Edit MR form +merge_request: 26053 +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 b9dc19b10ab737441e80ce844fa1829901785108..1959a14c5853aab0e9d9dfe887d83b5706d53beb 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 @@ -38,6 +38,16 @@ describe('EE Approvals MRRules', () => { .find(UserAvatarList) .props('items'); const findRuleControls = () => wrapper.find('td.js-controls').find(RuleControls); + const setTargetBranchInputValue = () => { + const value = 'new value'; + const element = document.querySelector('#merge_request_target_branch'); + element.value = value; + return value; + }; + const callTargetBranchHandler = MutationObserverSpy => { + const onTargetBranchMutationHandler = MutationObserverSpy.mock.calls[0][0]; + return onTargetBranchMutationHandler(); + }; beforeEach(() => { store = createStoreOptions(MREditModule()); @@ -55,6 +65,66 @@ describe('EE Approvals MRRules', () => { approvalRules = null; }); + describe('when editing a MR', () => { + const initialTargetBranch = 'master'; + let targetBranchInputElement; + let MutationObserverSpy; + + beforeEach(() => { + MutationObserverSpy = jest.spyOn(global, 'MutationObserver'); + + targetBranchInputElement = document.createElement('input'); + targetBranchInputElement.id = 'merge_request_target_branch'; + targetBranchInputElement.value = initialTargetBranch; + document.body.appendChild(targetBranchInputElement); + + store.modules.approvals.actions = { + fetchRules: jest.fn(), + addEmptyRule: jest.fn(), + setEmptyRule: jest.fn(), + }; + store.state.settings.mrSettingsPath = 'some/path'; + store.state.settings.eligibleApproversDocsPath = 'some/path'; + store.state.settings.allowMultiRule = true; + }); + + afterEach(() => { + targetBranchInputElement.parentNode.removeChild(targetBranchInputElement); + MutationObserverSpy.mockClear(); + }); + + it('sets the target branch data to be the same value as the target branch dropdown', () => { + factory(); + + expect(wrapper.vm.targetBranch).toBe(initialTargetBranch); + }); + + it('updates the target branch data when the target branch dropdown is changed', () => { + factory(); + const newValue = setTargetBranchInputValue(); + callTargetBranchHandler(MutationObserverSpy); + expect(wrapper.vm.targetBranch).toBe(newValue); + }); + + it('re-fetches rules when target branch has changed', () => { + factory(); + setTargetBranchInputValue(); + callTargetBranchHandler(MutationObserverSpy); + expect(store.modules.approvals.actions.fetchRules).toHaveBeenCalled(); + }); + + it('disconnects MutationObserver when component gets destroyed', () => { + const mockDisconnect = jest.fn(); + MutationObserverSpy.mockImplementation(() => ({ + disconnect: mockDisconnect, + observe: jest.fn(), + })); + factory(); + wrapper.destroy(); + expect(mockDisconnect).toHaveBeenCalled(); + }); + }); + describe('when allow multiple rules', () => { beforeEach(() => { store.state.settings.allowMultiRule = true;