diff options
author | Matija Čupić <matteeyah@gmail.com> | 2018-01-05 01:38:13 +0300 |
---|---|---|
committer | Matija Čupić <matteeyah@gmail.com> | 2018-01-05 01:38:13 +0300 |
commit | 9c9f7dc639768a0d6b930ec11c050a1333df934e (patch) | |
tree | 7a54287bb3e8eec9a57223bdace347de72beb936 /spec/models | |
parent | 6fb4a533b74c861a1e533604da462efb6d309de0 (diff) | |
parent | 6f1b4dc76b4619f538b7216ad3a10ca9336d0c2b (diff) |
Merge branch 'master' into 41249-clearing-the-cache
Diffstat (limited to 'spec/models')
26 files changed, 370 insertions, 90 deletions
diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index 71aa51e1857..38fb98d4f50 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -34,19 +34,19 @@ describe Ability do end it 'returns false for a guest user' do - project.team << [user, :guest] + project.add_guest(user) expect(described_class.can_edit_note?(user, note)).to be_falsy end it 'returns false for a developer' do - project.team << [user, :developer] + project.add_developer(user) expect(described_class.can_edit_note?(user, note)).to be_falsy end it 'returns true for a master' do - project.team << [user, :master] + project.add_master(user) expect(described_class.can_edit_note?(user, note)).to be_truthy end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index a1f63a2534b..7bef798a782 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1440,7 +1440,7 @@ describe Ci::Pipeline, :mailer do end before do - project.team << [pipeline.user, Gitlab::Access::DEVELOPER] + project.add_developer(pipeline.user) pipeline.user.global_notification_setting .update(level: 'custom', failed_pipeline: true, success_pipeline: true) diff --git a/spec/models/ci/trigger_spec.rb b/spec/models/ci/trigger_spec.rb index bd9c837402f..d4b72205203 100644 --- a/spec/models/ci/trigger_spec.rb +++ b/spec/models/ci/trigger_spec.rb @@ -69,7 +69,7 @@ describe Ci::Trigger do context 'and is member of the project' do before do - project.team << [owner, :developer] + project.add_developer(owner) end it { is_expected.to eq(true) } diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index cd955a5eb69..4f02dc33cd8 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -193,8 +193,8 @@ eos let(:commiter) { create :user } before do - project.team << [commiter, :developer] - other_project.team << [commiter, :developer] + project.add_developer(commiter) + other_project.add_developer(commiter) end it 'detects issues that this commit is marked as closing' do diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb index 8b545aec7f5..c73ea6aa94c 100644 --- a/spec/models/concerns/mentionable_spec.rb +++ b/spec/models/concerns/mentionable_spec.rb @@ -62,7 +62,7 @@ describe Issue, "Mentionable" do context 'when the current user can see the issue' do before do - private_project.team << [user, Gitlab::Access::DEVELOPER] + private_project.add_developer(user) end it 'includes the reference' do @@ -107,7 +107,7 @@ describe Issue, "Mentionable" do let(:issues) { create_list(:issue, 2, project: project, author: author) } before do - project.team << [author, Gitlab::Access::DEVELOPER] + project.add_developer(author) end context 'before changes are persisted' do diff --git a/spec/models/concerns/milestoneish_spec.rb b/spec/models/concerns/milestoneish_spec.rb index 673c609f534..87bf731340f 100644 --- a/spec/models/concerns/milestoneish_spec.rb +++ b/spec/models/concerns/milestoneish_spec.rb @@ -24,8 +24,8 @@ describe Milestone, 'Milestoneish' do let(:label_3) { create(:label, title: 'label_3', project: project) } before do - project.team << [member, :developer] - project.team << [guest, :guest] + project.add_developer(member) + project.add_guest(guest) end describe '#sorted_issues' do diff --git a/spec/models/concerns/resolvable_discussion_spec.rb b/spec/models/concerns/resolvable_discussion_spec.rb index 1616c2ea985..2a2ef5a304d 100644 --- a/spec/models/concerns/resolvable_discussion_spec.rb +++ b/spec/models/concerns/resolvable_discussion_spec.rb @@ -190,7 +190,7 @@ describe Discussion, ResolvableDiscussion do context "when the signed in user can push to the project" do before do - subject.project.team << [current_user, :master] + subject.project.add_master(current_user) end it "returns true" do diff --git a/spec/models/diff_discussion_spec.rb b/spec/models/diff_discussion_spec.rb index fa02434b0fd..50b19000799 100644 --- a/spec/models/diff_discussion_spec.rb +++ b/spec/models/diff_discussion_spec.rb @@ -47,8 +47,20 @@ describe DiffDiscussion do diff_note.save! end - it 'returns the diff ID for the version to show' do - expect(subject.merge_request_version_params).to eq(diff_id: merge_request_diff1.id) + context 'when commit_id is not present' do + it 'returns the diff ID for the version to show' do + expect(subject.merge_request_version_params).to eq(diff_id: merge_request_diff1.id) + end + end + + context 'when commit_id is present' do + before do + diff_note.update_attribute(:commit_id, 'commit_123') + end + + it 'includes the commit_id in the result' do + expect(subject.merge_request_version_params).to eq(diff_id: merge_request_diff1.id, commit_id: 'commit_123') + end end end @@ -70,8 +82,20 @@ describe DiffDiscussion do diff_note.save! end - it 'returns the diff ID and start sha of the versions to compare' do - expect(subject.merge_request_version_params).to eq(diff_id: merge_request_diff3.id, start_sha: merge_request_diff1.head_commit_sha) + context 'when commit_id is not present' do + it 'returns the diff ID and start sha of the versions to compare' do + expect(subject.merge_request_version_params).to eq(diff_id: merge_request_diff3.id, start_sha: merge_request_diff1.head_commit_sha) + end + end + + context 'when commit_id is present' do + before do + diff_note.update_attribute(:commit_id, 'commit_123') + end + + it 'includes the commit_id in the result' do + expect(subject.merge_request_version_params).to eq(diff_id: merge_request_diff3.id, start_sha: merge_request_diff1.head_commit_sha, commit_id: 'commit_123') + end end end @@ -83,8 +107,20 @@ describe DiffDiscussion do diff_note.save! end - it 'returns nil' do - expect(subject.merge_request_version_params).to be_nil + context 'when commit_id is not present' do + it 'returns empty hash' do + expect(subject.merge_request_version_params).to eq(nil) + end + end + + context 'when commit_id is present' do + before do + diff_note.update_attribute(:commit_id, 'commit_123') + end + + it 'returns the commit_id' do + expect(subject.merge_request_version_params).to eq(commit_id: 'commit_123') + end end end end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index aa7a8342a4c..67f49348acb 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -125,8 +125,8 @@ describe Event do let(:event) { described_class.new(project: project, target: target, author_id: author.id) } before do - project.team << [member, :developer] - project.team << [guest, :guest] + project.add_developer(member) + project.add_guest(guest) end context 'commit note event' do @@ -347,6 +347,22 @@ describe Event do end end + describe '#target' do + it 'eager loads the author of an event target' do + create(:closed_issue_event) + + events = described_class.preload(:target).all.to_a + count = ActiveRecord::QueryRecorder + .new { events.first.target.author }.count + + # This expectation exists to make sure the test doesn't pass when the + # author is for some reason not loaded at all. + expect(events.first.target.author).to be_an_instance_of(User) + + expect(count).to be_zero + end + end + def create_push_event(project, user) event = create(:push_event, project: project, author: user) diff --git a/spec/models/generic_commit_status_spec.rb b/spec/models/generic_commit_status_spec.rb index 7f1909710d8..673049d1cc4 100644 --- a/spec/models/generic_commit_status_spec.rb +++ b/spec/models/generic_commit_status_spec.rb @@ -43,7 +43,7 @@ describe GenericCommitStatus do context 'when user has ability to see datails' do before do - project.team << [user, :developer] + project.add_developer(user) end it 'details path points to an external URL' do diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb index 431e3db9f00..0e965f541d8 100644 --- a/spec/models/hooks/system_hook_spec.rb +++ b/spec/models/hooks/system_hook_spec.rb @@ -62,7 +62,7 @@ describe SystemHook do end it "project_create hook" do - project.team << [user, :master] + project.add_master(user) expect(WebMock).to have_requested(:post, system_hook.url).with( body: /user_add_to_team/, @@ -71,7 +71,7 @@ describe SystemHook do end it "project_destroy hook" do - project.team << [user, :master] + project.add_master(user) project.project_members.destroy_all expect(WebMock).to have_requested(:post, system_hook.url).with( diff --git a/spec/models/issue_collection_spec.rb b/spec/models/issue_collection_spec.rb index 34d98a3c975..580a98193af 100644 --- a/spec/models/issue_collection_spec.rb +++ b/spec/models/issue_collection_spec.rb @@ -42,7 +42,7 @@ describe IssueCollection do context 'using a user that has reporter access to the project' do it 'returns the issues of the project' do - project.team << [user, :reporter] + project.add_reporter(user) expect(collection.updatable_by_user(user)).to eq([issue1, issue2]) end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 0ea287d007a..5ced000cdb6 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -23,6 +23,32 @@ describe Issue do it { is_expected.to have_db_index(:deleted_at) } end + describe 'callbacks' do + describe '#ensure_metrics' do + it 'creates metrics after saving' do + issue = create(:issue) + + expect(issue.metrics).to be_persisted + expect(Issue::Metrics.count).to eq(1) + end + + it 'does not create duplicate metrics for an issue' do + issue = create(:issue) + + issue.close! + + expect(issue.metrics).to be_persisted + expect(Issue::Metrics.count).to eq(1) + end + + it 'records current metrics' do + expect_any_instance_of(Issue::Metrics).to receive(:record!) + + create(:issue) + end + end + end + describe '#order_by_position_and_priority' do let(:project) { create :project } let(:p1) { create(:label, title: 'P1', project: project, priority: 1) } @@ -238,7 +264,7 @@ describe Issue do let(:issue) { create(:issue, project: project) } before do - project.team << [user, :reporter] + project.add_reporter(user) end it { is_expected.to eq true } @@ -254,7 +280,7 @@ describe Issue do context 'destination project allowed' do before do - to_project.team << [user, :reporter] + to_project.add_reporter(user) end it { is_expected.to eq true } @@ -262,7 +288,7 @@ describe Issue do context 'destination project not allowed' do before do - to_project.team << [user, :guest] + to_project.add_guest(user) end it { is_expected.to eq false } @@ -550,7 +576,7 @@ describe Issue do context 'when the user is the project owner' do before do - project.team << [user, :master] + project.add_master(user) end it 'returns true for a regular issue' do @@ -574,7 +600,7 @@ describe Issue do context 'using a public project' do before do - project.team << [user, Gitlab::Access::DEVELOPER] + project.add_developer(user) end it 'returns true for a regular issue' do @@ -594,7 +620,7 @@ describe Issue do let(:project) { create(:project, :internal) } before do - project.team << [user, Gitlab::Access::DEVELOPER] + project.add_developer(user) end it 'returns true for a regular issue' do @@ -614,7 +640,7 @@ describe Issue do let(:project) { create(:project, :private) } before do - project.team << [user, Gitlab::Access::DEVELOPER] + project.add_developer(user) end it 'returns true for a regular issue' do diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 0a017c068ad..6aa0e7f49c3 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -62,12 +62,12 @@ describe Member do @owner_user = create(:user).tap { |u| group.add_owner(u) } @owner = group.members.find_by(user_id: @owner_user.id) - @master_user = create(:user).tap { |u| project.team << [u, :master] } + @master_user = create(:user).tap { |u| project.add_master(u) } @master = project.members.find_by(user_id: @master_user.id) @blocked_user = create(:user).tap do |u| - project.team << [u, :master] - project.team << [u, :developer] + project.add_master(u) + project.add_developer(u) u.block! end @@ -527,7 +527,7 @@ describe Member do it "refreshes user's authorized projects" do project = create(:project, :private) user = create(:user) - member = project.team << [user, :reporter] + member = project.add_reporter(user) member.destroy diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index fa3e80ba062..3e46fa36375 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -88,8 +88,8 @@ describe ProjectMember do @user_1 = create :user @user_2 = create :user - @project_1.team << [@user_1, :developer] - @project_2.team << [@user_2, :reporter] + @project_1.add_developer(@user_1) + @project_2.add_reporter(@user_2) @status = @project_2.team.import(@project_1) end @@ -136,8 +136,8 @@ describe ProjectMember do @user_1 = create :user @user_2 = create :user - @project_1.team << [@user_1, :developer] - @project_2.team << [@user_2, :reporter] + @project_1.add_developer(@user_1) + @project_2.add_reporter(@user_2) described_class.truncate_teams([@project_1.id, @project_2.id]) end diff --git a/spec/models/merge_request/metrics_spec.rb b/spec/models/merge_request/metrics_spec.rb index 9353d5c3c8a..02ff7839739 100644 --- a/spec/models/merge_request/metrics_spec.rb +++ b/spec/models/merge_request/metrics_spec.rb @@ -1,16 +1,11 @@ require 'spec_helper' describe MergeRequest::Metrics do - subject { create(:merge_request) } + subject { described_class.new } - describe "when recording the default set of metrics on merge request save" do - it "records the merge time" do - time = Time.now - Timecop.freeze(time) { subject.mark_as_merged } - metrics = subject.metrics - - expect(metrics).to be_present - expect(metrics.merged_at).to be_like_time(time) - end + describe 'associations' do + it { is_expected.to belong_to(:merge_request) } + it { is_expected.to belong_to(:latest_closed_by).class_name('User') } + it { is_expected.to belong_to(:merged_by).class_name('User') } end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index bb63abd167b..d8ebd46faab 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -65,6 +65,25 @@ describe MergeRequest do end end + describe 'callbacks' do + describe '#ensure_merge_request_metrics' do + it 'creates metrics after saving' do + merge_request = create(:merge_request) + + expect(merge_request.metrics).to be_persisted + expect(MergeRequest::Metrics.count).to eq(1) + end + + it 'does not duplicate metrics for a merge request' do + merge_request = create(:merge_request) + + merge_request.mark_as_merged! + + expect(MergeRequest::Metrics.count).to eq(1) + end + end + end + describe 'respond to' do it { is_expected.to respond_to(:unchecked?) } it { is_expected.to respond_to(:can_be_merged?) } @@ -195,7 +214,7 @@ describe MergeRequest do describe '#cache_merge_request_closes_issues!' do before do - subject.project.team << [subject.author, :developer] + subject.project.add_developer(subject.author) subject.target_branch = subject.project.default_branch end @@ -481,7 +500,7 @@ describe MergeRequest do let(:commit2) { double('commit2', safe_message: "Fixes #{issue1.to_reference}") } before do - subject.project.team << [subject.author, :developer] + subject.project.add_developer(subject.author) allow(subject).to receive(:commits).and_return([commit0, commit1, commit2]) end @@ -509,7 +528,7 @@ describe MergeRequest do let(:commit) { double('commit', safe_message: "Fixes #{closing_issue.to_reference}") } it 'detects issues mentioned in description but not closed' do - subject.project.team << [subject.author, :developer] + subject.project.add_developer(subject.author) subject.description = "Is related to #{mentioned_issue.to_reference} and #{closing_issue.to_reference}" allow(subject).to receive(:commits).and_return([commit]) @@ -521,7 +540,7 @@ describe MergeRequest do context 'when the project has an external issue tracker' do before do - subject.project.team << [subject.author, :developer] + subject.project.add_developer(subject.author) commit = double(:commit, safe_message: 'Fixes TEST-3') create(:jira_service, project: subject.project) @@ -660,7 +679,7 @@ describe MergeRequest do it 'includes its closed issues in the body' do issue = create(:issue, project: subject.project) - subject.project.team << [subject.author, :developer] + subject.project.add_developer(subject.author) subject.description = "This issue Closes #{issue.to_reference}" allow(subject.project).to receive(:default_branch) @@ -1688,7 +1707,7 @@ describe MergeRequest do let(:mr_sha) { merge_request.diff_head_sha } before do - project.team << [developer, :developer] + project.add_developer(developer) end context 'when autocomplete_precheck is set to true' do diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index b7c6286fd83..0678cae9b93 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -203,7 +203,7 @@ describe Namespace do context 'with subgroups' do let(:parent) { create(:group, name: 'parent', path: 'parent') } let(:child) { create(:group, name: 'child', path: 'child', parent: parent) } - let!(:project) { create(:project_empty_repo, path: 'the-project', namespace: child) } + let!(:project) { create(:project_empty_repo, path: 'the-project', namespace: child, skip_disk_validation: true) } let(:uploads_dir) { File.join(CarrierWave.root, FileUploader.base_dir) } let(:pages_dir) { File.join(TestEnv.pages_path) } @@ -240,6 +240,20 @@ describe Namespace do end end end + + it 'updates project full path in .git/config for each project inside namespace' do + parent = create(:group, name: 'mygroup', path: 'mygroup') + subgroup = create(:group, name: 'mysubgroup', path: 'mysubgroup', parent: parent) + project_in_parent_group = create(:project, :repository, namespace: parent, name: 'foo1') + hashed_project_in_subgroup = create(:project, :repository, :hashed, namespace: subgroup, name: 'foo2') + legacy_project_in_subgroup = create(:project, :repository, namespace: subgroup, name: 'foo3') + + parent.update(path: 'mygroup_new') + + expect(project_in_parent_group.repo.config['gitlab.fullpath']).to eq "mygroup_new/#{project_in_parent_group.path}" + expect(hashed_project_in_subgroup.repo.config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{hashed_project_in_subgroup.path}" + expect(legacy_project_in_subgroup.repo.config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{legacy_project_in_subgroup.path}" + end end describe '#rm_dir', 'callback' do diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index cefbf60b28c..3d030927036 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -195,7 +195,7 @@ describe Note do describe "cross_reference_not_visible_for?" do let(:private_user) { create(:user) } - let(:private_project) { create(:project, namespace: private_user.namespace) { |p| p.team << [private_user, :master] } } + let(:private_project) { create(:project, namespace: private_user.namespace) { |p| p.add_master(private_user) } } let(:private_issue) { create(:issue, project: private_project) } let(:ext_proj) { create(:project, :public) } diff --git a/spec/models/project_feature_spec.rb b/spec/models/project_feature_spec.rb index de3e86b627f..63c6fbda3f2 100644 --- a/spec/models/project_feature_spec.rb +++ b/spec/models/project_feature_spec.rb @@ -37,7 +37,7 @@ describe ProjectFeature do end it "returns true when user is a team member" do - project.team << [user, :developer] + project.add_developer(user) features.each do |feature| project.project_feature.update_attribute("#{feature}_access_level".to_sym, ProjectFeature::PRIVATE) diff --git a/spec/models/project_services/kubernetes_service_spec.rb b/spec/models/project_services/kubernetes_service_spec.rb index f037ee77a94..6980ba335b8 100644 --- a/spec/models/project_services/kubernetes_service_spec.rb +++ b/spec/models/project_services/kubernetes_service_spec.rb @@ -52,12 +52,75 @@ describe KubernetesService, :use_clean_rails_memory_store_caching do context 'when service is inactive' do before do + subject.project = project subject.active = false end it { is_expected.not_to validate_presence_of(:api_url) } it { is_expected.not_to validate_presence_of(:token) } end + + context 'with a deprecated service' do + let(:kubernetes_service) { create(:kubernetes_service) } + + before do + kubernetes_service.update_attribute(:active, false) + kubernetes_service.properties[:namespace] = "foo" + end + + it 'should not update attributes' do + expect(kubernetes_service.save).to be_falsy + end + + it 'should include an error with a deprecation message' do + kubernetes_service.valid? + expect(kubernetes_service.errors[:base].first).to match(/Kubernetes service integration has been deprecated/) + end + end + + context 'with a non-deprecated service' do + let(:kubernetes_service) { create(:kubernetes_service) } + + it 'should update attributes' do + kubernetes_service.properties[:namespace] = 'foo' + expect(kubernetes_service.save).to be_truthy + end + end + + context 'with an active and deprecated service' do + let(:kubernetes_service) { create(:kubernetes_service) } + + before do + kubernetes_service.active = false + kubernetes_service.properties[:namespace] = 'foo' + kubernetes_service.save + end + + it 'should deactive the service' do + expect(kubernetes_service.active?).to be_falsy + end + + it 'should not include a deprecation message as error' do + expect(kubernetes_service.errors.messages.count).to eq(0) + end + + it 'should update attributes' do + expect(kubernetes_service.properties[:namespace]).to eq("foo") + end + end + + context 'with a template service' do + let(:kubernetes_service) { create(:kubernetes_service, template: true, active: false) } + + before do + kubernetes_service.properties[:namespace] = 'foo' + end + + it 'should update attributes' do + expect(kubernetes_service.save).to be_truthy + expect(kubernetes_service.properties[:namespace]).to eq('foo') + end + end end describe '#initialize_properties' do @@ -318,4 +381,42 @@ describe KubernetesService, :use_clean_rails_memory_store_caching do it { is_expected.to eq(pods: []) } end end + + describe "#deprecated?" do + let(:kubernetes_service) { create(:kubernetes_service) } + + context 'with an active kubernetes service' do + it 'should return false' do + expect(kubernetes_service.deprecated?).to be_falsy + end + end + + context 'with a inactive kubernetes service' do + it 'should return true' do + kubernetes_service.update_attribute(:active, false) + expect(kubernetes_service.deprecated?).to be_truthy + end + end + end + + describe "#deprecation_message" do + let(:kubernetes_service) { create(:kubernetes_service) } + + it 'should indicate the service is deprecated' do + expect(kubernetes_service.deprecation_message).to match(/Kubernetes service integration has been deprecated/) + end + + context 'if the services is active' do + it 'should return a message' do + expect(kubernetes_service.deprecation_message).to match(/Your cluster information on this page is still editable/) + end + end + + context 'if the service is not active' do + it 'should return a message' do + kubernetes_service.update_attribute(:active, false) + expect(kubernetes_service.deprecation_message).to match(/Fields on this page are now uneditable/) + end + end + end end diff --git a/spec/models/project_services/pipelines_email_service_spec.rb b/spec/models/project_services/pipelines_email_service_spec.rb index be07ca2d945..75ae2207910 100644 --- a/spec/models/project_services/pipelines_email_service_spec.rb +++ b/spec/models/project_services/pipelines_email_service_spec.rb @@ -37,7 +37,7 @@ describe PipelinesEmailService, :mailer do let(:user) { create(:user) } before do - project.team << [user, :developer] + project.add_developer(user) end it 'builds test data' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index cbeac2f05d3..cea22bbd184 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -93,7 +93,7 @@ describe Project do let(:developer) { create(:user) } before do project.request_access(requester) - project.team << [developer, :developer] + project.add_developer(developer) end it_behaves_like 'members and requesters associations' do @@ -520,7 +520,7 @@ describe Project do let(:user) { create(:user) } before do - project.team << [user, :developer] + project.add_developer(user) end context 'with default issues tracker' do @@ -1435,35 +1435,35 @@ describe Project do let(:user) { create(:user) } it 'returns false when default_branch_protection is in full protection and user is developer' do - project.team << [user, :developer] + project.add_developer(user) stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_FULL) expect(project.user_can_push_to_empty_repo?(user)).to be_falsey end it 'returns false when default_branch_protection only lets devs merge and user is dev' do - project.team << [user, :developer] + project.add_developer(user) stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE) expect(project.user_can_push_to_empty_repo?(user)).to be_falsey end it 'returns true when default_branch_protection lets devs push and user is developer' do - project.team << [user, :developer] + project.add_developer(user) stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH) expect(project.user_can_push_to_empty_repo?(user)).to be_truthy end it 'returns true when default_branch_protection is unprotected and user is developer' do - project.team << [user, :developer] + project.add_developer(user) stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) expect(project.user_can_push_to_empty_repo?(user)).to be_truthy end it 'returns true when user is master' do - project.team << [user, :master] + project.add_master(user) expect(project.user_can_push_to_empty_repo?(user)).to be_truthy end @@ -2626,6 +2626,14 @@ describe Project do project.rename_repo end end + + it 'updates project full path in .git/config' do + allow(project_storage).to receive(:rename_repo).and_return(true) + + project.rename_repo + + expect(project.repo.config['gitlab.fullpath']).to eq(project.full_path) + end end describe '#pages_path' do @@ -2668,14 +2676,12 @@ describe Project do end context 'hashed storage' do - let(:project) { create(:project, :repository) } + let(:project) { create(:project, :repository, skip_disk_validation: true) } let(:gitlab_shell) { Gitlab::Shell.new } - let(:hash) { '6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b' } + let(:hash) { Digest::SHA2.hexdigest(project.id.to_s) } before do stub_application_setting(hashed_storage_enabled: true) - allow(Digest::SHA2).to receive(:hexdigest) { hash } - allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) end describe '#legacy_storage?' do @@ -2698,13 +2704,13 @@ describe Project do describe '#base_dir' do it 'returns base_dir based on hash of project id' do - expect(project.base_dir).to eq('@hashed/6b/86') + expect(project.base_dir).to eq("@hashed/#{hash[0..1]}/#{hash[2..3]}") end end describe '#disk_path' do it 'returns disk_path based on hash of project id' do - hashed_path = '@hashed/6b/86/6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b' + hashed_path = "@hashed/#{hash[0..1]}/#{hash[2..3]}/#{hash}" expect(project.disk_path).to eq(hashed_path) end @@ -2712,7 +2718,9 @@ describe Project do describe '#ensure_storage_path_exists' do it 'delegates to gitlab_shell to ensure namespace is created' do - expect(gitlab_shell).to receive(:add_namespace).with(project.repository_storage_path, '@hashed/6b/86') + allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) + + expect(gitlab_shell).to receive(:add_namespace).with(project.repository_storage_path, "@hashed/#{hash[0..1]}/#{hash[2..3]}") project.ensure_storage_path_exists end @@ -2772,7 +2780,7 @@ describe Project do end context 'when not rolled out' do - let(:project) { create(:project, :repository, storage_version: 1) } + let(:project) { create(:project, :repository, storage_version: 1, skip_disk_validation: true) } it 'moves pages folder to new location' do expect_any_instance_of(Gitlab::UploadsTransfer).to receive(:rename_project) @@ -2781,6 +2789,12 @@ describe Project do end end end + + it 'updates project full path in .git/config' do + project.rename_repo + + expect(project.repo.config['gitlab.fullpath']).to eq(project.full_path) + end end describe '#pages_path' do @@ -3141,4 +3155,26 @@ describe Project do it { is_expected.to eq(platform_kubernetes) } end end + + describe '#write_repository_config' do + set(:project) { create(:project, :repository) } + + it 'writes full path in .git/config when key is missing' do + project.write_repository_config + + expect(project.repo.config['gitlab.fullpath']).to eq project.full_path + end + + it 'updates full path in .git/config when key is present' do + project.write_repository_config(gl_full_path: 'old/path') + + expect { project.write_repository_config }.to change { project.repo.config['gitlab.fullpath'] }.from('old/path').to(project.full_path) + end + + it 'does not raise an error with an empty repository' do + project = create(:project_empty_repo) + + expect { project.write_repository_config }.not_to raise_error + end + end end diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index 314824b32da..e07c522800a 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -291,8 +291,8 @@ describe ProjectTeam do group.add_master(master) group.add_developer(developer) - members_project.team << [developer, :developer] - members_project.team << [master, :master] + members_project.add_developer(developer) + members_project.add_master(master) create(:project_group_link, project: shared_project, group: group) end diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 0f2f906c667..540615de117 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -254,4 +254,22 @@ describe Service do end end end + + describe "#deprecated?" do + let(:project) { create(:project, :repository) } + + it 'should return false by default' do + service = create(:service, project: project) + expect(service.deprecated?).to be_falsy + end + end + + describe "#deprecation_message" do + let(:project) { create(:project, :repository) } + + it 'should be empty by default' do + service = create(:service, project: project) + expect(service.deprecation_message).to be_nil + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index e58e7588df0..8d0eaf565a7 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -22,7 +22,9 @@ describe User do describe 'associations' do it { is_expected.to have_one(:namespace) } it { is_expected.to have_many(:snippets).dependent(:destroy) } - it { is_expected.to have_many(:project_members).dependent(:destroy) } + it { is_expected.to have_many(:members) } + it { is_expected.to have_many(:project_members) } + it { is_expected.to have_many(:group_members) } it { is_expected.to have_many(:groups) } it { is_expected.to have_many(:keys).dependent(:destroy) } it { is_expected.to have_many(:deploy_keys).dependent(:destroy) } @@ -134,6 +136,16 @@ describe User do end end + it 'has a DB-level NOT NULL constraint on projects_limit' do + user = create(:user) + + expect(user.persisted?).to eq(true) + + expect do + user.update_columns(projects_limit: nil) + end.to raise_error(ActiveRecord::StatementInvalid) + end + it { is_expected.to validate_presence_of(:projects_limit) } it { is_expected.to validate_numericality_of(:projects_limit) } it { is_expected.to allow_value(0).for(:projects_limit) } @@ -760,7 +772,7 @@ describe User do before do # add user to project - project.team << [user, :master] + project.add_master(user) # create invite to projet create(:project_member, :developer, project: project, invite_token: '1234', invite_email: 'inviteduser1@example.com') @@ -805,6 +817,13 @@ describe User do expect(user.can_create_group).to be_falsey expect(user.theme_id).to eq(1) end + + it 'does not undo projects_limit setting if it matches old DB default of 10' do + # If the real default project limit is 10 then this test is worthless + expect(Gitlab.config.gitlab.default_projects_limit).not_to eq(10) + user = described_class.new(projects_limit: 10) + expect(user.projects_limit).to eq(10) + end end context 'when current_application_settings.user_default_external is true' do @@ -1448,8 +1467,8 @@ describe User do let!(:merge_event) { create(:event, :created, project: project3, target: merge_request, author: subject) } before do - project1.team << [subject, :master] - project2.team << [subject, :master] + project1.add_master(subject) + project2.add_master(subject) end it "includes IDs for projects the user has pushed to" do @@ -1548,7 +1567,7 @@ describe User do user = create(:user) project = create(:project, :private) - project.team << [user, Gitlab::Access::MASTER] + project.add_master(user) expect(user.authorized_projects(Gitlab::Access::REPORTER)) .to contain_exactly(project) @@ -1567,7 +1586,7 @@ describe User do user2 = create(:user) project = create(:project, :private, namespace: user1.namespace) - project.team << [user2, Gitlab::Access::DEVELOPER] + project.add_developer(user2) expect(user2.authorized_projects).to include(project) end @@ -1612,7 +1631,7 @@ describe User do user2 = create(:user) project = create(:project, :private, namespace: user1.namespace) - project.team << [user2, Gitlab::Access::DEVELOPER] + project.add_developer(user2) expect(user2.authorized_projects).to include(project) @@ -1702,7 +1721,7 @@ describe User do shared_examples :member do context 'when the user is a master' do before do - add_user(Gitlab::Access::MASTER) + add_user(:master) end it 'loads' do @@ -1712,7 +1731,7 @@ describe User do context 'when the user is a developer' do before do - add_user(Gitlab::Access::DEVELOPER) + add_user(:developer) end it 'does not load' do @@ -1736,7 +1755,7 @@ describe User do let(:project) { create(:project) } def add_user(access) - project.team << [user, access] + project.add_role(user, access) end it_behaves_like :member @@ -1749,8 +1768,8 @@ describe User do let(:user) { create(:user) } before do - project1.team << [user, :reporter] - project2.team << [user, :guest] + project1.add_reporter(user) + project2.add_guest(user) end it 'returns the projects when using a single project ID' do @@ -1892,8 +1911,8 @@ describe User do let(:user) { create(:user) } before do - project1.team << [user, :reporter] - project2.team << [user, :guest] + project1.add_reporter(user) + project2.add_guest(user) user.project_authorizations.delete_all user.refresh_authorized_projects |