EcomDev / sync-magento-2-migration

Release of rough proof of concept from 2018 that allows to import and export millions of products quickly
Open Software License 3.0
59 stars 11 forks source link

Mismatch between import and export CSV enclosure characters #6

Closed erfanimani closed 1 year ago

erfanimani commented 2 years ago

Export uses a single quote ': https://github.com/EcomDev/sync-magento-2-migration/blob/main/src/CsvFactory.php#L56

Import uses double quotes ": https://github.com/EcomDev/sync-magento-2-migration/blob/main/src/CsvReader.php#L24

Bug introduced in this commit: https://github.com/EcomDev/sync-magento-2-migration/commit/518807c02088e4ff6a0cbd4a09786e9d57bc12cd

ProxiBlue commented 2 years ago

Well spotted, for context, the site used was USA based with Inch (") character everywhere in fields, example '10" Pipe'

ProxiBlue commented 2 years ago

Consider I then later introduced 'and base64 encode/decode options' I think the change was in fact no longer needed to adjust to ' ( from " )

So, I would say that should have been removed, as the fields would then be encoded, and not textual

erfanimani commented 2 years ago

Cool, yeah it would not be needed in that case. This would also be a pretty big BC break I imagine (if others are using this).

ProxiBlue commented 2 years ago

agreed. is also why I added encode/decode as an option, so it is BC. I had the issue with textuals using " character in a lot of places, so encode/decode made more practical sense, and worked a charm.

ProxiBlue commented 2 years ago

the best fix would be to revert back to " in all code ;) so it was same as before, preventing BC break