Skip to content

Commit

Permalink
🐛 Ensure consistent logic path for when iiif viewer
Browse files Browse the repository at this point in the history
There is a conflict between the `IiifPrint` and `Hyrax::IiifAv` gems;
namely they both have strong opinions about how to sniff out if we
should use the `iiif_viewer?`.  Compounding this, is that IiifPrint
decorates `Hyrax::WorkShowPresenter` and Hyku extends
`Hyrax::WorkShowPresenter` then includes
`Hyrax::IiifAv::DisplaysIiifAv`.

The end result is that the logic to determine if we should show pages
split from the PDF is never called.  Yet, if we were to solely use
IiifPrint we'd ignore rendering audio and vidoe in the iiif viewer.

So this commit peels that back so that we're using the logic (brought
forward by IiifPrint) but ensuring our Hyku presenters are using that
logic.

Why move the logic out of the Hyku instance and into a module?  Because
that module contains the per-tenant antics of IiifPrint and its PDF
relationship.

There is larger work to do in regards to incorporating this logic into
IiifPrint and the Hyrax::IiifAv gem.

Does this work?  Please pull down a branch and check.  I'm at the end of
my day (and then some).

Related to:

- #659
  • Loading branch information
jeremyf committed Aug 11, 2023
1 parent 9864421 commit 34cfa9e
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 21 deletions.
26 changes: 7 additions & 19 deletions app/presenters/hyku/work_show_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ class WorkShowPresenter < Hyrax::WorkShowPresenter
# Hyrax::MemberPresenterFactory.file_presenter_class = Hyrax::FileSetPresenter
# Adds behaviors for hyrax-iiif_av plugin.
include Hyrax::IiifAv::DisplaysIiifAv

##
# NOTE: IIIF Print prepends a IiifPrint::WorkShowPresenterDecorator to Hyrax::WorkShowPresenter
# However, with the above `include Hyrax::IiifAv::DisplaysIiifAv` we obliterate that logic. So
# we need to re-introduce that logic.
prepend IiifPrint::TenantConfig::WorkShowPresenterDecorator

Hyrax::MemberPresenterFactory.file_presenter_class = Hyrax::IiifAv::IiifFileSetPresenter

delegate :title_or_label, :extent, :additional_information, :source, :bibliographic_citation, :admin_note, :date, to: :solr_document
Expand Down Expand Up @@ -69,27 +76,8 @@ def user_can_feature_collection?
end
# End Featured Collections Methods

# @return [Boolean] render a IIIF viewer
def iiif_viewer?
Hyrax.config.iiif_image_server? &&
representative_id.present? &&
representative_presenter.present? &&
iiif_media? &&
members_include_viewable?
end

private

def iiif_media?(presenter: representative_presenter)
presenter.image? || presenter.video? || presenter.audio? || presenter.pdf?
end

def members_include_viewable?
file_set_presenters.any? do |presenter|
iiif_media?(presenter: presenter) && current_ability.can?(:read, presenter.id)
end
end

def extract_from_identifier(rgx)
if solr_document['identifier_tesim'].present?
ref = solr_document['identifier_tesim'].map do |str|
Expand Down
40 changes: 40 additions & 0 deletions app/services/iiif_print/tenant_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,46 @@ def service
super
end
end

##
# OVERRIDE IiifPrint::WorkShowPresenterDecorator
# OVERRIDE Hyrax::WorkShowPresenter
#
# In IiifPrint we overrided #members_include_viewable_image? to query for both file sets and
# child works. (Child works being the pages split off of a PDF)
#
# In Hyrax::WorkShowPresenter we're only looking at the underlying file_sets. But IiifPrint
# needs to look at multiple places.
module WorkShowPresenterDecorator
##
# @return [Array<Symbol>] predicate methods (e.g. ending in "?") that reflect the types
# of files we want to consider for showing in the IIIF Viewer.
def iiif_media_predicates
if TenantConfig.use_iiif_print?
[:image?, :audio?, :video?, :pdf?]
else
[:image?, :audio?, :video?]
end
end

def iiif_media?(presenter: representative_presenter)
iiif_media_predicates.any? { |predicate| presenter.try(predicate) || presenter.try(:solr_document).try(predicate) }
end

##
# @return [Boolean] render a IIIF viewer
def iiif_viewer?
Hyrax.config.iiif_image_server? &&
representative_id.present? &&
representative_presenter.present? &&
iiif_media? &&
members_include_viewable_image?
end

def file_type_and_permissions_valid?(presenter)
iiif_media?(presenter: presenter) && current_ability.can?(:read, presenter.id)
end
end
end
end

Expand Down
2 changes: 2 additions & 0 deletions config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ class Application < Rails::Application
#
# @see https://github.com/scientist-softserv/iiif_print/blob/9e7837ce4bd08bf8fff9126455d0e0e2602f6018/lib/iiif_print/engine.rb#L54 Where we do the override.
Hyrax::Actors::FileSetActor.prepend(IiifPrint::TenantConfig::FileSetActorDecorator)

Hyrax::WorkShowPresenter.prepend(IiifPrint::TenantConfig::WorkShowPresenterDecorator)
end
end
end
38 changes: 36 additions & 2 deletions spec/presenters/hyku/work_show_presenter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,36 @@
before do
allow(solr_document).to receive(:representative_id).and_return(solr_document.member_ids.first)
allow(ability).to receive(:can?).and_return true
allow_any_instance_of(Hyrax::IiifAv::IiifFileSetPresenter).to receive(:image?).and_return true
allow_any_instance_of(Hyrax::IiifAv::IiifFileSetPresenter).to receive(:image?).and_return false
allow_any_instance_of(Hyrax::IiifAv::IiifFileSetPresenter).to receive(:pdf?).and_return false
allow_any_instance_of(Hyrax::IiifAv::IiifFileSetPresenter).to receive(:video?).and_return false
allow_any_instance_of(Hyrax::IiifAv::IiifFileSetPresenter).to receive(:audio?).and_return false
end

it { is_expected.to be true }
context 'method owner' do
# I was noticing load logic issues, so I'm adding this spec for verification
subject { presenter.method(:iiif_viewer?).owner }

it { is_expected.to eq(IiifPrint::TenantConfig::WorkShowPresenterDecorator) }
end

context "for a PDF file" do
let!(:test_strategy) { Flipflop::FeatureSet.current.test! }

before { allow_any_instance_of(Hyrax::IiifAv::IiifFileSetPresenter).to receive(:pdf?).and_return true }

context 'when the tenant is not configured to use IIIF Print' do
before { test_strategy.switch!(:use_iiif_print, false) }

it { is_expected.to be false }
end

context 'when the tenant is configured to use IIIF Print' do
before { test_strategy.switch!(:use_iiif_print, true) }

it { is_expected.to be true }
end
end

context "for an audio file" do
before do
Expand All @@ -35,6 +61,14 @@
it { is_expected.to be true }
end

context "for an image file" do
before do
allow_any_instance_of(Hyrax::IiifAv::IiifFileSetPresenter).to receive(:image?).and_return true
end

it { is_expected.to be true }
end

context "for a video file" do
before do
allow_any_instance_of(Hyrax::IiifAv::IiifFileSetPresenter).to receive(:video?).and_return true
Expand Down
18 changes: 18 additions & 0 deletions spec/services/iiif_print/tenant_config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -184,5 +184,23 @@ def initialize(file_set); end
it { is_expected.to match_array([IiifPrint::TenantConfig::DerivativeService, Hyrax::FileSetDerivativesService]) }
end
end

describe Hyrax::WorkShowPresenter do
let(:instance) { described_class.new(:solr_doc, :ability) }

describe '#iiif_media_predicates' do
context 'when the feature is flipped to false' do
before { test_strategy.switch!(:use_iiif_print, false) }

it { is_expected.to eq([:image?, :audio?, :video?]) }
end

context 'when the feature is flipped to true' do
before { test_strategy.switch!(:use_iiif_print, true) }

it { is_expected.to eq([:image?, :audio?, :video?, :pdf?]) }
end
end
end
end
# rubocop:enable RSpec/DescribeClass

0 comments on commit 34cfa9e

Please sign in to comment.