diff options
Diffstat (limited to 'spec/services/groups')
4 files changed, 185 insertions, 176 deletions
diff --git a/spec/services/groups/group_links/create_service_spec.rb b/spec/services/groups/group_links/create_service_spec.rb index 03dac14be54..bfbaedbd06f 100644 --- a/spec/services/groups/group_links/create_service_spec.rb +++ b/spec/services/groups/group_links/create_service_spec.rb @@ -3,23 +3,13 @@ require 'spec_helper' RSpec.describe Groups::GroupLinks::CreateService, '#execute' do - let(:parent_group_user) { create(:user) } - let(:group_user) { create(:user) } - let(:child_group_user) { create(:user) } - let(:prevent_sharing) { false } + let_it_be(:shared_with_group_parent) { create(:group, :private) } + let_it_be(:shared_with_group) { create(:group, :private, parent: shared_with_group_parent) } + let_it_be(:shared_with_group_child) { create(:group, :private, parent: shared_with_group) } let_it_be(:group_parent) { create(:group, :private) } - let_it_be(:group) { create(:group, :private, parent: group_parent) } - let_it_be(:group_child) { create(:group, :private, parent: group) } - let(:ns_for_parent) { create(:namespace_settings, prevent_sharing_groups_outside_hierarchy: prevent_sharing) } - let(:shared_group_parent) { create(:group, :private, namespace_settings: ns_for_parent) } - let(:shared_group) { create(:group, :private, parent: shared_group_parent) } - let(:shared_group_child) { create(:group, :private, parent: shared_group) } - - let(:project_parent) { create(:project, group: shared_group_parent) } - let(:project) { create(:project, group: shared_group) } - let(:project_child) { create(:project, group: shared_group_child) } + let(:group) { create(:group, :private, parent: group_parent) } let(:opts) do { @@ -28,127 +18,161 @@ RSpec.describe Groups::GroupLinks::CreateService, '#execute' do } end - let(:user) { group_user } + subject { described_class.new(group, shared_with_group, user, opts) } - subject { described_class.new(shared_group, group, user, opts) } + shared_examples_for 'not shareable' do + it 'does not share and returns an error' do + expect do + result = subject.execute - before do - group.add_guest(group_user) - shared_group.add_owner(group_user) + expect(result[:status]).to eq(:error) + expect(result[:http_status]).to eq(404) + end.not_to change { group.shared_with_group_links.count } + end end - it 'adds group to another group' do - expect { subject.execute }.to change { group.shared_group_links.count }.from(0).to(1) - end + shared_examples_for 'shareable' do + it 'adds group to another group' do + expect do + result = subject.execute - it 'returns false if shared group is blank' do - expect { described_class.new(nil, group, user, opts) }.not_to change { group.shared_group_links.count } + expect(result[:status]).to eq(:success) + end.to change { group.shared_with_group_links.count }.from(0).to(1) + end end - context 'user does not have access to group' do - let(:user) { create(:user) } - - before do - shared_group.add_owner(user) - end + context 'when user has proper membership to share a group' do + let_it_be(:group_user) { create(:user) } - it 'returns error' do - result = subject.execute + let(:user) { group_user } - expect(result[:status]).to eq(:error) - expect(result[:http_status]).to eq(404) + before do + shared_with_group.add_guest(group_user) + group.add_owner(group_user) end - end - context 'user does not have admin access to shared group' do - let(:user) { create(:user) } + it_behaves_like 'shareable' - before do - group.add_guest(user) - shared_group.add_developer(user) - end + context 'when sharing outside the hierarchy is disabled' do + let_it_be(:group_parent) do + create(:group, + namespace_settings: create(:namespace_settings, prevent_sharing_groups_outside_hierarchy: true)) + end - it 'returns error' do - result = subject.execute + it_behaves_like 'not shareable' - expect(result[:status]).to eq(:error) - expect(result[:http_status]).to eq(404) - end - end + context 'when group is inside hierarchy' do + let(:shared_with_group) { create(:group, :private, parent: group_parent) } - context 'project authorizations based on group hierarchies' do - before do - group_parent.add_owner(parent_group_user) - group.add_owner(group_user) - group_child.add_owner(child_group_user) + it_behaves_like 'shareable' + end end - context 'project authorizations refresh' do - it 'is executed only for the direct members of the group' do - expect(UserProjectAccessChangedService).to receive(:new).with(contain_exactly(group_user.id)).and_call_original + context 'project authorizations based on group hierarchies' do + let_it_be(:child_group_user) { create(:user) } + let_it_be(:parent_group_user) { create(:user) } - subject.execute + before do + shared_with_group_parent.add_owner(parent_group_user) + shared_with_group.add_owner(group_user) + shared_with_group_child.add_owner(child_group_user) end - end - context 'project authorizations' do - context 'group user' do - let(:user) { group_user } + context 'project authorizations refresh' do + it 'is executed only for the direct members of the group' do + expect(UserProjectAccessChangedService).to receive(:new).with(contain_exactly(group_user.id)) + .and_call_original - it 'create proper authorizations' do subject.execute - - expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey - expect(Ability.allowed?(user, :read_project, project)).to be_truthy - expect(Ability.allowed?(user, :read_project, project_child)).to be_truthy end end - context 'parent group user' do - let(:user) { parent_group_user } + context 'project authorizations' do + let(:group_child) { create(:group, :private, parent: group) } + let(:project_parent) { create(:project, group: group_parent) } + let(:project) { create(:project, group: group) } + let(:project_child) { create(:project, group: group_child) } - it 'create proper authorizations' do - subject.execute + context 'group user' do + let(:user) { group_user } + + it 'create proper authorizations' do + subject.execute - expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey - expect(Ability.allowed?(user, :read_project, project)).to be_falsey - expect(Ability.allowed?(user, :read_project, project_child)).to be_falsey + expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey + expect(Ability.allowed?(user, :read_project, project)).to be_truthy + expect(Ability.allowed?(user, :read_project, project_child)).to be_truthy + end end - end - context 'child group user' do - let(:user) { child_group_user } + context 'parent group user' do + let(:user) { parent_group_user } - it 'create proper authorizations' do - subject.execute + it 'create proper authorizations' do + subject.execute + + expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey + expect(Ability.allowed?(user, :read_project, project)).to be_falsey + expect(Ability.allowed?(user, :read_project, project_child)).to be_falsey + end + end - expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey - expect(Ability.allowed?(user, :read_project, project)).to be_falsey - expect(Ability.allowed?(user, :read_project, project_child)).to be_falsey + context 'child group user' do + let(:user) { child_group_user } + + it 'create proper authorizations' do + subject.execute + + expect(Ability.allowed?(user, :read_project, project_parent)).to be_falsey + expect(Ability.allowed?(user, :read_project, project)).to be_falsey + expect(Ability.allowed?(user, :read_project, project_child)).to be_falsey + end end end end end - context 'sharing outside the hierarchy is disabled' do - let(:prevent_sharing) { true } + context 'user does not have access to group' do + let(:user) { create(:user) } - it 'prevents sharing with a group outside the hierarchy' do - result = subject.execute + before do + group.add_owner(user) + end - expect(group.reload.shared_group_links.count).to eq(0) - expect(result[:status]).to eq(:error) - expect(result[:http_status]).to eq(404) + it_behaves_like 'not shareable' + end + + context 'user does not have admin access to shared group' do + let(:user) { create(:user) } + + before do + shared_with_group.add_guest(user) + group.add_developer(user) end - it 'allows sharing with a group within the hierarchy' do - sibling_group = create(:group, :private, parent: shared_group_parent) - sibling_group.add_guest(group_user) + it_behaves_like 'not shareable' + end + + context 'when group is blank' do + let(:group_user) { create(:user) } + let(:user) { group_user } + let(:group) { nil } - result = described_class.new(shared_group, sibling_group, user, opts).execute + it 'does not share and returns an error' do + expect do + result = subject.execute - expect(sibling_group.reload.shared_group_links.count).to eq(1) - expect(result[:status]).to eq(:success) + expect(result[:status]).to eq(:error) + expect(result[:http_status]).to eq(404) + end.not_to change { shared_with_group.shared_group_links.count } end end + + context 'when shared_with_group is blank' do + let(:group_user) { create(:user) } + let(:user) { group_user } + let(:shared_with_group) { nil } + + it_behaves_like 'not shareable' + end end diff --git a/spec/services/groups/group_links/destroy_service_spec.rb b/spec/services/groups/group_links/destroy_service_spec.rb index e63adc07313..6aaf5f45069 100644 --- a/spec/services/groups/group_links/destroy_service_spec.rb +++ b/spec/services/groups/group_links/destroy_service_spec.rb @@ -3,54 +3,77 @@ require 'spec_helper' RSpec.describe Groups::GroupLinks::DestroyService, '#execute' do - let(:user) { create(:user) } - + let_it_be(:user) { create(:user) } let_it_be(:group) { create(:group, :private) } let_it_be(:shared_group) { create(:group, :private) } let_it_be(:project) { create(:project, group: shared_group) } let_it_be(:owner) { create(:user) } - before do - group.add_developer(owner) - shared_group.add_owner(owner) - end - subject { described_class.new(shared_group, owner) } - context 'single link' do - let!(:link) { create(:group_group_link, shared_group: shared_group, shared_with_group: group) } + context 'when authorizing by user' do + before do + group.add_developer(owner) + shared_group.add_owner(owner) + end + + context 'single link' do + let!(:link) { create(:group_group_link, shared_group: shared_group, shared_with_group: group) } - it 'destroys link' do - expect { subject.execute(link) }.to change { shared_group.shared_with_group_links.count }.from(1).to(0) + it 'destroys the link' do + expect { subject.execute(link) }.to change { shared_group.shared_with_group_links.count }.from(1).to(0) + end + + it 'revokes project authorization', :sidekiq_inline do + group.add_developer(user) + + expect { subject.execute(link) }.to( + change { Ability.allowed?(user, :read_project, project) }.from(true).to(false)) + end end - it 'revokes project authorization', :sidekiq_inline do - group.add_developer(user) + context 'multiple links' do + let_it_be(:another_group) { create(:group, :private) } + let_it_be(:another_shared_group) { create(:group, :private) } + + let!(:links) do + [ + create(:group_group_link, shared_group: shared_group, shared_with_group: group), + create(:group_group_link, shared_group: shared_group, shared_with_group: another_group), + create(:group_group_link, shared_group: another_shared_group, shared_with_group: group), + create(:group_group_link, shared_group: another_shared_group, shared_with_group: another_group) + ] + end - expect { subject.execute(link) }.to( - change { Ability.allowed?(user, :read_project, project) }.from(true).to(false)) + it 'updates project authorization once per group' do + expect(GroupGroupLink).to receive(:delete).and_call_original + expect(group).to receive(:refresh_members_authorized_projects).with(direct_members_only: true, blocking: false).once + expect(another_group).to receive(:refresh_members_authorized_projects).with(direct_members_only: true, blocking: false).once + + subject.execute(links) + end end end - context 'multiple links' do - let_it_be(:another_group) { create(:group, :private) } - let_it_be(:another_shared_group) { create(:group, :private) } - - let!(:links) do - [ - create(:group_group_link, shared_group: shared_group, shared_with_group: group), - create(:group_group_link, shared_group: shared_group, shared_with_group: another_group), - create(:group_group_link, shared_group: another_shared_group, shared_with_group: group), - create(:group_group_link, shared_group: another_shared_group, shared_with_group: another_group) - ] + context 'when skipping authorization' do + let!(:link) { create(:group_group_link, shared_group: shared_group, shared_with_group: group) } + + context 'with provided group and owner' do + it 'destroys the link' do + expect do + subject.execute(link, skip_authorization: true) + end.to change { shared_group.shared_with_group_links.count }.from(1).to(0) + end end - it 'updates project authorization once per group' do - expect(GroupGroupLink).to receive(:delete).and_call_original - expect(group).to receive(:refresh_members_authorized_projects).with(direct_members_only: true, blocking: false).once - expect(another_group).to receive(:refresh_members_authorized_projects).with(direct_members_only: true, blocking: false).once + context 'without providing group or owner' do + subject { described_class.new(nil, nil) } - subject.execute(links) + it 'destroys the link' do + expect do + subject.execute(link, skip_authorization: true) + end.to change { shared_group.shared_with_group_links.count }.from(1).to(0) + end end end end diff --git a/spec/services/groups/open_issues_count_service_spec.rb b/spec/services/groups/open_issues_count_service_spec.rb index 7dd8c2a59a0..fca09bfdebe 100644 --- a/spec/services/groups/open_issues_count_service_spec.rb +++ b/spec/services/groups/open_issues_count_service_spec.rb @@ -3,18 +3,12 @@ require 'spec_helper' RSpec.describe Groups::OpenIssuesCountService, :use_clean_rails_memory_store_caching do - let_it_be(:group) { create(:group, :public) } + let_it_be(:group) { create(:group, :public)} let_it_be(:project) { create(:project, :public, namespace: group) } - let_it_be(:admin) { create(:user, :admin) } let_it_be(:user) { create(:user) } - let_it_be(:banned_user) { create(:user, :banned) } - - before do - create(:issue, :opened, project: project) - create(:issue, :opened, confidential: true, project: project) - create(:issue, :opened, author: banned_user, project: project) - create(:issue, :closed, project: project) - end + let_it_be(:issue) { create(:issue, :opened, project: project) } + let_it_be(:confidential) { create(:issue, :opened, confidential: true, project: project) } + let_it_be(:closed) { create(:issue, :closed, project: project) } subject { described_class.new(group, user) } @@ -26,27 +20,17 @@ RSpec.describe Groups::OpenIssuesCountService, :use_clean_rails_memory_store_cac it 'uses the IssuesFinder to scope issues' do expect(IssuesFinder) .to receive(:new) - .with(user, group_id: group.id, state: 'opened', non_archived: true, include_subgroups: true, public_only: true, include_hidden: false) + .with(user, group_id: group.id, state: 'opened', non_archived: true, include_subgroups: true, public_only: true) subject.count end end describe '#count' do - shared_examples 'counts public issues, does not count hidden or confidential' do - it 'counts only public issues' do - expect(subject.count).to eq(1) - end - - it 'uses PUBLIC_COUNT_WITHOUT_HIDDEN_KEY cache key' do - expect(subject.cache_key).to include('group_open_public_issues_without_hidden_count') - end - end - context 'when user is nil' do - let(:user) { nil } - - it_behaves_like 'counts public issues, does not count hidden or confidential' + it 'does not include confidential issues in the issue count' do + expect(described_class.new(group).count).to eq(1) + end end context 'when user is provided' do @@ -55,13 +39,9 @@ RSpec.describe Groups::OpenIssuesCountService, :use_clean_rails_memory_store_cac group.add_reporter(user) end - it 'includes confidential issues and does not include hidden issues in count' do + it 'returns the right count with confidential issues' do expect(subject.count).to eq(2) end - - it 'uses TOTAL_COUNT_WITHOUT_HIDDEN_KEY cache key' do - expect(subject.cache_key).to include('group_open_issues_without_hidden_count') - end end context 'when user cannot read confidential issues' do @@ -69,24 +49,8 @@ RSpec.describe Groups::OpenIssuesCountService, :use_clean_rails_memory_store_cac group.add_guest(user) end - it_behaves_like 'counts public issues, does not count hidden or confidential' - end - - context 'when user is an admin' do - let(:user) { admin } - - context 'when admin mode is enabled', :enable_admin_mode do - it 'includes confidential and hidden issues in count' do - expect(subject.count).to eq(3) - end - - it 'uses TOTAL_COUNT_KEY cache key' do - expect(subject.cache_key).to include('group_open_issues_including_hidden_count') - end - end - - context 'when admin mode is disabled' do - it_behaves_like 'counts public issues, does not count hidden or confidential' + it 'does not include confidential issues' do + expect(subject.count).to eq(1) end end @@ -97,13 +61,11 @@ RSpec.describe Groups::OpenIssuesCountService, :use_clean_rails_memory_store_cac describe '#clear_all_cache_keys' do it 'calls `Rails.cache.delete` with the correct keys' do expect(Rails.cache).to receive(:delete) - .with(['groups', 'open_issues_count_service', 1, group.id, described_class::PUBLIC_COUNT_WITHOUT_HIDDEN_KEY]) + .with(['groups', 'open_issues_count_service', 1, group.id, described_class::PUBLIC_COUNT_KEY]) expect(Rails.cache).to receive(:delete) .with(['groups', 'open_issues_count_service', 1, group.id, described_class::TOTAL_COUNT_KEY]) - expect(Rails.cache).to receive(:delete) - .with(['groups', 'open_issues_count_service', 1, group.id, described_class::TOTAL_COUNT_WITHOUT_HIDDEN_KEY]) - described_class.new(group).clear_all_cache_keys + subject.clear_all_cache_keys end end end diff --git a/spec/services/groups/transfer_service_spec.rb b/spec/services/groups/transfer_service_spec.rb index 1c4b7aac87e..20ea8b2bf1b 100644 --- a/spec/services/groups/transfer_service_spec.rb +++ b/spec/services/groups/transfer_service_spec.rb @@ -574,7 +574,7 @@ RSpec.describe Groups::TransferService, :sidekiq_inline do context 'resets project authorizations' do let_it_be(:old_parent_group) { create(:group) } - let_it_be_with_reload(:group) { create(:group, :private, parent: old_parent_group) } + let_it_be_with_refind(:group) { create(:group, :private, parent: old_parent_group) } let_it_be(:new_group_member) { create(:user) } let_it_be(:old_group_member) { create(:user) } let_it_be(:unique_subgroup_member) { create(:user) } |