Fixed issue - #19346 Import data 2.2.6 Value for 'product_type' attribute contains incorrect value

[GovindaSharma]

Description (*)

Fixed issue - #19346
Import data 2.2.6 Value for 'product_type' attribute contains incorrect value

Since 'product_attribute' is reserved attribute,so we can restrict this type of attribute to be created by user.

Fixed Issues (if relevant)

Fixed issue - #19346
Import data 2.2.6 Value for 'product_type' attribute contains incorrect value

Manual testing scenarios (*)

1. create an attribute with code product_type http://i.prntscr.com/AwUJuhizQKatBwNwRa1S7Q.png with type: dropdown scope: global sortable, comparable: no
2. add it to a family and create a product.
3. go to import profiles download sample file then reimport sample file

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 on Travis CI are green)

[magento-cicd2]

All committers have signed the CLA.

[magento-engcom-team]

Hi @GovindaSharma. 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-engcom-team give me test instance - deploy test instance based on PR changes
• @magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

[orlangur]

Please fill all description fields according to template and apply code changes, then squash it into single commit and force push.

[orlangur]

Please add a proper check into

if ($attribute->getId() && !$attributeId) {
            $message = strlen($this->getRequest()->getParam('attribute_code'))
                ? __('An attribute with this code already exists.')
                : __('An attribute with the same code (%1) already exists.', $attributeCode);

condition, do not introduce a constant - just compare with 'product_type' string literal.

[GovindaSharma]

so instead of introducing a constant...i should compare the string..if string is similar then throw msg right?

[orlangur]

@GovindaSharma exactly, no need to overcomplicate things. When we need more than one such forbidden not-really-an-attribute, we may think about array.

[orlangur]

@GovindaSharma,

if(strcmp($attributeCode,'product_type')==false){
    		$message = 	__('Cannot create attribute (%1) as it is already used in catalog',$attributeCode);

still not combined with first condition as requested. $attributeCode !== 'product_type' should do the trick.

[GovindaSharma]

hi @orlangur

"Please fill all description fields according to template and apply code changes, then squash it into single commit and force push."

this was your feedback..i did not understand it.can you please tell me give any idea what changes i need to apply in code?

[orlangur]

@GovindaSharma check this: https://github.com/magento/magento2/pull/19408/files#r236223352

You need to add new check into existing if instead of introducing a new one.

[orlangur]

@GovindaSharma still not fixed

[GovindaSharma]

changes done as per request

[GovindaSharma]

@orlangur can you please tell me what is the exact issue in first check which is not passed?i have tested the code in php 7.2 version and it's working.Please let me the exact issue so that i can fix the same

[orlangur]

@GovindaSharma,

can you please tell me what is the exact issue in first check which is not passed?

I don't understand this question, please reword.

[GovindaSharma]

since two checks has been passed but one is not..so what issue occurred in that checkes for which it has not been passed?

[GovindaSharma]

@orlangur please look into the changes and let me know what other things i need to do?

[GovindaSharma]

@magento-engcom-team and @orlangur ,can you please review the changes as i have done all necessary changes suggested by @orlangur.Let me know your feedback on this.

[orlangur]

Revert this change)

[GovindaSharma]

are you talking about the extra space in file? that i need to remove?

[GovindaSharma]

@orlangur .i made the chnages.Please review this.
Thank You.

[orlangur]

Try git checkout 2.3-develop -- app/code/Magento/Catalog/Model/ResourceModel/Eav/Attribute.php to get this file in correct state.

[GovindaSharma]

@orlangur file is in correct state now.I made the changes.

[GovindaSharma]

@magento-engcom-team and @orlangur .i have done changes requested by @orlangur .Please review this and let me know.

[orlangur]

@GovindaSharma please check https://github.com/magento/magento2/pull/19408/files#r237182111. All needed changes are not done yet.

[GovindaSharma]

@orlangur , now i have added check into existing if,previously i added extra if just to show different message because "product_type" attribute in real does not exist in magento,so that was the reason to add extra if.But now i added the checks into existing if condition.Please review the changes and let me know about it.

Thank You

[orlangur]

@GovindaSharma thanks! Looks good now, please squash changes into a single commit (incuding my commits) and we are good to go.

[GovindaSharma]

@orlangur .I have now squash all commits into one and done force push.I think all the changes has been done now.Please check and approve the changes.
Thank You

[magento-engcom-team]

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

[magento-engcom-team]

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

[magento-engcom-team]

Hi @GovindaSharma. Thank you for your contribution.
We will aim to release these changes as part of 2.3.1.
Please check the release notes for final confirmation.

[GovindaSharma]

@magento-engcom-team ,thanks for your confirmation.