diff options
Diffstat (limited to 'spec/finders')
-rw-r--r-- | spec/finders/ci/runners_finder_spec.rb | 12 | ||||
-rw-r--r-- | spec/finders/environments/environments_by_deployments_finder_spec.rb | 12 | ||||
-rw-r--r-- | spec/finders/fork_targets_finder_spec.rb | 8 | ||||
-rw-r--r-- | spec/finders/group_descendants_finder_spec.rb | 334 | ||||
-rw-r--r-- | spec/finders/group_members_finder_spec.rb | 116 | ||||
-rw-r--r-- | spec/finders/groups/user_groups_finder_spec.rb | 17 | ||||
-rw-r--r-- | spec/finders/merge_requests_finder_spec.rb | 101 | ||||
-rw-r--r-- | spec/finders/packages/conan/package_file_finder_spec.rb | 30 | ||||
-rw-r--r-- | spec/finders/packages/go/package_finder_spec.rb | 2 | ||||
-rw-r--r-- | spec/finders/packages/maven/package_finder_spec.rb | 2 | ||||
-rw-r--r-- | spec/finders/packages/npm/package_finder_spec.rb | 2 | ||||
-rw-r--r-- | spec/finders/packages/nuget/package_finder_spec.rb | 2 | ||||
-rw-r--r-- | spec/finders/packages/package_file_finder_spec.rb | 28 | ||||
-rw-r--r-- | spec/finders/user_group_notification_settings_finder_spec.rb | 238 | ||||
-rw-r--r-- | spec/finders/user_recent_events_finder_spec.rb | 36 |
15 files changed, 500 insertions, 440 deletions
diff --git a/spec/finders/ci/runners_finder_spec.rb b/spec/finders/ci/runners_finder_spec.rb index 7e3c1abd6d1..dac244e4300 100644 --- a/spec/finders/ci/runners_finder_spec.rb +++ b/spec/finders/ci/runners_finder_spec.rb @@ -206,7 +206,7 @@ RSpec.describe Ci::RunnersFinder do sub_group_4.runners << runner_sub_group_4 end - describe '#execute' do + shared_examples '#execute' do subject { described_class.new(current_user: user, params: params).execute } shared_examples 'membership equal to :descendants' do @@ -349,6 +349,16 @@ RSpec.describe Ci::RunnersFinder do end end + it_behaves_like '#execute' + + context 'when the FF ci_find_runners_by_ci_mirrors is disabled' do + before do + stub_feature_flags(ci_find_runners_by_ci_mirrors: false) + end + + it_behaves_like '#execute' + end + describe '#sort_key' do subject { described_class.new(current_user: user, params: params.merge(group: group)).sort_key } diff --git a/spec/finders/environments/environments_by_deployments_finder_spec.rb b/spec/finders/environments/environments_by_deployments_finder_spec.rb index 1b86aced67d..8349092c79e 100644 --- a/spec/finders/environments/environments_by_deployments_finder_spec.rb +++ b/spec/finders/environments/environments_by_deployments_finder_spec.rb @@ -22,16 +22,6 @@ RSpec.describe Environments::EnvironmentsByDeploymentsFinder do create(:deployment, :success, environment: environment_two, ref: 'v1.1.0', tag: true, sha: project.commit('HEAD~1').id) end - it 'returns environment when with_tags is set' do - expect(described_class.new(project, user, ref: 'master', commit: commit, with_tags: true).execute) - .to contain_exactly(environment, environment_two) - end - - it 'does not return environment when no with_tags is set' do - expect(described_class.new(project, user, ref: 'master', commit: commit).execute) - .to be_empty - end - it 'does not return environment when commit is not part of deployment' do expect(described_class.new(project, user, ref: 'master', commit: project.commit('feature')).execute) .to be_empty @@ -41,7 +31,7 @@ RSpec.describe Environments::EnvironmentsByDeploymentsFinder do # This tests to ensure we don't call one CommitIsAncestor per environment it 'only calls Gitaly twice when multiple environments are present', :request_store do expect do - result = described_class.new(project, user, ref: 'master', commit: commit, with_tags: true, find_latest: true).execute + result = described_class.new(project, user, ref: 'v1.1.0', commit: commit, find_latest: true).execute expect(result).to contain_exactly(environment_two) end.to change { Gitlab::GitalyClient.get_request_count }.by(2) diff --git a/spec/finders/fork_targets_finder_spec.rb b/spec/finders/fork_targets_finder_spec.rb index 12f01227af8..fe5b50ef030 100644 --- a/spec/finders/fork_targets_finder_spec.rb +++ b/spec/finders/fork_targets_finder_spec.rb @@ -16,7 +16,9 @@ RSpec.describe ForkTargetsFinder do end let!(:developer_group) do - create(:group).tap { |g| g.add_developer(user) } + create(:group, project_creation_level: ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS).tap do |g| + g.add_developer(user) + end end let!(:reporter_group) do @@ -33,11 +35,11 @@ RSpec.describe ForkTargetsFinder do describe '#execute' do it 'returns all user manageable namespaces' do - expect(finder.execute).to match_array([user.namespace, maintained_group, owned_group, project.namespace]) + expect(finder.execute).to match_array([user.namespace, maintained_group, owned_group, project.namespace, developer_group]) end it 'returns only groups when only_groups option is passed' do - expect(finder.execute(only_groups: true)).to match_array([maintained_group, owned_group, project.namespace]) + expect(finder.execute(only_groups: true)).to match_array([maintained_group, owned_group, project.namespace, developer_group]) end it 'returns groups relation when only_groups option is passed' do diff --git a/spec/finders/group_descendants_finder_spec.rb b/spec/finders/group_descendants_finder_spec.rb index f6b87f7eeab..59eeb078e9e 100644 --- a/spec/finders/group_descendants_finder_spec.rb +++ b/spec/finders/group_descendants_finder_spec.rb @@ -17,262 +17,250 @@ RSpec.describe GroupDescendantsFinder do described_class.new(current_user: user, parent_group: group, params: params) end - shared_examples 'group descentants finder examples' do - describe '#has_children?' do + describe '#has_children?' do + it 'is true when there are projects' do + create(:project, namespace: group) + + expect(finder.has_children?).to be_truthy + end + + context 'when there are subgroups' do it 'is true when there are projects' do - create(:project, namespace: group) + create(:group, parent: group) expect(finder.has_children?).to be_truthy end + end + end - context 'when there are subgroups' do - it 'is true when there are projects' do - create(:group, parent: group) + describe '#execute' do + it 'includes projects' do + project = create(:project, namespace: group) - expect(finder.has_children?).to be_truthy - end - end + expect(finder.execute).to contain_exactly(project) end - describe '#execute' do - it 'includes projects' do + context 'when archived is `true`' do + let(:params) { { archived: 'true' } } + + it 'includes archived projects' do + archived_project = create(:project, namespace: group, archived: true) project = create(:project, namespace: group) - expect(finder.execute).to contain_exactly(project) + expect(finder.execute).to contain_exactly(archived_project, project) end + end - context 'when archived is `true`' do - let(:params) { { archived: 'true' } } + context 'when archived is `only`' do + let(:params) { { archived: 'only' } } - it 'includes archived projects' do - archived_project = create(:project, namespace: group, archived: true) - project = create(:project, namespace: group) + it 'includes only archived projects' do + archived_project = create(:project, namespace: group, archived: true) + _project = create(:project, namespace: group) - expect(finder.execute).to contain_exactly(archived_project, project) - end + expect(finder.execute).to contain_exactly(archived_project) end + end - context 'when archived is `only`' do - let(:params) { { archived: 'only' } } + it 'does not include archived projects' do + _archived_project = create(:project, :archived, namespace: group) - it 'includes only archived projects' do - archived_project = create(:project, namespace: group, archived: true) - _project = create(:project, namespace: group) + expect(finder.execute).to be_empty + end - expect(finder.execute).to contain_exactly(archived_project) - end - end + context 'with a filter' do + let(:params) { { filter: 'test' } } - it 'does not include archived projects' do - _archived_project = create(:project, :archived, namespace: group) + it 'includes only projects matching the filter' do + _other_project = create(:project, namespace: group) + matching_project = create(:project, namespace: group, name: 'testproject') - expect(finder.execute).to be_empty + expect(finder.execute).to contain_exactly(matching_project) end + end - context 'with a filter' do - let(:params) { { filter: 'test' } } + it 'sorts elements by name as default' do + project1 = create(:project, namespace: group, name: 'z') + project2 = create(:project, namespace: group, name: 'a') - it 'includes only projects matching the filter' do - _other_project = create(:project, namespace: group) - matching_project = create(:project, namespace: group, name: 'testproject') + expect(subject.execute).to match_array([project2, project1]) + end - expect(finder.execute).to contain_exactly(matching_project) - end + context 'sorting by name' do + let!(:project1) { create(:project, namespace: group, name: 'a', path: 'project-a') } + let!(:project2) { create(:project, namespace: group, name: 'z', path: 'project-z') } + let(:params) do + { + sort: 'name_asc' + } end - it 'sorts elements by name as default' do - project1 = create(:project, namespace: group, name: 'z') - project2 = create(:project, namespace: group, name: 'a') - - expect(subject.execute).to match_array([project2, project1]) + it 'sorts elements by name' do + expect(subject.execute).to eq( + [ + project1, + project2 + ] + ) end - context 'sorting by name' do - let!(:project1) { create(:project, namespace: group, name: 'a', path: 'project-a') } - let!(:project2) { create(:project, namespace: group, name: 'z', path: 'project-z') } - let(:params) do - { - sort: 'name_asc' - } - end + context 'with nested groups' do + let!(:subgroup1) { create(:group, parent: group, name: 'a', path: 'sub-a') } + let!(:subgroup2) { create(:group, parent: group, name: 'z', path: 'sub-z') } it 'sorts elements by name' do expect(subject.execute).to eq( [ + subgroup1, + subgroup2, project1, project2 ] ) end - - context 'with nested groups' do - let!(:subgroup1) { create(:group, parent: group, name: 'a', path: 'sub-a') } - let!(:subgroup2) { create(:group, parent: group, name: 'z', path: 'sub-z') } - - it 'sorts elements by name' do - expect(subject.execute).to eq( - [ - subgroup1, - subgroup2, - project1, - project2 - ] - ) - end - end end + end - it 'does not include projects shared with the group' do - project = create(:project, namespace: group) - other_project = create(:project) - other_project.project_group_links.create!(group: group, - group_access: Gitlab::Access::MAINTAINER) + it 'does not include projects shared with the group' do + project = create(:project, namespace: group) + other_project = create(:project) + other_project.project_group_links.create!(group: group, + group_access: Gitlab::Access::MAINTAINER) - expect(finder.execute).to contain_exactly(project) - end + expect(finder.execute).to contain_exactly(project) end + end - context 'with shared groups' do - let_it_be(:other_group) { create(:group) } - let_it_be(:shared_group_link) do - create(:group_group_link, - shared_group: group, - shared_with_group: other_group) - end + context 'with shared groups' do + let_it_be(:other_group) { create(:group) } + let_it_be(:shared_group_link) do + create(:group_group_link, + shared_group: group, + shared_with_group: other_group) + end - context 'without common ancestor' do + context 'without common ancestor' do + it { expect(finder.execute).to be_empty } + end + + context 'with common ancestor' do + let_it_be(:common_ancestor) { create(:group) } + let_it_be(:other_group) { create(:group, parent: common_ancestor) } + let_it_be(:group) { create(:group, parent: common_ancestor) } + + context 'querying under the common ancestor' do it { expect(finder.execute).to be_empty } end - context 'with common ancestor' do - let_it_be(:common_ancestor) { create(:group) } - let_it_be(:other_group) { create(:group, parent: common_ancestor) } - let_it_be(:group) { create(:group, parent: common_ancestor) } - - context 'querying under the common ancestor' do - it { expect(finder.execute).to be_empty } + context 'querying the common ancestor' do + subject(:finder) do + described_class.new(current_user: user, parent_group: common_ancestor, params: params) end - context 'querying the common ancestor' do - subject(:finder) do - described_class.new(current_user: user, parent_group: common_ancestor, params: params) - end - - it 'contains shared subgroups' do - expect(finder.execute).to contain_exactly(group, other_group) - end + it 'contains shared subgroups' do + expect(finder.execute).to contain_exactly(group, other_group) end end end + end - context 'with nested groups' do - let!(:project) { create(:project, namespace: group) } - let!(:subgroup) { create(:group, :private, parent: group) } + context 'with nested groups' do + let!(:project) { create(:project, namespace: group) } + let!(:subgroup) { create(:group, :private, parent: group) } - describe '#execute' do - it 'contains projects and subgroups' do - expect(finder.execute).to contain_exactly(subgroup, project) - end + describe '#execute' do + it 'contains projects and subgroups' do + expect(finder.execute).to contain_exactly(subgroup, project) + end - it 'does not include subgroups the user does not have access to' do - subgroup.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + it 'does not include subgroups the user does not have access to' do + subgroup.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) - public_subgroup = create(:group, :public, parent: group, path: 'public-group') - other_subgroup = create(:group, :private, parent: group, path: 'visible-private-group') - other_user = create(:user) - other_subgroup.add_developer(other_user) + public_subgroup = create(:group, :public, parent: group, path: 'public-group') + other_subgroup = create(:group, :private, parent: group, path: 'visible-private-group') + other_user = create(:user) + other_subgroup.add_developer(other_user) - finder = described_class.new(current_user: other_user, parent_group: group) + finder = described_class.new(current_user: other_user, parent_group: group) - expect(finder.execute).to contain_exactly(public_subgroup, other_subgroup) - end + expect(finder.execute).to contain_exactly(public_subgroup, other_subgroup) + end - it 'only includes public groups when no user is given' do - public_subgroup = create(:group, :public, parent: group) - _private_subgroup = create(:group, :private, parent: group) + it 'only includes public groups when no user is given' do + public_subgroup = create(:group, :public, parent: group) + _private_subgroup = create(:group, :private, parent: group) - finder = described_class.new(current_user: nil, parent_group: group) + finder = described_class.new(current_user: nil, parent_group: group) - expect(finder.execute).to contain_exactly(public_subgroup) - end + expect(finder.execute).to contain_exactly(public_subgroup) + end - context 'when archived is `true`' do - let(:params) { { archived: 'true' } } + context 'when archived is `true`' do + let(:params) { { archived: 'true' } } - it 'includes archived projects in the count of subgroups' do - create(:project, namespace: subgroup, archived: true) + it 'includes archived projects in the count of subgroups' do + create(:project, namespace: subgroup, archived: true) - expect(finder.execute.first.preloaded_project_count).to eq(1) - end + expect(finder.execute.first.preloaded_project_count).to eq(1) end + end - context 'with a filter' do - let(:params) { { filter: 'test' } } + context 'with a filter' do + let(:params) { { filter: 'test' } } - it 'contains only matching projects and subgroups' do - matching_project = create(:project, namespace: group, name: 'Testproject') - matching_subgroup = create(:group, name: 'testgroup', parent: group) + it 'contains only matching projects and subgroups' do + matching_project = create(:project, namespace: group, name: 'Testproject') + matching_subgroup = create(:group, name: 'testgroup', parent: group) - expect(finder.execute).to contain_exactly(matching_subgroup, matching_project) - end + expect(finder.execute).to contain_exactly(matching_subgroup, matching_project) + end - it 'does not include subgroups the user does not have access to' do - _invisible_subgroup = create(:group, :private, parent: group, name: 'test1') - other_subgroup = create(:group, :private, parent: group, name: 'test2') - public_subgroup = create(:group, :public, parent: group, name: 'test3') - other_subsubgroup = create(:group, :private, parent: other_subgroup, name: 'test4') - other_user = create(:user) - other_subgroup.add_developer(other_user) + it 'does not include subgroups the user does not have access to' do + _invisible_subgroup = create(:group, :private, parent: group, name: 'test1') + other_subgroup = create(:group, :private, parent: group, name: 'test2') + public_subgroup = create(:group, :public, parent: group, name: 'test3') + other_subsubgroup = create(:group, :private, parent: other_subgroup, name: 'test4') + other_user = create(:user) + other_subgroup.add_developer(other_user) - finder = described_class.new(current_user: other_user, - parent_group: group, - params: params) + finder = described_class.new(current_user: other_user, + parent_group: group, + params: params) - expect(finder.execute).to contain_exactly(other_subgroup, public_subgroup, other_subsubgroup) - end + expect(finder.execute).to contain_exactly(other_subgroup, public_subgroup, other_subsubgroup) + end - context 'with matching children' do - it 'includes a group that has a subgroup matching the query and its parent' do - matching_subgroup = create(:group, :private, name: 'testgroup', parent: subgroup) + context 'with matching children' do + it 'includes a group that has a subgroup matching the query and its parent' do + matching_subgroup = create(:group, :private, name: 'testgroup', parent: subgroup) - expect(finder.execute).to contain_exactly(subgroup, matching_subgroup) - end + expect(finder.execute).to contain_exactly(subgroup, matching_subgroup) + end - it 'includes the parent of a matching project' do - matching_project = create(:project, namespace: subgroup, name: 'Testproject') + it 'includes the parent of a matching project' do + matching_project = create(:project, namespace: subgroup, name: 'Testproject') - expect(finder.execute).to contain_exactly(subgroup, matching_project) - end + expect(finder.execute).to contain_exactly(subgroup, matching_project) + end - context 'with a small page size' do - let(:params) { { filter: 'test', per_page: 1 } } + context 'with a small page size' do + let(:params) { { filter: 'test', per_page: 1 } } - it 'contains all the ancestors of a matching subgroup regardless the page size' do - subgroup = create(:group, :private, parent: group) - matching = create(:group, :private, name: 'testgroup', parent: subgroup) + it 'contains all the ancestors of a matching subgroup regardless the page size' do + subgroup = create(:group, :private, parent: group) + matching = create(:group, :private, name: 'testgroup', parent: subgroup) - expect(finder.execute).to contain_exactly(subgroup, matching) - end + expect(finder.execute).to contain_exactly(subgroup, matching) end + end - it 'does not include the parent itself' do - group.update!(name: 'test') + it 'does not include the parent itself' do + group.update!(name: 'test') - expect(finder.execute).not_to include(group) - end + expect(finder.execute).not_to include(group) end end end end end - - it_behaves_like 'group descentants finder examples' - - context 'when feature flag :linear_group_descendants_finder is disabled' do - before do - stub_feature_flags(linear_group_descendants_finder: false) - end - - it_behaves_like 'group descentants finder examples' - end end diff --git a/spec/finders/group_members_finder_spec.rb b/spec/finders/group_members_finder_spec.rb index 0d797b7923c..a9a8e9d19b8 100644 --- a/spec/finders/group_members_finder_spec.rb +++ b/spec/finders/group_members_finder_spec.rb @@ -3,83 +3,93 @@ require 'spec_helper' RSpec.describe GroupMembersFinder, '#execute' do - let(:group) { create(:group) } - let(:sub_group) { create(:group, parent: group) } - let(:sub_sub_group) { create(:group, parent: sub_group) } - let(:user1) { create(:user) } - let(:user2) { create(:user) } - let(:user3) { create(:user) } - let(:user4) { create(:user) } - let(:user5) { create(:user, :two_factor_via_otp) } + let_it_be(:group) { create(:group) } + let_it_be(:sub_group) { create(:group, parent: group) } + let_it_be(:sub_sub_group) { create(:group, parent: sub_group) } + let_it_be(:public_shared_group) { create(:group, :public) } + let_it_be(:private_shared_group) { create(:group, :private) } + let_it_be(:user1) { create(:user) } + let_it_be(:user2) { create(:user) } + let_it_be(:user3) { create(:user) } + let_it_be(:user4) { create(:user) } + let_it_be(:user5) { create(:user, :two_factor_via_otp) } + + let!(:link) do + create(:group_group_link, shared_group: group, shared_with_group: public_shared_group) + create(:group_group_link, shared_group: sub_group, shared_with_group: private_shared_group) + end let(:groups) do { - group: group, - sub_group: sub_group, - sub_sub_group: sub_sub_group + group: group, + sub_group: sub_group, + sub_sub_group: sub_sub_group, + public_shared_group: public_shared_group, + private_shared_group: private_shared_group } end context 'relations' do let!(:members) do { - user1_sub_sub_group: create(:group_member, :maintainer, group: sub_sub_group, user: user1), - user1_sub_group: create(:group_member, :developer, group: sub_group, user: user1), - user1_group: create(:group_member, :reporter, group: group, user: user1), - user2_sub_sub_group: create(:group_member, :reporter, group: sub_sub_group, user: user2), - user2_sub_group: create(:group_member, :developer, group: sub_group, user: user2), - user2_group: create(:group_member, :maintainer, group: group, user: user2), - user3_sub_sub_group: create(:group_member, :developer, group: sub_sub_group, user: user3, expires_at: 1.day.from_now), - user3_sub_group: create(:group_member, :developer, group: sub_group, user: user3, expires_at: 2.days.from_now), - user3_group: create(:group_member, :reporter, group: group, user: user3), - user4_sub_sub_group: create(:group_member, :reporter, group: sub_sub_group, user: user4), - user4_sub_group: create(:group_member, :developer, group: sub_group, user: user4, expires_at: 1.day.from_now), - user4_group: create(:group_member, :developer, group: group, user: user4, expires_at: 2.days.from_now) + user1_sub_sub_group: create(:group_member, :maintainer, group: sub_sub_group, user: user1), + user1_sub_group: create(:group_member, :developer, group: sub_group, user: user1), + user1_group: create(:group_member, :reporter, group: group, user: user1), + user1_public_shared_group: create(:group_member, :maintainer, group: public_shared_group, user: user1), + user1_private_shared_group: create(:group_member, :maintainer, group: private_shared_group, user: user1), + user2_sub_sub_group: create(:group_member, :reporter, group: sub_sub_group, user: user2), + user2_sub_group: create(:group_member, :developer, group: sub_group, user: user2), + user2_group: create(:group_member, :maintainer, group: group, user: user2), + user2_public_shared_group: create(:group_member, :developer, group: public_shared_group, user: user2), + user2_private_shared_group: create(:group_member, :developer, group: private_shared_group, user: user2), + user3_sub_sub_group: create(:group_member, :developer, group: sub_sub_group, user: user3, expires_at: 1.day.from_now), + user3_sub_group: create(:group_member, :developer, group: sub_group, user: user3, expires_at: 2.days.from_now), + user3_group: create(:group_member, :reporter, group: group, user: user3), + user3_public_shared_group: create(:group_member, :reporter, group: public_shared_group, user: user3), + user3_private_shared_group: create(:group_member, :reporter, group: private_shared_group, user: user3), + user4_sub_sub_group: create(:group_member, :reporter, group: sub_sub_group, user: user4), + user4_sub_group: create(:group_member, :developer, group: sub_group, user: user4, expires_at: 1.day.from_now), + user4_group: create(:group_member, :developer, group: group, user: user4, expires_at: 2.days.from_now), + user4_public_shared_group: create(:group_member, :developer, group: public_shared_group, user: user4), + user4_private_shared_group: create(:group_member, :developer, group: private_shared_group, user: user4) } end it 'raises an error if a non-supported relation type is used' do expect do described_class.new(group).execute(include_relations: [:direct, :invalid_relation_type]) - end.to raise_error(ArgumentError, "invalid_relation_type is not a valid relation type. Valid relation types are direct, inherited, descendants.") + end.to raise_error(ArgumentError, "invalid_relation_type is not a valid relation type. Valid relation types are direct, inherited, descendants, shared_from_groups.") end using RSpec::Parameterized::TableSyntax where(:subject_relations, :subject_group, :expected_members) do - nil | :group | [:user1_group, :user2_group, :user3_group, :user4_group] - [:direct] | :group | [:user1_group, :user2_group, :user3_group, :user4_group] - [:inherited] | :group | [] - [:descendants] | :group | [:user1_sub_sub_group, :user2_sub_group, :user3_sub_group, :user4_sub_group] - [:direct, :inherited] | :group | [:user1_group, :user2_group, :user3_group, :user4_group] - [:direct, :descendants] | :group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group] - [:descendants, :inherited] | :group | [:user1_sub_sub_group, :user2_sub_group, :user3_sub_group, :user4_sub_group] - [:direct, :descendants, :inherited] | :group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group] - nil | :sub_group | [:user1_sub_group, :user2_group, :user3_sub_group, :user4_group] - [:direct] | :sub_group | [:user1_sub_group, :user2_sub_group, :user3_sub_group, :user4_sub_group] - [:inherited] | :sub_group | [:user1_group, :user2_group, :user3_group, :user4_group] - [:descendants] | :sub_group | [:user1_sub_sub_group, :user2_sub_sub_group, :user3_sub_sub_group, :user4_sub_sub_group] - [:direct, :inherited] | :sub_group | [:user1_sub_group, :user2_group, :user3_sub_group, :user4_group] - [:direct, :descendants] | :sub_group | [:user1_sub_sub_group, :user2_sub_group, :user3_sub_group, :user4_sub_group] - [:descendants, :inherited] | :sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_sub_group, :user4_group] - [:direct, :descendants, :inherited] | :sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group] - nil | :sub_sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group] - [:direct] | :sub_sub_group | [:user1_sub_sub_group, :user2_sub_sub_group, :user3_sub_sub_group, :user4_sub_sub_group] - [:inherited] | :sub_sub_group | [:user1_sub_group, :user2_group, :user3_sub_group, :user4_group] - [:descendants] | :sub_sub_group | [] - [:direct, :inherited] | :sub_sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group] - [:direct, :descendants] | :sub_sub_group | [:user1_sub_sub_group, :user2_sub_sub_group, :user3_sub_sub_group, :user4_sub_sub_group] - [:descendants, :inherited] | :sub_sub_group | [:user1_sub_group, :user2_group, :user3_sub_group, :user4_group] - [:direct, :descendants, :inherited] | :sub_sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group] + [] | :group | [] + GroupMembersFinder::DEFAULT_RELATIONS | :group | [:user1_group, :user2_group, :user3_group, :user4_group] + [:direct] | :group | [:user1_group, :user2_group, :user3_group, :user4_group] + [:inherited] | :group | [] + [:descendants] | :group | [:user1_sub_sub_group, :user2_sub_group, :user3_sub_group, :user4_sub_group] + [:shared_from_groups] | :group | [:user1_public_shared_group, :user2_public_shared_group, :user3_public_shared_group, :user4_public_shared_group] + [:direct, :inherited, :descendants, :shared_from_groups] | :group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_public_shared_group] + [] | :sub_group | [] + GroupMembersFinder::DEFAULT_RELATIONS | :sub_group | [:user1_sub_group, :user2_group, :user3_sub_group, :user4_group] + [:direct] | :sub_group | [:user1_sub_group, :user2_sub_group, :user3_sub_group, :user4_sub_group] + [:inherited] | :sub_group | [:user1_group, :user2_group, :user3_group, :user4_group] + [:descendants] | :sub_group | [:user1_sub_sub_group, :user2_sub_sub_group, :user3_sub_sub_group, :user4_sub_sub_group] + [:shared_from_groups] | :sub_group | [] + [:direct, :inherited, :descendants, :shared_from_groups] | :sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group] + [] | :sub_sub_group | [] + GroupMembersFinder::DEFAULT_RELATIONS | :sub_sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group] + [:direct] | :sub_sub_group | [:user1_sub_sub_group, :user2_sub_sub_group, :user3_sub_sub_group, :user4_sub_sub_group] + [:inherited] | :sub_sub_group | [:user1_sub_group, :user2_group, :user3_sub_group, :user4_group] + [:descendants] | :sub_sub_group | [] + [:shared_from_groups] | :sub_sub_group | [] + [:direct, :inherited, :descendants, :shared_from_groups] | :sub_sub_group | [:user1_sub_sub_group, :user2_group, :user3_sub_group, :user4_group] end with_them do it 'returns correct members' do - result = if subject_relations - described_class.new(groups[subject_group]).execute(include_relations: subject_relations) - else - described_class.new(groups[subject_group]).execute - end + result = described_class.new(groups[subject_group]).execute(include_relations: subject_relations) expect(result.to_a).to match_array(expected_members.map { |name| members[name] }) end diff --git a/spec/finders/groups/user_groups_finder_spec.rb b/spec/finders/groups/user_groups_finder_spec.rb index 4cce3ab72eb..a4a9b8d16d0 100644 --- a/spec/finders/groups/user_groups_finder_spec.rb +++ b/spec/finders/groups/user_groups_finder_spec.rb @@ -59,23 +59,6 @@ RSpec.describe Groups::UserGroupsFinder do ) end - context 'when paginatable_namespace_drop_down_for_project_creation feature flag is disabled' do - before do - stub_feature_flags(paginatable_namespace_drop_down_for_project_creation: false) - end - - it 'ignores project creation scope and returns all groups where the user is a direct member' do - is_expected.to match( - [ - public_maintainer_group, - private_maintainer_group, - public_developer_group, - guest_group - ] - ) - end - end - context 'when search is provided' do let(:arguments) { { permission_scope: :create_projects, search: 'maintainer' } } diff --git a/spec/finders/merge_requests_finder_spec.rb b/spec/finders/merge_requests_finder_spec.rb index 03639bc0b98..0b6c438fd02 100644 --- a/spec/finders/merge_requests_finder_spec.rb +++ b/spec/finders/merge_requests_finder_spec.rb @@ -278,33 +278,38 @@ RSpec.describe MergeRequestsFinder do end describe 'draft state' do - let!(:wip_merge_request1) { create(:merge_request, :simple, author: user, source_project: project5, target_project: project5, title: 'WIP: thing') } - let!(:wip_merge_request2) { create(:merge_request, :simple, author: user, source_project: project6, target_project: project6, title: 'wip thing') } - let!(:wip_merge_request3) { create(:merge_request, :simple, author: user, source_project: project1, target_project: project1, title: '[wip] thing') } - let!(:wip_merge_request4) { create(:merge_request, :simple, author: user, source_project: project1, target_project: project2, title: 'wip: thing') } - let!(:draft_merge_request1) { create(:merge_request, :simple, author: user, source_branch: 'draft1', source_project: project5, target_project: project5, title: 'Draft: thing') } - let!(:draft_merge_request2) { create(:merge_request, :simple, author: user, source_branch: 'draft2', source_project: project6, target_project: project6, title: '[draft] thing') } - let!(:draft_merge_request3) { create(:merge_request, :simple, author: user, source_branch: 'draft3', source_project: project1, target_project: project1, title: '(draft) thing') } - let!(:draft_merge_request4) { create(:merge_request, :simple, author: user, source_branch: 'draft4', source_project: project1, target_project: project2, title: 'Draft - thing') } - - [:wip, :draft].each do |draft_param_key| - it "filters by #{draft_param_key}" do - params = { draft_param_key => 'yes' } + shared_examples 'draft MRs filtering' do |draft_param_key, draft_param_value, title_prefix, draft_only| + it "filters by #{draft_param_key} => #{draft_param_value}" do + merge_request1.reload.update!(title: "#{title_prefix} #{merge_request1.title}") + + params = { draft_param_key => draft_param_value } merge_requests = described_class.new(user, params).execute - expect(merge_requests).to contain_exactly( - merge_request4, merge_request5, wip_merge_request1, wip_merge_request2, wip_merge_request3, wip_merge_request4, - draft_merge_request1, draft_merge_request2, draft_merge_request3, draft_merge_request4 - ) + if draft_only + expect(merge_requests).to contain_exactly(merge_request1, merge_request4, merge_request5) + else + expect(merge_requests).to contain_exactly(merge_request2, merge_request3) + end end + end - it "filters by not #{draft_param_key}" do - params = { draft_param_key => 'no' } - - merge_requests = described_class.new(user, params).execute + { + wip: ["WIP:", "wip", "[wip]"], + draft: ["Draft:", "Draft -", "[Draft]", "(Draft)"] + }.each do |draft_param_key, title_prefixes| + title_prefixes.each do |title_prefix| + it_behaves_like 'draft MRs filtering', draft_param_key, 1, title_prefix, true + it_behaves_like 'draft MRs filtering', draft_param_key, '1', title_prefix, true + it_behaves_like 'draft MRs filtering', draft_param_key, true, title_prefix, true + it_behaves_like 'draft MRs filtering', draft_param_key, 'true', title_prefix, true + it_behaves_like 'draft MRs filtering', draft_param_key, 'yes', title_prefix, true - expect(merge_requests).to contain_exactly(merge_request1, merge_request2, merge_request3) + it_behaves_like 'draft MRs filtering', draft_param_key, 0, title_prefix, false + it_behaves_like 'draft MRs filtering', draft_param_key, '0', title_prefix, false + it_behaves_like 'draft MRs filtering', draft_param_key, false, title_prefix, false + it_behaves_like 'draft MRs filtering', draft_param_key, 'false', title_prefix, false + it_behaves_like 'draft MRs filtering', draft_param_key, 'no', title_prefix, false end it "returns all items if no valid #{draft_param_key} param exists" do @@ -313,43 +318,41 @@ RSpec.describe MergeRequestsFinder do merge_requests = described_class.new(user, params).execute expect(merge_requests).to contain_exactly( - merge_request1, merge_request2, merge_request3, merge_request4, - merge_request5, wip_merge_request1, wip_merge_request2, wip_merge_request3, wip_merge_request4, - draft_merge_request1, draft_merge_request2, draft_merge_request3, draft_merge_request4 + merge_request1, merge_request2, merge_request3, merge_request4, merge_request5 ) end end + end - context 'filter by deployment' do - let_it_be(:project_with_repo) { create(:project, :repository) } + context 'filter by deployment' do + let_it_be(:project_with_repo) { create(:project, :repository) } - it 'returns the relevant merge requests' do - deployment1 = create( - :deployment, - project: project_with_repo, - sha: project_with_repo.commit.id - ) - deployment2 = create( - :deployment, - project: project_with_repo, - sha: project_with_repo.commit.id - ) - deployment1.link_merge_requests(MergeRequest.where(id: [merge_request1.id, merge_request2.id])) - deployment2.link_merge_requests(MergeRequest.where(id: merge_request3.id)) + it 'returns the relevant merge requests' do + deployment1 = create( + :deployment, + project: project_with_repo, + sha: project_with_repo.commit.id + ) + deployment2 = create( + :deployment, + project: project_with_repo, + sha: project_with_repo.commit.id + ) + deployment1.link_merge_requests(MergeRequest.where(id: [merge_request1.id, merge_request2.id])) + deployment2.link_merge_requests(MergeRequest.where(id: merge_request3.id)) - params = { deployment_id: deployment1.id } - merge_requests = described_class.new(user, params).execute + params = { deployment_id: deployment1.id } + merge_requests = described_class.new(user, params).execute - expect(merge_requests).to contain_exactly(merge_request1, merge_request2) - end + expect(merge_requests).to contain_exactly(merge_request1, merge_request2) + end - context 'when a deployment does not contain any merge requests' do - it 'returns an empty result' do - params = { deployment_id: create(:deployment, project: project_with_repo, sha: project_with_repo.commit.sha).id } - merge_requests = described_class.new(user, params).execute + context 'when a deployment does not contain any merge requests' do + it 'returns an empty result' do + params = { deployment_id: create(:deployment, project: project_with_repo, sha: project_with_repo.commit.sha).id } + merge_requests = described_class.new(user, params).execute - expect(merge_requests).to be_empty - end + expect(merge_requests).to be_empty end end end diff --git a/spec/finders/packages/conan/package_file_finder_spec.rb b/spec/finders/packages/conan/package_file_finder_spec.rb index c2f445c58f7..3da7da456c2 100644 --- a/spec/finders/packages/conan/package_file_finder_spec.rb +++ b/spec/finders/packages/conan/package_file_finder_spec.rb @@ -8,7 +8,7 @@ RSpec.describe ::Packages::Conan::PackageFileFinder do let(:package_file_name) { package_file.file_name } let(:params) { {} } - RSpec.shared_examples 'package file finder examples' do + shared_examples 'package file finder examples' do it { is_expected.to eq(package_file) } context 'with conan_file_type' do @@ -39,11 +39,37 @@ RSpec.describe ::Packages::Conan::PackageFileFinder do end end + shared_examples 'not returning pending_destruction package files' do + let_it_be(:recent_package_file_pending_destruction) do + create(:package_file, :pending_destruction, package: package, file_name: package_file.file_name) + end + + it 'returns the correct package file' do + expect(package.package_files.last).to eq(recent_package_file_pending_destruction) + + expect(subject).to eq(package_file) + end + + context 'with packages_installable_package_files disabled' do + before do + stub_feature_flags(packages_installable_package_files: false) + end + + it 'returns the correct package file' do + expect(package.package_files.last).to eq(recent_package_file_pending_destruction) + + expect(subject).to eq(recent_package_file_pending_destruction) + end + end + end + describe '#execute' do subject { described_class.new(package, package_file_name, params).execute } it_behaves_like 'package file finder examples' + it_behaves_like 'not returning pending_destruction package files' + context 'with unknown file_name' do let(:package_file_name) { 'unknown.jpg' } @@ -56,6 +82,8 @@ RSpec.describe ::Packages::Conan::PackageFileFinder do it_behaves_like 'package file finder examples' + it_behaves_like 'not returning pending_destruction package files' + context 'with unknown file_name' do let(:package_file_name) { 'unknown.jpg' } diff --git a/spec/finders/packages/go/package_finder_spec.rb b/spec/finders/packages/go/package_finder_spec.rb index dbcb8255d47..b928336f958 100644 --- a/spec/finders/packages/go/package_finder_spec.rb +++ b/spec/finders/packages/go/package_finder_spec.rb @@ -59,7 +59,7 @@ RSpec.describe Packages::Go::PackageFinder do let(:version_name) { version.name } before do - package.update_column(:status, 1) + package.update_column(:status, :error) end it { is_expected.to eq(nil) } diff --git a/spec/finders/packages/maven/package_finder_spec.rb b/spec/finders/packages/maven/package_finder_spec.rb index 38fc3b7cce4..8b45dbdad51 100644 --- a/spec/finders/packages/maven/package_finder_spec.rb +++ b/spec/finders/packages/maven/package_finder_spec.rb @@ -39,7 +39,7 @@ RSpec.describe ::Packages::Maven::PackageFinder do let(:param_path) { package.maven_metadatum.path } before do - package.update_column(:status, 1) + package.update_column(:status, :error) end it { expect { subject }.to raise_error(ActiveRecord::RecordNotFound) } diff --git a/spec/finders/packages/npm/package_finder_spec.rb b/spec/finders/packages/npm/package_finder_spec.rb index 230d267e508..7fabb3eed86 100644 --- a/spec/finders/packages/npm/package_finder_spec.rb +++ b/spec/finders/packages/npm/package_finder_spec.rb @@ -52,7 +52,7 @@ RSpec.describe ::Packages::Npm::PackageFinder do context 'with an uninstallable package' do before do - package.update_column(:status, 1) + package.update_column(:status, :error) end it { is_expected.to be_empty } diff --git a/spec/finders/packages/nuget/package_finder_spec.rb b/spec/finders/packages/nuget/package_finder_spec.rb index 045dba295ac..415bf796a72 100644 --- a/spec/finders/packages/nuget/package_finder_spec.rb +++ b/spec/finders/packages/nuget/package_finder_spec.rb @@ -36,7 +36,7 @@ RSpec.describe Packages::Nuget::PackageFinder do context 'with an uninstallable package' do before do - package1.update_column(:status, 1) + package1.update_column(:status, :error) end it { is_expected.to contain_exactly(package2) } diff --git a/spec/finders/packages/package_file_finder_spec.rb b/spec/finders/packages/package_file_finder_spec.rb index 8014f04d917..8b21c9cd3ec 100644 --- a/spec/finders/packages/package_file_finder_spec.rb +++ b/spec/finders/packages/package_file_finder_spec.rb @@ -8,7 +8,7 @@ RSpec.describe Packages::PackageFileFinder do let(:package_file_name) { package_file.file_name } let(:params) { {} } - RSpec.shared_examples 'package file finder examples' do + shared_examples 'package file finder examples' do it { is_expected.to eq(package_file) } context 'with file_name_like' do @@ -19,11 +19,35 @@ RSpec.describe Packages::PackageFileFinder do end end + shared_examples 'not returning pending_destruction package files' do + let_it_be(:recent_package_file_pending_destruction) do + create(:package_file, :pending_destruction, package: package, file_name: package_file.file_name) + end + + it 'returns the correct package file' do + expect(package.package_files.last).to eq(recent_package_file_pending_destruction) + + expect(subject).to eq(package_file) + end + + context 'with packages_installable_package_files disabled' do + before do + stub_feature_flags(packages_installable_package_files: false) + end + + it 'returns them' do + expect(subject).to eq(recent_package_file_pending_destruction) + end + end + end + describe '#execute' do subject { described_class.new(package, package_file_name, params).execute } it_behaves_like 'package file finder examples' + it_behaves_like 'not returning pending_destruction package files' + context 'with unknown file_name' do let(:package_file_name) { 'unknown.jpg' } @@ -36,6 +60,8 @@ RSpec.describe Packages::PackageFileFinder do it_behaves_like 'package file finder examples' + it_behaves_like 'not returning pending_destruction package files' + context 'with unknown file_name' do let(:package_file_name) { 'unknown.jpg' } diff --git a/spec/finders/user_group_notification_settings_finder_spec.rb b/spec/finders/user_group_notification_settings_finder_spec.rb index ea44688bc8d..ac59a42d813 100644 --- a/spec/finders/user_group_notification_settings_finder_spec.rb +++ b/spec/finders/user_group_notification_settings_finder_spec.rb @@ -11,167 +11,155 @@ RSpec.describe UserGroupNotificationSettingsFinder do subject.map(&proc).uniq end - shared_examples 'user group notifications settings tests' do - context 'when the groups have no existing notification settings' do - context 'when the groups have no ancestors' do - let_it_be(:groups) { create_list(:group, 3) } - - it 'will be a default Global notification setting', :aggregate_failures do - expect(subject.count).to eq(3) - expect(attributes(&:notification_email)).to match_array([nil]) - expect(attributes(&:level)).to match_array(['global']) - end + context 'when the groups have no existing notification settings' do + context 'when the groups have no ancestors' do + let_it_be(:groups) { create_list(:group, 3) } + + it 'will be a default Global notification setting', :aggregate_failures do + expect(subject.count).to eq(3) + expect(attributes(&:notification_email)).to match_array([nil]) + expect(attributes(&:level)).to match_array(['global']) end + end - context 'when the groups have ancestors' do - context 'when an ancestor has a level other than Global' do - let_it_be(:ancestor_a) { create(:group) } - let_it_be(:group_a) { create(:group, parent: ancestor_a) } - let_it_be(:ancestor_b) { create(:group) } - let_it_be(:group_b) { create(:group, parent: ancestor_b) } - let_it_be(:email) { create(:email, :confirmed, email: 'ancestor@example.com', user: user) } - - let_it_be(:groups) { [group_a, group_b] } + context 'when the groups have ancestors' do + context 'when an ancestor has a level other than Global' do + let_it_be(:ancestor_a) { create(:group) } + let_it_be(:group_a) { create(:group, parent: ancestor_a) } + let_it_be(:ancestor_b) { create(:group) } + let_it_be(:group_b) { create(:group, parent: ancestor_b) } + let_it_be(:email) { create(:email, :confirmed, email: 'ancestor@example.com', user: user) } - before do - create(:notification_setting, user: user, source: ancestor_a, level: 'participating', notification_email: email.email) - create(:notification_setting, user: user, source: ancestor_b, level: 'participating', notification_email: email.email) - end + let_it_be(:groups) { [group_a, group_b] } - it 'has the same level set' do - expect(attributes(&:level)).to match_array(['participating']) - end + before do + create(:notification_setting, user: user, source: ancestor_a, level: 'participating', notification_email: email.email) + create(:notification_setting, user: user, source: ancestor_b, level: 'participating', notification_email: email.email) + end - it 'has the same email set' do - expect(attributes(&:notification_email)).to match_array(['ancestor@example.com']) - end + it 'has the same level set' do + expect(attributes(&:level)).to match_array(['participating']) + end - it 'only returns the two queried groups' do - expect(subject.count).to eq(2) - end + it 'has the same email set' do + expect(attributes(&:notification_email)).to match_array(['ancestor@example.com']) end - context 'when an ancestor has a Global level but has an email set' do - let_it_be(:grand_ancestor) { create(:group) } - let_it_be(:ancestor) { create(:group, parent: grand_ancestor) } - let_it_be(:group) { create(:group, parent: ancestor) } - let_it_be(:ancestor_email) { create(:email, :confirmed, email: 'ancestor@example.com', user: user) } - let_it_be(:grand_email) { create(:email, :confirmed, email: 'grand@example.com', user: user) } - - let_it_be(:groups) { [group] } - - before do - create(:notification_setting, user: user, source: grand_ancestor, level: 'participating', notification_email: grand_email.email) - create(:notification_setting, user: user, source: ancestor, level: 'global', notification_email: ancestor_email.email) - end - - it 'has the same email and level set', :aggregate_failures do - expect(subject.count).to eq(1) - expect(attributes(&:level)).to match_array(['global']) - expect(attributes(&:notification_email)).to match_array(['ancestor@example.com']) - end + it 'only returns the two queried groups' do + expect(subject.count).to eq(2) end + end - context 'when the group has parent_id set but that does not belong to any group' do - let_it_be(:group) { create(:group) } - let_it_be(:groups) { [group] } + context 'when an ancestor has a Global level but has an email set' do + let_it_be(:grand_ancestor) { create(:group) } + let_it_be(:ancestor) { create(:group, parent: grand_ancestor) } + let_it_be(:group) { create(:group, parent: ancestor) } + let_it_be(:ancestor_email) { create(:email, :confirmed, email: 'ancestor@example.com', user: user) } + let_it_be(:grand_email) { create(:email, :confirmed, email: 'grand@example.com', user: user) } - before do - # Let's set a parent_id for a group that definitely doesn't exist - group.update_columns(parent_id: 19283746) - end + let_it_be(:groups) { [group] } - it 'returns a default Global notification setting' do - expect(subject.count).to eq(1) - expect(attributes(&:level)).to match_array(['global']) - expect(attributes(&:notification_email)).to match_array([nil]) - end + before do + create(:notification_setting, user: user, source: grand_ancestor, level: 'participating', notification_email: grand_email.email) + create(:notification_setting, user: user, source: ancestor, level: 'global', notification_email: ancestor_email.email) end - context 'when the group has a private parent' do - let_it_be(:ancestor) { create(:group, :private) } - let_it_be(:group) { create(:group, :private, parent: ancestor) } - let_it_be(:ancestor_email) { create(:email, :confirmed, email: 'ancestor@example.com', user: user) } - let_it_be(:groups) { [group] } - - before do - group.add_reporter(user) - # Adding the user creates a NotificationSetting, so we remove it here - user.notification_settings.where(source: group).delete_all - - create(:notification_setting, user: user, source: ancestor, level: 'participating', notification_email: ancestor_email.email) - end - - it 'still inherits the notification settings' do - expect(subject.count).to eq(1) - expect(attributes(&:level)).to match_array(['participating']) - expect(attributes(&:notification_email)).to match_array([ancestor_email.email]) - end + it 'has the same email and level set', :aggregate_failures do + expect(subject.count).to eq(1) + expect(attributes(&:level)).to match_array(['global']) + expect(attributes(&:notification_email)).to match_array(['ancestor@example.com']) end + end - it 'does not cause an N+1', :aggregate_failures do - parent = create(:group) - child = create(:group, parent: parent) - - control = ActiveRecord::QueryRecorder.new do - described_class.new(user, Group.where(id: child.id)).execute - end - - other_parent = create(:group) - other_children = create_list(:group, 2, parent: other_parent) - - result = nil + context 'when the group has parent_id set but that does not belong to any group' do + let_it_be(:group) { create(:group) } + let_it_be(:groups) { [group] } - expect do - result = described_class.new(user, Group.where(id: other_children.append(child).map(&:id))).execute - end.not_to exceed_query_limit(control) + before do + # Let's set a parent_id for a group that definitely doesn't exist + group.update_columns(parent_id: 19283746) + end - expect(result.count).to eq(3) + it 'returns a default Global notification setting' do + expect(subject.count).to eq(1) + expect(attributes(&:level)).to match_array(['global']) + expect(attributes(&:notification_email)).to match_array([nil]) end end - end - context 'preloading `emails_disabled`' do - let_it_be(:root_group) { create(:group) } - let_it_be(:sub_group) { create(:group, parent: root_group) } - let_it_be(:sub_sub_group) { create(:group, parent: sub_group) } + context 'when the group has a private parent' do + let_it_be(:ancestor) { create(:group, :private) } + let_it_be(:group) { create(:group, :private, parent: ancestor) } + let_it_be(:ancestor_email) { create(:email, :confirmed, email: 'ancestor@example.com', user: user) } + let_it_be(:groups) { [group] } - let_it_be(:another_root_group) { create(:group) } - let_it_be(:sub_group_with_emails_disabled) { create(:group, emails_disabled: true, parent: another_root_group) } - let_it_be(:another_sub_sub_group) { create(:group, parent: sub_group_with_emails_disabled) } + before do + group.add_reporter(user) + # Adding the user creates a NotificationSetting, so we remove it here + user.notification_settings.where(source: group).delete_all - let_it_be(:root_group_with_emails_disabled) { create(:group, emails_disabled: true) } - let_it_be(:group) { create(:group, parent: root_group_with_emails_disabled) } - - let(:groups) { Group.where(id: [sub_sub_group, another_sub_sub_group, group]) } + create(:notification_setting, user: user, source: ancestor, level: 'participating', notification_email: ancestor_email.email) + end - before do - described_class.new(user, groups).execute + it 'still inherits the notification settings' do + expect(subject.count).to eq(1) + expect(attributes(&:level)).to match_array(['participating']) + expect(attributes(&:notification_email)).to match_array([ancestor_email.email]) + end end - it 'preloads the `group.emails_disabled` method' do - recorder = ActiveRecord::QueryRecorder.new do - groups.each(&:emails_disabled?) + it 'does not cause an N+1', :aggregate_failures do + parent = create(:group) + child = create(:group, parent: parent) + + control = ActiveRecord::QueryRecorder.new do + described_class.new(user, Group.where(id: child.id)).execute end - expect(recorder.count).to eq(0) - end + other_parent = create(:group) + other_children = create_list(:group, 2, parent: other_parent) - it 'preloads the `group.emails_disabled` method correctly' do - groups.each do |group| - expect(group.emails_disabled?).to eq(Group.find(group.id).emails_disabled?) # compare the memoized and the freshly loaded value - end + result = nil + + expect do + result = described_class.new(user, Group.where(id: other_children.append(child).map(&:id))).execute + end.not_to exceed_query_limit(control) + + expect(result.count).to eq(3) end end end - it_behaves_like 'user group notifications settings tests' + context 'preloading `emails_disabled`' do + let_it_be(:root_group) { create(:group) } + let_it_be(:sub_group) { create(:group, parent: root_group) } + let_it_be(:sub_sub_group) { create(:group, parent: sub_group) } + + let_it_be(:another_root_group) { create(:group) } + let_it_be(:sub_group_with_emails_disabled) { create(:group, emails_disabled: true, parent: another_root_group) } + let_it_be(:another_sub_sub_group) { create(:group, parent: sub_group_with_emails_disabled) } + + let_it_be(:root_group_with_emails_disabled) { create(:group, emails_disabled: true) } + let_it_be(:group) { create(:group, parent: root_group_with_emails_disabled) } + + let(:groups) { Group.where(id: [sub_sub_group, another_sub_sub_group, group]) } - context 'when feature flag :linear_user_group_notification_settings_finder_ancestors_scopes is disabled' do before do - stub_feature_flags(linear_user_group_notification_settings_finder_ancestors_scopes: false) + described_class.new(user, groups).execute + end + + it 'preloads the `group.emails_disabled` method' do + recorder = ActiveRecord::QueryRecorder.new do + groups.each(&:emails_disabled?) + end + + expect(recorder.count).to eq(0) end - it_behaves_like 'user group notifications settings tests' + it 'preloads the `group.emails_disabled` method correctly' do + groups.each do |group| + expect(group.emails_disabled?).to eq(Group.find(group.id).emails_disabled?) # compare the memoized and the freshly loaded value + end + end end end diff --git a/spec/finders/user_recent_events_finder_spec.rb b/spec/finders/user_recent_events_finder_spec.rb index 74c563b9bf6..6019d22059d 100644 --- a/spec/finders/user_recent_events_finder_spec.rb +++ b/spec/finders/user_recent_events_finder_spec.rb @@ -59,14 +59,46 @@ RSpec.describe UserRecentEventsFinder do expect(events.size).to eq(6) end + context 'selected events' do + let!(:push_event) { create(:push_event, project: public_project, author: project_owner) } + let!(:push_event_second_user) { create(:push_event, project: public_project_second_user, author: second_user) } + + it 'only includes selected events (PUSH) from all users', :aggregate_failures do + event_filter = EventFilter.new(EventFilter::PUSH) + events = described_class.new(current_user, [project_owner, second_user], event_filter, params).execute + + expect(events).to contain_exactly(push_event, push_event_second_user) + end + end + it 'does not include events from users with private profile', :aggregate_failures do allow(Ability).to receive(:allowed?).and_call_original allow(Ability).to receive(:allowed?).with(current_user, :read_user_profile, second_user).and_return(false) events = described_class.new(current_user, [project_owner, second_user], nil, params).execute - expect(events).to include(private_event, internal_event, public_event) - expect(events.size).to eq(3) + expect(events).to contain_exactly(private_event, internal_event, public_event) + end + + context 'with pagination params' do + using RSpec::Parameterized::TableSyntax + + where(:limit, :offset, :ordered_expected_events) do + nil | nil | lazy { [public_event_second_user, internal_event_second_user, private_event_second_user, public_event, internal_event, private_event] } + 2 | nil | lazy { [public_event_second_user, internal_event_second_user] } + nil | 4 | lazy { [internal_event, private_event] } + 2 | 2 | lazy { [private_event_second_user, public_event] } + end + + with_them do + let(:params) { { limit: limit, offset: offset }.compact } + + it 'returns paginated events sorted by id (DESC)' do + events = described_class.new(current_user, [project_owner, second_user], nil, params).execute + + expect(events).to eq(ordered_expected_events) + end + end end end |