Merge branch 'revert-a15a6f06' into 'master'

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

See merge request gitlab-org/gitlab!56600

app/assets/javascripts/api.js

@@ -31,7 +31,6 @@ 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',
@@ -242,20 +241,6 @@ 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,11 +135,13 @@ export default {
         .then((results) => ({ results }));
     },
     fetchGroups(term) {
-      const hasTerm = term.trim().length > 0;
+      // 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;
 
-      return Api.projectGroups(this.projectId, {
+      return Api.groups(term, {
         skip_groups: this.skipGroupIds,
-        ...(hasTerm ? { search: term } : {}),
+        all_available: includeAll,
       });
     },
     fetchUsers(term) {

ee/spec/features/merge_request/user_edits_approval_rules_mr_spec.rb

@@ -73,7 +73,11 @@ 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,12 +87,10 @@ 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.add_developer(user)
         group.add_developer(other_user)
 
-        visit project_new_merge_request_path(group_project, merge_request: { target_branch: 'master', source_branch: 'feature' })
+        visit project_new_merge_request_path(project, merge_request: { target_branch: 'master', source_branch: 'feature' })
+
         open_modal(text: 'Add approval rule')
         open_approver_select
 
@@ -158,13 +156,9 @@ 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(group_project, group_project_merge_request)
+        visit edit_project_merge_request_path(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, 'projectGroups').mockResolvedValue(TEST_GROUPS);
+    jest.spyOn(Api, 'groups').mockResolvedValue(TEST_GROUPS);
     jest.spyOn(Api, 'projectUsers').mockReturnValue(Promise.resolve(TEST_USERS));
   });
 
@@ -126,10 +126,7 @@ describe('Approvals ApproversSelect', () => {
     });
 
     it('fetches all available groups', () => {
-      expect(Api.projectGroups).toHaveBeenCalledWith(TEST_PROJECT_ID, {
-        skip_groups: [],
-        search: term,
-      });
+      expect(Api.groups).toHaveBeenCalledWith(term, { skip_groups: [], all_available: true });
     });
 
     it('fetches users', () => {
@@ -160,8 +157,9 @@ describe('Approvals ApproversSelect', () => {
     });
 
     it('skips groups and does not fetch all available', () => {
-      expect(Api.projectGroups).toHaveBeenCalledWith(TEST_PROJECT_ID, {
+      expect(Api.groups).toHaveBeenCalledWith('', {
         skip_groups: skipGroupIds,
+        all_available: false,
       });
     });
 

ee/spec/support/shared_contexts/project_approval_rules_shared_context.rb

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

spec/frontend/api_spec.js

@@ -352,20 +352,6 @@ 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';