Merge pull request #2812 from manyfold3d/organize-in-background

Organize models in background after editing

app/controllers/models_controller.rb

@@ -89,7 +89,10 @@ def create
   end
 
   def update
-    if @model.update(model_params)
+    hash = model_params
+    organize = hash.delete(:organize) == "1"
+    if @model.update(hash)
+      ModelOrganizeJob.perform_later(@model.id) if organize
       redirect_to @model, notice: t(".success")
     else
       redirect_back_or_to edit_model_path(@model), alert: t(".failure")
@@ -142,7 +145,7 @@ def bulk_update
         model.tag_list = existing_tags + add_tags - remove_tags
         model.save
       end
-      model.update! organize: true if organize
+      ModelOrganizeJob.perform_later(model.id) if organize
     end
     redirect_back_or_to edit_models_path(@filters), notice: t(".success")
   end

app/jobs/organize_model_job.rb

@@ -0,0 +1,8 @@
+class OrganizeModelJob < ApplicationJob
+  queue_as :default
+
+  def perform(model_id)
+    model = Model.find(model_id)
+    model&.organize!
+  end
+end

app/jobs/process_uploaded_file_job.rb

@@ -24,7 +24,7 @@ def perform(library_id, uploaded_file, owner: nil, creator_id: nil, collection_i
       if model.nil?
         model = library.models.create!(data)
         model.grant_permission_to "own", owner
-        model.update! organize: true
+        model.organize!
         new_model = true
       end
       # Handle different file types

app/models/model.rb

@@ -26,15 +26,9 @@ class Model < ApplicationRecord
   # In Rails 7.1 we will be able to do this instead:
   # normalizes :license, with: -> license { license.blank? ? nil : license }
 
-  before_validation :autoupdate_path, if: :organize
   before_update :move_files, if: :need_to_move_files?
   after_commit :check_integrity, on: :update
 
-  attr_reader :organize
-  def organize=(value)
-    @organize = ActiveRecord::Type::Boolean.new.cast(value)
-  end
-
   validates :name, presence: true
   validates :path, presence: true, uniqueness: {scope: :library}
   validate :check_for_submodels, on: :update, if: :need_to_move_files?
@@ -124,6 +118,11 @@ def exists_on_storage?
     library.has_folder?(path)
   end
 
+  def organize!
+    autoupdate_path
+    save!
+  end
+
   private
 
   def normalize_license

spec/jobs/organize_model_job_spec.rb

@@ -0,0 +1,9 @@
+require "rails_helper"
+
+RSpec.describe OrganizeModelJob do
+  subject(:job) { described_class.new }
+
+  let(:model) { create(:model) }
+
+  it "should call organise"
+end

spec/models/model_spec.rb

@@ -180,21 +180,21 @@
     }
 
     it "moves model folder" do # rubocop:todo RSpec/MultipleExpectations
-      expect { model.update! organize: true }.not_to raise_error
+      expect { model.organize! }.not_to raise_error
       expect(Dir.exist?(File.join(library.path, "original"))).to be false
       expect(Dir.exist?(File.join(library.path, "@untagged", "test-model#1"))).to be true
     end
 
     it "has a validation error if the destination path already exists, and does not move anything" do # rubocop:todo RSpec/MultipleExpectations
       FileUtils.mkdir_p(File.join(library.path, "@untagged/test-model#1"))
-      expect { model.update! organize: true }.to raise_error(ActiveRecord::RecordInvalid)
+      expect { model.organize! }.to raise_error(ActiveRecord::RecordInvalid)
       expect(model.errors.full_messages).to include("Path already exists")
       expect(Dir.exist?(File.join(library.path, "original"))).to be true
     end
 
-    it "has a validation error if the model has submodels, and does not move anything" do # rubocop:todo RSpec/MultipleExpectations
+    it "throws an error if the model has submodels, and does not move anything" do # rubocop:todo RSpec/MultipleExpectations
       create(:model, library: library, name: "sub model", path: "original/submodel")
-      expect { model.update! organize: true }.to raise_error(ActiveRecord::RecordInvalid)
+      expect { model.organize! }.to raise_error(ActiveRecord::RecordInvalid)
       expect(model.errors.full_messages).to include("Path can't be changed, model contains other models")
       expect(Dir.exist?(File.join(library.path, "original"))).to be true
       expect(Dir.exist?(File.join(library.path, "@untagged", "test-model#1"))).to be false