Add nested_attributes flag for object: mappings#1193
Merged
Conversation
Field mappings can now declare nested_attributes: true alongside an object: target. When the flag is set, Bulkrax routes the imported data through parsed_metadata['<name>_attributes'] as a numbered-key hash with '_destroy: false' markers — the shape Reform's nested-attributes machinery and similar populators consume directly. Without the flag, callers had to add hardcoded translators (e.g. ValkyrieObjectFactory#convert_based_near_to_attributes) to rename parsed_metadata['<name>'] into the form their downstream form expected. The flag generalizes that rename so each new nested-attribute consumer declares its needs in the mapping rather than in factory code. Also makes object_metadata on export tolerant of plain hashes (in addition to the legacy stringified-hash literal). Resources backed by JSONB or similar non-string persistence return Ruby hashes directly; calling eval on them would fail. The legacy stringified path still works for ActiveFedora-backed resources.
4 tasks
Confirms that a single field-mapping declaration drives both directions: imported numbered columns become _attributes-shaped data the form populator consumes, and the persisted plain-hash array exports back to the same numbered columns.
When a field-mapping declares nested_attributes: true, the parsed metadata arrives at the object factory with a *_attributes virtual key (e.g. redirects_attributes). The slice in #transform_attributes was dropping these keys because permitted_attributes only listed bare schema properties, so downstream form populators received nothing and the data silently disappeared. permitted_attributes now allows a *_attributes key through whenever the corresponding bare property is itself permitted. Generic — any nested-attributes property added in the future works without further changes here.
When a property declared on a model (e.g. redirects) is the target of one or more `object:` field-mappings, the CSV template now emits each of those mappings' `from:` columns (e.g. redirect_path, redirect_canonical, redirect_sequence) instead of the bare property name. Adopters get a template that matches the column shape Bulkrax actually consumes for nested-attribute imports.
Headers ending in `_<digits>` previously bypassed validation regardless of the base name, which masked typos like `creater_1` at validation time even though the real importer would fail to map the column. A suffixed header now passes validation only when its base name is itself recognised — either present in valid_headers directly or resolvable via the mapping manager to a known property — mirroring the recognition rule already used for unsuffixed headers in find_unrecognized_validation_headers. Adds spec coverage for both the bare and numbered forms of an `object:` mapping with `nested_attributes: true`, and a regression spec for the typo-with-suffix case the new rule now catches.
The CSV builder prunes columns whose data rows are all blank, so per-
child columns from `object:` mappings (e.g. redirect_path) were getting
dropped from the downloaded template even when they appeared in the
column-builder output. The row builder didn't recognise them as
belonging to a known property — `mapped_to_key('redirect_path')`
returned `path`, which isn't in the model's properties list.
ValueDeterminer now consults the mapping's `object:` value: if the
key isn't a property but its object name is, the cell is filled
("Required"/"Optional") based on the parent property. The columns
survive the empty-column pruning and reach the user.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds end-to-end support for object: field mappings that need Reform-style *_attributes virtual properties (via a new nested_attributes mapping flag), and updates CSV template/export/validation behavior to treat object-mapped child columns as first-class.
Changes:
- Route
object:imports to<object>_attributeswhen mappings declarenested_attributes: true, and permit those keys through object factories. - Improve CSV template generation/value determination to emit and retain per-child object columns (instead of the bare parent property).
- Tighten CSV header validation so
_<digits>suffixes are only accepted when the base header is recognized; update export to handle plain-Hash object values.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/services/bulkrax/csv_template/value_determiner_spec.rb | Adds coverage for required/optional determination of object-mapped child columns. |
| spec/services/bulkrax/csv_template/mapping_manager_spec.rb | Adds coverage for collecting child columns targeting an object: parent. |
| spec/services/bulkrax/csv_template/csv_parser_template_spec.rb | Adds coverage for rejecting misspelled suffixed headers (e.g., creater_1). |
| spec/services/bulkrax/csv_template/column_builder_spec.rb | Adds coverage for emitting object child columns in templates instead of bare parent properties. |
| spec/parsers/bulkrax/csv_parser/csv_validation_helpers_spec.rb | Adds coverage for accepting bare + numbered per-child columns for nested_attributes: true mappings. |
| spec/models/bulkrax/object_factory_spec.rb | Ensures *_attributes keys survive attribute slicing when the bare property is permitted. |
| spec/models/bulkrax/csv_entry_spec.rb | Adds coverage for nested-attributes import shape and for export from plain-Hash object arrays, plus round-trip behavior. |
| app/services/bulkrax/csv_template/value_determiner.rb | Uses object: parent property to decide required/optional for child columns. |
| app/services/bulkrax/csv_template/mapping_manager.rb | Adds get_object_name and object_columns_for helpers for template generation. |
| app/services/bulkrax/csv_template/column_builder.rb | Emits object child columns for properties targeted by object: mappings. |
| app/parsers/concerns/bulkrax/csv_parser/csv_validation.rb | Restricts acceptance of _<digits> suffixed headers to recognized bases. |
| app/models/concerns/bulkrax/has_matchers.rb | Implements nested_attributes: true routing to <object>_attributes with numbered keys and _destroy. |
| app/models/bulkrax/csv_entry.rb | Allows object export when object values are plain Hashes (Valkyrie/JSONB) as well as legacy stringified forms. |
| app/factories/bulkrax/valkyrie_object_factory.rb | Permits *_attributes keys when the corresponding bare property is permitted (schema-aware). |
| app/factories/bulkrax/object_factory_interface.rb | Permits *_attributes keys when the corresponding bare property is permitted (ActiveFedora path). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ShanaLMoore
approved these changes
May 8, 2026
The known-property set was being rebuilt on every suffixed header header_base_recognized? checked. Computing the set once in check_headers and passing it down avoids the repeated allocations during validation of CSVs with many numbered columns. Behavior is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
object:field mappings that route through Reform-style*_attributesvirtual properties — a single mapping declaration now drives import, export, and the downloadable CSV template, with no per-attribute factory code required.Details
The problem
Bulkrax's
object:field-mapping pattern groups numbered CSV columns (e.g.creator_first_name_1,creator_first_name_2) into a single nested attribute on the resource. This works for resources whose attribute setter accepts a plain array of hashes — but many form objects (notably Reform-basedHyrax::Forms::ResourceFormsubclasses) strip the bare attribute name during deserialization and only accept the*_attributesform: a numbered-key hash with_destroymarkers per row.For those targets, the only existing path was a hardcoded translator in
Bulkrax::ValkyrieObjectFactory#transform_attributes(theconvert_based_near_to_attributesmethod). Every new nested-attribute property would require a parallel translator. There was no declarative mapping option, and several other parts of the pipeline silently broke when the data shape didn't fit:permitted_attributesslice dropped any*_attributeskey before it reached the form.object_metadatacalledevalon each entry, which fails on plain hashes from JSONB-backed resources.redirects) instead of the per-child columns adopters actually need._<digits>-suffixed header was accepted unconditionally, masking typos in numbered columns.The change
Each piece of the pipeline now treats
object:mappings as first-class:nested_attributes: truemapping flag. When set, Bulkrax writes imported data toparsed_metadata['<object>_attributes']as a numbered-key hash with_destroy: 'false'per row — the shape Reform's nested-attributes machinery consumes. Adopters declare the flag in their field mapping; no factory edit required.*_attributeskeys. When the bare property is permitted, the corresponding*_attributeskey passes throughtransform_attributesinstead of being sliced out. Generic — any nested-attribute property added in the future works without further changes.object_metadataaccepts either a stringified Ruby hash literal (legacy ActiveFedora form) or a plain Hash (Valkyrie/JSONB form). The legacy path still works.object:mappings, the template emits each mapping'sfrom:column instead of the bare property name. Numbering is intentionally omitted — adopters add_1,_2, etc. at import time.object:value is treated as belonging to its parent property, so the empty-column pruner doesn't drop it from the download._<digits>is recognized only when its base name is itself a known property or mapping alias. This fixes a pre-existing typo-masking issue surfaced during this work.Backward compatibility
The existing
convert_based_near_to_attributestranslator stays in place. Adopters using based_near keep working without changes. A follow-up issue (#1194) tracks deprecating that translator in favor of the new flag.Testing
nested_attributes: trueagainst a Reform-based form (e.g. a HyraxResourceFormsubclass with a virtual*_attributesproperty). Import a CSV and confirm the data populates the resource end-to-end.object:mapping without the flag still produces the original array-of-hashes shape onparsed_metadata[<name>](no regression on unflagged callers).object:-mapped attribute returns plain Ruby hashes (Valkyrie/JSONB-backed) and confirm the numbered CSV columns are populated correctly.object:-mapped attribute returns the legacy stringified-hash form and confirm it still round-trips.redirect_path,redirect_canonical) appear in the downloaded file with explanation rows populated.creater_1). Confirm the validator flags it as unrecognized.