sonata-project / SonataAdminBundle

The missing Symfony Admin Generator
https://docs.sonata-project.org/projects/SonataAdminBundle
MIT License
2.11k stars 1.26k forks source link

hasDisplayableFilters checks not equivalent in PHP and Twig files (Because of isDefaultFilter) #6641

Closed Kalyse closed 3 years ago

Kalyse commented 3 years ago

Environment

Sonata packages

sonata-project/admin-bundle              3.68.0 3.81.1 The missing Symfony A...
sonata-project/block-bundle              3.19.0 4.4.0  Symfony SonataBlockBu...
sonata-project/cache                     2.0.1  2.0.1  Cache library
sonata-project/classification-bundle     3.11.1 3.14.0 Symfony SonataClassif...
sonata-project/core-bundle               3.19.2 3.20.0 Symfony SonataCoreBun...
Package sonata-project/core-bundle is abandoned, you should avoid using it. No replacement was suggested.
sonata-project/datagrid-bundle           2.5.0  3.2.0  Symfony SonataDatagri...
sonata-project/doctrine-extensions       1.9.1  1.10.1 Doctrine2 behavioral ...
sonata-project/doctrine-orm-admin-bundle 3.9.0  3.26.0 Symfony Sonata / Inte...
sonata-project/easy-extends-bundle       2.5.0  2.5.0  Symfony SonataEasyExt...
sonata-project/exporter                  2.4.1  2.4.1  Lightweight Exporter ...
sonata-project/intl-bundle               2.9.0  2.9.0  Symfony SonataIntlBundle
sonata-project/media-bundle              3.24.0 3.29.0 Symfony SonataMediaBu...
sonata-project/user-bundle               4.5.3  4.10.1 Symfony SonataUserBundle

Subject

When you view an Admin, if that Admin has filters, there are two condition checks made which are performed to see if the display should render any filters. Where an Admin has a Default Filter, Twig checks for this, but the Admin code does not.

The first is in Datagrid.php https://github.com/sonata-project/SonataAdminBundle/blob/3.x/src/Datagrid/Datagrid.php#L232 Then in the twig files we have https://github.com/sonata-project/SonataAdminBundle/blob/3.x/src/Resources/views/CRUD/base_list.html.twig#L283 and https://github.com/sonata-project/SonataAdminBundle/blob/3.x/src/Resources/views/CRUD/base_list.html.twig#L293

The issue however, is that the method hasDisplayableFilters returns true, when the inner condition checks return false. They are checking for different things.

The end result means that you end up with a data grid with no filters showing, but the container box instead. For example, my application shows:

Selection_284

So, the Admin.php can be boiled down too...

if( (A && !B) OR B );

Whereas, the Twig conditions, it's performing

if( (A && !B) OR B) AND NOT C);

Code:

$showFilter = $filter->getOption('show_filter', null);
if (($filter->isActive() && null === $showFilter) || (true === $showFilter)) {

Code:

{% set filterActive = ((filter.isActive() and filter.options['show_filter'] is null) or (filter.options['show_filter'] is same as(true))) and not admin.isDefaultFilter(filter.formName) %}

In essence, the twig filter, is checking for an addition constraint by also requiring the filter to not be a default filter.

Expected results

I'm unsure what the correct expected results here should be for Sonata. The options are either do not show default filters, or do show default filters, but both the Admin and the Twig templates need to agree which is the correct approach.

If you would like default filters to show, then you should remove the

and not admin.isDefaultFilter(filter.formName)
Kalyse commented 3 years ago

Just checked the latest branch, and this issue has already been fixed. Closing.