From 50043e7eca94a94d436b5d51bc5a1aec070847b3 Mon Sep 17 00:00:00 2001 From: Shana Moore Date: Wed, 13 Nov 2024 15:38:01 -0800 Subject: [PATCH] [i886] - update logic to fix collection thumbnail bug The default work thumbnail was displaying even though the default collection thumbnail was set. Issue: - https://github.com/scientist-softserv/adventist_knapsack/issues/886 --- app/helpers/application_helper.rb | 15 ++--- .../hyrax/thumbnail_path_service_decorator.rb | 7 +- spec/helpers/application_helper_spec.rb | 64 ++++++++++++++++++- 3 files changed, 74 insertions(+), 12 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index fdac5b1f95..d07f32170e 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -18,13 +18,15 @@ def group_navigation_presenter # - fallback to Hyrax's default image def collection_thumbnail(document, _image_options = {}, url_options = {}) view_class = url_options[:class] - # The correct thumbnail SHOULD be indexed on the object - return image_tag(document['thumbnail_path_ss'], class: view_class, alt: alttext_for(document)) if document['thumbnail_path_ss'].present? + # Use collection branding thumbnail if it exists + if document['thumbnail_path_ss'].present? && !document['thumbnail_path_ss'].include?('default_work_images') + return image_tag(document['thumbnail_path_ss'], class: view_class, alt: alttext_for(document)) + end - # If nothing is indexed, we just fall back to site default - return image_tag(Site.instance.default_collection_image&.url, alt: alttext_for(document), class: view_class) if Site.instance.default_collection_image.present? + # If no branding thumbnail, use the site's default collection image + return image_tag(Site.instance.default_collection_image.url, alt: alttext_for(document), class: view_class) if Site.instance.default_collection_image.present? - # fall back to Hyrax default if no site default + # Fallback to Hyrax's default image if no site default tag.span("", class: [Hyrax::ModelIcon.css_class_for(::Collection), view_class], alt: alttext_for(document)) end @@ -41,7 +43,6 @@ def alttext_for(collection) def hint_for(term:, record_class: nil) hint = locale_for(type: 'hints', term:, record_class:) - return hint unless missing_translation(hint) end @@ -50,7 +51,6 @@ def locale_for(type:, term:, record_class:) record_class = record_class.to_s.downcase work_or_collection = record_class == Hyrax.config.collection_model.downcase ? 'collection' : 'defaults' locale = t("hyrax.#{record_class}.#{type}.#{term}") - if missing_translation(locale) (t("simple_form.#{type}.#{work_or_collection}.#{term}")).try(:html_safe) else @@ -66,7 +66,6 @@ def missing_translation(value, _options = {}) def markdown(text) return text unless Flipflop.treat_some_user_inputs_as_markdown? - # Consider extracting these options to a Hyku::Application # configuration/class attribute. options = %i[ diff --git a/app/services/hyrax/thumbnail_path_service_decorator.rb b/app/services/hyrax/thumbnail_path_service_decorator.rb index 876c87a7cd..e9dc571676 100644 --- a/app/services/hyrax/thumbnail_path_service_decorator.rb +++ b/app/services/hyrax/thumbnail_path_service_decorator.rb @@ -7,13 +7,18 @@ module ThumbnailPathServiceDecorator def call(object) return super unless object.try(:collection?) + # Check if a collection-specific branding thumbnail exists collection_thumbnail = CollectionBrandingInfo.where(collection_id: object.id.to_s, role: "thumbnail").first return collection_thumbnail.local_path.gsub(Hyrax.config.branding_path.to_s, '/branding') if collection_thumbnail - return default_image if object.try(:thumbnail_id).blank? + # Fallback to site-wide default collection image if no branding thumbnail + return default_collection_image if object.thumbnail_id.blank? + # Retrieve thumbnail if it exists, otherwise use the default collection image thumb = fetch_thumbnail(object) return default_collection_image unless thumb + + # Process thumbnail for audio or default image types return call(thumb) unless thumb.file_set? if audio?(thumb) audio_image diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 99b36b8998..4b2aa1a1db 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -1,5 +1,4 @@ # frozen_string_literal: true - RSpec.describe ApplicationHelper, type: :helper do describe "#markdown" do let(:header) { '# header' } @@ -8,7 +7,6 @@ context 'when treat_some_user_inputs_as_markdown is true' do it 'renders markdown into html' do allow(Flipflop).to receive(:treat_some_user_inputs_as_markdown?).and_return(true) - expect(helper.markdown(header)).to eq("

header

\n") expect(helper.markdown(bold)).to eq("

bold

\n") end @@ -17,7 +15,6 @@ context 'when treat_some_user_inputs_as_markdown is false' do it 'does not render markdown into html' do allow(Flipflop).to receive(:treat_some_user_inputs_as_markdown?).and_return(false) - expect(helper.markdown(header)).to eq('# header') expect(helper.markdown(bold)).to eq('*bold*') end @@ -31,4 +28,65 @@ end end end + + describe '#collection_thumbnail' do + let(:document) { instance_double('SolrDocument', id: '123', title_or_label: 'Test Collection') } + let(:url_options) { { class: 'view-class' } } + + context 'when collection has a branding thumbnail' do + before do + allow(document).to receive(:[]).with('thumbnail_path_ss').and_return('/path/to/thumbnail.jpg') + end + + it 'returns the branding thumbnail image tag' do + expect(helper.collection_thumbnail(document, {}, url_options)).to include('src="/path/to/thumbnail.jpg"') + expect(helper.collection_thumbnail(document, {}, url_options)).to include('class="view-class"') + end + end + + context 'when collection has no branding thumbnail but site has default image' do + let(:site_instance) { instance_double('Site') } + let(:default_image) { instance_double('DefaultImage', url: '/default/image.jpg') } + + before do + allow(document).to receive(:[]).with('thumbnail_path_ss').and_return(nil) + allow(Site).to receive(:instance).and_return(site_instance) + allow(site_instance).to receive(:default_collection_image).and_return(default_image) + allow(site_instance).to receive(:default_collection_image).and_return(default_image) + end + + it 'returns the site default collection image tag' do + expect(helper.collection_thumbnail(document, {}, url_options)).to include('src="/default/image.jpg"') + expect(helper.collection_thumbnail(document, {}, url_options)).to include('class="view-class"') + end + end + + context 'when collection has no branding thumbnail and no site default image' do + before do + allow(document).to receive(:[]).with('thumbnail_path_ss').and_return(nil) + allow(Site).to receive(:instance).and_return(instance_double('Site', default_collection_image: nil)) + allow(Hyrax::ModelIcon).to receive(:css_class_for).with(::Collection).and_return('collection-icon-class') + end + + it 'returns the Hyrax default icon span' do + result = helper.collection_thumbnail(document, {}, url_options) + expect(result).to include('class="collection-icon-class view-class"') + expect(result).to include('