No documentation re: fixing source IDs with spaces

it would be preferable to have a code solution to allow spaces in CSV column names and simply remove spaces before mapping columns to source IDs

Totally agree ! And I don't even understand why there is such a heavy restriction on IDs titles since they are not even stored afaik ? The cell value is stored, not the header label, this requirement needs clarification IMO.

Apparently this comes from a problem in \Drupal\migrate\Plugin\migrate\id_map\Sql::getSourceIdsHash https://www.drupal.org/project/migrate_source_csv/issues/3379184 📌 Document that source IDs can only contain alphanumeric characters or underscores Fixed ...

It looks like the only problem is a warning on rollbacks in such case where you have spaces in the header.

[warning] Undefined array key "some CSV header key" Sql.php:232

This warning seems totally non blocking (hence does not justify the constraint in ids) and perhaphs should be tackled in Sql.php ?

There you go, as I noticed the warning also is symptom of not actually doing the rollback (only in migrate_map, the actual content is not rollbacked), I patch Sql.php in core to map the values to correct IDs.

Branch migrate_source_csv-3413894 ready for merge once core is updated (hopefully).

Fix for 🐛 Fixing source IDs with spaces in Sql.php Fixed has been committed to 10.2.x-dev, so it should be out in the next 10.2 release. I think this issue then can be converted to be a revert of 📌 Add Drupal 10 support. Fixed and this commit: https://git.drupalcode.org/project/migrate_source_csv/-/commit/91b1488b8...

If it's possible, a test should be added that rolling back with CSV columns containing spaces doesn't emit warnings like [warning] Undefined array key "Some Header with Space" Sql.php:232. I'm not sure whether a core version constraint should be added as well.

Pushed MR with commit to revert. Haven't added the test I mentioned, so leaving at Needs work.

Could we do something less drastic and not remove all the test coverage, but rather adjust it to handle spaces too? Same with the exception?

Could we do something less drastic and not remove all the test coverage, but rather adjust it to handle spaces too? Same with the exception?

There's no longer any restriction on source column names. See https://git.drupalcode.org/project/drupal/-/blob/f45ae4c1201426086212343...

Resolved merge conflict and rebased. As mentioned in #13, core has test coverage to make sure spaces and special characters in source names work (see https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/migrate/tests/src/Unit/MigrateSqlIdMapTest.php?ref_type=heads#L644), so I don't think it's necessary to duplicate here.

Moving to NR.

Thanks for your contributions.