Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CreateQuery and searchAction have conflict #5569

Closed
VincentLanglet opened this issue Jun 3, 2019 · 9 comments · Fixed by sonata-project/SonataDoctrineORMAdminBundle#1358
Closed
Labels

Comments

@VincentLanglet
Copy link
Member

VincentLanglet commented Jun 3, 2019

Environment

Sonata packages

sonata-project/admin-bundle              3.40.3 3.49.0 The missing Symfony Admin Generator
sonata-project/block-bundle              3.15.0 3.15.0 Symfony SonataBlockBundle
sonata-project/cache                     2.0.1  2.0.1  Cache library
sonata-project/core-bundle               3.17.0 3.17.0 Symfony SonataCoreBundle
sonata-project/datagrid-bundle           2.5.0  2.5.0  Symfony SonataDatagridBundle
sonata-project/doctrine-extensions       1.3.0  1.3.0  Doctrine2 behavioral extensions
sonata-project/doctrine-orm-admin-bundle 3.6.3  3.9.0  Symfony Sonata / Integrate Doctrine ORM into the SonataAdminBundle
sonata-project/exporter                  1.11.1 1.11.1 Lightweight Exporter library

Symfony packages

symfony/monolog-bundle     v3.1.2  v3.3.1  Symfony MonologBundle
symfony/phpunit-bridge     v4.2.9  v4.3.0  Symfony PHPUnit Bridge
symfony/polyfill-apcu      v1.2.0  v1.11.0 Symfony polyfill backporting apcu_* functions to lower PHP versions
symfony/polyfill-ctype     v1.11.0 v1.11.0 Symfony polyfill for ctype functions
symfony/polyfill-iconv     v1.11.0 v1.11.0 Symfony polyfill for the Iconv extension
symfony/polyfill-intl-icu  v1.11.0 v1.11.0 Symfony polyfill for intl's ICU-related data and classes
symfony/polyfill-intl-idn  v1.11.0 v1.11.0 Symfony polyfill for intl's idn_to_ascii and idn_to_utf8 functions
symfony/polyfill-mbstring  v1.11.0 v1.11.0 Symfony polyfill for the Mbstring extension
symfony/polyfill-php56     v1.11.0 v1.11.0 Symfony polyfill backporting some PHP 5.6+ features to lower PHP versions
symfony/polyfill-php70     v1.11.0 v1.11.0 Symfony polyfill backporting some PHP 7.0+ features to lower PHP versions
symfony/polyfill-php72     v1.11.0 v1.11.0 Symfony polyfill backporting some PHP 7.2+ features to lower PHP versions
symfony/polyfill-util      v1.11.0 v1.11.0 Symfony utilities for portability of PHP codes
symfony/security-acl       v3.0.2  v3.0.2  Symfony Security Component - ACL (Access Control List)
symfony/swiftmailer-bundle v3.2.6  v3.2.7  Symfony SwiftmailerBundle
symfony/symfony            v3.4.28 v4.3.0  The Symfony PHP framework

PHP version

PHP 7.2.17-1+0~20190412071344.20+stretch~1.gbp23a36d (cli) (built: Apr 12 2019 07:13:45) ( NTS )
Copyright (c) 1997-2018 The PHP Group
Zend Engine v3.2.0, Copyright (c) 1998-2018 Zend Technologies
    with Zend OPcache v7.2.17-1+0~20190412071344.20+stretch~1.gbp23a36d, Copyright (c) 1999-2018, by Zend Technologies
    with Xdebug v2.6.0, Copyright (c) 2002-2018, by Derick Rethans

Subject

I use the createQuery function to limiting entities in the list (here). I recently used the sonata search bar and I was expected the createQuery used in the search to filter the possible results found. But the way the createQuery just makes the search bar useless. Instead of looking for my research and then filtering with the createQuery, the search action is looking for all entities matching with my research OR the createQuery, so it returns all the entities I can see in the list...

Steps to reproduce

  1. Create an Admin without createQuery
  2. Use the search bar and search for fooBarThingDoesntExist
  3. Get no result
  4. Add this is the createQuery
    public function createQuery($context = 'list')
    {
        $query = parent::createQuery($context);

        $o = $query->getRootAliases()[0];
        $query->andWhere("$o.id IS NOT NULL");

        return $query;
    }
  1. Use the search bar and search for fooBarThingDoesntExist
  2. Get a lot of result

Expected results

The request used in the search action should be

SELECT 
  count(DISTINCT a0_.id) AS sclr_0 
FROM 
  asv_client a0_ 
WHERE 
  (
    a0_.id IS NOT NULL 
    AND (
        a0_.firstname LIKE ?
        OR a0_.usagename LIKE ?
    )
  ) 
  AND a0_.type IN ('client') 
LIMIT 
  25

Actual results

In fact the request is the following, so the search return all possible result in the list...

SELECT 
  count(DISTINCT a0_.id) AS sclr_0 
FROM 
  asv_client a0_ 
WHERE 
  (
    a0_.id IS NOT NULL 
    OR a0_.firstname LIKE ?
    OR a0_.usagename LIKE ?
  ) 
  AND a0_.type IN ('client') 
LIMIT 
  25
@VincentLanglet
Copy link
Member Author

VincentLanglet commented Jun 5, 2019

Can someone confirm this ? Is it a real bug or am I doing something wrong ? We need to implement the searchAction at our company and we wonder if we will have to reimplement the page.

Thanks for the help.

@phansys
Copy link
Member

phansys commented Jun 5, 2019

I can't reproduce the issue. The conditions added at createQuery() method are taken into account when performing the search. Could you please create a small reproducer or even better a test covering what are you reporting?

@VincentLanglet
Copy link
Member Author

Hi @phansys !

I made this : https://github.com/VincentLanglet/sonataBug
You'll need a database, mine was mysql -u root -proot. Then you install the composer dependencies composer install, run the migration with bin/console doctrine:migration:migrate and start the project with bin/console server:run.

Then create some entities
image

If you search for Vincent you got too much results
image

Remove the createQuery and it works
image

@phansys
Copy link
Member

phansys commented Jun 6, 2019

Have you tested the behavior updating the query to something more explicit?
Mean:

$query
    ->andWhere($query->expr()->like("$o.firstName", ':firstName'))
    ->setParameter('firstName', '%Jean%');

Also, please take into account that the query you've shared is referencing a column named is in the table asv_client, while the query builder snippet is referencing the property id. Is that right?

@VincentLanglet
Copy link
Member Author

@phansys I made a typo in the query shared. My table was asv_client and I tried with several different query. When I wrote this issue, I wanted to provide a more simple case so I gave the id is not null case.

I tried your example

$query
    ->andWhere($query->expr()->like("$o.firstName", ':firstName'))
    ->setParameter('firstName', '%Jean%');

This is the result
image
image

And the list is filtered
image

@phansys
Copy link
Member

phansys commented Jun 9, 2019

After some debug, I can confirm what you are experiencing.
I think the problem is that the search ends applying the filters using QueryBuilder::orWhere() without wrapping these clauses with Expr::andX():

$filter->setCondition(FilterInterface::CONDITION_OR);

$filter->apply($this->query, $data[$filterFormName]);

https://github.com/sonata-project/SonataDoctrineORMAdminBundle/blob/df5c97b9bd3793e7baf0a1ba19d92eab2e90242d/src/Filter/Filter.php#L56

By now, the only approach I can think to solve this issue is defining the grouping "AND" expression at the datagrid and passing it to each filter in order to stack the "OR" clauses inside the same group, but I don't know how to achieve this without changing FilterInterface.

@VincentLanglet
Copy link
Member Author

@phansys I agree with your analysis.

How would you modify the FilterInterface ?
Since, I think, this bug won't be fixed before some time, is there a way to override some properties/classes/... in order to fix this in my project ?

Thanks

@phansys
Copy link
Member

phansys commented Jun 23, 2019

This issue is a duplicate of #3368.

@VincentLanglet
Copy link
Member Author

Indeed !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment