From f2be199e0b3b252aaee655805367a06518cfa8c3 Mon Sep 17 00:00:00 2001 From: dnoneill <noneill@stanford.edu> Date: Fri, 14 Mar 2025 16:09:45 -0400 Subject: [PATCH] show no_download pdfs in content list, use probe service to build messaging --- app/assets/stylesheets/companion_window.css | 8 +- .../authorization_messages_component.html.erb | 3 +- .../authorization_messages_component.rb | 10 +-- .../content_list_component.rb | 2 +- .../location_restriction_component.html.erb | 4 - .../location_restriction_component.rb | 5 -- app/components/model_component.html.erb | 20 +++-- app/components/model_component.rb | 4 - app/components/pdf_component.html.erb | 14 ++-- .../restricted_message_component.html.erb | 4 + .../restricted_message_component.rb | 4 + .../controllers/file_auth_controller.js | 15 +--- .../iiif_auth_restriction_controller.js | 13 +-- app/viewers/embed/viewer/document_viewer.rb | 19 ----- .../content_list_component_spec.rb | 4 +- spec/components/pdf_component_spec.rb | 2 +- ...b => restricted_message_component_spec.rb} | 6 +- spec/features/document_viewer_spec.rb | 8 -- spec/lib/embed/viewer/document_viewer_spec.rb | 80 ------------------- 19 files changed, 49 insertions(+), 176 deletions(-) delete mode 100644 app/components/location_restriction_component.html.erb delete mode 100644 app/components/location_restriction_component.rb create mode 100644 app/components/restricted_message_component.html.erb create mode 100644 app/components/restricted_message_component.rb rename spec/components/{location_restriction_component_spec.rb => restricted_message_component_spec.rb} (74%) delete mode 100644 spec/lib/embed/viewer/document_viewer_spec.rb diff --git a/app/assets/stylesheets/companion_window.css b/app/assets/stylesheets/companion_window.css index 9310e4825..55fb99c4f 100644 --- a/app/assets/stylesheets/companion_window.css +++ b/app/assets/stylesheets/companion_window.css @@ -337,20 +337,26 @@ body { .vert-tabs, .left-drawer, .modal-components-popover, + .authLinkWrapper span, .authLinkWrapper, .sul-embed-geo-sidebar-header, .copy-to-clipboard, .sul-embed-was-seed-list-item { + --display: inline-block; svg { fill: currentColor; width: 1em; height: 1em; - display: inline-block; + display: var(--display); font-size: 1.5rem; flex-shrink: 0; } } + .authLinkWrapper span { + --display: block; + } + .companion-window-component-body { display: flex; flex-grow: 1; diff --git a/app/components/companion_windows/authorization_messages_component.html.erb b/app/components/companion_windows/authorization_messages_component.html.erb index 334f97afb..2e28a8a78 100644 --- a/app/components/companion_windows/authorization_messages_component.html.erb +++ b/app/components/companion_windows/authorization_messages_component.html.erb @@ -1,5 +1,4 @@ <div data-controller="iiif-auth-restriction" data-action="<%= authorization_actions %>"> <%= render LoginComponent.new %> - <%= render LocationRestrictionComponent.new %> - <%= render CompanionWindows::ContentNotAvailableComponent.new unless available? %> + <%= render RestrictedMessageComponent.new %> </div> diff --git a/app/components/companion_windows/authorization_messages_component.rb b/app/components/companion_windows/authorization_messages_component.rb index e7e8a1372..9146d534f 100644 --- a/app/components/companion_windows/authorization_messages_component.rb +++ b/app/components/companion_windows/authorization_messages_component.rb @@ -2,21 +2,13 @@ module CompanionWindows class AuthorizationMessagesComponent < ViewComponent::Base - def initialize(available:) - @available = available - end - - def available? - @available - end - # bindings to stimulus actions def authorization_actions %w[auth-denied@window->iiif-auth-restriction#displayMessage needs-login@window->iiif-auth-restriction#displayLoginPrompt auth-success@window->iiif-auth-restriction#hideLoginPrompt login-success@window->iiif-auth-restriction#showMessagePanel - thumbnail-clicked@window->iiif-auth-restriction#clearLocationRestriction].join(' ') + thumbnail-clicked@window->iiif-auth-restriction#clearRestrictedMessage].join(' ') end end end diff --git a/app/components/companion_windows/content_list_component.rb b/app/components/companion_windows/content_list_component.rb index 28e1a7e23..0ad096681 100644 --- a/app/components/companion_windows/content_list_component.rb +++ b/app/components/companion_windows/content_list_component.rb @@ -13,7 +13,7 @@ def document_viewer? end def resource_files_collection - viewer.purl_object.resource_files.reject(&:no_download?) + viewer.purl_object.resource_files end end end diff --git a/app/components/location_restriction_component.html.erb b/app/components/location_restriction_component.html.erb deleted file mode 100644 index c91ee30b8..000000000 --- a/app/components/location_restriction_component.html.erb +++ /dev/null @@ -1,4 +0,0 @@ -<div class="authLinkWrapper" data-iiif-auth-restriction-target="locationRestriction" hidden> - <%= render Icons::LockGlobeComponent.new %> - <p class="loginMessage" data-iiif-auth-restriction-target="locationRestrictionMessage"></p> -</div> diff --git a/app/components/location_restriction_component.rb b/app/components/location_restriction_component.rb deleted file mode 100644 index da6e7e2fd..000000000 --- a/app/components/location_restriction_component.rb +++ /dev/null @@ -1,5 +0,0 @@ -# frozen_string_literal: true - -# This is only used by the PdfComponent -class LocationRestrictionComponent < ViewComponent::Base -end diff --git a/app/components/model_component.html.erb b/app/components/model_component.html.erb index f9a5b85dd..33a10d023 100644 --- a/app/components/model_component.html.erb +++ b/app/components/model_component.html.erb @@ -8,22 +8,20 @@ <% end %> <% component.with_authorization_messages do %> - <%= render CompanionWindows::AuthorizationMessagesComponent.new(available: viewable_content?) %> + <%= render CompanionWindows::AuthorizationMessagesComponent.new %> <% end %> <% component.with_body do %> - <div data-controller="locked-poster" data-action="auth-success@window->locked-poster#hide needs-login@window->locked-poster#show auth-denied@window->locked-poster#show" role="presentation" style="flex: 1 0 100%; text-align: center;" <%= 'hidden' if viewable_content? %>> + <div data-controller="locked-poster" data-action="auth-success@window->locked-poster#hide needs-login@window->locked-poster#show auth-denied@window->locked-poster#show" role="presentation" style="flex: 1 0 100%; text-align: center;" hidden> <%= render LockedStatusComponent.new %> </div> - <% if viewable_content? %> - <div class="sul-embed-3d sul-embed-body" data-controller="model" data-action="iiif-manifest-received@window->file-auth#parseFiles"> - <div class="buttons"> - <button aria-label="Zoom in" data-action="model#zoomIn" class="zoom-in">+</button> - <button aria-label="Zoom out" data-action="model#zoomOut" class="zoom-out">-</button> - </div> - <div data-model-target="modelViewerContainer" class="model-viewer-container" data-action="auth-success@window->model#show"> - </div> + <div class="sul-embed-3d sul-embed-body" data-controller="model" data-action="iiif-manifest-received@window->file-auth#parseFiles"> + <div class="buttons"> + <button aria-label="Zoom in" data-action="model#zoomIn" class="zoom-in">+</button> + <button aria-label="Zoom out" data-action="model#zoomOut" class="zoom-out">-</button> </div> - <% end %> + <div data-model-target="modelViewerContainer" class="model-viewer-container" data-action="auth-success@window->model#show"> + </div> + </div> <% end %> <% end %> diff --git a/app/components/model_component.rb b/app/components/model_component.rb index 7b5bd9fca..ea251cd24 100644 --- a/app/components/model_component.rb +++ b/app/components/model_component.rb @@ -8,8 +8,4 @@ def initialize(viewer:) attr_reader :viewer delegate :purl_object, to: :viewer - - def viewable_content? - viewer.three_dimensional_files.any? - end end diff --git a/app/components/pdf_component.html.erb b/app/components/pdf_component.html.erb index 61b3fdb60..55eb3593e 100644 --- a/app/components/pdf_component.html.erb +++ b/app/components/pdf_component.html.erb @@ -8,21 +8,19 @@ <% end %> <% component.with_authorization_messages do %> - <%= render CompanionWindows::AuthorizationMessagesComponent.new(available: viewer.available?) %> + <%= render CompanionWindows::AuthorizationMessagesComponent.new %> <% end %> <% component.with_body do %> - <div data-controller="locked-poster" data-action="auth-success@window->locked-poster#hide needs-login@window->locked-poster#show auth-denied@window->locked-poster#show needs-login@window->locked-poster#show" role="presentation" style="flex: 1 0 100%; text-align: center;" <%= 'hidden' if viewer.available? %>> + <div data-controller="locked-poster" data-action="auth-success@window->locked-poster#hide needs-login@window->locked-poster#show auth-denied@window->locked-poster#show needs-login@window->locked-poster#show" role="presentation" style="flex: 1 0 100%; text-align: center;" hidden> <%= render LockedStatusComponent.new %> </div> <div class="sul-embed-body" style="flex-basis: 100%"> <div style="display: flex; flex-direction: column; height: 100%"> - <% if viewer.available? %> - <div class="sul-embed-pdf" style="height: 100%" - data-controller="pdf" - data-action="thumbnail-clicked@window->file-auth#authFileAndDisplay iiif-manifest-received@window->file-auth#parseFiles auth-success@window->pdf#show"> - </div> - <% end %> + <div class="sul-embed-pdf" style="height: 100%" + data-controller="pdf" + data-action="thumbnail-clicked@window->file-auth#authFileAndDisplay iiif-manifest-received@window->file-auth#parseFiles auth-success@window->pdf#show"> + </div> </div> </div> <% end %> diff --git a/app/components/restricted_message_component.html.erb b/app/components/restricted_message_component.html.erb new file mode 100644 index 000000000..fa4600fe8 --- /dev/null +++ b/app/components/restricted_message_component.html.erb @@ -0,0 +1,4 @@ +<div class="authLinkWrapper" data-iiif-auth-restriction-target="restrictedContainer" hidden> + <span class="icon" data-iiif-auth-restriction-target="restrictedIcon"></span> + <p class="loginMessage" data-iiif-auth-restriction-target="restrictedMessage"></p> +</div> \ No newline at end of file diff --git a/app/components/restricted_message_component.rb b/app/components/restricted_message_component.rb new file mode 100644 index 000000000..374d7c9e3 --- /dev/null +++ b/app/components/restricted_message_component.rb @@ -0,0 +1,4 @@ +# frozen_string_literal: true + +class RestrictedMessageComponent < ViewComponent::Base +end diff --git a/app/javascript/controllers/file_auth_controller.js b/app/javascript/controllers/file_auth_controller.js index a06d819c4..78966eea8 100644 --- a/app/javascript/controllers/file_auth_controller.js +++ b/app/javascript/controllers/file_auth_controller.js @@ -136,10 +136,9 @@ export default class extends Controller { .then((result) => this.renderViewer(result)) .catch((authResponse) => { - // Intercept the response and check for location restriction before trying to log in, because - // logging in won't help the fact that we're not in an authorized location. - if (this.isLocationRestricted(authResponse)) - return this.authDenied(authResponse, accessService) + // Intercept the response and check for files that can't be accessed before trying to log in, because + // logging in won't help the fact that we're not in an authorized location/file is no download/embargoed (without stanford login). + if (authResponse.status == '403') return this.authDenied(authResponse, accessService) // Check if non-expired token already exists in local storage, // and if it exists, query probe service with it @@ -258,12 +257,4 @@ export default class extends Controller { const event = new CustomEvent('auth-denied', { detail: { accessService, authResponse } } ) window.dispatchEvent(event) } - - // Checks the result of the probe auth request to see if access is restricted to location - // This code depends on the text returned by probe service, so changes to the heading - // should be reflected here as well. - isLocationRestricted(json) { - return json.status == '401' && 'heading' in json && 'en' in json.heading && json.heading.en.length - && json.heading.en[0].startsWith('Access is restricted to the') - } } diff --git a/app/javascript/controllers/iiif_auth_restriction_controller.js b/app/javascript/controllers/iiif_auth_restriction_controller.js index b3f67cc0c..be9f2477d 100644 --- a/app/javascript/controllers/iiif_auth_restriction_controller.js +++ b/app/javascript/controllers/iiif_auth_restriction_controller.js @@ -2,7 +2,7 @@ import { Controller } from "@hotwired/stimulus" // Display auth controls for the file_auth_controller.js export default class extends Controller { - static targets = ["locationRestriction", "locationRestrictionMessage", "messagePanel", "loginPanel", + static targets = ["restrictedContainer", "restrictedMessage", "restrictedIcon", "messagePanel", "loginPanel", "loginButton", "loginMessage"] // Bound to auth-denied CustomEvents @@ -11,20 +11,21 @@ export default class extends Controller { this.resetMessages() const detail = event.detail - this.locationRestrictionTarget.hidden = false - this.locationRestrictionMessageTarget.innerHTML = this.#retrieveAuthResponseMessage(detail.authResponse) + this.restrictedContainerTarget.hidden = false + this.restrictedMessageTarget.innerHTML = this.#retrieveAuthResponseMessage(detail.authResponse) + this.restrictedIconTarget.innerHTML = detail.authResponse['icon'] } resetMessages() { this.hideLoginPrompt() this.hideMessagePanel() - this.clearLocationRestriction() + this.clearRestrictedMessage() } // Called when switching to a new file - clearLocationRestriction() { - this.locationRestrictionTarget.hidden = true + clearRestrictedMessage() { + this.restrictedContainerTarget.hidden = true } // Allow the user to dismiss the message diff --git a/app/viewers/embed/viewer/document_viewer.rb b/app/viewers/embed/viewer/document_viewer.rb index 9fbfc9a6a..cb3b2511b 100644 --- a/app/viewers/embed/viewer/document_viewer.rb +++ b/app/viewers/embed/viewer/document_viewer.rb @@ -15,12 +15,6 @@ def stylesheet 'document.css' end - def pdf_files - document_resource_files.map do |file| - file.file_url(version: purl_object.version_id) - end - end - def self.show_download? true end @@ -28,19 +22,6 @@ def self.show_download? def fullscreen? true end - - # This indicates if the first PDF is downloadable (though it could be stanford only) - # as well as location restricted. Authorization for other documents will be - # checked as the user clicks on items in the content side bar. - def available? - !document_resource_files.first&.no_download? - end - - private - - def document_resource_files - purl_object.contents.select { |content| content.type == 'document' }.map(&:files).flatten - end end end end diff --git a/spec/components/companion_windows/content_list_component_spec.rb b/spec/components/companion_windows/content_list_component_spec.rb index 7d77ab3e4..decf2fbb2 100644 --- a/spec/components/companion_windows/content_list_component_spec.rb +++ b/spec/components/companion_windows/content_list_component_spec.rb @@ -40,8 +40,8 @@ end describe '#resource_files_collection' do - it 'returns a list of all purl objects that do not have no_download? returning true' do - expect(component.resource_files_collection).to eq([download_file]) + it 'returns a list of all purl objects' do + expect(component.resource_files_collection).to eq([no_download_file, download_file]) end end end diff --git a/spec/components/pdf_component_spec.rb b/spec/components/pdf_component_spec.rb index 494921da6..58c3fd8b9 100644 --- a/spec/components/pdf_component_spec.rb +++ b/spec/components/pdf_component_spec.rb @@ -26,7 +26,7 @@ end it 'includes access restriction method section' do - expect(page).to have_css('div[data-iiif-auth-restriction-target="locationRestriction"]', visible: :all) + expect(page).to have_css('div[data-iiif-auth-restriction-target="restrictedContainer"]', visible: :all) end context 'when hide_title is passed' do diff --git a/spec/components/location_restriction_component_spec.rb b/spec/components/restricted_message_component_spec.rb similarity index 74% rename from spec/components/location_restriction_component_spec.rb rename to spec/components/restricted_message_component_spec.rb index a1df3bac7..e04e961bc 100644 --- a/spec/components/location_restriction_component_spec.rb +++ b/spec/components/restricted_message_component_spec.rb @@ -2,17 +2,17 @@ require 'rails_helper' -RSpec.describe LocationRestrictionComponent, type: :component do +RSpec.describe RestrictedMessageComponent, type: :component do before do render_inline(described_class.new) end # The location restriction banner is hidden so search with visible: :all it 'renders banner target for the file auth controller' do - expect(page).to have_css('div[data-iiif-auth-restriction-target="locationRestriction"]', visible: :all) + expect(page).to have_css('div[data-iiif-auth-restriction-target="restrictedContainer"]', visible: :all) end it 'renders message target for the file auth controller' do - expect(page).to have_css('p[data-iiif-auth-restriction-target="locationRestrictionMessage"]', visible: :all) + expect(page).to have_css('p[data-iiif-auth-restriction-target="restrictedMessage"]', visible: :all) end end diff --git a/spec/features/document_viewer_spec.rb b/spec/features/document_viewer_spec.rb index 78ed97a19..c5de414d6 100644 --- a/spec/features/document_viewer_spec.rb +++ b/spec/features/document_viewer_spec.rb @@ -15,12 +15,4 @@ expect(page).to have_css('.sul-embed-pdf') end end - - context 'when no download' do - let(:purl) { build(:purl, :document_no_download) } - - it 'renders the PDF viewer for documents with restriction message' do - expect(page).to have_content('This item cannot be accessed online') - end - end end diff --git a/spec/lib/embed/viewer/document_viewer_spec.rb b/spec/lib/embed/viewer/document_viewer_spec.rb deleted file mode 100644 index 349f6e12a..000000000 --- a/spec/lib/embed/viewer/document_viewer_spec.rb +++ /dev/null @@ -1,80 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe Embed::Viewer::DocumentViewer do - subject(:pdf_viewer) { described_class.new(request) } - - let(:purl) do - instance_double( - Embed::Purl, - contents: [ - instance_double(Embed::Purl::Resource, type: 'file', - files: [ - instance_double(Embed::Purl::ResourceFile, title: 'file-abc123.pdf', file_url: '//one'), - instance_double(Embed::Purl::ResourceFile, title: 'file-xyz321.pdf', file_url: '//two') - ]), - instance_double(Embed::Purl::Resource, type: 'document', - files: [ - instance_double(Embed::Purl::ResourceFile, title: 'doc-abc123.pdf', file_url: '//file/druid:abc123/version/1/doc-abc123.pdf'), - instance_double(Embed::Purl::ResourceFile, title: 'doc-xyz321.pdf', file_url: '//file/druid:abc123/version/1/doc-xyz321.pdf') - ]) - ], - druid: 'abc123', - version_id: '1' - ) - end - let(:request) { instance_double(Embed::Request, purl_object: purl) } - - describe '#pdf_files' do - it 'returns the full file URL for the PDFs in an object' do - expect(pdf_viewer.pdf_files.length).to eq 2 - expect(pdf_viewer.pdf_files.first).to match(%r{/file/druid:abc123/version/1/doc-abc123\.pdf$}) - expect(pdf_viewer.pdf_files.last).to match(%r{/file/druid:abc123/version/1/doc-xyz321\.pdf}) - end - end - - describe '#available?' do - context 'when the first file in the documents is location restricted and downloadable' do - let(:purl) do - instance_double( - Embed::Purl, - contents: [ - instance_double(Embed::Purl::Resource, type: 'document', files: [instance_double(Embed::Purl::ResourceFile, title: 'doc-abc123.pdf', location_restricted?: true, no_download?: false)]) - ], - druid: 'abc123' - ) - end - - it { expect(pdf_viewer.available?).to be true } - end - - context 'when the first file in the document is not location restricted and downloadable' do - let(:purl) do - instance_double( - Embed::Purl, - contents: [ - instance_double(Embed::Purl::Resource, type: 'document', files: [instance_double(Embed::Purl::ResourceFile, title: 'doc-abc123.pdf', location_restricted?: false, no_download?: false)]) - ], - druid: 'abc123' - ) - end - - it { expect(pdf_viewer.available?).to be true } - end - - context 'when the first file in the document is not downloadable' do - let(:purl) do - instance_double( - Embed::Purl, - contents: [ - instance_double(Embed::Purl::Resource, type: 'document', files: [instance_double(Embed::Purl::ResourceFile, title: 'doc-abc123.pdf', location_restricted?: false, no_download?: true)]) - ], - druid: 'abc123' - ) - end - - it { expect(pdf_viewer.available?).to be false } - end - end -end