From c906fbb5656be08e5e16bbd3f281dac3e488d3fc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Wed, 20 Sep 2017 18:11:50 +0200
Subject: [PATCH 01/53] Extract methods

---
 .../organization_metadata_export_service.rb   | 20 +++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/app/services/carto/organization_metadata_export_service.rb b/app/services/carto/organization_metadata_export_service.rb
index 8564db97c5..8bbc3b5915 100644
--- a/app/services/carto/organization_metadata_export_service.rb
+++ b/app/services/carto/organization_metadata_export_service.rb
@@ -190,10 +190,9 @@ module Carto
 
     def import_from_directory(meta_path)
       # Import organization
-      organization_file = Dir["#{meta_path}/organization_*.json"].first
-      organization = build_organization_from_json_export(File.read(organization_file))
+      organization = load_organization_from_directory(meta_path)
 
-      organization_redis_file = Dir["#{meta_path}/redis_organization_*.json"].first
+      organization_redis_file = get_redis_filename(meta_path)
       Carto::RedisExportService.new.restore_redis_from_json_export(File.read(organization_redis_file))
 
       # Groups and notifications must be saved after users
@@ -204,7 +203,7 @@ module Carto
 
       save_imported_organization(organization)
 
-      user_list = Dir["#{meta_path}/user_*"]
+      user_list = get_user_list(meta_path)
 
       # In order to get permissions right, we first import all users, then all datasets and finally, all maps
       organization.users = user_list.map do |user_path|
@@ -218,6 +217,19 @@ module Carto
       organization
     end
 
+    def get_user_list(meta_path)
+      Dir["#{meta_path}/user_*"]
+    end
+
+    def get_redis_filename(meta_path)
+      Dir["#{meta_path}/redis_organization_*.json"].first
+    end
+    
+    def load_organization_from_directory(meta_path)
+      organization_file = Dir["#{meta_path}/organization_*.json"].first
+      organization = build_organization_from_json_export(File.read(organization_file))
+    end
+
     def import_metadata_from_directory(organization, path)
       organization.users.each do |user|
         Carto::UserMetadataExportService.new.import_user_visualizations_from_directory(

From 0ed5ea56077c902c69f7d5bd0a554f4b46104cc2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Wed, 20 Sep 2017 18:12:15 +0200
Subject: [PATCH 02/53] Add rollback methods

---
 .../organization_metadata_export_service.rb   | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/app/services/carto/organization_metadata_export_service.rb b/app/services/carto/organization_metadata_export_service.rb
index 8bbc3b5915..6cf3163f24 100644
--- a/app/services/carto/organization_metadata_export_service.rb
+++ b/app/services/carto/organization_metadata_export_service.rb
@@ -217,6 +217,24 @@ module Carto
       organization
     end
 
+    def rollback_import_from_directory(meta_path)
+      organization_redis_file = get_redis_filename(meta_path)
+      Carto::RedisExportService.new.remove_redis_from_json_export(File.read(organization_redis_file))
+      organization = load_organization_from_directory(meta_path)
+
+      user_list = get_user_list(meta_path)
+      user_list.map do |user_path|
+        Carto::UserMetadataExportService.new.rollback_import_from_directory(user_path)
+      end
+
+      organization = Cartodb::Organization.find(organization.id)
+      return unless organization
+      organization.groups.delete
+      organization.notifications.delete
+      organization.users.delete
+      organization.delete
+    end
+
     def get_user_list(meta_path)
       Dir["#{meta_path}/user_*"]
     end
@@ -254,5 +272,17 @@ module Carto
 
       organization
     end
+
+    def rollback_import_metadata_from_directory(organization, path)
+      organization.users.each do |user|
+        Carto::UserMetadataExportService.new.import_user_visualizations_from_directory(
+          user, Carto::Visualization::TYPE_REMOTE, "#{path}/user_#{user.id}"
+        )
+
+        Carto::UserMetadataExportService.new.import_user_visualizations_from_directory(
+          user, Carto::Visualization::TYPE_CANONICAL, "#{path}/user_#{user.id}"
+        )
+      end
+    end
   end
 end

From a4ea4a1cd1c22f11437a7bc7811ab0a74c229588 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Wed, 20 Sep 2017 18:13:37 +0200
Subject: [PATCH 03/53] Extract meethods

---
 .../carto/user_metadata_export_service.rb     | 21 +++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/app/services/carto/user_metadata_export_service.rb b/app/services/carto/user_metadata_export_service.rb
index 1bd151439d..a5359155c5 100644
--- a/app/services/carto/user_metadata_export_service.rb
+++ b/app/services/carto/user_metadata_export_service.rb
@@ -197,12 +197,9 @@ module Carto
     end
 
     def import_from_directory(path)
-      user_file = Dir["#{path}/user_*.json"].first
-      user = build_user_from_json_export(File.read(user_file))
+      save_imported_user(user_from_file(path))
 
-      save_imported_user(user)
-
-      Carto::RedisExportService.new.restore_redis_from_json_export(File.read(Dir["#{path}/redis_user_*.json"].first))
+      Carto::RedisExportService.new.restore_redis_from_json_export(redis_user_file(path))
 
       user
     end
@@ -228,13 +225,25 @@ module Carto
     end
 
     def import_search_tweets_from_directory(user, meta_path)
-      user_file = Dir["#{meta_path}/user_*.json"].first
+      user_file = user_file_dir(meta_path)
       search_tweets = build_search_tweets_from_json_export(File.read(user_file))
       search_tweets.each { |st| save_imported_search_tweet(st, user) }
     end
 
     private
 
+    def user_from_file(path)
+      build_user_from_json_export(File.read(user_file_dir(path)))
+    end
+
+    def user_file_dir(path)
+      Dir["#{path}/user_*.json"].first
+    end
+
+    def redis_user_file(path)
+      File.read(Dir["#{path}/redis_user_*.json"].first)
+    end
+
     def export_user_visualizations_to_directory(user, type, path)
       root_dir = Pathname.new(path)
       user.visualizations.where(type: type).each do |visualization|

From 231781fdedf22b8e29c69be656180c2f83fad709 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Wed, 20 Sep 2017 18:13:53 +0200
Subject: [PATCH 04/53] Add rollback

---
 app/services/carto/user_metadata_export_service.rb | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/app/services/carto/user_metadata_export_service.rb b/app/services/carto/user_metadata_export_service.rb
index a5359155c5..989a44bdbd 100644
--- a/app/services/carto/user_metadata_export_service.rb
+++ b/app/services/carto/user_metadata_export_service.rb
@@ -204,6 +204,14 @@ module Carto
       user
     end
 
+    def rollback_import_from_directory(path)
+      user = user_from_file(path)
+
+      user && user.delete
+
+      Carto::RedisExportService.new.remove_redis_from_json_export(redis_user_file(path))
+    end
+
     def import_user_visualizations_from_directory(user, type, meta_path)
       with_non_viewer_user(user) do
         Dir["#{meta_path}/#{type}_*#{Carto::VisualizationExporter::EXPORT_EXTENSION}"].each do |fname|

From 5b2b338ad1e3a00a57ad45db8a59fa5193ea7c6b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Wed, 20 Sep 2017 18:14:09 +0200
Subject: [PATCH 05/53] add rollback

---
 services/user-mover/import_user.rb | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/services/user-mover/import_user.rb b/services/user-mover/import_user.rb
index f08a207511..40941e84f7 100644
--- a/services/user-mover/import_user.rb
+++ b/services/user-mover/import_user.rb
@@ -61,6 +61,14 @@ module CartoDB
         end
       end
 
+      def rollback!
+        if @pack_config['organization']
+          rollback_org
+        else
+          rollback_user
+        end
+      end
+
       def organization_import?
         @pack_config['organization'] != nil
       end

From e1d844034a082136cb622c3efb431e5c7164e9d5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Wed, 20 Sep 2017 18:14:38 +0200
Subject: [PATCH 06/53] Add redis removal methods

---
 app/services/carto/redis_export_service.rb | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/app/services/carto/redis_export_service.rb b/app/services/carto/redis_export_service.rb
index 6966ec917d..8ba5db086b 100644
--- a/app/services/carto/redis_export_service.rb
+++ b/app/services/carto/redis_export_service.rb
@@ -18,23 +18,43 @@ module Carto
       restore_redis_from_hash_export(JSON.parse(exported_json_string).deep_symbolize_keys)
     end
 
+    def remove_redis_from_json_export(exported_json_string)
+      remove_redis_from_hash_export(JSON.parse(exported_json_string).deep_symbolize_keys)
+    end
+
     def restore_redis_from_hash_export(exported_hash)
       raise 'Wrong export version' unless compatible_version?(exported_hash[:version])
 
       restore_redis(exported_hash[:redis])
     end
 
+    def remove_redis_from_json_export(exported_hash)
+      raise 'Wrong export version' unless compatible_version?(exported_hash[:version])
+
+      remove_redis(exported_hash[:redis])
+    end
+
     private
 
     def restore_redis(redis_export)
       restore_keys($users_metadata, redis_export[:users_metadata])
     end
 
+    def remove_redis(redis_export)
+      remove_keys($users_metadata, redis_export[:users_metadata])
+    end
+
     def restore_keys(redis_db, redis_keys)
       redis_keys.each do |key, value|
         redis_db.restore(key, value[:ttl], Base64.decode64(value[:value]))
       end
     end
+
+    def remove_keys(redis_db, redis_keys)
+      redis_keys.each do |key|
+        redis_db.del(key)
+      end
+    end
   end
 
   module RedisExportServiceExporter

From 6276d34f258a43d117dda393ec3ba0827d25513a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Wed, 20 Sep 2017 18:15:57 +0200
Subject: [PATCH 07/53] Handle exceptions and do rollbacks

---
 app/models/carto/user_migration_import.rb | 29 ++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/app/models/carto/user_migration_import.rb b/app/models/carto/user_migration_import.rb
index 63eb3b27fa..f08de470f0 100644
--- a/app/models/carto/user_migration_import.rb
+++ b/app/models/carto/user_migration_import.rb
@@ -66,17 +66,40 @@ module Carto
     def import(service, package)
       if import_metadata?
         log.append('=== Importing metadata ===')
-        imported = service.import_from_directory(package.meta_dir)
+        begin
+          imported = service.import_from_directory(package.meta_dir)
+        rescue => e
+          log.append('=== Error importing metadata. Rollback! ===')
+          service.rollback_import_from_directory(package.meta_dir)
+          raise e
+        end
         org_import? ? self.organization = imported : self.user = imported
         save!
       end
 
       log.append('=== Importing data ===')
-      CartoDB::DataMover::ImportJob.new(import_job_arguments(package.data_dir)).run!
+      import_job = CartoDB::DataMover::ImportJob.new(import_job_arguments(package.data_dir))
+      begin
+        import_job.run!
+      rescue => e
+        log.append('=== Error importing data. Rollback!')
+        import_job.rollback!
+        service.rollback_import_from_directory(package.meta_dir)
+        raise e
+      end
 
       if import_metadata?
         log.append('=== Importing visualizations and search tweets ===')
-        service.import_metadata_from_directory(imported, package.meta_dir)
+        begin
+          ActiveRecord::Base.transaction do
+            service.import_metadata_from_directory(imported, package.meta_dir)
+          end
+        rescue => e
+          log.append('=== Error importing visualizations and search tweets. Rollback! ===')
+          service.rollback_import_metadata_from_directory(imported, package.meta_dir)
+          import_job.rollback!
+          raise e
+        end
       end
     end
 

From 9ddf6ba4e012cd7cc496e8744f12c49ffba860f7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Wed, 20 Sep 2017 18:41:40 +0200
Subject: [PATCH 08/53] Fix rollback order

---
 app/models/carto/user_migration_import.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/models/carto/user_migration_import.rb b/app/models/carto/user_migration_import.rb
index f08de470f0..f3116fd9fd 100644
--- a/app/models/carto/user_migration_import.rb
+++ b/app/models/carto/user_migration_import.rb
@@ -84,7 +84,7 @@ module Carto
       rescue => e
         log.append('=== Error importing data. Rollback!')
         import_job.rollback!
-        service.rollback_import_from_directory(package.meta_dir)
+        service.rollback_import_from_directory(package.meta_dir) if import_metadata?
         raise e
       end
 
@@ -96,8 +96,8 @@ module Carto
           end
         rescue => e
           log.append('=== Error importing visualizations and search tweets. Rollback! ===')
-          service.rollback_import_metadata_from_directory(imported, package.meta_dir)
           import_job.rollback!
+          service.rollback_import_from_directory(imported, package.meta_dir)
           raise e
         end
       end

From f529718491fa8b0e5e5076bda9c31d6ee17d930b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Wed, 20 Sep 2017 18:49:00 +0200
Subject: [PATCH 09/53] Extract methods

---
 app/models/carto/user_migration_import.rb | 56 ++++++++++++++---------
 1 file changed, 35 insertions(+), 21 deletions(-)

diff --git a/app/models/carto/user_migration_import.rb b/app/models/carto/user_migration_import.rb
index f3116fd9fd..5f5c532167 100644
--- a/app/models/carto/user_migration_import.rb
+++ b/app/models/carto/user_migration_import.rb
@@ -65,18 +65,31 @@ module Carto
 
     def import(service, package)
       if import_metadata?
-        log.append('=== Importing metadata ===')
-        begin
-          imported = service.import_from_directory(package.meta_dir)
-        rescue => e
-          log.append('=== Error importing metadata. Rollback! ===')
-          service.rollback_import_from_directory(package.meta_dir)
-          raise e
+        imported = import_metadata(package, service)
+      end
+
+      import_job = import_data(package, service)
+
+      if import_metadata?
+        import_visualizations(import_job, imported, package, service)
+      end
+    end
+
+    def import_visualizations(import_job, imported, package, service)
+      log.append('=== Importing visualizations and search tweets ===')
+      begin
+        ActiveRecord::Base.transaction do
+          service.import_metadata_from_directory(imported, package.meta_dir)
         end
-        org_import? ? self.organization = imported : self.user = imported
-        save!
+      rescue => e
+        log.append('=== Error importing visualizations and search tweets. Rollback! ===')
+        import_job.rollback!
+        service.rollback_import_from_directory(imported, package.meta_dir)
+        raise e
       end
+    end
 
+    def import_data(package, service)
       log.append('=== Importing data ===')
       import_job = CartoDB::DataMover::ImportJob.new(import_job_arguments(package.data_dir))
       begin
@@ -87,20 +100,21 @@ module Carto
         service.rollback_import_from_directory(package.meta_dir) if import_metadata?
         raise e
       end
+      import_job
+    end
 
-      if import_metadata?
-        log.append('=== Importing visualizations and search tweets ===')
-        begin
-          ActiveRecord::Base.transaction do
-            service.import_metadata_from_directory(imported, package.meta_dir)
-          end
-        rescue => e
-          log.append('=== Error importing visualizations and search tweets. Rollback! ===')
-          import_job.rollback!
-          service.rollback_import_from_directory(imported, package.meta_dir)
-          raise e
-        end
+    def import_metadata(package, service)
+      log.append('=== Importing metadata ===')
+      begin
+        imported = service.import_from_directory(package.meta_dir)
+      rescue => e
+        log.append('=== Error importing metadata. Rollback! ===')
+        service.rollback_import_from_directory(package.meta_dir)
+        raise e
       end
+      org_import? ? self.organization = imported : self.user = imported
+      save!
+      imported
     end
 
     def import_only_data?

From d2511ceb0f6eaeaef60ce9706bace164796bf50c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Wed, 20 Sep 2017 18:51:29 +0200
Subject: [PATCH 10/53] Fix method name (typo)

---
 app/services/carto/redis_export_service.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/services/carto/redis_export_service.rb b/app/services/carto/redis_export_service.rb
index 8ba5db086b..1c9ef3365a 100644
--- a/app/services/carto/redis_export_service.rb
+++ b/app/services/carto/redis_export_service.rb
@@ -28,7 +28,7 @@ module Carto
       restore_redis(exported_hash[:redis])
     end
 
-    def remove_redis_from_json_export(exported_hash)
+    def remove_redis_from_hash_export(exported_hash)
       raise 'Wrong export version' unless compatible_version?(exported_hash[:version])
 
       remove_redis(exported_hash[:redis])

From f47ff74bc1a03effa03ac48b93fba9709ce6248a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Thu, 21 Sep 2017 12:01:08 +0200
Subject: [PATCH 11/53] Method and ActiveRecord magic collisions

---
 app/models/carto/user_migration_import.rb | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/models/carto/user_migration_import.rb b/app/models/carto/user_migration_import.rb
index 5f5c532167..60900df5ad 100644
--- a/app/models/carto/user_migration_import.rb
+++ b/app/models/carto/user_migration_import.rb
@@ -65,10 +65,10 @@ module Carto
 
     def import(service, package)
       if import_metadata?
-        imported = import_metadata(package, service)
+        imported = do_import_metadata(package, service)
       end
 
-      import_job = import_data(package, service)
+      import_job = do_import_data(package, service)
 
       if import_metadata?
         import_visualizations(import_job, imported, package, service)
@@ -89,7 +89,7 @@ module Carto
       end
     end
 
-    def import_data(package, service)
+    def do_import_data(package, service)
       log.append('=== Importing data ===')
       import_job = CartoDB::DataMover::ImportJob.new(import_job_arguments(package.data_dir))
       begin
@@ -103,7 +103,7 @@ module Carto
       import_job
     end
 
-    def import_metadata(package, service)
+    def do_import_metadata(package, service)
       log.append('=== Importing metadata ===')
       begin
         imported = service.import_from_directory(package.meta_dir)

From 30863cad8902e59188ff88addafccdac32cb903a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Thu, 21 Sep 2017 12:01:26 +0200
Subject: [PATCH 12/53] Fix number of arguments

---
 app/models/carto/user_migration_import.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/carto/user_migration_import.rb b/app/models/carto/user_migration_import.rb
index 60900df5ad..1c8615cb17 100644
--- a/app/models/carto/user_migration_import.rb
+++ b/app/models/carto/user_migration_import.rb
@@ -84,7 +84,7 @@ module Carto
       rescue => e
         log.append('=== Error importing visualizations and search tweets. Rollback! ===')
         import_job.rollback!
-        service.rollback_import_from_directory(imported, package.meta_dir)
+        service.rollback_import_from_directory(package.meta_dir)
         raise e
       end
     end

From 1a85548a7b1a7e1e81e8f3e9768268c4268daa79 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Thu, 21 Sep 2017 12:01:35 +0200
Subject: [PATCH 13/53] Improve logging

---
 app/models/carto/user_migration_import.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/models/carto/user_migration_import.rb b/app/models/carto/user_migration_import.rb
index 1c8615cb17..b8f6f15172 100644
--- a/app/models/carto/user_migration_import.rb
+++ b/app/models/carto/user_migration_import.rb
@@ -83,6 +83,8 @@ module Carto
         end
       rescue => e
         log.append('=== Error importing visualizations and search tweets. Rollback! ===')
+        log.append(e)
+        log.append(e.backtrace.join("\n"))
         import_job.rollback!
         service.rollback_import_from_directory(package.meta_dir)
         raise e

From 1fc6f54c9e403ac5cc2aa9bed151f253e6a21757 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Thu, 21 Sep 2017 12:02:44 +0200
Subject: [PATCH 14/53] Keep user

---
 app/services/carto/user_metadata_export_service.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/services/carto/user_metadata_export_service.rb b/app/services/carto/user_metadata_export_service.rb
index 989a44bdbd..efdc3545ea 100644
--- a/app/services/carto/user_metadata_export_service.rb
+++ b/app/services/carto/user_metadata_export_service.rb
@@ -197,7 +197,8 @@ module Carto
     end
 
     def import_from_directory(path)
-      save_imported_user(user_from_file(path))
+      user = user_from_file(path)
+      save_imported_user(user)
 
       Carto::RedisExportService.new.restore_redis_from_json_export(redis_user_file(path))
 

From e5c96891de27d060d9d0cf4240b67b6e30a06124 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Thu, 21 Sep 2017 12:04:13 +0200
Subject: [PATCH 15/53] Get rid of unnecessary code

This code is now run inside a transaction that is the one creating data that is being queried, never finding it, throwing an error, and causing it to fail. Carto::VisualizationInvalidationService#invalidate is now in charge of this task
---
 .../carto/visualizations_export_persistence_service.rb        | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/app/services/carto/visualizations_export_persistence_service.rb b/app/services/carto/visualizations_export_persistence_service.rb
index a32c5a3e12..7187e604c1 100644
--- a/app/services/carto/visualizations_export_persistence_service.rb
+++ b/app/services/carto/visualizations_export_persistence_service.rb
@@ -106,10 +106,6 @@ module Carto
         end
       end
 
-      # Propagate changes (named maps, default permissions and so on)
-      visualization_member = CartoDB::Visualization::Member.new(id: visualization.id).fetch
-      visualization_member.store
-
       visualization
     end
 

From 293ab46a1da2b19af2709eedea836892ddd2a5b1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Thu, 21 Sep 2017 16:12:34 +0200
Subject: [PATCH 16/53] Update news

---
 NEWS.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/NEWS.md b/NEWS.md
index aae90738c9..3c496b713e 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -355,6 +355,7 @@ More information at [Dropbox migration guide](https://www.dropbox.com/developers
 * Show infowindow when user reaches max layer limit (#12167)
 * Format quota infowindow numbers (#11743)
 * Improved analysis error tooltip (#12250)
+* Rollback failed user/organization imports
 * Enable user migrations across clouds (#12795)
 
 ### Bug fixes

From 47fce5f2fc0a8fbd2cb77459f67c28d8d79c83c4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Fri, 22 Sep 2017 15:09:58 +0200
Subject: [PATCH 17/53] Remove unused code

---
 .../carto/organization_metadata_export_service.rb    | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/app/services/carto/organization_metadata_export_service.rb b/app/services/carto/organization_metadata_export_service.rb
index 6cf3163f24..86475192d1 100644
--- a/app/services/carto/organization_metadata_export_service.rb
+++ b/app/services/carto/organization_metadata_export_service.rb
@@ -272,17 +272,5 @@ module Carto
 
       organization
     end
-
-    def rollback_import_metadata_from_directory(organization, path)
-      organization.users.each do |user|
-        Carto::UserMetadataExportService.new.import_user_visualizations_from_directory(
-          user, Carto::Visualization::TYPE_REMOTE, "#{path}/user_#{user.id}"
-        )
-
-        Carto::UserMetadataExportService.new.import_user_visualizations_from_directory(
-          user, Carto::Visualization::TYPE_CANONICAL, "#{path}/user_#{user.id}"
-        )
-      end
-    end
   end
 end

From 22bda4856f983049a18f3eeec54a22a25d464b05 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Fri, 22 Sep 2017 15:11:17 +0200
Subject: [PATCH 18/53] Update exception handling

---
 app/models/carto/user_migration_import.rb | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/app/models/carto/user_migration_import.rb b/app/models/carto/user_migration_import.rb
index f9c5a3e13a..c303f57813 100644
--- a/app/models/carto/user_migration_import.rb
+++ b/app/models/carto/user_migration_import.rb
@@ -82,10 +82,9 @@ module Carto
           service.import_metadata_from_directory(imported, package.meta_dir)
         end
       rescue => e
+        ActiveRecord::Base.connection.close
         log.append('=== Error importing visualizations and search tweets. Rollback! ===')
-        log.append(e)
-        log.append(e.backtrace.join("\n"))
-        import_job.rollback!
+        rollback_import_data(import_job)
         service.rollback_import_from_directory(package.meta_dir)
         raise e
       end
@@ -98,13 +97,21 @@ module Carto
         import_job.run!
       rescue => e
         log.append('=== Error importing data. Rollback!')
-        import_job.rollback!
+        rollback_import_data(import_job)
         service.rollback_import_from_directory(package.meta_dir) if import_metadata?
         raise e
       end
       import_job
     end
 
+    def rollback_import_data(import_job)
+      import_job.add_options(rollback: true,
+                             mode: :rollback,
+                             drop_database: true,
+                             drop_roles: true)
+      import_job.rollback!
+    end
+
     def do_import_metadata(package, service)
       log.append('=== Importing metadata ===')
       begin
@@ -126,7 +133,7 @@ module Carto
         user.database_host = database_host
         user.save!
         ::User[user.id].reload # This is because Sequel models are being cached along request. This forces reload.
-                               # It's being used in visualizations_export_persistence_service.rb#save_import
+        # It's being used in visualizations_export_persistence_service.rb#save_import
       end
     end
 

From e19f4b6a47b48514d8e78366d388769da4eb1ae3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Fri, 22 Sep 2017 15:11:41 +0200
Subject: [PATCH 19/53] Delete user in rollback

---
 app/services/carto/user_metadata_export_service.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/services/carto/user_metadata_export_service.rb b/app/services/carto/user_metadata_export_service.rb
index 090ff954b2..ecb346c56f 100644
--- a/app/services/carto/user_metadata_export_service.rb
+++ b/app/services/carto/user_metadata_export_service.rb
@@ -209,7 +209,7 @@ module Carto
     def rollback_import_from_directory(path)
       user = user_from_file(path)
 
-      user && user.delete
+      user && Carto::User.find(user.id).delete
 
       Carto::RedisExportService.new.remove_redis_from_json_export(redis_user_file(path))
     end

From df39fc11dbc0bc19b1c281ba812d2d838f407bdd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Fri, 22 Sep 2017 15:12:10 +0200
Subject: [PATCH 20/53] Accept options so rollback can be performed

---
 services/user-mover/import_user.rb | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/services/user-mover/import_user.rb b/services/user-mover/import_user.rb
index 40941e84f7..e92e136e54 100644
--- a/services/user-mover/import_user.rb
+++ b/services/user-mover/import_user.rb
@@ -53,6 +53,10 @@ module CartoDB
                        }
       end
 
+      def add_options(options)
+        @options.merge!(options)
+      end
+
       def run!
         if @pack_config['organization']
           process_org

From db506048e0dd2137d7ba291bb5d041ccc9a4fa6a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Fri, 22 Sep 2017 15:13:02 +0200
Subject: [PATCH 21/53] Drop if exists

---
 services/user-mover/import_user.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/services/user-mover/import_user.rb b/services/user-mover/import_user.rb
index e92e136e54..14d1f79096 100644
--- a/services/user-mover/import_user.rb
+++ b/services/user-mover/import_user.rb
@@ -278,7 +278,7 @@ module CartoDB
       end
 
       def drop_role(role)
-        superuser_pg_conn.query("DROP ROLE \"#{role}\"")
+        superuser_pg_conn.query("DROP ROLE IF EXISTS \"#{role}\"")
       end
 
       def get_org_info(orgname)
@@ -332,7 +332,7 @@ module CartoDB
       end
 
       def drop_database(db_name)
-        superuser_pg_conn.query("DROP DATABASE \"#{db_name}\"")
+        superuser_pg_conn.query("DROP DATABASE IF EXISTS \"#{db_name}\"")
       end
 
       def clean_oids(user_id, user_schema)

From b32dfcd083615274aa969c6131c9cab00e926aef Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Fri, 22 Sep 2017 15:16:29 +0200
Subject: [PATCH 22/53] Update test

---
 spec/models/carto/user_migration_spec.rb | 112 ++++++++++++++++++++---
 1 file changed, 98 insertions(+), 14 deletions(-)

diff --git a/spec/models/carto/user_migration_spec.rb b/spec/models/carto/user_migration_spec.rb
index 7a27d4ca4e..d5d301c5ac 100644
--- a/spec/models/carto/user_migration_spec.rb
+++ b/spec/models/carto/user_migration_spec.rb
@@ -74,26 +74,90 @@ describe 'UserMigration' do
   it_should_behave_like 'migrating metadata', true
   it_should_behave_like 'migrating metadata', false
 
+  describe 'failing imports should rollback' do
+    before :each do
+      @user = create_user_with_visualizations
+      @carto_user = Carto::User.find(@user.id)
+      @user_attributes = @carto_user.attributes
+
+      @export = Carto::UserMigrationExport.create(
+        user: @carto_user,
+        export_metadata: true
+      )
+      @export.run_export
+      destroy_user
+    end
+
+    after :each do
+      @carto_user.destroy
+    end
+
+    it 'import failing in import_metadata should rollback' do
+      Carto::RedisExportService.any_instance.stubs(:restore_redis_from_hash_export).raises('Some exception')
+
+      import.run_import.should eq false
+
+      Carto::RedisExportService.any_instance.unstub(:restore_redis_from_hash_export)
+
+      import.run_import.should eq true
+    end
+
+    it 'import failing in JobImport#run!' do
+      CartoDB::DataMover::ImportJob.any_instance.stubs(:grant_user_role).raises('Some exception')
+
+      import.run_import.should eq false
+
+      CartoDB::DataMover::ImportJob.any_instance.unstub(:grant_user_role)
+
+      import.run_import.should eq true
+    end
+
+    it 'import failing creating user database and roles' do
+      CartoDB::DataMover::ImportJob.any_instance.stubs(:import_pgdump).raises('Some exception')
+
+      import.run_import.should eq false
+
+      CartoDB::DataMover::ImportJob.any_instance.unstub(:import_pgdump)
+
+      import.run_import.should eq true
+    end
+
+    it 'import failing importing visualizations' do
+      Carto::UserMetadataExportService.any_instance.stubs(:import_search_tweets_from_directory).raises('Some exception')
+
+      import.run_import.should eq false
+
+      Carto::UserMetadataExportService.any_instance.unstub(:import_search_tweets_from_directory)
+
+      import.run_import.should eq true
+    end
+
+    private
+
+    def import
+      Carto::UserMigrationImport.create(
+        exported_file: @export.exported_file,
+        database_host: @user_attributes['database_host'],
+        org_import: false,
+        json_file: @export.json_file,
+        import_metadata: true
+      )
+    end
+
+    def destroy_user
+      @carto_user.client_applications.each(&:destroy)
+      @user.destroy
+    end
+  end
+
   it 'exports and imports a user with a data import with two tables' do
     CartoDB::UserModule::DBService.any_instance.stubs(:enable_remote_db_user).returns(true)
 
-    user = FactoryGirl.build(:valid_user).save
+    user = create_user_with_visualizations
+
     carto_user = Carto::User.find(user.id)
     user_attributes = carto_user.attributes
 
-    filepath = "#{Rails.root}/services/importer/spec/fixtures/visualization_export_with_two_tables.carto"
-    data_import = DataImport.create(
-      user_id: user.id,
-      data_source: filepath,
-      updated_at: Time.now.utc,
-      append: false,
-      create_visualization: true
-    )
-    data_import.values[:data_source] = filepath
-
-    data_import.run_import!
-    data_import.success.should eq true
-
     source_visualizations = carto_user.visualizations.map(&:name).sort
 
     export = Carto::UserMigrationExport.create(
@@ -258,4 +322,24 @@ describe 'UserMigration' do
   def attributes_to_test(user_attributes)
     user_attributes.keys - %w(created_at updated_at period_end_date)
   end
+
+  private
+
+  def create_user_with_visualizations
+    user = FactoryGirl.build(:valid_user).save
+
+    filepath = "#{Rails.root}/services/importer/spec/fixtures/visualization_export_with_two_tables.carto"
+    data_import = DataImport.create(
+      user_id: user.id,
+      data_source: filepath,
+      updated_at: Time.now.utc,
+      append: false,
+      create_visualization: true
+    )
+    data_import.values[:data_source] = filepath
+
+    data_import.run_import!
+    data_import.success.should eq true
+    return user
+  end
 end

From d12c16eb9bffe344777c4dc3c50579d742cb2162 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Tue, 26 Sep 2017 10:17:30 +0200
Subject: [PATCH 23/53] Make import_job instance attribute

---
 app/models/carto/user_migration_import.rb | 34 ++++++++++-------------
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/app/models/carto/user_migration_import.rb b/app/models/carto/user_migration_import.rb
index c303f57813..f6368268b9 100644
--- a/app/models/carto/user_migration_import.rb
+++ b/app/models/carto/user_migration_import.rb
@@ -64,27 +64,21 @@ module Carto
     end
 
     def import(service, package)
-      if import_metadata?
-        imported = do_import_metadata(package, service)
-      end
-
-      import_job = do_import_data(package, service)
-
-      if import_metadata?
-        import_visualizations(import_job, imported, package, service)
-      end
+      @import_job = CartoDB::DataMover::ImportJob.new(import_job_arguments(package.data_dir))
+      imported = do_import_metadata(package, service) if import_metadata?
+      do_import_data(package, service)
+      import_visualizations(imported, package, service) if import_metadata?
     end
 
-    def import_visualizations(import_job, imported, package, service)
+    def import_visualizations(imported, package, service)
       log.append('=== Importing visualizations and search tweets ===')
       begin
         ActiveRecord::Base.transaction do
           service.import_metadata_from_directory(imported, package.meta_dir)
         end
       rescue => e
-        ActiveRecord::Base.connection.close
         log.append('=== Error importing visualizations and search tweets. Rollback! ===')
-        rollback_import_data(import_job)
+        rollback_import_data
         service.rollback_import_from_directory(package.meta_dir)
         raise e
       end
@@ -92,24 +86,24 @@ module Carto
 
     def do_import_data(package, service)
       log.append('=== Importing data ===')
-      import_job = CartoDB::DataMover::ImportJob.new(import_job_arguments(package.data_dir))
       begin
-        import_job.run!
+        @import_job.run!
       rescue => e
-        log.append('=== Error importing data. Rollback!')
-        rollback_import_data(import_job)
+        log.append('=== Error importing data. Rollback! ===')
+        rollback_import_data
         service.rollback_import_from_directory(package.meta_dir) if import_metadata?
         raise e
+      ensure
+        @import_job.terminate_connections
       end
-      import_job
     end
 
-    def rollback_import_data(import_job)
-      import_job.add_options(rollback: true,
+    def rollback_import_data
+      @import_job.add_options(rollback: true,
                              mode: :rollback,
                              drop_database: true,
                              drop_roles: true)
-      import_job.rollback!
+      @import_job.rollback!
     end
 
     def do_import_metadata(package, service)

From 0e19f7dd3bd0013578ea95e4080a87a654d5fbac Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Tue, 26 Sep 2017 10:18:04 +0200
Subject: [PATCH 24/53] Typo?!?!?

---
 app/services/carto/organization_metadata_export_service.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/services/carto/organization_metadata_export_service.rb b/app/services/carto/organization_metadata_export_service.rb
index 86475192d1..655d8064e6 100644
--- a/app/services/carto/organization_metadata_export_service.rb
+++ b/app/services/carto/organization_metadata_export_service.rb
@@ -227,7 +227,7 @@ module Carto
         Carto::UserMetadataExportService.new.rollback_import_from_directory(user_path)
       end
 
-      organization = Cartodb::Organization.find(organization.id)
+      organization = Carto::Organization.find(organization.id)
       return unless organization
       organization.groups.delete
       organization.notifications.delete

From dfd8a09775810cd09918d9ffb2d739047f91b4ec Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Tue, 26 Sep 2017 10:19:51 +0200
Subject: [PATCH 25/53] Close ghost connections

---
 services/user-mover/import_user.rb | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/services/user-mover/import_user.rb b/services/user-mover/import_user.rb
index 14d1f79096..fba60ec277 100644
--- a/services/user-mover/import_user.rb
+++ b/services/user-mover/import_user.rb
@@ -66,6 +66,7 @@ module CartoDB
       end
 
       def rollback!
+        close_all_database_connections
         if @pack_config['organization']
           rollback_org
         else
@@ -301,9 +302,23 @@ module CartoDB
         superuser_pg_conn.query("ALTER DATABASE #{superuser_pg_conn.quote_ident(db)} SET statement_timeout = #{timeout}")
       end
 
+      def close_all_database_connections(database_name = @target_dbname)
+
+        superuser_pg_conn.query("SELECT pg_terminate_backend(pg_stat_activity.pid)
+                                  FROM pg_stat_activity
+                                WHERE pg_stat_activity.datname = '#{database_name}'
+                                AND pid <> pg_backend_pid();")
+        terminate_connections
+      end
+
       def terminate_connections
+        @user_conn && @user_conn.close
         @user_conn = nil
+
+        @superuser_user_conn && @superuser_user_conn.close
         @superuser_user_conn = nil
+
+        @superuser_conn && @superuser_conn.close
         @superuser_conn = nil
       end
 
@@ -332,6 +347,7 @@ module CartoDB
       end
 
       def drop_database(db_name)
+        close_all_database_connections(db_name)
         superuser_pg_conn.query("DROP DATABASE IF EXISTS \"#{db_name}\"")
       end
 

From 727321ac7d351fd65c99250fdcec2240ae283b87 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Tue, 26 Sep 2017 14:59:54 +0200
Subject: [PATCH 26/53] Add test cases

---
 spec/models/carto/user_migration_spec.rb | 101 ++++++++++++++++++++---
 1 file changed, 90 insertions(+), 11 deletions(-)

diff --git a/spec/models/carto/user_migration_spec.rb b/spec/models/carto/user_migration_spec.rb
index d5d301c5ac..e3e7acdf6f 100644
--- a/spec/models/carto/user_migration_spec.rb
+++ b/spec/models/carto/user_migration_spec.rb
@@ -131,22 +131,76 @@ describe 'UserMigration' do
 
       import.run_import.should eq true
     end
+  end
 
-    private
+  describe 'failing organization organizations should rollback' do
+    include_context 'organization with users helper'
+    before :each do
+      owner = @carto_organization.owner
+      filepath = "#{Rails.root}/services/importer/spec/fixtures/visualization_export_with_two_tables.carto"
+      data_import = DataImport.create(
+        user_id: owner.id,
+        data_source: filepath,
+        updated_at: Time.now.utc,
+        append: false,
+        create_visualization: true
+      )
+      data_import.values[:data_source] = filepath
 
-    def import
-      Carto::UserMigrationImport.create(
-        exported_file: @export.exported_file,
-        database_host: @user_attributes['database_host'],
-        org_import: false,
-        json_file: @export.json_file,
-        import_metadata: true
+      data_import.run_import!
+      data_import.success.should eq true
+
+      @export = Carto::UserMigrationExport.create(
+        organization: @carto_organization,
+        export_metadata: true
       )
+      @export.run_export
+      @organization.destroy_cascade
     end
 
-    def destroy_user
-      @carto_user.client_applications.each(&:destroy)
-      @user.destroy
+    after :each do
+      @organization.users.each(&:destroy_cascade)
+      @organization.destroy_cascade
+    end
+
+    it 'import failing in import_metadata should rollback' do
+      Carto::RedisExportService.any_instance.stubs(:restore_redis_from_hash_export).raises('Some exception')
+
+      org_import.run_import.should eq false
+
+      Carto::RedisExportService.any_instance.unstub(:restore_redis_from_hash_export)
+
+      org_import.run_import.should eq true
+    end
+
+    it 'import failing in JobImport#run!' do
+      CartoDB::DataMover::ImportJob.any_instance.stubs(:grant_user_role).raises('Some exception')
+
+      org_import.run_import.should eq false
+
+      CartoDB::DataMover::ImportJob.any_instance.unstub(:grant_user_role)
+
+      org_import.run_import.should eq true
+    end
+
+    it 'import failing creating user database and roles' do
+      CartoDB::DataMover::ImportJob.any_instance.stubs(:import_pgdump).raises('Some exception')
+
+      org_import.run_import.should eq false
+
+      CartoDB::DataMover::ImportJob.any_instance.unstub(:import_pgdump)
+
+      org_import.run_import.should eq true
+    end
+
+    it 'import failing importing visualizations' do
+      Carto::UserMetadataExportService.any_instance.stubs(:import_search_tweets_from_directory).raises('Some exception')
+
+      org_import.run_import.should eq false
+
+      Carto::UserMetadataExportService.any_instance.unstub(:import_search_tweets_from_directory)
+
+      org_import.run_import.should eq true
     end
   end
 
@@ -342,4 +396,29 @@ describe 'UserMigration' do
     data_import.success.should eq true
     return user
   end
+
+  def org_import
+    Carto::UserMigrationImport.create(
+      exported_file: @export.exported_file,
+      database_host: @carto_organization.owner.attributes['database_host'],
+      org_import: true,
+      json_file: @export.json_file,
+      import_metadata: true
+    )
+  end
+
+  def import
+    Carto::UserMigrationImport.create(
+      exported_file: @export.exported_file,
+      database_host: @user_attributes['database_host'],
+      org_import: false,
+      json_file: @export.json_file,
+      import_metadata: true
+    )
+  end
+
+  def destroy_user
+    @carto_user.client_applications.each(&:destroy)
+    @user.destroy
+  end
 end

From 23b26826508cb783321b02f527dc6c4589ca046e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Tue, 26 Sep 2017 15:00:19 +0200
Subject: [PATCH 27/53] Do not fail if user is not found during rollback

---
 app/services/carto/user_metadata_export_service.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/services/carto/user_metadata_export_service.rb b/app/services/carto/user_metadata_export_service.rb
index ecb346c56f..9541289691 100644
--- a/app/services/carto/user_metadata_export_service.rb
+++ b/app/services/carto/user_metadata_export_service.rb
@@ -212,6 +212,8 @@ module Carto
       user && Carto::User.find(user.id).delete
 
       Carto::RedisExportService.new.remove_redis_from_json_export(redis_user_file(path))
+    rescue ActiveRecord::RecordNotFound => e
+      #User was not created so not found and no redis removal needed
     end
 
     def import_user_visualizations_from_directory(user, type, meta_path)

From 09eaf8ea3fb5991eb7ffd0102d7d2af42b354582 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Tue, 26 Sep 2017 16:33:30 +0200
Subject: [PATCH 28/53] Fix exception message parsing

---
 app/models/user/db_service.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/models/user/db_service.rb b/app/models/user/db_service.rb
index 05ff19f32a..bbf41e94e2 100644
--- a/app/models/user/db_service.rb
+++ b/app/models/user/db_service.rb
@@ -361,6 +361,7 @@ module CartoDB
           if !retried && e.message =~ /cannot be dropped because some objects depend on it/
             retried = true
             e.message =~ /object[s]? in database (.*)$/
+            e.message =~ /privileges for database (.*)$/ unless $1
             if database_with_conflicts == $1
               raise e
             else

From a5079d9c38ae2b9110500e81a310145b4e9f9c50 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Tue, 26 Sep 2017 16:33:54 +0200
Subject: [PATCH 29/53] Change describe block

---
 spec/models/carto/user_migration_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/models/carto/user_migration_spec.rb b/spec/models/carto/user_migration_spec.rb
index e3e7acdf6f..23f99414f0 100644
--- a/spec/models/carto/user_migration_spec.rb
+++ b/spec/models/carto/user_migration_spec.rb
@@ -74,7 +74,7 @@ describe 'UserMigration' do
   it_should_behave_like 'migrating metadata', true
   it_should_behave_like 'migrating metadata', false
 
-  describe 'failing imports should rollback' do
+  describe 'failing user imports should rollback' do
     before :each do
       @user = create_user_with_visualizations
       @carto_user = Carto::User.find(@user.id)

From c79cc29c10d72dfbe804b56c6a601085483c0663 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Tue, 26 Sep 2017 16:34:19 +0200
Subject: [PATCH 30/53] Generate export once per group

---
 spec/models/carto/user_migration_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/models/carto/user_migration_spec.rb b/spec/models/carto/user_migration_spec.rb
index 23f99414f0..a064aab8f1 100644
--- a/spec/models/carto/user_migration_spec.rb
+++ b/spec/models/carto/user_migration_spec.rb
@@ -135,7 +135,7 @@ describe 'UserMigration' do
 
   describe 'failing organization organizations should rollback' do
     include_context 'organization with users helper'
-    before :each do
+    before :all do
       owner = @carto_organization.owner
       filepath = "#{Rails.root}/services/importer/spec/fixtures/visualization_export_with_two_tables.carto"
       data_import = DataImport.create(

From f4d3bc40776b78471d7eb511a8f2801848d4b3ba Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Tue, 26 Sep 2017 16:34:38 +0200
Subject: [PATCH 31/53] Remove organization after each test

---
 spec/models/carto/user_migration_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/models/carto/user_migration_spec.rb b/spec/models/carto/user_migration_spec.rb
index a064aab8f1..8f49da3fb6 100644
--- a/spec/models/carto/user_migration_spec.rb
+++ b/spec/models/carto/user_migration_spec.rb
@@ -159,7 +159,7 @@ describe 'UserMigration' do
     end
 
     after :each do
-      @organization.users.each(&:destroy_cascade)
+      @organization.reload
       @organization.destroy_cascade
     end
 

From 55b2bd24264e891ae123c4ce691dfae92eb84d2c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Tue, 26 Sep 2017 18:40:33 +0200
Subject: [PATCH 32/53] Raise exception if organization already exists

---
 app/models/carto/user_migration_import.rb              | 10 ++++++++++
 .../carto/organization_metadata_export_service.rb      |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/app/models/carto/user_migration_import.rb b/app/models/carto/user_migration_import.rb
index f6368268b9..292e687bc8 100644
--- a/app/models/carto/user_migration_import.rb
+++ b/app/models/carto/user_migration_import.rb
@@ -27,6 +27,7 @@ module Carto
     validate :valid_org_import
 
     def run_import
+      assert_organization_does_not_exist
       log.append('=== Downloading ===')
       update_attributes(state: STATE_DOWNLOADING)
       package = UserMigrationPackage.for_import(id, log)
@@ -55,6 +56,12 @@ module Carto
 
     private
 
+    def assert_organization_does_not_exist
+      if organization.present? && Carto::Organization.where(id: organization.id)
+        raise OrganizationAlreadyExists.new
+      end
+    end
+
     def valid_org_import
       if org_import?
         errors.add(:user_id, "user_id can't be present") if user_id.present?
@@ -110,6 +117,9 @@ module Carto
       log.append('=== Importing metadata ===')
       begin
         imported = service.import_from_directory(package.meta_dir)
+      rescue OrganizationAlreadyExists => e
+        log.append('Organization already exists. Skipping!')
+        raise e
       rescue => e
         log.append('=== Error importing metadata. Rollback! ===')
         service.rollback_import_from_directory(package.meta_dir)
diff --git a/app/services/carto/organization_metadata_export_service.rb b/app/services/carto/organization_metadata_export_service.rb
index 655d8064e6..616c60ac48 100644
--- a/app/services/carto/organization_metadata_export_service.rb
+++ b/app/services/carto/organization_metadata_export_service.rb
@@ -166,6 +166,7 @@ module Carto
     end
   end
 
+  class OrganizationAlreadyExists < RuntimeError; end
   # Both String and Hash versions are provided because `deep_symbolize_keys` won't symbolize through arrays
   # and having separated methods make handling and testing much easier.
   class OrganizationMetadataExportService
@@ -191,6 +192,7 @@ module Carto
     def import_from_directory(meta_path)
       # Import organization
       organization = load_organization_from_directory(meta_path)
+      raise OrganizationAlreadyExists.new if ::Carto::Organization.where(id: organization.id).exists?
 
       organization_redis_file = get_redis_filename(meta_path)
       Carto::RedisExportService.new.restore_redis_from_json_export(File.read(organization_redis_file))

From 9f9e7c8100c491b829bbfde02a83e6edde146223 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Tue, 26 Sep 2017 18:41:00 +0200
Subject: [PATCH 33/53] Add test

---
 spec/models/carto/user_migration_spec.rb | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/spec/models/carto/user_migration_spec.rb b/spec/models/carto/user_migration_spec.rb
index 8f49da3fb6..95021ee215 100644
--- a/spec/models/carto/user_migration_spec.rb
+++ b/spec/models/carto/user_migration_spec.rb
@@ -202,6 +202,11 @@ describe 'UserMigration' do
 
       org_import.run_import.should eq true
     end
+
+    it 'should fail if importing an already existing organization' do
+      org_import.run_import.should eq true
+      org_import.run_import.should eq false
+    end
   end
 
   it 'exports and imports a user with a data import with two tables' do

From 53535aac9c2f5ebdb52274a9f53d0fc0ba867305 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Tue, 26 Sep 2017 18:58:39 +0200
Subject: [PATCH 34/53] Minor fixes

---
 app/models/carto/user_migration_import.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/models/carto/user_migration_import.rb b/app/models/carto/user_migration_import.rb
index 292e687bc8..393a7c99ff 100644
--- a/app/models/carto/user_migration_import.rb
+++ b/app/models/carto/user_migration_import.rb
@@ -47,7 +47,7 @@ module Carto
       update_attributes(state: STATE_FAILURE)
       false
     ensure
-      package.cleanup
+      package.present? && package.cleanup
     end
 
     def enqueue
@@ -57,7 +57,7 @@ module Carto
     private
 
     def assert_organization_does_not_exist
-      if organization.present? && Carto::Organization.where(id: organization.id)
+      if organization.present? && Carto::Organization.where(id: organization.id).any?
         raise OrganizationAlreadyExists.new
       end
     end

From 2ce02fe2516680201ae89cc5dee7251081556349 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Tue, 26 Sep 2017 18:58:58 +0200
Subject: [PATCH 35/53] Add another test

---
 spec/models/carto/user_migration_spec.rb | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/spec/models/carto/user_migration_spec.rb b/spec/models/carto/user_migration_spec.rb
index 95021ee215..11bb2683c4 100644
--- a/spec/models/carto/user_migration_spec.rb
+++ b/spec/models/carto/user_migration_spec.rb
@@ -203,10 +203,17 @@ describe 'UserMigration' do
       org_import.run_import.should eq true
     end
 
-    it 'should fail if importing an already existing organization' do
+    it 'should fail if importing an already existing organization with metadata' do
       org_import.run_import.should eq true
       org_import.run_import.should eq false
     end
+
+    it 'should fail if importing an already existing organization' do
+      imp = org_import
+      imp.stubs(:assert_organization_does_not_exist).raises(Carto::OrganizationAlreadyExists.new)
+      imp.run_import.should eq false
+      org_import.run_import.should eq true
+    end
   end
 
   it 'exports and imports a user with a data import with two tables' do

From b41c2a762ca3b9b864cc25860bfdedbd763f094f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Wed, 27 Sep 2017 11:02:00 +0200
Subject: [PATCH 36/53] Fix test

---
 spec/models/carto/user_migration_import_spec.rb | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/spec/models/carto/user_migration_import_spec.rb b/spec/models/carto/user_migration_import_spec.rb
index d15ac9a385..a36c30cd9e 100644
--- a/spec/models/carto/user_migration_import_spec.rb
+++ b/spec/models/carto/user_migration_import_spec.rb
@@ -18,7 +18,7 @@ describe Carto::UserMigrationImport do
       should_import_metadata_for_user(@user_mock)
       @import.org_import = false
       should_update_database_host_for_users([@user_mock])
-
+  
       @import.run_import
     end
 
@@ -44,6 +44,12 @@ describe Carto::UserMigrationImport do
     end
 
     def setup_mocks
+      @organization_mock = Carto::Organization.new
+      @organization_mock.stubs(:id).returns(:irrelevant_organization_id)
+      @import.stubs(:organization).returns(@organization_mock)
+      query_mock = Object.new
+      query_mock.stubs(:any?).returns(false)
+      Carto::Organization.stubs(:where).with(id: :irrelevant_organization_id).returns(query_mock)
       @user_migration_package_mock = Object.new
       Carto::UserMigrationPackage.stubs(:for_import).returns @user_migration_package_mock
       @user_migration_package_mock.stubs(:download).with(:irrelevant_file)
@@ -52,11 +58,11 @@ describe Carto::UserMigrationImport do
       @user_mock = Carto::User.new
       @export_job_mock = Object.new
       @export_job_mock.expects(:run!).once
+      @export_job_mock.expects(:terminate_connections).once
       CartoDB::DataMover::ImportJob.stubs(:new).returns @export_job_mock
       @user_migration_package_mock.stubs(:cleanup)
       @import.expects(:save!).once.returns @import
 
-      @organization_mock = Carto::Organization.new
     end
 
     def expected_job_arguments

From 89db5daa690d03498349919529f79c4be2d07fb2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Wed, 27 Sep 2017 11:37:50 +0200
Subject: [PATCH 37/53] Hound tips

---
 app/models/carto/user_migration_import.rb                  | 6 +++---
 app/services/carto/organization_metadata_export_service.rb | 2 +-
 app/services/carto/user_metadata_export_service.rb         | 4 ++--
 services/user-mover/import_user.rb                         | 1 -
 spec/models/carto/user_migration_import_spec.rb            | 2 +-
 spec/models/carto/user_migration_spec.rb                   | 2 +-
 6 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/app/models/carto/user_migration_import.rb b/app/models/carto/user_migration_import.rb
index 393a7c99ff..289e27a7b2 100644
--- a/app/models/carto/user_migration_import.rb
+++ b/app/models/carto/user_migration_import.rb
@@ -107,9 +107,9 @@ module Carto
 
     def rollback_import_data
       @import_job.add_options(rollback: true,
-                             mode: :rollback,
-                             drop_database: true,
-                             drop_roles: true)
+                              mode: :rollback,
+                              drop_database: true,
+                              drop_roles: true)
       @import_job.rollback!
     end
 
diff --git a/app/services/carto/organization_metadata_export_service.rb b/app/services/carto/organization_metadata_export_service.rb
index 616c60ac48..082475764e 100644
--- a/app/services/carto/organization_metadata_export_service.rb
+++ b/app/services/carto/organization_metadata_export_service.rb
@@ -244,7 +244,7 @@ module Carto
     def get_redis_filename(meta_path)
       Dir["#{meta_path}/redis_organization_*.json"].first
     end
-    
+
     def load_organization_from_directory(meta_path)
       organization_file = Dir["#{meta_path}/organization_*.json"].first
       organization = build_organization_from_json_export(File.read(organization_file))
diff --git a/app/services/carto/user_metadata_export_service.rb b/app/services/carto/user_metadata_export_service.rb
index 9541289691..1aa939dd04 100644
--- a/app/services/carto/user_metadata_export_service.rb
+++ b/app/services/carto/user_metadata_export_service.rb
@@ -212,8 +212,8 @@ module Carto
       user && Carto::User.find(user.id).delete
 
       Carto::RedisExportService.new.remove_redis_from_json_export(redis_user_file(path))
-    rescue ActiveRecord::RecordNotFound => e
-      #User was not created so not found and no redis removal needed
+    rescue ActiveRecord::RecordNotFound
+      # User was not created so not found and no redis removal needed
     end
 
     def import_user_visualizations_from_directory(user, type, meta_path)
diff --git a/services/user-mover/import_user.rb b/services/user-mover/import_user.rb
index fba60ec277..46c68cacac 100644
--- a/services/user-mover/import_user.rb
+++ b/services/user-mover/import_user.rb
@@ -303,7 +303,6 @@ module CartoDB
       end
 
       def close_all_database_connections(database_name = @target_dbname)
-
         superuser_pg_conn.query("SELECT pg_terminate_backend(pg_stat_activity.pid)
                                   FROM pg_stat_activity
                                 WHERE pg_stat_activity.datname = '#{database_name}'
diff --git a/spec/models/carto/user_migration_import_spec.rb b/spec/models/carto/user_migration_import_spec.rb
index a36c30cd9e..f6faa70b23 100644
--- a/spec/models/carto/user_migration_import_spec.rb
+++ b/spec/models/carto/user_migration_import_spec.rb
@@ -18,7 +18,7 @@ describe Carto::UserMigrationImport do
       should_import_metadata_for_user(@user_mock)
       @import.org_import = false
       should_update_database_host_for_users([@user_mock])
-  
+
       @import.run_import
     end
 
diff --git a/spec/models/carto/user_migration_spec.rb b/spec/models/carto/user_migration_spec.rb
index 11bb2683c4..7f13ef2ff2 100644
--- a/spec/models/carto/user_migration_spec.rb
+++ b/spec/models/carto/user_migration_spec.rb
@@ -406,7 +406,7 @@ describe 'UserMigration' do
 
     data_import.run_import!
     data_import.success.should eq true
-    return user
+    user
   end
 
   def org_import

From 82079f5f73f1183b0b0b045d8ed91781e84dc1f4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartinj@gmail.com>
Date: Wed, 27 Sep 2017 11:42:02 +0200
Subject: [PATCH 38/53] Update organization_metadata_export_service.rb

Remove unused var
---
 app/services/carto/organization_metadata_export_service.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/services/carto/organization_metadata_export_service.rb b/app/services/carto/organization_metadata_export_service.rb
index 082475764e..9f646fc837 100644
--- a/app/services/carto/organization_metadata_export_service.rb
+++ b/app/services/carto/organization_metadata_export_service.rb
@@ -247,7 +247,7 @@ module Carto
 
     def load_organization_from_directory(meta_path)
       organization_file = Dir["#{meta_path}/organization_*.json"].first
-      organization = build_organization_from_json_export(File.read(organization_file))
+      build_organization_from_json_export(File.read(organization_file))
     end
 
     def import_metadata_from_directory(organization, path)

From 05bd8738f4612500266b27b410dc4de9ddc44165 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Wed, 27 Sep 2017 18:31:49 +0200
Subject: [PATCH 39/53] CR Comments

---
 app/models/carto/user_migration_import.rb     | 73 ++++++++++---------
 .../organization_metadata_export_service.rb   |  2 +-
 .../carto/user_metadata_export_service.rb     |  2 +
 3 files changed, 43 insertions(+), 34 deletions(-)

diff --git a/app/models/carto/user_migration_import.rb b/app/models/carto/user_migration_import.rb
index 289e27a7b2..190e6ace3b 100644
--- a/app/models/carto/user_migration_import.rb
+++ b/app/models/carto/user_migration_import.rb
@@ -28,6 +28,7 @@ module Carto
 
     def run_import
       assert_organization_does_not_exist
+      assert_user_does_not_exist
       log.append('=== Downloading ===')
       update_attributes(state: STATE_DOWNLOADING)
       package = UserMigrationPackage.for_import(id, log)
@@ -47,7 +48,7 @@ module Carto
       update_attributes(state: STATE_FAILURE)
       false
     ensure
-      package.present? && package.cleanup
+      package.try(:cleanup)
     end
 
     def enqueue
@@ -57,9 +58,11 @@ module Carto
     private
 
     def assert_organization_does_not_exist
-      if organization.present? && Carto::Organization.where(id: organization.id).any?
-        raise OrganizationAlreadyExists.new
-      end
+      raise OrganizationAlreadyExists.new if organization.present? && Carto::Organization.exists?(organization.id)
+    end
+
+    def assert_user_does_not_exist
+      raise UserAlreadyExists.new if user.present && Carto::User.exists?(user.id)
     end
 
     def valid_org_import
@@ -71,64 +74,68 @@ module Carto
     end
 
     def import(service, package)
-      @import_job = CartoDB::DataMover::ImportJob.new(import_job_arguments(package.data_dir))
       imported = do_import_metadata(package, service) if import_metadata?
       do_import_data(package, service)
       import_visualizations(imported, package, service) if import_metadata?
     end
 
-    def import_visualizations(imported, package, service)
-      log.append('=== Importing visualizations and search tweets ===')
+    def do_import_metadata(package, service)
+      log.append('=== Importing metadata ===')
       begin
-        ActiveRecord::Base.transaction do
-          service.import_metadata_from_directory(imported, package.meta_dir)
-        end
+        imported = service.import_from_directory(package.meta_dir)
+      rescue OrganizationAlreadyExists, OrganizationAlreadyExists => e
+        log.append('Organization already exists. Skipping!')
+        raise e
       rescue => e
-        log.append('=== Error importing visualizations and search tweets. Rollback! ===')
-        rollback_import_data
+        log.append('=== Error importing metadata. Rollback! ===')
         service.rollback_import_from_directory(package.meta_dir)
         raise e
       end
+      org_import? ? self.organization = imported : self.user = imported
+      update_database_host
+      save!
+      imported
     end
 
     def do_import_data(package, service)
       log.append('=== Importing data ===')
+      import_job = CartoDB::DataMover::ImportJob.new(import_job_arguments(package.data_dir))
       begin
-        @import_job.run!
+        import_job.run!
       rescue => e
         log.append('=== Error importing data. Rollback! ===')
         rollback_import_data
         service.rollback_import_from_directory(package.meta_dir) if import_metadata?
         raise e
       ensure
-        @import_job.terminate_connections
+        import_job.terminate_connections
       end
     end
 
-    def rollback_import_data
-      @import_job.add_options(rollback: true,
-                              mode: :rollback,
-                              drop_database: true,
-                              drop_roles: true)
-      @import_job.rollback!
-    end
-
-    def do_import_metadata(package, service)
-      log.append('=== Importing metadata ===')
+    def import_visualizations(imported, package, service)
+      log.append('=== Importing visualizations and search tweets ===')
       begin
-        imported = service.import_from_directory(package.meta_dir)
-      rescue OrganizationAlreadyExists => e
-        log.append('Organization already exists. Skipping!')
-        raise e
+        ActiveRecord::Base.transaction do
+          service.import_metadata_from_directory(imported, package.meta_dir)
+        end
       rescue => e
-        log.append('=== Error importing metadata. Rollback! ===')
+        log.append('=== Error importing visualizations and search tweets. Rollback! ===')
+        rollback_import_data
         service.rollback_import_from_directory(package.meta_dir)
         raise e
       end
-      org_import? ? self.organization = imported : self.user = imported
-      update_database_host
-      save!
-      imported
+    end
+
+    def rollback_import_data
+      import_job = CartoDB::DataMover::ImportJob.new(
+        import_job_arguments(package.data_dir).merge!(rollback: true,
+                                                              mode: :rollback,
+                                                              drop_database: true,
+                                                              drop_roles: true)
+      )
+
+      import_job.rollback!
+      import_job.terminate_connections;
     end
 
     def update_database_host
diff --git a/app/services/carto/organization_metadata_export_service.rb b/app/services/carto/organization_metadata_export_service.rb
index 9f646fc837..b71c9ef301 100644
--- a/app/services/carto/organization_metadata_export_service.rb
+++ b/app/services/carto/organization_metadata_export_service.rb
@@ -192,7 +192,7 @@ module Carto
     def import_from_directory(meta_path)
       # Import organization
       organization = load_organization_from_directory(meta_path)
-      raise OrganizationAlreadyExists.new if ::Carto::Organization.where(id: organization.id).exists?
+      raise OrganizationAlreadyExists.new if ::Carto::Organization.exists?(id: organization.id)
 
       organization_redis_file = get_redis_filename(meta_path)
       Carto::RedisExportService.new.restore_redis_from_json_export(File.read(organization_redis_file))
diff --git a/app/services/carto/user_metadata_export_service.rb b/app/services/carto/user_metadata_export_service.rb
index 1aa939dd04..b93bbd8483 100644
--- a/app/services/carto/user_metadata_export_service.rb
+++ b/app/services/carto/user_metadata_export_service.rb
@@ -176,6 +176,8 @@ module Carto
     end
   end
 
+  class UserAlreadyExists < RuntimeError; end
+
   # Both String and Hash versions are provided because `deep_symbolize_keys` won't symbolize through arrays
   # and having separated methods make handling and testing much easier.
   class UserMetadataExportService

From 74f3f3262701c811c79a8ae03866ec00d297511f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Thu, 28 Sep 2017 11:18:42 +0200
Subject: [PATCH 40/53] Fix tests

---
 app/models/carto/user_migration_import.rb       |  8 ++++----
 spec/models/carto/user_migration_import_spec.rb |  7 ++-----
 spec/models/carto/user_migration_spec.rb        | 17 +++++++++++++++--
 3 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/app/models/carto/user_migration_import.rb b/app/models/carto/user_migration_import.rb
index 190e6ace3b..d6e03ce1be 100644
--- a/app/models/carto/user_migration_import.rb
+++ b/app/models/carto/user_migration_import.rb
@@ -104,7 +104,7 @@ module Carto
         import_job.run!
       rescue => e
         log.append('=== Error importing data. Rollback! ===')
-        rollback_import_data
+        rollback_import_data(package)
         service.rollback_import_from_directory(package.meta_dir) if import_metadata?
         raise e
       ensure
@@ -120,13 +120,13 @@ module Carto
         end
       rescue => e
         log.append('=== Error importing visualizations and search tweets. Rollback! ===')
-        rollback_import_data
+        rollback_import_data(package)
         service.rollback_import_from_directory(package.meta_dir)
         raise e
       end
     end
 
-    def rollback_import_data
+    def rollback_import_data(package)
       import_job = CartoDB::DataMover::ImportJob.new(
         import_job_arguments(package.data_dir).merge!(rollback: true,
                                                               mode: :rollback,
@@ -134,7 +134,7 @@ module Carto
                                                               drop_roles: true)
       )
 
-      import_job.rollback!
+      import_job.run!
       import_job.terminate_connections;
     end
 
diff --git a/spec/models/carto/user_migration_import_spec.rb b/spec/models/carto/user_migration_import_spec.rb
index f6faa70b23..5ecff377cf 100644
--- a/spec/models/carto/user_migration_import_spec.rb
+++ b/spec/models/carto/user_migration_import_spec.rb
@@ -45,11 +45,8 @@ describe Carto::UserMigrationImport do
 
     def setup_mocks
       @organization_mock = Carto::Organization.new
-      @organization_mock.stubs(:id).returns(:irrelevant_organization_id)
-      @import.stubs(:organization).returns(@organization_mock)
-      query_mock = Object.new
-      query_mock.stubs(:any?).returns(false)
-      Carto::Organization.stubs(:where).with(id: :irrelevant_organization_id).returns(query_mock)
+      @import.stubs(:assert_organization_does_not_exist)
+      @import.stubs(:assert_user_does_not_exist)
       @user_migration_package_mock = Object.new
       Carto::UserMigrationPackage.stubs(:for_import).returns @user_migration_package_mock
       @user_migration_package_mock.stubs(:download).with(:irrelevant_file)
diff --git a/spec/models/carto/user_migration_spec.rb b/spec/models/carto/user_migration_spec.rb
index 7f13ef2ff2..c183761704 100644
--- a/spec/models/carto/user_migration_spec.rb
+++ b/spec/models/carto/user_migration_spec.rb
@@ -50,6 +50,8 @@ describe 'UserMigration' do
         json_file: export.json_file,
         import_metadata: migrate_metadata
       )
+      import.stubs(:assert_organization_does_not_exist)
+      import.stubs(:assert_user_does_not_exist)
       import.run_import
 
       puts import.log.entries if import.state != Carto::UserMigrationImport::STATE_COMPLETE
@@ -245,6 +247,8 @@ describe 'UserMigration' do
       json_file: export.json_file,
       import_metadata: true
     )
+    import.stubs(:assert_organization_does_not_exist)
+    import.stubs(:assert_user_does_not_exist)
     import.run_import
 
     puts import.log.entries if import.state != Carto::UserMigrationImport::STATE_COMPLETE
@@ -341,6 +345,8 @@ describe 'UserMigration' do
           json_file: export.json_file,
           import_metadata: migrate_metadata
         )
+        import.stubs(:assert_organization_does_not_exist)
+        import.stubs(:assert_user_does_not_exist)
         import.run_import
 
         puts import.log.entries if import.state != Carto::UserMigrationImport::STATE_COMPLETE
@@ -410,23 +416,30 @@ describe 'UserMigration' do
   end
 
   def org_import
-    Carto::UserMigrationImport.create(
+    imp = Carto::UserMigrationImport.create(
       exported_file: @export.exported_file,
       database_host: @carto_organization.owner.attributes['database_host'],
       org_import: true,
       json_file: @export.json_file,
       import_metadata: true
     )
+
+    imp.stubs(:assert_organization_does_not_exist)
+    imp.stubs(:assert_user_does_not_exist)
+    imp
   end
 
   def import
-    Carto::UserMigrationImport.create(
+    imp = Carto::UserMigrationImport.create(
       exported_file: @export.exported_file,
       database_host: @user_attributes['database_host'],
       org_import: false,
       json_file: @export.json_file,
       import_metadata: true
     )
+    imp.stubs(:assert_organization_does_not_exist)
+    imp.stubs(:assert_user_does_not_exist)
+    imp
   end
 
   def destroy_user

From 353f7c84bfea7159e60ba527f38bb9882392ed3f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Thu, 28 Sep 2017 13:12:17 +0200
Subject: [PATCH 41/53] Skip table dropping

---
 app/models/user.rb | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 2e0e9f38a0..8c027f7b99 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -465,6 +465,18 @@ class User < Sequel::Model
     invalidate_varnish_cache
 
     # Delete the DB or the schema
+    drop_tables(error_happened, has_organization) unless skip_table_drop
+
+    # Remove metadata from redis last (to avoid cutting off access to SQL API if db deletion fails)
+    unless error_happened
+      $users_metadata.DEL(key)
+      $users_metadata.DEL(timeout_key)
+    end
+
+    feature_flags_user.each(&:delete)
+  end
+
+  def drop_tables(error_happened, has_organization)
     if has_organization
       unless error_happened
         db_service.drop_organization_user(
@@ -484,14 +496,6 @@ class User < Sequel::Model
       }.join
       db_service.monitor_user_notification
     end
-
-    # Remove metadata from redis last (to avoid cutting off access to SQL API if db deletion fails)
-    unless error_happened
-      $users_metadata.DEL(key)
-      $users_metadata.DEL(timeout_key)
-    end
-
-    feature_flags_user.each(&:delete)
   end
 
   def delete_external_data_imports

From 850ba77a08c84702f38465d2f4555a2912e737b9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Thu, 28 Sep 2017 13:12:39 +0200
Subject: [PATCH 42/53] Call before_destroy

---
 app/services/carto/user_metadata_export_service.rb | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/app/services/carto/user_metadata_export_service.rb b/app/services/carto/user_metadata_export_service.rb
index b93bbd8483..3cb89e4c54 100644
--- a/app/services/carto/user_metadata_export_service.rb
+++ b/app/services/carto/user_metadata_export_service.rb
@@ -210,8 +210,11 @@ module Carto
 
     def rollback_import_from_directory(path)
       user = user_from_file(path)
+      return unless user
 
-      user && Carto::User.find(user.id).delete
+      user = ::User[user.id]
+      Carto::User.find(user.id).destroy
+      user.before_destroy(skip_table_drop: true)
 
       Carto::RedisExportService.new.remove_redis_from_json_export(redis_user_file(path))
     rescue ActiveRecord::RecordNotFound

From e591ca290a4734b785e594684a4bf20d07a9b1de Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Thu, 28 Sep 2017 16:38:12 +0200
Subject: [PATCH 43/53] Add argument

---
 app/models/user.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 8c027f7b99..0f1b60dac7 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -399,7 +399,7 @@ class User < Sequel::Model
     @force_destroy = true
   end
 
-  def before_destroy
+  def before_destroy(skip_table_drop: true)
     ensure_nonviewer
 
     @org_id_for_org_wipe = nil

From 749f7da6f90630b74f3f99e060180bef2a6a9b89 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Thu, 28 Sep 2017 16:38:48 +0200
Subject: [PATCH 44/53] Change default value

---
 app/models/user.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 0f1b60dac7..0f45dc90c9 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -399,7 +399,7 @@ class User < Sequel::Model
     @force_destroy = true
   end
 
-  def before_destroy(skip_table_drop: true)
+  def before_destroy(skip_table_drop: false)
     ensure_nonviewer
 
     @org_id_for_org_wipe = nil

From eea7295d0e326498b821d930921dcbebb9a752f7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Thu, 28 Sep 2017 16:43:21 +0200
Subject: [PATCH 45/53] Method rename

---
 app/services/carto/organization_metadata_export_service.rb | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/services/carto/organization_metadata_export_service.rb b/app/services/carto/organization_metadata_export_service.rb
index b71c9ef301..caca31ff4b 100644
--- a/app/services/carto/organization_metadata_export_service.rb
+++ b/app/services/carto/organization_metadata_export_service.rb
@@ -194,7 +194,7 @@ module Carto
       organization = load_organization_from_directory(meta_path)
       raise OrganizationAlreadyExists.new if ::Carto::Organization.exists?(id: organization.id)
 
-      organization_redis_file = get_redis_filename(meta_path)
+      organization_redis_file = redis_filename(meta_path)
       Carto::RedisExportService.new.restore_redis_from_json_export(File.read(organization_redis_file))
 
       # Groups and notifications must be saved after users
@@ -220,7 +220,7 @@ module Carto
     end
 
     def rollback_import_from_directory(meta_path)
-      organization_redis_file = get_redis_filename(meta_path)
+      organization_redis_file = redis_filename(meta_path)
       Carto::RedisExportService.new.remove_redis_from_json_export(File.read(organization_redis_file))
       organization = load_organization_from_directory(meta_path)
 
@@ -241,7 +241,7 @@ module Carto
       Dir["#{meta_path}/user_*"]
     end
 
-    def get_redis_filename(meta_path)
+    def redis_filename(meta_path)
       Dir["#{meta_path}/redis_organization_*.json"].first
     end
 

From aa4da814578eccff72b6d058add229cc5c1b40bd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Thu, 28 Sep 2017 17:24:40 +0200
Subject: [PATCH 46/53] Raise exception if imported user already exists

---
 app/services/carto/user_metadata_export_service.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/services/carto/user_metadata_export_service.rb b/app/services/carto/user_metadata_export_service.rb
index 3cb89e4c54..d8160c8b2f 100644
--- a/app/services/carto/user_metadata_export_service.rb
+++ b/app/services/carto/user_metadata_export_service.rb
@@ -201,6 +201,7 @@ module Carto
 
     def import_from_directory(path)
       user = user_from_file(path)
+      raise UserAlreadyExists.new if ::Carto::User.exists?(id: user.id)
       save_imported_user(user)
 
       Carto::RedisExportService.new.restore_redis_from_json_export(redis_user_file(path))

From b25870e930b2a0a1a469ee0c3ae93c21fb41adc0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Fri, 29 Sep 2017 12:44:38 +0200
Subject: [PATCH 47/53] CR comments

---
 app/models/carto/user_migration_import.rb | 18 ++-------
 app/models/user.rb                        | 19 ++++------
 services/user-mover/import_user.rb        |  4 --
 spec/models/carto/user_migration_spec.rb  | 46 +++++++++++++++--------
 4 files changed, 42 insertions(+), 45 deletions(-)

diff --git a/app/models/carto/user_migration_import.rb b/app/models/carto/user_migration_import.rb
index d6e03ce1be..6b79d9eb92 100644
--- a/app/models/carto/user_migration_import.rb
+++ b/app/models/carto/user_migration_import.rb
@@ -27,8 +27,6 @@ module Carto
     validate :valid_org_import
 
     def run_import
-      assert_organization_does_not_exist
-      assert_user_does_not_exist
       log.append('=== Downloading ===')
       update_attributes(state: STATE_DOWNLOADING)
       package = UserMigrationPackage.for_import(id, log)
@@ -57,14 +55,6 @@ module Carto
 
     private
 
-    def assert_organization_does_not_exist
-      raise OrganizationAlreadyExists.new if organization.present? && Carto::Organization.exists?(organization.id)
-    end
-
-    def assert_user_does_not_exist
-      raise UserAlreadyExists.new if user.present && Carto::User.exists?(user.id)
-    end
-
     def valid_org_import
       if org_import?
         errors.add(:user_id, "user_id can't be present") if user_id.present?
@@ -128,10 +118,10 @@ module Carto
 
     def rollback_import_data(package)
       import_job = CartoDB::DataMover::ImportJob.new(
-        import_job_arguments(package.data_dir).merge!(rollback: true,
-                                                              mode: :rollback,
-                                                              drop_database: true,
-                                                              drop_roles: true)
+        import_job_arguments(package.data_dir).merge(rollback: true,
+                                                             mode: :rollback,
+                                                             drop_database: true,
+                                                             drop_roles: true)
       )
 
       import_job.run!
diff --git a/app/models/user.rb b/app/models/user.rb
index 0f45dc90c9..12f62e4c60 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -464,8 +464,7 @@ class User < Sequel::Model
     # Invalidate user cache
     invalidate_varnish_cache
 
-    # Delete the DB or the schema
-    drop_tables(error_happened, has_organization) unless skip_table_drop
+    drop_database(has_organization) unless skip_table_drop || error_happened
 
     # Remove metadata from redis last (to avoid cutting off access to SQL API if db deletion fails)
     unless error_happened
@@ -476,19 +475,17 @@ class User < Sequel::Model
     feature_flags_user.each(&:delete)
   end
 
-  def drop_tables(error_happened, has_organization)
+  def drop_database(has_organization)
     if has_organization
-      unless error_happened
-        db_service.drop_organization_user(
-          organization_id,
-          is_owner: !@org_id_for_org_wipe.nil?,
-          force_destroy: @force_destroy
-        )
-      end
+      db_service.drop_organization_user(
+        organization_id,
+        is_owner: !@org_id_for_org_wipe.nil?,
+        force_destroy: @force_destroy
+      )
     elsif ::User.where(database_name: database_name).count > 1
       raise CartoDB::BaseCartoDBError.new(
         'The user is not supposed to be in a organization but another user has the same database_name. Not dropping it')
-    elsif !error_happened
+    else
       Thread.new {
         conn = in_database(as: :cluster_admin)
         db_service.drop_database_and_user(conn)
diff --git a/services/user-mover/import_user.rb b/services/user-mover/import_user.rb
index 46c68cacac..0a604e9993 100644
--- a/services/user-mover/import_user.rb
+++ b/services/user-mover/import_user.rb
@@ -53,10 +53,6 @@ module CartoDB
                        }
       end
 
-      def add_options(options)
-        @options.merge!(options)
-      end
-
       def run!
         if @pack_config['organization']
           process_org
diff --git a/spec/models/carto/user_migration_spec.rb b/spec/models/carto/user_migration_spec.rb
index c183761704..36b4ee90b8 100644
--- a/spec/models/carto/user_migration_spec.rb
+++ b/spec/models/carto/user_migration_spec.rb
@@ -50,8 +50,6 @@ describe 'UserMigration' do
         json_file: export.json_file,
         import_metadata: migrate_metadata
       )
-      import.stubs(:assert_organization_does_not_exist)
-      import.stubs(:assert_user_does_not_exist)
       import.run_import
 
       puts import.log.entries if import.state != Carto::UserMigrationImport::STATE_COMPLETE
@@ -97,7 +95,9 @@ describe 'UserMigration' do
     it 'import failing in import_metadata should rollback' do
       Carto::RedisExportService.any_instance.stubs(:restore_redis_from_hash_export).raises('Some exception')
 
-      import.run_import.should eq false
+      imp = import
+      imp.run_import.should eq false
+      imp.state.should eq 'failure'
 
       Carto::RedisExportService.any_instance.unstub(:restore_redis_from_hash_export)
 
@@ -107,7 +107,9 @@ describe 'UserMigration' do
     it 'import failing in JobImport#run!' do
       CartoDB::DataMover::ImportJob.any_instance.stubs(:grant_user_role).raises('Some exception')
 
-      import.run_import.should eq false
+      imp = import
+      imp.run_import.should eq false
+      imp.state.should eq 'failure'
 
       CartoDB::DataMover::ImportJob.any_instance.unstub(:grant_user_role)
 
@@ -117,7 +119,9 @@ describe 'UserMigration' do
     it 'import failing creating user database and roles' do
       CartoDB::DataMover::ImportJob.any_instance.stubs(:import_pgdump).raises('Some exception')
 
-      import.run_import.should eq false
+      imp = import
+      imp.run_import.should eq false
+      imp.state.should eq 'failure'
 
       CartoDB::DataMover::ImportJob.any_instance.unstub(:import_pgdump)
 
@@ -127,12 +131,19 @@ describe 'UserMigration' do
     it 'import failing importing visualizations' do
       Carto::UserMetadataExportService.any_instance.stubs(:import_search_tweets_from_directory).raises('Some exception')
 
-      import.run_import.should eq false
+      imp = import
+      imp.run_import.should eq false
+      imp.state.should eq 'failure'
 
       Carto::UserMetadataExportService.any_instance.unstub(:import_search_tweets_from_directory)
 
       import.run_import.should eq true
     end
+
+    it 'fails importing an already existing user' do
+      import.run_import.should eq true
+      import.run_import.should eq false
+    end
   end
 
   describe 'failing organization organizations should rollback' do
@@ -168,7 +179,9 @@ describe 'UserMigration' do
     it 'import failing in import_metadata should rollback' do
       Carto::RedisExportService.any_instance.stubs(:restore_redis_from_hash_export).raises('Some exception')
 
-      org_import.run_import.should eq false
+      imp = org_import
+      imp.run_import.should eq false
+      imp.state.should eq 'failure'
 
       Carto::RedisExportService.any_instance.unstub(:restore_redis_from_hash_export)
 
@@ -178,7 +191,9 @@ describe 'UserMigration' do
     it 'import failing in JobImport#run!' do
       CartoDB::DataMover::ImportJob.any_instance.stubs(:grant_user_role).raises('Some exception')
 
-      org_import.run_import.should eq false
+      imp = org_import
+      imp.run_import.should eq false
+      imp.state.should eq 'failure'
 
       CartoDB::DataMover::ImportJob.any_instance.unstub(:grant_user_role)
 
@@ -188,7 +203,9 @@ describe 'UserMigration' do
     it 'import failing creating user database and roles' do
       CartoDB::DataMover::ImportJob.any_instance.stubs(:import_pgdump).raises('Some exception')
 
-      org_import.run_import.should eq false
+      imp = org_import
+      imp.run_import.should eq false
+      imp.state.should eq 'failure'
 
       CartoDB::DataMover::ImportJob.any_instance.unstub(:import_pgdump)
 
@@ -198,7 +215,9 @@ describe 'UserMigration' do
     it 'import failing importing visualizations' do
       Carto::UserMetadataExportService.any_instance.stubs(:import_search_tweets_from_directory).raises('Some exception')
 
-      org_import.run_import.should eq false
+      imp = org_import
+      imp.run_import.should eq false
+      imp.state.should eq 'failure'
 
       Carto::UserMetadataExportService.any_instance.unstub(:import_search_tweets_from_directory)
 
@@ -207,14 +226,9 @@ describe 'UserMigration' do
 
     it 'should fail if importing an already existing organization with metadata' do
       org_import.run_import.should eq true
-      org_import.run_import.should eq false
-    end
-
-    it 'should fail if importing an already existing organization' do
       imp = org_import
-      imp.stubs(:assert_organization_does_not_exist).raises(Carto::OrganizationAlreadyExists.new)
       imp.run_import.should eq false
-      org_import.run_import.should eq true
+      imp.state.should eq 'failure'
     end
   end
 

From 7ef450647749f62f391519f4d46ef456080ce894 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=83=C2=ADn?= <amartin@carto.com>
Date: Fri, 29 Sep 2017 15:00:46 +0200
Subject: [PATCH 48/53] Hound

---
 app/models/carto/user_migration_import.rb | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/app/models/carto/user_migration_import.rb b/app/models/carto/user_migration_import.rb
index 6b79d9eb92..61674f5c01 100644
--- a/app/models/carto/user_migration_import.rb
+++ b/app/models/carto/user_migration_import.rb
@@ -119,13 +119,13 @@ module Carto
     def rollback_import_data(package)
       import_job = CartoDB::DataMover::ImportJob.new(
         import_job_arguments(package.data_dir).merge(rollback: true,
-                                                             mode: :rollback,
-                                                             drop_database: true,
-                                                             drop_roles: true)
+                                                     mode: :rollback,
+                                                     drop_database: true,
+                                                     drop_roles: true)
       )
 
       import_job.run!
-      import_job.terminate_connections;
+      import_job.terminate_connections
     end
 
     def update_database_host
@@ -133,8 +133,9 @@ module Carto
         Rollbar.info("Updating database conection for user #{user.username} to #{database_host}")
         user.database_host = database_host
         user.save!
-        ::User[user.id].reload # This is because Sequel models are being cached along request. This forces reload.
+        # This is because Sequel models are being cached along request. This forces reload.
         # It's being used in visualizations_export_persistence_service.rb#save_import
+        ::User[user.id].reload
       end
     end
 

From cc8b36eeab934fb0eebb78a29ed48ccdfcb1ed0f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Fri, 29 Sep 2017 16:51:18 +0200
Subject: [PATCH 49/53] Fix

---
 app/services/carto/organization_metadata_export_service.rb | 2 +-
 app/services/carto/user_metadata_export_service.rb         | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/app/services/carto/organization_metadata_export_service.rb b/app/services/carto/organization_metadata_export_service.rb
index caca31ff4b..b5f536430a 100644
--- a/app/services/carto/organization_metadata_export_service.rb
+++ b/app/services/carto/organization_metadata_export_service.rb
@@ -228,9 +228,9 @@ module Carto
       user_list.map do |user_path|
         Carto::UserMetadataExportService.new.rollback_import_from_directory(user_path)
       end
+      return unless Carto::Organization.exists?(organization.id)
 
       organization = Carto::Organization.find(organization.id)
-      return unless organization
       organization.groups.delete
       organization.notifications.delete
       organization.users.delete
diff --git a/app/services/carto/user_metadata_export_service.rb b/app/services/carto/user_metadata_export_service.rb
index d8160c8b2f..7ec345f3e2 100644
--- a/app/services/carto/user_metadata_export_service.rb
+++ b/app/services/carto/user_metadata_export_service.rb
@@ -214,6 +214,8 @@ module Carto
       return unless user
 
       user = ::User[user.id]
+      return unless user
+
       Carto::User.find(user.id).destroy
       user.before_destroy(skip_table_drop: true)
 

From 32fbda1cad6945f22a9a867b59575fdcf6213a19 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Tue, 3 Oct 2017 12:22:18 +0200
Subject: [PATCH 50/53] Prevent rollback from failing

---
 app/models/carto/user_migration_import.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/models/carto/user_migration_import.rb b/app/models/carto/user_migration_import.rb
index 61674f5c01..8972ae7e1d 100644
--- a/app/models/carto/user_migration_import.rb
+++ b/app/models/carto/user_migration_import.rb
@@ -126,6 +126,8 @@ module Carto
 
       import_job.run!
       import_job.terminate_connections
+    rescue => e
+      log.append('There was an error while rolling back import data:' + e.to_s)
     end
 
     def update_database_host

From 12eeb5674171abe4ca14154ed4ff2c02ad08aa94 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Tue, 3 Oct 2017 12:22:27 +0200
Subject: [PATCH 51/53] Fail if user exists

---
 app/models/carto/user_migration_import.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/carto/user_migration_import.rb b/app/models/carto/user_migration_import.rb
index 8972ae7e1d..b0fc35276a 100644
--- a/app/models/carto/user_migration_import.rb
+++ b/app/models/carto/user_migration_import.rb
@@ -73,7 +73,7 @@ module Carto
       log.append('=== Importing metadata ===')
       begin
         imported = service.import_from_directory(package.meta_dir)
-      rescue OrganizationAlreadyExists, OrganizationAlreadyExists => e
+      rescue UserAlreadyExists, OrganizationAlreadyExists => e
         log.append('Organization already exists. Skipping!')
         raise e
       rescue => e

From 8ab8203d59e609747b110a1c547748805e872fa0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Tue, 3 Oct 2017 12:22:32 +0200
Subject: [PATCH 52/53] Add tests

---
 spec/models/carto/user_migration_spec.rb | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/spec/models/carto/user_migration_spec.rb b/spec/models/carto/user_migration_spec.rb
index 36b4ee90b8..82ddc0e94a 100644
--- a/spec/models/carto/user_migration_spec.rb
+++ b/spec/models/carto/user_migration_spec.rb
@@ -144,6 +144,21 @@ describe 'UserMigration' do
       import.run_import.should eq true
       import.run_import.should eq false
     end
+
+    it 'should continue with rollback if data import rollback fails' do
+      CartoDB::DataMover::ImportJob.any_instance.stubs(:grant_user_role).raises('Some exception')
+      CartoDB::DataMover::ImportJob.any_instance.stubs(:rollback_user).raises('Some exception')
+      import.run_import.should eq false
+      CartoDB::DataMover::ImportJob.any_instance.unstub(:grant_user_role)
+      CartoDB::DataMover::ImportJob.any_instance.unstub(:rollback_user)
+      import.run_import.should eq true
+    end
+
+    it 'should not remove user if already exists while importing' do
+      import.run_import.should eq true
+      import.run_import.should eq false
+      Carto::User.exists?(@user.id).should eq true
+    end
   end
 
   describe 'failing organization organizations should rollback' do

From e9e8f973ce3f8d6efe7b8561671da4c784ec3832 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Mart=C3=ADn?= <amartin@carto.com>
Date: Tue, 3 Oct 2017 18:19:51 +0200
Subject: [PATCH 53/53] Avoid test from failing if database does not exist

---
 spec/models/carto/user_migration_spec.rb | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/spec/models/carto/user_migration_spec.rb b/spec/models/carto/user_migration_spec.rb
index 82ddc0e94a..2b854bca91 100644
--- a/spec/models/carto/user_migration_spec.rb
+++ b/spec/models/carto/user_migration_spec.rb
@@ -187,8 +187,11 @@ describe 'UserMigration' do
     end
 
     after :each do
-      @organization.reload
-      @organization.destroy_cascade
+      begin
+        @organization.reload
+        @organization.destroy_cascade
+      rescue
+      end
     end
 
     it 'import failing in import_metadata should rollback' do