From 216a695051e7f7f89d6ed01ff5c74d0640782249 Mon Sep 17 00:00:00 2001
From: Alisha Evans <alisha@scientist.com>
Date: Thu, 20 Jul 2023 17:44:52 -0500
Subject: [PATCH 01/18] fix the specs in embargo_actor_spec.rb

---
 .../actors/hyrax/actors/embargo_actor_spec.rb | 77 +++++++------------
 1 file changed, 27 insertions(+), 50 deletions(-)

diff --git a/spec/actors/hyrax/actors/embargo_actor_spec.rb b/spec/actors/hyrax/actors/embargo_actor_spec.rb
index 88cbce4c76..137041c041 100644
--- a/spec/actors/hyrax/actors/embargo_actor_spec.rb
+++ b/spec/actors/hyrax/actors/embargo_actor_spec.rb
@@ -4,64 +4,41 @@
   let(:authenticated_vis) { Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_AUTHENTICATED }
   let(:public_vis) { Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_PUBLIC }
 
-  describe "#destroy" do
-    let(:work) do
-      FactoryBot.valkyrie_create(:hyrax_resource, embargo: embargo)
-    end
+  describe '#destroy' do
+    let(:work) { FactoryBot.valkyrie_create(:hyrax_resource, embargo: embargo) }
     let(:embargo) { FactoryBot.create(:hyrax_embargo) }
 
-    before do
-      work.visibility = authenticated_vis
-      Hyrax::AccessControlList(work).save
-    end
-
-    it "removes the embargo" do
-      expect { actor.destroy }
-        .to change { Hyrax::EmbargoManager.new(resource: work).under_embargo? }
-        .from(true)
-        .to false
-    end
-
-    it "change the visibility" do
-      expect { actor.destroy }
-        .to change { work.visibility }
-        .from(authenticated_vis)
-        .to public_vis
-    end
-
-    context "with an expired embargo" do
-      let(:work) do
-        FactoryBot.valkyrie_create(:hyrax_resource, embargo: embargo)
-      end
-
-      let(:embargo) { FactoryBot.create(:hyrax_embargo) }
+    context 'on a Valkyrie backed model', if: Hyrax.config.use_valkyrie? do
       let(:embargo_manager) { Hyrax::EmbargoManager.new(resource: work) }
-      let(:embargo_release_date) { work.embargo.embargo_release_date }
+      let(:active_embargo_release_date) { work.embargo.embargo_release_date }
 
       before do
-        allow(Hyrax::TimeService)
-          .to receive(:time_in_utc)
-          .and_return(embargo_release_date.to_datetime + 1)
-        expect(embargo_manager.under_embargo?).to eq false
+        work.visibility = authenticated_vis
+        Hyrax::AccessControlList(work).save
       end
 
-      it "removes the embargo" do
+      it 'removes the embargo' do
         expect { actor.destroy }
-          .to change { work.embargo.embargo_release_date }
-          .from(embargo_release_date)
-          .to nil
+          .to change { embargo_manager.under_embargo? }
+          .from(true)
+          .to false
       end
 
-      it "releases the embargo" do
-        expect(embargo_manager.enforced?).to eq true
+      it 'releases the embargo' do
         expect { actor.destroy }
           .to change { embargo_manager.enforced? }
           .from(true)
           .to false
       end
 
-      it "changes the visibility" do
-        expect(work.embargo.visibility_after_embargo).to eq public_vis
+      it 'changes the embargo release date' do
+        expect { actor.destroy }
+          .to change { work.embargo.embargo_release_date }
+          .from(active_embargo_release_date)
+          .to nil
+      end
+
+      it 'changes the visibility' do
         expect { actor.destroy }
           .to change { work.visibility }
           .from(authenticated_vis)
@@ -69,23 +46,23 @@
       end
     end
 
-    context "with a ActiveFedora model" do
+    context 'with an ActiveFedora model', unless: Hyrax.config.use_valkyrie? do
       let(:work) do
         GenericWork.new do |work|
           work.apply_depositor_metadata 'foo'
-          work.title = ["test"]
+          work.title = ['test']
           work.visibility =
             work.visibility_during_embargo = authenticated_vis
           work.visibility_after_embargo = public_vis
-          work.embargo_release_date = release_date.to_s
+          work.embargo_release_date = embargo_release_date.to_s
           work.save(validate: false)
         end
       end
 
-      context "with an active embargo" do
-        let(:release_date) { Time.zone.today + 2 }
+      context 'with an active embargo' do
+        let(:embargo_release_date) { Time.zone.today + 2 }
 
-        it "removes the embargo" do
+        it 'removes the embargo' do
           actor.destroy
           expect(work.reload.embargo_release_date).to be_nil
           expect(work.visibility).to eq authenticated_vis
@@ -93,9 +70,9 @@
       end
 
       context 'with an expired embargo' do
-        let(:release_date) { Time.zone.today - 2 }
+        let(:embargo_release_date) { Time.zone.today - 2 }
 
-        it "removes the embargo" do
+        it 'removes the embargo' do
           actor.destroy
           expect(work.reload.embargo_release_date).to be_nil
           expect(work.visibility).to eq public_vis

From b79835eca033728d5f2855d247971e449e003664 Mon Sep 17 00:00:00 2001
From: Alisha Evans <alisha@scientist.com>
Date: Thu, 20 Jul 2023 18:31:35 -0500
Subject: [PATCH 02/18] wip: resource_spec.rb

---
 spec/models/hyrax/resource_spec.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/spec/models/hyrax/resource_spec.rb b/spec/models/hyrax/resource_spec.rb
index 28f3c46b7d..8490fd4724 100644
--- a/spec/models/hyrax/resource_spec.rb
+++ b/spec/models/hyrax/resource_spec.rb
@@ -21,9 +21,10 @@
 
     it 'saves the embargo id' do
       resource.embargo = Hyrax.persister.save(resource: embargo)
+      release_date = Hyrax.config.use_valkyrie? ? embargo.embargo_release_date : embargo.embargo_release_date.to_datetime
 
       expect(Hyrax.persister.save(resource: resource).embargo)
-        .to have_attributes(embargo_release_date: embargo.embargo_release_date)
+        .to have_attributes(embargo_release_date: release_date)
     end
   end
 

From 3ca717f15d74cf4c921cdc34ddabe438a481bd0e Mon Sep 17 00:00:00 2001
From: Alisha Evans <alisha@scientist.com>
Date: Fri, 21 Jul 2023 10:35:37 -0500
Subject: [PATCH 03/18] create the embargo in embargo_manager_spec.rb instead
 of building it so that it gets saved to the resource and embargo_manager
 properly.

---
 spec/factories/hyrax_embargo.rb             | 2 +-
 spec/services/hyrax/embargo_manager_spec.rb | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/factories/hyrax_embargo.rb b/spec/factories/hyrax_embargo.rb
index 7b71250155..2ac0196707 100644
--- a/spec/factories/hyrax_embargo.rb
+++ b/spec/factories/hyrax_embargo.rb
@@ -12,7 +12,7 @@
     end
 
     trait :expired do
-      embargo_release_date { Time.zone.today - 1 }
+      embargo_release_date { (Time.zone.today - 1).to_s }
     end
   end
 end
diff --git a/spec/services/hyrax/embargo_manager_spec.rb b/spec/services/hyrax/embargo_manager_spec.rb
index e60bb9c648..072f4d984a 100644
--- a/spec/services/hyrax/embargo_manager_spec.rb
+++ b/spec/services/hyrax/embargo_manager_spec.rb
@@ -10,7 +10,7 @@
 
   shared_context 'with expired embargo' do
     let(:resource) { FactoryBot.build(:hyrax_resource, embargo: embargo) }
-    let(:embargo)  { FactoryBot.build(:hyrax_embargo, :expired) }
+    let(:embargo)  { FactoryBot.create(:hyrax_embargo, :expired) }
   end
 
   describe '#apply' do

From a6a6f564aafb45f37d27e2f0f56b2191d7c0da86 Mon Sep 17 00:00:00 2001
From: Alisha Evans <alisha@scientist.com>
Date: Fri, 21 Jul 2023 11:13:40 -0500
Subject: [PATCH 04/18] more spec fixes

---
 spec/models/hyrax/resource_spec.rb          | 6 ++++--
 spec/services/hyrax/embargo_manager_spec.rb | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/spec/models/hyrax/resource_spec.rb b/spec/models/hyrax/resource_spec.rb
index 8490fd4724..a64df99082 100644
--- a/spec/models/hyrax/resource_spec.rb
+++ b/spec/models/hyrax/resource_spec.rb
@@ -17,7 +17,7 @@
 
   describe '#embargo' do
     subject(:resource) { described_class.new(embargo: embargo) }
-    let(:embargo)      { FactoryBot.build(:hyrax_embargo) }
+    let(:embargo)      { FactoryBot.create(:hyrax_embargo) }
 
     it 'saves the embargo id' do
       resource.embargo = Hyrax.persister.save(resource: embargo)
@@ -33,8 +33,10 @@
     let(:lease)        { FactoryBot.build(:hyrax_lease) }
 
     it 'saves the lease' do
+      expiration_date = Hyrax.config.use_valkyrie? ? lease.lease_expiration_date.to_s : lease.lease_expiration_date
+
       expect(Hyrax.persister.save(resource: resource).lease)
-        .to have_attributes(lease_expiration_date: lease.lease_expiration_date)
+        .to have_attributes(lease_expiration_date: expiration_date)
     end
   end
 
diff --git a/spec/services/hyrax/embargo_manager_spec.rb b/spec/services/hyrax/embargo_manager_spec.rb
index 072f4d984a..a03a364b11 100644
--- a/spec/services/hyrax/embargo_manager_spec.rb
+++ b/spec/services/hyrax/embargo_manager_spec.rb
@@ -129,7 +129,7 @@
         expect(manager.embargo)
           .to have_attributes visibility_after_embargo: 'open',
                               visibility_during_embargo: 'authenticated',
-                              embargo_release_date: an_instance_of(String),
+                              embargo_release_date: an_instance_of(Hyrax.config.use_valkyrie? ? String : DateTime),
                               embargo_history: match_array([])
       end
     end

From 1b12f24b3f92bd03d6d9351878ba22bb14eebb3d Mon Sep 17 00:00:00 2001
From: Alisha Evans <alisha@scientist.com>
Date: Fri, 21 Jul 2023 11:59:01 -0500
Subject: [PATCH 05/18] add an expired use case to the embargo actor specs for
 koppie

---
 .../actors/hyrax/actors/embargo_actor_spec.rb | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/spec/actors/hyrax/actors/embargo_actor_spec.rb b/spec/actors/hyrax/actors/embargo_actor_spec.rb
index 137041c041..5cb8b43b1a 100644
--- a/spec/actors/hyrax/actors/embargo_actor_spec.rb
+++ b/spec/actors/hyrax/actors/embargo_actor_spec.rb
@@ -5,10 +5,9 @@
   let(:public_vis) { Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_PUBLIC }
 
   describe '#destroy' do
-    let(:work) { FactoryBot.valkyrie_create(:hyrax_resource, embargo: embargo) }
-    let(:embargo) { FactoryBot.create(:hyrax_embargo) }
-
     context 'on a Valkyrie backed model', if: Hyrax.config.use_valkyrie? do
+      let(:work) { FactoryBot.valkyrie_create(:hyrax_resource, embargo: embargo) }
+      let(:embargo) { FactoryBot.create(:hyrax_embargo) }
       let(:embargo_manager) { Hyrax::EmbargoManager.new(resource: work) }
       let(:active_embargo_release_date) { work.embargo.embargo_release_date }
 
@@ -44,6 +43,20 @@
           .from(authenticated_vis)
           .to public_vis
       end
+
+      context 'with an expired embargo' do
+        let(:work) { valkyrie_create(:hyrax_resource, embargo: expired_embargo) }
+        let(:expired_embargo) { create(:hyrax_embargo, :expired) }
+        let(:embargo_manager) { Hyrax::EmbargoManager.new(resource: work) }
+        let(:embargo_release_date) { work.embargo.embargo_release_date }
+
+        it 'removes the embargo' do
+          expect { actor.destroy }
+            .to change { work.embargo.embargo_release_date }
+            .from(embargo_release_date)
+            .to nil
+        end
+      end
     end
 
     context 'with an ActiveFedora model', unless: Hyrax.config.use_valkyrie? do

From 8e77bca43a05a5ba3b0f731ee4c951f681fba1f4 Mon Sep 17 00:00:00 2001
From: Alisha Evans <alishaevn2@gmail.com>
Date: Fri, 4 Aug 2023 15:44:37 -0500
Subject: [PATCH 06/18] add some additionally functionality that was discovered
 while fixing leases in #6127

---
 .../hyrax/valkyrie_file_set_indexer.rb        | 13 +++++++
 app/services/hyrax/embargo_manager.rb         | 38 +++++++++++++++++--
 .../base/_form_permission_embargo.html.erb    |  2 +-
 lib/hyrax/transactions/steps/add_file_sets.rb |  6 +++
 lib/wings/model_transformer.rb                |  6 ++-
 5 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/app/indexers/hyrax/valkyrie_file_set_indexer.rb b/app/indexers/hyrax/valkyrie_file_set_indexer.rb
index 30afec87e0..4ee1d6c01b 100644
--- a/app/indexers/hyrax/valkyrie_file_set_indexer.rb
+++ b/app/indexers/hyrax/valkyrie_file_set_indexer.rb
@@ -22,6 +22,7 @@ def to_solr # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Met
         solr_doc['hasRelatedMediaFragment_ssim'] = resource.representative_id.to_s
         solr_doc['hasRelatedImage_ssim']         = resource.thumbnail_id.to_s
 
+        index_embargo(solr_doc)
         # Add in metadata from the original file.
         file_metadata = Hyrax::FileSetFileService.new(file_set: resource).original_file
         return solr_doc unless file_metadata
@@ -117,4 +118,16 @@ def file_format(file)
       end
     end
   end
+
+  def index_embargo(doc)
+    if resource.embargo&.active?
+      doc['embargo_release_date_dtsi'] = resource.embargo.embargo_release_date&.to_datetime
+      doc['visibility_after_embargo_ssim'] = resource.embargo.visibility_after_embargo
+      doc['visibility_during_embargo_ssim'] = resource.embargo.visibility_during_embargo
+    else
+      doc['embargo_history_ssim'] = resource&.embargo&.embargo_history
+    end
+
+    doc
+  end
 end
diff --git a/app/services/hyrax/embargo_manager.rb b/app/services/hyrax/embargo_manager.rb
index 5462064f0e..21fcc3a7eb 100644
--- a/app/services/hyrax/embargo_manager.rb
+++ b/app/services/hyrax/embargo_manager.rb
@@ -118,6 +118,35 @@ def release_embargo_for!(resource:, query_service: Hyrax.query_service)
         new(resource: resource, query_service: query_service)
           .release!
       end
+
+      # Creates or updates an existing embargo on a member to match the embargo on the parent work
+      # @param [Array<Valkyrie::Resource>] members
+      # @param [Hyrax::Work] work
+      # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
+      def create_or_update_embargo_on_members(members, work)
+        # TODO: account for all members and levels, not just file sets. ref: #6131
+
+        members.each do |member|
+          member_embargo_needs_updating = work.embargo.updated_at > member.embargo&.updated_at if member.embargo
+
+          if member.embargo && member_embargo_needs_updating
+            member.embargo.embargo_release_date = work.embargo['embargo_release_date']
+            member.embargo.visibility_during_embargo = work.embargo['visibility_during_embargo']
+            member.embargo.visibility_after_embargo = work.embargo['visibility_after_embargo']
+            member.embargo = Hyrax.persister.save(resource: member.embargo)
+          else
+            work_embargo_manager = Hyrax::EmbargoManager.new(resource: work)
+            work_embargo_manager.copy_embargo_to(target: member)
+            member = Hyrax.persister.save(resource: member)
+          end
+
+          user ||= ::User.find_by_user_key(member.depositor)
+          # the line below works in that it indexes the file set with the necessary lease properties
+          # I do not know however if this is the best event_id to pass
+          Hyrax.publisher.publish('object.metadata.updated', object: member, user: user)
+        end
+      end
+      # rubocop:enable Metrics/AbcSize, Metrics/MethodLength
     end
 
     # Deactivates the embargo and logs a message to the embargo_history property
@@ -126,13 +155,13 @@ def deactivate!
       embargo_record = embargo_history_message(
         embargo_state,
         Time.zone.today,
-        DateTime.parse(embargo.embargo_release_date),
+        embargo.embargo_release_date,
         embargo.visibility_during_embargo,
         embargo.visibility_after_embargo
       )
 
-      nullify(force: true)
       release(force: true)
+      nullify(force: true)
       embargo.embargo_history += [embargo_record]
     end
 
@@ -180,14 +209,17 @@ def embargo
     end
 
     ##
-    # Drop the embargo by setting its release date to `nil`.
+    # Drop the embargo by setting its release date and visibility settings to `nil`.
     #
     # @param force [boolean] force the nullify even when the embargo period is current
     #
     # @return [void]
     def nullify(force: false)
       return false if !force && under_embargo?
+
       embargo.embargo_release_date = nil
+      embargo.visibility_during_embargo = nil
+      embargo.visibility_after_embargo = nil
     end
 
     ##
diff --git a/app/views/hyrax/base/_form_permission_embargo.html.erb b/app/views/hyrax/base/_form_permission_embargo.html.erb
index 8688f60311..596246ecdb 100644
--- a/app/views/hyrax/base/_form_permission_embargo.html.erb
+++ b/app/views/hyrax/base/_form_permission_embargo.html.erb
@@ -1,6 +1,6 @@
 <div class="form-inline">
   <%= visibility_badge(Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_EMBARGO) %>
   <%= f.input :visibility_during_embargo, wrapper: :inline, collection: visibility_options(:restrict), include_blank: false %>
-  <%= f.input :embargo_release_date, wrapper: :inline, input_html: { value: f.object.embargo_release_date || Date.tomorrow, class: 'datepicker' } %>
+  <%= f.input :embargo_release_date, wrapper: :inline, input_html: { value: f.object.embargo_release_date&.to_date || Date.tomorrow, class: 'datepicker' } %>
   <%= f.input :visibility_after_embargo, wrapper: :inline, collection: visibility_options(:loosen), include_blank: false %>
 </div>
diff --git a/lib/hyrax/transactions/steps/add_file_sets.rb b/lib/hyrax/transactions/steps/add_file_sets.rb
index 4a22b4bb36..dbdbcf1e5e 100644
--- a/lib/hyrax/transactions/steps/add_file_sets.rb
+++ b/lib/hyrax/transactions/steps/add_file_sets.rb
@@ -25,6 +25,12 @@ def initialize(handler: Hyrax::WorkUploadsHandler)
         # @return [Dry::Monads::Result]
         def call(obj, uploaded_files: [], file_set_params: [])
           if @handler.new(work: obj).add(files: uploaded_files, file_set_params: file_set_params).attach
+            file_sets = obj.member_ids.map do |member|
+              Hyrax.query_service.find_by(id: member) if Hyrax.query_service.find_by(id: member).is_a? Hyrax::FileSet
+            end
+
+            Hyrax::LeaseManager.create_or_update_lease_on_members(file_sets, obj)if obj.lease
+            Hyrax::EmbargoManager.create_or_update_embargo_on_members(file_sets, obj)if obj.embargo
             Success(obj)
           else
             Failure[:failed_to_attach_file_sets, uploaded_files]
diff --git a/lib/wings/model_transformer.rb b/lib/wings/model_transformer.rb
index 091e644cf9..af3b972d19 100644
--- a/lib/wings/model_transformer.rb
+++ b/lib/wings/model_transformer.rb
@@ -53,7 +53,11 @@ def build
       attrs = attributes.tap { |hash| hash[:new_record] = pcdm_object.new_record? }
       attrs[:alternate_ids] = [::Valkyrie::ID.new(pcdm_object.id)] if pcdm_object.id
 
-      klass.new(**attrs).tap { |resource| ensure_current_permissions(resource) }
+      klass.new(**attrs).tap do |resource|
+        resource.lease = pcdm_object.lease&.valkyrie_resource if pcdm_object.respond_to?(:lease) && pcdm_object.lease
+        resource.embargo = pcdm_object.embargo&.valkyrie_resource if pcdm_object.respond_to?(:embargo) && pcdm_object.embargo
+        ensure_current_permissions(resource)
+      end
     end
 
     def ensure_current_permissions(resource)

From 6cd8798d229d602892273e40f19aa148018fc2b3 Mon Sep 17 00:00:00 2001
From: Alisha Evans <alishaevn2@gmail.com>
Date: Fri, 4 Aug 2023 17:50:43 -0500
Subject: [PATCH 07/18] update some more specs like whats in #6127.

---
 .../actors/hyrax/actors/embargo_actor_spec.rb |  9 ++++---
 spec/factories/hyrax_embargo.rb               |  4 +--
 spec/features/embargo_spec.rb                 | 10 +++----
 .../hyrax/valkyrie_file_set_indexer_spec.rb   | 26 +++++++++++++++++++
 4 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/spec/actors/hyrax/actors/embargo_actor_spec.rb b/spec/actors/hyrax/actors/embargo_actor_spec.rb
index 5cb8b43b1a..39a7c4fb79 100644
--- a/spec/actors/hyrax/actors/embargo_actor_spec.rb
+++ b/spec/actors/hyrax/actors/embargo_actor_spec.rb
@@ -17,10 +17,11 @@
       end
 
       it 'removes the embargo' do
-        expect { actor.destroy }
-          .to change { embargo_manager.under_embargo? }
-          .from(true)
-          .to false
+        actor.destroy
+
+        expect(work.embargo.embargo_release_date).to eq nil
+        expect(work.embargo.visibility_after_embargo).to eq nil
+        expect(work.embargo.visibility_during_embargo).to eq nil
       end
 
       it 'releases the embargo' do
diff --git a/spec/factories/hyrax_embargo.rb b/spec/factories/hyrax_embargo.rb
index 2ac0196707..2111df5281 100644
--- a/spec/factories/hyrax_embargo.rb
+++ b/spec/factories/hyrax_embargo.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 FactoryBot.define do
   factory :hyrax_embargo, class: "Hyrax::Embargo" do
-    embargo_release_date      { (Time.zone.today + 10).to_s }
+    embargo_release_date      { (Time.zone.today + 10).to_datetime }
     visibility_after_embargo  { 'open' }
     visibility_during_embargo { 'authenticated' }
 
@@ -12,7 +12,7 @@
     end
 
     trait :expired do
-      embargo_release_date { (Time.zone.today - 1).to_s }
+      embargo_release_date { (Time.zone.today - 1).to_datetime }
     end
   end
 end
diff --git a/spec/features/embargo_spec.rb b/spec/features/embargo_spec.rb
index 3b9204748f..f0c59e71ab 100644
--- a/spec/features/embargo_spec.rb
+++ b/spec/features/embargo_spec.rb
@@ -6,8 +6,8 @@
     sign_in user
   end
   describe 'creating an embargoed object' do
-    let(:future_date) { 5.days.from_now }
-    let(:later_future_date) { 10.days.from_now }
+    let(:future_date) { Time.zone.today + 5 }
+    let(:later_future_date) { Time.zone.today + 10 }
 
     it 'can be created, displayed and updated', :clean_repo, :workflow do
       visit '/concern/generic_works/new'
@@ -20,19 +20,19 @@
 
       # chosen embargo date is on the show page
       skip 'embargogeddon' do
-        expect(page).to have_content(future_date.to_date.to_formatted_s(:standard))
+        expect(page).to have_content(future_date.to_formatted_s(:standard))
       end
 
       click_link 'Edit'
       click_link 'Embargo Management Page'
 
       expect(page).to have_content('This Generic Work is under embargo.')
-      expect(page).to have_xpath("//input[@name='generic_work[embargo_release_date]' and @value='#{future_date.to_datetime.iso8601}']") # current embargo date is pre-populated in edit field
+      expect(page).to have_xpath("//input[@name='generic_work[embargo_release_date]' and @value='#{future_date.iso8601}']") # current embargo date is pre-populated in edit field
 
       fill_in 'until', with: later_future_date.to_s
 
       click_button 'Update Embargo'
-      expect(page).to have_content(later_future_date.to_date.to_formatted_s(:standard))
+      expect(page).to have_content(later_future_date.to_formatted_s(:standard))
 
       click_link 'Edit'
       fill_in 'Title', with: 'Embargo test CHANGED'
diff --git a/spec/indexers/hyrax/valkyrie_file_set_indexer_spec.rb b/spec/indexers/hyrax/valkyrie_file_set_indexer_spec.rb
index 2825a88a0a..1fdbcc7580 100644
--- a/spec/indexers/hyrax/valkyrie_file_set_indexer_spec.rb
+++ b/spec/indexers/hyrax/valkyrie_file_set_indexer_spec.rb
@@ -239,6 +239,32 @@
         expect(subject['original_file_id_ssi']).to eq "#{file_set.id}/files/#{file_set.original_file_id}"
       end
     end
+
+    context 'with a valid embargo' do
+      let(:embargo) { FactoryBot.create(:hyrax_embargo) }
+
+      before { allow(file_set).to receive(:embargo_id).and_return(embargo.id) }
+
+      it 'sets the embargo expiration date and visibility settings' do
+        expect(subject['embargo_release_date_dtsi']).to eq embargo.embargo_release_date
+        expect(subject['visibility_after_embargo_ssim']).to eq embargo.visibility_after_embargo
+        expect(subject['visibility_during_embargo_ssim']).to eq embargo.visibility_during_embargo
+        expect(subject['embargo_history_ssim']).to be nil
+      end
+    end
+
+    context 'with an expired embargo' do
+      let(:embargo) { FactoryBot.create(:hyrax_embargo, :expired) }
+
+      before { allow(file_set).to receive(:embargo_id).and_return(embargo.id) }
+
+      it 'sets the embargo expiration date and visibility settings' do
+        expect(subject['embargo_release_date_dtsi']).to be nil
+        expect(subject['visibility_after_embargo_ssim']).to be nil
+        expect(subject['visibility_during_embargo_ssim']).to be nil
+        expect(subject['embargo_history_ssim']).to eq embargo.embargo_history
+      end
+    end
   end
 
   describe '#file_format' do

From 34268c5e421cd37afa654433b9efa10bc607b2ff Mon Sep 17 00:00:00 2001
From: Alisha Evans <alishaevn2@gmail.com>
Date: Fri, 4 Aug 2023 17:51:16 -0500
Subject: [PATCH 08/18] remove all references to embargogeddon to see what we
 are working with now

---
 .../actors/interpret_visibility_actor_spec.rb | 12 +++---
 spec/features/actor_stack_spec.rb             |  4 +-
 spec/features/embargo_spec.rb                 |  4 +-
 spec/features/work_show_spec.rb               | 24 +++++------
 spec/forms/hyrax/forms/resource_form_spec.rb  | 12 +++---
 spec/models/hyrax/resource_spec.rb            |  6 +--
 spec/requests/riiif_spec.rb                   |  8 ++--
 spec/services/hyrax/embargo_manager_spec.rb   | 40 +++++++------------
 spec/wings/model_transformer_spec.rb          |  8 +---
 9 files changed, 45 insertions(+), 73 deletions(-)

diff --git a/spec/actors/hyrax/actors/interpret_visibility_actor_spec.rb b/spec/actors/hyrax/actors/interpret_visibility_actor_spec.rb
index 992300602b..a808fa1483 100644
--- a/spec/actors/hyrax/actors/interpret_visibility_actor_spec.rb
+++ b/spec/actors/hyrax/actors/interpret_visibility_actor_spec.rb
@@ -125,13 +125,11 @@
         let(:date) { Time.zone.today + 2 }
 
         it 'interprets and apply embargo and lease visibility settings' do
-          skip 'embargogeddon' do
-            subject.create(env)
-            expect(curation_concern.visibility_during_embargo).to eq 'authenticated'
-            expect(curation_concern.visibility_after_embargo).to eq 'open'
-            expect(curation_concern.visibility).to eq 'authenticated'
-            expect(curation_concern.reload).to be_under_embargo
-          end
+          subject.create(env)
+          expect(curation_concern.visibility_during_embargo).to eq 'authenticated'
+          expect(curation_concern.visibility_after_embargo).to eq 'open'
+          expect(curation_concern.visibility).to eq 'authenticated'
+          expect(curation_concern.reload).to be_under_embargo
         end
       end
 
diff --git a/spec/features/actor_stack_spec.rb b/spec/features/actor_stack_spec.rb
index 0982642dda..3211144dba 100644
--- a/spec/features/actor_stack_spec.rb
+++ b/spec/features/actor_stack_spec.rb
@@ -130,9 +130,7 @@ def create(env)
         expect(work.embargo_release_date).to be_falsey
         actor.create(env)
 
-        skip 'maybe embargogeddon....maybe not?' do
-          expect(work.class.find(work.id).embargo_release_date).to be_present
-        end
+        expect(work.class.find(work.id).embargo_release_date).to be_present
       end
 
       context 'and the embargo date is in the past' do
diff --git a/spec/features/embargo_spec.rb b/spec/features/embargo_spec.rb
index f0c59e71ab..174599da3f 100644
--- a/spec/features/embargo_spec.rb
+++ b/spec/features/embargo_spec.rb
@@ -19,9 +19,7 @@
       click_button 'Save'
 
       # chosen embargo date is on the show page
-      skip 'embargogeddon' do
-        expect(page).to have_content(future_date.to_formatted_s(:standard))
-      end
+      expect(page).to have_content(future_date.to_formatted_s(:standard))
 
       click_link 'Edit'
       click_link 'Embargo Management Page'
diff --git a/spec/features/work_show_spec.rb b/spec/features/work_show_spec.rb
index 1fccbc9e5c..d1fd779a24 100644
--- a/spec/features/work_show_spec.rb
+++ b/spec/features/work_show_spec.rb
@@ -70,19 +70,17 @@
     end
 
     it "allows adding work to a collection", clean_repo: true, js: true do
-      skip 'maybe embargogeddon....maybe not?' do
-        click_button "Add to collection" # opens the modal
-        # Really ensure that this Collection model is persisted
-        Collection.all.map(&:destroy!)
-        persisted_collection = FactoryBot.create(:collection_lw, user: user, collection_type: multi_membership_type_1)
-        select_member_of_collection(persisted_collection)
-        click_button 'Save changes'
-
-        # forwards to collection show page
-        expect(page).to have_content persisted_collection.title.first
-        expect(page).to have_content work.title.first
-        expect(page).to have_selector '.alert-success', text: 'Collection was successfully updated.'
-      end
+      click_button "Add to collection" # opens the modal
+      # Really ensure that this Collection model is persisted
+      Collection.all.map(&:destroy!)
+      persisted_collection = FactoryBot.create(:collection_lw, user: user, collection_type: multi_membership_type_1)
+      select_member_of_collection(persisted_collection)
+      click_button 'Save changes'
+
+      # forwards to collection show page
+      expect(page).to have_content persisted_collection.title.first
+      expect(page).to have_content work.title.first
+      expect(page).to have_selector '.alert-success', text: 'Collection was successfully updated.'
     end
   end
 
diff --git a/spec/forms/hyrax/forms/resource_form_spec.rb b/spec/forms/hyrax/forms/resource_form_spec.rb
index c289361dcd..3d58f46ec2 100644
--- a/spec/forms/hyrax/forms/resource_form_spec.rb
+++ b/spec/forms/hyrax/forms/resource_form_spec.rb
@@ -440,13 +440,11 @@
       it 'builds an embargo' do
         form.validate(params)
 
-        skip 'embargogeddon' do
-          expect { form.sync }
-            .to change { work.embargo }
-            .to have_attributes(embargo_release_date: Date.tomorrow.to_s,
-                                visibility_after_embargo: "open",
-                                visibility_during_embargo: "restricted")
-        end
+        expect { form.sync }
+          .to change { work.embargo }
+          .to have_attributes(embargo_release_date: Date.tomorrow.to_s,
+                              visibility_after_embargo: "open",
+                              visibility_during_embargo: "restricted")
       end
 
       it 'sets visibility to "during" value' do
diff --git a/spec/models/hyrax/resource_spec.rb b/spec/models/hyrax/resource_spec.rb
index b423d6b716..f0e09330de 100644
--- a/spec/models/hyrax/resource_spec.rb
+++ b/spec/models/hyrax/resource_spec.rb
@@ -23,10 +23,8 @@
       resource.embargo = Hyrax.persister.save(resource: embargo)
       release_date = Hyrax.config.use_valkyrie? ? embargo.embargo_release_date : embargo.embargo_release_date.to_datetime
 
-      skip 'embargogeddon' do
-        expect(Hyrax.persister.save(resource: resource).embargo)
-          .to have_attributes(embargo_release_date: embargo.embargo_release_date)
-      end
+      expect(Hyrax.persister.save(resource: resource).embargo)
+        .to have_attributes(embargo_release_date: embargo.embargo_release_date)
     end
   end
 
diff --git a/spec/requests/riiif_spec.rb b/spec/requests/riiif_spec.rb
index 48e59f2cd4..e23d391acb 100644
--- a/spec/requests/riiif_spec.rb
+++ b/spec/requests/riiif_spec.rb
@@ -17,11 +17,9 @@
       it "returns an image" do
         login_as user
 
-        skip 'maybe embargogeddon....maybe not?' do
-          get Riiif::Engine.routes.url_helpers.image_path(file.id, size: size, format: 'jpg', channels: nil)
-          expect(response).to have_http_status(:ok)
-          expect(response.content_type).to eq 'image/jpeg'
-        end
+        get Riiif::Engine.routes.url_helpers.image_path(file.id, size: size, format: 'jpg', channels: nil)
+        expect(response).to have_http_status(:ok)
+        expect(response.content_type).to eq 'image/jpeg'
       end
     end
 
diff --git a/spec/services/hyrax/embargo_manager_spec.rb b/spec/services/hyrax/embargo_manager_spec.rb
index 1c43cbf188..869aa6e4af 100644
--- a/spec/services/hyrax/embargo_manager_spec.rb
+++ b/spec/services/hyrax/embargo_manager_spec.rb
@@ -109,9 +109,7 @@
 
       before { resource.visibility = embargo.visibility_during_embargo }
 
-      skip 'embargogeddon' do
-        it { is_expected.to be_enforced }
-      end
+      it { is_expected.to be_enforced }
     end
   end
 
@@ -128,13 +126,11 @@
       end
 
       it 'has embargo attributes' do
-        skip 'embargogeddon' do
-          expect(manager.embargo)
-            .to have_attributes visibility_after_embargo: 'open',
-                                visibility_during_embargo: 'authenticated',
-                                embargo_release_date: an_instance_of(String),
-                                embargo_history: match_array([])
-        end
+        expect(manager.embargo)
+          .to have_attributes visibility_after_embargo: 'open',
+                              visibility_during_embargo: 'authenticated',
+                              embargo_release_date: an_instance_of(String),
+                              embargo_history: match_array([])
       end
     end
   end
@@ -150,12 +146,10 @@
     context 'with expired embargo' do
       include_context 'with expired embargo'
 
-      skip 'embargogeddon' do
-        it 'ensures the embargo release date is set to nil' do
-          expect(resource.embargo.embargo_release_date).to_not eq nil
-          manager.nullify
-          expect(resource.embargo.embargo_release_date).to eq nil
-        end
+      it 'ensures the embargo release date is set to nil' do
+        expect(resource.embargo.embargo_release_date).to_not eq nil
+        manager.nullify
+        expect(resource.embargo.embargo_release_date).to eq nil
       end
     end
 
@@ -192,21 +186,17 @@
       it 'ensures the post-embargo visibility is set' do
         manager.release
 
-        skip 'embargogeddon' do
-          expect(resource.visibility).to eq embargo.visibility_after_embargo
-        end
+        expect(resource.visibility).to eq embargo.visibility_after_embargo
       end
 
       context 'and embargo was applied' do
         before { resource.visibility = embargo.visibility_during_embargo }
 
         it 'ensures the post-embargo visibility is set' do
-          skip 'embargogeddon' do
-            expect { manager.release }
-              .to change { resource.visibility }
-              .from(embargo.visibility_during_embargo)
-              .to embargo.visibility_after_embargo
-          end
+          expect { manager.release }
+            .to change { resource.visibility }
+            .from(embargo.visibility_during_embargo)
+            .to embargo.visibility_after_embargo
         end
       end
     end
diff --git a/spec/wings/model_transformer_spec.rb b/spec/wings/model_transformer_spec.rb
index 22b4d93cf2..ae881ce2cc 100644
--- a/spec/wings/model_transformer_spec.rb
+++ b/spec/wings/model_transformer_spec.rb
@@ -141,9 +141,7 @@ class Book < Hyrax::Resource
       let(:work) { FactoryBot.build(:embargoed_work) }
 
       it 'has the correct embargo details' do
-        skip 'embargogeddon' do
-          expect(factory.build.embargo).to have_attributes work.embargo.attributes.symbolize_keys
-        end
+        expect(factory.build.embargo).to have_attributes work.embargo.attributes.symbolize_keys
       end
     end
 
@@ -153,9 +151,7 @@ class Book < Hyrax::Resource
       it 'has the correct embargo id' do
         work.embargo.save
 
-        skip 'embargogeddon' do
-          expect(subject.build.embargo.id.id).to eq work.embargo.id
-        end
+        expect(subject.build.embargo.id.id).to eq work.embargo.id
       end
     end
 

From 9ef88350d9294476aa9ba07f707cfb37f7a56afa Mon Sep 17 00:00:00 2001
From: Alisha Evans <alishaevn2@gmail.com>
Date: Fri, 4 Aug 2023 18:01:56 -0500
Subject: [PATCH 09/18] rubocop

---
 app/services/hyrax/embargo_manager.rb         | 2 +-
 lib/hyrax/transactions/steps/add_file_sets.rb | 4 ++--
 spec/models/hyrax/resource_spec.rb            | 1 -
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/app/services/hyrax/embargo_manager.rb b/app/services/hyrax/embargo_manager.rb
index 21fcc3a7eb..43757ab005 100644
--- a/app/services/hyrax/embargo_manager.rb
+++ b/app/services/hyrax/embargo_manager.rb
@@ -75,7 +75,7 @@ module Hyrax
   #   resource.visibility # => 'open'
   #   manager.enforced? => false
   #
-  class EmbargoManager
+  class EmbargoManager # robocop:disable Metrics/ClassLength
     ##
     # @!attribute [rw] resource
     #   @return [Hyrax::Resource]
diff --git a/lib/hyrax/transactions/steps/add_file_sets.rb b/lib/hyrax/transactions/steps/add_file_sets.rb
index dbdbcf1e5e..8d1d44a578 100644
--- a/lib/hyrax/transactions/steps/add_file_sets.rb
+++ b/lib/hyrax/transactions/steps/add_file_sets.rb
@@ -29,8 +29,8 @@ def call(obj, uploaded_files: [], file_set_params: [])
               Hyrax.query_service.find_by(id: member) if Hyrax.query_service.find_by(id: member).is_a? Hyrax::FileSet
             end
 
-            Hyrax::LeaseManager.create_or_update_lease_on_members(file_sets, obj)if obj.lease
-            Hyrax::EmbargoManager.create_or_update_embargo_on_members(file_sets, obj)if obj.embargo
+            Hyrax::LeaseManager.create_or_update_lease_on_members(file_sets, obj) if obj.lease
+            Hyrax::EmbargoManager.create_or_update_embargo_on_members(file_sets, obj) if obj.embargo
             Success(obj)
           else
             Failure[:failed_to_attach_file_sets, uploaded_files]
diff --git a/spec/models/hyrax/resource_spec.rb b/spec/models/hyrax/resource_spec.rb
index 55c6544d45..dfa57e1fce 100644
--- a/spec/models/hyrax/resource_spec.rb
+++ b/spec/models/hyrax/resource_spec.rb
@@ -21,7 +21,6 @@
 
     it 'saves the embargo id' do
       resource.embargo = Hyrax.persister.save(resource: embargo)
-      release_date = Hyrax.config.use_valkyrie? ? embargo.embargo_release_date : embargo.embargo_release_date.to_datetime
 
       expect(Hyrax.persister.save(resource: resource).embargo)
         .to have_attributes(embargo_release_date: embargo.embargo_release_date)

From 417132dc987359c98fe07768cba61cae237d64f6 Mon Sep 17 00:00:00 2001
From: Alisha Evans <alishaevn2@gmail.com>
Date: Fri, 4 Aug 2023 18:03:57 -0500
Subject: [PATCH 10/18] misspelled rubocop...

---
 app/services/hyrax/embargo_manager.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/services/hyrax/embargo_manager.rb b/app/services/hyrax/embargo_manager.rb
index 43757ab005..36903ac0a2 100644
--- a/app/services/hyrax/embargo_manager.rb
+++ b/app/services/hyrax/embargo_manager.rb
@@ -75,7 +75,7 @@ module Hyrax
   #   resource.visibility # => 'open'
   #   manager.enforced? => false
   #
-  class EmbargoManager # robocop:disable Metrics/ClassLength
+  class EmbargoManager # rubocop:disable Metrics/ClassLength
     ##
     # @!attribute [rw] resource
     #   @return [Hyrax::Resource]

From f82d1a4a6d7a671e75ce139e517f93ec601afb5e Mon Sep 17 00:00:00 2001
From: Alisha Evans <alishaevn2@gmail.com>
Date: Fri, 4 Aug 2023 18:35:35 -0500
Subject: [PATCH 11/18] accidentally put #index_embargo outside the
 ValkyrieFileSetIndexer class when fixing the merge conflicts

---
 .../hyrax/valkyrie_file_set_indexer.rb        | 20 +++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/app/indexers/hyrax/valkyrie_file_set_indexer.rb b/app/indexers/hyrax/valkyrie_file_set_indexer.rb
index 0fcf0dc7b2..fb2bbd7517 100644
--- a/app/indexers/hyrax/valkyrie_file_set_indexer.rb
+++ b/app/indexers/hyrax/valkyrie_file_set_indexer.rb
@@ -131,17 +131,17 @@ def index_lease(doc)
 
       doc
     end
-  end
 
-  def index_embargo(doc)
-    if resource.embargo&.active?
-      doc['embargo_release_date_dtsi'] = resource.embargo.embargo_release_date&.to_datetime
-      doc['visibility_after_embargo_ssim'] = resource.embargo.visibility_after_embargo
-      doc['visibility_during_embargo_ssim'] = resource.embargo.visibility_during_embargo
-    else
-      doc['embargo_history_ssim'] = resource&.embargo&.embargo_history
-    end
+    def index_embargo(doc)
+      if resource.embargo&.active?
+        doc['embargo_release_date_dtsi'] = resource.embargo.embargo_release_date&.to_datetime
+        doc['visibility_after_embargo_ssim'] = resource.embargo.visibility_after_embargo
+        doc['visibility_during_embargo_ssim'] = resource.embargo.visibility_during_embargo
+      else
+        doc['embargo_history_ssim'] = resource&.embargo&.embargo_history
+      end
 
-    doc
+      doc
+    end
   end
 end

From dd8f1f5b1bf63a905c81c37637b548ab47513cdd Mon Sep 17 00:00:00 2001
From: Alisha Evans <alishaevn2@gmail.com>
Date: Fri, 4 Aug 2023 18:50:53 -0500
Subject: [PATCH 12/18] fix a few more specs

---
 spec/features/embargo_spec.rb               | 16 ++++++++--------
 spec/models/hyrax/resource_spec.rb          |  2 +-
 spec/services/hyrax/embargo_manager_spec.rb |  2 +-
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/spec/features/embargo_spec.rb b/spec/features/embargo_spec.rb
index 174599da3f..36ab7ea6fe 100644
--- a/spec/features/embargo_spec.rb
+++ b/spec/features/embargo_spec.rb
@@ -25,7 +25,7 @@
       click_link 'Embargo Management Page'
 
       expect(page).to have_content('This Generic Work is under embargo.')
-      expect(page).to have_xpath("//input[@name='generic_work[embargo_release_date]' and @value='#{future_date.iso8601}']") # current embargo date is pre-populated in edit field
+      expect(page).to have_xpath("//input[@name='generic_work[embargo_release_date]' and @value='#{future_date}']") # current embargo date is pre-populated in edit field
 
       fill_in 'until', with: later_future_date.to_s
 
@@ -63,13 +63,13 @@
                          description: ["A description"],
                          with_permission_template: {})
     end
-    let(:future_date) { 5.days.from_now }
-    let(:later_future_date) { 10.days.from_now }
-    let(:invalid_future_date) { 185.days.from_now } # More than 6 months
+    let(:future_date) { Time.zone.today + 5 }
+    let(:later_future_date) { Time.zone.today + 10 }
+    let(:invalid_future_date) { Time.zone.today + 185 } # More than 6 months
     let(:admin) { create(:admin) }
     let(:work) do
       create(:work, title: ['embargoed work1'],
-                    embargo_release_date: future_date.to_datetime.iso8601,
+                    embargo_release_date: future_date,
                     visibility_after_embargo: 'open',
                     visibility_during_embargo: 'restricted',
                     admin_set_id: my_admin_set.id,
@@ -83,12 +83,12 @@
       click_link 'Embargo Management Page'
 
       expect(page).to have_content('This Generic Work is under embargo.')
-      expect(page).to have_xpath("//input[@name='generic_work[embargo_release_date]' and @value='#{future_date.to_datetime.iso8601}']") # current embargo date is pre-populated in edit field
+      expect(page).to have_xpath("//input[@name='generic_work[embargo_release_date]' and @value='#{future_date}']") # current embargo date is pre-populated in edit field
 
       fill_in 'until', with: later_future_date.to_s
 
       click_button 'Update Embargo'
-      expect(page).to have_content(later_future_date.to_date.to_formatted_s(:standard))
+      expect(page).to have_content(later_future_date.to_formatted_s(:standard))
       expect(page).to have_content(my_admin_set.title.first)
     end
 
@@ -99,7 +99,7 @@
       click_link 'Embargo Management Page'
 
       expect(page).to have_content('This Generic Work is under embargo.')
-      expect(page).to have_xpath("//input[@name='generic_work[embargo_release_date]' and @value='#{future_date.to_datetime.iso8601}']") # current embargo date is pre-populated in edit field
+      expect(page).to have_xpath("//input[@name='generic_work[embargo_release_date]' and @value='#{future_date}']") # current embargo date is pre-populated in edit field
 
       fill_in 'until', with: invalid_future_date.to_s
 
diff --git a/spec/models/hyrax/resource_spec.rb b/spec/models/hyrax/resource_spec.rb
index dfa57e1fce..f1543d4b17 100644
--- a/spec/models/hyrax/resource_spec.rb
+++ b/spec/models/hyrax/resource_spec.rb
@@ -35,7 +35,7 @@
       resource.lease = Hyrax.persister.save(resource: lease)
 
       expect(Hyrax.persister.save(resource: resource).lease)
-        .to have_attributes(lease_expiration_date: expiration_date)
+        .to have_attributes(lease_expiration_date: lease.lease_expiration_date)
     end
   end
 
diff --git a/spec/services/hyrax/embargo_manager_spec.rb b/spec/services/hyrax/embargo_manager_spec.rb
index 869aa6e4af..f6a8cc55f4 100644
--- a/spec/services/hyrax/embargo_manager_spec.rb
+++ b/spec/services/hyrax/embargo_manager_spec.rb
@@ -129,7 +129,7 @@
         expect(manager.embargo)
           .to have_attributes visibility_after_embargo: 'open',
                               visibility_during_embargo: 'authenticated',
-                              embargo_release_date: an_instance_of(String),
+                              embargo_release_date: an_instance_of(DateTime),
                               embargo_history: match_array([])
       end
     end

From a642e2f1a57bcd340291dc09b953616835b19910 Mon Sep 17 00:00:00 2001
From: Alisha Evans <alishaevn2@gmail.com>
Date: Fri, 4 Aug 2023 18:53:41 -0500
Subject: [PATCH 13/18] rubocop

---
 app/indexers/hyrax/valkyrie_file_set_indexer.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/indexers/hyrax/valkyrie_file_set_indexer.rb b/app/indexers/hyrax/valkyrie_file_set_indexer.rb
index fb2bbd7517..618cb59262 100644
--- a/app/indexers/hyrax/valkyrie_file_set_indexer.rb
+++ b/app/indexers/hyrax/valkyrie_file_set_indexer.rb
@@ -3,7 +3,7 @@
 module Hyrax
   ##
   # Indexes Hyrax::FileSet objects
-  class ValkyrieFileSetIndexer < Hyrax::ValkyrieIndexer
+  class ValkyrieFileSetIndexer < Hyrax::ValkyrieIndexer # rubocop:disable Metrics/ClassLength
     include Hyrax::ResourceIndexer
     include Hyrax::PermissionIndexer
     include Hyrax::VisibilityIndexer

From cf7b699271d73721a0f33c5c6af7a7862aaa9a49 Mon Sep 17 00:00:00 2001
From: Alisha Evans <alishaevn2@gmail.com>
Date: Fri, 4 Aug 2023 18:59:40 -0500
Subject: [PATCH 14/18] refactor #apply_attributes_to_model so that it contains
 #perform_lease_conversion and #perform_embargo_conversion

---
 lib/wings/active_fedora_converter.rb | 30 ++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/lib/wings/active_fedora_converter.rb b/lib/wings/active_fedora_converter.rb
index 1734c0a24e..dc8bfbf20a 100644
--- a/lib/wings/active_fedora_converter.rb
+++ b/lib/wings/active_fedora_converter.rb
@@ -130,7 +130,6 @@ def normal_attributes
 
     ##
     # apply attributes to the ActiveFedora model
-    # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength
     def apply_attributes_to_model(af_object)
       case af_object
       when Hydra::AccessControl
@@ -142,20 +141,13 @@ def apply_attributes_to_model(af_object)
         members = Array.wrap(converted_attrs.delete(:members))
         files = converted_attrs.delete(:files)
         af_object.attributes = converted_attrs
-        if resource.try(:lease) && af_object.reflections.include?(:lease)
-          # TODO(#6134): af_object.lease.class has the same name as resource.lease.class; however, each class has a different object_id
-          # so a type mismatch happens. the code below coerces the one object into the other
-          unless af_object.lease&.id
-            resource_lease_dup = af_object.reflections.fetch(:lease).klass.new(resource.lease.attributes.except(:id, :internal_resource, :created_at, :updated_at, :new_record))
-            af_object.lease = resource_lease_dup
-          end
-        end
+        perform_lease_conversion(af_object: af_object, resource: resource) if resource.try(:lease) && af_object.reflections.include?(:lease)
+        perform_embargo_conversion(af_object: af_object, resource: resource) if resource.try(:embargo) && af_object.reflections.include?(:embargo)
         members.empty? ? af_object.try(:ordered_members)&.clear : af_object.try(:ordered_members=, members)
         af_object.try(:members)&.replace(members)
         af_object.files.build_or_set(files) if files
       end
     end
-    # rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength
 
     # Add attributes from resource which aren't AF properties into af_object
     def add_access_control_attributes(af_object)
@@ -171,5 +163,23 @@ def add_file_attributes(af_object)
       af_object.metadata_node.type = new_type if new_type
       af_object.mime_type = resource.mime_type
     end
+
+    def perform_lease_conversion((af_object:, resource:))
+      # TODO(#6134): af_object.lease.class has the same name as resource.lease.class; however, each class has a different object_id
+      # so a type mismatch happens. the code below coerces the one object into the other
+      unless af_object.lease&.id
+        resource_lease_dup = af_object.reflections.fetch(:lease).klass.new(resource.lease.attributes.except(:id, :internal_resource, :created_at, :updated_at, :new_record))
+        af_object.lease = resource_lease_dup
+      end
+    end
+
+    def perform_embargo_conversion((af_object:, resource:))
+      # TODO(#6134): af_object.embargo.class has the same name as resource.embargo.class; however, each class has a different object_id
+      # so a type mismatch happens. the code below coerces the one object into the other
+      unless af_object.embargo&.id
+        resource_embargo_dup = af_object.reflections.fetch(:embargo).klass.new(resource.embargo.attributes.except(:id, :internal_resource, :created_at, :updated_at, :new_record))
+        af_object.embargo = resource_embargo_dup
+      end
+    end
   end
 end

From 5196d1459f0293b79d43dc95efa04359a55bc347 Mon Sep 17 00:00:00 2001
From: Alisha Evans <alishaevn2@gmail.com>
Date: Fri, 4 Aug 2023 19:26:18 -0500
Subject: [PATCH 15/18] remove the double parens for #perform_lease_conversion
 and #perform_embargo_conversion

---
 lib/wings/active_fedora_converter.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/wings/active_fedora_converter.rb b/lib/wings/active_fedora_converter.rb
index dc8bfbf20a..552a8cf50a 100644
--- a/lib/wings/active_fedora_converter.rb
+++ b/lib/wings/active_fedora_converter.rb
@@ -164,7 +164,7 @@ def add_file_attributes(af_object)
       af_object.mime_type = resource.mime_type
     end
 
-    def perform_lease_conversion((af_object:, resource:))
+    def perform_lease_conversion(af_object:, resource:)
       # TODO(#6134): af_object.lease.class has the same name as resource.lease.class; however, each class has a different object_id
       # so a type mismatch happens. the code below coerces the one object into the other
       unless af_object.lease&.id
@@ -173,7 +173,7 @@ def perform_lease_conversion((af_object:, resource:))
       end
     end
 
-    def perform_embargo_conversion((af_object:, resource:))
+    def perform_embargo_conversion(af_object:, resource:)
       # TODO(#6134): af_object.embargo.class has the same name as resource.embargo.class; however, each class has a different object_id
       # so a type mismatch happens. the code below coerces the one object into the other
       unless af_object.embargo&.id

From 7f3efe08946be55086ba01ab91cbbb860131dda0 Mon Sep 17 00:00:00 2001
From: Alisha Evans <alishaevn2@gmail.com>
Date: Fri, 4 Aug 2023 19:38:21 -0500
Subject: [PATCH 16/18] more rubocop

---
 lib/wings/active_fedora_converter.rb | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/lib/wings/active_fedora_converter.rb b/lib/wings/active_fedora_converter.rb
index 552a8cf50a..4a4e6c0dca 100644
--- a/lib/wings/active_fedora_converter.rb
+++ b/lib/wings/active_fedora_converter.rb
@@ -18,7 +18,7 @@ module Wings
   #
   # @note the `Valkyrie::Resource` object passed to this class **must** have an
   #   `#internal_resource` mapping it to an `ActiveFedora::Base` class.
-  class ActiveFedoraConverter
+  class ActiveFedoraConverter # rubocop:disable Metrics/ClassLength
     ##
     # Accesses the Class implemented for handling resource attributes
     # @return [Class]
@@ -130,6 +130,7 @@ def normal_attributes
 
     ##
     # apply attributes to the ActiveFedora model
+    # rubocop:disable Metrics/CyclomaticComplexity, Metrics/MethodLength
     def apply_attributes_to_model(af_object)
       case af_object
       when Hydra::AccessControl
@@ -148,6 +149,7 @@ def apply_attributes_to_model(af_object)
         af_object.files.build_or_set(files) if files
       end
     end
+    # rubocop:enable Metrics/CyclomaticComplexity, Metrics/MethodLength
 
     # Add attributes from resource which aren't AF properties into af_object
     def add_access_control_attributes(af_object)
@@ -167,19 +169,19 @@ def add_file_attributes(af_object)
     def perform_lease_conversion(af_object:, resource:)
       # TODO(#6134): af_object.lease.class has the same name as resource.lease.class; however, each class has a different object_id
       # so a type mismatch happens. the code below coerces the one object into the other
-      unless af_object.lease&.id
-        resource_lease_dup = af_object.reflections.fetch(:lease).klass.new(resource.lease.attributes.except(:id, :internal_resource, :created_at, :updated_at, :new_record))
-        af_object.lease = resource_lease_dup
-      end
+      return if af_object.lease&.id
+
+      resource_lease_dup = af_object.reflections.fetch(:lease).klass.new(resource.lease.attributes.except(:id, :internal_resource, :created_at, :updated_at, :new_record))
+      af_object.lease = resource_lease_dup
     end
 
     def perform_embargo_conversion(af_object:, resource:)
       # TODO(#6134): af_object.embargo.class has the same name as resource.embargo.class; however, each class has a different object_id
       # so a type mismatch happens. the code below coerces the one object into the other
-      unless af_object.embargo&.id
-        resource_embargo_dup = af_object.reflections.fetch(:embargo).klass.new(resource.embargo.attributes.except(:id, :internal_resource, :created_at, :updated_at, :new_record))
-        af_object.embargo = resource_embargo_dup
-      end
+      return if af_object.embargo&.id
+
+      resource_embargo_dup = af_object.reflections.fetch(:embargo).klass.new(resource.embargo.attributes.except(:id, :internal_resource, :created_at, :updated_at, :new_record))
+      af_object.embargo = resource_embargo_dup
     end
   end
 end

From c954d38c714be84f94d4eb4af84d03590a3e353b Mon Sep 17 00:00:00 2001
From: Alisha Evans <alishaevn2@gmail.com>
Date: Mon, 7 Aug 2023 09:53:17 -0500
Subject: [PATCH 17/18] skip spec/requests/riiif_spec.rb:17 and
 spec/features/work_show_spec.rb:72 because the failures are unrelated to the
 #5844 embargo work.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

both of these specs were failing on main before #6098 was accidentally merged to main with a failing pipeline. in an attempt to get main back to a green pipeline I skipped all failing specs, including these two. ref: https://github.com/samvera/hyrax/commit/58de8c60d9d85bf6392964a087d8717e6bba59df.

this pr was to fix the broken embargo specs from #6098. these two are still unrelated however and will remain skipped. only in valkyrie, as that is where they are failing. there is work being done on the valkyrie spec suite to fix its several problems. hopefully those fixes will also resolve these issues.

as for the riif spec specifically, @orangewolf did some further digging and found: "It should have never been passing because Riiif looks up the file metadata but no file metadata is ever created. Before our valk work, riiif wasn’t working at all. So I do not know how this could have ever passed in Valkyrie land."
---
 spec/features/work_show_spec.rb | 2 +-
 spec/requests/riiif_spec.rb     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/features/work_show_spec.rb b/spec/features/work_show_spec.rb
index d1fd779a24..2aa172597e 100644
--- a/spec/features/work_show_spec.rb
+++ b/spec/features/work_show_spec.rb
@@ -69,7 +69,7 @@
       )
     end
 
-    it "allows adding work to a collection", clean_repo: true, js: true do
+    it "allows adding work to a collection", clean_repo: true, js: true, skip: Hyrax.config.use_valkyrie? && 'this failure is unrelated to embargoes (#5844). waiting for valkyrie spec suite improvements' do
       click_button "Add to collection" # opens the modal
       # Really ensure that this Collection model is persisted
       Collection.all.map(&:destroy!)
diff --git a/spec/requests/riiif_spec.rb b/spec/requests/riiif_spec.rb
index e23d391acb..d48d64e5c5 100644
--- a/spec/requests/riiif_spec.rb
+++ b/spec/requests/riiif_spec.rb
@@ -14,7 +14,7 @@
 
   describe 'GET /images/:id' do
     context "when the user is authorized" do
-      it "returns an image" do
+      it "returns an image", skip: Hyrax.config.use_valkyrie? && 'this failure is unrelated to embargoes (#5844). waiting for valkyrie spec suite improvements' do
         login_as user
 
         get Riiif::Engine.routes.url_helpers.image_path(file.id, size: size, format: 'jpg', channels: nil)

From a50fb5aed9d9c0889b8c8d79eca7e7c85e59a9ee Mon Sep 17 00:00:00 2001
From: Alisha Evans <alishaevn2@gmail.com>
Date: Mon, 7 Aug 2023 09:57:15 -0500
Subject: [PATCH 18/18] rubocop

---
 spec/features/work_show_spec.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/spec/features/work_show_spec.rb b/spec/features/work_show_spec.rb
index 2aa172597e..ffe058d3ae 100644
--- a/spec/features/work_show_spec.rb
+++ b/spec/features/work_show_spec.rb
@@ -69,7 +69,8 @@
       )
     end
 
-    it "allows adding work to a collection", clean_repo: true, js: true, skip: Hyrax.config.use_valkyrie? && 'this failure is unrelated to embargoes (#5844). waiting for valkyrie spec suite improvements' do
+    it "allows adding work to a collection", clean_repo: true, js: true,
+                                             skip: Hyrax.config.use_valkyrie? && 'this failure is unrelated to embargoes (#5844). waiting for valkyrie spec suite improvements' do
       click_button "Add to collection" # opens the modal
       # Really ensure that this Collection model is persisted
       Collection.all.map(&:destroy!)