diff options
author | Grzegorz Bizon <grzesiek.bizon@gmail.com> | 2017-04-13 15:08:52 +0200 |
---|---|---|
committer | Grzegorz Bizon <grzesiek.bizon@gmail.com> | 2017-04-13 15:08:52 +0200 |
commit | a8231ea1befd803fb5892ea3e6679219f5d7d8e5 (patch) | |
tree | a5bfe0cec13735bb36ba010c7dbacfaf13ba135f /spec/models | |
parent | 57f8f2d7ff851fc4f5d1c81a28a023855f1985b7 (diff) | |
parent | 37ab389139a21a8ab10ddbbddec1b61f720b27ab (diff) | |
download | gitlab-ce-a8231ea1befd803fb5892ea3e6679219f5d7d8e5.tar.gz |
Merge branch 'master' into feature/gb/manual-actions-protected-branches-permissions
* master: (641 commits)
Revert "Fix registry for projects with uppercases in path"
Fix registry for projects with uppercases in path
Move event icons into events_helper
Reset New branch button when issue state changes
Add link to environments on kubernetes.md
Indent system notes on desktop screens
Improve webpack-dev-server compatibility with non-localhost setups.
Add changelog entry
Fix recent searches icon alignment in Safari
Use preload to avoid Rails using JOIN
Fix NUMBER_OF_TRUNCATED_DIFF_LINES re-definition error
Prepare for zero downtime migrations
Fix filtered search input width for IE
Fix the `gitlab:gitlab_shell:check` task
Fixed random failures with Poll spec
Include CONTRIBUTING.md file when importing .gitlab-ci.yml templates
Let uses hide verbose output by default
Separate examples for each other
Collapse similar sibling scenarios
Use empty_project for resources that are independent of the repo
...
Conflicts:
app/views/projects/ci/builds/_build.html.haml
Diffstat (limited to 'spec/models')
45 files changed, 3621 insertions, 2087 deletions
diff --git a/spec/models/award_emoji_spec.rb b/spec/models/award_emoji_spec.rb index cb3c592f8cd..2a9a27752c1 100644 --- a/spec/models/award_emoji_spec.rb +++ b/spec/models/award_emoji_spec.rb @@ -25,6 +25,20 @@ describe AwardEmoji, models: true do expect(new_award).not_to be_valid end + + # Assume User A and User B both created award emoji of the same name + # on the same awardable. When User A is deleted, User A's award emoji + # is moved to the ghost user. When User B is deleted, User B's award emoji + # also needs to be moved to the ghost user - this cannot happen unless + # the uniqueness validation is disabled for ghost users. + it "allows duplicate award emoji for ghost users" do + user = create(:user, :ghost) + issue = create(:issue) + create(:award_emoji, user: user, awardable: issue) + new_award = build(:award_emoji, user: user, awardable: issue) + + expect(new_award).to be_valid + end end end end diff --git a/spec/models/blob_spec.rb b/spec/models/blob_spec.rb index 0f29766db41..e5dd57fc4bb 100644 --- a/spec/models/blob_spec.rb +++ b/spec/models/blob_spec.rb @@ -55,13 +55,13 @@ describe Blob do describe '#pdf?' do it 'is falsey when file extension is not .pdf' do - git_blob = double(name: 'git_blob.txt') + git_blob = Gitlab::Git::Blob.new(name: 'git_blob.txt') expect(described_class.decorate(git_blob)).not_to be_pdf end it 'is truthy when file extension is .pdf' do - git_blob = double(name: 'git_blob.pdf') + git_blob = Gitlab::Git::Blob.new(name: 'git_blob.pdf') expect(described_class.decorate(git_blob)).to be_pdf end @@ -140,7 +140,7 @@ describe Blob do stl?: false ) - described_class.decorate(double).tap do |blob| + described_class.decorate(Gitlab::Git::Blob.new({})).tap do |blob| allow(blob).to receive_messages(overrides) end end @@ -158,7 +158,7 @@ describe Blob do it 'handles SVGs' do blob = stubbed_blob(text?: true, svg?: true) - expect(blob.to_partial_path(project)).to eq 'image' + expect(blob.to_partial_path(project)).to eq 'svg' end it 'handles images' do @@ -167,7 +167,7 @@ describe Blob do end it 'handles text' do - blob = stubbed_blob(text?: true) + blob = stubbed_blob(text?: true, name: 'test.txt') expect(blob.to_partial_path(project)).to eq 'text' end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index cdceca975e5..b2f9a61f7f3 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -17,8 +17,9 @@ describe Ci::Build, :models do it { is_expected.to belong_to(:trigger_request) } it { is_expected.to belong_to(:erased_by) } it { is_expected.to have_many(:deployments) } - it { is_expected.to validate_presence_of :ref } - it { is_expected.to respond_to :trace_html } + it { is_expected.to validate_presence_of(:ref) } + it { is_expected.to respond_to(:has_trace?) } + it { is_expected.to respond_to(:trace) } describe '#actionize' do context 'when build is a created' do @@ -78,32 +79,6 @@ describe Ci::Build, :models do end end - describe '#append_trace' do - subject { build.trace_html } - - context 'when build.trace hides runners token' do - let(:token) { 'my_secret_token' } - - before do - build.project.update(runners_token: token) - build.append_trace(token, 0) - end - - it { is_expected.not_to include(token) } - end - - context 'when build.trace hides build token' do - let(:token) { 'my_secret_token' } - - before do - build.update(token: token) - build.append_trace(token, 0) - end - - it { is_expected.not_to include(token) } - end - end - describe '#artifacts?' do subject { build.artifacts? } @@ -272,12 +247,98 @@ describe Ci::Build, :models do describe '#update_coverage' do context "regarding coverage_regex's value," do - it "saves the correct extracted coverage value" do + before do build.coverage_regex = '\(\d+.\d+\%\) covered' - allow(build).to receive(:trace) { 'Coverage 1033 / 1051 LOC (98.29%) covered' } - expect(build).to receive(:update_attributes).with(coverage: 98.29) { true } - expect(build.update_coverage).to be true + build.trace.set('Coverage 1033 / 1051 LOC (98.29%) covered') + end + + it "saves the correct extracted coverage value" do + expect(build.update_coverage).to be(true) + expect(build.coverage).to eq(98.29) + end + end + end + + describe '#trace' do + subject { build.trace } + + it { is_expected.to be_a(Gitlab::Ci::Trace) } + end + + describe '#has_trace?' do + subject { build.has_trace? } + + it "expect to call exist? method" do + expect_any_instance_of(Gitlab::Ci::Trace).to receive(:exist?) + .and_return(true) + + is_expected.to be(true) + end + end + + describe '#trace=' do + it "expect to fail trace=" do + expect { build.trace = "new" }.to raise_error(NotImplementedError) + end + end + + describe '#old_trace' do + subject { build.old_trace } + + before do + build.update_column(:trace, 'old trace') + end + + it "expect to receive data from database" do + is_expected.to eq('old trace') + end + end + + describe '#erase_old_trace!' do + subject { build.send(:read_attribute, :trace) } + + before do + build.send(:write_attribute, :trace, 'old trace') + end + + it "expect to receive data from database" do + build.erase_old_trace! + + is_expected.to be_nil + end + end + + describe '#hide_secrets' do + let(:subject) { build.hide_secrets(data) } + + context 'hide runners token' do + let(:data) { 'new token data'} + + before do + build.project.update(runners_token: 'token') + end + + it { is_expected.to eq('new xxxxx data') } + end + + context 'hide build token' do + let(:data) { 'new token data'} + + before do + build.update(token: 'token') + end + + it { is_expected.to eq('new xxxxx data') } + end + + context 'hide build token' do + let(:data) { 'new token data'} + + before do + build.update(token: 'token') end + + it { is_expected.to eq('new xxxxx data') } end end @@ -438,7 +499,7 @@ describe Ci::Build, :models do end it 'erases build trace in trace file' do - expect(build.trace).to be_empty + expect(build).not_to have_trace end it 'sets erased to true' do @@ -532,38 +593,6 @@ describe Ci::Build, :models do end end - describe '#extract_coverage' do - context 'valid content & regex' do - subject { build.extract_coverage('Coverage 1033 / 1051 LOC (98.29%) covered', '\(\d+.\d+\%\) covered') } - - it { is_expected.to eq(98.29) } - end - - context 'valid content & bad regex' do - subject { build.extract_coverage('Coverage 1033 / 1051 LOC (98.29%) covered', 'very covered') } - - it { is_expected.to be_nil } - end - - context 'no coverage content & regex' do - subject { build.extract_coverage('No coverage for today :sad:', '\(\d+.\d+\%\) covered') } - - it { is_expected.to be_nil } - end - - context 'multiple results in content & regex' do - subject { build.extract_coverage(' (98.39%) covered. (98.29%) covered', '\(\d+.\d+\%\) covered') } - - it { is_expected.to eq(98.29) } - end - - context 'using a regex capture' do - subject { build.extract_coverage('TOTAL 9926 3489 65%', 'TOTAL\s+\d+\s+\d+\s+(\d{1,3}\%)') } - - it { is_expected.to eq(65) } - end - end - describe '#first_pending' do let!(:first) { create(:ci_build, pipeline: pipeline, status: 'pending', created_at: Date.yesterday) } let!(:second) { create(:ci_build, pipeline: pipeline, status: 'pending') } @@ -735,40 +764,6 @@ describe Ci::Build, :models do end end - describe '#has_commands?' do - context 'when build has commands' do - let(:build) do - create(:ci_build, commands: 'rspec') - end - - it 'has commands' do - expect(build).to have_commands - end - end - - context 'when does not have commands' do - context 'when commands are an empty string' do - let(:build) do - create(:ci_build, commands: '') - end - - it 'has no commands' do - expect(build).not_to have_commands - end - end - - context 'when commands are not set at all' do - let(:build) do - create(:ci_build, commands: nil) - end - - it 'has no commands' do - expect(build).not_to have_commands - end - end - end - end - describe '#has_tags?' do context 'when build has tags' do subject { create(:ci_build, tag_list: ['tag']) } @@ -969,32 +964,6 @@ describe Ci::Build, :models do it { is_expected.to eq(project.name) } end - describe '#raw_trace' do - subject { build.raw_trace } - - context 'when build.trace hides runners token' do - let(:token) { 'my_secret_token' } - - before do - build.project.update(runners_token: token) - build.update(trace: token) - end - - it { is_expected.not_to include(token) } - end - - context 'when build.trace hides build token' do - let(:token) { 'my_secret_token' } - - before do - build.update(token: token) - build.update(trace: token) - end - - it { is_expected.not_to include(token) } - end - end - describe '#ref_slug' do { 'master' => 'master', @@ -1060,61 +1029,6 @@ describe Ci::Build, :models do end end - describe '#trace' do - it 'obfuscates project runners token' do - allow(build).to receive(:raw_trace).and_return("Test: #{build.project.runners_token}") - - expect(build.trace).to eq("Test: xxxxxxxxxxxxxxxxxxxx") - end - - it 'empty project runners token' do - allow(build).to receive(:raw_trace).and_return(test_trace) - # runners_token can't normally be set to nil - allow(build.project).to receive(:runners_token).and_return(nil) - - expect(build.trace).to eq(test_trace) - end - - context 'when build does not have trace' do - it 'is is empty' do - expect(build.trace).to be_nil - end - end - - context 'when trace contains text' do - let(:text) { 'example output' } - before do - build.trace = text - end - - it { expect(build.trace).to eq(text) } - end - - context 'when trace hides runners token' do - let(:token) { 'my_secret_token' } - - before do - build.update(trace: token) - build.project.update(runners_token: token) - end - - it { expect(build.trace).not_to include(token) } - it { expect(build.raw_trace).to include(token) } - end - - context 'when build.trace hides build token' do - let(:token) { 'my_secret_token' } - - before do - build.update(trace: token) - build.update(token: token) - end - - it { expect(build.trace).not_to include(token) } - it { expect(build.raw_trace).to include(token) } - end - end - describe '#has_expiring_artifacts?' do context 'when artifacts have expiration date set' do before { build.update(artifacts_expire_at: 1.day.from_now) } @@ -1133,66 +1047,6 @@ describe Ci::Build, :models do end end - describe '#has_trace_file?' do - context 'when there is no trace' do - it { expect(build.has_trace_file?).to be_falsey } - it { expect(build.trace).to be_nil } - end - - context 'when there is a trace' do - context 'when trace is stored in file' do - let(:build_with_trace) { create(:ci_build, :trace) } - - it { expect(build_with_trace.has_trace_file?).to be_truthy } - it { expect(build_with_trace.trace).to eq('BUILD TRACE') } - end - - context 'when trace is stored in old file' do - before do - allow(build.project).to receive(:ci_id).and_return(999) - allow(File).to receive(:exist?).with(build.path_to_trace).and_return(false) - allow(File).to receive(:exist?).with(build.old_path_to_trace).and_return(true) - allow(File).to receive(:read).with(build.old_path_to_trace).and_return(test_trace) - end - - it { expect(build.has_trace_file?).to be_truthy } - it { expect(build.trace).to eq(test_trace) } - end - - context 'when trace is stored in DB' do - before do - allow(build.project).to receive(:ci_id).and_return(nil) - allow(build).to receive(:read_attribute).with(:trace).and_return(test_trace) - allow(File).to receive(:exist?).with(build.path_to_trace).and_return(false) - allow(File).to receive(:exist?).with(build.old_path_to_trace).and_return(false) - end - - it { expect(build.has_trace_file?).to be_falsey } - it { expect(build.trace).to eq(test_trace) } - end - end - end - - describe '#trace_file_path' do - context 'when trace is stored in file' do - before do - allow(build).to receive(:has_trace_file?).and_return(true) - allow(build).to receive(:has_old_trace_file?).and_return(false) - end - - it { expect(build.trace_file_path).to eq(build.path_to_trace) } - end - - context 'when trace is stored in old file' do - before do - allow(build).to receive(:has_trace_file?).and_return(true) - allow(build).to receive(:has_old_trace_file?).and_return(true) - end - - it { expect(build.trace_file_path).to eq(build.old_path_to_trace) } - end - end - describe '#update_project_statistics' do let!(:build) { create(:ci_build, artifacts_size: 23) } @@ -1446,7 +1300,7 @@ describe Ci::Build, :models do { key: 'CI_REGISTRY', value: 'registry.example.com', public: true } end let(:ci_registry_image) do - { key: 'CI_REGISTRY_IMAGE', value: project.container_registry_repository_url, public: true } + { key: 'CI_REGISTRY_IMAGE', value: project.container_registry_url, public: true } end context 'and is disabled for project' do diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index e4a24fd63c2..d7d6a75d38d 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -12,10 +12,13 @@ describe Ci::Pipeline, models: true do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:user) } + it { is_expected.to belong_to(:auto_canceled_by) } it { is_expected.to have_many(:statuses) } it { is_expected.to have_many(:trigger_requests) } it { is_expected.to have_many(:builds) } + it { is_expected.to have_many(:auto_canceled_pipelines) } + it { is_expected.to have_many(:auto_canceled_jobs) } it { is_expected.to validate_presence_of :sha } it { is_expected.to validate_presence_of :status } @@ -134,6 +137,43 @@ describe Ci::Pipeline, models: true do end end + describe '#auto_canceled?' do + subject { pipeline.auto_canceled? } + + context 'when it is canceled' do + before do + pipeline.cancel + end + + context 'when there is auto_canceled_by' do + before do + pipeline.update(auto_canceled_by: create(:ci_empty_pipeline)) + end + + it 'is auto canceled' do + is_expected.to be_truthy + end + end + + context 'when there is no auto_canceled_by' do + it 'is not auto canceled' do + is_expected.to be_falsey + end + end + + context 'when it is retried and canceled manually' do + before do + pipeline.enqueue + pipeline.cancel + end + + it 'is not auto canceled' do + is_expected.to be_falsey + end + end + end + end + describe 'pipeline stages' do before do create(:commit_status, pipeline: pipeline, @@ -335,6 +375,14 @@ describe Ci::Pipeline, models: true do end end + describe 'pipeline caching' do + it 'executes ExpirePipelinesCacheService' do + expect_any_instance_of(Ci::ExpirePipelineCacheService).to receive(:execute).with(pipeline) + + pipeline.cancel + end + end + def create_build(name, queued_at = current, started_from = 0) create(:ci_build, name: name, @@ -1031,19 +1079,6 @@ describe Ci::Pipeline, models: true do end end - describe '#update_status' do - let(:pipeline) { create(:ci_pipeline, sha: '123456') } - - it 'updates the cached status' do - fake_status = double - # after updating the status, the status is set to `skipped` for this pipeline's builds - expect(Ci::PipelineStatus).to receive(:new).with(pipeline.project, sha: '123456', status: 'skipped').and_return(fake_status) - expect(fake_status).to receive(:store_in_cache_if_needed) - - pipeline.update_status - end - end - describe 'notifications when pipeline success or failed' do let(:project) { create(:project, :repository) } diff --git a/spec/models/ci/pipeline_status_spec.rb b/spec/models/ci/pipeline_status_spec.rb deleted file mode 100644 index bc5b71666c2..00000000000 --- a/spec/models/ci/pipeline_status_spec.rb +++ /dev/null @@ -1,173 +0,0 @@ -require 'spec_helper' - -describe Ci::PipelineStatus do - let(:project) { create(:project) } - let(:pipeline_status) { described_class.new(project) } - - describe '.load_for_project' do - it "loads the status" do - expect_any_instance_of(described_class).to receive(:load_status) - - described_class.load_for_project(project) - end - end - - describe '#has_status?' do - it "is false when the status wasn't loaded yet" do - expect(pipeline_status.has_status?).to be_falsy - end - - it 'is true when all status information was loaded' do - fake_commit = double - allow(fake_commit).to receive(:status).and_return('failed') - allow(fake_commit).to receive(:sha).and_return('failed424d1b73bc0d3cb726eb7dc4ce17a4d48552f8c6') - allow(pipeline_status).to receive(:commit).and_return(fake_commit) - allow(pipeline_status).to receive(:has_cache?).and_return(false) - - pipeline_status.load_status - - expect(pipeline_status.has_status?).to be_truthy - end - end - - describe '#load_status' do - it 'loads the status from the cache when there is one' do - expect(pipeline_status).to receive(:has_cache?).and_return(true) - expect(pipeline_status).to receive(:load_from_cache) - - pipeline_status.load_status - end - - it 'loads the status from the project commit when there is no cache' do - allow(pipeline_status).to receive(:has_cache?).and_return(false) - - expect(pipeline_status).to receive(:load_from_commit) - - pipeline_status.load_status - end - - it 'stores the status in the cache when it loading it from the project' do - allow(pipeline_status).to receive(:has_cache?).and_return(false) - allow(pipeline_status).to receive(:load_from_commit) - - expect(pipeline_status).to receive(:store_in_cache) - - pipeline_status.load_status - end - - it 'sets the state to loaded' do - pipeline_status.load_status - - expect(pipeline_status).to be_loaded - end - - it 'only loads the status once' do - expect(pipeline_status).to receive(:has_cache?).and_return(true).exactly(1) - expect(pipeline_status).to receive(:load_from_cache).exactly(1) - - pipeline_status.load_status - pipeline_status.load_status - end - end - - describe "#load_from_commit" do - let!(:pipeline) { create(:ci_pipeline, :success, project: project, sha: project.commit.sha) } - - it 'reads the status from the pipeline for the commit' do - pipeline_status.load_from_commit - - expect(pipeline_status.status).to eq('success') - expect(pipeline_status.sha).to eq(project.commit.sha) - end - - it "doesn't fail for an empty project" do - status_for_empty_commit = described_class.new(create(:empty_project)) - - status_for_empty_commit.load_status - - expect(status_for_empty_commit).to be_loaded - end - end - - describe "#store_in_cache", :redis do - it "sets the object in redis" do - pipeline_status.sha = '123456' - pipeline_status.status = 'failed' - - pipeline_status.store_in_cache - read_sha, read_status = Gitlab::Redis.with { |redis| redis.hmget("projects/#{project.id}/build_status", :sha, :status) } - - expect(read_sha).to eq('123456') - expect(read_status).to eq('failed') - end - end - - describe '#store_in_cache_if_needed', :redis do - it 'stores the state in the cache when the sha is the HEAD of the project' do - create(:ci_pipeline, :success, project: project, sha: project.commit.sha) - build_status = described_class.load_for_project(project) - - build_status.store_in_cache_if_needed - sha, status = Gitlab::Redis.with { |redis| redis.hmget("projects/#{project.id}/build_status", :sha, :status) } - - expect(sha).not_to be_nil - expect(status).not_to be_nil - end - - it "doesn't store the status in redis when the sha is not the head of the project" do - other_status = described_class.new(project, sha: "123456", status: "failed") - - other_status.store_in_cache_if_needed - sha, status = Gitlab::Redis.with { |redis| redis.hmget("projects/#{project.id}/build_status", :sha, :status) } - - expect(sha).to be_nil - expect(status).to be_nil - end - - it "deletes the cache if the repository doesn't have a head commit" do - empty_project = create(:empty_project) - Gitlab::Redis.with { |redis| redis.mapped_hmset("projects/#{empty_project.id}/build_status", { sha: "sha", status: "pending" }) } - other_status = described_class.new(empty_project, sha: "123456", status: "failed") - - other_status.store_in_cache_if_needed - sha, status = Gitlab::Redis.with { |redis| redis.hmget("projects/#{empty_project.id}/build_status", :sha, :status) } - - expect(sha).to be_nil - expect(status).to be_nil - end - end - - describe "with a status in redis", :redis do - let(:status) { 'success' } - let(:sha) { '424d1b73bc0d3cb726eb7dc4ce17a4d48552f8c6' } - - before do - Gitlab::Redis.with { |redis| redis.mapped_hmset("projects/#{project.id}/build_status", { sha: sha, status: status }) } - end - - describe '#load_from_cache' do - it 'reads the status from redis' do - pipeline_status.load_from_cache - - expect(pipeline_status.sha).to eq(sha) - expect(pipeline_status.status).to eq(status) - end - end - - describe '#has_cache?' do - it 'knows the status is cached' do - expect(pipeline_status.has_cache?).to be_truthy - end - end - - describe '#delete_from_cache' do - it 'deletes values from redis' do - pipeline_status.delete_from_cache - - key_exists = Gitlab::Redis.with { |redis| redis.exists("projects/#{project.id}/build_status") } - - expect(key_exists).to be_falsy - end - end - end -end diff --git a/spec/models/ci/trigger_schedule_spec.rb b/spec/models/ci/trigger_schedule_spec.rb new file mode 100644 index 00000000000..75d21541cee --- /dev/null +++ b/spec/models/ci/trigger_schedule_spec.rb @@ -0,0 +1,76 @@ +require 'spec_helper' + +describe Ci::TriggerSchedule, models: true do + it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:trigger) } + it { is_expected.to respond_to(:ref) } + + describe '#set_next_run_at' do + context 'when creates new TriggerSchedule' do + before do + trigger_schedule = create(:ci_trigger_schedule, :nightly) + @expected_next_run_at = Gitlab::Ci::CronParser.new(trigger_schedule.cron, trigger_schedule.cron_timezone) + .next_time_from(Time.now) + end + + it 'updates next_run_at automatically' do + expect(Ci::TriggerSchedule.last.next_run_at).to eq(@expected_next_run_at) + end + end + + context 'when updates cron of exsisted TriggerSchedule' do + before do + trigger_schedule = create(:ci_trigger_schedule, :nightly) + new_cron = '0 0 1 1 *' + trigger_schedule.update!(cron: new_cron) # Subject + @expected_next_run_at = Gitlab::Ci::CronParser.new(new_cron, trigger_schedule.cron_timezone) + .next_time_from(Time.now) + end + + it 'updates next_run_at automatically' do + expect(Ci::TriggerSchedule.last.next_run_at).to eq(@expected_next_run_at) + end + end + end + + describe '#schedule_next_run!' do + context 'when reschedules after 10 days from now' do + before do + trigger_schedule = create(:ci_trigger_schedule, :nightly) + time_future = Time.now + 10.days + allow(Time).to receive(:now).and_return(time_future) + trigger_schedule.schedule_next_run! # Subject + @expected_next_run_at = Gitlab::Ci::CronParser.new(trigger_schedule.cron, trigger_schedule.cron_timezone) + .next_time_from(time_future) + end + + it 'points to proper next_run_at' do + expect(Ci::TriggerSchedule.last.next_run_at).to eq(@expected_next_run_at) + end + end + + context 'when cron is invalid' do + before do + trigger_schedule = create(:ci_trigger_schedule, :nightly) + trigger_schedule.cron = 'Invalid-cron' + trigger_schedule.schedule_next_run! # Subject + end + + it 'sets nil to next_run_at' do + expect(Ci::TriggerSchedule.last.next_run_at).to be_nil + end + end + + context 'when cron_timezone is invalid' do + before do + trigger_schedule = create(:ci_trigger_schedule, :nightly) + trigger_schedule.cron_timezone = 'Invalid-cron_timezone' + trigger_schedule.schedule_next_run! # Subject + end + + it 'sets nil to next_run_at' do + expect(Ci::TriggerSchedule.last.next_run_at).to be_nil + end + end + end +end diff --git a/spec/models/ci/trigger_spec.rb b/spec/models/ci/trigger_spec.rb index 1bcb673cb16..d26121018ce 100644 --- a/spec/models/ci/trigger_spec.rb +++ b/spec/models/ci/trigger_spec.rb @@ -7,6 +7,7 @@ describe Ci::Trigger, models: true do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:owner) } it { is_expected.to have_many(:trigger_requests) } + it { is_expected.to have_one(:trigger_schedule) } end describe 'before_validation' do @@ -16,8 +17,8 @@ describe Ci::Trigger, models: true do expect(trigger.token).not_to be_nil end - it 'does not set an random token if one provided' do - trigger = create(:ci_trigger, project: project) + it 'does not set a random token if one provided' do + trigger = create(:ci_trigger, project: project, token: 'token') expect(trigger.token).to eq('token') end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 980a1b70ef5..ce31c8ed94c 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -389,31 +389,32 @@ eos end end - describe '#raw_diffs' do - context 'Gitaly commit_raw_diffs feature enabled' do - before do - allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:commit_raw_diffs).and_return(true) - end - - context 'when a truthy deltas_only is not passed to args' do - it 'fetches diffs from Gitaly server' do - expect(Gitlab::GitalyClient::Commit).to receive(:diff_from_parent). - with(commit) - - commit.raw_diffs - end - end - - context 'when a truthy deltas_only is passed to args' do - it 'fetches diffs using Rugged' do - opts = { deltas_only: true } - - expect(Gitlab::GitalyClient::Commit).not_to receive(:diff_from_parent) - expect(commit.raw).to receive(:diffs).with(opts) - - commit.raw_diffs(opts) - end - end - end - end + # describe '#raw_diffs' do + # TODO: Uncomment when feature is reenabled + # context 'Gitaly commit_raw_diffs feature enabled' do + # before do + # allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:commit_raw_diffs).and_return(true) + # end + # + # context 'when a truthy deltas_only is not passed to args' do + # it 'fetches diffs from Gitaly server' do + # expect(Gitlab::GitalyClient::Commit).to receive(:diff_from_parent). + # with(commit) + # + # commit.raw_diffs + # end + # end + # + # context 'when a truthy deltas_only is passed to args' do + # it 'fetches diffs using Rugged' do + # opts = { deltas_only: true } + # + # expect(Gitlab::GitalyClient::Commit).not_to receive(:diff_from_parent) + # expect(commit.raw).to receive(:diffs).with(opts) + # + # commit.raw_diffs(opts) + # end + # end + # end + # end end diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 7343b735a74..0ee85489574 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -16,6 +16,7 @@ describe CommitStatus, :models do it { is_expected.to belong_to(:pipeline) } it { is_expected.to belong_to(:user) } it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:auto_canceled_by) } it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_inclusion_of(:status).in_array(%w(pending running failed success canceled)) } @@ -101,6 +102,32 @@ describe CommitStatus, :models do end end + describe '#auto_canceled?' do + subject { commit_status.auto_canceled? } + + context 'when it is canceled' do + before do + commit_status.update(status: 'canceled') + end + + context 'when there is auto_canceled_by' do + before do + commit_status.update(auto_canceled_by: create(:ci_empty_pipeline)) + end + + it 'is auto canceled' do + is_expected.to be_truthy + end + end + + context 'when there is no auto_canceled_by' do + it 'is not auto canceled' do + is_expected.to be_falsey + end + end + end + end + describe '#duration' do subject { commit_status.duration } diff --git a/spec/models/concerns/discussion_on_diff_spec.rb b/spec/models/concerns/discussion_on_diff_spec.rb new file mode 100644 index 00000000000..0002a00770f --- /dev/null +++ b/spec/models/concerns/discussion_on_diff_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper' + +describe DiffDiscussion, DiscussionOnDiff, model: true do + subject { create(:diff_note_on_merge_request).to_discussion } + + describe "#truncated_diff_lines" do + let(:truncated_lines) { subject.truncated_diff_lines } + + context "when diff is greater than allowed number of truncated diff lines " do + it "returns fewer lines" do + expect(subject.diff_lines.count).to be > described_class::NUMBER_OF_TRUNCATED_DIFF_LINES + + expect(truncated_lines.count).to be <= described_class::NUMBER_OF_TRUNCATED_DIFF_LINES + end + end + + context "when some diff lines are meta" do + it "returns no meta lines" do + expect(subject.diff_lines).to include(be_meta) + expect(truncated_lines).not_to include(be_meta) + end + end + end +end diff --git a/spec/models/concerns/has_status_spec.rb b/spec/models/concerns/has_status_spec.rb index 82abad0e2f6..67dae7cf4c0 100644 --- a/spec/models/concerns/has_status_spec.rb +++ b/spec/models/concerns/has_status_spec.rb @@ -231,6 +231,18 @@ describe HasStatus do end end + describe '.created_or_pending' do + subject { CommitStatus.created_or_pending } + + %i[created pending].each do |status| + it_behaves_like 'containing the job', status + end + + %i[running failed success].each do |status| + it_behaves_like 'not containing the job', status + end + end + describe '.finished' do subject { CommitStatus.finished } diff --git a/spec/models/concerns/ignorable_column_spec.rb b/spec/models/concerns/ignorable_column_spec.rb new file mode 100644 index 00000000000..dba9fe43327 --- /dev/null +++ b/spec/models/concerns/ignorable_column_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +describe IgnorableColumn do + let :base_class do + Class.new do + def self.columns + # This method does not have access to "double" + [Struct.new(:name).new('id'), Struct.new(:name).new('title')] + end + end + end + + let :model do + Class.new(base_class) do + include IgnorableColumn + end + end + + describe '.columns' do + it 'returns the columns, excluding the ignored ones' do + model.ignore_column(:title) + + expect(model.columns.map(&:name)).to eq(%w(id)) + end + end + + describe '.ignored_columns' do + it 'returns a Set' do + expect(model.ignored_columns).to be_an_instance_of(Set) + end + + it 'returns the names of the ignored columns' do + model.ignore_column(:title) + + expect(model.ignored_columns).to eq(Set.new(%w(title))) + end + end +end diff --git a/spec/models/concerns/noteable_spec.rb b/spec/models/concerns/noteable_spec.rb new file mode 100644 index 00000000000..92cc8859a8c --- /dev/null +++ b/spec/models/concerns/noteable_spec.rb @@ -0,0 +1,261 @@ +require 'spec_helper' + +describe MergeRequest, Noteable, model: true do + let!(:active_diff_note1) { create(:diff_note_on_merge_request) } + let(:project) { active_diff_note1.project } + subject { active_diff_note1.noteable } + let!(:active_diff_note2) { create(:diff_note_on_merge_request, project: project, noteable: subject, in_reply_to: active_diff_note1) } + let!(:active_diff_note3) { create(:diff_note_on_merge_request, project: project, noteable: subject, position: active_position2) } + let!(:outdated_diff_note1) { create(:diff_note_on_merge_request, project: project, noteable: subject, position: outdated_position) } + let!(:outdated_diff_note2) { create(:diff_note_on_merge_request, project: project, noteable: subject, in_reply_to: outdated_diff_note1) } + let!(:discussion_note1) { create(:discussion_note_on_merge_request, project: project, noteable: subject) } + let!(:discussion_note2) { create(:discussion_note_on_merge_request, in_reply_to: discussion_note1) } + let!(:commit_diff_note1) { create(:diff_note_on_commit, project: project) } + let!(:commit_diff_note2) { create(:diff_note_on_commit, project: project, in_reply_to: commit_diff_note1) } + let!(:commit_note1) { create(:note_on_commit, project: project) } + let!(:commit_note2) { create(:note_on_commit, project: project) } + let!(:commit_discussion_note1) { create(:discussion_note_on_commit, project: project) } + let!(:commit_discussion_note2) { create(:discussion_note_on_commit, in_reply_to: commit_discussion_note1) } + let!(:commit_discussion_note3) { create(:discussion_note_on_commit, project: project) } + let!(:note1) { create(:note, project: project, noteable: subject) } + let!(:note2) { create(:note, project: project, noteable: subject) } + + let(:active_position2) do + Gitlab::Diff::Position.new( + old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: 16, + new_line: 22, + diff_refs: subject.diff_refs + ) + end + + let(:outdated_position) do + Gitlab::Diff::Position.new( + old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: 9, + diff_refs: project.commit("874797c3a73b60d2187ed6e2fcabd289ff75171e").diff_refs + ) + end + + describe '#discussions' do + let(:discussions) { subject.discussions } + + it 'includes discussions for diff notes, commit diff notes, commit notes, and regular notes' do + expect(discussions).to eq([ + DiffDiscussion.new([active_diff_note1, active_diff_note2], subject), + DiffDiscussion.new([active_diff_note3], subject), + DiffDiscussion.new([outdated_diff_note1, outdated_diff_note2], subject), + Discussion.new([discussion_note1, discussion_note2], subject), + DiffDiscussion.new([commit_diff_note1, commit_diff_note2], subject), + OutOfContextDiscussion.new([commit_note1, commit_note2], subject), + Discussion.new([commit_discussion_note1, commit_discussion_note2], subject), + Discussion.new([commit_discussion_note3], subject), + IndividualNoteDiscussion.new([note1], subject), + IndividualNoteDiscussion.new([note2], subject) + ]) + end + end + + describe '#grouped_diff_discussions' do + let(:grouped_diff_discussions) { subject.grouped_diff_discussions } + + it "includes active discussions" do + discussions = grouped_diff_discussions.values.flatten + + expect(discussions.count).to eq(2) + expect(discussions.map(&:id)).to eq([active_diff_note1.discussion_id, active_diff_note3.discussion_id]) + expect(discussions.all?(&:active?)).to be true + + expect(discussions.first.notes).to eq([active_diff_note1, active_diff_note2]) + expect(discussions.last.notes).to eq([active_diff_note3]) + end + + it "doesn't include outdated discussions" do + expect(grouped_diff_discussions.values.flatten.map(&:id)).not_to include(outdated_diff_note1.discussion_id) + end + + it "groups the discussions by line code" do + expect(grouped_diff_discussions[active_diff_note1.line_code].first.id).to eq(active_diff_note1.discussion_id) + expect(grouped_diff_discussions[active_diff_note3.line_code].first.id).to eq(active_diff_note3.discussion_id) + end + end + + context "discussion status" do + let(:first_discussion) { build_stubbed(:discussion_note_on_merge_request, noteable: subject, project: project).to_discussion } + let(:second_discussion) { build_stubbed(:discussion_note_on_merge_request, noteable: subject, project: project).to_discussion } + let(:third_discussion) { build_stubbed(:discussion_note_on_merge_request, noteable: subject, project: project).to_discussion } + + before do + allow(subject).to receive(:resolvable_discussions).and_return([first_discussion, second_discussion, third_discussion]) + end + + describe "#discussions_resolvable?" do + context "when all discussions are unresolvable" do + before do + allow(first_discussion).to receive(:resolvable?).and_return(false) + allow(second_discussion).to receive(:resolvable?).and_return(false) + allow(third_discussion).to receive(:resolvable?).and_return(false) + end + + it "returns false" do + expect(subject.discussions_resolvable?).to be false + end + end + + context "when some discussions are unresolvable and some discussions are resolvable" do + before do + allow(first_discussion).to receive(:resolvable?).and_return(true) + allow(second_discussion).to receive(:resolvable?).and_return(false) + allow(third_discussion).to receive(:resolvable?).and_return(true) + end + + it "returns true" do + expect(subject.discussions_resolvable?).to be true + end + end + + context "when all discussions are resolvable" do + before do + allow(first_discussion).to receive(:resolvable?).and_return(true) + allow(second_discussion).to receive(:resolvable?).and_return(true) + allow(third_discussion).to receive(:resolvable?).and_return(true) + end + + it "returns true" do + expect(subject.discussions_resolvable?).to be true + end + end + end + + describe "#discussions_resolved?" do + context "when discussions are not resolvable" do + before do + allow(subject).to receive(:discussions_resolvable?).and_return(false) + end + + it "returns false" do + expect(subject.discussions_resolved?).to be false + end + end + + context "when discussions are resolvable" do + before do + allow(subject).to receive(:discussions_resolvable?).and_return(true) + + allow(first_discussion).to receive(:resolvable?).and_return(true) + allow(second_discussion).to receive(:resolvable?).and_return(false) + allow(third_discussion).to receive(:resolvable?).and_return(true) + end + + context "when all resolvable discussions are resolved" do + before do + allow(first_discussion).to receive(:resolved?).and_return(true) + allow(third_discussion).to receive(:resolved?).and_return(true) + end + + it "returns true" do + expect(subject.discussions_resolved?).to be true + end + end + + context "when some resolvable discussions are not resolved" do + before do + allow(first_discussion).to receive(:resolved?).and_return(true) + allow(third_discussion).to receive(:resolved?).and_return(false) + end + + it "returns false" do + expect(subject.discussions_resolved?).to be false + end + end + end + end + + describe "#discussions_to_be_resolved?" do + context "when discussions are not resolvable" do + before do + allow(subject).to receive(:discussions_resolvable?).and_return(false) + end + + it "returns false" do + expect(subject.discussions_to_be_resolved?).to be false + end + end + + context "when discussions are resolvable" do + before do + allow(subject).to receive(:discussions_resolvable?).and_return(true) + + allow(first_discussion).to receive(:resolvable?).and_return(true) + allow(second_discussion).to receive(:resolvable?).and_return(false) + allow(third_discussion).to receive(:resolvable?).and_return(true) + end + + context "when all resolvable discussions are resolved" do + before do + allow(first_discussion).to receive(:resolved?).and_return(true) + allow(third_discussion).to receive(:resolved?).and_return(true) + end + + it "returns false" do + expect(subject.discussions_to_be_resolved?).to be false + end + end + + context "when some resolvable discussions are not resolved" do + before do + allow(first_discussion).to receive(:resolved?).and_return(true) + allow(third_discussion).to receive(:resolved?).and_return(false) + end + + it "returns true" do + expect(subject.discussions_to_be_resolved?).to be true + end + end + end + end + + describe "#discussions_to_be_resolved" do + before do + allow(first_discussion).to receive(:to_be_resolved?).and_return(true) + allow(second_discussion).to receive(:to_be_resolved?).and_return(false) + allow(third_discussion).to receive(:to_be_resolved?).and_return(false) + end + + it 'includes only discussions that need to be resolved' do + expect(subject.discussions_to_be_resolved).to eq([first_discussion]) + end + end + + describe '#discussions_can_be_resolved_by?' do + let(:user) { build(:user) } + + context 'all discussions can be resolved by the user' do + before do + allow(first_discussion).to receive(:can_resolve?).with(user).and_return(true) + allow(second_discussion).to receive(:can_resolve?).with(user).and_return(true) + allow(third_discussion).to receive(:can_resolve?).with(user).and_return(true) + end + + it 'allows a user to resolve the discussions' do + expect(subject.discussions_can_be_resolved_by?(user)).to be(true) + end + end + + context 'one discussion cannot be resolved by the user' do + before do + allow(first_discussion).to receive(:can_resolve?).with(user).and_return(true) + allow(second_discussion).to receive(:can_resolve?).with(user).and_return(true) + allow(third_discussion).to receive(:can_resolve?).with(user).and_return(false) + end + + it 'allows a user to resolve the discussions' do + expect(subject.discussions_can_be_resolved_by?(user)).to be(false) + end + end + end + end +end diff --git a/spec/models/concerns/resolvable_discussion_spec.rb b/spec/models/concerns/resolvable_discussion_spec.rb new file mode 100644 index 00000000000..18327fe262d --- /dev/null +++ b/spec/models/concerns/resolvable_discussion_spec.rb @@ -0,0 +1,548 @@ +require 'spec_helper' + +describe Discussion, ResolvableDiscussion, models: true do + subject { described_class.new([first_note, second_note, third_note]) } + + let(:first_note) { create(:discussion_note_on_merge_request) } + let(:merge_request) { first_note.noteable } + let(:project) { first_note.project } + let(:second_note) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project, in_reply_to: first_note) } + let(:third_note) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } + + describe "#resolvable?" do + context "when potentially resolvable" do + before do + allow(subject).to receive(:potentially_resolvable?).and_return(true) + end + + context "when all notes are unresolvable" do + before do + allow(first_note).to receive(:resolvable?).and_return(false) + allow(second_note).to receive(:resolvable?).and_return(false) + allow(third_note).to receive(:resolvable?).and_return(false) + end + + it "returns false" do + expect(subject.resolvable?).to be false + end + end + + context "when some notes are unresolvable and some notes are resolvable" do + before do + allow(first_note).to receive(:resolvable?).and_return(true) + allow(second_note).to receive(:resolvable?).and_return(false) + allow(third_note).to receive(:resolvable?).and_return(true) + end + + it "returns true" do + expect(subject.resolvable?).to be true + end + end + + context "when all notes are resolvable" do + before do + allow(first_note).to receive(:resolvable?).and_return(true) + allow(second_note).to receive(:resolvable?).and_return(true) + allow(third_note).to receive(:resolvable?).and_return(true) + end + + it "returns true" do + expect(subject.resolvable?).to be true + end + end + end + + context "when not potentially resolvable" do + before do + allow(subject).to receive(:potentially_resolvable?).and_return(false) + end + + it "returns false" do + expect(subject.resolvable?).to be false + end + end + end + + describe "#resolved?" do + context "when not resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(false) + end + + it "returns false" do + expect(subject.resolved?).to be false + end + end + + context "when resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(true) + + allow(first_note).to receive(:resolvable?).and_return(true) + allow(second_note).to receive(:resolvable?).and_return(false) + allow(third_note).to receive(:resolvable?).and_return(true) + end + + context "when all resolvable notes are resolved" do + before do + allow(first_note).to receive(:resolved?).and_return(true) + allow(third_note).to receive(:resolved?).and_return(true) + end + + it "returns true" do + expect(subject.resolved?).to be true + end + end + + context "when some resolvable notes are not resolved" do + before do + allow(first_note).to receive(:resolved?).and_return(true) + allow(third_note).to receive(:resolved?).and_return(false) + end + + it "returns false" do + expect(subject.resolved?).to be false + end + end + end + end + + describe "#to_be_resolved?" do + context "when not resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(false) + end + + it "returns false" do + expect(subject.to_be_resolved?).to be false + end + end + + context "when resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(true) + + allow(first_note).to receive(:resolvable?).and_return(true) + allow(second_note).to receive(:resolvable?).and_return(false) + allow(third_note).to receive(:resolvable?).and_return(true) + end + + context "when all resolvable notes are resolved" do + before do + allow(first_note).to receive(:resolved?).and_return(true) + allow(third_note).to receive(:resolved?).and_return(true) + end + + it "returns false" do + expect(subject.to_be_resolved?).to be false + end + end + + context "when some resolvable notes are not resolved" do + before do + allow(first_note).to receive(:resolved?).and_return(true) + allow(third_note).to receive(:resolved?).and_return(false) + end + + it "returns true" do + expect(subject.to_be_resolved?).to be true + end + end + end + end + + describe "#can_resolve?" do + let(:current_user) { create(:user) } + + context "when not resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(false) + end + + it "returns false" do + expect(subject.can_resolve?(current_user)).to be false + end + end + + context "when resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(true) + end + + context "when not signed in" do + let(:current_user) { nil } + + it "returns false" do + expect(subject.can_resolve?(current_user)).to be false + end + end + + context "when signed in" do + context "when the signed in user is the noteable author" do + before do + subject.noteable.author = current_user + end + + it "returns true" do + expect(subject.can_resolve?(current_user)).to be true + end + end + + context "when the signed in user can push to the project" do + before do + subject.project.team << [current_user, :master] + end + + it "returns true" do + expect(subject.can_resolve?(current_user)).to be true + end + end + + context "when the signed in user is a random user" do + it "returns false" do + expect(subject.can_resolve?(current_user)).to be false + end + end + end + end + end + + describe "#resolve!" do + let(:current_user) { create(:user) } + + context "when not resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(false) + end + + it "returns nil" do + expect(subject.resolve!(current_user)).to be_nil + end + + it "doesn't set resolved_at" do + subject.resolve!(current_user) + + expect(subject.resolved_at).to be_nil + end + + it "doesn't set resolved_by" do + subject.resolve!(current_user) + + expect(subject.resolved_by).to be_nil + end + + it "doesn't mark as resolved" do + subject.resolve!(current_user) + + expect(subject.resolved?).to be false + end + end + + context "when resolvable" do + let(:user) { create(:user) } + let(:second_note) { create(:diff_note_on_commit) } # unresolvable + + before do + allow(subject).to receive(:resolvable?).and_return(true) + end + + context "when all resolvable notes are resolved" do + before do + first_note.resolve!(user) + third_note.resolve!(user) + + first_note.reload + third_note.reload + end + + it "doesn't change resolved_at on the resolved notes" do + expect(first_note.resolved_at).not_to be_nil + expect(third_note.resolved_at).not_to be_nil + + expect { subject.resolve!(current_user) }.not_to change { first_note.resolved_at } + expect { subject.resolve!(current_user) }.not_to change { third_note.resolved_at } + end + + it "doesn't change resolved_by on the resolved notes" do + expect(first_note.resolved_by).to eq(user) + expect(third_note.resolved_by).to eq(user) + + expect { subject.resolve!(current_user) }.not_to change { first_note.resolved_by } + expect { subject.resolve!(current_user) }.not_to change { third_note.resolved_by } + end + + it "doesn't change the resolved state on the resolved notes" do + expect(first_note.resolved?).to be true + expect(third_note.resolved?).to be true + + expect { subject.resolve!(current_user) }.not_to change { first_note.resolved? } + expect { subject.resolve!(current_user) }.not_to change { third_note.resolved? } + end + + it "doesn't change resolved_at" do + expect(subject.resolved_at).not_to be_nil + + expect { subject.resolve!(current_user) }.not_to change { subject.resolved_at } + end + + it "doesn't change resolved_by" do + expect(subject.resolved_by).to eq(user) + + expect { subject.resolve!(current_user) }.not_to change { subject.resolved_by } + end + + it "doesn't change resolved state" do + expect(subject.resolved?).to be true + + expect { subject.resolve!(current_user) }.not_to change { subject.resolved? } + end + end + + context "when some resolvable notes are resolved" do + before do + first_note.resolve!(user) + end + + it "doesn't change resolved_at on the resolved note" do + expect(first_note.resolved_at).not_to be_nil + + expect { subject.resolve!(current_user) }. + not_to change { first_note.reload.resolved_at } + end + + it "doesn't change resolved_by on the resolved note" do + expect(first_note.resolved_by).to eq(user) + + expect { subject.resolve!(current_user) }. + not_to change { first_note.reload && first_note.resolved_by } + end + + it "doesn't change the resolved state on the resolved note" do + expect(first_note.resolved?).to be true + + expect { subject.resolve!(current_user) }. + not_to change { first_note.reload && first_note.resolved? } + end + + it "sets resolved_at on the unresolved note" do + subject.resolve!(current_user) + third_note.reload + + expect(third_note.resolved_at).not_to be_nil + end + + it "sets resolved_by on the unresolved note" do + subject.resolve!(current_user) + third_note.reload + + expect(third_note.resolved_by).to eq(current_user) + end + + it "marks the unresolved note as resolved" do + subject.resolve!(current_user) + third_note.reload + + expect(third_note.resolved?).to be true + end + + it "sets resolved_at" do + subject.resolve!(current_user) + + expect(subject.resolved_at).not_to be_nil + end + + it "sets resolved_by" do + subject.resolve!(current_user) + + expect(subject.resolved_by).to eq(current_user) + end + + it "marks as resolved" do + subject.resolve!(current_user) + + expect(subject.resolved?).to be true + end + end + + context "when no resolvable notes are resolved" do + it "sets resolved_at on the unresolved notes" do + subject.resolve!(current_user) + first_note.reload + third_note.reload + + expect(first_note.resolved_at).not_to be_nil + expect(third_note.resolved_at).not_to be_nil + end + + it "sets resolved_by on the unresolved notes" do + subject.resolve!(current_user) + first_note.reload + third_note.reload + + expect(first_note.resolved_by).to eq(current_user) + expect(third_note.resolved_by).to eq(current_user) + end + + it "marks the unresolved notes as resolved" do + subject.resolve!(current_user) + first_note.reload + third_note.reload + + expect(first_note.resolved?).to be true + expect(third_note.resolved?).to be true + end + + it "sets resolved_at" do + subject.resolve!(current_user) + first_note.reload + third_note.reload + + expect(subject.resolved_at).not_to be_nil + end + + it "sets resolved_by" do + subject.resolve!(current_user) + first_note.reload + third_note.reload + + expect(subject.resolved_by).to eq(current_user) + end + + it "marks as resolved" do + subject.resolve!(current_user) + first_note.reload + third_note.reload + + expect(subject.resolved?).to be true + end + end + end + end + + describe "#unresolve!" do + context "when not resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(false) + end + + it "returns nil" do + expect(subject.unresolve!).to be_nil + end + end + + context "when resolvable" do + let(:user) { create(:user) } + + before do + allow(subject).to receive(:resolvable?).and_return(true) + + allow(first_note).to receive(:resolvable?).and_return(true) + allow(second_note).to receive(:resolvable?).and_return(false) + allow(third_note).to receive(:resolvable?).and_return(true) + end + + context "when all resolvable notes are resolved" do + before do + first_note.resolve!(user) + third_note.resolve!(user) + end + + it "unsets resolved_at on the resolved notes" do + subject.unresolve! + first_note.reload + third_note.reload + + expect(first_note.resolved_at).to be_nil + expect(third_note.resolved_at).to be_nil + end + + it "unsets resolved_by on the resolved notes" do + subject.unresolve! + first_note.reload + third_note.reload + + expect(first_note.resolved_by).to be_nil + expect(third_note.resolved_by).to be_nil + end + + it "unmarks the resolved notes as resolved" do + subject.unresolve! + first_note.reload + third_note.reload + + expect(first_note.resolved?).to be false + expect(third_note.resolved?).to be false + end + + it "unsets resolved_at" do + subject.unresolve! + first_note.reload + third_note.reload + + expect(subject.resolved_at).to be_nil + end + + it "unsets resolved_by" do + subject.unresolve! + first_note.reload + third_note.reload + + expect(subject.resolved_by).to be_nil + end + + it "unmarks as resolved" do + subject.unresolve! + + expect(subject.resolved?).to be false + end + end + + context "when some resolvable notes are resolved" do + before do + first_note.resolve!(user) + end + + it "unsets resolved_at on the resolved note" do + subject.unresolve! + + expect(subject.first_note.resolved_at).to be_nil + end + + it "unsets resolved_by on the resolved note" do + subject.unresolve! + + expect(subject.first_note.resolved_by).to be_nil + end + + it "unmarks the resolved note as resolved" do + subject.unresolve! + + expect(subject.first_note.resolved?).to be false + end + end + end + end + + describe "#first_note_to_resolve" do + it "returns the first note that still needs to be resolved" do + allow(first_note).to receive(:to_be_resolved?).and_return(false) + allow(second_note).to receive(:to_be_resolved?).and_return(true) + + expect(subject.first_note_to_resolve).to eq(second_note) + end + end + + describe "#last_resolved_note" do + let(:current_user) { create(:user) } + + before do + first_note.resolve!(current_user) + third_note.resolve!(current_user) + second_note.resolve!(current_user) + end + + it "returns the last note that was resolved" do + expect(subject.last_resolved_note).to eq(second_note) + end + end +end diff --git a/spec/models/concerns/resolvable_note_spec.rb b/spec/models/concerns/resolvable_note_spec.rb new file mode 100644 index 00000000000..1503ccdff11 --- /dev/null +++ b/spec/models/concerns/resolvable_note_spec.rb @@ -0,0 +1,329 @@ +require 'spec_helper' + +describe Note, ResolvableNote, models: true do + let(:project) { create(:project) } + let(:merge_request) { create(:merge_request, source_project: project) } + subject { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } + + context 'resolvability scopes' do + let!(:note1) { create(:note, project: project) } + let!(:note2) { create(:diff_note_on_commit, project: project) } + let!(:note3) { create(:diff_note_on_merge_request, :resolved, noteable: merge_request, project: project) } + let!(:note4) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } + let!(:note5) { create(:discussion_note_on_issue, project: project) } + let!(:note6) { create(:discussion_note_on_merge_request, :system, noteable: merge_request, project: project) } + + describe '.potentially_resolvable' do + it 'includes diff and discussion notes on merge requests' do + expect(Note.potentially_resolvable).to match_array([note3, note4, note6]) + end + end + + describe '.resolvable' do + it 'includes non-system diff and discussion notes on merge requests' do + expect(Note.resolvable).to match_array([note3, note4]) + end + end + + describe '.resolved' do + it 'includes resolved non-system diff and discussion notes on merge requests' do + expect(Note.resolved).to match_array([note3]) + end + end + + describe '.unresolved' do + it 'includes non-resolved non-system diff and discussion notes on merge requests' do + expect(Note.unresolved).to match_array([note4]) + end + end + end + + describe ".resolve!" do + let(:current_user) { create(:user) } + let!(:commit_note) { create(:diff_note_on_commit, project: project) } + let!(:resolved_note) { create(:discussion_note_on_merge_request, :resolved, noteable: merge_request, project: project) } + let!(:unresolved_note) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } + + before do + described_class.resolve!(current_user) + + commit_note.reload + resolved_note.reload + unresolved_note.reload + end + + it 'resolves only the resolvable, not yet resolved notes' do + expect(commit_note.resolved_at).to be_nil + expect(resolved_note.resolved_by).not_to eq(current_user) + expect(unresolved_note.resolved_at).not_to be_nil + expect(unresolved_note.resolved_by).to eq(current_user) + end + end + + describe ".unresolve!" do + let!(:resolved_note) { create(:discussion_note_on_merge_request, :resolved, noteable: merge_request, project: project) } + + before do + described_class.unresolve! + + resolved_note.reload + end + + it 'unresolves the resolved notes' do + expect(resolved_note.resolved_by).to be_nil + expect(resolved_note.resolved_at).to be_nil + end + end + + describe '#resolvable?' do + context "when potentially resolvable" do + before do + allow(subject).to receive(:potentially_resolvable?).and_return(true) + end + + context "when a system note" do + before do + subject.system = true + end + + it "returns false" do + expect(subject.resolvable?).to be false + end + end + + context "when a regular note" do + it "returns true" do + expect(subject.resolvable?).to be true + end + end + end + + context "when not potentially resolvable" do + before do + allow(subject).to receive(:potentially_resolvable?).and_return(false) + end + + it "returns false" do + expect(subject.resolvable?).to be false + end + end + end + + describe "#to_be_resolved?" do + context "when not resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(false) + end + + it "returns false" do + expect(subject.to_be_resolved?).to be false + end + end + + context "when resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(true) + end + + context "when resolved" do + before do + allow(subject).to receive(:resolved?).and_return(true) + end + + it "returns false" do + expect(subject.to_be_resolved?).to be false + end + end + + context "when not resolved" do + before do + allow(subject).to receive(:resolved?).and_return(false) + end + + it "returns true" do + expect(subject.to_be_resolved?).to be true + end + end + end + end + + describe "#resolved?" do + let(:current_user) { create(:user) } + + context 'when not resolvable' do + before do + subject.resolve!(current_user) + + allow(subject).to receive(:resolvable?).and_return(false) + end + + it 'returns false' do + expect(subject.resolved?).to be_falsey + end + end + + context 'when resolvable' do + context 'when the note has been resolved' do + before do + subject.resolve!(current_user) + end + + it 'returns true' do + expect(subject.resolved?).to be_truthy + end + end + + context 'when the note has not been resolved' do + it 'returns false' do + expect(subject.resolved?).to be_falsey + end + end + end + end + + describe "#resolve!" do + let(:current_user) { create(:user) } + + context "when not resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(false) + end + + it "returns nil" do + expect(subject.resolve!(current_user)).to be_nil + end + + it "doesn't set resolved_at" do + subject.resolve!(current_user) + + expect(subject.resolved_at).to be_nil + end + + it "doesn't set resolved_by" do + subject.resolve!(current_user) + + expect(subject.resolved_by).to be_nil + end + + it "doesn't mark as resolved" do + subject.resolve!(current_user) + + expect(subject.resolved?).to be false + end + end + + context "when resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(true) + end + + context "when already resolved" do + let(:user) { create(:user) } + + before do + subject.resolve!(user) + end + + it "returns nil" do + expect(subject.resolve!(current_user)).to be_nil + end + + it "doesn't change resolved_at" do + expect(subject.resolved_at).not_to be_nil + + expect { subject.resolve!(current_user) }.not_to change { subject.resolved_at } + end + + it "doesn't change resolved_by" do + expect(subject.resolved_by).to eq(user) + + expect { subject.resolve!(current_user) }.not_to change { subject.resolved_by } + end + + it "doesn't change resolved status" do + expect(subject.resolved?).to be true + + expect { subject.resolve!(current_user) }.not_to change { subject.resolved? } + end + end + + context "when not yet resolved" do + it "returns true" do + expect(subject.resolve!(current_user)).to be true + end + + it "sets resolved_at" do + subject.resolve!(current_user) + + expect(subject.resolved_at).not_to be_nil + end + + it "sets resolved_by" do + subject.resolve!(current_user) + + expect(subject.resolved_by).to eq(current_user) + end + + it "marks as resolved" do + subject.resolve!(current_user) + + expect(subject.resolved?).to be true + end + end + end + end + + describe "#unresolve!" do + context "when not resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(false) + end + + it "returns nil" do + expect(subject.unresolve!).to be_nil + end + end + + context "when resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(true) + end + + context "when resolved" do + let(:user) { create(:user) } + + before do + subject.resolve!(user) + end + + it "returns true" do + expect(subject.unresolve!).to be true + end + + it "unsets resolved_at" do + subject.unresolve! + + expect(subject.resolved_at).to be_nil + end + + it "unsets resolved_by" do + subject.unresolve! + + expect(subject.resolved_by).to be_nil + end + + it "unmarks as resolved" do + subject.unresolve! + + expect(subject.resolved?).to be false + end + end + + context "when not resolved" do + it "returns nil" do + expect(subject.unresolve!).to be_nil + end + end + end + end +end diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb index 677e60e1282..f191605dbdb 100644 --- a/spec/models/concerns/routable_spec.rb +++ b/spec/models/concerns/routable_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Group, 'Routable' do - let!(:group) { create(:group) } + let!(:group) { create(:group, name: 'foo') } describe 'Validations' do it { is_expected.to validate_presence_of(:route) } @@ -81,6 +81,113 @@ describe Group, 'Routable' do it { is_expected.to eq([nested_group]) } end + describe '.member_self_and_descendants' do + let!(:user) { create(:user) } + let!(:nested_group) { create(:group, parent: group) } + + before { group.add_owner(user) } + subject { described_class.member_self_and_descendants(user.id) } + + it { is_expected.to match_array [group, nested_group] } + end + + describe '.member_hierarchy' do + # foo/bar would also match foo/barbaz instead of just foo/bar and foo/bar/baz + let!(:user) { create(:user) } + + # group + # _______ (foo) _______ + # | | + # | | + # nested_group_1 nested_group_2 + # (bar) (barbaz) + # | | + # | | + # nested_group_1_1 nested_group_2_1 + # (baz) (baz) + # + let!(:nested_group_1) { create :group, parent: group, name: 'bar' } + let!(:nested_group_1_1) { create :group, parent: nested_group_1, name: 'baz' } + let!(:nested_group_2) { create :group, parent: group, name: 'barbaz' } + let!(:nested_group_2_1) { create :group, parent: nested_group_2, name: 'baz' } + + context 'user is not a member of any group' do + subject { described_class.member_hierarchy(user.id) } + + it 'returns an empty array' do + is_expected.to eq [] + end + end + + context 'user is member of all groups' do + before do + group.add_owner(user) + nested_group_1.add_owner(user) + nested_group_1_1.add_owner(user) + nested_group_2.add_owner(user) + nested_group_2_1.add_owner(user) + end + subject { described_class.member_hierarchy(user.id) } + + it 'returns all groups' do + is_expected.to match_array [ + group, + nested_group_1, nested_group_1_1, + nested_group_2, nested_group_2_1 + ] + end + end + + context 'user is member of the top group' do + before { group.add_owner(user) } + subject { described_class.member_hierarchy(user.id) } + + it 'returns all groups' do + is_expected.to match_array [ + group, + nested_group_1, nested_group_1_1, + nested_group_2, nested_group_2_1 + ] + end + end + + context 'user is member of the first child (internal node), branch 1' do + before { nested_group_1.add_owner(user) } + subject { described_class.member_hierarchy(user.id) } + + it 'returns the groups in the hierarchy' do + is_expected.to match_array [ + group, + nested_group_1, nested_group_1_1 + ] + end + end + + context 'user is member of the first child (internal node), branch 2' do + before { nested_group_2.add_owner(user) } + subject { described_class.member_hierarchy(user.id) } + + it 'returns the groups in the hierarchy' do + is_expected.to match_array [ + group, + nested_group_2, nested_group_2_1 + ] + end + end + + context 'user is member of the last child (leaf node)' do + before { nested_group_1_1.add_owner(user) } + subject { described_class.member_hierarchy(user.id) } + + it 'returns the groups in the hierarchy' do + is_expected.to match_array [ + group, + nested_group_1, nested_group_1_1 + ] + end + end + end + describe '#full_path' do let(:group) { create(:group) } let(:nested_group) { create(:group, parent: group) } diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb new file mode 100644 index 00000000000..6d6c9f2adfc --- /dev/null +++ b/spec/models/container_repository_spec.rb @@ -0,0 +1,224 @@ +require 'spec_helper' + +describe ContainerRepository do + let(:group) { create(:group, name: 'group') } + let(:project) { create(:project, path: 'test', group: group) } + + let(:repository) do + create(:container_repository, name: 'my_image', project: project) + end + + before do + stub_container_registry_config(enabled: true, + api_url: 'http://registry.gitlab', + host_port: 'registry.gitlab') + + stub_request(:get, 'http://registry.gitlab/v2/group/test/my_image/tags/list') + .with(headers: { 'Accept' => 'application/vnd.docker.distribution.manifest.v2+json' }) + .to_return( + status: 200, + body: JSON.dump(tags: ['test_tag']), + headers: { 'Content-Type' => 'application/json' }) + end + + describe 'associations' do + it 'belongs to the project' do + expect(repository).to belong_to(:project) + end + end + + describe '#tag' do + it 'has a test tag' do + expect(repository.tag('test')).not_to be_nil + end + end + + describe '#path' do + it 'returns a full path to the repository' do + expect(repository.path).to eq('group/test/my_image') + end + end + + describe '#manifest' do + it 'returns non-empty manifest' do + expect(repository.manifest).not_to be_nil + end + end + + describe '#valid?' do + it 'is a valid repository' do + expect(repository).to be_valid + end + end + + describe '#tags' do + it 'returns non-empty tags list' do + expect(repository.tags).not_to be_empty + end + end + + describe '#has_tags?' do + it 'has tags' do + expect(repository).to have_tags + end + end + + describe '#delete_tags!' do + let(:repository) do + create(:container_repository, name: 'my_image', + tags: %w[latest rc1], + project: project) + end + + context 'when action succeeds' do + it 'returns status that indicates success' do + expect(repository.client) + .to receive(:delete_repository_tag) + .and_return(true) + + expect(repository.delete_tags!).to be_truthy + end + end + + context 'when action fails' do + it 'returns status that indicates failure' do + expect(repository.client) + .to receive(:delete_repository_tag) + .and_return(false) + + expect(repository.delete_tags!).to be_falsey + end + end + end + + describe '#location' do + context 'when registry is running on a custom port' do + before do + stub_container_registry_config(enabled: true, + api_url: 'http://registry.gitlab:5000', + host_port: 'registry.gitlab:5000') + end + + it 'returns a full location of the repository' do + expect(repository.location) + .to eq 'registry.gitlab:5000/group/test/my_image' + end + end + end + + describe '#root_repository?' do + context 'when repository is a root repository' do + let(:repository) { create(:container_repository, :root) } + + it 'returns true' do + expect(repository).to be_root_repository + end + end + + context 'when repository is not a root repository' do + it 'returns false' do + expect(repository).not_to be_root_repository + end + end + end + + describe '.build_from_path' do + let(:registry_path) do + ContainerRegistry::Path.new(project.full_path + '/some/image') + end + + let(:repository) do + described_class.build_from_path(registry_path) + end + + it 'fabricates repository assigned to a correct project' do + expect(repository.project).to eq project + end + + it 'fabricates repository with a correct name' do + expect(repository.name).to eq 'some/image' + end + + it 'is not persisted' do + expect(repository).not_to be_persisted + end + end + + describe '.create_from_path!' do + let(:repository) do + described_class.create_from_path!(ContainerRegistry::Path.new(path)) + end + + let(:repository_path) { ContainerRegistry::Path.new(path) } + + context 'when received multi-level repository path' do + let(:path) { project.full_path + '/some/image' } + + it 'fabricates repository assigned to a correct project' do + expect(repository.project).to eq project + end + + it 'fabricates repository with a correct name' do + expect(repository.name).to eq 'some/image' + end + end + + context 'when path is too long' do + let(:path) do + project.full_path + '/a/b/c/d/e/f/g/h/i/j/k/l/n/o/p/s/t/u/x/y/z' + end + + it 'does not create repository and raises error' do + expect { repository }.to raise_error( + ContainerRegistry::Path::InvalidRegistryPathError) + end + end + + context 'when received multi-level repository with nested groups' do + let(:group) { create(:group, :nested, name: 'nested') } + let(:path) { project.full_path + '/some/image' } + + it 'fabricates repository assigned to a correct project' do + expect(repository.project).to eq project + end + + it 'fabricates repository with a correct name' do + expect(repository.name).to eq 'some/image' + end + + it 'has path including a nested group' do + expect(repository.path).to include 'nested/test/some/image' + end + end + + context 'when received root repository path' do + let(:path) { project.full_path } + + it 'fabricates repository assigned to a correct project' do + expect(repository.project).to eq project + end + + it 'fabricates repository with an empty name' do + expect(repository.name).to be_empty + end + end + end + + describe '.build_root_repository' do + let(:repository) do + described_class.build_root_repository(project) + end + + it 'fabricates a root repository object' do + expect(repository).to be_root_repository + end + + it 'assignes it to the correct project' do + expect(repository.project).to eq project + end + + it 'does not persist it' do + expect(repository).not_to be_persisted + end + end +end diff --git a/spec/models/diff_discussion_spec.rb b/spec/models/diff_discussion_spec.rb new file mode 100644 index 00000000000..48e7c0a822c --- /dev/null +++ b/spec/models/diff_discussion_spec.rb @@ -0,0 +1,19 @@ +require 'spec_helper' + +describe DiffDiscussion, model: true do + subject { described_class.new([first_note, second_note, third_note]) } + + let(:first_note) { create(:diff_note_on_merge_request) } + let(:merge_request) { first_note.noteable } + let(:project) { first_note.project } + let(:second_note) { create(:diff_note_on_merge_request, noteable: merge_request, project: project, in_reply_to: first_note) } + let(:third_note) { create(:diff_note_on_merge_request, noteable: merge_request, project: project, in_reply_to: first_note) } + + describe '#reply_attributes' do + it 'includes position and original_position' do + attributes = subject.reply_attributes + expect(attributes[:position]).to eq(first_note.position.to_json) + expect(attributes[:original_position]).to eq(first_note.original_position.to_json) + end + end +end diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb index 9ea3a4b7020..f32b6b99b3d 100644 --- a/spec/models/diff_note_spec.rb +++ b/spec/models/diff_note_spec.rb @@ -31,43 +31,6 @@ describe DiffNote, models: true do subject { create(:diff_note_on_merge_request, project: project, position: position, noteable: merge_request) } - describe ".resolve!" do - let(:current_user) { create(:user) } - let!(:commit_note) { create(:diff_note_on_commit) } - let!(:resolved_note) { create(:diff_note_on_merge_request, :resolved) } - let!(:unresolved_note) { create(:diff_note_on_merge_request) } - - before do - described_class.resolve!(current_user) - - commit_note.reload - resolved_note.reload - unresolved_note.reload - end - - it 'resolves only the resolvable, not yet resolved notes' do - expect(commit_note.resolved_at).to be_nil - expect(resolved_note.resolved_by).not_to eq(current_user) - expect(unresolved_note.resolved_at).not_to be_nil - expect(unresolved_note.resolved_by).to eq(current_user) - end - end - - describe ".unresolve!" do - let!(:resolved_note) { create(:diff_note_on_merge_request, :resolved) } - - before do - described_class.unresolve! - - resolved_note.reload - end - - it 'unresolves the resolved notes' do - expect(resolved_note.resolved_by).to be_nil - expect(resolved_note.resolved_at).to be_nil - end - end - describe "#position=" do context "when provided a string" do it "sets the position" do @@ -94,6 +57,32 @@ describe DiffNote, models: true do end end + describe "#original_position=" do + context "when provided a string" do + it "sets the original position" do + subject.original_position = new_position.to_json + + expect(subject.original_position).to eq(new_position) + end + end + + context "when provided a hash" do + it "sets the original position" do + subject.original_position = new_position.to_h + + expect(subject.original_position).to eq(new_position) + end + end + + context "when provided a position object" do + it "sets the original position" do + subject.original_position = new_position + + expect(subject.original_position).to eq(new_position) + end + end + end + describe "#diff_file" do it "returns the correct diff file" do diff_file = subject.diff_file @@ -166,6 +155,23 @@ describe DiffNote, models: true do end end + describe '#latest_merge_request_diff' do + context 'when active' do + it 'returns the current merge request diff' do + expect(subject.latest_merge_request_diff).to eq(merge_request.merge_request_diff) + end + end + + context 'when outdated' do + let!(:old_merge_request_diff) { merge_request.merge_request_diff } + let!(:new_merge_request_diff) { merge_request.merge_request_diffs.create(diff_refs: commit.diff_refs) } + + it 'returns the latest merge request diff that this diff note applied to' do + expect(subject.latest_merge_request_diff).to eq(old_merge_request_diff) + end + end + end + describe "creation" do describe "updating of position" do context "when noteable is a commit" do @@ -226,252 +232,6 @@ describe DiffNote, models: true do end end - describe "#resolvable?" do - context "when noteable is a commit" do - subject { create(:diff_note_on_commit, project: project, position: position) } - - it "returns false" do - expect(subject.resolvable?).to be false - end - end - - context "when noteable is a merge request" do - context "when a system note" do - before do - subject.system = true - end - - it "returns false" do - expect(subject.resolvable?).to be false - end - end - - context "when a regular note" do - it "returns true" do - expect(subject.resolvable?).to be true - end - end - end - end - - describe "#to_be_resolved?" do - context "when not resolvable" do - before do - allow(subject).to receive(:resolvable?).and_return(false) - end - - it "returns false" do - expect(subject.to_be_resolved?).to be false - end - end - - context "when resolvable" do - before do - allow(subject).to receive(:resolvable?).and_return(true) - end - - context "when resolved" do - before do - allow(subject).to receive(:resolved?).and_return(true) - end - - it "returns false" do - expect(subject.to_be_resolved?).to be false - end - end - - context "when not resolved" do - before do - allow(subject).to receive(:resolved?).and_return(false) - end - - it "returns true" do - expect(subject.to_be_resolved?).to be true - end - end - end - end - - describe "#resolve!" do - let(:current_user) { create(:user) } - - context "when not resolvable" do - before do - allow(subject).to receive(:resolvable?).and_return(false) - end - - it "returns nil" do - expect(subject.resolve!(current_user)).to be_nil - end - - it "doesn't set resolved_at" do - subject.resolve!(current_user) - - expect(subject.resolved_at).to be_nil - end - - it "doesn't set resolved_by" do - subject.resolve!(current_user) - - expect(subject.resolved_by).to be_nil - end - - it "doesn't mark as resolved" do - subject.resolve!(current_user) - - expect(subject.resolved?).to be false - end - end - - context "when resolvable" do - before do - allow(subject).to receive(:resolvable?).and_return(true) - end - - context "when already resolved" do - let(:user) { create(:user) } - - before do - subject.resolve!(user) - end - - it "returns nil" do - expect(subject.resolve!(current_user)).to be_nil - end - - it "doesn't change resolved_at" do - expect(subject.resolved_at).not_to be_nil - - expect { subject.resolve!(current_user) }.not_to change { subject.resolved_at } - end - - it "doesn't change resolved_by" do - expect(subject.resolved_by).to eq(user) - - expect { subject.resolve!(current_user) }.not_to change { subject.resolved_by } - end - - it "doesn't change resolved status" do - expect(subject.resolved?).to be true - - expect { subject.resolve!(current_user) }.not_to change { subject.resolved? } - end - end - - context "when not yet resolved" do - it "returns true" do - expect(subject.resolve!(current_user)).to be true - end - - it "sets resolved_at" do - subject.resolve!(current_user) - - expect(subject.resolved_at).not_to be_nil - end - - it "sets resolved_by" do - subject.resolve!(current_user) - - expect(subject.resolved_by).to eq(current_user) - end - - it "marks as resolved" do - subject.resolve!(current_user) - - expect(subject.resolved?).to be true - end - end - end - end - - describe "#unresolve!" do - context "when not resolvable" do - before do - allow(subject).to receive(:resolvable?).and_return(false) - end - - it "returns nil" do - expect(subject.unresolve!).to be_nil - end - end - - context "when resolvable" do - before do - allow(subject).to receive(:resolvable?).and_return(true) - end - - context "when resolved" do - let(:user) { create(:user) } - - before do - subject.resolve!(user) - end - - it "returns true" do - expect(subject.unresolve!).to be true - end - - it "unsets resolved_at" do - subject.unresolve! - - expect(subject.resolved_at).to be_nil - end - - it "unsets resolved_by" do - subject.unresolve! - - expect(subject.resolved_by).to be_nil - end - - it "unmarks as resolved" do - subject.unresolve! - - expect(subject.resolved?).to be false - end - end - - context "when not resolved" do - it "returns nil" do - expect(subject.unresolve!).to be_nil - end - end - end - end - - describe "#discussion" do - context "when not resolvable" do - before do - allow(subject).to receive(:resolvable?).and_return(false) - end - - it "returns nil" do - expect(subject.discussion).to be_nil - end - end - - context "when resolvable" do - let!(:diff_note2) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: subject.position) } - let!(:diff_note3) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: active_position2) } - - let(:active_position2) do - Gitlab::Diff::Position.new( - old_path: "files/ruby/popen.rb", - new_path: "files/ruby/popen.rb", - old_line: 16, - new_line: 22, - diff_refs: merge_request.diff_refs - ) - end - - it "returns the discussion this note is in" do - discussion = subject.discussion - - expect(discussion.id).to eq(subject.discussion_id) - expect(discussion.notes).to eq([subject, diff_note2]) - end - end - end - describe "#discussion_id" do let(:note) { create(:diff_note_on_merge_request) } @@ -496,29 +256,4 @@ describe DiffNote, models: true do end end end - - describe "#original_discussion_id" do - let(:note) { create(:diff_note_on_merge_request) } - - context "when it is newly created" do - it "has a discussion id" do - expect(note.original_discussion_id).not_to be_nil - expect(note.original_discussion_id).to match(/\A\h{40}\z/) - end - end - - context "when it didn't store a discussion id before" do - before do - note.update_column(:original_discussion_id, nil) - end - - it "has a discussion id" do - # The original_discussion_id is set in `after_initialize`, so `reload` won't work - reloaded_note = Note.find(note.id) - - expect(reloaded_note.original_discussion_id).not_to be_nil - expect(reloaded_note.original_discussion_id).to match(/\A\h{40}\z/) - end - end - end end diff --git a/spec/models/discussion_spec.rb b/spec/models/discussion_spec.rb index bc32fadd391..0221e23ced8 100644 --- a/spec/models/discussion_spec.rb +++ b/spec/models/discussion_spec.rb @@ -4,618 +4,27 @@ describe Discussion, model: true do subject { described_class.new([first_note, second_note, third_note]) } let(:first_note) { create(:diff_note_on_merge_request) } - let(:second_note) { create(:diff_note_on_merge_request) } + let(:merge_request) { first_note.noteable } + let(:second_note) { create(:diff_note_on_merge_request, in_reply_to: first_note) } let(:third_note) { create(:diff_note_on_merge_request) } - describe "#resolvable?" do - context "when a diff discussion" do - before do - allow(subject).to receive(:diff_discussion?).and_return(true) - end - - context "when all notes are unresolvable" do - before do - allow(first_note).to receive(:resolvable?).and_return(false) - allow(second_note).to receive(:resolvable?).and_return(false) - allow(third_note).to receive(:resolvable?).and_return(false) - end - - it "returns false" do - expect(subject.resolvable?).to be false - end - end - - context "when some notes are unresolvable and some notes are resolvable" do - before do - allow(first_note).to receive(:resolvable?).and_return(true) - allow(second_note).to receive(:resolvable?).and_return(false) - allow(third_note).to receive(:resolvable?).and_return(true) - end - - it "returns true" do - expect(subject.resolvable?).to be true - end - end - - context "when all notes are resolvable" do - before do - allow(first_note).to receive(:resolvable?).and_return(true) - allow(second_note).to receive(:resolvable?).and_return(true) - allow(third_note).to receive(:resolvable?).and_return(true) - end - - it "returns true" do - expect(subject.resolvable?).to be true - end - end - end - - context "when not a diff discussion" do - before do - allow(subject).to receive(:diff_discussion?).and_return(false) - end - - it "returns false" do - expect(subject.resolvable?).to be false - end - end - end - - describe "#resolved?" do - context "when not resolvable" do - before do - allow(subject).to receive(:resolvable?).and_return(false) - end - - it "returns false" do - expect(subject.resolved?).to be false - end - end - - context "when resolvable" do - before do - allow(subject).to receive(:resolvable?).and_return(true) - - allow(first_note).to receive(:resolvable?).and_return(true) - allow(second_note).to receive(:resolvable?).and_return(false) - allow(third_note).to receive(:resolvable?).and_return(true) - end - - context "when all resolvable notes are resolved" do - before do - allow(first_note).to receive(:resolved?).and_return(true) - allow(third_note).to receive(:resolved?).and_return(true) - end - - it "returns true" do - expect(subject.resolved?).to be true - end - end - - context "when some resolvable notes are not resolved" do - before do - allow(first_note).to receive(:resolved?).and_return(true) - allow(third_note).to receive(:resolved?).and_return(false) - end - - it "returns false" do - expect(subject.resolved?).to be false - end - end - end - end - - describe "#to_be_resolved?" do - context "when not resolvable" do - before do - allow(subject).to receive(:resolvable?).and_return(false) - end - - it "returns false" do - expect(subject.to_be_resolved?).to be false - end - end - - context "when resolvable" do - before do - allow(subject).to receive(:resolvable?).and_return(true) - - allow(first_note).to receive(:resolvable?).and_return(true) - allow(second_note).to receive(:resolvable?).and_return(false) - allow(third_note).to receive(:resolvable?).and_return(true) - end - - context "when all resolvable notes are resolved" do - before do - allow(first_note).to receive(:resolved?).and_return(true) - allow(third_note).to receive(:resolved?).and_return(true) - end - - it "returns false" do - expect(subject.to_be_resolved?).to be false - end - end - - context "when some resolvable notes are not resolved" do - before do - allow(first_note).to receive(:resolved?).and_return(true) - allow(third_note).to receive(:resolved?).and_return(false) - end - - it "returns true" do - expect(subject.to_be_resolved?).to be true - end - end - end - end - - describe "#can_resolve?" do - let(:current_user) { create(:user) } - - context "when not resolvable" do - before do - allow(subject).to receive(:resolvable?).and_return(false) - end - - it "returns false" do - expect(subject.can_resolve?(current_user)).to be false - end - end - - context "when resolvable" do - before do - allow(subject).to receive(:resolvable?).and_return(true) - end - - context "when not signed in" do - let(:current_user) { nil } - - it "returns false" do - expect(subject.can_resolve?(current_user)).to be false - end - end - - context "when signed in" do - context "when the signed in user is the noteable author" do - before do - subject.noteable.author = current_user - end - - it "returns true" do - expect(subject.can_resolve?(current_user)).to be true - end - end - - context "when the signed in user can push to the project" do - before do - subject.project.team << [current_user, :master] - end - - it "returns true" do - expect(subject.can_resolve?(current_user)).to be true - end - end - - context "when the signed in user is a random user" do - it "returns false" do - expect(subject.can_resolve?(current_user)).to be false - end - end - end - end - end - - describe "#resolve!" do - let(:current_user) { create(:user) } - - context "when not resolvable" do - before do - allow(subject).to receive(:resolvable?).and_return(false) - end - - it "returns nil" do - expect(subject.resolve!(current_user)).to be_nil - end - - it "doesn't set resolved_at" do - subject.resolve!(current_user) - - expect(subject.resolved_at).to be_nil - end - - it "doesn't set resolved_by" do - subject.resolve!(current_user) - - expect(subject.resolved_by).to be_nil - end - - it "doesn't mark as resolved" do - subject.resolve!(current_user) - - expect(subject.resolved?).to be false - end - end - - context "when resolvable" do - let(:user) { create(:user) } - let(:second_note) { create(:diff_note_on_commit) } # unresolvable - - before do - allow(subject).to receive(:resolvable?).and_return(true) - end - - context "when all resolvable notes are resolved" do - before do - first_note.resolve!(user) - third_note.resolve!(user) - - first_note.reload - third_note.reload - end - - it "doesn't change resolved_at on the resolved notes" do - expect(first_note.resolved_at).not_to be_nil - expect(third_note.resolved_at).not_to be_nil - - expect { subject.resolve!(current_user) }.not_to change { first_note.resolved_at } - expect { subject.resolve!(current_user) }.not_to change { third_note.resolved_at } - end - - it "doesn't change resolved_by on the resolved notes" do - expect(first_note.resolved_by).to eq(user) - expect(third_note.resolved_by).to eq(user) - - expect { subject.resolve!(current_user) }.not_to change { first_note.resolved_by } - expect { subject.resolve!(current_user) }.not_to change { third_note.resolved_by } - end - - it "doesn't change the resolved state on the resolved notes" do - expect(first_note.resolved?).to be true - expect(third_note.resolved?).to be true - - expect { subject.resolve!(current_user) }.not_to change { first_note.resolved? } - expect { subject.resolve!(current_user) }.not_to change { third_note.resolved? } - end - - it "doesn't change resolved_at" do - expect(subject.resolved_at).not_to be_nil - - expect { subject.resolve!(current_user) }.not_to change { subject.resolved_at } - end - - it "doesn't change resolved_by" do - expect(subject.resolved_by).to eq(user) - - expect { subject.resolve!(current_user) }.not_to change { subject.resolved_by } - end - - it "doesn't change resolved state" do - expect(subject.resolved?).to be true - - expect { subject.resolve!(current_user) }.not_to change { subject.resolved? } - end - end - - context "when some resolvable notes are resolved" do - before do - first_note.resolve!(user) - end - - it "doesn't change resolved_at on the resolved note" do - expect(first_note.resolved_at).not_to be_nil - - expect { subject.resolve!(current_user) }. - not_to change { first_note.reload.resolved_at } - end - - it "doesn't change resolved_by on the resolved note" do - expect(first_note.resolved_by).to eq(user) - - expect { subject.resolve!(current_user) }. - not_to change { first_note.reload && first_note.resolved_by } - end - - it "doesn't change the resolved state on the resolved note" do - expect(first_note.resolved?).to be true - - expect { subject.resolve!(current_user) }. - not_to change { first_note.reload && first_note.resolved? } - end - - it "sets resolved_at on the unresolved note" do - subject.resolve!(current_user) - third_note.reload - - expect(third_note.resolved_at).not_to be_nil - end - - it "sets resolved_by on the unresolved note" do - subject.resolve!(current_user) - third_note.reload - - expect(third_note.resolved_by).to eq(current_user) - end - - it "marks the unresolved note as resolved" do - subject.resolve!(current_user) - third_note.reload - - expect(third_note.resolved?).to be true - end - - it "sets resolved_at" do - subject.resolve!(current_user) - - expect(subject.resolved_at).not_to be_nil - end - - it "sets resolved_by" do - subject.resolve!(current_user) - - expect(subject.resolved_by).to eq(current_user) - end - - it "marks as resolved" do - subject.resolve!(current_user) - - expect(subject.resolved?).to be true - end - end - - context "when no resolvable notes are resolved" do - it "sets resolved_at on the unresolved notes" do - subject.resolve!(current_user) - first_note.reload - third_note.reload - - expect(first_note.resolved_at).not_to be_nil - expect(third_note.resolved_at).not_to be_nil - end - - it "sets resolved_by on the unresolved notes" do - subject.resolve!(current_user) - first_note.reload - third_note.reload - - expect(first_note.resolved_by).to eq(current_user) - expect(third_note.resolved_by).to eq(current_user) - end - - it "marks the unresolved notes as resolved" do - subject.resolve!(current_user) - first_note.reload - third_note.reload - - expect(first_note.resolved?).to be true - expect(third_note.resolved?).to be true - end - - it "sets resolved_at" do - subject.resolve!(current_user) - first_note.reload - third_note.reload - - expect(subject.resolved_at).not_to be_nil - end - - it "sets resolved_by" do - subject.resolve!(current_user) - first_note.reload - third_note.reload - - expect(subject.resolved_by).to eq(current_user) - end - - it "marks as resolved" do - subject.resolve!(current_user) - first_note.reload - third_note.reload - - expect(subject.resolved?).to be true - end - end - end - end - - describe "#unresolve!" do - context "when not resolvable" do - before do - allow(subject).to receive(:resolvable?).and_return(false) - end - - it "returns nil" do - expect(subject.unresolve!).to be_nil - end - end - - context "when resolvable" do - let(:user) { create(:user) } - - before do - allow(subject).to receive(:resolvable?).and_return(true) - - allow(first_note).to receive(:resolvable?).and_return(true) - allow(second_note).to receive(:resolvable?).and_return(false) - allow(third_note).to receive(:resolvable?).and_return(true) - end - - context "when all resolvable notes are resolved" do - before do - first_note.resolve!(user) - third_note.resolve!(user) - end - - it "unsets resolved_at on the resolved notes" do - subject.unresolve! - first_note.reload - third_note.reload - - expect(first_note.resolved_at).to be_nil - expect(third_note.resolved_at).to be_nil - end - - it "unsets resolved_by on the resolved notes" do - subject.unresolve! - first_note.reload - third_note.reload - - expect(first_note.resolved_by).to be_nil - expect(third_note.resolved_by).to be_nil - end - - it "unmarks the resolved notes as resolved" do - subject.unresolve! - first_note.reload - third_note.reload - - expect(first_note.resolved?).to be false - expect(third_note.resolved?).to be false - end - - it "unsets resolved_at" do - subject.unresolve! - first_note.reload - third_note.reload - - expect(subject.resolved_at).to be_nil - end - - it "unsets resolved_by" do - subject.unresolve! - first_note.reload - third_note.reload - - expect(subject.resolved_by).to be_nil - end - - it "unmarks as resolved" do - subject.unresolve! - - expect(subject.resolved?).to be false - end - end - - context "when some resolvable notes are resolved" do - before do - first_note.resolve!(user) - end - - it "unsets resolved_at on the resolved note" do - subject.unresolve! - - expect(subject.first_note.resolved_at).to be_nil - end - - it "unsets resolved_by on the resolved note" do - subject.unresolve! - - expect(subject.first_note.resolved_by).to be_nil - end - - it "unmarks the resolved note as resolved" do - subject.unresolve! - - expect(subject.first_note.resolved?).to be false - end - end + describe '.build' do + it 'returns a discussion of the right type' do + discussion = described_class.build([first_note, second_note], merge_request) + expect(discussion).to be_a(DiffDiscussion) + expect(discussion.notes.count).to be(2) + expect(discussion.first_note).to be(first_note) + expect(discussion.noteable).to be(merge_request) end end - describe "#first_note_to_resolve" do - it "returns the first not that still needs to be resolved" do - allow(first_note).to receive(:to_be_resolved?).and_return(false) - allow(second_note).to receive(:to_be_resolved?).and_return(true) - - expect(subject.first_note_to_resolve).to eq(second_note) - end - end - - describe "#collapsed?" do - context "when a diff discussion" do - before do - allow(subject).to receive(:diff_discussion?).and_return(true) - end - - context "when resolvable" do - before do - allow(subject).to receive(:resolvable?).and_return(true) - end - - context "when resolved" do - before do - allow(subject).to receive(:resolved?).and_return(true) - end - - it "returns true" do - expect(subject.collapsed?).to be true - end - end - - context "when not resolved" do - before do - allow(subject).to receive(:resolved?).and_return(false) - end - - it "returns false" do - expect(subject.collapsed?).to be false - end - end - end - - context "when not resolvable" do - before do - allow(subject).to receive(:resolvable?).and_return(false) - end - - context "when active" do - before do - allow(subject).to receive(:active?).and_return(true) - end - - it "returns false" do - expect(subject.collapsed?).to be false - end - end - - context "when outdated" do - before do - allow(subject).to receive(:active?).and_return(false) - end - - it "returns true" do - expect(subject.collapsed?).to be true - end - end - end - end - - context "when not a diff discussion" do - before do - allow(subject).to receive(:diff_discussion?).and_return(false) - end - - it "returns false" do - expect(subject.collapsed?).to be false - end - end - end - - describe "#truncated_diff_lines" do - let(:truncated_lines) { subject.truncated_diff_lines } - - context "when diff is greater than allowed number of truncated diff lines " do - it "returns fewer lines" do - expect(subject.diff_lines.count).to be > described_class::NUMBER_OF_TRUNCATED_DIFF_LINES - - expect(truncated_lines.count).to be <= described_class::NUMBER_OF_TRUNCATED_DIFF_LINES - end - end - - context "when some diff lines are meta" do - it "returns no meta lines" do - expect(subject.diff_lines).to include(be_meta) - expect(truncated_lines).not_to include(be_meta) - end + describe '.build_collection' do + it 'returns an array of discussions of the right type' do + discussions = described_class.build_collection([first_note, second_note, third_note], merge_request) + expect(discussions).to eq([ + DiffDiscussion.new([first_note, second_note], merge_request), + DiffDiscussion.new([third_note], merge_request) + ]) end end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 9e00f2247e8..28e5c3f80f4 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -100,13 +100,28 @@ describe Environment, models: true do let(:head_commit) { project.commit } let(:commit) { project.commit.parent } - it 'returns deployment id for the environment' do - expect(environment.first_deployment_for(commit)).to eq deployment1 - end + context 'Gitaly find_ref_name feature disabled' do + it 'returns deployment id for the environment' do + expect(environment.first_deployment_for(commit)).to eq deployment1 + end - it 'return nil when no deployment is found' do - expect(environment.first_deployment_for(head_commit)).to eq nil + it 'return nil when no deployment is found' do + expect(environment.first_deployment_for(head_commit)).to eq nil + end end + + # TODO: Uncomment when feature is reenabled + # context 'Gitaly find_ref_name feature enabled' do + # before do + # allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:find_ref_name).and_return(true) + # end + # + # it 'calls GitalyClient' do + # expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:find_ref_name) + # + # environment.first_deployment_for(commit) + # end + # end end describe '#environment_type' do diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 5d87938235a..8ffde6f7fbb 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -55,6 +55,8 @@ describe Group, models: true do it { is_expected.to validate_uniqueness_of(:name).scoped_to(:parent_id) } it { is_expected.to validate_presence_of :path } it { is_expected.not_to validate_presence_of :owner } + it { is_expected.to validate_presence_of :two_factor_grace_period } + it { is_expected.to validate_numericality_of(:two_factor_grace_period).is_greater_than_or_equal_to(0) } end describe '.visible_to_user' do @@ -315,4 +317,44 @@ describe Group, models: true do to include(master.id, developer.id) end end + + describe '#update_two_factor_requirement' do + let(:user) { create(:user) } + + before do + group.add_user(user, GroupMember::OWNER) + end + + it 'is called when require_two_factor_authentication is changed' do + expect_any_instance_of(User).to receive(:update_two_factor_requirement) + + group.update!(require_two_factor_authentication: true) + end + + it 'is called when two_factor_grace_period is changed' do + expect_any_instance_of(User).to receive(:update_two_factor_requirement) + + group.update!(two_factor_grace_period: 23) + end + + it 'is not called when other attributes are changed' do + expect_any_instance_of(User).not_to receive(:update_two_factor_requirement) + + group.update!(description: 'foobar') + end + + it 'calls #update_two_factor_requirement on each group member' do + other_user = create(:user) + group.add_user(other_user, GroupMember::OWNER) + + calls = 0 + allow_any_instance_of(User).to receive(:update_two_factor_requirement) do + calls += 1 + end + + group.update!(require_two_factor_authentication: true, two_factor_grace_period: 23) + + expect(calls).to eq 2 + end + end end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 4bdd46a581d..d057c9cf6e9 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -134,15 +134,6 @@ describe Issue, models: true do end end - describe '#is_being_reassigned?' do - it 'returns issues assigned to user' do - user = create(:user) - create_list(:issue, 2, assignee: user) - - expect(Issue.open_for(user).count).to eq 2 - end - end - describe '#closed_by_merge_requests' do let(:project) { create(:project, :repository) } let(:issue) { create(:issue, project: project)} diff --git a/spec/models/label_spec.rb b/spec/models/label_spec.rb index a9139f7d4ab..80ca19acdda 100644 --- a/spec/models/label_spec.rb +++ b/spec/models/label_spec.rb @@ -42,11 +42,27 @@ describe Label, models: true do end end + describe '#color' do + it 'strips color' do + label = described_class.new(color: ' #abcdef ') + label.valid? + + expect(label.color).to eq('#abcdef') + end + end + describe '#title' do it 'sanitizes title' do label = described_class.new(title: '<b>foo & bar?</b>') expect(label.title).to eq('foo & bar?') end + + it 'strips title' do + label = described_class.new(title: ' label ') + label.valid? + + expect(label.title).to eq('label') + end end describe 'priorization' do diff --git a/spec/models/legacy_diff_discussion_spec.rb b/spec/models/legacy_diff_discussion_spec.rb new file mode 100644 index 00000000000..153e757a0ef --- /dev/null +++ b/spec/models/legacy_diff_discussion_spec.rb @@ -0,0 +1,11 @@ +require 'spec_helper' + +describe LegacyDiffDiscussion, models: true do + subject { create(:legacy_diff_note_on_merge_request).to_discussion } + + describe '#reply_attributes' do + it 'includes line_code' do + expect(subject.reply_attributes[:line_code]).to eq(subject.line_code) + end + end +end diff --git a/spec/models/legacy_diff_note_spec.rb b/spec/models/legacy_diff_note_spec.rb deleted file mode 100644 index 81517a18b74..00000000000 --- a/spec/models/legacy_diff_note_spec.rb +++ /dev/null @@ -1,101 +0,0 @@ -require 'spec_helper' - -describe LegacyDiffNote, models: true do - describe "Commit diff line notes" do - let!(:note) { create(:legacy_diff_note_on_commit, note: "+1 from me") } - let!(:commit) { note.noteable } - - it "saves a valid note" do - expect(note.commit_id).to eq(commit.id) - expect(note.noteable.id).to eq(commit.id) - end - - it "is recognized by #legacy_diff_note?" do - expect(note).to be_legacy_diff_note - end - end - - describe '#active?' do - it 'is always true when the note has no associated diff line' do - note = build(:legacy_diff_note_on_merge_request) - - expect(note).to receive(:diff_line).and_return(nil) - - expect(note).to be_active - end - - it 'is never true when the note has no noteable associated' do - note = build(:legacy_diff_note_on_merge_request) - - expect(note).to receive(:diff_line).and_return(double) - expect(note).to receive(:noteable).and_return(nil) - - expect(note).not_to be_active - end - - it 'returns the memoized value if defined' do - note = build(:legacy_diff_note_on_merge_request) - - note.instance_variable_set(:@active, 'foo') - expect(note).not_to receive(:find_noteable_diff) - - expect(note.active?).to eq 'foo' - end - - context 'for a merge request noteable' do - it 'is false when noteable has no matching diff' do - merge = build_stubbed(:merge_request, :simple) - note = build(:legacy_diff_note_on_merge_request, noteable: merge) - - allow(note).to receive(:diff_line).and_return(double) - expect(note).to receive(:find_noteable_diff).and_return(nil) - - expect(note).not_to be_active - end - - it 'is true when noteable has a matching diff' do - merge = create(:merge_request, :simple) - - # Generate a real line_code value so we know it will match. We use a - # random line from a random diff just for funsies. - diff = merge.raw_diffs.to_a.sample - line = Gitlab::Diff::Parser.new.parse(diff.diff.each_line).to_a.sample - code = Gitlab::Diff::LineCode.generate(diff.new_path, line.new_pos, line.old_pos) - - # We're persisting in order to trigger the set_diff callback - note = create(:legacy_diff_note_on_merge_request, noteable: merge, - line_code: code, - project: merge.source_project) - - # Make sure we don't get a false positive from a guard clause - expect(note).to receive(:find_noteable_diff).and_call_original - expect(note).to be_active - end - end - end - - describe "#discussion_id" do - let(:note) { create(:note) } - - context "when it is newly created" do - it "has a discussion id" do - expect(note.discussion_id).not_to be_nil - expect(note.discussion_id).to match(/\A\h{40}\z/) - end - end - - context "when it didn't store a discussion id before" do - before do - note.update_column(:discussion_id, nil) - end - - it "has a discussion id" do - # The discussion_id is set in `after_initialize`, so `reload` won't work - reloaded_note = Note.find(note.id) - - expect(reloaded_note.discussion_id).not_to be_nil - expect(reloaded_note.discussion_id).to match(/\A\h{40}\z/) - end - end - end -end diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index 370aeb9e0a9..024380b7ebb 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -61,7 +61,7 @@ describe GroupMember, models: true do describe '#after_accept_request' do it 'calls NotificationService.accept_group_access_request' do - member = create(:group_member, user: build_stubbed(:user), requested_at: Time.now) + member = create(:group_member, user: build(:user), requested_at: Time.now) expect_any_instance_of(NotificationService).to receive(:new_group_member) @@ -75,4 +75,19 @@ describe GroupMember, models: true do it { is_expected.to eq 'Group' } end end + + describe '#update_two_factor_requirement' do + let(:user) { build :user } + let(:group_member) { build :group_member, user: user } + + it 'is called after creation and deletion' do + expect(user).to receive(:update_two_factor_requirement) + + group_member.save + + expect(user).to receive(:update_two_factor_requirement) + + group_member.destroy + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 24e7c1b17d9..90b3a2ba42d 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -441,7 +441,7 @@ describe MergeRequest, models: true do end it "can't be removed when its a protected branch" do - allow(subject.source_project).to receive(:protected_branch?).and_return(true) + allow(ProtectedBranch).to receive(:protected?).and_return(true) expect(subject.can_remove_source_branch?(user)).to be_falsey end @@ -1224,182 +1224,6 @@ describe MergeRequest, models: true do end end - context "discussion status" do - let(:first_discussion) { Discussion.new([create(:diff_note_on_merge_request)]) } - let(:second_discussion) { Discussion.new([create(:diff_note_on_merge_request)]) } - let(:third_discussion) { Discussion.new([create(:diff_note_on_merge_request)]) } - - before do - allow(subject).to receive(:diff_discussions).and_return([first_discussion, second_discussion, third_discussion]) - end - - describe '#resolvable_discussions' do - before do - allow(first_discussion).to receive(:to_be_resolved?).and_return(true) - allow(second_discussion).to receive(:to_be_resolved?).and_return(false) - allow(third_discussion).to receive(:to_be_resolved?).and_return(false) - end - - it 'includes only discussions that need to be resolved' do - expect(subject.resolvable_discussions).to eq([first_discussion]) - end - end - - describe '#discussions_can_be_resolved_by? user' do - let(:user) { build(:user) } - - context 'all discussions can be resolved by the user' do - before do - allow(first_discussion).to receive(:can_resolve?).with(user).and_return(true) - allow(second_discussion).to receive(:can_resolve?).with(user).and_return(true) - allow(third_discussion).to receive(:can_resolve?).with(user).and_return(true) - end - - it 'allows a user to resolve the discussions' do - expect(subject.discussions_can_be_resolved_by?(user)).to be(true) - end - end - - context 'one discussion cannot be resolved by the user' do - before do - allow(first_discussion).to receive(:can_resolve?).with(user).and_return(true) - allow(second_discussion).to receive(:can_resolve?).with(user).and_return(true) - allow(third_discussion).to receive(:can_resolve?).with(user).and_return(false) - end - - it 'allows a user to resolve the discussions' do - expect(subject.discussions_can_be_resolved_by?(user)).to be(false) - end - end - end - - describe "#discussions_resolvable?" do - context "when all discussions are unresolvable" do - before do - allow(first_discussion).to receive(:resolvable?).and_return(false) - allow(second_discussion).to receive(:resolvable?).and_return(false) - allow(third_discussion).to receive(:resolvable?).and_return(false) - end - - it "returns false" do - expect(subject.discussions_resolvable?).to be false - end - end - - context "when some discussions are unresolvable and some discussions are resolvable" do - before do - allow(first_discussion).to receive(:resolvable?).and_return(true) - allow(second_discussion).to receive(:resolvable?).and_return(false) - allow(third_discussion).to receive(:resolvable?).and_return(true) - end - - it "returns true" do - expect(subject.discussions_resolvable?).to be true - end - end - - context "when all discussions are resolvable" do - before do - allow(first_discussion).to receive(:resolvable?).and_return(true) - allow(second_discussion).to receive(:resolvable?).and_return(true) - allow(third_discussion).to receive(:resolvable?).and_return(true) - end - - it "returns true" do - expect(subject.discussions_resolvable?).to be true - end - end - end - - describe "#discussions_resolved?" do - context "when discussions are not resolvable" do - before do - allow(subject).to receive(:discussions_resolvable?).and_return(false) - end - - it "returns false" do - expect(subject.discussions_resolved?).to be false - end - end - - context "when discussions are resolvable" do - before do - allow(subject).to receive(:discussions_resolvable?).and_return(true) - - allow(first_discussion).to receive(:resolvable?).and_return(true) - allow(second_discussion).to receive(:resolvable?).and_return(false) - allow(third_discussion).to receive(:resolvable?).and_return(true) - end - - context "when all resolvable discussions are resolved" do - before do - allow(first_discussion).to receive(:resolved?).and_return(true) - allow(third_discussion).to receive(:resolved?).and_return(true) - end - - it "returns true" do - expect(subject.discussions_resolved?).to be true - end - end - - context "when some resolvable discussions are not resolved" do - before do - allow(first_discussion).to receive(:resolved?).and_return(true) - allow(third_discussion).to receive(:resolved?).and_return(false) - end - - it "returns false" do - expect(subject.discussions_resolved?).to be false - end - end - end - end - - describe "#discussions_to_be_resolved?" do - context "when discussions are not resolvable" do - before do - allow(subject).to receive(:discussions_resolvable?).and_return(false) - end - - it "returns false" do - expect(subject.discussions_to_be_resolved?).to be false - end - end - - context "when discussions are resolvable" do - before do - allow(subject).to receive(:discussions_resolvable?).and_return(true) - - allow(first_discussion).to receive(:resolvable?).and_return(true) - allow(second_discussion).to receive(:resolvable?).and_return(false) - allow(third_discussion).to receive(:resolvable?).and_return(true) - end - - context "when all resolvable discussions are resolved" do - before do - allow(first_discussion).to receive(:resolved?).and_return(true) - allow(third_discussion).to receive(:resolved?).and_return(true) - end - - it "returns false" do - expect(subject.discussions_to_be_resolved?).to be false - end - end - - context "when some resolvable discussions are not resolved" do - before do - allow(first_discussion).to receive(:resolved?).and_return(true) - allow(third_discussion).to receive(:resolved?).and_return(false) - end - - it "returns true" do - expect(subject.discussions_to_be_resolved?).to be true - end - end - end - end - end - describe '#conflicts_can_be_resolved_in_ui?' do def create_merge_request(source_branch) create(:merge_request, source_branch: source_branch, target_branch: 'conflict-start') do |mr| diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index f3f48f951a8..e3e8e6d571c 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -109,18 +109,6 @@ describe Milestone, models: true do it { expect(milestone.percent_complete(user)).to eq(75) } end - describe '#is_empty?' do - before do - milestone.issues << create(:issue, project: project) - milestone.issues << create(:closed_issue, project: project) - milestone.merge_requests << create(:merge_request) - end - - it { expect(milestone.closed_items_count(user)).to eq(1) } - it { expect(milestone.total_items_count(user)).to eq(3) } - it { expect(milestone.is_empty?(user)).to be_falsey } - end - describe '#can_be_closed?' do it { expect(milestone.can_be_closed?).to be_truthy } end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index d9216112259..e406d0a16bd 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -148,18 +148,22 @@ describe Namespace, models: true do expect(@namespace.move_dir).to be_truthy end - context "when any project has container tags" do + context "when any project has container images" do + let(:container_repository) { create(:container_repository) } + before do stub_container_registry_config(enabled: true) - stub_container_registry_tags('tag') + stub_container_registry_tags(repository: :any, tags: ['tag']) - create(:empty_project, namespace: @namespace) + create(:empty_project, namespace: @namespace, container_repositories: [container_repository]) allow(@namespace).to receive(:path_was).and_return(@namespace.path) allow(@namespace).to receive(:path).and_return('new_path') end - it { expect { @namespace.move_dir }.to raise_error('Namespace cannot be moved, because at least one project has tags in container registry') } + it 'raises an error about not movable project' do + expect { @namespace.move_dir }.to raise_error(/Namespace cannot be moved/) + end end context 'with subgroups' do diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 33536487c41..557ea97b008 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -245,14 +245,28 @@ describe Note, models: true do end end + describe '.find_discussion' do + let!(:note) { create(:discussion_note_on_merge_request) } + let!(:note2) { create(:discussion_note_on_merge_request, in_reply_to: note) } + let(:merge_request) { note.noteable } + + it 'returns a discussion with multiple notes' do + discussion = merge_request.notes.find_discussion(note.discussion_id) + + expect(discussion).not_to be_nil + expect(discussion.notes).to match_array([note, note2]) + expect(discussion.first_note.discussion_id).to eq(note.discussion_id) + end + end + describe ".grouped_diff_discussions" do let!(:merge_request) { create(:merge_request) } let(:project) { merge_request.project } let!(:active_diff_note1) { create(:diff_note_on_merge_request, project: project, noteable: merge_request) } - let!(:active_diff_note2) { create(:diff_note_on_merge_request, project: project, noteable: merge_request) } + let!(:active_diff_note2) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, in_reply_to: active_diff_note1) } let!(:active_diff_note3) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: active_position2) } let!(:outdated_diff_note1) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: outdated_position) } - let!(:outdated_diff_note2) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: outdated_position) } + let!(:outdated_diff_note2) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, in_reply_to: outdated_diff_note1) } let(:active_position2) do Gitlab::Diff::Position.new( @@ -277,7 +291,7 @@ describe Note, models: true do subject { merge_request.notes.grouped_diff_discussions } it "includes active discussions" do - discussions = subject.values + discussions = subject.values.flatten expect(discussions.count).to eq(2) expect(discussions.map(&:id)).to eq([active_diff_note1.discussion_id, active_diff_note3.discussion_id]) @@ -288,37 +302,12 @@ describe Note, models: true do end it "doesn't include outdated discussions" do - expect(subject.values.map(&:id)).not_to include(outdated_diff_note1.discussion_id) + expect(subject.values.flatten.map(&:id)).not_to include(outdated_diff_note1.discussion_id) end it "groups the discussions by line code" do - expect(subject[active_diff_note1.line_code].id).to eq(active_diff_note1.discussion_id) - expect(subject[active_diff_note3.line_code].id).to eq(active_diff_note3.discussion_id) - end - end - - describe "#discussion_id" do - let(:note) { create(:note) } - - context "when it is newly created" do - it "has a discussion id" do - expect(note.discussion_id).not_to be_nil - expect(note.discussion_id).to match(/\A\h{40}\z/) - end - end - - context "when it didn't store a discussion id before" do - before do - note.update_column(:discussion_id, nil) - end - - it "has a discussion id" do - # The discussion_id is set in `after_initialize`, so `reload` won't work - reloaded_note = Note.find(note.id) - - expect(reloaded_note.discussion_id).not_to be_nil - expect(reloaded_note.discussion_id).to match(/\A\h{40}\z/) - end + expect(subject[active_diff_note1.line_code].first.id).to eq(active_diff_note1.discussion_id) + expect(subject[active_diff_note3.line_code].first.id).to eq(active_diff_note3.discussion_id) end end @@ -388,15 +377,267 @@ describe Note, models: true do end end + describe '#can_be_discussion_note?' do + context 'for a note on a merge request' do + it 'returns true' do + note = build(:note_on_merge_request) + + expect(note.can_be_discussion_note?).to be_truthy + end + end + + context 'for a note on an issue' do + it 'returns true' do + note = build(:note_on_issue) + + expect(note.can_be_discussion_note?).to be_truthy + end + end + + context 'for a note on a commit' do + it 'returns true' do + note = build(:note_on_commit) + + expect(note.can_be_discussion_note?).to be_truthy + end + end + + context 'for a note on a snippet' do + it 'returns true' do + note = build(:note_on_project_snippet) + + expect(note.can_be_discussion_note?).to be_truthy + end + end + + context 'for a diff note on merge request' do + it 'returns false' do + note = build(:diff_note_on_merge_request) + + expect(note.can_be_discussion_note?).to be_falsey + end + end + + context 'for a diff note on commit' do + it 'returns false' do + note = build(:diff_note_on_commit) + + expect(note.can_be_discussion_note?).to be_falsey + end + end + + context 'for a discussion note' do + it 'returns false' do + note = build(:discussion_note_on_merge_request) + + expect(note.can_be_discussion_note?).to be_falsey + end + end + end + + describe '#discussion_class' do + let(:note) { build(:note_on_commit) } + let(:merge_request) { create(:merge_request) } + + context 'when the note is displayed out of context' do + it 'returns OutOfContextDiscussion' do + expect(note.discussion_class(merge_request)).to be(OutOfContextDiscussion) + end + end + + context 'when the note is displayed in the original context' do + it 'returns IndividualNoteDiscussion' do + expect(note.discussion_class(note.noteable)).to be(IndividualNoteDiscussion) + end + end + end + + describe "#discussion_id" do + let(:note) { create(:note_on_commit) } + + context "when it is newly created" do + it "has a discussion id" do + expect(note.discussion_id).not_to be_nil + expect(note.discussion_id).to match(/\A\h{40}\z/) + end + end + + context "when it didn't store a discussion id before" do + before do + note.update_column(:discussion_id, nil) + end + + it "has a discussion id" do + # The discussion_id is set in `after_initialize`, so `reload` won't work + reloaded_note = Note.find(note.id) + + expect(reloaded_note.discussion_id).not_to be_nil + expect(reloaded_note.discussion_id).to match(/\A\h{40}\z/) + end + end + + context 'when the note is displayed out of context' do + let(:merge_request) { create(:merge_request) } + + it 'overrides the discussion id' do + expect(note.discussion_id(merge_request)).not_to eq(note.discussion_id) + end + end + end + + describe '#to_discussion' do + subject { create(:discussion_note_on_merge_request) } + let!(:note2) { create(:discussion_note_on_merge_request, project: subject.project, noteable: subject.noteable, in_reply_to: subject) } + + it "returns a discussion with just this note" do + discussion = subject.to_discussion + + expect(discussion.id).to eq(subject.discussion_id) + expect(discussion.notes).to eq([subject]) + end + end + + describe "#discussion" do + let!(:note1) { create(:discussion_note_on_merge_request) } + let!(:note2) { create(:diff_note_on_merge_request, project: note1.project, noteable: note1.noteable) } + + context 'when the note is part of a discussion' do + subject { create(:discussion_note_on_merge_request, project: note1.project, noteable: note1.noteable, in_reply_to: note1) } + + it "returns the discussion this note is in" do + discussion = subject.discussion + + expect(discussion.id).to eq(subject.discussion_id) + expect(discussion.notes).to eq([note1, subject]) + end + end + + context 'when the note is not part of a discussion' do + subject { create(:note) } + + it "returns a discussion with just this note" do + discussion = subject.discussion + + expect(discussion.id).to eq(subject.discussion_id) + expect(discussion.notes).to eq([subject]) + end + end + end + + describe "#part_of_discussion?" do + context 'for a regular note' do + let(:note) { build(:note) } + + it 'returns false' do + expect(note.part_of_discussion?).to be_falsey + end + end + + context 'for a diff note' do + let(:note) { build(:diff_note_on_commit) } + + it 'returns true' do + expect(note.part_of_discussion?).to be_truthy + end + end + + context 'for a discussion note' do + let(:note) { build(:discussion_note_on_merge_request) } + + it 'returns true' do + expect(note.part_of_discussion?).to be_truthy + end + end + end + + describe '#in_reply_to?' do + context 'for a note' do + context 'when part of a discussion' do + subject { create(:discussion_note_on_issue) } + let(:note) { create(:discussion_note_on_issue, in_reply_to: subject) } + + it 'checks if the note is in reply to the other discussion' do + expect(subject).to receive(:in_reply_to?).with(note).and_call_original + expect(subject).to receive(:in_reply_to?).with(note.noteable).and_call_original + expect(subject).to receive(:in_reply_to?).with(note.to_discussion).and_call_original + + subject.in_reply_to?(note) + end + end + + context 'when not part of a discussion' do + subject { create(:note) } + let(:note) { create(:note, in_reply_to: subject) } + + it 'checks if the note is in reply to the other noteable' do + expect(subject).to receive(:in_reply_to?).with(note).and_call_original + expect(subject).to receive(:in_reply_to?).with(note.noteable).and_call_original + + subject.in_reply_to?(note) + end + end + end + + context 'for a discussion' do + context 'when part of the same discussion' do + subject { create(:diff_note_on_merge_request) } + let(:note) { create(:diff_note_on_merge_request, in_reply_to: subject) } + + it 'returns true' do + expect(subject.in_reply_to?(note.to_discussion)).to be_truthy + end + end + + context 'when not part of the same discussion' do + subject { create(:diff_note_on_merge_request) } + let(:note) { create(:diff_note_on_merge_request) } + + it 'returns false' do + expect(subject.in_reply_to?(note.to_discussion)).to be_falsey + end + end + end + + context 'for a noteable' do + context 'when a comment on the same noteable' do + subject { create(:note) } + let(:note) { create(:note, in_reply_to: subject) } + + it 'returns true' do + expect(subject.in_reply_to?(note.noteable)).to be_truthy + end + end + + context 'when not a comment on the same noteable' do + subject { create(:note) } + let(:note) { create(:note) } + + it 'returns false' do + expect(subject.in_reply_to?(note.noteable)).to be_falsey + end + end + end + end + describe 'expiring ETag cache' do let(:note) { build(:note_on_issue) } - it "expires cache for note's issue when note is saved" do + def expect_expiration(note) expect_any_instance_of(Gitlab::EtagCaching::Store) .to receive(:touch) .with("/#{note.project.namespace.to_param}/#{note.project.to_param}/noteable/issue/#{note.noteable.id}/notes") + end + + it "expires cache for note's issue when note is saved" do + expect_expiration(note) note.save! end + + it "expires cache for note's issue when note is destroyed" do + expect_expiration(note) + + note.destroy! + end end end diff --git a/spec/models/project_services/chat_message/issue_message_spec.rb b/spec/models/project_services/chat_message/issue_message_spec.rb index 190ff4c535d..34e2d94b1ed 100644 --- a/spec/models/project_services/chat_message/issue_message_spec.rb +++ b/spec/models/project_services/chat_message/issue_message_spec.rb @@ -7,7 +7,8 @@ describe ChatMessage::IssueMessage, models: true do { user: { name: 'Test User', - username: 'test.user' + username: 'test.user', + avatar_url: 'http://someavatar.com' }, project_name: 'project_name', project_url: 'http://somewhere.com', @@ -25,43 +26,84 @@ describe ChatMessage::IssueMessage, models: true do } end - let(:color) { '#C95823' } + context 'without markdown' do + let(:color) { '#C95823' } - context '#initialize' do - before do - args[:object_attributes][:description] = nil + context '#initialize' do + before do + args[:object_attributes][:description] = nil + end + + it 'returns a non-null description' do + expect(subject.description).to eq('') + end end - it 'returns a non-null description' do - expect(subject.description).to eq('') + context 'open' do + it 'returns a message regarding opening of issues' do + expect(subject.pretext).to eq( + '[<http://somewhere.com|project_name>] Issue opened by test.user') + expect(subject.attachments).to eq([ + { + title: "#100 Issue title", + title_link: "http://url.com", + text: "issue description", + color: color, + } + ]) + end end - end - context 'open' do - it 'returns a message regarding opening of issues' do - expect(subject.pretext).to eq( - '[<http://somewhere.com|project_name>] Issue opened by test.user') - expect(subject.attachments).to eq([ - { - title: "#100 Issue title", - title_link: "http://url.com", - text: "issue description", - color: color, - } - ]) + context 'close' do + before do + args[:object_attributes][:action] = 'close' + args[:object_attributes][:state] = 'closed' + end + + it 'returns a message regarding closing of issues' do + expect(subject.pretext). to eq( + '[<http://somewhere.com|project_name>] Issue <http://url.com|#100 Issue title> closed by test.user') + expect(subject.attachments).to be_empty + end end end - context 'close' do + context 'with markdown' do before do - args[:object_attributes][:action] = 'close' - args[:object_attributes][:state] = 'closed' + args[:markdown] = true end - it 'returns a message regarding closing of issues' do - expect(subject.pretext). to eq( - '[<http://somewhere.com|project_name>] Issue <http://url.com|#100 Issue title> closed by test.user') - expect(subject.attachments).to be_empty + context 'open' do + it 'returns a message regarding opening of issues' do + expect(subject.pretext).to eq( + '[[project_name](http://somewhere.com)] Issue opened by test.user') + expect(subject.attachments).to eq('issue description') + expect(subject.activity).to eq({ + title: 'Issue opened by test.user', + subtitle: 'in [project_name](http://somewhere.com)', + text: '[#100 Issue title](http://url.com)', + image: 'http://someavatar.com' + }) + end + end + + context 'close' do + before do + args[:object_attributes][:action] = 'close' + args[:object_attributes][:state] = 'closed' + end + + it 'returns a message regarding closing of issues' do + expect(subject.pretext). to eq( + '[[project_name](http://somewhere.com)] Issue [#100 Issue title](http://url.com) closed by test.user') + expect(subject.attachments).to be_empty + expect(subject.activity).to eq({ + title: 'Issue closed by test.user', + subtitle: 'in [project_name](http://somewhere.com)', + text: '[#100 Issue title](http://url.com)', + image: 'http://someavatar.com' + }) + end end end end diff --git a/spec/models/project_services/chat_message/merge_message_spec.rb b/spec/models/project_services/chat_message/merge_message_spec.rb index cc154112e90..fa0a1f4a5b7 100644 --- a/spec/models/project_services/chat_message/merge_message_spec.rb +++ b/spec/models/project_services/chat_message/merge_message_spec.rb @@ -7,45 +7,84 @@ describe ChatMessage::MergeMessage, models: true do { user: { name: 'Test User', - username: 'test.user' + username: 'test.user', + avatar_url: 'http://someavatar.com' }, project_name: 'project_name', project_url: 'http://somewhere.com', object_attributes: { - title: "Issue title\nSecond line", + title: "Merge Request title\nSecond line", id: 10, iid: 100, assignee_id: 1, url: 'http://url.com', state: 'opened', - description: 'issue description', + description: 'merge request description', source_branch: 'source_branch', target_branch: 'target_branch', } } end - let(:color) { '#345' } + context 'without markdown' do + let(:color) { '#345' } - context 'open' do - it 'returns a message regarding opening of merge requests' do - expect(subject.pretext).to eq( - 'test.user opened <http://somewhere.com/merge_requests/100|merge request !100> '\ - 'in <http://somewhere.com|project_name>: *Issue title*') - expect(subject.attachments).to be_empty + context 'open' do + it 'returns a message regarding opening of merge requests' do + expect(subject.pretext).to eq( + 'test.user opened <http://somewhere.com/merge_requests/100|!100 *Merge Request title*> in <http://somewhere.com|project_name>: *Merge Request title*') + expect(subject.attachments).to be_empty + end + end + + context 'close' do + before do + args[:object_attributes][:state] = 'closed' + end + it 'returns a message regarding closing of merge requests' do + expect(subject.pretext).to eq( + 'test.user closed <http://somewhere.com/merge_requests/100|!100 *Merge Request title*> in <http://somewhere.com|project_name>: *Merge Request title*') + expect(subject.attachments).to be_empty + end end end - context 'close' do + context 'with markdown' do before do - args[:object_attributes][:state] = 'closed' + args[:markdown] = true + end + + context 'open' do + it 'returns a message regarding opening of merge requests' do + expect(subject.pretext).to eq( + 'test.user opened [!100 *Merge Request title*](http://somewhere.com/merge_requests/100) in [project_name](http://somewhere.com): *Merge Request title*') + expect(subject.attachments).to be_empty + expect(subject.activity).to eq({ + title: 'Merge Request opened by test.user', + subtitle: 'in [project_name](http://somewhere.com)', + text: '[!100 *Merge Request title*](http://somewhere.com/merge_requests/100)', + image: 'http://someavatar.com' + }) + end end - it 'returns a message regarding closing of merge requests' do - expect(subject.pretext).to eq( - 'test.user closed <http://somewhere.com/merge_requests/100|merge request !100> '\ - 'in <http://somewhere.com|project_name>: *Issue title*') - expect(subject.attachments).to be_empty + + context 'close' do + before do + args[:object_attributes][:state] = 'closed' + end + + it 'returns a message regarding closing of merge requests' do + expect(subject.pretext).to eq( + 'test.user closed [!100 *Merge Request title*](http://somewhere.com/merge_requests/100) in [project_name](http://somewhere.com): *Merge Request title*') + expect(subject.attachments).to be_empty + expect(subject.activity).to eq({ + title: 'Merge Request closed by test.user', + subtitle: 'in [project_name](http://somewhere.com)', + text: '[!100 *Merge Request title*](http://somewhere.com/merge_requests/100)', + image: 'http://someavatar.com' + }) + end end end end diff --git a/spec/models/project_services/chat_message/note_message_spec.rb b/spec/models/project_services/chat_message/note_message_spec.rb index da700a08e57..7cd9c61ee2b 100644 --- a/spec/models/project_services/chat_message/note_message_spec.rb +++ b/spec/models/project_services/chat_message/note_message_spec.rb @@ -1,130 +1,190 @@ require 'spec_helper' describe ChatMessage::NoteMessage, models: true do - let(:color) { '#345' } + subject { described_class.new(args) } - before do - @args = { - user: { - name: 'Test User', - username: 'test.user', - avatar_url: 'http://fakeavatar' - }, - project_name: 'project_name', - project_url: 'http://somewhere.com', - repository: { - name: 'project_name', - url: 'http://somewhere.com', - }, - object_attributes: { - id: 10, - note: 'comment on a commit', - url: 'http://url.com', - noteable_type: 'Commit' - } + let(:color) { '#345' } + let(:args) do + { + user: { + name: 'Test User', + username: 'test.user', + avatar_url: 'http://fakeavatar' + }, + project_name: 'project_name', + project_url: 'http://somewhere.com', + repository: { + name: 'project_name', + url: 'http://somewhere.com', + }, + object_attributes: { + id: 10, + note: 'comment on a commit', + url: 'http://url.com', + noteable_type: 'Commit' + } } end context 'commit notes' do before do - @args[:object_attributes][:note] = 'comment on a commit' - @args[:object_attributes][:noteable_type] = 'Commit' - @args[:commit] = { - id: '5f163b2b95e6f53cbd428f5f0b103702a52b9a23', - message: "Added a commit message\ndetails\n123\n" + args[:object_attributes][:note] = 'comment on a commit' + args[:object_attributes][:noteable_type] = 'Commit' + args[:commit] = { + id: '5f163b2b95e6f53cbd428f5f0b103702a52b9a23', + message: "Added a commit message\ndetails\n123\n" } end - it 'returns a message regarding notes on commits' do - message = described_class.new(@args) - expect(message.pretext).to eq("test.user <http://url.com|commented on " \ - "commit 5f163b2b> in <http://somewhere.com|project_name>: " \ - "*Added a commit message*") - expected_attachments = [ - { - text: "comment on a commit", - color: color, - } - ] - expect(message.attachments).to eq(expected_attachments) + context 'without markdown' do + it 'returns a message regarding notes on commits' do + expect(subject.pretext).to eq("test.user <http://url.com|commented on " \ + "commit 5f163b2b> in <http://somewhere.com|project_name>: " \ + "*Added a commit message*") + expect(subject.attachments).to eq([{ + text: 'comment on a commit', + color: color + }]) + end + end + + context 'with markdown' do + before do + args[:markdown] = true + end + + it 'returns a message regarding notes on commits' do + expect(subject.pretext).to eq( + 'test.user [commented on commit 5f163b2b](http://url.com) in [project_name](http://somewhere.com): *Added a commit message*' + ) + expect(subject.attachments).to eq('comment on a commit') + expect(subject.activity).to eq({ + title: 'test.user [commented on commit 5f163b2b](http://url.com)', + subtitle: 'in [project_name](http://somewhere.com)', + text: 'Added a commit message', + image: 'http://fakeavatar' + }) + end end end context 'merge request notes' do before do - @args[:object_attributes][:note] = 'comment on a merge request' - @args[:object_attributes][:noteable_type] = 'MergeRequest' - @args[:merge_request] = { - id: 1, - iid: 30, - title: "merge request title\ndetails\n" + args[:object_attributes][:note] = 'comment on a merge request' + args[:object_attributes][:noteable_type] = 'MergeRequest' + args[:merge_request] = { + id: 1, + iid: 30, + title: "merge request title\ndetails\n" } end - it 'returns a message regarding notes on a merge request' do - message = described_class.new(@args) - expect(message.pretext).to eq("test.user <http://url.com|commented on " \ - "merge request !30> in <http://somewhere.com|project_name>: " \ - "*merge request title*") - expected_attachments = [ - { - text: "comment on a merge request", - color: color, - } - ] - expect(message.attachments).to eq(expected_attachments) + context 'without markdown' do + it 'returns a message regarding notes on a merge request' do + expect(subject.pretext).to eq("test.user <http://url.com|commented on " \ + "merge request !30> in <http://somewhere.com|project_name>: " \ + "*merge request title*") + expect(subject.attachments).to eq([{ + text: 'comment on a merge request', + color: color + }]) + end + end + + context 'with markdown' do + before do + args[:markdown] = true + end + + it 'returns a message regarding notes on a merge request' do + expect(subject.pretext).to eq( + 'test.user [commented on merge request !30](http://url.com) in [project_name](http://somewhere.com): *merge request title*') + expect(subject.attachments).to eq('comment on a merge request') + expect(subject.activity).to eq({ + title: 'test.user [commented on merge request !30](http://url.com)', + subtitle: 'in [project_name](http://somewhere.com)', + text: 'merge request title', + image: 'http://fakeavatar' + }) + end end end context 'issue notes' do before do - @args[:object_attributes][:note] = 'comment on an issue' - @args[:object_attributes][:noteable_type] = 'Issue' - @args[:issue] = { - id: 1, - iid: 20, - title: "issue title\ndetails\n" + args[:object_attributes][:note] = 'comment on an issue' + args[:object_attributes][:noteable_type] = 'Issue' + args[:issue] = { + id: 1, + iid: 20, + title: "issue title\ndetails\n" } end - it 'returns a message regarding notes on an issue' do - message = described_class.new(@args) - expect(message.pretext).to eq( - "test.user <http://url.com|commented on " \ - "issue #20> in <http://somewhere.com|project_name>: " \ - "*issue title*") - expected_attachments = [ - { - text: "comment on an issue", - color: color, - } - ] - expect(message.attachments).to eq(expected_attachments) + context 'without markdown' do + it 'returns a message regarding notes on an issue' do + expect(subject.pretext).to eq( + "test.user <http://url.com|commented on " \ + "issue #20> in <http://somewhere.com|project_name>: " \ + "*issue title*") + expect(subject.attachments).to eq([{ + text: 'comment on an issue', + color: color + }]) + end + end + + context 'with markdown' do + before do + args[:markdown] = true + end + + it 'returns a message regarding notes on an issue' do + expect(subject.pretext).to eq( + 'test.user [commented on issue #20](http://url.com) in [project_name](http://somewhere.com): *issue title*') + expect(subject.attachments).to eq('comment on an issue') + expect(subject.activity).to eq({ + title: 'test.user [commented on issue #20](http://url.com)', + subtitle: 'in [project_name](http://somewhere.com)', + text: 'issue title', + image: 'http://fakeavatar' + }) + end end end context 'project snippet notes' do before do - @args[:object_attributes][:note] = 'comment on a snippet' - @args[:object_attributes][:noteable_type] = 'Snippet' - @args[:snippet] = { - id: 5, - title: "snippet title\ndetails\n" + args[:object_attributes][:note] = 'comment on a snippet' + args[:object_attributes][:noteable_type] = 'Snippet' + args[:snippet] = { + id: 5, + title: "snippet title\ndetails\n" } end - it 'returns a message regarding notes on a project snippet' do - message = described_class.new(@args) - expect(message.pretext).to eq("test.user <http://url.com|commented on " \ - "snippet #5> in <http://somewhere.com|project_name>: " \ - "*snippet title*") - expected_attachments = [ - { - text: "comment on a snippet", - color: color, - } - ] - expect(message.attachments).to eq(expected_attachments) + context 'without markdown' do + it 'returns a message regarding notes on a project snippet' do + expect(subject.pretext).to eq("test.user <http://url.com|commented on " \ + "snippet $5> in <http://somewhere.com|project_name>: " \ + "*snippet title*") + expect(subject.attachments).to eq([{ + text: 'comment on a snippet', + color: color + }]) + end + end + + context 'with markdown' do + before do + args[:markdown] = true + end + + it 'returns a message regarding notes on a project snippet' do + expect(subject.pretext).to eq( + 'test.user [commented on snippet $5](http://url.com) in [project_name](http://somewhere.com): *snippet title*') + expect(subject.attachments).to eq('comment on a snippet') + end end end end diff --git a/spec/models/project_services/chat_message/pipeline_message_spec.rb b/spec/models/project_services/chat_message/pipeline_message_spec.rb index bf2a9616455..ec5c6c5e0ed 100644 --- a/spec/models/project_services/chat_message/pipeline_message_spec.rb +++ b/spec/models/project_services/chat_message/pipeline_message_spec.rb @@ -2,8 +2,8 @@ require 'spec_helper' describe ChatMessage::PipelineMessage do subject { described_class.new(args) } - let(:user) { { name: 'hacker' } } + let(:user) { { name: 'hacker' } } let(:args) do { object_attributes: { @@ -14,54 +14,122 @@ describe ChatMessage::PipelineMessage do status: status, duration: duration }, - project: { path_with_namespace: 'project_name', - web_url: 'http://example.gitlab.com' }, + project: { + path_with_namespace: 'project_name', + web_url: 'http://example.gitlab.com' + }, user: user } end - let(:message) { build_message } + context 'without markdown' do + context 'pipeline succeeded' do + let(:status) { 'success' } + let(:color) { 'good' } + let(:duration) { 10 } + let(:message) { build_message('passed') } + + it 'returns a message with information about succeeded build' do + expect(subject.pretext).to be_empty + expect(subject.fallback).to eq(message) + expect(subject.attachments).to eq([text: message, color: color]) + end + end - context 'pipeline succeeded' do - let(:status) { 'success' } - let(:color) { 'good' } - let(:duration) { 10 } - let(:message) { build_message('passed') } + context 'pipeline failed' do + let(:status) { 'failed' } + let(:color) { 'danger' } + let(:duration) { 10 } + let(:message) { build_message } - it 'returns a message with information about succeeded build' do - verify_message + it 'returns a message with information about failed build' do + expect(subject.pretext).to be_empty + expect(subject.fallback).to eq(message) + expect(subject.attachments).to eq([text: message, color: color]) + end + + context 'when triggered by API therefore lacking user' do + let(:user) { nil } + let(:message) { build_message(status, 'API') } + + it 'returns a message stating it is by API' do + expect(subject.pretext).to be_empty + expect(subject.fallback).to eq(message) + expect(subject.attachments).to eq([text: message, color: color]) + end + end end - end - context 'pipeline failed' do - let(:status) { 'failed' } - let(:color) { 'danger' } - let(:duration) { 10 } + def build_message(status_text = status, name = user[:name]) + "<http://example.gitlab.com|project_name>:" \ + " Pipeline <http://example.gitlab.com/pipelines/123|#123>" \ + " of <http://example.gitlab.com/commits/develop|develop> branch" \ + " by #{name} #{status_text} in #{duration} #{'second'.pluralize(duration)}" + end + end - it 'returns a message with information about failed build' do - verify_message + context 'with markdown' do + before do + args[:markdown] = true end - context 'when triggered by API therefore lacking user' do - let(:user) { nil } - let(:message) { build_message(status, 'API') } + context 'pipeline succeeded' do + let(:status) { 'success' } + let(:color) { 'good' } + let(:duration) { 10 } + let(:message) { build_markdown_message('passed') } - it 'returns a message stating it is by API' do - verify_message + it 'returns a message with information about succeeded build' do + expect(subject.pretext).to be_empty + expect(subject.attachments).to eq(message) + expect(subject.activity).to eq({ + title: 'Pipeline [#123](http://example.gitlab.com/pipelines/123) of [develop](http://example.gitlab.com/commits/develop) branch by hacker passed', + subtitle: 'in [project_name](http://example.gitlab.com)', + text: 'in 10 seconds', + image: '' + }) end end - end - def verify_message - expect(subject.pretext).to be_empty - expect(subject.fallback).to eq(message) - expect(subject.attachments).to eq([text: message, color: color]) - end + context 'pipeline failed' do + let(:status) { 'failed' } + let(:color) { 'danger' } + let(:duration) { 10 } + let(:message) { build_markdown_message } + + it 'returns a message with information about failed build' do + expect(subject.pretext).to be_empty + expect(subject.attachments).to eq(message) + expect(subject.activity).to eq({ + title: 'Pipeline [#123](http://example.gitlab.com/pipelines/123) of [develop](http://example.gitlab.com/commits/develop) branch by hacker failed', + subtitle: 'in [project_name](http://example.gitlab.com)', + text: 'in 10 seconds', + image: '' + }) + end - def build_message(status_text = status, name = user[:name]) - "<http://example.gitlab.com|project_name>:" \ - " Pipeline <http://example.gitlab.com/pipelines/123|#123>" \ - " of <http://example.gitlab.com/commits/develop|develop> branch" \ - " by #{name} #{status_text} in #{duration} #{'second'.pluralize(duration)}" + context 'when triggered by API therefore lacking user' do + let(:user) { nil } + let(:message) { build_markdown_message(status, 'API') } + + it 'returns a message stating it is by API' do + expect(subject.pretext).to be_empty + expect(subject.attachments).to eq(message) + expect(subject.activity).to eq({ + title: 'Pipeline [#123](http://example.gitlab.com/pipelines/123) of [develop](http://example.gitlab.com/commits/develop) branch by API failed', + subtitle: 'in [project_name](http://example.gitlab.com)', + text: 'in 10 seconds', + image: '' + }) + end + end + end + + def build_markdown_message(status_text = status, name = user[:name]) + "[project_name](http://example.gitlab.com):" \ + " Pipeline [#123](http://example.gitlab.com/pipelines/123)" \ + " of [develop](http://example.gitlab.com/commits/develop)" \ + " branch by #{name} #{status_text} in #{duration} #{'second'.pluralize(duration)}" + end end end diff --git a/spec/models/project_services/chat_message/push_message_spec.rb b/spec/models/project_services/chat_message/push_message_spec.rb index 24928873bad..63eb078c44e 100644 --- a/spec/models/project_services/chat_message/push_message_spec.rb +++ b/spec/models/project_services/chat_message/push_message_spec.rb @@ -10,6 +10,7 @@ describe ChatMessage::PushMessage, models: true do project_name: 'project_name', ref: 'refs/heads/master', user_name: 'test.user', + user_avatar: 'http://someavatar.com', project_url: 'http://url.com' } end @@ -24,18 +25,36 @@ describe ChatMessage::PushMessage, models: true do ] end - it 'returns a message regarding pushes' do - expect(subject.pretext).to eq( - 'test.user pushed to branch <http://url.com/commits/master|master> of '\ - '<http://url.com|project_name> (<http://url.com/compare/before...after|Compare changes>)' - ) - expect(subject.attachments).to eq([ - { - text: "<http://url1.com|abcdefgh>: message1 - author1\n"\ - "<http://url2.com|12345678>: message2 - author2", + context 'without markdown' do + it 'returns a message regarding pushes' do + expect(subject.pretext).to eq( + 'test.user pushed to branch <http://url.com/commits/master|master> of '\ + '<http://url.com|project_name> (<http://url.com/compare/before...after|Compare changes>)') + expect(subject.attachments).to eq([{ + text: "<http://url1.com|abcdefgh>: message1 - author1\n\n"\ + "<http://url2.com|12345678>: message2 - author2", color: color, - } - ]) + }]) + end + end + + context 'with markdown' do + before do + args[:markdown] = true + end + + it 'returns a message regarding pushes' do + expect(subject.pretext).to eq( + 'test.user pushed to branch [master](http://url.com/commits/master) of [project_name](http://url.com) ([Compare changes](http://url.com/compare/before...after))') + expect(subject.attachments).to eq( + "[abcdefgh](http://url1.com): message1 - author1\n\n[12345678](http://url2.com): message2 - author2") + expect(subject.activity).to eq({ + title: 'test.user pushed to branch', + subtitle: 'in [project_name](http://url.com)', + text: '[Compare changes](http://url.com/compare/before...after)', + image: 'http://someavatar.com' + }) + end end end @@ -47,15 +66,36 @@ describe ChatMessage::PushMessage, models: true do project_name: 'project_name', ref: 'refs/tags/new_tag', user_name: 'test.user', + user_avatar: 'http://someavatar.com', project_url: 'http://url.com' } end - it 'returns a message regarding pushes' do - expect(subject.pretext).to eq('test.user pushed new tag ' \ - '<http://url.com/commits/new_tag|new_tag> to ' \ - '<http://url.com|project_name>') - expect(subject.attachments).to be_empty + context 'without markdown' do + it 'returns a message regarding pushes' do + expect(subject.pretext).to eq('test.user pushed new tag ' \ + '<http://url.com/commits/new_tag|new_tag> to ' \ + '<http://url.com|project_name>') + expect(subject.attachments).to be_empty + end + end + + context 'with markdown' do + before do + args[:markdown] = true + end + + it 'returns a message regarding pushes' do + expect(subject.pretext).to eq( + 'test.user pushed new tag [new_tag](http://url.com/commits/new_tag) to [project_name](http://url.com)') + expect(subject.attachments).to be_empty + expect(subject.activity).to eq({ + title: 'test.user created tag', + subtitle: 'in [project_name](http://url.com)', + text: '[Compare changes](http://url.com/compare/0000000000000000000000000000000000000000...after)', + image: 'http://someavatar.com' + }) + end end end @@ -64,12 +104,31 @@ describe ChatMessage::PushMessage, models: true do args[:before] = Gitlab::Git::BLANK_SHA end - it 'returns a message regarding a new branch' do - expect(subject.pretext).to eq( - 'test.user pushed new branch <http://url.com/commits/master|master> to '\ - '<http://url.com|project_name>' - ) - expect(subject.attachments).to be_empty + context 'without markdown' do + it 'returns a message regarding a new branch' do + expect(subject.pretext).to eq( + 'test.user pushed new branch <http://url.com/commits/master|master> to '\ + '<http://url.com|project_name>') + expect(subject.attachments).to be_empty + end + end + + context 'with markdown' do + before do + args[:markdown] = true + end + + it 'returns a message regarding a new branch' do + expect(subject.pretext).to eq( + 'test.user pushed new branch [master](http://url.com/commits/master) to [project_name](http://url.com)') + expect(subject.attachments).to be_empty + expect(subject.activity).to eq({ + title: 'test.user created branch', + subtitle: 'in [project_name](http://url.com)', + text: '[Compare changes](http://url.com/compare/0000000000000000000000000000000000000000...after)', + image: 'http://someavatar.com' + }) + end end end @@ -78,11 +137,30 @@ describe ChatMessage::PushMessage, models: true do args[:after] = Gitlab::Git::BLANK_SHA end - it 'returns a message regarding a removed branch' do - expect(subject.pretext).to eq( - 'test.user removed branch master from <http://url.com|project_name>' - ) - expect(subject.attachments).to be_empty + context 'without markdown' do + it 'returns a message regarding a removed branch' do + expect(subject.pretext).to eq( + 'test.user removed branch master from <http://url.com|project_name>') + expect(subject.attachments).to be_empty + end + end + + context 'with markdown' do + before do + args[:markdown] = true + end + + it 'returns a message regarding a removed branch' do + expect(subject.pretext).to eq( + 'test.user removed branch master from [project_name](http://url.com)') + expect(subject.attachments).to be_empty + expect(subject.activity).to eq({ + title: 'test.user removed branch', + subtitle: 'in [project_name](http://url.com)', + text: '[Compare changes](http://url.com/compare/before...0000000000000000000000000000000000000000)', + image: 'http://someavatar.com' + }) + end end end end diff --git a/spec/models/project_services/chat_message/wiki_page_message_spec.rb b/spec/models/project_services/chat_message/wiki_page_message_spec.rb index a2ad61e38e7..0df7db2abc2 100644 --- a/spec/models/project_services/chat_message/wiki_page_message_spec.rb +++ b/spec/models/project_services/chat_message/wiki_page_message_spec.rb @@ -7,7 +7,8 @@ describe ChatMessage::WikiPageMessage, models: true do { user: { name: 'Test User', - username: 'test.user' + username: 'test.user', + avatar_url: 'http://someavatar.com' }, project_name: 'project_name', project_url: 'http://somewhere.com', @@ -19,54 +20,128 @@ describe ChatMessage::WikiPageMessage, models: true do } end - describe '#pretext' do - context 'when :action == "create"' do - before { args[:object_attributes][:action] = 'create' } + context 'without markdown' do + describe '#pretext' do + context 'when :action == "create"' do + before { args[:object_attributes][:action] = 'create' } - it 'returns a message that a new wiki page was created' do - expect(subject.pretext).to eq( - 'test.user created <http://url.com|wiki page> in <http://somewhere.com|project_name>: '\ - '*Wiki page title*') + it 'returns a message that a new wiki page was created' do + expect(subject.pretext).to eq( + 'test.user created <http://url.com|wiki page> in <http://somewhere.com|project_name>: '\ + '*Wiki page title*') + end + end + + context 'when :action == "update"' do + before { args[:object_attributes][:action] = 'update' } + + it 'returns a message that a wiki page was updated' do + expect(subject.pretext).to eq( + 'test.user edited <http://url.com|wiki page> in <http://somewhere.com|project_name>: '\ + '*Wiki page title*') + end end end - context 'when :action == "update"' do - before { args[:object_attributes][:action] = 'update' } + describe '#attachments' do + let(:color) { '#345' } - it 'returns a message that a wiki page was updated' do - expect(subject.pretext).to eq( - 'test.user edited <http://url.com|wiki page> in <http://somewhere.com|project_name>: '\ - '*Wiki page title*') + context 'when :action == "create"' do + before { args[:object_attributes][:action] = 'create' } + + it 'returns the attachment for a new wiki page' do + expect(subject.attachments).to eq([ + { + text: "Wiki page description", + color: color, + } + ]) + end + end + + context 'when :action == "update"' do + before { args[:object_attributes][:action] = 'update' } + + it 'returns the attachment for an updated wiki page' do + expect(subject.attachments).to eq([ + { + text: "Wiki page description", + color: color, + } + ]) + end end end end - describe '#attachments' do - let(:color) { '#345' } + context 'with markdown' do + before do + args[:markdown] = true + end + + describe '#pretext' do + context 'when :action == "create"' do + before { args[:object_attributes][:action] = 'create' } + + it 'returns a message that a new wiki page was created' do + expect(subject.pretext).to eq( + 'test.user created [wiki page](http://url.com) in [project_name](http://somewhere.com): *Wiki page title*') + end + end - context 'when :action == "create"' do - before { args[:object_attributes][:action] = 'create' } + context 'when :action == "update"' do + before { args[:object_attributes][:action] = 'update' } - it 'returns the attachment for a new wiki page' do - expect(subject.attachments).to eq([ - { - text: "Wiki page description", - color: color, - } - ]) + it 'returns a message that a wiki page was updated' do + expect(subject.pretext).to eq( + 'test.user edited [wiki page](http://url.com) in [project_name](http://somewhere.com): *Wiki page title*') + end end end - context 'when :action == "update"' do - before { args[:object_attributes][:action] = 'update' } + describe '#attachments' do + context 'when :action == "create"' do + before { args[:object_attributes][:action] = 'create' } + + it 'returns the attachment for a new wiki page' do + expect(subject.attachments).to eq('Wiki page description') + end + end + + context 'when :action == "update"' do + before { args[:object_attributes][:action] = 'update' } + + it 'returns the attachment for an updated wiki page' do + expect(subject.attachments).to eq('Wiki page description') + end + end + end + + describe '#activity' do + context 'when :action == "create"' do + before { args[:object_attributes][:action] = 'create' } + + it 'returns the attachment for a new wiki page' do + expect(subject.activity).to eq({ + title: 'test.user created [wiki page](http://url.com)', + subtitle: 'in [project_name](http://somewhere.com)', + text: 'Wiki page title', + image: 'http://someavatar.com' + }) + end + end + + context 'when :action == "update"' do + before { args[:object_attributes][:action] = 'update' } - it 'returns the attachment for an updated wiki page' do - expect(subject.attachments).to eq([ - { - text: "Wiki page description", - color: color, - } - ]) + it 'returns the attachment for an updated wiki page' do + expect(subject.activity).to eq({ + title: 'test.user edited [wiki page](http://url.com)', + subtitle: 'in [project_name](http://somewhere.com)', + text: 'Wiki page title', + image: 'http://someavatar.com' + }) + end end end end diff --git a/spec/models/project_services/microsoft_teams_service_spec.rb b/spec/models/project_services/microsoft_teams_service_spec.rb new file mode 100644 index 00000000000..facc034f69c --- /dev/null +++ b/spec/models/project_services/microsoft_teams_service_spec.rb @@ -0,0 +1,277 @@ +require 'spec_helper' + +describe MicrosoftTeamsService, models: true do + let(:chat_service) { described_class.new } + let(:webhook_url) { 'https://example.gitlab.com/' } + + describe "Associations" do + it { is_expected.to belong_to :project } + it { is_expected.to have_one :service_hook } + end + + describe 'Validations' do + context 'when service is active' do + before { subject.active = true } + + it { is_expected.to validate_presence_of(:webhook) } + it_behaves_like 'issue tracker service URL attribute', :webhook + end + + context 'when service is inactive' do + before { subject.active = false } + + it { is_expected.not_to validate_presence_of(:webhook) } + end + end + + describe "#execute" do + let(:user) { create(:user) } + let(:project) { create(:project, :repository) } + + before do + allow(chat_service).to receive_messages( + project: project, + project_id: project.id, + service_hook: true, + webhook: webhook_url + ) + + WebMock.stub_request(:post, webhook_url) + end + + context 'with push events' do + let(:push_sample_data) do + Gitlab::DataBuilder::Push.build_sample(project, user) + end + + it "calls Microsoft Teams API for push events" do + chat_service.execute(push_sample_data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + + it 'specifies the webhook when it is configured' do + expect(MicrosoftTeams::Notifier).to receive(:new).with(webhook_url).and_return(double(:microsoft_teams_service).as_null_object) + + chat_service.execute(push_sample_data) + end + end + + context 'with issue events' do + let(:opts) { { title: 'Awesome issue', description: 'please fix' } } + let(:issues_sample_data) do + service = Issues::CreateService.new(project, user, opts) + issue = service.execute + service.hook_data(issue, 'open') + end + + it "calls Microsoft Teams API" do + chat_service.execute(issues_sample_data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + + context 'with merge events' do + let(:opts) do + { + title: 'Awesome merge_request', + description: 'please fix', + source_branch: 'feature', + target_branch: 'master' + } + end + + let(:merge_sample_data) do + service = MergeRequests::CreateService.new(project, user, opts) + merge_request = service.execute + service.hook_data(merge_request, 'open') + end + + it "calls Microsoft Teams API" do + chat_service.execute(merge_sample_data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + + context 'with wiki page events' do + let(:opts) do + { + title: "Awesome wiki_page", + content: "Some text describing some thing or another", + format: "md", + message: "user created page: Awesome wiki_page" + } + end + + let(:wiki_page_sample_data) do + service = WikiPages::CreateService.new(project, user, opts) + wiki_page = service.execute + service.hook_data(wiki_page, 'create') + end + + it "calls Microsoft Teams API" do + chat_service.execute(wiki_page_sample_data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + end + + describe "Note events" do + let(:user) { create(:user) } + let(:project) { create(:project, :repository, creator: user) } + + before do + allow(chat_service).to receive_messages( + project: project, + project_id: project.id, + service_hook: true, + webhook: webhook_url + ) + + WebMock.stub_request(:post, webhook_url) + end + + context 'when commit comment event executed' do + let(:commit_note) do + create(:note_on_commit, author: user, + project: project, + commit_id: project.repository.commit.id, + note: 'a comment on a commit') + end + + it "calls Microsoft Teams API for commit comment events" do + data = Gitlab::DataBuilder::Note.build(commit_note, user) + + chat_service.execute(data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + + context 'when merge request comment event executed' do + let(:merge_request_note) do + create(:note_on_merge_request, project: project, + note: "merge request note") + end + + it "calls Microsoft Teams API for merge request comment events" do + data = Gitlab::DataBuilder::Note.build(merge_request_note, user) + + chat_service.execute(data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + + context 'when issue comment event executed' do + let(:issue_note) do + create(:note_on_issue, project: project, note: "issue note") + end + + it "calls Microsoft Teams API for issue comment events" do + data = Gitlab::DataBuilder::Note.build(issue_note, user) + + chat_service.execute(data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + + context 'when snippet comment event executed' do + let(:snippet_note) do + create(:note_on_project_snippet, project: project, + note: "snippet note") + end + + it "calls Microsoft Teams API for snippet comment events" do + data = Gitlab::DataBuilder::Note.build(snippet_note, user) + + chat_service.execute(data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + end + + describe 'Pipeline events' do + let(:user) { create(:user) } + let(:project) { create(:project, :repository) } + + let(:pipeline) do + create(:ci_pipeline, + project: project, status: status, + sha: project.commit.sha, ref: project.default_branch) + end + + before do + allow(chat_service).to receive_messages( + project: project, + service_hook: true, + webhook: webhook_url + ) + end + + shared_examples 'call Microsoft Teams API' do + before do + WebMock.stub_request(:post, webhook_url) + end + + it 'calls Microsoft Teams API for pipeline events' do + data = Gitlab::DataBuilder::Pipeline.build(pipeline) + + chat_service.execute(data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end + end + + context 'with failed pipeline' do + let(:status) { 'failed' } + + it_behaves_like 'call Microsoft Teams API' + end + + context 'with succeeded pipeline' do + let(:status) { 'success' } + + context 'with default to notify_only_broken_pipelines' do + it 'does not call Microsoft Teams API for pipeline events' do + data = Gitlab::DataBuilder::Pipeline.build(pipeline) + result = chat_service.execute(data) + + expect(result).to be_falsy + end + end + + context 'with setting notify_only_broken_pipelines to false' do + before do + chat_service.notify_only_broken_pipelines = false + end + + it_behaves_like 'call Microsoft Teams API' + end + end + + context 'only notify for the default branch' do + context 'when enabled' do + let(:pipeline) do + create(:ci_pipeline, project: project, status: 'failed', ref: 'not-the-default-branch') + end + + before do + chat_service.notify_only_default_branch = true + end + + it 'does not call the Microsoft Teams API for pipeline events' do + data = Gitlab::DataBuilder::Pipeline.build(pipeline) + result = chat_service.execute(data) + + expect(result).to be_falsy + end + end + end + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 59a2560ca06..92d420337f9 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -22,6 +22,7 @@ describe Project, models: true do it { is_expected.to have_many(:protected_branches).dependent(:destroy) } it { is_expected.to have_one(:forked_project_link).dependent(:destroy) } it { is_expected.to have_one(:slack_service).dependent(:destroy) } + it { is_expected.to have_one(:microsoft_teams_service).dependent(:destroy) } it { is_expected.to have_one(:mattermost_service).dependent(:destroy) } it { is_expected.to have_one(:pushover_service).dependent(:destroy) } it { is_expected.to have_one(:asana_service).dependent(:destroy) } @@ -57,6 +58,7 @@ describe Project, models: true do it { is_expected.to have_many(:builds) } it { is_expected.to have_many(:runner_projects) } it { is_expected.to have_many(:runners) } + it { is_expected.to have_many(:active_runners) } it { is_expected.to have_many(:variables) } it { is_expected.to have_many(:triggers) } it { is_expected.to have_many(:pages_domains) } @@ -702,25 +704,6 @@ describe Project, models: true do end end - describe '#open_branches' do - let(:project) { create(:project, :repository) } - - before do - project.protected_branches.create(name: 'master') - end - - it { expect(project.open_branches.map(&:name)).to include('feature') } - it { expect(project.open_branches.map(&:name)).not_to include('master') } - - it "includes branches matching a protected branch wildcard" do - expect(project.open_branches.map(&:name)).to include('feature') - - create(:protected_branch, name: 'feat*', project: project) - - expect(Project.find(project.id).open_branches.map(&:name)).to include('feature') - end - end - describe '#star_count' do it 'counts stars from multiple users' do user1 = create :user @@ -1157,11 +1140,12 @@ describe Project, models: true do # Project#gitlab_shell returns a new instance of Gitlab::Shell on every # call. This makes testing a bit easier. allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) - allow(project).to receive(:previous_changes).and_return('path' => ['foo']) end it 'renames a repository' do + stub_container_registry_config(enabled: false) + expect(gitlab_shell).to receive(:mv_repository). ordered. with(project.repository_storage_path, "#{project.namespace.full_path}/foo", "#{project.full_path}"). @@ -1185,10 +1169,13 @@ describe Project, models: true do project.rename_repo end - context 'container registry with tags' do + context 'container registry with images' do + let(:container_repository) { create(:container_repository) } + before do stub_container_registry_config(enabled: true) - stub_container_registry_tags('tag') + stub_container_registry_tags(repository: :any, tags: ['tag']) + project.container_repositories << container_repository end subject { project.rename_repo } @@ -1291,62 +1278,6 @@ describe Project, models: true do end end - describe '#protected_branch?' do - context 'existing project' do - let(:project) { create(:project, :repository) } - - it 'returns true when the branch matches a protected branch via direct match' do - create(:protected_branch, project: project, name: "foo") - - expect(project.protected_branch?('foo')).to eq(true) - end - - it 'returns true when the branch matches a protected branch via wildcard match' do - create(:protected_branch, project: project, name: "production/*") - - expect(project.protected_branch?('production/some-branch')).to eq(true) - end - - it 'returns false when the branch does not match a protected branch via direct match' do - expect(project.protected_branch?('foo')).to eq(false) - end - - it 'returns false when the branch does not match a protected branch via wildcard match' do - create(:protected_branch, project: project, name: "production/*") - - expect(project.protected_branch?('staging/some-branch')).to eq(false) - end - end - - context "new project" do - let(:project) { create(:empty_project) } - - it 'returns false when default_protected_branch is unprotected' do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) - - expect(project.protected_branch?('master')).to be false - end - - it 'returns false when default_protected_branch lets developers push' do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH) - - expect(project.protected_branch?('master')).to be false - end - - it 'returns true when default_branch_protection does not let developers push but let developer merge branches' do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE) - - expect(project.protected_branch?('master')).to be true - end - - it 'returns true when default_branch_protection is in full protection' do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_FULL) - - expect(project.protected_branch?('master')).to be true - end - end - end - describe '#user_can_push_to_empty_repo?' do let(:project) { create(:empty_project) } let(:user) { create(:user) } @@ -1386,38 +1317,17 @@ describe Project, models: true do end end - describe '#container_registry_path_with_namespace' do - let(:project) { create(:empty_project, path: 'PROJECT') } - - subject { project.container_registry_path_with_namespace } - - it { is_expected.not_to eq(project.path_with_namespace) } - it { is_expected.to eq(project.path_with_namespace.downcase) } - end - - describe '#container_registry_repository' do + describe '#container_registry_url' do let(:project) { create(:empty_project) } - before { stub_container_registry_config(enabled: true) } - - subject { project.container_registry_repository } - - it { is_expected.not_to be_nil } - end - - describe '#container_registry_repository_url' do - let(:project) { create(:empty_project) } - - subject { project.container_registry_repository_url } + subject { project.container_registry_url } before { stub_container_registry_config(**registry_settings) } context 'for enabled registry' do let(:registry_settings) do - { - enabled: true, - host_port: 'example.com', - } + { enabled: true, + host_port: 'example.com' } end it { is_expected.not_to be_nil } @@ -1425,9 +1335,7 @@ describe Project, models: true do context 'for disabled registry' do let(:registry_settings) do - { - enabled: false - } + { enabled: false } end it { is_expected.to be_nil } @@ -1437,28 +1345,60 @@ describe Project, models: true do describe '#has_container_registry_tags?' do let(:project) { create(:empty_project) } - subject { project.has_container_registry_tags? } - - context 'for enabled registry' do + context 'when container registry is enabled' do before { stub_container_registry_config(enabled: true) } - context 'with tags' do - before { stub_container_registry_tags('test', 'test2') } + context 'when tags are present for multi-level registries' do + before do + create(:container_repository, project: project, name: 'image') - it { is_expected.to be_truthy } + stub_container_registry_tags(repository: /image/, + tags: %w[latest rc1]) + end + + it 'should have image tags' do + expect(project).to have_container_registry_tags + end end - context 'when no tags' do - before { stub_container_registry_tags } + context 'when tags are present for root repository' do + before do + stub_container_registry_tags(repository: project.full_path, + tags: %w[latest rc1 pre1]) + end + + it 'should have image tags' do + expect(project).to have_container_registry_tags + end + end + + context 'when there are no tags at all' do + before do + stub_container_registry_tags(repository: :any, tags: []) + end - it { is_expected.to be_falsey } + it 'should not have image tags' do + expect(project).not_to have_container_registry_tags + end end end - context 'for disabled registry' do + context 'when container registry is disabled' do before { stub_container_registry_config(enabled: false) } - it { is_expected.to be_falsey } + it 'should not have image tags' do + expect(project).not_to have_container_registry_tags + end + + it 'should not check root repository tags' do + expect(project).not_to receive(:full_path) + expect(project).not_to have_container_registry_tags + end + + it 'should iterate through container repositories' do + expect(project).to receive(:container_repositories) + expect(project).not_to have_container_registry_tags + end end end @@ -1934,7 +1874,7 @@ describe Project, models: true do describe '#pipeline_status' do let(:project) { create(:project) } it 'builds a pipeline status' do - expect(project.pipeline_status).to be_a(Ci::PipelineStatus) + expect(project.pipeline_status).to be_a(Gitlab::Cache::Ci::ProjectPipelineStatus) end it 'hase a loaded pipeline status' do diff --git a/spec/models/protectable_dropdown_spec.rb b/spec/models/protectable_dropdown_spec.rb new file mode 100644 index 00000000000..4c9bade592b --- /dev/null +++ b/spec/models/protectable_dropdown_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' + +describe ProtectableDropdown, models: true do + let(:project) { create(:project, :repository) } + let(:subject) { described_class.new(project, :branches) } + + describe '#protectable_ref_names' do + before do + project.protected_branches.create(name: 'master') + end + + it { expect(subject.protectable_ref_names).to include('feature') } + it { expect(subject.protectable_ref_names).not_to include('master') } + + it "includes branches matching a protected branch wildcard" do + expect(subject.protectable_ref_names).to include('feature') + + create(:protected_branch, name: 'feat*', project: project) + + subject = described_class.new(project.reload, :branches) + + expect(subject.protectable_ref_names).to include('feature') + end + end +end diff --git a/spec/models/protected_branch_spec.rb b/spec/models/protected_branch_spec.rb index 8bf0d24a128..179a443c43d 100644 --- a/spec/models/protected_branch_spec.rb +++ b/spec/models/protected_branch_spec.rb @@ -113,8 +113,8 @@ describe ProtectedBranch, models: true do staging = build(:protected_branch, name: "staging") expect(ProtectedBranch.matching("production")).to be_empty - expect(ProtectedBranch.matching("production", protected_branches: [production, staging])).to include(production) - expect(ProtectedBranch.matching("production", protected_branches: [production, staging])).not_to include(staging) + expect(ProtectedBranch.matching("production", protected_refs: [production, staging])).to include(production) + expect(ProtectedBranch.matching("production", protected_refs: [production, staging])).not_to include(staging) end end @@ -132,8 +132,64 @@ describe ProtectedBranch, models: true do staging = build(:protected_branch, name: "staging/*") expect(ProtectedBranch.matching("production/some-branch")).to be_empty - expect(ProtectedBranch.matching("production/some-branch", protected_branches: [production, staging])).to include(production) - expect(ProtectedBranch.matching("production/some-branch", protected_branches: [production, staging])).not_to include(staging) + expect(ProtectedBranch.matching("production/some-branch", protected_refs: [production, staging])).to include(production) + expect(ProtectedBranch.matching("production/some-branch", protected_refs: [production, staging])).not_to include(staging) + end + end + end + + describe '#protected?' do + context 'existing project' do + let(:project) { create(:project, :repository) } + + it 'returns true when the branch matches a protected branch via direct match' do + create(:protected_branch, project: project, name: "foo") + + expect(ProtectedBranch.protected?(project, 'foo')).to eq(true) + end + + it 'returns true when the branch matches a protected branch via wildcard match' do + create(:protected_branch, project: project, name: "production/*") + + expect(ProtectedBranch.protected?(project, 'production/some-branch')).to eq(true) + end + + it 'returns false when the branch does not match a protected branch via direct match' do + expect(ProtectedBranch.protected?(project, 'foo')).to eq(false) + end + + it 'returns false when the branch does not match a protected branch via wildcard match' do + create(:protected_branch, project: project, name: "production/*") + + expect(ProtectedBranch.protected?(project, 'staging/some-branch')).to eq(false) + end + end + + context "new project" do + let(:project) { create(:empty_project) } + + it 'returns false when default_protected_branch is unprotected' do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) + + expect(ProtectedBranch.protected?(project, 'master')).to be false + end + + it 'returns false when default_protected_branch lets developers push' do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH) + + expect(ProtectedBranch.protected?(project, 'master')).to be false + end + + it 'returns true when default_branch_protection does not let developers push but let developer merge branches' do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE) + + expect(ProtectedBranch.protected?(project, 'master')).to be true + end + + it 'returns true when default_branch_protection is in full protection' do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_FULL) + + expect(ProtectedBranch.protected?(project, 'master')).to be true end end end diff --git a/spec/models/protected_tag_spec.rb b/spec/models/protected_tag_spec.rb new file mode 100644 index 00000000000..51353852a93 --- /dev/null +++ b/spec/models/protected_tag_spec.rb @@ -0,0 +1,12 @@ +require 'spec_helper' + +describe ProtectedTag, models: true do + describe 'Associations' do + it { is_expected.to belong_to(:project) } + end + + describe 'Validation' do + it { is_expected.to validate_presence_of(:project) } + it { is_expected.to validate_presence_of(:name) } + end +end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index d805e65b3c6..5e5c2b016b6 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1283,8 +1283,6 @@ describe Repository, models: true do describe '#after_import' do it 'flushes and builds the cache' do expect(repository).to receive(:expire_content_cache) - expect(repository).to receive(:expire_tags_cache) - expect(repository).to receive(:expire_branches_cache) repository.after_import end @@ -1831,16 +1829,17 @@ describe Repository, models: true do end end - describe '#is_ancestor?' do - context 'Gitaly is_ancestor feature enabled' do - it 'asks Gitaly server if it\'s an ancestor' do - commit = repository.commit - allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:is_ancestor).and_return(true) - expect(Gitlab::GitalyClient::Commit).to receive(:is_ancestor). - with(repository.raw_repository, commit.id, commit.id).and_return(true) - - expect(repository.is_ancestor?(commit.id, commit.id)).to be true - end - end - end + # TODO: Uncomment when feature is reenabled + # describe '#is_ancestor?' do + # context 'Gitaly is_ancestor feature enabled' do + # it 'asks Gitaly server if it\'s an ancestor' do + # commit = repository.commit + # allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:is_ancestor).and_return(true) + # expect(Gitlab::GitalyClient::Commit).to receive(:is_ancestor). + # with(repository.raw_repository, commit.id, commit.id).and_return(true) + # + # expect(repository.is_ancestor?(commit.id, commit.id)).to be true + # end + # end + # end end diff --git a/spec/models/sent_notification_spec.rb b/spec/models/sent_notification_spec.rb new file mode 100644 index 00000000000..6b7eef388be --- /dev/null +++ b/spec/models/sent_notification_spec.rb @@ -0,0 +1,166 @@ +require 'spec_helper' + +describe SentNotification, model: true do + describe 'validation' do + describe 'note validity' do + context "when the project doesn't match the noteable's project" do + subject { build(:sent_notification, noteable: create(:issue)) } + + it "is invalid" do + expect(subject).not_to be_valid + end + end + + context "when the project doesn't match the discussion project" do + let(:discussion_id) { create(:note).discussion_id } + subject { build(:sent_notification, in_reply_to_discussion_id: discussion_id) } + + it "is invalid" do + expect(subject).not_to be_valid + end + end + + context "when the noteable project and discussion project match" do + let(:project) { create(:project) } + let(:issue) { create(:issue, project: project) } + let(:discussion_id) { create(:note, project: project, noteable: issue).discussion_id } + subject { build(:sent_notification, project: project, noteable: issue, in_reply_to_discussion_id: discussion_id) } + + it "is valid" do + expect(subject).to be_valid + end + end + end + end + + describe '.record' do + let(:user) { create(:user) } + let(:issue) { create(:issue) } + + it 'creates a new SentNotification' do + expect { described_class.record(issue, user.id) }.to change { SentNotification.count }.by(1) + end + end + + describe '.record_note' do + let(:user) { create(:user) } + let(:note) { create(:diff_note_on_merge_request) } + + it 'creates a new SentNotification' do + expect { described_class.record_note(note, user.id) }.to change { SentNotification.count }.by(1) + end + end + + describe '#create_reply' do + context 'for issue' do + let(:issue) { create(:issue) } + subject { described_class.record(issue, issue.author.id) } + + it 'creates a comment on the issue' do + note = subject.create_reply('Test') + expect(note.in_reply_to?(issue)).to be_truthy + end + end + + context 'for issue comment' do + let(:note) { create(:note_on_issue) } + subject { described_class.record_note(note, note.author.id) } + + it 'creates a comment on the issue' do + new_note = subject.create_reply('Test') + expect(new_note.in_reply_to?(note)).to be_truthy + end + end + + context 'for issue discussion' do + let(:note) { create(:discussion_note_on_issue) } + subject { described_class.record_note(note, note.author.id) } + + it 'creates a reply on the discussion' do + new_note = subject.create_reply('Test') + expect(new_note.in_reply_to?(note)).to be_truthy + end + end + + context 'for merge request' do + let(:merge_request) { create(:merge_request) } + subject { described_class.record(merge_request, merge_request.author.id) } + + it 'creates a comment on the merge_request' do + note = subject.create_reply('Test') + expect(note.in_reply_to?(merge_request)).to be_truthy + end + end + + context 'for merge request comment' do + let(:note) { create(:note_on_merge_request) } + subject { described_class.record_note(note, note.author.id) } + + it 'creates a comment on the merge request' do + new_note = subject.create_reply('Test') + expect(new_note.in_reply_to?(note)).to be_truthy + end + end + + context 'for merge request diff discussion' do + let(:note) { create(:diff_note_on_merge_request) } + subject { described_class.record_note(note, note.author.id) } + + it 'creates a reply on the discussion' do + new_note = subject.create_reply('Test') + expect(new_note.in_reply_to?(note)).to be_truthy + end + end + + context 'for merge request non-diff discussion' do + let(:note) { create(:discussion_note_on_merge_request) } + subject { described_class.record_note(note, note.author.id) } + + it 'creates a reply on the discussion' do + new_note = subject.create_reply('Test') + expect(new_note.in_reply_to?(note)).to be_truthy + end + end + + context 'for commit' do + let(:project) { create(:project) } + let(:commit) { project.commit } + subject { described_class.record(commit, project.creator.id) } + + it 'creates a comment on the commit' do + note = subject.create_reply('Test') + expect(note.in_reply_to?(commit)).to be_truthy + end + end + + context 'for commit comment' do + let(:note) { create(:note_on_commit) } + subject { described_class.record_note(note, note.author.id) } + + it 'creates a comment on the commit' do + new_note = subject.create_reply('Test') + expect(new_note.in_reply_to?(note)).to be_truthy + end + end + + context 'for commit diff discussion' do + let(:note) { create(:diff_note_on_commit) } + subject { described_class.record_note(note, note.author.id) } + + it 'creates a reply on the discussion' do + new_note = subject.create_reply('Test') + expect(new_note.in_reply_to?(note)).to be_truthy + end + end + + context 'for commit non-diff discussion' do + let(:note) { create(:discussion_note_on_commit) } + subject { described_class.record_note(note, note.author.id) } + + it 'creates a reply on the discussion' do + new_note = subject.create_reply('Test') + expect(new_note.in_reply_to?(note)).to be_truthy + end + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a9e37be1157..9de16c41e94 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -28,7 +28,6 @@ describe User, models: true do it { is_expected.to have_many(:merge_requests).dependent(:destroy) } it { is_expected.to have_many(:assigned_merge_requests).dependent(:nullify) } it { is_expected.to have_many(:identities).dependent(:destroy) } - it { is_expected.to have_one(:abuse_report) } it { is_expected.to have_many(:spam_logs).dependent(:destroy) } it { is_expected.to have_many(:todos).dependent(:destroy) } it { is_expected.to have_many(:award_emoji).dependent(:destroy) } @@ -37,6 +36,34 @@ describe User, models: true do it { is_expected.to have_many(:pipelines).dependent(:nullify) } it { is_expected.to have_many(:chat_names).dependent(:destroy) } it { is_expected.to have_many(:uploads).dependent(:destroy) } + it { is_expected.to have_many(:reported_abuse_reports).dependent(:destroy).class_name('AbuseReport') } + + describe "#abuse_report" do + let(:current_user) { create(:user) } + let(:other_user) { create(:user) } + + it { is_expected.to have_one(:abuse_report) } + + it "refers to the abuse report whose user_id is the current user" do + abuse_report = create(:abuse_report, reporter: other_user, user: current_user) + + expect(current_user.abuse_report).to eq(abuse_report) + end + + it "does not refer to the abuse report whose reporter_id is the current user" do + create(:abuse_report, reporter: current_user, user: other_user) + + expect(current_user.abuse_report).to be_nil + end + + it "does not update the user_id of an abuse report when the user is updated" do + abuse_report = create(:abuse_report, reporter: current_user, user: other_user) + + current_user.block + + expect(abuse_report.reload.user).to eq(other_user) + end + end describe '#group_members' do it 'does not include group memberships for which user is a requester' do @@ -288,7 +315,7 @@ describe User, models: true do end describe "Respond to" do - it { is_expected.to respond_to(:is_admin?) } + it { is_expected.to respond_to(:admin?) } it { is_expected.to respond_to(:name) } it { is_expected.to respond_to(:private_token) } it { is_expected.to respond_to(:external?) } @@ -559,7 +586,7 @@ describe User, models: true do describe 'normal user' do let(:user) { create(:user, name: 'John Smith') } - it { expect(user.is_admin?).to be_falsey } + it { expect(user.admin?).to be_falsey } it { expect(user.require_ssh_key?).to be_truthy } it { expect(user.can_create_group?).to be_truthy } it { expect(user.can_create_project?).to be_truthy } @@ -1407,6 +1434,17 @@ describe User, models: true do it { expect(user.nested_groups).to eq([nested_group]) } end + describe '#all_expanded_groups' do + let!(:user) { create(:user) } + let!(:group) { create(:group) } + let!(:nested_group_1) { create(:group, parent: group) } + let!(:nested_group_2) { create(:group, parent: group) } + + before { nested_group_1.add_owner(user) } + + it { expect(user.all_expanded_groups).to match_array [group, nested_group_1] } + end + describe '#nested_groups_projects' do let!(:user) { create(:user) } let!(:group) { create(:group) } @@ -1521,4 +1559,76 @@ describe User, models: true do end end end + + describe '#update_two_factor_requirement' do + let(:user) { create :user } + + context 'with 2FA requirement on groups' do + let(:group1) { create :group, require_two_factor_authentication: true, two_factor_grace_period: 23 } + let(:group2) { create :group, require_two_factor_authentication: true, two_factor_grace_period: 32 } + + before do + group1.add_user(user, GroupMember::OWNER) + group2.add_user(user, GroupMember::OWNER) + + user.update_two_factor_requirement + end + + it 'requires 2FA' do + expect(user.require_two_factor_authentication_from_group).to be true + end + + it 'uses the shortest grace period' do + expect(user.two_factor_grace_period).to be 23 + end + end + + context 'with 2FA requirement on nested parent group' do + let!(:group1) { create :group, require_two_factor_authentication: true } + let!(:group1a) { create :group, require_two_factor_authentication: false, parent: group1 } + + before do + group1a.add_user(user, GroupMember::OWNER) + + user.update_two_factor_requirement + end + + it 'requires 2FA' do + expect(user.require_two_factor_authentication_from_group).to be true + end + end + + context 'with 2FA requirement on nested child group' do + let!(:group1) { create :group, require_two_factor_authentication: false } + let!(:group1a) { create :group, require_two_factor_authentication: true, parent: group1 } + + before do + group1.add_user(user, GroupMember::OWNER) + + user.update_two_factor_requirement + end + + it 'requires 2FA' do + expect(user.require_two_factor_authentication_from_group).to be true + end + end + + context 'without 2FA requirement on groups' do + let(:group) { create :group } + + before do + group.add_user(user, GroupMember::OWNER) + + user.update_two_factor_requirement + end + + it 'does not require 2FA' do + expect(user.require_two_factor_authentication_from_group).to be false + end + + it 'falls back to the default grace period' do + expect(user.two_factor_grace_period).to be 48 + end + end + end end |