Added error handling when changing existing variants

As the parent can be changed for variants this is now catched, as a variant cannot change its parent anymore.
turbo-ele · Nov 9, 2023 · 2ac1474 · 2ac1474
1 parent 976a7df
commit 2ac1474
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 6 deletions.
diff --git a/src/Resolver/Location/FindParentStrategy.php b/src/Resolver/Location/FindParentStrategy.php
@@ -16,6 +16,7 @@
 namespace Pimcore\Bundle\DataImporterBundle\Resolver\Location;
 
 use Pimcore\Bundle\DataImporterBundle\Exception\InvalidConfigurationException;
+use Pimcore\Bundle\DataImporterBundle\Exception\InvalidInputException;
 use Pimcore\Bundle\DataImporterBundle\Tool\DataObjectLoader;
 use Pimcore\Model\DataObject;
 use Pimcore\Model\DataObject\ClassDefinition;
@@ -133,11 +134,19 @@ public function updateParent(ElementInterface $element, array $inputData): Eleme
         }
 
         if ($newParent) {
+            if ($newParent->getType() === DataObject::OBJECT_TYPE_VARIANT) {
+                throw new InvalidInputException(
+                    'The elements desired parent is a variant which cannot have any child elements'
+                );
+            }
+
+            // Check if element should be saved as a variant.
             if (
-                $this->saveAsVariant &&
-                $newParent instanceof DataObject\Concrete &&
-                $newParent::class === $element::class &&
-                $newParent->getClass()->getAllowVariants()
+                $this->saveAsVariant
+                && $element instanceof DataObject\Concrete
+                && $element::class === $newParent::class
+                && $element->getClass()->getAllowVariants()
+                && empty($element->getChildren()->getData())
             ) {
                 /** @var DataObject\Concrete $element */
                 $element->setType(DataObject::OBJECT_TYPE_VARIANT);
@@ -146,8 +155,6 @@ public function updateParent(ElementInterface $element, array $inputData): Eleme
             return $element->setParent($newParent);
         }
 
-        // Save the element as variant: The parent and element need to be of the same dataobject type.
-
         return $element;
     }
 

diff --git a/src/Resolver/Resolver.php b/src/Resolver/Resolver.php
@@ -15,10 +15,12 @@
 
 namespace Pimcore\Bundle\DataImporterBundle\Resolver;
 
+use Pimcore\Bundle\DataImporterBundle\Exception\InvalidInputException;
 use Pimcore\Bundle\DataImporterBundle\Resolver\Factory\FactoryInterface;
 use Pimcore\Bundle\DataImporterBundle\Resolver\Load\LoadStrategyInterface;
 use Pimcore\Bundle\DataImporterBundle\Resolver\Location\LocationStrategyInterface;
 use Pimcore\Bundle\DataImporterBundle\Resolver\Publish\PublishStrategyInterface;
+use Pimcore\Model\DataObject;
 use Pimcore\Model\Element\ElementInterface;
 
 class Resolver
@@ -149,7 +151,15 @@ public function loadOrCreateAndPrepareElement(array $inputData, bool $createNew
             $this->getCreateLocationStrategy()->updateParent($element, $inputData);
             $justCreated = true;
         } else {
+            $oldParentId = $element->getParentId();
             $this->getLocationUpdateStrategy()->updateParent($element, $inputData);
+
+            // The parent of a variant cannot be changed anymore.
+            if ($oldParentId !== $element->getParentId() && $element->getType() === DataObject::OBJECT_TYPE_VARIANT) {
+                throw new InvalidInputException(
+                    "Element with id `{$element->getId()}` is a variant and cannot change its parent anymore"
+                );
+            }
         }
 
         $this->getPublishingStrategy()->updatePublishState($element, $justCreated, $inputData);