[Bug]: Gallery Operator does not work properly when you uncheck "Overwrite: if target is not empty"

[micru]

Expected behavior

When a transform pipeline results in a Gallery and Overwrite: if target is not empty in DataTarget (Direct) is unchecked (default: checked), it should import/assign images to the ImageGallery concerned, if there were no items in the ImageGallery before

Actual behavior

When Overwrite: if target is not empty in DataTarget (Direct) is unchecked, no images are imported when an empty gallery exists (which seems to be the case as soon as an object was saved before)

Steps to reproduce

1. add a ImageGallery to a Class definition
2. create an Object of that class and save it (with no images in the gallery)
3. create an import definition with a transformation pipeline resulting in a gallery (by using Gallery Operator)
4. Use DataTarget Direct and uncheck Overwrite: if target is not empty
5. Run an import which assings images to the gallery => no images are assigned, no error

My guess: there should be an check if $currentData->getIems() is empty (if gallery or if that mehtod exists) here https://github.com/pimcore/data-importer/blob/1.x/src/Mapping/DataTarget/Direct.php#L140

[dvesh3]

It works without any issues. Also tried on https://demo.pimcore.com/admin/ with additional images mapping.

[webdev-dp]

The issue still exists, and the previous testing might have given a false impression of its resolution. While it appeared to work when "If target is not empty" was set to true in the additional images mapping, the problem becomes evident when this setting is changed to false.

Upon closer examination of the code, it's clear that the root cause lies in the checkAssignData function. Currently, it checks if $currentData is empty, but this condition is not appropriate when dealing with objects.

To address this issue properly, we should modify the condition as follows:

$currentData = $valueContainer->$getter($this->language);
  if ($currentData instanceof ImageGallery) {
      $currentData = $currentData->getItems();
  }

  if (!empty($currentData) && $this->writeIfTargetIsNotEmpty === false) {
      return false;
  }

By making this change, we ensure that we correctly evaluate whether the object contains any values or items, which is essential in scenarios like the one described.

This modification should resolve the problem when "If target is not empty" is set to false, as it accurately checks the object's content rather than just its existence.

It works without any issues. Also tried on https://demo.pimcore.com/admin/ with additional images mapping.

[micru]

It does not work. If you uncheck Overwrite: if target is not empty (resulting in $this->writeIfTargetIsNotEmpty === false) it should only write, if it was not holding any data before, right? But it will never write any data, cause if (!empty($currentData)... will always be false, as $currentData is of type ImageGallery, even if there are no images assigned yet. Therefore the check empty($currentData) will always be truthy. The correct check in this case should be if (!empty($currentData->getItems()...). https://github.com/pimcore/data-importer/blob/1.x/src/Mapping/DataTarget/Direct.php#L140

[dvesh3]

@micru @webdev-dp It seems that other data types are also impacted (e.g. QuantityValue). Implemented a generic fix for calling isEmpty check on field definitions instead of simply checking with !empty. Could you please test the changes of #361? thanks!

[micru]

@dvesh3 looking good, it works! Thank you

[dvesh3]

@micru thanks for checking! let's keep it open until the PR is merged :blush:

[dvesh3]

Fixed by #361