Merge branch '199790-incorrect-approval-rule-when-update-target-branch' into 'master'

Display correct approval rules based on target branch in Edit MR form

Closes #199790

See merge request gitlab-org/gitlab!26053

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);
+      }
+    },
   },
 };
 </script>

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,

ee/changelogs/unreleased/199790-incorrect-approval-rule-when-update-target-branch.yml

+---
+title: Display correct approval rules based on target branch in Edit MR form
+merge_request: 26053
+author:
+type: added

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;