Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add handling for missing derivatives to manifest generation #5419

Merged
merged 2 commits into from
Oct 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions app/models/iiif_canvas_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down
3 changes: 1 addition & 2 deletions app/models/iiif_manifest_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ 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.select { |mf| mf.derivative_ids.size > 0 }
master_files
end

def work_presenters
Expand Down
10 changes: 8 additions & 2 deletions app/models/iiif_playlist_canvas_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions app/models/iiif_playlist_manifest_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ en:
receiving this message, please contact us at <a href="mailto:%s">%s</a>.
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: <a href="mailto:%s">%s</a>.
controlled_vocabulary_error: |
%s Please contact support to report this error: <a href="mailto:%s">%s</a>.
messages:
Expand Down Expand Up @@ -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"

Expand Down
34 changes: 26 additions & 8 deletions spec/models/iiif_canvas_presenter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
15 changes: 13 additions & 2 deletions spec/models/iiif_playlist_canvas_presenter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down