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

[Bug]: Invalid Query Param Values on MultiSelectFilter cause error #2033

Closed
leesherwood opened this issue Nov 9, 2024 · 7 comments
Closed
Labels
bug Something isn't working

Comments

@leesherwood
Copy link

What happened?

TypeError is thrown if you pass an invalid value in the query parameter of a MultiSelectFilter

htmlspecialchars(): Argument #1 ($string) must be of type string, array given

How to reproduce the bug

Create a table with any MultiSelectFilter, such as:

MultiSelectFilter::make('Country')
                ->options(['uk' => 'United Kingdom', 'fr' => 'France', 'ge' => 'Germany'])

Enable the filter selecting only ONE value, then modify the URL query string to have an invalid filter value:

http://localhost/admin/testpage?table-filters[country][0]=no

Hit enter and the error will appear

Package Version

3.5.0

PHP Version

8.1.x

Laravel Version

11.10.0

Alpine Version

No response

Theme

Tailwind 3.x

Notes

When the filter values are passed through this function: https://github.com/rappasoft/laravel-livewire-tables/blob/master/src/Views/Filters/MultiSelectFilter.php#L37 , it returns an empty array as it correctly discards our edited invalid value.

This empty array is then output here: https://github.com/rappasoft/laravel-livewire-tables/blob/master/resources/views/components/tools/filter-pills.blade.php#L42

@if(is_array($filterPillValue) && !empty($filterPillValue))
    @foreach($filterPillValue as $filterPillArrayValue)
        {{ $filterPillArrayValue }}{!! $separator !!}
    @endforeach
@else
    {{ $filterPillValue }}
@endif

The first condition is false, so it drops into the else which is then running {{ [] }} or <?php echo e([]); ?>


The easiest fix is to remove the && !empty($filterPillValue) on the first condition. It will show the filter applied with no values, but better than an error and not entirely an unexpected result (I think this would be fine).

A "better" fix might be to disable the filter if there's no valid value post-sanitisation.

Error Message

No response

@leesherwood leesherwood added the bug Something isn't working label Nov 9, 2024
@lrljoe
Copy link
Collaborator

lrljoe commented Nov 9, 2024

Will look at this tomorrow, thanks for reporting

I will likely remove the empty and use a forelse instead of foreach, which will cater for not displaying empty values

@lrljoe
Copy link
Collaborator

lrljoe commented Nov 9, 2024

So a few more tweaks for this one that I first thought to make it a bit cleaner/more logical.

Instead of that logic being in the blade, I'm going to move it into the method that retrieves it, ensuring that it's validated before being retrieved for display.

At the same time, I'm going to fix the errant (superfluous) separator after the last item in the array.

@lrljoe
Copy link
Collaborator

lrljoe commented Nov 9, 2024

The final change with this fix, is just a minor adjustment to the blade, to utilise the method

@class([])

Where "class=''" is being used, and splitting up the layout/color classes

This bit is just prep work for when we add in the appropriate methods to manipulate which classes are applied to these elements down the line

@lrljoe
Copy link
Collaborator

lrljoe commented Nov 9, 2024

#2035 - Is the PR that should address this. It should be merged into development shortly, alongside an additional fix for the NumberFilter.

@lrljoe
Copy link
Collaborator

lrljoe commented Nov 10, 2024

This is now in the development branch, and will be in the next release (later today)

@leesherwood - please do feel free to test it while it's in the "dev-development" branch

@lrljoe
Copy link
Collaborator

lrljoe commented Nov 10, 2024

This should now be released.

@leesherwood
Copy link
Author

Can confirm latest release resolves the issue.

Only been working with the library for a couple of hours so not familiar with it enough to anticipate and test for side effects but if I do find any I will report back.

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

2 participants