Module catalog, Attribute Repository code validation regex #29015

drc0 · 2020-07-06T18:11:26Z

Regex modified to accept the A-Z range and check for Magento\Eav\Model\Entity\Attribute::ATTRIBUTE_CODE_MAX_LENGTH instead of the old hardcoded value (30)

Description (*)

In the magento module catalog, in the method validateCode for product attribute attribute_code field, there were a check for the attribute length that do not respect the Magento\Eav\Model\Entity\Attribute::ATTRIBUTE_CODE_MAX_LENGTH constant, also I think it should accept chars of the A-Z range.

fixes #29017

Regex modified to accept the A-Z range and check for Magento\Eav\Model\Entity\Attribute::ATTRIBUTE_CODE_MAX_LENGTH instead of the old hardcoded value (30)

m2-assistant · 2020-07-06T18:11:28Z

Hi @drc0. 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

❗ Automated tests can be triggered manually with an appropriate comment:

@magento run all tests - run or re-run all required tests against the PR changes
@magento run <test-build(s)> - run or re-run specific test build(s)
For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

Database Compare
Functional Tests CE
Functional Tests EE,
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

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

drc0 · 2020-07-06T19:48:50Z

@magento run all tests

VladimirZaets · 2020-08-20T13:13:05Z

@magento run all tests

gabrieldagama

Hi @drc0, thanks for the pull request, it looks good!

Please follow below some comments on it, can you fix the static tests failures?

We also need to cover the changes with some tests, we could create some integration tests to validate the changes proposed on this PR, let me know if you are able to create these tests!

Thank you!

app/code/Magento/Catalog/Model/Product/Attribute/Repository.php

…g the docs

drc0 · 2020-08-25T13:05:08Z

Hi @drc0, thanks for the pull request, it looks good!

Please follow below some comments on it, can you fix the static tests failures?

We also need to cover the changes with some tests, we could create some integration tests to validate the changes proposed on this PR, let me know if you are able to create these tests!

Thank you!

For now I'm unable to create a test for the proposed changes. should app/code/Magento/Catalog/Test/Unit/Model/Product/Attribute/RepositoryTest.php be modified to check for this validation on the length of the attribute code adding a new class method for the test case?

gabrieldagama · 2020-08-25T13:24:34Z

Hi @drc0, yeah I guess we can update that unit test to validate the proposed changes. Thanks!

engcom-Charlie · 2020-08-27T07:01:18Z

Hi @drc0 can you please look at #29015 (comment) ?
Thank you.

engcom-Charlie · 2020-08-31T13:51:13Z

Hi @drc0, I'll take care of test coverage.

drc0 · 2020-08-31T17:40:57Z

Hi @drc0, I'll take care of test coverage.

awesome, thank you.

engcom-Charlie · 2020-09-02T06:51:05Z

Hi @drc0, can you please take a look at #29017 (comment)?
Thank you.

engcom-Charlie · 2020-09-03T07:04:40Z

I close PR because the issue isn't produced anymore.

m2-assistant · 2020-09-03T07:04:42Z

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

magento-engcom-team · 2020-09-17T14:51:41Z

Hi @gabrieldagama, thank you for the review.
ENGCOM-8229 has been created to process this Pull Request
✳️ @gabrieldagama, 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

magento-engcom-team · 2020-09-17T14:51:43Z

@drc0 thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

engcom-Delta · 2020-09-24T07:11:38Z

✔️ QA passed
Result:
Before:
❌ Attribute with code length more than 30 but less than 60 symbols cannot be created

After:
✔️ Attribute with code length more than 30 but less than 60 symbols is successfully created

engcom-Delta · 2020-09-24T07:12:54Z

Note: Functional Tests B2B are failed

#29015

m2-assistant · 2020-09-25T19:04:03Z

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

Module catalog, Attribute Repository code validation regex

d2c352d

Regex modified to accept the A-Z range and check for Magento\Eav\Model\Entity\Attribute::ATTRIBUTE_CODE_MAX_LENGTH instead of the old hardcoded value (30)

ghost added this to Pending Review in Pull Requests Dashboard Jul 6, 2020

magento-engcom-team added Component: Catalog Release Line: 2.4 labels Jul 6, 2020

ghost added the Progress: pending review label Jul 6, 2020

sidolov added Priority: P3 May be fixed according to the position in the backlog. Severity: S3 Affects non-critical data or functionality and does not force users to employ a workaround. labels Aug 18, 2020

Merge branch '2.4-develop' into 2.4-attrcoderegex

37dbcf7

gabrieldagama suggested changes Aug 25, 2020

View reviewed changes

app/code/Magento/Catalog/Model/Product/Attribute/Repository.php Outdated Show resolved Hide resolved

app/code/Magento/Catalog/Model/Product/Attribute/Repository.php Outdated Show resolved Hide resolved

ghost moved this from Pending Review to Changes Requested in Pull Requests Dashboard Aug 25, 2020

ghost assigned gabrieldagama Aug 25, 2020

ghost added Progress: needs update and removed Progress: pending review labels Aug 25, 2020

drc0 added 2 commits August 25, 2020 14:00

chore: remove trailing space

2663ff4

fix: remove upper case regex check rule for attribute codes respectin…

78ed862

…g the docs

engcom-Charlie self-assigned this Aug 27, 2020

gabrieldagama added the Risk: low label Aug 28, 2020

engcom-Charlie closed this Sep 3, 2020

ghost removed this from Changes Requested in Pull Requests Dashboard Sep 3, 2020

engcom-Charlie requested review from gabrieldagama and sidolov September 17, 2020 13:13

gabrieldagama approved these changes Sep 17, 2020

View reviewed changes

ghost moved this from Review in Progress to Ready for Testing in Pull Requests Dashboard Sep 17, 2020

ghost assigned gabrieldagama Sep 17, 2020

ghost added Progress: ready for testing and removed Progress: review labels Sep 17, 2020

gabrieldagama added the Auto-Tests: Covered All changes in Pull Request is covered by auto-tests label Sep 17, 2020

engcom-Delta self-assigned this Sep 23, 2020

engcom-Delta moved this from Ready for Testing to Testing in Progress in Pull Requests Dashboard Sep 23, 2020

ghost added Progress: testing in progress and removed Progress: ready for testing labels Sep 23, 2020

engcom-Delta moved this from Testing in Progress to Extended Testing (optional) in Pull Requests Dashboard Sep 24, 2020

ghost added Progress: extended testing and removed Progress: testing in progress labels Sep 24, 2020

engcom-Charlie added the QA: Ready to add to Regression Scope Should be analyzed and added to Regression Testing Scope(if applicable) label Sep 24, 2020

engcom-Delta added QA: Added to Regression Scope Scenario was analysed and added to Regression Testing Scope and removed QA: Ready to add to Regression Scope Should be analyzed and added to Regression Testing Scope(if applicable) labels Sep 24, 2020

engcom-Charlie moved this from Extended Testing (optional) to Merge in Progress in Pull Requests Dashboard Sep 24, 2020

ghost added Progress: accept and removed Progress: extended testing labels Sep 24, 2020

magento-engcom-team mentioned this pull request Sep 25, 2020

Product Attribute Repository do not respect attribute_code max length #29017

Closed

5 tasks

magento-engcom-team pushed a commit that referenced this pull request Sep 25, 2020

ENGCOM-8229: Module catalog, Attribute Repository code validation regex

6250fd7

#29015

magento-engcom-team merged commit 58b60ef into magento:2.4-develop Sep 25, 2020

ghost moved this from Merge in Progress to Recently Merged in Pull Requests Dashboard Sep 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Module catalog, Attribute Repository code validation regex #29015

Module catalog, Attribute Repository code validation regex #29015

drc0 commented Jul 6, 2020 •

edited

Loading

m2-assistant bot commented Jul 6, 2020

drc0 commented Jul 6, 2020

VladimirZaets commented Aug 20, 2020

gabrieldagama left a comment

drc0 commented Aug 25, 2020

gabrieldagama commented Aug 25, 2020

engcom-Charlie commented Aug 27, 2020

engcom-Charlie commented Aug 31, 2020

drc0 commented Aug 31, 2020

engcom-Charlie commented Sep 2, 2020

engcom-Charlie commented Sep 3, 2020

m2-assistant bot commented Sep 3, 2020

magento-engcom-team commented Sep 17, 2020

magento-engcom-team commented Sep 17, 2020

engcom-Delta commented Sep 24, 2020

engcom-Delta commented Sep 24, 2020

m2-assistant bot commented Sep 25, 2020

Module catalog, Attribute Repository code validation regex #29015

Module catalog, Attribute Repository code validation regex #29015

Conversation

drc0 commented Jul 6, 2020 • edited Loading

Description (*)

m2-assistant bot commented Jul 6, 2020

drc0 commented Jul 6, 2020

VladimirZaets commented Aug 20, 2020

gabrieldagama left a comment

Choose a reason for hiding this comment

drc0 commented Aug 25, 2020

gabrieldagama commented Aug 25, 2020

engcom-Charlie commented Aug 27, 2020

engcom-Charlie commented Aug 31, 2020

drc0 commented Aug 31, 2020

engcom-Charlie commented Sep 2, 2020

engcom-Charlie commented Sep 3, 2020

m2-assistant bot commented Sep 3, 2020

magento-engcom-team commented Sep 17, 2020

magento-engcom-team commented Sep 17, 2020

engcom-Delta commented Sep 24, 2020

engcom-Delta commented Sep 24, 2020

m2-assistant bot commented Sep 25, 2020

drc0 commented Jul 6, 2020 •

edited

Loading