From 17b9f7e873ca1dee5eb06a605f59909be0d64f95 Mon Sep 17 00:00:00 2001
From: Chris Colvard <chris.colvard@gmail.com>
Date: Wed, 14 Sep 2022 16:30:17 -0400
Subject: [PATCH 1/9] Valkyrize batch editing

---
 .../hyrax/batch_edits_controller.rb           |  20 +-
 .../hyrax/forms/resource_batch_edit_form.rb   |  83 +++
 .../hyrax/batch_edits_controller_spec.rb      | 510 ++++++++++++------
 .../forms/resource_batch_edit_form_spec.rb    | 109 ++++
 4 files changed, 550 insertions(+), 172 deletions(-)
 create mode 100644 app/forms/hyrax/forms/resource_batch_edit_form.rb
 create mode 100644 spec/forms/hyrax/forms/resource_batch_edit_form_spec.rb

diff --git a/app/controllers/hyrax/batch_edits_controller.rb b/app/controllers/hyrax/batch_edits_controller.rb
index 9e9055460d..a302c833fa 100644
--- a/app/controllers/hyrax/batch_edits_controller.rb
+++ b/app/controllers/hyrax/batch_edits_controller.rb
@@ -60,11 +60,27 @@ def update_document(obj)
       obj.save
     end
 
+    def valkyrie_update_document(obj)
+      form = Hyrax::Forms::ResourceBatchEditForm.new(obj, current_ability, nil)
+      return unless form.validate(params[form_class.model_class.model_name.param_key])
+      result = transactions['change_set.update_work']
+               .with_step_args('work_resource.save_acl' => { permissions_params: form.input_params["permissions"] })
+               .call(form)
+      obj = result.value!
+
+      InheritPermissionsJob.perform_now(obj)
+      VisibilityCopyJob.perform_now(obj)
+    end
+
     def update
       case params["update_type"]
       when "update"
         batch.each do |doc_id|
-          update_document(Hyrax.query_service.find_by_alternate_identifier(alternate_identifier: doc_id, use_valkyrie: false))
+          if Hyrax.config.use_valkyrie?
+            valkyrie_update_document(Hyrax.query_service.find_by(id: doc_id))
+          else
+            update_document(Hyrax.query_service.find_by_alternate_identifier(alternate_identifier: doc_id, use_valkyrie: false))
+          end
         end
         flash[:notice] = "Batch update complete"
         after_update
@@ -97,7 +113,7 @@ def destroy_batch
     end
 
     def form_class
-      Forms::BatchEditForm
+      Hyrax.config.use_valkyrie? ? Forms::ResourceBatchEditForm : Forms::BatchEditForm
     end
 
     def terms
diff --git a/app/forms/hyrax/forms/resource_batch_edit_form.rb b/app/forms/hyrax/forms/resource_batch_edit_form.rb
new file mode 100644
index 0000000000..4ef1ae9942
--- /dev/null
+++ b/app/forms/hyrax/forms/resource_batch_edit_form.rb
@@ -0,0 +1,83 @@
+# frozen_string_literal: true
+module Hyrax
+  module Forms
+    class ResourceBatchEditForm < Hyrax::Forms::ResourceForm
+      include Hyrax::FormFields(:basic_metadata)
+
+      self.required_fields = []
+      self.model_class = Hyrax.primary_work_type
+
+      # Contains a list of titles of all the works in the batch
+      attr_accessor :names
+
+      # @param [Hyrax::Work] model the model backing the form
+      # @param [Ability] current_ability the user authorization model
+      # @param [Array<String>] batch_document_ids a list of document ids in the batch
+      def initialize(model, _current_ability, batch_document_ids)
+        @names = []
+        @batch_document_ids = batch_document_ids
+        if @batch_document_ids.present?
+          super(model.class.new(initialize_combined_fields))
+        else
+          super(model)
+        end
+      end
+
+      def terms
+        [:creator, :contributor, :description,
+         :keyword, :resource_type, :license, :publisher, :date_created,
+         :subject, :language, :identifier, :based_near,
+         :related_url]
+      end
+
+      attr_reader :batch_document_ids
+
+      # Returns a list of parameters we accept from the form
+      # rubocop:disable Metrics/MethodLength
+      def self.build_permitted_params
+        [{ creator: [] },
+         { contributor: [] },
+         { description: [] },
+         { keyword: [] },
+         { resource_type: [] },
+         { license: [] },
+         { publisher: [] },
+         { date_created: [] },
+         { subject: [] },
+         { language: [] },
+         { identifier: [] },
+         { based_near: [] },
+         { related_url: [] },
+         { permissions_attributes: [:type, :name, :access, :id, :_destroy] },
+         :on_behalf_of,
+         :version,
+         :add_works_to_collection,
+         :visibility_during_embargo,
+         :embargo_release_date,
+         :visibility_after_embargo,
+         :visibility_during_lease,
+         :lease_expiration_date,
+         :visibility_after_lease,
+         :visibility,
+         { based_near_attributes: [:id, :_destroy] }]
+      end
+      # rubocop:enable Metrics/MethodLength
+
+      private
+
+      # override this method if you need to initialize more complex RDF assertions (b-nodes)
+      # @return [Hash<String, Array>] the list of unique values per field
+      def initialize_combined_fields
+        # For each of the files in the batch, set the attributes to be the concatenation of all the attributes
+        batch_document_ids.each_with_object({}) do |doc_id, combined_attributes|
+          work = Hyrax.query_service.find_by(id: doc_id)
+          terms.each do |field|
+            combined_attributes[field] ||= []
+            combined_attributes[field] = (combined_attributes[field] + work[field].to_a).uniq
+          end
+          names << work.to_s
+        end
+      end
+    end
+  end
+end
diff --git a/spec/controllers/hyrax/batch_edits_controller_spec.rb b/spec/controllers/hyrax/batch_edits_controller_spec.rb
index 25ee1ce715..6a49948fe4 100644
--- a/spec/controllers/hyrax/batch_edits_controller_spec.rb
+++ b/spec/controllers/hyrax/batch_edits_controller_spec.rb
@@ -29,37 +29,33 @@
       expect(response).to render_template('dashboard')
       expect(assigns[:form].model.creator).to match_array ["Fred", "Wilma"]
     end
+
+    context "with work resources" do
+      let(:one) { FactoryBot.valkyrie_create(:monograph, creator: ["Fred"], title: ["abc"], language: ['en']) }
+      let(:two) { FactoryBot.valkyrie_create(:monograph, creator: ["Wilma"], title: ["abc2"], publisher: ['Rand McNally'], language: ['en'], resource_type: ['bar']) }
+      let(:three) { FactoryBot.valkyrie_create(:monograph, creator: ["Dino"], title: ["abc3"]) }
+
+      it "is successful" do
+        expect(controller).to receive(:add_breadcrumb).with('Home', Hyrax::Engine.routes.url_helpers.root_path(locale: 'en'))
+        expect(controller).to receive(:add_breadcrumb).with(I18n.t('hyrax.dashboard.title'), Hyrax::Engine.routes.url_helpers.dashboard_path(locale: 'en'))
+        expect(controller).to receive(:add_breadcrumb).with(I18n.t('hyrax.dashboard.my.works'), Hyrax::Engine.routes.url_helpers.my_works_path(locale: 'en'))
+        get :edit
+        expect(response).to be_successful
+        expect(response).to render_template('dashboard')
+        expect(assigns[:form].model.creator).to match_array ["Fred", "Wilma"]
+      end
+    end
   end
 
   describe "update" do
-    let(:user) { build(:user) }
-    let(:admin_set) { create(:admin_set) }
     let(:permission_template) { create(:permission_template, source_id: admin_set.id) }
     let!(:workflow) { create(:workflow, allows_access_grant: true, active: true, permission_template_id: permission_template.id) }
-    let!(:one) do
-      create(:work, admin_set_id: admin_set.id, creator: ["Fred"], title: ["abc"], language: ['en'], user: user)
-    end
-
-    let!(:two) do
-      create(:work, admin_set_id: admin_set.id, creator: ["Fred"], title: ["abc"], language: ['en'], user: user)
-    end
-
-    let!(:three) do
-      create(:work, admin_set_id: admin_set.id, creator: ["Fred"], title: ["abc"], language: ['en'])
-    end
-
-    let!(:file_set) do
-      create(:file_set, creator: ["Fred"])
-    end
 
     let(:release_date) { Time.zone.today + 2 }
 
     let(:mycontroller) { "hyrax/my/works" }
 
     before do
-      one.members << file_set
-      one.save!
-
       # TODO: why aren't we just submitting batch_document_ids[] as a parameter?
       controller.batch = [one.id, two.id, three.id]
       expect(controller).to receive(:can?).with(:edit, one.id).and_return(true)
@@ -67,191 +63,365 @@
       expect(controller).to receive(:can?).with(:edit, three.id).and_return(false)
     end
 
-    it "is successful" do
-      put :update, params: { update_type: "delete_all" }
-      expect(response).to redirect_to(dashboard_path(locale: 'en'))
-      expect { GenericWork.find(one.id) }.to raise_error(Ldp::Gone)
-      expect { GenericWork.find(two.id) }.to raise_error(Ldp::Gone)
-      expect(GenericWork).to exist(three.id)
-    end
+    context "with ActiveFedora works" do
+      let(:admin_set) { create(:admin_set) }
+      let!(:one) do
+        create(:work, admin_set_id: admin_set.id, creator: ["Fred"], title: ["abc"], language: ['en'], user: user)
+      end
 
-    it "redirects to the return controller" do
-      put :update, params: { update_type: "delete_all", return_controller: mycontroller }
-      expect(response).to redirect_to(Hyrax::Engine.routes.url_for(controller: mycontroller, only_path: true, locale: 'en'))
-    end
+      let!(:two) do
+        create(:work, admin_set_id: admin_set.id, creator: ["Fred"], title: ["abc"], language: ['en'], user: user)
+      end
 
-    it "updates the records" do
-      put :update, params: { update_type: "update", generic_work: { subject: ["zzz"] } }
-      expect(response).to be_redirect
-      expect(GenericWork.find(one.id).subject).to eq ["zzz"]
-      expect(GenericWork.find(two.id).subject).to eq ["zzz"]
-      expect(GenericWork.find(three.id).subject).to be_empty
-    end
+      let!(:three) do
+        create(:work, admin_set_id: admin_set.id, creator: ["Fred"], title: ["abc"], language: ['en'])
+      end
 
-    it "updates permissions" do
-      put :update, params: { update_type: "update", generic_work: { visibility: "authenticated" } }
-      expect(response).to be_redirect
+      let!(:file_set) do
+        create(:file_set, creator: ["Fred"])
+      end
 
-      work1 = GenericWork.find(one.id)
-      expect(work1.visibility).to eq "authenticated"
-      expect(work1.file_sets.map(&:visibility)).to eq ["authenticated"]
+      before do
+        one.members << file_set
+        one.save!
+      end
 
-      expect(GenericWork.find(two.id).visibility).to eq "authenticated"
-      expect(GenericWork.find(three.id).visibility).to eq "restricted"
-    end
+      it "is successful" do
+        put :update, params: { update_type: "delete_all" }
+        expect(response).to redirect_to(dashboard_path(locale: 'en'))
+        expect { GenericWork.find(one.id) }.to raise_error(Ldp::Gone)
+        expect { GenericWork.find(two.id) }.to raise_error(Ldp::Gone)
+        expect(GenericWork).to exist(three.id)
+      end
 
-    it 'creates leases' do
-      put :update, params: { update_type: "update",
-                             generic_work: { visibility: "lease", lease_expiration_date: release_date, visibility_during_lease: 'open', visibility_after_lease: 'restricted' } }
-      expect(response).to be_redirect
-
-      work1 = GenericWork.find(one.id)
-      expect(work1.visibility).to eq "open"
-      expect(work1.visibility_during_lease).to eq 'open'
-      expect(work1.visibility_after_lease).to eq 'restricted'
-      expect(work1.lease_expiration_date).to eq release_date
-      expect(work1.file_sets.map(&:visibility)).to eq ['open']
-      expect(work1.file_sets.map(&:visibility_during_lease)).to eq ['open']
-      expect(work1.file_sets.map(&:visibility_after_lease)).to eq ['restricted']
-      expect(work1.file_sets.map(&:lease_expiration_date)).to eq [release_date]
-
-      work2 = GenericWork.find(two.id)
-      expect(work2.visibility).to eq 'open'
-      expect(work2.visibility_during_lease).to eq 'open'
-      expect(work2.visibility_after_lease).to eq 'restricted'
-      expect(work2.lease_expiration_date).to eq release_date
-
-      work3 = GenericWork.find(three.id)
-      expect(work3.visibility).to eq 'restricted'
-      expect(work3.visibility_during_lease).to be_nil
-      expect(work3.visibility_after_lease).to be_nil
-      expect(work3.lease_expiration_date).to be_nil
-    end
+      it "redirects to the return controller" do
+        put :update, params: { update_type: "delete_all", return_controller: mycontroller }
+        expect(response).to redirect_to(Hyrax::Engine.routes.url_for(controller: mycontroller, only_path: true, locale: 'en'))
+      end
 
-    it 'creates embargoes' do
-      put :update, params: { update_type: "update",
-                             generic_work: { visibility: "embargo", embargo_release_date: release_date, visibility_during_embargo: 'authenticated', visibility_after_embargo: 'open' } }
-      expect(response).to be_redirect
-
-      work1 = GenericWork.find(one.id)
-      expect(work1.visibility).to eq "authenticated"
-      expect(work1.visibility_during_embargo).to eq 'authenticated'
-      expect(work1.visibility_after_embargo).to eq 'open'
-      expect(work1.embargo_release_date).to eq release_date
-      expect(work1.file_sets.map(&:visibility)).to eq ['authenticated']
-      expect(work1.file_sets.map(&:visibility_during_embargo)).to eq ['authenticated']
-      expect(work1.file_sets.map(&:visibility_after_embargo)).to eq ['open']
-      expect(work1.file_sets.map(&:embargo_release_date)).to eq [release_date]
-
-      work2 = GenericWork.find(two.id)
-      expect(work2.visibility).to eq 'authenticated'
-      expect(work2.visibility_during_embargo).to eq 'authenticated'
-      expect(work2.visibility_after_embargo).to eq 'open'
-      expect(work2.embargo_release_date).to eq release_date
-
-      work3 = GenericWork.find(three.id)
-      expect(work3.visibility).to eq 'restricted'
-      expect(work3.visibility_during_embargo).to be_nil
-      expect(work3.visibility_after_embargo).to be_nil
-      expect(work3.embargo_release_date).to be_nil
-    end
+      it "updates the records" do
+        put :update, params: { update_type: "update", generic_work: { subject: ["zzz"] } }
+        expect(response).to be_redirect
+        expect(GenericWork.find(one.id).subject).to eq ["zzz"]
+        expect(GenericWork.find(two.id).subject).to eq ["zzz"]
+        expect(GenericWork.find(three.id).subject).to be_empty
+      end
 
-    context 'with roles' do
-      it 'updates roles' do
-        put :update, params: { update_type: "update", generic_work: { permissions_attributes: [{ type: 'person', access: 'read', name: 'foo@bar.com' }] } }
+      it "updates permissions" do
+        put :update, params: { update_type: "update", generic_work: { visibility: "authenticated" } }
         expect(response).to be_redirect
 
         work1 = GenericWork.find(one.id)
-        expect(work1.read_users).to include "foo@bar.com"
-        expect(work1.file_sets.map(&:read_users)).to eq [["foo@bar.com"]]
+        expect(work1.visibility).to eq "authenticated"
+        expect(work1.file_sets.map(&:visibility)).to eq ["authenticated"]
 
-        expect(GenericWork.find(two.id).read_users).to eq ["foo@bar.com"]
-        expect(GenericWork.find(three.id).read_users).to eq []
+        expect(GenericWork.find(two.id).visibility).to eq "authenticated"
+        expect(GenericWork.find(three.id).visibility).to eq "restricted"
       end
-    end
-  end
 
-  describe "#destroy_collection" do
-    let(:user) { create(:user) }
+      it 'creates leases' do
+        put :update, params: { update_type: "update",
+                               generic_work: { visibility: "lease", lease_expiration_date: release_date, visibility_during_lease: 'open', visibility_after_lease: 'restricted' } }
+        expect(response).to be_redirect
 
-    let(:collection1) do
-      create(:public_collection_lw, title: ["My First Collection"],
-                                    description: ["My incredibly detailed description of the collection"],
-                                    user: user)
-    end
+        work1 = GenericWork.find(one.id)
+        expect(work1.visibility).to eq "open"
+        expect(work1.visibility_during_lease).to eq 'open'
+        expect(work1.visibility_after_lease).to eq 'restricted'
+        expect(work1.lease_expiration_date).to eq release_date
+        expect(work1.file_sets.map(&:visibility)).to eq ['open']
+        expect(work1.file_sets.map(&:visibility_during_lease)).to eq ['open']
+        expect(work1.file_sets.map(&:visibility_after_lease)).to eq ['restricted']
+        expect(work1.file_sets.map(&:lease_expiration_date)).to eq [release_date]
+
+        work2 = GenericWork.find(two.id)
+        expect(work2.visibility).to eq 'open'
+        expect(work2.visibility_during_lease).to eq 'open'
+        expect(work2.visibility_after_lease).to eq 'restricted'
+        expect(work2.lease_expiration_date).to eq release_date
+
+        work3 = GenericWork.find(three.id)
+        expect(work3.visibility).to eq 'restricted'
+        expect(work3.visibility_during_lease).to be_nil
+        expect(work3.visibility_after_lease).to be_nil
+        expect(work3.lease_expiration_date).to be_nil
+      end
 
-    let(:collection2) do
-      create(:public_collection_lw, title: ["My Other Collection"],
-                                    description: ["My incredibly detailed description of the other collection"],
-                                    user: user)
-    end
+      it 'creates embargoes' do
+        put :update, params: { update_type: "update",
+                               generic_work: { visibility: "embargo", embargo_release_date: release_date, visibility_during_embargo: 'authenticated', visibility_after_embargo: 'open' } }
+        expect(response).to be_redirect
+
+        work1 = GenericWork.find(one.id)
+        expect(work1.visibility).to eq "authenticated"
+        expect(work1.visibility_during_embargo).to eq 'authenticated'
+        expect(work1.visibility_after_embargo).to eq 'open'
+        expect(work1.embargo_release_date).to eq release_date
+        expect(work1.file_sets.map(&:visibility)).to eq ['authenticated']
+        expect(work1.file_sets.map(&:visibility_during_embargo)).to eq ['authenticated']
+        expect(work1.file_sets.map(&:visibility_after_embargo)).to eq ['open']
+        expect(work1.file_sets.map(&:embargo_release_date)).to eq [release_date]
+
+        work2 = GenericWork.find(two.id)
+        expect(work2.visibility).to eq 'authenticated'
+        expect(work2.visibility_during_embargo).to eq 'authenticated'
+        expect(work2.visibility_after_embargo).to eq 'open'
+        expect(work2.embargo_release_date).to eq release_date
+
+        work3 = GenericWork.find(three.id)
+        expect(work3.visibility).to eq 'restricted'
+        expect(work3.visibility_during_embargo).to be_nil
+        expect(work3.visibility_after_embargo).to be_nil
+        expect(work3.embargo_release_date).to be_nil
+      end
 
-    let!(:work1) { create(:work, title: ["First of the Assets"], member_of_collections: [collection1], user: user) }
-    let(:work2)  { create(:work, title: ["Second of the Assets"], user: user) }
+      context 'with roles' do
+        it 'updates roles' do
+          put :update, params: { update_type: "update", generic_work: { permissions_attributes: [{ type: 'person', access: 'read', name: 'foo@bar.com' }] } }
+          expect(response).to be_redirect
 
-    let(:mycontroller) { "hyrax/my/works" }
-    let(:curation_concern) { create(:work1, user: user) }
-
-    context 'when user has edit access' do
-      it "deletes collections with and without works in it" do
-        controller.batch = [collection1.id, collection2.id]
-        delete :destroy_collection, params: { update_type: "delete_all" }
-        expect { Collection.find(collection1.id) }.to raise_error(Ldp::Gone)
-        expect { Collection.find(collection2.id) }.to raise_error(Ldp::Gone)
+          work1 = GenericWork.find(one.id)
+          expect(work1.read_users).to include "foo@bar.com"
+          expect(work1.file_sets.map(&:read_users)).to eq [["foo@bar.com"]]
+
+          expect(GenericWork.find(two.id).read_users).to eq ["foo@bar.com"]
+          expect(GenericWork.find(three.id).read_users).to eq []
+        end
       end
     end
 
-    context 'when user does not have edit access' do
-      let(:user2) { create(:user) }
+    context "with work resources" do
+      let(:admin_set) { FactoryBot.valkyrie_create(:hyrax_admin_set) }
+      let!(:one) do
+        FactoryBot.valkyrie_create(:monograph, :with_member_file_sets, admin_set_id: admin_set.id, creator: ["Fred"], title: ["abc"], language: ['en'], depositor: user.user_key)
+      end
 
-      let(:collection3) do
-        create(:public_collection_lw, title: ["User2's Collection"],
-                                      description: ["Collection created by user2"],
-                                      user: user2)
+      let!(:two) do
+        FactoryBot.valkyrie_create(:monograph, admin_set_id: admin_set.id, creator: ["Fred"], title: ["abc"], language: ['en'], depositor: user.user_key)
       end
 
-      before do
-        allow(controller).to receive(:current_user).and_return(user2)
+      let!(:three) do
+        FactoryBot.valkyrie_create(:monograph, admin_set_id: admin_set.id, creator: ["Fred"], title: ["abc"], language: ['en'])
+      end
+
+      let(:work1) { Hyrax.query_service.find_by(id: one.id) }
+      let(:work2) { Hyrax.query_service.find_by(id: two.id) }
+      let(:work3) { Hyrax.query_service.find_by(id: three.id) }
+
+      it "is successful" do
+        put :update, params: { update_type: "delete_all" }
+        expect(response).to redirect_to(dashboard_path(locale: 'en'))
+        expect { work1 }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        expect { work2 }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        expect { work3 }.not_to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+      end
+
+      it "redirects to the return controller" do
+        put :update, params: { update_type: "delete_all", return_controller: mycontroller }
+        expect(response).to redirect_to(Hyrax::Engine.routes.url_for(controller: mycontroller, only_path: true, locale: 'en'))
+      end
+
+      it "updates the records" do
+        put :update, params: { update_type: "update", monograph: { subject: ["zzz"] } }
+        expect(response).to be_redirect
+        expect(work1.subject).to eq ["zzz"]
+        expect(work2.subject).to eq ["zzz"]
+        expect(work3.subject).to be_empty
       end
 
-      it "fails to delete collections when user does not have edit access" do
-        controller.batch = [collection1.id, collection3.id]
-        delete :destroy_collection, params: { update_type: "delete_all" }
-        expect(Collection.exists?(collection1.id)).to eq true
-        expect(Collection.exists?(collection2.id)).to eq true
+      it "updates permissions" do
+        put :update, params: { update_type: "update", monograph: { visibility: "authenticated" } }
+        expect(response).to be_redirect
+
+        expect(work1.visibility).to eq "authenticated"
+        expect(Hyrax.query_service.find_many_by_ids(ids: work1.member_ids).map(&:visibility)).to eq ["authenticated", "authenticated"]
+
+        expect(work2.visibility).to eq "authenticated"
+        expect(work3.visibility).to eq "restricted"
       end
 
-      it "deletes collections where user has edit access, failing to delete those where user does not have edit access" do
-        controller.batch = [collection1.id, collection3.id]
-        delete :destroy_collection, params: { update_type: "delete_all" }
-        expect(Collection.exists?(collection1.id)).to eq true
-        expect(Collection.exists?(collection2.id)).to eq true
-        expect { Collection.find(collection3.id) }.to raise_error(Ldp::Gone)
+      it 'creates leases' do
+        put :update, params: { update_type: "update",
+                               monograph: { visibility: "lease", lease_expiration_date: release_date, visibility_during_lease: 'open', visibility_after_lease: 'restricted' } }
+        expect(response).to be_redirect
+
+        expect(work1.visibility).to eq "open"
+        expect(work1.lease.visibility_during_lease).to eq 'open'
+        expect(work1.lease.visibility_after_lease).to eq 'restricted'
+        expect(work1.lease.lease_expiration_date).to eq release_date.to_s
+        file_sets = Hyrax.query_service.find_many_by_ids(ids: work1.member_ids)
+        expect(file_sets.map(&:visibility)).to eq ['open', 'open']
+        expect(file_sets.map(&:lease).map(&:visibility_during_lease)).to eq ['open', 'open']
+        expect(file_sets.map(&:lease).map(&:visibility_after_lease)).to eq ['restricted', 'restricted']
+        expect(file_sets.map(&:lease).map(&:lease_expiration_date)).to eq [release_date.to_s, release_date.to_s]
+
+        expect(work2.visibility).to eq 'open'
+        expect(work2.lease.visibility_during_lease).to eq 'open'
+        expect(work2.lease.visibility_after_lease).to eq 'restricted'
+        expect(work2.lease.lease_expiration_date).to eq release_date.to_s
+
+        expect(work3.visibility).to eq 'restricted'
+        expect(work3.lease).to be_nil
+      end
+
+      it 'creates embargoes' do
+        put :update, params: { update_type: "update",
+                               monograph: { visibility: "embargo", embargo_release_date: release_date, visibility_during_embargo: 'authenticated', visibility_after_embargo: 'open' } }
+        expect(response).to be_redirect
+
+        expect(work1.visibility).to eq "authenticated"
+        expect(work1.embargo.visibility_during_embargo).to eq 'authenticated'
+        expect(work1.embargo.visibility_after_embargo).to eq 'open'
+        expect(work1.embargo.embargo_release_date).to eq release_date.to_s
+        file_sets = Hyrax.query_service.find_many_by_ids(ids: work1.member_ids)
+        expect(file_sets.map(&:visibility)).to eq ['authenticated', 'authenticated']
+        expect(file_sets.map(&:embargo).map(&:visibility_during_embargo)).to eq ['authenticated', 'authenticated']
+        expect(file_sets.map(&:embargo).map(&:visibility_after_embargo)).to eq ['open', 'open']
+        expect(file_sets.map(&:embargo).map(&:embargo_release_date)).to eq [release_date.to_s, release_date.to_s]
+
+        expect(work2.visibility).to eq 'authenticated'
+        expect(work2.embargo.visibility_during_embargo).to eq 'authenticated'
+        expect(work2.embargo.visibility_after_embargo).to eq 'open'
+        expect(work2.embargo.embargo_release_date).to eq release_date.to_s
+
+        expect(work3.visibility).to eq 'restricted'
+        expect(work3.embargo).to be_nil
+      end
+
+      context 'with roles' do
+        it 'updates roles' do
+          put :update, params: { update_type: "update", monograph: { permissions_attributes: { "0" => { type: 'person', access: 'read', name: 'foo@bar.com' } } } }
+          expect(response).to be_redirect
+
+          expect(work1.read_users.to_a).to include "foo@bar.com"
+          expect(Hyrax.query_service.find_many_by_ids(ids: work1.member_ids).map(&:read_users).map(&:to_a)).to eq [["foo@bar.com"], ["foo@bar.com"]]
+
+          expect(work2.read_users.to_a).to eq ["foo@bar.com"]
+          expect(work3.read_users.to_a).to eq []
+        end
       end
     end
   end
 
-  context "with valkyrie resources" do
-    describe "#edit" do
-      let(:one) { FactoryBot.valkyrie_create(:monograph, creator: ["Fred"], title: ["abc"], language: ['en']) }
-      let(:two) { FactoryBot.valkyrie_create(:monograph, creator: ["Wilma"], title: ["abc2"], publisher: ['Rand McNally'], language: ['en'], resource_type: ['bar']) }
-      let(:three) { FactoryBot.valkyrie_create(:monograph, creator: ["Dino"], title: ["abc3"]) }
+  describe "#destroy_collection" do
+    context 'with ActiveFedora objects' do
+      let(:collection1) do
+        create(:public_collection_lw, title: ["My First Collection"],
+                                      description: ["My incredibly detailed description of the collection"],
+                                      user: user)
+      end
 
-      before do
-        controller.batch = [one.id, two.id, three.id]
-        expect(controller).to receive(:can?).with(:edit, one.id).and_return(true)
-        expect(controller).to receive(:can?).with(:edit, two.id).and_return(true)
-        expect(controller).to receive(:can?).with(:edit, three.id).and_return(false)
+      let(:collection2) do
+        create(:public_collection_lw, title: ["My Other Collection"],
+                                      description: ["My incredibly detailed description of the other collection"],
+                                      user: user)
       end
 
-      it "is successful" do
-        expect(controller).to receive(:add_breadcrumb).with('Home', Hyrax::Engine.routes.url_helpers.root_path(locale: 'en'))
-        expect(controller).to receive(:add_breadcrumb).with(I18n.t('hyrax.dashboard.title'), Hyrax::Engine.routes.url_helpers.dashboard_path(locale: 'en'))
-        expect(controller).to receive(:add_breadcrumb).with(I18n.t('hyrax.dashboard.my.works'), Hyrax::Engine.routes.url_helpers.my_works_path(locale: 'en'))
-        get :edit
-        expect(response).to be_successful
-        expect(response).to render_template('dashboard')
-        expect(assigns[:form].model.creator).to match_array ["Fred", "Wilma"]
+      let!(:work1) { create(:work, title: ["First of the Assets"], member_of_collections: [collection1], user: user) }
+      let(:work2)  { create(:work, title: ["Second of the Assets"], user: user) }
+
+      context 'when user has edit access' do
+        before do
+          controller.batch = [collection1.id, collection2.id]
+          allow(controller).to receive(:can?).with(:edit, collection1.id).and_return(true)
+          allow(controller).to receive(:can?).with(:edit, collection2.id).and_return(true)
+        end
+
+        it "deletes collections with and without works in it" do
+          delete :destroy_collection, params: { update_type: "delete_all" }
+          expect { Collection.find(collection1.id) }.to raise_error(Ldp::Gone)
+          expect { Collection.find(collection2.id) }.to raise_error(Ldp::Gone)
+        end
+      end
+
+      context 'when user does not have edit access' do
+        let(:user2) { create(:user) }
+
+        let(:collection3) do
+          create(:public_collection_lw, title: ["User2's Collection"],
+                                        description: ["Collection created by user2"],
+                                        user: user2)
+        end
+
+        before do
+          sign_in user2
+          controller.batch = [collection1.id, collection3.id]
+          allow(controller).to receive(:can?).with(:edit, collection1.id).and_return(false)
+          allow(controller).to receive(:can?).with(:edit, collection2.id).and_return(false)
+          allow(controller).to receive(:can?).with(:edit, collection3.id).and_return(true)
+        end
+
+        it "fails to delete collections when user does not have edit access" do
+          delete :destroy_collection, params: { update_type: "delete_all" }
+          expect(Collection.exists?(collection1.id)).to eq true
+          expect(Collection.exists?(collection2.id)).to eq true
+        end
+
+        it "deletes collections where user has edit access, failing to delete those where user does not have edit access" do
+          delete :destroy_collection, params: { update_type: "delete_all" }
+          expect(Collection.exists?(collection1.id)).to eq true
+          expect(Collection.exists?(collection2.id)).to eq true
+          expect { Collection.find(collection3.id) }.to raise_error(Ldp::Gone)
+        end
+      end
+    end
+
+    context 'with work resources' do
+      let(:collection1) do
+        FactoryBot.valkyrie_create(:hyrax_collection, title: ["My First Collection"],
+                                                      edit_users: [user.user_key])
+      end
+
+      let(:collection2) do
+        FactoryBot.valkyrie_create(:hyrax_collection, title: ["My Other Collection"],
+                                                      edit_users: [user.user_key])
+      end
+
+      let!(:work1) { FactoryBot.valkyrie_create(:monograph, title: ["First of the Assets"], member_of_collection_ids: [collection1.id], depositor: user.user_key, edit_users: [user.user_key]) }
+      let(:work2)  { FactoryBot.valkyrie_create(:monograph, title: ["Second of the Assets"], depositor: user.user_key, edit_users: [user.user_key]) }
+
+      context 'when user has edit access' do
+        before do
+          controller.batch = [collection1.id, collection2.id]
+          allow(controller).to receive(:can?).with(:edit, collection1.id).and_return(true)
+          allow(controller).to receive(:can?).with(:edit, collection2.id).and_return(true)
+        end
+
+        it "deletes collections with and without works in it" do
+          delete :destroy_collection, params: { update_type: "delete_all" }
+          expect { Hyrax.query_service.find_by(id: collection1.id) }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+          expect { Hyrax.query_service.find_by(id: collection2.id) }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        end
+      end
+
+      context 'when user does not have edit access' do
+        let(:user2) { create(:user) }
+
+        let(:collection3) do
+          FactoryBot.valkyrie_create(:hyrax_collection, title: ["User2's Collection"],
+                                                        edit_users: [user2])
+        end
+
+        before do
+          sign_in user2
+          controller.batch = [collection1.id, collection3.id]
+          allow(controller).to receive(:can?).with(:edit, collection1.id).and_return(false)
+          allow(controller).to receive(:can?).with(:edit, collection2.id).and_return(false)
+          allow(controller).to receive(:can?).with(:edit, collection3.id).and_return(true)
+        end
+
+        it "fails to delete collections when user does not have edit access" do
+          delete :destroy_collection, params: { update_type: "delete_all" }
+          expect { Hyrax.query_service.find_by(id: collection1.id) }.not_to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+          expect { Hyrax.query_service.find_by(id: collection2.id) }.not_to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        end
+
+        it "deletes collections where user has edit access, failing to delete those where user does not have edit access" do
+          delete :destroy_collection, params: { update_type: "delete_all" }
+          expect { Hyrax.query_service.find_by(id: collection1.id) }.not_to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+          expect { Hyrax.query_service.find_by(id: collection2.id) }.not_to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+          expect { Hyrax.query_service.find_by(id: collection3.id) }.to raise_error(Valkyrie::Persistence::ObjectNotFoundError)
+        end
       end
     end
   end
diff --git a/spec/forms/hyrax/forms/resource_batch_edit_form_spec.rb b/spec/forms/hyrax/forms/resource_batch_edit_form_spec.rb
new file mode 100644
index 0000000000..6f582dea75
--- /dev/null
+++ b/spec/forms/hyrax/forms/resource_batch_edit_form_spec.rb
@@ -0,0 +1,109 @@
+# frozen_string_literal: true
+RSpec.describe Hyrax::Forms::ResourceBatchEditForm do
+  let(:model) { FactoryBot.build(:monograph) }
+  let(:work1) do
+    FactoryBot.valkyrie_create(:monograph,
+           title: ["title 1"],
+           keyword: ["abc"],
+           creator: ["Wilma"],
+           language: ['en'],
+           contributor: ['contributor1'],
+           description: ['description1'],
+           license: ['license1'],
+           subject: ['subject1'],
+           identifier: ['id1'],
+           based_near: ['based_near1'],
+           related_url: ['related_url1'])
+  end
+
+  let(:work2) do
+    FactoryBot.valkyrie_create(:monograph,
+      title: ["title 2"],
+      keyword: ["123"],
+      creator: ["Fred"],
+      publisher: ['Rand McNally'],
+      language: ['en'],
+      resource_type: ['bar'],
+      contributor: ['contributor2'],
+      description: ['description2'],
+      license: ['license2'],
+      subject: ['subject2'],
+      identifier: ['id2'],
+      based_near: ['based_near2'],
+      related_url: ['related_url2'])
+  end
+
+  let(:batch) { [work1.id, work2.id] }
+  let(:form) { described_class.new(model, nil, batch) }
+  # let(:ability) { Ability.new(user) }
+  let(:user) { build(:user, display_name: 'Jill Z. User') }
+
+  describe "#terms" do
+    subject { form.terms }
+
+    it do
+      is_expected.to include(:creator,
+                         :contributor,
+                         :description,
+                         :keyword,
+                         :resource_type,
+                         :license,
+                         :publisher,
+                         :date_created,
+                         :subject,
+                         :language,
+                         :identifier,
+                         :based_near,
+                         :related_url)
+    end
+  end
+
+  describe "#model" do
+    it "combines the models in the batch" do
+      expect(form.model.creator).to match_array ["Wilma", "Fred"]
+      expect(form.model.contributor).to match_array ["contributor1", "contributor2"]
+      expect(form.model.description).to match_array ["description1", "description2"]
+      expect(form.model.keyword).to match_array ["abc", "123"]
+      expect(form.model.resource_type).to match_array ["bar"]
+      expect(form.model.license).to match_array ["license1", "license2"]
+      expect(form.model.publisher).to match_array ["Rand McNally"]
+      expect(form.model.subject).to match_array ["subject1", "subject2"]
+      expect(form.model.language).to match_array ["en"]
+      expect(form.model.identifier).to match_array ["id1", "id2"]
+      expect(form.model.based_near).to match_array ["based_near1", "based_near2"]
+      expect(form.model.related_url).to match_array ["related_url1", "related_url2"]
+    end
+  end
+
+  describe ".build_permitted_params" do
+    subject { described_class.build_permitted_params }
+
+    it do
+      is_expected.to eq [{ creator: [] },
+                         { contributor: [] },
+                         { description: [] },
+                         { keyword: [] },
+                         { resource_type: [] },
+                         { license: [] },
+                         { publisher: [] },
+                         { date_created: [] },
+                         { subject: [] },
+                         { language: [] },
+                         { identifier: [] },
+                         { based_near: [] },
+                         { related_url: [] },
+                         { permissions_attributes: [:type, :name, :access, :id, :_destroy] },
+                         :on_behalf_of,
+                         :version,
+                         :add_works_to_collection,
+                         :visibility_during_embargo,
+                         :embargo_release_date,
+                         :visibility_after_embargo,
+                         :visibility_during_lease,
+                         :lease_expiration_date,
+                         :visibility_after_lease,
+                         :visibility,
+                         { based_near_attributes: [:id, :_destroy] }]
+    end
+  end
+end

From cf502daee6ec59047b7602b02aadc47f36dd601e Mon Sep 17 00:00:00 2001
From: tamsin johnson <tomjohnson@ucsb.edu>
Date: Thu, 10 Aug 2023 16:39:40 -0700
Subject: [PATCH 2/9] wings: provide a convenience method DefaultWork#file_sets

Hydra works has this, and it will be nice to provide it to Wings users handling
Valkyrie native models cast back to AF.
---
 lib/wings/active_fedora_converter/default_work.rb | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/wings/active_fedora_converter/default_work.rb b/lib/wings/active_fedora_converter/default_work.rb
index 9e9b8e9257..344ceec7dd 100644
--- a/lib/wings/active_fedora_converter/default_work.rb
+++ b/lib/wings/active_fedora_converter/default_work.rb
@@ -137,6 +137,10 @@ def enforce_future_date_for_lease?
         false
       end
 
+      def file_sets
+        members.select(&:file_set?)
+      end
+
       def indexing_service
         Hyrax::ValkyrieIndexer.for(resource: valkyrie_resource)
       end

From 356b0b7469bc273d1007738b8a97f163e73dc24d Mon Sep 17 00:00:00 2001
From: tamsin johnson <tomjohnson@ucsb.edu>
Date: Thu, 10 Aug 2023 16:52:18 -0700
Subject: [PATCH 3/9] refactor some specs to avoid LDP errors during test setup

i don't know why these older tests run into LDP errors during setup after the
rewrite in 361b1135a, but we don't want to use these factories in setup anyway.

since the issue occurs before any production code is executed, it's safe to just
refactor to setup using the Valkyrie based factories. we'll want to complete
this refactor throughout this file anyway.
---
 .../hyrax/batch_edits_controller_spec.rb      | 44 ++++++++++---------
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/spec/controllers/hyrax/batch_edits_controller_spec.rb b/spec/controllers/hyrax/batch_edits_controller_spec.rb
index 6a49948fe4..944b6b538b 100644
--- a/spec/controllers/hyrax/batch_edits_controller_spec.rb
+++ b/spec/controllers/hyrax/batch_edits_controller_spec.rb
@@ -306,31 +306,32 @@
   describe "#destroy_collection" do
     context 'with ActiveFedora objects' do
       let(:collection1) do
-        create(:public_collection_lw, title: ["My First Collection"],
-                                      description: ["My incredibly detailed description of the collection"],
-                                      user: user)
+        FactoryBot.valkyrie_create(:hyrax_collection,
+                                   :public,
+                                   title: ["My First Collection"],
+                                   user: user)
       end
 
       let(:collection2) do
-        create(:public_collection_lw, title: ["My Other Collection"],
-                                      description: ["My incredibly detailed description of the other collection"],
-                                      user: user)
+        FactoryBot.valkyrie_create(:hyrax_collection,
+                                   :public,
+                                   title: ["My Other Collection"],
+                                   user: user)
       end
 
-      let!(:work1) { create(:work, title: ["First of the Assets"], member_of_collections: [collection1], user: user) }
-      let(:work2)  { create(:work, title: ["Second of the Assets"], user: user) }
+      let!(:work1) { FactoryBot.valkyrie_create(:hyrax_work, title: ["First of the Assets"], member_of_collection_ids: [collection1.id], depositor: user.user_key) }
 
       context 'when user has edit access' do
         before do
-          controller.batch = [collection1.id, collection2.id]
-          allow(controller).to receive(:can?).with(:edit, collection1.id).and_return(true)
-          allow(controller).to receive(:can?).with(:edit, collection2.id).and_return(true)
+          controller.batch = [collection1.id.to_s, collection2.id.to_s]
+          allow(controller).to receive(:can?).with(:edit, collection1.id.to_s).and_return(true)
+          allow(controller).to receive(:can?).with(:edit, collection2.id.to_s).and_return(true)
         end
 
         it "deletes collections with and without works in it" do
           delete :destroy_collection, params: { update_type: "delete_all" }
-          expect { Collection.find(collection1.id) }.to raise_error(Ldp::Gone)
-          expect { Collection.find(collection2.id) }.to raise_error(Ldp::Gone)
+          expect { Collection.find(collection1.id.to_s) }.to raise_error(Ldp::Gone)
+          expect { Collection.find(collection2.id.to_s) }.to raise_error(Ldp::Gone)
         end
       end
 
@@ -338,9 +339,10 @@
         let(:user2) { create(:user) }
 
         let(:collection3) do
-          create(:public_collection_lw, title: ["User2's Collection"],
-                                        description: ["Collection created by user2"],
-                                        user: user2)
+          FactoryBot.valkyrie_create(:hyrax_collection,
+                                     :public,
+                                     title: ["User2's Collection"],
+                                     user: user2)
         end
 
         before do
@@ -353,15 +355,15 @@
 
         it "fails to delete collections when user does not have edit access" do
           delete :destroy_collection, params: { update_type: "delete_all" }
-          expect(Collection.exists?(collection1.id)).to eq true
-          expect(Collection.exists?(collection2.id)).to eq true
+          expect(Collection.exists?(collection1.id.to_s)).to eq true
+          expect(Collection.exists?(collection2.id.to_s)).to eq true
         end
 
         it "deletes collections where user has edit access, failing to delete those where user does not have edit access" do
           delete :destroy_collection, params: { update_type: "delete_all" }
-          expect(Collection.exists?(collection1.id)).to eq true
-          expect(Collection.exists?(collection2.id)).to eq true
-          expect { Collection.find(collection3.id) }.to raise_error(Ldp::Gone)
+          expect(Collection.exists?(collection1.id.to_s)).to eq true
+          expect(Collection.exists?(collection2.id.to_s)).to eq true
+          expect { Collection.find(collection3.id.to_s) }.to raise_error(Ldp::Gone)
         end
       end
     end

From df8e737a0a1f294d25e78600aab5a9031af9e135 Mon Sep 17 00:00:00 2001
From: Longshou Situ <lsitu@ucsd.edu>
Date: Fri, 11 Aug 2023 11:20:54 -0700
Subject: [PATCH 4/9] Fix failed controller tests for batch edit.

---
 app/services/hyrax/visibility_propagator.rb       |  2 +-
 .../hyrax/batch_edits_controller_spec.rb          | 15 ++++++++++-----
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/app/services/hyrax/visibility_propagator.rb b/app/services/hyrax/visibility_propagator.rb
index 8e579cdffa..132867aee6 100644
--- a/app/services/hyrax/visibility_propagator.rb
+++ b/app/services/hyrax/visibility_propagator.rb
@@ -10,7 +10,7 @@ class VisibilityPropagator
     # @return [#propagate]
     def self.for(source:)
       case source
-      when Hyrax::WorkBehavior # ActiveFedora
+      when ActiveFedora::Base # ActiveFedora
         FileSetVisibilityPropagator.new(source: source)
       when Hyrax::Resource # Valkyrie
         ResourceVisibilityPropagator.new(source: source)
diff --git a/spec/controllers/hyrax/batch_edits_controller_spec.rb b/spec/controllers/hyrax/batch_edits_controller_spec.rb
index 944b6b538b..8be442cca2 100644
--- a/spec/controllers/hyrax/batch_edits_controller_spec.rb
+++ b/spec/controllers/hyrax/batch_edits_controller_spec.rb
@@ -204,6 +204,11 @@
         FactoryBot.valkyrie_create(:monograph, admin_set_id: admin_set.id, creator: ["Fred"], title: ["abc"], language: ['en'])
       end
 
+      let(:form_class) do
+        Hyrax.config.use_valkyrie? ? Hyrax::Forms::ResourceBatchEditForm : Hyrax::Forms::BatchEditForm
+      end
+      let(:param_key) { form_class.model_name.param_key }
+
       let(:work1) { Hyrax.query_service.find_by(id: one.id) }
       let(:work2) { Hyrax.query_service.find_by(id: two.id) }
       let(:work3) { Hyrax.query_service.find_by(id: three.id) }
@@ -222,7 +227,7 @@
       end
 
       it "updates the records" do
-        put :update, params: { update_type: "update", monograph: { subject: ["zzz"] } }
+        put :update, params: { update_type: "update", "#{param_key}": { subject: ["zzz"] } }
         expect(response).to be_redirect
         expect(work1.subject).to eq ["zzz"]
         expect(work2.subject).to eq ["zzz"]
@@ -230,7 +235,7 @@
       end
 
       it "updates permissions" do
-        put :update, params: { update_type: "update", monograph: { visibility: "authenticated" } }
+        put :update, params: { update_type: "update", "#{param_key}": { visibility: "authenticated" } }
         expect(response).to be_redirect
 
         expect(work1.visibility).to eq "authenticated"
@@ -242,7 +247,7 @@
 
       it 'creates leases' do
         put :update, params: { update_type: "update",
-                               monograph: { visibility: "lease", lease_expiration_date: release_date, visibility_during_lease: 'open', visibility_after_lease: 'restricted' } }
+                               "#{param_key}": { visibility: "lease", lease_expiration_date: release_date, visibility_during_lease: 'open', visibility_after_lease: 'restricted' } }
         expect(response).to be_redirect
 
         expect(work1.visibility).to eq "open"
@@ -266,7 +271,7 @@
 
       it 'creates embargoes' do
         put :update, params: { update_type: "update",
-                               monograph: { visibility: "embargo", embargo_release_date: release_date, visibility_during_embargo: 'authenticated', visibility_after_embargo: 'open' } }
+                               "#{param_key}": { visibility: "embargo", embargo_release_date: release_date, visibility_during_embargo: 'authenticated', visibility_after_embargo: 'open' } }
         expect(response).to be_redirect
 
         expect(work1.visibility).to eq "authenticated"
@@ -290,7 +295,7 @@
 
       context 'with roles' do
         it 'updates roles' do
-          put :update, params: { update_type: "update", monograph: { permissions_attributes: { "0" => { type: 'person', access: 'read', name: 'foo@bar.com' } } } }
+          put :update, params: { update_type: "update", "#{param_key}": { permissions_attributes: { "0" => { type: 'person', access: 'read', name: 'foo@bar.com' } } } }
           expect(response).to be_redirect
 
           expect(work1.read_users.to_a).to include "foo@bar.com"

From 9f821510d385265a43e6df99a5262d5a75366841 Mon Sep 17 00:00:00 2001
From: Longshou Situ <lsitu@ucsd.edu>
Date: Fri, 11 Aug 2023 14:14:51 -0700
Subject: [PATCH 5/9] Fix the inconsistent param_key issue.

---
 app/controllers/hyrax/batch_edits_controller.rb       | 4 ++--
 spec/controllers/hyrax/batch_edits_controller_spec.rb | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/controllers/hyrax/batch_edits_controller.rb b/app/controllers/hyrax/batch_edits_controller.rb
index a302c833fa..17eb099fc2 100644
--- a/app/controllers/hyrax/batch_edits_controller.rb
+++ b/app/controllers/hyrax/batch_edits_controller.rb
@@ -61,7 +61,7 @@ def update_document(obj)
     end
 
     def valkyrie_update_document(obj)
-      form = Hyrax::Forms::ResourceBatchEditForm.new(obj, current_ability, nil)
+      form = form_class.new(obj, current_ability, nil)
       return unless form.validate(params[form_class.model_class.model_name.param_key])
       result = transactions['change_set.update_work']
                .with_step_args('work_resource.save_acl' => { permissions_params: form.input_params["permissions"] })
@@ -121,7 +121,7 @@ def terms
     end
 
     def work_params(extra_params = {})
-      work_params = params[form_class.model_name.param_key] || ActionController::Parameters.new
+      work_params = params[form_class.model_class.model_name.param_key] || ActionController::Parameters.new
       form_class.model_attributes(work_params.merge(extra_params))
     end
 
diff --git a/spec/controllers/hyrax/batch_edits_controller_spec.rb b/spec/controllers/hyrax/batch_edits_controller_spec.rb
index 8be442cca2..6bbc072cee 100644
--- a/spec/controllers/hyrax/batch_edits_controller_spec.rb
+++ b/spec/controllers/hyrax/batch_edits_controller_spec.rb
@@ -207,7 +207,7 @@
       let(:form_class) do
         Hyrax.config.use_valkyrie? ? Hyrax::Forms::ResourceBatchEditForm : Hyrax::Forms::BatchEditForm
       end
-      let(:param_key) { form_class.model_name.param_key }
+      let(:param_key) { form_class.model_class.model_name.param_key }
 
       let(:work1) { Hyrax.query_service.find_by(id: one.id) }
       let(:work2) { Hyrax.query_service.find_by(id: two.id) }

From bfc509cf9c42926e9d6a9684ba6a20b02770a206 Mon Sep 17 00:00:00 2001
From: Longshou Situ <lsitu@ucsd.edu>
Date: Mon, 14 Aug 2023 08:01:28 -0700
Subject: [PATCH 6/9] Add column_for_attribute for active fedora compatibility.

---
 app/forms/hyrax/forms/resource_batch_edit_form.rb | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/app/forms/hyrax/forms/resource_batch_edit_form.rb b/app/forms/hyrax/forms/resource_batch_edit_form.rb
index 4ef1ae9942..c466e1f80d 100644
--- a/app/forms/hyrax/forms/resource_batch_edit_form.rb
+++ b/app/forms/hyrax/forms/resource_batch_edit_form.rb
@@ -63,6 +63,13 @@ def self.build_permitted_params
       end
       # rubocop:enable Metrics/MethodLength
 
+      # @param name [Symbol]
+      # @return [Symbol]
+      # @note Added for ActiveModel compatibility.
+      def column_for_attribute(name)
+        name
+      end
+
       private
 
       # override this method if you need to initialize more complex RDF assertions (b-nodes)

From 49a5c9eabe86fc50d75b1ed07e2902e05c63de59 Mon Sep 17 00:00:00 2001
From: Longshou Situ <lsitu@ucsd.edu>
Date: Tue, 15 Aug 2023 07:53:13 -0700
Subject: [PATCH 7/9] Clean up valkyrie form fields to avoid transaction
 errors:

* Fix transaction error undefined method `<' for nil:NilClass

* Fix transaction error implicit conversion of Symbol into Integer.

* Refactor to fix lint error with the Metrics/AbcSize cop.
---
 app/controllers/hyrax/batch_edits_controller.rb | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/app/controllers/hyrax/batch_edits_controller.rb b/app/controllers/hyrax/batch_edits_controller.rb
index 17eb099fc2..22ff4c7bd4 100644
--- a/app/controllers/hyrax/batch_edits_controller.rb
+++ b/app/controllers/hyrax/batch_edits_controller.rb
@@ -63,6 +63,9 @@ def update_document(obj)
     def valkyrie_update_document(obj)
       form = form_class.new(obj, current_ability, nil)
       return unless form.validate(params[form_class.model_class.model_name.param_key])
+
+      cleanu_form_fields form
+
       result = transactions['change_set.update_work']
                .with_step_args('work_resource.save_acl' => { permissions_params: form.input_params["permissions"] })
                .call(form)
@@ -151,5 +154,15 @@ def redirect_to_return_controller
         redirect_to hyrax.dashboard_path
       end
     end
+
+    # Clean up form fields
+    # @param form Hyrax::Froms::ResourceBatchEditForm
+    def cleanu_form_fields(form)
+      form.lease = nil if form.lease && form.lease.fields['lease_expiration_date'].nil?
+      form.embargo = nil if form.embargo && form.embargo.fields['embargo_release_date'].nil?
+      form.fields.keys.each do |k|
+        form.fields[k] = nil if form.fields[k].is_a?(Array) && form.fields[k].blank?
+      end
+    end
   end
 end

From 8dfb21e1c49a7127d0e1b0b9fbfcd1c810e472cf Mon Sep 17 00:00:00 2001
From: Longshou Situ <lsitu@ucsd.edu>
Date: Tue, 15 Aug 2023 09:37:16 -0700
Subject: [PATCH 8/9] Correct permissions_attributes value format.

---
 spec/controllers/hyrax/batch_edits_controller_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/controllers/hyrax/batch_edits_controller_spec.rb b/spec/controllers/hyrax/batch_edits_controller_spec.rb
index 6bbc072cee..bf11343a39 100644
--- a/spec/controllers/hyrax/batch_edits_controller_spec.rb
+++ b/spec/controllers/hyrax/batch_edits_controller_spec.rb
@@ -177,7 +177,7 @@
 
       context 'with roles' do
         it 'updates roles' do
-          put :update, params: { update_type: "update", generic_work: { permissions_attributes: [{ type: 'person', access: 'read', name: 'foo@bar.com' }] } }
+          put :update, params: { update_type: "update", generic_work: { permissions_attributes: { "0" => { type: 'person', access: 'read', name: 'foo@bar.com' } } } }
           expect(response).to be_redirect
 
           work1 = GenericWork.find(one.id)

From 6f3099a11921415b7ee14ea1ea227f66b292a9ad Mon Sep 17 00:00:00 2001
From: Longshou Situ <lsitu@ucsd.edu>
Date: Tue, 15 Aug 2023 15:37:59 -0700
Subject: [PATCH 9/9] Fix failed integration tests for valkyrie.

---
 app/controllers/hyrax/batch_edits_controller.rb |  4 ++--
 spec/features/batch_edit_spec.rb                | 12 ++++++++----
 spec/support/features/batch_edit_actions.rb     | 10 +++++++++-
 3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/app/controllers/hyrax/batch_edits_controller.rb b/app/controllers/hyrax/batch_edits_controller.rb
index 22ff4c7bd4..3ed8ed30f2 100644
--- a/app/controllers/hyrax/batch_edits_controller.rb
+++ b/app/controllers/hyrax/batch_edits_controller.rb
@@ -64,7 +64,7 @@ def valkyrie_update_document(obj)
       form = form_class.new(obj, current_ability, nil)
       return unless form.validate(params[form_class.model_class.model_name.param_key])
 
-      cleanu_form_fields form
+      cleanup_form_fields form
 
       result = transactions['change_set.update_work']
                .with_step_args('work_resource.save_acl' => { permissions_params: form.input_params["permissions"] })
@@ -157,7 +157,7 @@ def redirect_to_return_controller
 
     # Clean up form fields
     # @param form Hyrax::Froms::ResourceBatchEditForm
-    def cleanu_form_fields(form)
+    def cleanup_form_fields(form)
       form.lease = nil if form.lease && form.lease.fields['lease_expiration_date'].nil?
       form.embargo = nil if form.embargo && form.embargo.fields['embargo_release_date'].nil?
       form.fields.keys.each do |k|
diff --git a/spec/features/batch_edit_spec.rb b/spec/features/batch_edit_spec.rb
index 61bd0919c1..4c88e15a0b 100644
--- a/spec/features/batch_edit_spec.rb
+++ b/spec/features/batch_edit_spec.rb
@@ -6,8 +6,8 @@
   let(:permission_template) { create(:permission_template, source_id: admin_set.id) }
   let!(:workflow) { create(:workflow, allows_access_grant: true, active: true, permission_template_id: permission_template.id) }
 
-  let!(:work1)       { create(:public_work, admin_set_id: admin_set.id, user: current_user, ordered_members: [file_set]) }
-  let!(:work2)       { create(:public_work, admin_set_id: admin_set.id, user: current_user) }
+  let!(:work1)       { create(:public_work, creator: ["Creator"], admin_set_id: admin_set.id, user: current_user, ordered_members: [file_set]) }
+  let!(:work2)       { create(:public_work, creator: ["Creator"], admin_set_id: admin_set.id, user: current_user) }
   let!(:file_set)    { create(:file_set) }
 
   before do
@@ -77,10 +77,12 @@
         batch_edit_expand('permissions_visibility')
         find('#generic_work_visibility_authenticated').click
         find('#permissions_visibility_save').click
+
+        ajax_wait(30)
         # This was `expect(page).to have_content 'Changes Saved'`, however in debugging,
         # the `have_content` check was ignoring the `within` scoping and finding
         # "Changes Saved" for other field areas
-        find('.status', text: 'Changes Saved', wait: 5)
+        find('.status', text: 'Changes Saved')
       end
 
       within "#form_permissions" do
@@ -91,10 +93,12 @@
         find('#collapse_permissions_sharing table', text: 'View/Download')
         find('#collapse_permissions_sharing table', text: 'donor')
         find('#permissions_sharing_save').click
+
+        ajax_wait(30)
         # This was `expect(page).to have_content 'Changes Saved'`, however in debugging,
         # the `have_content` check was ignoring the `within` scoping and finding
         # "Changes Saved" for other field areas
-        find('.status', text: 'Changes Saved', wait: 5)
+        find('.status', text: 'Changes Saved')
       end
 
       # Visit work permissions and verify
diff --git a/spec/support/features/batch_edit_actions.rb b/spec/support/features/batch_edit_actions.rb
index c6babcba8c..96a0631c7e 100644
--- a/spec/support/features/batch_edit_actions.rb
+++ b/spec/support/features/batch_edit_actions.rb
@@ -15,10 +15,12 @@ def fill_in_batch_edit_fields_and_verify!
       fill_in "generic_work_#{field_id}", with: "NEW #{field_id}"
 
       find("##{field_id}_save").click
+
+      ajax_wait(15)
       # This was `expect(page).to have_content 'Changes Saved'`, however in debugging,
       # the `have_content` check was ignoring the `within` scoping and finding
       # "Changes Saved" for other field areas
-      find('.status', text: 'Changes Saved', wait: 5)
+      find('.status', text: 'Changes Saved')
     end
   end
 end
@@ -27,3 +29,9 @@ def batch_edit_expand(field)
   find("#expand_link_#{field}").click
   yield if block_given?
 end
+
+def ajax_wait(s)
+  Timeout.timeout(s) do
+    loop until page.evaluate_script("jQuery.active").zero?
+  end
+end