Normalize new line symbols in Product Text Option (type=area) #26033

Leone · 2019-12-13T18:31:47Z

Description (*)

Normalize new line symbols in Product Text Option (type=area).

If you add a customizable option to the product (type = area, max characters X), the length of the option value is counts differently by javascript validator and php.
I added the method to normalize new line symbols in text option.

Fixed Issues (if relevant)

Amount of characters on a 'Area' Customizable Option counted differently on backend/frontend #25974: Amount of characters on a 'Area' Customizable Option counted differently on backend/frontend

Manual testing scenarios (*)

Steps to reproduce from #25974

Try to put this product in your basket with a message 'Is twenty characters' => OK on front and can add to cart.
Try to put this product in your basket with a message with a newline:
'Is
twenty
characters'
=> Ok on front, but error on adding to cart: 'The text is too long.'
Error shown although the Javascript validator passed.

Questions or comments

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds are green)

…Option counted differently on backend/frontend"

m2-assistant · 2019-12-13T18:31:51Z

Hi @Leone. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento give me test instance - deploy test instance based on PR changes
@magento give me 2.4-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

lbajsarowicz

How your change would behave if some extension code will provide null or other unexpected input?

lbajsarowicz · 2019-12-14T20:40:33Z

app/code/Magento/Catalog/Model/Product/Option/Type/Text.php

+     */
+    private function normalizeNewLineSymbols($value)
+    {
+        return str_replace(["\r\n", "\n\r", "\r"], ["\n", "\n", "\n"], $value);


Second argument does not have to be an array: https://3v4l.org/lkhbV

echo str_replace(['a','b','c'], 'd', 'abcdef');

lbajsarowicz · 2019-12-14T20:40:48Z

app/code/Magento/Catalog/Model/Product/Option/Type/Text.php

+     * @param string $value
+     * @return string
+     */
+    private function normalizeNewLineSymbols($value)


Add type hints

lbajsarowicz · 2019-12-14T20:41:10Z

dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Option/Type/TextTest.php

+/**
+ * Test for customizable product option with "Text" type
+ */
+class TextTest extends \PHPUnit\Framework\TestCase


Import class.

lbajsarowicz · 2019-12-14T20:42:25Z

dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Option/Type/TextTest.php

+    /**
+     * @var Text
+     */
+    protected $model;


Good practice is to use class name / class abstract instead of generic model.

lbajsarowicz · 2019-12-14T20:42:33Z

dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Option/Type/TextTest.php

+    protected $model;
+
+    /**
+     * @var \Magento\Framework\ObjectManagerInterface


Import class.

lbajsarowicz · 2019-12-14T20:42:54Z

dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Option/Type/TextTest.php

+    protected function setUp()
+    {
+        $this->objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
+        $this->model = $this->objectManager->create(


You don't need to split that into multiple lines.

lbajsarowicz · 2019-12-14T20:43:02Z

dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Option/Type/TextTest.php

+     */
+    protected function setUp()
+    {
+        $this->objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();


Import class.

lbajsarowicz · 2019-12-14T20:45:51Z

dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Option/Type/TextTest.php

+    {
+        return [
+            [
+                // $productOptionData


Instead of adding comments, just replace that with:

[self::STUB_OPTION_DATA, "string \r\n string", "string \n string"],

And extract ['id' => 11, 'type' => 'area'] to private const.

Leone · 2019-12-16T07:29:15Z

@lbajsarowicz thank you for feedback.

How your change would behave if some extension code will provide null or other unexpected input?

null or other unexpected input will be cast to string because there is $value = trim($this->getUserValue()); above my change in validateUserValue($values) method.

lbajsarowicz · 2019-12-16T10:08:24Z

dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Option/Type/TextTest.php

+
+namespace Magento\Catalog\Model\Product\Option\Type;
+
+use Magento\Catalog\Model\Product\Option;


You can organize imports (CTRL + SHIFT + O in PHPStorm)

lbajsarowicz

Thanks for your contribution. You may organize imports.

magento-engcom-team · 2019-12-16T10:09:38Z

Hi @lbajsarowicz, thank you for the review.
ENGCOM-6482 has been created to process this Pull Request
✳️ @lbajsarowicz, could you please add one of the following labels to the Pull Request?

Label	Description
`Auto-Tests: Covered`	All changes in Pull Request is covered by auto-tests
`Auto-Tests: Not Covered`	Changes in Pull Request requires coverage by auto-tests
`Auto-Tests: Not Required`	Changes in Pull Request does not require coverage by auto-tests

engcom-Charlie · 2019-12-16T11:16:20Z

✔️ QA Passed

engcom-Foxtrot · 2019-12-16T11:34:48Z

@Leone, please fix the failed tests.

sivaschenko · 2019-12-16T18:28:54Z

If eliminating Session dependency requires too much effort, the test can be skipped by adding
@SuppressWarnings(PHPMD.CookieAndSessionMisuse)
to the class phpdoc

magento-engcom-team · 2019-12-18T06:20:47Z

Hi @lbajsarowicz, thank you for the review.
ENGCOM-6482 has been created to process this Pull Request

Leone · 2019-12-18T09:27:04Z

@sivaschenko Indeed, eliminating Session dependency requires too much effort. I added
@SuppressWarnings(PHPMD.CookieAndSessionMisuse)
and tests passed. Thank you for this tip.

engcom-Charlie · 2019-12-18T09:37:33Z

✔️ QA Passed

…area) #26033

m2-assistant · 2019-12-29T04:46:47Z

Hi @Leone, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

[issue-25974] Fix for "Amount of characters on a 'Area' Customizable …

284e3ed

…Option counted differently on backend/frontend"

Leone requested a review from akaplya as a code owner December 13, 2019 18:31

magento-engcom-team added Partner: creativestyle partners-contribution Pull Request is created by Magento Partner labels Dec 13, 2019

lbajsarowicz suggested changes Dec 14, 2019

View reviewed changes

ghost assigned lbajsarowicz Dec 14, 2019

ghost added the Progress: needs update label Dec 14, 2019

Adjusted code after MR

2ae6838

Leone requested a review from lbajsarowicz December 16, 2019 07:30

lbajsarowicz reviewed Dec 16, 2019

View reviewed changes

lbajsarowicz approved these changes Dec 16, 2019

View reviewed changes

ghost added Progress: ready for testing and removed Progress: needs update labels Dec 16, 2019

magento-engcom-team added Release Line: 2.4 Component: Catalog labels Dec 16, 2019

engcom-Charlie self-assigned this Dec 16, 2019

ghost added Progress: testing in progress and removed Progress: ready for testing labels Dec 16, 2019

ghost added Progress: extended testing and removed Progress: testing in progress labels Dec 16, 2019

engcom-Foxtrot self-assigned this Dec 16, 2019

engcom-Foxtrot added the Auto-Tests: Covered All changes in Pull Request is covered by auto-tests label Dec 16, 2019

ghost added Progress: needs update and removed Progress: extended testing labels Dec 16, 2019

Leone added 2 commits December 18, 2019 06:58

Added @SuppressWarnings(PHPMD.CookieAndSessionMisuse) to Text class

d2a4865

Organized imports in test class

6e68a47

lbajsarowicz approved these changes Dec 18, 2019

View reviewed changes

ghost added Progress: ready for testing and removed Progress: needs update labels Dec 18, 2019

ghost added Progress: testing in progress and removed Progress: ready for testing labels Dec 18, 2019

ghost added Progress: extended testing Progress: accept and removed Progress: testing in progress Progress: extended testing labels Dec 18, 2019

VladimirZaets mentioned this pull request Dec 29, 2019

Amount of characters on a 'Area' Customizable Option counted differently on backend/frontend #25974

Closed

magento-engcom-team pushed a commit that referenced this pull request Dec 29, 2019

ENGCOM-6482: Normalize new line symbols in Product Text Option (type=…

a397d1c

…area) #26033

magento-engcom-team merged commit 6e68a47 into magento:2.4-develop Dec 29, 2019

sdzhepa mentioned this pull request May 9, 2022

tyutyu sdzhepa/magento2#5

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Normalize new line symbols in Product Text Option (type=area) #26033

Normalize new line symbols in Product Text Option (type=area) #26033

Leone commented Dec 13, 2019 •

edited

Loading

m2-assistant bot commented Dec 13, 2019

lbajsarowicz left a comment

lbajsarowicz Dec 14, 2019

lbajsarowicz Dec 14, 2019

lbajsarowicz Dec 14, 2019

lbajsarowicz Dec 14, 2019

lbajsarowicz Dec 14, 2019

lbajsarowicz Dec 14, 2019

lbajsarowicz Dec 14, 2019

lbajsarowicz Dec 14, 2019

Leone commented Dec 16, 2019 •

edited

Loading

lbajsarowicz Dec 16, 2019

lbajsarowicz left a comment

magento-engcom-team commented Dec 16, 2019

engcom-Charlie commented Dec 16, 2019

engcom-Foxtrot commented Dec 16, 2019

sivaschenko commented Dec 16, 2019 •

edited

Loading

magento-engcom-team commented Dec 18, 2019

Leone commented Dec 18, 2019

engcom-Charlie commented Dec 18, 2019

m2-assistant bot commented Dec 29, 2019


		namespace Magento\Catalog\Model\Product\Option\Type;

		use Magento\Catalog\Model\Product\Option;

Normalize new line symbols in Product Text Option (type=area) #26033

Normalize new line symbols in Product Text Option (type=area) #26033

Conversation

Leone commented Dec 13, 2019 • edited Loading

Description (*)

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

m2-assistant bot commented Dec 13, 2019

lbajsarowicz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Leone commented Dec 16, 2019 • edited Loading

Choose a reason for hiding this comment

lbajsarowicz left a comment

Choose a reason for hiding this comment

magento-engcom-team commented Dec 16, 2019

engcom-Charlie commented Dec 16, 2019

engcom-Foxtrot commented Dec 16, 2019

sivaschenko commented Dec 16, 2019 • edited Loading

magento-engcom-team commented Dec 18, 2019

Leone commented Dec 18, 2019

engcom-Charlie commented Dec 18, 2019

m2-assistant bot commented Dec 29, 2019

Leone commented Dec 13, 2019 •

edited

Loading

Leone commented Dec 16, 2019 •

edited

Loading

sivaschenko commented Dec 16, 2019 •

edited

Loading