diff options
Diffstat (limited to 'spec/models/group_spec.rb')
| -rw-r--r-- | spec/models/group_spec.rb | 502 |
1 files changed, 380 insertions, 122 deletions
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 2f82d8a0bbe..5cc5c4d86d6 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -28,7 +28,7 @@ RSpec.describe Group do it { is_expected.to have_many(:container_repositories) } it { is_expected.to have_many(:milestones) } it { is_expected.to have_many(:group_deploy_keys) } - it { is_expected.to have_many(:services) } + it { is_expected.to have_many(:integrations) } it { is_expected.to have_one(:dependency_proxy_setting) } it { is_expected.to have_many(:dependency_proxy_blobs) } it { is_expected.to have_many(:dependency_proxy_manifests) } @@ -395,18 +395,94 @@ RSpec.describe Group do end end - context 'assigning a new parent' do - let!(:old_parent) { create(:group) } - let!(:new_parent) { create(:group) } + context 'assign a new parent' do let!(:group) { create(:group, parent: old_parent) } + let(:recorded_queries) { ActiveRecord::QueryRecorder.new } + + subject do + recorded_queries.record do + group.update(parent: new_parent) + end + end before do - group.update(parent: new_parent) + subject reload_models(old_parent, new_parent, group) end - it 'updates traversal_ids' do - expect(group.traversal_ids).to eq [new_parent.id, group.id] + context 'within the same hierarchy' do + let!(:root) { create(:group).reload } + let!(:old_parent) { create(:group, parent: root) } + let!(:new_parent) { create(:group, parent: root) } + + it 'updates traversal_ids' do + expect(group.traversal_ids).to eq [root.id, new_parent.id, group.id] + end + + it_behaves_like 'hierarchy with traversal_ids' + it_behaves_like 'locked row' do + let(:row) { root } + end + end + + context 'to another hierarchy' do + let!(:old_parent) { create(:group) } + let!(:new_parent) { create(:group) } + let!(:group) { create(:group, parent: old_parent) } + + it 'updates traversal_ids' do + expect(group.traversal_ids).to eq [new_parent.id, group.id] + end + + it_behaves_like 'locked rows' do + let(:rows) { [old_parent, new_parent] } + end + + context 'old hierarchy' do + let(:root) { old_parent.root_ancestor } + + it_behaves_like 'hierarchy with traversal_ids' + end + + context 'new hierarchy' do + let(:root) { new_parent.root_ancestor } + + it_behaves_like 'hierarchy with traversal_ids' + end + end + + context 'from being a root ancestor' do + let!(:old_parent) { nil } + let!(:new_parent) { create(:group) } + + it 'updates traversal_ids' do + expect(group.traversal_ids).to eq [new_parent.id, group.id] + end + + it_behaves_like 'locked rows' do + let(:rows) { [group, new_parent] } + end + + it_behaves_like 'hierarchy with traversal_ids' do + let(:root) { new_parent } + end + end + + context 'to being a root ancestor' do + let!(:old_parent) { create(:group) } + let!(:new_parent) { nil } + + it 'updates traversal_ids' do + expect(group.traversal_ids).to eq [group.id] + end + + it_behaves_like 'locked rows' do + let(:rows) { [old_parent, group] } + end + + it_behaves_like 'hierarchy with traversal_ids' do + let(:root) { group } + end end end @@ -427,6 +503,58 @@ RSpec.describe Group do end end + context 'traversal queries' do + let_it_be(:group, reload: true) { create(:group, :nested) } + + context 'recursive' do + before do + stub_feature_flags(use_traversal_ids: false) + end + + it_behaves_like 'namespace traversal' + + describe '#self_and_descendants' do + it { expect(group.self_and_descendants.to_sql).not_to include 'traversal_ids @>' } + end + + describe '#descendants' do + it { expect(group.descendants.to_sql).not_to include 'traversal_ids @>' } + end + + describe '#ancestors' do + it { expect(group.ancestors.to_sql).not_to include 'traversal_ids <@' } + end + end + + context 'linear' do + it_behaves_like 'namespace traversal' + + describe '#self_and_descendants' do + it { expect(group.self_and_descendants.to_sql).to include 'traversal_ids @>' } + end + + describe '#descendants' do + it { expect(group.descendants.to_sql).to include 'traversal_ids @>' } + end + + describe '#ancestors' do + it { expect(group.ancestors.to_sql).to include "\"namespaces\".\"id\" = #{group.parent_id}" } + + it 'hierarchy order' do + expect(group.ancestors(hierarchy_order: :asc).to_sql).to include 'ORDER BY "depth" ASC' + end + + context 'ancestor linear queries feature flag disabled' do + before do + stub_feature_flags(use_traversal_ids_for_ancestors: false) + end + + it { expect(group.ancestors.to_sql).not_to include 'traversal_ids <@' } + end + end + end + end + describe '.without_integration' do let(:another_group) { create(:group) } let(:instance_integration) { build(:jira_service, :instance) } @@ -504,6 +632,16 @@ RSpec.describe Group do it { is_expected.to match_array([private_group, internal_group]) } end + describe 'with_onboarding_progress' do + subject { described_class.with_onboarding_progress } + + it 'joins onboarding_progress' do + create(:onboarding_progress, namespace: group) + + expect(subject).to eq([group]) + end + end + describe 'for_authorized_group_members' do let_it_be(:group_member1) { create(:group_member, source: private_group, user_id: user1.id, access_level: Gitlab::Access::OWNER) } @@ -579,7 +717,9 @@ RSpec.describe Group do it "is false if avatar is html page" do group.update_attribute(:avatar, 'uploads/avatar.html') - expect(group.avatar_type).to eq(["file format is not supported. Please try one of the following supported formats: png, jpg, jpeg, gif, bmp, tiff, ico, webp"]) + group.avatar_type + + expect(group.errors.added?(:avatar, "file format is not supported. Please try one of the following supported formats: png, jpg, jpeg, gif, bmp, tiff, ico, webp")).to be true end end @@ -953,140 +1093,167 @@ RSpec.describe Group do it { expect(subject.parent).to be_kind_of(described_class) } end - describe '#max_member_access_for_user' do - context 'group shared with another group' do - let(:parent_group_user) { create(:user) } - let(:group_user) { create(:user) } - let(:child_group_user) { create(:user) } - - 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_it_be(:shared_group_parent) { create(:group, :private) } - let_it_be(:shared_group) { create(:group, :private, parent: shared_group_parent) } - let_it_be(:shared_group_child) { create(:group, :private, parent: shared_group) } - - before do - group_parent.add_owner(parent_group_user) - group.add_owner(group_user) - group_child.add_owner(child_group_user) - - create(:group_group_link, { shared_with_group: group, - shared_group: shared_group, - group_access: GroupMember::DEVELOPER }) - end + context "with member access" do + let_it_be(:group_user) { create(:user) } + describe '#max_member_access_for_user' do context 'with user in the group' do - let(:user) { group_user } + before do + group.add_owner(group_user) + end it 'returns correct access level' do - expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) - expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::DEVELOPER) - expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::DEVELOPER) + expect(group.max_member_access_for_user(group_user)).to eq(Gitlab::Access::OWNER) end + end - context 'with lower group access level than max access level for share' do - let(:user) { create(:user) } + context 'when user is nil' do + it 'returns NO_ACCESS' do + expect(group.max_member_access_for_user(nil)).to eq(Gitlab::Access::NO_ACCESS) + end + end - it 'returns correct access level' do - group.add_reporter(user) + context 'evaluating admin access level' do + let_it_be(:admin) { create(:admin) } - expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) - expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::REPORTER) - expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::REPORTER) + context 'when admin mode is enabled', :enable_admin_mode do + it 'returns OWNER by default' do + expect(group.max_member_access_for_user(admin)).to eq(Gitlab::Access::OWNER) end end - end - context 'with user in the parent group' do - let(:user) { parent_group_user } + context 'when admin mode is disabled' do + it 'returns NO_ACCESS' do + expect(group.max_member_access_for_user(admin)).to eq(Gitlab::Access::NO_ACCESS) + end + end - it 'returns correct access level' do - expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) - expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) - expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) + it 'returns NO_ACCESS when only concrete membership should be considered' do + expect(group.max_member_access_for_user(admin, only_concrete_membership: true)) + .to eq(Gitlab::Access::NO_ACCESS) end end - context 'with user in the child group' do - let(:user) { child_group_user } + context 'when max_access_for_group is set' do + let(:max_member_access) { 111 } - it 'returns correct access level' do - expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) - expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) - expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) + before do + group_user.max_access_for_group[group.id] = max_member_access end - end - - context 'unrelated project owner' do - let(:common_id) { [Project.maximum(:id).to_i, Namespace.maximum(:id).to_i].max + 999 } - let!(:group) { create(:group, id: common_id) } - let!(:unrelated_project) { create(:project, id: common_id) } - let(:user) { unrelated_project.owner } - it 'returns correct access level' do - expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) - expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) - expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) + it 'uses the cached value' do + expect(group.max_member_access_for_user(group_user)).to eq(max_member_access) end end + end - context 'user without accepted access request' do - let!(:user) { create(:user) } + describe '#max_member_access' do + context 'group shared with another group' do + let_it_be(:parent_group_user) { create(:user) } + let_it_be(:child_group_user) { create(:user) } + + 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_it_be(:shared_group_parent) { create(:group, :private) } + let_it_be(:shared_group) { create(:group, :private, parent: shared_group_parent) } + let_it_be(:shared_group_child) { create(:group, :private, parent: shared_group) } before do - create(:group_member, :developer, :access_request, user: user, group: group) + group_parent.add_owner(parent_group_user) + group.add_owner(group_user) + group_child.add_owner(child_group_user) + + create(:group_group_link, { shared_with_group: group, + shared_group: shared_group, + group_access: GroupMember::DEVELOPER }) end - it 'returns correct access level' do - expect(shared_group_parent.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) - expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) - expect(shared_group_child.max_member_access_for_user(user)).to eq(Gitlab::Access::NO_ACCESS) + context 'with user in the group' do + it 'returns correct access level' do + expect(shared_group_parent.max_member_access(group_user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group.max_member_access(group_user)).to eq(Gitlab::Access::DEVELOPER) + expect(shared_group_child.max_member_access(group_user)).to eq(Gitlab::Access::DEVELOPER) + end + + context 'with lower group access level than max access level for share' do + let(:user) { create(:user) } + + it 'returns correct access level' do + group.add_reporter(user) + + expect(shared_group_parent.max_member_access(user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group.max_member_access(user)).to eq(Gitlab::Access::REPORTER) + expect(shared_group_child.max_member_access(user)).to eq(Gitlab::Access::REPORTER) + end + end end - end - end - context 'multiple groups shared with group' do - let(:user) { create(:user) } - let(:group) { create(:group, :private) } - let(:shared_group_parent) { create(:group, :private) } - let(:shared_group) { create(:group, :private, parent: shared_group_parent) } + context 'with user in the parent group' do + it 'returns correct access level' do + expect(shared_group_parent.max_member_access(parent_group_user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group.max_member_access(parent_group_user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group_child.max_member_access(parent_group_user)).to eq(Gitlab::Access::NO_ACCESS) + end + end - before do - group.add_owner(user) + context 'with user in the child group' do + it 'returns correct access level' do + expect(shared_group_parent.max_member_access(child_group_user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group.max_member_access(child_group_user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group_child.max_member_access(child_group_user)).to eq(Gitlab::Access::NO_ACCESS) + end + end - create(:group_group_link, { shared_with_group: group, - shared_group: shared_group, - group_access: GroupMember::DEVELOPER }) - create(:group_group_link, { shared_with_group: group, - shared_group: shared_group_parent, - group_access: GroupMember::MAINTAINER }) - end + context 'unrelated project owner' do + let(:common_id) { [Project.maximum(:id).to_i, Namespace.maximum(:id).to_i].max + 999 } + let!(:group) { create(:group, id: common_id) } + let!(:unrelated_project) { create(:project, id: common_id) } + let(:user) { unrelated_project.owner } - it 'returns correct access level' do - expect(shared_group.max_member_access_for_user(user)).to eq(Gitlab::Access::MAINTAINER) - end - end + it 'returns correct access level' do + expect(shared_group_parent.max_member_access(user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group.max_member_access(user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group_child.max_member_access(user)).to eq(Gitlab::Access::NO_ACCESS) + end + end + + context 'user without accepted access request' do + let!(:user) { create(:user) } - context 'evaluating admin access level' do - let_it_be(:admin) { create(:admin) } + before do + create(:group_member, :developer, :access_request, user: user, group: group) + end - context 'when admin mode is enabled', :enable_admin_mode do - it 'returns OWNER by default' do - expect(group.max_member_access_for_user(admin)).to eq(Gitlab::Access::OWNER) + it 'returns correct access level' do + expect(shared_group_parent.max_member_access(user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group.max_member_access(user)).to eq(Gitlab::Access::NO_ACCESS) + expect(shared_group_child.max_member_access(user)).to eq(Gitlab::Access::NO_ACCESS) + end end end - context 'when admin mode is disabled' do - it 'returns NO_ACCESS' do - expect(group.max_member_access_for_user(admin)).to eq(Gitlab::Access::NO_ACCESS) + context 'multiple groups shared with group' do + let(:user) { create(:user) } + let(:group) { create(:group, :private) } + let(:shared_group_parent) { create(:group, :private) } + let(:shared_group) { create(:group, :private, parent: shared_group_parent) } + + before do + group.add_owner(user) + + create(:group_group_link, { shared_with_group: group, + shared_group: shared_group, + group_access: GroupMember::DEVELOPER }) + create(:group_group_link, { shared_with_group: group, + shared_group: shared_group_parent, + group_access: GroupMember::MAINTAINER }) end - end - it 'returns NO_ACCESS when only concrete membership should be considered' do - expect(group.max_member_access_for_user(admin, only_concrete_membership: true)) - .to eq(Gitlab::Access::NO_ACCESS) + it 'returns correct access level' do + expect(shared_group.max_member_access(user)).to eq(Gitlab::Access::MAINTAINER) + end end end end @@ -1118,7 +1285,7 @@ RSpec.describe Group do end end - describe '#members_with_parents' do + shared_examples_for 'members_with_parents' do let!(:group) { create(:group, :nested) } let!(:maintainer) { group.parent.add_user(create(:user), GroupMember::MAINTAINER) } let!(:developer) { group.add_user(create(:user), GroupMember::DEVELOPER) } @@ -1142,6 +1309,50 @@ RSpec.describe Group do end end + describe '#members_with_parents' do + it_behaves_like 'members_with_parents' + end + + describe '#authorizable_members_with_parents' do + let(:group) { create(:group) } + + it_behaves_like 'members_with_parents' + + context 'members with associated user but also having invite_token' do + let!(:member) { create(:group_member, :developer, :invited, user: create(:user), group: group) } + + it 'includes such members in the result' do + expect(group.authorizable_members_with_parents).to include(member) + end + end + + context 'invited members' do + let!(:member) { create(:group_member, :developer, :invited, group: group) } + + it 'does not include such members in the result' do + expect(group.authorizable_members_with_parents).not_to include(member) + end + end + + context 'members from group shares' do + let(:shared_group) { group } + let(:shared_with_group) { create(:group) } + + before do + create(:group_group_link, shared_group: shared_group, shared_with_group: shared_with_group) + end + + context 'an invited member that is part of the shared_with_group' do + let!(:member) { create(:group_member, :developer, :invited, group: shared_with_group) } + + it 'does not include such members in the result' do + expect(shared_group.authorizable_members_with_parents).not_to( + include(member)) + end + end + end + end + describe '#members_from_self_and_ancestors_with_effective_access_level' do let!(:group_parent) { create(:group, :private) } let!(:group) { create(:group, :private, parent: group_parent) } @@ -1769,13 +1980,35 @@ RSpec.describe Group do allow(project).to receive(:protected_for?).with('ref').and_return(true) end - it 'returns all variables belong to the group and parent groups' do - expected_array1 = [protected_variable, ci_variable] - expected_array2 = [variable_child, variable_child_2, variable_child_3] - got_array = group_child_3.ci_variables_for('ref', project).to_a + context 'traversal queries' do + shared_examples 'correct ancestor order' do + it 'returns all variables belong to the group and parent groups' do + expected_array1 = [protected_variable, ci_variable] + expected_array2 = [variable_child, variable_child_2, variable_child_3] + got_array = group_child_3.ci_variables_for('ref', project).to_a + + expect(got_array.shift(2)).to contain_exactly(*expected_array1) + expect(got_array).to eq(expected_array2) + end + end + + context 'recursive' do + before do + stub_feature_flags(use_traversal_ids: false) + end - expect(got_array.shift(2)).to contain_exactly(*expected_array1) - expect(got_array).to eq(expected_array2) + include_examples 'correct ancestor order' + end + + context 'linear' do + before do + stub_feature_flags(use_traversal_ids: true) + + group_child_3.reload # make sure traversal_ids are reloaded + end + + include_examples 'correct ancestor order' + end end end end @@ -2012,22 +2245,31 @@ RSpec.describe Group do end describe '#access_request_approvers_to_be_notified' do - it 'returns a maximum of ten, active, non_requested owners of the group in recent_sign_in descending order' do - group = create(:group, :public) + let_it_be(:group) { create(:group, :public) } + it 'returns a maximum of ten owners of the group in recent_sign_in descending order' do users = create_list(:user, 12, :with_sign_ins) active_owners = users.map do |user| create(:group_member, :owner, group: group, user: user) end - create(:group_member, :owner, :blocked, group: group) - create(:group_member, :maintainer, group: group) - create(:group_member, :access_request, :owner, group: group) - - active_owners_in_recent_sign_in_desc_order = group.members_and_requesters.where(id: active_owners).order_recent_sign_in.limit(10) + active_owners_in_recent_sign_in_desc_order = group.members_and_requesters + .id_in(active_owners) + .order_recent_sign_in.limit(10) expect(group.access_request_approvers_to_be_notified).to eq(active_owners_in_recent_sign_in_desc_order) end + + it 'returns active, non_invited, non_requested owners of the group' do + owner = create(:group_member, :owner, source: group) + + create(:group_member, :maintainer, group: group) + create(:group_member, :owner, :invited, group: group) + create(:group_member, :owner, :access_request, group: group) + create(:group_member, :owner, :blocked, group: group) + + expect(group.access_request_approvers_to_be_notified.to_a).to eq([owner]) + end end describe '.groups_including_descendants_by' do @@ -2214,17 +2456,17 @@ RSpec.describe Group do end describe "#default_branch_name" do - context "group.namespace_settings does not have a default branch name" do + context "when group.namespace_settings does not have a default branch name" do it "returns nil" do expect(group.default_branch_name).to be_nil end end - context "group.namespace_settings has a default branch name" do + context "when group.namespace_settings has a default branch name" do let(:example_branch_name) { "example_branch_name" } before do - expect(group.namespace_settings) + allow(group.namespace_settings) .to receive(:default_branch_name) .and_return(example_branch_name) end @@ -2361,4 +2603,20 @@ RSpec.describe Group do it { is_expected.to eq(Set.new([child_1.id])) } end + + describe '#to_ability_name' do + it 'returns group' do + group = build(:group) + + expect(group.to_ability_name).to eq('group') + end + end + + describe '#activity_path' do + it 'returns the group activity_path' do + expected_path = "/groups/#{group.name}/-/activity" + + expect(group.activity_path).to eq(expected_path) + end + end end |