New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle boolean returns from RequestSql validation #10426

Merged
merged 1 commit into from Sep 14, 2018

Conversation

Projects
None yet
6 participants
@matks
Contributor

matks commented Sep 13, 2018

Questions Answers
Branch? develop
Description? Avoid SqlQueryValidator crash (fatal error) if SQL Parser returns a boolean error instead of an array
Type? improvement
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket?
How to test? Follow #10316 behavior and see that the "symfony error" page does not show anymore, instead user has a nice error message and can keep working.

This eases behavior of issue #10316.
It does not fix it but at least user has an error message instead of a 500 error page.


This change is Reviewable

@@ -84,6 +84,10 @@ private function getErrors(array $sqlErrors)
$errors = [];
foreach ($sqlErrors as $key => $sqlError) {
if (false === is_array($sqlError)) {
$sqlError = [];

This comment has been minimized.

@PierreRambaud

PierreRambaud Sep 13, 2018

Contributor

Should it be $sqlError = [$sqlError];?

@PierreRambaud

PierreRambaud Sep 13, 2018

Contributor

Should it be $sqlError = [$sqlError];?

This comment has been minimized.

@matks

matks Sep 13, 2018

Contributor

I'm not 100% sure, but the usecase we handle here is $sqlError is a boolean and your idea would not go well with what happens next in the code
https://github.com/PrestaShop/PrestaShop/blob/develop/src/Adapter/SqlManager/SqlQueryValidator.php#L82

@matks

matks Sep 13, 2018

Contributor

I'm not 100% sure, but the usecase we handle here is $sqlError is a boolean and your idea would not go well with what happens next in the code
https://github.com/PrestaShop/PrestaShop/blob/develop/src/Adapter/SqlManager/SqlQueryValidator.php#L82

This comment has been minimized.

@PierreRambaud

PierreRambaud Sep 13, 2018

Contributor

Ok I understand what you mean =)

@PierreRambaud

PierreRambaud Sep 13, 2018

Contributor

Ok I understand what you mean =)

@PierreRambaud PierreRambaud added this to the 1.7.5.0 milestone Sep 13, 2018

@ntiepresta ntiepresta added QA ✔️ and removed waiting for QA labels Sep 14, 2018

@eternoendless

This comment has been minimized.

Show comment
Hide comment
@eternoendless

eternoendless Sep 14, 2018

Member

Thank you @matks

Member

eternoendless commented Sep 14, 2018

Thank you @matks

@eternoendless eternoendless merged commit 66815ab into PrestaShop:develop Sep 14, 2018

2 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment