Merge branch '300973-add-external-approvals-to-store' into 'master'

Add external approvals to project approvals store

See merge request gitlab-org/gitlab!57158

ee/app/assets/javascripts/approvals/constants.js

@@ -15,6 +15,7 @@ export const RULE_TYPE_REGULAR = 'regular';
 export const RULE_TYPE_REPORT_APPROVER = 'report_approver';
 export const RULE_TYPE_CODE_OWNER = 'code_owner';
 export const RULE_TYPE_ANY_APPROVER = 'any_approver';
+export const RULE_TYPE_EXTERNAL_APPROVAL = 'external_approval';
 export const RULE_NAME_ANY_APPROVER = 'All Members';
 
 export const VULNERABILITY_CHECK_NAME = 'Vulnerability-Check';

ee/app/assets/javascripts/approvals/mappers.js

-import { RULE_TYPE_REGULAR, RULE_TYPE_ANY_APPROVER } from './constants';
+import {
+  RULE_TYPE_REGULAR,
+  RULE_TYPE_ANY_APPROVER,
+  RULE_TYPE_EXTERNAL_APPROVAL,
+} from './constants';
 
 const visibleTypes = new Set([RULE_TYPE_ANY_APPROVER, RULE_TYPE_REGULAR]);
 
@@ -20,10 +24,17 @@ function withDefaultEmptyRule(rules = []) {
       ruleType: RULE_TYPE_ANY_APPROVER,
       protectedBranches: [],
       overridden: false,
+      external_url: null,
     },
   ];
 }
 
+export const mapExternalApprovalRuleRequest = (req) => ({
+  name: req.name,
+  protected_branch_ids: req.protectedBranchIds,
+  external_url: req.externalUrl,
+});
+
 export const mapApprovalRuleRequest = (req) => ({
   name: req.name,
   approvals_required: req.approvalsRequired,
@@ -50,6 +61,16 @@ export const mapApprovalRuleResponse = (res) => ({
   ruleType: res.rule_type,
   protectedBranches: res.protected_branches,
   overridden: res.overridden,
+  externalUrl: res.external_url,
+});
+
+export const mapExternalApprovalRuleResponse = (res) => ({
+  ...mapApprovalRuleResponse(res),
+  ruleType: RULE_TYPE_EXTERNAL_APPROVAL,
+});
+
+export const mapExternalApprovalResponse = (res) => ({
+  rules: withDefaultEmptyRule(res.map(mapExternalApprovalRuleResponse)),
 });
 
 export const mapApprovalSettingsResponse = (res) => ({

ee/app/assets/javascripts/approvals/stores/modules/project_settings/actions.js

-import createFlash from '~/flash';
-import axios from '~/lib/utils/axios_utils';
-import { __ } from '~/locale';
 import {
+  mapExternalApprovalRuleRequest,
   mapApprovalRuleRequest,
   mapApprovalSettingsResponse,
   mapApprovalFallbackRuleRequest,
-} from '../../../mappers';
+  mapExternalApprovalResponse,
+} from 'ee/approvals/mappers';
+import { joinRuleResponses } from 'ee/approvals/utils';
+import createFlash from '~/flash';
+import axios from '~/lib/utils/axios_utils';
+import { __ } from '~/locale';
 import * as types from '../base/mutation_types';
 
+const fetchSettings = ({ settingsPath }) => {
+  return axios.get(settingsPath).then((res) => mapApprovalSettingsResponse(res.data));
+};
+
+const fetchExternalApprovalRules = ({ externalApprovalRulesPath }) => {
+  return axios.get(externalApprovalRulesPath).then((res) => mapExternalApprovalResponse(res.data));
+};
+
 export const requestRules = ({ commit }) => {
   commit(types.SET_LOADING, true);
 };
@@ -24,13 +35,16 @@ export const receiveRulesError = () => {
 };
 
 export const fetchRules = ({ rootState, dispatch }) => {
-  const { settingsPath } = rootState.settings;
-
   dispatch('requestRules');
 
-  return axios
-    .get(settingsPath)
-    .then((response) => dispatch('receiveRulesSuccess', mapApprovalSettingsResponse(response.data)))
+  const requests = [fetchSettings(rootState.settings)];
+
+  if (gon?.features?.ffComplianceApprovalGates) {
+    requests.push(fetchExternalApprovalRules(rootState.settings));
+  }
+
+  return Promise.all(requests)
+    .then((responses) => dispatch('receiveRulesSuccess', joinRuleResponses(responses)))
     .catch(() => dispatch('receiveRulesError'));
 };
 
@@ -39,6 +53,31 @@ export const postRuleSuccess = ({ dispatch }) => {
   dispatch('fetchRules');
 };
 
+export const putExternalApprovalRule = ({ rootState, dispatch }, { id, ...newRule }) => {
+  const { externalApprovalRulesPath } = rootState.settings;
+
+  return axios
+    .put(`${externalApprovalRulesPath}/${id}`, mapExternalApprovalRuleRequest(newRule))
+    .then(() => dispatch('postRuleSuccess'));
+};
+
+export const deleteExternalApprovalRule = ({ rootState, dispatch }, id) => {
+  const { externalApprovalRulesPath } = rootState.settings;
+
+  return axios
+    .delete(`${externalApprovalRulesPath}/${id}`)
+    .then(() => dispatch('deleteRuleSuccess'))
+    .catch(() => dispatch('deleteRuleError'));
+};
+
+export const postExternalApprovalRule = ({ rootState, dispatch }, rule) => {
+  const { externalApprovalRulesPath } = rootState.settings;
+
+  return axios
+    .post(externalApprovalRulesPath, mapExternalApprovalRuleRequest(rule))
+    .then(() => dispatch('postRuleSuccess'));
+};
+
 export const postRule = ({ rootState, dispatch }, rule) => {
   const { rulesPath } = rootState.settings;
 

ee/app/assets/javascripts/approvals/utils.js

+import { flatten } from 'lodash';
+
+export const joinRuleResponses = (responsesArray) =>
+  Object.assign({}, ...responsesArray, {
+    rules: flatten(responsesArray.map(({ rules }) => rules)),
+  });

ee/app/helpers/ee/projects_helper.rb

@@ -111,7 +111,7 @@ module EE
     end
 
     def approvals_app_data(project = @project)
-      { data: { 'project_id': project.id,
+      data = { 'project_id': project.id,
       'can_edit': can_modify_approvers.to_s,
       'project_path': expose_path(api_v4_projects_path(id: project.id)),
       'settings_path': expose_path(api_v4_projects_approval_settings_path(id: project.id)),
@@ -121,7 +121,13 @@ module EE
       'security_approvals_help_page_path': help_page_path('user/application_security/index.md', anchor: 'security-approvals-in-merge-requests'),
       'security_configuration_path': project_security_configuration_path(project),
       'vulnerability_check_help_page_path': help_page_path('user/application_security/index', anchor: 'enabling-security-approvals-within-a-project'),
-      'license_check_help_page_path': help_page_path('user/application_security/index', anchor: 'enabling-license-approvals-within-a-project') } }
+      'license_check_help_page_path': help_page_path('user/application_security/index', anchor: 'enabling-license-approvals-within-a-project') }
+
+      if ::Feature.enabled?(:ff_compliance_approval_gates, project, default_enabled: :yaml)
+        data[:external_approval_rules_path] = expose_path(api_v4_projects_external_approval_rules_path(id: project.id))
+      end
+
+      { data: data }
     end
 
     def can_modify_approvers(project = @project)

ee/spec/frontend/approvals/stores/modules/project_settings/actions_spec.js

 import MockAdapter from 'axios-mock-adapter';
-import { mapApprovalRuleRequest, mapApprovalSettingsResponse } from 'ee/approvals/mappers';
+import {
+  mapApprovalRuleRequest,
+  mapApprovalSettingsResponse,
+  mapExternalApprovalResponse,
+} from 'ee/approvals/mappers';
 import * as types from 'ee/approvals/stores/modules/base/mutation_types';
 import * as actions from 'ee/approvals/stores/modules/project_settings/actions';
+import { joinRuleResponses } from 'ee/approvals/utils';
 import testAction from 'helpers/vuex_action_helper';
 import createFlash from '~/flash';
 import axios from '~/lib/utils/axios_utils';
+import httpStatus from '~/lib/utils/http_status';
 
 jest.mock('~/flash');
 
@@ -16,6 +22,11 @@ const TEST_RULE_REQUEST = {
   groups: [7],
   users: [8, 9],
 };
+const TEST_EXTERNAL_RULE_REQUEST = {
+  name: 'Lorem',
+  protected_branch_ids: [],
+  external_url: 'https://www.gitlab.com',
+};
 const TEST_RULE_RESPONSE = {
   id: 7,
   name: 'Ipsum',
@@ -26,14 +37,19 @@ const TEST_RULE_RESPONSE = {
 };
 const TEST_SETTINGS_PATH = 'projects/9/approval_settings';
 const TEST_RULES_PATH = 'projects/9/approval_settings/rules';
+const TEST_EXTERNAL_RULES_PATH = 'projects/9/external_approval_rules';
 
 describe('EE approvals project settings module actions', () => {
   let state;
   let mock;
+  let originalGon;
 
   beforeEach(() => {
+    originalGon = { ...window.gon };
+    window.gon = { features: { ffComplianceApprovalGates: true } };
     state = {
       settings: {
+        externalApprovalRulesPath: TEST_EXTERNAL_RULES_PATH,
         projectId: TEST_PROJECT_ID,
         settingsPath: TEST_SETTINGS_PATH,
         rulesPath: TEST_RULES_PATH,
@@ -44,6 +60,7 @@ describe('EE approvals project settings module actions', () => {
 
   afterEach(() => {
     mock.restore();
+    window.gon = originalGon;
   });
 
   describe('requestRules', () => {
@@ -89,27 +106,37 @@ describe('EE approvals project settings module actions', () => {
   });
 
   describe('fetchRules', () => {
-    it('dispatches request/receive', () => {
-      const data = { rules: [TEST_RULE_RESPONSE] };
-      mock.onGet(TEST_SETTINGS_PATH).replyOnce(200, data);
-
+    const testFetchRuleAction = (payload, history) => {
       return testAction(
         actions.fetchRules,
         null,
         state,
         [],
-        [
-          { type: 'requestRules' },
-          { type: 'receiveRulesSuccess', payload: mapApprovalSettingsResponse(data) },
-        ],
+        [{ type: 'requestRules' }, { type: 'receiveRulesSuccess', payload }],
         () => {
-          expect(mock.history.get.map((x) => x.url)).toEqual([TEST_SETTINGS_PATH]);
+          expect(mock.history.get.map((x) => x.url)).toEqual(history);
         },
       );
+    };
+
+    it('dispatches request/receive', () => {
+      const data = { rules: [TEST_RULE_RESPONSE] };
+      mock.onGet(TEST_SETTINGS_PATH).replyOnce(httpStatus.OK, data);
+
+      const externalRuleData = [TEST_RULE_RESPONSE];
+      mock.onGet(TEST_EXTERNAL_RULES_PATH).replyOnce(httpStatus.OK, externalRuleData);
+
+      return testFetchRuleAction(
+        joinRuleResponses([
+          mapApprovalSettingsResponse(data),
+          mapExternalApprovalResponse(externalRuleData),
+        ]),
+        [TEST_SETTINGS_PATH, TEST_EXTERNAL_RULES_PATH],
+      );
     });
 
     it('dispatches request/receive on error', () => {
-      mock.onGet(TEST_SETTINGS_PATH).replyOnce(500);
+      mock.onGet(TEST_SETTINGS_PATH).replyOnce(httpStatus.INTERNAL_SERVER_ERROR);
 
       return testAction(
         actions.fetchRules,
@@ -119,6 +146,21 @@ describe('EE approvals project settings module actions', () => {
         [{ type: 'requestRules' }, { type: 'receiveRulesError' }],
       );
     });
+
+    describe('when the ffComplianceApprovalGates feature flag is disabled', () => {
+      beforeEach(() => {
+        window.gon = { features: { ffComplianceApprovalGates: false } };
+      });
+
+      it('dispatches request/receive for a single request', () => {
+        const data = { rules: [TEST_RULE_RESPONSE] };
+        mock.onGet(TEST_SETTINGS_PATH).replyOnce(httpStatus.OK, data);
+
+        return testFetchRuleAction(joinRuleResponses([mapApprovalSettingsResponse(data)]), [
+          TEST_SETTINGS_PATH,
+        ]);
+      });
+    });
   });
 
   describe('postRuleSuccess', () => {
@@ -133,43 +175,44 @@ describe('EE approvals project settings module actions', () => {
     });
   });
 
-  describe('postRule', () => {
-    it('dispatches success on success', () => {
-      mock.onPost(TEST_RULES_PATH).replyOnce(200);
+  describe('POST', () => {
+    it.each`
+      action                        | path                        | request
+      ${'postRule'}                 | ${TEST_RULES_PATH}          | ${TEST_RULE_REQUEST}
+      ${'postExternalApprovalRule'} | ${TEST_EXTERNAL_RULES_PATH} | ${TEST_EXTERNAL_RULE_REQUEST}
+    `('dispatches success on success for $action', ({ action, path, request }) => {
+      mock.onPost(path).replyOnce(httpStatus.OK);
 
-      return testAction(
-        actions.postRule,
-        TEST_RULE_REQUEST,
-        state,
-        [],
-        [{ type: 'postRuleSuccess' }],
-        () => {
+      return testAction(actions[action], request, state, [], [{ type: 'postRuleSuccess' }], () => {
         expect(mock.history.post).toEqual([
           expect.objectContaining({
-              url: TEST_RULES_PATH,
-              data: JSON.stringify(mapApprovalRuleRequest(TEST_RULE_REQUEST)),
+            url: path,
+            data: JSON.stringify(mapApprovalRuleRequest(request)),
           }),
         ]);
-        },
-      );
+      });
     });
   });
 
-  describe('putRule', () => {
-    it('dispatches success on success', () => {
-      mock.onPut(`${TEST_RULES_PATH}/${TEST_RULE_ID}`).replyOnce(200);
+  describe('PUT', () => {
+    it.each`
+      action                       | path                        | request
+      ${'putRule'}                 | ${TEST_RULES_PATH}          | ${TEST_RULE_REQUEST}
+      ${'putExternalApprovalRule'} | ${TEST_EXTERNAL_RULES_PATH} | ${TEST_EXTERNAL_RULE_REQUEST}
+    `('dispatches success on success for $action', ({ action, path, request }) => {
+      mock.onPut(`${path}/${TEST_RULE_ID}`).replyOnce(httpStatus.OK);
 
       return testAction(
-        actions.putRule,
-        { id: TEST_RULE_ID, ...TEST_RULE_REQUEST },
+        actions[action],
+        { id: TEST_RULE_ID, ...request },
         state,
         [],
         [{ type: 'postRuleSuccess' }],
         () => {
           expect(mock.history.put).toEqual([
             expect.objectContaining({
-              url: `${TEST_RULES_PATH}/${TEST_RULE_ID}`,
-              data: JSON.stringify(mapApprovalRuleRequest(TEST_RULE_REQUEST)),
+              url: `${path}/${TEST_RULE_ID}`,
+              data: JSON.stringify(mapApprovalRuleRequest(request)),
             }),
           ]);
         },
@@ -201,12 +244,16 @@ describe('EE approvals project settings module actions', () => {
     });
   });
 
-  describe('deleteRule', () => {
-    it('dispatches success on success', () => {
-      mock.onDelete(`${TEST_RULES_PATH}/${TEST_RULE_ID}`).replyOnce(200);
+  describe('DELETE', () => {
+    it.each`
+      action                          | path
+      ${'deleteRule'}                 | ${TEST_RULES_PATH}
+      ${'deleteExternalApprovalRule'} | ${TEST_EXTERNAL_RULES_PATH}
+    `('dispatches success on success for $action', ({ action, path }) => {
+      mock.onDelete(`${path}/${TEST_RULE_ID}`).replyOnce(httpStatus.OK);
 
       return testAction(
-        actions.deleteRule,
+        actions[action],
         TEST_RULE_ID,
         state,
         [],
@@ -214,7 +261,7 @@ describe('EE approvals project settings module actions', () => {
         () => {
           expect(mock.history.delete).toEqual([
             expect.objectContaining({
-              url: `${TEST_RULES_PATH}/${TEST_RULE_ID}`,
+              url: `${path}/${TEST_RULE_ID}`,
             }),
           ]);
         },
@@ -222,7 +269,9 @@ describe('EE approvals project settings module actions', () => {
     });
 
     it('dispatches error on error', () => {
-      mock.onDelete(`${TEST_RULES_PATH}/${TEST_RULE_ID}`).replyOnce(500);
+      mock
+        .onDelete(`${TEST_RULES_PATH}/${TEST_RULE_ID}`)
+        .replyOnce(httpStatus.INTERNAL_SERVER_ERROR);
 
       return testAction(actions.deleteRule, TEST_RULE_ID, state, [], [{ type: 'deleteRuleError' }]);
     });

ee/spec/frontend/approvals/utils_spec.js

+import * as Utils from 'ee/approvals/utils';
+
+describe('Utils', () => {
+  describe('joinRuleResponses', () => {
+    it('should join multiple response objects and concatenate the rules array of all objects', () => {
+      const resX = { foo: 'bar', rules: [1, 2, 3] };
+      const resY = { foo: 'something', rules: [4, 5] };
+
+      expect(Utils.joinRuleResponses([resX, resY])).toStrictEqual({
+        foo: 'something',
+        rules: [1, 2, 3, 4, 5],
+      });
+    });
+  });
+});

ee/spec/helpers/projects_helper_spec.rb

@@ -644,4 +644,28 @@ RSpec.describe ProjectsHelper do
       end
     end
   end
+
+  describe '#approvals_app_data' do
+    subject { helper.approvals_app_data(project) }
+
+    let(:user) { instance_double(User, admin?: false) }
+
+    before do
+      allow(helper).to receive(:current_user).and_return(user)
+      allow(helper).to receive(:can?).and_return(true)
+    end
+
+    context 'with the approval gate feature flag' do
+      where(feature_flag_enabled: [true, false])
+      with_them do
+        before do
+          stub_feature_flags(ff_compliance_approval_gates: feature_flag_enabled)
+        end
+
+        it 'includes external_approval_rules_path only when enabled' do
+          expect(subject[:data].key?(:external_approval_rules_path)).to eq(feature_flag_enabled)
+        end
+      end
+    end
+  end
 end