ransack
ransack copied to clipboard
Simple form integration
EDIT
This PR will be turned into a separate gem. I'll let it open for discussion until we release them.
Related to #1160
This PR introduces the Ransack::Helpers::SimpleFormBuilder
, which extends Ransack::Helpers::FormBuilder
and integrates SimpleForm::FormBuilder
. This approach avoids the need for monkey patching, using environment variables, or potential code collisions by:
- Establishing a distinct
Ransack::Helpers::SimpleFormBuilder
to facilitate integration with SimpleForm. - Allowing concurrent use of Ransack, SimpleForm, and the combined form builders within the same application through the new
search_simple_form_for
method. - Providing
has_attribute?
andtype_for_attribute
methods to enable SimpleForm to accurately infer field types. - Ensuring that boolean predicates such as _blank or _not_null are correctly handled by generating appropriate boolean fields.
- Striking out previous limitations regarding the handling of non-text fields.
Additional contributions include:
- Adding specific tests for this feature.
- Updating the documentation to reflect these changes.
After this PR one can use SimpleForm like:
<%= search_simple_form_for @q do |f| %>
<%= f.input :name_cont %>
<%= f.input :employee_name_present %>
<%= f.input :created_at_gteq %>
<%= f.button :submit %>
<% end %>
I am currently conducting further tests and welcome any feedback on this PR. I would appreciate any contributions to refine this feature and am looking forward to potentially merging it.
Here some screenshots of testing the feature in my app.
- employee_name_present is correctly identified as a boolean predicate and a check box is rendered.
- creation_date_gteq is correctly identified as a datetime attribute and the selects are rendered.
- Compound conditions like name_or_destination_name_cont are handled correctly.
- The labels are correctly translated to the default locale (pt-BR)
- The Bootstrap 5 wrapper divs are put in place
@abinoam would you consider publishing this as a separate gem?
Hi @scarroll32,
I completely understand your perspective, and ideally, making a separate gem would be a good approach. This PR is submitted directly to Ransack mainly because there is already a preliminary, albeit non-functional, integration in place.
Ransack's form builder methods.
The current structure of Ransack’s code doesn’t clearly define extension points for the functionality this PR introduces. I found myself having to utilize send to access private methods, which isn’t ideal for maintenance or clarity (from a external gem perspective).
Given these constraints, creating an external gem without a stable public API to hook into would be likely prone to break with future updates. By integrating these changes directly within Ransack, we can ensure any incompatibilities are immediately flagged by failing tests, thus maintaining stability.
However, if the guidance is to minimize coupling with code that is not intrinsic to Ransack as much as possible, I would be happy to convert this PR into a separate gem. In this case, I would also suggest considering the removal of the current integration code, since it has caused frustrations due to not working properly (I could do a PR to remove it).
I also would ask you if you would accept a PR that extract some methods into public apis in Ransack so I could hook into them from the external gem? These would not alter in any form the current behavior of Ransack.
It's important to note that there's already gems that do that. I tested and they didn't work as I expected:
- https://github.com/kaspernj/simple_form_ransack
- https://github.com/iquest/ransack_simple_form
One of the thing that is noticeable in these gems it's that they rewrite some Ransack code. So, when Ransack changes, it stops working. I don't plan to incur in the same problem.
Let me know your final verdict and I will start to work on it.
Thanks for your time in maintaining Ransack! 👏
However, if the guidance is to minimize coupling with code that is not intrinsic to Ransack as much as possible, I would be happy to convert this PR into a separate gem. In this case, I would also suggest considering the removal of the current integration code, since it has caused frustrations due to not working properly (I could do a PR to remove it).
@abinoam thank you for your PR and these comments. It's great to have someone interested in contributing to Ransack... I am hesitant to add code for a library that is used by some users only. There was a similar discussion recently about full text search.
I also would ask you if you would accept a PR that extract some methods into public apis in Ransack so I could hook into them from the external gem? These would not alter in any form the current behavior of Ransack.
This sounds fine, I'd really appreciate any refactoring that you can do on this! We can also add a link to the new gem in the Ransack docs.
See also https://activerecord-hackery.github.io/ransack/going-further/other-notes/#using-simpleform
@abinoam thank you for your PR and these comments. It's great to have someone interested in contributing to Ransack... I am hesitant to add code for a library that is used by some users only. There was a similar discussion recently about full text search.
I completely understand you.
I also would ask you if you would accept a PR that extract some methods into public apis in Ransack so I could hook into them from the external gem? These would not alter in any form the current behavior of Ransack.
This sounds fine, I'd really appreciate any refactoring that you can do on this! We can also add a link to the new gem in the Ransack docs.
Nice. I'll get working on that gem soon— probably not this week, but it's on my to-do list! I definitely need it for a side project of mine, so it will happen! 😄🚀