search

FriendsOfSylius / SyliusImportExportPlugin

Sylius plugin to import / export data
MIT License
118 stars 82 forks source link

fixed the plugin for use in sylius 1.9 - Hotfix #252

Closed debugteam closed 3 years ago

debugteam commented 3 years ago
Q A
Bug fix? yes
New feature? no
BC breaks? no
Fixed tickets ?

made a quick debugging session after installing the plugin on a sylius 1.9 and saw it wasn't working. Instead of learning how the architecture works i just created a decorator around the givven entityRepository to add getImagetype functionality to the repository object and by putting an emtpy string to the initialisation of treebuilder i suppressed an error..

I did a testinstall made a test export and import but i don't recommend this change for the long run and will definatly improve it as soon as i spend more time with the code.

debugteam commented 3 years ago

checks have failed... i have asked about it in the dev chat of sylius on why they didn't use 2.0 instead of 1.9 as new version, since it breaks something. They answered with:

Hey "@jschultz", it’s very sad to hear that. But keep in mind, that you can expect from us what is written in our Backward Compatibility Promise. If you found something, that we broke, but we shouldn’t please raise an issue. A lot of these changes are forced by changes in external libraries and not doing them would slow down the ecosystem. Still, being sad to said that, but this is the trade-off that we’ve decided to do. What is more, version 1.4 has reached EOL over a year ago(according to our Release Cycle). This plugin should be upgraded at least to v1.8.

debugteam commented 3 years ago

Also i personally recommend upgrading to php 7.4. PHP 7.2 has strong issues and shouldn't be used anymore!

lsmith77 commented 3 years ago

thank you .. we will likely merge https://github.com/FriendsOfSylius/SyliusImportExportPlugin/pull/251 sometime this week and make a new release. if there is some hold up there, we may consider a hotfix.

debugteam commented 3 years ago

Didn't check for PRs... but i guess i also recommend merging #251 instead of my 15 minute patch... Trying a new shop platform with a new customer on a video chat.. and then havving to fix the product import is a quiet challenging task... mastered it somehow and thought i share..

lsmith77 commented 3 years ago

no longer needed as #251 was merged. thx for the contribution however!