FeedsCSVParser.inc converts all field headers to lower case. From the comments in the code, and the issue at http://drupal.org/node/754938, this appears to be intentional.
But, this breaks Feeds plugins when used with mixed-case CSV headers. The plugins expect column names to exactly match the headers as they're defined at admin/build/feeds/edit/*/mapping, while FeedsCSVParser is modifying them.
I'm not certain why the CSV parser would need to work in lowercase (other parsers don't seem to have that requirement), so I made a patch that removes all the drupal_strtolower() calls. CSVParser's override of parent::getSourceElement() could probably be removed as well since, but I didn't do so in this patch.
(This patch is working well in my tests. But, I don't know enough about Feeds internals to be sure that it wouldn't cause serious problems elsewhere -- there was a certainly a reason why the strtolower calls were put there in the first place, though I'm not sure what that might be)
Comment | File | Size | Author |
---|---|---|---|
#7 | feeds-feedscsvparser_forced_lowercase_fix-1133724-7.patch | 1.92 KB | mrfree |
feeds_csv_parser_dont_user_lowercase.patch | 1.91 KB | anschauung | |
Comments
Comment #1
twistor CreditAttribution: twistor commentedI'd guess the reason that the source elements are changed to lowercase is to simplify things for the user. Doesn't really seem necessary, but changing it at this point would break importing for people that have specified "title" in their mappings when in the CSV file the header specifies "Title".
I'm pretty sure we could get away with lowercasing every source element across the board which would at least be consistent.
Comment #2
twistor CreditAttribution: twistor commentedBump to 7.x and backport.
Comment #3
MrHaroldA CreditAttribution: MrHaroldA commentedThis should be a nice reason to create a 3.x branch and fix this once and for all. Changing this in a minor release would be most unwelcome.
It has to be fixed sometime, why not soon?
Comment #4
emackn CreditAttribution: emackn commentedlets get a release before we start talking about a 3.x branch ;)
Comment #5
twistor CreditAttribution: twistor commentedGoing ahead and postponing this umm... for a long time.
Comment #6
criznach CreditAttribution: criznach commentedThis is breaking also feeds tamper if the headers have upper case letters. Could we add a setting where the current behavior is the default? It's not always possible to tailor your headers to the import process.
Comment #7
mrfree CreditAttribution: mrfree commentedI've rebased the patch on the latest dev and stripped out the useless getSourceElement() method override.
Comment #8
Anonymous (not verified) CreditAttribution: Anonymous commentedThis does seem to be a strange decision and at least there should be a config option to "Ignore case" rather than just assuming the headers will be lower case.
I've been using Feeds XLS until now which keeps the upper/lower case of header names as they are, so when I had a customer who wanted to use CSV, everything broke.
Comment #9
mikran CreditAttribution: mikran commented#2258601: Relation import fails with missing GUID/entity id closed as duplicate to this.