import icon indicating copy to clipboard operation
import copied to clipboard

Extract functionality to clean-up empty columns from AbstractOberserver::mergeEntity() method

Open wagnert opened this issue 5 years ago • 1 comments

Actual Situation Up from version 16.1.0, the method AbstractOberserver::mergeEntity() contains a check if the invoking subject implements the CleanUpColumnsSubjectInterface to clean-up columns that has been configured in the clean-up-empty-columns array in the configuration, as described in the documentation.

Additionally, the actual implementation assumes that the column equals the member names

// query whether or not the subject has columns that has to be cleaned-up
if (($subject = $this->getSubject()) instanceof CleanUpColumnsSubjectInterface) {
    // load the columns that has to be cleaned-up
    $cleanUpColumns =  $subject->getCleanUpColumns();
    // load the column/member names from the attributes
    $columnNames = array_keys($attr);

    // iterate over the column names
    foreach ($columnNames as $columnName) {
        // we do NOT clean-up members that HAS a value or ARE in
        // the array with column names that has to be cleaned-up
        if ($this->hasValue($columnName) || in_array($columnName, $cleanUpColumns)) {
            continue;
        }
        // unset the column, because it has to be cleaned-up
        unset($attr[$columnName]);
    }
}

This should also be refactored by add the possibility to map the column to the attribute names.

Expected Situation The functionality to clean-up the empty columns should be extracted to a utility class, injected, and integrated like the functionality for the state detection. This refactoring should result in something like

protected function mergeEntity(array $entity, array $attr, $changeSetName = null)
{
    return array_merge(
        $entity, 
        $attr, 
        array(
            EntityStatus::MEMBER_NAME => $this->detectState(
                $entity, 
                $this->cleanUpEntity($attr), 
                $changeSetName
         )
    )
);

wagnert avatar May 15 '20 08:05 wagnert

@wagnert it would be great if you could fix this somehow soon because right now it is only possible to import the backorders - column with a little hack. backorders in database is allow_backorders in csv

To make it work you need to add both columns with the same value.

BTW: Thanks for leaving the todo comment in the code otherwise i would not have found the problem so fast

pointia avatar Jun 23 '21 12:06 pointia

Much more simpler. fixed in product import 25.1.4 https://github.com/techdivision/import-product/commit/8551581b46b4f644997e1efd8f25023b33203613

Mardl avatar Mar 06 '23 17:03 Mardl