scientist-softserv / louisville-hyku

Other
0 stars 0 forks source link

i173: Fix catalog not searching all_text_tsimv field #183

Closed bkiahstroud closed 1 year ago

bkiahstroud commented 1 year ago

Ref #173

TODO: specs && add testing instruction for QA

Now, when doing a "normal" catalog search, if a term is found in a child work's searchable_text field or a file set's all_text_tsimv field, the parent work will show up in the results.

This used to work, but seems to have been broken by recent slug work. Using slugs instead of IDs seems to fix the problem.

In addition to these updates brought in, we added app_indexer_spec.rb. While creating the test coverage for app_indexer.rb we noticed:

all_descendent_file_sets(object) method was also including the children works, which was unexpected to me while testing I only expected to ever receive the file_set's slug/id. We added clarity around the naming of the method and updated it to descendent_member_ids_for(object). This is due to the file_set_ids_ssim solr field in Hyrax upstream adding the children works to the array, you can read more about that here: https://github.com/scientist-softserv/iiif_print/pull/216.

The ancestor_ids(object) method's test were failing in the app_indexer.rb as well as the file_indexer.rb. Through setting up the rspec tests we were able to conclude that the objects needed to be indexed on the to_param method which gives the object.slug priority over object.id.

Code before:

def a_ids(o)
    a_ids = []
    o.in_works.each do |work|
      a_ids << work.to_param
      a_ids += ancestor_ids(work) if work.is_child
    end
    a_ids
  end

Our solution is very similar to the descendent_member_ids_for(object) method's.

ancestor_ids_array << object.members.map(&:to_param)
ancestor_ids_array.flatten.uniq.compact

For a solution that works for the expected array:

def ancestor_ids(object)
    ancestor_ids_array = []
    object.in_works.each do |work|
      ancestor_ids_array << work.to_param
      ancestor_ids_array += ancestor_ids(work) if work.is_child
    end
    ancestor_ids_array << object.members.map(&:to_param)
    ancestor_ids_array.flatten.uniq.compact
  end

As I am writing this I am wondering if we even need that method? the 2 (descendent_member_ids_for(object) && ancestor_ids(object)) are very similar if not exact now that we have the knowledge that the file_sets AND child_works are indexed into the array.

Note: This is also getting contributed back into the iiif_print gem here: https://github.com/scientist-softserv/iiif_print/pull/216

Co-authored-by: Alisha Evans alishaevn2@gmail.com Co-authored-by: April Rieger april.rieger@yahoo.com Co-authored-by: Shana Moore shana@scientist.com

alishaevn commented 1 year ago

closed in favor of #188