Merge remote-tracking branch 'origin/master' into 14566-confidential-issue-branches

5 files changed, 459 insertions, 112 deletions

diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index ed982d8a9d4..da1c673653a 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -195,7 +195,7 @@ describe Issue, models: true do
 
     before do
       allow(subject.project.repository).to receive(:branch_names).
-                                            and_return(["mpempe", "#{subject.iid}mepmep", subject.to_branch_name, "branch-#{subject.iid}"])
+                                            and_return(["mpempe", "#{subject.iid}mepmep", subject.to_branch_name, "#{subject.iid}-branch"])
 
       # Without this stub, the `create(:merge_request)` above fails because it can't find
       # the source branch. This seems like a reasonable compromise, in comparison with
@@ -204,17 +204,24 @@ describe Issue, models: true do
     end
 
     it "selects the right branches when there are no referenced merge requests" do
-      expect(subject.related_branches(user)).to eq([subject.to_branch_name, "branch-#{subject.iid}"])
+      expect(subject.related_branches(user)).to eq([subject.to_branch_name, "#{subject.iid}-branch"])
     end
 
     it "selects the right branches when there is a referenced merge request" do
       merge_request = create(:merge_request, { description: "Closes ##{subject.iid}",
                                                source_project: subject.project,
-                                               source_branch: "branch-#{subject.iid}" })
+                                               source_branch: "#{subject.iid}-branch" })
       merge_request.create_cross_references!(user)
       expect(subject.referenced_merge_requests).to_not be_empty
       expect(subject.related_branches(user)).to eq([subject.to_branch_name])
     end
+
+    it 'excludes stable branches from the related branches' do
+      allow(subject.project.repository).to receive(:branch_names).
+        and_return(["#{subject.iid}-0-stable"])
+
+      expect(subject.related_branches(user)).to eq []
+    end
   end
 
   it_behaves_like 'an editable mentionable' do
@@ -231,12 +238,12 @@ describe Issue, models: true do
   describe "#to_branch_name" do
     let(:issue) { create(:issue, title: 'testing-issue') }
 
-    it "ends with the issue iid" do
-      expect(issue.to_branch_name).to match /-#{issue.iid}\z/
+    it 'starts with the issue iid' do
+      expect(issue.to_branch_name).to match /\A#{issue.iid}-[A-Za-z\-]+\z/
     end
 
     it "contains the issue title if not confidential" do
-      expect(issue.to_branch_name).to match /\Atesting-issue/
+      expect(issue.to_branch_name).to match /testing-issue\z/
     end
 
     it "does not contain the issue title if confidential" do
diff --git a/spec/models/project_services/bamboo_service_spec.rb b/spec/models/project_services/bamboo_service_spec.rb
index c34b2487ecf..31b2c90122d 100644
--- a/spec/models/project_services/bamboo_service_spec.rb
+++ b/spec/models/project_services/bamboo_service_spec.rb
@@ -21,74 +21,232 @@
 require 'spec_helper'
 
 describe BambooService, models: true do
-  describe "Associations" do
+  describe 'Associations' do
     it { is_expected.to belong_to :project }
     it { is_expected.to have_one :service_hook }
   end
 
-  describe "Execute" do
-    let(:user)    { create(:user) }
-    let(:project) { create(:project) }
-
-    context "when a password was previously set" do
-      before do
-        @bamboo_service = BambooService.create(
-          project: create(:project),
-          properties: {
-            bamboo_url: 'http://gitlab.com',
-            username: 'mic',
-            password: "password"
-          }
-        )
+  describe 'Validations' do
+    describe '#bamboo_url' do
+      it 'does not validate the presence of bamboo_url if service is not active' do
+        bamboo_service = service
+        bamboo_service.active = false
+
+        expect(bamboo_service).not_to validate_presence_of(:bamboo_url)
+      end
+
+      it 'validates the presence of bamboo_url if service is active' do
+        bamboo_service = service
+        bamboo_service.active = true
+
+        expect(bamboo_service).to validate_presence_of(:bamboo_url)
+      end
+    end
+
+    describe '#build_key' do
+      it 'does not validate the presence of build_key if service is not active' do
+        bamboo_service = service
+        bamboo_service.active = false
+
+        expect(bamboo_service).not_to validate_presence_of(:build_key)
       end
-  
-      it "reset password if url changed" do
-        @bamboo_service.bamboo_url = 'http://gitlab1.com'
-        @bamboo_service.save
-        expect(@bamboo_service.password).to be_nil
+
+      it 'validates the presence of build_key if service is active' do
+        bamboo_service = service
+        bamboo_service.active = true
+
+        expect(bamboo_service).to validate_presence_of(:build_key)
+      end
+    end
+
+    describe '#username' do
+      it 'does not validate the presence of username if service is not active' do
+        bamboo_service = service
+        bamboo_service.active = false
+
+        expect(bamboo_service).not_to validate_presence_of(:username)
+      end
+
+      it 'does not validate the presence of username if username is nil' do
+        bamboo_service = service
+        bamboo_service.active = true
+        bamboo_service.password = nil
+
+        expect(bamboo_service).not_to validate_presence_of(:username)
+      end
+
+      it 'validates the presence of username if service is active and username is present' do
+        bamboo_service = service
+        bamboo_service.active = true
+        bamboo_service.password = 'secret'
+
+        expect(bamboo_service).to validate_presence_of(:username)
       end
-  
-      it "does not reset password if username changed" do
-        @bamboo_service.username = "some_name"
-        @bamboo_service.save
-        expect(@bamboo_service.password).to eq("password")
+    end
+
+    describe '#password' do
+      it 'does not validate the presence of password if service is not active' do
+        bamboo_service = service
+        bamboo_service.active = false
+
+        expect(bamboo_service).not_to validate_presence_of(:password)
       end
 
-      it "does not reset password if new url is set together with password, even if it's the same password" do
-        @bamboo_service.bamboo_url = 'http://gitlab_edited.com'
-        @bamboo_service.password = 'password'
-        @bamboo_service.save
-        expect(@bamboo_service.password).to eq("password")
-        expect(@bamboo_service.bamboo_url).to eq("http://gitlab_edited.com")
+      it 'does not validate the presence of password if username is nil' do
+        bamboo_service = service
+        bamboo_service.active = true
+        bamboo_service.username = nil
+
+        expect(bamboo_service).not_to validate_presence_of(:password)
       end
 
-      it "should reset password if url changed, even if setter called multiple times" do
-        @bamboo_service.bamboo_url = 'http://gitlab1.com'
-        @bamboo_service.bamboo_url = 'http://gitlab1.com'
-        @bamboo_service.save
-        expect(@bamboo_service.password).to be_nil
+      it 'validates the presence of password if service is active and username is present' do
+        bamboo_service = service
+        bamboo_service.active = true
+        bamboo_service.username = 'john'
+
+        expect(bamboo_service).to validate_presence_of(:password)
       end
     end
-    
-    context "when no password was previously set" do
-      before do
-        @bamboo_service = BambooService.create(
-          project: create(:project),
-          properties: {
-            bamboo_url: 'http://gitlab.com',
-            username: 'mic'
-          }
-        )
+  end
+
+  describe 'Callbacks' do
+    describe 'before_update :reset_password' do
+      context 'when a password was previously set' do
+        it 'resets password if url changed' do
+          bamboo_service = service
+
+          bamboo_service.bamboo_url = 'http://gitlab1.com'
+          bamboo_service.save
+
+          expect(bamboo_service.password).to be_nil
+        end
+
+        it 'does not reset password if username changed' do
+          bamboo_service = service
+
+          bamboo_service.username = 'some_name'
+          bamboo_service.save
+
+          expect(bamboo_service.password).to eq('password')
+        end
+
+        it "does not reset password if new url is set together with password, even if it's the same password" do
+          bamboo_service = service
+
+          bamboo_service.bamboo_url = 'http://gitlab_edited.com'
+          bamboo_service.password = 'password'
+          bamboo_service.save
+
+          expect(bamboo_service.password).to eq('password')
+          expect(bamboo_service.bamboo_url).to eq('http://gitlab_edited.com')
+        end
       end
 
-      it "saves password if new url is set together with password" do
-        @bamboo_service.bamboo_url = 'http://gitlab_edited.com'
-        @bamboo_service.password = 'password'
-        @bamboo_service.save
-        expect(@bamboo_service.password).to eq("password")
-        expect(@bamboo_service.bamboo_url).to eq("http://gitlab_edited.com")
+      it 'saves password if new url is set together with password when no password was previously set' do
+        bamboo_service = service
+        bamboo_service.password = nil
+
+        bamboo_service.bamboo_url = 'http://gitlab_edited.com'
+        bamboo_service.password = 'password'
+        bamboo_service.save
+
+        expect(bamboo_service.password).to eq('password')
+        expect(bamboo_service.bamboo_url).to eq('http://gitlab_edited.com')
       end
+    end
+  end
+
+  describe '#build_page' do
+    it 'returns a specific URL when status is 500' do
+      stub_request(status: 500)
+
+      expect(service.build_page('123', 'unused')).to eq('http://gitlab.com/browse/foo')
+    end
+
+    it 'returns a specific URL when response has no results' do
+      stub_request(body: %Q({"results":{"results":{"size":"0"}}}))
+
+      expect(service.build_page('123', 'unused')).to eq('http://gitlab.com/browse/foo')
+    end
+
+    it 'returns a build URL when bamboo_url has no trailing slash' do
+      stub_request(body: %Q({"results":{"results":{"result":{"planResultKey":{"key":"42"}}}}}))
+
+      expect(service(bamboo_url: 'http://gitlab.com').build_page('123', 'unused')).to eq('http://gitlab.com/browse/42')
+    end
+
+    it 'returns a build URL when bamboo_url has a trailing slash' do
+      stub_request(body: %Q({"results":{"results":{"result":{"planResultKey":{"key":"42"}}}}}))
+
+      expect(service(bamboo_url: 'http://gitlab.com/').build_page('123', 'unused')).to eq('http://gitlab.com/browse/42')
+    end
+  end
+
+  describe '#commit_status' do
+    it 'sets commit status to :error when status is 500' do
+      stub_request(status: 500)
+
+      expect(service.commit_status('123', 'unused')).to eq(:error)
+    end
+
+    it 'sets commit status to "pending" when status is 404' do
+      stub_request(status: 404)
+
+      expect(service.commit_status('123', 'unused')).to eq('pending')
+    end
+
+    it 'sets commit status to "pending" when response has no results' do
+      stub_request(body: %Q({"results":{"results":{"size":"0"}}}))
+
+      expect(service.commit_status('123', 'unused')).to eq('pending')
+    end
+
+    it 'sets commit status to "success" when build state contains Success' do
+      stub_request(build_state: 'YAY Success!')
 
+      expect(service.commit_status('123', 'unused')).to eq('success')
     end
+
+    it 'sets commit status to "failed" when build state contains Failed' do
+      stub_request(build_state: 'NO Failed!')
+
+      expect(service.commit_status('123', 'unused')).to eq('failed')
+    end
+
+    it 'sets commit status to "pending" when build state contains Pending' do
+      stub_request(build_state: 'NO Pending!')
+
+      expect(service.commit_status('123', 'unused')).to eq('pending')
+    end
+
+    it 'sets commit status to :error when build state is unknown' do
+      stub_request(build_state: 'FOO BAR!')
+
+      expect(service.commit_status('123', 'unused')).to eq(:error)
+    end
+  end
+
+  def service(bamboo_url: 'http://gitlab.com')
+    described_class.create(
+      project: build_stubbed(:empty_project),
+      properties: {
+        bamboo_url: bamboo_url,
+        username: 'mic',
+        password: 'password',
+        build_key: 'foo'
+      }
+    )
+  end
+
+  def stub_request(status: 200, body: nil, build_state: 'success')
+    bamboo_full_url = 'http://mic:password@gitlab.com/rest/api/latest/result?label=123&os_authType=basic'
+    body ||= %Q({"results":{"results":{"result":{"buildState":"#{build_state}"}}}})
+
+    WebMock.stub_request(:get, bamboo_full_url).to_return(
+      status: status,
+      headers: { 'Content-Type' => 'application/json' },
+      body: body
+    )
   end
 end
diff --git a/spec/models/project_services/builds_email_service_spec.rb b/spec/models/project_services/builds_email_service_spec.rb
index 2ccbff553f0..7c23c2efccd 100644
--- a/spec/models/project_services/builds_email_service_spec.rb
+++ b/spec/models/project_services/builds_email_service_spec.rb
@@ -3,9 +3,10 @@ require 'spec_helper'
 describe BuildsEmailService do
   let(:build) { create(:ci_build) }
   let(:data) { Gitlab::BuildDataBuilder.build(build) }
-  let(:service) { BuildsEmailService.new }
+  let!(:project) { create(:project, :public, ci_id: 1) }
+  let(:service) { described_class.new(project: project, active: true) }
 
-  describe :execute do
+  describe '#execute' do
     it 'sends email' do
       service.recipients = 'test@gitlab.com'
       data[:build_status] = 'failed'
@@ -40,4 +41,36 @@ describe BuildsEmailService do
       service.execute(data)
     end
   end
+
+  describe 'validations' do
+
+    context 'when pusher is not added' do
+      before { service.add_pusher = false }
+
+      it 'does not allow empty recipient input' do
+        service.recipients = ''
+        expect(service.valid?).to be false
+      end
+
+      it 'does allow non-empty recipient input' do
+        service.recipients = 'test@example.com'
+        expect(service.valid?).to be true
+      end
+
+    end
+
+    context 'when pusher is added' do
+      before { service.add_pusher = true }
+
+      it 'does allow empty recipient input' do
+        service.recipients = ''
+        expect(service.valid?).to be true
+      end
+
+      it 'does allow non-empty recipient input' do
+        service.recipients = 'test@example.com'
+        expect(service.valid?).to be true
+      end
+    end
+  end
 end
diff --git a/spec/models/project_services/teamcity_service_spec.rb b/spec/models/project_services/teamcity_service_spec.rb
index f26b47a856c..bc7423cee69 100644
--- a/spec/models/project_services/teamcity_service_spec.rb
+++ b/spec/models/project_services/teamcity_service_spec.rb
@@ -21,73 +21,220 @@
 require 'spec_helper'
 
 describe TeamcityService, models: true do
-  describe "Associations" do
+  describe 'Associations' do
     it { is_expected.to belong_to :project }
     it { is_expected.to have_one :service_hook }
   end
 
-  describe "Execute" do
-    let(:user)    { create(:user) }
-    let(:project) { create(:project) }
-
-    context "when a password was previously set" do
-      before do
-        @teamcity_service = TeamcityService.create(
-          project: create(:project),
-          properties: {
-            teamcity_url: 'http://gitlab.com',
-            username: 'mic',
-            password: "password"
-          }
-        )
+  describe 'Validations' do
+    describe '#teamcity_url' do
+      it 'does not validate the presence of teamcity_url if service is not active' do
+        teamcity_service = service
+        teamcity_service.active = false
+
+        expect(teamcity_service).not_to validate_presence_of(:teamcity_url)
       end
-  
-      it "reset password if url changed" do
-        @teamcity_service.teamcity_url = 'http://gitlab1.com'
-        @teamcity_service.save
-        expect(@teamcity_service.password).to be_nil
+
+      it 'validates the presence of teamcity_url if service is active' do
+        teamcity_service = service
+        teamcity_service.active = true
+
+        expect(teamcity_service).to validate_presence_of(:teamcity_url)
+      end
+    end
+
+    describe '#build_type' do
+      it 'does not validate the presence of build_type if service is not active' do
+        teamcity_service = service
+        teamcity_service.active = false
+
+        expect(teamcity_service).not_to validate_presence_of(:build_type)
+      end
+
+      it 'validates the presence of build_type if service is active' do
+        teamcity_service = service
+        teamcity_service.active = true
+
+        expect(teamcity_service).to validate_presence_of(:build_type)
       end
-  
-      it "does not reset password if username changed" do
-        @teamcity_service.username = "some_name"
-        @teamcity_service.save
-        expect(@teamcity_service.password).to eq("password")
+    end
+
+    describe '#username' do
+      it 'does not validate the presence of username if service is not active' do
+        teamcity_service = service
+        teamcity_service.active = false
+
+        expect(teamcity_service).not_to validate_presence_of(:username)
       end
 
-      it "does not reset password if new url is set together with password, even if it's the same password" do
-        @teamcity_service.teamcity_url = 'http://gitlab_edited.com'
-        @teamcity_service.password = 'password'
-        @teamcity_service.save
-        expect(@teamcity_service.password).to eq("password")
-        expect(@teamcity_service.teamcity_url).to eq("http://gitlab_edited.com")
+      it 'does not validate the presence of username if username is nil' do
+        teamcity_service = service
+        teamcity_service.active = true
+        teamcity_service.password = nil
+
+        expect(teamcity_service).not_to validate_presence_of(:username)
       end
 
-      it "should reset password if url changed, even if setter called multiple times" do
-        @teamcity_service.teamcity_url = 'http://gitlab1.com'
-        @teamcity_service.teamcity_url = 'http://gitlab1.com'
-        @teamcity_service.save
-        expect(@teamcity_service.password).to be_nil
+      it 'validates the presence of username if service is active and username is present' do
+        teamcity_service = service
+        teamcity_service.active = true
+        teamcity_service.password = 'secret'
+
+        expect(teamcity_service).to validate_presence_of(:username)
       end
     end
-    
-    context "when no password was previously set" do
-      before do
-        @teamcity_service = TeamcityService.create(
-          project: create(:project),
-          properties: {
-            teamcity_url: 'http://gitlab.com',
-            username: 'mic'
-          }
-        )
+
+    describe '#password' do
+      it 'does not validate the presence of password if service is not active' do
+        teamcity_service = service
+        teamcity_service.active = false
+
+        expect(teamcity_service).not_to validate_presence_of(:password)
+      end
+
+      it 'does not validate the presence of password if username is nil' do
+        teamcity_service = service
+        teamcity_service.active = true
+        teamcity_service.username = nil
+
+        expect(teamcity_service).not_to validate_presence_of(:password)
       end
 
-      it "saves password if new url is set together with password" do
-        @teamcity_service.teamcity_url = 'http://gitlab_edited.com'
-        @teamcity_service.password = 'password'
-        @teamcity_service.save
-        expect(@teamcity_service.password).to eq("password")
-        expect(@teamcity_service.teamcity_url).to eq("http://gitlab_edited.com")
+      it 'validates the presence of password if service is active and username is present' do
+        teamcity_service = service
+        teamcity_service.active = true
+        teamcity_service.username = 'john'
+
+        expect(teamcity_service).to validate_presence_of(:password)
       end
     end
   end
+
+  describe 'Callbacks' do
+    describe 'before_update :reset_password' do
+      context 'when a password was previously set' do
+        it 'resets password if url changed' do
+          teamcity_service = service
+
+          teamcity_service.teamcity_url = 'http://gitlab1.com'
+          teamcity_service.save
+
+          expect(teamcity_service.password).to be_nil
+        end
+
+        it 'does not reset password if username changed' do
+          teamcity_service = service
+
+          teamcity_service.username = 'some_name'
+          teamcity_service.save
+
+          expect(teamcity_service.password).to eq('password')
+        end
+
+        it "does not reset password if new url is set together with password, even if it's the same password" do
+          teamcity_service = service
+
+          teamcity_service.teamcity_url = 'http://gitlab_edited.com'
+          teamcity_service.password = 'password'
+          teamcity_service.save
+
+          expect(teamcity_service.password).to eq('password')
+          expect(teamcity_service.teamcity_url).to eq('http://gitlab_edited.com')
+        end
+      end
+
+      it 'saves password if new url is set together with password when no password was previously set' do
+        teamcity_service = service
+        teamcity_service.password = nil
+
+        teamcity_service.teamcity_url = 'http://gitlab_edited.com'
+        teamcity_service.password = 'password'
+        teamcity_service.save
+
+        expect(teamcity_service.password).to eq('password')
+        expect(teamcity_service.teamcity_url).to eq('http://gitlab_edited.com')
+      end
+    end
+  end
+
+  describe '#build_page' do
+    it 'returns a specific URL when status is 500' do
+      stub_request(status: 500)
+
+      expect(service.build_page('123', 'unused')).to eq('http://gitlab.com/viewLog.html?buildTypeId=foo')
+    end
+
+    it 'returns a build URL when teamcity_url has no trailing slash' do
+      stub_request(body: %Q({"build":{"id":"666"}}))
+
+      expect(service(teamcity_url: 'http://gitlab.com').build_page('123', 'unused')).to eq('http://gitlab.com/viewLog.html?buildId=666&buildTypeId=foo')
+    end
+
+    it 'returns a build URL when teamcity_url has a trailing slash' do
+      stub_request(body: %Q({"build":{"id":"666"}}))
+
+      expect(service(teamcity_url: 'http://gitlab.com/').build_page('123', 'unused')).to eq('http://gitlab.com/viewLog.html?buildId=666&buildTypeId=foo')
+    end
+  end
+
+  describe '#commit_status' do
+    it 'sets commit status to :error when status is 500' do
+      stub_request(status: 500)
+
+      expect(service.commit_status('123', 'unused')).to eq(:error)
+    end
+
+    it 'sets commit status to "pending" when status is 404' do
+      stub_request(status: 404)
+
+      expect(service.commit_status('123', 'unused')).to eq('pending')
+    end
+
+    it 'sets commit status to "success" when build status contains SUCCESS' do
+      stub_request(build_status: 'YAY SUCCESS!')
+
+      expect(service.commit_status('123', 'unused')).to eq('success')
+    end
+
+    it 'sets commit status to "failed" when build status contains FAILURE' do
+      stub_request(build_status: 'NO FAILURE!')
+
+      expect(service.commit_status('123', 'unused')).to eq('failed')
+    end
+
+    it 'sets commit status to "pending" when build status contains Pending' do
+      stub_request(build_status: 'NO Pending!')
+
+      expect(service.commit_status('123', 'unused')).to eq('pending')
+    end
+
+    it 'sets commit status to :error when build status is unknown' do
+      stub_request(build_status: 'FOO BAR!')
+
+      expect(service.commit_status('123', 'unused')).to eq(:error)
+    end
+  end
+
+  def service(teamcity_url: 'http://gitlab.com')
+    described_class.create(
+      project: build_stubbed(:empty_project),
+      properties: {
+        teamcity_url: teamcity_url,
+        username: 'mic',
+        password: 'password',
+        build_type: 'foo'
+      }
+    )
+  end
+
+  def stub_request(status: 200, body: nil, build_status: 'success')
+    teamcity_full_url = 'http://mic:password@gitlab.com/httpAuth/app/rest/builds/branch:unspecified:any,number:123'
+    body ||= %Q({"build":{"status":"#{build_status}","id":"666"}})
+
+    WebMock.stub_request(:get, teamcity_full_url).to_return(
+      status: status,
+      headers: { 'Content-Type' => 'application/json' },
+      body: body
+    )
+  end
 end
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 4e49c413f23..c3a4016fa49 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -393,6 +393,8 @@ describe Repository, models: true do
   describe '#expire_cache' do
     it 'expires all caches' do
       expect(repository).to receive(:expire_branch_cache)
+      expect(repository).to receive(:expire_branch_count_cache)
+      expect(repository).to receive(:expire_tag_count_cache)
 
       repository.expire_cache
     end