Commit 7b2a4941 authored by Doug Stull's avatar Doug Stull

Allow admins to invite groups they are not a member of for group

- this was mistakenly changed to affect groups and should have
  only affected projects.  This fixes that issue and restores
  the ability.

Changelog: fixed
parent f0c53b2b
...@@ -24,10 +24,6 @@ export default { ...@@ -24,10 +24,6 @@ export default {
prop: 'selectedGroup', prop: 'selectedGroup',
}, },
props: { props: {
accessLevels: {
type: Object,
required: true,
},
groupsFilter: { groupsFilter: {
type: String, type: String,
required: false, required: false,
...@@ -58,13 +54,6 @@ export default { ...@@ -58,13 +54,6 @@ export default {
isFetchResultEmpty() { isFetchResultEmpty() {
return this.groups.length === 0; return this.groups.length === 0;
}, },
defaultFetchOptions() {
return {
exclude_internal: true,
active: true,
min_access_level: this.accessLevels.Guest,
};
},
}, },
watch: { watch: {
searchTerm() { searchTerm() {
...@@ -107,9 +96,13 @@ export default { ...@@ -107,9 +96,13 @@ export default {
fetchGroups() { fetchGroups() {
switch (this.groupsFilter) { switch (this.groupsFilter) {
case GROUP_FILTERS.DESCENDANT_GROUPS: case GROUP_FILTERS.DESCENDANT_GROUPS:
return getDescendentGroups(this.parentGroupId, this.searchTerm, this.defaultFetchOptions); return getDescendentGroups(
this.parentGroupId,
this.searchTerm,
this.$options.defaultFetchOptions,
);
default: default:
return getGroups(this.searchTerm, this.defaultFetchOptions); return getGroups(this.searchTerm, this.$options.defaultFetchOptions);
} }
}, },
}, },
...@@ -118,6 +111,10 @@ export default { ...@@ -118,6 +111,10 @@ export default {
searchPlaceholder: s__('GroupSelect|Search groups'), searchPlaceholder: s__('GroupSelect|Search groups'),
emptySearchResult: s__('GroupSelect|No matching results'), emptySearchResult: s__('GroupSelect|No matching results'),
}, },
defaultFetchOptions: {
exclude_internal: true,
active: true,
},
}; };
</script> </script>
<template> <template>
......
...@@ -155,7 +155,6 @@ export default { ...@@ -155,7 +155,6 @@ export default {
<template #select> <template #select>
<group-select <group-select
v-model="groupToBeSharedWith" v-model="groupToBeSharedWith"
:access-levels="accessLevels"
:groups-filter="groupSelectFilter" :groups-filter="groupSelectFilter"
:parent-group-id="groupSelectParentId" :parent-group-id="groupSelectParentId"
:invalid-groups="invalidGroups" :invalid-groups="invalidGroups"
......
...@@ -3,7 +3,6 @@ ...@@ -3,7 +3,6 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe 'Groups > Members > Manage groups', :js do RSpec.describe 'Groups > Members > Manage groups', :js do
include Select2Helper
include Spec::Support::Helpers::Features::MembersHelpers include Spec::Support::Helpers::Features::MembersHelpers
include Spec::Support::Helpers::Features::InviteMembersModalHelper include Spec::Support::Helpers::Features::InviteMembersModalHelper
include Spec::Support::Helpers::ModalHelpers include Spec::Support::Helpers::ModalHelpers
...@@ -119,16 +118,92 @@ RSpec.describe 'Groups > Members > Manage groups', :js do ...@@ -119,16 +118,92 @@ RSpec.describe 'Groups > Members > Manage groups', :js do
describe 'group search results' do describe 'group search results' do
let_it_be(:group, refind: true) { create(:group) } let_it_be(:group, refind: true) { create(:group) }
let_it_be(:group_within_hierarchy) { create(:group, parent: group) }
let_it_be(:group_outside_hierarchy) { create(:group) }
before_all do context 'with instance admin considerations' do
group.add_owner(user) let_it_be(:group_to_share) { create(:group) }
group_within_hierarchy.add_owner(user)
group_outside_hierarchy.add_owner(user) context 'when user is an admin' do
let_it_be(:admin) { create(:admin) }
before do
sign_in(admin)
gitlab_enable_admin_mode_sign_in(admin)
end
it 'shows groups where the admin has no direct membership' do
visit group_group_members_path(group)
click_on 'Invite a group'
click_on 'Select a group'
wait_for_requests
page.within(group_dropdown_selector) do
expect_to_have_group(group_to_share)
expect_not_to_have_group(group)
end
end
it 'shows groups where the admin has at least guest level membership' do
group_to_share.add_guest(admin)
visit group_group_members_path(group)
click_on 'Invite a group'
click_on 'Select a group'
wait_for_requests
page.within(group_dropdown_selector) do
expect_to_have_group(group_to_share)
expect_not_to_have_group(group)
end
end
end
context 'when user is not an admin' do
before do
group.add_owner(user)
end
it 'shows groups where the user has no direct membership' do
visit group_group_members_path(group)
click_on 'Invite a group'
click_on 'Select a group'
wait_for_requests
page.within(group_dropdown_selector) do
expect_not_to_have_group(group_to_share)
expect_not_to_have_group(group)
end
end
it 'shows groups where the user has at least guest level membership' do
group_to_share.add_guest(user)
visit group_group_members_path(group)
click_on 'Invite a group'
click_on 'Select a group'
wait_for_requests
page.within(group_dropdown_selector) do
expect_to_have_group(group_to_share)
expect_not_to_have_group(group)
end
end
end
end end
context 'when the invite members group modal is enabled' do context 'when user is not an admin and there are hierarchy considerations' do
let_it_be(:group_within_hierarchy) { create(:group, parent: group) }
let_it_be(:group_outside_hierarchy) { create(:group) }
before_all do
group.add_owner(user)
group_within_hierarchy.add_owner(user)
group_outside_hierarchy.add_owner(user)
end
it 'does not show self or ancestors', :aggregate_failures do it 'does not show self or ancestors', :aggregate_failures do
group_sibbling = create(:group, parent: group) group_sibbling = create(:group, parent: group)
group_sibbling.add_owner(user) group_sibbling.add_owner(user)
...@@ -139,40 +214,45 @@ RSpec.describe 'Groups > Members > Manage groups', :js do ...@@ -139,40 +214,45 @@ RSpec.describe 'Groups > Members > Manage groups', :js do
click_on 'Select a group' click_on 'Select a group'
wait_for_requests wait_for_requests
page.within('[data-testid="group-select-dropdown"]') do page.within(group_dropdown_selector) do
expect(page).to have_selector("[entity-id='#{group_outside_hierarchy.id}']") expect_to_have_group(group_outside_hierarchy)
expect(page).to have_selector("[entity-id='#{group_sibbling.id}']") expect_to_have_group(group_sibbling)
expect(page).not_to have_selector("[entity-id='#{group.id}']") expect_not_to_have_group(group)
expect(page).not_to have_selector("[entity-id='#{group_within_hierarchy.id}']") expect_not_to_have_group(group_within_hierarchy)
end end
end end
end
context 'when sharing with groups outside the hierarchy is enabled' do context 'when sharing with groups outside the hierarchy is enabled' do
it 'shows groups within and outside the hierarchy in search results' do it 'shows groups within and outside the hierarchy in search results' do
visit group_group_members_path(group) visit group_group_members_path(group)
click_on 'Invite a group' click_on 'Invite a group'
click_on 'Select a group' click_on 'Select a group'
wait_for_requests
expect(page).to have_text group_within_hierarchy.name page.within(group_dropdown_selector) do
expect(page).to have_text group_outside_hierarchy.name expect_to_have_group(group_within_hierarchy)
expect_to_have_group(group_outside_hierarchy)
end
end
end end
end
context 'when sharing with groups outside the hierarchy is disabled' do context 'when sharing with groups outside the hierarchy is disabled' do
before do before do
group.namespace_settings.update!(prevent_sharing_groups_outside_hierarchy: true) group.namespace_settings.update!(prevent_sharing_groups_outside_hierarchy: true)
end end
it 'shows only groups within the hierarchy in search results' do it 'shows only groups within the hierarchy in search results' do
visit group_group_members_path(group) visit group_group_members_path(group)
click_on 'Invite a group' click_on 'Invite a group'
click_on 'Select a group' click_on 'Select a group'
expect(page).to have_text group_within_hierarchy.name page.within(group_dropdown_selector) do
expect(page).not_to have_text group_outside_hierarchy.name expect_to_have_group(group_within_hierarchy)
expect_not_to_have_group(group_outside_hierarchy)
end
end
end end
end end
end end
......
...@@ -17,20 +17,18 @@ RSpec.describe 'Project > Members > Invite group', :js do ...@@ -17,20 +17,18 @@ RSpec.describe 'Project > Members > Invite group', :js do
visit project_project_members_path(project) visit project_project_members_path(project)
expect(page).to have_selector('button[data-test-id="invite-group-button"]') expect(page).to have_selector(invite_group_selector)
end end
it 'does not display the button when visiting the page not signed in' do it 'does not display the button when visiting the page not signed in' do
project = create(:project, namespace: create(:group)) project = create(:project, namespace: create(:group))
visit project_project_members_path(project) visit project_project_members_path(project)
expect(page).not_to have_selector('button[data-test-id="invite-group-button"]') expect(page).not_to have_selector(invite_group_selector)
end end
describe 'Share with group lock' do describe 'Share with group lock' do
let(:invite_group_selector) { 'button[data-test-id="invite-group-button"]' }
shared_examples 'the project can be shared with groups' do shared_examples 'the project can be shared with groups' do
it 'the "Invite a group" button exists' do it 'the "Invite a group" button exists' do
visit project_project_members_path(project) visit project_project_members_path(project)
...@@ -158,21 +156,95 @@ RSpec.describe 'Project > Members > Invite group', :js do ...@@ -158,21 +156,95 @@ RSpec.describe 'Project > Members > Invite group', :js do
describe 'the groups dropdown' do describe 'the groups dropdown' do
let_it_be(:parent_group) { create(:group, :public) } let_it_be(:parent_group) { create(:group, :public) }
let_it_be(:project_group) { create(:group, :public, parent: parent_group) } let_it_be(:project_group) { create(:group, :public, parent: parent_group) }
let_it_be(:public_sub_subgroup) { create(:group, :public, parent: project_group) }
let_it_be(:public_sibbling_group) { create(:group, :public, parent: parent_group) }
let_it_be(:private_sibbling_group) { create(:group, :private, parent: parent_group) }
let_it_be(:private_membership_group) { create(:group, :private) }
let_it_be(:public_membership_group) { create(:group, :public) }
let_it_be(:project) { create(:project, group: project_group) } let_it_be(:project) { create(:project, group: project_group) }
before do context 'with instance admin considerations' do
private_membership_group.add_guest(maintainer) let_it_be(:group_to_share) { create(:group) }
public_membership_group.add_maintainer(maintainer)
sign_in(maintainer) context 'when user is an admin' do
let_it_be(:admin) { create(:admin) }
before do
sign_in(admin)
gitlab_enable_admin_mode_sign_in(admin)
end
it 'shows groups where the admin has no direct membership' do
visit project_project_members_path(project)
click_on 'Invite a group'
click_on 'Select a group'
wait_for_requests
page.within(group_dropdown_selector) do
expect_to_have_group(group_to_share)
end
end
it 'shows groups where the admin has at least guest level membership' do
group_to_share.add_guest(admin)
visit project_project_members_path(project)
click_on 'Invite a group'
click_on 'Select a group'
wait_for_requests
page.within(group_dropdown_selector) do
expect_to_have_group(group_to_share)
end
end
end
context 'when user is not an admin' do
before do
project.add_maintainer(maintainer)
sign_in(maintainer)
end
it 'does not show groups where the user has no direct membership' do
visit project_project_members_path(project)
click_on 'Invite a group'
click_on 'Select a group'
wait_for_requests
page.within(group_dropdown_selector) do
expect_not_to_have_group(group_to_share)
end
end
it 'shows groups where the user has at least guest level membership' do
group_to_share.add_guest(maintainer)
visit project_project_members_path(project)
click_on 'Invite a group'
click_on 'Select a group'
wait_for_requests
page.within(group_dropdown_selector) do
expect_to_have_group(group_to_share)
end
end
end
end end
context 'for a project in a nested group' do context 'for a project in a nested group' do
let_it_be(:public_sub_subgroup) { create(:group, :public, parent: project_group) }
let_it_be(:public_sibbling_group) { create(:group, :public, parent: parent_group) }
let_it_be(:private_sibbling_group) { create(:group, :private, parent: parent_group) }
let_it_be(:private_membership_group) { create(:group, :private) }
let_it_be(:public_membership_group) { create(:group, :public) }
let_it_be(:project) { create(:project, group: project_group) }
before do
private_membership_group.add_guest(maintainer)
public_membership_group.add_maintainer(maintainer)
sign_in(maintainer)
end
it 'does not show the groups inherited from projects' do it 'does not show the groups inherited from projects' do
project.add_maintainer(maintainer) project.add_maintainer(maintainer)
public_sibbling_group.add_maintainer(maintainer) public_sibbling_group.add_maintainer(maintainer)
...@@ -183,7 +255,7 @@ RSpec.describe 'Project > Members > Invite group', :js do ...@@ -183,7 +255,7 @@ RSpec.describe 'Project > Members > Invite group', :js do
click_on 'Select a group' click_on 'Select a group'
wait_for_requests wait_for_requests
page.within('[data-testid="group-select-dropdown"]') do page.within(group_dropdown_selector) do
expect_to_have_group(public_membership_group) expect_to_have_group(public_membership_group)
expect_to_have_group(public_sibbling_group) expect_to_have_group(public_sibbling_group)
expect_to_have_group(private_membership_group) expect_to_have_group(private_membership_group)
...@@ -204,7 +276,7 @@ RSpec.describe 'Project > Members > Invite group', :js do ...@@ -204,7 +276,7 @@ RSpec.describe 'Project > Members > Invite group', :js do
click_on 'Select a group' click_on 'Select a group'
wait_for_requests wait_for_requests
page.within('[data-testid="group-select-dropdown"]') do page.within(group_dropdown_selector) do
expect_to_have_group(public_membership_group) expect_to_have_group(public_membership_group)
expect_to_have_group(public_sibbling_group) expect_to_have_group(public_sibbling_group)
expect_to_have_group(private_membership_group) expect_to_have_group(private_membership_group)
...@@ -215,14 +287,10 @@ RSpec.describe 'Project > Members > Invite group', :js do ...@@ -215,14 +287,10 @@ RSpec.describe 'Project > Members > Invite group', :js do
expect_not_to_have_group(project_group) expect_not_to_have_group(project_group)
end end
end end
def expect_to_have_group(group)
expect(page).to have_selector("[entity-id='#{group.id}']")
end
def expect_not_to_have_group(group)
expect(page).not_to have_selector("[entity-id='#{group.id}']")
end
end end
end end
def invite_group_selector
'button[data-test-id="invite-group-button"]'
end
end end
...@@ -4,7 +4,6 @@ import waitForPromises from 'helpers/wait_for_promises'; ...@@ -4,7 +4,6 @@ import waitForPromises from 'helpers/wait_for_promises';
import * as groupsApi from '~/api/groups_api'; import * as groupsApi from '~/api/groups_api';
import GroupSelect from '~/invite_members/components/group_select.vue'; import GroupSelect from '~/invite_members/components/group_select.vue';
const accessLevels = { Guest: 10, Reporter: 20, Developer: 30, Maintainer: 40, Owner: 50 };
const group1 = { id: 1, full_name: 'Group One', avatar_url: 'test' }; const group1 = { id: 1, full_name: 'Group One', avatar_url: 'test' };
const group2 = { id: 2, full_name: 'Group Two', avatar_url: 'test' }; const group2 = { id: 2, full_name: 'Group Two', avatar_url: 'test' };
const allGroups = [group1, group2]; const allGroups = [group1, group2];
...@@ -13,7 +12,6 @@ const createComponent = (props = {}) => { ...@@ -13,7 +12,6 @@ const createComponent = (props = {}) => {
return mount(GroupSelect, { return mount(GroupSelect, {
propsData: { propsData: {
invalidGroups: [], invalidGroups: [],
accessLevels,
...props, ...props,
}, },
}); });
...@@ -66,9 +64,8 @@ describe('GroupSelect', () => { ...@@ -66,9 +64,8 @@ describe('GroupSelect', () => {
resolveApiRequest({ data: allGroups }); resolveApiRequest({ data: allGroups });
expect(groupsApi.getGroups).toHaveBeenCalledWith(group1.name, { expect(groupsApi.getGroups).toHaveBeenCalledWith(group1.name, {
active: true,
exclude_internal: true, exclude_internal: true,
min_access_level: accessLevels.Guest, active: true,
}); });
}); });
......
...@@ -43,6 +43,18 @@ module Spec ...@@ -43,6 +43,18 @@ module Spec
fill_in 'YYYY-MM-DD', with: expires_at.strftime('%Y-%m-%d') if expires_at fill_in 'YYYY-MM-DD', with: expires_at.strftime('%Y-%m-%d') if expires_at
end end
def group_dropdown_selector
'[data-testid="group-select-dropdown"]'
end
def expect_to_have_group(group)
expect(page).to have_selector("[entity-id='#{group.id}']")
end
def expect_not_to_have_group(group)
expect(page).not_to have_selector("[entity-id='#{group.id}']")
end
end end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment