Fixing review issues

Author: tclaus

app/controllers/users_controller.rb

@@ -252,7 +252,7 @@ def copy_import_files(user_data)
   end
 
   def copy_import_file(tmp_file)
-    return if tmp_file.empty?
+    return if tmp_file.blank?
 
     file_path_to_save_to = Rails.root.join("public", "uploads", "users",
                                            "#{current_user.username}_#{tmp_file.original_filename}")

app/services/import_service.rb

@@ -3,30 +3,35 @@
 class ImportService
   include Diaspora::Logging
 
-  def import_by_user(user_name, import_parameters)
+  def import_by_user(user, import_parameters)
     profile_path = import_parameters["profile_path"]
     photos_path = import_parameters["photos_path"]
 
-    import_by_files(profile_path, photos_path, user_name)
+    import_by_files(user, profile_path, photos_path)
   end
 
-  def import_by_files(path_to_profile, path_to_photos, username, opts={})
-    user = User.find_by(username: username)
-    raise ArgumentError, "Username #{username} should exist before uploading photos." if user.nil?
+  def import_by_files(user, path_to_profile, path_to_photos, opts={})
+    import_profile_if_present(opts, path_to_photos, path_to_profile, user.username)
 
-    if path_to_profile.present?
-      logger.info "Import for profile #{username} at path #{path_to_profile} requested"
-      import_user_profile(path_to_profile, username, opts.merge(photo_migration: path_to_photos.present?))
-    end
+    import_photos_if_present(path_to_photos, user)
+    remove_import_files(path_to_profile, path_to_photos)
+  end
+
+  private
 
+  def import_photos_if_present(path_to_photos, user)
     if path_to_photos.present?
-      logger.info("Importing photos from import file for '#{username}' from #{path_to_photos}")
+      logger.info("Importing photos from import file for '#{user.username}' from #{path_to_photos}")
       import_user_photos(user, path_to_photos)
     end
-    remove_import_files(path_to_profile, path_to_photos)
   end
 
-  private
+  def import_profile_if_present(opts, path_to_photos, path_to_profile, username)
+    if path_to_profile.present?
+      logger.info "Import for profile #{username} at path #{path_to_profile} requested"
+      import_user_profile(path_to_profile, username, opts.merge(photo_migration: path_to_photos.present?))
+    end
+  end
 
   def import_user_profile(path_to_profile, username, opts)
     raise ArgumentError, "Profile file not found at path: #{path_to_profile}" unless File.exist?(path_to_profile)

app/views/users/_edit.haml

@@ -238,7 +238,7 @@
             = t(".import.warning")
         .form-group
           .btn.btn-primary{id: "import_account", data: {toggle: "modal", target: "#importAccountModal"}}
-            Import another account...
+            = t(".import.account")
           = render "import_account_modal"
     .row
       .col-md-12

app/workers/import_user.rb

@@ -6,7 +6,7 @@ class ImportUser < ArchiveBase
 
     def perform_archive_job(user_id, import_parameters)
       user = User.find(user_id)
-      ImportService.new.import_by_user(user.username, import_parameters)
+      ImportService.new.import_by_user(user, import_parameters)
     end
   end
 end

config/locales/diaspora/en.yml

@@ -1320,6 +1320,7 @@ en:
                To do this, first export your data and photos from the pod you want to move from. Then, import those data and photo archives into this account.
                Your old account on the other pod will be deleted once this process has completed."
         warning: "There is no way back; that account can’t be restored if you change your mind."
+        account: "Import another account..."
 
       close_account:
         dont_go: "Hey, please don’t go!"

lib/tasks/accounts.rake

@@ -10,9 +10,12 @@ namespace :accounts do
     process_arguments(args)
     start_time = Time.now.getlocal
     if args[:new_user_name].present? && (args[:archive_path].present? || args[:photos_path].present?)
-      ImportService.new.import_by_files(args[:archive_path], args[:photos_path], args[:new_user_name],
-                                        args.slice(:import_settings, :import_profile))
-      puts "\n Migration completed in #{Time.now.getlocal - start_time} seconds. (Photos might still be processed in)"
+      user = User.find_by(username: args[:new_username])
+      if user.nil?
+        puts("Username #{args[:new_username]} should exist before uploading photos.")
+      else
+        import_user(user, start_time, args)
+      end
     else
       puts "Must set a user name and a archive file path or photos file path"
     end
@@ -49,4 +52,10 @@ namespace :accounts do
 
     response[0] == "y"
   end
+
+  def import_user(user, start_time, args)
+    ImportService.new.import_by_files(user, args[:archive_path], args[:photos_path],
+                                      args.slice(:import_settings, :import_profile))
+    puts "\n Migration completed in #{Time.now.getlocal - start_time} seconds. (Photos might still be processed in)"
+  end
 end

spec/integration/import_service_spec.rb

@@ -20,7 +20,7 @@
     it "imports the photo with the same name" do
       old_random_string = photo.random_string
 
-      inlined_jobs { ImportService.new.import_by_files(nil, photo_archive.current_path, user.username) }
+      inlined_jobs { ImportService.new.import_by_files(user, nil, photo_archive.current_path) }
 
       imported_photo = photo.reload
       expect(imported_photo.random_string).to include(old_random_string)
@@ -46,7 +46,7 @@
         dispatcher
       end
 
-      inlined_jobs { ImportService.new.import_by_files(nil, photo_archive_path, user.username) }
+      inlined_jobs { ImportService.new.import_by_files(user, nil, photo_archive_path) }
 
       imported_photo = photo.reload
       new_random_string = imported_photo.random_string