Allow tenant configuration to ignore showing PDFs in universal viewer #659

jeremyf · 2023-08-08T17:01:01Z

Summary

We want a feature flipper that turns on and off the PDF processing for iiif print. This means a tenant can decide if they want PDFs showing in the viewer or not.

Testing instructions

Create a PDF work. Go view it to see that it creates with a thumbnail.
Go to the Settings, Features, and toggle the use iiif print feature.
Upload a PDF and allow it to process.
View the work and confirm you can see the PDF in the viewer.

Notes

There should be one single flipflop feature for the following tickets:

Testing Instructions can be found here:

🎁 Allowing per tenant ability to disable IIIF Print #666

The three tickets can all be tested in one swoop.

The text was updated successfully, but these errors were encountered:

The current (as of <2023-08-10 Thu>) implementation of IIIF Print assumes that we generate derivatives at the application level. However, we want options for generating at the tenant level. As noted, I have chosen to default using IIIF print to `true` because that was the behavior before this commit. In other words, this commit is functionally a non-change to the production code-base (assuming no one goes and flips some switches). Related to: - #656 - #657 - #658 - #659

crisr15 · 2023-08-11T18:35:08Z

This fails SoftServ QA:

When I turn off the feature flipper for iiif print and upload a PDF, the file uploads properly and I do not see derivatives created, but the UV still displays an empty viewer. I should see the thumbnail for the work and not have a viewer.
Example work:
https://dev.commons-archive.org/concern/generic_works/4616118a-f8e4-4ace-bb9f-00f80e9073ae?locale=en

Screenshot:

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

crisr15 · 2023-08-16T23:00:59Z

Passes internal QA. Viewer does not show on works that do not have processed files.

jeremyf · 2023-08-17T15:37:57Z

Here's a thread reporting that this has been returned to SoftServ for further fixes: https://assaydepot.slack.com/archives/C0313NKC08L/p1692255870043969

Looking at the logs I'm seeing:

F, [2023-08-17T15:35:26.309458 #1] FATAL -- : [a0b4305bc8cff4fdb06cd6a1ffc94499] ActionView::Template::Error (private method `solr_document' called for #<Hyrax::IiifAv::IiifFileSetPresenter:0x00007f34682eab00>
Did you mean?  solr_document=):
F, [2023-08-17T15:35:26.309569 #1] FATAL -- : [a0b4305bc8cff4fdb06cd6a1ffc94499]     1: <%# Overridden from Hyrax 3.5.0 - To add extra download restrictions %>
[a0b4305bc8cff4fdb06cd6a1ffc94499]     2: <% if display_media_download_link?(file_set: file_set) && (Site.account.settings[:allow_downloads].nil? || Site.account.settings[:allow_downloads].to_i.nonzero?) %>
[a0b4305bc8cff4fdb06cd6a1ffc94499]     3:   <div>
[a0b4305bc8cff4fdb06cd6a1ffc94499]     4:       <h2 class="sr-only"><%= t('hyrax.file_set.show.downloadable_content.heading') %></h2>
[a0b4305bc8cff4fdb06cd6a1ffc94499]     5:       <%= image_tag thumbnail_url(file_set),
F, [2023-08-17T15:35:26.309598 #1] FATAL -- : [a0b4305bc8cff4fdb06cd6a1ffc94499]   
F, [2023-08-17T15:35:26.309625 #1] FATAL -- : [a0b4305bc8cff4fdb06cd6a1ffc94499] app/helpers/hyrax/file_set_helper.rb:16:in

And here's the exception : https://scientist-inc.sentry.io/issues/4368889816/?environment=production&project=6707374&referrer=project-issue-stream

Prior to this commit, we had a private `#solr_document` method. This resulted in an error in the `Hyrax::Ability` as follows: ``` NoMethodError private method `solr_document' called for #<Hyrax::IiifAv::IiifFileSetPresenter:0x00007ff118124808> Did you mean? solr_document= ``` With this commit, I'm making public two methods that might cause issues being private. (And adding a test verifying the interface) Oh dear reviewer, were that I were able to duplicate the bug locally. I tried toggling on and off the feature flag. I also tried toggling the allow downloads. But to no avail. Alas, I cannot replicate but must instead rely on addressing the specific error. Related to: - #659 - https://scientist-inc.sentry.io/issues/4368889816/?environment=production&project=6707374&referrer=project-issue-stream

The current (as of <2023-08-10 Thu>) implementation of IIIF Print assumes that we generate derivatives at the application level. However, we want options for generating at the tenant level. As noted, I have chosen to default using IIIF print to `true` because that was the behavior before this commit. In other words, this commit is functionally a non-change to the production code-base (assuming no one goes and flips some switches). Related to: - scientist-softserv/palni-palci#656 - scientist-softserv/palni-palci#657 - scientist-softserv/palni-palci#658 - scientist-softserv/palni-palci#659

From scientist-softserv/palni-palci#675 > 🐛 Ensure consistent logic path for when iiif viewer > > 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: > > - scientist-softserv/palni-palci#659 Related to: - scientist-softserv/palni-palci#659 - scientist-softserv/palni-palci#675

jeremyf mentioned this issue Aug 8, 2023

🦄 Spike: Determine How to Enable/Disable IIIF Prints Derivatives on a per Tenant Basis #656

Closed

3 tasks

jeremyf assigned jeremyf and unassigned jeremyf Aug 8, 2023

This was referenced Aug 8, 2023

Allow tenant configuration to ignore IIIF Print derivative generation #657

Closed

Allow tenant configuration to ignore IIIF Print splitting PDFs consideration #658

Closed

jeremyf mentioned this issue Aug 10, 2023

🎁 Allowing per tenant ability to disable IIIF Print #666

Merged

crisr15 added the Needs Rework label Aug 11, 2023

jeremyf mentioned this issue Aug 11, 2023

IIIF Print and AV Interplay and Per Tenant Options scientist-softserv/iiif_print#274

Open

crisr15 added the ReShare https://docs.google.com/document/d/1vcA7NNNHuQMn6ocg2LvSNQpcXGzua3cEGyswKQvjEIc/edit# label Aug 14, 2023

crisr15 mentioned this issue Aug 14, 2023

Solr document error when loading show page - after Hyku upgrade #673

Open

1 task

jeremyf linked a pull request Aug 15, 2023 that will close this issue

🐛 Ensure consistent logic path for when iiif viewer #675

Merged

crisr15 added this to palni-palci Aug 16, 2023

crisr15 moved this to PALs QA in palni-palci Aug 16, 2023

labradford self-assigned this Aug 16, 2023

labradford mentioned this issue Aug 16, 2023

Change default for use iiif_print to false per client request #704

Merged

jeremyf mentioned this issue Aug 17, 2023

🐛 Fix issue of calling private solr_document method #706

Merged

jeremyf moved this from PALs QA to Code Review in palni-palci Aug 17, 2023

jeremyf moved this from Code Review to Deploy to Staging in palni-palci Aug 17, 2023

labradford moved this from Deploy to Staging to SoftServ QA in palni-palci Aug 17, 2023

labradford moved this from SoftServ QA to Client Verification in palni-palci Aug 17, 2023

ndroark moved this from Client Verification to Done in palni-palci Aug 24, 2023

ShanaLMoore mentioned this issue Nov 28, 2023

Contribute back PDFjs work #906

Closed

jeremyf mentioned this issue Dec 18, 2023

PALS contribute back to prime samvera/hyku#2061

Closed

jeremyf mentioned this issue Dec 22, 2023

Merge hyrax 5 into pals samvera/hyku#2114

Closed

jeremyf mentioned this issue Jan 8, 2024

Merge pals features in to main samvera/hyku#2138

Merged

jeremyf closed this as completed Jan 8, 2024

jeremyf mentioned this issue Jan 10, 2024

Merge main into i35 samvera/hyku#2153

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow tenant configuration to ignore showing PDFs in universal viewer #659

Allow tenant configuration to ignore showing PDFs in universal viewer #659

jeremyf commented Aug 8, 2023 •

edited by crisr15

Loading

crisr15 commented Aug 11, 2023 •

edited

Loading

crisr15 commented Aug 16, 2023

jeremyf commented Aug 17, 2023

Allow tenant configuration to ignore showing PDFs in universal viewer #659

Allow tenant configuration to ignore showing PDFs in universal viewer #659

Comments

jeremyf commented Aug 8, 2023 • edited by crisr15 Loading

Summary

Testing instructions

Notes

crisr15 commented Aug 11, 2023 • edited Loading

crisr15 commented Aug 16, 2023

jeremyf commented Aug 17, 2023

jeremyf commented Aug 8, 2023 •

edited by crisr15

Loading

crisr15 commented Aug 11, 2023 •

edited

Loading