From ef2051b6a9cde8072d3ea2bf01d87ef85d847e23 Mon Sep 17 00:00:00 2001 From: conorom Date: Tue, 28 Jan 2025 16:25:47 -0500 Subject: [PATCH] no prev/next to expired permission resources also check role and share link for visibility clause --- app/presenters/hyrax/file_set_presenter.rb | 18 ++++++++++----- spec/features/show_file_set_spec.rb | 26 +++++++++++++++++++++- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/app/presenters/hyrax/file_set_presenter.rb b/app/presenters/hyrax/file_set_presenter.rb index c20c439b5..2179b5289 100644 --- a/app/presenters/hyrax/file_set_presenter.rb +++ b/app/presenters/hyrax/file_set_presenter.rb @@ -65,10 +65,16 @@ def subjects parent.subject end - # If a user can load the current FileSet in draft mode then they should be able to get to draft siblings... - # using prev/next. Examples are logged-in admins/editors or authors using a share link to view a draft book. + # representatives/covers and tombstones should never appear in prev/next. + def prev_next_shared_clauses + "+monograph_id_ssim:#{monograph_id} AND -hidden_representative_bsi:true AND -tombstone_ssim:yes AND -permissions_expiration_date_ssim:[* TO #{Time.zone.today.strftime('%Y-%m-%d')}]" + end + + # A user who can load the stats dashboard has a role of analyst or above, and so should be able to get to draft... + # siblings using prev/next. Authors using a share link to view a draft book should also be able to do this, but... + # anonymous users should not. def prev_next_visibility_clause - if visibility == 'restricted' + if @valid_share_link || current_ability.can?(:read, :stats_dashboard) nil else 'AND -visibility_ssi:restricted ' @@ -85,8 +91,8 @@ def previous_id @previous_id = if solr_document['monograph_position_isi'] == 0 nil else - # representatives/covers and tombstones should never appear in prev/next. See above RE: prev_next_visibility_clause - ActiveFedora::SolrService.query("+monograph_id_ssim:#{monograph_id} AND -hidden_representative_bsi:true AND -tombstone_ssim:yes #{prev_next_visibility_clause}AND monograph_position_isi:[* TO #{solr_document['monograph_position_isi'] - 1}]", + + ActiveFedora::SolrService.query("#{prev_next_shared_clauses} #{prev_next_visibility_clause}AND monograph_position_isi:[* TO #{solr_document['monograph_position_isi'] - 1}]", sort: 'monograph_position_isi desc', rows: 1)&.first&.id end @@ -99,7 +105,7 @@ def next_id? def next_id return @next_id if @next_id.present? - @next_id = ActiveFedora::SolrService.query("+monograph_id_ssim:#{monograph_id} AND -hidden_representative_bsi:true AND -tombstone_ssim:yes #{prev_next_visibility_clause}AND monograph_position_isi:[#{solr_document['monograph_position_isi'] + 1} TO *]", + @next_id = ActiveFedora::SolrService.query("#{prev_next_shared_clauses} #{prev_next_visibility_clause}AND monograph_position_isi:[#{solr_document['monograph_position_isi'] + 1} TO *]", sort: 'monograph_position_isi asc', rows: 1)&.first&.id end diff --git a/spec/features/show_file_set_spec.rb b/spec/features/show_file_set_spec.rb index f388f91d8..f1a1f6d9d 100644 --- a/spec/features/show_file_set_spec.rb +++ b/spec/features/show_file_set_spec.rb @@ -26,6 +26,8 @@ monograph.ordered_members << FactoryBot.create(:file_set) << FactoryBot.create(:public_file_set) monograph.ordered_members << tombstoned_file_set << epub << FactoryBot.create(:public_file_set) + monograph.ordered_members << FactoryBot.create(:public_file_set, permissions_expiration_date: '2025-01-01') + monograph.ordered_members << FactoryBot.create(:public_file_set) monograph.save! FileSet.all.each(&:save!) @@ -63,7 +65,7 @@ # no next link to the tombstone or EPUB representative... expect(page).to_not have_link('Next', href: monograph.ordered_members.to_a[6].id) expect(page).to_not have_link('Next', href: monograph.ordered_members.to_a[7].id) - # ... instead the next link goes to the final published resource + # ... instead the next link goes to the published resource after that expect(page).to have_link('Next', href: monograph.ordered_members.to_a[8].id) visit hyrax_file_set_path(monograph.ordered_members.to_a[8].id) @@ -72,6 +74,16 @@ expect(page).not_to have_link('Previous', href: monograph.ordered_members.to_a[6].id) # ... instead the previous link goes to the previous published resource expect(page).to have_link('Previous', href: monograph.ordered_members.to_a[5].id) + # no next link to the FileSet that's tombstoned by being past its "Permissions Expiration Date"... + expect(page).not_to have_link('Next', href: monograph.ordered_members.to_a[9].id) + # ... instead the next link goes to the published resource after that + expect(page).to have_link('Next', href: monograph.ordered_members.to_a[10].id) + + visit hyrax_file_set_path(monograph.ordered_members.to_a[10].id) + # no previous link to the FileSet that's tombstoned by being past its "Permissions Expiration Date"... + expect(page).not_to have_link('Previous', href: monograph.ordered_members.to_a[9].id) + # ... instead the previous link goes to the previous published resource + expect(page).to have_link('Previous', href: monograph.ordered_members.to_a[8].id) # no next link at all, this is the end of the chain expect(page).to_not have_link('Next') end @@ -97,6 +109,8 @@ end monograph.ordered_members << tombstoned_file_set << epub << FactoryBot.create(:file_set) + monograph.ordered_members << FactoryBot.create(:public_file_set, permissions_expiration_date: '2025-01-01') + monograph.ordered_members << FactoryBot.create(:public_file_set) monograph.save! FileSet.all.each(&:save!) @@ -147,6 +161,16 @@ expect(page).not_to have_link('Previous', href: monograph.ordered_members.to_a[6].id) # ... instead the previous link goes to the previous published resource expect(page).to have_link('Previous', href: monograph.ordered_members.to_a[5].id) + # no next link to the FileSet that's tombstoned by being past its "Permissions Expiration Date"... + expect(page).not_to have_link('Next', href: monograph.ordered_members.to_a[9].id) + # ... instead the next link goes to the published resource after that + expect(page).to have_link('Next', href: monograph.ordered_members.to_a[10].id) + + visit hyrax_file_set_path(monograph.ordered_members.to_a[10].id) + # no previous link to the FileSet that's tombstoned by being past its "Permissions Expiration Date"... + expect(page).not_to have_link('Previous', href: monograph.ordered_members.to_a[9].id) + # ... instead the previous link goes to the previous published resource + expect(page).to have_link('Previous', href: monograph.ordered_members.to_a[8].id) # no next link at all, this is the end of the chain expect(page).to_not have_link('Next') end