Skip to content

Commit

Permalink
♻️ Favor member_ids_ssim over file_set_ids_ssim
Browse files Browse the repository at this point in the history
For several years Hyrax has index `file_set_ids_ssim` as a verbatim copy
of `member_ids_ssim`.  With Hyrax 5, we're removing the
`file_set_ids_ssim` from indexing; And given that it's been a verbatim
copy since 2017 or so, it's relatively safe to assume that we can favor,
without application impact, the `member_ids_ssim` over the
`file_set_ids_ssim` value.

It would be nice to have `file_set_ids_ssim` but not as a verbatim copy.
Someday, we might have nice things.

Related to:
- samvera/hyrax#6513
- samvera/hyrax@7108409
  • Loading branch information
jeremyf committed Feb 23, 2024
1 parent e96015e commit ddf6138
Show file tree
Hide file tree
Showing 8 changed files with 17 additions and 12 deletions.
6 changes: 4 additions & 2 deletions app/models/concerns/iiif_print/solr/document.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,11 @@ def respond_to_missing?(method_name, include_private = false)
iiif_print_solr_field_names.include?(method_name.to_s) || super
end

# TODO: consider configuring this field name; we use the magic field in lots of places.
# @see https://github.com/samvera/hyrax/commit/7108409c619cd2ba4ae8c836b9f3b429a7e9837b
def file_set_ids
self['file_set_ids_ssim']
# Yes, this looks a little odd. But the truth is the prior key (e.g. `file_set_ids_ssim`) was
# an alias of `member_ids_ssim`.
self['member_ids_ssim']
end

def any_highlighting?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def build
presenter_class.for(solr_doc)
elsif Hyrax.config.curation_concerns.include?(solr_doc.hydra_model)
# look up file set ids and loop through those
file_set_docs = load_file_set_docs(solr_doc.try(:file_set_ids) || solr_doc.try(:[], 'file_set_ids_ssim'))
file_set_docs = load_file_set_docs(solr_doc.try(:member_ids) || solr_doc.try(:[], 'member_ids_ssim'))
file_set_docs.map { |doc| presenter_class.for(doc) } if file_set_docs.length
end
end.flatten.compact
Expand Down
7 changes: 5 additions & 2 deletions app/presenters/iiif_print/work_show_presenter_decorator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

module IiifPrint
module WorkShowPresenterDecorator
delegate :file_set_ids, to: :solr_document
delegate :member_ids, to: :solr_document
alias file_set_ids member_ids

# OVERRIDE Hyrax 2.9.6 to remove check for representative_presenter.image?
# @return [Boolean] render a IIIF viewer
Expand All @@ -20,8 +21,10 @@ def iiif_viewer?
# overriding Hyrax to include file sets for both work and child works (file set ids include both)
# process each id, short-circuiting the loop once one true value is found. This speeds up the test
# by not loading more member_presenters than needed.
#
# @todo Review if this is necessary for Hyrax 5.
def members_include_viewable_image?
all_member_ids = solr_document.try(:file_set_ids) || solr_document.try(:[], 'file_set_ids_ssim')
all_member_ids = solr_document.try(:member_ids) || solr_document.try(:[], 'member_ids_ssim')
Array.wrap(all_member_ids).each do |id|
return true if file_type_and_permissions_valid?(member_presenters_for([id]).first)
end
Expand Down
4 changes: 2 additions & 2 deletions lib/iiif_print/blacklight_iiif_search/annotation_decorator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,10 @@ def base_url
def file_set_id
return document['id'] if document.file_set?

file_set_ids = document['file_set_ids_ssim']
file_set_ids = document['member_ids_ssim']
raise "#{self.class}: NO FILE SET ID" if file_set_ids.blank?

# Since a parent work's `file_set_ids_ssim` can contain child work ids as well as file set ids,
# Since a parent work's `member_ids_ssim` can contain child work ids as well as file set ids,
# this will ensure that the file set id is indeed a `FileSet`
file_set_ids.detect { |id| SolrDocument.find(id).file_set? }
end
Expand Down
2 changes: 1 addition & 1 deletion spec/factories/newspaper_page_solr_document.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
title_tesim: ['Page 1'],
has_model_ssim: ['NewspaperPage'],
issue_id_ssi: 'abc123',
file_set_ids_ssim: [file_set.id],
member_ids_ssim: [file_set.id],
thumbnail_path_ss: '/downloads/123456?file=thumbnail')
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
describe '#annotation_id' do
subject { iiif_search_annotation.annotation_id }
it 'returns a properly formatted URL' do
expect(subject).to include("#{page_document[:issue_id_ssi]}/manifest/canvas/#{page_document[:file_set_ids_ssim].first}/annotation/0")
expect(subject).to include("#{page_document[:issue_id_ssi]}/manifest/canvas/#{page_document[:member_ids_ssim].first}/annotation/0")
end
end

Expand All @@ -44,7 +44,7 @@

subject { iiif_search_annotation.canvas_uri_for_annotation }
it 'returns a properly formatted URL' do
expect(subject).to include("#{page_document[:issue_id_ssi]}/manifest/canvas/#{page_document[:file_set_ids_ssim].first}")
expect(subject).to include("#{page_document[:issue_id_ssi]}/manifest/canvas/#{page_document[:member_ids_ssim].first}")
end

describe 'private methods' do
Expand Down
2 changes: 1 addition & 1 deletion spec/models/solr_document_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
require 'spec_helper'
RSpec.describe SolrDocument do
let(:solr_doc) { described_class.new(id: 'foo', file_set_ids_ssim: ['bar']) }
let(:solr_doc) { described_class.new(id: 'foo', member_ids_ssim: ['bar']) }

describe 'file_set_ids' do
it 'responds to #file_set_ids' do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
{ "id" => "child_work123",
"title_tesim" => ["My Child Image"],
"has_model_ssim" => ["Image"],
"file_set_ids_ssim" => ["child_image_fs123"] }
"member_ids_ssim" => ["child_image_fs123"] }
end
let(:child_fs_attributes) do
{ "id" => "child_fs123",
Expand Down

0 comments on commit ddf6138

Please sign in to comment.