make search string used to look up objects configurable (#884)

* make search string used to look up objects configurable

* fix lint require for ci, add example to initializer

* fix typo

* move object factory specs to all be in one place

* make rubocop happy

* unpin dry-monads. its not a dependency of bulkrax, but previously needed to be pinned for specs

---------

Co-authored-by: Jeremy Friesen <[email protected]>

.rubocop.yml

@@ -1,7 +1,9 @@
 # Bixby coding conventions
+require: rubocop-factory_bot
+
 # Added AllCops config for further modification
 AllCops:
-  TargetRubyVersion: 2.6
+  TargetRubyVersion: 2.7
   DisabledByDefault: true
   DisplayCopNames: true
   Exclude:

Gemfile

@@ -34,4 +34,5 @@ end
 
 group :lint do
   gem 'bixby'
+  gem 'rubocop-factory_bot', require: false
 end

app/controllers/bulkrax/importers_controller.rb

@@ -241,7 +241,7 @@ def importer_params
     end
 
     def list_external_sets
-      url = params[:base_url] || (@harvester ? @harvester.base_url : nil)
+      url = params[:base_url] || @harvester&.base_url
       setup_client(url) if url.present?
 
       @sets = [['All', 'all']]

app/factories/bulkrax/object_factory.rb

@@ -28,15 +28,16 @@ class ObjectFactory # rubocop:disable Metrics/ClassLength
     class_attribute :transformation_removes_blank_hash_values, default: false
 
     define_model_callbacks :save, :create
-    attr_reader :attributes, :object, :source_identifier_value, :klass, :replace_files, :update_files, :work_identifier, :related_parents_parsed_mapping, :importer_run_id
+    attr_reader :attributes, :object, :source_identifier_value, :klass, :replace_files, :update_files, :work_identifier, :work_identifier_search_field, :related_parents_parsed_mapping, :importer_run_id
 
     # rubocop:disable Metrics/ParameterLists
-    def initialize(attributes:, source_identifier_value:, work_identifier:, related_parents_parsed_mapping: nil, replace_files: false, user: nil, klass: nil, importer_run_id: nil, update_files: false)
+    def initialize(attributes:, source_identifier_value:, work_identifier:, work_identifier_search_field:, related_parents_parsed_mapping: nil, replace_files: false, user: nil, klass: nil, importer_run_id: nil, update_files: false)
       @attributes = ActiveSupport::HashWithIndifferentAccess.new(attributes)
       @replace_files = replace_files
       @update_files = update_files
       @user = user || User.batch_user
       @work_identifier = work_identifier
+      @work_identifier_search_field = work_identifier_search_field
       @related_parents_parsed_mapping = related_parents_parsed_mapping
       @source_identifier_value = source_identifier_value
       @klass = klass || Bulkrax.default_work_type.constantize
@@ -103,12 +104,7 @@ def find_or_create
     end
 
     def search_by_identifier
-      # TODO(alishaevn): return the proper `work_index` value below
-      # ref: https://github.com/samvera-labs/bulkrax/issues/866
-      # ref:https://github.com/samvera-labs/bulkrax/issues/867
-      # work_index = ::ActiveFedora.index_field_mapper.solr_name(work_identifier, :facetable)
-      work_index = work_identifier
-      query = { work_index =>
+      query = { work_identifier_search_field =>
                 source_identifier_value }
       # Query can return partial matches (something6 matches both something6 and something68)
       # so we need to weed out any that are not the correct full match. But other items might be

app/models/concerns/bulkrax/import_behavior.rb

@@ -179,6 +179,7 @@ def factory
       @factory ||= of.new(attributes: self.parsed_metadata,
                           source_identifier_value: identifier,
                           work_identifier: parser.work_identifier,
+                          work_identifier_search_field: parser.work_identifier_search_field,
                           related_parents_parsed_mapping: parser.related_parents_parsed_mapping,
                           replace_files: replace_files,
                           user: user,

app/parsers/bulkrax/application_parser.rb

@@ -81,6 +81,13 @@ def work_identifier
       @work_identifier ||= get_field_mapping_hash_for('source_identifier')&.keys&.first&.to_sym || :source
     end
 
+    # @return [Symbol] the solr property of the source_identifier. Used for searching.
+    #         defaults to work_identifier value + "_sim"
+    # @see #work_identifier
+    def work_identifier_search_field
+      @work_identifier_search_field ||= get_field_mapping_hash_for('source_identifier')&.values&.first&.[]('search_field')&.first&.to_s || "#{work_identifier}_sim"
+    end
+
     # @return [String]
     def generated_metadata_mapping
       @generated_metadata_mapping ||= 'generated'

lib/generators/bulkrax/templates/config/initializers/bulkrax.rb

@@ -47,7 +47,7 @@
   #   (For more info on importing relationships, see Bulkrax Wiki: https://github.com/samvera-labs/bulkrax/wiki/Configuring-Bulkrax#parent-child-relationship-field-mappings)
   #
   # #   e.g. to add the required source_identifier field
-  #   #   config.field_mappings["Bulkrax::CsvParser"]["source_id"] = { from: ["old_source_id"], source_identifier: true  }
+  #   #   config.field_mappings["Bulkrax::CsvParser"]["source_id"] = { from: ["old_source_id"], source_identifier: true, search_field: 'source_id_sim' }
   # If you want Bulkrax to fill in source_identifiers for you, see below
 
   # To duplicate a set of mappings from one parser to another

lib/tasks/bulkrax_tasks.rake

@@ -124,7 +124,7 @@ namespace :bulkrax do
   end
 
   def make_new_exports
-    Bulkrax::Exporter.all.each { |e| Bulkrax::ExporterJob.perform_later(e.id) }
+    Bulkrax::Exporter.find_each { |e| Bulkrax::ExporterJob.perform_later(e.id) }
   rescue => e
     puts "(#{e.message})"
   end

spec/factories/bulkrax/object_factory_spec.rb

@@ -1,29 +1,4 @@
 # frozen_string_literal: true
 
-require 'rails_helper'
-
-RSpec.describe Bulkrax::ObjectFactory do
-  # Why aren't there more tests?  In part because so much of the ObjectFactory require that we boot
-  # up Fedora and SOLR; something that remains non-desirous due to speed.
-  describe "#transform_attributes" do
-    context 'default behavior' do
-      it "does not empty arrays that only have empty values" do
-        attributes = { empty_array: ["", ""], empty_string: "", filled_array: ["A", "B"], filled_string: "A" }
-        factory = described_class.new(attributes: attributes, source_identifier_value: 123, work_identifier: "filled_string")
-        factory.base_permitted_attributes = %i[empty_array empty_string filled_array filled_string]
-        expect(factory.send(:transform_attributes)).to eq(attributes.stringify_keys)
-      end
-    end
-
-    context 'when :transformation_removes_blank_hash_values = true' do
-      it "empties arrays that only have empty values" do
-        attributes = { empty_array: ["", ""], empty_string: "", filled_array: ["A", "B"], filled_string: "A" }
-        factory = described_class.new(attributes: attributes, source_identifier_value: 123, work_identifier: "filled_string")
-        factory.base_permitted_attributes = %i[empty_array empty_string filled_array filled_string]
-        factory.transformation_removes_blank_hash_values = true
-        expect(factory.send(:transform_attributes))
-          .to eq({ empty_array: [], empty_string: nil, filled_array: ["A", "B"], filled_string: "A" }.stringify_keys)
-      end
-    end
-  end
-end
+# NOTE: do not put any specs here. FactoryBot autoloads every ruby file in spec/factories so even through it better
+# mirrors the app/ path, we put the ObjectFactory specs in spec/models/bulkrax/object_factory_spec.rb

spec/factories/bulkrax_object_factories.rb

@@ -6,7 +6,8 @@
       new(
         attributes: {},
         source_identifier_value: :source_identifier,
-        work_identifier: :source
+        work_identifier: :source,
+        work_identifier_search_field: 'source_sim'
       )
     end
   end

spec/models/bulkrax/object_factory_spec.rb

@@ -5,11 +5,42 @@
 module Bulkrax
   # NOTE: Unable to put this file in spec/factories/bulkrax (where it would mirror the path in app/) because
   # (presumably) FactoryBot autoloads all files in spec/factories, which would always run this spec.
+  # Why aren't there more tests?  In part because so much of the ObjectFactory require that we boot
+  # up Fedora and SOLR; something that remains non-desirous due to speed.
+
   RSpec.describe ObjectFactory do
     subject(:object_factory) { build(:object_factory) }
 
     describe 'is capable of looking up records dynamically' do
       include_examples 'dynamic record lookup'
     end
+
+    describe "#transform_attributes" do
+      context 'default behavior' do
+        it "does not empty arrays that only have empty values" do
+          attributes = { empty_array: ["", ""], empty_string: "", filled_array: ["A", "B"], filled_string: "A" }
+          factory = described_class.new(attributes: attributes,
+                                        source_identifier_value: 123,
+                                        work_identifier: "filled_string",
+                                        work_identifier_search_field: 'filled_string_sim')
+          factory.base_permitted_attributes = %i[empty_array empty_string filled_array filled_string]
+          expect(factory.send(:transform_attributes)).to eq(attributes.stringify_keys)
+        end
+      end
+
+      context 'when :transformation_removes_blank_hash_values = true' do
+        it "empties arrays that only have empty values" do
+          attributes = { empty_array: ["", ""], empty_string: "", filled_array: ["A", "B"], filled_string: "A" }
+          factory = described_class.new(attributes: attributes,
+                                        source_identifier_value: 123,
+                                        work_identifier: "filled_string",
+                                        work_identifier_search_field: 'filled_string_sim')
+          factory.base_permitted_attributes = %i[empty_array empty_string filled_array filled_string]
+          factory.transformation_removes_blank_hash_values = true
+          expect(factory.send(:transform_attributes))
+            .to eq({ empty_array: [], empty_string: nil, filled_array: ["A", "B"], filled_string: "A" }.stringify_keys)
+        end
+      end
+    end
   end
 end

spec/rails_helper.rb

@@ -69,7 +69,7 @@
   end
 
   # Remove this line if you're not using ActiveRecord or ActiveRecord fixtures
-  config.fixture_path = "#{::Rails.root}/spec/fixtures"
+  config.fixture_path = Rails.root.join('spec', 'fixtures')
 
   # If you're not using ActiveRecord, or you'd prefer not to run each of your
   # examples within a transaction, remove the following line or assign false