Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make search string used to look up objects configurable #884

Merged
merged 9 commits into from
Nov 29, 2023
4 changes: 3 additions & 1 deletion .rubocop.yml
Original file line number Diff line number Diff line change
@@ -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:
Expand Down
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,5 @@ end

group :lint do
gem 'bixby'
gem 'rubocop-factory_bot', require: false
end
2 changes: 1 addition & 1 deletion app/controllers/bulkrax/importers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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']]
Expand Down
12 changes: 4 additions & 8 deletions app/factories/bulkrax/object_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions app/models/concerns/bulkrax/import_behavior.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 7 additions & 0 deletions app/parsers/bulkrax/application_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/tasks/bulkrax_tasks.rake
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

rescue => e
puts "(#{e.message})"
end
Expand Down
29 changes: 2 additions & 27 deletions spec/factories/bulkrax/object_factory_spec.rb
Original file line number Diff line number Diff line change
@@ -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
3 changes: 2 additions & 1 deletion spec/factories/bulkrax_object_factories.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 31 additions & 0 deletions spec/models/bulkrax/object_factory_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion spec/rails_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading