search

akeneo / pim-community-dev

[Community Development Repository] The open source Product Information Management (PIM)
http://www.akeneo.com
Other
951 stars 516 forks source link

SKU with forward slash results in problems while exporting media files #4119

Closed hostep closed 8 years ago

hostep commented 8 years ago

Hi guys

A client of ours has certain products which include a forward slash in the SKU. For example ABC/D.

If we then perform an export using the EnhancedConnectorBundle module, this puts the image associated with that particular product inside files/ABC/D/image.

We then try to import this using PimGento into Magento. But PimGento isn't picking up the file, since it assumes all the first level subdirectories inside the files directory have the name of the SKU of the product.

In Akeneo this can probably be fixed by replacing the / with for example an _ in the $identifier variable on this line: https://github.com/akeneo/pim-community-dev/blob/cfd6a307a243ae179242c85506e1005b2602899a/src/Pim/Component/Connector/Writer/File/MediaExporterPathGenerator.php#L40 This results in the directory files/ABC_D/image to be generated and the correct filename is being used in the resulting csv file.

But in PimGento, I can't see for a way to fix this right now. See: https://github.com/Agence-DnD/PIMGento/blob/0788350a15946ee6edd026f5f400de6ad6fb7d17/app/code/community/Pimgento/Image/Model/Import.php#L121-L126

You could do a reverse replace on the $sku variable where you replace _ again with /, but this would result in incorrect behavior should SKU's actually exists with _ characters inside them.

I do believe something is really wrong with how PimGento is solving this, but haven't taken a good look at properly solving this.

What do you guys think about all this?

hostep commented 8 years ago

The more I think about it, the more I believe this is an issue in PimGento and not really in Akeneo. So if you guys believe this has nothing to do with Akeneo, feel free to close this ticket.

pierallard commented 8 years ago

Hi @hostep ! Thanks for this complete report!

You're right, exporting with paths containing "/" is a bad idea and can raise issues with other connectors. We added it in our board, with the reference IM-496.

Feel free to create a Pull Request, this one is an easy-pick, we can help you to merge it quickly.

Regards

hostep commented 8 years ago

Hi @pierallard

Thanks for the feedback. How do you suggest to fix this, because I have no idea how to make this work with PimGento without changing code in PimGento?

But regardless of that one connector, I do believe the paths should be filtered on certain characters, but not sure if "/" is actually one of them... (since you only support Linux I believe?)

LaureBrosseau commented 8 years ago

Hello @hostep, We have found a fix and it will be available in the next patch release. We're planning to release it by the end of the week. I'll keep you updated.

Thanks!

LaureBrosseau commented 8 years ago

hello @hostep, the 1.5.5 is out, it contains the fix for this issue. You can now upgrade your PIM.

Best regards,

Laure

hostep commented 8 years ago

Cool, tnx @LaureBro, I'll check it out when I find some time.

Is there a chance this can get back ported to version 1.4.x ?

LaureBrosseau commented 8 years ago

Hello @hostep,

Sorry for the late reply! Unfortunately we will not back port this fix in the 1.4.X version - we mostly fix issues on the latest or Master version. But you can mayve back port it in your version, you'll find the fix here: https://github.com/akeneo/pim-community-dev/pull/4608/files (otherwise why don't you upgrade on the 1.5.X?)

Kind regards,

Laure