Skip to content

magento/magento2#14510: Creating custom customer attribute with default value 0 will cause not saving value for customer entity. #16915

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

Conversation

swnsma
Copy link
Contributor

@swnsma swnsma commented Jul 18, 2018

  • fix default value check in case of type conversion;
  • add boolean attribute default value assignment;

Description

Fixed Issues

  1. Creating custom customer attribute with default value 0 will cause not saving value for customer entity #14510: Creating custom customer attribute with default value 0 will cause not saving value for customer entity

Manual testing scenarios

Steps to

  1. Create attribute with type "boolean" and default value via Data Install.

         $eavSetup = $this->eavSetupFactory->create(['setup' => $setup]);
         $eavSetup->addAttribute(
             \Magento\Catalog\Model\Product::ENTITY,
             'custom_product',
             [
                 'type' => 'int',
                 'label' => 'Custom Product',
                 'input' => 'boolean',
                 'source' => \Magento\Eav\Model\Entity\Attribute\Source\Boolean::class,
                 'global' => \Magento\Eav\Model\Entity\Attribute\ScopedAttributeInterface::SCOPE_GLOBAL,
                 'backend' => \Magento\Catalog\Model\Product\Attribute\Backend\Boolean::class,
                 'visible' => true,
                 'required' => true,
                 'user_defined' => false,
                 'default' => '0',
                 'searchable' => false,
                 'filterable' => false,
                 'comparable' => false,
                 'visible_on_front' => false,
                 'used_in_product_listing' => true,
                 'unique' => false,
             ]
         );
    
         $entityTypeId = $eavSetup->getEntityTypeId(\Magento\Catalog\Model\Product::ENTITY);
         $attributeSetId = $eavSetup->getDefaultAttributeSetId($entityTypeId);
         $attributeGroupId = $eavSetup->getDefaultAttributeGroupId($entityTypeId, $attributeSetId);
         $this->attributeManagement->assign(
             \Magento\Catalog\Model\Product::ENTITY,
             $attributeSetId,
             $attributeGroupId,
             'custom_product',
             100
         );
    
  2. Open Swagger.

  3. Create new product with REST API via CATALOGPRODUCTREPOSITORYV1. Attribute 'custom_product' should be missed.
    { "product": { "status": 2, "visibility": 4, "type_id": "simple", "sku": "testattr", "name": "TEST ATTR", "attribute_set_id": 4, "price": 0.5, "weight": 0.0, "extension_attributes": { "stock_item": { "qty": 0, "is_in_stock": true, "use_config_backorders": false, "backorders": 1, "use_config_manage_stock": false } }, "custom_attributes": [ { "attribute_code": "description", "value": "" }, { "attribute_code": "short_description", "value": "" }, { "attribute_code": "weight", "value": "" } ] } }

  4. Check a product attribute 'custom_product' value in DB.

Expected result

  1. Default attribute value '0' was set for attribute 'custom_product'.

Actual result

  1. No value was set for attribute ' custom_product' was set.

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-engcom-team magento-engcom-team added Partner: ISM eCompany Pull Request is created by partner ISM eCompany partners-contribution Pull Request is created by Magento Partner labels Jul 18, 2018
@magento-engcom-team
Copy link
Contributor

Hi @swnsma. 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 {$VERSION} instance - deploy vanilla Magento instance

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

@sidolov
Copy link
Contributor

sidolov commented Jul 20, 2018

Hi @swnsma , please, take a look at failed integration tests

…0 will cause not saving value for customer entity.

- fix default value check in case of type conversion;
- add boolean attribute default value assignment;
@swnsma swnsma force-pushed the Magento-Attribute-Default-Value-Boolean-Attribute branch from c8f5238 to 5fa4d5f Compare July 20, 2018 14:13
@swnsma
Copy link
Contributor Author

swnsma commented Jul 21, 2018

Hi @sidolov,
Thank you for your review.

I have checked the error, and found next:

  1. Problem related to Downloadable products (DP).
  2. Attribute of DP - 'links_exists' - has default value '0', which was not applied to products before.
  3. In other place in code expected to receive 'null' default value of the attribute.
    $hasLinks = $product->getData('links_exist');

My assumption of the solution:

  1. Create upgrade script, which will remove default value for the attribute.

…0 will cause not saving value for customer entity.

- remove default value for 'links_exist' attribute of Downloadable Product;
@swnsma swnsma force-pushed the Magento-Attribute-Default-Value-Boolean-Attribute branch from 03ac7a3 to 0698aa6 Compare July 23, 2018 07:45
}

/**
* {@inheritdoc}
Copy link
Member

Choose a reason for hiding this comment

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

Please remove curly braces {}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the review!
selection_220
But I have fixed :)

Copy link
Member

Choose a reason for hiding this comment

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

Thank you! The sniffer checks only changed files so that's okay

@slavvka slavvka self-assigned this Oct 4, 2018
…0 will cause not saving value for customer entity.

Remove curly braces.
*/
public function getDefaultValue()
{
if ($this->_defaultValue === null) {
if ($this->getAttribute()->getDefaultValue()) {
if ($this->getAttribute()->getDefaultValue() !== null) {
Copy link
Member

Choose a reason for hiding this comment

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

Here you touch the logic that relates to all attributes. Are you sure that all other attributes have null as default value? Won't it break their work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @slavvka,

I have two arguments for this point.
First: auto tests have passed successfully.
Second: if you look at the screenshot under, there are list of attributes with default value '0'. After smoke check I have not found strict check '===' with null or 0 values anymore (exception - 'links_exist' attribute).
So, I assume, there should not be any problems for existing set of attributes.
P.S. have checked on Open Source Magento.
image

@magento-engcom-team
Copy link
Contributor

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

@magento-engcom-team
Copy link
Contributor

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

Please, consider to port this solution to 2.3 release line.
You may use Porting tool to port commits automatically.

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

Successfully merging this pull request may close these issues.

4 participants