Merge branch...

Merge branch '3233-project-mirror-not-actually-active-when-new-project-is-created-fix' into 'master'

Fixes activation of project mirror when project is created.

Closes #3233

See merge request !2756

app/services/projects/create_service.rb

 module Projects
   class CreateService < BaseService
+    prepend ::EE::Projects::CreateService
+
     def initialize(user, params)
       @current_user, @params = user, params.dup
     end
@@ -47,6 +49,8 @@ module Projects
         @project.namespace_id = current_user.namespace_id
       end
 
+      yield(@project)
+
       @project.creator = current_user
 
       if forked_from_project_id

app/services/projects/update_service.rb

 module Projects
   class UpdateService < BaseService
+    prepend ::EE::Projects::UpdateService
+
     def execute
       # Repository size limit comes as MB from the view
       limit = params.delete(:repository_size_limit)

app/views/shared/_import_form.html.haml

@@ -20,13 +20,4 @@
         Once imported, repositories can be mirrored over SSH. Read more
         = link_to 'here', help_page_path('/workflow/repository_mirroring.md', anchor: 'ssh-authentication')
 
-.form-group
-  = f.label :mirror, class: 'label-light' do
-    = f.check_box :mirror, disabled: true
-    %strong
-      Mirror repository
-  .help-block
-    Automatically update this project's branches and tags from the upstream
-    repository every hour. The Git LFS objects will not be synced.
-
-= f.hidden_field :mirror_user_id, value: current_user.id
+= render 'shared/ee/import_form', f: f

changelogs/unreleased-ee/3233-project-mirror-not-actually-active-when-new-project-is-created-fix.yml

+---
+title: Fixes activation of project mirror when new project is created.
+merge_request: 2756
+author:
+type: fixed

ee/app/controllers/ee/projects_controller.rb

 module EE
   module ProjectsController
     def project_params_attributes
-      attrs = super + project_params_ee
-      attrs += repository_mirrors_params if project&.feature_available?(:repository_mirrors)
-
-      attrs
+      super + project_params_ee
     end
 
     private
@@ -21,11 +18,6 @@ module EE
         repository_size_limit
         reset_approvals_on_push
         service_desk_enabled
-      ]
-    end
-
-    def repository_mirrors_params
-      %i[
         mirror
         mirror_trigger_builds
         mirror_user_id

ee/app/services/ee/projects/create_service.rb

+module EE
+  module Projects
+    module CreateService
+      def execute
+        raise NotImplementedError unless defined?(super)
+
+        mirror = params.delete(:mirror)
+        mirror_user_id = params.delete(:mirror_user_id)
+        mirror_trigger_builds = params.delete(:mirror_trigger_builds)
+
+        super do |project|
+          if mirror && project.feature_available?(:repository_mirrors)
+            project.mirror = mirror
+            project.mirror_user_id = mirror_user_id
+            project.mirror_trigger_builds = mirror_trigger_builds
+          end
+        end
+      end
+    end
+  end
+end

ee/app/services/ee/projects/update_service.rb

+module EE
+  module Projects
+    module UpdateService
+      def execute
+        raise NotImplementedError unless defined?(super)
+
+        unless project.feature_available?(:repository_mirrors)
+          params.delete(:mirror)
+          params.delete(:mirror_user_id)
+          params.delete(:mirror_trigger_builds)
+        end
+
+        super
+      end
+    end
+  end
+end

ee/app/views/shared/ee/_import_form.html.haml

+- if License.feature_available?(:repository_mirrors)
+  .form-group
+    = f.label :mirror, class: 'label-light' do
+      = f.check_box :mirror, disabled: true
+      %strong
+        Mirror repository
+    .help-block
+      Automatically update this project's branches and tags from the upstream
+      repository every hour. The Git LFS objects will not be synced.
+    - if current_application_settings.should_check_namespace_plan?
+      .help-block
+        Mirroring will only be available if the feature is included in the plan of the selected group or user.
+
+
+  = f.hidden_field :mirror_user_id, value: current_user.id

spec/ee/spec/controllers/ee/projects_controller_spec.rb

@@ -9,6 +9,49 @@ describe ProjectsController do
     sign_in(user)
   end
 
+  describe 'POST create' do
+    let!(:params) do
+      {
+        path: 'foo',
+        description: 'bar',
+        import_url: project.http_url_to_repo,
+        namespace_id: user.namespace.id,
+        visibility_level: Gitlab::VisibilityLevel::PUBLIC,
+        mirror: true,
+        mirror_user_id: user.id,
+        mirror_trigger_builds: true
+      }
+    end
+
+    context 'with licensed repository mirrors' do
+      before do
+        stub_licensed_features(repository_mirrors: true)
+      end
+
+      it 'has mirror enabled in new project' do
+        post :create, project: params
+
+        created_project = Project.find_by_path('foo')
+        expect(created_project.reload.mirror).to be true
+        expect(created_project.reload.mirror_user.id).to eq(user.id)
+      end
+    end
+
+    context 'with unlicensed repository mirrors' do
+      before do
+        stub_licensed_features(repository_mirrors: false)
+      end
+
+      it 'has mirror enabled in new project' do
+        post :create, project: params
+
+        created_project = Project.find_by_path('foo')
+        expect(created_project.reload.mirror).to be false
+        expect(created_project.reload.mirror_user).to be nil
+      end
+    end
+  end
+
   describe 'PUT #update' do
     before do
       controller.instance_variable_set(:@project, project)
@@ -115,48 +158,46 @@ describe ProjectsController do
       expect(project.service_desk_enabled).to eq(true)
     end
 
-    context 'repository mirrors licensed' do
-      before do
-        stub_licensed_features(repository_mirrors: true)
-      end
-
-      it 'updates repository mirror attributes' do
-        params = {
+    context 'repository mirrors' do
+      let(:params) do
+        {
           mirror: true,
           mirror_trigger_builds: true,
           mirror_user_id: user.id
         }
+      end
+
+      context 'when licensed' do
+        before do
+          stub_licensed_features(repository_mirrors: true)
+        end
 
-        put :update,
+        it 'updates repository mirror attributes' do
+          put :update,
             namespace_id: project.namespace,
             id: project.id,
             project: params
 
-        params.each do |param, value|
-          expect(project.public_send(param)).to eq(value)
+          params.each do |param, value|
+            expect(project.public_send(param)).to eq(value)
+          end
         end
       end
-    end
-
-    context 'repository mirrors unlicensed' do
-      before do
-        stub_licensed_features(repository_mirrors: false)
-      end
 
-      it 'does not update repository mirror attributes' do
-        params = {
-          mirror: true,
-          mirror_trigger_builds: true,
-          mirror_user_id: user.id
-        }
+      context 'when unlicensed' do
+        before do
+          stub_licensed_features(repository_mirrors: false)
+        end
 
-        params.each do |param, _value|
-          expect do
-            put :update,
+        it 'does not update repository mirror attributes' do
+          params.each do |param, _value|
+            expect do
+              put :update,
                 namespace_id: project.namespace,
                 id: project.id,
                 project: params
-          end.not_to change(project, param)
+            end.not_to change(project, param)
+          end
         end
       end
     end

spec/ee/spec/features/projects/new_project_spec.rb

+require 'spec_helper'
+
+feature 'New project' do
+  let(:user) { create(:admin) }
+
+  before do
+    sign_in(user)
+  end
+
+  context 'repository mirrors' do
+    context 'when licensed' do
+      before do
+        stub_licensed_features(repository_mirrors: true)
+      end
+
+      it 'shows mirror repository checkbox enabled', :js do
+        visit new_project_path
+        first('.import_git').click
+
+        expect(page).to have_unchecked_field('Mirror repository', disabled: false)
+      end
+    end
+
+    context 'when unlicensed' do
+      before do
+        stub_licensed_features(repository_mirrors: false)
+      end
+
+      it 'does not show mirror repository option' do
+        visit new_project_path
+        first('.import_git').click
+
+        expect(page).not_to have_content('Mirror repository')
+      end
+    end
+  end
+end

spec/ee/spec/services/ee/projects/create_service_spec.rb

@@ -33,6 +33,45 @@ describe Projects::CreateService, '#execute' do
     end
   end
 
+  context 'repository mirror' do
+    before do
+      opts.merge!(import_url: 'http://foo.com',
+                  mirror: true,
+                  mirror_user_id: user.id,
+                  mirror_trigger_builds: true)
+    end
+
+    context 'when licensed' do
+      before do
+        stub_licensed_features(repository_mirrors: true)
+      end
+
+      it 'sets the correct attributes' do
+        project = create_project(user, opts)
+
+        expect(project).to be_valid
+        expect(project.mirror).to be true
+        expect(project.mirror_user_id).to eq(user.id)
+        expect(project.mirror_trigger_builds).to be true
+      end
+    end
+
+    context 'when unlicensed' do
+      before do
+        stub_licensed_features(repository_mirrors: false)
+      end
+
+      it 'does not set mirror attributes' do
+        project = create_project(user, opts)
+
+        expect(project).to be_valid
+        expect(project.mirror).to be false
+        expect(project.mirror_user_id).to be_nil
+        expect(project.mirror_trigger_builds).to be false
+      end
+    end
+  end
+
   context 'git hook sample' do
     let!(:sample) { create(:push_rule_sample) }
 

spec/ee/spec/services/ee/projects/update_service_spec.rb

+require 'spec_helper'
+
+describe Projects::UpdateService, '#execute' do
+  let(:user) { create(:user) }
+  let(:project) { create(:project, creator: user, namespace: user.namespace) }
+
+  context 'repository mirror' do
+    let!(:opts) do
+      {
+        import_url: 'http://foo.com',
+        mirror: true,
+        mirror_user_id: user.id,
+        mirror_trigger_builds: true
+      }
+    end
+
+    context 'when licensed' do
+      before do
+        stub_licensed_features(repository_mirrors: true)
+      end
+
+      it 'updates the correct attributes' do
+        update_project(project, user, opts)
+
+        updated_project = project.reload
+        expect(updated_project).to be_valid
+        expect(updated_project.mirror).to be true
+        expect(updated_project.mirror_user_id).to eq(user.id)
+        expect(updated_project.mirror_trigger_builds).to be true
+      end
+    end
+
+    context 'when unlicensed' do
+      before do
+        stub_licensed_features(repository_mirrors: false)
+      end
+
+      it 'does not update mirror attributes' do
+        update_project(project, user, opts)
+
+        updated_project = project.reload
+        expect(updated_project).to be_valid
+        expect(updated_project.mirror).to be false
+        expect(updated_project.mirror_user_id).to be_nil
+        expect(updated_project.mirror_trigger_builds).to be false
+      end
+    end
+  end
+
+  def update_project(project, user, opts)
+    Projects::UpdateService.new(project, user, opts).execute
+  end
+end

spec/features/projects/new_project_spec.rb

@@ -125,10 +125,6 @@ feature 'New project' do
         expect(git_import_instructions).to be_visible
         expect(git_import_instructions).to have_content 'Git repository URL'
       end
-
-      it 'shows mirror repository checkbox enabled', :js do
-        expect(page).to have_unchecked_field('Mirror repository', disabled: false)
-      end
     end
 
     context 'from GitHub' do