From 6e7caa86c9ae27af6ddc8c94ec089a3e47dc7cd1 Mon Sep 17 00:00:00 2001 From: James Smith Date: Fri, 27 Sep 2024 21:24:27 +0100 Subject: [PATCH 1/3] refactor organize! into its own method not a transient attribute any more --- app/controllers/models_controller.rb | 2 +- app/jobs/process_uploaded_file_job.rb | 2 +- app/models/model.rb | 11 +++++------ spec/models/model_spec.rb | 8 ++++---- 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/app/controllers/models_controller.rb b/app/controllers/models_controller.rb index bfbb3b1b8..ed88f5a03 100644 --- a/app/controllers/models_controller.rb +++ b/app/controllers/models_controller.rb @@ -142,7 +142,7 @@ def bulk_update model.tag_list = existing_tags + add_tags - remove_tags model.save end - model.update! organize: true if organize + model.organize! if organize end redirect_back_or_to edit_models_path(@filters), notice: t(".success") end diff --git a/app/jobs/process_uploaded_file_job.rb b/app/jobs/process_uploaded_file_job.rb index 394a68e03..800351a58 100644 --- a/app/jobs/process_uploaded_file_job.rb +++ b/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 diff --git a/app/models/model.rb b/app/models/model.rb index a0a938496..b8ce435ef 100644 --- a/app/models/model.rb +++ b/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 diff --git a/spec/models/model_spec.rb b/spec/models/model_spec.rb index 16711baee..28d432e82 100644 --- a/spec/models/model_spec.rb +++ b/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 From 48e8a5ddd28ab0a2c39c5b9f5cadb40910dd9b22 Mon Sep 17 00:00:00 2001 From: James Smith Date: Fri, 27 Sep 2024 21:33:45 +0100 Subject: [PATCH 2/3] move synchronous organize! calls into background runner --- app/controllers/models_controller.rb | 7 +++++-- app/jobs/organize_model_job.rb | 9 +++++++++ spec/jobs/organize_model_job_spec.rb | 9 +++++++++ 3 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 app/jobs/organize_model_job.rb create mode 100644 spec/jobs/organize_model_job_spec.rb diff --git a/app/controllers/models_controller.rb b/app/controllers/models_controller.rb index ed88f5a03..d67f188fc 100644 --- a/app/controllers/models_controller.rb +++ b/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.organize! if organize + ModelOrganizeJob.perform_later(model.id) if organize end redirect_back_or_to edit_models_path(@filters), notice: t(".success") end diff --git a/app/jobs/organize_model_job.rb b/app/jobs/organize_model_job.rb new file mode 100644 index 000000000..562732e4b --- /dev/null +++ b/app/jobs/organize_model_job.rb @@ -0,0 +1,9 @@ +class OrganizeModelJob < ApplicationJob + queue_as :default + + def perform(model_id) + model = Model.find(model_id) + model&.organize! + end + +end diff --git a/spec/jobs/organize_model_job_spec.rb b/spec/jobs/organize_model_job_spec.rb new file mode 100644 index 000000000..9d459f549 --- /dev/null +++ b/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 From aabac93e10a5aaca6223c5c56ee6a7e452bffde6 Mon Sep 17 00:00:00 2001 From: James Smith Date: Fri, 27 Sep 2024 21:41:55 +0100 Subject: [PATCH 3/3] linty lint lint --- app/jobs/organize_model_job.rb | 1 - spec/jobs/organize_model_job_spec.rb | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/jobs/organize_model_job.rb b/app/jobs/organize_model_job.rb index 562732e4b..0cc46c685 100644 --- a/app/jobs/organize_model_job.rb +++ b/app/jobs/organize_model_job.rb @@ -5,5 +5,4 @@ def perform(model_id) model = Model.find(model_id) model&.organize! end - end diff --git a/spec/jobs/organize_model_job_spec.rb b/spec/jobs/organize_model_job_spec.rb index 9d459f549..c1bad5a29 100644 --- a/spec/jobs/organize_model_job_spec.rb +++ b/spec/jobs/organize_model_job_spec.rb @@ -3,7 +3,7 @@ RSpec.describe OrganizeModelJob do subject(:job) { described_class.new } - let(:model) { create :model } + let(:model) { create(:model) } it "should call organise" end