Skip to content
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

Fix potential TypeError in ProductLazyArray.php #34376

Merged
merged 1 commit into from Nov 6, 2023

Conversation

hugofintecture
Copy link
Contributor

@hugofintecture hugofintecture commented Oct 24, 2023

Questions Answers
Branch? develop
Description? See bellow
Type? bug fix
Category? FO
BC breaks? no
Deprecations? no
How to test? See bellow.
UI Tests
Fixed issue or discussion? Fixes #34483
Related PRs N/A
Sponsor company Fintecture

Fix: Fatal error: Uncaught TypeError: reset(): ProductLazyArray.php(564)

When switch from French to English gb in the order confirmation page we have this error:
image

Don't know if it's normal to receive a string however but at least it don't break the page :)

I tried to print all values with this code in the function to find the responsible value:

echo '<pre>';
var_dump($this->product['attributes']);
echo '</pre>';

And here's the problematic string:

string(9) "Taille: S"

@hugofintecture hugofintecture requested a review from a team as a code owner October 24, 2023 13:45
@prestonBot prestonBot added 8.1.x Branch Bug fix Type: Bug fix labels Oct 24, 2023
Copy link
Member

@boherm boherm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution @hugofintecture 😉

@Hlavtox
Copy link
Contributor

Hlavtox commented Oct 25, 2023

@hugofintecture Nice catch Hugo! I checked the code and the issue happens because ProductLazyArray can be inflated by two data sources.

Normally, attributes contains Product::getAttributesParams -

$attributes = Product::getAttributesParams($row['id_product'], $row['id_product_attribute']);

But in context of a cart, it's a string of joined attributes -

self::$_attributesLists[$productAttributeKey] ?? self::DEFAULT_ATTRIBUTES_KEYS

Not sure how to fix it safely...

@mflasquin mflasquin added the Waiting for QA Status: action required, waiting for test feedback label Nov 2, 2023
@florine2623
Copy link
Contributor

Hello @hugofintecture ,

I can't reproduce your issue without the PR.

Screen.Recording.2023-11-03.at.14.25.49.mov

Normally you should have created an issue before so we can validate it before creating a PR to fix it ^^

@florine2623 florine2623 added Waiting for author Status: action required, waiting for author feedback and removed Waiting for QA Status: action required, waiting for test feedback labels Nov 3, 2023
@PrestaShop PrestaShop deleted a comment from prestonBot Nov 6, 2023
@Hlavtox
Copy link
Contributor

Hlavtox commented Nov 6, 2023

Hi, @AureRita just reproduced here - #34483, can you do the QA?
Not sure why @florine2623 didn't reproduce. 🤔 Maybe this code issue is fatal only on develop?

This definitely is a working fix however. :-)

@Hlavtox Hlavtox added Waiting for QA Status: action required, waiting for test feedback and removed Waiting for author Status: action required, waiting for author feedback labels Nov 6, 2023
@florine2623 florine2623 self-assigned this Nov 6, 2023
Copy link
Contributor

@florine2623 florine2623 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is QA ✅

Before :
Screenshot 2023-11-06 at 16 28 47

After :

Screen.Recording.2023-11-06.at.16.30.21.mov

Tested with other languages as well, works as expected.

Thanks 💪

@florine2623 florine2623 added QA ✔️ Status: check done, code approved and removed Waiting for QA Status: action required, waiting for test feedback labels Nov 6, 2023
@prestonBot
Copy link
Collaborator

QA approved, well done! Message to the maintainers: do not forget to milestone it before the merge.

@Hlavtox
Copy link
Contributor

Hlavtox commented Nov 6, 2023

All good! Merging, thanks everyone

@Hlavtox Hlavtox merged commit aeb76e3 into PrestaShop:develop Nov 6, 2023
18 checks passed
@Hlavtox Hlavtox added this to the 9.0.0 milestone Nov 6, 2023
@prestonBot prestonBot added the develop Branch label Nov 6, 2023
@hugofintecture hugofintecture deleted the patch-3 branch December 18, 2023 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.1.x Branch Bug fix Type: Bug fix develop Branch QA ✔️ Status: check done, code approved
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Order Confirmation : Switch language is broken
9 participants