Skip to content

Commit fae856f

Browse files
committed
Handle column headers with numeric suffixes
1 parent 415739c commit fae856f

4 files changed

Lines changed: 354 additions & 6 deletions

File tree

app/services/bulkrax/csv_validation_service/validator.rb

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,14 @@ def initialize(csv_headers, valid_headers, field_metadata, mapping_manager, file
3434
end
3535

3636
# Find required fields that are missing from the CSV
37+
# Headers with numeric suffixes (_1, _2, etc.) are normalized before checking
38+
# (e.g., 'title_1' satisfies the 'title' requirement)
3739
#
3840
# @return [Array<Hash>] Array of hashes with :model and :field keys
3941
def missing_required_fields
4042
@missing_required_fields ||= begin
41-
csv_hdrs = @csv_headers.map { |h| @mapping_manager.mapped_to_key(h) }
43+
# Map headers through field mappings and normalize suffixes
44+
csv_hdrs = @csv_headers.map { |h| normalize_header(@mapping_manager.mapped_to_key(h)) }.uniq
4245

4346
missing = []
4447
@field_metadata.each do |model, metadata|
@@ -52,10 +55,17 @@ def missing_required_fields
5255
end
5356

5457
# Find headers in CSV that are not recognized as valid fields
58+
# Headers with numeric suffixes (_1, _2, etc.) are normalized to their base form
59+
# before comparison (e.g., 'creator_1' is treated as 'creator')
5560
#
5661
# @return [Array<String>] Array of unrecognized header names
5762
def unrecognized_headers
58-
@unrecognized_headers ||= @csv_headers - @valid_headers
63+
@unrecognized_headers ||= begin
64+
@csv_headers.reject do |header|
65+
normalized = normalize_header(header)
66+
@valid_headers.include?(header) || @valid_headers.include?(normalized)
67+
end
68+
end
5969
end
6070

6171
# Check if CSV is valid (no missing required fields and has headers)
@@ -71,6 +81,23 @@ def valid?
7181
def warnings?
7282
unrecognized_headers.any? || (@file_validator&.missing_files&.any? || false)
7383
end
84+
85+
private
86+
87+
# Normalize a header by stripping numeric suffixes
88+
# Handles multi-valued fields where CSV columns like 'creator_1', 'creator_2'
89+
# should be recognized as valid if 'creator' is a valid field
90+
#
91+
# @param header [String] The header to normalize
92+
# @return [String] The normalized header (without numeric suffix)
93+
#
94+
# @example
95+
# normalize_header('creator_1') # => 'creator'
96+
# normalize_header('title_2') # => 'title'
97+
# normalize_header('source_identifier') # => 'source_identifier'
98+
def normalize_header(header)
99+
header.sub(/_\d+\z/, '')
100+
end
74101
end
75102
end
76103
end

app/services/bulkrax/stepper_response_formatter.rb

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ def format
8484
# Build formatted response with messages structure
8585
{
8686
headers: @data[:headers],
87-
missingRequired: @data[:missingRequired],
87+
missingRequired: normalize_missing_required(@data[:missingRequired]),
8888
unrecognized: @data[:unrecognized],
8989
rowCount: @data[:rowCount],
9090
isValid: @data[:isValid],
@@ -112,6 +112,19 @@ def already_formatted?
112112
@data[:messages].key?(:validationStatus)
113113
end
114114

115+
# Normalize missingRequired to array of strings
116+
# Converts [{model: 'Work', field: 'title'}] to ['title']
117+
#
118+
# @param missing_required [Array] Array of hashes or strings
119+
# @return [Array<String>] Array of field names only
120+
def normalize_missing_required(missing_required)
121+
return [] unless missing_required
122+
123+
missing_required.map do |item|
124+
item.is_a?(Hash) ? item[:field] : item
125+
end
126+
end
127+
115128
# Build the messages structure with validationStatus and issues
116129
#
117130
# @return [Hash] Messages structure for frontend
@@ -173,14 +186,16 @@ def details_message(recognized)
173186
#
174187
# @return [Hash] Missing required fields issue structure
175188
def missing_required_issue
189+
normalized = normalize_missing_required(@data[:missingRequired])
190+
176191
{
177192
type: 'missing_required_fields',
178193
severity: 'error',
179194
icon: 'fa-times-circle',
180195
title: 'Missing Required Fields',
181-
count: @data[:missingRequired].length,
196+
count: normalized.length,
182197
description: 'These required columns must be added to your CSV:',
183-
items: @data[:missingRequired].map { |field| { field: field, message: 'add this column to your CSV' } },
198+
items: normalized.map { |field| { field: field, message: 'add this column to your CSV' } },
184199
defaultOpen: false
185200
}
186201
end
Lines changed: 252 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,252 @@
1+
# frozen_string_literal: true
2+
3+
require 'rails_helper'
4+
5+
RSpec.describe Bulkrax::CsvValidationService::Validator do
6+
let(:mapping_manager) { instance_double(Bulkrax::CsvValidationService::MappingManager) }
7+
let(:file_validator) { nil }
8+
9+
let(:field_metadata) do
10+
{
11+
'Work' => {
12+
properties: %w[title creator description],
13+
required_terms: %w[title source_identifier],
14+
controlled_vocab_terms: []
15+
},
16+
'Collection' => {
17+
properties: %w[title description],
18+
required_terms: %w[title],
19+
controlled_vocab_terms: []
20+
}
21+
}
22+
end
23+
24+
let(:valid_headers) do
25+
%w[model source_identifier title creator description parent file]
26+
end
27+
28+
describe '#unrecognized_headers' do
29+
context 'with standard headers' do
30+
it 'returns empty array when all headers are recognized' do
31+
csv_headers = %w[title creator description]
32+
validator = described_class.new(csv_headers, valid_headers, field_metadata, mapping_manager)
33+
34+
expect(validator.unrecognized_headers).to be_empty
35+
end
36+
37+
it 'identifies unrecognized headers' do
38+
csv_headers = %w[title creator invalid_field another_bad_one]
39+
validator = described_class.new(csv_headers, valid_headers, field_metadata, mapping_manager)
40+
41+
expect(validator.unrecognized_headers).to contain_exactly('invalid_field', 'another_bad_one')
42+
end
43+
end
44+
45+
context 'with numeric suffixes' do
46+
it 'recognizes headers with _1 suffix as valid' do
47+
csv_headers = %w[title_1 creator_1 description_1]
48+
validator = described_class.new(csv_headers, valid_headers, field_metadata, mapping_manager)
49+
50+
expect(validator.unrecognized_headers).to be_empty
51+
end
52+
53+
it 'recognizes headers with _2 suffix as valid' do
54+
csv_headers = %w[title_2 creator_2]
55+
validator = described_class.new(csv_headers, valid_headers, field_metadata, mapping_manager)
56+
57+
expect(validator.unrecognized_headers).to be_empty
58+
end
59+
60+
it 'recognizes headers with multi-digit suffixes' do
61+
csv_headers = %w[title_10 creator_99 description_123]
62+
validator = described_class.new(csv_headers, valid_headers, field_metadata, mapping_manager)
63+
64+
expect(validator.unrecognized_headers).to be_empty
65+
end
66+
67+
it 'identifies unrecognized headers even with numeric suffixes' do
68+
csv_headers = %w[title_1 invalid_field_1 creator_2 another_bad_2]
69+
validator = described_class.new(csv_headers, valid_headers, field_metadata, mapping_manager)
70+
71+
expect(validator.unrecognized_headers).to contain_exactly('invalid_field_1', 'another_bad_2')
72+
end
73+
74+
it 'handles mix of suffixed and non-suffixed headers' do
75+
csv_headers = %w[title title_1 title_2 creator creator_1 description]
76+
validator = described_class.new(csv_headers, valid_headers, field_metadata, mapping_manager)
77+
78+
expect(validator.unrecognized_headers).to be_empty
79+
end
80+
81+
it 'does not strip non-numeric suffixes' do
82+
csv_headers = %w[title_abc creator_xyz]
83+
validator = described_class.new(csv_headers, valid_headers, field_metadata, mapping_manager)
84+
85+
expect(validator.unrecognized_headers).to contain_exactly('title_abc', 'creator_xyz')
86+
end
87+
88+
it 'handles underscores in field names correctly' do
89+
csv_headers = %w[source_identifier source_identifier_1 source_identifier_2]
90+
validator = described_class.new(csv_headers, valid_headers, field_metadata, mapping_manager)
91+
92+
expect(validator.unrecognized_headers).to be_empty
93+
end
94+
end
95+
end
96+
97+
describe '#missing_required_fields' do
98+
before do
99+
allow(mapping_manager).to receive(:mapped_to_key) { |h| h }
100+
end
101+
102+
it 'returns empty array when all required fields are present' do
103+
csv_headers = %w[title source_identifier model]
104+
validator = described_class.new(csv_headers, valid_headers, field_metadata, mapping_manager)
105+
106+
expect(validator.missing_required_fields).to be_empty
107+
end
108+
109+
it 'identifies missing required fields' do
110+
csv_headers = %w[creator description]
111+
validator = described_class.new(csv_headers, valid_headers, field_metadata, mapping_manager)
112+
113+
missing = validator.missing_required_fields
114+
expect(missing).to include({ model: 'Work', field: 'title' })
115+
expect(missing).to include({ model: 'Work', field: 'source_identifier' })
116+
expect(missing).to include({ model: 'Collection', field: 'title' })
117+
end
118+
119+
it 'returns unique missing fields across models' do
120+
csv_headers = %w[description]
121+
validator = described_class.new(csv_headers, valid_headers, field_metadata, mapping_manager)
122+
123+
missing = validator.missing_required_fields
124+
title_missing = missing.select { |m| m[:field] == 'title' }
125+
126+
# Both Work and Collection require title, so we should see 2 entries
127+
expect(title_missing.length).to eq(2)
128+
end
129+
130+
it 'works with mapped column names' do
131+
allow(mapping_manager).to receive(:mapped_to_key) do |header|
132+
{ 'work_title' => 'title', 'identifier' => 'source_identifier' }[header] || header
133+
end
134+
135+
csv_headers = %w[work_title identifier]
136+
validator = described_class.new(csv_headers, valid_headers, field_metadata, mapping_manager)
137+
138+
expect(validator.missing_required_fields).to be_empty
139+
end
140+
141+
context 'with numeric suffixes' do
142+
it 'recognizes title_1 as satisfying title requirement' do
143+
csv_headers = %w[title_1 source_identifier]
144+
validator = described_class.new(csv_headers, valid_headers, field_metadata, mapping_manager)
145+
146+
expect(validator.missing_required_fields).to be_empty
147+
end
148+
149+
it 'recognizes multiple suffixed headers as satisfying requirement' do
150+
csv_headers = %w[title_1 title_2 source_identifier_1]
151+
validator = described_class.new(csv_headers, valid_headers, field_metadata, mapping_manager)
152+
153+
expect(validator.missing_required_fields).to be_empty
154+
end
155+
156+
it 'still identifies missing required fields when suffixed headers do not match' do
157+
csv_headers = %w[creator_1 description_2]
158+
validator = described_class.new(csv_headers, valid_headers, field_metadata, mapping_manager)
159+
160+
missing = validator.missing_required_fields
161+
expect(missing).to include({ model: 'Work', field: 'title' })
162+
expect(missing).to include({ model: 'Work', field: 'source_identifier' })
163+
expect(missing).to include({ model: 'Collection', field: 'title' })
164+
end
165+
166+
it 'works with mix of suffixed and non-suffixed required fields' do
167+
csv_headers = %w[title source_identifier_1]
168+
validator = described_class.new(csv_headers, valid_headers, field_metadata, mapping_manager)
169+
170+
expect(validator.missing_required_fields).to be_empty
171+
end
172+
end
173+
end
174+
175+
describe '#valid?' do
176+
before do
177+
allow(mapping_manager).to receive(:mapped_to_key) { |h| h }
178+
end
179+
180+
it 'returns true when all required fields are present' do
181+
csv_headers = %w[title source_identifier model]
182+
validator = described_class.new(csv_headers, valid_headers, field_metadata, mapping_manager)
183+
184+
expect(validator).to be_valid
185+
end
186+
187+
it 'returns false when required fields are missing' do
188+
csv_headers = %w[description creator]
189+
validator = described_class.new(csv_headers, valid_headers, field_metadata, mapping_manager)
190+
191+
expect(validator).not_to be_valid
192+
end
193+
194+
it 'returns false when CSV has no headers' do
195+
csv_headers = []
196+
validator = described_class.new(csv_headers, valid_headers, field_metadata, mapping_manager)
197+
198+
expect(validator).not_to be_valid
199+
end
200+
end
201+
202+
describe '#warnings?' do
203+
before do
204+
allow(mapping_manager).to receive(:mapped_to_key) { |h| h }
205+
end
206+
207+
it 'returns true when there are unrecognized headers' do
208+
csv_headers = %w[title creator invalid_field]
209+
validator = described_class.new(csv_headers, valid_headers, field_metadata, mapping_manager)
210+
211+
expect(validator.warnings?).to be true
212+
end
213+
214+
it 'returns false when all headers are recognized' do
215+
csv_headers = %w[title creator description]
216+
validator = described_class.new(csv_headers, valid_headers, field_metadata, mapping_manager)
217+
218+
expect(validator.warnings?).to be false
219+
end
220+
221+
context 'with file validator' do
222+
let(:file_validator) do
223+
instance_double(
224+
Bulkrax::CsvValidationService::FileValidator,
225+
missing_files: missing_files
226+
)
227+
end
228+
229+
context 'when files are missing' do
230+
let(:missing_files) { ['image1.jpg', 'doc.pdf'] }
231+
232+
it 'returns true' do
233+
csv_headers = %w[title creator]
234+
validator = described_class.new(csv_headers, valid_headers, field_metadata, mapping_manager, file_validator)
235+
236+
expect(validator.warnings?).to be true
237+
end
238+
end
239+
240+
context 'when no files are missing' do
241+
let(:missing_files) { [] }
242+
243+
it 'returns false when headers are also valid' do
244+
csv_headers = %w[title creator]
245+
validator = described_class.new(csv_headers, valid_headers, field_metadata, mapping_manager, file_validator)
246+
247+
expect(validator.warnings?).to be false
248+
end
249+
end
250+
end
251+
end
252+
end

0 commit comments

Comments
 (0)