Multiple choice field with predefined default values

pavelkameisha commented 3 years ago

Environment

Sonata packages

sonata-project/admin-bundle              3.105.3 4.0.1  The missing Symfony Admin Generator
sonata-project/block-bundle              3.21.0  4.7.0  Symfony SonataBlockBundle
sonata-project/cache                     2.0.1   2.2.0  Cache library
sonata-project/doctrine-extensions       1.13.1  1.14.0 Doctrine2 behavioral extensions
sonata-project/doctrine-orm-admin-bundle 3.35.0  4.0.0  Integrate Doctrine ORM into the SonataAdminBundle
sonata-project/exporter                  2.7.0   2.8.0  Lightweight Exporter library
sonata-project/form-extensions           1.12.0  1.12.0 Symfony form extensions
sonata-project/twig-extensions           1.7.0   1.9.0  Sonata twig extensions

Symfony packages

symfony/contracts                v1.1.10 v2.4.0  A set of abstractions extracted out of the Symfony components
symfony/deprecation-contracts    v2.4.0  v2.4.0  A generic function and convention to trigger deprecation notices
symfony/monolog-bundle           v3.6.0  v3.7.0  Symfony MonologBundle
symfony/phpunit-bridge           v5.3.3  v5.3.8  Provides utilities for PHPUnit, especially user deprecation notices management
symfony/polyfill-ctype           v1.23.0 v1.23.0 Symfony polyfill for ctype functions
symfony/polyfill-iconv           v1.23.0 v1.23.0 Symfony polyfill for the Iconv extension
symfony/polyfill-intl-grapheme   v1.23.1 v1.23.1 Symfony polyfill for intl's grapheme_* functions
symfony/polyfill-intl-icu        v1.23.0 v1.23.0 Symfony polyfill for intl's ICU-related data and classes
symfony/polyfill-intl-idn        v1.23.0 v1.23.0 Symfony polyfill for intl's idn_to_ascii and idn_to_utf8 functions
symfony/polyfill-intl-normalizer v1.23.0 v1.23.0 Symfony polyfill for intl's Normalizer class and related functions
symfony/polyfill-mbstring        v1.23.1 v1.23.1 Symfony polyfill for the Mbstring extension
symfony/polyfill-php72           v1.23.0 v1.23.0 Symfony polyfill backporting some PHP 7.2+ features to lower PHP versions
symfony/polyfill-php73           v1.23.0 v1.23.0 Symfony polyfill backporting some PHP 7.3+ features to lower PHP versions
symfony/polyfill-php80           v1.23.1 v1.23.1 Symfony polyfill backporting some PHP 8.0+ features to lower PHP versions
symfony/polyfill-php81           v1.23.0 v1.23.0 Symfony polyfill backporting some PHP 8.1+ features to lower PHP versions
symfony/security-acl             v3.1.1  v3.2.0  Symfony Security Component - ACL (Access Control List)
symfony/string                   v5.3.3  v5.3.7  Provides an object-oriented API to strings and deals with bytes, UTF-8 code points and grapheme clusters in a unified way
symfony/swiftmailer-bundle       v3.5.2  v3.5.2  Symfony SwiftmailerBundle
symfony/symfony                  v4.4.26 v5.3.9  The Symfony PHP framework
symfony/test-pack                v1.0.8  v1.0.9  A pack for functional and end-to-end testing within a Symfony app

PHP version

PHP 7.4.22 (cli) (built: Jul 30 2021 13:08:17) ( NTS )
Copyright (c) The PHP Group
Zend Engine v3.4.0, Copyright (c) Zend Technologies
    with Xdebug v3.0.4, Copyright (c) 2002-2021, by Derick Rethans
    with Zend OPcache v7.4.22, Copyright (c), by Zend Technologies

Subject

I have caught a bug in version 3.90+ I have a filter field with multiple choice and default values. Since 3.90 I can't just clear this field, because after reloading page I get filled field with default values.

Steps to reproduce

protected function configureDatagridFilters(DatagridMapper $filter): void
{
    $filter->add(
                'orderType',
                'doctrine_orm_choice',
                [],
                ChoiceType::class,
                [
                    'multiple' => true,
                    'choices'  => Order::getOrderTypesListForChoices(),
                ]
            );
}

protected function configureDefaultFilterValues(array &$filterValues): void
    {
        $filterValues['orderType'] = ['type' => '', 'value' => [Order::TYPE_ONE, Order::TYPE_TWO]];
    }

Expected results

After submitting the filter with empty orderType field I get all available orders.

Actual results

After submitting the filter with empty orderType field I get orders filtered by default values. And orderType field is filled.

Notes

Maybe this row causes the error:

$parameters = ParametersManipulator::merge($parameters, $filters);

VincentLanglet commented 2 years ago

Since we tried to reduce the url to only add submitted filter a lot of changes were made. https://github.com/sonata-project/SonataAdminBundle/pull/6895 https://github.com/sonata-project/SonataAdminBundle/pull/6871 https://github.com/sonata-project/SonataAdminBundle/pull/7254

Maybe we forgot one case, cc @kirya-dev

But if you think it's related to 3.90 version and https://github.com/sonata-project/SonataAdminBundle/pull/6877, I'll ping @willemverspyck too.

VincentLanglet commented 2 years ago

After some tries, this is what happened:

Prior to all the filters changes, this was the query parameters:

In this example, operationOrigin is a choice with multiple values. As you can see the value is not submitted since it's an empty array. But since the type was submitted, the key was here and so we still override the default values for operationOrigin when doing the array_merge.

Then @kirya-dev remove all the filters with default value from the url/request. This means that the type is not submitted anymore. Since both the value (because an empty value) and the type (because it's a default value) are not submitted, the array_merge doesn't override anymore the default value.

I tried to keep the type submitted when the value was an empty array. But with the introduction of ParametersManipulator::merge it's not enough to fix the issue. So it would require to modify this merge logic too.

But I just thought that this would say that the bug was already here for filter without a type field (and my fix won't be able to fix it).

I see two solutions so far

Find a hacky way to submit a value for an empty array this way we'll have a value in the request to override the default one.
Only remove the non-array fields from the url with .find('[name*=filter][name$="[]"]'), and change the php logic for the merge: If the value is an array, we don't keep the old one since it's supposed to always be submitted.

Any better idea ? @kirya-dev @sonata-project/contributors ?

kirya-dev commented 2 years ago

Hi guys! I think problem in understadning represantation parameters in HTTP uri. When you are submitting form field (multiple) and no selected options browser dont present this params in uri. In my small opinion in this case send reserved value that none selected. (Convert empty array to empty string?)

Its solves by JS. But JS disabled problem will saved.

For checkbox best practie is add empty form element with same name.

PS.This problem was before deleting non default params..

sonata-project / SonataAdminBundle