From f8521e444346742bb3660526f3f29940e26b2c98 Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Thu, 4 Jun 2026 15:25:22 -0400 Subject: [PATCH 01/32] Add compound metadata schema reader and flag Hyrax can read compound (hierarchical) metadata declarations off a resource's schema, returning each compound's sub-fields, groups, and display mode in both flexible and non-flexible modes. This is the read-only foundation the compound form, indexer, and show renderer build on; the compound_metadata_enabled? config flag (default on) gates whether the rest of the feature activates. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/services/hyrax/compound_schema.rb | 187 ++++++++++++++++++++ lib/hyrax/configuration.rb | 11 ++ spec/services/hyrax/compound_schema_spec.rb | 164 +++++++++++++++++ 3 files changed, 362 insertions(+) create mode 100644 app/services/hyrax/compound_schema.rb create mode 100644 spec/services/hyrax/compound_schema_spec.rb diff --git a/app/services/hyrax/compound_schema.rb b/app/services/hyrax/compound_schema.rb new file mode 100644 index 0000000000..ec04b1bc5c --- /dev/null +++ b/app/services/hyrax/compound_schema.rb @@ -0,0 +1,187 @@ +# frozen_string_literal: true + +module Hyrax + ## + # @api public + # + # Reads the compound metadata declarations off a resource's schema. A compound + # is a `type: hash, multiple: true` attribute carrying a `subfields:` map; the + # declaration drives the generic form, indexer, and renderer so a hierarchical + # field can be defined in YAML alone. See documentation/forms/compound_fields.md. + # + # Declarations are read from each attribute's Dry type `meta`, which both + # schema loaders populate identically, so the result is the same in both flex + # modes. + class CompoundSchema + ## + # Build a CompoundSchema for a resource instance or class. + # + # @param [Valkyrie::Resource, Class] resource a resource instance, or a + # class (used by class-level callers that only see class-declared + # compounds — i.e. non-flex). + def self.for(resource) + new(*schema_sources_for(resource)) + end + + # For a class, its own schema. For an instance, both the class schema + # (non-flexible) and the singleton schema (flexible: the m3 attributes are + # applied to the singleton at load time). Unioning both makes {.for} work in + # both flex modes. + def self.schema_sources_for(resource) + if resource.is_a?(Class) + [(resource.schema if resource.respond_to?(:schema))] + else + sources = [] + sources << resource.class.schema if resource.class.respond_to?(:schema) + sources << resource.singleton_class.schema if resource.respond_to?(:singleton_class) && resource.singleton_class.respond_to?(:schema) + sources + end.compact + end + private_class_method :schema_sources_for + + attr_reader :schema_sources + + # @param [Array<#each>] schema_sources one or more Dry schemas (each a + # collection of property types responding to `name` and `meta`). + def initialize(*schema_sources) + @schema_sources = schema_sources.flatten.compact + end + + ## + # @return [Boolean] whether the given attribute is a compound (declares + # `subfields:`) + def compound?(attribute_name) + definitions.key?(attribute_name.to_sym) + end + + ## + # @return [Array] the names of every compound attribute on the + # resource + def compound_names + definitions.keys + end + + ## + # @return [Array] compounds displayed inline in the metadata list + # (the default) + def inline_compound_names + definitions.reject { |_name, d| d[:display_mode] == :card }.keys + end + + ## + # @return [Array] compounds displayed as their own card on show + # pages (`view: { display: card }`) + def card_compound_names + definitions.select { |_name, d| d[:display_mode] == :card }.keys + end + + ## + # @return [Boolean] whether the compound displays as a card + def card?(attribute_name) + definition_for(attribute_name)&.dig(:display_mode) == :card + end + + ## + # @param [#to_sym] attribute_name + # + # @return [Hash{Symbol => Object}, nil] the compound's declaration: + # `{ subfields:, groups:, index_subfields: }`, or nil when the attribute + # is not a compound. + def definition_for(attribute_name) + definitions[attribute_name.to_sym] + end + + ## + # @param [#to_sym] attribute_name + # + # @return [Array] the ordered sub-field keys declared for the + # compound (empty when not a compound). + def subfield_keys(attribute_name) + definition = definition_for(attribute_name) + return [] unless definition + definition[:subfields].keys + end + + ## + # @return [Hash{Symbol => Hash}] a map from compound attribute name to its + # normalized declaration. Memoized per instance. + def definitions + @definitions ||= build_definitions + end + + private + + def build_definitions + schema_sources.each_with_object({}) do |schema, memo| + next unless schema.respond_to?(:each) + + schema.each do |property| + meta = property.respond_to?(:meta) ? property.meta : nil + next if meta.nil? + + name = property.name.to_sym + next if memo.key?(name) + + config = meta.with_indifferent_access + subfields = config['subfields'] + next if subfields.blank? + + memo[name] = normalize(config, subfields) + end + end + end + + # Normalizes the raw declaration into the symbol-keyed shape the form, + # indexer, and renderer consume. See documentation/forms/compound_fields.md + # for the meaning of each sub-field key. + def normalize(config, subfields) + sub = subfields.each_with_object({}) do |(key, opts), memo| + opts = (opts.is_a?(Hash) ? opts : {}).with_indifferent_access + memo[key.to_s] = { type: (opts['type'] || 'string').to_s, + authority: opts['authority']&.to_s, + values: normalize_values(opts['values']), + index_keys: normalize_index_keys(opts), + display: opts.fetch('display', true) != false } + end + + view = config['view'] + display_mode = view.is_a?(Hash) && view['display'].to_s == 'card' ? :card : :inline + + { subfields: sub, groups: normalize_groups(config['groups'], sub.keys), display_mode: display_mode } + end + + # Reads `index_keys:` (non-flexible) or `indexing:` (flexible), filtering the + # non-field control tokens, mirroring AttributeDefinition#index_keys. + INDEX_CONTROL_TOKENS = %w[facetable stored_searchable admin_only editor_only].freeze + def normalize_index_keys(opts) + raw = opts['indexing'] || opts['index_keys'] || [] + Array(raw).reject { |k| INDEX_CONTROL_TOKENS.include?(k.to_s) }.map(&:to_s) + end + + # Normalizes an inline option list into `[[label, id], ...]`; nil when none. + def normalize_values(values) + return nil if values.blank? + + Array(values).map do |entry| + if entry.is_a?(Hash) + h = entry.with_indifferent_access + id = (h['id'] || h['value'] || h['label']).to_s + [(h['label'] || id).to_s, id] + else + [entry.to_s, entry.to_s] + end + end + end + + def normalize_groups(groups, all_keys) + return [{ label: nil, cols: 6, fields: all_keys }] if groups.blank? + + Array(groups).map do |group| + group = group.with_indifferent_access + { label: group['label'], + cols: (group['cols'] || 6).to_i, + fields: Array(group['fields']).map(&:to_s) } + end + end + end +end diff --git a/lib/hyrax/configuration.rb b/lib/hyrax/configuration.rb index 510edafd47..c6e74dd23d 100644 --- a/lib/hyrax/configuration.rb +++ b/lib/hyrax/configuration.rb @@ -350,6 +350,17 @@ def redirects_enabled? end alias redirects_enabled redirects_enabled? + # Whether the sample compound metadata schema ships on works and collections + # by default. Defaults to true; respects +HYRAX_COMPOUND_METADATA_ENABLED+. + # + # @see documentation/forms/compound_fields.md + attr_writer :compound_metadata_enabled + def compound_metadata_enabled? + return @compound_metadata_enabled if defined?(@compound_metadata_enabled) && !@compound_metadata_enabled.nil? + @compound_metadata_enabled = ActiveModel::Type::Boolean.new.cast(ENV.fetch('HYRAX_COMPOUND_METADATA_ENABLED', true)) + end + alias compound_metadata_enabled compound_metadata_enabled? + # @return [Boolean] true when both feature gates are open. Single # source of truth for "is the redirects feature actively in use # right now?". Short-circuits on the config so the Flipflop call diff --git a/spec/services/hyrax/compound_schema_spec.rb b/spec/services/hyrax/compound_schema_spec.rb new file mode 100644 index 0000000000..5cf9e9052f --- /dev/null +++ b/spec/services/hyrax/compound_schema_spec.rb @@ -0,0 +1,164 @@ +# frozen_string_literal: true + +RSpec.describe Hyrax::CompoundSchema do + let(:resource_class) do + Class.new(Hyrax::Resource) do + def self.name + 'TestResourceWithCompounds' + end + + attribute :title, Valkyrie::Types::Array.of(Valkyrie::Types::String) + attribute :contributors, + Valkyrie::Types::Array.of(Dry::Types['hash']).meta( + subfields: { + 'given_name' => { 'type' => 'string', 'index_keys' => %w[contributors_given_name_sim contributors_given_name_tesim] }, + 'family_name' => { 'type' => 'string', 'index_keys' => %w[contributors_family_name_tesim] }, + 'role_label' => { 'type' => 'controlled', 'authority' => 'contributor_role', 'display' => false } + }, + groups: [ + { 'label' => 'Identity', 'cols' => 6, 'fields' => %w[given_name family_name] }, + { 'label' => 'Role', 'cols' => 6, 'fields' => %w[role_label] } + ] + ) + attribute :identifiers, + Valkyrie::Types::Array.of(Dry::Types['hash']).meta( + subfields: { + 'value' => { 'type' => 'string' }, + 'identifier_type' => { 'type' => 'controlled', 'authority' => 'identifier_type' } + } + ) + attribute :agent, + Valkyrie::Types::Array.of(Dry::Types['hash']).meta( + subfields: { + 'agent_name' => { 'type' => 'string' }, + 'agent_role' => { 'type' => 'controlled', 'values' => ['Author', { 'id' => 'ed', 'label' => 'Editor' }] } + } + ) + attribute :relationships, + Valkyrie::Types::Array.of(Dry::Types['hash']).meta( + subfields: { + 'related_item' => { 'type' => 'work_or_url' }, + 'relationship_type' => { 'type' => 'controlled', 'authority' => 'relationship_type' } + }, + view: { 'display' => 'card' } + ) + end + end + + subject(:schema) { described_class.for(resource_class) } + + describe '#compound_names' do + it 'returns only the attributes that declare subfields' do + expect(schema.compound_names).to contain_exactly(:contributors, :identifiers, :agent, :relationships) + end + + it 'does not treat a plain scalar attribute as a compound' do + expect(schema.compound_names).not_to include(:title) + end + end + + describe 'display mode (view: { display: card })' do + describe '#inline_compound_names' do + it 'excludes compounds declared as cards' do + expect(schema.inline_compound_names).to contain_exactly(:contributors, :identifiers, :agent) + end + end + + describe '#card_compound_names' do + it 'returns only compounds declared as cards' do + expect(schema.card_compound_names).to contain_exactly(:relationships) + end + end + + describe '#card?' do + it 'is true for a compound declared view: { display: card }' do + expect(schema.card?(:relationships)).to be true + expect(schema.card?('relationships')).to be true + end + + it 'is false for an inline (default) compound' do + expect(schema.card?(:contributors)).to be false + end + + it 'is false for a non-compound attribute' do + expect(schema.card?(:title)).to be false + end + end + + it 'records the display_mode on the definition' do + expect(schema.definition_for(:relationships)[:display_mode]).to eq(:card) + expect(schema.definition_for(:contributors)[:display_mode]).to eq(:inline) + end + end + + describe '#compound?' do + it 'is true for a declared compound' do + expect(schema.compound?(:contributors)).to be true + expect(schema.compound?('contributors')).to be true + end + + it 'is false for a non-compound attribute' do + expect(schema.compound?(:title)).to be false + end + end + + describe '#subfield_keys' do + it 'returns the ordered sub-field keys' do + expect(schema.subfield_keys(:contributors)).to eq(%w[given_name family_name role_label]) + end + + it 'is empty for a non-compound' do + expect(schema.subfield_keys(:title)).to eq([]) + end + end + + describe '#definition_for' do + it 'normalizes subfields with type, authority, index_keys and display' do + definition = schema.definition_for(:contributors) + expect(definition[:subfields]['role_label']) + .to eq(type: 'controlled', authority: 'contributor_role', values: nil, index_keys: [], display: false) + expect(definition[:subfields]['given_name']) + .to eq(type: 'string', authority: nil, values: nil, + index_keys: %w[contributors_given_name_sim contributors_given_name_tesim], display: true) + end + + it 'reads per-sub-field index_keys (literal Solr field names)' do + expect(schema.definition_for(:contributors)[:subfields]['family_name'][:index_keys]) + .to eq(%w[contributors_family_name_tesim]) + end + + it 'defaults a sub-field with no index declaration to no index_keys' do + expect(schema.definition_for(:agent)[:subfields]['agent_name'][:index_keys]).to eq([]) + end + + it 'defaults display to true and honors display: false' do + expect(schema.definition_for(:contributors)[:subfields]['given_name'][:display]).to be true + expect(schema.definition_for(:contributors)[:subfields]['role_label'][:display]).to be false + end + + it 'normalizes an inline controlled-vocabulary values list to [label, id] pairs' do + values = schema.definition_for(:agent)[:subfields]['agent_role'][:values] + expect(values).to eq([%w[Author Author], %w[Editor ed]]) + end + + it 'carries the declared groups' do + groups = schema.definition_for(:contributors)[:groups] + expect(groups).to eq([ + { label: 'Identity', cols: 6, fields: %w[given_name family_name] }, + { label: 'Role', cols: 6, fields: %w[role_label] } + ]) + end + + it 'defaults groups to a single all-fields group when none declared' do + groups = schema.definition_for(:identifiers)[:groups] + expect(groups).to eq([{ label: nil, cols: 6, fields: %w[value identifier_type] }]) + end + end + + describe '.for' do + it 'builds from a resource instance' do + expect(described_class.for(resource_class.new).compound_names) + .to contain_exactly(:contributors, :identifiers, :agent, :relationships) + end + end +end From f8e1f04c23272555253ac9e73509359d029de46d Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Thu, 4 Jun 2026 15:30:58 -0400 Subject: [PATCH 02/32] Render compound metadata fields on forms Resource edit forms render a repeatable, schema-driven section per compound attribute, with add/remove-row controls and a generic populator that converts the nested-attributes payload to the persisted array-of-hashes shape. Each sub-field renders by its declared type (text, controlled select, url, or a work-or-url picker). Compounds are kept out of the primary/secondary scalar terms so they render only through the compound section. The wiring is a no-op until a schema declares a compound, in both flexible and non-flexible modes. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/assets/javascripts/hyrax.js | 1 + .../javascripts/hyrax/compound_metadata.js | 98 +++++++++++++ .../concerns/hyrax/compound_field_behavior.rb | 92 ++++++++++++ app/forms/hyrax/forms.rb | 7 + app/forms/hyrax/forms/pcdm_collection_form.rb | 6 + app/forms/hyrax/forms/resource_form.rb | 21 ++- app/helpers/hyrax/compound_fields_helper.rb | 134 ++++++++++++++++++ app/helpers/hyrax/hyrax_helper_behavior.rb | 1 + app/views/hyrax/base/_form_metadata.html.erb | 5 + .../hyrax/compounds/_compound_row.html.erb | 82 +++++++++++ .../compounds/_compound_section.html.erb | 59 ++++++++ .../dashboard/collections/_form.html.erb | 6 + config/locales/hyrax.en.yml | 4 + lib/hyrax/form_fields.rb | 23 +++ .../hyrax/compound_field_behavior_spec.rb | 110 ++++++++++++++ .../hyrax/compound_fields_helper_spec.rb | 134 ++++++++++++++++++ 16 files changed, 781 insertions(+), 2 deletions(-) create mode 100644 app/assets/javascripts/hyrax/compound_metadata.js create mode 100644 app/forms/concerns/hyrax/compound_field_behavior.rb create mode 100644 app/helpers/hyrax/compound_fields_helper.rb create mode 100644 app/views/hyrax/compounds/_compound_row.html.erb create mode 100644 app/views/hyrax/compounds/_compound_section.html.erb create mode 100644 spec/forms/concerns/hyrax/compound_field_behavior_spec.rb create mode 100644 spec/helpers/hyrax/compound_fields_helper_spec.rb diff --git a/app/assets/javascripts/hyrax.js b/app/assets/javascripts/hyrax.js index cb70a7135d..f76be1a0e6 100644 --- a/app/assets/javascripts/hyrax.js +++ b/app/assets/javascripts/hyrax.js @@ -46,6 +46,7 @@ //= require hyrax/single_use_links_manager //= require hyrax/copy_permalink_button //= require hyrax/redirects +//= require hyrax/compound_metadata //= require hyrax/dashboard_actions //= require hyrax/batch //= require hyrax/flot_stats diff --git a/app/assets/javascripts/hyrax/compound_metadata.js b/app/assets/javascripts/hyrax/compound_metadata.js new file mode 100644 index 0000000000..b5a7099de2 --- /dev/null +++ b/app/assets/javascripts/hyrax/compound_metadata.js @@ -0,0 +1,98 @@ +// Behavior for compound metadata fields on resource edit forms (add/remove row, +// work_or_url select2). Event-delegated so dynamically-added rows need no +// rebinding. The sentinel guards against the IIFE running twice (Turbolinks +// re-evaluates module scripts on some navigation paths), which would stack +// duplicate listeners. Mirrors hyrax/redirects.js. +(function() { + if (document.hyraxCompoundsBound) return; + document.hyraxCompoundsBound = true; + + // Bind select2 to a `work_or_url` sub-field. The v3 API (Hyrax bundles + // select2-rails 3.x) binds to a hidden input; createSearchChoice lets a + // typed external URL be selected as-is. + function bindWorkOrUrlInputs(root) { + if (typeof jQuery === 'undefined' || !jQuery.fn.select2) return; + jQuery(root).find('[data-hyrax-compound-work-input]').each(function() { + var $el = jQuery(this); + if ($el.hasClass('select2-offscreen') || $el.data('select2')) return; // already bound + $el.select2({ + width: '100%', + allowClear: true, + placeholder: 'Search for a work or enter a URL', + minimumInputLength: 2, + // Let a typed value (e.g. an external URL) be selected as-is. + createSearchChoice: function(term) { + return { id: term, text: term }; + }, + // Render the current value's label (work title or the URL) on load. + initSelection: function(element, callback) { + var val = element.val(); + if (!val) return; + callback({ id: val, text: element.data('label') || val }); + }, + ajax: { + url: $el.data('autocomplete-url'), + dataType: 'json', + quietMillis: 250, + data: function(term, page) { return { q: term }; }, + results: function(data, page) { + return { + results: data.map(function(obj) { + return { id: obj.id, text: [].concat(obj.label)[0] }; + }) + }; + } + } + }); + }); + } + + // Bind saved rows once the DOM is ready (at script-eval time the form + // inputs don't exist yet, so the select2 would never attach). Covers both a + // fresh load and Turbolinks navigation. + function bindAll() { bindWorkOrUrlInputs(document); } + if (document.readyState === 'loading') { + document.addEventListener('DOMContentLoaded', bindAll); + } else { + bindAll(); + } + document.addEventListener('turbolinks:load', bindAll); + + document.addEventListener('click', function(event) { + var removeButton = event.target.closest('[data-hyrax-compound-remove-row]'); + if (removeButton) { + var row = removeButton.closest('[data-hyrax-compound-row]'); + if (!row) return; + var destroyFlag = row.querySelector('[data-hyrax-compound-destroy-flag]'); + if (destroyFlag && destroyFlag.value !== undefined) { + // Persisted rows: flip the _destroy flag and hide so the + // populator drops the row server-side. + destroyFlag.value = '1'; + row.style.display = 'none'; + } else { + row.parentNode.removeChild(row); + } + return; + } + + var addButton = event.target.closest('[data-hyrax-compound-add-row]'); + if (!addButton) return; + var section = addButton.closest('[data-hyrax-compound]'); + if (!section) return; + var template = section.querySelector('[data-hyrax-compound-row-template]'); + var rowsHost = section.querySelector('[data-hyrax-compound-rows]'); + if (!template || !rowsHost) return; + + // Monotonic counter on the section; never recycle an index after a + // row is removed. Fallback to row count when the attribute is missing. + var nextIndex = parseInt(section.dataset.nextIndex, 10); + if (isNaN(nextIndex)) { + nextIndex = rowsHost.querySelectorAll('[data-hyrax-compound-row]').length; + } + var html = template.innerHTML.replace(/__INDEX__/g, nextIndex); + rowsHost.insertAdjacentHTML('beforeend', html); + // Bind select2 on any work_or_url inputs in the row just added. + bindWorkOrUrlInputs(rowsHost.lastElementChild || rowsHost); + section.dataset.nextIndex = String(nextIndex + 1); + }); +})(); diff --git a/app/forms/concerns/hyrax/compound_field_behavior.rb b/app/forms/concerns/hyrax/compound_field_behavior.rb new file mode 100644 index 0000000000..f5ca4bf3c0 --- /dev/null +++ b/app/forms/concerns/hyrax/compound_field_behavior.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true + +module Hyrax + ## + # Generic, schema-driven form handling for compound metadata fields (see + # {Hyrax::CompoundSchema}). The generic equivalent of the hand-written + # {Hyrax::RedirectsFieldBehavior}: it discovers every compound on the form's + # model and wires each one's virtual `_attributes` property and + # populator from the schema, in both flex modes. + module CompoundFieldBehavior + # Register the virtual `_attributes` populator properties on the + # singleton class for every compound on the resource. Must run before Reform + # builds the deserialization schema; the flexible-mode init path calls this, + # while non-flexible mode wires the same properties at class load via + # {Hyrax::FormFields}. + # + # @param [Valkyrie::Resource] resource the form's resource (the form's own + # `model` may not be set yet when this runs). + def register_compound_fields!(resource) + Hyrax::CompoundSchema.for(resource).compound_names.each do |name| + next if singleton_class.method_defined?(:"#{name}_attributes=") + + singleton_class.property :"#{name}_attributes", + virtual: true, + populator: :compound_attributes_populator + end + rescue StandardError => e + Hyrax.logger.debug("CompoundFieldBehavior: register failed for #{self.class}: #{e.message}") + end + + # Strip each compound's renamed bare key so the `_attributes` + # populator is the single write entry point (the Field Behavior contract; + # see documentation/forms/field_behaviors.md). + def deserialize!(params) + result = super + return result unless result.respond_to?(:delete) + + compound_field_names.each do |name| + result.delete(name.to_s) + result.delete(name.to_sym) + end + result + end + + private + + def compound_field_names + return [] unless respond_to?(:model) && model + Hyrax::CompoundSchema.for(model).compound_names + rescue StandardError => e + Hyrax.logger.debug("CompoundFieldBehavior: could not read compounds for #{self.class}: #{e.message}") + [] + end + + # One populator serves every compound (Reform passes the property name as + # `as:`). Builds the replacement array of plain hashes — declared sub-field + # keys only, dropping `_destroy` and all-blank rows. + def compound_attributes_populator(fragment:, as:, **_options) + name = as.to_s.delete_suffix('_attributes') + return unless respond_to?(name) + + allowed = Hyrax::CompoundSchema.for(model).subfield_keys(name) + public_send(:"#{name}=", build_compound_rows(fragment, allowed)) + end + + def build_compound_rows(fragment, allowed_keys) + compound_fragment_pairs(fragment) + .sort_by { |key, _row| key.to_i } + .map { |_key, row| compound_row_from(row, allowed_keys) } + .compact + end + + def compound_fragment_pairs(fragment) + return {} if fragment.nil? + fragment.respond_to?(:to_unsafe_h) ? fragment.to_unsafe_h : fragment.to_h + end + + # Returns nil for a row marked for destruction or whose declared sub-fields + # are all blank, otherwise the persisted hash for that row. + def compound_row_from(row, allowed_keys) + row = row.respond_to?(:to_unsafe_h) ? row.to_unsafe_h : row.to_h + return nil if %w[true 1].include?(row['_destroy'].to_s) + + entry = allowed_keys.each_with_object({}) do |key, memo| + value = row[key] + memo[key] = value.is_a?(String) ? value.strip : value + end + return nil if entry.values.all?(&:blank?) + entry + end + end +end diff --git a/app/forms/hyrax/forms.rb b/app/forms/hyrax/forms.rb index 10575e9c21..3794bcc8d3 100644 --- a/app/forms/hyrax/forms.rb +++ b/app/forms/hyrax/forms.rb @@ -14,6 +14,13 @@ def self.PcdmObjectForm(work_class) # rubocop:disable Naming/MethodName Class.new(Hyrax::Forms::PcdmObjectForm) do self.model_class = work_class include Hyrax::FormFields(:core_metadata) if work_class.ancestors.detect { |k| k.inspect == "Hyrax::Schema(core_metadata)" } + # Wire the compound form properties only for works whose model includes + # the compound_metadata schema (non-flexible mode); the m3 loader + # supplies them at init in flexible mode. + if Hyrax.config.compound_metadata_enabled? && !Hyrax.config.flexible? && + work_class.ancestors.detect { |k| k.inspect == "Hyrax::Schema(compound_metadata)" } + include Hyrax::FormFields(:compound_metadata) + end ## # @return [String] diff --git a/app/forms/hyrax/forms/pcdm_collection_form.rb b/app/forms/hyrax/forms/pcdm_collection_form.rb index 542829651d..a007bc3807 100644 --- a/app/forms/hyrax/forms/pcdm_collection_form.rb +++ b/app/forms/hyrax/forms/pcdm_collection_form.rb @@ -7,6 +7,12 @@ module Forms # @see https://github.com/samvera/valkyrie/wiki/ChangeSets-and-Dirty-Tracking class PcdmCollectionForm < Hyrax::Forms::ResourceForm # rubocop:disable Metrics/ClassLength include Hyrax::FormFields(:core_metadata) if Hyrax.config.collection_include_metadata? + # Wire the compound form properties (non-flexible mode), mirroring the + # work-side wiring in the PcdmObjectForm factory. + if Hyrax.config.compound_metadata_enabled? && !Hyrax.config.flexible? && + Hyrax::PcdmCollection.ancestors.detect { |k| k.inspect == "Hyrax::Schema(compound_metadata)" } + include Hyrax::FormFields(:compound_metadata) + end check_if_flexible(Hyrax::PcdmCollection) BannerInfoPrepopulator = lambda do |**_options| diff --git a/app/forms/hyrax/forms/resource_form.rb b/app/forms/hyrax/forms/resource_form.rb index 386764d2a8..28c6108182 100644 --- a/app/forms/hyrax/forms/resource_form.rb +++ b/app/forms/hyrax/forms/resource_form.rb @@ -28,6 +28,7 @@ class ResourceForm < Hyrax::ChangeSet # rubocop:disable Metrics/ClassLength include BasedNearFieldBehavior include RedirectsFieldBehavior + include CompoundFieldBehavior include Hyrax::FormFields(:redirects) if Hyrax.config.redirects_enabled? && Hyrax.config.work_include_metadata? class_attribute :model_class @@ -79,6 +80,10 @@ def initialize(deprecated_resource = nil, resource: nil) current_schema_fields.each do |field_name, options| singleton_class.property field_name.to_sym, options.merge(display: options.fetch(:display, true), default: []) end + # Register the virtual `_attributes` populators on the + # singleton before `super`, so they are part of Reform's schema for + # this instance (non-flexible mode wires them at class load). + register_compound_fields!(r) if respond_to?(:register_compound_fields!) end if resource.nil? @@ -122,6 +127,7 @@ def inherited(subclass) # subclass's model. subclass.prepend(BasedNearFieldBehavior) subclass.prepend(RedirectsFieldBehavior) + subclass.prepend(CompoundFieldBehavior) super end @@ -226,7 +232,7 @@ def model_class # rubocop:disable Rails/Delegate def primary_terms terms = _form_field_definitions .select { |_, definition| definition[:primary] } - .keys.map(&:to_sym) + .keys.map(&:to_sym) - compound_terms terms = [:schema_version, :contexts] + terms if model.flexible? terms @@ -237,7 +243,18 @@ def primary_terms def secondary_terms _form_field_definitions .select { |_, definition| definition[:display] && !definition[:primary] } - .keys.map(&:to_sym) + .keys.map(&:to_sym) - compound_terms + end + + ## + # @return [Array] compound attribute terms, rendered by the + # compound form partials rather than as scalar fields (and so excluded + # from {#primary_terms} / {#secondary_terms}). + def compound_terms + return [] unless respond_to?(:model) && model + Hyrax::CompoundSchema.for(model).compound_names + rescue StandardError + [] end ## diff --git a/app/helpers/hyrax/compound_fields_helper.rb b/app/helpers/hyrax/compound_fields_helper.rb new file mode 100644 index 0000000000..a1a80ec7db --- /dev/null +++ b/app/helpers/hyrax/compound_fields_helper.rb @@ -0,0 +1,134 @@ +# frozen_string_literal: true + +module Hyrax + # View helpers for rendering compound (hierarchical) metadata fields on forms + # and show pages. See documentation/forms/compound_fields.md. + module CompoundFieldsHelper + ## + # Renders one compound section (a repeatable stack of sub-field rows) for the + # given attribute via the `hyrax/compounds/*` partials. + # + # @return [String, nil] rendered HTML, or nil when the attribute is not a + # declared compound on the model. + def render_compound_field(f, compound_name) + schema = Hyrax::CompoundSchema.for(f.object.model) + definition = schema.definition_for(compound_name) + return nil if definition.nil? + + render 'hyrax/compounds/compound_section', + f: f, + compound_name: compound_name.to_sym, + definition: definition, + display_label: compound_field_label(compound_name) + end + + ## + # @return [Boolean] whether the field is a card-display compound + # (`view: { display: card }`). Show views use this to skip card compounds + # in the inline metadata list. + def compound_card_field?(presenter, field) + klass = compound_resource_class_for(presenter) + return false unless klass + Hyrax::CompoundSchema.for(klass).card?(field) + rescue StandardError + false + end + + ## + # Render every card-display compound that has a value as its own titled card + # (matching the relationships/items cards). + # + # @param [Object] presenter a work or collection show presenter + # @return [ActiveSupport::SafeBuffer] + def render_compound_cards(presenter) + klass = compound_resource_class_for(presenter) + return ''.html_safe unless klass + + safe_join(Hyrax::CompoundSchema.for(klass).card_compound_names.map do |name| + next ''.html_safe unless presenter.respond_to?(name) && presenter.public_send(name).present? + + render 'hyrax/compounds/compound_card', presenter: presenter, field: name + end) + rescue StandardError => e + Hyrax.logger.debug("render_compound_cards: #{e.message}") + ''.html_safe + end + + # The resource class for a show presenter, to read its compound schema. + # Works and collections resolve it differently. + def compound_resource_class_for(presenter) + if presenter.respond_to?(:solr_document) && presenter.solr_document.respond_to?(:hydra_model) + presenter.solr_document.hydra_model + elsif presenter.is_a?(Hyrax::CollectionPresenter) + Hyrax.config.collection_class + end + end + + ## + # Options for a `controlled` sub-field's `` dropdown | Options come from either an inline `values:` list or a QA local authority named by `authority:` (see below). The row's stored value is preserved even if it is no longer offered, matching the `include_current_value` convention of the ordinary controlled-field partials. | + +A `controlled` sub-field sources its options one of two ways: + +```yaml +subfields: + # (a) inline list — no authority file needed + agent_role: + type: controlled + values: [Author, Editor, Contributor] + # (a') inline list with distinct id and label + status: + type: controlled + values: + - { id: pub, label: Published } + - { id: dft, label: Draft } + # (b) an existing QA local authority (config/authorities/.yml) + identifier_type: + type: controlled + authority: identifier_type +``` + +Inline `values:` is convenient for small, stable lists and keeps the whole +declaration in one file. `authority:` reuses an existing QA local authority — +the same `config/authorities/*.yml` files ordinary controlled fields use. When +both are given, `values:` wins. Authority options are read through +`Hyrax::TolerantSelectService`, so an authority file that omits the `active:` +key behaves the same as it does for ordinary fields (terms default to active). + +A `work_or_url` sub-field's work search is broader than the stock "my works" +picker: `Hyrax::CompoundWorkPickerBuilder` matches a typed term against any +indexed query field **or** a partial/prefix title (so "jour" matches "Journal +of …"), restricted to works. It subclasses `Hyrax::SearchBuilder`, so the +catalog's read-permission filtering is retained — a user only sees works they +can read. The picker is mounted as the `compound_works` QA authority at +`/authorities/search/compound_works`. + +`db_table` (a typeahead backed by an ActiveRecord lookup table, with +add-new) and `geocode` (a Geonames/coordinate lookup, like `based_near`) are +planned additional sub-field types; the form's row partial has an explicit +extension point for them. + +### `groups:` (optional) + +Visual clustering of sub-fields within each entry's card. Each group is +`{ label:, cols:, fields: [...] }`, where `cols` is the Bootstrap column width +(out of 12) for each field in the group. When omitted, all sub-fields render in +a single unlabeled group. + +### Indexing sub-fields (`index_keys:` / `indexing:`) + +Indexing is declared **per sub-field**, exactly as for scalar fields: list the +literal Solr field names the sub-field's value should be written to, under +`index_keys:` (non-flexible mode) or `indexing:` (flexible mode). The Solr +suffix on each name chooses the behavior — `_tesim` (stored + searchable, +multi-valued), `_sim` (facetable, not stored), `_ssim` (stored + facetable), +etc. — the same conventions scalar fields use. The author picks both the field +names and the suffixes. + +A sub-field with no `index_keys:`/`indexing:` is **not** written to a Solr field +of its own; it can still appear on the show page (see below). As with scalar +fields, the non-field control tokens (`facetable`, `stored_searchable`, +`admin_only`, `editor_only`) are filtered out if present. + +Indexing a sub-field makes it **searchable/facetable** in Solr; it does **not** +make it appear as a column on the catalog/search-results page. The catalog +renders only the fields explicitly registered with Blacklight +(`config.add_index_field`) — compound sub-fields are not registered by default. +To show a sub-field in search results, register its Solr field name in your +`CatalogController` (e.g. +`config.add_index_field 'agent_name_tesim', label: 'Agent'`). The show page and +edit form render compounds regardless. + +### `display:` (optional, default true) + +Controls whether a sub-field is included in the `_json_ss` blob that +the show page renders from. Combined with indexing, each sub-field can be: + +- **display + searchable** — has `index_keys:` and `display` not false +- **display-only** — no `index_keys:` (renders on show, not separately searchable) +- **searchable-only** — has `index_keys:` and `display: false` (indexed, hidden on show) +- **neither** — no `index_keys:` and `display: false` + +## Persisted shape + +A compound persists as an array of plain string-keyed hashes — exactly the +keys declared in `subfields:`: + +```ruby +work.contributors +# => [{ "given_name" => "Ada", "family_name" => "Lovelace", "role_label" => "author" }, +# { "given_name" => "Alan", "family_name" => "Turing", "role_label" => "author" }] +``` + +This shape round-trips cleanly through Postgres JSONB in both flex modes. (Use +this plain-hash shape rather than nesting a `Valkyrie::Resource`; nested +resources round-trip poorly and lack form-layer support — see +[`field_behaviors.md`](field_behaviors.md).) + +## What the foundation provides + +Adding a compound to the schema is enough to get all of the following, with no +per-compound code: + +- **Form rendering** — `Hyrax::Forms::ResourceForm#compound_terms` lists the + compounds, and the work form renders each via the + `hyrax/compounds/_compound_section` partials (a repeatable card stack with + add/remove-row controls). The add/remove client behavior is the engine asset + `app/assets/javascripts/hyrax/compound_metadata.js` (required via + `hyrax.js`). Compounds are excluded from `primary_terms` / `secondary_terms` + so they are not also rendered as scalar inputs. +- **Form → storage conversion** — `Hyrax::CompoundFieldBehavior` registers a + virtual `_attributes` property per compound and a shared populator that + builds the persisted array, dropping `_destroy` and all-blank rows and + keeping only declared sub-field keys. +- **Read-path defense** — `Hyrax::CompoundNormalization`, included on the + resource, guards against Valkyrie's single-element-array key-splay quirk on + reload. +- **Indexing** — `Hyrax::Indexers::CompoundIndexer`, included on the indexer, + writes each sub-field's value to the Solr field names it declares + (`index_keys:`/`indexing:`) and stores the displayable sub-fields as a + `_json_ss` blob for the show page. + +A resource and its indexer opt in with one include each: + +```ruby +class GenericWorkResource < Hyrax::Work + include Hyrax::Schema(:generic_work_resource) + include Hyrax::CompoundNormalization +end + +class GenericWorkResourceIndexer < Hyrax::ValkyrieWorkIndexer + include Hyrax::Indexer(:generic_work_resource) + include Hyrax::Indexers::CompoundIndexer +end +``` + +## Sample compounds shipped with Hyrax + +Hyrax ships sample compounds on **works and collections** by default, +demonstrating the supported sub-field types: + +- **`agent`** — `title`, `agent_name`, and a controlled `agent_role` (inline + `values:`). A person or organization associated with the work or collection. +- **`identifiers`** — an open-entry `identifier` plus a controlled + `identifier_type` (inline `values:`). +- **`compound_rights`** — controlled `rights_statement` and `license` backed by + *existing* QA local authorities (`authority:`), plus an open `rights_notes`. +- **`relationships`** — a `work_or_url` `related_item` (search an internal work + or enter an external URL; linked on show) plus a controlled + `relationship_type`. + +They are declared in `config/metadata/compound_metadata.yaml` (non-flexible +mode) and in the default m3 profile (`config/metadata_profiles/m3_profile.yaml`, +flexible mode, `available_on` both `Hyrax::Work` and the collection class). +The engine base `Hyrax::Work` and `Hyrax::PcdmCollection` include them, and the +base work and collection indexers flatten them, when +`Hyrax.config.compound_metadata_enabled?` is true (the default; set +`compound_metadata_enabled = false`, or `HYRAX_COMPOUND_METADATA_ENABLED=false`, +to omit them). Applications can add their own compounds the same way and remove +or override the samples. + +## Show-page rendering + +Because the show page renders from Solr (not the live resource), the indexer +also stores each compound's ordered rows (limited to sub-fields with `display:` +not false) as a single JSON field (`_json_ss`). The SolrDocument exposes a `` reader that +parses it back into an array of hashes, and the generic +`Hyrax::Renderers::CompoundAttributeRenderer` (selected by `view: render_as: +compound`) renders each entry's populated sub-fields as a small definition +list. Works render compounds through the standard `attribute_to_html` / +`render_as` path; collections render them through the same shared renderer, +invoked from the collection show view for terms the presenter reports as +compounds. Sub-field labels come from the `hyrax.compound_fields..` +i18n keys (humanized fallback). + +For a `controlled` sub-field, the show page displays the authority/value-list +**term**, not the stored id — `Hyrax::CompoundSubfieldLabeler` resolves the id +through the sub-field's inline `values:` list or its QA `authority:` (falling +back to the id when no term matches), the same way scalar controlled fields +display their term. + +### Inline vs. card display (`view: { display: card }`) + +By default a compound renders **inline** in the metadata list, like any other +attribute. Declaring `view: { display: card }` on the compound instead renders +it as its **own titled card** on the show page — matching the Relationships and +Items cards — for compounds that read better as a standalone block: + +```yaml +relationships: + type: hash + multiple: true + view: + render_as: compound + display: card + subfields: + related_item: { type: work_or_url } + relationship_type: { type: controlled, authority: relationship_type } +``` + +`Hyrax::CompoundSchema` reports each compound's `display_mode` (`:inline` or +`:card`); `card_compound_names` lists the card compounds. The view helper +`render_compound_cards(presenter)` renders every card compound that has a value: + +- On a **work** show page, the cards render above the Relationships and Items + cards. +- On a **collection** show page, the cards render after the search-within bar + and before the works list. + +Inline compounds are still listed by the presenter's `terms`; card compounds are +excluded from the inline list (`inline_compound_names`) so they appear only as +their card. A collection presenter delegates the card compound's reader to its +SolrDocument so the same `attribute_to_html(:, render_as: :compound)` +path resolves on a collection as on a work. + +## Profile validation (flexible mode) + +In flexible mode, `Hyrax::FlexibleSchemaValidators::CompoundValidator` checks +compound declarations when an m3 profile is saved/uploaded, so a +misconfiguration fails with a clear message instead of producing dead Solr +fields or unrenderable values. It enforces: `subfields:` is a mapping; each +sub-field config is a mapping; a `type: controlled` sub-field declares an +option source (`authority:` or `values:`); and the compound does **not** carry +a top-level `indexing:` (indexing is declared per sub-field — a top-level +`indexing:` would point the catalog at a `_tesim` field the indexer +never writes). + +## Validation + +Sub-field format and controlled-vocabulary correctness belong in an +`ActiveModel::EachValidator` wired through the form's `validation` block — not +in the populator, whose only job is shape conversion. This mirrors the +`field_behaviors.md` guidance. + +## Bulkrax + +A compound round-trips through Bulkrax import and export with no additional +code, using Bulkrax's declarative `nested_attributes: true` field-mapping flag +(Bulkrax v9.4.3+). Declare the flag on every sibling mapping that shares the +compound's `object:` value, and keep each mapping key equal to the sub-field +key. See `field_behaviors.md` for the column conventions. Note: Bulkrax's +controlled-URI sanitization does not currently descend into compound +sub-fields. diff --git a/documentation/forms/field_behaviors.md b/documentation/forms/field_behaviors.md index 30dd68b630..e150959eb1 100644 --- a/documentation/forms/field_behaviors.md +++ b/documentation/forms/field_behaviors.md @@ -15,6 +15,17 @@ Two examples ship with Hyrax: This document covers the contract a Field Behavior must satisfy, the decision points for a new behavior, and the worked examples. +> **For new multi-sub-field compounds, prefer the generic, schema-driven path.** +> A field whose entries are a hash of named sub-fields (each an open-entry +> string or a controlled-vocabulary lookup) can be declared entirely in the +> schema YAML / m3 profile and rendered, populated, and indexed with no +> per-field Ruby or ERB — see [`compound_fields.md`](compound_fields.md). The +> hand-written Field Behaviors documented below remain the right tool for +> special cases: single-value-per-entry controlled URIs with a presenter +> (`based_near`), or compounds with bespoke behavior such as a radio-group +> selection, write-time normalization, global-uniqueness validation, or +> feature gating (`redirects`). + ## Why this pattern exists Reform's `FormBuilderMethods#deserialize!` rewrites the submitted `_attributes` key to `` before `from_hash` runs. If the form *also* has a property named `` (e.g. from an `include Hyrax::Schema(:foo)`), Reform's `from_hash` writes the raw fragment hash onto the property — bypassing the populator entirely. From f23f344aae364cba44d20ba88ae812f663d4fd8a Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Thu, 4 Jun 2026 16:30:51 -0400 Subject: [PATCH 07/32] Rename agent sample compound to agents The sample compound now uses the attribute name `agents`, avoiding a collision with the internal ACL `agent` attribute that shadowed it in the Valkyrie permissive schema. The collision dropped one entry from the permissive schema (and broke its spec); the sub-field names (agent_name, agent_role) are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../config/metadata_profiles/m3_profile.yaml | 4 +-- .../concerns/hyrax/solr_document/metadata.rb | 2 +- config/locales/hyrax.en.yml | 2 +- config/metadata/compound_metadata.yaml | 7 +++-- config/metadata_profiles/m3_profile.yaml | 4 +-- documentation/forms/compound_fields.md | 2 +- .../forms/compound_metadata_form_spec.rb | 28 +++++++++---------- .../hyrax/simple_schema_loader_spec.rb | 2 +- 8 files changed, 26 insertions(+), 25 deletions(-) diff --git a/.koppie/config/metadata_profiles/m3_profile.yaml b/.koppie/config/metadata_profiles/m3_profile.yaml index db5d29193b..3e501302cd 100644 --- a/.koppie/config/metadata_profiles/m3_profile.yaml +++ b/.koppie/config/metadata_profiles/m3_profile.yaml @@ -930,7 +930,7 @@ properties: property_uri: http://vocabulary.samvera.org/ns#transcriptIds # Sample compound (hierarchical) metadata, demonstrating the Hyrax compound # foundation in flexible mode. See documentation/forms/compound_fields.md. - agent: + agents: available_on: class: - GenericWork @@ -939,7 +939,7 @@ properties: type: hash range: http://www.w3.org/2001/XMLSchema#string display_label: - default: Agent + default: Agents form: primary: false property_uri: http://id.loc.gov/vocabulary/relators/asn diff --git a/app/models/concerns/hyrax/solr_document/metadata.rb b/app/models/concerns/hyrax/solr_document/metadata.rb index 3d9f6e8d15..2b438b631d 100644 --- a/app/models/concerns/hyrax/solr_document/metadata.rb +++ b/app/models/concerns/hyrax/solr_document/metadata.rb @@ -125,7 +125,7 @@ def alt_text_for_view attribute :embargo_release_date, Solr::Date, Hydra.config.permissions.embargo.release_date attribute :lease_expiration_date, Solr::Date, Hydra.config.permissions.lease.expiration_date # Sample compound attributes shipped with Hyrax. - compound_attribute :agent + compound_attribute :agents compound_attribute :identifiers compound_attribute :compound_rights compound_attribute :relationships diff --git a/config/locales/hyrax.en.yml b/config/locales/hyrax.en.yml index 442ee89515..f898aca9ad 100644 --- a/config/locales/hyrax.en.yml +++ b/config/locales/hyrax.en.yml @@ -862,7 +862,7 @@ en: add_row: Add %{label} remove_row: Remove remove_row_with_label: Remove %{label} - agent: + agents: label: Agents title: Title agent_name: Name diff --git a/config/metadata/compound_metadata.yaml b/config/metadata/compound_metadata.yaml index 4c5e3977d2..f622c71e5c 100644 --- a/config/metadata/compound_metadata.yaml +++ b/config/metadata/compound_metadata.yaml @@ -12,9 +12,10 @@ # app-level schema, or the m3 profile under HYRAX_FLEXIBLE=true) and may remove # these samples by setting `Hyrax.config.compound_metadata_enabled = false`. attributes: - # An agent associated with the work: a person or organization with a display - # title, a name, and a controlled role. - agent: + # Agents associated with the work: a person or organization with a display + # title, a name, and a controlled role. (Named `agents` to avoid colliding + # with the internal ACL `agent` attribute in hyrax_internal_metadata.yaml.) + agents: type: hash multiple: true predicate: http://id.loc.gov/vocabulary/relators/asn diff --git a/config/metadata_profiles/m3_profile.yaml b/config/metadata_profiles/m3_profile.yaml index 031e271739..2265afbf9e 100644 --- a/config/metadata_profiles/m3_profile.yaml +++ b/config/metadata_profiles/m3_profile.yaml @@ -956,7 +956,7 @@ properties: # `type: hash, multiple: true` property whose entries are hashes of the # `subfields:` keys, rendered/populated/indexed generically by the compound # foundation. See documentation/forms/compound_fields.md. - agent: + agents: available_on: class: - Hyrax::Work @@ -965,7 +965,7 @@ properties: type: hash range: http://www.w3.org/2001/XMLSchema#string display_label: - default: Agent + default: Agents form: primary: false property_uri: http://id.loc.gov/vocabulary/relators/asn diff --git a/documentation/forms/compound_fields.md b/documentation/forms/compound_fields.md index aa0a2827c2..d5d6808779 100644 --- a/documentation/forms/compound_fields.md +++ b/documentation/forms/compound_fields.md @@ -204,7 +204,7 @@ end Hyrax ships sample compounds on **works and collections** by default, demonstrating the supported sub-field types: -- **`agent`** — `title`, `agent_name`, and a controlled `agent_role` (inline +- **`agents`** — `title`, `agent_name`, and a controlled `agent_role` (inline `values:`). A person or organization associated with the work or collection. - **`identifiers`** — an open-entry `identifier` plus a controlled `identifier_type` (inline `values:`). diff --git a/spec/forms/hyrax/forms/compound_metadata_form_spec.rb b/spec/forms/hyrax/forms/compound_metadata_form_spec.rb index 2116199594..8e21554908 100644 --- a/spec/forms/hyrax/forms/compound_metadata_form_spec.rb +++ b/spec/forms/hyrax/forms/compound_metadata_form_spec.rb @@ -41,35 +41,35 @@ describe 'property registration' do it 'exposes the compound readers so the form partial can render existing rows' do - expect(form).to respond_to(:agent) + expect(form).to respond_to(:agents) expect(form).to respond_to(:identifiers) end it 'exposes the virtual `_attributes` setters for nested form params' do - expect(form).to respond_to(:agent_attributes=) + expect(form).to respond_to(:agents_attributes=) expect(form).to respond_to(:identifiers_attributes=) end it 'lists the compounds as compound_terms (and keeps them out of primary/secondary terms)' do - expect(form.compound_terms).to include(:agent, :identifiers, :compound_rights) - expect(form.primary_terms).not_to include(:agent, :identifiers, :compound_rights) - expect(form.secondary_terms).not_to include(:agent, :identifiers, :compound_rights) + expect(form.compound_terms).to include(:agents, :identifiers, :compound_rights) + expect(form.primary_terms).not_to include(:agents, :identifiers, :compound_rights) + expect(form.secondary_terms).not_to include(:agents, :identifiers, :compound_rights) end - it 'registers `agent_attributes` as a real Reform property (not just method-missing)' do + it 'registers `agents_attributes` as a real Reform property (not just method-missing)' do # This is the property that has to exist in Reform's schema *before* # validate runs; if it is registered too late the populator never fires. # In non-flexible mode it is registered on the form class via FormFields; # check the class definitions registry (the same lens the redirects # coverage uses). definition_keys = form.class.definitions.keys.map(&:to_s) - expect(definition_keys).to include('agent_attributes') + expect(definition_keys).to include('agents_attributes') end end describe 'validate + sync writes the compound to the model' do let(:params) do - { 'agent_attributes' => + { 'agents_attributes' => { '0' => { 'title' => 'Dr', 'agent_name' => 'Ada Lovelace', 'agent_role' => 'Author' } }, 'identifiers_attributes' => { '0' => { 'identifier' => '10.1234/x', 'identifier_type' => 'DOI' } } } @@ -77,7 +77,7 @@ it 'populates the compound on the form during validate' do form.validate(params) - expect(form.agent) + expect(form.agents) .to eq([{ 'title' => 'Dr', 'agent_name' => 'Ada Lovelace', 'agent_role' => 'Author' }]) expect(form.identifiers) .to eq([{ 'identifier' => '10.1234/x', 'identifier_type' => 'DOI' }]) @@ -86,19 +86,19 @@ it 'writes the compound through to the model on sync' do form.validate(params) form.sync - expect(form.model.agent) + expect(form.model.agents) .to eq([{ 'title' => 'Dr', 'agent_name' => 'Ada Lovelace', 'agent_role' => 'Author' }]) expect(form.model.identifiers) .to eq([{ 'identifier' => '10.1234/x', 'identifier_type' => 'DOI' }]) end it 'drops `_destroy` and all-blank rows' do - form.validate('agent_attributes' => + form.validate('agents_attributes' => { '0' => { 'agent_name' => 'Keep', 'agent_role' => 'Author' }, '1' => { 'agent_name' => 'Remove', '_destroy' => 'true' }, '2' => { 'title' => '', 'agent_name' => '', 'agent_role' => '' } }) form.sync - expect(form.model.agent).to eq([{ 'title' => nil, 'agent_name' => 'Keep', 'agent_role' => 'Author' }]) + expect(form.model.agents).to eq([{ 'title' => nil, 'agent_name' => 'Keep', 'agent_role' => 'Author' }]) end end @@ -109,13 +109,13 @@ skip 'requires a Valkyrie (non-Fedora) persister' if Hyrax.persister.is_a?(Wings::Valkyrie::Persister) - form.validate('agent_attributes' => + form.validate('agents_attributes' => { '0' => { 'agent_name' => 'Grace Hopper', 'agent_role' => 'Editor' } }) form.sync saved = Hyrax.persister.save(resource: form.model) reloaded = Hyrax.query_service.find_by(id: saved.id) - expect(reloaded.agent.first['agent_name'] || reloaded.agent.first[:agent_name]) + expect(reloaded.agents.first['agent_name'] || reloaded.agents.first[:agent_name]) .to eq('Grace Hopper') end end diff --git a/spec/services/hyrax/simple_schema_loader_spec.rb b/spec/services/hyrax/simple_schema_loader_spec.rb index 25fc43dcd8..98732db2af 100644 --- a/spec/services/hyrax/simple_schema_loader_spec.rb +++ b/spec/services/hyrax/simple_schema_loader_spec.rb @@ -140,7 +140,7 @@ let(:permissive_schema) { schema_loader.permissive_schema_for_valkrie_adapter } it 'provides the expected hash' do - expect(permissive_schema.size).to eq(68) + expect(permissive_schema.size).to eq(72) expect(permissive_schema.values.all? { |v| v.is_a? RDF::URI }).to be_truthy expect(permissive_schema.values.all? { |v| v.value.present? }).to be_truthy expect(permissive_schema[:sample_attribute].value).to eq("http://hyrax-example.com/sample_attribute") From 5b8b54f858cb70478d19e160611b8bf3c90e7925 Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Thu, 4 Jun 2026 18:41:44 -0400 Subject: [PATCH 08/32] Fix compound metadata display on collections Compound metadata fields no longer show up twice on the collection form, the collection creation page now displays validation errors, and those error messages read cleanly. The collection form was listing each compound field both in its own section and again as an ordinary single field; when a sub-field was required, the duplicate stopped the form from saving. The create page also wasn't showing error messages at all, and the messages it built read awkwardly. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../hyrax/dashboard/collections_controller.rb | 10 +++++----- app/forms/hyrax/forms/pcdm_collection_form.rb | 4 ++-- app/views/hyrax/dashboard/collections/new.html.erb | 1 + .../forms/hyrax/forms/pcdm_collection_form_spec.rb | 14 ++++++++++++++ 4 files changed, 22 insertions(+), 7 deletions(-) diff --git a/app/controllers/hyrax/dashboard/collections_controller.rb b/app/controllers/hyrax/dashboard/collections_controller.rb index 80646f0f15..155f6f04ed 100644 --- a/app/controllers/hyrax/dashboard/collections_controller.rb +++ b/app/controllers/hyrax/dashboard/collections_controller.rb @@ -271,11 +271,11 @@ def valkyrie_destroy end def form_err_msg(form) - errmsg = [] - form.errors.messages.each do |fld, err| - errmsg << "#{fld} #{err.to_sentence}" - end - errmsg.to_sentence + # Use ActiveModel's full_messages: it humanizes the attribute name + # ("collection_type_gid" -> "Collection type gid") and renders :base + # errors (e.g. the compound-required messages) verbatim, instead of the + # raw "#{field_key} #{message}" we'd otherwise emit ("base ..."). + form.errors.full_messages.to_sentence end def default_collection_type diff --git a/app/forms/hyrax/forms/pcdm_collection_form.rb b/app/forms/hyrax/forms/pcdm_collection_form.rb index a007bc3807..6aa8b2f2f0 100644 --- a/app/forms/hyrax/forms/pcdm_collection_form.rb +++ b/app/forms/hyrax/forms/pcdm_collection_form.rb @@ -71,7 +71,7 @@ def required_fields def primary_terms terms = _form_field_definitions .select { |_, definition| definition[:primary] } - .keys.map(&:to_sym) + .keys.map(&:to_sym) - compound_terms terms = [:schema_version] + terms if model.flexible? terms @@ -82,7 +82,7 @@ def primary_terms def secondary_terms _form_field_definitions .select { |_, definition| definition[:display] && !definition[:primary] } - .keys.map(&:to_sym) + .keys.map(&:to_sym) - compound_terms end ## diff --git a/app/views/hyrax/dashboard/collections/new.html.erb b/app/views/hyrax/dashboard/collections/new.html.erb index ccdfc3d10f..858ef220ed 100644 --- a/app/views/hyrax/dashboard/collections/new.html.erb +++ b/app/views/hyrax/dashboard/collections/new.html.erb @@ -7,6 +7,7 @@
+ <%= render 'flash_msg' %> <%= render 'form' %>
diff --git a/spec/forms/hyrax/forms/pcdm_collection_form_spec.rb b/spec/forms/hyrax/forms/pcdm_collection_form_spec.rb index 30feca14bb..e9cff6cbcd 100644 --- a/spec/forms/hyrax/forms/pcdm_collection_form_spec.rb +++ b/spec/forms/hyrax/forms/pcdm_collection_form_spec.rb @@ -28,6 +28,20 @@ end end + # Compounds render via the dedicated compound section, not as scalar fields; + # they must be excluded from primary/secondary terms or they would render a + # second, broken scalar input (a `required` compound would block form submit + # client-side). PcdmCollectionForm overrides these terms, so it needs the same + # exclusion ResourceForm has. + describe 'compound terms are excluded from scalar term lists', if: Hyrax.config.compound_metadata_enabled? do + it 'keeps compounds out of primary_terms and secondary_terms' do + compounds = form.compound_terms + skip 'no compounds declared on the collection in this configuration' if compounds.empty? + expect(form.primary_terms).not_to include(*compounds) + expect(form.secondary_terms).not_to include(*compounds) + end + end + describe '#banner_info' do let(:banner_info) do CollectionBrandingInfo.new( From a4363ab827e3ec20963736ab891fc8fd449280d2 Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Thu, 4 Jun 2026 18:43:30 -0400 Subject: [PATCH 09/32] Validate required compound sub-fields A compound can now mark sub-fields required (filled in every entry) and mark itself required (at least one entry). On save the form blocks with one error per compound, naming the field; required fields show a * on the form. Works the same in both flexible and non-flexible modes, on works and collections. The rules live in Hyrax::CompoundEntryValidation, a plain object decoupled from the form so they can be reused and tested directly; Hyrax::CompoundEntryValidator wires it onto the resource form. The shipped sample compounds require their sub-fields within an entry but remain optional overall. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../config/metadata_profiles/m3_profile.yaml | 6 + .../concerns/hyrax/flexible_form_behavior.rb | 15 +++ app/forms/hyrax/forms/resource_form.rb | 11 ++ .../hyrax/compound_entry_validation.rb | 72 ++++++++++++ app/services/hyrax/compound_schema.rb | 55 ++++++++-- .../hyrax/compound_entry_validator.rb | 64 +++++++++++ .../hyrax/compounds/_compound_row.html.erb | 5 +- .../compounds/_compound_section.html.erb | 3 +- config/locales/hyrax.en.yml | 3 + config/metadata/compound_metadata.yaml | 6 + config/metadata_profiles/m3_profile.yaml | 6 + documentation/forms/compound_fields.md | 43 ++++++++ lib/hyrax/form_fields.rb | 9 +- .../forms/compound_metadata_form_spec.rb | 42 +++++++ .../hyrax/compound_entry_validation_spec.rb | 77 +++++++++++++ spec/services/hyrax/compound_schema_spec.rb | 71 +++++++++++- .../hyrax/compound_entry_validator_spec.rb | 103 ++++++++++++++++++ 17 files changed, 572 insertions(+), 19 deletions(-) create mode 100644 app/services/hyrax/compound_entry_validation.rb create mode 100644 app/validators/hyrax/compound_entry_validator.rb create mode 100644 spec/services/hyrax/compound_entry_validation_spec.rb create mode 100644 spec/validators/hyrax/compound_entry_validator_spec.rb diff --git a/.koppie/config/metadata_profiles/m3_profile.yaml b/.koppie/config/metadata_profiles/m3_profile.yaml index 3e501302cd..083f7873d0 100644 --- a/.koppie/config/metadata_profiles/m3_profile.yaml +++ b/.koppie/config/metadata_profiles/m3_profile.yaml @@ -947,9 +947,11 @@ properties: title: { type: string } agent_name: type: string + required: true indexing: [agent_name_sim, agent_name_tesim] agent_role: type: controlled + required: true values: [Author, Editor, Contributor, Creator, Data Collector, Funder, Publisher, Other] indexing: [agent_role_sim, agent_role_tesim] groups: @@ -973,9 +975,11 @@ properties: subfields: identifier: type: string + required: true indexing: [identifier_value_sim, identifier_value_tesim] identifier_type: type: controlled + required: true values: [DOI, Handle, ISBN, ISSN, URL, URN, ARK, Other] indexing: [identifier_type_sim] view: @@ -1025,9 +1029,11 @@ properties: subfields: related_item: type: work_or_url + required: true indexing: [relationships_item_ssim] relationship_type: type: controlled + required: true values: [References, Is Referenced By, Is Part Of, Has Part, Is Version Of, Replaces, Requires, Other] indexing: [relationships_type_sim] view: diff --git a/app/forms/concerns/hyrax/flexible_form_behavior.rb b/app/forms/concerns/hyrax/flexible_form_behavior.rb index 0d1198e4c5..fa59896c4d 100644 --- a/app/forms/concerns/hyrax/flexible_form_behavior.rb +++ b/app/forms/concerns/hyrax/flexible_form_behavior.rb @@ -13,6 +13,10 @@ def validate_flexible_required_fields required_fields = singleton_class.schema_definitions.select { |_, opts| opts[:required] }.keys required_fields.each do |field| + # Compound fields carry their own requiredness rules (per-row sub-field + # checks); Hyrax::CompoundEntryValidator owns them, so skip the generic + # blank check to avoid a duplicate "can't be blank" error. + next if compound_field?(field) value = send(field) errors.add(field, :blank) if value.blank? end @@ -25,6 +29,17 @@ def schema private + # Whether the field is a compound (declares `subfields:`), looked up from + # the resource's compound schema — the flex form definitions don't carry + # `subfields`. Memoized per form instance. + def compound_field?(field) + return false unless Hyrax.config.respond_to?(:compound_metadata_enabled?) && Hyrax.config.compound_metadata_enabled? + @compound_field_names ||= Hyrax::CompoundSchema.for(model).compound_names + @compound_field_names.include?(field.to_sym) + rescue StandardError + false + end + # OVERRIDE valkyrie 3.0.1 to make schema dynamic def field(field_name) singleton_class.schema_definitions.fetch(field_name.to_s) diff --git a/app/forms/hyrax/forms/resource_form.rb b/app/forms/hyrax/forms/resource_form.rb index 28c6108182..c2cbab8578 100644 --- a/app/forms/hyrax/forms/resource_form.rb +++ b/app/forms/hyrax/forms/resource_form.rb @@ -64,6 +64,17 @@ class ResourceForm < Hyrax::ChangeSet # rubocop:disable Metrics/ClassLength end end + # Required-compound / required-sub-field validation. Wired through a + # `validation { ... }` block (not a bare `validates_with`) because these + # are Reform/Disposable forms — a bare `validates_with` does not hook into + # Reform's `validate`, so it would never run. Record-level (no + # `attributes:`), so it is replay-safe. + if Hyrax.config.compound_metadata_enabled? + validation(name: :default, inherit: true) do + validates_with Hyrax::CompoundEntryValidator + end + end + ## # @api public # diff --git a/app/services/hyrax/compound_entry_validation.rb b/app/services/hyrax/compound_entry_validation.rb new file mode 100644 index 0000000000..b8d5d04e26 --- /dev/null +++ b/app/services/hyrax/compound_entry_validation.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +module Hyrax + ## + # @api public + # + # Pure validation logic for a single compound's entries, decoupled from + # ActiveModel and Reform so it can be reused (e.g. a future Bulkrax-side + # check) and unit-tested directly. {Hyrax::CompoundEntryValidator} wraps this + # for the form layer. See documentation/forms/compound_fields.md. + # + # Rules (driven by the normalized definition from {Hyrax::CompoundSchema}): + # * a compound marked `required` must have at least one populated row; + # * every populated row must fill all of the compound's `required` sub-fields. + # + # Rows are the post-populator persisted hashes (all-blank rows already + # dropped), so a no-required compound with no rows is valid. + class CompoundEntryValidation + # @param definition [Hash] the normalized compound definition + # @param entries [Array] the compound's persisted rows + def initialize(definition, entries) + @definition = definition || {} + @entries = Array(entries) + end + + # @return [Array] one violation per problem, each + # `{ type:, missing: [keys] }`. Empty when the compound is valid. + # `type` is `:required_but_empty` or `:missing_required_subfields`. + def violations + return [{ type: :required_but_empty, missing: required_keys }] if required_but_empty? + + rows_missing_required.map { |missing| { type: :missing_required_subfields, missing: missing } } + end + + # @return [Boolean] + def valid? + violations.empty? + end + + private + + attr_reader :definition, :entries + + def required_keys + definition.fetch(:subfields, {}).select { |_k, spec| spec[:required] }.keys + end + + def required_but_empty? + definition[:required] && populated_rows.empty? + end + + # The set of required keys missing from each populated row that omits any of + # them (one entry per offending row; deduped so identical gaps collapse to + # one message). + def rows_missing_required + return [] if required_keys.empty? + + populated_rows.filter_map do |row| + missing = required_keys.reject { |key| value_present?(row, key) } + missing unless missing.empty? + end.uniq + end + + def populated_rows + entries.select { |row| row.is_a?(::Hash) && row.values.any?(&:present?) } + end + + def value_present?(row, key) + (row[key] || row[key.to_sym]).present? + end + end +end diff --git a/app/services/hyrax/compound_schema.rb b/app/services/hyrax/compound_schema.rb index ec04b1bc5c..fbdcec56ed 100644 --- a/app/services/hyrax/compound_schema.rb +++ b/app/services/hyrax/compound_schema.rb @@ -12,7 +12,7 @@ module Hyrax # Declarations are read from each attribute's Dry type `meta`, which both # schema loaders populate identically, so the result is the same in both flex # modes. - class CompoundSchema + class CompoundSchema # rubocop:disable Metrics/ClassLength ## # Build a CompoundSchema for a resource instance or class. # @@ -102,6 +102,22 @@ def subfield_keys(attribute_name) definition[:subfields].keys end + ## + # @return [Boolean] whether the compound itself is required (at least one + # row must be present to save). + def required?(attribute_name) + definition_for(attribute_name)&.dig(:required) || false + end + + ## + # @return [Array] the sub-field keys declared `required: true` for + # the compound (each must be filled in every populated row). + def required_subfield_keys(attribute_name) + definition = definition_for(attribute_name) + return [] unless definition + definition[:subfields].select { |_key, spec| spec[:required] }.keys + end + ## # @return [Hash{Symbol => Hash}] a map from compound attribute name to its # normalized declaration. Memoized per instance. @@ -135,19 +151,38 @@ def build_definitions # indexer, and renderer consume. See documentation/forms/compound_fields.md # for the meaning of each sub-field key. def normalize(config, subfields) - sub = subfields.each_with_object({}) do |(key, opts), memo| - opts = (opts.is_a?(Hash) ? opts : {}).with_indifferent_access - memo[key.to_s] = { type: (opts['type'] || 'string').to_s, - authority: opts['authority']&.to_s, - values: normalize_values(opts['values']), - index_keys: normalize_index_keys(opts), - display: opts.fetch('display', true) != false } - end + sub = subfields.each_with_object({}) { |(key, opts), memo| memo[key.to_s] = normalize_subfield(opts) } view = config['view'] display_mode = view.is_a?(Hash) && view['display'].to_s == 'card' ? :card : :inline - { subfields: sub, groups: normalize_groups(config['groups'], sub.keys), display_mode: display_mode } + { subfields: sub, + groups: normalize_groups(config['groups'], sub.keys), + display_mode: display_mode, + required: compound_required?(config) } + end + + def normalize_subfield(opts) + opts = (opts.is_a?(Hash) ? opts : {}).with_indifferent_access + { type: (opts['type'] || 'string').to_s, + authority: opts['authority']&.to_s, + values: normalize_values(opts['values']), + index_keys: normalize_index_keys(opts), + display: opts.fetch('display', true) != false, + required: truthy?(opts['required']) } + end + + # Whether the compound itself is required (at least one row must exist). + # Reads `required: true` (non-flexible) or a minimum cardinality >= 1 + # (flexible), mirroring how M3AttributeDefinition derives requirement. + def compound_required?(config) + return true if truthy?(config['required']) + cardinality = config['cardinality'] + cardinality.is_a?(Hash) && cardinality['minimum'].present? && cardinality['minimum'].to_i >= 1 + end + + def truthy?(value) + ActiveModel::Type::Boolean.new.cast(value) == true end # Reads `index_keys:` (non-flexible) or `indexing:` (flexible), filtering the diff --git a/app/validators/hyrax/compound_entry_validator.rb b/app/validators/hyrax/compound_entry_validator.rb new file mode 100644 index 0000000000..352679c34f --- /dev/null +++ b/app/validators/hyrax/compound_entry_validator.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +module Hyrax + # Validates every compound (hierarchical) metadata attribute on a form, + # blocking save when a required compound has no row or a populated row omits a + # required sub-field. Adds one error per compound, keyed on the compound name. + # + # Record-level (not an EachValidator) because the compound set is schema-driven + # and not known at form-class-definition time. The per-compound rules live in + # the reusable {Hyrax::CompoundEntryValidation}. See + # documentation/forms/compound_fields.md. + class CompoundEntryValidator < ActiveModel::Validator + def validate(record) + return unless Hyrax.config.compound_metadata_enabled? + + schema = Hyrax::CompoundSchema.for(schema_source(record)) + schema.definitions.each do |name, definition| + next unless record.respond_to?(name) + validate_compound(record, name, definition) + end + rescue StandardError => e + Hyrax.logger.debug("CompoundEntryValidator: #{e.message}") + end + + private + + # The schema declarations live on the underlying resource, not the form + # wrapper: a Reform form exposes it as `model`. Fall back to the record + # itself (e.g. a plain ActiveModel object in unit tests). + def schema_source(record) + record.respond_to?(:model) ? record.model : record + end + + def validate_compound(record, name, definition) + entries = Array(record.public_send(name)) + Hyrax::CompoundEntryValidation.new(definition, entries).violations.each do |violation| + # Attach to :base, not the attribute, because the work and collection + # forms render errors differently (raw messages vs. full_messages): + # keying on the attribute would either double the field name + # ("Agents Agents ...") or print the raw key ("agents ..."). A :base + # error renders verbatim everywhere, and the message names the compound. + record.errors.add(:base, message_for(name, violation)) + end + end + + # The message names the compound itself (it is attached to :base, so no + # attribute-name prefix is added by the view). + def message_for(name, violation) + I18n.t("hyrax.compound_fields.errors.#{violation[:type]}", + compound: compound_label(name), + fields: subfield_labels(name, violation[:missing])) + end + + def compound_label(name) + I18n.t("hyrax.compound_fields.#{name}.label", default: name.to_s.humanize) + end + + def subfield_labels(name, keys) + Array(keys).map do |key| + I18n.t("hyrax.compound_fields.#{name}.#{key}", default: key.to_s.humanize) + end.join(', ') + end + end +end diff --git a/app/views/hyrax/compounds/_compound_row.html.erb b/app/views/hyrax/compounds/_compound_row.html.erb index d9dee8d6f4..a5ea6cd834 100644 --- a/app/views/hyrax/compounds/_compound_row.html.erb +++ b/app/views/hyrax/compounds/_compound_row.html.erb @@ -40,7 +40,10 @@ <% input_name = "#{f.object_name}[#{compound_name}_attributes][#{index}][#{sub_field}]" %> <% value = row_value.call(sub_field) %>
- <%= label_tag input_id, compound_subfield_label(compound_name, sub_field), class: 'form-label small text-muted' %> + <%= label_tag input_id, class: 'form-label small text-muted' do %> + <%= compound_subfield_label(compound_name, sub_field) %><% + if spec[:required] %>*<% end %> + <% end %> <% case spec[:type] %> <% when 'controlled' %> <% select_classes = ['form-control', 'form-control-sm'] %> diff --git a/app/views/hyrax/compounds/_compound_section.html.erb b/app/views/hyrax/compounds/_compound_section.html.erb index 8272ddc9a9..0eec2654dd 100644 --- a/app/views/hyrax/compounds/_compound_section.html.erb +++ b/app/views/hyrax/compounds/_compound_section.html.erb @@ -19,7 +19,8 @@
<%# A label element, not a heading, so it matches the form's field labels rather than inheriting Bootstrap's h1–h6 sizing. role=heading for a11y. %> - + <%# Always-submitted marker so the populator runs even when every row is removed from the DOM. The populator drops the `_marker` row. %> diff --git a/config/locales/hyrax.en.yml b/config/locales/hyrax.en.yml index f898aca9ad..3938c7294b 100644 --- a/config/locales/hyrax.en.yml +++ b/config/locales/hyrax.en.yml @@ -862,6 +862,9 @@ en: add_row: Add %{label} remove_row: Remove remove_row_with_label: Remove %{label} + errors: + required_but_empty: "%{compound} requires at least one entry" + missing_required_subfields: "Each %{compound} entry requires: %{fields}" agents: label: Agents title: Title diff --git a/config/metadata/compound_metadata.yaml b/config/metadata/compound_metadata.yaml index f622c71e5c..1bc6141ebe 100644 --- a/config/metadata/compound_metadata.yaml +++ b/config/metadata/compound_metadata.yaml @@ -29,11 +29,13 @@ attributes: title: { type: string } agent_name: type: string + required: true index_keys: [agent_name_sim, agent_name_tesim] # A controlled sub-field can take its options from an inline list (below) # or from an existing QA local authority (`authority: `). agent_role: type: controlled + required: true values: [Author, Editor, Contributor, Creator, Data Collector, Funder, Publisher, Other] index_keys: [agent_role_sim, agent_role_tesim] groups: @@ -53,9 +55,11 @@ attributes: subfields: identifier: type: string + required: true index_keys: [identifier_value_sim, identifier_value_tesim] identifier_type: type: controlled + required: true values: [DOI, Handle, ISBN, ISSN, URL, URN, ARK, Other] index_keys: [identifier_type_sim] view: @@ -98,9 +102,11 @@ attributes: subfields: related_item: type: work_or_url + required: true index_keys: [relationships_item_ssim] relationship_type: type: controlled + required: true values: [References, Is Referenced By, Is Part Of, Has Part, Is Version Of, Replaces, Requires, Other] index_keys: [relationships_type_sim] view: diff --git a/config/metadata_profiles/m3_profile.yaml b/config/metadata_profiles/m3_profile.yaml index 2265afbf9e..61b8de53e7 100644 --- a/config/metadata_profiles/m3_profile.yaml +++ b/config/metadata_profiles/m3_profile.yaml @@ -975,9 +975,11 @@ properties: title: { type: string } agent_name: type: string + required: true indexing: [agent_name_sim, agent_name_tesim] agent_role: type: controlled + required: true values: [Author, Editor, Contributor, Creator, Data Collector, Funder, Publisher, Other] indexing: [agent_role_sim, agent_role_tesim] groups: @@ -1001,9 +1003,11 @@ properties: subfields: identifier: type: string + required: true indexing: [identifier_value_sim, identifier_value_tesim] identifier_type: type: controlled + required: true values: [DOI, Handle, ISBN, ISSN, URL, URN, ARK, Other] indexing: [identifier_type_sim] view: @@ -1053,9 +1057,11 @@ properties: subfields: related_item: type: work_or_url + required: true indexing: [relationships_item_ssim] relationship_type: type: controlled + required: true values: [References, Is Referenced By, Is Part Of, Has Part, Is Version Of, Replaces, Requires, Other] indexing: [relationships_type_sim] view: diff --git a/documentation/forms/compound_fields.md b/documentation/forms/compound_fields.md index d5d6808779..b8a64e387b 100644 --- a/documentation/forms/compound_fields.md +++ b/documentation/forms/compound_fields.md @@ -145,6 +145,12 @@ the show page renders from. Combined with indexing, each sub-field can be: - **searchable-only** — has `index_keys:` and `display: false` (indexed, hidden on show) - **neither** — no `index_keys:` and `display: false` +### `required:` (optional, default false) + +Marks a sub-field as required within each populated row, or — when set on the +compound itself — marks the whole compound as required. See +[Required sub-fields](#required-sub-fields) below. + ## Persisted shape A compound persists as an array of plain string-keyed hashes — exactly the @@ -290,6 +296,43 @@ a top-level `indexing:` (indexing is declared per sub-field — a top-level `indexing:` would point the catalog at a `_tesim` field the indexer never writes). +## Required sub-fields + +A compound declares requiredness at two levels: + +- **Sub-field level** — `required: true` on a sub-field means every populated + row must fill it. A row that fills some-but-not-all of its required + sub-fields blocks save (e.g. a relationship row with a related item but no + type). +- **Compound level** — `required: true` on the compound (or, in flexible mode, + a minimum cardinality of 1) means at least one row must be present to save. + An optional compound with no rows is always valid; the sub-field rules only + apply to rows the user actually adds. + +```yaml +relationships: + type: hash + multiple: true + required: true # the work must have at least one relationship + subfields: + related_item: { type: work_or_url, required: true } + relationship_type: { type: controlled, authority: relationship_type, required: true } + note: { type: string } # optional +``` + +Required sub-fields and required compounds render a `*` marker on the form +label. On save, `Hyrax::CompoundEntryValidator` (wired on the resource form, +gated by `compound_metadata_enabled?`) adds one error per compound, keyed on the +compound's attribute name, so the failure is reported against that field. The +rules are the same in both flex modes. + +The decision logic lives in `Hyrax::CompoundEntryValidation`, a plain object +decoupled from ActiveModel and Reform — given a compound's definition and its +rows it returns the violations. That seam keeps the rules reusable (e.g. a +future Bulkrax-side check) and unit-testable without a form. Note: Bulkrax +import does not currently run this validation, so a required sub-field can be +left empty by an import. + ## Validation Sub-field format and controlled-vocabulary correctness belong in an diff --git a/lib/hyrax/form_fields.rb b/lib/hyrax/form_fields.rb index 3e26c0c075..a6a823f2be 100644 --- a/lib/hyrax/form_fields.rb +++ b/lib/hyrax/form_fields.rb @@ -68,11 +68,16 @@ def included(descendant) configs = attribute_configs form_field_definitions.each do |field_name, options| descendant.property field_name.to_sym, options.merge(display: options.fetch(:display, true), default: []) - descendant.validates field_name.to_sym, presence: true if options.fetch(:required, false) + is_compound = compound_field?(configs, field_name) + # A compound's requiredness is enforced by Hyrax::CompoundEntryValidator + # (which understands per-row sub-field rules); a plain `presence` check + # on the array would both duplicate that and pass on a present-but-empty + # row. So only scalar fields get the generic presence validator. + descendant.validates field_name.to_sym, presence: true if options.fetch(:required, false) && !is_compound # A compound also needs a virtual `_attributes` populator # property, registered here so it is part of Reform's schema before # `validate` runs (cf. Hyrax::RedirectsFieldBehavior). - next unless compound_field?(configs, field_name) + next unless is_compound descendant.property :"#{field_name}_attributes", virtual: true, populator: :compound_attributes_populator diff --git a/spec/forms/hyrax/forms/compound_metadata_form_spec.rb b/spec/forms/hyrax/forms/compound_metadata_form_spec.rb index 8e21554908..4bcd4fe3c2 100644 --- a/spec/forms/hyrax/forms/compound_metadata_form_spec.rb +++ b/spec/forms/hyrax/forms/compound_metadata_form_spec.rb @@ -102,6 +102,48 @@ end end + describe 'required-subfield validation blocks save' do + # Exercises the real form + Hyrax::CompoundEntryValidator end to end against + # the *shipped* compound schema (no stub). The shipped samples require + # sub-fields within a row (e.g. agents needs agent_name + agent_role, + # relationships needs related_item + relationship_type) but none is required + # at the compound level, so an empty compound is valid. Asserting through the + # real schema is what catches a validator that reads the form wrapper instead + # of `form.model`. + # + # Compound errors are attached to :base with the compound named in the + # message (so they render cleanly on both the work and collection forms). + def base_errors(form) + form.valid? + form.errors[:base] + end + + it 'allows an empty compound (none is required at the compound level)' do + form.validate('agents_attributes' => { '_marker' => { '_destroy' => '1' } }) + expect(base_errors(form)).to be_empty + end + + it 'flags an agents row that omits a required sub-field' do + form.validate('agents_attributes' => { '0' => { 'agent_role' => 'Author' } }) + expect(base_errors(form)).to include(a_string_including('Agents')) + end + + it 'flags a relationships row that omits a required sub-field' do + form.validate('relationships_attributes' => { '0' => { 'relationship_type' => 'References' } }) + expect(base_errors(form)).to include(a_string_including('Relationships')) + end + + it 'does not flag agents when its required sub-fields are filled' do + form.validate('agents_attributes' => { '0' => { 'agent_name' => 'Ada', 'agent_role' => 'Author' } }) + expect(base_errors(form)).not_to include(a_string_including('Agents')) + end + + it 'never flags `compound_rights` (no required sub-fields)' do + form.validate('agents_attributes' => { '0' => { 'agent_name' => 'Ada', 'agent_role' => 'Author' } }) + expect(base_errors(form)).not_to include(a_string_including('Rights')) + end + end + describe 'persistence round-trip', :active_fedora do # Skips on the Fedora adapter, which cannot serialize plain-hash compounds # to RDF; the compound feature targets the Valkyrie/Postgres path. diff --git a/spec/services/hyrax/compound_entry_validation_spec.rb b/spec/services/hyrax/compound_entry_validation_spec.rb new file mode 100644 index 0000000000..2e6c01cf8c --- /dev/null +++ b/spec/services/hyrax/compound_entry_validation_spec.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +RSpec.describe Hyrax::CompoundEntryValidation do + # A definition shaped like Hyrax::CompoundSchema#definition_for produces. + def build_definition(required: false, subfields: {}) + { required: required, subfields: subfields } + end + + def sub(required: false) + { type: 'string', required: required } + end + + describe 'a compound with no required sub-fields and not required itself' do + let(:definition) { build_definition(subfields: { 'a' => sub, 'b' => sub }) } + + it 'is valid with no rows' do + expect(described_class.new(definition, []).violations).to be_empty + end + + it 'is valid with a partially-filled row' do + expect(described_class.new(definition, [{ 'a' => 'x' }]).violations).to be_empty + end + end + + describe 'a compound with required sub-fields (optional compound)' do + let(:definition) { build_definition(subfields: { 'item' => sub(required: true), 'type' => sub(required: true), 'note' => sub }) } + + it 'is valid with no rows (compound itself is optional)' do + expect(described_class.new(definition, []).violations).to be_empty + end + + it 'is valid when every populated row fills all required sub-fields' do + entries = [{ 'item' => 'a', 'type' => 't' }, { 'item' => 'b', 'type' => 't', 'note' => 'n' }] + expect(described_class.new(definition, entries).violations).to be_empty + end + + it 'flags a row missing a required sub-field' do + violations = described_class.new(definition, [{ 'item' => 'a' }]).violations + expect(violations).to contain_exactly(type: :missing_required_subfields, missing: ['type']) + end + + it 'reports one violation per distinct missing-key set (deduped)' do + entries = [{ 'item' => 'a' }, { 'item' => 'b' }] # both miss only `type` + violations = described_class.new(definition, entries).violations + expect(violations.size).to eq(1) + end + + it 'accepts symbol-keyed rows' do + expect(described_class.new(definition, [{ item: 'a', type: 't' }]).violations).to be_empty + end + end + + describe 'a required compound' do + let(:definition) { build_definition(required: true, subfields: { 'a' => sub(required: true) }) } + + it 'flags an empty compound' do + expect(described_class.new(definition, []).violations) + .to contain_exactly(type: :required_but_empty, missing: ['a']) + end + + it 'is valid with a complete row' do + expect(described_class.new(definition, [{ 'a' => 'x' }]).violations).to be_empty + end + + it 'flags an incomplete row rather than emptiness when a row is present' do + violations = described_class.new(definition, [{ 'a' => '' }]).violations + # an all-blank row is not "populated", so the compound reads as empty + expect(violations).to contain_exactly(type: :required_but_empty, missing: ['a']) + end + end + + describe '#valid?' do + it 'is true when there are no violations' do + expect(described_class.new(build_definition(subfields: { 'a' => sub }), []).valid?).to be true + end + end +end diff --git a/spec/services/hyrax/compound_schema_spec.rb b/spec/services/hyrax/compound_schema_spec.rb index 5cf9e9052f..4625f1e5f3 100644 --- a/spec/services/hyrax/compound_schema_spec.rb +++ b/spec/services/hyrax/compound_schema_spec.rb @@ -42,6 +42,23 @@ def self.name }, view: { 'display' => 'card' } ) + # Required at the compound level (non-flexible `required: true`) with two + # required sub-fields and one optional. + attribute :required_compound, + Valkyrie::Types::Array.of(Dry::Types['hash']).meta( + required: true, + subfields: { + 'item' => { 'type' => 'string', 'required' => true }, + 'kind' => { 'type' => 'string', 'required' => true }, + 'note' => { 'type' => 'string' } + } + ) + # Required at the compound level via m3 minimum cardinality (flexible). + attribute :cardinality_compound, + Valkyrie::Types::Array.of(Dry::Types['hash']).meta( + cardinality: { 'minimum' => 1 }, + subfields: { 'value' => { 'type' => 'string' } } + ) end end @@ -49,7 +66,8 @@ def self.name describe '#compound_names' do it 'returns only the attributes that declare subfields' do - expect(schema.compound_names).to contain_exactly(:contributors, :identifiers, :agent, :relationships) + expect(schema.compound_names).to contain_exactly(:contributors, :identifiers, :agent, :relationships, + :required_compound, :cardinality_compound) end it 'does not treat a plain scalar attribute as a compound' do @@ -60,7 +78,8 @@ def self.name describe 'display mode (view: { display: card })' do describe '#inline_compound_names' do it 'excludes compounds declared as cards' do - expect(schema.inline_compound_names).to contain_exactly(:contributors, :identifiers, :agent) + expect(schema.inline_compound_names).to contain_exactly(:contributors, :identifiers, :agent, + :required_compound, :cardinality_compound) end end @@ -91,6 +110,47 @@ def self.name end end + describe 'required declarations' do + describe '#required?' do + it 'is true when the compound declares required: true' do + expect(schema.required?(:required_compound)).to be true + end + + it 'is true when a minimum cardinality of 1 is declared (m3/flexible)' do + expect(schema.required?(:cardinality_compound)).to be true + end + + it 'is false for an optional compound' do + expect(schema.required?(:relationships)).to be false + end + + it 'is false for a non-compound attribute' do + expect(schema.required?(:title)).to be false + end + end + + describe '#required_subfield_keys' do + it 'returns the sub-fields declared required: true' do + expect(schema.required_subfield_keys(:required_compound)).to eq(%w[item kind]) + end + + it 'is empty when no sub-field is required' do + expect(schema.required_subfield_keys(:relationships)).to eq([]) + end + + it 'is empty for a non-compound' do + expect(schema.required_subfield_keys(:title)).to eq([]) + end + end + + it 'records required on the definition (subfield + compound level)' do + definition = schema.definition_for(:required_compound) + expect(definition[:required]).to be true + expect(definition[:subfields]['item'][:required]).to be true + expect(definition[:subfields]['note'][:required]).to be false + end + end + describe '#compound?' do it 'is true for a declared compound' do expect(schema.compound?(:contributors)).to be true @@ -116,10 +176,10 @@ def self.name it 'normalizes subfields with type, authority, index_keys and display' do definition = schema.definition_for(:contributors) expect(definition[:subfields]['role_label']) - .to eq(type: 'controlled', authority: 'contributor_role', values: nil, index_keys: [], display: false) + .to eq(type: 'controlled', authority: 'contributor_role', values: nil, index_keys: [], display: false, required: false) expect(definition[:subfields]['given_name']) .to eq(type: 'string', authority: nil, values: nil, - index_keys: %w[contributors_given_name_sim contributors_given_name_tesim], display: true) + index_keys: %w[contributors_given_name_sim contributors_given_name_tesim], display: true, required: false) end it 'reads per-sub-field index_keys (literal Solr field names)' do @@ -158,7 +218,8 @@ def self.name describe '.for' do it 'builds from a resource instance' do expect(described_class.for(resource_class.new).compound_names) - .to contain_exactly(:contributors, :identifiers, :agent, :relationships) + .to contain_exactly(:contributors, :identifiers, :agent, :relationships, + :required_compound, :cardinality_compound) end end end diff --git a/spec/validators/hyrax/compound_entry_validator_spec.rb b/spec/validators/hyrax/compound_entry_validator_spec.rb new file mode 100644 index 0000000000..c9760a0799 --- /dev/null +++ b/spec/validators/hyrax/compound_entry_validator_spec.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true + +RSpec.describe Hyrax::CompoundEntryValidator do + let(:record_class) do + Class.new do + include ActiveModel::Validations + attr_accessor :relationships + validates_with Hyrax::CompoundEntryValidator + end + end + let(:record) { record_class.new.tap { |r| r.relationships = entries } } + let(:entries) { [] } + + # A schema with one required-sub-field compound (`relationships`), optional + # at the compound level. + let(:definition) do + { required: false, + subfields: { 'related_item' => { type: 'work_or_url', required: true }, + 'relationship_type' => { type: 'controlled', required: true } } } + end + let(:schema) { instance_double(Hyrax::CompoundSchema, definitions: { relationships: definition }) } + + before do + allow(Hyrax.config).to receive(:compound_metadata_enabled?).and_return(true) + allow(Hyrax::CompoundSchema).to receive(:for).and_return(schema) + end + + # Compound errors are attached to :base (so the work and collection forms, + # which render errors differently, both show them cleanly), and the message + # names the compound. + context 'when the feature is disabled' do + let(:entries) { [{ 'related_item' => 'x' }] } # would otherwise be invalid + before { allow(Hyrax.config).to receive(:compound_metadata_enabled?).and_return(false) } + + it 'does not validate' do + record.valid? + expect(record.errors[:base]).to be_empty + end + end + + context 'with no rows (optional compound)' do + it 'is valid' do + record.valid? + expect(record.errors[:base]).to be_empty + end + end + + context 'with a complete row' do + let(:entries) { [{ 'related_item' => 'work-1', 'relationship_type' => 'References' }] } + + it 'is valid' do + record.valid? + expect(record.errors[:base]).to be_empty + end + end + + context 'with a row missing a required sub-field' do + let(:entries) { [{ 'related_item' => 'work-1' }] } + + it 'adds one base error' do + record.valid? + expect(record.errors[:base].size).to eq(1) + end + + it 'names the compound and the missing sub-field in the message' do + record.valid? + expect(record.errors[:base].first).to include('Relationships').and include('Relationship type') + end + end + + context 'when the compound itself is required' do + let(:definition) { super().merge(required: true) } + + context 'and empty' do + it 'flags the empty compound by name' do + record.valid? + expect(record.errors[:base].first).to include('Relationships').and include('at least one entry') + end + end + + context 'and populated completely' do + let(:entries) { [{ 'related_item' => 'work-1', 'relationship_type' => 'References' }] } + + it 'is valid' do + record.valid? + expect(record.errors[:base]).to be_empty + end + end + end + + context 'when the record does not respond to the compound reader' do + let(:record) do + Class.new do + include ActiveModel::Validations + validates_with Hyrax::CompoundEntryValidator + end.new + end + + it 'skips it without error' do + expect { record.valid? }.not_to raise_error + end + end +end From bb0ca4942689ace0c083535b99bba46477f0ac34 Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Thu, 4 Jun 2026 19:30:31 -0400 Subject: [PATCH 10/32] Resolve compound card schema from Solr doc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Compound metadata cards now render on show pages in flexible mode. The card helpers resolved the compound declarations from the resource class, but in flexible mode the class carries none — they are applied per instance at load — so no cards appeared. They now resolve from the show document's indexed schema_version, reading that version's attributes without loading the resource; non-flexible mode still reads the class. Also corrects a collection-controller spec that doubled the form errors object with a class lacking full_messages, matching the real Reform errors object the controller now reads. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/helpers/hyrax/compound_fields_helper.rb | 30 +++++----- app/services/hyrax/compound_schema.rb | 60 ++++++++++++++++--- .../dashboard/collections_controller_spec.rb | 16 +++-- .../hyrax/compound_fields_helper_spec.rb | 20 ++----- spec/services/hyrax/compound_schema_spec.rb | 39 ++++++++++++ 5 files changed, 120 insertions(+), 45 deletions(-) diff --git a/app/helpers/hyrax/compound_fields_helper.rb b/app/helpers/hyrax/compound_fields_helper.rb index a1a80ec7db..8ea7a5b305 100644 --- a/app/helpers/hyrax/compound_fields_helper.rb +++ b/app/helpers/hyrax/compound_fields_helper.rb @@ -27,9 +27,7 @@ def render_compound_field(f, compound_name) # (`view: { display: card }`). Show views use this to skip card compounds # in the inline metadata list. def compound_card_field?(presenter, field) - klass = compound_resource_class_for(presenter) - return false unless klass - Hyrax::CompoundSchema.for(klass).card?(field) + compound_schema_for(presenter).card?(field) rescue StandardError false end @@ -41,10 +39,7 @@ def compound_card_field?(presenter, field) # @param [Object] presenter a work or collection show presenter # @return [ActiveSupport::SafeBuffer] def render_compound_cards(presenter) - klass = compound_resource_class_for(presenter) - return ''.html_safe unless klass - - safe_join(Hyrax::CompoundSchema.for(klass).card_compound_names.map do |name| + safe_join(compound_schema_for(presenter).card_compound_names.map do |name| next ''.html_safe unless presenter.respond_to?(name) && presenter.public_send(name).present? render 'hyrax/compounds/compound_card', presenter: presenter, field: name @@ -54,14 +49,19 @@ def render_compound_cards(presenter) ''.html_safe end - # The resource class for a show presenter, to read its compound schema. - # Works and collections resolve it differently. - def compound_resource_class_for(presenter) - if presenter.respond_to?(:solr_document) && presenter.solr_document.respond_to?(:hydra_model) - presenter.solr_document.hydra_model - elsif presenter.is_a?(Hyrax::CollectionPresenter) - Hyrax.config.collection_class - end + # The compound schema for a show presenter, resolved from the backing Solr + # document so it works in flexible mode (where the class carries no + # compounds). Memoized per request. See {Hyrax::CompoundSchema.for_solr_document}. + def compound_schema_for(presenter) + @compound_schemas ||= {} + @compound_schemas[presenter.object_id] ||= + if presenter.respond_to?(:solr_document) && presenter.solr_document.respond_to?(:hydra_model) + Hyrax::CompoundSchema.for_solr_document(presenter.solr_document) + elsif presenter.is_a?(Hyrax::CollectionPresenter) + Hyrax::CompoundSchema.for(Hyrax.config.collection_class) + else + Hyrax::CompoundSchema.new + end end ## diff --git a/app/services/hyrax/compound_schema.rb b/app/services/hyrax/compound_schema.rb index fbdcec56ed..e1f89f3650 100644 --- a/app/services/hyrax/compound_schema.rb +++ b/app/services/hyrax/compound_schema.rb @@ -23,6 +23,43 @@ def self.for(resource) new(*schema_sources_for(resource)) end + ## + # Build a CompoundSchema for a show-page Solr document. In flexible mode the + # resource class carries no compounds (they are applied per-instance at + # load), so resolve them from the document's indexed `schema_version` + # without loading the resource; fall back to the model class (non-flexible). + # + # @param document [#hydra_model] a SolrDocument-like object + def self.for_solr_document(document) + new(*solr_document_schema_sources(document)) + end + + def self.solr_document_schema_sources(document) + klass = document.hydra_model if document.respond_to?(:hydra_model) + version = document['schema_version_ssi'] if document.respond_to?(:[]) + + if klass && version.present? + attrs = flexible_attributes_for(klass, version) + return [attrs] if attrs.present? + end + + schema_sources_for(klass) + rescue StandardError => e + Hyrax.logger.debug("CompoundSchema.for_solr_document: #{e.message}") + [] + end + private_class_method :solr_document_schema_sources + + # The `{ name => dry_type }` attribute map for a class at a flexible schema + # version; nil when the loader is unavailable (non-flexible installs). + def self.flexible_attributes_for(klass, version) + loader = Hyrax::Schema.m3_schema_loader + loader.attributes_for(schema: klass.name, version: version, contexts: []) + rescue StandardError + nil + end + private_class_method :flexible_attributes_for + # For a class, its own schema. For an instance, both the class schema # (non-flexible) and the singleton schema (flexible: the m3 attributes are # applied to the singleton at load time). Unioning both makes {.for} work in @@ -129,14 +166,8 @@ def definitions def build_definitions schema_sources.each_with_object({}) do |schema, memo| - next unless schema.respond_to?(:each) - - schema.each do |property| - meta = property.respond_to?(:meta) ? property.meta : nil - next if meta.nil? - - name = property.name.to_sym - next if memo.key?(name) + name_meta_pairs(schema).each do |name, meta| + next if meta.nil? || memo.key?(name) config = meta.with_indifferent_access subfields = config['subfields'] @@ -147,6 +178,19 @@ def build_definitions end end + # `[name, meta]` pairs for a schema source, which is either a Dry schema (an + # iterable of properties with `name`/`meta`) or a `{ name => dry_type }` Hash + # (the `SchemaLoader#attributes_for` output used for version resolution). + def name_meta_pairs(schema) + if schema.is_a?(::Hash) + schema.map { |name, type| [name.to_sym, (type.meta if type.respond_to?(:meta))] } + elsif schema.respond_to?(:each) + schema.map { |property| [property.name.to_sym, (property.meta if property.respond_to?(:meta))] } + else + [] + end + end + # Normalizes the raw declaration into the symbol-keyed shape the form, # indexer, and renderer consume. See documentation/forms/compound_fields.md # for the meaning of each sub-field key. diff --git a/spec/controllers/hyrax/dashboard/collections_controller_spec.rb b/spec/controllers/hyrax/dashboard/collections_controller_spec.rb index da829a1722..0a89d667a6 100644 --- a/spec/controllers/hyrax/dashboard/collections_controller_spec.rb +++ b/spec/controllers/hyrax/dashboard/collections_controller_spec.rb @@ -231,9 +231,11 @@ context "in validations" do let(:form) { instance_double(Hyrax::Forms::PcdmCollectionForm, errors: errors) } - let(:errors) { instance_double(Reform::Contract::CustomError, messages: messages) } - let(:messages) { { publisher: ["must exist"] } } - let(:errmsg) { "publisher must exist" } + # Plain double: a real Reform form's errors responds to full_messages, + # but Reform::Contract::CustomError (what instance_double would verify) does not. + let(:errors) { double("errors", full_messages: full_messages) } + let(:full_messages) { ["Publisher must exist"] } + let(:errmsg) { "Publisher must exist" } before do skip("these validations only apply to Valkyrie forms") if @@ -459,9 +461,11 @@ context "in validations" do let(:form) { instance_double(Hyrax::Forms::PcdmCollectionForm, errors: errors) } - let(:errors) { instance_double(Reform::Contract::CustomError, messages: messages) } - let(:messages) { { publisher: ["must exist"] } } - let(:errmsg) { "publisher must exist" } + # Plain double: a real Reform form's errors responds to full_messages, + # but Reform::Contract::CustomError (what instance_double would verify) does not. + let(:errors) { double("errors", full_messages: full_messages) } + let(:full_messages) { ["Publisher must exist"] } + let(:errmsg) { "Publisher must exist" } before do skip("these validations only apply to Valkyrie forms") if diff --git a/spec/helpers/hyrax/compound_fields_helper_spec.rb b/spec/helpers/hyrax/compound_fields_helper_spec.rb index 1febbe5cbe..097bcf4485 100644 --- a/spec/helpers/hyrax/compound_fields_helper_spec.rb +++ b/spec/helpers/hyrax/compound_fields_helper_spec.rb @@ -52,26 +52,14 @@ card_compound_names: [:relationships]) end + # Show presenters resolve their compound schema from the backing Solr + # document (so flexible mode works), so stub that resolution path. before do - allow(Hyrax::CompoundSchema).to receive(:for).and_return(schema) + allow(Hyrax::CompoundSchema).to receive(:for_solr_document).and_return(schema) allow(schema).to receive(:card?).with(:relationships).and_return(true) allow(schema).to receive(:card?).with(:contributors).and_return(false) end - describe '#compound_resource_class_for' do - it 'uses the work hydra_model for a work presenter' do - solr_document = instance_double(SolrDocument, hydra_model: GenericWork) - presenter = instance_double(Hyrax::WorkShowPresenter, solr_document: solr_document) - allow(presenter).to receive(:respond_to?).with(:solr_document).and_return(true) - expect(helper.compound_resource_class_for(presenter)).to eq(GenericWork) - end - - it 'uses the configured collection class for a collection presenter' do - presenter = Hyrax::CollectionPresenter.allocate - expect(helper.compound_resource_class_for(presenter)).to eq(Hyrax.config.collection_class) - end - end - describe '#compound_card_field?' do let(:solr_document) { instance_double(SolrDocument, hydra_model: GenericWork) } let(:presenter) do @@ -88,7 +76,7 @@ expect(helper.compound_card_field?(presenter, :contributors)).to be false end - it 'is false (not raising) when the resource class cannot be resolved' do + it 'is false (not raising) when the schema cannot be resolved' do expect(helper.compound_card_field?(Object.new, :relationships)).to be false end end diff --git a/spec/services/hyrax/compound_schema_spec.rb b/spec/services/hyrax/compound_schema_spec.rb index 4625f1e5f3..2848bee7a0 100644 --- a/spec/services/hyrax/compound_schema_spec.rb +++ b/spec/services/hyrax/compound_schema_spec.rb @@ -222,4 +222,43 @@ def self.name :required_compound, :cardinality_compound) end end + + describe '.for_solr_document' do + # The attribute map a flexible schema loader returns for a version: + # `{ name => dry_type_with_meta }` — the same shape SchemaLoader#attributes_for + # produces. Show pages resolve compounds from this (via schema_version) + # without loading the resource. + let(:version_attributes) do + resource_class.schema.index_by(&:name) + end + + context 'in flexible mode (document has a schema_version)' do + let(:document) { instance_double(SolrDocument, hydra_model: resource_class) } + + before do + allow(document).to receive(:[]).with('schema_version_ssi').and_return('7') + loader = instance_double(Hyrax::M3SchemaLoader) + allow(Hyrax::Schema).to receive(:m3_schema_loader).and_return(loader) + allow(loader).to receive(:attributes_for) + .with(schema: 'TestResourceWithCompounds', version: '7', contexts: []) + .and_return(version_attributes) + end + + it 'resolves compounds from the version attribute map (no resource load)' do + expect(described_class.for_solr_document(document).card_compound_names) + .to contain_exactly(:relationships) + end + end + + context 'when the document has no schema_version (non-flexible)' do + let(:document) { instance_double(SolrDocument, hydra_model: resource_class) } + + before { allow(document).to receive(:[]).with('schema_version_ssi').and_return(nil) } + + it 'falls back to the model class schema' do + expect(described_class.for_solr_document(document).compound_names) + .to include(:relationships, :agent) + end + end + end end From 70cc1719a27bf5b230807662d55855027df32d28 Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Thu, 4 Jun 2026 19:59:08 -0400 Subject: [PATCH 11/32] Show collection compounds in flexible mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Inline compound metadata (e.g. agents, identifiers) now appears on a collection's show page in flexible mode. The presenter resolved its displayed terms and compound readers from the collection class, but a flexible schema is per-resource — keyed by the document's indexed schema_version and mutable at runtime — so the class often carries none. It now resolves them from the backing document, the same way the compound cards already do. --- app/presenters/hyrax/collection_presenter.rb | 56 +++++++++++++------ .../hyrax/collection_presenter_spec.rb | 48 ++++++++-------- 2 files changed, 63 insertions(+), 41 deletions(-) diff --git a/app/presenters/hyrax/collection_presenter.rb b/app/presenters/hyrax/collection_presenter.rb index cf1046a0fc..d5a4f980e7 100644 --- a/app/presenters/hyrax/collection_presenter.rb +++ b/app/presenters/hyrax/collection_presenter.rb @@ -40,38 +40,51 @@ def collection_type :title_or_label, :collection_type_gid, :create_date, :modified_date, :visibility, :edit_groups, :edit_people, to: :solr_document - # Terms is the list of fields displayed by - # app/views/collections/_show_descriptions.html.erb + # The standard metadata terms displayed by + # app/views/collections/_show_descriptions.html.erb. Compound terms are + # appended per instance (see {#terms}); they can't be resolved at the class + # level because a flexible schema is per-resource (keyed by the indexed + # schema_version) and mutable at runtime. + DEFAULT_TERMS = [:total_items, :alternative_title, :size, :resource_type, :creator, :contributor, + :keyword, :license, :publisher, :date_created, :subject, :language, :identifier, + :based_near, :related_url].freeze + + # @deprecated kept for backward compatibility; the standard terms only. Use + # the instance {#terms} to include this collection's compound terms. def self.terms - [:total_items, :alternative_title, :size, :resource_type, :creator, :contributor, :keyword, :license, :publisher, :date_created, :subject, - :language, :identifier, :based_near, :related_url] + compound_terms + DEFAULT_TERMS.dup end - # Compound terms that render inline in the collection metadata list. Card - # compounds are excluded (they render via `render_compound_cards`). See - # documentation/forms/compound_fields.md. - def self.compound_terms - return [] unless Hyrax.config.compound_metadata_enabled? - Hyrax::CompoundSchema.for(Hyrax.config.collection_class).inline_compound_names - rescue StandardError - [] + # This collection's displayed terms: the standard terms plus its inline + # compound terms, resolved from the backing document so it is correct in + # flexible mode. + def terms + DEFAULT_TERMS + compound_terms end def terms_with_values - self.class.terms.select { |t| self[t].present? } + terms.select { |t| self[t].present? } end + # Inline compound terms for this collection (card compounds render + # separately via `render_compound_cards`). Resolved from the backing + # document. See documentation/forms/compound_fields.md. def compound_terms - self.class.compound_terms + return [] unless Hyrax.config.compound_metadata_enabled? + compound_schema.inline_compound_names + rescue StandardError + [] end def compound_term?(term) compound_terms.include?(term.to_sym) end - def self.all_compound_names + # All compound names (inline + card) declared for this collection, used to + # delegate the readers to the solr document below. + def all_compound_names return [] unless Hyrax.config.compound_metadata_enabled? - Hyrax::CompoundSchema.for(Hyrax.config.collection_class).compound_names + compound_schema.compound_names rescue StandardError [] end @@ -80,14 +93,21 @@ def self.all_compound_names # resolves on a collection as on a work. Via method_missing because the # compound set is schema-driven, not known at class-definition time. def respond_to_missing?(name, include_private = false) - self.class.all_compound_names.include?(name.to_sym) || super + all_compound_names.include?(name.to_sym) || super end def method_missing(name, *args, &block) - return solr_document.send(name, *args, &block) if self.class.all_compound_names.include?(name.to_sym) + return solr_document.send(name, *args, &block) if all_compound_names.include?(name.to_sym) super end + # The compound schema for this collection, resolved from the backing Solr + # document (so it reflects the resource's own schema_version in flexible + # mode, rather than an empty or stale class-level schema). Memoized. + def compound_schema + @compound_schema ||= Hyrax::CompoundSchema.for_solr_document(solr_document) + end + ## # @param [Symbol] key # @return [Object] diff --git a/spec/presenters/hyrax/collection_presenter_spec.rb b/spec/presenters/hyrax/collection_presenter_spec.rb index 9c69cf31c1..2c18818651 100644 --- a/spec/presenters/hyrax/collection_presenter_spec.rb +++ b/spec/presenters/hyrax/collection_presenter_spec.rb @@ -18,40 +18,42 @@ it { is_expected.to delegate_method(:date_created).to(:solr_document) } describe ".terms" do + # The class method is the default terms only; compound terms are resolved + # per instance (see "#terms") because a flexible schema is per-resource. subject { described_class.terms } - context 'with compound metadata disabled' do - before { allow(Hyrax.config).to receive(:compound_metadata_enabled?).and_return(false) } + it 'is the default terms' do + is_expected.to eq [:total_items, :alternative_title, :size, :resource_type, :creator, + :contributor, :keyword, :license, :publisher, + :date_created, :subject, :language, :identifier, + :based_near, :related_url] + end + end - it do - is_expected.to eq [:total_items, :alternative_title, :size, :resource_type, :creator, - :contributor, :keyword, :license, :publisher, - :date_created, :subject, :language, :identifier, - :based_near, :related_url] - end + describe "#terms" do + it 'starts with the default terms' do + expect(presenter.terms.first(15)).to eq described_class::DEFAULT_TERMS end - it 'starts with the base scalar terms' do - expect(subject.first(15)).to eq [:total_items, :alternative_title, :size, :resource_type, :creator, - :contributor, :keyword, :license, :publisher, - :date_created, :subject, :language, :identifier, - :based_near, :related_url] + it 'appends this collection\'s inline compound terms' do + allow(presenter).to receive(:compound_terms).and_return([:agents, :identifiers]) + expect(presenter.terms).to end_with(:agents, :identifiers) end - it 'appends compound terms declared on the collection model when enabled' do - compound_terms = described_class.compound_terms - # Under a flexible-mode stack the collection class composition is fixed at - # boot, so the shipped compounds are only present when the active flex - # schema declares them; guard rather than assert an empty splat. - skip 'no compound terms declared on the collection class in this configuration' if compound_terms.empty? - expect(subject).to include(*compound_terms) + context 'with compound metadata disabled' do + before { allow(Hyrax.config).to receive(:compound_metadata_enabled?).and_return(false) } + + it 'is the default terms only' do + expect(presenter.terms).to eq described_class::DEFAULT_TERMS + end end end describe "compound attribute delegation" do - context "when a compound is declared on the collection model" do + # Resolution is per instance, from the backing document's compound schema. + context "when a compound is declared for the collection" do before do - allow(described_class).to receive(:all_compound_names).and_return([:relationships]) + allow(presenter).to receive(:all_compound_names).and_return([:relationships]) allow(solr_doc).to receive(:relationships).and_return([{ 'related_item' => 'https://example.org' }]) end @@ -65,7 +67,7 @@ end context "for an undeclared method" do - before { allow(described_class).to receive(:all_compound_names).and_return([]) } + before { allow(presenter).to receive(:all_compound_names).and_return([]) } it "does not respond to it" do expect(presenter).not_to respond_to(:not_a_compound) From c3417801ab835762368352e275f3b5f5b92bb089 Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Thu, 4 Jun 2026 20:11:01 -0400 Subject: [PATCH 12/32] Build collection terms on the class term list The instance term list now derives from the class-level terms plus the collection's compound terms, instead of a hardcoded constant. This restores the effect of a downstream override of the class `terms` method (e.g. one that removes a term), which the previous instance implementation bypassed. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/presenters/hyrax/collection_presenter.rb | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/app/presenters/hyrax/collection_presenter.rb b/app/presenters/hyrax/collection_presenter.rb index d5a4f980e7..9837fc10e2 100644 --- a/app/presenters/hyrax/collection_presenter.rb +++ b/app/presenters/hyrax/collection_presenter.rb @@ -49,17 +49,18 @@ def collection_type :keyword, :license, :publisher, :date_created, :subject, :language, :identifier, :based_near, :related_url].freeze - # @deprecated kept for backward compatibility; the standard terms only. Use - # the instance {#terms} to include this collection's compound terms. + # The standard (non-compound) terms. Class-level so downstream apps can + # override the list (e.g. to remove a term); the instance {#terms} builds on + # this and appends the per-resource compound terms. def self.terms DEFAULT_TERMS.dup end - # This collection's displayed terms: the standard terms plus its inline - # compound terms, resolved from the backing document so it is correct in - # flexible mode. + # This collection's displayed terms: the class-level standard terms (honoring + # any downstream override) plus its inline compound terms, which are resolved + # from the backing document so they are correct in flexible mode. def terms - DEFAULT_TERMS + compound_terms + self.class.terms + compound_terms end def terms_with_values From 18777a1190ec009ba86d27cf290c35be29331d97 Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Thu, 4 Jun 2026 20:22:11 -0400 Subject: [PATCH 13/32] Fix compound card display in flexible mode Three fixes for compound cards on show pages: - Sub-field specs now resolve from the backing document, not the class, so in flexible mode a card's controlled terms display their labels and a work-or-url value links correctly (the class carries no compounds in flexible mode, so the renderer was getting raw values). - A work-or-url value that is neither a URL nor a resolvable indexed work now renders as plain text instead of a broken link. - Compound values sit flush with the value column instead of carrying a fixed indent that misaligned under downstream layouts. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../stylesheets/hyrax/_compound_metadata.scss | 7 +++-- app/presenters/hyrax/presents_attributes.rb | 9 ++++-- .../renderers/compound_attribute_renderer.rb | 13 +++++---- app/services/hyrax/compound_work_resolver.rb | 28 +++++++++++++++++-- .../compound_attribute_renderer_spec.rb | 17 +++++++++-- .../hyrax/compound_work_resolver_spec.rb | 20 +++++++++++++ 6 files changed, 77 insertions(+), 17 deletions(-) diff --git a/app/assets/stylesheets/hyrax/_compound_metadata.scss b/app/assets/stylesheets/hyrax/_compound_metadata.scss index d162a87976..1f46da0218 100644 --- a/app/assets/stylesheets/hyrax/_compound_metadata.scss +++ b/app/assets/stylesheets/hyrax/_compound_metadata.scss @@ -9,9 +9,10 @@ display: block !important; flex-direction: column; border: 0 !important; - // Indent to align with scalar metadata values (their `ul.tabular` keeps the - // browser default ~40px list padding). - padding: 0 0 0 40px !important; + // Sit flush with the value column. No indent of our own: scalar values get + // theirs from a `ul.tabular` wrapper that apps and themes style differently, + // so a fixed pad here would misalign in some of them. + padding: 0 !important; max-width: 100%; // Force block flow on the entries/sub-fields; the label/value spans stay diff --git a/app/presenters/hyrax/presents_attributes.rb b/app/presenters/hyrax/presents_attributes.rb index ed165b8e38..7f35f546cb 100644 --- a/app/presenters/hyrax/presents_attributes.rb +++ b/app/presenters/hyrax/presents_attributes.rb @@ -52,9 +52,12 @@ def microdata_type_to_html # controlled ids to their terms; nil if the resource class can't be # resolved (the renderer then renders raw values). def compound_subfields_for(field) - klass = solr_document.try(:hydra_model) if respond_to?(:solr_document) - return nil unless klass - Hyrax::CompoundSchema.for(klass).definition_for(field)&.fetch(:subfields, nil) + return nil unless respond_to?(:solr_document) && solr_document.respond_to?(:hydra_model) + # Resolve from the backing document, not the class: in flexible mode the + # class carries no compounds, so a class lookup would drop the sub-field + # specs and the renderer would fall back to raw (unlinked, untranslated) + # values. + Hyrax::CompoundSchema.for_solr_document(solr_document).definition_for(field)&.fetch(:subfields, nil) rescue StandardError => e Hyrax.logger.debug("compound_subfields_for(#{field}): #{e.message}") nil diff --git a/app/renderers/hyrax/renderers/compound_attribute_renderer.rb b/app/renderers/hyrax/renderers/compound_attribute_renderer.rb index cdbc8718e3..f19b2fae10 100644 --- a/app/renderers/hyrax/renderers/compound_attribute_renderer.rb +++ b/app/renderers/hyrax/renderers/compound_attribute_renderer.rb @@ -75,13 +75,14 @@ def value_markup(sub_field, value) end end + # Link a URL or a resolvable work; render anything else as plain text so + # we never emit a broken link to a non-existent work. def work_or_url_markup(value) - if Hyrax::CompoundWorkResolver.url?(value) - auto_link(ERB::Util.h(value.to_s)) - else - title, path = Hyrax::CompoundWorkResolver.title_and_path(value) - link_to(ERB::Util.h(title), path) - end + return auto_link(ERB::Util.h(value.to_s)) if Hyrax::CompoundWorkResolver.url?(value) + + title, path = Hyrax::CompoundWorkResolver.resolve(value) + return ERB::Util.h(value.to_s) if title.nil? + link_to(ERB::Util.h(title), path) end def display_value(sub_field, value) diff --git a/app/services/hyrax/compound_work_resolver.rb b/app/services/hyrax/compound_work_resolver.rb index 519e82c3df..f25235b94f 100644 --- a/app/services/hyrax/compound_work_resolver.rb +++ b/app/services/hyrax/compound_work_resolver.rb @@ -22,13 +22,35 @@ def self.title_and_path(id) [title_for(id), path_for(id)] end + ## + # Resolve an internal work id to its display title and show path, but only + # when a matching work is actually indexed. Returns nil when nothing + # matches, so callers can render a bare, unlinked value rather than a broken + # link to a non-existent work. + # + # @param id [String] + # @return [Array(String, String), nil] `[title, path]`, or nil when unresolved + def self.resolve(id) + title = indexed_title_for(id) + return nil if title.nil? + [title, path_for(id)] + end + def self.title_for(id) + indexed_title_for(id) || id.to_s + end + + # The indexed title for a work id, or nil when no such work is indexed + # (distinguishes "resolved to a real work" from "no match"). + def self.indexed_title_for(id) doc = Hyrax::SolrService.query("{!field f=id}#{id}", fl: 'title_tesim', rows: 1).first - Array(doc&.fetch('title_tesim', nil)).first.presence || id.to_s + return nil if doc.nil? + Array(doc['title_tesim']).first.presence || id.to_s rescue StandardError => e - Hyrax.logger.debug("CompoundWorkResolver.title_for(#{id}): #{e.message}") - id.to_s + Hyrax.logger.debug("CompoundWorkResolver.indexed_title_for(#{id}): #{e.message}") + nil end + private_class_method :indexed_title_for # The model-agnostic Blacklight show route (`/catalog/:id`) links any # indexed work without knowing its class. diff --git a/spec/renderers/hyrax/renderers/compound_attribute_renderer_spec.rb b/spec/renderers/hyrax/renderers/compound_attribute_renderer_spec.rb index f2b04e88e9..d95811f1f7 100644 --- a/spec/renderers/hyrax/renderers/compound_attribute_renderer_spec.rb +++ b/spec/renderers/hyrax/renderers/compound_attribute_renderer_spec.rb @@ -113,12 +113,12 @@ end end - context 'when the value is an internal work id' do + context 'when the value resolves to an indexed work' do let(:values) { [{ 'related_item' => 'work-123' }] } let(:renderer) { described_class.new(:relationships, values, label: 'Relationships', html_dl: true, subfields: subfields) } before do - allow(Hyrax::CompoundWorkResolver).to receive(:title_and_path) + allow(Hyrax::CompoundWorkResolver).to receive(:resolve) .with('work-123').and_return(['Linked Work', '/catalog/work-123']) end @@ -128,5 +128,18 @@ expect(markup).to include('Linked Work') end end + + context 'when the value is neither a URL nor a resolvable work' do + let(:values) { [{ 'related_item' => 'not-a-real-id' }] } + let(:renderer) { described_class.new(:relationships, values, label: 'Relationships', html_dl: true, subfields: subfields) } + + before { allow(Hyrax::CompoundWorkResolver).to receive(:resolve).with('not-a-real-id').and_return(nil) } + + it 'renders the value as plain text without a link' do + markup = renderer.render_dl_row + expect(markup).to include('not-a-real-id') + expect(markup).not_to include(' ['Resolved Title'] }]) + expect(described_class.resolve(id)).to eq(['Resolved Title', '/catalog/work-123']) + end + + it 'returns nil when no work matches (so the caller renders plain text)' do + allow(Hyrax::SolrService).to receive(:query).and_return([]) + expect(described_class.resolve(id)).to be_nil + end + + it 'returns nil when the Solr lookup raises' do + allow(Hyrax::SolrService).to receive(:query).and_raise(StandardError) + expect(described_class.resolve(id)).to be_nil + end + end end From 62d1e9040e37c9a1d385b2092549409ec0e29b45 Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Thu, 4 Jun 2026 20:35:19 -0400 Subject: [PATCH 14/32] Drop duplicate label from compound cards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A compound rendered as a card showed its label twice — once as the card title and again as the field label inside the body. The card body now renders the entry values only, via a new value_only option on attribute_to_html, leaving the card title as the single label. --- app/presenters/hyrax/presents_attributes.rb | 12 +++++++++--- app/views/hyrax/compounds/_compound_card.html.erb | 7 +++---- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/app/presenters/hyrax/presents_attributes.rb b/app/presenters/hyrax/presents_attributes.rb index 7f35f546cb..54925cb013 100644 --- a/app/presenters/hyrax/presents_attributes.rb +++ b/app/presenters/hyrax/presents_attributes.rb @@ -13,6 +13,9 @@ module PresentsAttributes # @option options [String] :label The default label for the field if no translation is found # @option options [TrueClass, FalseClass] :include_empty should we display a row if there are no values? # @option options [String] :work_type name of work type class (e.g., "GenericWork") + # @option options [TrueClass, FalseClass] :value_only render only the value + # markup, without the field-label row — used by compound cards, which + # already show the label as the card title. def attribute_to_html(field, options = {}) unless respond_to?(field) Hyrax.logger.warn("#{self.class} attempted to render #{field}, but no method exists with that name.") @@ -20,11 +23,14 @@ def attribute_to_html(field, options = {}) end options = options.merge(subfields: compound_subfields_for(field)) if options[:render_as].to_s == 'compound' + renderer = renderer_for(field, options).new(field, send(field), options) - if options[:html_dl] - renderer_for(field, options).new(field, send(field), options).render_dl_row + if options[:value_only] && renderer.respond_to?(:render_value) + renderer.render_value + elsif options[:html_dl] + renderer.render_dl_row else - renderer_for(field, options).new(field, send(field), options).render + renderer.render end end diff --git a/app/views/hyrax/compounds/_compound_card.html.erb b/app/views/hyrax/compounds/_compound_card.html.erb index 05ad41c465..337600000f 100644 --- a/app/views/hyrax/compounds/_compound_card.html.erb +++ b/app/views/hyrax/compounds/_compound_card.html.erb @@ -12,9 +12,8 @@

<%= compound_field_label(field) %>

-
- <%= presenter.attribute_to_html(field.to_sym, render_as: :compound, html_dl: true, - label: compound_field_label(field)) %> -
+ <%# value_only: the compound's label is already the card title above, so + render just the entries (no duplicate field label). %> + <%= presenter.attribute_to_html(field.to_sym, render_as: :compound, value_only: true) %>
From ff1b7e90164a5a5cad0130de3f5e7f87309f4648 Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Thu, 4 Jun 2026 22:18:59 -0400 Subject: [PATCH 15/32] Align collection metadata values past long URLs A long unbreakable value such as a rights-statement or license URL link was shrinking the flex label column and shifting the value column left, so compound rows no longer lined up with the rows above them. Hold the label column fixed and let the value column shrink so long URLs wrap within it. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/assets/stylesheets/hyrax/_collections.scss | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/app/assets/stylesheets/hyrax/_collections.scss b/app/assets/stylesheets/hyrax/_collections.scss index 3fe27c0bff..98edae4e35 100644 --- a/app/assets/stylesheets/hyrax/_collections.scss +++ b/app/assets/stylesheets/hyrax/_collections.scss @@ -610,11 +610,18 @@ button.branding-banner-remove:hover { dt { flex-basis: 115px; + // Hold the label column fixed so every value column (dd) starts at the + // same x; otherwise a long unbreakable value (e.g. a URL link) shrinks + // this column and shifts the value left. + flex-shrink: 0; font-weight: bold; } dd { flex-basis: auto; + // Let the value column shrink to its flex space so long URLs wrap within + // it rather than forcing the row wider. + min-width: 0; } } From 032264b64dddeae53aef19746d8ae7974f527fcd Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Thu, 4 Jun 2026 22:19:31 -0400 Subject: [PATCH 16/32] Improve compound metadata show rendering - Compounds use a declared display_label, resolved the same way ordinary properties are (the rights sample now shows "Rights"); the card title uses it and no longer duplicates the field label. - A controlled sub-field whose value is a URI (rights statement, license) links the term to that URI; a work-or-url value that resolves to no indexed work renders as plain text instead of a broken link. - Inline compound values align with the surrounding metadata: work-page values share the ul.tabular wrapper, and collection values resolve their definition from the document so they render correctly in flexible mode. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../stylesheets/hyrax/_compound_metadata.scss | 3 --- app/helpers/hyrax/collections_helper.rb | 19 ++++++++------- app/helpers/hyrax/compound_fields_helper.rb | 24 +++++++++++++++---- .../renderers/compound_attribute_renderer.rb | 21 +++++++++++++++- app/services/hyrax/compound_schema.rb | 11 ++++++++- .../hyrax/compounds/_compound_card.html.erb | 2 +- config/metadata/compound_metadata.yaml | 2 ++ .../hyrax/compound_fields_helper_spec.rb | 10 ++++++++ .../compound_attribute_renderer_spec.rb | 19 +++++++++++++++ spec/services/hyrax/compound_schema_spec.rb | 11 +++++++++ 10 files changed, 104 insertions(+), 18 deletions(-) diff --git a/app/assets/stylesheets/hyrax/_compound_metadata.scss b/app/assets/stylesheets/hyrax/_compound_metadata.scss index 1f46da0218..15e54343b7 100644 --- a/app/assets/stylesheets/hyrax/_compound_metadata.scss +++ b/app/assets/stylesheets/hyrax/_compound_metadata.scss @@ -9,9 +9,6 @@ display: block !important; flex-direction: column; border: 0 !important; - // Sit flush with the value column. No indent of our own: scalar values get - // theirs from a `ul.tabular` wrapper that apps and themes style differently, - // so a fixed pad here would misalign in some of them. padding: 0 !important; max-width: 100%; diff --git a/app/helpers/hyrax/collections_helper.rb b/app/helpers/hyrax/collections_helper.rb index 31786029a9..6fb60d1c79 100644 --- a/app/helpers/hyrax/collections_helper.rb +++ b/app/helpers/hyrax/collections_helper.rb @@ -30,6 +30,11 @@ def available_parent_collections_data(collection:) # @since 3.0.0 # @return [#to_s] def collection_metadata_label(collection, field) + if collection.respond_to?(:compound_term?) && collection.compound_term?(field) + definition = compound_schema_for(collection).definition_for(field) + return compound_field_label(field, display_label: definition&.fetch(:display_label, nil)) + end + Hyrax::PresenterRenderer.new(collection, self).label(field) end @@ -37,16 +42,14 @@ def collection_metadata_label(collection, field) # @since 3.0.0 # @return [#to_s] def collection_metadata_value(collection, field) - # Compounds render through the shared CompoundAttributeRenderer. The show - # partials already wrap label/value in
/
, so render only the entry - # rows; pass the sub-field specs so controlled ids display as their term. - # See documentation/forms/compound_fields.md. + # Compounds render through the shared CompoundAttributeRenderer, resolving + # the definition from the presenter's document (correct in flexible mode). + # Collection scalar values render as bare spans (no `ul.tabular`), so the + # entries render flush to match. See documentation/forms/compound_fields.md. if collection.respond_to?(:compound_term?) && collection.compound_term?(field) - definition = Hyrax::CompoundSchema.for(Hyrax.config.collection_class).definition_for(field) + definition = compound_schema_for(collection).definition_for(field) return Hyrax::Renderers::CompoundAttributeRenderer - .new(field, collection[field], - label: collection_metadata_label(collection, field), - subfields: definition&.fetch(:subfields, nil)) + .new(field, collection[field], subfields: definition&.fetch(:subfields, nil)) .render_value end diff --git a/app/helpers/hyrax/compound_fields_helper.rb b/app/helpers/hyrax/compound_fields_helper.rb index 8ea7a5b305..6e8c5f0635 100644 --- a/app/helpers/hyrax/compound_fields_helper.rb +++ b/app/helpers/hyrax/compound_fields_helper.rb @@ -19,7 +19,7 @@ def render_compound_field(f, compound_name) f: f, compound_name: compound_name.to_sym, definition: definition, - display_label: compound_field_label(compound_name) + display_label: compound_field_label(compound_name, display_label: definition[:display_label]) end ## @@ -100,9 +100,25 @@ def compound_work_or_url_option(value) [title, value.to_s] end - def compound_field_label(compound_name) - t("hyrax.compound_fields.#{compound_name}.label", - default: compound_name.to_s.humanize) + # The label for a compound. A declared `display_label` is resolved through + # the same path ordinary properties use ({Hyrax::AttributesHelper#conform_options}); + # otherwise it falls back to the `hyrax.compound_fields..label` key. + # + # @param display_label [Hash, nil] the compound's `{ locale => label }` hash + def compound_field_label(compound_name, display_label: nil) + if display_label.present? + conform_options(compound_name.to_s, display_label: display_label)[:label] + else + t("hyrax.compound_fields.#{compound_name}.label", default: compound_name.to_s.humanize) + end + rescue StandardError + t("hyrax.compound_fields.#{compound_name}.label", default: compound_name.to_s.humanize) + end + + # The card title for a compound on a show page (resolves the compound's + # declared display_label from the presenter's schema). + def compound_card_label(presenter, field) + compound_field_label(field, display_label: compound_schema_for(presenter).definition_for(field)&.dig(:display_label)) end def compound_subfield_label(compound_name, sub_field) diff --git a/app/renderers/hyrax/renderers/compound_attribute_renderer.rb b/app/renderers/hyrax/renderers/compound_attribute_renderer.rb index f19b2fae10..d474ef5ca5 100644 --- a/app/renderers/hyrax/renderers/compound_attribute_renderer.rb +++ b/app/renderers/hyrax/renderers/compound_attribute_renderer.rb @@ -18,7 +18,11 @@ def render def render_dl_row return '' if blank_values? && !options[:include_empty] - %(
#{label}
\n
#{entries_markup}
).html_safe + # Wrap in the same `ul.tabular` structure ordinary fields use, so inline + # compound values inherit the same indentation as the surrounding + # metadata (whatever an app/theme styles `ul.tabular` to) rather than a + # fixed pad that misaligns downstream. + %(
#{label}
\n
  • #{entries_markup}
).html_safe end # Just the entry rows, without a surrounding label/row wrapper. Used by @@ -70,11 +74,26 @@ def value_markup(sub_field, value) auto_link(ERB::Util.h(value.to_s)) when 'work_or_url' work_or_url_markup(value) + when 'controlled' + controlled_markup(sub_field, value) else ERB::Util.h(display_value(sub_field, value)) end end + # A controlled value displays its authority/value-list term. When the + # stored value is itself a linkable URI (e.g. a rights-statement or license + # URI), link the term to that URI — mirroring the ordinary rights/license + # renderer. Non-URI controlled values (e.g. inline option ids) stay plain. + def controlled_markup(sub_field, value) + label = display_value(sub_field, value) + if Hyrax::AuthorityRenderingHelper.linkable_uri?(value) + %(
#{ERB::Util.h(label)}).html_safe + else + ERB::Util.h(label) + end + end + # Link a URL or a resolvable work; render anything else as plain text so # we never emit a broken link to a non-existent work. def work_or_url_markup(value) diff --git a/app/services/hyrax/compound_schema.rb b/app/services/hyrax/compound_schema.rb index e1f89f3650..d45fe1ece6 100644 --- a/app/services/hyrax/compound_schema.rb +++ b/app/services/hyrax/compound_schema.rb @@ -203,7 +203,16 @@ def normalize(config, subfields) { subfields: sub, groups: normalize_groups(config['groups'], sub.keys), display_mode: display_mode, - required: compound_required?(config) } + required: compound_required?(config), + display_label: normalize_display_label(config) } + end + + # The declared display label as `{ locale => String }` (the m3 + # `display_label` shape), or nil when none is declared. + def normalize_display_label(config) + raw = config['display_label'] + return nil if raw.blank? + raw.is_a?(Hash) ? raw.with_indifferent_access : { default: raw.to_s }.with_indifferent_access end def normalize_subfield(opts) diff --git a/app/views/hyrax/compounds/_compound_card.html.erb b/app/views/hyrax/compounds/_compound_card.html.erb index 337600000f..24f70ce0d1 100644 --- a/app/views/hyrax/compounds/_compound_card.html.erb +++ b/app/views/hyrax/compounds/_compound_card.html.erb @@ -9,7 +9,7 @@ %>
-

<%= compound_field_label(field) %>

+

<%= compound_card_label(presenter, field) %>

<%# value_only: the compound's label is already the card title above, so diff --git a/config/metadata/compound_metadata.yaml b/config/metadata/compound_metadata.yaml index 1bc6141ebe..2e4fafb90e 100644 --- a/config/metadata/compound_metadata.yaml +++ b/config/metadata/compound_metadata.yaml @@ -72,6 +72,8 @@ attributes: type: hash multiple: true predicate: http://purl.org/dc/terms/rights + display_label: + default: Rights form: primary: false subfields: diff --git a/spec/helpers/hyrax/compound_fields_helper_spec.rb b/spec/helpers/hyrax/compound_fields_helper_spec.rb index 097bcf4485..e73986d934 100644 --- a/spec/helpers/hyrax/compound_fields_helper_spec.rb +++ b/spec/helpers/hyrax/compound_fields_helper_spec.rb @@ -44,6 +44,16 @@ end end + describe '#compound_field_label' do + it 'uses a declared display_label' do + expect(helper.compound_field_label(:compound_rights, display_label: { 'default' => 'Rights' })).to eq('Rights') + end + + it 'falls back to a humanized name when no display_label or translation exists' do + expect(helper.compound_field_label(:nonexistent_compound)).to eq('Nonexistent compound') + end + end + describe 'card display' do # A schema whose only card compound is :relationships. let(:schema) do diff --git a/spec/renderers/hyrax/renderers/compound_attribute_renderer_spec.rb b/spec/renderers/hyrax/renderers/compound_attribute_renderer_spec.rb index d95811f1f7..8d505874dc 100644 --- a/spec/renderers/hyrax/renderers/compound_attribute_renderer_spec.rb +++ b/spec/renderers/hyrax/renderers/compound_attribute_renderer_spec.rb @@ -82,6 +82,25 @@ end end + describe 'controlled sub-field with a linkable URI value' do + let(:uri) { 'http://rightsstatements.org/vocab/InC/1.0/' } + let(:values) { [{ 'rights_statement' => uri }] } + let(:subfields) do + { 'rights_statement' => { type: 'controlled', authority: 'rights_statements', values: nil } } + end + let(:renderer) { described_class.new(:compound_rights, values, label: 'Rights', html_dl: true, subfields: subfields) } + + before do + allow(Hyrax::CompoundSubfieldLabeler).to receive(:label_for) + .with(subfields['rights_statement'], uri).and_return('In Copyright') + end + + it 'links the resolved term to its URI' do + markup = renderer.render_dl_row + expect(markup).to include(%(In Copyright)) + end + end + describe 'url sub-field auto-linking' do let(:values) { [{ 'related_item_url' => 'https://example.org/item/42', 'note' => 'see also' }] } let(:subfields) do diff --git a/spec/services/hyrax/compound_schema_spec.rb b/spec/services/hyrax/compound_schema_spec.rb index 2848bee7a0..43661427a3 100644 --- a/spec/services/hyrax/compound_schema_spec.rb +++ b/spec/services/hyrax/compound_schema_spec.rb @@ -40,6 +40,7 @@ def self.name 'related_item' => { 'type' => 'work_or_url' }, 'relationship_type' => { 'type' => 'controlled', 'authority' => 'relationship_type' } }, + display_label: { 'default' => 'Related works' }, view: { 'display' => 'card' } ) # Required at the compound level (non-flexible `required: true`) with two @@ -110,6 +111,16 @@ def self.name end end + describe 'display_label' do + it 'normalizes a declared display_label to a locale hash' do + expect(schema.definition_for(:relationships)[:display_label]).to eq('default' => 'Related works') + end + + it 'is nil when none is declared' do + expect(schema.definition_for(:contributors)[:display_label]).to be_nil + end + end + describe 'required declarations' do describe '#required?' do it 'is true when the compound declares required: true' do From d1458c078c14cc4e0c33f89d9149a262d2d6d9f9 Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Fri, 5 Jun 2026 17:42:23 -0400 Subject: [PATCH 17/32] Dynamic solr doc method creation for compounds The SolrDocument now exposes a reader for every compound in its active schema, resolved dynamically. An application that declares its own compounds renders them on the show page with no per-document setup. --- .../concerns/hyrax/solr_document/metadata.rb | 52 +++++++++++++++++-- documentation/forms/compound_fields.md | 6 ++- 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/app/models/concerns/hyrax/solr_document/metadata.rb b/app/models/concerns/hyrax/solr_document/metadata.rb index 2b438b631d..6c6edbd5a6 100644 --- a/app/models/concerns/hyrax/solr_document/metadata.rb +++ b/app/models/concerns/hyrax/solr_document/metadata.rb @@ -20,6 +20,51 @@ def compound_attribute(name) end end + # Resolve a reader for any compound on the document, derived from the + # stored data rather than a hardcoded list. The compound set is + # schema-driven (see {Hyrax::CompoundSchema}) and, in flexible mode, + # per-document — so it cannot be enumerated at class-load time. A name that + # has the `_json_ss` blob the compound indexer writes is parsed back + # into its rows. This keeps the show page working for an application's own + # compounds with no per-app declaration. + def method_missing(name, *args, &block) + return super unless args.empty? && block.nil? + return super unless compound_attribute?(name) + + Solr::CompoundEntries.coerce(self["#{name}_json_ss"]) + end + + def respond_to_missing?(name, include_private = false) + compound_attribute?(name) || super + end + + # A name is a dynamic compound reader when either the document carries the + # `_json_ss` blob the compound indexer writes (the fast path, no + # schema load), or the document's active schema declares it as a compound. + # The schema check is what lets a *declared but empty* compound still + # answer (returning `[]`), which callers such as the collection presenter's + # `terms_with_values` rely on. The schema-name set is memoized per document + # and guarded so resolving it never recurses back through method_missing. + def compound_attribute?(name) + return false if @resolving_compound_names + return true if key?("#{name}_json_ss") + + schema_compound_names.include?(name.to_sym) + end + private :compound_attribute? + + def schema_compound_names + return @schema_compound_names if defined?(@schema_compound_names) + + @resolving_compound_names = true + @schema_compound_names = Hyrax::CompoundSchema.for_solr_document(self).compound_names + rescue StandardError + @schema_compound_names = [] + ensure + @resolving_compound_names = false + end + private :schema_compound_names + module Solr class Array # @return [Array] @@ -124,11 +169,8 @@ def alt_text_for_view attribute :modified_date, Solr::Date, "system_modified_dtsi" attribute :embargo_release_date, Solr::Date, Hydra.config.permissions.embargo.release_date attribute :lease_expiration_date, Solr::Date, Hydra.config.permissions.lease.expiration_date - # Sample compound attributes shipped with Hyrax. - compound_attribute :agents - compound_attribute :identifiers - compound_attribute :compound_rights - compound_attribute :relationships + # Compound readers are resolved dynamically per document (see + # #method_missing); they are schema-driven, not enumerated here. end end end diff --git a/documentation/forms/compound_fields.md b/documentation/forms/compound_fields.md index b8a64e387b..8d098a3973 100644 --- a/documentation/forms/compound_fields.md +++ b/documentation/forms/compound_fields.md @@ -234,8 +234,10 @@ or override the samples. Because the show page renders from Solr (not the live resource), the indexer also stores each compound's ordered rows (limited to sub-fields with `display:` -not false) as a single JSON field (`_json_ss`). The SolrDocument exposes a `` reader that -parses it back into an array of hashes, and the generic +not false) as a single JSON field (`_json_ss`). The SolrDocument +exposes a `` reader for any compound that has this blob — resolved +dynamically, so an application's own compounds work with no per-document +declaration — that parses it back into an array of hashes, and the generic `Hyrax::Renderers::CompoundAttributeRenderer` (selected by `view: render_as: compound`) renders each entry's populated sub-fields as a small definition list. Works render compounds through the standard `attribute_to_html` / From 803338e3598a36c30a7142deb93555a230e670a8 Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Fri, 5 Jun 2026 17:44:22 -0400 Subject: [PATCH 18/32] Rename the sample agents compound to participants The shipped sample compound `agents` is now `participants`. Agents as a term has too many other uses and meanings. Additionally, it also refers to sipity agents, permissions agents, Fedora ACl agents, etc, meaning it could create a collision of terms & break the app. --- .../hyrax/compound_entry_validator.rb | 3 +- config/locales/hyrax.en.yml | 8 +-- config/metadata/compound_metadata.yaml | 17 +++--- config/metadata_profiles/m3_profile.yaml | 14 ++--- documentation/forms/compound_fields.md | 5 +- .../forms/compound_metadata_form_spec.rb | 60 +++++++++---------- .../hyrax/collection_presenter_spec.rb | 4 +- 7 files changed, 56 insertions(+), 55 deletions(-) diff --git a/app/validators/hyrax/compound_entry_validator.rb b/app/validators/hyrax/compound_entry_validator.rb index 352679c34f..eea7da423e 100644 --- a/app/validators/hyrax/compound_entry_validator.rb +++ b/app/validators/hyrax/compound_entry_validator.rb @@ -37,7 +37,8 @@ def validate_compound(record, name, definition) # Attach to :base, not the attribute, because the work and collection # forms render errors differently (raw messages vs. full_messages): # keying on the attribute would either double the field name - # ("Agents Agents ...") or print the raw key ("agents ..."). A :base + # ("Participants Participants ...") or print the raw key + # ("participants ..."). A :base # error renders verbatim everywhere, and the message names the compound. record.errors.add(:base, message_for(name, violation)) end diff --git a/config/locales/hyrax.en.yml b/config/locales/hyrax.en.yml index 3938c7294b..14f6d5498e 100644 --- a/config/locales/hyrax.en.yml +++ b/config/locales/hyrax.en.yml @@ -865,11 +865,11 @@ en: errors: required_but_empty: "%{compound} requires at least one entry" missing_required_subfields: "Each %{compound} entry requires: %{fields}" - agents: - label: Agents + participants: + label: Participants title: Title - agent_name: Name - agent_role: Role + participant_name: Name + participant_role: Role identifiers: label: Identifiers identifier: Identifier diff --git a/config/metadata/compound_metadata.yaml b/config/metadata/compound_metadata.yaml index 2e4fafb90e..c62dc16793 100644 --- a/config/metadata/compound_metadata.yaml +++ b/config/metadata/compound_metadata.yaml @@ -12,10 +12,9 @@ # app-level schema, or the m3 profile under HYRAX_FLEXIBLE=true) and may remove # these samples by setting `Hyrax.config.compound_metadata_enabled = false`. attributes: - # Agents associated with the work: a person or organization with a display - # title, a name, and a controlled role. (Named `agents` to avoid colliding - # with the internal ACL `agent` attribute in hyrax_internal_metadata.yaml.) - agents: + # Participants associated with the work: a person or organization with a + # display title, a name, and a controlled role. + participants: type: hash multiple: true predicate: http://id.loc.gov/vocabulary/relators/asn @@ -27,19 +26,19 @@ attributes: # no `index_keys:` is display-only (rendered from the json blob, not its # own searchable field). `display: false` would omit it from the blob. title: { type: string } - agent_name: + participant_name: type: string required: true - index_keys: [agent_name_sim, agent_name_tesim] + index_keys: [participant_name_sim, participant_name_tesim] # A controlled sub-field can take its options from an inline list (below) # or from an existing QA local authority (`authority: `). - agent_role: + participant_role: type: controlled required: true values: [Author, Editor, Contributor, Creator, Data Collector, Funder, Publisher, Other] - index_keys: [agent_role_sim, agent_role_tesim] + index_keys: [participant_role_sim, participant_role_tesim] groups: - - { label: ~, cols: 4, fields: [title, agent_name, agent_role] } + - { label: ~, cols: 4, fields: [title, participant_name, participant_role] } view: render_as: compound html_dl: true diff --git a/config/metadata_profiles/m3_profile.yaml b/config/metadata_profiles/m3_profile.yaml index 61b8de53e7..1bb100d144 100644 --- a/config/metadata_profiles/m3_profile.yaml +++ b/config/metadata_profiles/m3_profile.yaml @@ -956,7 +956,7 @@ properties: # `type: hash, multiple: true` property whose entries are hashes of the # `subfields:` keys, rendered/populated/indexed generically by the compound # foundation. See documentation/forms/compound_fields.md. - agents: + participants: available_on: class: - Hyrax::Work @@ -965,7 +965,7 @@ properties: type: hash range: http://www.w3.org/2001/XMLSchema#string display_label: - default: Agents + default: Participants form: primary: false property_uri: http://id.loc.gov/vocabulary/relators/asn @@ -973,17 +973,17 @@ properties: # Indexing per sub-field via `indexing:` (literal Solr field names), the # same mechanism scalar properties use. No `indexing:` => display-only. title: { type: string } - agent_name: + participant_name: type: string required: true - indexing: [agent_name_sim, agent_name_tesim] - agent_role: + indexing: [participant_name_sim, participant_name_tesim] + participant_role: type: controlled required: true values: [Author, Editor, Contributor, Creator, Data Collector, Funder, Publisher, Other] - indexing: [agent_role_sim, agent_role_tesim] + indexing: [participant_role_sim, participant_role_tesim] groups: - - { label: ~, cols: 4, fields: [title, agent_name, agent_role] } + - { label: ~, cols: 4, fields: [title, participant_name, participant_role] } view: render_as: compound html_dl: true diff --git a/documentation/forms/compound_fields.md b/documentation/forms/compound_fields.md index 8d098a3973..a364cedb46 100644 --- a/documentation/forms/compound_fields.md +++ b/documentation/forms/compound_fields.md @@ -210,8 +210,9 @@ end Hyrax ships sample compounds on **works and collections** by default, demonstrating the supported sub-field types: -- **`agents`** — `title`, `agent_name`, and a controlled `agent_role` (inline - `values:`). A person or organization associated with the work or collection. +- **`participants`** — `title`, `participant_name`, and a controlled + `participant_role` (inline `values:`). A person or organization associated + with the work or collection. - **`identifiers`** — an open-entry `identifier` plus a controlled `identifier_type` (inline `values:`). - **`compound_rights`** — controlled `rights_statement` and `license` backed by diff --git a/spec/forms/hyrax/forms/compound_metadata_form_spec.rb b/spec/forms/hyrax/forms/compound_metadata_form_spec.rb index 4bcd4fe3c2..f955af1f6b 100644 --- a/spec/forms/hyrax/forms/compound_metadata_form_spec.rb +++ b/spec/forms/hyrax/forms/compound_metadata_form_spec.rb @@ -41,44 +41,44 @@ describe 'property registration' do it 'exposes the compound readers so the form partial can render existing rows' do - expect(form).to respond_to(:agents) + expect(form).to respond_to(:participants) expect(form).to respond_to(:identifiers) end it 'exposes the virtual `_attributes` setters for nested form params' do - expect(form).to respond_to(:agents_attributes=) + expect(form).to respond_to(:participants_attributes=) expect(form).to respond_to(:identifiers_attributes=) end it 'lists the compounds as compound_terms (and keeps them out of primary/secondary terms)' do - expect(form.compound_terms).to include(:agents, :identifiers, :compound_rights) - expect(form.primary_terms).not_to include(:agents, :identifiers, :compound_rights) - expect(form.secondary_terms).not_to include(:agents, :identifiers, :compound_rights) + expect(form.compound_terms).to include(:participants, :identifiers, :compound_rights) + expect(form.primary_terms).not_to include(:participants, :identifiers, :compound_rights) + expect(form.secondary_terms).not_to include(:participants, :identifiers, :compound_rights) end - it 'registers `agents_attributes` as a real Reform property (not just method-missing)' do + it 'registers `participants_attributes` as a real Reform property (not just method-missing)' do # This is the property that has to exist in Reform's schema *before* # validate runs; if it is registered too late the populator never fires. # In non-flexible mode it is registered on the form class via FormFields; # check the class definitions registry (the same lens the redirects # coverage uses). definition_keys = form.class.definitions.keys.map(&:to_s) - expect(definition_keys).to include('agents_attributes') + expect(definition_keys).to include('participants_attributes') end end describe 'validate + sync writes the compound to the model' do let(:params) do - { 'agents_attributes' => - { '0' => { 'title' => 'Dr', 'agent_name' => 'Ada Lovelace', 'agent_role' => 'Author' } }, + { 'participants_attributes' => + { '0' => { 'title' => 'Dr', 'participant_name' => 'Ada Lovelace', 'participant_role' => 'Author' } }, 'identifiers_attributes' => { '0' => { 'identifier' => '10.1234/x', 'identifier_type' => 'DOI' } } } end it 'populates the compound on the form during validate' do form.validate(params) - expect(form.agents) - .to eq([{ 'title' => 'Dr', 'agent_name' => 'Ada Lovelace', 'agent_role' => 'Author' }]) + expect(form.participants) + .to eq([{ 'title' => 'Dr', 'participant_name' => 'Ada Lovelace', 'participant_role' => 'Author' }]) expect(form.identifiers) .to eq([{ 'identifier' => '10.1234/x', 'identifier_type' => 'DOI' }]) end @@ -86,26 +86,26 @@ it 'writes the compound through to the model on sync' do form.validate(params) form.sync - expect(form.model.agents) - .to eq([{ 'title' => 'Dr', 'agent_name' => 'Ada Lovelace', 'agent_role' => 'Author' }]) + expect(form.model.participants) + .to eq([{ 'title' => 'Dr', 'participant_name' => 'Ada Lovelace', 'participant_role' => 'Author' }]) expect(form.model.identifiers) .to eq([{ 'identifier' => '10.1234/x', 'identifier_type' => 'DOI' }]) end it 'drops `_destroy` and all-blank rows' do - form.validate('agents_attributes' => - { '0' => { 'agent_name' => 'Keep', 'agent_role' => 'Author' }, - '1' => { 'agent_name' => 'Remove', '_destroy' => 'true' }, - '2' => { 'title' => '', 'agent_name' => '', 'agent_role' => '' } }) + form.validate('participants_attributes' => + { '0' => { 'participant_name' => 'Keep', 'participant_role' => 'Author' }, + '1' => { 'participant_name' => 'Remove', '_destroy' => 'true' }, + '2' => { 'title' => '', 'participant_name' => '', 'participant_role' => '' } }) form.sync - expect(form.model.agents).to eq([{ 'title' => nil, 'agent_name' => 'Keep', 'agent_role' => 'Author' }]) + expect(form.model.participants).to eq([{ 'title' => nil, 'participant_name' => 'Keep', 'participant_role' => 'Author' }]) end end describe 'required-subfield validation blocks save' do # Exercises the real form + Hyrax::CompoundEntryValidator end to end against # the *shipped* compound schema (no stub). The shipped samples require - # sub-fields within a row (e.g. agents needs agent_name + agent_role, + # sub-fields within a row (e.g. participants needs participant_name + participant_role, # relationships needs related_item + relationship_type) but none is required # at the compound level, so an empty compound is valid. Asserting through the # real schema is what catches a validator that reads the form wrapper instead @@ -119,13 +119,13 @@ def base_errors(form) end it 'allows an empty compound (none is required at the compound level)' do - form.validate('agents_attributes' => { '_marker' => { '_destroy' => '1' } }) + form.validate('participants_attributes' => { '_marker' => { '_destroy' => '1' } }) expect(base_errors(form)).to be_empty end - it 'flags an agents row that omits a required sub-field' do - form.validate('agents_attributes' => { '0' => { 'agent_role' => 'Author' } }) - expect(base_errors(form)).to include(a_string_including('Agents')) + it 'flags a participants row that omits a required sub-field' do + form.validate('participants_attributes' => { '0' => { 'participant_role' => 'Author' } }) + expect(base_errors(form)).to include(a_string_including('Participants')) end it 'flags a relationships row that omits a required sub-field' do @@ -133,13 +133,13 @@ def base_errors(form) expect(base_errors(form)).to include(a_string_including('Relationships')) end - it 'does not flag agents when its required sub-fields are filled' do - form.validate('agents_attributes' => { '0' => { 'agent_name' => 'Ada', 'agent_role' => 'Author' } }) - expect(base_errors(form)).not_to include(a_string_including('Agents')) + it 'does not flag participants when its required sub-fields are filled' do + form.validate('participants_attributes' => { '0' => { 'participant_name' => 'Ada', 'participant_role' => 'Author' } }) + expect(base_errors(form)).not_to include(a_string_including('Participants')) end it 'never flags `compound_rights` (no required sub-fields)' do - form.validate('agents_attributes' => { '0' => { 'agent_name' => 'Ada', 'agent_role' => 'Author' } }) + form.validate('participants_attributes' => { '0' => { 'participant_name' => 'Ada', 'participant_role' => 'Author' } }) expect(base_errors(form)).not_to include(a_string_including('Rights')) end end @@ -151,13 +151,13 @@ def base_errors(form) skip 'requires a Valkyrie (non-Fedora) persister' if Hyrax.persister.is_a?(Wings::Valkyrie::Persister) - form.validate('agents_attributes' => - { '0' => { 'agent_name' => 'Grace Hopper', 'agent_role' => 'Editor' } }) + form.validate('participants_attributes' => + { '0' => { 'participant_name' => 'Grace Hopper', 'participant_role' => 'Editor' } }) form.sync saved = Hyrax.persister.save(resource: form.model) reloaded = Hyrax.query_service.find_by(id: saved.id) - expect(reloaded.agents.first['agent_name'] || reloaded.agents.first[:agent_name]) + expect(reloaded.participants.first['participant_name'] || reloaded.participants.first[:participant_name]) .to eq('Grace Hopper') end end diff --git a/spec/presenters/hyrax/collection_presenter_spec.rb b/spec/presenters/hyrax/collection_presenter_spec.rb index 2c18818651..773d68c36d 100644 --- a/spec/presenters/hyrax/collection_presenter_spec.rb +++ b/spec/presenters/hyrax/collection_presenter_spec.rb @@ -36,8 +36,8 @@ end it 'appends this collection\'s inline compound terms' do - allow(presenter).to receive(:compound_terms).and_return([:agents, :identifiers]) - expect(presenter.terms).to end_with(:agents, :identifiers) + allow(presenter).to receive(:compound_terms).and_return([:participants, :identifiers]) + expect(presenter.terms).to end_with(:participants, :identifiers) end context 'with compound metadata disabled' do From 4b52b20040c832449cec8f73f0b163beabdedb04 Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Fri, 5 Jun 2026 18:55:11 -0400 Subject: [PATCH 19/32] Define real SolrDocument readers for compounds The SolrDocument now defines a real reader method for each compound it carries (its `_json_ss` blob) when the document is built, so an application's own compounds render on the show page with no per-document declaration, in both flex modes. --- .../concerns/hyrax/solr_document/metadata.rb | 68 +++++++------------ documentation/forms/compound_fields.md | 6 +- spec/models/solr_document_spec.rb | 45 ++++++++++++ .../hyrax/compound_entry_validator_spec.rb | 15 ++-- 4 files changed, 82 insertions(+), 52 deletions(-) diff --git a/app/models/concerns/hyrax/solr_document/metadata.rb b/app/models/concerns/hyrax/solr_document/metadata.rb index 6c6edbd5a6..1e4e63ca85 100644 --- a/app/models/concerns/hyrax/solr_document/metadata.rb +++ b/app/models/concerns/hyrax/solr_document/metadata.rb @@ -20,50 +20,23 @@ def compound_attribute(name) end end - # Resolve a reader for any compound on the document, derived from the - # stored data rather than a hardcoded list. The compound set is - # schema-driven (see {Hyrax::CompoundSchema}) and, in flexible mode, - # per-document — so it cannot be enumerated at class-load time. A name that - # has the `_json_ss` blob the compound indexer writes is parsed back - # into its rows. This keeps the show page working for an application's own - # compounds with no per-app declaration. - def method_missing(name, *args, &block) - return super unless args.empty? && block.nil? - return super unless compound_attribute?(name) - - Solr::CompoundEntries.coerce(self["#{name}_json_ss"]) - end - - def respond_to_missing?(name, include_private = false) - compound_attribute?(name) || super - end - - # A name is a dynamic compound reader when either the document carries the - # `_json_ss` blob the compound indexer writes (the fast path, no - # schema load), or the document's active schema declares it as a compound. - # The schema check is what lets a *declared but empty* compound still - # answer (returning `[]`), which callers such as the collection presenter's - # `terms_with_values` rely on. The schema-name set is memoized per document - # and guarded so resolving it never recurses back through method_missing. - def compound_attribute?(name) - return false if @resolving_compound_names - return true if key?("#{name}_json_ss") - - schema_compound_names.include?(name.to_sym) - end - private :compound_attribute? - - def schema_compound_names - return @schema_compound_names if defined?(@schema_compound_names) - - @resolving_compound_names = true - @schema_compound_names = Hyrax::CompoundSchema.for_solr_document(self).compound_names - rescue StandardError - @schema_compound_names = [] - ensure - @resolving_compound_names = false + # Defines a reader for each compound on the document (its `_json_ss` + # blob, written by {Hyrax::Indexers::CompoundIndexer}). Uses a real method + # rather than #method_missing on purpose: a Blacklight/Solr document can + # answer to an arbitrary field name with a nil-returning accessor that + # would shadow #method_missing, so a real method is required to win. + def define_compound_readers! + keys.each do |key| + next unless key.to_s.end_with?('_json_ss') + + name = key.to_s.delete_suffix('_json_ss') + next if singleton_class.method_defined?(name) + + define_singleton_method(name) do + Solr::CompoundEntries.coerce(self["#{name}_json_ss"]) + end + end end - private :schema_compound_names module Solr class Array @@ -169,8 +142,13 @@ def alt_text_for_view attribute :modified_date, Solr::Date, "system_modified_dtsi" attribute :embargo_release_date, Solr::Date, Hydra.config.permissions.embargo.release_date attribute :lease_expiration_date, Solr::Date, Hydra.config.permissions.lease.expiration_date - # Compound readers are resolved dynamically per document (see - # #method_missing); they are schema-driven, not enumerated here. + + # Define compound readers per-instance so an application's own compounds + # render with no per-app declaration. See #define_compound_readers!. + def initialize(source_doc = {}, *args) + super + define_compound_readers! + end end end end diff --git a/documentation/forms/compound_fields.md b/documentation/forms/compound_fields.md index a364cedb46..6d0cb5d8e7 100644 --- a/documentation/forms/compound_fields.md +++ b/documentation/forms/compound_fields.md @@ -236,9 +236,9 @@ or override the samples. Because the show page renders from Solr (not the live resource), the indexer also stores each compound's ordered rows (limited to sub-fields with `display:` not false) as a single JSON field (`_json_ss`). The SolrDocument -exposes a `` reader for any compound that has this blob — resolved -dynamically, so an application's own compounds work with no per-document -declaration — that parses it back into an array of hashes, and the generic +defines a `` reader for each such blob it carries — so an +application's own compounds work with no per-document declaration — that +parses it back into an array of hashes, and the generic `Hyrax::Renderers::CompoundAttributeRenderer` (selected by `view: render_as: compound`) renders each entry's populated sub-fields as a small definition list. Works render compounds through the standard `attribute_to_html` / diff --git a/spec/models/solr_document_spec.rb b/spec/models/solr_document_spec.rb index f2c63cc3b9..cc82fd1427 100644 --- a/spec/models/solr_document_spec.rb +++ b/spec/models/solr_document_spec.rb @@ -241,4 +241,49 @@ class Mimes is_expected.to eq ['foo'] end end + + describe "compound readers" do + # A compound (hierarchical) attribute is indexed as a `_json_ss` + # blob. The document defines a reader per such field at construction, so an + # application's own compounds render without a per-app declaration and + # without depending on the (possibly stale) flexible schema. See + # documentation/forms/compound_fields.md. + let(:attributes) do + { 'contributors_json_ss' => + [%([{"given_name":"Grace","family_name":"Hopper"}])], + 'funding_references_json_ss' => + [%([{"funder_name":"NSF","award_number":"123"}])], + 'empty_compound_json_ss' => [] } + end + + it "defines a reader for each compound present on the document" do + expect(document).to respond_to(:contributors) + expect(document).to respond_to(:funding_references) + end + + it "defines the reader as a real method (not via method_missing)" do + # A real method is required so the reader wins over a same-named dynamic + # field accessor that would otherwise shadow it and return nil. + expect(document.singleton_class.method_defined?(:contributors)).to be true + expect(document.method(:contributors).owner).to eq(document.singleton_class) + end + + it "parses the stored rows back into an array of hashes" do + expect(document.contributors) + .to eq([{ 'given_name' => 'Grace', 'family_name' => 'Hopper' }]) + end + + it "handles multi-word compound names" do + expect(document.funding_references) + .to eq([{ 'funder_name' => 'NSF', 'award_number' => '123' }]) + end + + it "returns an empty array for a present-but-blank compound blob" do + expect(document.empty_compound).to eq([]) + end + + it "does not define a reader for a compound that is not on the document" do + expect(document).not_to respond_to(:nonexistent_compound) + end + end end diff --git a/spec/validators/hyrax/compound_entry_validator_spec.rb b/spec/validators/hyrax/compound_entry_validator_spec.rb index c9760a0799..afce3841f7 100644 --- a/spec/validators/hyrax/compound_entry_validator_spec.rb +++ b/spec/validators/hyrax/compound_entry_validator_spec.rb @@ -63,8 +63,12 @@ end it 'names the compound and the missing sub-field in the message' do - record.valid? - expect(record.errors[:base].first).to include('Relationships').and include('Relationship type') + # Assert the English wording under :en, so the test is independent of the + # suite's default locale (other locales need only the same key structure). + I18n.with_locale(:en) do + record.valid? + expect(record.errors[:base].first).to include('Relationships').and include('Relationship type') + end end end @@ -73,8 +77,11 @@ context 'and empty' do it 'flags the empty compound by name' do - record.valid? - expect(record.errors[:base].first).to include('Relationships').and include('at least one entry') + # English wording; see the note above on locale. + I18n.with_locale(:en) do + record.valid? + expect(record.errors[:base].first).to include('Relationships').and include('at least one entry') + end end end From d82788ad0afd29730871ab8d6603d5686ddcb541 Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Fri, 5 Jun 2026 19:52:55 -0400 Subject: [PATCH 20/32] Show collections with declared but empty compounds Collection show pages now render when a compound is declared for the collection but the record has no entries for it, instead of raising NoMethodError. The collection presenter lists every declared compound among its terms, but the SolrDocument only defines a reader for a compound that has indexed data. Reading a declared-but-empty compound now returns no entries rather than calling an undefined method. --- app/presenters/hyrax/collection_presenter.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/app/presenters/hyrax/collection_presenter.rb b/app/presenters/hyrax/collection_presenter.rb index 9837fc10e2..61c6a1fb08 100644 --- a/app/presenters/hyrax/collection_presenter.rb +++ b/app/presenters/hyrax/collection_presenter.rb @@ -119,10 +119,19 @@ def [](key) when :total_items total_items else + # A declared compound with no indexed `_json_ss` blob has no reader + # defined on the document; treat it as empty rather than raising. + return compound_value(key) if compound_term?(key) && !solr_document.respond_to?(key) solr_document.send key end end + # The rows for a declared-but-empty compound term, coerced from the absent + # blob exactly as the SolrDocument reader would (nil => []). + def compound_value(key) + Hyrax::SolrDocument::Metadata::Solr::CompoundEntries.coerce(solr_document["#{key}_json_ss"]) + end + # @deprecated to be removed in 4.0.0; this feature was replaced with a # hard-coded null implementation # @return [String] 'unknown' From a54d3f7abd4547e273409888de2b0321c89574ac Mon Sep 17 00:00:00 2001 From: LaRita Robinson Date: Fri, 5 Jun 2026 21:06:50 -0400 Subject: [PATCH 21/32] Rename compound "subfield" to "subproperty" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Compound (hierarchical) metadata now calls its nested members "subproperties" instead of "subfields" — the YAML key is `subproperties:`, and the supporting classes, methods, i18n keys, and CSS classes follow the same name. This is a vocabulary rename only; the declaration structure, the normalized internal shape, indexing, rendering, and validation are unchanged. It sets up a later change that expresses subproperties as composable, first-class properties. --- .../config/metadata_profiles/m3_profile.yaml | 8 +- .../javascripts/hyrax/compound_metadata.js | 2 +- .../stylesheets/hyrax/_compound_metadata.scss | 16 ++-- .../qa/authorities/compound_works.rb | 2 +- .../concerns/hyrax/compound_field_behavior.rb | 8 +- .../concerns/hyrax/flexible_form_behavior.rb | 6 +- app/forms/hyrax/forms/resource_form.rb | 2 +- app/helpers/hyrax/collections_helper.rb | 2 +- app/helpers/hyrax/compound_fields_helper.rb | 20 ++-- .../hyrax/indexers/compound_indexer.rb | 22 ++--- .../hyrax/indexers/pcdm_collection_indexer.rb | 2 +- .../hyrax/indexers/pcdm_object_indexer.rb | 2 +- .../concerns/hyrax/compound_normalization.rb | 2 +- app/presenters/hyrax/presents_attributes.rb | 12 +-- .../renderers/compound_attribute_renderer.rb | 54 +++++------ .../hyrax/compound_work_picker_builder.rb | 2 +- .../hyrax/compound_entry_validation.rb | 9 +- app/services/hyrax/compound_schema.rb | 41 ++++---- ...ler.rb => compound_subproperty_labeler.rb} | 12 +-- app/services/hyrax/compound_work_resolver.rb | 2 +- .../flexible_schema_validator_service.rb | 4 +- .../compound_validator.rb | 30 +++--- app/services/hyrax/schema_loader.rb | 2 +- .../hyrax/compound_entry_validator.rb | 7 +- .../hyrax/compounds/_compound_row.html.erb | 22 ++--- .../compounds/_compound_section.html.erb | 4 +- config/locales/hyrax.en.yml | 10 +- config/metadata/compound_metadata.yaml | 24 ++--- config/metadata_profiles/m3_profile.yaml | 12 +-- documentation/forms/compound_fields.md | 96 +++++++++---------- documentation/forms/field_behaviors.md | 12 +-- lib/hyrax/form_fields.rb | 8 +- .../hyrax/compound_field_behavior_spec.rb | 4 +- .../forms/compound_metadata_form_spec.rb | 12 +-- .../hyrax/compound_fields_helper_spec.rb | 26 ++--- .../hyrax/indexers/compound_indexer_spec.rb | 12 +-- .../hyrax/compound_normalization_spec.rb | 2 +- .../compound_attribute_renderer_spec.rb | 50 +++++----- .../hyrax/compound_entry_validation_spec.rb | 22 ++--- spec/services/hyrax/compound_schema_spec.rb | 62 ++++++------ ...b => compound_subproperty_labeler_spec.rb} | 4 +- .../compound_validator_spec.rb | 30 +++--- .../hyrax/compound_entry_validator_spec.rb | 10 +- 43 files changed, 347 insertions(+), 344 deletions(-) rename app/services/hyrax/{compound_subfield_labeler.rb => compound_subproperty_labeler.rb} (79%) rename spec/services/hyrax/{compound_subfield_labeler_spec.rb => compound_subproperty_labeler_spec.rb} (93%) diff --git a/.koppie/config/metadata_profiles/m3_profile.yaml b/.koppie/config/metadata_profiles/m3_profile.yaml index 083f7873d0..d8f58af69f 100644 --- a/.koppie/config/metadata_profiles/m3_profile.yaml +++ b/.koppie/config/metadata_profiles/m3_profile.yaml @@ -943,7 +943,7 @@ properties: form: primary: false property_uri: http://id.loc.gov/vocabulary/relators/asn - subfields: + subproperties: title: { type: string } agent_name: type: string @@ -972,7 +972,7 @@ properties: form: primary: false property_uri: http://purl.org/dc/terms/identifier - subfields: + subproperties: identifier: type: string required: true @@ -998,7 +998,7 @@ properties: form: primary: false property_uri: http://purl.org/dc/terms/rights - subfields: + subproperties: rights_statement: type: controlled authority: rights_statements @@ -1026,7 +1026,7 @@ properties: form: primary: false property_uri: http://purl.org/dc/terms/relation - subfields: + subproperties: related_item: type: work_or_url required: true diff --git a/app/assets/javascripts/hyrax/compound_metadata.js b/app/assets/javascripts/hyrax/compound_metadata.js index b5a7099de2..3f09bb7ed6 100644 --- a/app/assets/javascripts/hyrax/compound_metadata.js +++ b/app/assets/javascripts/hyrax/compound_metadata.js @@ -7,7 +7,7 @@ if (document.hyraxCompoundsBound) return; document.hyraxCompoundsBound = true; - // Bind select2 to a `work_or_url` sub-field. The v3 API (Hyrax bundles + // Bind select2 to a `work_or_url` sub-property. The v3 API (Hyrax bundles // select2-rails 3.x) binds to a hidden input; createSearchChoice lets a // typed external URL be selected as-is. function bindWorkOrUrlInputs(root) { diff --git a/app/assets/stylesheets/hyrax/_compound_metadata.scss b/app/assets/stylesheets/hyrax/_compound_metadata.scss index 15e54343b7..17e8c6fcee 100644 --- a/app/assets/stylesheets/hyrax/_compound_metadata.scss +++ b/app/assets/stylesheets/hyrax/_compound_metadata.scss @@ -12,17 +12,17 @@ padding: 0 !important; max-width: 100%; - // Force block flow on the entries/sub-fields; the label/value spans stay - // inline so each sub-field reads as "Label: value" on one wrapping line. + // Force block flow on the entries/sub-properties; the label/value spans stay + // inline so each sub-property reads as "Label: value" on one wrapping line. .hyrax-compound-entry, - .hyrax-compound-subfield { + .hyrax-compound-subproperty { display: block !important; } .hyrax-compound-entry, - .hyrax-compound-subfield, - .hyrax-compound-subfield-label, - .hyrax-compound-subfield-value { + .hyrax-compound-subproperty, + .hyrax-compound-subproperty-label, + .hyrax-compound-subproperty-value { border: 0 !important; padding: 0 !important; margin: 0; @@ -36,14 +36,14 @@ border-top: 1px solid $gray-200 !important; } - .hyrax-compound-subfield { + .hyrax-compound-subproperty { display: block; line-height: 1.4; overflow-wrap: break-word; word-break: break-word; } - .hyrax-compound-subfield-label { + .hyrax-compound-subproperty-label { font-weight: 600; margin-right: 0.25rem; } diff --git a/app/authorities/qa/authorities/compound_works.rb b/app/authorities/qa/authorities/compound_works.rb index 66e70cba84..3f84882291 100644 --- a/app/authorities/qa/authorities/compound_works.rb +++ b/app/authorities/qa/authorities/compound_works.rb @@ -2,7 +2,7 @@ module Qa::Authorities ## - # Autocomplete authority for the compound `work_or_url` sub-field's work + # Autocomplete authority for the compound `work_or_url` sub-property's work # picker, mounted at `/authorities/search/compound_works`. Returns readable # works matched by {Hyrax::CompoundWorkPickerBuilder}. class CompoundWorks < Qa::Authorities::Base diff --git a/app/forms/concerns/hyrax/compound_field_behavior.rb b/app/forms/concerns/hyrax/compound_field_behavior.rb index f5ca4bf3c0..1897f53ef6 100644 --- a/app/forms/concerns/hyrax/compound_field_behavior.rb +++ b/app/forms/concerns/hyrax/compound_field_behavior.rb @@ -53,13 +53,13 @@ def compound_field_names end # One populator serves every compound (Reform passes the property name as - # `as:`). Builds the replacement array of plain hashes — declared sub-field - # keys only, dropping `_destroy` and all-blank rows. + # `as:`). Builds the replacement array of plain hashes — declared + # sub-property keys only, dropping `_destroy` and all-blank rows. def compound_attributes_populator(fragment:, as:, **_options) name = as.to_s.delete_suffix('_attributes') return unless respond_to?(name) - allowed = Hyrax::CompoundSchema.for(model).subfield_keys(name) + allowed = Hyrax::CompoundSchema.for(model).subproperty_keys(name) public_send(:"#{name}=", build_compound_rows(fragment, allowed)) end @@ -75,7 +75,7 @@ def compound_fragment_pairs(fragment) fragment.respond_to?(:to_unsafe_h) ? fragment.to_unsafe_h : fragment.to_h end - # Returns nil for a row marked for destruction or whose declared sub-fields + # Returns nil for a row marked for destruction or whose declared sub-properties # are all blank, otherwise the persisted hash for that row. def compound_row_from(row, allowed_keys) row = row.respond_to?(:to_unsafe_h) ? row.to_unsafe_h : row.to_h diff --git a/app/forms/concerns/hyrax/flexible_form_behavior.rb b/app/forms/concerns/hyrax/flexible_form_behavior.rb index fa59896c4d..f0874b1a23 100644 --- a/app/forms/concerns/hyrax/flexible_form_behavior.rb +++ b/app/forms/concerns/hyrax/flexible_form_behavior.rb @@ -13,7 +13,7 @@ def validate_flexible_required_fields required_fields = singleton_class.schema_definitions.select { |_, opts| opts[:required] }.keys required_fields.each do |field| - # Compound fields carry their own requiredness rules (per-row sub-field + # Compound fields carry their own requiredness rules (per-row sub-property # checks); Hyrax::CompoundEntryValidator owns them, so skip the generic # blank check to avoid a duplicate "can't be blank" error. next if compound_field?(field) @@ -29,9 +29,9 @@ def schema private - # Whether the field is a compound (declares `subfields:`), looked up from + # Whether the field is a compound (declares `subproperties:`), looked up from # the resource's compound schema — the flex form definitions don't carry - # `subfields`. Memoized per form instance. + # `subproperties`. Memoized per form instance. def compound_field?(field) return false unless Hyrax.config.respond_to?(:compound_metadata_enabled?) && Hyrax.config.compound_metadata_enabled? @compound_field_names ||= Hyrax::CompoundSchema.for(model).compound_names diff --git a/app/forms/hyrax/forms/resource_form.rb b/app/forms/hyrax/forms/resource_form.rb index c2cbab8578..d0c2c4adff 100644 --- a/app/forms/hyrax/forms/resource_form.rb +++ b/app/forms/hyrax/forms/resource_form.rb @@ -64,7 +64,7 @@ class ResourceForm < Hyrax::ChangeSet # rubocop:disable Metrics/ClassLength end end - # Required-compound / required-sub-field validation. Wired through a + # Required-compound / required-sub-property validation. Wired through a # `validation { ... }` block (not a bare `validates_with`) because these # are Reform/Disposable forms — a bare `validates_with` does not hook into # Reform's `validate`, so it would never run. Record-level (no diff --git a/app/helpers/hyrax/collections_helper.rb b/app/helpers/hyrax/collections_helper.rb index 6fb60d1c79..70cfbc3307 100644 --- a/app/helpers/hyrax/collections_helper.rb +++ b/app/helpers/hyrax/collections_helper.rb @@ -49,7 +49,7 @@ def collection_metadata_value(collection, field) if collection.respond_to?(:compound_term?) && collection.compound_term?(field) definition = compound_schema_for(collection).definition_for(field) return Hyrax::Renderers::CompoundAttributeRenderer - .new(field, collection[field], subfields: definition&.fetch(:subfields, nil)) + .new(field, collection[field], subproperties: definition&.fetch(:subproperties, nil)) .render_value end diff --git a/app/helpers/hyrax/compound_fields_helper.rb b/app/helpers/hyrax/compound_fields_helper.rb index 6e8c5f0635..aac4e5722b 100644 --- a/app/helpers/hyrax/compound_fields_helper.rb +++ b/app/helpers/hyrax/compound_fields_helper.rb @@ -5,7 +5,7 @@ module Hyrax # and show pages. See documentation/forms/compound_fields.md. module CompoundFieldsHelper ## - # Renders one compound section (a repeatable stack of sub-field rows) for the + # Renders one compound section (a repeatable stack of sub-property rows) for the # given attribute via the `hyrax/compounds/*` partials. # # @return [String, nil] rendered HTML, or nil when the attribute is not a @@ -65,29 +65,29 @@ def compound_schema_for(presenter) end ## - # Options for a `controlled` sub-field's ``: an inline `values:` # list when present, otherwise the named QA authority. A stored value not # among the options is appended so it still renders (`include_current_value`). # # @return [Array] `[[label, id], ...]` - def compound_subfield_options(spec, current_value = nil) + def compound_subproperty_options(spec, current_value = nil) options = spec[:values].presence || authority_options(spec[:authority]) ensure_current_value(options, current_value) end ## # @return [Boolean] whether +current_value+ is present but not among the - # sub-field's offered options — i.e. a forced/stale value. The select + # sub-property's offered options — i.e. a forced/stale value. The select # gets the +force-select+ class in that case, matching the ordinary # controlled-field convention. - def compound_subfield_forced?(spec, current_value = nil) + def compound_subproperty_forced?(spec, current_value = nil) return false if current_value.blank? base = spec[:values].presence || authority_options(spec[:authority]) base.none? { |(_label, id)| id.to_s == current_value.to_s } end ## - # The pre-selected `[label, value]` option for a `work_or_url` sub-field's + # The pre-selected `[label, value]` option for a `work_or_url` sub-property's # select2, or nil when empty. An internal work id resolves to its title; an # external URL is shown as-is. # @@ -121,9 +121,9 @@ def compound_card_label(presenter, field) compound_field_label(field, display_label: compound_schema_for(presenter).definition_for(field)&.dig(:display_label)) end - def compound_subfield_label(compound_name, sub_field) - t("hyrax.compound_fields.#{compound_name}.#{sub_field}", - default: sub_field.to_s.humanize) + def compound_subproperty_label(compound_name, sub_property) + t("hyrax.compound_fields.#{compound_name}.#{sub_property}", + default: sub_property.to_s.humanize) end private @@ -135,7 +135,7 @@ def authority_options(authority_name) return [] if authority_name.blank? Hyrax::TolerantSelectService.new(authority_name).select_active_options rescue StandardError => e - Hyrax.logger.debug("compound_subfield_options: #{authority_name}: #{e.message}") + Hyrax.logger.debug("compound_subproperty_options: #{authority_name}: #{e.message}") [] end diff --git a/app/indexers/hyrax/indexers/compound_indexer.rb b/app/indexers/hyrax/indexers/compound_indexer.rb index 3983166396..33fd72e80f 100644 --- a/app/indexers/hyrax/indexers/compound_indexer.rb +++ b/app/indexers/hyrax/indexers/compound_indexer.rb @@ -3,11 +3,11 @@ module Hyrax module Indexers ## - # Indexer mixin that projects compound metadata sub-fields into Solr. For + # Indexer mixin that projects compound metadata sub-properties into Solr. For # every compound on the resource (see {Hyrax::CompoundSchema}), it writes - # each sub-field's declared `index_keys:`/`indexing:` Solr fields and stores - # the displayable rows as a `_json_ss` blob the show page renders - # from. See documentation/forms/compound_fields.md. + # each sub-property's declared `index_keys:`/`indexing:` Solr fields and + # stores the displayable rows as a `_json_ss` blob the show page + # renders from. See documentation/forms/compound_fields.md. # # @example # class WorkIndexer < Hyrax::Indexers::PcdmObjectIndexer @@ -31,15 +31,15 @@ def compound_schema def index_compound(document, compound_name, definition) rows = Array(resource.public_send(compound_name)) - index_searchable_subfields(document, definition, rows) + index_searchable_subproperties(document, definition, rows) index_display_blob(document, compound_name, definition, rows) end - def index_searchable_subfields(document, definition, rows) - definition[:subfields].each do |sub_field, spec| + def index_searchable_subproperties(document, definition, rows) + definition[:subproperties].each do |sub_property, spec| next if spec[:index_keys].blank? - values = rows.map { |row| compound_entry_value(row, sub_field) }.reject(&:blank?) + values = rows.map { |row| compound_entry_value(row, sub_property) }.reject(&:blank?) next if values.empty? spec[:index_keys].each { |index_key| document[index_key] = values } @@ -47,7 +47,7 @@ def index_searchable_subfields(document, definition, rows) end def index_display_blob(document, compound_name, definition, rows) - display_keys = definition[:subfields].select { |_k, spec| spec[:display] }.keys + display_keys = definition[:subproperties].select { |_k, spec| spec[:display] }.keys normalized = rows.map { |row| display_entry(row, display_keys) }.reject(&:empty?) document["#{compound_name}_json_ss"] = normalized.to_json unless normalized.empty? end @@ -60,9 +60,9 @@ def display_entry(row, display_keys) end end - def compound_entry_value(row, sub_field) + def compound_entry_value(row, sub_property) return nil unless row.respond_to?(:[]) - row[sub_field] || row[sub_field.to_sym] + row[sub_property] || row[sub_property.to_sym] end end end diff --git a/app/indexers/hyrax/indexers/pcdm_collection_indexer.rb b/app/indexers/hyrax/indexers/pcdm_collection_indexer.rb index d356fdb934..ca3daa6726 100644 --- a/app/indexers/hyrax/indexers/pcdm_collection_indexer.rb +++ b/app/indexers/hyrax/indexers/pcdm_collection_indexer.rb @@ -11,7 +11,7 @@ class PcdmCollectionIndexer < Hyrax::Indexers::ResourceIndexer include Hyrax::ThumbnailIndexer include Hyrax::Indexer(:core_metadata) if Hyrax.config.collection_include_metadata? include Hyrax::Indexers::RedirectsIndexer if Hyrax.config.redirects_enabled? - # Flatten compound (hierarchical) metadata sub-fields into Solr. No-op for + # Flatten compound (hierarchical) metadata sub-properties into Solr. No-op for # collections without compounds. See documentation/forms/compound_fields.md. include Hyrax::Indexers::CompoundIndexer if Hyrax.config.compound_metadata_enabled? check_if_flexible(Hyrax::PcdmCollection) diff --git a/app/indexers/hyrax/indexers/pcdm_object_indexer.rb b/app/indexers/hyrax/indexers/pcdm_object_indexer.rb index 58100c64bb..7cb547a646 100644 --- a/app/indexers/hyrax/indexers/pcdm_object_indexer.rb +++ b/app/indexers/hyrax/indexers/pcdm_object_indexer.rb @@ -11,7 +11,7 @@ class PcdmObjectIndexer < Hyrax::Indexers::ResourceIndexer include Hyrax::ThumbnailIndexer include Hyrax::WorkflowIndexer include Hyrax::Indexers::RedirectsIndexer if Hyrax.config.redirects_enabled? - # Flatten compound (hierarchical) metadata sub-fields into Solr. No-op for + # Flatten compound (hierarchical) metadata sub-properties into Solr. No-op for # resources without compounds. See documentation/forms/compound_fields.md. include Hyrax::Indexers::CompoundIndexer if Hyrax.config.compound_metadata_enabled? diff --git a/app/models/concerns/hyrax/compound_normalization.rb b/app/models/concerns/hyrax/compound_normalization.rb index 62f7fdf5af..1cec6b78fc 100644 --- a/app/models/concerns/hyrax/compound_normalization.rb +++ b/app/models/concerns/hyrax/compound_normalization.rb @@ -2,7 +2,7 @@ module Hyrax ## - # Defends compound attributes (`type: hash, multiple: true` with `subfields:` — + # Defends compound attributes (`type: hash, multiple: true` with `subproperties:` — # see {Hyrax::CompoundSchema}) against a read-path quirk in Valkyrie's Postgres # orm_converter: `JSONValueMapper` unwraps a single-element array to its first # element and re-symbolizes the Hash's keys, then dry-struct's `Array.of(Hash)` diff --git a/app/presenters/hyrax/presents_attributes.rb b/app/presenters/hyrax/presents_attributes.rb index 54925cb013..18f70b0db3 100644 --- a/app/presenters/hyrax/presents_attributes.rb +++ b/app/presenters/hyrax/presents_attributes.rb @@ -22,7 +22,7 @@ def attribute_to_html(field, options = {}) return end - options = options.merge(subfields: compound_subfields_for(field)) if options[:render_as].to_s == 'compound' + options = options.merge(subproperties: compound_subproperties_for(field)) if options[:render_as].to_s == 'compound' renderer = renderer_for(field, options).new(field, send(field), options) if options[:value_only] && renderer.respond_to?(:render_value) @@ -54,18 +54,18 @@ def microdata_type_to_html private - # Normalized sub-field specs for a compound, so the renderer can translate + # Normalized sub-property specs for a compound, so the renderer can translate # controlled ids to their terms; nil if the resource class can't be # resolved (the renderer then renders raw values). - def compound_subfields_for(field) + def compound_subproperties_for(field) return nil unless respond_to?(:solr_document) && solr_document.respond_to?(:hydra_model) # Resolve from the backing document, not the class: in flexible mode the - # class carries no compounds, so a class lookup would drop the sub-field + # class carries no compounds, so a class lookup would drop the sub-property # specs and the renderer would fall back to raw (unlinked, untranslated) # values. - Hyrax::CompoundSchema.for_solr_document(solr_document).definition_for(field)&.fetch(:subfields, nil) + Hyrax::CompoundSchema.for_solr_document(solr_document).definition_for(field)&.fetch(:subproperties, nil) rescue StandardError => e - Hyrax.logger.debug("compound_subfields_for(#{field}): #{e.message}") + Hyrax.logger.debug("compound_subproperties_for(#{field}): #{e.message}") nil end diff --git a/app/renderers/hyrax/renderers/compound_attribute_renderer.rb b/app/renderers/hyrax/renderers/compound_attribute_renderer.rb index d474ef5ca5..4807632b0a 100644 --- a/app/renderers/hyrax/renderers/compound_attribute_renderer.rb +++ b/app/renderers/hyrax/renderers/compound_attribute_renderer.rb @@ -5,9 +5,9 @@ module Renderers ## # Renders a compound metadata attribute on a show page (selected via # `view: { render_as: compound }`). Each value is one entry — a hash of - # sub-fields produced by the SolrDocument `compound_attribute` reader — and - # renders as a block of its populated sub-fields. Sub-field labels come from - # the `hyrax.compound_fields..` i18n keys. + # sub-properties produced by the SolrDocument `compound_attribute` reader — + # and renders as a block of its populated sub-properties. Sub-property labels + # come from the `hyrax.compound_fields..` i18n keys. class CompoundAttributeRenderer < AttributeRenderer def render return '' if blank_values? && !options[:include_empty] @@ -51,33 +51,33 @@ def entry_markup(entry) pairs = entry_to_pairs(entry) return '' if pairs.empty? - items = pairs.map { |sub_field, value| subfield_markup(sub_field, value) }.join + items = pairs.map { |sub_property, value| subproperty_markup(sub_property, value) }.join %(
#{items}
) end - def subfield_markup(sub_field, value) - label_html = ERB::Util.h(sub_field_label(sub_field)) - %(
) + - %(#{label_html}: ) + - %(#{value_markup(sub_field, value)}) + + def subproperty_markup(sub_property, value) + label_html = ERB::Util.h(sub_property_label(sub_property)) + %(
) + + %(#{label_html}: ) + + %(#{value_markup(sub_property, value)}) + %(
) end - # Display markup for one sub-field value, by sub-field type: `url` and + # Display markup for one sub-property value, by sub-property type: `url` and # `work_or_url` are linked; otherwise escaped text with controlled ids # translated to their term. - def value_markup(sub_field, value) - return ERB::Util.h(display_value(sub_field, value)) if value.blank? + def value_markup(sub_property, value) + return ERB::Util.h(display_value(sub_property, value)) if value.blank? - case subfield_spec(sub_field)&.dig(:type).to_s + case subproperty_spec(sub_property)&.dig(:type).to_s when 'url' auto_link(ERB::Util.h(value.to_s)) when 'work_or_url' work_or_url_markup(value) when 'controlled' - controlled_markup(sub_field, value) + controlled_markup(sub_property, value) else - ERB::Util.h(display_value(sub_field, value)) + ERB::Util.h(display_value(sub_property, value)) end end @@ -85,8 +85,8 @@ def value_markup(sub_field, value) # stored value is itself a linkable URI (e.g. a rights-statement or license # URI), link the term to that URI — mirroring the ordinary rights/license # renderer. Non-URI controlled values (e.g. inline option ids) stay plain. - def controlled_markup(sub_field, value) - label = display_value(sub_field, value) + def controlled_markup(sub_property, value) + label = display_value(sub_property, value) if Hyrax::AuthorityRenderingHelper.linkable_uri?(value) %(#{ERB::Util.h(label)}).html_safe else @@ -104,19 +104,19 @@ def work_or_url_markup(value) link_to(ERB::Util.h(title), path) end - def display_value(sub_field, value) - Hyrax::CompoundSubfieldLabeler.label_for(subfield_spec(sub_field), value) + def display_value(sub_property, value) + Hyrax::CompoundSubpropertyLabeler.label_for(subproperty_spec(sub_property), value) end - # The normalized sub-field spec, supplied by the caller via - # `options[:subfields]`. - def subfield_spec(sub_field) - specs = options[:subfields] + # The normalized sub-property spec, supplied by the caller via + # `options[:subproperties]`. + def subproperty_spec(sub_property) + specs = options[:subproperties] return nil unless specs.is_a?(Hash) - specs[sub_field] || specs[sub_field.to_sym] + specs[sub_property] || specs[sub_property.to_sym] end - # Populated [sub_field, value] pairs for one entry; blanks dropped. + # Populated [sub_property, value] pairs for one entry; blanks dropped. def entry_to_pairs(entry) return [] unless entry.respond_to?(:each_pair) || entry.is_a?(::Hash) entry.to_h.each_with_object([]) do |(key, value), memo| @@ -125,8 +125,8 @@ def entry_to_pairs(entry) end end - def sub_field_label(sub_field) - I18n.t("hyrax.compound_fields.#{field}.#{sub_field}", default: sub_field.to_s.humanize) + def sub_property_label(sub_property) + I18n.t("hyrax.compound_fields.#{field}.#{sub_property}", default: sub_property.to_s.humanize) end end end diff --git a/app/search_builders/hyrax/compound_work_picker_builder.rb b/app/search_builders/hyrax/compound_work_picker_builder.rb index 78299d3f50..7c215df118 100644 --- a/app/search_builders/hyrax/compound_work_picker_builder.rb +++ b/app/search_builders/hyrax/compound_work_picker_builder.rb @@ -2,7 +2,7 @@ module Hyrax ## - # Search builder for the compound `work_or_url` sub-field's work picker. Finds + # Search builder for the compound `work_or_url` sub-property's work picker. Finds # works the current user can read (it subclasses {Hyrax::SearchBuilder}, so # permission filtering is retained), matching any indexed query term OR a # partial/prefix title. diff --git a/app/services/hyrax/compound_entry_validation.rb b/app/services/hyrax/compound_entry_validation.rb index b8d5d04e26..737ebe906a 100644 --- a/app/services/hyrax/compound_entry_validation.rb +++ b/app/services/hyrax/compound_entry_validation.rb @@ -11,7 +11,8 @@ module Hyrax # # Rules (driven by the normalized definition from {Hyrax::CompoundSchema}): # * a compound marked `required` must have at least one populated row; - # * every populated row must fill all of the compound's `required` sub-fields. + # * every populated row must fill all of the compound's `required` + # sub-properties. # # Rows are the post-populator persisted hashes (all-blank rows already # dropped), so a no-required compound with no rows is valid. @@ -25,11 +26,11 @@ def initialize(definition, entries) # @return [Array] one violation per problem, each # `{ type:, missing: [keys] }`. Empty when the compound is valid. - # `type` is `:required_but_empty` or `:missing_required_subfields`. + # `type` is `:required_but_empty` or `:missing_required_subproperties`. def violations return [{ type: :required_but_empty, missing: required_keys }] if required_but_empty? - rows_missing_required.map { |missing| { type: :missing_required_subfields, missing: missing } } + rows_missing_required.map { |missing| { type: :missing_required_subproperties, missing: missing } } end # @return [Boolean] @@ -42,7 +43,7 @@ def valid? attr_reader :definition, :entries def required_keys - definition.fetch(:subfields, {}).select { |_k, spec| spec[:required] }.keys + definition.fetch(:subproperties, {}).select { |_k, spec| spec[:required] }.keys end def required_but_empty? diff --git a/app/services/hyrax/compound_schema.rb b/app/services/hyrax/compound_schema.rb index d45fe1ece6..51a63ea8c4 100644 --- a/app/services/hyrax/compound_schema.rb +++ b/app/services/hyrax/compound_schema.rb @@ -5,9 +5,10 @@ module Hyrax # @api public # # Reads the compound metadata declarations off a resource's schema. A compound - # is a `type: hash, multiple: true` attribute carrying a `subfields:` map; the - # declaration drives the generic form, indexer, and renderer so a hierarchical - # field can be defined in YAML alone. See documentation/forms/compound_fields.md. + # is a `type: hash, multiple: true` attribute carrying a `subproperties:` map; + # the declaration drives the generic form, indexer, and renderer so a + # hierarchical field can be defined in YAML alone. See + # documentation/forms/compound_fields.md. # # Declarations are read from each attribute's Dry type `meta`, which both # schema loaders populate identically, so the result is the same in both flex @@ -86,7 +87,7 @@ def initialize(*schema_sources) ## # @return [Boolean] whether the given attribute is a compound (declares - # `subfields:`) + # `subproperties:`) def compound?(attribute_name) definitions.key?(attribute_name.to_sym) end @@ -122,8 +123,8 @@ def card?(attribute_name) # @param [#to_sym] attribute_name # # @return [Hash{Symbol => Object}, nil] the compound's declaration: - # `{ subfields:, groups:, index_subfields: }`, or nil when the attribute - # is not a compound. + # `{ subproperties:, groups: }`, or nil when the attribute is not a + # compound. def definition_for(attribute_name) definitions[attribute_name.to_sym] end @@ -131,12 +132,12 @@ def definition_for(attribute_name) ## # @param [#to_sym] attribute_name # - # @return [Array] the ordered sub-field keys declared for the + # @return [Array] the ordered sub-property keys declared for the # compound (empty when not a compound). - def subfield_keys(attribute_name) + def subproperty_keys(attribute_name) definition = definition_for(attribute_name) return [] unless definition - definition[:subfields].keys + definition[:subproperties].keys end ## @@ -147,12 +148,12 @@ def required?(attribute_name) end ## - # @return [Array] the sub-field keys declared `required: true` for + # @return [Array] the sub-property keys declared `required: true` for # the compound (each must be filled in every populated row). - def required_subfield_keys(attribute_name) + def required_subproperty_keys(attribute_name) definition = definition_for(attribute_name) return [] unless definition - definition[:subfields].select { |_key, spec| spec[:required] }.keys + definition[:subproperties].select { |_key, spec| spec[:required] }.keys end ## @@ -170,10 +171,10 @@ def build_definitions next if meta.nil? || memo.key?(name) config = meta.with_indifferent_access - subfields = config['subfields'] - next if subfields.blank? + subproperties = config['subproperties'] + next if subproperties.blank? - memo[name] = normalize(config, subfields) + memo[name] = normalize(config, subproperties) end end end @@ -193,14 +194,14 @@ def name_meta_pairs(schema) # Normalizes the raw declaration into the symbol-keyed shape the form, # indexer, and renderer consume. See documentation/forms/compound_fields.md - # for the meaning of each sub-field key. - def normalize(config, subfields) - sub = subfields.each_with_object({}) { |(key, opts), memo| memo[key.to_s] = normalize_subfield(opts) } + # for the meaning of each sub-property key. + def normalize(config, subproperties) + sub = subproperties.each_with_object({}) { |(key, opts), memo| memo[key.to_s] = normalize_subproperty(opts) } view = config['view'] display_mode = view.is_a?(Hash) && view['display'].to_s == 'card' ? :card : :inline - { subfields: sub, + { subproperties: sub, groups: normalize_groups(config['groups'], sub.keys), display_mode: display_mode, required: compound_required?(config), @@ -215,7 +216,7 @@ def normalize_display_label(config) raw.is_a?(Hash) ? raw.with_indifferent_access : { default: raw.to_s }.with_indifferent_access end - def normalize_subfield(opts) + def normalize_subproperty(opts) opts = (opts.is_a?(Hash) ? opts : {}).with_indifferent_access { type: (opts['type'] || 'string').to_s, authority: opts['authority']&.to_s, diff --git a/app/services/hyrax/compound_subfield_labeler.rb b/app/services/hyrax/compound_subproperty_labeler.rb similarity index 79% rename from app/services/hyrax/compound_subfield_labeler.rb rename to app/services/hyrax/compound_subproperty_labeler.rb index 4d0e2d4663..caed10e306 100644 --- a/app/services/hyrax/compound_subfield_labeler.rb +++ b/app/services/hyrax/compound_subproperty_labeler.rb @@ -4,14 +4,14 @@ module Hyrax ## # @api public # - # Resolves a `controlled` compound sub-field's stored id to its display term - # (via inline `values:` or a QA `authority:`). Non-controlled sub-fields and + # Resolves a `controlled` compound sub-property's stored id to its display term + # (via inline `values:` or a QA `authority:`). Non-controlled sub-properties and # ids with no matching term fall back to the value itself. - class CompoundSubfieldLabeler + class CompoundSubpropertyLabeler ## - # @param spec [Hash, nil] the normalized sub-field spec + # @param spec [Hash, nil] the normalized sub-property spec # (`{ type:, authority:, values: }`) from {Hyrax::CompoundSchema} - # @param value [Object] the stored value (id for controlled sub-fields) + # @param value [Object] the stored value (id for controlled sub-properties) # # @return [String] the term to display def self.label_for(spec, value) @@ -35,7 +35,7 @@ def self.label_from_values(values, value) def self.label_from_authority(authority_name, value) Hyrax::TolerantSelectService.new(authority_name).label(value.to_s) { value.to_s } rescue StandardError => e - Hyrax.logger.debug("CompoundSubfieldLabeler: #{authority_name}: #{e.message}") + Hyrax.logger.debug("CompoundSubpropertyLabeler: #{authority_name}: #{e.message}") value.to_s end private_class_method :label_from_authority diff --git a/app/services/hyrax/compound_work_resolver.rb b/app/services/hyrax/compound_work_resolver.rb index f25235b94f..b4cdec0a18 100644 --- a/app/services/hyrax/compound_work_resolver.rb +++ b/app/services/hyrax/compound_work_resolver.rb @@ -4,7 +4,7 @@ module Hyrax ## # @api public # - # Helpers for the `work_or_url` compound sub-field, whose stored value is + # Helpers for the `work_or_url` compound sub-property, whose stored value is # either an external URL or an internal work id. Distinguishes the two and # resolves an internal work id to a display title (from Solr) and a show path. class CompoundWorkResolver diff --git a/app/services/hyrax/flexible_schema_validator_service.rb b/app/services/hyrax/flexible_schema_validator_service.rb index 45db289b9c..f79a3c434d 100644 --- a/app/services/hyrax/flexible_schema_validator_service.rb +++ b/app/services/hyrax/flexible_schema_validator_service.rb @@ -134,8 +134,8 @@ def validate_redirects end # Validates compound (hierarchical) metadata properties — those declaring - # `subfields:` — for well-formed sub-fields and correct (per-sub-field) - # indexing declaration. + # `subproperties:` — for well-formed sub-properties and correct + # (per-sub-property) indexing declaration. # # @return [void] def validate_compound diff --git a/app/services/hyrax/flexible_schema_validators/compound_validator.rb b/app/services/hyrax/flexible_schema_validators/compound_validator.rb index e2bda68a32..0089f2de06 100644 --- a/app/services/hyrax/flexible_schema_validators/compound_validator.rb +++ b/app/services/hyrax/flexible_schema_validators/compound_validator.rb @@ -6,9 +6,9 @@ module FlexibleSchemaValidators # @api private # # Validates compound metadata properties (a `type: hash` property declaring - # `subfields:` — see {Hyrax::CompoundSchema}) in an m3 profile at save time, - # so a misconfiguration fails with a clear message instead of producing dead - # Solr fields or unrenderable values. See + # `subproperties:` — see {Hyrax::CompoundSchema}) in an m3 profile at save + # time, so a misconfiguration fails with a clear message instead of producing + # dead Solr fields or unrenderable values. See # documentation/forms/compound_fields.md for the rules. class CompoundValidator ## @@ -22,44 +22,44 @@ def initialize(profile:, errors:) # @return [void] def validate! compound_properties.each do |name, config| - validate_subfields(name, config['subfields']) + validate_subproperties(name, config['subproperties']) validate_no_top_level_indexing(name, config) end end private - # Compounds are detected by `subfields:` presence (not `type`), so other - # hash fields like redirects stay out of scope. + # Compounds are detected by `subproperties:` presence (not `type`), so + # other hash fields like redirects stay out of scope. def compound_properties (@profile&.dig('properties') || {}).select do |_name, config| - config.is_a?(Hash) && config['subfields'].present? + config.is_a?(Hash) && config['subproperties'].present? end end - def validate_subfields(name, subfields) - unless subfields.is_a?(Hash) - @errors << t('subfields_not_hash', property: name) + def validate_subproperties(name, subproperties) + unless subproperties.is_a?(Hash) + @errors << t('subproperties_not_hash', property: name) return end - subfields.each { |sub_name, sub_config| validate_subfield(name, sub_name, sub_config) } + subproperties.each { |sub_name, sub_config| validate_subproperty(name, sub_name, sub_config) } end - def validate_subfield(name, sub_name, sub_config) + def validate_subproperty(name, sub_name, sub_config) unless sub_config.is_a?(Hash) - @errors << t('subfield_not_hash', property: name, subfield: sub_name, actual: sub_config.class.to_s) + @errors << t('subproperty_not_hash', property: name, subproperty: sub_name, actual: sub_config.class.to_s) return end return unless sub_config['type'].to_s == 'controlled' return if sub_config['authority'].present? || sub_config['values'].present? - @errors << t('controlled_without_source', property: name, subfield: sub_name) + @errors << t('controlled_without_source', property: name, subproperty: sub_name) end # A top-level `indexing:` would point the catalog at a `_tesim` - # field the indexer never writes; indexing is per sub-field. + # field the indexer never writes; indexing is per sub-property. def validate_no_top_level_indexing(name, config) return if config['indexing'].blank? @errors << t('top_level_indexing', property: name) diff --git a/app/services/hyrax/schema_loader.rb b/app/services/hyrax/schema_loader.rb index a23deb946c..9641f1a115 100644 --- a/app/services/hyrax/schema_loader.rb +++ b/app/services/hyrax/schema_loader.rb @@ -162,7 +162,7 @@ def self.of(type) # # Recognized values: # - `id`, `uri`, `date_time` — Valkyrie type shortcuts. - # - `hash` — for attributes whose entries carry multiple sub-fields + # - `hash` — for attributes whose entries carry multiple sub-properties # (e.g. redirects, with path / canonical / sequence). Use this # instead of nesting a Valkyrie::Resource. See # `documentation/redirects.md` for a worked example. diff --git a/app/validators/hyrax/compound_entry_validator.rb b/app/validators/hyrax/compound_entry_validator.rb index eea7da423e..5b47afc1da 100644 --- a/app/validators/hyrax/compound_entry_validator.rb +++ b/app/validators/hyrax/compound_entry_validator.rb @@ -3,7 +3,8 @@ module Hyrax # Validates every compound (hierarchical) metadata attribute on a form, # blocking save when a required compound has no row or a populated row omits a - # required sub-field. Adds one error per compound, keyed on the compound name. + # required sub-property. Adds one error per compound, keyed on the compound + # name. # # Record-level (not an EachValidator) because the compound set is schema-driven # and not known at form-class-definition time. The per-compound rules live in @@ -49,14 +50,14 @@ def validate_compound(record, name, definition) def message_for(name, violation) I18n.t("hyrax.compound_fields.errors.#{violation[:type]}", compound: compound_label(name), - fields: subfield_labels(name, violation[:missing])) + fields: subproperty_labels(name, violation[:missing])) end def compound_label(name) I18n.t("hyrax.compound_fields.#{name}.label", default: name.to_s.humanize) end - def subfield_labels(name, keys) + def subproperty_labels(name, keys) Array(keys).map do |key| I18n.t("hyrax.compound_fields.#{name}.#{key}", default: key.to_s.humanize) end.join(', ') diff --git a/app/views/hyrax/compounds/_compound_row.html.erb b/app/views/hyrax/compounds/_compound_row.html.erb index a5ea6cd834..c3bdf0bd3c 100644 --- a/app/views/hyrax/compounds/_compound_row.html.erb +++ b/app/views/hyrax/compounds/_compound_row.html.erb @@ -3,17 +3,17 @@ Locals: f - form builder for the parent resource compound_name - Symbol, e.g. :contributors - definition - Hash{ subfields:, groups:, index_subfields: } + definition - Hash{ subproperties:, groups: } row - Hash of persisted values, or nil for a new row index - Integer position, or the literal "__INDEX__" placeholder when rendered inside the JS template row_label_singular - "Contributor", "License", etc. - Each sub-field renders according to its declared `type:` (string, + Each sub-property renders according to its declared `type:` (string, controlled, url, work_or_url). db_table / geocode are future types; the `else` branch is the extension seam. %> -<% subfields = definition[:subfields] %> +<% subproperties = definition[:subproperties] %> <% groups = definition[:groups] %> <% row_position = index.is_a?(String) ? '' : (index.to_i + 1).to_s %> <% row_value = ->(key) { row.is_a?(Hash) ? (row[key] || row[key.to_sym]) : nil } %> @@ -34,22 +34,22 @@

<%= group[:label] %>

<% end %>
- <% group[:fields].each do |sub_field| %> - <% spec = subfields[sub_field] || { type: 'string' } %> - <% input_id = "#{f.object_name}_#{compound_name}_attributes_#{index}_#{sub_field}" %> - <% input_name = "#{f.object_name}[#{compound_name}_attributes][#{index}][#{sub_field}]" %> - <% value = row_value.call(sub_field) %> + <% group[:fields].each do |sub_property| %> + <% spec = subproperties[sub_property] || { type: 'string' } %> + <% input_id = "#{f.object_name}_#{compound_name}_attributes_#{index}_#{sub_property}" %> + <% input_name = "#{f.object_name}[#{compound_name}_attributes][#{index}][#{sub_property}]" %> + <% value = row_value.call(sub_property) %>
<%= label_tag input_id, class: 'form-label small text-muted' do %> - <%= compound_subfield_label(compound_name, sub_field) %><% + <%= compound_subproperty_label(compound_name, sub_property) %><% if spec[:required] %>*<% end %> <% end %> <% case spec[:type] %> <% when 'controlled' %> <% select_classes = ['form-control', 'form-control-sm'] %> - <% select_classes << 'force-select' if compound_subfield_forced?(spec, value) %> + <% select_classes << 'force-select' if compound_subproperty_forced?(spec, value) %> <%= select_tag input_name, - options_for_select(compound_subfield_options(spec, value), value), + options_for_select(compound_subproperty_options(spec, value), value), id: input_id, include_blank: true, class: select_classes.join(' ') %> diff --git a/app/views/hyrax/compounds/_compound_section.html.erb b/app/views/hyrax/compounds/_compound_section.html.erb index 0eec2654dd..bd80b032f7 100644 --- a/app/views/hyrax/compounds/_compound_section.html.erb +++ b/app/views/hyrax/compounds/_compound_section.html.erb @@ -5,10 +5,10 @@ Locals: f - form builder for the parent resource compound_name - Symbol, e.g. :contributors - definition - Hash{ subfields:, groups: } from CompoundSchema#definition_for + definition - Hash{ subproperties:, groups: } from CompoundSchema#definition_for display_label - String shown as the section heading - Form params: [_attributes][][] + Form params: [_attributes][][] %> <%# Read rows from the form when it exposes the compound reader, else the model. %> <% source = f.object.respond_to?(compound_name) ? f.object : f.object.model %> diff --git a/config/locales/hyrax.en.yml b/config/locales/hyrax.en.yml index 14f6d5498e..98d9f54139 100644 --- a/config/locales/hyrax.en.yml +++ b/config/locales/hyrax.en.yml @@ -864,7 +864,7 @@ en: remove_row_with_label: Remove %{label} errors: required_but_empty: "%{compound} requires at least one entry" - missing_required_subfields: "Each %{compound} entry requires: %{fields}" + missing_required_subproperties: "Each %{compound} entry requires: %{fields}" participants: label: Participants title: Title @@ -1417,10 +1417,10 @@ en: flexible_schema_validators: compound_validator: errors: - subfields_not_hash: "m3 profile compound property `%{property}` must declare `subfields` as a mapping of sub-field name to its configuration" - subfield_not_hash: "m3 profile compound property `%{property}` sub-field `%{subfield}` must be a mapping (got `%{actual}`)" - controlled_without_source: "m3 profile compound property `%{property}` sub-field `%{subfield}` is `type: controlled` but declares neither `authority` nor `values`" - top_level_indexing: "m3 profile compound property `%{property}` must not declare top-level `indexing`; declare `indexing` (or `index_keys`) on each sub-field instead" + subproperties_not_hash: "m3 profile compound property `%{property}` must declare `subproperties` as a mapping of sub-property name to its configuration" + subproperty_not_hash: "m3 profile compound property `%{property}` sub-property `%{subproperty}` must be a mapping (got `%{actual}`)" + controlled_without_source: "m3 profile compound property `%{property}` sub-property `%{subproperty}` is `type: controlled` but declares neither `authority` nor `values`" + top_level_indexing: "m3 profile compound property `%{property}` must not declare top-level `indexing`; declare `indexing` (or `index_keys`) on each sub-property instead" redirects_validator: errors: invalid_available_on: "m3 profile `redirects` property must be available on at least one work or collection class declared in this profile" diff --git a/config/metadata/compound_metadata.yaml b/config/metadata/compound_metadata.yaml index c62dc16793..5f091cf1d8 100644 --- a/config/metadata/compound_metadata.yaml +++ b/config/metadata/compound_metadata.yaml @@ -1,13 +1,13 @@ # Sample compound (hierarchical) metadata that ships with Hyrax. # # Each attribute here is a compound: a `type: hash, multiple: true` field whose -# entries are plain hashes of named sub-fields. The sub-fields are declared in -# `subfields:` and rendered, populated, and indexed generically by the compound +# entries are plain hashes of named sub-properties. The sub-properties are declared in +# `subproperties:` and rendered, populated, and indexed generically by the compound # foundation — no per-field Ruby or ERB. See # `documentation/forms/compound_fields.md`. # # These are intentionally generic examples demonstrating the supported -# sub-field input types (open-entry `string` and `controlled` vocabulary). +# sub-property input types (open-entry `string` and `controlled` vocabulary). # Applications can add their own compounds the same way (in this file, an # app-level schema, or the m3 profile under HYRAX_FLEXIBLE=true) and may remove # these samples by setting `Hyrax.config.compound_metadata_enabled = false`. @@ -20,9 +20,9 @@ attributes: predicate: http://id.loc.gov/vocabulary/relators/asn form: primary: false - subfields: - # Indexing is declared per sub-field via `index_keys:` (the literal Solr - # field names + suffixes), exactly like scalar fields. A sub-field with + subproperties: + # Indexing is declared per sub-property via `index_keys:` (the literal Solr + # field names + suffixes), exactly like scalar fields. A sub-property with # no `index_keys:` is display-only (rendered from the json blob, not its # own searchable field). `display: false` would omit it from the blob. title: { type: string } @@ -30,7 +30,7 @@ attributes: type: string required: true index_keys: [participant_name_sim, participant_name_tesim] - # A controlled sub-field can take its options from an inline list (below) + # A controlled sub-property can take its options from an inline list (below) # or from an existing QA local authority (`authority: `). participant_role: type: controlled @@ -51,7 +51,7 @@ attributes: predicate: http://purl.org/dc/terms/identifier form: primary: false - subfields: + subproperties: identifier: type: string required: true @@ -64,7 +64,7 @@ attributes: view: render_as: compound html_dl: true - # A rights grouping demonstrating controlled sub-fields backed by *existing* + # A rights grouping demonstrating controlled sub-properties backed by *existing* # QA local authorities (rather than inline `values:`), alongside an # open-ended note. compound_rights: @@ -75,7 +75,7 @@ attributes: default: Rights form: primary: false - subfields: + subproperties: rights_statement: type: controlled authority: rights_statements @@ -90,7 +90,7 @@ attributes: view: render_as: compound html_dl: true - # A related-item grouping demonstrating the `work_or_url` sub-field: the user + # A related-item grouping demonstrating the `work_or_url` sub-property: the user # searches for an internal work (typeahead) OR types an external URL. The # stored value is a work id or a URL; show pages link to the internal work # (by title) or auto-link the external URL. @@ -100,7 +100,7 @@ attributes: predicate: http://purl.org/dc/terms/relation form: primary: false - subfields: + subproperties: related_item: type: work_or_url required: true diff --git a/config/metadata_profiles/m3_profile.yaml b/config/metadata_profiles/m3_profile.yaml index 1bb100d144..3d1acb5a21 100644 --- a/config/metadata_profiles/m3_profile.yaml +++ b/config/metadata_profiles/m3_profile.yaml @@ -954,7 +954,7 @@ properties: property_uri: http://vocabulary.samvera.org/ns#transcriptIds # Sample compound (hierarchical) metadata shipped with Hyrax. Each is a # `type: hash, multiple: true` property whose entries are hashes of the - # `subfields:` keys, rendered/populated/indexed generically by the compound + # `subproperties:` keys, rendered/populated/indexed generically by the compound # foundation. See documentation/forms/compound_fields.md. participants: available_on: @@ -969,8 +969,8 @@ properties: form: primary: false property_uri: http://id.loc.gov/vocabulary/relators/asn - subfields: - # Indexing per sub-field via `indexing:` (literal Solr field names), the + subproperties: + # Indexing per sub-property via `indexing:` (literal Solr field names), the # same mechanism scalar properties use. No `indexing:` => display-only. title: { type: string } participant_name: @@ -1000,7 +1000,7 @@ properties: form: primary: false property_uri: http://purl.org/dc/terms/identifier - subfields: + subproperties: identifier: type: string required: true @@ -1026,7 +1026,7 @@ properties: form: primary: false property_uri: http://purl.org/dc/terms/rights - subfields: + subproperties: rights_statement: type: controlled authority: rights_statements @@ -1054,7 +1054,7 @@ properties: form: primary: false property_uri: http://purl.org/dc/terms/relation - subfields: + subproperties: related_item: type: work_or_url required: true diff --git a/documentation/forms/compound_fields.md b/documentation/forms/compound_fields.md index 6d0cb5d8e7..50b0b5829e 100644 --- a/documentation/forms/compound_fields.md +++ b/documentation/forms/compound_fields.md @@ -1,7 +1,7 @@ # Compound (hierarchical) metadata fields A **compound field** is a metadata attribute whose value is a list of entries, -where each entry is itself a small set of named **sub-fields**. It is the +where each entry is itself a small set of named **sub-properties**. It is the Hyrax foundation for hierarchical metadata: contributors with a name, role and identifier; dates with a value and type; funding references with a funder name and award number; and so on. @@ -15,14 +15,14 @@ and behave identically in both flex modes. ## Declaring a compound A compound is a `type: hash, multiple: true` attribute that also carries a -`subfields:` map: +`subproperties:` map: ```yaml contributors: type: hash multiple: true predicate: https://prvoices.org/terms/contributor - subfields: + subproperties: given_name: type: string index_keys: [contributors_given_name_sim, contributors_given_name_tesim] @@ -42,7 +42,7 @@ contributors: In the m3 profile the same declaration uses `data_type: array`, `type: hash`, and `available_on: class:` to scope the property, with the identical -`subfields:` / `groups:` keys; sub-field index targets are declared with +`subproperties:` / `groups:` keys; sub-property index targets are declared with `indexing:` (the flexible-mode spelling of `index_keys:`). > **Flexible mode:** the active schema is the `Hyrax::FlexibleSchema` row in the @@ -53,9 +53,9 @@ and `available_on: class:` to scope the property, with the identical > singleton class at load time, so a stale running process serves the old > attribute set even when the profile YAML is current. -### `subfields:` +### `subproperties:` -An ordered map of `sub_field_key => { type:, ... }`. Supported `type:` values: +An ordered map of `sub_property_key => { type:, ... }`. Supported `type:` values: | `type:` | Renders as | Notes | |--------------|---------------------|-------| @@ -64,10 +64,10 @@ An ordered map of `sub_field_key => { type:, ... }`. Supported `type:` values: | `work_or_url` | select2 typeahead → linked on show | Searches internal works (via the `compound_works` QA authority, backed by `Hyrax::CompoundWorkPickerBuilder`) **or** accepts a typed external URL. The stored value is a work id or a URL; on show, a work id links to the work's show page with its title (resolved by `Hyrax::CompoundWorkResolver`), a URL is auto-linked. | | `controlled` | `