Merge branch 'add_validation_for_rule_yaml_mode_switch' into 'master'

Add validation for rule/yaml modes switch

See merge request gitlab-org/gitlab!82451

ee/app/assets/javascripts/threat_monitoring/components/policy_editor/scan_result_policy/lib/actions.js

@@ -53,3 +53,47 @@ export function decomposeApprovers(action, approvers) {
   );
   return { ...newAction, ...approversInfo };
 }
+
+/*
+  Check if users are present in approvers
+*/
+function usersOutOfSync(action, approvers) {
+  const users = approvers.filter((approver) => approver.type === USER_TYPE);
+  const usersIDs =
+    action?.user_approvers_ids?.some((id) => !users.find((approver) => approver.id === id)) ||
+    false;
+  const usersNames =
+    action?.user_approvers?.some(
+      (userName) => !users.find((approver) => approver.username === userName),
+    ) || false;
+  const userLength =
+    (action?.user_approvers?.length || 0) + (action?.user_approvers_ids?.length || 0);
+
+  return usersIDs || usersNames || userLength !== users.length;
+}
+
+/*
+  Check if groups are present in approvers
+*/
+function groupsOutOfSync(action, approvers) {
+  const groups = approvers.filter((approver) => approver.type === GROUP_TYPE);
+  const groupsIDs =
+    action?.group_approvers_ids?.some((id) => !groups.find((approver) => approver.id === id)) ||
+    false;
+  const groupsPaths =
+    action?.group_approvers?.some(
+      (path) => !groups.find((approver) => approver.full_path === path),
+    ) || false;
+  const groupLength =
+    (action?.group_approvers?.length || 0) + (action?.group_approvers_ids?.length || 0);
+
+  return groupsIDs || groupsPaths || groupLength !== groups.length;
+}
+
+/*
+  Check if yaml is out of sync with available approvers
+*/
+export function approversOutOfSync(action, existingApprovers) {
+  const approvers = groupApprovers(existingApprovers);
+  return usersOutOfSync(action, approvers) || groupsOutOfSync(action, approvers);
+}

ee/app/assets/javascripts/threat_monitoring/components/policy_editor/scan_result_policy/lib/index.js

 export { fromYaml } from './from_yaml';
 export { toYaml } from './to_yaml';
 export { buildRule } from './rules';
+export { approversOutOfSync } from './actions';
 export * from './humanize';
 
 export const DEFAULT_SCAN_RESULT_POLICY = `type: scan_result_policy

ee/app/assets/javascripts/threat_monitoring/components/policy_editor/scan_result_policy/policy_action_builder.vue

@@ -46,6 +46,7 @@ export default {
   watch: {
     approvers(values) {
       this.action = decomposeApprovers(this.action, values);
+      this.$emit('approversUpdated', this.approvers);
     },
     approversToAdd(val) {
       this.approvers.push(val[0]);

ee/app/assets/javascripts/threat_monitoring/components/policy_editor/scan_result_policy/scan_result_policy_editor.vue

@@ -22,7 +22,7 @@ import { assignSecurityPolicyProject, modifyPolicy } from '../utils';
 import DimDisableContainer from '../dim_disable_container.vue';
 import PolicyActionBuilder from './policy_action_builder.vue';
 import PolicyRuleBuilder from './policy_rule_builder.vue';
-import { DEFAULT_SCAN_RESULT_POLICY, fromYaml, toYaml, buildRule } from './lib';
+import { DEFAULT_SCAN_RESULT_POLICY, fromYaml, toYaml, buildRule, approversOutOfSync } from './lib';
 
 export default {
   SECURITY_POLICY_ACTIONS,
@@ -100,6 +100,7 @@ export default {
       ),
       yamlEditorError: null,
       mode: EDITOR_MODE_RULE,
+      existingApprovers: this.scanResultPolicyApprovers,
     };
   },
   computed: {
@@ -207,8 +208,15 @@ export default {
       this.mode = mode;
       if (mode === EDITOR_MODE_YAML && !this.hasParsingError) {
         this.yamlEditorValue = toYaml(this.policy);
+      } else if (mode === EDITOR_MODE_RULE && !this.hasParsingError) {
+        if (approversOutOfSync(this.policy.actions[0], this.existingApprovers)) {
+          this.yamlEditorError = new Error();
+        }
       }
     },
+    updatePolicyApprovers(values) {
+      this.existingApprovers = values;
+    },
   },
 };
 </script>
@@ -296,8 +304,9 @@ export default {
           :key="index"
           class="gl-mb-4"
           :init-action="action"
-          :existing-approvers="scanResultPolicyApprovers"
+          :existing-approvers="existingApprovers"
           @changed="updateAction(index, $event)"
+          @approversUpdated="updatePolicyApprovers"
         />
       </dim-disable-container>
     </template>

ee/spec/frontend/threat_monitoring/components/policy_editor/scan_result_policy/lib/actions_spec.js

@@ -3,6 +3,7 @@ import {
   userIds,
   groupApprovers,
   decomposeApprovers,
+  approversOutOfSync,
 } from 'ee/threat_monitoring/components/policy_editor/scan_result_policy/lib/actions';
 
 // As returned by endpoints based on API::Entities::UserBasic
@@ -10,7 +11,7 @@ const userApprover = {
   id: 1,
   name: null,
   state: null,
-  username: null,
+  username: 'user name',
   avatar_url: null,
   web_url: null,
 };
@@ -20,7 +21,7 @@ const groupApprover = {
   id: 2,
   name: null,
   full_name: null,
-  full_path: null,
+  full_path: 'full path',
   avatar_url: null,
   web_url: null,
 };
@@ -48,13 +49,13 @@ describe('groupApprovers', () => {
           name: null,
           state: null,
           type: 'user',
-          username: null,
+          username: 'user name',
           web_url: null,
         },
         {
           avatar_url: null,
           full_name: null,
-          full_path: null,
+          full_path: 'full path',
           id: groupApprover.id,
           name: null,
           type: 'group',
@@ -79,7 +80,7 @@ describe('groupApprovers', () => {
       {
         avatar_url: null,
         full_name: null,
-        full_path: null,
+        full_path: 'full path',
         id: groupApprover.id,
         name: null,
         type: 'group',
@@ -96,7 +97,7 @@ describe('groupApprovers', () => {
         name: null,
         state: null,
         type: 'user',
-        username: null,
+        username: 'user name',
         web_url: null,
       },
     ]);
@@ -168,3 +169,174 @@ describe('groupIds', () => {
     expect(groupIds(groupedApprovers)).toStrictEqual([groupApprover.id]);
   });
 });
+
+describe('approversOutOfSync', () => {
+  describe('with user_approvers_ids only', () => {
+    it.each`
+      ids       | approvers         | result
+      ${[1]}    | ${[userApprover]} | ${false}
+      ${[]}     | ${[]}             | ${false}
+      ${[]}     | ${[userApprover]} | ${true}
+      ${[1]}    | ${[]}             | ${true}
+      ${[1, 2]} | ${[userApprover]} | ${true}
+      ${[2]}    | ${[]}             | ${true}
+      ${[2]}    | ${[userApprover]} | ${true}
+    `(
+      'return $result when ids and approvers length equal to $ids and $approvers.length',
+      ({ ids, approvers, result }) => {
+        const action = {
+          approvals_required: 1,
+          type: 'require_approval',
+          user_approvers_ids: ids,
+        };
+        expect(approversOutOfSync(action, approvers)).toBe(result);
+      },
+    );
+  });
+  describe('with user_approvers only', () => {
+    it.each`
+      usernames                       | approvers         | result
+      ${['user name']}                | ${[userApprover]} | ${false}
+      ${[]}                           | ${[]}             | ${false}
+      ${[]}                           | ${[userApprover]} | ${true}
+      ${['user name']}                | ${[]}             | ${true}
+      ${['user name', 'not present']} | ${[userApprover]} | ${true}
+      ${['not present']}              | ${[]}             | ${true}
+      ${['not present']}              | ${[userApprover]} | ${true}
+    `(
+      'return $result when usernames and approvers length equal to $usernames and $approvers.length',
+      ({ usernames, approvers, result }) => {
+        const action = {
+          approvals_required: 1,
+          type: 'require_approval',
+          user_approvers: usernames,
+        };
+        expect(approversOutOfSync(action, approvers)).toBe(result);
+      },
+    );
+  });
+  describe('with user_approvers and user_approvers_ids', () => {
+    it.each`
+      ids    | usernames          | approvers         | result
+      ${[]}  | ${['user name']}   | ${[userApprover]} | ${false}
+      ${[1]} | ${[]}              | ${[userApprover]} | ${false}
+      ${[]}  | ${[]}              | ${[]}             | ${false}
+      ${[1]} | ${['user name']}   | ${[userApprover]} | ${true}
+      ${[1]} | ${['not present']} | ${[userApprover]} | ${true}
+      ${[2]} | ${['user name']}   | ${[userApprover]} | ${true}
+    `(
+      'return $result when ids, usernames and approvers length equal to $ids, $usernames and $approvers.length',
+      ({ ids, usernames, approvers, result }) => {
+        const action = {
+          approvals_required: 1,
+          type: 'require_approval',
+          user_approvers: usernames,
+          user_approvers_ids: ids,
+        };
+        expect(approversOutOfSync(action, approvers)).toBe(result);
+      },
+    );
+  });
+  describe('with group_approvers_ids only', () => {
+    it.each`
+      ids       | approvers          | result
+      ${[2]}    | ${[groupApprover]} | ${false}
+      ${[]}     | ${[]}              | ${false}
+      ${[]}     | ${[groupApprover]} | ${true}
+      ${[2]}    | ${[]}              | ${true}
+      ${[2, 3]} | ${[groupApprover]} | ${true}
+      ${[3]}    | ${[]}              | ${true}
+      ${[3]}    | ${[groupApprover]} | ${true}
+    `(
+      'return $result when ids and approvers length equal to $ids and $approvers.length',
+      ({ ids, approvers, result }) => {
+        const action = {
+          approvals_required: 1,
+          type: 'require_approval',
+          group_approvers_ids: ids,
+        };
+        expect(approversOutOfSync(action, approvers)).toBe(result);
+      },
+    );
+  });
+  describe('with user_approvers, user_approvers_ids and group_approvers_ids', () => {
+    it.each`
+      user_ids | usernames        | group_ids | approvers          | result
+      ${[]}    | ${['user name']} | ${[2]}    | ${allApprovers}    | ${false}
+      ${[1]}   | ${[]}            | ${[2]}    | ${allApprovers}    | ${false}
+      ${[]}    | ${[]}            | ${[]}     | ${[]}              | ${false}
+      ${[1]}   | ${['user name']} | ${[2]}    | ${allApprovers}    | ${true}
+      ${[]}    | ${['user name']} | ${[3]}    | ${allApprovers}    | ${true}
+      ${[1]}   | ${[]}            | ${[3]}    | ${allApprovers}    | ${true}
+      ${[]}    | ${[]}            | ${[2]}    | ${[groupApprover]} | ${false}
+      ${[1]}   | ${[]}            | ${[2]}    | ${[groupApprover]} | ${true}
+      ${[]}    | ${['user name']} | ${[2]}    | ${[groupApprover]} | ${true}
+      ${[]}    | ${['user name']} | ${[]}     | ${[userApprover]}  | ${false}
+      ${[1]}   | ${[]}            | ${[]}     | ${[userApprover]}  | ${false}
+      ${[1]}   | ${[]}            | ${[2]}    | ${[userApprover]}  | ${true}
+    `(
+      'return $result when user_ids, usernames, group_ids and approvers length equal to $user_ids, $usernames, $group_ids and $approvers.length',
+      ({ user_ids, usernames, group_ids, approvers, result }) => {
+        const action = {
+          approvals_required: 1,
+          type: 'require_approval',
+          user_approvers: usernames,
+          user_approvers_ids: user_ids,
+          group_approvers_ids: group_ids,
+        };
+        expect(approversOutOfSync(action, approvers)).toBe(result);
+      },
+    );
+  });
+  describe('with group_approvers only', () => {
+    it.each`
+      fullPath                        | approvers          | result
+      ${['full path']}                | ${[groupApprover]} | ${false}
+      ${[]}                           | ${[]}              | ${false}
+      ${[]}                           | ${[groupApprover]} | ${true}
+      ${['full path']}                | ${[]}              | ${true}
+      ${['full path', 'not present']} | ${[groupApprover]} | ${true}
+      ${['not present']}              | ${[]}              | ${true}
+      ${['not present']}              | ${[groupApprover]} | ${true}
+    `(
+      'return $result when fullPath and approvers length equal to $fullPath and $approvers.length',
+      ({ fullPath, approvers, result }) => {
+        const action = {
+          approvals_required: 1,
+          type: 'require_approval',
+          group_approvers: fullPath,
+        };
+        expect(approversOutOfSync(action, approvers)).toBe(result);
+      },
+    );
+  });
+  describe('with user_approvers, user_approvers_ids, group_approvers_ids and group_approvers', () => {
+    it.each`
+      user_ids | usernames        | group_ids | group_paths        | approvers           | result
+      ${[]}    | ${['user name']} | ${[2]}    | ${[]}              | ${allApprovers}     | ${false}
+      ${[1]}   | ${[]}            | ${[2]}    | ${[]}              | ${allApprovers}     | ${false}
+      ${[1]}   | ${[]}            | ${[]}     | ${['full path']}   | ${allApprovers}     | ${false}
+      ${[]}    | ${['user name']} | ${[]}     | ${['full path']}   | ${allApprovers}     | ${false}
+      ${[]}    | ${[]}            | ${[]}     | ${[]}              | ${[]}               | ${false}
+      ${[]}    | ${['user name']} | ${[3]}    | ${[]}              | ${allApprovers}     | ${true}
+      ${[1]}   | ${[]}            | ${[3]}    | ${[]}              | ${allApprovers}     | ${true}
+      ${[1]}   | ${[]}            | ${[]}     | ${['not present']} | ${allApprovers}     | ${true}
+      ${[]}    | ${['user name']} | ${[]}     | ${['not present']} | ${allApprovers}     | ${true}
+      ${[1]}   | ${[]}            | ${[]}     | ${['full path']}   | ${[groupApprovers]} | ${true}
+      ${[]}    | ${['user name']} | ${[]}     | ${['full path']}   | ${[groupApprovers]} | ${true}
+    `(
+      'return $result when user_ids, usernames, group_ids, group_paths and approvers length equal to $user_ids, $usernames, $group_ids, $group_paths and $approvers.length',
+      ({ user_ids, usernames, group_ids, group_paths, approvers, result }) => {
+        const action = {
+          approvals_required: 1,
+          type: 'require_approval',
+          user_approvers: usernames,
+          user_approvers_ids: user_ids,
+          group_approvers_ids: group_ids,
+          group_approvers: group_paths,
+        };
+        expect(approversOutOfSync(action, approvers)).toBe(result);
+      },
+    );
+  });
+});

ee/spec/frontend/threat_monitoring/components/policy_editor/scan_result_policy/scan_result_policy_editor_spec.js

@@ -52,7 +52,7 @@ describe('ScanResultPolicyEditor', () => {
     branch: 'main',
     fullPath: 'path/to/existing-project',
   };
-  const scanResultPolicyApprovers = [{ id: 1, username: 'username', state: 'active' }];
+  const scanResultPolicyApprovers = [{ id: 1, username: 'the.one', state: 'active' }];
 
   const factory = ({ propsData = {}, provide = {} } = {}) => {
     wrapper = shallowMount(ScanResultPolicyEditor, {
@@ -315,5 +315,26 @@ describe('ScanResultPolicyEditor', () => {
 
       expect(findPolicyActionBuilder().props('initAction')).toEqual(UPDATED_ACTION);
     });
+
+    it('does not show alert when policy matches existing approvers', async () => {
+      factoryWithExistingPolicy();
+
+      expect(findAlert().exists()).toBe(false);
+
+      await findPolicyEditorLayout().vm.$emit('update-editor-mode', EDITOR_MODE_RULE);
+
+      expect(findAlert().exists()).toBe(false);
+    });
+
+    it('shows alert when policy does not match existing approvers', async () => {
+      factory();
+
+      expect(findAlert().exists()).toBe(false);
+
+      await findPolicyEditorLayout().vm.$emit('update-editor-mode', EDITOR_MODE_RULE);
+
+      expect(findAlert().exists()).toBe(true);
+      expect(findAlert().isVisible()).toBe(true);
+    });
   });
 });