Merge remote-tracking branch 'origin/10.6' into 11.0

# Conflicts: # bundles/AdminBundle/Helper/GridHelperService.php # bundles/AdminBundle/Service/PreviewGenerator.php # bundles/CoreBundle/src/Migrations/Version20230616085142.php # doc/20_Extending_Pimcore/24_Add_Your_Own_Permissions.md # models/DataObject/ClassDefinition/Data/Relations/ManyToManyRelationTrait.php # models/DataObject/Data/ObjectMetadata/Dao.php # tests/Model/Relations/FieldcollectionTest.php
aryaantony92 · Jul 14, 2023 · 35c2b6c · 35c2b6c
2 parents 316f288 + ed386f8
commit 35c2b6c
Show file tree

Hide file tree

Showing 11 changed files with 123 additions and 69 deletions.
diff --git a/doc/20_Extending_Pimcore/24_Add_Your_Own_Permissions.md b/doc/20_Extending_Pimcore/24_Add_Your_Own_Permissions.md
@@ -11,27 +11,27 @@ You should now be able to select the permission in the users/roles tabs:
 ```php
 namespace App\Controller;
 
-use Pimcore\Bundle\AdminBundle\Controller\AdminController as PimcoreAdminController;
+use Pimcore\Controller\UserAwareController;
+use Pimcore\Controller\Traits\JsonHelperTrait;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Routing\Annotation\Route;
 
-class AdminController extends PimcoreAdminController
+class AdminController extends UserAwareController
 {
+    use JsonHelperTrait;
     /**
      * @Route("/admin/my-admin-action")
      */
     public function myAdminAction(Request $request): Response
     {
-        /** @var \Pimcore\Security\User\User $user */
-        $user = $this->getUser();
-        $pimcoreUser = $user->getUser();
+        $pimcoreUser = $this->getPimcoreUser();
 
-        if ($pimcoreUser->isAllowed('my_permission')) {
+        if ($pimcoreUser?->isAllowed('my_permission')) {
             // ...
         }
 
-        // ...
+        return $this->jsonResponse(['success' => true]);
     }
 }
 ```
@@ -40,7 +40,7 @@ class AdminController extends PimcoreAdminController
 ```js
 document.addEventListener(pimcore.events.pimcoreReady, (e) => {
     if(pimcore.currentuser.permissions.indexOf("my_permission") >= 0) {
-        ...
+        //...
     }
 });
 ```
diff --git a/..._Installation_and_Upgrade/03_System_Setup_and_Hosting/02_Nginx_Configuration.md b/..._Installation_and_Upgrade/03_System_Setup_and_Hosting/02_Nginx_Configuration.md
@@ -126,7 +126,7 @@ server {
     # Still use a allowlist approach to prevent each and every missing asset to go through the PHP Engine.
     # If you are using remote storages like S3 or Google Cloud Storage, this doesn't work. You either deactivate it and handle it in PHP
     # or redirect these suffixes directly to your CDN URL. Additionally you should configure the frontend url prefixes accordingly, see: https://pimcore.com/docs/pimcore/current/Development_Documentation/Installation_and_Upgrade/System_Setup_and_Hosting/File_Storage_Setup.html
-    location ~* ^(?!/admin)(.+?)\.((?:css|js)(?:\.map)?|jpe?g|gif|png|svgz?|eps|exe|gz|zip|mp\d|m4a|ogg|ogv|webm|pdf|docx?|xlsx?|pptx?)$ {
+    location ~* ^(?!/admin)(.+?)\.((?:css|js)(?:\.map)?|jpe?g|gif|png|svgz?|eps|exe|gz|zip|mp\d|m4a|ogg|ogv|webp|webm|pdf|docx?|xlsx?|pptx?)$ {
         try_files /var/assets$uri $uri =404;
         expires 2w;
         access_log off;
@@ -412,7 +412,7 @@ server {
     # Still use a allowlist approach to prevent each and every missing asset to go through the PHP Engine.
     # If you are using remote storages like S3 or Google Cloud Storage, this doesn't work. You either deactivate it and handle it in PHP
     # or redirect these suffixes directly to your CDN URL. Additionally you should configure the frontend url prefixes accordingly, see: https://pimcore.com/docs/pimcore/current/Development_Documentation/Installation_and_Upgrade/System_Setup_and_Hosting/File_Storage_Setup.html
-    location ~* ^(?!/admin)(.+?)\.((?:css|js)(?:\.map)?|jpe?g|gif|png|svgz?|eps|exe|gz|zip|mp\d|m4a|ogg|ogv|webm|pdf|docx?|xlsx?|pptx?)$ {
+    location ~* ^(?!/admin)(.+?)\.((?:css|js)(?:\.map)?|jpe?g|gif|png|svgz?|eps|exe|gz|zip|mp\d|m4a|ogg|ogv|webp|webm|pdf|docx?|xlsx?|pptx?)$ {
         try_files /var/assets$uri $uri =404;
         expires 2w;
         access_log off;

diff --git a/lib/Controller/Traits/JsonHelperTrait.php b/lib/Controller/Traits/JsonHelperTrait.php
@@ -20,8 +20,6 @@
 use Symfony\Component\HttpFoundation\JsonResponse;
 
 /**
- * @internal
- *
  * @property ContainerInterface $container
  */
 trait JsonHelperTrait

diff --git a/models/DataObject/ClassDefinition/Data/AdvancedManyToManyObjectRelation.php b/models/DataObject/ClassDefinition/Data/AdvancedManyToManyObjectRelation.php
@@ -442,25 +442,12 @@ public function resolveDependencies(mixed $data): array
 
     public function save(Localizedfield|AbstractData|\Pimcore\Model\DataObject\Objectbrick\Data\AbstractData|Concrete $object, array $params = []): void
     {
-        if (!DataObject::isDirtyDetectionDisabled() && $object instanceof Element\DirtyIndicatorInterface) {
-            if ($object instanceof DataObject\Localizedfield) {
-                if ($object->getObject() instanceof Element\DirtyIndicatorInterface) {
-                    if (!$object->hasDirtyFields()) {
-                        return;
-                    }
-                }
-            } else {
-                if ($this->supportsDirtyDetection()) {
-                    if (!$object->isFieldDirty($this->getName())) {
-                        return;
-                    }
-                }
-            }
+        if ($this->skipSaveCheck($object, $params)) {
+            return;
         }
 
         $objectsMetadata = $this->getDataFromObjectParam($object, $params);
 
-        $classId = null;
         $objectId = null;
 
         if ($object instanceof DataObject\Concrete) {

diff --git a/models/DataObject/ClassDefinition/Data/AdvancedManyToManyRelation.php b/models/DataObject/ClassDefinition/Data/AdvancedManyToManyRelation.php
@@ -533,25 +533,12 @@ public function getForCsvExport(DataObject\Localizedfield|DataObject\Fieldcollec
 
     public function save(Localizedfield|AbstractData|\Pimcore\Model\DataObject\Objectbrick\Data\AbstractData|Concrete $object, array $params = []): void
     {
-        if (!DataObject::isDirtyDetectionDisabled() && $object instanceof Element\DirtyIndicatorInterface) {
-            if ($object instanceof DataObject\Localizedfield) {
-                if ($object->getObject() instanceof Element\DirtyIndicatorInterface) {
-                    if (!$object->hasDirtyFields()) {
-                        return;
-                    }
-                }
-            } else {
-                if ($this->supportsDirtyDetection()) {
-                    if (!$object->isFieldDirty($this->getName())) {
-                        return;
-                    }
-                }
-            }
+        if ($this->skipSaveCheck($object, $params)) {
+            return;
         }
 
         $multihrefMetadata = $this->getDataFromObjectParam($object, $params);
 
-        $classId = null;
         $objectId = null;
 
         if ($object instanceof DataObject\Concrete) {

diff --git a/models/DataObject/ClassDefinition/Data/Relations/ManyToManyRelationTrait.php b/models/DataObject/ClassDefinition/Data/Relations/ManyToManyRelationTrait.php
@@ -24,27 +24,40 @@
 
 trait ManyToManyRelationTrait
 {
-    public function save(Localizedfield|AbstractData|\Pimcore\Model\DataObject\Objectbrick\Data\AbstractData|Concrete $object, array $params = []): void
+    /**
+     * Unless forceSave is set to true, this method will check if the field is dirty and skip the save if not
+     *
+     * @param object $object
+     * @param array $params
+     *
+     * @return bool
+     */
+    protected function skipSaveCheck(Localizedfield|AbstractData|\Pimcore\Model\DataObject\Objectbrick\Data\AbstractData|Concrete $object, array $params = []): bool
     {
-        if (!isset($params['forceSave']) || $params['forceSave'] !== true) {
-            if (!DataObject::isDirtyDetectionDisabled() && $object instanceof DirtyIndicatorInterface) {
-                if ($object instanceof DataObject\Localizedfield) {
-                    if ($object->getObject() instanceof DirtyIndicatorInterface) {
-                        if (!$object->hasDirtyFields()) {
-                            return;
-                        }
-                    }
-                } else {
-                    if ($this->supportsDirtyDetection()) {
-                        if (!$object->isFieldDirty($this->getName())) {
-                            return;
-                        }
-                    }
+        $forceSave = $params['forceSave'] ?? false;
+
+        if (
+            $forceSave === false &&
+            !DataObject::isDirtyDetectionDisabled() &&
+            $object instanceof DirtyIndicatorInterface
+        ) {
+            if ($object instanceof DataObject\Localizedfield) {
+                if ($object->getObject() instanceof DirtyIndicatorInterface && !$object->hasDirtyFields()) {
+                    return true;
                 }
+            } elseif ($this->supportsDirtyDetection() && !$object->isFieldDirty($this->getName())) {
+                return true;
             }
         }
 
-        $data = $this->getDataFromObjectParam($object, $params);
+        return false;
+    }
+
+    public function save(Localizedfield|AbstractData|\Pimcore\Model\DataObject\Objectbrick\Data\AbstractData|Concrete $object, array $params = []): void
+    {
+        if ($this->skipSaveCheck($object, $params)) {
+            return;
+        }
 
         parent::save($object, $params);
     }

diff --git a/models/DataObject/Data/ObjectMetadata/Dao.php b/models/DataObject/Data/ObjectMetadata/Dao.php
@@ -33,7 +33,7 @@ public function save(DataObject\Concrete $object, string $ownertype, string $own
     {
         $table = $this->getTablename($object);
 
-        $dataTemplate = ['id' => $object->getId(),
+        $dataTemplate = ['o_id' => $object->getId(),
             'dest_id' => $this->model->getElement()->getId(),
             'fieldname' => $this->model->getFieldname(),
             'ownertype' => $ownertype,
@@ -85,7 +85,8 @@ public function createOrUpdateTable(DataObject\ClassDefinition $class): void
         $table = 'object_metadata_' . $classId;
 
         $this->db->executeQuery('CREATE TABLE IF NOT EXISTS `' . $table . "` (
-              `id` int(11) UNSIGNED NOT NULL default '0',
+              `id` int(11) UNSIGNED NOT NULL AUTO_INCREMENT,
+              `o_id` int(11) UNSIGNED NOT NULL default '0',
               `dest_id` int(11) NOT NULL default '0',
 	          `type` VARCHAR(50) NOT NULL DEFAULT '',
               `fieldname` varchar(71) NOT NULL,
@@ -95,15 +96,19 @@ public function createOrUpdateTable(DataObject\ClassDefinition $class): void
               `ownername` VARCHAR(70) NOT NULL DEFAULT '',
               `position` VARCHAR(70) NOT NULL DEFAULT '0',
               `index` int(11) unsigned NOT NULL DEFAULT '0',
-              PRIMARY KEY (`id`, `dest_id`, `type`, `fieldname`, `column`, `ownertype`, `ownername`, `position`, `index`),
+              PRIMARY KEY (`id`),
+              UNIQUE KEY `metadata_un` (
+                `o_id`, `dest_id`, `type`, `fieldname`, `column`, `ownertype`, `ownername`, `position`, `index`
+              ),
               INDEX `dest_id` (`dest_id`),
               INDEX `fieldname` (`fieldname`),
               INDEX `column` (`column`),
               INDEX `ownertype` (`ownertype`),
               INDEX `ownername` (`ownername`),
               INDEX `position` (`position`),
               INDEX `index` (`index`),
-              CONSTRAINT `".self::getForeignKeyName($table, 'id').'` FOREIGN KEY (`id`) REFERENCES objects (`id`) ON DELETE CASCADE
+              CONSTRAINT `".self::getForeignKeyName($table, 'o_id').'` FOREIGN KEY (`o_id`)
+              REFERENCES objects (`o_id`) ON DELETE CASCADE
 		) DEFAULT CHARSET=utf8mb4;');
 
         $this->handleEncryption($class, [$table]);

diff --git a/models/Document/PageSnippet.php b/models/Document/PageSnippet.php
@@ -387,25 +387,25 @@ public function getEditable(string $name): ?Editable
             return $editables[$name];
         }
 
+        $inheritedEditable = null;
         if (array_key_exists($name, $this->inheritedEditables)) {
-            return $this->inheritedEditables[$name];
+            $inheritedEditable = $this->inheritedEditables[$name];
         }
 
-        // check for content main document (inherit data)
-        if ($contentMainDocument = $this->getContentMainDocument()) {
-            if ($contentMainDocument instanceof self) {
+        if (!$inheritedEditable) {
+            // check for content main document (inherit data)
+            $contentMainDocument = $this->getContentMainDocument();
+            if ($contentMainDocument instanceof self && $contentMainDocument->getId() != $this->getId()) {
                 $inheritedEditable = $contentMainDocument->getEditable($name);
                 if ($inheritedEditable) {
                     $inheritedEditable = clone $inheritedEditable;
                     $inheritedEditable->setInherited(true);
                     $this->inheritedEditables[$name] = $inheritedEditable;
-
-                    return $inheritedEditable;
                 }
             }
         }
 
-        return null;
+        return $inheritedEditable;
     }
 
     /**

diff --git a/models/Document/Service.php b/models/Document/Service.php
@@ -199,6 +199,7 @@ public function copyAsChild(Document $target, Document $source, bool $enableInhe
 
         if ($enableInheritance && ($new instanceof Document\PageSnippet) && $new->supportsContentMain()) {
             $new->setEditables([]);
+            $new->setMissingRequiredEditable(false);
             $new->setContentMainDocumentId($source->getId(), true);
         }
 

diff --git a/tests/Model/Relations/FieldcollectionTest.php b/tests/Model/Relations/FieldcollectionTest.php
@@ -16,7 +16,9 @@
 
 namespace Pimcore\Tests\Model\Relations;
 
+use Pimcore\Model\Asset;
 use Pimcore\Model\DataObject;
+use Pimcore\Model\DataObject\Data\ElementMetadata;
 use Pimcore\Model\DataObject\Fieldcollection;
 use Pimcore\Model\DataObject\RelationTest;
 use Pimcore\Model\DataObject\Service;
@@ -119,6 +121,56 @@ public function testRelationFieldInsideFieldCollection(): void
         $this->assertEquals([], $rel);
     }
 
+    public function testAdvancedRelationFieldInsideFieldCollection(): void
+    {
+        $object = TestHelper::createEmptyObject();
+        $items = new Fieldcollection();
+
+        $target1 = new Asset();
+        $target1->setParent(Asset::getByPath('/'));
+        $target1->setKey('mytarget1');
+        $target1->save();
+
+        $target2 = new Asset();
+        $target2->setParent(Asset::getByPath('/'));
+        $target2->setKey('mytarget2');
+        $target2->save();
+
+        $target3 = new Asset();
+        $target3->setParent(Asset::getByPath('/'));
+        $target3->setKey('mytarget3');
+        $target3->save();
+
+        $item1 = new FieldCollection\Data\Unittestfieldcollection();
+        $item1->setAdvancedFieldRelation([new ElementMetadata('metadataUpper', [], $target1)]);
+
+        $item2 = new FieldCollection\Data\Unittestfieldcollection();
+        $item2->setAdvancedFieldRelation([new ElementMetadata('metadataUpper', [], $target2)]);
+
+        $items->add($item1);
+        $items->add($item2);
+
+        $object->setFieldcollection($items);
+        $object->save();
+
+        // Test by deleting the target2 element
+        $target2->delete();
+
+        $object = DataObject::getById($object->getId(), ['force' => true]);
+        $object->save();
+
+        $object = DataObject::getById($object->getId(), ['force' => true]);
+        //check if target1 is still there
+        $loadedFieldcollectionItem = $object->getFieldcollection()->get(0);
+        $rel = $loadedFieldcollectionItem->getAdvancedFieldRelation();
+        $this->assertEquals($target1->getId(), isset($rel[0]) ? $rel[0]->getElementId() : false);
+
+        //check if target2 is removed
+        $loadedFieldcollectionItem = $object->getFieldcollection()->get(1);
+        $rel = $loadedFieldcollectionItem->getAdvancedFieldRelation();
+        $this->assertEquals(false, isset($rel[0]));
+    }
+
     public function testLocalizedFieldInsideFieldCollection(): void
     {
         $target1 = new RelationTest();

diff --git a/tests/Support/Helper/Model.php b/tests/Support/Helper/Model.php
@@ -674,6 +674,17 @@ public function setupFieldcollection_Unittestfieldcollection(string $name = 'uni
                 ->setDocumentTypes([])->setAssetTypes([])->setClasses([])
                 ->setDocumentsAllowed(true)->setAssetsAllowed(true)->setObjectsAllowed(true));
 
+            $panel->addChild($this->createDataChild('manyToManyRelation', 'fieldRelation')
+                ->setDocumentTypes([])->setAssetTypes([])->setClasses([])
+                ->setDocumentsAllowed(true)->setAssetsAllowed(true)->setObjectsAllowed(true));
+
+            $panel->addChild($this->createDataChild('advancedManyToManyRelation', 'advancedFieldRelation')
+                ->setAllowMultipleAssignments(false)
+                ->setDocumentTypes([])->setAssetTypes([])->setClasses([])
+                ->setDocumentsAllowed(true)->setAssetsAllowed(true)->setObjectsAllowed(true)
+                ->setColumns([ ['position' => 1, 'key' => 'metadataUpper', 'type' => 'text', 'label' => 'meta'],
+                ]));
+
             $panel->addChild($this->createDataChild('manyToManyRelation', 'fieldLazyRelation')
                 ->setDocumentTypes([])->setAssetTypes([])->setClasses([])
                 ->setDocumentsAllowed(true)->setAssetsAllowed(true)->setObjectsAllowed(true));