From b45b1ff775d8f2d5cf0e57557a9544d6ef4adf99 Mon Sep 17 00:00:00 2001 From: Daniel Pierce Date: Thu, 28 Sep 2023 10:21:03 -0400 Subject: [PATCH] Fix based_near/Location for resource models Access location service via Hyrax config to allow easier mock responses in specs Add form (pre)populator to handle the data sent/received from the controlled vocabulary input Auto-include any properly named form field behaviors --- .../basic_metadata_form_fields_behavior.rb | 38 +++++++++++++++++++ app/indexers/hyrax/location_indexer.rb | 29 ++++++++++++++ app/indexers/hyrax/valkyrie_work_indexer.rb | 1 + app/inputs/controlled_vocabulary_input.rb | 2 +- lib/hyrax/configuration.rb | 5 +++ lib/hyrax/controlled_vocabularies/location.rb | 8 +++- lib/hyrax/form_fields.rb | 6 +++ lib/hyrax/specs/shared_specs/indexers.rb | 30 ++++++++++++--- .../forms/resource_batch_edit_form_spec.rb | 4 ++ spec/forms/hyrax/forms/resource_form_spec.rb | 28 ++++++++++++++ 10 files changed, 143 insertions(+), 8 deletions(-) create mode 100644 app/forms/concerns/hyrax/basic_metadata_form_fields_behavior.rb create mode 100644 app/indexers/hyrax/location_indexer.rb diff --git a/app/forms/concerns/hyrax/basic_metadata_form_fields_behavior.rb b/app/forms/concerns/hyrax/basic_metadata_form_fields_behavior.rb new file mode 100644 index 0000000000..a2f13f9ea0 --- /dev/null +++ b/app/forms/concerns/hyrax/basic_metadata_form_fields_behavior.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true +module Hyrax + module BasicMetadataFormFieldsBehavior + # Provides compatibility with the behavior of the based_near (location) controlled vocabulary form field. + # The form expects a ControlledVocabularies::Location object as input and produces a hash like those + # used with accepts_nested_attributes_for. + def self.included(descendant) + descendant.property :based_near_attributes, virtual: true, populator: :based_near_populator, prepopulator: :based_near_prepopulator + end + + private + + def based_near_populator(fragment:, **_options) + adds = [] + deletes = [] + fragment.each do |_, h| + uri = RDF::URI.parse(h["id"]).to_s + if h["_destroy"] == "true" + deletes << uri + else + adds << uri + end + end + self.based_near = ((model.based_near + adds) - deletes).uniq + end + + def based_near_prepopulator + self.based_near = based_near.map do |loc| + uri = RDF::URI.parse(loc) + if uri + Hyrax::ControlledVocabularies::Location.new(uri) + else + loc + end + end + end + end +end diff --git a/app/indexers/hyrax/location_indexer.rb b/app/indexers/hyrax/location_indexer.rb new file mode 100644 index 0000000000..94508508ab --- /dev/null +++ b/app/indexers/hyrax/location_indexer.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module Hyrax + ## + # Indexes properties common to Hyrax::Resource types + module LocationIndexer + def to_solr + super.tap do |index_document| + index_document[:based_near_label_tesim] = based_near_label_lookup(resource.based_near) if resource.respond_to? :based_near + end + end + + private + + def based_near_label_lookup(locations) + locations.map do |loc| + if URI.parse(loc) + location_service.full_label(loc) + else + loc + end + end + end + + def location_service + Hyrax.config.location_service + end + end +end diff --git a/app/indexers/hyrax/valkyrie_work_indexer.rb b/app/indexers/hyrax/valkyrie_work_indexer.rb index ef04954c3c..480326c959 100644 --- a/app/indexers/hyrax/valkyrie_work_indexer.rb +++ b/app/indexers/hyrax/valkyrie_work_indexer.rb @@ -7,6 +7,7 @@ class ValkyrieWorkIndexer < Hyrax::ValkyrieIndexer include Hyrax::ResourceIndexer include Hyrax::PermissionIndexer include Hyrax::VisibilityIndexer + include Hyrax::LocationIndexer include Hyrax::ThumbnailIndexer include Hyrax::Indexer(:core_metadata) diff --git a/app/inputs/controlled_vocabulary_input.rb b/app/inputs/controlled_vocabulary_input.rb index 8d9956d5fd..362854bed5 100644 --- a/app/inputs/controlled_vocabulary_input.rb +++ b/app/inputs/controlled_vocabulary_input.rb @@ -44,7 +44,7 @@ def destroy_widget(attribute_name, index) def hidden_id_field(value, index) name = name_for(attribute_name, index, 'id') id = id_for(attribute_name, index, 'id') - hidden_value = value.node? ? '' : value.rdf_subject + hidden_value = value.try(:node?) ? '' : value.rdf_subject @builder.hidden_field(attribute_name, name: name, id: id, value: hidden_value, data: { id: 'remote' }) end diff --git a/lib/hyrax/configuration.rb b/lib/hyrax/configuration.rb index f5140d3091..0087c92303 100644 --- a/lib/hyrax/configuration.rb +++ b/lib/hyrax/configuration.rb @@ -778,6 +778,11 @@ def geonames_username=(username) Qa::Authorities::Geonames.username = username end + def location_service + @location_service ||= Hyrax::LocationService.new + end + attr_writer :location_service + attr_writer :active_deposit_agreement_acceptance def active_deposit_agreement_acceptance? return true if @active_deposit_agreement_acceptance.nil? diff --git a/lib/hyrax/controlled_vocabularies/location.rb b/lib/hyrax/controlled_vocabularies/location.rb index d33cb4edeb..c4d21c9462 100644 --- a/lib/hyrax/controlled_vocabularies/location.rb +++ b/lib/hyrax/controlled_vocabularies/location.rb @@ -14,7 +14,13 @@ def solrize end def full_label - Hyrax::LocationService.new.full_label(rdf_subject.to_s) + location_service.full_label(rdf_subject.to_s) + end + + private + + def location_service + Hyrax.config.location_service end end end diff --git a/lib/hyrax/form_fields.rb b/lib/hyrax/form_fields.rb index 1b31af4260..a3b2f28208 100644 --- a/lib/hyrax/form_fields.rb +++ b/lib/hyrax/form_fields.rb @@ -51,6 +51,12 @@ def included(descendant) descendant.property field_name.to_sym, options.merge(display: true, default: []) descendant.validates field_name.to_sym, presence: true if options.fetch(:required, false) end + + # Auto include any matching FormFieldBehaviors + schema_name = name.to_s.camelcase + behavior = "#{schema_name}FormFieldsBehavior".safe_constantize || + "Hyrax::#{schema_name}FormFieldsBehavior".safe_constantize + descendant.include(behavior) if behavior end end end diff --git a/lib/hyrax/specs/shared_specs/indexers.rb b/lib/hyrax/specs/shared_specs/indexers.rb index 592d928516..d8d6c736bf 100644 --- a/lib/hyrax/specs/shared_specs/indexers.rb +++ b/lib/hyrax/specs/shared_specs/indexers.rb @@ -89,8 +89,6 @@ before do raise 'indexer_class must be set with `let(:indexer_class)`' unless defined? indexer_class raise 'resource must be set with `let(:resource)` and is expected to be a kind of Hyrax::Resource' unless defined?(resource) && resource.kind_of?(Hyrax::Resource) - # optionally can pass in default_visibility by setting it with a let statement if your application changes the default; Hyrax defines this as 'restricted' - # See samvera/hyrda-head hydra-access-controls/app/models/concerns/hydra/access_controls/access_rights.rb for possible VISIBILITY_TEXT_VALUE_...' end subject(:indexer) { indexer_class.new(resource: resource) } let(:thumbnail_path) { '/downloads/foo12345?file=thumbnail' } @@ -106,6 +104,27 @@ end end +RSpec.shared_examples 'a location indexer' do + before do + raise 'indexer_class must be set with `let(:indexer_class)`' unless defined? indexer_class + raise 'resource must be set with `let(:resource)` and is expected to be a kind of Hyrax::Resource' unless defined?(resource) && resource.kind_of?(Hyrax::Resource) + resource.based_near = based_near + end + subject(:indexer) { indexer_class.new(resource: resource) } + let(:based_near) { ["https://sws.geonames.org/4254679/"] } + let(:based_near_label) { ["Bloomington, Indiana, United States"] } + + before do + allow(Hyrax.config.location_service).to receive(:full_label).with(based_near[0]).and_return(based_near_label[0]) + end + + describe '#to_solr' do + it 'indexes locations' do + expect(indexer.to_solr).to include(based_near_tesim: based_near, based_near_label_tesim: based_near_label) + end + end +end + RSpec.shared_examples 'a Core metadata indexer' do before do raise 'indexer_class must be set with `let(:indexer_class)`' unless defined? indexer_class @@ -138,9 +157,10 @@ end subject(:indexer) { indexer_class.new(resource: resource) } + it_behaves_like 'a location indexer' + let(:attributes) do { - based_near: ['helsinki', 'finland'], date_created: ['tuesday'], keyword: ['comic strip'], related_url: ['http://example.com/moomin'], @@ -152,9 +172,7 @@ describe '#to_solr' do it 'indexes basic metadata' do expect(indexer.to_solr) - .to include(based_near_tesim: a_collection_containing_exactly(*attributes[:based_near]), - based_near_sim: a_collection_containing_exactly(*attributes[:based_near]), - date_created_tesim: a_collection_containing_exactly(*attributes[:date_created]), + .to include(date_created_tesim: a_collection_containing_exactly(*attributes[:date_created]), keyword_sim: a_collection_containing_exactly(*attributes[:keyword]), related_url_tesim: a_collection_containing_exactly(*attributes[:related_url]), resource_type_tesim: a_collection_containing_exactly(*attributes[:resource_type]), diff --git a/spec/forms/hyrax/forms/resource_batch_edit_form_spec.rb b/spec/forms/hyrax/forms/resource_batch_edit_form_spec.rb index 2fad0f7b61..a3bce11c88 100644 --- a/spec/forms/hyrax/forms/resource_batch_edit_form_spec.rb +++ b/spec/forms/hyrax/forms/resource_batch_edit_form_spec.rb @@ -38,6 +38,10 @@ # let(:ability) { Ability.new(user) } let(:user) { build(:user, display_name: 'Jill Z. User') } + before do + allow(Hyrax.config.location_service).to receive(:full_label) # Avoid external request to geonames + end + describe "#terms" do subject { form.terms } diff --git a/spec/forms/hyrax/forms/resource_form_spec.rb b/spec/forms/hyrax/forms/resource_form_spec.rb index 3d58f46ec2..29bf661038 100644 --- a/spec/forms/hyrax/forms/resource_form_spec.rb +++ b/spec/forms/hyrax/forms/resource_form_spec.rb @@ -86,6 +86,34 @@ end end + describe '#based_near' do + subject(:form) { form_class.new(work) } + + let(:work) { build(:monograph) } + let(:geonames_uri) { "https://sws.geonames.org/4254679/" } + + let(:form_class) do + Class.new(Hyrax::Forms::ResourceForm(work.class)) do + include Hyrax::FormFields(:basic_metadata) + end + end + + it 'runs the based_near prepopulator' do + work.based_near = [geonames_uri] + form.prepopulate! + expect(form.based_near) + .to contain_exactly(an_instance_of(Hyrax::ControlledVocabularies::Location)) + end + + it 'runs the based_near populator' do + form.validate(based_near_attributes: { "0" => { "hidden_label" => geonames_uri, + "id" => geonames_uri, + "_destroy" => "" } }) + expect(form.based_near) + .to contain_exactly(geonames_uri) + end + end + describe '#embargo_release_date' do context 'without an embargo' do it 'is nil' do