Merge pull request #2244 from samvera/branding-fixes

Branding fixes

app/controllers/hyrax/dashboard/collections_controller_decorator.rb

@@ -2,9 +2,10 @@
 
 # OVERRIDE Hyrax v5.0.0rc2
 # - Fix file upload in logo and banner
-# - Use work titles for collection thumbnail select & to add an option to reset to the default thumbnail
-
-# OVERRIDE Hyrax v5.0.0 to add the ability to upload a collection thumbnail
+# - ensure user is allowed to change visibility
+# - add the ability to upload a collection thumbnail
+# - add altext to collection banner
+# @TODO clean up of unnecessary methods now that we are using Valkyrie transactions to set branding
 
 module Hyrax
   module Dashboard
@@ -19,21 +20,14 @@ def show
         super
       end
 
-      # OVERRIDE Hyrax v5.0.0 to add the ability to upload a collection thumbnail - START
-      def process_branding
-        process_banner_input
-        process_logo_input
-        process_thumbnail_input
-      end
-
       # rubocop:disable Metrics/AbcSize
       def update_valkyrie_collection
         return after_update_errors(form_err_msg(form)) unless form.validate(collection_params)
 
         result = transactions['change_set.update_collection']
                  .with_step_args(
                           'collection_resource.save_collection_banner' => { update_banner_file_ids: params["banner_files"],
-                                                                            banner_unchanged_indicator: params["banner_unchanged"] },
+                                                                            alttext: params["banner_text"]&.first },
                           'collection_resource.save_collection_logo' => { update_logo_file_ids: params["logo_files"],
                                                                           alttext_values: params["alttext"],
                                                                           linkurl_values: params["linkurl"] },
@@ -48,12 +42,11 @@ def update_valkyrie_collection
         after_update_response
       end
       # rubocop:enable Metrics/AbcSize
-      # OVERRIDE Hyrax v5.0.0 to add the ability to upload a collection thumbnail - END
 
       def edit
         form
         # Gets original filename of an uploaded thumbnail. See #update
-        return unless ::SolrDocument.find(@collection.id).thumbnail_path.include?("uploaded_collection_thumbnails") && uploaded_thumbnail?
+        return unless ::SolrDocument.find(@collection.id).thumbnail_path&.include?("uploaded_collection_thumbnails") && uploaded_thumbnail?
         @thumbnail_filename = File.basename(uploaded_thumbnail_files.reject { |f| File.basename(f).include? @collection.id }.first)
       end
 
@@ -72,6 +65,14 @@ def update
         super
       end
 
+      # OVERRIDE Hyrax v5.0.0 to add the ability to upload a collection thumbnail
+      # Not used with Valkyrie
+      def process_branding
+        process_banner_input
+        process_logo_input
+        process_thumbnail_input
+      end
+
       # Deletes any previous thumbnails. The thumbnail indexer (see services/hyrax/indexes_thumbnails)
       # checks if an uploaded thumbnail exists in the public folder before indexing the thumbnail path.
       def delete_uploaded_thumbnail
@@ -124,7 +125,7 @@ def configure_show_sort_fields
         blacklight_config.sort_fields = CatalogController.blacklight_config.sort_fields
       end
 
-      # branding specific methods
+      ## Branding Methods not used with Valkyrie
       def process_banner_input
         return update_existing_banner if params["banner_unchanged"] == "true"
         remove_banner

app/helpers/application_helper.rb

@@ -12,17 +12,33 @@ def group_navigation_presenter
     @group_navigation_presenter ||= Hyku::Admin::Group::NavigationPresenter.new(params:)
   end
 
-  def collection_thumbnail(document, _image_options = {}, _url_options = {})
-    return image_tag(document['thumbnail_path_ss']) if document['thumbnail_path_ss'].present?
-    return super if Site.instance.default_collection_image.blank?
+  # Return collection thumbnail formatted for display:
+  #  - use collection's branding thumbnail if it exists
+  #  - use site's default collection image if one exists
+  #  - 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?
 
-    image_tag(Site.instance.default_collection_image&.url)
+    # 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?
+
+    # fall back to Hyrax default if no site default
+    tag.span("", class: [Hyrax::ModelIcon.css_class_for(::Collection), view_class],
+                 alt: alttext_for(document))
   end
 
   def label_for(term:, record_class: nil)
     locale_for(type: 'labels', term:, record_class:)
   end
 
+  def alttext_for(collection)
+    thumbnail = CollectionBrandingInfo.where(collection_id: collection.id, role: "thumbnail")&.first
+    return thumbnail.alt_text if thumbnail
+    block_for(name: 'default_collection_image_text') || "#{collection.title_or_label} #{t('hyrax.dashboard.my.sr.thumbnail')}"
+  end
+
   def hint_for(term:, record_class: nil)
     hint = locale_for(type: 'hints', term:, record_class:)
 

app/presenters/hyrax/collection_presenter_decorator.rb

@@ -81,14 +81,9 @@ def banner_file
                        end
     end
 
+    # use either the indexed thumbnail or find the branding for the collection
     def thumbnail_file
-      @thumbnail_file ||= CollectionBrandingInfo.where(collection_id: id, role: "thumbnail")
-                                                .select(:local_path, :alt_text, :target_url).map do |thumbnail|
-        { alttext: thumbnail.alt_text,
-          file: File.split(thumbnail.local_path).last,
-          file_location: "/#{thumbnail.local_path.split('/')[-4..-1].join('/')}",
-          linkurl: thumbnail.target_url }
-      end
+      @thumbnail_file ||= collection_thumbnail(solr_document)
     end
 
     # Begin Featured Collections Methods

app/services/hyrax/thumbnail_path_service_decorator.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# OVERRIDE Hyrax v5.0.0rc2 - use site defaults instead of app wide defaults
+# OVERRIDE Hyrax v5.0.0rc2 - index using site defaults instead of app wide defaults
 
 module Hyrax
   module ThumbnailPathServiceDecorator
@@ -10,7 +10,11 @@ def call(object)
       collection_thumbnail = CollectionBrandingInfo.where(collection_id: object.id.to_s, role: "thumbnail").first
       return collection_thumbnail.local_path.gsub(Rails.public_path.to_s, '') if collection_thumbnail
 
-      default_image
+      default_collection_image
+    end
+
+    def default_collection_image
+      Site.instance.default_collection_image&.url || ActionController::Base.helpers.image_path('default.png')
     end
 
     def default_image

app/views/hyrax/collections/_media_display.html.erb

@@ -1,8 +1,3 @@
-<% if presenter.thumbnail_path %>
-  <%= image_tag presenter.thumbnail_path,
-                class: "representative-media",
-                alt: block_for(name: 'default_collection_image_text'),
-                role: "presentation" %>
-<% else %>
-  <%= image_tag(Site.instance.default_collection_image) %>
-<% end %>
+<%# OVERRIDE Hyrax 6.0: %>
+<%# show uploaded thumbnail or default with alttext %>
+<%= collection_thumbnail(presenter.solr_document, "", {class: "representative-media"}) %>

app/views/hyrax/collections/show.html.erb

@@ -1,4 +1,7 @@
-<%# OVERRIDE Hyrax 5.0.1 add show actions buttons to collection show page %>
+<%# OVERRIDE Hyrax 5.0.1: %>
+<%# add show actions buttons to collection show page %>
+<%# add branding text for banner image %>
+<%# remove duplicate items count originating in Hyrax view %>
 
 <% provide :page_title, construct_page_title(@presenter.title) %>
 <div class="hyc-container" itemscope itemtype="http://schema.org/CollectionPage">
@@ -36,16 +39,15 @@
       <% unless @presenter.total_viewable_items.blank? %>
           <div class="hyc-bugs">
             <div class="hyc-item-count">
-              <b><%= @presenter.total_viewable_items %></b>
               <%= pluralize(@presenter.total_viewable_items, t('.item_count')) %></div>
 
-            <% unless @presenter.creator.blank? %>
-                <div class="hyc-created-by">Created by: <%= @presenter.creator.first %></div>
-            <% end %>
+              <% unless @presenter.creator.blank? %>
+                  <div class="hyc-created-by">Created by: <%= @presenter.creator.first %></div>
+              <% end %>
 
-            <% unless @presenter.modified_date.blank? %>
-                <div class="hyc-last-updated">Last Updated: <%= @presenter.modified_date %></div>
-            <% end %>
+              <% unless @presenter.modified_date.blank? %>
+                  <div class="hyc-last-updated">Last Updated: <%= @presenter.modified_date %></div>
+              <% end %>
           </div>
       <% end %>
 

app/views/hyrax/dashboard/collections/_list_collections.html.erb

@@ -22,19 +22,13 @@
   <td>
     <div class="thumbnail-title-wrapper">
       <div class="thumbnail-wrapper">
-        <% if (collection_presenter.thumbnail_path == nil) %>
-          <%= image_tag(Site.instance.default_collection_image) %>
-        <% else %>
-          <%# OVERRIDE begin %>
-          <%= image_tag(collection_presenter.thumbnail_path, alt: block_for(name: 'default_collection_image_text') || "#{collection_presenter.title_or_label} #{t('hyrax.dashboard.my.sr.thumbnail')}") %>
-          <%# OVERRIDE end %>
-        <% end %>
+        <%# Use appropriate collection thumbnail + alttext %>
+        <%= collection_thumbnail(collection_presenter.solr_document) %>
       </div>
       <%= link_to collection_presenter.show_path do %>
           <span class="sr-only"><%= t("hyrax.dashboard.my.sr.show_label") %> </span>
           <%= markdown(collection_presenter.title_or_label) %>
       <% end %>
-
       <%# Expand arrow %>
       <a href="#" class="small show-more" title="Click for more details">
         <i id="expand_<%= id %>" class="fa fa-chevron-right" aria-hidden="true"></i>

app/views/hyrax/my/collections/_list_collections.html.erb

@@ -24,13 +24,8 @@
   <td>
     <div class="thumbnail-title-wrapper">
       <div class="thumbnail-wrapper">
-        <% if (collection_presenter.thumbnail_path == nil) %>
-          <span class="<%= Hyrax::ModelIcon.css_class_for(::Collection) %> collection-icon-small"></span>
-        <% else %>
-          <%# OVERRIDE begin %>
-          <%= image_tag(collection_presenter.thumbnail_path, alt: collection_presenter.thumbnail_file.first&.[](:alttext) || block_for(name: 'default_collection_image_text') || "#{collection_presenter.title_or_label} #{t('hyrax.dashboard.my.sr.thumbnail')}") %>
-          <%# OVERRIDE end %>
-        <% end %>
+        <%# Use appropriate collection thumbnail + alttext %>
+        <%= collection_thumbnail(collection_presenter.solr_document) %>
       </div>
       <%= link_to collection_presenter.show_path, id: "src_copy_link#{id}" do %>
         <span class="sr-only"><%= t("hyrax.dashboard.my.sr.show_label") %></span>

lib/hyrax/transactions/steps/save_collection_banner_decorator.rb

@@ -1,22 +1,41 @@
 # frozen_string_literal: true
 
-# OVERRIDE Hyrax v5.0.0 to save the collection banner in 'public/uploads'
+# OVERRIDE Hyrax v5.0.0 to save the collection banner in 'public/uploads' and include alttext
 
 module Hyrax
   module Transactions
     module Steps
       module SaveCollectionBannerDecorator
         include Hyku::CollectionBrandingBehavior
 
-        def add_new_banner(collection_id:, uploaded_file_ids:)
+        def call(collection_resource, update_banner_file_ids: nil, alttext: nil)
+          collection_id = collection_resource.id.to_s
+          process_banner_input(collection_id:, update_banner_file_ids:, alttext:)
+          Success(collection_resource)
+        end
+
+        def process_banner_input(collection_id:, update_banner_file_ids:, alttext:)
+          if !update_banner_file_ids && !alttext
+            remove_banner(collection_id:)
+          elsif update_banner_file_ids
+            remove_banner(collection_id:)
+            add_new_banner(collection_id:, uploaded_file_ids: update_banner_file_ids, alttext:)
+          elsif alttext
+            CollectionBrandingInfo
+              .where(collection_id:, role: "banner")
+              .first.update_column(:alt_text, alttext) # rubocop:disable Rails/SkipsModelValidations
+          end
+        end
+
+        def add_new_banner(collection_id:, uploaded_file_ids:, alttext:)
           f = uploaded_files(uploaded_file_ids).first
           file_location = process_file_location(f)
 
           banner_info = CollectionBrandingInfo.new(
             collection_id:,
             filename: File.split(f.file_url).last,
             role: "banner",
-            alt_txt: "",
+            alt_txt: alttext,
             target_url: ""
           )
           banner_info.save file_location