diff --git a/app/assets/javascripts/api.js b/app/assets/javascripts/api.js
index 48005787d81aab864c6036f1308f0380bae84b64..2b589b711632950fc853ae748055b2ceedae91e7 100644
--- a/app/assets/javascripts/api.js
+++ b/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));
diff --git a/ee/app/assets/javascripts/approvals/components/approvers_select.vue b/ee/app/assets/javascripts/approvals/components/approvers_select.vue
index fd2678c22b6b9155e5fdc1d4cf28cf34326cf46e..bd203f4f78236a25b9c7e5a09c623a5de3418ffd 100644
--- a/ee/app/assets/javascripts/approvals/components/approvers_select.vue
+++ b/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) {
diff --git a/ee/spec/features/merge_request/user_edits_approval_rules_mr_spec.rb b/ee/spec/features/merge_request/user_edits_approval_rules_mr_spec.rb
index 7d7a651f20eec3461eb3f6eea978d5347796b870..07ec86431fc7dc843683f7327f30275f96f3cab7 100644
--- a/ee/spec/features/merge_request/user_edits_approval_rules_mr_spec.rb
+++ b/ee/spec/features/merge_request/user_edits_approval_rules_mr_spec.rb
@@ -73,11 +73,7 @@ def add_approval_rule_member(type, name)
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
diff --git a/ee/spec/features/merge_request/user_sets_approvers_spec.rb b/ee/spec/features/merge_request/user_sets_approvers_spec.rb
index 09c8cc89fe7c08aaa4114c845bc32961997ccd48..1958ee0ffc3943f202afdc3a83629a774d5bbbda 100644
--- a/ee/spec/features/merge_request/user_sets_approvers_spec.rb
+++ b/ee/spec/features/merge_request/user_sets_approvers_spec.rb
@@ -87,10 +87,12 @@ def non_collapse_approval_rules
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 @@ def non_collapse_approval_rules
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
diff --git a/ee/spec/features/projects/settings/merge_requests_settings_spec.rb b/ee/spec/features/projects/settings/merge_requests_settings_spec.rb
index 4efb445239f831fb1414bd9ded3753270f38828d..b5e828a1a653dcda54b73a802b2aed20c7cba15d 100644
--- a/ee/spec/features/projects/settings/merge_requests_settings_spec.rb
+++ b/ee/spec/features/projects/settings/merge_requests_settings_spec.rb
@@ -6,8 +6,8 @@
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' }
diff --git a/ee/spec/frontend/approvals/components/approvers_select_spec.js b/ee/spec/frontend/approvals/components/approvers_select_spec.js
index ce210f631a94d865a3038a99e3c23905b42c75b6..c82e51f825cbd400f83edd68a997d98bd3f112a4 100644
--- a/ee/spec/frontend/approvals/components/approvers_select_spec.js
+++ b/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,
});
});
diff --git a/ee/spec/support/shared_contexts/project_approval_rules_shared_context.rb b/ee/spec/support/shared_contexts/project_approval_rules_shared_context.rb
index cfda0e9dd6838e6671433443ff98bf4ec0b7477b..24491e4c4ab90c72260081bf3e7d6220ee413995 100644
--- a/ee/spec/support/shared_contexts/project_approval_rules_shared_context.rb
+++ b/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)
diff --git a/spec/frontend/api_spec.js b/spec/frontend/api_spec.js
index d6e1b170dd3d3478744f818a6dadb9d97683cef1..a03aabf9e4fb26166c7b8930ad3657009984c639 100644
--- a/spec/frontend/api_spec.js
+++ b/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';