From d3948610280503a36dd83318c38c098c17f41556 Mon Sep 17 00:00:00 2001 From: Rob Kaufman Date: Thu, 9 Nov 2023 15:16:19 -0800 Subject: [PATCH] make search string used to look up objects configurable --- .rubocop.yml | 4 +++- app/controllers/bulkrax/importers_controller.rb | 2 +- app/factories/bulkrax/object_factory.rb | 12 ++++-------- app/models/concerns/bulkrax/import_behavior.rb | 1 + app/parsers/bulkrax/application_parser.rb | 7 +++++++ spec/factories/bulkrax/object_factory_spec.rb | 10 ++++++++-- spec/factories/bulkrax_object_factories.rb | 3 ++- 7 files changed, 26 insertions(+), 13 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index f08dee44a..7c702ccbf 100644 --- a/.rubocop.yml +++ b/.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: diff --git a/app/controllers/bulkrax/importers_controller.rb b/app/controllers/bulkrax/importers_controller.rb index cb762e0a6..c44fa60de 100644 --- a/app/controllers/bulkrax/importers_controller.rb +++ b/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']] diff --git a/app/factories/bulkrax/object_factory.rb b/app/factories/bulkrax/object_factory.rb index ca0ba791a..3fcf86418 100644 --- a/app/factories/bulkrax/object_factory.rb +++ b/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_string = 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 diff --git a/app/models/concerns/bulkrax/import_behavior.rb b/app/models/concerns/bulkrax/import_behavior.rb index c095ab568..eea56afac 100644 --- a/app/models/concerns/bulkrax/import_behavior.rb +++ b/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, diff --git a/app/parsers/bulkrax/application_parser.rb b/app/parsers/bulkrax/application_parser.rb index 9c5a1280f..3cc390ea7 100644 --- a/app/parsers/bulkrax/application_parser.rb +++ b/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' diff --git a/spec/factories/bulkrax/object_factory_spec.rb b/spec/factories/bulkrax/object_factory_spec.rb index ce807ac73..d12f98879 100644 --- a/spec/factories/bulkrax/object_factory_spec.rb +++ b/spec/factories/bulkrax/object_factory_spec.rb @@ -9,7 +9,10 @@ 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 = 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 @@ -18,7 +21,10 @@ 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 = described_class.new(attributes: attributes, + source_identifier_value: 123, + work_identifier: "filled_string", + work_identifier_search_string: '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)) diff --git a/spec/factories/bulkrax_object_factories.rb b/spec/factories/bulkrax_object_factories.rb index 0e49a0e42..59e116351 100644 --- a/spec/factories/bulkrax_object_factories.rb +++ b/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