Handle InProgressEtd::NO_EMBARGO values in the actor stack

app/actors/hyrax/actors/pregrad_embargo.rb

@@ -14,6 +14,9 @@ def create(env)
         def pregraduation_embargo_attributes(env)
           return {} unless env.attributes.key?(:embargo_length)
 
+          return handle_malformed_data(env) if
+            env.attributes.fetch(:embargo_length, nil) == InProgressEtd::NO_EMBARGO
+
           open = Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_PUBLIC
           embargo = Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_EMBARGO
 
@@ -22,6 +25,25 @@ def pregraduation_embargo_attributes(env)
             visibility_after_embargo:  open,
             visibility_during_embargo: open }
         end
+
+        ##
+        # When creating an `Etd` with no embargo, `InProgressEtd` passes a
+        # particular string (`InProgressEtd::NO_EMBARGO'). For the moment, we
+        # need to handle
+        #
+        # @todo rework callers to avoid passing this "malformed" data. We
+        #   shouldn't need to interpret strings passed to this value until
+        #   graduation.
+        def handle_malformed_data(env)
+          warn "#{self.class} is cleaning up non-date data passed to the " \
+               ":embargo_length attribute. #{env.attributes[:embargo_length]} " \
+               "is being interpreted as a request for no embargo on " \
+               "#{env.attributes[:title]}."
+
+          env.attributes.delete(:embargo_length)
+
+          {}
+        end
     end
   end
 end

app/models/in_progress_etd.rb

@@ -19,6 +19,8 @@
 # the data on the `Etd` record that it represents.
 
 class InProgressEtd < ApplicationRecord
+  NO_EMBARGO = 'None - open access immediately'
+
   after_create :add_id_to_data_store
 
   # custom validators check for presence of tab-determined set of fields based on presence of tab-identifying data
@@ -54,17 +56,17 @@ def add_data(new_data)
   # currently the EtdForm uses the boolean param "no_embargoes", so we need to send or remove it (seems a good candidate for refactoring in EtdForm)
 
   def add_no_embargoes(new_data)
-    resulting_data = new_data[:embargo_length] == 'None - open access immediately' ? new_data.merge("no_embargoes" => "1") : nil
+    resulting_data = new_data[:embargo_length] == NO_EMBARGO ? new_data.merge("no_embargoes" => "1") : nil
 
     resulting_data.nil? ? new_data : resulting_data
   end
 
-  # Remove embargo_type, if new_data[:embargo_length] == 'None - open access immediately'
-  # Remove no_embargoes if new_data[:embargo_length] != 'None - open access immediately'
+  # Remove embargo_type, if new_data[:embargo_length] == NO_EMBARGO
+  # Remove no_embargoes if new_data[:embargo_length] != NO_EMBARGO
 
   def remove_stale_embargo_data(existing_data, new_data)
-    existing_data.delete('no_embargoes') if existing_data.keys.include?('no_embargoes') && new_data[:embargo_length] != 'None - open access immediately'
-    existing_data.delete('embargo_type') if new_data[:embargo_length] == 'None - open access immediately' && existing_data.keys.include?('embargo_type')
+    existing_data.delete('no_embargoes') if existing_data.keys.include?('no_embargoes') && new_data[:embargo_length] != NO_EMBARGO
+    existing_data.delete('embargo_type') if new_data[:embargo_length] == NO_EMBARGO && existing_data.keys.include?('embargo_type')
     existing_data
   end
 

spec/actors/hyrax/actors/pregrad_embargo_spec.rb

@@ -19,6 +19,21 @@
       expect { middleware.create(env) }.not_to change { env.attributes }
     end
 
+    context 'with a specific string passed from InProgressEtd' do
+      let(:attributes) do
+        { 'title' => ['good fun'],
+          'creator' => ['Sneddon, River'],
+          'school' => ['Emory College'],
+          'embargo_length' => InProgressEtd::NO_EMBARGO }
+      end
+
+      it 'does not apply an embargo' do
+        expect { middleware.create(env) }
+          .to change { env.attributes }
+          .to attributes.except('embargo_length')
+      end
+    end
+
     context 'with a requested embargo' do
       let(:six_years_from_today) { Time.zone.today + 6.years }
 

spec/models/in_progress_etd_spec.rb

@@ -299,11 +299,11 @@
 
       context 'with existing no_embargoes and new no_embargoes' do
         let(:old_data) { { no_embargoes: '1' } }
-        let(:new_data) { { 'embargo_length': 'None - open access immediately' } }
+        let(:new_data) { { 'embargo_length': described_class::NO_EMBARGO } }
 
         it "preserves the no_embargoes and adds the new embargo_length data" do
           expect(resulting_data).to eq({
-            'embargo_length' => 'None - open access immediately',
+            'embargo_length' => described_class::NO_EMBARGO,
             'no_embargoes' => '1',
             "schoolHasChanged" => false
           })
@@ -326,11 +326,11 @@
       context 'with existing embargoes and new no embargo data' do
         let(:old_data) { { 'embargo_length': '1 Year', 'embargo_type': 'files_embargoed' } }
 
-        let(:new_data) { { 'embargo_length': 'None - open access immediately' } }
+        let(:new_data) { { 'embargo_length': described_class::NO_EMBARGO } }
 
         it 'removes old embargo lengths and types and sets no_embargoes' do
           expect(resulting_data).to eq({
-            'embargo_length' => 'None - open access immediately',
+            'embargo_length' => described_class::NO_EMBARGO,
             'no_embargoes' => '1',
             "schoolHasChanged" => false
           })