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

Use regexp instead of simple explode for complex attributes in CartPresenter #8425

Merged
merged 1 commit into from Oct 31, 2017

Conversation

Projects
None yet
5 participants
@Quetzacoalt91
Member

Quetzacoalt91 commented Oct 17, 2017

Questions Answers
Branch? develop
Description? Fix conversion from string -> array of product attributes, when they have the current separator in their label or value
Type? bug fix
Category? FO
BC breaks? Nope
Deprecations? Nope
Fixed ticket? http://forge.prestashop.com/browse/BOOM-3861
How to test? Create a new product attribute with the current separator in it (By default: -), and use it on a product. Adding it to the cart will throw a notice until you try this PR. Also, it includes unit tests.

This PR is another suggestion from #8422


This change is Reviewable

@codacy-bot

This comment has been minimized.

Show comment
Hide comment
@codacy-bot

codacy-bot Oct 17, 2017

Codacy Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- src/Adapter/Cart/CartPresenter.php  2
- tests/TestCase/UnitTestCase.php  2
         

Complexity decreasing per file
==============================
+ tests/Unit/Adapter/Module/Tab/ModuleTabRegisterTest.php  -1
         

See the complete overview on Codacy

codacy-bot commented Oct 17, 2017

Codacy Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- src/Adapter/Cart/CartPresenter.php  2
- tests/TestCase/UnitTestCase.php  2
         

Complexity decreasing per file
==============================
+ tests/Unit/Adapter/Module/Tab/ModuleTabRegisterTest.php  -1
         

See the complete overview on Codacy

@MDWeb-Lille

This comment has been minimized.

Show comment
Hide comment
@MDWeb-Lille

MDWeb-Lille Oct 18, 2017

Contributor

Works great with our test case!

Contributor

MDWeb-Lille commented Oct 18, 2017

Works great with our test case!

@mickaelandrieu

This comment has been minimized.

Show comment
Hide comment
@mickaelandrieu
Contributor

mickaelandrieu commented Oct 31, 2017

Thank you @Quetzacoalt91

@mickaelandrieu mickaelandrieu merged commit f299586 into PrestaShop:develop Oct 31, 2017

2 checks passed

codacy/pr Good work! A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Quetzacoalt91 Quetzacoalt91 deleted the Quetzacoalt91:BOOM-3861 branch Nov 16, 2017

@eternoendless eternoendless added this to the 1.7.3.0 milestone Apr 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment