Merge branch 'ux/suppress-ci-yml-warning' into 'master'

Suppress warning about missing `.gitlab-ci.yml` if builds are disabled

When user disables GitLab Ci Service in project's settings then warning
about missing `.gitlab-ci.yml` file should be supressed. This a matter
of user experience as stated in #3761 (closes #3761).

cc @ayufan 

See merge request !2014

CHANGELOG

@@ -51,6 +51,9 @@ v 8.3.0 (unreleased)
   - Display referenced merge request statuses in the issue description (Greg Smethells)
   - Implement new sidebar for issue and merge request pages
   - Emoji picker improvements
+  - Suppress warning about missing `.gitlab-ci.yml` if builds are disabled
+  - Do not show build status unless builds are enabled and `.gitlab-ci.yml` is present
+  - Persist runners registration token in database
 
 v 8.2.3
   - Fix application settings cache not expiring after changes (Stan Hu)

app/helpers/ci_status_helper.rb

@@ -57,7 +57,7 @@ module CiStatusHelper
 
   def render_ci_status(ci_commit)
     link_to ci_status_path(ci_commit),
-      class: "c#{ci_status_color(ci_commit)}",
+      class: "ci-status-link c#{ci_status_color(ci_commit)}",
       title: "Build #{ci_status_label(ci_commit)}",
       data: { toggle: 'tooltip', placement: 'left' } do
       ci_status_icon(ci_commit)

app/models/ci/commit.rb

@@ -218,6 +218,16 @@ module Ci
       update!(committed_at: DateTime.now)
     end
 
+    ##
+    # This method checks if build status should be displayed.
+    #
+    # Build status should be available only if builds are enabled
+    # on project level and `.gitlab-ci.yml` file is present.
+    #
+    def show_build_status?
+      project.builds_enabled? && ci_yaml_file
+    end
+
     private
 
     def save_yaml_error(error)

app/views/projects/commit/_commit_box.html.haml

@@ -40,7 +40,7 @@
   - @commit.parents.each do |parent|
     = link_to parent.short_id, namespace_project_commit_path(@project.namespace, @project, parent), class: "monospace"
 
-- if @ci_commit
+- if @ci_commit && @ci_commit.show_build_status?
   .pull-right
     = link_to ci_status_path(@ci_commit), class: "ci-status ci-#{@ci_commit.status}" do
       = ci_status_icon(@ci_commit)

app/views/projects/commits/_commit.html.haml

@@ -9,7 +9,7 @@
 - cache_key.push(ci_commit.status) if ci_commit
 
 = cache(cache_key) do
-  %li.commit.js-toggle-container
+  %li.commit.js-toggle-container{ id: "commit-#{commit.short_id}" }
     .commit-row-title
       %strong.str-truncated
         = link_to_gfm commit.title, namespace_project_commit_path(project.namespace, project, commit.id), class: "commit-row-message"
@@ -17,7 +17,7 @@
           %a.text-expander.js-toggle-button ...
 
       .pull-right
-        - if ci_commit
+        - if ci_commit && ci_commit.show_build_status?
           = render_ci_status(ci_commit)
            
         = clipboard_button(clipboard_text: commit.id)

features/project/commits/commits.feature

@@ -19,7 +19,8 @@ Feature: Project Commits
 
   Scenario: I browse commit with ci from list
     Given commit has ci status
-    And I click on commit link
+    And repository contains ".gitlab-ci.yml" file
+    When I click on commit link
     Then I see commit ci info
     And I click status link
     Then I see builds list

features/steps/project/commits/commits.rb

@@ -108,6 +108,10 @@ class Spinach::Features::ProjectCommits < Spinach::FeatureSteps
     create :ci_build, commit: ci_commit
   end
 
+  step 'repository contains ".gitlab-ci.yml" file' do
+    allow_any_instance_of(Ci::Commit).to receive(:ci_yaml_file).and_return(String.new)
+  end
+
   step 'I see commit ci info' do
     expect(page).to have_content "build: pending"
   end

spec/features/commits_spec.rb

@@ -1,73 +1,116 @@
 require 'spec_helper'
 
-describe "Commits" do
+describe 'Commits' do
   include CiStatusHelper
 
   let(:project) { create(:project) }
 
-  describe "CI" do
+  describe 'CI' do
     before do
       login_as :user
       project.team << [@user, :master]
-      @commit = FactoryGirl.create :ci_commit, project: project, sha: project.commit.sha
-      @build = FactoryGirl.create :ci_build, commit: @commit
-      @generic_status = FactoryGirl.create :generic_commit_status, commit: @commit
-    end
-
-    before do
       stub_ci_commit_to_return_yaml_file
     end
 
-    describe "GET /:project/commits/:sha/ci" 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 }
+    let!(:commit) do
+      FactoryGirl.create :ci_commit, project: project, sha: project.commit.sha
     end
 
-    context "Download artifacts" do
+    let!(:build) { FactoryGirl.create :ci_build, commit: commit }
+
+    describe 'Project commits' do
+      context 'builds enabled' do
+        context '.gitlab-ci.yml found' do
+          before do
+            visit namespace_project_commits_path(project.namespace, project, :master)
+          end
+
+          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
+
+        context 'no .gitlab-ci.yml found' do
+          before do
+            stub_ci_commit_yaml_file(nil)
+            visit namespace_project_commits_path(project.namespace, project, :master)
+          end
+
+          it 'should not show build status' do
+            page.within("//li[@id='commit-#{commit.short_sha}']") do
+              expect(page).to have_no_css(".ci-status-link")
+            end
+          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 '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)
+        build.update_attributes(artifacts_file: artifacts_file)
       end
 
       it do
-        visit ci_status_path(@commit)
-        click_on "Download artifacts"
+        visit ci_status_path(commit)
+        click_on 'Download artifacts'
         expect(page.response_headers['Content-Type']).to eq(artifacts_file.content_type)
       end
     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"
+    describe 'Cancel all builds' do
+      it 'cancels commit' do
+        visit ci_status_path(commit)
+        click_on 'Cancel running'
+        expect(page).to have_content 'canceled'
       end
     end
 
-    describe "Cancel build" do
-      it "cancels build" do
-        visit ci_status_path(@commit)
-        click_on "Cancel"
-        expect(page).to have_content "canceled"
+    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
-      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"
+    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
 
-      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"
+      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'
+        end
       end
     end
   end

spec/support/stub_gitlab_calls.rb

@@ -21,6 +21,10 @@ module StubGitlabCalls
     allow_any_instance_of(Ci::Commit).to receive(:ci_yaml_file) { ci_yaml }
   end
 
+  def stub_ci_builds_disabled
+    allow_any_instance_of(Project).to receive(:builds_enabled?).and_return(false)
+  end
+
   private
 
   def gitlab_url