Merge pull request #6114 from samvera/5844-embargo-specs

5844 embargo specs

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
@@ -23,6 +23,7 @@ def to_solr # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Met
         solr_doc['hasRelatedImage_ssim']         = resource.thumbnail_id.to_s
 
         index_lease(solr_doc)
+        index_embargo(solr_doc)
 
         # Add in metadata from the original file.
         file_metadata = Hyrax::FileSetFileService.new(file_set: resource).original_file
@@ -130,5 +131,17 @@ def index_lease(doc)
 
       doc
     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
 end

app/services/hyrax/embargo_manager.rb

@@ -75,7 +75,7 @@ module Hyrax
   #   resource.visibility # => 'open'
   #   manager.enforced? => false
   #
-  class EmbargoManager
+  class EmbargoManager # rubocop:disable Metrics/ClassLength
     ##
     # @!attribute [rw] resource
     #   @return [Hyrax::Resource]
@@ -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

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>

lib/hyrax/transactions/steps/add_file_sets.rb

@@ -25,14 +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
-            if obj.lease
-              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)
+            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]

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,7 +130,7 @@ def normal_attributes
 
     ##
     # apply attributes to the ActiveFedora model
-    # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength
+    # rubocop:disable Metrics/CyclomaticComplexity, Metrics/MethodLength
     def apply_attributes_to_model(af_object)
       case af_object
       when Hydra::AccessControl
@@ -142,20 +142,14 @@ 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
+    # 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)
@@ -171,5 +165,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
+      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
+      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

lib/wings/model_transformer.rb

@@ -56,9 +56,9 @@ def build
 
       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
-      # rubocop:enable Metrics/AbcSize
     end
 
     def ensure_current_permissions(resource)

spec/actors/hyrax/actors/embargo_actor_spec.rb

@@ -4,108 +4,89 @@
   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
-    let(:embargo) { FactoryBot.create(:hyrax_embargo) }
+  describe '#destroy' do
+    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 }
 
-    before do
-      work.visibility = authenticated_vis
-      Hyrax::AccessControlList(work).save
-    end
+      before do
+        work.visibility = authenticated_vis
+        Hyrax::AccessControlList(work).save
+      end
+
+      it 'removes the embargo' do
+        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 "removes the embargo" do
-      skip 'embargogeddon' do
+      it 'releases the embargo' do
         expect { actor.destroy }
-          .to change { Hyrax::EmbargoManager.new(resource: work).under_embargo? }
+          .to change { embargo_manager.enforced? }
           .from(true)
           .to false
       end
-    end
 
-    it "change the visibility" do
-      skip 'embargogeddon' do
+      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)
           .to public_vis
       end
-    end
 
-    context "with an expired embargo" do
-      let(:work) do
-        FactoryBot.valkyrie_create(:hyrax_resource, embargo: embargo)
-      end
-
-      let(:embargo) { FactoryBot.create(:hyrax_embargo) }
-      let(:embargo_manager) { Hyrax::EmbargoManager.new(resource: work) }
-      let(: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
-      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
-        skip 'embargogeddon' do
+        it 'removes the embargo' do
           expect { actor.destroy }
             .to change { work.embargo.embargo_release_date }
             .from(embargo_release_date)
             .to nil
         end
       end
-
-      it "releases the embargo" do
-        skip 'embargogeddon' do
-          expect(embargo_manager.enforced?).to eq true
-          expect { actor.destroy }
-            .to change { embargo_manager.enforced? }
-            .from(true)
-            .to false
-        end
-      end
-
-      it "changes the visibility" do
-        skip 'embargogeddon' do
-          expect(work.embargo.visibility_after_embargo).to eq public_vis
-          expect { actor.destroy }
-            .to change { work.visibility }
-            .from(authenticated_vis)
-            .to public_vis
-        end
-      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
         end
       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

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
 

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 }
+      embargo_release_date { (Time.zone.today - 1).to_datetime }
     end
   end
 end

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