diff --git a/app/actors/hyrax/actors/pregrad_embargo.rb b/app/actors/hyrax/actors/pregrad_embargo.rb index aea0dd026..7bf1414ad 100644 --- a/app/actors/hyrax/actors/pregrad_embargo.rb +++ b/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 diff --git a/app/models/in_progress_etd.rb b/app/models/in_progress_etd.rb index 504e39ae3..936ec9d89 100644 --- a/app/models/in_progress_etd.rb +++ b/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 diff --git a/spec/actors/hyrax/actors/pregrad_embargo_spec.rb b/spec/actors/hyrax/actors/pregrad_embargo_spec.rb index c3dbea638..f14f02f48 100644 --- a/spec/actors/hyrax/actors/pregrad_embargo_spec.rb +++ b/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 } diff --git a/spec/models/in_progress_etd_spec.rb b/spec/models/in_progress_etd_spec.rb index 4e2021354..9fa0f2562 100644 --- a/spec/models/in_progress_etd_spec.rb +++ b/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 })