Skip to content

Commit

Permalink
Merge pull request #6047 from avalonmediasystem/missing_dates
Browse files Browse the repository at this point in the history
Date issued is no longer a required field
  • Loading branch information
cjcolvar authored Sep 24, 2024
2 parents 59a6c4c + 749215c commit 6328acc
Show file tree
Hide file tree
Showing 12 changed files with 14 additions and 22 deletions.
2 changes: 1 addition & 1 deletion app/controllers/bookmarks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ def status_action documents
else
case params['action']
when 'publish'
if media_object.title.nil? || media_object.date_issued.nil?
if media_object.title.nil?
errors += ["#{id}, Unable to Publish Item. Missing required fields."]
else
success_ids << id
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/media_objects_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ def update_media_object
error_messages = []
unless @media_object.valid?
invalid_fields = @media_object.errors.attribute_names
required_fields = [:title, :date_issued]
required_fields = [:title]
unless required_fields.any? { |f| invalid_fields.include? f }
invalid_fields.each do |field|
#NOTE this will erase all values for fields with multiple values
Expand Down Expand Up @@ -413,7 +413,7 @@ def update_status
begin
case status
when 'publish'
unless media_object.title.present? && media_object.date_issued.present?
unless media_object.title.present?
errors += ["#{media_object&.title} (#{id}) (missing required fields)"]
next
end
Expand Down
4 changes: 2 additions & 2 deletions app/models/batch_entries.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class BatchEntries < ActiveRecord::Base
def ensure_mininum_viable_metadata
return nil if minimal_viable_metadata?
self.error = true
self.error_message = 'To successfully ingest, either title and date issued must be set or a bibliographic id must be provided'
self.error_message = 'To successfully ingest, either title must be set or a bibliographic id must be provided'
end

def queue
Expand All @@ -54,7 +54,7 @@ def minimal_viable_metadata?
return false if payload.nil? # nil guard
fields = JSON.parse(payload)['fields']
return false if fields.blank?
return false if (fields['date_issued'].blank? || fields['title'].blank?) && fields['bibliographic_id'].blank?
return false if fields['title'].blank? && fields['bibliographic_id'].blank?
true
end

Expand Down
1 change: 1 addition & 0 deletions app/models/iiif_manifest_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ def metadata_field(label, value, default = nil)
end

def combined_display_date(media_object)
#FIXME Does this need to change now that date_issued is not required and thus could be nil
result = media_object.date_issued
result += " (Creation date: #{media_object.date_created})" if media_object.date_created.present?
result
Expand Down
1 change: 0 additions & 1 deletion app/models/media_object.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ class MediaObject < ActiveFedora::Base
# validates :governing_policies, presence: true if Proc.new { |mo| mo.changes["governing_policy_ids"].empty? }

validates :title, presence: true, if: :resource_description_active?
validates :date_issued, presence: true, if: :resource_description_active?
validate :validate_language, if: :resource_description_active?
validate :validate_related_items, if: :resource_description_active?
validate :validate_dates, if: :resource_description_active?
Expand Down
2 changes: 1 addition & 1 deletion app/views/media_objects/_resource_description.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ Unless required by applicable law or agreed to in writing, software distributed
<%= render partial: 'text_field',
locals: {form: form, field: :date_issued,
options: {display_label: 'Publication date', required: true}} %>
options: {display_label: 'Publication date'}} %>
<%= render partial: 'text_field',
locals: {form: form, field: :creator,
Expand Down
1 change: 0 additions & 1 deletion spec/controllers/bookmarks_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@
it "should show a warning for incomplete items" do
invalid_mo = media_objects.first
invalid_mo.title = nil
invalid_mo.date_issued = nil
invalid_mo.save

post 'publish'
Expand Down
1 change: 0 additions & 1 deletion spec/controllers/master_files_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
context "must provide a valid media object" do
before do
media_object.title = nil
media_object.date_issued = nil
media_object.workflow.last_completed_step = 'file-upload'
media_object.save(validate: false)
end
Expand Down
9 changes: 2 additions & 7 deletions spec/controllers/media_objects_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@
expect(new_media_object.bibliographic_id).to eq({source: "local", id: bib_id})
expect(new_media_object.title).to eq ex_media_object.title
expect(new_media_object.creator).to eq [] #creator no longer required, so supplied value won't be used
expect(new_media_object.date_issued).to eq ex_media_object.date_issued
expect(new_media_object.date_issued).to eq nil #date_issued no longer required, so supplied value won't be used
end
it "should create a new media_object, removing invalid data for non-required fields" do
media_object = FactoryBot.create(:media_object)
Expand Down Expand Up @@ -618,13 +618,12 @@
media_object.save
login_user media_object.collection.managers.first

put :update, params: { id: media_object.id, step: 'resource-description', media_object: {title: '', date_issued: ''} }
put :update, params: { id: media_object.id, step: 'resource-description', media_object: {title: ''} }
expect(response.response_code).to eq(200)
expect(flash[:error]).not_to be_empty
media_object.reload
expect(media_object.valid?).to be_truthy
expect(media_object.title).not_to be_blank
expect(media_object.date_issued).not_to be_blank
end
end

Expand Down Expand Up @@ -1470,7 +1469,6 @@
it "item is invalid" do
media_object = FactoryBot.create(:media_object, collection: collection)
media_object.title = nil
media_object.date_issued = nil
media_object.workflow.last_completed_step = 'file-upload'
media_object.save!(validate: false)
get 'update_status', params: { id: media_object.id, status: 'publish' }
Expand All @@ -1482,7 +1480,6 @@
it "item is invalid and no last_completed_step" do
media_object = FactoryBot.create(:media_object, collection: collection)
media_object.title = nil
media_object.date_issued = nil
media_object.workflow.last_completed_step = ''
media_object.save!(validate: false)
get 'update_status', params: { id: media_object.id, status: 'publish' }
Expand All @@ -1504,7 +1501,6 @@
it 'unpublishes invalid items' do
media_object = FactoryBot.create(:published_media_object, collection: collection)
media_object.title = nil
media_object.date_issued = nil
media_object.save!(validate: false)
get 'update_status', params: { id: media_object.id, status: 'unpublish' }
media_object.reload
Expand All @@ -1514,7 +1510,6 @@
it 'unpublishes invalid items and no last completed step' do
media_object = FactoryBot.create(:published_media_object, collection: collection)
media_object.title = nil
media_object.date_issued = nil
media_object.workflow.last_completed_step = ''
media_object.save!(validate: false)
get 'update_status', params: { id: media_object.id, status: 'unpublish' }
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/avalon/batch/entry_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
let(:testdir) {'spec/fixtures/'}
let(:filename) {'videoshort.mp4'}
let(:collection) {FactoryBot.build(:collection)}
let(:entry_fields) {{ title: Faker::Lorem.sentence, date_issued: "#{DateTime.now.strftime('%F')}" }}
let(:entry_fields) {{ title: Faker::Lorem.sentence }}
let(:entry_files) { [{ file: File.join(testdir, filename), skip_transcoding: false }] }
let(:entry_opts) { {user_key: '[email protected]', collection: collection} }
let(:entry) { Avalon::Batch::Entry.new(entry_fields, entry_files, entry_opts, nil, nil) }
Expand Down
8 changes: 4 additions & 4 deletions spec/models/batch_entries_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@

describe 'validating payload' do
describe 'with sufficient metadata' do
it 'does not record an error when title and date_issued are present' do
payload = { fields: { title: 'foo', date_issued: Time.now.utc } }
it 'does not record an error when title is present' do
payload = { fields: { title: 'foo' } }
be = BatchEntries.new(payload: payload.to_json, batch_registries: batch_registry)
be.save
be.reload
Expand All @@ -58,12 +58,12 @@
be.reload
expect(be.error).to be_truthy
end
it 'records an error when only title is present' do
it 'does not record an error when only title is present' do
payload = { fields: { title: 'foo' } }
be = BatchEntries.new(payload: payload.to_json, batch_registries: batch_registry)
be.save
be.reload
expect(be.error).to be_truthy
expect(be.error).to be_falsey
end
it 'records an error when only date issued is present' do
payload = { fields: { date_issued: Time.now.utc } }
Expand Down
1 change: 0 additions & 1 deletion spec/models/media_object_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,6 @@
# Force the validations to run by being on the resource-description workflow step
subject(:media_object) { FactoryBot.build(:media_object).tap {|mo| mo.workflow.last_completed_step = "resource-description"} }

it {is_expected.to validate_presence_of(:date_issued)}
it {is_expected.to validate_presence_of(:title)}
end

Expand Down

0 comments on commit 6328acc

Please sign in to comment.