Fix Group MultiSelect Relation Fetch (#96)

* fix group multiselect data fetch, see #95 * fix formatting * add database reference * add deep object clean-up in tests
dachcom-digital · May 8, 2019 · b4139f6 · b4139f6
1 parent ea12c6c
commit b4139f6
Show file tree

Hide file tree

Showing 17 changed files with 255 additions and 30 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -81,10 +81,10 @@ matrix:
             php: 7.3
             env:
                 - PIMCORE_SKELETON_BRANCH="tags/v1.3.0"
-        allow_failures:
-            -   env:
-                    - PIMCORE_SKELETON_BRANCH="tags/v1.3.0"
-                    - DACHCOM_TEST_SECTION=ecs
+    allow_failures:
+        -   env:
+                - PIMCORE_SKELETON_BRANCH="tags/v1.3.0"
+                - DACHCOM_TEST_SECTION=ecs
     fast_finish: true
 
 cache:

diff --git a/README.md b/README.md
@@ -106,7 +106,7 @@ If you want to use the [Customer Data Framework](https://github.com/pimcore/cust
 | Name | Field Type | Comment |
 |---------------------|-------------|-------------------------------|
 | name | Input |  |
-| roles | Multiselection | Set "Options Provider Class or Service Name" to `MembersBundle\CoreExtension\Provider\RoleOptionsProvider` |
+| roles | Multiselection | Set "Options Provider Class or Service Name" to `@MembersBundle\CoreExtension\Provider\RoleOptionsProvider` |
 
 > `membersGroup` is the default name, you may want to change it. Read [here](docs/20_CustomClassName.md) how to achieve that.
 

diff --git a/src/MembersBundle/CoreExtension/GroupMultiselect.php b/src/MembersBundle/CoreExtension/GroupMultiselect.php
@@ -2,6 +2,7 @@
 
 namespace MembersBundle\CoreExtension;
 
+use MembersBundle\Tool\VersionHelper;
 use Pimcore\Model\Element;
 use Pimcore\Model\DataObject;
 use Pimcore\Model\DataObject\ClassDefinition\Data\Relations\AbstractRelations;
@@ -81,18 +82,18 @@ public function getDataForEditmode($data, $object = null, $params = [])
     }
 
     /**
-     * @see DataObject\ClassDefinition\Data::getDataFromEditmode
-     *
      * @param array                          $data
      * @param null|DataObject\AbstractObject $object
      * @param mixed                          $params
      *
      * @return array
+     *
+     * @see DataObject\ClassDefinition\Data::getDataFromEditmode
      */
     public function getDataFromEditmode($data, $object = null, $params = [])
     {
         //if not set, return null
-        if ($data === null or $data === false) {
+        if ($data === null || $data === false) {
             return null;
         }
 
@@ -219,19 +220,36 @@ public function prepareDataForPersistence($data, $object = null, $params = [])
      */
     public function getDataFromResource($data = [], $object = null, $params = [])
     {
-        $elements = [];
+        $elements = [
+            'dirty' => false,
+            'data'  => []
+        ];
+
         if (is_array($data) && count($data) > 0) {
             foreach ($data as $element) {
                 $e = null;
                 if ($element['type'] == 'object') {
                     $e = DataObject::getById($element['dest_id']);
                 }
                 if ($e instanceof Element\ElementInterface) {
-                    $elements[] = $e;
+                    $elements['data'][] = $e;
+                } else {
+                    $elements['dirty'] = true;
                 }
             }
         }
 
-        return $elements;
+        // only return, if pimcore is equal or greater than 5.8.0
+        // since they changed return value structure significant and without any warnings.
+        if (VersionHelper::pimcoreVersionIsGreaterOrEqualThan('5.8.0')) {
+            return $elements;
+        }
+
+        $legacyElements = [];
+        foreach ($elements['data'] as $element) {
+            $legacyElements[] = $element;
+        }
+
+        return $legacyElements;
     }
 }
diff --git a/src/MembersBundle/CoreExtension/Provider/RoleOptionsProvider.php b/src/MembersBundle/CoreExtension/Provider/RoleOptionsProvider.php
@@ -4,15 +4,9 @@
 
 use Pimcore\Model\DataObject\ClassDefinition\DynamicOptionsProvider\MultiSelectOptionsProviderInterface;
 use Pimcore\Model\DataObject\ClassDefinition\Data;
-use Psr\Container\ContainerInterface;
 
 class RoleOptionsProvider implements MultiSelectOptionsProviderInterface
 {
-    /**
-     * @var ContainerInterface
-     */
-    protected $container;
-
     /**
      * @var array
      */
@@ -25,12 +19,8 @@ class RoleOptionsProvider implements MultiSelectOptionsProviderInterface
         'ROLE_PIMCORE_ADMIN'
     ];
 
-    /**
-     * RoleOptionsProvider constructor.
-     */
-    public function __construct()
+    public function __construct(array $systemRoles)
     {
-        $systemRoles = \Pimcore::getKernel()->getContainer()->getParameter('security.role_hierarchy.roles');
         $this->originalRoles = array_diff_key($systemRoles, array_flip($this->invalidRoles));
     }
 

diff --git a/src/MembersBundle/Migrations/Version20190508092938.php b/src/MembersBundle/Migrations/Version20190508092938.php
@@ -0,0 +1,78 @@
+<?php
+
+namespace MembersBundle\Migrations;
+
+use Doctrine\DBAL\Schema\Schema;
+use MembersBundle\Adapter\Group\AbstractGroup;
+use Pimcore\Migrations\Migration\AbstractPimcoreMigration;
+use Pimcore\Model\DataObject;
+
+class Version20190508092938 extends AbstractPimcoreMigration
+{
+    /**
+     * @return bool
+     */
+    public function doesSqlMigrations(): bool
+    {
+        return false;
+    }
+
+    /**
+     * @param Schema $schema
+     */
+    public function up(Schema $schema)
+    {
+        $groupFound = false;
+
+        $list = new DataObject\ClassDefinition\Listing();
+        $list = $list->load();
+
+        foreach ($list as $class) {
+            if ($groupFound === true) {
+                break;
+            }
+
+            if ($class->getParentClass() === sprintf('\%s', AbstractGroup::class)) {
+                if (is_array($class->getFieldDefinitions())) {
+                    foreach ($class->getFieldDefinitions() as $fieldDefinition) {
+                        if ($fieldDefinition->getName() === 'roles') {
+                            if ($fieldDefinition instanceof DataObject\ClassDefinition\Data\Multiselect) {
+                                if ($fieldDefinition->getOptionsProviderClass() === '@MembersBundle\CoreExtension\Provider\RoleOptionsProvider') {
+                                    // already a service. skip....
+                                    $this->writeMessage('RoleOptionsProvider already defined as service. skipping...');
+                                    $groupFound = true;
+
+                                    break;
+                                } elseif ($fieldDefinition->getOptionsProviderClass() === 'MembersBundle\CoreExtension\Provider\RoleOptionsProvider') {
+                                    $groupFound = true;
+                                    $fieldDefinition->setOptionsProviderClass('@MembersBundle\CoreExtension\Provider\RoleOptionsProvider');
+                                    $this->writeMessage('Convert RoleOptionsProvider to symfony service...');
+                                    $class->save();
+
+                                    break;
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        }
+
+        if ($groupFound === false) {
+            $this->write(
+                sprintf(
+                    '<error>No valid Members Group Class found. Please change "options provider class" of field "roles" to "%s" manually.</error>',
+                    '@MembersBundle\CoreExtension\Provider\RoleOptionsProvider'
+                )
+            );
+        }
+    }
+
+    /**
+     * @param Schema $schema
+     */
+    public function down(Schema $schema)
+    {
+        // this down() migration is auto-generated, please modify it to your needs
+    }
+}
diff --git a/src/MembersBundle/Resources/config/services/options_provider.yml b/src/MembersBundle/Resources/config/services/options_provider.yml
@@ -0,0 +1,11 @@
+services:
+
+    _defaults:
+        autowire: true
+        autoconfigure: true
+        public: true
+
+    MembersBundle\CoreExtension\Provider\RoleOptionsProvider:
+        arguments:
+            $systemRoles: '%security.role_hierarchy.roles%'
+
diff --git a/src/MembersBundle/Resources/install/classes/class_MembersGroup_export.json b/src/MembersBundle/Resources/install/classes/class_MembersGroup_export.json
@@ -66,7 +66,7 @@
                         "width": "",
                         "height": "",
                         "maxItems": "",
-                        "optionsProviderClass": "MembersBundle\\CoreExtension\\Provider\\RoleOptionsProvider",
+                        "optionsProviderClass": "@MembersBundle\\CoreExtension\\Provider\\RoleOptionsProvider",
                         "optionsProviderData": "",
                         "queryColumnType": "text",
                         "columnType": "text",

diff --git a/src/MembersBundle/Tool/Install.php b/src/MembersBundle/Tool/Install.php
@@ -8,6 +8,7 @@
 use Doctrine\DBAL\Migrations\Version;
 use MembersBundle\Configuration\Configuration;
 use Pimcore\Bundle\AdminBundle\Security\User\TokenStorageUserResolver;
+use Pimcore\Db\PimcoreExtensionsTrait;
 use Pimcore\Extension\Bundle\Installer\MigrationInstaller;
 use Pimcore\Migrations\Migration\InstallMigration;
 use Pimcore\Model\Asset;
@@ -327,6 +328,7 @@ public function installTranslations()
      */
     public function installDbStructure()
     {
+        /** @var PimcoreExtensionsTrait $db */
         $db = \Pimcore\Db::get();
         $db->query(file_get_contents($this->getInstallSourcesPath() . '/sql/install.sql'));
     }

diff --git a/tests/_support/Util/VersionHelper.php → src/MembersBundle/Tool/VersionHelper.php b/tests/_support/Util/VersionHelper.php → src/MembersBundle/Tool/VersionHelper.php
@@ -1,6 +1,6 @@
 <?php
 
-namespace DachcomBundle\Test\Util;
+namespace MembersBundle\Tool;
 
 class VersionHelper
 {

diff --git a/tests/_etc/config/system/config.yml b/tests/_etc/config/system/config.yml
@@ -8,6 +8,10 @@ imports:
 swiftmailer:
     disable_delivery: true
 
+security:
+    role_hierarchy:
+        ROLE_MEMBERS_MODERATOR: [ROLE_USER]
+
 framework:
     profiler:
         enabled: true

diff --git a/tests/_support/Helper/PimcoreBackend.php b/tests/_support/Helper/PimcoreBackend.php
@@ -37,9 +37,8 @@ public function _before(TestInterface $test)
      */
     public function _after(TestInterface $test)
     {
-        TestHelper::cleanUp();
-        FileGeneratorHelper::cleanUp();
         MembersHelper::cleanUp();
+        FileGeneratorHelper::cleanUp();
         MembersHelper::reCreateMembersStructure();
 
         parent::_after($test);

diff --git a/tests/_support/Test/DachcomBundleTestCase.php b/tests/_support/Test/DachcomBundleTestCase.php
@@ -16,7 +16,6 @@ abstract class DachcomBundleTestCase extends TestCase
 {
     protected function _after()
     {
-        TestHelper::cleanUp();
         MembersHelper::cleanUp();
         MembersHelper::reCreateMembersStructure();
         FileGeneratorHelper::cleanUp();
@@ -55,17 +54,31 @@ protected function createUser($published = false, $groups = [])
         return $user;
     }
 
-    protected function createUserGroup()
+    /**
+     * @param string $key
+     * @param array  $roles
+     *
+     * @return DataObject\MembersGroup
+     * @throws \Exception
+     */
+    protected function createUserGroup($key = 'group-1', $roles = [])
     {
         $group = new DataObject\MembersGroup();
-        $group->setKey('group-1');
+        $group->setKey($key);
+        $group->setName(MembersHelper::DEFAULT_FEG_NAME);
         $group->setPublished(true);
         $group->setParent(DataObject::getByPath('/'));
+        $group->setRoles($roles);
         $group->save();
 
         return $group;
     }
 
+    /**
+     * @param array $groups
+     *
+     * @return \Pimcore\Model\Document\Page
+     */
     protected function createRestrictedDocument($groups = [])
     {
         $document = TestHelper::createEmptyDocumentPage('restricted-document');

diff --git a/tests/_support/Util/MembersHelper.php b/tests/_support/Util/MembersHelper.php
@@ -3,6 +3,9 @@
 namespace DachcomBundle\Test\Util;
 
 use MembersBundle\Tool\Install;
+use MembersBundle\Tool\VersionHelper;
+use Pimcore\Model\DataObject;
+use Pimcore\Tests\Util\TestHelper;
 
 class MembersHelper
 {
@@ -11,8 +14,20 @@ class MembersHelper
     const DEFAULT_FEU_EMAIL = '[email protected]';
     const DEFAULT_FEU_PASSWORD = 'default-password';
 
+    const DEFAULT_FEG_NAME = 'Default Group';
+
     public static function cleanUp()
     {
+        TestHelper::cleanUp();
+
+        $objectList = new DataObject\Listing();
+        $objectList->setCondition('o_id != 1');
+        $objectList->setUnpublished(true);
+
+        foreach ($objectList->getObjects() as $object) {
+            $object->delete();
+        }
+
         $db = \Pimcore\Db::get();
         $db->exec('TRUNCATE TABLE members_restrictions');
         $db->exec('TRUNCATE TABLE members_group_relations');

diff --git a/tests/rest/ObjectTest.php b/tests/rest/ObjectTest.php
@@ -15,7 +15,6 @@ class UserTest extends RestTestCase
 {
     protected function _after()
     {
-        TestHelper::cleanUp();
         MembersHelper::cleanUp();
         MembersHelper::reCreateMembersStructure();
 

diff --git a/tests/unit/Dao/GroupTest.php b/tests/unit/Dao/GroupTest.php
@@ -0,0 +1,24 @@
+<?php
+
+namespace DachcomBundle\Test\unit\Dao;
+
+use DachcomBundle\Test\Test\DachcomBundleTestCase;
+use DachcomBundle\Test\Util\MembersHelper;
+use Pimcore\Model\DataObject\MembersGroup;
+
+class GroupTest extends DachcomBundleTestCase
+{
+    /**
+     * @throws \Exception
+     */
+    public function testGroupDaoEntity()
+    {
+        $group = $this->createUserGroup('group-1', ['ROLE_MEMBERS_MODERATOR']);
+        $storedGroup = MembersGroup::getById($group->getId(), true);
+
+        $this->assertInstanceOf(MembersGroup::class, $group);
+        $this->assertEquals(MembersHelper::DEFAULT_FEG_NAME, $storedGroup->getName());
+        $this->assertCount(1, $storedGroup->getRoles());
+        $this->assertEquals('ROLE_MEMBERS_MODERATOR', $storedGroup->getRoles()[0]);
+    }
+}