Merge branch 'removes_empty_approvers' into 'master'

Remove empty approvers

See merge request gitlab-org/gitlab!82338

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

+import { omitBy, isEmpty } from 'lodash';
+
 export const USER_TYPE = 'user';
 const GROUP_TYPE = 'group';
 
@@ -41,12 +43,13 @@ export function groupApprovers(existingApprovers) {
   Convert approvers into yaml fields (user_approvers, users_approvers_ids) in relation to action.
 */
 export function decomposeApprovers(action, approvers) {
-  const newAction = { ...action };
-  delete newAction.group_approvers;
-  delete newAction.user_approvers;
-  return {
-    ...newAction,
-    user_approvers_ids: userIds(approvers),
-    group_approvers_ids: groupIds(approvers),
-  };
+  const newAction = { type: action.type, approvals_required: action.approvals_required };
+  const approversInfo = omitBy(
+    {
+      user_approvers_ids: userIds(approvers),
+      group_approvers_ids: groupIds(approvers),
+    },
+    isEmpty,
+  );
+  return { ...newAction, ...approversInfo };
 }

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

@@ -25,12 +25,19 @@ const groupApprover = {
   web_url: null,
 };
 
+const actionDataWithoutApprovers = {
+  approvals_required: 1,
+  type: 'require_approval',
+};
+
 const unknownApprover = { id: 3, name: null };
 
 const allApprovers = [userApprover, groupApprover];
 
 const groupedApprovers = groupApprovers(allApprovers);
 
+const userOnlyGroupedApprovers = groupApprovers([userApprover]);
+
 describe('groupApprovers', () => {
   describe('with mixed approvers', () => {
     it('returns a copy of the input values with their proper type attribute', () => {
@@ -104,7 +111,8 @@ describe('groupApprovers', () => {
 
 describe('decomposeApprovers', () => {
   it('returns a copy of approvers adding id fields for both group and users', () => {
-    expect(decomposeApprovers({}, groupedApprovers)).toStrictEqual({
+    expect(decomposeApprovers(actionDataWithoutApprovers, groupedApprovers)).toStrictEqual({
+      ...actionDataWithoutApprovers,
       group_approvers_ids: [groupApprover.id],
       user_approvers_ids: [userApprover.id],
     });
@@ -112,27 +120,41 @@ describe('decomposeApprovers', () => {
 
   it('removes group_approvers and user_approvers keys only keeping the id fields', () => {
     expect(
-      decomposeApprovers({ user_approvers: null, group_approvers: null }, groupedApprovers),
+      decomposeApprovers(
+        { ...actionDataWithoutApprovers, user_approvers: null, group_approvers: null },
+        groupedApprovers,
+      ),
     ).toStrictEqual({
+      ...actionDataWithoutApprovers,
       group_approvers_ids: [groupApprover.id],
       user_approvers_ids: [userApprover.id],
     });
   });
 
-  it('preserves any other keys in addition to the id fields', () => {
-    expect(decomposeApprovers({ existingKey: null }, groupedApprovers)).toStrictEqual({
-      group_approvers_ids: [groupApprover.id],
+  it('returns only user info when group info is empty', () => {
+    expect(
+      decomposeApprovers({ ...actionDataWithoutApprovers }, userOnlyGroupedApprovers),
+    ).toStrictEqual({
+      ...actionDataWithoutApprovers,
       user_approvers_ids: [userApprover.id],
-      existingKey: null,
     });
   });
 
-  it('returns empty id fields if there is only unknown types', () => {
-    expect(decomposeApprovers({}, [unknownApprover])).toStrictEqual({
-      group_approvers_ids: [],
-      user_approvers_ids: [],
+  it('removes unrelated keys', () => {
+    expect(
+      decomposeApprovers({ ...actionDataWithoutApprovers, existingKey: null }, groupedApprovers),
+    ).toStrictEqual({
+      ...actionDataWithoutApprovers,
+      group_approvers_ids: [groupApprover.id],
+      user_approvers_ids: [userApprover.id],
     });
   });
+
+  it('does not returns any approvers for unknown types', () => {
+    expect(decomposeApprovers(actionDataWithoutApprovers, [unknownApprover])).toStrictEqual(
+      actionDataWithoutApprovers,
+    );
+  });
 });
 
 describe('userIds', () => {

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

@@ -117,7 +117,6 @@ describe('PolicyActionBuilder', () => {
         {
           approvals_required: ACTION.approvals_required,
           user_approvers_ids: [APPROVER_2.id],
-          group_approvers_ids: [],
         },
       ],
     ]);
@@ -140,7 +139,6 @@ describe('PolicyActionBuilder', () => {
         {
           approvals_required: ACTION.approvals_required,
           user_approvers_ids: [APPROVER_1.id, APPROVER_2.id, NEW_APPROVER.id],
-          group_approvers_ids: [],
         },
       ],
     ]);