Merge remote-tracking branch 'origin/master' into 2979-personal-access-tokens

7 files changed, 274 insertions, 15 deletions

diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index 7660ea2659c..2beb6cc598d 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -219,7 +219,7 @@ describe Ci::Build, models: true do
 
         context 'and trigger variables' do
           let(:trigger) { create(:ci_trigger, project: project) }
-          let(:trigger_request) { create(:ci_trigger_request_with_variables, commit: pipeline, trigger: trigger) }
+          let(:trigger_request) { create(:ci_trigger_request_with_variables, pipeline: pipeline, trigger: trigger) }
           let(:trigger_variables) do
             [
               { key: :TRIGGER_KEY, value: 'TRIGGER_VALUE', public: false }
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 1b7cbc3efda..3b199f4d98d 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -455,4 +455,157 @@ describe MergeRequest, models: true do
       expect(user2.assigned_open_merge_request_count).to eq(1)
     end
   end
+
+  describe '#check_if_can_be_merged' do
+    let(:project) { create(:project, only_allow_merge_if_build_succeeds: true) }
+
+    subject { create(:merge_request, source_project: project, merge_status: :unchecked) }
+
+    context 'when it is not broken and has no conflicts' do
+      it 'is marked as mergeable' do
+        allow(subject).to receive(:broken?) { false }
+        allow(project).to receive_message_chain(:repository, :can_be_merged?) { true }
+
+        expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('can_be_merged')
+      end
+    end
+
+    context 'when broken' do
+      before { allow(subject).to receive(:broken?) { true } }
+
+      it 'becomes unmergeable' do
+        expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('cannot_be_merged')
+      end
+    end
+
+    context 'when it has conflicts' do
+      before do
+        allow(subject).to receive(:broken?) { false }
+        allow(project).to receive_message_chain(:repository, :can_be_merged?) { false }
+      end
+
+      it 'becomes unmergeable' do
+        expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('cannot_be_merged')
+      end
+    end
+  end
+
+  describe '#mergeable?' do
+    let(:project) { create(:project) }
+
+    subject { create(:merge_request, source_project: project) }
+
+    it 'returns false if #mergeable_state? is false' do
+      expect(subject).to receive(:mergeable_state?) { false }
+
+      expect(subject.mergeable?).to be_falsey
+    end
+
+    it 'return true if #mergeable_state? is true and the MR #can_be_merged? is true' do
+      allow(subject).to receive(:mergeable_state?) { true }
+      expect(subject).to receive(:check_if_can_be_merged)
+      expect(subject).to receive(:can_be_merged?) { true }
+
+      expect(subject.mergeable?).to be_truthy
+    end
+  end
+
+  describe '#mergeable_state?' do
+    let(:project) { create(:project) }
+
+    subject { create(:merge_request, source_project: project) }
+
+    it 'checks if merge request can be merged' do
+      allow(subject).to receive(:mergeable_ci_state?) { true }
+      expect(subject).to receive(:check_if_can_be_merged)
+
+      subject.mergeable?
+    end
+
+    context 'when not open' do
+      before { subject.close }
+
+      it 'returns false' do
+        expect(subject.mergeable_state?).to be_falsey
+      end
+    end
+
+    context 'when working in progress' do
+      before { subject.title = 'WIP MR' }
+
+      it 'returns false' do
+        expect(subject.mergeable_state?).to be_falsey
+      end
+    end
+
+    context 'when broken' do
+      before { allow(subject).to receive(:broken?) { true } }
+
+      it 'returns false' do
+        expect(subject.mergeable_state?).to be_falsey
+      end
+    end
+
+    context 'when failed' do
+      before { allow(subject).to receive(:broken?) { false } }
+
+      context 'when project settings restrict to merge only if build succeeds and build failed' do
+        before do
+          project.only_allow_merge_if_build_succeeds = true
+          allow(subject).to receive(:mergeable_ci_state?) { false }
+        end
+
+        it 'returns false' do
+          expect(subject.mergeable_state?).to be_falsey
+        end
+      end
+    end
+  end
+
+  describe '#mergeable_ci_state?' do
+    let(:project) { create(:empty_project, only_allow_merge_if_build_succeeds: true) }
+    let(:pipeline) { create(:ci_empty_pipeline) }
+
+    subject { build(:merge_request, target_project: project) }
+
+    context 'when it is only allowed to merge when build is green' do
+      context 'and a failed pipeline is associated' do
+        before do
+          pipeline.statuses << create(:commit_status, status: 'failed', project: project)
+          allow(subject).to receive(:pipeline) { pipeline }
+        end
+
+        it { expect(subject.mergeable_ci_state?).to be_falsey }
+      end
+
+      context 'when no pipeline is associated' do
+        before do
+          allow(subject).to receive(:pipeline) { nil }
+        end
+
+        it { expect(subject.mergeable_ci_state?).to be_truthy }
+      end
+    end
+
+    context 'when merges are not restricted to green builds' do
+      subject { build(:merge_request, target_project: build(:empty_project, only_allow_merge_if_build_succeeds: false)) }
+
+      context 'and a failed pipeline is associated' do
+        before do
+          pipeline.statuses << create(:commit_status, status: 'failed', project: project)
+          allow(subject).to receive(:pipeline) { pipeline }
+        end
+
+        it { expect(subject.mergeable_ci_state?).to be_truthy }
+      end
+
+      context 'when no pipeline is associated' do
+        before do
+          allow(subject).to receive(:pipeline) { nil }
+        end
+
+        it { expect(subject.mergeable_ci_state?).to be_truthy }
+      end
+    end
+  end
 end
diff --git a/spec/models/notification_setting_spec.rb b/spec/models/notification_setting_spec.rb
index 295081e9da1..4e24e89b008 100644
--- a/spec/models/notification_setting_spec.rb
+++ b/spec/models/notification_setting_spec.rb
@@ -10,7 +10,6 @@ RSpec.describe NotificationSetting, type: :model do
     subject { NotificationSetting.new(source_id: 1, source_type: 'Project') }
 
     it { is_expected.to validate_presence_of(:user) }
-    it { is_expected.to validate_presence_of(:source) }
     it { is_expected.to validate_presence_of(:level) }
     it { is_expected.to validate_uniqueness_of(:user_id).scoped_to([:source_id, :source_type]).with_message(/already exists in source/) }
   end
diff --git a/spec/models/project_services/bamboo_service_spec.rb b/spec/models/project_services/bamboo_service_spec.rb
index e771f35811e..ec81f05fc7a 100644
--- a/spec/models/project_services/bamboo_service_spec.rb
+++ b/spec/models/project_services/bamboo_service_spec.rb
@@ -194,7 +194,7 @@ describe BambooService, models: true do
 
   def service(bamboo_url: 'http://gitlab.com')
     described_class.create(
-      project: build_stubbed(:empty_project),
+      project: create(:empty_project),
       properties: {
         bamboo_url: bamboo_url,
         username: 'mic',
diff --git a/spec/models/project_services/teamcity_service_spec.rb b/spec/models/project_services/teamcity_service_spec.rb
index ad24b895170..24a708ca849 100644
--- a/spec/models/project_services/teamcity_service_spec.rb
+++ b/spec/models/project_services/teamcity_service_spec.rb
@@ -182,7 +182,7 @@ describe TeamcityService, models: true do
 
   def service(teamcity_url: 'http://gitlab.com')
     described_class.create(
-      project: build_stubbed(:empty_project),
+      project: create(:empty_project),
       properties: {
         teamcity_url: teamcity_url,
         username: 'mic',
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 553556ed326..de8815f5a38 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -53,7 +53,6 @@ describe Project, models: true do
     it { is_expected.to validate_length_of(:path).is_within(0..255) }
     it { is_expected.to validate_length_of(:description).is_within(0..2000) }
     it { is_expected.to validate_presence_of(:creator) }
-    it { is_expected.to validate_length_of(:issues_tracker_id).is_within(0..255) }
     it { is_expected.to validate_presence_of(:namespace) }
 
     it 'should not allow new projects beyond user limits' do
@@ -258,24 +257,66 @@ describe Project, models: true do
     end
   end
 
-  describe :can_have_issues_tracker_id? do
+  describe :external_issue_tracker do
     let(:project) { create(:project) }
     let(:ext_project) { create(:redmine_project) }
 
-    it 'should be true for projects with external issues tracker if issues enabled' do
-      expect(ext_project.can_have_issues_tracker_id?).to be_truthy
+    context 'on existing projects with no value for has_external_issue_tracker' do
+      before(:each) do
+        project.update_column(:has_external_issue_tracker, nil)
+        ext_project.update_column(:has_external_issue_tracker, nil)
+      end
+
+      it 'updates the has_external_issue_tracker boolean' do
+        expect do
+          project.external_issue_tracker
+        end.to change { project.reload.has_external_issue_tracker }.to(false)
+
+        expect do
+          ext_project.external_issue_tracker
+        end.to change { ext_project.reload.has_external_issue_tracker }.to(true)
+      end
+    end
+
+    it 'returns nil and does not query services when there is no external issue tracker' do
+      project.build_missing_services
+      project.reload
+
+      expect(project).not_to receive(:services)
+
+      expect(project.external_issue_tracker).to eq(nil)
+    end
+
+    it 'retrieves external_issue_tracker querying services and cache it when there is external issue tracker' do
+      ext_project.reload # Factory returns a project with changed attributes
+      ext_project.build_missing_services
+      ext_project.reload
+
+      expect(ext_project).to receive(:services).once.and_call_original
+
+      2.times { expect(ext_project.external_issue_tracker).to be_a_kind_of(RedmineService) }
     end
+  end
+
+  describe :cache_has_external_issue_tracker do
+    let(:project) { create(:project) }
 
-    it 'should be false for projects with internal issue tracker if issues enabled' do
-      expect(project.can_have_issues_tracker_id?).to be_falsey
+    it 'stores true if there is any external_issue_tracker' do
+      services = double(:service, external_issue_trackers: [RedmineService.new])
+      expect(project).to receive(:services).and_return(services)
+
+      expect do
+        project.cache_has_external_issue_tracker
+      end.to change { project.has_external_issue_tracker}.to(true)
     end
 
-    it 'should be always false if issues disabled' do
-      project.issues_enabled = false
-      ext_project.issues_enabled = false
+    it 'stores false if there is no external_issue_tracker' do
+      services = double(:service, external_issue_trackers: [])
+      expect(project).to receive(:services).and_return(services)
 
-      expect(project.can_have_issues_tracker_id?).to be_falsey
-      expect(ext_project.can_have_issues_tracker_id?).to be_falsey
+      expect do
+        project.cache_has_external_issue_tracker
+      end.to change { project.has_external_issue_tracker}.to(false)
     end
   end
 
@@ -859,4 +900,37 @@ describe Project, models: true do
       it { is_expected.to be_falsey }
     end
   end
+
+  describe '.where_paths_in' do
+    context 'without any paths' do
+      it 'returns an empty relation' do
+        expect(Project.where_paths_in([])).to eq([])
+      end
+    end
+
+    context 'without any valid paths' do
+      it 'returns an empty relation' do
+        expect(Project.where_paths_in(%w[foo])).to eq([])
+      end
+    end
+
+    context 'with valid paths' do
+      let!(:project1) { create(:project) }
+      let!(:project2) { create(:project) }
+
+      it 'returns the projects matching the paths' do
+        projects = Project.where_paths_in([project1.path_with_namespace,
+                                           project2.path_with_namespace])
+
+        expect(projects).to contain_exactly(project1, project2)
+      end
+
+      it 'returns projects regardless of the casing of paths' do
+        projects = Project.where_paths_in([project1.path_with_namespace.upcase,
+                                           project2.path_with_namespace.upcase])
+
+        expect(projects).to contain_exactly(project1, project2)
+      end
+    end
+  end
 end
diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb
index 8592e112c50..2f000dbc01a 100644
--- a/spec/models/service_spec.rb
+++ b/spec/models/service_spec.rb
@@ -204,4 +204,37 @@ describe Service, models: true do
       expect(service.bamboo_url_was).to be_nil
     end
   end
+
+  describe "callbacks" do
+    let(:project) { create(:project) }
+    let!(:service) do
+      RedmineService.new(
+        project: project,
+        active: true,
+        properties: {
+          project_url: 'http://redmine/projects/project_name_in_redmine',
+          issues_url: "http://redmine/#{project.id}/project_name_in_redmine/:id",
+          new_issue_url: 'http://redmine/projects/project_name_in_redmine/issues/new'
+        }
+      )
+    end
+
+    describe "on create" do
+      it "updates the has_external_issue_tracker boolean" do
+        expect do
+          service.save!
+        end.to change { service.project.has_external_issue_tracker }.from(nil).to(true)
+      end
+    end
+
+    describe "on update" do
+      it "updates the has_external_issue_tracker boolean" do
+        service.save!
+
+        expect do
+          service.update_attributes(active: false)
+        end.to change { service.project.has_external_issue_tracker }.from(true).to(false)
+      end
+    end
+  end
 end