Incorporate another round of feedback

app/models/ci/pipeline.rb

@@ -351,9 +351,9 @@ module Ci
 
       @ci_yaml_file =
         case config_source
-        when :repository_source, :unknown_source
+        when "repository_source", "unknown_source"
           ci_yaml_from_repo
-        when :auto_devops_source
+        when "auto_devops_source"
           implied_ci_yaml_file
         end
 

app/models/project.rb

@@ -469,7 +469,7 @@ class Project < ActiveRecord::Base
   end
 
   def auto_devops_enabled?
-    if auto_devops && !auto_devops.enabled.nil?
+    if auto_devops && auto_devops.enabled.present?
       auto_devops.enabled?
     else
       current_application_settings.auto_devops_enabled?

app/models/project_auto_devops.rb

@@ -1,5 +1,5 @@
 class ProjectAutoDevops < ActiveRecord::Base
   belongs_to :project
 
-  validates :domain, presence: true
+  validates :domain, presence: true, if: :enabled?
 end

db/migrate/20170828093725_create_project_auto_dev_ops.rb

@@ -9,7 +9,7 @@ class CreateProjectAutoDevOps < ActiveRecord::Migration
     create_table :project_auto_devops do |t|
       t.belongs_to :project, null: false, index: { unique: true }
       t.boolean :enabled, default: nil, null: true
-      t.string :domain, null: false
+      t.string :domain
     end
 
     add_timestamps_with_timezone(:project_auto_devops, null: false)

db/schema.rb

@@ -1125,7 +1125,7 @@ ActiveRecord::Schema.define(version: 20170831092813) do
   create_table "project_auto_devops", force: :cascade do |t|
     t.integer "project_id", null: false
     t.boolean "enabled"
-    t.string "domain", null: false
+    t.string "domain"
     t.datetime_with_timezone "created_at", null: false
     t.datetime_with_timezone "updated_at", null: false
   end

spec/models/ci/pipeline_spec.rb

@@ -783,10 +783,44 @@ describe Ci::Pipeline, :mailer do
     end
   end
 
+  describe '#detect_ci_yaml_file' do
+    context 'when the repo has a config file' do
+      it 'returns that configuration' do
+        allow(pipeline.project.repository).to receive(:gitlab_ci_yml_for)
+          .and_return('config')
+
+        expect(pipeline.detect_ci_yaml_file).to be_a(String)
+        expect(pipeline.repository_source?).to be(true)
+      end
+    end
+
+    context 'when the repo does not have a config file' do
+      let(:implied_yml) { Gitlab::Template::GitlabCiYmlTemplate.find('Auto-DevOps').content }
+
+      context 'auto devops enabled' do
+        before do
+          allow_any_instance_of(ApplicationSetting)
+            .to receive(:auto_devops_enabled?) { true }
+        end
+
+        it 'returns the implied ci file' do
+          allow(pipeline.project).to receive(:ci_config_path) { 'custom' }
+
+          expect(pipeline.detect_ci_yaml_file).to eq(implied_yml)
+          expect(pipeline.auto_devops_source?).to be(true)
+        end
+      end
+    end
+  end
+
   describe '#ci_yaml_file' do
     let(:implied_yml) { Gitlab::Template::GitlabCiYmlTemplate.find('Auto-DevOps').content }
 
-    context 'when AutoDevops is enabled' do
+    before { pipeline.detect_ci_yaml_file }
+
+    context 'the source is unknown' do
+      before { pipeline.unknown_source! }
+
       it 'returns the configuration if found' do
         allow(pipeline.project.repository).to receive(:gitlab_ci_yml_for)
           .and_return('config')
@@ -794,49 +828,37 @@ describe Ci::Pipeline, :mailer do
         expect(pipeline.ci_yaml_file).to be_a(String)
         expect(pipeline.ci_yaml_file).not_to eq(implied_yml)
         expect(pipeline.yaml_errors).to be_nil
-        expect(pipeline.repository?).to be(true)
       end
 
-      context 'when the implied configuration will be used' do
-        before do
-          allow_any_instance_of(ApplicationSetting)
-            .to receive(:auto_devops_enabled?) { true }
-        end
-
-        it 'returns the implied configuration when its not found' do
-          allow(pipeline.project).to receive(:ci_config_path) { 'custom' }
-
-          expect(pipeline.ci_yaml_file).to eq(implied_yml)
-        end
-
-        it 'sets the config source' do
-          allow(pipeline.project).to receive(:ci_config_path) { 'custom' }
-
-          expect(pipeline.ci_yaml_file).to eq(implied_yml)
-          expect(pipeline.auto_devops?).to be(true)
-        end
+      it 'sets yaml errors if not found' do
+        expect(pipeline.ci_yaml_file).to be_nil
+        expect(pipeline.yaml_errors)
+            .to start_with('Failed to load CI/CD config file')
       end
     end
 
-    context 'when AudoDevOps is disabled' do
-      context 'when an invalid path is given' do
-        it 'sets the yaml errors' do
-          allow(pipeline.project).to receive(:ci_config_path) { 'custom' }
+    context 'the source is the repository' do
+      before { pipeline.repository_source! }
 
-          expect(pipeline.ci_yaml_file).to be_nil
-          expect(pipeline.yaml_errors)
-            .to start_with('Failed to load CI/CD config file')
-        end
+      it 'returns the configuration if found' do
+        allow(pipeline.project.repository).to receive(:gitlab_ci_yml_for)
+          .and_return('config')
+
+        expect(pipeline.ci_yaml_file).to be_a(String)
+        expect(pipeline.ci_yaml_file).not_to eq(implied_yml)
+        expect(pipeline.yaml_errors).to be_nil
       end
+    end
 
-      context 'when the config file can be found' do
-        it 'has no yaml_errors' do
-          allow(pipeline.project.repository).to receive(:gitlab_ci_yml_for)
-            .and_return('config')
+    context 'when the source is auto_devops_source' do
+      before { pipeline.auto_devops_source! }
 
-          expect(pipeline.ci_yaml_file).to eq('config')
-          expect(pipeline.yaml_errors).to be_nil
-        end
+      it 'finds the implied config' do
+        allow_any_instance_of(ApplicationSetting)
+          .to receive(:auto_devops_enabled?) { true }
+
+        expect(pipeline.ci_yaml_file).to eq(implied_yml)
+        expect(pipeline.yaml_errors).to be_nil
       end
     end
   end