Brille24 / SyliusCustomOptionsPlugin

A Sylius plugin that adds customer options
MIT License
47 stars 34 forks source link

Use Sylius 1.12 stable #124

Closed Prometee closed 1 year ago

Prometee commented 1 year ago

Now that Sylius 1.12 has been released 🎉, the stable version can be used. In this PR I just updated the build Sylius version from ~1.12.x-dev to ~1.12.0.

Prometee commented 1 year ago

I will see tomorrow, what caused the build to fail, I think it's due to the support of Symfony 6, so maybe Sylius 1.12 must be only tested along with Symfony ^6.0

Prometee commented 1 year ago

@mamazu can you get an eye on the BeHat error hapenning only with Sylius 1.12 build ? Seems to be related to a recent code change I think.

mamazu commented 1 year ago

I will have a look at it. But looking at the changes it looks like we are dropping 4.4 with this MR?

Prometee commented 1 year ago

Yes we can't keep the test app to be compatible with Symfony 4.4 because the class used in the Kernel class is only available starting Symfony 5.2. I don't found a way to keep both compatibility and I doubt it's possible

mamazu commented 1 year ago

That sounds like a great idea. And in this case I would also combine that with dropping support for php7.3 and lower. But it's odd that the error only occurs on Sylius 1.12

Prometee commented 1 year ago

I agree, it's time !

About the error I can reproduce it on my local env too, I will see if I'm able to xdebug it step by step and see if something as change.

mamazu commented 1 year ago

Well the problem is that in 1.12 we get a different error message when a custom option is not a number. See #126 where I fixed the build for 1.12 but everything else breaks.

Prometee commented 1 year ago

Arf ! Then detecting the version of Sylius to change the error message can be a way to fix it :

\Sylius\Bundle\CoreBundle\Application\Kernel::VERSION_ID >= '11200'
mamazu commented 1 year ago

Well then cherry-pick this commit: https://github.com/Brille24/SyliusCustomOptionsPlugin/pull/126/commits/f364e647f2de519b6c3b5146f2ca3488f853b03c

In the hasInvalidNumberValidationMessage function, for the old versions it's: 'This value is not valid.'

and for the new version it's: 'Please enter a number.'

Prometee commented 1 year ago

And it works 😄

mamazu commented 1 year ago

Perfect thanks. Great work.

Prometee commented 1 year ago

Good team 😉