Merge branch 'allow-blank-import-url' into 'master'

Allow a project import URL to be blank to prevent false positives in validation

Projects that happen to have blank import URLs were failing validation, causing project settings not to be saved. Loosen the requirement and don't try to use the import data if the URL isn't present.

Closes #19893

See merge request !5309

app/models/project.rb

@@ -162,7 +162,7 @@ class Project < ActiveRecord::Base
   validates :namespace, presence: true
   validates_uniqueness_of :name, scope: :namespace_id
   validates_uniqueness_of :path, scope: :namespace_id
-  validates :import_url, addressable_url: true, if: :import_url
+  validates :import_url, addressable_url: true, if: :external_import?
   validates :star_count, numericality: { greater_than_or_equal_to: 0 }
   validate :check_limit, on: :create
   validate :avatar_type,
@@ -482,7 +482,7 @@ class Project < ActiveRecord::Base
   end
 
   def create_or_update_import_data(data: nil, credentials: nil)
-    return unless valid_import_url?
+    return unless import_url.present? && valid_import_url?
 
     project_import_data = import_data || build_import_data
     if data

spec/lib/gitlab/bitbucket_import/client_spec.rb

@@ -54,12 +54,12 @@ describe Gitlab::BitbucketImport::Client, lib: true do
   context 'project import' do
     it 'calls .from_project with no errors' do
       project = create(:empty_project)
+      project.import_url = "ssh://git@bitbucket.org/test/test.git"
       project.create_or_update_import_data(credentials:
                                              { user: "git",
                                                password: nil,
                                                bb_session: { bitbucket_access_token: "test",
                                                              bitbucket_access_token_secret: "test" } })
-      project.import_url = "ssh://git@bitbucket.org/test/test.git"
 
       expect { described_class.from_project(project) }.not_to raise_error
     end

spec/models/project_spec.rb

@@ -142,10 +142,10 @@ describe Project, models: true do
       expect(project2).to be_valid
     end
 
-    it 'does not allow to introduce an empty URI' do
+    it 'allows an empty URI' do
       project2 = build(:project, import_url: '')
 
-      expect(project2).not_to be_valid
+      expect(project2).to be_valid
     end
 
     it 'does not produce import data on an empty URI' do