From 59f6230903b3a9512e5444e8ad1cda4deabe5ef4 Mon Sep 17 00:00:00 2001 From: Mason Ballengee Date: Wed, 11 Oct 2023 12:54:26 -0400 Subject: [PATCH 1/2] Add handling for missing derivatives to manifest gen --- app/models/iiif_canvas_presenter.rb | 21 ++++++++---- app/models/iiif_manifest_presenter.rb | 2 +- app/models/iiif_playlist_canvas_presenter.rb | 10 ++++-- .../iiif_playlist_manifest_presenter.rb | 3 +- config/locales/en.yml | 6 ++++ spec/models/iiif_canvas_presenter_spec.rb | 34 ++++++++++++++----- .../iiif_playlist_canvas_presenter_spec.rb | 15 ++++++-- 7 files changed, 70 insertions(+), 21 deletions(-) diff --git a/app/models/iiif_canvas_presenter.rb b/app/models/iiif_canvas_presenter.rb index 968761cfa6..e0b576ba68 100644 --- a/app/models/iiif_canvas_presenter.rb +++ b/app/models/iiif_canvas_presenter.rb @@ -36,6 +36,7 @@ def range # @return [IIIFManifest::V3::DisplayContent] the display content required by the manifest builder. def display_content + return if master_file.derivative_ids.empty? master_file.is_video? ? video_content : audio_content end @@ -59,12 +60,20 @@ def see_also end def placeholder_content - # height and width from /models/master_file/extract_still method - IIIFManifest::V3::DisplayContent.new( @master_file.poster_master_file_url(@master_file.id), - width: 1280, - height: 720, - type: 'Image', - format: 'image/jpeg') + if @master_file.derivative_ids.size > 0 + # height and width from /models/master_file/extract_still method + IIIFManifest::V3::DisplayContent.new( @master_file.poster_master_file_url(@master_file.id), + width: 1280, + height: 720, + type: 'Image', + format: 'image/jpeg') + else + support_email = Settings.email.support + IIIFManifest::V3::DisplayContent.new(nil, + label: I18n.t('errors.missing_derivatives_error') % [support_email, support_email], + type: 'Text', + format: 'text/plain') + end end private diff --git a/app/models/iiif_manifest_presenter.rb b/app/models/iiif_manifest_presenter.rb index 0504d6052a..bfb9b4a913 100644 --- a/app/models/iiif_manifest_presenter.rb +++ b/app/models/iiif_manifest_presenter.rb @@ -30,7 +30,7 @@ def initialize(media_object:, master_files:, lending_enabled: false) def file_set_presenters # Only return master files that have derivatives to avoid oddities in the manifest and failures in iiif_manifest - master_files.select { |mf| mf.derivative_ids.size > 0 } + master_files end def work_presenters diff --git a/app/models/iiif_playlist_canvas_presenter.rb b/app/models/iiif_playlist_canvas_presenter.rb index 14dd3b73b7..41f9b424aa 100644 --- a/app/models/iiif_playlist_canvas_presenter.rb +++ b/app/models/iiif_playlist_canvas_presenter.rb @@ -72,12 +72,18 @@ def annotation_content def placeholder_content if cannot_read_item IIIFManifest::V3::DisplayContent.new(nil, - label: 'You do not have permission to playback this item.', + label: I18n.t('playlist.restrictedText'), type: 'Text', format: 'text/plain') elsif master_file.nil? IIIFManifest::V3::DisplayContent.new(nil, - label: 'The source for this playlist item has been deleted.', + label: I18n.t('playlist.deletedText'), + type: 'Text', + format: 'text/plain') + elsif master_file.derivative_ids.empty? + support_email = Settings.email.support + IIIFManifest::V3::DisplayContent.new(nil, + label: I18n.t('errors.missing_derivatives_error') % [support_email, support_email], type: 'Text', format: 'text/plain') end diff --git a/app/models/iiif_playlist_manifest_presenter.rb b/app/models/iiif_playlist_manifest_presenter.rb index a0c22c5c03..838b5033ee 100644 --- a/app/models/iiif_playlist_manifest_presenter.rb +++ b/app/models/iiif_playlist_manifest_presenter.rb @@ -25,8 +25,7 @@ def initialize(playlist:, items:, can_edit_playlist: false) end def file_set_presenters - # Only return master files that have derivatives to avoid oddities in the manifest and failures in iiif_manifest - items.select { |item| item.master_file.derivative_ids.size.positive? } + items end def work_presenters diff --git a/config/locales/en.yml b/config/locales/en.yml index 23bb3e5c80..edd35fb42d 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -16,6 +16,9 @@ en: receiving this message, please contact us at %s. general_auth_error: You are not authorized to access this content. general_500_error: There was an error with this request. + missing_derivatives_error: | + This item is unable to be played because of missing streaming derivatives. + Please contact support to report this error: %s. controlled_vocabulary_error: | %s Please contact support to report this error: %s. messages: @@ -233,6 +236,9 @@ en: tags: label: "Tags" + restrictedText: "You do not have permission to playback this item." + deletedText: "The source for this playlist item has been deleted." + timeline: ago: "%{time} ago" diff --git a/spec/models/iiif_canvas_presenter_spec.rb b/spec/models/iiif_canvas_presenter_spec.rb index 2c583bb043..8bd8167381 100644 --- a/spec/models/iiif_canvas_presenter_spec.rb +++ b/spec/models/iiif_canvas_presenter_spec.rb @@ -100,17 +100,35 @@ describe '#placeholder_content' do subject { presenter.placeholder_content } - it 'has format' do - expect(subject.format).to eq "image/jpeg" - end + context 'when master file has derivatives' do + it 'has format' do + expect(subject.format).to eq "image/jpeg" + end - it 'has type' do - expect(subject.type).to eq "Image" + it 'has type' do + expect(subject.type).to eq "Image" + end + + it 'has height and width' do + expect(subject.width).to eq 1280 + expect(subject.height).to eq 720 + end end - it 'has height and width' do - expect(subject.width).to eq 1280 - expect(subject.height).to eq 720 + context 'when master file does not have derivatives' do + let(:master_file) { FactoryBot.build(:master_file, media_object: media_object) } + + it 'has format' do + expect(subject.format).to eq "text/plain" + end + + it 'has type' do + expect(subject.type).to eq "Text" + end + + it 'has label' do + expect(subject.label).to eq I18n.t('errors.missing_derivatives_error') % [Settings.email.support, Settings.email.support] + end end end diff --git a/spec/models/iiif_playlist_canvas_presenter_spec.rb b/spec/models/iiif_playlist_canvas_presenter_spec.rb index c97756e62a..76f7aa8b88 100644 --- a/spec/models/iiif_playlist_canvas_presenter_spec.rb +++ b/spec/models/iiif_playlist_canvas_presenter_spec.rb @@ -188,7 +188,7 @@ expect(subject).to be_present expect(subject.format).to eq "text/plain" expect(subject.type).to eq "Text" - expect(subject.label).to eq "You do not have permission to playback this item." + expect(subject.label).to eq I18n.t('playlist.restrictedText') end end @@ -200,7 +200,18 @@ expect(subject).to be_present expect(subject.format).to eq "text/plain" expect(subject.type).to eq "Text" - expect(subject.label).to eq "The source for this playlist item has been deleted." + expect(subject.label).to eq I18n.t('playlist.deletedText') + end + end + + context 'when an item has no derivatives' do + let(:master_file) { FactoryBot.build(:master_file, media_object: media_object) } + + it 'generates placeholder canvas' do + expect(subject).to be_present + expect(subject.format).to eq "text/plain" + expect(subject.type).to eq "Text" + expect(subject.label).to eq I18n.t('errors.missing_derivatives_error') % [Settings.email.support, Settings.email.support] end end end From 5001ec6c9c8ef54aefb51a303068d7a576f0caae Mon Sep 17 00:00:00 2001 From: Mason Ballengee <68433277+masaball@users.noreply.github.com> Date: Wed, 11 Oct 2023 13:23:59 -0400 Subject: [PATCH 2/2] Remove unneeded comment --- app/models/iiif_manifest_presenter.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/models/iiif_manifest_presenter.rb b/app/models/iiif_manifest_presenter.rb index bfb9b4a913..ed053a9496 100644 --- a/app/models/iiif_manifest_presenter.rb +++ b/app/models/iiif_manifest_presenter.rb @@ -29,7 +29,6 @@ def initialize(media_object:, master_files:, lending_enabled: false) end def file_set_presenters - # Only return master files that have derivatives to avoid oddities in the manifest and failures in iiif_manifest master_files end