bootstrap-ruby / bootstrap_form

Official repository of the bootstrap_form gem, a Rails form builder that makes it super easy to create beautiful-looking forms using Bootstrap 5.
MIT License
1.64k stars 352 forks source link

Add support for Ransack::Search object to the first argument of bootrap_form_for #549

Closed harada4atsushi closed 2 years ago

harada4atsushi commented 4 years ago

Fixes #546

Add Support for Ransack::Search object to the first argument of bootrap_form_for.

Usage example:

Controller

def index
  @q = Invoice.ransack(params[:q])
  @invoices = @q.result(distinct: true)
end

View

= bootstrap_form_for @q, url: invoices_path, method: :get do |f|
  = f.search_field :file_name_cont
  = f.submit
bootstrap-ruby-bot commented 4 years ago
2 Warnings
:warning: There are code changes, but no corresponding tests. Please include tests if this PR introduces any modifications in bootstrap_form’s behavior.
:warning: Please update CHANGELOG.md with a description of your changes. If this PR is not a user-facing change (e.g. just refactoring), you can disregard this.

Here's an example of a CHANGELOG.md entry (place it immediately under the * Your contribution here! line):

  * [#549](https://github.com/bootstrap-ruby/bootstrap_form/pull/549): Add support for Ransack::Search object to the first argument of bootrap_form_for - [@harada4atsushi](https://github.com/harada4atsushi).

Generated by :no_entry_sign: Danger

harada4atsushi commented 4 years ago

@lcreid Thank you for your reply. I also agree that opinion.

Therefore, Ransack objects are not referenced directly, compare object.klass with string Ransack::Search, exceptions will not occur even if there is a change on the Ransack side.

should I avoid using external class names as strings?

I'll consider another idea.

harada4atsushi commented 4 years ago

I Modified not to use the string Ransack::Search. What about this code?(0797262) I will add tests later.

      def object_class
        if object.class.is_a?(ActiveModel::Naming)
          object.class
        elsif object.respond_to?(:klass) && object.klass.is_a?(ActiveModel::Naming)
          object.klass
        else
          object.class
        end
      end
donv commented 2 years ago

@harada4atsushi Do you still want this change? If so, could you add a test for it?

donv commented 2 years ago

Hi @harada4atsushi !

Thanks for the PR! If you would like it merged, please add a test that demonstrates the usage. If not, I will close the PR.