update code based on feedback42327-import-from-gitlab-com-fails-destination-already-exists-and-is-not-an-empty-directory-error

7 files changed, 16 insertions, 7 deletions

diff --git a/app/models/project_services/emails_on_push_service.rb b/app/models/project_services/emails_on_push_service.rb
index 62c8dfc6cc3..b604d860a87 100644
--- a/app/models/project_services/emails_on_push_service.rb
+++ b/app/models/project_services/emails_on_push_service.rb
@@ -2,7 +2,7 @@ class EmailsOnPushService < Service
   boolean_accessor :send_from_committer_email
   boolean_accessor :disable_diffs
   prop_accessor :recipients
-  validates :recipients, presence: true, if: :activated?, unless: :importing?
+  validates :recipients, presence: true, if: :valid_recipients?
 
   def title
     'Emails on push'
diff --git a/app/models/project_services/irker_service.rb b/app/models/project_services/irker_service.rb
index 3d01cc73535..27bdf708c80 100644
--- a/app/models/project_services/irker_service.rb
+++ b/app/models/project_services/irker_service.rb
@@ -4,7 +4,7 @@ class IrkerService < Service
   prop_accessor :server_host, :server_port, :default_irc_uri
   prop_accessor :recipients, :channels
   boolean_accessor :colorize_messages
-  validates :recipients, presence: true, if: :activated?, unless: :importing?
+  validates :recipients, presence: true, if: :valid_recipients?
 
   before_validation :get_channels
 
diff --git a/app/models/project_services/pipelines_email_service.rb b/app/models/project_services/pipelines_email_service.rb
index 267ac80e0ca..9c7b58dead5 100644
--- a/app/models/project_services/pipelines_email_service.rb
+++ b/app/models/project_services/pipelines_email_service.rb
@@ -1,7 +1,7 @@
 class PipelinesEmailService < Service
   prop_accessor :recipients
   boolean_accessor :notify_only_broken_pipelines
-  validates :recipients, presence: true, if: :activated?, unless: :importing?
+  validates :recipients, presence: true, if: :valid_recipients?
 
   def initialize_properties
     self.properties ||= { notify_only_broken_pipelines: true }
diff --git a/app/models/service.rb b/app/models/service.rb
index 5daa70541ab..369cae2e85f 100644
--- a/app/models/service.rb
+++ b/app/models/service.rb
@@ -297,4 +297,8 @@ class Service < ActiveRecord::Base
       project.cache_has_external_wiki
     end
   end
+
+  def valid_recipients?
+    activated? && !importing?
+  end
 end
diff --git a/changelogs/unreleased/42327-import-from-gitlab-com-fails-destination-already-exists-and-is-not-an-empty-directory-error.yml b/changelogs/unreleased/42327-import-from-gitlab-com-fails-destination-already-exists-and-is-not-an-empty-directory-error.yml
index 02312efd4b9..660f4f5d42c 100644
--- a/changelogs/unreleased/42327-import-from-gitlab-com-fails-destination-already-exists-and-is-not-an-empty-directory-error.yml
+++ b/changelogs/unreleased/42327-import-from-gitlab-com-fails-destination-already-exists-and-is-not-an-empty-directory-error.yml
@@ -1,5 +1,5 @@
 ---
-title: Fixes destination already exists and is not an empty directory Import/Export
+title: Fixes destination already exists, and some particular service errors on Import/Export
   error
 merge_request: 16714
 author:
diff --git a/lib/gitlab/import_export/shared.rb b/lib/gitlab/import_export/shared.rb
index 71aec88a033..b34cafc6876 100644
--- a/lib/gitlab/import_export/shared.rb
+++ b/lib/gitlab/import_export/shared.rb
@@ -19,8 +19,13 @@ module Gitlab
       def error(error)
         error_out(error.message, caller[0].dup)
         @errors << error.message
+
         # Debug:
-        Rails.logger.error(error.backtrace&.join("\n"))
+        if error.backtrace
+          Rails.logger.error("Import/Export backtrace: #{error.backtrace.join("\n")}")
+        else
+          Rails.logger.error("No backtrace found")
+        end
       end
 
       private
diff --git a/spec/workers/repository_import_worker_spec.rb b/spec/workers/repository_import_worker_spec.rb
index b8bdc049482..2b1a617ee62 100644
--- a/spec/workers/repository_import_worker_spec.rb
+++ b/spec/workers/repository_import_worker_spec.rb
@@ -49,7 +49,7 @@ describe RepositoryImportWorker do
 
         expect do
           subject.perform(project.id)
-        end.to raise_error(StandardError, error)
+        end.to raise_error(RuntimeError, error)
         expect(project.reload.import_jid).not_to be_nil
       end
 
@@ -61,7 +61,7 @@ describe RepositoryImportWorker do
 
         expect do
           subject.perform(project.id)
-        end.to raise_error(StandardError, error)
+        end.to raise_error(RuntimeError, error)
 
         expect(project.reload.import_error).not_to be_nil
       end