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

Command icon 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:

About issue forks

Comments

b2f created an issue. See original summary.

b2f’s picture

b2f commented

Diff committed on branch 3422603-fixing-source-ids

b2f’s picture

b2f commented

Also should fix not only spaces but also special characters in migrate source CSV.

godotislate’s picture

godotislate
he/him
commented

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

b2f opened merge request !6705

b2f’s picture

b2f commented

@godotislate, 11.x done in 3422603- and 3422603-fixing-source-id-for-csv 10.2.x (sorry about branch names)

godotislate’s picture

godotislate
he/him
commented
Version: 10.1.x-dev » 11.x-dev
Category: Bug report » Feature request
Issue tags: +Needs tests

Thanks, @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.

godotislate’s picture

godotislate
he/him
commented
Status: Active » Needs review

Made some tweaks to address failing tests.

Add test coverage for source field names with spaces and special characters.

godotislate changed the visibility of the branch 3422603-fixing-source-ids to hidden.

godotislate changed the visibility of the branch 3422603-fixing-source-id-for-csv to hidden.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests +Needs Review Queue Initiative

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

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 11.x-dev » 10.2.x-dev
Category: Feature request » Bug report
Status: Reviewed & tested by the community » Fixed

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

  • alexpott committed a30616fb on 10.2.x 
    Issue #3422603 by b2f, godotislate: Fixing source IDs with spaces in Sql...

  • alexpott committed d67efa58 on 10.3.x 
    Issue #3422603 by b2f, godotislate: Fixing source IDs with spaces in Sql...

  • alexpott committed f45ae4c1 on 11.x 
    Issue #3422603 by b2f, godotislate: Fixing source IDs with spaces in Sql...
godotislate’s picture

godotislate
he/him
commented

Can someone with access please close the MR? Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.