Merge pull request #273 from projecthydra/dont_create_embargo

woohoo! less drag on the test suites.

hydra-access-controls/app/models/concerns/hydra/access_controls/embargoable.rb

@@ -45,12 +45,8 @@ def active_lease?
       # The lease_visibility! and embargo_visibility! methods rely on this to deactivate the lease when applicable.
       def visibility=(value)
         # If changing from embargo or lease, deactivate the lease/embargo and wipe out the associated metadata before proceeding
-        if !embargo_release_date.nil?
-          deactivate_embargo! unless value == visibility_during_embargo
-        end
-        if !lease_expiration_date.nil?
-          deactivate_lease! unless value == visibility_during_lease
-        end
+        deactivate_embargo! if deactivate_embargo?(value)
+        deactivate_lease! if deactivate_lease?(value)
         super
       end
 
@@ -150,6 +146,19 @@ def lease_visibility!
           end
         end
       end
+
+      private
+
+        # @return [true, false] true if there is an embargo set up and the visibility will change
+        def deactivate_embargo?(value)
+          embargo && embargo.embargo_release_date && value != embargo.visibility_during_embargo
+        end
+
+        # @return [true, false] true if there is a lease set up and the visibility will change
+        def deactivate_lease?(value)
+          lease && lease.lease_expiration_date && value != lease.visibility_during_lease
+        end
+
     end
   end
 end

hydra-access-controls/spec/unit/embargoable_spec.rb

@@ -44,15 +44,32 @@ def save(returning_value = true)
   end
 
   context 'visibility=' do
-    it "when changing from embargo, wipes out associated embargo metadata" do
-      subject.embargo_release_date = future_date.to_s
-      expect(subject).to receive(:deactivate_embargo!)
-      subject.visibility = Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_PUBLIC
+    context "when the object is not under embargo or lease" do
+      before do
+        subject.visibility = Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_PRIVATE
+      end
+      it "doesn't create embargo or lease" do
+        expect(subject.embargo).to be_nil
+        expect(subject.lease).to be_nil
+      end
+    end
+
+    context "when changing from embargo" do
+      before do
+        subject.embargo_release_date = future_date.to_s
+      end
+      it "wipes out associated embargo metadata" do
+        expect(subject).to receive(:deactivate_embargo!)
+        subject.visibility = Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_PUBLIC
+      end
     end
 
     context "when changing from lease" do
-      it "wipes out associated lease metadata and marks visibility as changed" do
+      before do
         subject.apply_lease(future_date.to_s, Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_AUTHENTICATED, Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_PRIVATE)
+      end
+
+      it "wipes out associated lease metadata and marks visibility as changed" do
         expect(subject).to receive(:deactivate_lease!)
         subject.visibility = Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_PUBLIC
         expect(subject).to be_visibility_changed

hydra-access-controls/spec/unit/visibility_spec.rb

@@ -12,14 +12,13 @@ class MockParent < ActiveFedora::Base
   end
 
   describe "setting visibility" do
-    before {
+    before do
       class Foo < ActiveFedora::Base
         include Hydra::AccessControls::Permissions
       end
-    }
-    after {
-      Object.send(:remove_const, :Foo)
-    }
+    end
+
+    after { Object.send(:remove_const, :Foo) }
 
     subject { Foo.new }