Advanced filter value lost when changing default value type

[kappaj]

Environment

Sonata packages

show

``` $ composer show --latest 'sonata-project/*' sonata-project/admin-bundle 3.89.0 3.89.0 The missing Symfony Admin Generator sonata-project/block-bundle 4.5.0 4.5.0 Symfony SonataBlockBundle sonata-project/cache 2.0.1 2.1.1 Cache library sonata-project/datagrid-bundle 3.3.0 3.3.0 Symfony SonataDatagridBundle sonata-project/doctrine-extensions 1.11.0 1.11.0 Doctrine2 behavioral extensions sonata-project/doctrine-orm-admin-bundle 3.29.0 3.29.0 Integrate Doctrine ORM into the SonataAdminBundle sonata-project/exporter 2.5.1 2.5.1 Lightweight Exporter library sonata-project/form-extensions 1.9.0 1.9.0 Symfony form extensions sonata-project/twig-extensions 1.5.1 1.5.1 Sonata twig extensions sonata-project/user-bundle 4.9.0 4.11.0 Symfony SonataUserBundle ```

Symfony packages

show

``` $ composer show --latest 'symfony/*' symfony/apache-pack v1.0.1 v1.0.1 A pack for Apache support in Symfony symfony/asset v4.4.19 v5.2.3 Manages URL generation and versioning of web assets such as CSS stylesheets, JavaScript files and image files symfony/browser-kit v4.4.19 v5.2.3 Simulates the behavior of a web browser, allowing you to make requests, click on links and submit forms programm... symfony/cache v4.4.19 v5.2.3 Provides an extended PSR-6, PSR-16 (and tags) implementation symfony/cache-contracts v2.2.0 v2.2.0 Generic abstractions related to caching symfony/config v4.4.19 v5.2.3 Helps you find, load, combine, autofill and validate configuration values of any kind symfony/console v4.4.19 v5.2.3 Eases the creation of beautiful and testable command line interfaces symfony/css-selector v4.4.19 v5.2.3 Converts CSS selectors to XPath expressions symfony/debug v4.4.19 v4.4.19 Provides tools to ease debugging PHP code symfony/debug-bundle v4.4.19 v5.2.3 Provides a tight integration of the Symfony Debug component into the Symfony full-stack framework symfony/debug-pack v1.0.9 v1.0.9 A debug pack for Symfony projects symfony/dependency-injection v4.4.19 v5.2.3 Allows you to standardize and centralize the way objects are constructed in your application symfony/deprecation-contracts v2.2.0 v2.2.0 A generic function and convention to trigger deprecation notices symfony/doctrine-bridge v4.4.19 v5.2.3 Provides integration for Doctrine with various Symfony components symfony/dom-crawler v4.4.19 v5.2.3 Eases DOM navigation for HTML and XML documents symfony/dotenv v4.4.19 v5.2.3 Registers environment variables from a .env file symfony/error-handler v4.4.19 v5.2.3 Provides tools to manage errors and ease debugging PHP code symfony/event-dispatcher v4.4.19 v5.2.3 Provides tools that allow your application components to communicate with each other by dispatching events and l... symfony/event-dispatcher-contracts v1.1.9 v2.2.0 Generic abstractions related to dispatching event symfony/expression-language v4.4.19 v5.2.3 Provides an engine that can compile and evaluate expressions symfony/filesystem v4.4.19 v5.2.3 Provides basic utilities for the filesystem symfony/finder v4.4.19 v5.2.3 Finds files and directories via an intuitive fluent interface symfony/flex v1.12.1 v1.12.1 Composer plugin for Symfony symfony/form v4.4.19 v5.2.3 Allows to easily create, process and reuse HTML forms symfony/framework-bundle v4.4.19 v5.2.3 Provides a tight integration between Symfony components and the Symfony full-stack framework symfony/http-client-contracts v2.3.1 v2.3.1 Generic abstractions related to HTTP clients symfony/http-foundation v4.4.19 v5.2.3 Defines an object-oriented layer for the HTTP specification symfony/http-kernel v4.4.19 v5.2.3 Provides a structured process for converting a Request into a Response symfony/inflector v4.4.19 v5.2.3 Converts words between their singular and plural forms (English only) symfony/intl v4.4.19 v5.2.3 Provides a PHP replacement layer for the C intl extension that includes additional data from the ICU library symfony/maker-bundle v1.29.1 v1.29.1 Symfony Maker helps you create empty commands, controllers, form classes, tests and more so you can forget about... symfony/mime v4.4.19 v5.2.3 Allows manipulating MIME messages symfony/monolog-bridge v4.4.19 v5.2.3 Provides integration for Monolog with various Symfony components symfony/monolog-bundle v3.6.0 v3.6.0 Symfony MonologBundle symfony/options-resolver v4.4.19 v5.2.3 Provides an improved replacement for the array_replace PHP function symfony/orm-pack v1.2.0 v2.1.0 A pack for the Doctrine ORM symfony/phpunit-bridge v5.2.3 v5.2.3 Provides utilities for PHPUnit, especially user deprecation notices management symfony/polyfill-intl-grapheme v1.22.1 v1.22.1 Symfony polyfill for intl's grapheme_* functions symfony/polyfill-intl-icu v1.22.1 v1.22.1 Symfony polyfill for intl's ICU-related data and classes symfony/polyfill-intl-idn v1.22.1 v1.22.1 Symfony polyfill for intl's idn_to_ascii and idn_to_utf8 functions symfony/polyfill-intl-normalizer v1.22.1 v1.22.1 Symfony polyfill for intl's Normalizer class and related functions symfony/polyfill-mbstring v1.22.1 v1.22.1 Symfony polyfill for the Mbstring extension symfony/polyfill-php72 v1.22.1 v1.22.1 Symfony polyfill backporting some PHP 7.2+ features to lower PHP versions symfony/polyfill-php73 v1.22.1 v1.22.1 Symfony polyfill backporting some PHP 7.3+ features to lower PHP versions symfony/polyfill-php80 v1.22.1 v1.22.1 Symfony polyfill backporting some PHP 8.0+ features to lower PHP versions symfony/profiler-pack v1.0.5 v1.0.5 A pack for the Symfony web profiler symfony/property-access v4.4.19 v5.2.3 Provides functions to read and write from/to an object or array using a simple string notation symfony/property-info v4.4.19 v5.2.3 Extracts information about PHP class' properties using metadata of popular sources symfony/routing v4.4.19 v5.2.3 Maps an HTTP request to a set of configuration variables symfony/security-acl v3.1.1 v3.1.1 Symfony Security Component - ACL (Access Control List) symfony/security-bundle v4.4.19 v5.2.3 Provides a tight integration of the Security component into the Symfony full-stack framework symfony/security-core v4.4.19 v5.2.3 Symfony Security Component - Core Library symfony/security-csrf v4.4.19 v5.2.3 Symfony Security Component - CSRF Library symfony/security-guard v4.4.19 v5.2.3 Symfony Security Component - Guard symfony/security-http v4.4.19 v5.2.3 Symfony Security Component - HTTP Integration symfony/serializer v4.4.19 v5.2.3 Handles serializing and deserializing data structures, including object graphs, into array structures or other f... symfony/service-contracts v2.2.0 v2.2.0 Generic abstractions related to writing services symfony/stopwatch v4.4.19 v5.2.3 Provides a way to profile code symfony/string v5.2.3 v5.2.3 Provides an object-oriented API to strings and deals with bytes, UTF-8 code points and grapheme clusters in a un... symfony/swiftmailer-bundle v3.5.2 v3.5.2 Symfony SwiftmailerBundle symfony/templating v4.4.19 v5.2.3 Provides all the tools needed to build any kind of template system symfony/translation v4.4.19 v5.2.3 Provides tools to internationalize your application symfony/translation-contracts v2.3.0 v2.3.0 Generic abstractions related to translation symfony/twig-bridge v4.4.19 v5.2.3 Provides integration for Twig with various Symfony components symfony/twig-bundle v4.4.19 v5.2.3 Provides a tight integration of Twig into the Symfony full-stack framework symfony/validator v4.4.19 v5.2.3 Provides tools to validate values symfony/var-dumper v4.4.19 v5.2.3 Provides mechanisms for walking through any arbitrary PHP variable symfony/var-exporter v4.4.19 v5.2.3 Allows exporting any serializable PHP data structure to plain PHP code symfony/web-profiler-bundle v4.4.19 v5.2.3 Provides a development tool that gives detailed information about the execution of any request symfony/webpack-encore-bundle v1.11.0 v1.11.0 Integration with your Symfony app & Webpack Encore! symfony/yaml v4.4.19 v5.2.3 Loads and dumps YAML files ```

PHP version

$ php -v
PHP 7.4.10 (cli) (built: Sep  1 2020 16:52:21) ( NTS Visual C++ 2017 x64 )
Copyright (c) The PHP Group
Zend Engine v3.4.0, Copyright (c) Zend Technologies
    with Zend OPcache v7.4.10, Copyright (c), by Zend Technologies

Subject

I have a admin list view with a default filter with activated advanced filter.

Here is my code to define the filters and default filter:

show

``` protected function configureDatagridFilters(DatagridMapper $datagridMapper) { $datagridMapper ->add('chatState', 'doctrine_orm_choice', [ 'global_search' => true, 'field_type' => ChoiceType::class, 'field_options' => [ 'choices' => ChatStates::getChoices() ] ] ); } protected function configureDefaultFilterValues(array &$filterValues) { $filterValues['chatState'] = [ 'type' => EqualOperatorType::TYPE_NOT_EQUAL, 'value' => 'archived', ]; } ```

When I change the filter type to 'equal' and press the 'Filter' the list should filter all items with chatState = 'archived'

Tested with sonata-admin verion 3.89. Have to step back to <3.86 to get old filter behavior.

Minimal repository with the bug

Steps to reproduce

Expected results

• List is filtered with chatState = 'archived'
• url params should look like this: list?filter[chatState][type]=1&filter[chatState][value]=archived

Actual results

• List is empty
• url params are: list?filter[chatState][type]=1 -> value is missing

[kirya-dev]

Hello! Im not agree with expected results. 1) You define this:

      $filterValues['chatState'] = [
            'type'  => EqualOperatorType::TYPE_NOT_EQUAL,
            'value' => 'archived',
        ];

But expects equals condition. 2) Default filters has already in your source code. If we pass all params into url its make url is too long.

[kappaj]

Hi kirye-dev. I am not sure if I explained it right. The default filter works fine. I open the list view first time and the list is filtered correctly: chatState != archived. But then I change the type to Equal in the frontend (to filter it chatState = archived) and press Filter. And then the unexpected result occurs. It looks like the value "archived" (defined in the default filter) is empty when I "overwrite" the type via get params.

[kirya-dev]

Hm. Its incorrect usage!

You must: 1) Use in admin $this->getFilterParameters() instead directly get query data from request 2) Or use $data['value'] in https://sonata-project.org/bundles/doctrine-orm-admin/master/doc/reference/filter_field_definition.html#callback in configuration of chatState datagrid filter

[kappaj]

1. Where should I use $this->getFilterParameters()? I don't think I get query data from request somewhere directly. Isn't it the default usage to implement configureDatagridFilters and configureDefaultFilterValues and sonata grep the query params internally?
2. So I have to use always a callback? Why does it work before?

[VincentLanglet]

@kirya-dev I confirm the bug

$filterValues['usagename'] = [
            'type' => ContainsOperatorType::TYPE_CONTAINS,
            'value' => 'a',
        ];

Is a valid way to declare a default filter value.

With this I get

If I change to

And I submit, you can notice that the a is disabled.

So the page reload with only the type loosing the value.

And, If I want to filter back on the default value, I can't. If I write

When I submit it's automatically disabled

So I still get the previous filter

[VincentLanglet]

Same happen for a filter without default value, If I use it

And then I want to remove the value

When I submit it's disabled

So on the page reload, I still have the value

For this reason, if I try to remove the default filter on usagename

On submit fields are disabled, so on page reload, it automatically come back

[kappaj]

I think I have it.

Line 712 in Sonata\AdminBundle\Admin\AbstractAdmin:

$parameters = array_merge($parameters, $filters);

But I think it must be:

$parameters = array_merge_recursive($parameters, $filters);

Otherwise the $filters array overwrite the property chatState.

[VincentLanglet]

To me, it's more the following criteria which is wrong

// Compare default and submitted filter values in `keyValue` representation. (`keyValue` ex: "filter[publish][value][end]=2020-12-12")
// Only allow to submit non default and non empty values, because empty values means they are not present.
var changedValues = submittedValues.filter(function (keyValue) {
    return defaultValues.indexOf(keyValue) === -1 && keyValue.split('=')[1] !== '';
});

We have to compare the value vs the default value

[kirya-dev]

$parameters = array_merge_recursive($parameters, $filters);

Hm. Current and your sudgest code dident worked correctly:

$data = ['id' => ['type' => 1, 'value' => 'foo']];
$data2 =  ['id' => ['type' => 2]]; // from request

var_dump(
    array_merge($data, $data2),
    array_merge_recursive($data, $data2)
);

output

array(1) {
  ["id"]=>
  array(1) {
    ["type"]=>
    int(2)
  }
}
array(1) {
  ["id"]=>
  array(2) {
    ["type"]=>
    array(2) {
      [0]=>
      int(1)
      [1]=>
      int(2)
    }
    ["value"]=>
    string(3) "foo"
  }
}

[VincentLanglet]

For the js, something like this should be done:

var defaultValues = $.param({'filter': JSON.parse(this.dataset.defaultValues)}).split('&'),
                submittedValues = $form.serialize().split('&');

            var defaultValuesByKey = [];
            defaultValues.forEach(function (keyValue) {
                defaultValuesByKey[keyValue.split('=')[0]] = keyValue.split('=')[1];
            })

            // Compare default and submitted filter values in `keyValue` representation. (`keyValue` ex: "filter[publish][value][end]=2020-12-12")
            // Only allow to submit non default and non empty values, because empty values means they are not present.
            // An empty value should be submitted if the filter has a default value, in order to override it
            var changedValues = submittedValues.filter(function (keyValue) {
                if (defaultValuesByKey[keyValue.split('=')[0]]) {
                    return defaultValuesByKey[keyValue.split('=')[0]] !== keyValue.split('=')[1]
                } else {
                    return keyValue.split('=')[1] !== '';
                }
            });

[VincentLanglet]

With both the JS modified and the array_merge modified I think the issue will be solved.

[kirya-dev]

No @VincentLanglet. Problem in backend only. Fronted works correctly.

Its can be covered by phpunit

[kappaj]

I got a right result with array_replace_recursive

            dump($parameters);
            dump($filters);
            $parameters = array_replace_recursive($parameters, $filters);
            dd($parameters);

[kirya-dev]

Your right. Im test array_merge_recursive

[kirya-dev]

But arrays values not correctly will merged with replace

[VincentLanglet]

No @VincentLanglet. Problem in backend only. Fronted works correctly.

I'm sorry, but I took one hour to test every possible cases, I take screenshot and I provide a solution. So yes, the problem is both backend AND frontend.

Because you disable the empty value you cannot remove a default filter. You have to disable the empty value ONLY IF it's not a filter with a default value.

Your right. Im test array_merge_recursive

array_merge_recursive works for the case provided.

But If I change the value from a to b, I will end with an array ['a', 'b'] so it won't work.

[kirya-dev]

Im says in another words: Need submit only CHANGED filters. Empty check needed for that didnt present in defaultValues

[kappaj]

But arrays values not correctly will merged with replace

Do you have en example?

[VincentLanglet]

Im says in another words: Need submit only CHANGED filters. Empty check needed for that didnt present in defaultValues

And that's not what's done by the current JS.

var changedValues = submittedValues.filter(function (keyValue) {
    return defaultValues.indexOf(keyValue) === -1 && keyValue.split('=')[1] !== '';
});

If my default value is usagename=a, and I submit an empty usagename, it will submit usagename=. So defaultValues.indexOf(keyValue) will be -1 and keyValue.split('=')[1] will be ''. Your check will remove the empty value I try to submit in order to remove the default filter.

It's because you looking for usagename= in an array containing usagename=a, when you should look for the key usagename only.

[willemverspyck]

Do you have en example?

@kappaj: For example a ChoiceType when you use "multiple" => "true":

$parameters:

^ array:5 [▼
  "_sort_order" => "DESC"
  "_sort_by" => "id"
  "status" => array:1 [▼
    "value" => array:3 [▼
      0 => "ACCEPT"
      1 => "DECLINE"
      2 => "REVIEW"
    ]
  ]
]

$filters:

^ array:3 [▼
  "status" => array:1 [▼
    "value" => array:3 [▼
      0 => "ACCEPT"
      1 => "DECLINE"
    ]
  ]
  "_page" => 1
  "_per_page" => 25
]

With "array_replace_recursive" it will be:

^ array:7 [▼
  "_sort_order" => "DESC"
  "_sort_by" => "id"
  "status" => array:1 [▼
    "value" => array:3 [▼
      0 => "ACCEPT"
      1 => "DECLINE"
      2 => "REVIEW"
    ]
  ]
  "_page" => 1
  "_per_page" => 25
]

But it must be:

^ array:7 [▼
  "_sort_order" => "DESC"
  "_sort_by" => "id"
  "status" => array:1 [▼
    "value" => array:3 [▼
      0 => "ACCEPT"
      1 => "DECLINE"
    ]
  ]
  "_page" => 1
  "_per_page" => 25
]

I think there is no PHP function for this, because on the third depth of the array you don't want to use "recursive" anymore. You have to use something like this:

function mergeParameters(array $parameters, array $filters): array {
    $data = [];

    foreach ($parameters as $key => $parameter) {
        $replace = array_key_exists($key, $filters) ? $filters[$key] : $parameter;

        if (is_array($parameter)) {
            $data[$key] = array_replace($parameter, $replace);
        } else {
            $data[$key] = $replace;
        }
    }

    return $data;
}

$parameters = mergeParameter($parameters, $filters);

I can make PR when #6871 is merged. It will be good to test this "mergeParameters" method to define all different situations.

[VincentLanglet]

I can make PR when #6871 is merged. It will be good to test this "mergeParameters" method to define all different situations.

You can also make a PR on the PR#6871. It will avoid us to merge a non-finished PR. Indeed, adding test could be great.

[kirya-dev]

@willemverspyck your example incorrect: any data from filters will be ignored

[willemverspyck]

You're correct @kirya-dev. I was to fast :-)

But I will make an PR on the PR#6871 with some tests.

[willemverspyck]

Sorry, was struggling with the CS tests. But the PR with tests are ready!