Problem/Motivation
We cannot use spaces in migrate source CSV because of the way fetchAssoc in Sql.php returns array our keys without spaces.
Steps to reproduce
Use Migrate Source CSV extension to migrate from a CSV source containing spaces in its header.
Proposed resolution
We keep track of spaces in source and maps them to values returned by Sql fetchAssoc
Remaining tasks
Please review my following patch and eventually update it to 10.2.x-dev if needed.
User interface changes
None
API changes
None
Data model changes
None
Release notes snippet
Issue fork drupal-3422603
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
b2f commentedDiff committed on branch 3422603-fixing-source-ids
Comment #3
b2f commentedAlso should fix not only spaces but also special characters in migrate source CSV.
Comment #4
godotislate@b2f Changes to core are applied to 11.x first, and then possibly cherry-picked down to 10.2.x-dev. Can you rebase your branch (or create a new branch) against 11.x and open an MR?
Comment #6
b2f commented@godotislate, 11.x done in 3422603- and 3422603-fixing-source-id-for-csv 10.2.x (sorry about branch names)
Comment #7
godotislateThanks, @bdf. I ran into the issue with having spaces in column names in my CSV a few months ago, so being able to migrate those without editing the source CSV or other handling would be ideal.
That being said, looks like there are quite a few automated test failures to address, and there probably needs to be test to cover that source ID keys with spaces or special characters works correctly.
Comment #8
godotislateMade some tweaks to address failing tests.
Add test coverage for source field names with spaces and special characters.
Comment #11
smustgrave commentedApologize had this one open the other and didn't finish
Removing the tests tag as that was added here https://git.drupalcode.org/issue/drupal-3422603/-/jobs/909281
Summary appears to match the solution and didn't cause any failures so believe no functionality has been lost.
Believe this is good to go.
Comment #12
alexpottThis is a bug fix and and nice one at that. Backporting to 10.2.x as this is a non-disruptive bug fix.
Committed and pushed 56f8d7594c to 11.x and d67efa58fc to 10.3.x and a30616fb87 to 10.2.x. Thanks!
Comment #16
godotislateCan someone with access please close the MR? Thanks!