From 7d7b4340c3eecdd6f8ba81bdc087dc6152f9144f Mon Sep 17 00:00:00 2001
From: LaRita Robinson <larita@scientist.com>
Date: Thu, 19 Sep 2024 16:27:49 -0400
Subject: [PATCH] Clean up viewer logic

- Updates Bulkrax
- Add documentation to work_show_presenter
- Update representative_media bootstrap classes
- Update _relationships partial to coordinate with current Hyrax
- Update some obsolete code in image_show/show.html.erb
---
 Gemfile                                       |  2 +-
 Gemfile.lock                                  | 40 +++++++----------
 app/presenters/hyku/work_show_presenter.rb    | 32 +++++++++----
 app/views/hyrax/base/_relationships.html.erb  | 45 ++++---------------
 .../hyrax/base/_representative_media.html.erb | 19 +++++---
 .../image_show/hyrax/base/show.html.erb       |  6 +--
 6 files changed, 66 insertions(+), 78 deletions(-)

diff --git a/Gemfile b/Gemfile
index 934fd791ec..f1a4b9e42b 100644
--- a/Gemfile
+++ b/Gemfile
@@ -30,7 +30,7 @@ gem 'blacklight_range_limit'
 gem 'bolognese', '>= 1.9.10'
 gem 'bootstrap', '~> 4.6'
 gem 'bootstrap-datepicker-rails'
-gem 'bulkrax', github: 'samvera/bulkrax', branch: 'main'
+gem 'bulkrax', '~> 8.0'
 gem 'byebug', group: %i[development test]
 gem 'capybara', group: %i[test]
 gem 'capybara-screenshot', '~> 1.0', group: %i[test]
diff --git a/Gemfile.lock b/Gemfile.lock
index f07a6c64e7..7898c56c17 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -118,28 +118,6 @@ GIT
       iiif_manifest (> 0.5)
       rails (>= 5.1, < 8.0)
 
-GIT
-  remote: https://github.com/samvera/bulkrax.git
-  revision: b4328d73e595f9de3daa8666066f25e7d4f44573
-  branch: main
-  specs:
-    bulkrax (8.1.0)
-      bagit (~> 0.4.6)
-      coderay
-      denormalize_fields
-      iso8601 (~> 0.9.0)
-      kaminari
-      language_list (~> 1.2, >= 1.2.1)
-      libxml-ruby (~> 3.2.4)
-      loofah (>= 2.2.3)
-      marcel
-      oai (>= 0.4, < 2.x)
-      rack (>= 2.0.6)
-      rails (>= 5.1.6)
-      rdf (>= 2.0.2, < 4.0)
-      rubyzip
-      simple_form
-
 GIT
   remote: https://github.com/samvera/hyrax.git
   revision: e4886f1e70a1ee9962c604a76a523ef4b32ccd46
@@ -406,6 +384,22 @@ GEM
       signet (~> 0.8)
       typhoeus
     builder (3.2.4)
+    bulkrax (8.2.0)
+      bagit (~> 0.4.6)
+      coderay
+      denormalize_fields
+      iso8601 (~> 0.9.0)
+      kaminari
+      language_list (~> 1.2, >= 1.2.1)
+      libxml-ruby (~> 3.2.4)
+      loofah (>= 2.2.3)
+      marcel
+      oai (>= 0.4, < 2.x)
+      rack (>= 2.0.6)
+      rails (>= 5.1.6)
+      rdf (>= 2.0.2, < 4.0)
+      rubyzip
+      simple_form
     byebug (11.1.3)
     cancancan (3.6.1)
     capybara (3.39.2)
@@ -1482,7 +1476,7 @@ DEPENDENCIES
   bolognese (>= 1.9.10)
   bootstrap (~> 4.6)
   bootstrap-datepicker-rails
-  bulkrax!
+  bulkrax (~> 8.0)
   byebug
   capybara
   capybara-screenshot (~> 1.0)
diff --git a/app/presenters/hyku/work_show_presenter.rb b/app/presenters/hyku/work_show_presenter.rb
index 2fd1da3cb0..1e399a0d4e 100644
--- a/app/presenters/hyku/work_show_presenter.rb
+++ b/app/presenters/hyku/work_show_presenter.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 # OVERRIDE here to add featured collection methods and to delegate collection presenters to the member presenter factory
-# OVERRIDE: Hyrax 5.0.0rc2 to add Hyrax IIIF AV
+# OVERRIDE: Hyrax 5 to add Hyrax IIIF AV and manage logic for which viewer to display
 
 module Hyku
   class WorkShowPresenter < Hyrax::WorkShowPresenter
@@ -72,6 +72,11 @@ def user_can_feature_collection?
     end
     # End Featured Collections Methods
 
+    ##
+    # Begin viewer determination logic
+    # note: iiif_viewer is defined in TenantConfig
+
+    # @return [Boolean] Use PDF.js viewer
     def show_pdf_viewer?
       return unless Flipflop.default_pdf_viewer?
       return unless show_pdf_viewer
@@ -80,6 +85,22 @@ def show_pdf_viewer?
       show_for_pdf?(show_pdf_viewer)
     end
 
+    # @return [Boolean] Use video embed viewer
+    def video_embed_viewer?
+      extract_video_embed_presence
+    end
+
+    # @return [Boolean] use any viewer
+    def viewer?
+      iiif_viewer? || video_embed_viewer? || show_pdf_viewer?
+    end
+
+    # The use of universal_viewer has been removed, but leaving
+    # an alias in case any knapsack apps use it
+    # @todo: is this method obsolete?
+    alias universal_viewer? iiif_viewer?
+
+    # @return [Boolean] allow download via button below viewer
     def show_pdf_download_button?
       return unless Hyrax.config.display_media_download_link?
       return unless file_set_presenters.any?(&:pdf?)
@@ -88,10 +109,6 @@ def show_pdf_download_button?
       show_for_pdf?(show_pdf_download_button)
     end
 
-    def viewer?
-      iiif_viewer? || video_embed_viewer? || show_pdf_viewer?
-    end
-
     def parent_works(current_user = nil)
       @parent_works ||= begin
                           docs = solr_document.load_parent_docs
@@ -104,12 +121,9 @@ def parent_works(current_user = nil)
                         end
     end
 
-    def video_embed_viewer?
-      extract_video_embed_presence
-    end
-
     private
 
+    # @todo: is this method obsolete?
     def members_include_viewable?
       file_set_presenters.any? do |presenter|
         iiif_media?(presenter:) && current_ability.can?(:read, presenter.id)
diff --git a/app/views/hyrax/base/_relationships.html.erb b/app/views/hyrax/base/_relationships.html.erb
index 72727e6c38..668de1be5f 100644
--- a/app/views/hyrax/base/_relationships.html.erb
+++ b/app/views/hyrax/base/_relationships.html.erb
@@ -1,39 +1,10 @@
-<%# OVERRIDE Hyrax 2.9 to make relationships only show if they exist %>
-<% if !current_user && presenter.grouped_presenters.present? %>
-  <%# Collection %>
-  <div class="work-show">
-    <h2><%= t('hyrax.base.show.relationships') %></h2>
-    <%= render 'relationships_parent_rows', presenter: presenter %>
-  </div>
-<% end %>
-
-<% if current_user %>
-  <div class="work-show">
-    <h2><%= t('hyrax.base.show.relationships') %></h2>
-    <%# Admin Set %>
-    <%= presenter.attribute_to_html(:admin_set, render_as: :faceted, html_dl: true) %>
-
-    <%# Collection %>
-    <% presenter.grouped_presenters(except: presenter.presenter_types).each_pair do |model_name, items| %>
-      <%= render 'relationships_parent_row', type: model_name, items: items, presenter: presenter %>
-    <% end %>
-    <%# Render grouped presenters. Show rows if there are any items of that type %>
-    <% presenter.presenter_types.each do |type| %>
-      <% presenter.grouped_presenters(filtered_by: type).each_pair do |_, items| %>
-        <%= render 'relationships_parent_row', type: type, items: items, presenter: presenter %>
-      <% end %>
-    <% end %>
+<%# OVERRIDE Hyrax 6 to include parent works %>
+<dl class="work-show">
+  <%= render 'relationships_parent_rows', presenter: presenter %>
+</dl>
 
-    <%# Parent Work %>
-    <%# Render a link back to its parent works %>
-    <% if presenter.parent_works(current_user).present? %>
-      <%= render 'relationships_parent_works_rows', presenter: presenter %>
-    <% end %>
-  </div>
-<% else %>
-  <%# Parent Work %>
-  <%# Render a link back to its parent works %>
-  <% if presenter.parent_works(current_user).present? %>
-    <%= render 'relationships_parent_works_rows', presenter: presenter %>
-  <% end %>
+<%# Parent Work %>
+<%# Render a link back to its parent works %>
+<% if presenter.parent_works(current_user).present? %>
+  <%= render 'relationships_parent_works_rows', presenter: presenter %>
 <% end %>
diff --git a/app/views/hyrax/base/_representative_media.html.erb b/app/views/hyrax/base/_representative_media.html.erb
index b575576780..b0747e4be0 100644
--- a/app/views/hyrax/base/_representative_media.html.erb
+++ b/app/views/hyrax/base/_representative_media.html.erb
@@ -1,12 +1,21 @@
 <% if presenter.representative_id.present? && presenter.representative_presenter.present? %>
+  <%# logic to determine which viewer appears is defined in: %>
+  <%# pdf_js_helper, work_show_presenter, and tenant_config %>
   <% if defined?(viewer) && viewer && presenter.iiif_viewer?%>
-    <%= iiif_viewer_display presenter %>
-  <% elsif Flipflop.default_pdf_viewer? && presenter.show_pdf_viewer? && presenter.file_set_presenters.any?(&:pdf?) %>
-    <%= render 'pdf_js', file_set_presenter: presenter.file_set_presenters.first %>
+    <div class="center-block">
+      <%= iiif_viewer_display presenter %>
+    </div>
+  <% elsif presenter.show_pdf_viewer? %>
+    <div class="center-block">
+      <%= render 'pdf_js', file_set_presenter: presenter.file_set_presenters.first %>
+    </div>
   <% else %>
-    <%= render media_display_partial(presenter.representative_presenter), file_set: presenter.representative_presenter %>
+    <div class="center-block">
+      <%= render media_display_partial(presenter.representative_presenter), file_set: presenter.representative_presenter %>
+    </div>
   <% end %>
+
 <% else %>
   <% alt = block_for(name: 'default_work_image_text') || 'Default work thumbnail' %>
-  <%= image_tag default_work_image, class: "canonical-image", alt: alt %>
+  <%= image_tag default_work_image, class: "canonical-image center-block", alt: alt %>
 <% end %>
diff --git a/app/views/themes/image_show/hyrax/base/show.html.erb b/app/views/themes/image_show/hyrax/base/show.html.erb
index 23f0705723..620d061501 100644
--- a/app/views/themes/image_show/hyrax/base/show.html.erb
+++ b/app/views/themes/image_show/hyrax/base/show.html.erb
@@ -16,7 +16,7 @@
       <div class="panel-body">
         <div class="row">
           <%= render 'workflow_actions_widget', presenter: @presenter %>
-          <% if @presenter.universal_viewer? %>
+          <% if @presenter.iiif_viewer? %>
             <div class="col-sm-12">
               <%= render 'representative_media', presenter: @presenter, viewer: true %>
             </div>
@@ -25,7 +25,7 @@
                 <%= render 'work_description', presenter: @presenter %>
               </div>
             </div>
-          <% elsif Flipflop.default_pdf_viewer? && @presenter.show_pdf_viewer? && @presenter.file_set_presenters.any?(&:pdf?) %>
+          <% elsif @presenter.show_pdf_viewer? %>
             <div class="col-sm-12">
               <%= render 'pdf_js', file_set_presenter: pdf_file_set_presenter(@presenter) %>
             </div>
@@ -34,7 +34,7 @@
             </div>
           <% else %>
             <div class="col-sm-3 mb-1">
-              <%= render 'representative_media', presenter: @presenter, viewer: false unless @presenter.universal_viewer? || @presenter.show_pdf_viewer? %>
+              <%= render 'representative_media', presenter: @presenter, viewer: false unless @presenter.iiif_viewer? || @presenter.show_pdf_viewer? %>
             </div>
             <div class="col-sm-9">
               <%= render 'work_description', presenter: @presenter %>