From 9dc93a4519d9d5d7be48ff274127136236a3adb3 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Tue, 20 Apr 2021 23:50:22 +0000 Subject: Add latest changes from gitlab-org/gitlab@13-11-stable-ee --- .../batch_destroy_dependent_associations_spec.rb | 1 + spec/models/concerns/cache_markdown_field_spec.rb | 10 +- spec/models/concerns/cacheable_attributes_spec.rb | 2 +- .../cascading_namespace_setting_attribute_spec.rb | 320 +++++++++++++++++++++ spec/models/concerns/ci/artifactable_spec.rb | 28 ++ spec/models/concerns/featurable_spec.rb | 3 +- spec/models/concerns/has_timelogs_report_spec.rb | 51 ++++ spec/models/concerns/issuable_spec.rb | 21 +- spec/models/concerns/milestoneable_spec.rb | 14 +- spec/models/concerns/milestoneish_spec.rb | 71 ++--- spec/models/concerns/participable_spec.rb | 71 ++++- spec/models/concerns/safe_url_spec.rb | 12 +- .../sidebars/container_with_html_options_spec.rb | 21 ++ .../concerns/sidebars/positionable_list_spec.rb | 59 ++++ spec/models/concerns/sortable_spec.rb | 25 ++ spec/models/concerns/subscribable_spec.rb | 51 +++- spec/models/concerns/token_authenticatable_spec.rb | 4 +- .../encrypted_spec.rb | 77 ++++- .../encryption_helper_spec.rb | 27 ++ 19 files changed, 788 insertions(+), 80 deletions(-) create mode 100644 spec/models/concerns/cascading_namespace_setting_attribute_spec.rb create mode 100644 spec/models/concerns/has_timelogs_report_spec.rb create mode 100644 spec/models/concerns/sidebars/container_with_html_options_spec.rb create mode 100644 spec/models/concerns/sidebars/positionable_list_spec.rb create mode 100644 spec/models/concerns/token_authenticatable_strategies/encryption_helper_spec.rb (limited to 'spec/models/concerns') diff --git a/spec/models/concerns/batch_destroy_dependent_associations_spec.rb b/spec/models/concerns/batch_destroy_dependent_associations_spec.rb index a8fcb714c64..993afd47a57 100644 --- a/spec/models/concerns/batch_destroy_dependent_associations_spec.rb +++ b/spec/models/concerns/batch_destroy_dependent_associations_spec.rb @@ -26,6 +26,7 @@ RSpec.describe BatchDestroyDependentAssociations do let_it_be(:project) { create(:project) } let_it_be(:build) { create(:ci_build, project: project) } let_it_be(:notification_setting) { create(:notification_setting, project: project) } + let!(:todos) { create(:todo, project: project) } it 'destroys multiple builds' do diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb index 6e62d4ef31b..33a4c8eac41 100644 --- a/spec/models/concerns/cache_markdown_field_spec.rb +++ b/spec/models/concerns/cache_markdown_field_spec.rb @@ -17,9 +17,13 @@ RSpec.describe CacheMarkdownField, :clean_gitlab_redis_cache do include CacheMarkdownField def initialize(args = {}) - @title, @description, @cached_markdown_version = args[:title], args[:description], args[:cached_markdown_version] - @title_html, @description_html = args[:title_html], args[:description_html] - @author, @project = args[:author], args[:project] + @title = args[:title] + @description = args[:description] + @cached_markdown_version = args[:cached_markdown_version] + @title_html = args[:title_html] + @description_html = args[:description_html] + @author = args[:author] + @project = args[:project] @parent_user = args[:parent_user] end diff --git a/spec/models/concerns/cacheable_attributes_spec.rb b/spec/models/concerns/cacheable_attributes_spec.rb index f2877bed9cf..dc80e30216a 100644 --- a/spec/models/concerns/cacheable_attributes_spec.rb +++ b/spec/models/concerns/cacheable_attributes_spec.rb @@ -205,7 +205,7 @@ RSpec.describe CacheableAttributes do end end - it 'uses RequestStore in addition to process memory cache', :request_store do + it 'uses RequestStore in addition to process memory cache', :request_store, :do_not_mock_admin_mode_setting do # Warm up the cache create(:application_setting).cache! diff --git a/spec/models/concerns/cascading_namespace_setting_attribute_spec.rb b/spec/models/concerns/cascading_namespace_setting_attribute_spec.rb new file mode 100644 index 00000000000..ddff9ce32b4 --- /dev/null +++ b/spec/models/concerns/cascading_namespace_setting_attribute_spec.rb @@ -0,0 +1,320 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe NamespaceSetting, 'CascadingNamespaceSettingAttribute' do + let(:group) { create(:group) } + let(:subgroup) { create(:group, parent: group) } + + def group_settings + group.namespace_settings + end + + def subgroup_settings + subgroup.namespace_settings + end + + describe '#delayed_project_removal' do + subject(:delayed_project_removal) { subgroup_settings.delayed_project_removal } + + context 'when the feature is disabled' do + before do + stub_feature_flags(cascading_namespace_settings: false) + + group_settings.update!(delayed_project_removal: true) + end + + it 'does not cascade' do + expect(delayed_project_removal).to eq(nil) + end + end + + context 'when there is no parent' do + context 'and the value is not nil' do + before do + group_settings.update!(delayed_project_removal: true) + end + + it 'returns the local value' do + expect(group_settings.delayed_project_removal).to eq(true) + end + end + + context 'and the value is nil' do + before do + group_settings.update!(delayed_project_removal: nil) + stub_application_setting(delayed_project_removal: false) + end + + it 'returns the application settings value' do + expect(group_settings.delayed_project_removal).to eq(false) + end + end + end + + context 'when parent does not lock the attribute' do + context 'and value is not nil' do + before do + group_settings.update!(delayed_project_removal: false) + end + + it 'returns local setting when present' do + subgroup_settings.update!(delayed_project_removal: true) + + expect(delayed_project_removal).to eq(true) + end + + it 'returns the parent value when local value is nil' do + subgroup_settings.update!(delayed_project_removal: nil) + + expect(delayed_project_removal).to eq(false) + end + + it 'returns the correct dirty value' do + subgroup_settings.delayed_project_removal = true + + expect(delayed_project_removal).to eq(true) + end + + it 'does not return the application setting value when parent value is false' do + stub_application_setting(delayed_project_removal: true) + + expect(delayed_project_removal).to eq(false) + end + end + + context 'and the value is nil' do + before do + group_settings.update!(delayed_project_removal: nil, lock_delayed_project_removal: false) + subgroup_settings.update!(delayed_project_removal: nil) + + subgroup_settings.clear_memoization(:delayed_project_removal) + end + + it 'cascades to the application settings value' do + expect(delayed_project_removal).to eq(false) + end + end + + context 'when multiple ancestors set a value' do + let(:third_level_subgroup) { create(:group, parent: subgroup) } + + before do + group_settings.update!(delayed_project_removal: true) + subgroup_settings.update!(delayed_project_removal: false) + end + + it 'returns the closest ancestor value' do + expect(third_level_subgroup.namespace_settings.delayed_project_removal).to eq(false) + end + end + end + + context 'when parent locks the attribute' do + before do + subgroup_settings.update!(delayed_project_removal: true) + group_settings.update!(lock_delayed_project_removal: true, delayed_project_removal: false) + + subgroup_settings.clear_memoization(:delayed_project_removal) + subgroup_settings.clear_memoization(:delayed_project_removal_locked_ancestor) + end + + it 'returns the parent value' do + expect(delayed_project_removal).to eq(false) + end + + it 'does not allow the local value to be saved' do + subgroup_settings.delayed_project_removal = nil + + expect { subgroup_settings.save! } + .to raise_error(ActiveRecord::RecordInvalid, /Delayed project removal cannot be changed because it is locked by an ancestor/) + end + end + + context 'when the application settings locks the attribute' do + before do + subgroup_settings.update!(delayed_project_removal: true) + stub_application_setting(lock_delayed_project_removal: true, delayed_project_removal: true) + end + + it 'returns the application setting value' do + expect(delayed_project_removal).to eq(true) + end + + it 'does not allow the local value to be saved' do + subgroup_settings.delayed_project_removal = nil + + expect { subgroup_settings.save! } + .to raise_error(ActiveRecord::RecordInvalid, /Delayed project removal cannot be changed because it is locked by an ancestor/) + end + end + end + + describe '#delayed_project_removal?' do + before do + subgroup_settings.update!(delayed_project_removal: true) + group_settings.update!(lock_delayed_project_removal: true, delayed_project_removal: false) + + subgroup_settings.clear_memoization(:delayed_project_removal) + subgroup_settings.clear_memoization(:delayed_project_removal_locked_ancestor) + end + + it 'aliases the method when the attribute is a boolean' do + expect(subgroup_settings.delayed_project_removal?).to eq(subgroup_settings.delayed_project_removal) + end + end + + describe '#delayed_project_removal_locked?' do + shared_examples 'not locked' do + it 'is not locked by an ancestor' do + expect(subgroup_settings.delayed_project_removal_locked_by_ancestor?).to eq(false) + end + + it 'is not locked by application setting' do + expect(subgroup_settings.delayed_project_removal_locked_by_application_setting?).to eq(false) + end + + it 'does not return a locked namespace' do + expect(subgroup_settings.delayed_project_removal_locked_ancestor).to be_nil + end + end + + context 'when the feature is disabled' do + before do + stub_feature_flags(cascading_namespace_settings: false) + + group_settings.update!(delayed_project_removal: true) + end + + it_behaves_like 'not locked' + end + + context 'when parent does not lock the attribute' do + it_behaves_like 'not locked' + end + + context 'when parent locks the attribute' do + before do + group_settings.update!(lock_delayed_project_removal: true, delayed_project_removal: false) + + subgroup_settings.clear_memoization(:delayed_project_removal) + subgroup_settings.clear_memoization(:delayed_project_removal_locked_ancestor) + end + + it 'is locked by an ancestor' do + expect(subgroup_settings.delayed_project_removal_locked_by_ancestor?).to eq(true) + end + + it 'is not locked by application setting' do + expect(subgroup_settings.delayed_project_removal_locked_by_application_setting?).to eq(false) + end + + it 'returns a locked namespace settings object' do + expect(subgroup_settings.delayed_project_removal_locked_ancestor.namespace_id).to eq(group_settings.namespace_id) + end + end + + context 'when not locked by application settings' do + before do + stub_application_setting(lock_delayed_project_removal: false) + end + + it_behaves_like 'not locked' + end + + context 'when locked by application settings' do + before do + stub_application_setting(lock_delayed_project_removal: true) + end + + it 'is not locked by an ancestor' do + expect(subgroup_settings.delayed_project_removal_locked_by_ancestor?).to eq(false) + end + + it 'is locked by application setting' do + expect(subgroup_settings.delayed_project_removal_locked_by_application_setting?).to eq(true) + end + + it 'does not return a locked namespace' do + expect(subgroup_settings.delayed_project_removal_locked_ancestor).to be_nil + end + end + end + + describe '#lock_delayed_project_removal=' do + context 'when parent locks the attribute' do + before do + group_settings.update!(lock_delayed_project_removal: true, delayed_project_removal: false) + + subgroup_settings.clear_memoization(:delayed_project_removal) + subgroup_settings.clear_memoization(:delayed_project_removal_locked_ancestor) + end + + it 'does not allow the attribute to be saved' do + subgroup_settings.lock_delayed_project_removal = true + + expect { subgroup_settings.save! } + .to raise_error(ActiveRecord::RecordInvalid, /Lock delayed project removal cannot be changed because it is locked by an ancestor/) + end + end + + context 'when parent does not lock the attribute' do + before do + group_settings.update!(lock_delayed_project_removal: false) + + subgroup_settings.lock_delayed_project_removal = true + end + + it 'allows the lock to be set when the attribute is not nil' do + subgroup_settings.delayed_project_removal = true + + expect(subgroup_settings.save).to eq(true) + end + + it 'does not allow the lock to be saved when the attribute is nil' do + subgroup_settings.delayed_project_removal = nil + + expect { subgroup_settings.save! } + .to raise_error(ActiveRecord::RecordInvalid, /Delayed project removal cannot be nil when locking the attribute/) + end + end + + context 'when application settings locks the attribute' do + before do + stub_application_setting(lock_delayed_project_removal: true) + end + + it 'does not allow the attribute to be saved' do + subgroup_settings.lock_delayed_project_removal = true + + expect { subgroup_settings.save! } + .to raise_error(ActiveRecord::RecordInvalid, /Lock delayed project removal cannot be changed because it is locked by an ancestor/) + end + end + + context 'when application_settings does not lock the attribute' do + before do + stub_application_setting(lock_delayed_project_removal: false) + end + + it 'allows the attribute to be saved' do + subgroup_settings.delayed_project_removal = true + subgroup_settings.lock_delayed_project_removal = true + + expect(subgroup_settings.save).to eq(true) + end + end + end + + describe 'after update callback' do + before do + subgroup_settings.update!(lock_delayed_project_removal: true, delayed_project_removal: false) + end + + it 'clears descendant locks' do + group_settings.update!(lock_delayed_project_removal: true, delayed_project_removal: true) + + expect(subgroup_settings.reload.lock_delayed_project_removal).to eq(false) + end + end +end diff --git a/spec/models/concerns/ci/artifactable_spec.rb b/spec/models/concerns/ci/artifactable_spec.rb index ebc838e86a6..62fc689a9ca 100644 --- a/spec/models/concerns/ci/artifactable_spec.rb +++ b/spec/models/concerns/ci/artifactable_spec.rb @@ -72,5 +72,33 @@ RSpec.describe Ci::Artifactable do expect(Ci::JobArtifact.expired(1).order_id_asc).to eq([recently_expired_artifact]) end end + + describe '.with_files_stored_locally' do + it 'returns artifacts stored locally' do + expect(Ci::JobArtifact.with_files_stored_locally).to contain_exactly(recently_expired_artifact, later_expired_artifact, not_expired_artifact) + end + end + + describe '.with_files_stored_remotely' do + let(:remote_artifact) { create(:ci_job_artifact, :remote_store) } + + before do + stub_artifacts_object_storage + end + + it 'returns artifacts stored remotely' do + expect(Ci::JobArtifact.with_files_stored_remotely).to contain_exactly(remote_artifact) + end + end + + describe '.project_id_in' do + context 'when artifacts belongs to projects' do + let(:project_ids) { [recently_expired_artifact.project.id, not_expired_artifact.project.id, non_existing_record_id] } + + it 'returns artifacts belonging to projects' do + expect(Ci::JobArtifact.project_id_in(project_ids)).to contain_exactly(recently_expired_artifact, not_expired_artifact) + end + end + end end end diff --git a/spec/models/concerns/featurable_spec.rb b/spec/models/concerns/featurable_spec.rb index b550d22f686..295f3523dd5 100644 --- a/spec/models/concerns/featurable_spec.rb +++ b/spec/models/concerns/featurable_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe Featurable do let_it_be(:user) { create(:user) } + let(:project) { create(:project) } let(:feature_class) { subject.class } let(:features) { feature_class::FEATURES } @@ -163,7 +164,7 @@ RSpec.describe Featurable do end def update_all_project_features(project, features, value) - project_feature_attributes = features.map { |f| ["#{f}_access_level", value] }.to_h + project_feature_attributes = features.to_h { |f| ["#{f}_access_level", value] } project.project_feature.update!(project_feature_attributes) end end diff --git a/spec/models/concerns/has_timelogs_report_spec.rb b/spec/models/concerns/has_timelogs_report_spec.rb new file mode 100644 index 00000000000..f694fc350ee --- /dev/null +++ b/spec/models/concerns/has_timelogs_report_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe HasTimelogsReport do + let(:user) { create(:user) } + let(:group) { create(:group) } + let(:issue) { create(:issue, project: create(:project, :public, group: group)) } + + describe '#timelogs' do + let!(:timelog1) { create_timelog(15.days.ago) } + let!(:timelog2) { create_timelog(10.days.ago) } + let!(:timelog3) { create_timelog(5.days.ago) } + let(:start_time) { 20.days.ago } + let(:end_time) { 8.days.ago } + + before do + group.add_developer(user) + end + + it 'returns collection of timelogs between given times' do + expect(group.timelogs(start_time, end_time).to_a).to match_array([timelog1, timelog2]) + end + + it 'returns empty collection if times are not present' do + expect(group.timelogs(nil, nil)).to be_empty + end + + it 'returns empty collection if time range is invalid' do + expect(group.timelogs(end_time, start_time)).to be_empty + end + end + + describe '#user_can_access_group_timelogs?' do + it 'returns true if user can access group timelogs' do + group.add_developer(user) + + expect(group).to be_user_can_access_group_timelogs(user) + end + + it 'returns false if user has insufficient permissions' do + group.add_guest(user) + + expect(group).not_to be_user_can_access_group_timelogs(user) + end + end + + def create_timelog(time) + create(:timelog, issue: issue, user: user, spent_at: time) + end +end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 3545c8e9686..14db9b530db 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -16,7 +16,7 @@ RSpec.describe Issuable do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:author) } it { is_expected.to have_many(:notes).dependent(:destroy) } - it { is_expected.to have_many(:todos).dependent(:destroy) } + it { is_expected.to have_many(:todos) } it { is_expected.to have_many(:labels) } it { is_expected.to have_many(:note_authors).through(:notes) } @@ -65,6 +65,23 @@ RSpec.describe Issuable do it { expect(issuable_class).to respond_to(:opened) } it { expect(issuable_class).to respond_to(:closed) } it { expect(issuable_class).to respond_to(:assigned) } + + describe '.includes_for_bulk_update' do + before do + stub_const('Example', Class.new(ActiveRecord::Base)) + + Example.class_eval do + include Issuable # adds :labels and :metrics, among others + + belongs_to :author + has_many :assignees + end + end + + it 'includes available associations' do + expect(Example.includes_for_bulk_update.includes_values).to eq([:author, :assignees, :labels, :metrics]) + end + end end describe 'author_name' do @@ -380,7 +397,7 @@ RSpec.describe Issuable do context 'user is a participant in the issue' do before do - allow(issue).to receive(:participants).with(user).and_return([user]) + allow(issue).to receive(:participant?).with(user).and_return(true) end it 'returns false when no subcription exists' do diff --git a/spec/models/concerns/milestoneable_spec.rb b/spec/models/concerns/milestoneable_spec.rb index 5fb3b39f734..961eac4710d 100644 --- a/spec/models/concerns/milestoneable_spec.rb +++ b/spec/models/concerns/milestoneable_spec.rb @@ -50,13 +50,13 @@ RSpec.describe Milestoneable do it 'returns true with a milestone from the issue project' do milestone = create(:milestone, project: project) - expect(build_milestoneable(milestone.id).milestone_available?).to be_truthy + expect(build_milestoneable(milestone.id).milestone_available?).to be(true) end it 'returns true with a milestone from the issue project group' do milestone = create(:milestone, group: group) - expect(build_milestoneable(milestone.id).milestone_available?).to be_truthy + expect(build_milestoneable(milestone.id).milestone_available?).to be(true) end it 'returns true with a milestone from the the parent of the issue project group' do @@ -64,19 +64,23 @@ RSpec.describe Milestoneable do group.update!(parent: parent) milestone = create(:milestone, group: parent) - expect(build_milestoneable(milestone.id).milestone_available?).to be_truthy + expect(build_milestoneable(milestone.id).milestone_available?).to be(true) + end + + it 'returns true with a blank milestone' do + expect(build_milestoneable('').milestone_available?).to be(true) end it 'returns false with a milestone from another project' do milestone = create(:milestone) - expect(build_milestoneable(milestone.id).milestone_available?).to be_falsey + expect(build_milestoneable(milestone.id).milestone_available?).to be(false) end it 'returns false with a milestone from another group' do milestone = create(:milestone, group: create(:group)) - expect(build_milestoneable(milestone.id).milestone_available?).to be_falsey + expect(build_milestoneable(milestone.id).milestone_available?).to be(false) end end end diff --git a/spec/models/concerns/milestoneish_spec.rb b/spec/models/concerns/milestoneish_spec.rb index 3b8fc465421..46a876f34e9 100644 --- a/spec/models/concerns/milestoneish_spec.rb +++ b/spec/models/concerns/milestoneish_spec.rb @@ -2,30 +2,28 @@ require 'spec_helper' -RSpec.describe Milestone, 'Milestoneish' do - let(:author) { create(:user) } - let(:assignee) { create(:user) } - let(:non_member) { create(:user) } - let(:member) { create(:user) } - let(:guest) { create(:user) } - let(:admin) { create(:admin) } - let(:project) { create(:project, :public) } - let(:milestone) { create(:milestone, project: project) } - let(:label1) { create(:label, project: project) } - let(:label2) { create(:label, project: project) } - let!(:issue) { create(:issue, project: project, milestone: milestone, assignees: [member], labels: [label1]) } - let!(:security_issue_1) { create(:issue, :confidential, project: project, author: author, milestone: milestone, labels: [label2]) } - let!(:security_issue_2) { create(:issue, :confidential, project: project, assignees: [assignee], milestone: milestone) } - let!(:closed_issue_1) { create(:issue, :closed, project: project, milestone: milestone) } - let!(:closed_issue_2) { create(:issue, :closed, project: project, milestone: milestone) } - let!(:closed_security_issue_1) { create(:issue, :confidential, :closed, project: project, author: author, milestone: milestone) } - let!(:closed_security_issue_2) { create(:issue, :confidential, :closed, project: project, assignees: [assignee], milestone: milestone) } - let!(:closed_security_issue_3) { create(:issue, :confidential, :closed, project: project, author: author, milestone: milestone) } - let!(:closed_security_issue_4) { create(:issue, :confidential, :closed, project: project, assignees: [assignee], milestone: milestone) } - let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, milestone: milestone) } - let(:label_1) { create(:label, title: 'label_1', project: project, priority: 1) } - let(:label_2) { create(:label, title: 'label_2', project: project, priority: 2) } - let(:label_3) { create(:label, title: 'label_3', project: project) } +RSpec.describe Milestone, 'Milestoneish', factory_default: :keep do + let_it_be(:author) { create(:user) } + let_it_be(:assignee) { create(:user) } + let_it_be(:non_member) { create(:user) } + let_it_be(:member) { create(:user) } + let_it_be(:guest) { create(:user) } + let_it_be(:admin) { create(:admin) } + let_it_be(:project, reload: true) { create_default(:project, :public, :empty_repo).freeze } + let_it_be(:milestone, refind: true) { create_default(:milestone, project: project) } + let_it_be(:label1) { create(:label) } + let_it_be(:label2) { create(:label) } + let_it_be(:issue, reload: true) { create(:issue, milestone: milestone, assignees: [member], labels: [label1]) } + let_it_be(:security_issue_1, reload: true) { create(:issue, :confidential, author: author, milestone: milestone, labels: [label2]) } + let_it_be(:security_issue_2, reload: true) { create(:issue, :confidential, assignees: [assignee], milestone: milestone) } + let_it_be(:closed_issue_1, reload: true) { create(:issue, :closed, milestone: milestone) } + let_it_be(:closed_issue_2, reload: true) { create(:issue, :closed, milestone: milestone) } + let_it_be(:closed_security_issue_1, reload: true) { create(:issue, :confidential, :closed, author: author, milestone: milestone) } + let_it_be(:closed_security_issue_2, reload: true) { create(:issue, :confidential, :closed, assignees: [assignee], milestone: milestone) } + let_it_be(:merge_request) { create(:merge_request, source_project: project, target_project: project, milestone: milestone) } + let_it_be(:label_1) { create(:label, title: 'label_1', priority: 1) } + let_it_be(:label_2) { create(:label, title: 'label_2', priority: 2) } + let_it_be(:label_3) { create(:label, title: 'label_3') } before do project.add_developer(member) @@ -63,7 +61,7 @@ RSpec.describe Milestone, 'Milestoneish' do end end - context 'attributes visibility' do + context 'with attributes visibility' do using RSpec::Parameterized::TableSyntax let(:users) do @@ -167,8 +165,6 @@ RSpec.describe Milestone, 'Milestoneish' do end describe '#merge_requests_visible_to_user' do - let(:merge_request) { create(:merge_request, source_project: project, milestone: milestone) } - context 'when project is private' do before do project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) @@ -211,10 +207,11 @@ RSpec.describe Milestone, 'Milestoneish' do end context 'when milestone is at parent level group' do - let(:parent_group) { create(:group) } - let(:group) { create(:group, parent: parent_group) } - let(:project) { create(:project, namespace: group) } - let(:milestone) { create(:milestone, group: parent_group) } + let_it_be(:parent_group) { create(:group) } + let_it_be(:group) { create(:group, parent: parent_group) } + let_it_be(:project) { create(:project, :empty_repo, namespace: group) } + let_it_be(:milestone) { create(:milestone, group: parent_group) } + let_it_be(:merge_request) { create(:merge_request, source_project: project, milestone: milestone) } it 'does not return any merge request for a non member' do merge_requests = milestone.merge_requests_visible_to_user(non_member) @@ -243,7 +240,7 @@ RSpec.describe Milestone, 'Milestoneish' do end describe '#percent_complete', :use_clean_rails_memory_store_caching do - context 'division by zero' do + context 'with division by zero' do let(:new_milestone) { build_stubbed(:milestone) } it { expect(new_milestone.percent_complete).to eq(0) } @@ -252,13 +249,19 @@ RSpec.describe Milestone, 'Milestoneish' do describe '#closed_issues_count' do it 'counts all closed issues including confidential' do - expect(milestone.closed_issues_count).to eq 6 + expect(milestone.closed_issues_count).to eq 4 end end describe '#total_issues_count' do it 'counts all issues including confidential' do - expect(milestone.total_issues_count).to eq 9 + expect(milestone.total_issues_count).to eq 7 + end + end + + describe '#total_merge_requests_count' do + it 'counts merge requests' do + expect(milestone.total_merge_requests_count).to eq 1 end end diff --git a/spec/models/concerns/participable_spec.rb b/spec/models/concerns/participable_spec.rb index 3376e337dc9..903c7ae16b6 100644 --- a/spec/models/concerns/participable_spec.rb +++ b/spec/models/concerns/participable_spec.rb @@ -39,11 +39,12 @@ RSpec.describe Participable do expect(participants).to include(user3) end - it 'caches the raw list of participants' do + it 'caches the list of filtered participants' do instance = model.new user1 = build(:user) - expect(instance).to receive(:raw_participants).once + expect(instance).to receive(:all_participants_hash).once.and_return({}) + expect(instance).to receive(:filter_by_ability).once instance.participants(user1) instance.participants(user1) @@ -91,5 +92,71 @@ RSpec.describe Participable do expect(ext_arg).to be_an_instance_of(Gitlab::ReferenceExtractor) end end + + context 'participable is a personal snippet' do + let(:model) { PersonalSnippet } + let(:instance) { model.new(author: user1) } + + let(:user1) { build(:user) } + let(:user2) { build(:user) } + let(:user3) { build(:user) } + + before do + allow(model).to receive(:participant_attrs).and_return([:foo, :bar]) + end + + it 'returns the list of participants' do + expect(instance).to receive(:foo).and_return(user1) + expect(instance).to receive(:bar).and_return(user2) + + participants = instance.participants(user1) + expect(participants).to contain_exactly(user1) + end + end + end + + describe '#participant?' do + let(:instance) { model.new } + + let(:user1) { build(:user) } + let(:user2) { build(:user) } + let(:user3) { build(:user) } + let(:project) { build(:project, :public) } + + before do + allow(model).to receive(:participant_attrs).and_return([:foo, :bar]) + end + + it 'returns whether the user is a participant' do + allow(instance).to receive(:foo).and_return(user2) + allow(instance).to receive(:bar).and_return(user3) + allow(instance).to receive(:project).and_return(project) + + expect(instance.participant?(user1)).to be false + expect(instance.participant?(user2)).to be true + expect(instance.participant?(user3)).to be true + end + + it 'caches the list of raw participants' do + expect(instance).to receive(:raw_participants).once.and_return([]) + expect(instance).to receive(:project).twice.and_return(project) + + instance.participant?(user1) + instance.participant?(user1) + end + + context 'participable is a personal snippet' do + let(:model) { PersonalSnippet } + let(:instance) { model.new(author: user1) } + + it 'returns whether the user is a participant' do + allow(instance).to receive(:foo).and_return(user1) + allow(instance).to receive(:bar).and_return(user2) + + expect(instance.participant?(user1)).to be true + expect(instance.participant?(user2)).to be false + expect(instance.participant?(user3)).to be false + end + end end end diff --git a/spec/models/concerns/safe_url_spec.rb b/spec/models/concerns/safe_url_spec.rb index 3d38c05bf11..c298e56b1b1 100644 --- a/spec/models/concerns/safe_url_spec.rb +++ b/spec/models/concerns/safe_url_spec.rb @@ -26,14 +26,16 @@ RSpec.describe SafeUrl do context 'when URL contains credentials' do let(:url) { 'http://foo:bar@example.com' } - it { is_expected.to eq('http://*****:*****@example.com')} + it 'masks username and password' do + is_expected.to eq('http://*****:*****@example.com') + end - context 'when username is whitelisted' do - subject { test_class.safe_url(usernames_whitelist: usernames_whitelist) } + context 'when username is allowed' do + subject { test_class.safe_url(allowed_usernames: usernames) } - let(:usernames_whitelist) { %w[foo] } + let(:usernames) { %w[foo] } - it 'does expect the whitelisted username not to be masked' do + it 'masks the password, but not the username' do is_expected.to eq('http://foo:*****@example.com') end end diff --git a/spec/models/concerns/sidebars/container_with_html_options_spec.rb b/spec/models/concerns/sidebars/container_with_html_options_spec.rb new file mode 100644 index 00000000000..cc83fc84113 --- /dev/null +++ b/spec/models/concerns/sidebars/container_with_html_options_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Sidebars::ContainerWithHtmlOptions do + subject do + Class.new do + include Sidebars::ContainerWithHtmlOptions + + def title + 'Foo' + end + end.new + end + + describe '#container_html_options' do + it 'includes by default aria-label attribute' do + expect(subject.container_html_options).to eq(aria: { label: 'Foo' }) + end + end +end diff --git a/spec/models/concerns/sidebars/positionable_list_spec.rb b/spec/models/concerns/sidebars/positionable_list_spec.rb new file mode 100644 index 00000000000..231aa5295dd --- /dev/null +++ b/spec/models/concerns/sidebars/positionable_list_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Sidebars::PositionableList do + subject do + Class.new do + include Sidebars::PositionableList + end.new + end + + describe '#add_element' do + it 'adds the element to the last position of the list' do + list = [1, 2] + + subject.add_element(list, 3) + + expect(list).to eq([1, 2, 3]) + end + end + + describe '#insert_element_before' do + let(:user) { build(:user) } + let(:list) { [1, user] } + + it 'adds element before the specific element class' do + subject.insert_element_before(list, User, 2) + + expect(list).to eq [1, 2, user] + end + + context 'when reference element does not exist' do + it 'adds the element to the top of the list' do + subject.insert_element_before(list, Project, 2) + + expect(list).to eq [2, 1, user] + end + end + end + + describe '#insert_element_after' do + let(:user) { build(:user) } + let(:list) { [1, user] } + + it 'adds element after the specific element class' do + subject.insert_element_after(list, Integer, 2) + + expect(list).to eq [1, 2, user] + end + + context 'when reference element does not exist' do + it 'adds the element to the end of the list' do + subject.insert_element_after(list, Project, 2) + + expect(list).to eq [1, user, 2] + end + end + end +end diff --git a/spec/models/concerns/sortable_spec.rb b/spec/models/concerns/sortable_spec.rb index bbfdaeec64c..cfa00bab025 100644 --- a/spec/models/concerns/sortable_spec.rb +++ b/spec/models/concerns/sortable_spec.rb @@ -3,6 +3,31 @@ require 'spec_helper' RSpec.describe Sortable do + describe 'scopes' do + describe 'secondary ordering by id' do + let(:sorted_relation) { Group.all.order_created_asc } + + def arel_orders(relation) + relation.arel.orders + end + + it 'allows secondary ordering by id ascending' do + orders = arel_orders(sorted_relation.with_order_id_asc) + + expect(orders.map { |arel| arel.expr.name }).to eq(%w(created_at id)) + expect(orders).to all(be_kind_of(Arel::Nodes::Ascending)) + end + + it 'allows secondary ordering by id descending' do + orders = arel_orders(sorted_relation.with_order_id_desc) + + expect(orders.map { |arel| arel.expr.name }).to eq(%w(created_at id)) + expect(orders.first).to be_kind_of(Arel::Nodes::Ascending) + expect(orders.last).to be_kind_of(Arel::Nodes::Descending) + end + end + end + describe '.order_by' do let(:arel_table) { Group.arel_table } let(:relation) { Group.all } diff --git a/spec/models/concerns/subscribable_spec.rb b/spec/models/concerns/subscribable_spec.rb index 3e52ca5cf63..a60a0a5e26d 100644 --- a/spec/models/concerns/subscribable_spec.rb +++ b/spec/models/concerns/subscribable_spec.rb @@ -7,50 +7,54 @@ RSpec.describe Subscribable, 'Subscribable' do let(:resource) { create(:issue, project: project) } let(:user_1) { create(:user) } - describe '#subscribed?' do + shared_examples 'returns expected values' do |method| context 'without user' do it 'returns false' do - expect(resource.subscribed?(nil, project)).to be_falsey + expect(resource.public_send(method, nil, project)).to be_falsey end end context 'without project' do it 'returns false when no subscription exists' do - expect(resource.subscribed?(user_1)).to be_falsey + expect(resource.public_send(method, user_1)).to be_falsey end - it 'returns true when a subcription exists and subscribed is true' do + it 'returns true when a subscription exists and subscribed is true' do resource.subscriptions.create!(user: user_1, subscribed: true) - expect(resource.subscribed?(user_1)).to be_truthy + expect(resource.public_send(method, user_1)).to be_truthy end - it 'returns false when a subcription exists and subscribed is false' do + it 'returns false when a subscription exists and subscribed is false' do resource.subscriptions.create!(user: user_1, subscribed: false) - expect(resource.subscribed?(user_1)).to be_falsey + expect(resource.public_send(method, user_1)).to be_falsey end end context 'with project' do it 'returns false when no subscription exists' do - expect(resource.subscribed?(user_1, project)).to be_falsey + expect(resource.public_send(method, user_1, project)).to be_falsey end - it 'returns true when a subcription exists and subscribed is true' do + it 'returns true when a subscription exists and subscribed is true' do resource.subscriptions.create!(user: user_1, project: project, subscribed: true) - expect(resource.subscribed?(user_1, project)).to be_truthy + expect(resource.public_send(method, user_1, project)).to be_truthy end - it 'returns false when a subcription exists and subscribed is false' do + it 'returns false when a subscription exists and subscribed is false' do resource.subscriptions.create!(user: user_1, project: project, subscribed: false) - expect(resource.subscribed?(user_1, project)).to be_falsey + expect(resource.public_send(method, user_1, project)).to be_falsey end end end + describe '#subscribed?' do + it_behaves_like 'returns expected values', :subscribed? + end + describe '#subscribers' do it 'returns [] when no subcribers exists' do expect(resource.subscribers(project)).to be_empty @@ -189,4 +193,27 @@ RSpec.describe Subscribable, 'Subscribable' do it_behaves_like 'setting subscriptions' end end + + describe '#lazy_subscription' do + let(:labels) { create_list(:group_label, 5) } + + before do + labels.each do |label| + create(:subscription, :group_label, user: user_1, subscribable: label) + end + end + + it 'executes only one SQL query' do + lazy_queries = ActiveRecord::QueryRecorder.new do + labels.each { |label| label.lazy_subscription(user_1) } + end + + preloaded_queries = ActiveRecord::QueryRecorder.new do + labels.each { |label| label.lazy_subscription(user_1)&.subscribed? } + end + + expect(lazy_queries.count).to eq(0) + expect(preloaded_queries.count).to eq(1) + end + end end diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb index 2df76684d71..4bdb3e0a32a 100644 --- a/spec/models/concerns/token_authenticatable_spec.rb +++ b/spec/models/concerns/token_authenticatable_spec.rb @@ -54,7 +54,7 @@ RSpec.describe ApplicationSetting, 'TokenAuthenticatable' do it 'persists new token as an encrypted string' do expect(subject).to eq settings.reload.runners_registration_token expect(settings.read_attribute('runners_registration_token_encrypted')) - .to eq Gitlab::CryptoHelper.aes256_gcm_encrypt(subject, nonce: Gitlab::CryptoHelper::AES256_GCM_IV_STATIC) + .to eq TokenAuthenticatableStrategies::EncryptionHelper.encrypt_token(subject) expect(settings).to be_persisted end @@ -243,7 +243,7 @@ RSpec.describe Ci::Build, 'TokenAuthenticatable' do it 'persists new token as an encrypted string' do build.ensure_token! - encrypted = Gitlab::CryptoHelper.aes256_gcm_encrypt(build.token, nonce: Gitlab::CryptoHelper::AES256_GCM_IV_STATIC) + encrypted = TokenAuthenticatableStrategies::EncryptionHelper.encrypt_token(build.token) expect(build.read_attribute('token_encrypted')).to eq encrypted end diff --git a/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb b/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb index 1e1cd97e410..b311e302a31 100644 --- a/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb +++ b/spec/models/concerns/token_authenticatable_strategies/encrypted_spec.rb @@ -7,6 +7,10 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do let(:instance) { double(:instance) } let(:encrypted) do + TokenAuthenticatableStrategies::EncryptionHelper.encrypt_token('my-value') + end + + let(:encrypted_with_static_iv) do Gitlab::CryptoHelper.aes256_gcm_encrypt('my-value') end @@ -15,12 +19,25 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do end describe '#find_token_authenticatable' do - context 'when using optional strategy' do + context 'when encryption is required' do + let(:options) { { encrypted: :required } } + + it 'finds the encrypted resource by cleartext' do + allow(model).to receive(:find_by) + .with('some_field_encrypted' => [encrypted, encrypted_with_static_iv]) + .and_return('encrypted resource') + + expect(subject.find_token_authenticatable('my-value')) + .to eq 'encrypted resource' + end + end + + context 'when encryption is optional' do let(:options) { { encrypted: :optional } } it 'finds the encrypted resource by cleartext' do allow(model).to receive(:find_by) - .with('some_field_encrypted' => encrypted) + .with('some_field_encrypted' => [encrypted, encrypted_with_static_iv]) .and_return('encrypted resource') expect(subject.find_token_authenticatable('my-value')) @@ -33,7 +50,7 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do .and_return('plaintext resource') allow(model).to receive(:find_by) - .with('some_field_encrypted' => encrypted) + .with('some_field_encrypted' => [encrypted, encrypted_with_static_iv]) .and_return(nil) expect(subject.find_token_authenticatable('my-value')) @@ -41,7 +58,7 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do end end - context 'when using migration strategy' do + context 'when encryption is migrating' do let(:options) { { encrypted: :migrating } } it 'finds the cleartext resource by cleartext' do @@ -65,12 +82,28 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do end describe '#get_token' do - context 'when using optional strategy' do - let(:options) { { encrypted: :optional } } + context 'when encryption is required' do + let(:options) { { encrypted: :required } } + + it 'returns decrypted token when an encrypted with static iv token is present' do + allow(instance).to receive(:read_attribute) + .with('some_field_encrypted') + .and_return(Gitlab::CryptoHelper.aes256_gcm_encrypt('my-test-value')) + + expect(subject.get_token(instance)).to eq 'my-test-value' + end + + it 'returns decrypted token when an encrypted token is present' do + allow(instance).to receive(:read_attribute) + .with('some_field_encrypted') + .and_return(encrypted) - before do - stub_feature_flags(dynamic_nonce_creation: false) + expect(subject.get_token(instance)).to eq 'my-value' end + end + + context 'when encryption is optional' do + let(:options) { { encrypted: :optional } } it 'returns decrypted token when an encrypted token is present' do allow(instance).to receive(:read_attribute) @@ -80,6 +113,14 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do expect(subject.get_token(instance)).to eq 'my-value' end + it 'returns decrypted token when an encrypted with static iv token is present' do + allow(instance).to receive(:read_attribute) + .with('some_field_encrypted') + .and_return(Gitlab::CryptoHelper.aes256_gcm_encrypt('my-test-value')) + + expect(subject.get_token(instance)).to eq 'my-test-value' + end + it 'returns the plaintext token when encrypted token is not present' do allow(instance).to receive(:read_attribute) .with('some_field_encrypted') @@ -93,7 +134,7 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do end end - context 'when using migration strategy' do + context 'when encryption is migrating' do let(:options) { { encrypted: :migrating } } it 'returns cleartext token when an encrypted token is present' do @@ -123,12 +164,22 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do end describe '#set_token' do - context 'when using optional strategy' do + context 'when encryption is required' do + let(:options) { { encrypted: :required } } + + it 'writes encrypted token and returns it' do + expect(instance).to receive(:[]=) + .with('some_field_encrypted', encrypted) + + expect(subject.set_token(instance, 'my-value')).to eq 'my-value' + end + end + context 'when encryption is optional' do let(:options) { { encrypted: :optional } } it 'writes encrypted token and removes plaintext token and returns it' do expect(instance).to receive(:[]=) - .with('some_field_encrypted', any_args) + .with('some_field_encrypted', encrypted) expect(instance).to receive(:[]=) .with('some_field', nil) @@ -136,12 +187,12 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do end end - context 'when using migration strategy' do + context 'when encryption is migrating' do let(:options) { { encrypted: :migrating } } it 'writes encrypted token and writes plaintext token' do expect(instance).to receive(:[]=) - .with('some_field_encrypted', any_args) + .with('some_field_encrypted', encrypted) expect(instance).to receive(:[]=) .with('some_field', 'my-value') diff --git a/spec/models/concerns/token_authenticatable_strategies/encryption_helper_spec.rb b/spec/models/concerns/token_authenticatable_strategies/encryption_helper_spec.rb new file mode 100644 index 00000000000..6f322a32a3b --- /dev/null +++ b/spec/models/concerns/token_authenticatable_strategies/encryption_helper_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe TokenAuthenticatableStrategies::EncryptionHelper do + let(:encrypted_token) { described_class.encrypt_token('my-value') } + + describe '.encrypt_token' do + it 'encrypts token' do + expect(encrypted_token).not_to eq('my-value') + end + end + + describe '.decrypt_token' do + it 'decrypts token with static iv' do + expect(described_class.decrypt_token(encrypted_token)).to eq('my-value') + end + + it 'decrypts token with dynamic iv' do + iv = ::Digest::SHA256.hexdigest('my-value').bytes.take(described_class::NONCE_SIZE).pack('c*') + token = Gitlab::CryptoHelper.aes256_gcm_encrypt('my-value', nonce: iv) + encrypted_token = "#{described_class::DYNAMIC_NONCE_IDENTIFIER}#{token}#{iv}" + + expect(described_class.decrypt_token(encrypted_token)).to eq('my-value') + end + end +end -- cgit v1.2.1