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

Fixes issue #10417 incorrect argument order #10428

Merged
merged 1 commit into from Sep 17, 2018

Conversation

Projects
None yet
5 participants
@benwallis

benwallis commented Sep 13, 2018

Questions Answers
Branch? develop
Description? The call to function getManufacturers had the arguments in the wrong order, meaning it would only ever work for lanugage with ID of 1. Also changed argument $getNbProducts from "true" to "false" as the resulting value is not used here.
Type? bug fix
Category? FO
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #10417
How to test? Check Manufacturer::getManufacturers() argument order

This change is Reviewable

Ben Wallis
Fixes issue #10417 incorrect argument order when calling Manufacturer…
…::getManufacturers()

Also changes true to false so as not to count number of products (as the resulting value is not used here)
@prestonBot

This comment has been minimized.

Show comment
Hide comment
@prestonBot

prestonBot Sep 13, 2018

Collaborator

Hello @benwallis!

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

Collaborator

prestonBot commented Sep 13, 2018

Hello @benwallis!

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

@PierreRambaud

This comment has been minimized.

Show comment
Hide comment
@PierreRambaud

PierreRambaud Sep 13, 2018

Contributor

Thanks @benwallis, nice catch :)
Is there a way to test this in BO or FO?

Contributor

PierreRambaud commented Sep 13, 2018

Thanks @benwallis, nice catch :)
Is there a way to test this in BO or FO?

@benwallis

This comment has been minimized.

Show comment
Hide comment
@benwallis

benwallis Sep 14, 2018

To reproduce error pre fix:

Change language to something other than English (English) (something with ID greater than 1).
Add a long description to a brand in Admin > Catalog > Brands & Suppliers
Edit a template in the current theme and add:

<pre>{Manufacturer::getLiteManufacturersList()|@print_r}</pre>

View on front end.
Result: The description is empty.

After applying the fix, the description is displayed.

benwallis commented Sep 14, 2018

To reproduce error pre fix:

Change language to something other than English (English) (something with ID greater than 1).
Add a long description to a brand in Admin > Catalog > Brands & Suppliers
Edit a template in the current theme and add:

<pre>{Manufacturer::getLiteManufacturersList()|@print_r}</pre>

View on front end.
Result: The description is empty.

After applying the fix, the description is displayed.

@ntiepresta

This comment has been minimized.

Show comment
Hide comment
@ntiepresta

ntiepresta Sep 14, 2018

Hi, @benwallis ,
without applying the fix, the description exist, and is not empty.
Best regards, Reddy Ntie

ntiepresta commented Sep 14, 2018

Hi, @benwallis ,
without applying the fix, the description exist, and is not empty.
Best regards, Reddy Ntie

@benwallis

This comment has been minimized.

Show comment
Hide comment
@benwallis

benwallis Sep 14, 2018

That's because the two brands already there (Studio Design and Graphic Corner) have descriptions with id_lang of 1.

If you changed default language and then edited the description (so language dropdown to right of textarea is not "en") then you would not see your updated description.
Alternatively add a new brand and set a description (for language other than "en").

Another way to test would be to manually delete the descriptions for language id #1 from the database (and select a different language in the admin):

UPDATE ps_manufacturer_lang SET description = NULL WHERE id_lang = 1

benwallis commented Sep 14, 2018

That's because the two brands already there (Studio Design and Graphic Corner) have descriptions with id_lang of 1.

If you changed default language and then edited the description (so language dropdown to right of textarea is not "en") then you would not see your updated description.
Alternatively add a new brand and set a description (for language other than "en").

Another way to test would be to manually delete the descriptions for language id #1 from the database (and select a different language in the admin):

UPDATE ps_manufacturer_lang SET description = NULL WHERE id_lang = 1

@mickaelandrieu

This comment has been minimized.

Show comment
Hide comment
@mickaelandrieu

mickaelandrieu Sep 17, 2018

Contributor

Thank you @benwallis :)

Contributor

mickaelandrieu commented Sep 17, 2018

Thank you @benwallis :)

@mickaelandrieu mickaelandrieu merged commit 0d0f779 into PrestaShop:develop Sep 17, 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