🔧 Match production Bulkrax split, source ID, and parser settings - #78
ShanaLMoore wants to merge 3 commits into
Conversation
Mirror utk-hyku production config: - Change split delimiter from string '\|' to regex /\s*[|]\s*/ to trim whitespace around pipes - Override default_field_mapping so unmapped fields also pipe-split - Set fill_in_blank_source_identifiers to generate tenant-scoped IDs - Add resource_types to qa_controlled_properties - Remove OAI and XML parsers (CSV and Bagit only) Assisted by: Opus 4.6
There was a problem hiding this comment.
🟡 Changes recommended
Make the QA property update idempotent and add the requested initializer coverage.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Updates Bulkrax configuration to match production behavior for field splitting, source identifiers, parser availability, and QA properties.
Changes:
- Trims whitespace around pipe-delimited fields.
- Adds tenant-scoped blank source identifiers and fallback mappings.
- Restricts parsers to CSV and Bagit and registers
resource_types.
File summaries
| File | Summary and review notes |
|---|---|
config/initializers/default_bulkrax_mappings.rb |
Updates split patterns. Nit (1 vote): add regression coverage for pipe and whitespace handling. |
config/initializers/bulkrax_config.rb |
Configures parsers, identifiers, mappings, and QA properties. Moderate (3 votes): make resource_types registration idempotent. Nit (3 votes): add source-identifier initializer coverage. |
Review details
Suppressed comments (1)
config/initializers/default_bulkrax_mappings.rb:9
- The split policy is changed across all of these mappings, but there is no automated spec for this initializer. A regression in the pipe regex or whitespace handling (including URI/colon values) would pass the existing suite; add focused assertions against the seeded CSV/Bagit mappings.
'abstract' => { from: ['abstract'], split: /\s*[|]\s*/ },
'acquisition_identifier' => { from: ['acquisition_identifier'], split: /\s*[|]\s*/ },
'alternative_title' => { from: ['alternative_title'], split: /\s*[|]\s*/ },
'archival_collection' => { from: ['archival_collection'], split: /\s*[|]\s*/ },
'ark' => { from: ['ark'], split: /\s*[|]\s*/ },
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Use set-union (|=) for qa_controlled_properties to stay idempotent across to_prepare reloads. Add spec covering source identifier format and idempotency of the controlled properties list. Assisted by: claude-opus-4-6
Rubocop: use lambda do..end for multiline, shorthand hash syntax. Add specs for parents/children relationship mapping flags and the semicolon+pipe split regex, restoring coverage lost when the migration-specific spec was deleted. Assisted by: claude-opus-4-6
|
@laritakr Thoughts about matching production? I saw your earlier commit for setting field mappings... |
We wouldn't need to totally match production, but my field mappings were based on their m3 profile and not tested (the default hyku settings weren't adequate because of the compound metadata and their custom profile). We'll still want to adjust for splitting configurations... I don't think the script I used to create the mappings accounted for splitting. |
Story
Refs
🔧 Match production Bulkrax split, source ID, and parser settings
9dd1cb2
Mirror utk-hyku production config:
trim whitespace around pipes
Assisted by: Opus 4.6
Expected Behavior Before Changes
Field splitting uses a literal pipe string with no whitespace trimming.
Unmapped fields get Bulkrax defaults (no split). No auto-generated
source identifiers. OAI and XML parsers are available but unused.
Expected Behavior After Changes
All fields split on pipe with optional surrounding whitespace, matching
production. Blank source identifiers auto-fill with
<tenant>-<importer_id>-<index>. Only CSV and Bagit parsers available.resource_typesincluded in QA-controlled properties.Screenshots / Video
Rails runner verification
Notes
notch8/utk-hykuconfig/initializers/bulkrax.rb(production)field mappings were completed in PR Add UTK Bulkrax field mappings #49