Merge branch 'approvers-use-new-project-api-fe' into 'master'

Update approvers dropdown to use new endpoint

See merge request gitlab-org/gitlab!53781

app/assets/javascripts/api.js

@@ -31,6 +31,7 @@ const Api = {
   projectLabelsPath: '/:namespace_path/:project_path/-/labels',
   projectFileSchemaPath: '/:namespace_path/:project_path/-/schema/:ref/:filename',
   projectUsersPath: '/api/:version/projects/:id/users',
+  projectGroupsPath: '/api/:version/projects/:id/groups.json',
   projectInvitationsPath: '/api/:version/projects/:id/invitations',
   projectMembersPath: '/api/:version/projects/:id/members',
   projectMergeRequestsPath: '/api/:version/projects/:id/merge_requests',
@@ -241,6 +242,20 @@ const Api = {
       .then(({ data }) => data);
   },
 
+  projectGroups(id, options) {
+    const url = Api.buildUrl(this.projectGroupsPath).replace(':id', encodeURIComponent(id));
+
+    return axios
+      .get(url, {
+        params: {
+          ...options,
+        },
+      })
+      .then(({ data }) => {
+        return data;
+      });
+  },
+
   addProjectMembersByUserId(id, data) {
     const url = Api.buildUrl(this.projectMembersPath).replace(':id', encodeURIComponent(id));
 

ee/app/assets/javascripts/approvals/components/approvers_select.vue

@@ -135,13 +135,11 @@ export default {
         .then((results) => ({ results }));
     },
     fetchGroups(term) {
-      // Don't includeAll when search is empty. Otherwise, the user could get a lot of garbage choices.
-      // https://gitlab.com/gitlab-org/gitlab/issues/11566
-      const includeAll = term.trim().length > 0;
+      const hasTerm = term.trim().length > 0;
 
-      return Api.groups(term, {
+      return Api.projectGroups(this.projectId, {
         skip_groups: this.skipGroupIds,
-        all_available: includeAll,
+        ...(hasTerm ? { search: term } : {}),
       });
     },
     fetchUsers(term) {

ee/spec/features/merge_request/user_edits_approval_rules_mr_spec.rb

@@ -73,11 +73,7 @@ RSpec.describe 'Merge request > User edits MR with approval rules', :js do
   end
 
   context "with public group" do
-    let_it_be(:group) { create(:group, :public) }
-
     before do
-      group.add_developer create(:user)
-
       click_button 'Approval rules'
       click_button "Add approval rule"
     end

ee/spec/features/merge_request/user_sets_approvers_spec.rb

@@ -87,10 +87,12 @@ RSpec.describe 'Merge request > User sets approvers', :js do
 
       it 'allows setting groups as approvers' do
         group = create :group
-        group.add_developer(other_user)
+        group_project = create :project, :repository, group: group
 
-        visit project_new_merge_request_path(project, merge_request: { target_branch: 'master', source_branch: 'feature' })
+        group.add_developer(user)
+        group.add_developer(other_user)
 
+        visit project_new_merge_request_path(group_project, merge_request: { target_branch: 'master', source_branch: 'feature' })
         open_modal(text: 'Add approval rule')
         open_approver_select
 
@@ -156,9 +158,13 @@ RSpec.describe 'Merge request > User sets approvers', :js do
 
       it 'allows setting groups as approvers' do
         group = create :group
+        group_project = create :project, :repository, group: group
+        group_project_merge_request = create :merge_request, source_project: group_project
+
+        group.add_developer(user)
         group.add_developer(other_user)
 
-        visit edit_project_merge_request_path(project, merge_request)
+        visit edit_project_merge_request_path(group_project, group_project_merge_request)
 
         open_modal(text: 'Add approval rule')
         open_approver_select

ee/spec/features/projects/settings/merge_requests_settings_spec.rb

@@ -6,8 +6,8 @@ RSpec.describe 'Project settings > [EE] Merge Requests', :js do
   include FeatureApprovalHelper
 
   let(:user) { create(:user) }
-  let(:project) { create(:project) }
   let(:group) { create(:group) }
+  let(:project) { create(:project, group: group) }
   let(:group_member) { create(:user) }
   let(:non_member) { create(:user) }
   let!(:config_selector) { '.js-approval-rules' }

ee/spec/frontend/approvals/components/approvers_select_spec.js

@@ -76,7 +76,7 @@ describe('Approvals ApproversSelect', () => {
   };
 
   beforeEach(() => {
-    jest.spyOn(Api, 'groups').mockResolvedValue(TEST_GROUPS);
+    jest.spyOn(Api, 'projectGroups').mockResolvedValue(TEST_GROUPS);
     jest.spyOn(Api, 'projectUsers').mockReturnValue(Promise.resolve(TEST_USERS));
   });
 
@@ -126,7 +126,10 @@ describe('Approvals ApproversSelect', () => {
     });
 
     it('fetches all available groups', () => {
-      expect(Api.groups).toHaveBeenCalledWith(term, { skip_groups: [], all_available: true });
+      expect(Api.projectGroups).toHaveBeenCalledWith(TEST_PROJECT_ID, {
+        skip_groups: [],
+        search: term,
+      });
     });
 
     it('fetches users', () => {
@@ -157,9 +160,8 @@ describe('Approvals ApproversSelect', () => {
     });
 
     it('skips groups and does not fetch all available', () => {
-      expect(Api.groups).toHaveBeenCalledWith('', {
+      expect(Api.projectGroups).toHaveBeenCalledWith(TEST_PROJECT_ID, {
         skip_groups: skipGroupIds,
-        all_available: false,
       });
     });
 

ee/spec/support/shared_contexts/project_approval_rules_shared_context.rb

@@ -3,7 +3,8 @@
 RSpec.shared_context 'with project with approval rules' do
   let_it_be(:approver) { create(:user) }
   let_it_be(:author) { create(:user) }
-  let_it_be(:project) { create(:project, :public, :repository) }
+  let_it_be(:group) { create(:group) }
+  let_it_be(:project) { create(:project, :public, :repository, group: group) }
 
   before do
     stub_licensed_features(multiple_approval_rules: true)

spec/frontend/api_spec.js

@@ -352,6 +352,20 @@ describe('Api', () => {
     });
   });
 
+  describe('projectGroups', () => {
+    it('fetches a project group', async () => {
+      const options = { unused: 'option' };
+      const projectId = 1;
+      const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/projects/${projectId}/groups.json`;
+      mock.onGet(expectedUrl, { params: options }).reply(httpStatus.OK, {
+        name: 'test',
+      });
+
+      const { name } = await Api.projectGroups(projectId, options);
+      expect(name).toBe('test');
+    });
+  });
+
   describe('projectUsers', () => {
     it('fetches all users of a particular project', (done) => {
       const query = 'dummy query';