5844 deactivated embargoes

app/actors/hyrax/actors/embargo_actor.rb

@@ -16,8 +16,9 @@ def destroy
         when Valkyrie::Resource
           embargo_manager = Hyrax::EmbargoManager.new(resource: work)
           return if embargo_manager.embargo.embargo_release_date.blank?
-          embargo_manager.nullify(force: true)
-          embargo_manager.release(force: true)
+
+          embargo_manager.deactivate!
+          work.embargo = Hyrax.persister.save(resource: embargo_manager.embargo)
           Hyrax::AccessControlList(work).save
         else
           work.embargo_visibility! # If the embargo has lapsed, update the current visibility.

app/controllers/concerns/hyrax/embargoes_controller_behavior.rb

@@ -15,7 +15,7 @@ def index
     # Removes a single embargo
     def destroy
       Hyrax::Actors::EmbargoActor.new(curation_concern).destroy
-      flash[:notice] = embargo_history(curation_concern)
+      flash[:notice] = embargo_history(curation_concern).last
       if curation_concern.work? && work_has_file_set_members?(curation_concern)
         redirect_to confirm_permission_path
       else

app/forms/hyrax/forms/work_embargo_form.rb

@@ -11,6 +11,7 @@ module Forms
     #   +EmbargoesControllerBehavior+.
     class WorkEmbargoForm < Hyrax::ChangeSet
       property :embargo, form: Hyrax::Forms::Embargo, populator: :embargo_populator, prepopulator: :embargo_populator
+      property :embargo_history, virtual: true, prepopulator: proc { |_opts| self.embargo_history = model.embargo&.embargo_history }
       property :embargo_release_date, virtual: true, prepopulator: proc { |_opts| self.embargo_release_date = model.embargo&.embargo_release_date }
       property :visibility_after_embargo, virtual: true, prepopulator: proc { |_opts| self.visibility_after_embargo = model.embargo&.visibility_after_embargo }
       property :visibility_during_embargo, virtual: true, prepopulator: proc { |_opts| self.visibility_during_embargo = model.embargo&.visibility_during_embargo }

app/indexers/hyrax/valkyrie_work_indexer.rb

@@ -44,11 +44,14 @@ def admin_set_label(resource)
     end
 
     def index_embargo(doc)
-      if Hyrax::EmbargoManager.new(resource: resource).under_embargo?
+      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
 

app/models/hyrax/resource.rb

@@ -35,7 +35,7 @@ class Resource < Valkyrie::Resource
     include Hyrax::WithEvents
 
     attribute :alternate_ids, Valkyrie::Types::Array.of(Valkyrie::Types::ID)
-    attribute :embargo,       Hyrax::Embargo.optional
+    attribute :embargo_id,    Valkyrie::Types::ID
     attribute :lease,         Hyrax::Lease.optional
 
     delegate :edit_groups, :edit_groups=,
@@ -103,6 +103,17 @@ def visibility
       visibility_reader.read
     end
 
+    def embargo=(value)
+      raise TypeError "can't convert #{value.class} into Hyrax::Embargo" unless value.is_a? Hyrax::Embargo
+
+      @embargo = value
+      self.embargo_id = @embargo.id
+    end
+
+    def embargo
+      @embargo ||= (Hyrax.query_service.find_by(id: embargo_id) if embargo_id.present?)
+    end
+
     protected
 
     def visibility_writer

app/services/hyrax/embargo_manager.rb

@@ -17,6 +17,7 @@ module Hyrax
   #    - "Enforced" means the object's visibility matches the pre-release
   #      visibility of the embargo; i.e. the embargo has been applied,
   #      but not released.
+  #      "Deactivate" means that the existing embargo will be removed
   #
   # Note that an resource may be `#under_embargo?` even if the embargo is not
   # be `#enforced?` (in this case, the application should seek to apply the
@@ -119,6 +120,22 @@ def release_embargo_for!(resource:, query_service: Hyrax.query_service)
       end
     end
 
+    # Deactivates the embargo and logs a message to the embargo_history property
+    def deactivate!
+      embargo_state = embargo.active? ? 'active' : 'expired'
+      embargo_record = embargo_history_message(
+        embargo_state,
+        Time.zone.today,
+        DateTime.parse(embargo.embargo_release_date),
+        embargo.visibility_during_embargo,
+        embargo.visibility_after_embargo
+      )
+
+      nullify(force: true)
+      release(force: true)
+      embargo.embargo_history += [embargo_record]
+    end
+
     ##
     # Copies and applies the embargo to a new (target) resource.
     #
@@ -128,7 +145,7 @@ def release_embargo_for!(resource:, query_service: Hyrax.query_service)
     def copy_embargo_to(target:)
       return false unless under_embargo?
 
-      target.embargo = Embargo.new(clone_attributes)
+      target.embargo = Hyrax.persister.save(resource: Embargo.new(clone_attributes))
       self.class.apply_embargo_for(resource: target)
     end
 
@@ -152,7 +169,8 @@ def apply!
     ##
     # @return [Boolean]
     def enforced?
-      embargo.visibility_during_embargo.to_s == resource.visibility
+      embargo.embargo_release_date.present? &&
+        (embargo.visibility_during_embargo.to_s == resource.visibility)
     end
 
     ##
@@ -213,5 +231,17 @@ def clone_attributes
     def core_attribute_keys
       [:visibility_after_embargo, :visibility_during_embargo, :embargo_release_date]
     end
+
+    protected
+
+    # Create the log message used when deactivating an embargo
+    def embargo_history_message(state, deactivate_date, release_date, visibility_during, visibility_after)
+      I18n.t 'hydra.embargo.history_message',
+              state: state,
+              deactivate_date: deactivate_date,
+              release_date: release_date,
+              visibility_during: visibility_during,
+              visibility_after: visibility_after
+    end
   end
 end

lib/hyrax/transactions/steps/save.rb

@@ -33,6 +33,7 @@ def call(change_set, user: nil)
           begin
             new_collections = changed_collection_membership(change_set)
             unsaved = change_set.sync
+            unsaved.embargo = @persister.save(resource: unsaved.embargo) if unsaved.embargo.present?
             saved = @persister.save(resource: unsaved)
           rescue StandardError => err
             return Failure(["Failed save on #{change_set}\n\t#{err.message}", change_set.resource])

spec/actors/hyrax/actors/embargo_actor_spec.rb

@@ -6,8 +6,9 @@
 
   describe "#destroy" do
     let(:work) do
-      FactoryBot.valkyrie_create(:hyrax_resource, :under_embargo)
+      FactoryBot.valkyrie_create(:hyrax_resource, embargo: embargo)
     end
+    let(:embargo) { FactoryBot.create(:hyrax_embargo) }
 
     before do
       work.visibility = authenticated_vis
@@ -33,14 +34,14 @@
         FactoryBot.valkyrie_create(:hyrax_resource, embargo: embargo)
       end
 
-      let(:embargo) { FactoryBot.build(:hyrax_embargo) }
+      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 + 1)
+          .and_return(embargo_release_date.to_datetime + 1)
         expect(embargo_manager.under_embargo?).to eq false
       end
 

spec/factories/hyrax_embargo.rb

@@ -1,12 +1,14 @@
 # frozen_string_literal: true
 FactoryBot.define do
   factory :hyrax_embargo, class: "Hyrax::Embargo" do
-    embargo_release_date      { Time.zone.today + 10 }
+    embargo_release_date      { (Time.zone.today + 10).to_s }
     visibility_after_embargo  { 'open' }
     visibility_during_embargo { 'authenticated' }
 
     to_create do |instance|
-      Valkyrie.config.metadata_adapter.persister.save(resource: instance)
+      saved_instance = Valkyrie.config.metadata_adapter.persister.save(resource: instance)
+      instance.id = saved_instance.id
+      saved_instance
     end
 
     trait :expired do

spec/factories/hyrax_resource.rb

@@ -5,7 +5,7 @@
 FactoryBot.define do
   factory :hyrax_resource, class: "Hyrax::Resource" do
     trait :under_embargo do
-      association :embargo, factory: :hyrax_embargo
+      embargo_id { FactoryBot.create(:hyrax_embargo).id }
     end
 
     trait :under_lease do

spec/models/hyrax/resource_spec.rb

@@ -19,7 +19,9 @@
     subject(:resource) { described_class.new(embargo: embargo) }
     let(:embargo)      { FactoryBot.build(:hyrax_embargo) }
 
-    it 'saves the embargo' do
+    it 'saves the embargo id' do
+      resource.embargo = Hyrax.persister.save(resource: embargo)
+
       expect(Hyrax.persister.save(resource: resource).embargo)
         .to have_attributes(embargo_release_date: embargo.embargo_release_date)
     end

spec/services/hyrax/embargo_manager_spec.rb

@@ -129,8 +129,8 @@
         expect(manager.embargo)
           .to have_attributes visibility_after_embargo: 'open',
                               visibility_during_embargo: 'authenticated',
-                              embargo_release_date: an_instance_of(Date),
-                              embargo_history: be_empty
+                              embargo_release_date: an_instance_of(String),
+                              embargo_history: match_array([])
       end
     end
   end