Merge branch 'fix/commit-status-artifacts' into 'master'

Use generic method to checks if artifacts are available

Closes #12626

See merge request !2576

CHANGELOG

@@ -10,6 +10,9 @@ v 8.4.2 (unreleased)
     track them in Performance Monitoring.
   - Increase contrast between highlighted code comments and inline diff marker.
 
+v 8.4.2 (unreleased)
+  - Fix method undefined when using external commit status in builds
+
 v 8.4.1
   - Apply security updates for Rails (4.2.5.1), rails-html-sanitizer (1.0.3),
     and Nokogiri (1.6.7.2)

app/views/projects/commit_statuses/_commit_status.html.haml

@@ -66,7 +66,7 @@
 
   %td
     .pull-right
-      - if current_user && can?(current_user, :read_build_artifacts, commit_status.project) && commit_status.artifacts?
+      - if current_user && can?(current_user, :read_build_artifacts, commit_status.project) && commit_status.artifacts_download_url
         = link_to commit_status.artifacts_download_url, title: 'Download artifacts' do
           %i.fa.fa-download
       - if current_user && can?(current_user, :manage_builds, commit_status.project)

spec/factories/commit_statuses.rb

 FactoryGirl.define do
   factory :commit_status, class: CommitStatus do
-    started_at 'Di 29. Okt 09:51:28 CET 2013'
-    finished_at 'Di 29. Okt 09:53:28 CET 2013'
     name 'default'
     status 'success'
     description 'commit status'
     commit factory: :ci_commit_with_one_job
+    started_at 'Tue, 26 Jan 2016 08:21:42 +0100'
+    finished_at 'Tue, 26 Jan 2016 08:23:42 +0100'
+
+    after(:build) do |build, evaluator|
+      build.project = build.commit.project
+    end
 
     factory :generic_commit_status, class: GenericCommitStatus do
       name 'generic'

spec/features/commits_spec.rb

@@ -16,83 +16,104 @@ describe 'Commits' do
       FactoryGirl.create :ci_commit, project: project, sha: project.commit.sha
     end
 
-    let!(:build) { FactoryGirl.create :ci_build, commit: commit }
+    context 'commit status is Generic Commit Status' do
+      let!(:status) { FactoryGirl.create :generic_commit_status, commit: commit }
 
-    describe 'Project commits' do
-      before do
-        visit namespace_project_commits_path(project.namespace, project, :master)
-      end
+      describe 'Commit builds' do
+        before do
+          visit ci_status_path(commit)
+        end
 
-      it 'should show build status' do
-        page.within("//li[@id='commit-#{commit.short_sha}']") do
-          expect(page).to have_css(".ci-status-link")
+        it { expect(page).to have_content commit.sha[0..7] }
+
+        it 'contains generic commit status build' do
+          page.within('.table-holder') do
+            expect(page).to have_content "##{status.id}" # build id
+            expect(page).to have_content 'generic'       # build name
+          end
         end
       end
     end
 
-    describe 'Commit builds' do
-      before do
-        visit ci_status_path(commit)
-      end
-
-      it { expect(page).to have_content commit.sha[0..7] }
-      it { expect(page).to have_content commit.git_commit_message }
-      it { expect(page).to have_content commit.git_author_name }
-    end
+    context 'commit status is Ci Build' do
+      let!(:build) { FactoryGirl.create :ci_build, commit: commit }
 
-    context 'Download artifacts' do
-      let(:artifacts_file) { fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') }
+      describe 'Project commits' do
+        before do
+          visit namespace_project_commits_path(project.namespace, project, :master)
+        end
 
-      before do
-        build.update_attributes(artifacts_file: artifacts_file)
+        it 'should show build status' do
+          page.within("//li[@id='commit-#{commit.short_sha}']") do
+            expect(page).to have_css(".ci-status-link")
+          end
+        end
       end
 
-      it do
-        visit ci_status_path(commit)
-        click_on 'Download artifacts'
-        expect(page.response_headers['Content-Type']).to eq(artifacts_file.content_type)
-      end
-    end
+      describe 'Commit builds' do
+        before do
+          visit ci_status_path(commit)
+        end
 
-    describe 'Cancel all builds' do
-      it 'cancels commit' do
-        visit ci_status_path(commit)
-        click_on 'Cancel running'
-        expect(page).to have_content 'canceled'
+        it { expect(page).to have_content commit.sha[0..7] }
+        it { expect(page).to have_content commit.git_commit_message }
+        it { expect(page).to have_content commit.git_author_name }
       end
-    end
 
-    describe 'Cancel build' do
-      it 'cancels build' do
-        visit ci_status_path(commit)
-        click_on 'Cancel'
-        expect(page).to have_content 'canceled'
-      end
-    end
+      context 'Download artifacts' do
+        let(:artifacts_file) { fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') }
+
+        before do
+          build.update_attributes(artifacts_file: artifacts_file)
+        end
 
-    describe '.gitlab-ci.yml not found warning' do
-      context 'ci builds enabled' do
-        it "does not show warning" do
+        it do
           visit ci_status_path(commit)
-          expect(page).not_to have_content '.gitlab-ci.yml not found in this commit'
+          click_on 'Download artifacts'
+          expect(page.response_headers['Content-Type']).to eq(artifacts_file.content_type)
         end
+      end
 
-        it 'shows warning' do
-          stub_ci_commit_yaml_file(nil)
+      describe 'Cancel all builds' do
+        it 'cancels commit' do
           visit ci_status_path(commit)
-          expect(page).to have_content '.gitlab-ci.yml not found in this commit'
+          click_on 'Cancel running'
+          expect(page).to have_content 'canceled'
         end
       end
 
-      context 'ci builds disabled' do
-        before do
-          stub_ci_builds_disabled
-          stub_ci_commit_yaml_file(nil)
+      describe 'Cancel build' do
+        it 'cancels build' do
           visit ci_status_path(commit)
+          click_on 'Cancel'
+          expect(page).to have_content 'canceled'
         end
+      end
+
+      describe '.gitlab-ci.yml not found warning' do
+        context 'ci builds enabled' do
+          it "does not show warning" do
+            visit ci_status_path(commit)
+            expect(page).not_to have_content '.gitlab-ci.yml not found in this commit'
+          end
+
+          it 'shows warning' do
+            stub_ci_commit_yaml_file(nil)
+            visit ci_status_path(commit)
+            expect(page).to have_content '.gitlab-ci.yml not found in this commit'
+          end
+        end
+
+        context 'ci builds disabled' do
+          before do
+            stub_ci_builds_disabled
+            stub_ci_commit_yaml_file(nil)
+            visit ci_status_path(commit)
+          end
 
-        it 'does not show warning' do
-          expect(page).not_to have_content '.gitlab-ci.yml not found in this commit'
+          it 'does not show warning' do
+            expect(page).not_to have_content '.gitlab-ci.yml not found in this commit'
+          end
         end
       end
     end