Skip to content

[Admin][improvement] Do not allow HTML tags for product attribute options when type is dropdown or multiple select#27417

Closed
vasilii-b wants to merge 9 commits intomagento:2.4-developfrom
vasilii-b:admin-product-attribute-no-html-tags-for-dropdown-options
Closed

[Admin][improvement] Do not allow HTML tags for product attribute options when type is dropdown or multiple select#27417
vasilii-b wants to merge 9 commits intomagento:2.4-developfrom
vasilii-b:admin-product-attribute-no-html-tags-for-dropdown-options

Conversation

@vasilii-b
Copy link

@vasilii-b vasilii-b commented Mar 24, 2020

Description (*)

This PR aims to add a new validation for the options of the product attribute type "Dropdown", "Multiple Select" that checks whenever the option value contains HTML tags.

HTML tags are not useful nor displayed on the storefront for the product attribute options, those the store manager should be aware of it.

Related Pull Requests

#27371

Fixed Issues (if relevant)

N/A

Manual testing scenarios (*)

  1. Go to the Admin area
  2. Go to Stores -> Attributes -> Product
  3. Press the button "Add New Attribute"
  4. Fill in the attribute default label
  5. Select the "Catalog Input Type for Store Owner" to "Dropdown" or "Multiple Select"
  6. In the section "Manage Options (Values of Your Attribute)" below add new options that contain HTML tags like <span>, </strong>, etc.
  7. Press the "Save and Continue Edit" or "Save Attribute" button
  8. See the error message
    Error message that says the option contains HTML tags

Questions or comments

Are the provided Unit and MFTF tests enough or there should be done more?

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)

Resolved issues:

  1. resolves [Issue] [Admin][improvement] Do not allow HTML tags for product attribute options when type is dropdown or multiple select #29602: [Admin][improvement] Do not allow HTML tags for product attribute options when type is dropdown or multiple select

@m2-assistant
Copy link

m2-assistant bot commented Mar 24, 2020

Hi @vasilii-b. 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.

@magento-engcom-team magento-engcom-team added Area: Frontend Component: Catalog Component: Eav Release Line: 2.4 Partner: Atwix Pull Request is created by partner Atwix partners-contribution Pull Request is created by Magento Partner labels Mar 24, 2020
@vasilii-b vasilii-b changed the title [Admin] Do not allow HTML tags for product attribute options when type is dropdown or multiple select [Admin][improvement] Do not allow HTML tags for product attribute options when type is dropdown or multiple select Mar 24, 2020
@vasilii-b
Copy link
Author

@magento run all tests

@rogyar rogyar self-assigned this Mar 25, 2020
@vasilii-b vasilii-b force-pushed the admin-product-attribute-no-html-tags-for-dropdown-options branch from 1280b66 to ee6a8c8 Compare March 25, 2020 16:16
And solve minor conflicts on the MFTF action groups
@rogyar
Copy link
Contributor

rogyar commented Apr 8, 2020

@magento run all tests

@rogyar
Copy link
Contributor

rogyar commented Apr 13, 2020

Hi @vasilii-b. Considering the fact that the only static tests were executed we can assume that Magento cannot be installed after the change you've provided (theoretically). I would recommend doing the following for confirming this assumption.

  • Switch to the 2.4-develop branch
  • Pull the latest changes
  • Merge your branch into the 2.4-develop branch
  • Run Magento installation with a clean database (via CLI)
  • Make sure that Magento is installed successfully

@vasilii-b
Copy link
Author

Hi @rogyar,
Thank you so much! I'll give it a try.

@sidolov sidolov added Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Severity: S3 Affects non-critical data or functionality and does not force users to employ a workaround. labels Aug 17, 2020
@sidolov
Copy link
Contributor

sidolov commented Aug 17, 2020

@magento create issue

/**
* Validation pattern for attribute options HTML tags
*/
const VALIDATION_HTML_TAGS_RULE_PATTERN = '/<[^<]+>/';
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, make const private

/**
* Validation message for attribute options with HTML tags
*/
const VALIDATION_HTML_TAGS_RULE_MESSAGE = 'HTML tags are not allowed for the attribute options. '.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, make const private

Copy link
Contributor

@sidolov sidolov left a comment

Choose a reason for hiding this comment

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

Please, take a look at the review comments

@engcom-Foxtrot engcom-Foxtrot self-assigned this Sep 10, 2020
…for-dropdown-options

# Conflicts:
#	app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/Attribute/ValidateTest.php
…t attribute options when type is dropdown or multiple select.
@engcom-Foxtrot
Copy link
Contributor

@magento run all tests

3 similar comments
@engcom-Foxtrot
Copy link
Contributor

@magento run all tests

@gabrieldagama
Copy link
Contributor

@magento run all tests

@engcom-Foxtrot
Copy link
Contributor

@magento run all tests

…t attribute options when type is dropdown or multiple select - installation fix.
@engcom-Foxtrot
Copy link
Contributor

@magento run all tests

@engcom-Foxtrot
Copy link
Contributor

@magento run all tests

@engcom-Foxtrot
Copy link
Contributor

@magento run all tests

@engcom-Foxtrot engcom-Foxtrot removed their assignment Oct 30, 2020
@engcom-Charlie engcom-Charlie self-assigned this Nov 2, 2020
@engcom-Charlie
Copy link
Contributor

Before changes ✔️

I can create dropdown product attribute with:

  • Visible on Catalog Pages on Storefront -> Yes
  • Allow HTML Tags on Storefront -> Yes

And option value with Html tags which visible on PDP
image

After changes ✖️

I try to create the same attribute and get the following error
image

@vasilii-b could you please take a look?
Thank you.

@engcom-Charlie
Copy link
Contributor

Hi @vasilii-b, I'm closing this PR now due to inactivity.
Please reopen and update if you wish to continue.
Thank you for your collaboration.

@m2-assistant
Copy link

m2-assistant bot commented Nov 16, 2020

Hi @vasilii-b, 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.

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

Labels

Area: Frontend Component: Catalog Component: Eav Partner: Atwix Pull Request is created by partner Atwix partners-contribution Pull Request is created by Magento Partner Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Release Line: 2.4 Severity: S3 Affects non-critical data or functionality and does not force users to employ a workaround.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Issue] [Admin][improvement] Do not allow HTML tags for product attribute options when type is dropdown or multiple select

7 participants