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

HYC-1354 - Fallback Thumbnails #1133

Merged
merged 25 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# frozen_string_literal: true
# https://github.com/projectblacklight/blacklight/blob/v8.7.0/app/presenters/blacklight/thumbnail_presenter.rb
module Blacklight
bbpennel marked this conversation as resolved.
Show resolved Hide resolved
class ThumbnailPresenter
private
# [hyc-override] Retrieve the thumbnail of the first file_set of any given work instead of using the default if it exists
def retrieve_values(field_config)
# Return the default thumbnail if the document object is nil
unless document
return FieldRetriever.new(document, field_config, view_context).fetch
end

solr_doc = extract_solr_document(document)
document_hash = solr_doc.to_h

# Update the `thumbnail_path_ss` dynamically if needed
if needs_thumbnail_path_update?(document_hash)
file_set_id = document_hash['file_set_ids_ssim']&.first
document_hash['thumbnail_path_ss'] = "/downloads/#{file_set_id}?file=thumbnail"
Rails.logger.info("Updated thumbnail_path_ss: #{document_hash['thumbnail_path_ss']} for work with id #{document_hash['id']}")
# Create a temporary SolrDocument from the updated hash
updated_document = SolrDocument.new(document_hash)
FieldRetriever.new(updated_document, field_config, view_context).fetch
else
FieldRetriever.new(solr_doc, field_config, view_context).fetch
end
end

# Extract the SolrDocument from the document object if it's nested
# Prevents errors when the document object is a presenter on work show pages
def extract_solr_document(doc)
if doc.is_a?(SolrDocument)
doc
elsif doc.respond_to?(:solr_document) && doc.solr_document.is_a?(SolrDocument)
doc.solr_document
end
end

def needs_thumbnail_path_update?(document)
thumbnail_path = document['thumbnail_path_ss'] || ''
file_set_ids = document['file_set_ids_ssim']
thumbnail_missing_or_default = thumbnail_path.blank? || thumbnail_path !~ %r{^/downloads/\w+\?file=thumbnail$}

# Returns true if file_set_ids are present and the thumbnail path is the default or missing entirely
file_set_ids.present? && thumbnail_missing_or_default
end
end
end
27 changes: 22 additions & 5 deletions app/overrides/presenters/hyrax/work_show_presenter_override.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,33 @@
# [hyc-override] Overriding helper in order to add doi to citation
# https://github.com/samvera/hyrax/blob/hyrax-v4.0.0/app/presenters/hyrax/work_show_presenter.rb
Hyrax::WorkShowPresenter.class_eval do
# delegating just :doi seems to exclude the other fields, so pull all fields in from original file
delegate :title, :date_created, :date_issued, :description, :doi, :creator, :place_of_publication,
:creator_display, :contributor, :subject, :publisher, :language, :embargo_release_date,
:lease_expiration_date, :license, :source, :rights_statement, :thumbnail_id, :representative_id,
:rendering_ids, :member_of_collection_ids, :alternative_title, :bibliographic_citation, to: :solr_document
bbpennel marked this conversation as resolved.
Show resolved Hide resolved
include Hyrax::SharedDelegates

# [hyc-override] Add default scholarly? method
# Indicates if the work is considered scholarly according to google scholar
# This method is not defined in hyrax, but it is referenced by GoogleScholarPresenter
def scholarly?
false
end

def fetch_primary_fileset_id
res = representative_id.blank? ? member_ids.first : representative_id
res
end

# [hyc-override] Use a work's first related fileset_id instead of the representative_id if it's nil
# @return FileSetPresenter presenter for the representative FileSets
def representative_presenter
@representative_presenter ||=
begin
primary_fileset_id = fetch_primary_fileset_id
return nil if primary_fileset_id.blank?
result = member_presenters([primary_fileset_id]).first
return nil if result.try(:id) == id
result.try(:representative_presenter) || result
rescue Hyrax::ObjectNotFoundError
Hyrax.logger.warn "Unable to find representative_id #{primary_fileset_id} for work #{id}"
return nil
end
end
end
6 changes: 1 addition & 5 deletions app/presenters/hyrax/data_set_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,7 @@
# `rails generate hyrax:work DataSet`
module Hyrax
class DataSetPresenter < Hyrax::WorkShowPresenter
delegate :abstract, :admin_note, :contributor_display, :copyright_date, :creator_display, :date_issued, :dcmi_type, :deposit_record, :doi,
:extent, :funder, :kind_of_data, :last_modified_date, :language_label,
:license_label, :methodology, :note, :orcid_label, :other_affiliation_label, :project_director_display, :researcher_display,
:rights_holder, :rights_statement_label, :sponsor, to: :solr_document

include Hyrax::SharedDelegates
# See: WorkShowPresenter.scholarly?
def scholarly?
true
Expand Down
15 changes: 15 additions & 0 deletions app/presenters/hyrax/shared_delegates.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# frozen_string_literal: true
module Hyrax
module SharedDelegates
extend ActiveSupport::Concern
included do
delegate :abstract, :admin_note, :alternative_title, :bibliographic_citation, :contributor, :contributor_display, :copyright_date,
:creator, :creator_display, :date_created, :date_issued, :dcmi_type, :deposit_record, :description, :doi, :embargo_release_date,
:extent, :funder, :human_readable_type, :identifier, :itemtype, :kind_of_data, :language, :language_label, :last_modified_date,
:lease_expiration_date, :license, :license_label, :member_ids, :member_of_collection_ids, :methodology, :note, :orcid_label,
:other_affiliation_label, :place_of_publication, :project_director_display, :publisher, :related_url, :rendering_ids, :representative_id, :researcher_display,
:resource_type, :rights_holder, :rights_notes, :rights_statement, :rights_statement_label, :sponsor, :source, :subject,
:thumbnail_id, :title, to: :solr_document
end
end
end
12 changes: 12 additions & 0 deletions app/views/hyrax/base/_representative_media.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<%# [hyc-override] https://github.com/samvera/hyrax/tree/hyrax-v4.0.0/app/views/hyrax/base/_representative_media.html.erb %>
<%# [hyc-override] Use custom function fetch_primary_fileset_id in place of representative_id %>
<!-- This renders the thumbnail for first file set associated with a work instead of the default in case the representative_id is nil. (Typically occurs after deleting the first file attached to a work) -->
<% if presenter.fetch_primary_fileset_id.present? && presenter.representative_presenter.present? %>
<% if defined?(viewer) && viewer %>
<%= iiif_viewer_display presenter %>
<% else %>
<%= render media_display_partial(presenter.representative_presenter), file_set: presenter.representative_presenter %>
<% end %>
<% else %>
<%= image_tag 'default.png', class: "canonical-image", alt: 'default representative image' %>
<% end %>
2 changes: 1 addition & 1 deletion app/views/hyrax/base/_show_actions.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
<div class="col-sm-6 text-right">
<% if presenter.editor? && !workflow_restriction?(presenter) %>
<%= link_to t('.edit'), edit_polymorphic_path([main_app, presenter]), class: 'btn btn-secondary' %>
<% if presenter.member_count > 1 %>
<% if presenter.member_count > 0 %>
<%= link_to t("hyrax.file_manager.link_text"), polymorphic_path([main_app, :file_manager, presenter]), class: 'btn btn-secondary' %>
<% end %>
<% if presenter.valid_child_concerns.length > 0 %>
Expand Down
2 changes: 1 addition & 1 deletion config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class Application < Rails::Application
overrides = "#{Rails.root}/app/overrides"
config.to_prepare do
Dir.glob("#{overrides}/**/*_override.rb").sort.each do |c|
Rails.configuration.cache_classes ? require(c) : load(c)
Rails.configuration.cache_classes ? require(c) : require_dependency(c)
end
end

Expand Down
51 changes: 51 additions & 0 deletions spec/presenters/blacklight/thumbnail_presenter_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# frozen_string_literal: true
require 'rails_helper'
require Rails.root.join('app/overrides/presenters/blacklight/thumbnail_presenter_override.rb')

RSpec.describe Blacklight::ThumbnailPresenter do
describe '#retrieve_values' do
it 'does not attempt to process a solr document if it is nil' do
# Mock field config and view context
field_config = double('FieldConfig')
view_context = double('ViewContext')
presenter = Blacklight::ThumbnailPresenter.new(nil, view_context, field_config)

retriever_instance = Blacklight::FieldRetriever.new(nil, nil, nil)
allow(Blacklight::FieldRetriever).to receive(:new).and_return(retriever_instance)
allow(retriever_instance).to receive(:fetch).and_return('default_thumbnail')
allow(presenter).to receive(:extract_solr_document)

result = presenter.send(:retrieve_values, field_config)
expect(Blacklight::FieldRetriever).to have_received(:new).with(nil, field_config, view_context)
# Presenter should not process the document if it's nil
expect(presenter).not_to have_received(:extract_solr_document)
expect(result).to eq('default_thumbnail')
end

it 'updates the thumbnail_path_ss if it needs an update' do
field_config = double('FieldConfig')
view_context = double('ViewContext')
retriever_instance = Blacklight::FieldRetriever.new(nil, nil, nil)
document_hash = {
'thumbnail_path_ss' => '/assets/work-default.png',
'file_set_ids_ssim' => ['file_set_1'],
'id' => '1'
}
solr_doc = SolrDocument.new(document_hash)
presenter = Blacklight::ThumbnailPresenter.new(solr_doc, view_context, field_config)

allow(presenter).to receive(:needs_thumbnail_path_update?).and_return(true)
allow(SolrDocument).to receive(:new).and_return(instance_double(SolrDocument))
allow(Blacklight::FieldRetriever).to receive(:new).and_return(retriever_instance)
allow(retriever_instance).to receive(:fetch).and_return('updated_thumbnail')
allow(Rails.logger).to receive(:info)

result = presenter.send(:retrieve_values, field_config)
expect(Rails.logger).to have_received(:info).with('Updated thumbnail_path_ss: /downloads/file_set_1?file=thumbnail for work with id 1')
expect(SolrDocument).to have_received(:new).with(
hash_including('thumbnail_path_ss' => '/downloads/file_set_1?file=thumbnail')
)
expect(result).to eq('updated_thumbnail')
end
end
end
16 changes: 16 additions & 0 deletions spec/presenters/hyrax/work_show_presenter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,20 @@
it { is_expected.to delegate_method(:resource_type).to(:solr_document) }
it { is_expected.to delegate_method(:keyword).to(:solr_document) }
it { is_expected.to delegate_method(:itemtype).to(:solr_document) }

describe '#representative_presenter' do
context 'when member_presenters raises a Hyrax::ObjectNotFoundError' do
before do
allow(presenter).to receive(:fetch_primary_fileset_id).and_return('file_set_id_1')
allow(presenter).to receive(:member_presenters).and_raise(Hyrax::ObjectNotFoundError)
allow(Hyrax.logger).to receive(:warn)
end

it 'logs a warning and returns nil' do
result = presenter.representative_presenter
expect(Hyrax.logger).to have_received(:warn).with('Unable to find representative_id file_set_id_1 for work 888888')
expect(result).to be_nil
end
end
end
end
Loading