EasyCorp / EasyAdminBundle

EasyAdmin is a fast, beautiful and modern admin generator for Symfony applications.
MIT License
4.07k stars 1.02k forks source link

Cannot search on enum fields #3902

Open grzegorz-stolarz opened 4 years ago

grzegorz-stolarz commented 4 years ago

Describe the bug In version 3.1.6 was added support for search on 2 level nested fields. It's fine, I've tested it and it works, but author completely forgot about enums fields which could be added and their type is non-string so search didn't work.

To Reproduce Use custom enum field with ORM

(OPTIONAL) Additional context In EntityRepository.php within method addSearchClause we can fix it within this code.

    private function addSearchClause(QueryBuilder $queryBuilder, SearchDto $searchDto, EntityDto $entityDto): void
    {
        $query = $searchDto->getQuery();
        $lowercaseQuery = mb_strtolower($query);
        $isNumericQuery = is_numeric($query);
        $isSmallIntegerQuery = ctype_digit($query) && $query >= -32768 && $query <= 32767;
        $isIntegerQuery = ctype_digit($query) && $query >= -2147483648 && $query <= 2147483647;
        $isUuidQuery = 1 === preg_match('/^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i', $query);
        $databasePlatform = new \ReflectionClass($this->doctrine->getConnection()->getDatabasePlatform());

        $dqlParameters = [
            // adding '0' turns the string into a numeric value
            'numeric_query' => is_numeric($query) ? 0 + $query : $query,
            'uuid_query' => $query,
            'text_query' => '%'.$lowercaseQuery.'%',
            'words_query' => explode(' ', $lowercaseQuery),
        ];

        $entitiesAlreadyJoined = [];
        $configuredSearchableProperties = $searchDto->getSearchableProperties();
        $searchableProperties = empty($configuredSearchableProperties) ? $entityDto->getAllPropertyNames() : $configuredSearchableProperties;
        foreach ($searchableProperties as $propertyName) {
            if ($entityDto->isAssociation($propertyName)) {
                // support arbitrarily nested associations (e.g. foo.bar.baz.qux)
                $associatedProperties = explode('.', $propertyName);
                $numAssociatedProperties = \count($associatedProperties);

                $originalPropertyName = $associatedProperties[0];
                $originalPropertyMetadata = $entityDto->getPropertyMetadata($originalPropertyName);
                $associatedEntityDto = $this->entityFactory->create($originalPropertyMetadata->get('targetEntity'));

                for ($i = 0; $i < $numAssociatedProperties - 1; ++$i) {
                    $associatedEntityName = $associatedProperties[$i];
                    $associatedPropertyName = $associatedProperties[$i + 1];

                    if (!\in_array($associatedEntityName, $entitiesAlreadyJoined, true)) {
                        $parentEntityName = 0 === $i ? 'entity' : $associatedProperties[$i - 1];
                        $queryBuilder->leftJoin($parentEntityName.'.'.$associatedEntityName, $associatedEntityName);
                        $entitiesAlreadyJoined[] = $associatedEntityName;
                    }

                    if ($i < $numAssociatedProperties - 2) {
                        $propertyMetadata = $associatedEntityDto->getPropertyMetadata($associatedPropertyName);
                        $targetEntity = $propertyMetadata->get('targetEntity');
                        $associatedEntityDto = $this->entityFactory->create($targetEntity);
                    }
                }

                $entityName = $associatedEntityName;
                $propertyName = $associatedPropertyName;
                $propertyDataType = $associatedEntityDto->getPropertyDataType($propertyName);
            } else {
                $entityName = 'entity';
                $propertyDataType = $entityDto->getPropertyDataType($propertyName);
            }

            $isSmallIntegerProperty = 'smallint' === $propertyDataType;
            $isIntegerProperty = 'integer' === $propertyDataType;
            $isNumericProperty = \in_array($propertyDataType, ['number', 'bigint', 'decimal', 'float']);
            // 'citext' is a PostgreSQL extension (https://github.com/EasyCorp/EasyAdminBundle/issues/2556)
            $isTextProperty = \in_array($propertyDataType, ['string', 'text', 'citext', 'array', 'simple_array']);
            $isGuidProperty = \in_array($propertyDataType, ['guid', 'uuid']);

            // this complex condition is needed to avoid issues on PostgreSQL databases
            if (
                ($isSmallIntegerProperty && $isSmallIntegerQuery) ||
                ($isIntegerProperty && $isIntegerQuery) ||
                ($isNumericProperty && $isNumericQuery)
            ) {
                $queryBuilder->orWhere(sprintf('%s.%s = :query_for_numbers', $entityName, $propertyName))
                    ->setParameter('query_for_numbers', $dqlParameters['numeric_query']);
            } elseif ($isGuidProperty && $isUuidQuery) {
                $queryBuilder->orWhere(sprintf('%s.%s = :query_for_uuids', $entityName, $propertyName))
                    ->setParameter('query_for_uuids', $dqlParameters['uuid_query']);
            } elseif ($isTextProperty) {
                $queryBuilder->orWhere(sprintf('LOWER(%s.%s) LIKE :query_for_text', $entityName, $propertyName))
                    ->setParameter('query_for_text', $dqlParameters['text_query']);
                $queryBuilder->orWhere(sprintf('LOWER(%s.%s) IN (:query_as_words)', $entityName, $propertyName))
                    ->setParameter('query_as_words', $dqlParameters['words_query']);
            } else if ($databasePlatform !== PostgreSQLPlatform::class) {
                $queryBuilder->orWhere(sprintf('LOWER(%s.%s) LIKE :query_for_text', $entityName, $propertyName))
                    ->setParameter('query_for_text', $dqlParameters['text_query']);
            }
        }
    }

I know, it's deprecated class but we can create a map for PostgreSQL classes which will be avoided to use this search. Hope I could help 👍

javiereguiluz commented 4 years ago

@grzegorz-stolarz I'm afraid I don't understand the proposed fix. Could you please post a diff of the code needed to support this? Generally speaking we don't support custom Doctrine types ... but if the needed changes are minimal, we could do something.

grzegorz-stolarz commented 3 years ago
Index: src/Orm/EntityRepository.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/Orm/EntityRepository.php    (revision c7e880ea107ee7f083b2ddeeaf9bae6fd4f1b5dc)
+++ src/Orm/EntityRepository.php    (date 1604482012748)
@@ -2,6 +2,10 @@

 namespace EasyCorp\Bundle\EasyAdminBundle\Orm;

+use Doctrine\DBAL\Platforms\PostgreSQL100Platform;
+use Doctrine\DBAL\Platforms\PostgreSQL91Platform;
+use Doctrine\DBAL\Platforms\PostgreSQL92Platform;
+use Doctrine\DBAL\Platforms\PostgreSQL94Platform;
 use Doctrine\ORM\QueryBuilder;
 use Doctrine\Persistence\ManagerRegistry;
 use EasyCorp\Bundle\EasyAdminBundle\Collection\FieldCollection;
@@ -25,6 +29,16 @@
     private $entityFactory;
     private $formFactory;

+    /**
+     * @var string[] PostgreSQL Platforms
+     */
+    private static $postgreSQLPlatforms = [
+        PostgreSQL91Platform::class,
+        PostgreSQL92Platform::class,
+        PostgreSQL94Platform::class,
+        PostgreSQL100Platform::class,
+    ];
+
     public function __construct(AdminContextProvider $adminContextProvider, ManagerRegistry $doctrine, EntityFactory $entityFactory, FormFactory $formFactory)
     {
         $this->adminContextProvider = $adminContextProvider;
@@ -64,6 +78,7 @@
         $isSmallIntegerQuery = ctype_digit($query) && $query >= -32768 && $query <= 32767;
         $isIntegerQuery = ctype_digit($query) && $query >= -2147483648 && $query <= 2147483647;
         $isUuidQuery = 1 === preg_match('/^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i', $query);
+        $databasePlatform = get_class($this->doctrine->getConnection()->getDatabasePlatform());

         $dqlParameters = [
             // adding '0' turns the string into a numeric value
@@ -134,6 +149,9 @@
                     ->setParameter('query_for_text', $dqlParameters['text_query']);
                 $queryBuilder->orWhere(sprintf('LOWER(%s.%s) IN (:query_as_words)', $entityName, $propertyName))
                     ->setParameter('query_as_words', $dqlParameters['words_query']);
+            } else if (!in_array($databasePlatform, self::$postgreSQLPlatforms)) {
+                $queryBuilder->orWhere(sprintf('LOWER(%s.%s) LIKE :query_for_text', $entityName, $propertyName))
+                    ->setParameter('query_for_text', $dqlParameters['text_query']);
             }
         }
     }
@@ -195,4 +213,4 @@
             ++$i;
         }
     }
-}
+}
\ No newline at end of file
grzegorz-stolarz commented 3 years ago

It's a patch ;-) I've only added checks for PostgreSQL platforms (even removed or flagged as deprecated). It will search as text.

Chris53897 commented 4 months ago

@grzegorz-stolarz I know this is an old issue. But would you be up for a PR?