Merge branch 'ensure-ci-project' into 'master'

Ensure GitLab CI project exists when CI service is activated manually

When I check activeated checkbox in project services for GitLab CI it
cause half-working state when gitlab_ci_project is missing. This patch
fixes it until we have proper behaviour implemented later

This fix also bring us to the point when fork of project is a bit broken and have unnecessary code so I made cleanup. 
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

cc @ayufan 

See merge request !1491

app/models/project.rb

@@ -751,13 +751,9 @@ class Project < ActiveRecord::Base
     gitlab_ci_project || create_gitlab_ci_project
   end
 
-  def enable_ci(user)
-    # Enable service
+  def enable_ci
     service = gitlab_ci_service || create_gitlab_ci_service
     service.active = true
     service.save
-
-    # Create Ci::Project
-    Ci::CreateProjectService.new.execute(user, self)
   end
 end

app/models/project_services/gitlab_ci_service.rb

@@ -22,12 +22,17 @@ class GitlabCiService < CiService
   include Gitlab::Application.routes.url_helpers
 
   after_save :compose_service_hook, if: :activated?
+  after_save :ensure_gitlab_ci_project, if: :activated?
 
   def compose_service_hook
     hook = service_hook || build_service_hook
     hook.save
   end
 
+  def ensure_gitlab_ci_project
+    project.ensure_gitlab_ci_project
+  end
+
   def supported_events
     %w(push tag_push)
   end
@@ -67,21 +72,6 @@ class GitlabCiService < CiService
     :error
   end
 
-  def fork_registration(new_project, current_user)
-    params = OpenStruct.new({
-      id:                  new_project.id,
-      default_branch:      new_project.default_branch
-    })
-
-    ci_project = Ci::Project.find_by!(gitlab_id: project.id)
-
-    Ci::CreateProjectService.new.execute(
-      current_user,
-      params,
-      ci_project
-    )
-  end
-
   def commit_coverage(sha, ref)
     get_ci_commit(sha, ref).coverage
   rescue ActiveRecord::RecordNotFound

app/services/ci/create_project_service.rb

-module Ci
-  class CreateProjectService
-    include Gitlab::Application.routes.url_helpers
-
-    def execute(current_user, params, forked_project = nil)
-      @project = Ci::Project.parse(params)
-
-      Ci::Project.transaction do
-        @project.save!
-
-        gl_project = ::Project.find(@project.gitlab_id)
-        gl_project.build_missing_services
-        gl_project.gitlab_ci_service.update_attributes(active: true)
-      end
-
-      if forked_project
-        # Copy settings
-        settings = forked_project.attributes.select do |attr_name, value|
-          ["public", "shared_runners_enabled", "allow_git_fetch"].include? attr_name
-        end
-
-        @project.update(settings)
-      end
-
-      Ci::EventService.new.create_project(current_user, @project)
-
-      @project
-    end
-  end
-end

app/services/git_push_service.rb

@@ -58,7 +58,7 @@ class GitPushService
     # If CI was disabled but .gitlab-ci.yml file was pushed
     # we enable CI automatically
     if !project.gitlab_ci? && gitlab_ci_yaml?(newrev)
-      project.enable_ci(user)
+      project.enable_ci
     end
 
     EventCreateService.new.push(project, user, @push_data)

app/services/projects/fork_service.rb

@@ -18,7 +18,13 @@ module Projects
 
       if new_project.persisted?
         if @project.gitlab_ci?
-          @project.gitlab_ci_service.fork_registration(new_project, @current_user)
+          new_project.enable_ci
+
+          settings = @project.gitlab_ci_project.attributes.select do |attr_name, value|
+            ["public", "shared_runners_enabled", "allow_git_fetch"].include? attr_name
+          end
+
+          new_project.gitlab_ci_project.update(settings)
         end
       end
 

features/steps/project/commits/commits.rb

@@ -103,7 +103,7 @@ class Spinach::Features::ProjectCommits < Spinach::FeatureSteps
   end
 
   step 'commit has ci status' do
-    @project.enable_ci(@user)
+    @project.enable_ci
     create :ci_commit, gl_project: @project, sha: sample_commit.id
   end
 

features/steps/shared/project.rb

@@ -199,7 +199,7 @@ module SharedProject
 
   step 'project "Shop" has CI enabled' do
     project = Project.find_by(name: "Shop")
-    project.enable_ci(@user)
+    project.enable_ci
   end
 
   step 'project "Shop" has CI build' do

spec/models/project_services/gitlab_ci_service_spec.rb

@@ -56,25 +56,4 @@ describe GitlabCiService do
       end
     end
   end
-
-  describe "Fork registration" do
-    before do
-      @old_project = create(:ci_project).gl_project
-      @project = create(:empty_project)
-      @user = create(:user)
-
-      @service = GitlabCiService.new
-      allow(@service).to receive_messages(
-        service_hook: true,
-        project_url: 'http://ci.gitlab.org/projects/2',
-        token: 'verySecret',
-        project: @old_project
-      )
-    end
-
-    it "creates fork on CI" do
-      expect_any_instance_of(Ci::CreateProjectService).to receive(:execute)
-      @service.fork_registration(@project, @user)
-    end
-  end
 end

spec/models/project_spec.rb

@@ -417,9 +417,8 @@ describe Project do
 
   describe :enable_ci do
     let(:project) { create :project }
-    let(:user) { create :user }
 
-    before { project.enable_ci(user) }
+    before { project.enable_ci }
 
     it { expect(project.gitlab_ci?).to be_truthy }
     it { expect(project.gitlab_ci_project).to be_a(Ci::Project) }

spec/services/ci/create_project_service_spec.rb

-require 'spec_helper'
-
-describe Ci::CreateProjectService do
-  let(:service) { Ci::CreateProjectService.new }
-  let(:current_user) { double.as_null_object }
-  let(:project) { FactoryGirl.create :project }
-
-  describe :execute do
-    context 'valid params' do
-      subject { service.execute(current_user, project) }
-
-      it { is_expected.to be_kind_of(Ci::Project) }
-      it { is_expected.to be_persisted }
-    end
-
-    context 'without project dump' do
-      it 'should raise exception' do
-        expect { service.execute(current_user, '', '') }.
-          to raise_error(NoMethodError)
-      end
-    end
-
-    context "forking" do
-      let(:ci_origin_project) do
-        FactoryGirl.create(:ci_project, shared_runners_enabled: true, public: true, allow_git_fetch: true)
-      end
-
-      subject { service.execute(current_user, project, ci_origin_project) }
-
-      it "uses project as a template for settings and jobs" do
-        expect(subject.shared_runners_enabled).to be_truthy
-        expect(subject.public).to be_truthy
-        expect(subject.allow_git_fetch).to be_truthy
-      end
-    end
-  end
-end

spec/services/projects/fork_service_spec.rb

@@ -43,14 +43,10 @@ describe Projects::ForkService do
     end
 
     context 'GitLab CI is enabled' do
-      it "calls fork registrator for CI" do
-        create(:ci_project, gl_project: @from_project)
-        @from_project.build_missing_services
-        @from_project.gitlab_ci_service.update_attributes(active: true)
-
-        expect_any_instance_of(Ci::CreateProjectService).to receive(:execute)
-
-        fork_project(@from_project, @to_user)
+      it "fork and enable CI for fork" do
+        @from_project.enable_ci
+        @to_project = fork_project(@from_project, @to_user)
+        expect(@to_project.gitlab_ci?).to be_truthy
       end
     end
   end