search

spatie / laravel-query-builder

Easily build Eloquent queries from API requests
https://spatie.be/docs/laravel-query-builder
MIT License
4.02k stars 395 forks source link

Fix `array_diff_assoc` BC break in v5.1.0 #827

Closed stevebauman closed 1 year ago

stevebauman commented 1 year ago

Closes #826

Passing tests: https://github.com/stevebauman/laravel-query-builder/actions/runs/3567702432

davidjr82 commented 1 year ago

hi @stevebauman,

this PR covers the multidimensional case my PR didn't cover, that's great!

My only doubt here is that the ignored values will apply to any of the nested values of the multidimensional array, right? For example, you can not ignore only "John Doe" in the key "value" of your example in #826 .

Thinking on how to improve it, right now ignored is just a collection of values. If we add the concept of key/value we would be able to ignore only when the combination of both happens, that is useful for multidimensional arrays.

But anyway, your PR improves the behaviour that has the package right now, so looks very good IMO.

stevebauman commented 1 year ago

Hey @davidjr82, thanks for reviewing the PR, much appreciated 🙏

My only doubt here is that the ignored values will apply to any of the nested values of the multidimensional array, right?

You're correct -- if any array value is contained in the ignored collection, then it will be filtered out.

I agree with your improvement. We could tackle that in a following PR so that the bug can be patched and then the additional feature could be placed into branch and release.

freekmurze commented 1 year ago

Thank you!