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

Using a ChoiceType (or EnumType) with multiple / expanded set to true as a filter breaks URL generation #151

Open
Tracked by #95
j0r1s opened this issue Nov 28, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@j0r1s
Copy link

j0r1s commented Nov 28, 2024

Hello Kreyu,

First of all, thank you for this amazing bundle! I've been using and following it for months, and it has been incredibly helpful.

However, I’ve encountered an issue when trying to implement a ChoiceType or EnumType filter with multiple: true and expanded: true. This setup seems to break URL generation in the DataTable. Specifically, every action within the DataTable appends all filter choices to the query, regardless of whether any are selected.

Here’s what I’ve done:

  1. Overriding the filter_clear_button block:
    To avoid translation errors when the filter value is an array, I had to override the filter_clear_button block as follows:
{% block filter_clear_button %}
    ...
    <span class="">{{ filter.vars.value is iterable ? filter.vars.value|map(value => value|trans)|join(', ') : filter.vars.value|trans({}, filter.vars.translation_domain) }}</span>
    ...
{% endblock %}
  1. Defining the filter:
    I configured the filter with these options:
->addFilter('color', StringFilterType::class, [
    'label' => 'Color',
    'form_type' => EnumType::class,
    'form_options' => [
        'class' => ColorEnum::class,
        'multiple' => true,
        'expanded' => true,
    ],
    'default_operator' => Operator::In,
])
  1. Observing the issue:
    Any interaction within the DataTable (e.g., sorting, pagination) results in all filter choices being appended to the query, even if none were selected.

To help debug this issue, I’ve created a small reproducer repository: https://github.com/j0r1s/datatable-choice-filter-multiple.

I’ve tried to investigate the code to identify the root cause but haven’t been able to pinpoint it yet. I’d be happy to assist further if I can!

Thank you in advance for looking into this.

@j0r1s
Copy link
Author

j0r1s commented Nov 28, 2024

I had some time to investigate the issue, and it seems the culprit is the FormUtil::getFormViewValueRecursive method. When the expanded and multiple options are both set to true, the form becomes compound, and this method iterates over the children of the filter.

As a result, all checkbox values are added to the query parameters, regardless of whether they are checked or not.

To address this, I added a small check inside the method to omit unchecked checkbox values. This appears to resolve the issue:

class FormUtil
{
    public static function getFormViewValueRecursive(FormView $view): mixed
    {
        $value = $view->vars['value'];
        
        if (!empty($view->children)) {
            $value = [];

            foreach ($view->children as $child) {
                $childValue = static::getFormViewValueRecursive($child);
                
                // Skip unchecked checkboxes for expanded and multiple options
                if ($view->vars['expanded'] && $view->vars['multiple'] && isset($child->vars['checked']) && !$child->vars['checked']) {
                    continue;
                }

                $value[$child->vars['name']] = $childValue;
            }
        }

        return $value;
    }
}

Would you be interested in me creating a PR with this fix? I believe the filter_clear_button block would also need the is iterable check to fully support filters with multiple values.

Additionally, would you be open to a PR implementing ChoiceFilterType and EnumFilterType? I think these could be valuable additions to the core of the bundle.

@Kreyu
Copy link
Owner

Kreyu commented Nov 29, 2024

Hey @j0r1s, thank you for your kind words and time! Great explanation of the issue, and reproducer repository.

That is an interesting case I haven't tested before. I wonder whether there's other way to handle this situation without relying on checking the type-specific variables in its view (such as expanded, multiple, etc.). I'll take a look into it.

Additionally, would you be open to a PR implementing ChoiceFilterType and EnumFilterType? I think these could be valuable additions to the core of the bundle.

Sure!

@maciazek
Copy link

Hi!
I have tried @j0r1s check in FormUtil, but it looks like it's not enough to make multiple filters usable.

  • When multiple filter is removed (by click), then filter form has an error (it's visible in profiler). When using multiple filter with other filters at the same time, then removing multiple filter results in all filters being removed.
  • When multiple filter is configured but not used, then using other filters breaks table functionality and no data is displayed.
  • Also exporting with multiple filter results in filter form error, so filters are not applied to export at all.

On a positive note - pagination and sorting works fine.

@Kreyu
Copy link
Owner

Kreyu commented Jan 2, 2025

Okay so, initially I assumed that we can retrieve value of each form field from its view object. This is because we have to convert FormView into query parameters to automatically append them on render. Thanks to this case I see this method will not suffice. I'll think of other ways to handle this.

@Kreyu Kreyu added the bug Something isn't working label Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants