Fixed SQL apostrophe (') incompatibility

[Lunyyx]

Questions	Answers
Branch?	8.1.x
Description?	Fixed the problem that was caused by apostrophes in SQL queries, this problem happened when the user write something like "Merci d'insérer votre personnalisation" as the apostrophe (') was considered as the end of the string.
Type?	bug fix
Category?	BO
BC breaks?	no
Deprecations?	no
How to test?	Try to duplicate an item with a apostrophe (') in the customization field
Fixed issue or discussion?	Fixes #34442

[prestonBot]

Hello @Lunyyx!

This is your first pull request on the PrestaShop project. Thank you, and welcome to this Open Source community!

[prestonBot]

Hi, thanks for this contribution!

I found some issues with the Pull Request description:

• Your pull request does not seem to fix any issue, consider creating one (see note below) and linking it by writing Fixes #1234.

Would you mind having a look at it? This will help us understand how interesting your contribution is, thank you very much!

About linked issues

Please consider opening an issue before submitting a Pull Request:

• If it's a bug fix, it helps maintainers verify that the bug is effectively due to a defect in the code, and that it hasn't been fixed already.
• It can help trigger a discussion about the best implementation path before a single line of code is written.
• It may lead the Core Product team to mark that issue as a priority, further attracting the maintainers' attention.

(Note: this is an automated message, but answering it will reach a real human)

[Lunyyx]

Hi, thanks for this contribution!

I found some issues with the Pull Request description:

• Your pull request does not seem to fix any issue, consider creating one (see note below) and linking it by writing Fixes #1234.

Would you mind having a look at it? This will help us understand how interesting your contribution is, thank you very much!

About linked issues

Please consider opening an issue before submitting a Pull Request:

• If it's a bug fix, it helps maintainers verify that the bug is effectively due to a defect in the code, and that it hasn't been fixed already.
• It can help trigger a discussion about the best implementation path before a single line of code is written.
• It may lead the Core Product team to mark that issue as a priority, further attracting the maintainers' attention.

(Note: this is an automated message, but answering it will reach a real human)

Fixed the old pull request

[Hlavtox]

I think it's OK. :-)

Some prepared statement or built in escape method would probably be better, but given the scope of that method and how it manually builds the query by joining the fields, it should work just fine. There should be no other possible fails that it could encounter, the rest of the query is not dynamic.

[M0rgan01]

Tests ui: https://github.com/M0rgan01/ga.tests.ui.pr/actions/runs/6731056352

[Hlavtox]

@M0rgan01 I think you ran the tests on develop instead of 8.1.x ;-)

[Lunyyx]

I think it's OK. :-)

Some prepared statement or built in escape method would probably be better, but given the scope of that method and how it manually builds the query by joining the fields, it should work just fine. There should be no other possible fails that it could encounter, the rest of the query is not dynamic.

Perfect ! :)

Yes, I think it's weird that there isn't any buildt in functions to do this, but anyway, it should work without any problem yes

[florine2623]

Hello @Lunyyx ,

The field is not duplicated anymore. The process to checkout is working as expected.

It is QA ✅
Thanks!

[Hlavtox]

@florine2623 I'm not sure I understand the message. :-)

This issue was about an error when duplicating a product in BO.
It didn't work before, now it should work with this PR.

[Hlavtox]

Block to avoid accidental merge

[Lunyyx]

@florine2623 I'm not sure I understand the message. :-)

This issue was about an error when duplicating a product in BO. It didn't work before, now it should work with this PR.

Yes it is, maybe I didn't explained correctly, it's just about duplication of products in the BO as you said.
Do I need to do something, or I just need to wait for someone to accept it ?

[matthieu-rolland]

why shouldn't the PR be merged @Hlavtox ?

[florine2623]

Hello @Hlavtox ,

The field was duplicated here in FO :

And with the PR, the problem does not show up anymore ^^

For me it is OK to be merged

[Hlavtox]

@florine2623 OK, but the problem was in the backoffice, as you can see on video from @paulnoelcholot here - #34442 (comment)

[florine2623]

Hello again @Hlavtox and @Lunyyx ,

Indeed, well it seems that 2 problems are fixed then !
No duplication in FO and no error while duplicating a product in BO, when using an apostrophe ✅

Although, now, I have a deprecation alert in FO whenever I duplicate a product. Whether there's an apostrophe or not :
Deprecated: preg_match(): Passing null to parameter #2 ($subject) of type string is deprecated in /my_shop/classes/Validate.php on line 683

Could you check ?

[kpodemski]

@florine2623 is it really because of this PR? did you check it on installation without it?

[florine2623]

Hi again ^^

This PR can be merged as seen with @Hlavtox .
The other problem that I have is related to #33492 and can be fixed later on !

[Hlavtox]

We checked with florine privately and the issue is not related. So all good. :) Thanks everyone!