Adjusting lease/embargo_enforced helper logic

This reflects moving the change from e1fd224 — "Fixing rendering logic
for visibility component" into the helper method.  This suggestion comes
from [no_reply's review comment][1], and makes sense.  I don't think we
want to push the logic further down the stack, as there's a case
statement that would add complexity, as well as it may have unintended
consequences.  The change at this level (e.g., a helper method) makes it
more clear that this is for rendering UI components.

Related to #4845.

[1]:https://github.com/samvera/hyrax/pull/4889/files#r625466486

app/helpers/hyrax/embargo_helper.rb

@@ -25,6 +25,10 @@ def assets_with_deactivated_embargoes
     #   holder until we switch to Valkyrie::ChangeSet instead of Form
     #   objects
     def embargo_enforced?(resource)
+      # This is a guard; from the UI rendering perspective, there's no
+      # active embargo enforcement until the object is saved.
+      return false unless resource.persisted?
+
       case resource
       when Hydra::AccessControls::Embargoable
         !resource.embargo_release_date.nil?

app/helpers/hyrax/lease_helper.rb

@@ -25,6 +25,10 @@ def assets_with_deactivated_leases
     #   holder until we switch to Valkyrie::ChangeSet instead of Form
     #   objects
     def lease_enforced?(resource)
+      # This is a guard; from the UI rendering perspective, there's no
+      # active lease enforcement until the object is saved.
+      return false unless resource.persisted?
+
       case resource
       when Hydra::AccessControls::Embargoable
         !resource.lease_expiration_date.nil?

app/views/hyrax/base/_form_visibility_component.html.erb

@@ -1,6 +1,6 @@
-<% if embargo_enforced?(f.object) && !f.object.new_record? %>
+<% if embargo_enforced?(f.object) %>
   <%= render 'form_permission_under_embargo', f: f %>
-<% elsif lease_enforced?(f.object) && !f.object.new_record? %>
+<% elsif lease_enforced?(f.object) %>
   <%= render 'form_permission_under_lease', f: f %>
 <% else %>
     <fieldset>

spec/helpers/hyrax/embargo_helper_spec.rb

@@ -4,6 +4,21 @@
   let(:resource) { build(:monograph) }
 
   describe 'embargo_enforced?' do
+    # Including this stub to preserve the spec structure before the #4845 change
+    before { allow(resource).to receive(:persisted?).and_return(true) }
+
+    context 'with a non-persisted object' do
+      let(:resource) { build(:hyrax_work, :under_embargo) }
+
+      before { Hyrax::EmbargoManager.apply_embargo_for!(resource: resource) }
+
+      it 'returns false' do
+        # Note: This spec echoes "embargo_enforced? with a Hyrax::Work when an embargo is enforced on the resource"
+        allow(resource).to receive(:persisted?).and_return false
+        expect(embargo_enforced?(resource)).to be false
+      end
+    end
+
     context 'with a Hyrax::Work' do
       let(:resource) { build(:hyrax_work) }
 
@@ -94,7 +109,8 @@
     end
 
     context 'with a HydraEditor::Form' do
-      let(:resource) { Hyrax::GenericWorkForm.new(build(:work), ability, form_controller) }
+      let(:resource) { Hyrax::GenericWorkForm.new(model, ability, form_controller) }
+      let(:model) { build(:work) }
       let(:ability) { :FAKE_ABILITY }
       let(:form_controller) { :FAKE_CONTROLLER }
 
@@ -103,17 +119,21 @@
       end
 
       context 'when the wrapped work is under embargo' do
-        let(:resource) { Hyrax::GenericWorkForm.new(build(:embargoed_work), ability, form_controller) }
+        let(:model) { build(:embargoed_work) }
 
         it 'returns true' do
+          # This allow call is a tweak to preserve spec for pre #4845 patch
+          allow(model).to receive(:persisted?).and_return(true)
+
           expect(embargo_enforced?(resource)).to be true
         end
       end
     end
 
     context 'with a Hyrax::Forms::FailedSubmissionFormWrapper' do
       let(:resource) { Hyrax::Forms::FailedSubmissionFormWrapper.new(form: form, input_params: {}, permitted_params: {}) }
-      let(:form) { Hyrax::GenericWorkForm.new(build(:work), ability, form_controller) }
+      let(:form) { Hyrax::GenericWorkForm.new(model, ability, form_controller) }
+      let(:model) { build(:work) }
       let(:ability) { :FAKE_ABILITY }
       let(:form_controller) { :FAKE_CONTROLLER }
 
@@ -122,9 +142,11 @@
       end
 
       context 'when the wrapped work is under embargo' do
-        let(:form) { Hyrax::GenericWorkForm.new(build(:embargoed_work), ability, form_controller) }
+        let(:model) { build(:embargoed_work) }
 
         it 'returns true' do
+          # This allow call is a tweak to preserve spec for pre #4845 patch
+          allow(model).to receive(:persisted?).and_return(true)
           expect(embargo_enforced?(resource)).to be true
         end
       end

spec/helpers/hyrax/lease_helper_spec.rb

@@ -4,6 +4,21 @@
   let(:resource) { build(:monograph) }
 
   describe 'lease_enforced?' do
+    # Including this stub to preserve the spec structure before the #4845 change
+    before { allow(resource).to receive(:persisted?).and_return(true) }
+
+    context 'with a non-persisted object' do
+      let(:resource) { build(:hyrax_work, :under_lease) }
+
+      before { Hyrax::LeaseManager.apply_lease_for!(resource: resource) }
+
+      it 'returns false' do
+        # Note: This spec echoes "lease_enforced? with a Hyrax::Work when a lease is enforced on the resource"
+        allow(resource).to receive(:persisted?).and_return false
+        expect(lease_enforced?(resource)).to be false
+      end
+    end
+
     context 'with a Hyrax::Work' do
       let(:resource) { build(:hyrax_work) }
 
@@ -92,7 +107,8 @@
     end
 
     context 'with a HydraEditor::Form' do
-      let(:resource) { Hyrax::GenericWorkForm.new(build(:work), ability, form_controller) }
+      let(:resource) { Hyrax::GenericWorkForm.new(model, ability, form_controller) }
+      let(:model) { build(:work) }
       let(:ability) { :FAKE_ABILITY }
       let(:form_controller) { :FAKE_CONTROLLER }
 
@@ -101,17 +117,20 @@
       end
 
       context 'when the wrapped work is under lease' do
-        let(:resource) { Hyrax::GenericWorkForm.new(build(:leased_work), ability, form_controller) }
+        let(:model) { build(:leased_work) }
 
         it 'returns true' do
+          # This allow call is a tweak to preserve spec for pre #4845 patch
+          allow(model).to receive(:persisted?).and_return(true)
           expect(lease_enforced?(resource)).to be true
         end
       end
     end
 
     context 'with a Hyrax::Forms::FailedSubmissionFormWrapper' do
       let(:resource) { Hyrax::Forms::FailedSubmissionFormWrapper.new(form: form, input_params: {}, permitted_params: {}) }
-      let(:form) { Hyrax::GenericWorkForm.new(build(:work), ability, form_controller) }
+      let(:model) { build(:work) }
+      let(:form) { Hyrax::GenericWorkForm.new(model, ability, form_controller) }
       let(:ability) { :FAKE_ABILITY }
       let(:form_controller) { :FAKE_CONTROLLER }
 
@@ -120,9 +139,11 @@
       end
 
       context 'when the wrapped work is under embargo' do
-        let(:form) { Hyrax::GenericWorkForm.new(build(:leased_work), ability, form_controller) }
+        let(:model) { build(:leased_work) }
 
         it 'returns true' do
+          # This allow call is a tweak to preserve spec for pre #4845 patch
+          allow(model).to receive(:persisted?).and_return(true)
           expect(lease_enforced?(resource)).to be true
         end
       end