diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-09-19 01:45:44 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-09-19 01:45:44 +0000 |
commit | 85dc423f7090da0a52c73eb66faf22ddb20efff9 (patch) | |
tree | 9160f299afd8c80c038f08e1545be119f5e3f1e1 /spec/finders | |
parent | 15c2c8c66dbe422588e5411eee7e68f1fa440bb8 (diff) | |
download | gitlab-ce-85dc423f7090da0a52c73eb66faf22ddb20efff9.tar.gz |
Add latest changes from gitlab-org/gitlab@13-4-stable-ee
Diffstat (limited to 'spec/finders')
-rw-r--r-- | spec/finders/ci/jobs_finder_spec.rb | 140 | ||||
-rw-r--r-- | spec/finders/concerns/finder_methods_spec.rb | 6 | ||||
-rw-r--r-- | spec/finders/design_management/designs_finder_spec.rb | 20 | ||||
-rw-r--r-- | spec/finders/feature_flags_finder_spec.rb | 94 | ||||
-rw-r--r-- | spec/finders/fork_targets_finder_spec.rb | 8 | ||||
-rw-r--r-- | spec/finders/group_members_finder_spec.rb | 1 | ||||
-rw-r--r-- | spec/finders/groups_finder_spec.rb | 8 | ||||
-rw-r--r-- | spec/finders/issues_finder_spec.rb | 219 | ||||
-rw-r--r-- | spec/finders/members_finder_spec.rb | 12 | ||||
-rw-r--r-- | spec/finders/merge_requests_finder_spec.rb | 66 | ||||
-rw-r--r-- | spec/finders/projects_finder_spec.rb | 36 | ||||
-rw-r--r-- | spec/finders/user_group_notification_settings_finder_spec.rb | 132 | ||||
-rw-r--r-- | spec/finders/users_finder_spec.rb | 17 |
13 files changed, 586 insertions, 173 deletions
diff --git a/spec/finders/ci/jobs_finder_spec.rb b/spec/finders/ci/jobs_finder_spec.rb index e6680afa15c..a6a41c36489 100644 --- a/spec/finders/ci/jobs_finder_spec.rb +++ b/spec/finders/ci/jobs_finder_spec.rb @@ -36,53 +36,135 @@ RSpec.describe Ci::JobsFinder, '#execute' do end end - context 'scope is present' do - let(:jobs) { [job_1, job_2, job_3] } - - where(:scope, :index) do - [ - ['pending', 0], - ['running', 1], - ['finished', 2] - ] + context 'with ci_jobs_finder_refactor ff enabled' do + before do + stub_feature_flags(ci_jobs_finder_refactor: true) end - with_them do - let(:params) { { scope: scope } } + context 'scope is present' do + let(:jobs) { [job_1, job_2, job_3] } + + where(:scope, :index) do + [ + ['pending', 0], + ['running', 1], + ['finished', 2] + ] + end + + with_them do + let(:params) { { scope: scope } } - it { expect(subject).to match_array([jobs[index]]) } + it { expect(subject).to match_array([jobs[index]]) } + end end - end - end - context 'a project is present' do - subject { described_class.new(current_user: user, project: project, params: params).execute } + context 'scope is an array' do + let(:jobs) { [job_1, job_2, job_3] } + let(:params) {{ scope: ['running'] }} - context 'user has access to the project' do + it 'filters by the job statuses in the scope' do + expect(subject).to match_array([job_2]) + end + end + end + + context 'with ci_jobs_finder_refactor ff disabled' do before do - project.add_maintainer(user) + stub_feature_flags(ci_jobs_finder_refactor: false) end - it 'returns jobs for the specified project' do - expect(subject).to match_array([job_3]) + context 'scope is present' do + let(:jobs) { [job_1, job_2, job_3] } + + where(:scope, :index) do + [ + ['pending', 0], + ['running', 1], + ['finished', 2] + ] + end + + with_them do + let(:params) { { scope: scope } } + + it { expect(subject).to match_array([jobs[index]]) } + end end end + end - context 'user has no access to project builds' do - before do - project.add_guest(user) + context 'with ci_jobs_finder_refactor ff enabled' do + before do + stub_feature_flags(ci_jobs_finder_refactor: true) + end + + context 'a project is present' do + subject { described_class.new(current_user: user, project: project, params: params).execute } + + context 'user has access to the project' do + before do + project.add_maintainer(user) + end + + it 'returns jobs for the specified project' do + expect(subject).to match_array([job_3]) + end end - it 'returns no jobs' do - expect(subject).to be_empty + context 'user has no access to project builds' do + before do + project.add_guest(user) + end + + it 'returns no jobs' do + expect(subject).to be_empty + end + end + + context 'without user' do + let(:user) { nil } + + it 'returns no jobs' do + expect(subject).to be_empty + end end end + end - context 'without user' do - let(:user) { nil } + context 'with ci_jobs_finder_refactor ff disabled' do + before do + stub_feature_flags(ci_jobs_finder_refactor: false) + end + context 'a project is present' do + subject { described_class.new(current_user: user, project: project, params: params).execute } - it 'returns no jobs' do - expect(subject).to be_empty + context 'user has access to the project' do + before do + project.add_maintainer(user) + end + + it 'returns jobs for the specified project' do + expect(subject).to match_array([job_3]) + end + end + + context 'user has no access to project builds' do + before do + project.add_guest(user) + end + + it 'returns no jobs' do + expect(subject).to be_empty + end + end + + context 'without user' do + let(:user) { nil } + + it 'returns no jobs' do + expect(subject).to be_empty + end end end end diff --git a/spec/finders/concerns/finder_methods_spec.rb b/spec/finders/concerns/finder_methods_spec.rb index 3e299c93eda..195449d70c3 100644 --- a/spec/finders/concerns/finder_methods_spec.rb +++ b/spec/finders/concerns/finder_methods_spec.rb @@ -7,8 +7,6 @@ RSpec.describe FinderMethods do Class.new do include FinderMethods - attr_reader :current_user - def initialize(user) @current_user = user end @@ -16,6 +14,10 @@ RSpec.describe FinderMethods do def execute Project.all.order(id: :desc) end + + private + + attr_reader :current_user end end diff --git a/spec/finders/design_management/designs_finder_spec.rb b/spec/finders/design_management/designs_finder_spec.rb index 0133095827d..feb78a4bc4b 100644 --- a/spec/finders/design_management/designs_finder_spec.rb +++ b/spec/finders/design_management/designs_finder_spec.rb @@ -42,26 +42,6 @@ RSpec.describe DesignManagement::DesignsFinder do is_expected.to eq([design3, design2, design1]) end - context 'when the :reorder_designs feature is enabled for the project' do - before do - stub_feature_flags(reorder_designs: project) - end - - it 'returns the designs sorted by their relative position' do - is_expected.to eq([design3, design2, design1]) - end - end - - context 'when the :reorder_designs feature is disabled' do - before do - stub_feature_flags(reorder_designs: false) - end - - it 'returns the designs sorted by ID' do - is_expected.to eq([design1, design2, design3]) - end - end - context 'when argument is the ids of designs' do let(:params) { { ids: [design1.id] } } diff --git a/spec/finders/feature_flags_finder_spec.rb b/spec/finders/feature_flags_finder_spec.rb new file mode 100644 index 00000000000..870447a1286 --- /dev/null +++ b/spec/finders/feature_flags_finder_spec.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe FeatureFlagsFinder do + include FeatureFlagHelpers + + let(:finder) { described_class.new(project, user, params) } + let(:project) { create(:project) } + let(:user) { developer } + let(:developer) { create(:user) } + let(:reporter) { create(:user) } + let(:params) { {} } + + before do + project.add_developer(developer) + project.add_reporter(reporter) + end + + describe '#execute' do + subject { finder.execute(args) } + + let!(:feature_flag_1) { create(:operations_feature_flag, name: 'flag-a', project: project) } + let!(:feature_flag_2) { create(:operations_feature_flag, name: 'flag-b', project: project) } + let(:args) { {} } + + it 'returns feature flags ordered by name' do + is_expected.to eq([feature_flag_1, feature_flag_2]) + end + + it 'preloads relations by default' do + expect(Operations::FeatureFlag).to receive(:preload_relations).and_call_original + + subject + end + + context 'when user is a reporter' do + let(:user) { reporter } + + it 'returns an empty list' do + is_expected.to be_empty + end + end + + context 'when scope is given' do + let!(:feature_flag_1) { create(:operations_feature_flag, project: project, active: true) } + let!(:feature_flag_2) { create(:operations_feature_flag, project: project, active: false) } + + context 'when scope is enabled' do + let(:params) { { scope: 'enabled' } } + + it 'returns active feature flag' do + is_expected.to eq([feature_flag_1]) + end + end + + context 'when scope is disabled' do + let(:params) { { scope: 'disabled' } } + + it 'returns inactive feature flag' do + is_expected.to eq([feature_flag_2]) + end + end + end + + context 'when preload option is false' do + let(:args) { { preload: false } } + + it 'does not preload relations' do + expect(Operations::FeatureFlag).not_to receive(:preload_relations) + + subject + end + end + + context 'when new version flags are enabled' do + let!(:feature_flag_3) { create(:operations_feature_flag, :new_version_flag, name: 'flag-c', project: project) } + + it 'returns new and legacy flags' do + is_expected.to eq([feature_flag_1, feature_flag_2, feature_flag_3]) + end + end + + context 'when new version flags are disabled' do + let!(:feature_flag_3) { create(:operations_feature_flag, :new_version_flag, name: 'flag-c', project: project) } + + it 'returns only legacy flags' do + stub_feature_flags(feature_flags_new_version: false) + + is_expected.to eq([feature_flag_1, feature_flag_2]) + end + end + end +end diff --git a/spec/finders/fork_targets_finder_spec.rb b/spec/finders/fork_targets_finder_spec.rb index 7208f46cfff..12f01227af8 100644 --- a/spec/finders/fork_targets_finder_spec.rb +++ b/spec/finders/fork_targets_finder_spec.rb @@ -35,5 +35,13 @@ RSpec.describe ForkTargetsFinder do it 'returns all user manageable namespaces' do expect(finder.execute).to match_array([user.namespace, maintained_group, owned_group, project.namespace]) 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]) + end + + it 'returns groups relation when only_groups option is passed' do + expect(finder.execute(only_groups: true)).to include(a_kind_of(Group)) + end end end diff --git a/spec/finders/group_members_finder_spec.rb b/spec/finders/group_members_finder_spec.rb index 68b120db227..67e7de5921a 100644 --- a/spec/finders/group_members_finder_spec.rb +++ b/spec/finders/group_members_finder_spec.rb @@ -16,6 +16,7 @@ RSpec.describe GroupMembersFinder, '#execute' do member1 = group.add_maintainer(user1) member2 = group.add_maintainer(user2) member3 = group.add_maintainer(user3) + create(:group_member, :minimal_access, user: create(:user), source: group) result = described_class.new(group).execute diff --git a/spec/finders/groups_finder_spec.rb b/spec/finders/groups_finder_spec.rb index 78764f79a6c..48e4c5dadc9 100644 --- a/spec/finders/groups_finder_spec.rb +++ b/spec/finders/groups_finder_spec.rb @@ -150,6 +150,14 @@ RSpec.describe GroupsFinder do end end end + + context 'being minimal access member of parent group' do + it 'do not return group with minimal_access access' do + create(:group_member, :minimal_access, user: user, source: parent_group) + + is_expected.to contain_exactly(public_subgroup, internal_subgroup) + end + end end end end diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index fb7d4e808fe..dbf5abe64a5 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' RSpec.describe IssuesFinder do + using RSpec::Parameterized::TableSyntax include_context 'IssuesFinder context' describe '#execute' do @@ -330,100 +331,139 @@ RSpec.describe IssuesFinder do end end - context 'filtering by label' do - let(:params) { { label_name: label.title } } + shared_examples ':label_name parameter' do + context 'filtering by label' do + let(:params) { { label_name: label.title } } - it 'returns issues with that label' do - expect(issues).to contain_exactly(issue2) - end + it 'returns issues with that label' do + expect(issues).to contain_exactly(issue2) + end - context 'using NOT' do - let(:params) { { not: { label_name: label.title } } } + context 'using NOT' do + let(:params) { { not: { label_name: label.title } } } - it 'returns issues that do not have that label' do - expect(issues).to contain_exactly(issue1, issue3, issue4) - end + it 'returns issues that do not have that label' do + expect(issues).to contain_exactly(issue1, issue3, issue4) + end - # IssuableFinder first filters using the outer params (the ones not inside the `not` key.) - # Afterwards, it applies the `not` params to that resultset. This means that things inside the `not` param - # do not take precedence over the outer params with the same name. - context 'shadowing the same outside param' do - let(:params) { { label_name: label2.title, not: { label_name: label.title } } } + # IssuableFinder first filters using the outer params (the ones not inside the `not` key.) + # Afterwards, it applies the `not` params to that resultset. This means that things inside the `not` param + # do not take precedence over the outer params with the same name. + context 'shadowing the same outside param' do + let(:params) { { label_name: label2.title, not: { label_name: label.title } } } - it 'does not take precedence over labels outside NOT' do - expect(issues).to contain_exactly(issue3) + it 'does not take precedence over labels outside NOT' do + expect(issues).to contain_exactly(issue3) + end end - end - context 'further filtering outside params' do - let(:params) { { label_name: label2.title, not: { assignee_username: user2.username } } } + context 'further filtering outside params' do + let(:params) { { label_name: label2.title, not: { assignee_username: user2.username } } } - it 'further filters on the returned resultset' do - expect(issues).to be_empty + it 'further filters on the returned resultset' do + expect(issues).to be_empty + end end end end - end - context 'filtering by multiple labels' do - let(:params) { { label_name: [label.title, label2.title].join(',') } } - let(:label2) { create(:label, project: project2) } + context 'filtering by multiple labels' do + let(:params) { { label_name: [label.title, label2.title].join(',') } } + let(:label2) { create(:label, project: project2) } - before do - create(:label_link, label: label2, target: issue2) - end + before do + create(:label_link, label: label2, target: issue2) + end - it 'returns the unique issues with all those labels' do - expect(issues).to contain_exactly(issue2) - end + it 'returns the unique issues with all those labels' do + expect(issues).to contain_exactly(issue2) + end - context 'using NOT' do - let(:params) { { not: { label_name: [label.title, label2.title].join(',') } } } + context 'using NOT' do + let(:params) { { not: { label_name: [label.title, label2.title].join(',') } } } - it 'returns issues that do not have any of the labels provided' do - expect(issues).to contain_exactly(issue1, issue4) + it 'returns issues that do not have any of the labels provided' do + expect(issues).to contain_exactly(issue1, issue4) + end end end - end - context 'filtering by a label that includes any or none in the title' do - let(:params) { { label_name: [label.title, label2.title].join(',') } } - let(:label) { create(:label, title: 'any foo', project: project2) } - let(:label2) { create(:label, title: 'bar none', project: project2) } + context 'filtering by a label that includes any or none in the title' do + let(:params) { { label_name: [label.title, label2.title].join(',') } } + let(:label) { create(:label, title: 'any foo', project: project2) } + let(:label2) { create(:label, title: 'bar none', project: project2) } - before do - create(:label_link, label: label2, target: issue2) - end + before do + create(:label_link, label: label2, target: issue2) + end + + it 'returns the unique issues with all those labels' do + expect(issues).to contain_exactly(issue2) + end - it 'returns the unique issues with all those labels' do - expect(issues).to contain_exactly(issue2) + context 'using NOT' do + let(:params) { { not: { label_name: [label.title, label2.title].join(',') } } } + + it 'returns issues that do not have ANY ONE of the labels provided' do + expect(issues).to contain_exactly(issue1, issue4) + end + end end - context 'using NOT' do - let(:params) { { not: { label_name: [label.title, label2.title].join(',') } } } + context 'filtering by no label' do + let(:params) { { label_name: described_class::Params::FILTER_NONE } } - it 'returns issues that do not have ANY ONE of the labels provided' do + it 'returns issues with no labels' do expect(issues).to contain_exactly(issue1, issue4) end end - end - context 'filtering by no label' do - let(:params) { { label_name: described_class::Params::FILTER_NONE } } + context 'filtering by any label' do + let(:params) { { label_name: described_class::Params::FILTER_ANY } } + + it 'returns issues that have one or more label' do + create_list(:label_link, 2, label: create(:label, project: project2), target: issue3) - it 'returns issues with no labels' do - expect(issues).to contain_exactly(issue1, issue4) + expect(issues).to contain_exactly(issue2, issue3) + end + end + + context 'when the same label exists on project and group levels' do + let(:issue1) { create(:issue, project: project1) } + let(:issue2) { create(:issue, project: project1) } + + # Skipping validation to reproduce a "real-word" scenario. + # We still have legacy labels on PRD that have the same title on the group and project levels, example: `bug` + let(:project_label) { build(:label, title: 'somelabel', project: project1).tap { |r| r.save!(validate: false) } } + let(:group_label) { create(:group_label, title: 'somelabel', group: project1.group) } + + let(:params) { { label_name: 'somelabel' } } + + before do + create(:label_link, label: group_label, target: issue1) + create(:label_link, label: project_label, target: issue2) + end + + it 'finds both issue records' do + expect(issues).to contain_exactly(issue1, issue2) + end end end - context 'filtering by any label' do - let(:params) { { label_name: described_class::Params::FILTER_ANY } } + context 'when `optimized_issuable_label_filter` feature flag is off' do + before do + stub_feature_flags(optimized_issuable_label_filter: false) + end - it 'returns issues that have one or more label' do - create_list(:label_link, 2, label: create(:label, project: project2), target: issue3) + it_behaves_like ':label_name parameter' + end - expect(issues).to contain_exactly(issue2, issue3) + context 'when `optimized_issuable_label_filter` feature flag is on' do + before do + stub_feature_flags(optimized_issuable_label_filter: true) end + + it_behaves_like ':label_name parameter' end context 'filtering by issue term' do @@ -949,10 +989,6 @@ RSpec.describe IssuesFinder do describe '#use_cte_for_search?' do let(:finder) { described_class.new(nil, params) } - before do - stub_feature_flags(attempt_group_search_optimizations: true) - end - context 'when there is no search param' do let(:params) { { attempt_group_search_optimizations: true } } @@ -969,47 +1005,50 @@ RSpec.describe IssuesFinder do end end - context 'when the attempt_group_search_optimizations flag is disabled' do - let(:params) { { search: 'foo', attempt_group_search_optimizations: true } } + context 'when all conditions are met' do + context "uses group search optimization" do + let(:params) { { search: 'foo', attempt_group_search_optimizations: true } } - before do - stub_feature_flags(attempt_group_search_optimizations: false) + it 'returns true' do + expect(finder.use_cte_for_search?).to be_truthy + end end - it 'returns false' do - expect(finder.use_cte_for_search?).to be_falsey + context "uses project search optimization" do + let(:params) { { search: 'foo', attempt_project_search_optimizations: true } } + + it 'returns true' do + expect(finder.use_cte_for_search?).to be_truthy + end end end + end - context 'when attempt_group_search_optimizations is unset and attempt_project_search_optimizations is set' do - let(:params) { { search: 'foo', attempt_project_search_optimizations: true } } + describe '#parent_param=' do + let(:finder) { described_class.new(nil) } - context 'and the corresponding feature flag is disabled' do - before do - stub_feature_flags(attempt_project_search_optimizations: false) - end + subject { finder.parent_param = obj } - it 'returns false' do - expect(finder.use_cte_for_search?).to be_falsey - end - end + where(:klass, :param) do + :Project | :project_id + :Group | :group_id + end - context 'and the corresponding feature flag is enabled' do - before do - stub_feature_flags(attempt_project_search_optimizations: true) - end + with_them do + let(:obj) { Object.const_get(klass, false).new } - it 'returns true' do - expect(finder.use_cte_for_search?).to be_truthy - end + it 'sets the params' do + subject + + expect(finder.params[param]).to eq(obj) end end - context 'when all conditions are met' do - let(:params) { { search: 'foo', attempt_group_search_optimizations: true } } + context 'unexpected parent' do + let(:obj) { MergeRequest.new } - it 'returns true' do - expect(finder.use_cte_for_search?).to be_truthy + it 'raises an error' do + expect { subject }.to raise_error('Unexpected parent: MergeRequest') end end end diff --git a/spec/finders/members_finder_spec.rb b/spec/finders/members_finder_spec.rb index 3ef8d6a01aa..3530858e2de 100644 --- a/spec/finders/members_finder_spec.rb +++ b/spec/finders/members_finder_spec.rb @@ -45,6 +45,18 @@ RSpec.describe MembersFinder, '#execute' do expect(result).to contain_exactly(member1) end + it 'does not return members of parent group with minimal access' do + nested_group.request_access(user1) + member1 = group.add_maintainer(user2) + member2 = nested_group.add_maintainer(user3) + member3 = project.add_maintainer(user4) + create(:group_member, :minimal_access, user: create(:user), source: group) + + result = described_class.new(project, user2).execute + + expect(result).to contain_exactly(member1, member2, member3) + end + it 'includes only non-invite members if user do not have amdin permissions on project' do create(:project_member, :invited, project: project, invite_email: create(:user).email) member1 = project.add_maintainer(user1) diff --git a/spec/finders/merge_requests_finder_spec.rb b/spec/finders/merge_requests_finder_spec.rb index 5b86c891e47..4f86323c7c6 100644 --- a/spec/finders/merge_requests_finder_spec.rb +++ b/spec/finders/merge_requests_finder_spec.rb @@ -167,38 +167,56 @@ RSpec.describe MergeRequestsFinder do end end - describe ':label_name parameter' do - let(:common_labels) { create_list(:label, 3) } - let(:distinct_labels) { create_list(:label, 3) } - let(:merge_requests) do - common_attrs = { - source_project: project1, target_project: project1, author: user - } - distinct_labels.map do |label| - labels = [label, *common_labels] - create(:labeled_merge_request, :closed, labels: labels, **common_attrs) + shared_examples ':label_name parameter' do + describe ':label_name parameter' do + let(:common_labels) { create_list(:label, 3) } + let(:distinct_labels) { create_list(:label, 3) } + let(:merge_requests) do + common_attrs = { + source_project: project1, target_project: project1, author: user + } + distinct_labels.map do |label| + labels = [label, *common_labels] + create(:labeled_merge_request, :closed, labels: labels, **common_attrs) + end end - end - def find(label_name) - described_class.new(user, label_name: label_name).execute - end + def find(label_name) + described_class.new(user, label_name: label_name).execute + end + + it 'accepts a single label' do + found = find(distinct_labels.first.title) + common = find(common_labels.first.title) + + expect(found).to contain_exactly(merge_requests.first) + expect(common).to match_array(merge_requests) + end - it 'accepts a single label' do - found = find(distinct_labels.first.title) - common = find(common_labels.first.title) + it 'accepts an array of labels, all of which must match' do + all_distinct = find(distinct_labels.pluck(:title)) + all_common = find(common_labels.pluck(:title)) - expect(found).to contain_exactly(merge_requests.first) - expect(common).to match_array(merge_requests) + expect(all_distinct).to be_empty + expect(all_common).to match_array(merge_requests) + end + end + end + + context 'when `optimized_issuable_label_filter` feature flag is off' do + before do + stub_feature_flags(optimized_issuable_label_filter: false) end - it 'accepts an array of labels, all of which must match' do - all_distinct = find(distinct_labels.pluck(:title)) - all_common = find(common_labels.pluck(:title)) + it_behaves_like ':label_name parameter' + end - expect(all_distinct).to be_empty - expect(all_common).to match_array(merge_requests) + context 'when `optimized_issuable_label_filter` feature flag is on' do + before do + stub_feature_flags(optimized_issuable_label_filter: true) end + + it_behaves_like ':label_name parameter' end it 'filters by source project id' do diff --git a/spec/finders/projects_finder_spec.rb b/spec/finders/projects_finder_spec.rb index 29b6dc61386..8ae19757c25 100644 --- a/spec/finders/projects_finder_spec.rb +++ b/spec/finders/projects_finder_spec.rb @@ -234,10 +234,40 @@ RSpec.describe ProjectsFinder, :do_not_mock_admin_mode do end describe 'filter by without_deleted' do - let(:params) { { without_deleted: true } } - let!(:pending_delete_project) { create(:project, :public, pending_delete: true) } + let_it_be(:pending_delete_project) { create(:project, :public, pending_delete: true) } - it { is_expected.to match_array([public_project, internal_project]) } + let(:params) { { without_deleted: without_deleted } } + + shared_examples 'returns all projects' do + it { expect(subject).to include(public_project, internal_project, pending_delete_project) } + end + + context 'when without_deleted is true' do + let(:without_deleted) { true } + + it 'returns projects that are not pending_delete' do + expect(subject).not_to include(pending_delete_project) + expect(subject).to include(public_project, internal_project) + end + end + + context 'when without_deleted is false' do + let(:without_deleted) { false } + + it_behaves_like 'returns all projects' + end + + context 'when without_deleted is nil' do + let(:without_deleted) { nil } + + it_behaves_like 'returns all projects' + end + + context 'when without_deleted is not present' do + let(:params) { {} } + + it_behaves_like 'returns all projects' + end end describe 'filter by last_activity_after' do diff --git a/spec/finders/user_group_notification_settings_finder_spec.rb b/spec/finders/user_group_notification_settings_finder_spec.rb new file mode 100644 index 00000000000..453da691866 --- /dev/null +++ b/spec/finders/user_group_notification_settings_finder_spec.rb @@ -0,0 +1,132 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe UserGroupNotificationSettingsFinder do + let_it_be(:user) { create(:user) } + + subject { described_class.new(user, Group.where(id: groups.map(&:id))).execute } + + def attributes(&proc) + subject.map(&proc).uniq + 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 eq([nil]) + expect(attributes(&:level)).to eq(['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] } + + 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 level set' do + expect(attributes(&:level)).to eq(['participating']) + end + + it 'has the same email set' do + expect(attributes(&:notification_email)).to eq(['ancestor@example.com']) + end + + it 'only returns the two queried groups' do + expect(subject.count).to eq(2) + end + 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 eq(['global']) + expect(attributes(&:notification_email)).to eq(['ancestor@example.com']) + 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] } + + before do + # Let's set a parent_id for a group that definitely doesn't exist + group.update_columns(parent_id: 19283746) + end + + it 'returns a default Global notification setting' do + expect(subject.count).to eq(1) + expect(attributes(&:level)).to eq(['global']) + expect(attributes(&:notification_email)).to eq([nil]) + end + 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 eq(['participating']) + expect(attributes(&:notification_email)).to eq([ancestor_email.email]) + 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 + + 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 +end diff --git a/spec/finders/users_finder_spec.rb b/spec/finders/users_finder_spec.rb index 17b36247b05..a04f5452fcd 100644 --- a/spec/finders/users_finder_spec.rb +++ b/spec/finders/users_finder_spec.rb @@ -12,7 +12,7 @@ RSpec.describe UsersFinder do it 'returns all users' do users = described_class.new(user).execute - expect(users).to contain_exactly(user, normal_user, blocked_user, omniauth_user) + expect(users).to contain_exactly(user, normal_user, blocked_user, omniauth_user, internal_user) end it 'filters by username' do @@ -54,7 +54,7 @@ RSpec.describe UsersFinder do it 'returns no external users' do users = described_class.new(user, external: true).execute - expect(users).to contain_exactly(user, normal_user, blocked_user, omniauth_user) + expect(users).to contain_exactly(user, normal_user, blocked_user, omniauth_user, internal_user) end it 'filters by created_at' do @@ -68,19 +68,25 @@ RSpec.describe UsersFinder do expect(users.map(&:username)).not_to include([filtered_user_before.username, filtered_user_after.username]) end + it 'filters by non internal users' do + users = described_class.new(user, non_internal: true).execute + + expect(users).to contain_exactly(user, normal_user, blocked_user, omniauth_user) + end + it 'does not filter by custom attributes' do users = described_class.new( user, custom_attributes: { foo: 'bar' } ).execute - expect(users).to contain_exactly(user, normal_user, blocked_user, omniauth_user) + expect(users).to contain_exactly(user, normal_user, blocked_user, omniauth_user, internal_user) end it 'orders returned results' do users = described_class.new(user, sort: 'id_asc').execute - expect(users).to eq([normal_user, blocked_user, omniauth_user, user]) + expect(users).to eq([normal_user, blocked_user, omniauth_user, internal_user, user]) end end @@ -96,13 +102,14 @@ RSpec.describe UsersFinder do it 'returns all users' do users = described_class.new(admin).execute - expect(users).to contain_exactly(admin, normal_user, blocked_user, external_user, omniauth_user) + expect(users).to contain_exactly(admin, normal_user, blocked_user, external_user, omniauth_user, internal_user) end it 'filters by custom attributes' do create :user_custom_attribute, user: normal_user, key: 'foo', value: 'foo' create :user_custom_attribute, user: normal_user, key: 'bar', value: 'bar' create :user_custom_attribute, user: blocked_user, key: 'foo', value: 'foo' + create :user_custom_attribute, user: internal_user, key: 'foo', value: 'foo' users = described_class.new( admin, |