From be62a456508d6431f42ddbcd196794b3802a18b2 Mon Sep 17 00:00:00 2001 From: Brian Kelly Date: Thu, 1 Aug 2024 13:26:37 -0500 Subject: [PATCH 1/4] Restricted downloads open in new tab --- app/helpers/geoblacklight_helper.rb | 5 ++- .../catalog/_downloads_collapse.html.erb | 2 +- spec/features/show_page_spec.rb | 44 ++++++++++++++----- 3 files changed, 37 insertions(+), 14 deletions(-) diff --git a/app/helpers/geoblacklight_helper.rb b/app/helpers/geoblacklight_helper.rb index 9271b1c5..d9322118 100644 --- a/app/helpers/geoblacklight_helper.rb +++ b/app/helpers/geoblacklight_helper.rb @@ -13,7 +13,7 @@ def iiif_jpg_url @document.references.iiif.endpoint.sub! 'info.json', 'full/full/0/default.jpg' end - def download_link_file(label, id, url) + def download_link_file(label, id, url, target = nil) link_to( label, url, @@ -22,7 +22,8 @@ def download_link_file(label, id, url) download: 'trigger', download_type: 'direct', download_id: id - } + }, + target: target ) end diff --git a/app/views/catalog/_downloads_collapse.html.erb b/app/views/catalog/_downloads_collapse.html.erb index 89013a8f..d38b2fa6 100644 --- a/app/views/catalog/_downloads_collapse.html.erb +++ b/app/views/catalog/_downloads_collapse.html.erb @@ -13,7 +13,7 @@ <% end %> <% end %> <% if document.direct_download[:download].is_a? String %> - <%= download_link_file(download_text(document.file_format), document.id, document.direct_download[:download]) %> + <%= download_link_file(download_text(document.file_format), document.id, document.direct_download[:download], document.restricted? ? "_blank" : nil) %> <% end %> <% end %> diff --git a/spec/features/show_page_spec.rb b/spec/features/show_page_spec.rb index dabd018a..e666b26d 100644 --- a/spec/features/show_page_spec.rb +++ b/spec/features/show_page_spec.rb @@ -2,19 +2,41 @@ describe 'Show page' do context 'with restricted NYU result - nyu-2451-34626' do - it 'the displays warning message' do - visit solr_document_path 'nyu-2451-34626' - expect(page).to have_content 'This dataset is only available to members of the New York University community' - end + context 'when signed out' do + it 'the displays warning message' do + visit solr_document_path 'nyu-2451-34626' + expect(page).to have_content 'This dataset is only available to members of the New York University community' + end - it 'does not display download' do - visit solr_document_path 'nyu-2451-34626' - expect(page).not_to have_content 'Export' + it 'does not display download' do + visit solr_document_path 'nyu-2451-34626' + expect(page).not_to have_content 'Export' + end + + it 'includes link to login' do + visit solr_document_path 'nyu-2451-34626' + expect(page).to have_link('Login to View and Download') + end end - it 'includes link to login' do - visit solr_document_path 'nyu-2451-34626' - expect(page).to have_link('Login to View and Download') + context 'when signed in' do + before do + login_as(create(:user), scope: :user) + end + + it 'shows a download button' do + visit solr_document_path 'nyu-2451-34626' + expect(page).to have_button('Download') + end + + it 'opens download links in a new tab' do + visit solr_document_path 'nyu-2451-34626' + click_button('Download') + + within_window(window_opened_by { click_link('Original Shapefile') }) do + expect(page.title).not_to include('NYU Spatial Data Repository') + end + end end end @@ -44,7 +66,7 @@ # TODO: Refactor this to play nice with Rubocop # rubocop:disable RSpec/ExampleLength, RSpec/MultipleExpectations - context 'with multiple downloads - nyu-2451-38645' do # + context 'with multiple downloads - nyu-2451-38645' do it 'includes six download links' do visit solr_document_path 'nyu-2451-38645' expect(page).to have_content 'Download' From 3d049c8ed02992f926ec8ba79395eeca7d367dcb Mon Sep 17 00:00:00 2001 From: Brian Kelly Date: Thu, 1 Aug 2024 15:11:31 -0500 Subject: [PATCH 2/4] Convert downlaod collapse to a view component with a spec --- .../downloads_collapse_component.html.erb | 32 +++++++++++++++ .../catalog/downloads_collapse_component.rb | 9 +++++ .../catalog/_downloads_collapse.html.erb | 1 - app/views/catalog/_show_downloads.html.erb | 2 +- .../downloads_collapse_component_spec.rb | 39 +++++++++++++++++++ 5 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 app/components/catalog/downloads_collapse_component.html.erb create mode 100644 app/components/catalog/downloads_collapse_component.rb create mode 100644 spec/components/catalog/downloads_collapse_component_spec.rb diff --git a/app/components/catalog/downloads_collapse_component.html.erb b/app/components/catalog/downloads_collapse_component.html.erb new file mode 100644 index 00000000..b9a92b34 --- /dev/null +++ b/app/components/catalog/downloads_collapse_component.html.erb @@ -0,0 +1,32 @@ +<%# Renders the options of the downloads dropdown button %> + +<% if @document.multi_direct_downloads.present? %> + <% @document.multi_direct_downloads.each do |download| %> + <%= download_link_file(download[0], @document.id, download[1]) %> + <% end %> +<% end %> +<% if @document.direct_download.present? %> + <% if @document.direct_download[:download].is_a? Array %> + <% @document.direct_download[:download].each do |download| %> + <%= download_link_file(download['label'], @document.id, download['url']) %> + <% end %> + <% end %> + <% if @document.direct_download[:download].is_a? String %> + <%= download_link_file(download_text(@document.file_format), @document.id, @document.direct_download[:download], @document.restricted? ? "_blank" : nil) %> + <% end %> +<% end %> + +<% if @document.hgl_download.present? %> + <%= download_link_hgl(download_text(@document.download_types.first[0]), @document) %> +<% end %> + +<% if @document.iiif_download.present? %> + <%= download_link_iiif %> +<% end %> + +<% if @document.download_types.present? %> + <% @document.download_types.each do |type| %> + <% next if type.first == :kmz %> + <%= download_link_generated(type.first, @document) %> + <% end %> +<% end %> diff --git a/app/components/catalog/downloads_collapse_component.rb b/app/components/catalog/downloads_collapse_component.rb new file mode 100644 index 00000000..47aa7846 --- /dev/null +++ b/app/components/catalog/downloads_collapse_component.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module Catalog + class DownloadsCollapseComponent < ViewComponent::Base + def initialize(document:) + @document = document + end + end +end diff --git a/app/views/catalog/_downloads_collapse.html.erb b/app/views/catalog/_downloads_collapse.html.erb index d38b2fa6..23024170 100644 --- a/app/views/catalog/_downloads_collapse.html.erb +++ b/app/views/catalog/_downloads_collapse.html.erb @@ -1,5 +1,4 @@ <%# Renders the options of the downloads dropdown button %> -<% document ||= @document %> <% if document.multi_direct_downloads.present? %> <% document.multi_direct_downloads.each do |download| %> diff --git a/app/views/catalog/_show_downloads.html.erb b/app/views/catalog/_show_downloads.html.erb index c52755ea..c8e26271 100644 --- a/app/views/catalog/_show_downloads.html.erb +++ b/app/views/catalog/_show_downloads.html.erb @@ -9,7 +9,7 @@
- <%= render 'downloads_collapse' %> + <%= render Catalog::DownloadsCollapseComponent.new(document: @document) %>
diff --git a/spec/components/catalog/downloads_collapse_component_spec.rb b/spec/components/catalog/downloads_collapse_component_spec.rb new file mode 100644 index 00000000..d9fd1a94 --- /dev/null +++ b/spec/components/catalog/downloads_collapse_component_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Catalog::DownloadsCollapseComponent, type: :component do + let(:document) { instance_double(SolrDocument, id: 123) } + + before do + allow(document).to receive_messages( + multi_direct_downloads: [], + file_format: 'Shapefile', + direct_download: { download: 'https://example.com/download' }, + restricted?: restricted, + hgl_download: false, + iiif_download: false, + download_types: [] + ) + end + + context 'when the download is restricted' do + let(:restricted) { true } + + it 'renders restricted links with a target of _blank' do + render_inline(described_class.new(document:)) + + expect(page).to have_link('Original Shapefile', href: 'https://example.com/download', target: '_blank') + end + end + + context 'when the download is not restricted' do + let(:restricted) { false } + + it 'renders the download link with no target' do + render_inline(described_class.new(document:)) + + expect(page).to have_link('Original Shapefile', href: 'https://example.com/download', target: nil) + end + end +end From ea9dd33a6441bf9f7656893cf3893999bfad640c Mon Sep 17 00:00:00 2001 From: Brian Kelly Date: Fri, 2 Aug 2024 11:11:51 -0500 Subject: [PATCH 3/4] Remove partial replaced by view component --- .../catalog/_downloads_collapse.html.erb | 32 ------------------- 1 file changed, 32 deletions(-) delete mode 100644 app/views/catalog/_downloads_collapse.html.erb diff --git a/app/views/catalog/_downloads_collapse.html.erb b/app/views/catalog/_downloads_collapse.html.erb deleted file mode 100644 index 23024170..00000000 --- a/app/views/catalog/_downloads_collapse.html.erb +++ /dev/null @@ -1,32 +0,0 @@ -<%# Renders the options of the downloads dropdown button %> - -<% if document.multi_direct_downloads.present? %> - <% document.multi_direct_downloads.each do |download| %> - <%= download_link_file(download[0], document.id, download[1]) %> - <% end %> -<% end %> -<% if document.direct_download.present? %> - <% if document.direct_download[:download].is_a? Array %> - <% document.direct_download[:download].each do |download| %> - <%= download_link_file(download['label'], document.id, download['url']) %> - <% end %> - <% end %> - <% if document.direct_download[:download].is_a? String %> - <%= download_link_file(download_text(document.file_format), document.id, document.direct_download[:download], document.restricted? ? "_blank" : nil) %> - <% end %> -<% end %> - -<% if document.hgl_download.present? %> - <%= download_link_hgl(download_text(document.download_types.first[0]), document) %> -<% end %> - -<% if document.iiif_download.present? %> - <%= download_link_iiif %> -<% end %> - -<% if document.download_types.present? %> - <% document.download_types.each do |type| %> - <% next if type.first == :kmz %> - <%= download_link_generated(type.first, document) %> - <% end %> -<% end %> From 3a62805a30f8c7f79db3d61aa034f18e8131589c Mon Sep 17 00:00:00 2001 From: Brian Kelly Date: Mon, 5 Aug 2024 08:54:38 -0500 Subject: [PATCH 4/4] View Component initializer needs to call super --- app/components/catalog/downloads_collapse_component.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/components/catalog/downloads_collapse_component.rb b/app/components/catalog/downloads_collapse_component.rb index 47aa7846..796aabb8 100644 --- a/app/components/catalog/downloads_collapse_component.rb +++ b/app/components/catalog/downloads_collapse_component.rb @@ -3,6 +3,7 @@ module Catalog class DownloadsCollapseComponent < ViewComponent::Base def initialize(document:) + super @document = document end end