Fix validations related to mirroring settings form.

Some of our users are changing to GitLab as their main repository, it
means that they're disabling the local mirror and setting up a remote
mirror instead. This fix helps to avoid the extra steps required in order
to achieve the desired configuration.

CHANGELOG-EE

@@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.13.0 (unreleased)
   - Fix 500 error updating mirror URLs for projects
+  - Fix validations related to mirroring settings form. !773
 
 v 8.12.4 (unreleased)
   - [ES] Indexer works with smaller batches of repositories to not exceed NOFILE limit

app/models/project.rb

@@ -139,7 +139,7 @@ class Project < ActiveRecord::Base
   has_many :runners, through: :runner_projects, source: :runner, class_name: 'Ci::Runner'
   has_many :variables, dependent: :destroy, class_name: 'Ci::Variable', foreign_key: :gl_project_id
   has_many :triggers, dependent: :destroy, class_name: 'Ci::Trigger', foreign_key: :gl_project_id
-  has_many :remote_mirrors, dependent: :destroy
+  has_many :remote_mirrors, inverse_of: :project, dependent: :destroy
   has_many :environments, dependent: :destroy
   has_many :deployments, dependent: :destroy
   has_many :path_locks, dependent: :destroy

app/models/remote_mirror.rb

@@ -9,10 +9,10 @@ class RemoteMirror < ActiveRecord::Base
                  insecure_mode: true,
                  algorithm: 'aes-256-cbc'
 
-  belongs_to :project
+  belongs_to :project, inverse_of: :remote_mirrors
 
   validates :url, presence: true, url: { protocols: %w(ssh git http https), allow_blank: true }
-  validate  :url_availability, if: :url_changed?
+  validate  :url_availability, if: -> (mirror) { mirror.url_changed? || mirror.enabled? }
 
   after_save :refresh_remote, if: :mirror_url_changed?
   after_update :reset_fields, if: :mirror_url_changed?
@@ -109,7 +109,7 @@ class RemoteMirror < ActiveRecord::Base
   private
 
   def url_availability
-    if project.import_url == url
+    if project.import_url == url && project.mirror?
       errors.add(:url, 'is already in use')
     end
   end

app/views/projects/mirrors/show.html.haml

 - page_title "Mirror Repository"
 
+.row
+  = form_errors(@project)
 .row.prepend-top-default.append-bottom-default
-  .col-lg-3
-    %h4.prepend-top-0
-      Pull from a remote repository
-    %p.light
-      Set up your project to automatically have its branches, tags, and commits updated from an upstream repository every hour.
-  .col-lg-9
-    %h5.prepend-top-0
-      Set up mirror repository
-    = form_for @project, url: namespace_project_mirror_path(@project.namespace, @project) do |f|
-      = form_errors(@project)
+  = form_for @project, url: namespace_project_mirror_path(@project.namespace, @project) do |f|
+    .col-lg-3
+      %h4.prepend-top-0
+        Pull from a remote repository
+      %p.light
+        Set up your project to automatically have its branches, tags, and commits updated from an upstream repository every hour.
+    .col-lg-9
+      %h5.prepend-top-0
+        Set up mirror repository
       = render "shared/mirror_update_button"
       - if @project.mirror_last_update_failed?
         .panel.panel-danger
@@ -42,17 +43,14 @@
           They need to have at least master access to this project.
       - if @project.builds_enabled?
         = render "shared/mirror_trigger_builds_setting", f: f
-      = f.submit "Update", class: "btn btn-save"
-  .col-sm-12
-    %hr
-  .col-lg-3
-    %h4.prepend-top-0
-      Push to a remote repository
-    %p.light
-      Set up the remote repository that you want to update with the content of the current repository every hour.
-  .col-lg-9
-    = form_for @project, url: namespace_project_mirror_path(@project.namespace, @project) do |f|
-      = form_errors(@project)
+    .col-sm-12
+      %hr
+    .col-lg-3
+      %h4.prepend-top-0
+        Push to a remote repository
+      %p.light
+        Set up the remote repository that you want to update with the content of the current repository every hour.
+    .col-lg-9
       = render "shared/remote_mirror_update_button", remote_mirror: @remote_mirror
       - if @remote_mirror.last_error.present?
         .panel.panel-danger
@@ -75,4 +73,6 @@
           = rm_form.label :url, "Git repository URL", class: "label-light"
           = rm_form.text_field :url, class: "form-control", placeholder: 'https://username:password@gitlab.company.com/group/project.git'
           = render "instructions"
-      = f.submit "Update", class: "btn btn-create"
+    .col-sm-12.text-center
+      %hr
+      = f.submit 'Save changes', class: 'btn btn-create', name: 'update_remote_mirror'

spec/controllers/projects/mirrors_controller_spec.rb

+require 'spec_helper'
+
+describe Projects::MirrorsController do
+  describe 'setting up a remote mirror' do
+    context 'when the current project is a mirror' do
+      before do
+        @project = create(:project, :mirror)
+        sign_in(@project.owner)
+      end
+
+      it 'allows to create a remote mirror' do
+        expect do
+          do_put(@project, remote_mirrors_attributes: { '0' => { 'enabled' => 1, 'url' => 'http://foo.com' } })
+        end.to change { RemoteMirror.count }.to(1)
+      end
+
+      context 'when remote mirror has the same URL' do
+        it 'does not allow to create the remote mirror' do
+          expect do
+            do_put(@project, remote_mirrors_attributes: { '0' => { 'enabled' => 1, 'url' => @project.import_url } })
+          end.not_to change { RemoteMirror.count }
+        end
+
+        context 'with disabled local mirror' do
+          it 'allows to create a remote mirror' do
+            expect do
+              do_put(@project, mirror: 0, remote_mirrors_attributes: { '0' => { 'enabled' => 1, 'url' => @project.import_url } })
+            end.to change { RemoteMirror.count }.to(1)
+          end
+        end
+      end
+    end
+
+    context 'when the current project is not a mirror' do
+      it 'allows to create a remote mirror' do
+        project = create(:project)
+        sign_in(project.owner)
+
+        expect do
+          do_put(project, remote_mirrors_attributes: { '0' => { 'enabled' => 1, 'url' => 'http://foo.com' } })
+        end.to change { RemoteMirror.count }.to(1)
+      end
+    end
+
+    context 'when the current project has a remote mirror' do
+      before do
+        @project = create(:project)
+        @remote_mirror = @project.remote_mirrors.create!(enabled: 1, url: 'http://local.dev')
+        sign_in(@project.owner)
+      end
+
+      context 'when trying to create a mirror with the same URL' do
+        it 'should not setup the mirror' do
+          do_put(@project, mirror: true, import_url: @remote_mirror.url)
+
+          expect(@project.reload.mirror).to be_falsey
+          expect(@project.reload.import_url).to be_blank
+        end
+      end
+
+      context 'when trying to create a mirror with a different URL' do
+        it 'should setup the mirror' do
+          do_put(@project, mirror: true, mirror_user_id: @project.owner.id, import_url: 'http://test.com')
+
+          expect(@project.reload.mirror).to eq(true)
+          expect(@project.reload.import_url).to eq('http://test.com')
+        end
+      end
+    end
+  end
+
+  def do_put(project, options)
+    attrs = { namespace_id: project.namespace.to_param, project_id: project.to_param }
+    attrs.merge!(project: options)
+
+    put :update, attrs
+  end
+end