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)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

twistor’s picture

I'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.

twistor’s picture

Version: 6.x-1.0-beta10 » 7.x-2.x-dev

Bump to 7.x and backport.

MrHaroldA’s picture

This 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?

emackn’s picture

Status: Needs review » Needs work

lets get a release before we start talking about a 3.x branch ;)

twistor’s picture

Status: Needs work » Postponed

Going ahead and postponing this umm... for a long time.

criznach’s picture

This 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.

mrfree’s picture

I've rebased the patch on the latest dev and stripped out the useless getSourceElement() method override.

Anonymous’s picture

This 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.

mikran’s picture