From 482fc5537f8273d82fadde7a68b609eda3e64543 Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Thu, 25 Jan 2018 16:21:23 +0100
Subject: fix validation error on services

---
 app/models/project_services/emails_on_push_service.rb  | 2 +-
 app/models/project_services/irker_service.rb           | 2 +-
 app/models/project_services/pipelines_email_service.rb | 2 +-
 app/models/service.rb                                  | 2 ++
 spec/lib/gitlab/import_export/project.json             | 2 +-
 5 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/app/models/project_services/emails_on_push_service.rb b/app/models/project_services/emails_on_push_service.rb
index 1a236e232f9..62c8dfc6cc3 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?
+  validates :recipients, presence: true, if: :activated?, unless: :importing?
 
   def title
     'Emails on push'
diff --git a/app/models/project_services/irker_service.rb b/app/models/project_services/irker_service.rb
index 19357f90810..3d01cc73535 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?
+  validates :recipients, presence: true, if: :activated?, unless: :importing?
 
   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 6a3118a11b8..267ac80e0ca 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?
+  validates :recipients, presence: true, if: :activated?, unless: :importing?
 
   def initialize_properties
     self.properties ||= { notify_only_broken_pipelines: true }
diff --git a/app/models/service.rb b/app/models/service.rb
index 96a064697f0..5daa70541ab 100644
--- a/app/models/service.rb
+++ b/app/models/service.rb
@@ -2,6 +2,8 @@
 # and implement a set of methods
 class Service < ActiveRecord::Base
   include Sortable
+  include Importable
+
   serialize :properties, JSON # rubocop:disable Cop/ActiveRecordSerialize
 
   default_value_for :active, false
diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json
index 4cf33778d15..b6c1f0c81cb 100644
--- a/spec/lib/gitlab/import_export/project.json
+++ b/spec/lib/gitlab/import_export/project.json
@@ -7096,7 +7096,7 @@
       "project_id": 5,
       "created_at": "2016-06-14T15:01:51.232Z",
       "updated_at": "2016-06-14T15:01:51.232Z",
-      "active": false,
+      "active": true,
       "properties": {
 
       },
-- 
cgit v1.2.1


From 85d47384de293b33907990896c10034ec36498fd Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Fri, 26 Jan 2018 09:01:56 +0100
Subject: add an extra spec

---
 spec/lib/gitlab/import_export/project_tree_saver_spec.rb | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb
index 08e5bbbd400..a93382d618f 100644
--- a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb
+++ b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb
@@ -164,6 +164,10 @@ describe Gitlab::ImportExport::ProjectTreeSaver do
         expect(saved_project_json['services'].first['type']).to eq('CustomIssueTrackerService')
       end
 
+      it 'saves the properties for a service' do
+        expect(saved_project_json['services'].first['properties']).to eq('one' => 'value')
+      end
+
       it 'has project feature' do
         project_feature = saved_project_json['project_feature']
         expect(project_feature).not_to be_empty
@@ -279,7 +283,7 @@ describe Gitlab::ImportExport::ProjectTreeSaver do
            commit_id: ci_build.pipeline.sha)
 
     create(:event, :created, target: milestone, project: project, author: user)
-    create(:service, project: project, type: 'CustomIssueTrackerService', category: 'issue_tracker')
+    create(:service, project: project, type: 'CustomIssueTrackerService', category: 'issue_tracker', properties: {one: 'value'})
 
     create(:project_custom_attribute, project: project)
     create(:project_custom_attribute, project: project)
-- 
cgit v1.2.1


From 865bb64a06f33b1076d1b9a202cd41c7ad0728c5 Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Fri, 26 Jan 2018 10:43:13 +0100
Subject: disable retry attempts for Import/Export until that is fixed

---
 app/models/project.rb                   |  2 ++
 app/workers/repository_import_worker.rb | 11 ++++++++++-
 lib/gitlab/import_export/shared.rb      |  2 +-
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/app/models/project.rb b/app/models/project.rb
index e19873f64ce..8a5895cea05 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -568,6 +568,8 @@ class Project < ActiveRecord::Base
         RepositoryForkWorker.perform_async(id,
                                            forked_from_project.repository_storage_path,
                                            forked_from_project.disk_path)
+      elsif gitlab_project_import?
+        RepositoryImportWorker.set(retry: false).perform_async(self.id)
       else
         RepositoryImportWorker.perform_async(self.id)
       end
diff --git a/app/workers/repository_import_worker.rb b/app/workers/repository_import_worker.rb
index 31e2798c36b..1a8be9d9a93 100644
--- a/app/workers/repository_import_worker.rb
+++ b/app/workers/repository_import_worker.rb
@@ -20,7 +20,12 @@ class RepositoryImportWorker
     # to those importers to mark the import process as complete.
     return if service.async?
 
-    raise result[:message] if result[:status] == :error
+    if result[:status] == :error
+
+      fail_import(project, result[:message]) if project.gitlab_project_import?
+
+      raise result[:message]
+    end
 
     project.after_import
   end
@@ -33,4 +38,8 @@ class RepositoryImportWorker
     Rails.logger.info("Project #{project.full_path} was in inconsistent state (#{project.import_status}) while importing.")
     false
   end
+
+  def fail_import(project, message)
+    project.mark_import_as_failed(message)
+  end
 end
diff --git a/lib/gitlab/import_export/shared.rb b/lib/gitlab/import_export/shared.rb
index d03cbc880fd..71aec88a033 100644
--- a/lib/gitlab/import_export/shared.rb
+++ b/lib/gitlab/import_export/shared.rb
@@ -20,7 +20,7 @@ module Gitlab
         error_out(error.message, caller[0].dup)
         @errors << error.message
         # Debug:
-        Rails.logger.error(error.backtrace.join("\n"))
+        Rails.logger.error(error.backtrace&.join("\n"))
       end
 
       private
-- 
cgit v1.2.1


From 7affc2311282a032722af245abb92f0bd2da8db9 Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Fri, 26 Jan 2018 11:14:54 +0100
Subject: add spec

---
 app/models/project.rb                         |  1 +
 app/workers/repository_import_worker.rb       |  1 -
 spec/workers/repository_import_worker_spec.rb | 13 +++++++++++++
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/app/models/project.rb b/app/models/project.rb
index 8a5895cea05..d0d0fd6e093 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -569,6 +569,7 @@ class Project < ActiveRecord::Base
                                            forked_from_project.repository_storage_path,
                                            forked_from_project.disk_path)
       elsif gitlab_project_import?
+        # Do not retry on Import/Export until https://gitlab.com/gitlab-org/gitlab-ce/issues/26189 is solved.
         RepositoryImportWorker.set(retry: false).perform_async(self.id)
       else
         RepositoryImportWorker.perform_async(self.id)
diff --git a/app/workers/repository_import_worker.rb b/app/workers/repository_import_worker.rb
index 1a8be9d9a93..d79b5ee5346 100644
--- a/app/workers/repository_import_worker.rb
+++ b/app/workers/repository_import_worker.rb
@@ -21,7 +21,6 @@ class RepositoryImportWorker
     return if service.async?
 
     if result[:status] == :error
-
       fail_import(project, result[:message]) if project.gitlab_project_import?
 
       raise result[:message]
diff --git a/spec/workers/repository_import_worker_spec.rb b/spec/workers/repository_import_worker_spec.rb
index 7274a9f00f9..b8bdc049482 100644
--- a/spec/workers/repository_import_worker_spec.rb
+++ b/spec/workers/repository_import_worker_spec.rb
@@ -52,6 +52,19 @@ describe RepositoryImportWorker do
         end.to raise_error(StandardError, error)
         expect(project.reload.import_jid).not_to be_nil
       end
+
+      it 'updates the error on Import/Export' do
+        error = %q{remote: Not Found fatal: repository 'https://user:pass@test.com/root/repoC.git/' not found }
+
+        project.update_attributes(import_jid: '123', import_type: 'gitlab_project')
+        expect_any_instance_of(Projects::ImportService).to receive(:execute).and_return({ status: :error, message: error })
+
+        expect do
+          subject.perform(project.id)
+        end.to raise_error(StandardError, error)
+
+        expect(project.reload.import_error).not_to be_nil
+      end
     end
 
     context 'when using an asynchronous importer' do
-- 
cgit v1.2.1


From 3b7575de10512d19c2e7d722608e8f84a3b4d0bb Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Fri, 26 Jan 2018 11:15:47 +0100
Subject: fix spec

---
 spec/lib/gitlab/import_export/project_tree_saver_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb
index a93382d618f..5804c45871e 100644
--- a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb
+++ b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb
@@ -283,7 +283,7 @@ describe Gitlab::ImportExport::ProjectTreeSaver do
            commit_id: ci_build.pipeline.sha)
 
     create(:event, :created, target: milestone, project: project, author: user)
-    create(:service, project: project, type: 'CustomIssueTrackerService', category: 'issue_tracker', properties: {one: 'value'})
+    create(:service, project: project, type: 'CustomIssueTrackerService', category: 'issue_tracker', properties: { one: 'value' })
 
     create(:project_custom_attribute, project: project)
     create(:project_custom_attribute, project: project)
-- 
cgit v1.2.1


From 4131b6e9e32acc7317704018801c96342ea8b578 Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Fri, 26 Jan 2018 11:16:56 +0100
Subject: add changelog

---
 ...stination-already-exists-and-is-not-an-empty-directory-error.yml | 6 ++++++
 1 file changed, 6 insertions(+)
 create mode 100644 changelogs/unreleased/42327-import-from-gitlab-com-fails-destination-already-exists-and-is-not-an-empty-directory-error.yml

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
new file mode 100644
index 00000000000..02312efd4b9
--- /dev/null
+++ b/changelogs/unreleased/42327-import-from-gitlab-com-fails-destination-already-exists-and-is-not-an-empty-directory-error.yml
@@ -0,0 +1,6 @@
+---
+title: Fixes destination already exists and is not an empty directory Import/Export
+  error
+merge_request: 16714
+author:
+type: fixed
-- 
cgit v1.2.1


From 41a14498c7d6fc1c422c9507393e889f96d964dc Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Fri, 26 Jan 2018 14:48:47 +0100
Subject: update code based on feedback

---
 app/models/project_services/emails_on_push_service.rb              | 2 +-
 app/models/project_services/irker_service.rb                       | 2 +-
 app/models/project_services/pipelines_email_service.rb             | 2 +-
 app/models/service.rb                                              | 4 ++++
 ...tination-already-exists-and-is-not-an-empty-directory-error.yml | 2 +-
 lib/gitlab/import_export/shared.rb                                 | 7 ++++++-
 spec/workers/repository_import_worker_spec.rb                      | 4 ++--
 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
-- 
cgit v1.2.1