fix: raise a clear error on blank CSV column headers (#197)#224
Merged
Conversation
A CSV with an empty header cell (e.g. a leading comma) produced a nil header, and prepare_headers crashed with `undefined method 'underscore' for nil`. Merely coercing the header to a string only moves the failure to import time (`Missing column for value <> at index 0`), since an unnamed column maps to no attribute. Detect blank headers up front and raise an ActiveAdminImport::Exception naming the offending column, so the user gets an actionable flash instead of a NoMethodError.
Generalize the blank-header spec into a shared example and exercise a blank column at the beginning (col 1), the middle (col 2) and the end (col 4), asserting the reported column position for each.
Merged
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
1 participant
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.
Closes #197.
Problem
A CSV whose header row contains an empty cell — e.g. a stray leading comma from a spreadsheet export:
parses the first header as
nil, andprepare_headerscrashed onnil.underscore:The fix suggested in the issue (coercing headers with
to_s) does not resolve it — it only moves the failure to import time, because an unnamed""column maps to no attribute:Fix
Detect blank header cells up front and raise an
ActiveAdminImport::Exceptionnaming the offending column (1‑based). That exception is already rescued by the import action, so the user sees an actionable flash instead of aNoMethodError:This is the option B behaviour (fail with a clear message) rather than silently dropping unnamed columns — a stray comma shouldn't quietly discard data.
Tests
New
with a blank column headercontext: uploads a fixture with an empty leading header, asserts the clear error, that noNoMethodErrorleaks, and that nothing is imported.Full suite green locally on Ruby 3.3 / Rails 8.1.3 / AA 3.5.1 / SQLite — 62 examples, 0 failures.