carmen-ruby / carmen-rails

NOT ACTIVELY MAINTAINED Rails adapter for Carmen (provides country_select and subregion_select)
MIT License
215 stars 159 forks source link

Rails 5 support, fix #60

Open kevinjbayer opened 7 years ago

kevinjbayer commented 7 years ago

If anyone else has issues getting this to work in Rails 5, I was able to get it by forking and adding this to form_helper.rb after line 223:

if Rails::VERSION::MAJOR == 5
      module Tags
        class Base
          def to_region_select_tag(parent_region, options = {}, html_options = {})
            html_options = html_options.stringify_keys
            add_default_name_and_id(html_options)
            options[:include_blank] ||= true unless options[:prompt]

            value = options[:selected] ? options[:selected] : value(object)
            priority_regions = options[:priority] || []
            opts = add_options(region_options_for_select(parent_region.subregions, value, 
                                                        :priority => priority_regions), 
                               options, value)
            select = content_tag("select", opts, html_options)
            if html_options["multiple"] && options.fetch(:include_hidden, true)
              tag("input", :disabled => html_options["disabled"], :name => html_options["name"], 
                           :type => "hidden", :value => "") + select
            else
              select
            end
          end
        end
      end
    end

On the line with options[:include_blank] ||= true unless options[:prompt] || select_not_required?(html_options), I had to remove select_not_required? as this seems to be deprecated. Seems to work without, but not sure if it should be replaced with something else.

eperezgarcia1201 commented 7 years ago

where can i find the form_helper.rb??

Envek commented 7 years ago

@eperezgarcia1201: it is inside gem: lib/carmen/rails/action_view/form_helper.rb:224

abrom commented 6 years ago

FYI it looks like select_not_required? was replaced in Rails 5 by it's functional inverse placeholder_required?.

If you take a look at the definition of rails select_content_tag you'll see:

if placeholder_required?(html_options)
  raise ArgumentError, "include_blank cannot be false for a required field." if options[:include_blank] == false
  options[:include_blank] ||= true unless options[:prompt]
end

I'd suggest using something like that for completeness.

Here's how I fixed it:

https://github.com/Studiosity/carmen-rails/blob/dff3ceee2f27ecf8ba1f3fffc2df15f03f97ccbd/lib/carmen/rails/action_view/form_helper.rb#L200-L228

Ich-bin-naoya commented 6 years ago

i do not know how to fix this "undefined method `to_region_select_tag' "

abrom commented 6 years ago

The project maintainer has said that they are not maintaining the project. I've patched it to support rails 5 at https://github.com/Studiosity/carmen-rails

I'd be happy to put a PR together with updated travis configs if you're interested @jim

Ich-bin-naoya commented 6 years ago

I got this "Git error: command `git clone 'git@github.com:Studiosity/carmen-rails.git'" when I was doing "bundle install"

abrom commented 6 years ago

how have you configured your Gemfile? Should have something like:

gem 'carmen-rails', git: 'git@github.com:Studiosity/carmen-rails.git'
kevinjbayer commented 6 years ago

@abrom ... it looks like Rails 5.2 breaks even more. Getting the good 'ol wrong number of arguments (given 1, expected 0) error now. Tried your branch to the same result. I'm guessing you haven't upgraded yet?

This gem is so outdated that I feel like your branch should become the master, but obviously, up to you. I tried diagnosing a little bit, but wasn't making any progress yet. Can't decide whether to spend more time on it or look for a new option, maybe JS based. I couldn't tag you in a new issue, plus, this one's still open.

kevinjbayer commented 6 years ago

After spending an hour or two debugging, I finally figured out where the error was coming from when using 5.2. Line 213 needs to be updated from: value = options[:selected] ? options[:selected] : value(object)

to: value = options[:selected] ? options[:selected] : value

Here's the file if anyone needs it, updated with @abrom's fixes too. https://github.com/kevinjbayer/carmen-rails/blob/master/lib/carmen/rails/action_view/form_helper.rb

abrom commented 6 years ago

FYI i've created a branch that addresses this to retain Rails 4 & Rails < 5.2 support. Just checks the value methods arity before calling:

value = options[:selected] ? options[:selected] : (method(:value).arity.zero? ? value : value(object))
yuri-zubov commented 5 years ago

@abrom this code is correct

value = options[:selected] ? options[:selected] : (method(:value).arity.zero? ? value() : value(object))
abrom commented 5 years ago

@yuri-zubov unless I'm missing something here, you've just added parentheses to the call to value ? i.e. functionally equivalent

Can you expand on your comment?

yuri-zubov commented 5 years ago

@abrom sure, ruby doesn't call method value. it takes nil value from a variable. for example, we have object Address with field country and initialize country with 'US'.

@address = Address.new(country: 'US')

in view

form_for @address do |f|
  = f.country_select :country, Address::FAVORITE_COUNTRIES, {}, { class: 'form-control'}

but we don't have filled the field with value 'US' on the form, because You have local variable value and ruby take this variable and doesn't call method value. if we want to call method we need to insert brackets or change name of local variable - new_value