Skip to content
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

CatalogImportExport doesn't support empty row values #7468

Closed
koenner01 opened this issue Nov 17, 2016 · 7 comments
Closed

CatalogImportExport doesn't support empty row values #7468

koenner01 opened this issue Nov 17, 2016 · 7 comments
Labels
bug report Component: ImportExport Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release

Comments

@koenner01
Copy link
Contributor

koenner01 commented Nov 17, 2016

If you have an attribute that has a value and you import an empty string because the value needs to be empty, the attribute will not be overwritten with the empty string.

Preconditions

  1. MG2.1.2
  2. PHP7.0

Steps to reproduce

  1. Enter a value for an attribute for a product through the backend of Magento
    (I'm using description as my example)
  2. Create an import csv and leave the column for the attribute blank
  3. Import the csv with Add/Update behaviour

Expected result

  1. Product attribute value should be empty

Actual result

  1. Product attribute keeps the previous value

This issue can be traced back to \Magento\CatalogImportExport\Model\Import\Product\Type\AbstractType where the clearEmptyData function unsets all empty rows.

public function clearEmptyData(array $rowData)
{
    foreach ($this->_getProductAttributes($rowData) as $attrCode => $attrParams) {
        if (!$attrParams['is_static'] && empty($rowData[$attrCode])) {
            unset($rowData[$attrCode]);
        }
    }
    return $rowData;
}

It is my opinion that support for empty rows should be allowed.
My suggested fix is adding a constant to the AbstractType class and set the row values that contain that constant value to an empty string. That way if the constant value is used in the import csv, the importer will know which fields are just empty and which fields contain empty values.

CONST EMPTY_ROW_VALUE = '__EMPTY__';

public function clearEmptyData(array $rowData)
{
    foreach ($this->_getProductAttributes($rowData) as $attrCode => $attrParams) {
        if (!$attrParams['is_static'] && empty($rowData[$attrCode])) {
            unset($rowData[$attrCode]);
            continue;
        }

        if ($rowData[$attrCode] === self::EMPTY_ROW_VALUE) {
            $rowData[$attrCode] = '';
        }
    }
    return $rowData;
}

It would be even better to allow for setting custom values for the EMPTY_ROW_VALUE constant through the backend.

@koenner01
Copy link
Contributor Author

koenner01 commented Nov 17, 2016

Addendum to this;

If chosen to support the constant value, support should also be implemented in \Magento\CatalogImportExport\Model\Import\Product\Validator in the isAttributeValid function.
To allow for every type of attribute to have support for empty values, something like this should be added

...
if (!strlen(trim($rowData[$attrCode]))) {
    return true;
}

if ($rowData[$attrCode] === \Magento\CatalogImportExport\Model\Import\Product\Type\AbstractType::EMPTY_ROW_VALUE) {
    return true;
}

switch ($attrParams['type']) {
...

@slopukhov
Copy link
Contributor

@koenner01 thank you for your feedback.
Internal issue created: MAGETWO-61593

@slopukhov slopukhov added the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label Nov 30, 2016
@slopukhov slopukhov removed their assignment Nov 30, 2016
@vherasymenko vherasymenko self-assigned this Mar 14, 2017
@koenner01
Copy link
Contributor Author

I have found a problem when using the empty logic;

If you try to import an empty value '' for a product's special_price, it will result in special_price 0.

The problem is an insert being doing in \Magento\CatalogImportExport\Model\Import\Product in function _saveProductAttributes:

protected function _saveProductAttributes(array $attributesData)
{
    foreach ($attributesData as $tableName => $skuData) {
        $tableData = [];
        foreach ($skuData as $sku => $attributes) {
            $linkId = $this->_connection->fetchOne(
                $this->_connection->select()
                    ->from($this->getResource()->getTable('catalog_product_entity'))
                    ->where('sku = ?', $sku)
                    ->columns($this->getProductEntityLinkField())
            );

            foreach ($attributes as $attributeId => $storeValues) {
                foreach ($storeValues as $storeId => $storeValue) {
                    $tableData[] = [
                        $this->getProductEntityLinkField() => $linkId,
                        'attribute_id' => $attributeId,
                        'store_id' => $storeId,
                        'value' => $storeValue,
                    ];
                }
            }
        }
        $this->_connection->insertOnDuplicate($tableName, $tableData, ['value']);
    }
    return $this;
}

If an empty string value is being inserted into the entity_int or entity_decimal tables they result into value 0.

@koenner01
Copy link
Contributor Author

Another issue in the same clearEmptyData function #10006

@PieterCappelle
Copy link
Contributor

Any update on this from the Magento team?

@magento-engcom-team magento-engcom-team added 2.1.x Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development bug report Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Component: ImportExport labels Sep 11, 2017
@magento-engcom-team magento-engcom-team added the Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed label Oct 3, 2017
@magento-engcom-team
Copy link
Contributor

@koenner01, thank you for your report.
We've created internal ticket(s) MAGETWO-61593 to track progress on the issue.

@magento-engcom-team magento-engcom-team added 2.2.x Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release labels Oct 11, 2017
@piotrekkaminski
Copy link
Contributor

This issue was moved to magento-engcom/import-export-improvements#49

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report Component: ImportExport Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release
Projects
None yet
Development

No branches or pull requests

9 participants