From b9519af3701c6343586dfe0ecbba29c81ab85c47 Mon Sep 17 00:00:00 2001 From: Enrico Ellguth Date: Wed, 21 Oct 2026 16:20:54 +0200 Subject: [PATCH] Fix clean-up-empty-columns for columns with a default value mergeEntity() looked up the attribute code in the clean-up column keys, which hold header positions. The lookup never matched, so an empty cell of a column with a default value never deleted the stored value, even when the column was listed in clean-up-empty-columns. The attribute code is translated into its header position first. Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 6 + src/Observers/AttributeObserverTrait.php | 6 +- .../AttributeObserverTraitMergeEntityImpl.php | 216 ++++++++++++++++++ .../AttributeObserverTraitMergeEntityTest.php | 131 +++++++++++ 4 files changed, 357 insertions(+), 2 deletions(-) create mode 100644 tests/unit/Observers/AttributeObserverTraitMergeEntityImpl.php create mode 100644 tests/unit/Observers/AttributeObserverTraitMergeEntityTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index cadb015..33d6d1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +# Version 18.3.1 + +## Bugfixes + +* Fix `AttributeObserverTrait::mergeEntity()`: columns with a default value (`default-values`) were never emptied by `clean-up-empty-columns`, because the attribute code was looked up in the list of header positions + # Version 18.3.0 ## Features diff --git a/src/Observers/AttributeObserverTrait.php b/src/Observers/AttributeObserverTrait.php index 4a147b6..e71eae1 100644 --- a/src/Observers/AttributeObserverTrait.php +++ b/src/Observers/AttributeObserverTrait.php @@ -465,9 +465,11 @@ protected function mergeEntity(array $entity, array $attr, $changeSetName = null // if the value is empty AND it is IN the array with default column values // BUT it is NOT in the array with columns we want to clean-up the default // column value has to be removed, because we do NOT want to override the - // value existing in the database + // value existing in the database. The clean-up column keys are the header + // positions of the columns, so the attribute code has to be translated first if (array_key_exists($this->attributeCode, $this->getDefaultColumnValues()) && - array_key_exists($this->attributeCode, $this->getCleanUpEmptyColumnKeys()) === false + !($this->hasHeader($this->attributeCode) && + in_array($this->getHeader($this->attributeCode), $this->getCleanUpEmptyColumnKeys() ?? [], true)) ) { // remove the value from the array with the column values, because // this is the default value from the database and it should NOT diff --git a/tests/unit/Observers/AttributeObserverTraitMergeEntityImpl.php b/tests/unit/Observers/AttributeObserverTraitMergeEntityImpl.php new file mode 100644 index 0000000..691113d --- /dev/null +++ b/tests/unit/Observers/AttributeObserverTraitMergeEntityImpl.php @@ -0,0 +1,216 @@ + + * @copyright 2026 TechDivision GmbH + * @license https://opensource.org/licenses/MIT + * @link https://github.com/techdivision/import + * @link http://www.techdivision.com + */ + +namespace TechDivision\Import\Observers; + +/** + * Observer double that makes mergeEntity() of the attribute observer trait callable. + * + * @author TechDivision GmbH + * @copyright 2026 TechDivision GmbH + * @license https://opensource.org/licenses/MIT + * @link https://github.com/techdivision/import + * @link http://www.techdivision.com + */ +class AttributeObserverTraitMergeEntityImpl extends AbstractAttributeObserver +{ + + /** + * The header positions by column name. + * + * @var array + */ + private $headers; + + /** + * Initializes the double. + * + * @param array $headers The header positions by column name + * @param array $defaultColumnValues The columns with a default value + * @param array|null $cleanUpColumnKeys The header positions of the columns to clean up + */ + public function __construct(array $headers, array $defaultColumnValues, ?array $cleanUpColumnKeys) + { + $this->headers = $headers; + $this->defaultColumnValues = $defaultColumnValues; + $this->cleanUpEmptyColumnKeys = $cleanUpColumnKeys; + } + + /** + * Merges an empty attribute value into an existing entity. + * + * @param string $attributeCode The attribute code + * @param array $entity The stored entity + * + * @return array The merged entity + */ + public function merge($attributeCode, array $entity) + { + $this->attributeCode = $attributeCode; + $this->attributeValue = ''; + return $this->mergeEntity($entity, array(\TechDivision\Import\Utils\MemberNames::VALUE => '')); + } + + /** + * Returns the operation the last merge has chosen. + * + * @return string The operation + */ + public function getOperation() + { + return $this->operation; + } + + /** + * {@inheritDoc} + */ + public function hasHeader($name) + { + return isset($this->headers[$name]); + } + + /** + * {@inheritDoc} + */ + public function getHeader($name) + { + return $this->headers[$name]; + } + + /** + * {@inheritDoc} + */ + protected function persistVarcharAttribute($attribute) + { + } + + /** + * {@inheritDoc} + */ + protected function persistIntAttribute($attribute) + { + } + + /** + * {@inheritDoc} + */ + protected function persistDecimalAttribute($attribute) + { + } + + /** + * {@inheritDoc} + */ + protected function persistDatetimeAttribute($attribute) + { + } + + /** + * {@inheritDoc} + */ + protected function persistTextAttribute($attribute) + { + } + + /** + * {@inheritDoc} + */ + protected function getAttributesByPrimaryKeyAndStoreId($pk, $storeId) + { + return array(); + } + + /** + * {@inheritDoc} + */ + protected function getPrimaryKey() + { + return null; + } + + /** + * {@inheritDoc} + */ + protected function getPrimaryKeyMemberName() + { + return 'entity_id'; + } + + /** + * {@inheritDoc} + */ + protected function getPrimaryKeyColumnName() + { + return 'sku'; + } + + /** + * {@inheritDoc} + */ + protected function storeViewHasBeenProcessed($pk, $storeViewCode) + { + return false; + } + + /** + * {@inheritDoc} + */ + protected function deleteDatetimeAttribute(array $row, $name = null) + { + } + + /** + * {@inheritDoc} + */ + protected function deleteDecimalAttribute(array $row, $name = null) + { + } + + /** + * {@inheritDoc} + */ + protected function deleteIntAttribute(array $row, $name = null) + { + } + + /** + * {@inheritDoc} + */ + protected function deleteTextAttribute(array $row, $name = null) + { + } + + /** + * {@inheritDoc} + */ + protected function deleteVarcharAttribute(array $row, $name = null) + { + } + + /** + * {@inheritDoc} + */ + protected function hasChanges(array $entity) + { + return true; + } + + /** + * {@inheritDoc} + */ + public function hasValue($name) + { + return false; + } +} diff --git a/tests/unit/Observers/AttributeObserverTraitMergeEntityTest.php b/tests/unit/Observers/AttributeObserverTraitMergeEntityTest.php new file mode 100644 index 0000000..0657c09 --- /dev/null +++ b/tests/unit/Observers/AttributeObserverTraitMergeEntityTest.php @@ -0,0 +1,131 @@ + + * @copyright 2026 TechDivision GmbH + * @license https://opensource.org/licenses/MIT + * @link https://github.com/techdivision/import + * @link http://www.techdivision.com + */ + +namespace TechDivision\Import\Observers; + +use PHPUnit\Framework\TestCase; +use TechDivision\Import\Utils\MemberNames; +use TechDivision\Import\Utils\OperationNames; + +/** + * Test class for the mergeEntity() method of the attribute observer trait. + * + * An empty cell of a column with a default value keeps the stored value, unless the column + * is listed in clean-up-empty-columns. The clean-up column keys hold the header positions + * of the columns, not the attribute codes. + * + * @author TechDivision GmbH + * @copyright 2026 TechDivision GmbH + * @license https://opensource.org/licenses/MIT + * @link https://github.com/techdivision/import + * @link http://www.techdivision.com + */ +class AttributeObserverTraitMergeEntityTest extends TestCase +{ + + /** + * The header positions of the test columns. + * + * @var array + */ + private $headers = array('sku' => 0, 'aup_is_new' => 1, 'aup_plain' => 2); + + /** + * Creates the observer double. + * + * @param array $defaultColumnValues The columns with a default value + * @param array|null $cleanUpColumnKeys The header positions of the columns to clean up + * + * @return \TechDivision\Import\Observers\AttributeObserverTraitMergeEntityImpl The observer + */ + private function createObserver(array $defaultColumnValues, ?array $cleanUpColumnKeys) + { + return new AttributeObserverTraitMergeEntityImpl($this->headers, $defaultColumnValues, $cleanUpColumnKeys); + } + + /** + * An empty cell of a default value column that is listed to be cleaned up deletes the value. + * + * @return void + */ + public function testListedDefaultValueColumnIsDeleted() + { + $observer = $this->createObserver(array('aup_is_new' => ''), array($this->headers['aup_is_new'])); + + $result = $observer->merge('aup_is_new', array(MemberNames::VALUE => '0')); + + $this->assertSame(OperationNames::DELETE, $observer->getOperation()); + $this->assertSame('', $result[MemberNames::VALUE]); + } + + /** + * An empty cell of a default value column that is NOT listed keeps the stored value. + * + * @return void + */ + public function testUnlistedDefaultValueColumnKeepsTheStoredValue() + { + $observer = $this->createObserver(array('aup_is_new' => ''), array($this->headers['aup_plain'])); + + $result = $observer->merge('aup_is_new', array(MemberNames::VALUE => '0')); + + $this->assertSame(OperationNames::UPDATE, $observer->getOperation()); + $this->assertSame('0', $result[MemberNames::VALUE]); + } + + /** + * Without any clean-up column the value of a default value column is kept as well. + * + * @return void + */ + public function testDefaultValueColumnWithoutCleanUpColumnsKeepsTheStoredValue() + { + $observer = $this->createObserver(array('aup_is_new' => ''), null); + + $result = $observer->merge('aup_is_new', array(MemberNames::VALUE => '0')); + + $this->assertSame(OperationNames::UPDATE, $observer->getOperation()); + $this->assertSame('0', $result[MemberNames::VALUE]); + } + + /** + * A column without a default value is deleted on an empty cell, whatever the clean-up list says. + * + * @return void + */ + public function testColumnWithoutDefaultValueIsDeleted() + { + $observer = $this->createObserver(array(), array()); + + $result = $observer->merge('aup_plain', array(MemberNames::VALUE => 'x')); + + $this->assertSame(OperationNames::DELETE, $observer->getOperation()); + $this->assertSame('', $result[MemberNames::VALUE]); + } + + /** + * A default value column that is not part of the file is not translated and keeps the stored value. + * + * @return void + */ + public function testDefaultValueColumnWithoutHeaderKeepsTheStoredValue() + { + $observer = $this->createObserver(array('aup_missing' => ''), array($this->headers['aup_is_new'])); + + $result = $observer->merge('aup_missing', array(MemberNames::VALUE => '0')); + + $this->assertSame(OperationNames::UPDATE, $observer->getOperation()); + $this->assertSame('0', $result[MemberNames::VALUE]); + } +}