search

railsadminteam / rails_admin

RailsAdmin is a Rails engine that provides an easy-to-use interface for managing your data
MIT License
7.9k stars 2.26k forks source link

Fix filter on enum value in list view v2 #3684

Closed robotfelix closed 5 months ago

robotfelix commented 5 months ago

Filtering on legacy enums in a list view gives errors under Rails Admin 3.x (see #3651)

This PR adds to #3647 by adding a test for the fix provided there, as well as providing a similar fix for the Fields::Types::Enum#enum_method method.

I've left the original commit by @fuegas as-is to give due credit, but happy to squash the fix with the associated tests if preferred.

I don't fully grasp why the method is called without bindings (not even an empty hash), and it's not clear if that is something that should ever be expected or is a symptom of a bug somewhere else, however either way it is true that the code paths using abstract_model were not covered by the existing tests.

mshibuya commented 5 months ago

For now I'm not merging this as https://github.com/railsadminteam/rails_admin/commit/d62f604cc8d7d1434f7dfe0c5aca3aaf3dc2547b will be the proper fix, but I really appreciate your attempt to address this. Thank you so much, and looking forward to another contribution 👍

robotfelix commented 5 months ago

@mshibuya Thanks. https://github.com/railsadminteam/rails_admin/commit/d62f604cc8d7d1434f7dfe0c5aca3aaf3dc2547b definitely looks like a better fix! 🎉

Let me know if you'd like me to adapt the tests into some "when the binding don't provide an object" test cases (eg: with an empty binding rather than no binding) instead to add test coverage for the code paths through abstract_model.