Merge pull request #3430 from fhp/real-bulk-edit

Add "Update All Models" functionality to bulk edit models

Author: Floppy

Diff for: app/controllers/models_controller.rb

@@ -139,8 +139,17 @@ def bulk_update
     organize = hash.delete(:organize) == "1"
     add_tags = Set.new(hash.delete(:add_tags))
     remove_tags = Set.new(hash.delete(:remove_tags))
-    ids = params[:models].select { |k, v| v == "1" }.keys
-    policy_scope(Model).where(public_id: ids).find_each do |model|
+
+    models_to_update = if params[:commit] == t("models.bulk_edit.update_all")
+      # If "Update All Models" was clicked, update all models in the filtered set
+      filtered_models(@filters)
+    else
+      # If "Update Selected Models" was clicked, only update checked models
+      ids = params[:models].select { |k, v| v == "1" }.keys
+      policy_scope(Model).where(public_id: ids)
+    end
+
+    models_to_update.find_each do |model|
       if model&.update(hash)
         existing_tags = Set.new(model.tag_list)
         model.tag_list = existing_tags + add_tags - remove_tags

Diff for: app/views/models/bulk_edit.html.erb

@@ -58,6 +58,7 @@
       <%= form.hidden_field :library, value: @filters[:library] if @filters[:library] %>
       <%= form.hidden_field :creator, value: @filters[:creator] if @filters[:creator] %>
       <%= form.submit translate(".submit"), class: "btn btn-primary" %>
+      <%= form.submit translate(".update_all"), class: "btn btn-secondary" %>
 
     </div>
     <% if !@filters.empty? %>

Diff for: config/locales/models/de.yml

@@ -10,6 +10,7 @@ de:
       select_all: Alle Modelle auswählen
       submit: Ausgewählte Modelle aktualisieren
       title: Modelle massenweise bearbeiten
+      update_all: Alle Modelle aktualisieren
     bulk_fields:
       add_tags: Tags hinzufügen
     bulk_update:

Diff for: config/locales/models/en.yml

@@ -10,6 +10,7 @@ en:
       select_all: Select all models
       submit: Update Selected Models
       title: Bulk Edit Models
+      update_all: Update All Models
     bulk_fields:
       add_tags: Add tags
     bulk_update:

Diff for: config/locales/models/es.yml

@@ -10,6 +10,7 @@ es:
       select_all: Seleccione todos los modelos
       submit: Actualizar modelos seleccionados
       title: Edición masiva de modelos
+      update_all: Actualizar todos los modelos
     bulk_fields:
       add_tags: Añadir etiquetas
     bulk_update:

Diff for: config/locales/models/fr.yml

@@ -10,6 +10,7 @@ fr:
       select_all: Sélectionner tous les modèles
       submit: Mettre à jour les modèles sélectionnés
       title: Modifier les modèles en lot
+      update_all: Mettre à jour tous les modèles
     bulk_fields:
       add_tags: Ajouter des étiquettes
     bulk_update:

Diff for: config/locales/models/pl.yml

@@ -10,6 +10,7 @@ pl:
       select_all: Zaznacz wszystkie modele
       submit: Aktualizuj zaznaczone modele
       title: Edytuj zbiorczo modele
+      update_all: Aktualizuj wszystkie modele
     bulk_fields:
       add_tags: Dodaj tagi
     bulk_update:

Diff for: spec/factories/creator.rb

@@ -1,5 +1,6 @@
 FactoryBot.define do
   factory :creator do
-    name { Faker::Name.name }
+    sequence(:name) { |n| "Creator #{n}" }
+    sequence(:public_id) { |n| "creator_#{n}" }
   end
 end

Diff for: spec/factories/library.rb

@@ -1,5 +1,7 @@
 FactoryBot.define do
   factory :library do
+    sequence(:name) { |n| "Library #{n}" }
+    sequence(:public_id) { |n| "library_#{n}" }
     path {
       dir = Dir.mktmpdir(Faker::File.file_name, "/tmp")
       at_exit { FileUtils.remove_entry(dir) }

Diff for: spec/factories/model.rb

@@ -1,8 +1,40 @@
 FactoryBot.define do
   factory :model do
+    sequence(:name) { |n| "Model #{n}" }
     library
-    name { Faker::Creature::Animal.name }
+    sequence(:public_id) { |n| "model_#{n}" }
     path { Faker::File.dir }
     license { "MIT" }
+
+    trait :with_tags do
+      transient do
+        tags_count { 2 }
+      end
+
+      after(:create) do |model, evaluator|
+        evaluator.tags_count.times do |i|
+          model.tag_list.add("tag_#{i}")
+        end
+        model.save
+      end
+    end
+
+    trait :needs_organizing do
+      after(:create) do |model|
+        model.update!(needs_organizing: true)
+      end
+    end
+
+    trait :sensitive do
+      sensitive { true }
+    end
+
+    trait :with_creator do
+      creator
+    end
+
+    trait :with_collection do
+      collection
+    end
   end
 end

Diff for: spec/factories/tag.rb

@@ -0,0 +1,5 @@
+FactoryBot.define do
+  factory :tag, class: "ActsAsTaggableOn::Tag" do
+    sequence(:name) { |n| "tag_#{n}" }
+  end
+end

Diff for: spec/requests/models_spec.rb

@@ -116,17 +116,35 @@
 
     describe "GET /models/edit" do # rubocop:todo RSpec/RepeatedExampleGroupBody
       it "shows bulk edit page", :as_moderator do
-        get "/models/edit"
+        get edit_models_path
         expect(response).to have_http_status(:success)
       end
 
       it "sets returnable session param", :as_moderator do
-        get "/models/edit"
+        get edit_models_path
         expect(session[:return_after_new]).to eq "/models/edit"
       end
 
       it "is denied to non-moderators", :as_contributor do
-        expect { get "/models/edit" }.to raise_error(Pundit::NotAuthorizedError)
+        expect { get edit_models_path }.to raise_error(Pundit::NotAuthorizedError)
+      end
+
+      context "with filters", :as_moderator do
+        let(:tag) { create(:tag) }
+        let!(:tagged_model) { create(:model, library: library, tag_list: [tag.name]) }
+
+        it "shows filtered models" do
+          get edit_models_path(tag: [tag.name])
+          expect(response.body).to include(tagged_model.name)
+        end
+
+        it "doesn't show other models" do
+          get edit_models_path(tag: [tag.name])
+          library.models.each do |model|
+            next if model == tagged_model
+            expect(response.body).not_to include(model.name)
+          end
+        end
       end
     end
 
@@ -145,7 +163,7 @@
         update[models[0].to_param] = 1
         update[models[1].to_param] = 1
 
-        patch "/models/update", params: {models: update, creator_id: creator.id}
+        patch update_models_path, params: {models: update, creator_id: creator.id}
 
         expect(response).to have_http_status(:redirect)
         models.each { |model| model.reload }
@@ -154,7 +172,7 @@
       end
 
       it "adds tags to models", :as_moderator do # rubocop:todo RSpec/ExampleLength, RSpec/MultipleExpectations
-        patch "/models/update", params: {models: model_params, add_tags: ["a", "b", "c"]}
+        patch update_models_path, params: {models: model_params, add_tags: ["a", "b", "c"]}
 
         expect(response).to have_http_status(:redirect)
         library.models.take(2).each do |model|
@@ -168,7 +186,7 @@
           model.save
         end
 
-        patch "/models/update", params: {models: model_params, remove_tags: ["a", "b"]}
+        patch update_models_path, params: {models: model_params, remove_tags: ["a", "b"]}
 
         expect(response).to have_http_status(:redirect)
         library.models.take(2).each do |model|
@@ -178,7 +196,7 @@
       end
 
       it "clears returnable session param", :as_moderator do
-        patch "/models/update", params: {models: model_params, remove_tags: ["a", "b"]}
+        patch update_models_path, params: {models: model_params, remove_tags: ["a", "b"]}
         expect(session[:return_after_new]).to be_nil
       end
 
@@ -187,7 +205,45 @@
         library.models.take(2).each do |model|
           update[model.to_param] = 1
         end
-        expect { patch "/models/update", params: {models: model_params, remove_tags: ["a", "b"]} }.to raise_error(Pundit::NotAuthorizedError)
+        expect { patch update_models_path, params: {models: model_params, remove_tags: ["a", "b"]} }.to raise_error(Pundit::NotAuthorizedError)
+      end
+
+      context "when updating all filtered models", :as_moderator do # rubocop:todo RSpec/MultipleMemoizedHelpers
+        let(:tag) { create(:tag) }
+        let!(:tagged_model) { create(:model, library: library, tag_list: [tag.name]) }
+        let(:new_library) { create(:library) }
+
+        let(:params) do
+          {
+            commit: I18n.t("models.bulk_edit.update_all"),
+            new_library_id: new_library.id,
+            tag: [tag.name]
+          }
+        end
+
+        it "updates all models matching the filter" do
+          patch update_models_path, params: params
+
+          library.models.each do |model|
+            next if model == tagged_model
+            expect(model.reload.library_id).to eq(library.id)
+          end
+        end
+      end
+
+      context "with organization", :as_moderator do
+        let(:params) do
+          {
+            models: library.models.take(2).map { |m| [m.to_param, "1"] }.to_h,
+            organize: "1"
+          }
+        end
+
+        it "enqueues organize jobs for selected models" do
+          expect {
+            patch update_models_path, params: params
+          }.to have_enqueued_job(OrganizeModelJob).exactly(2).times
+        end
       end
     end