Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In Rails 5 params.merge raises exception #26875

Closed
gnodiah opened this issue Oct 24, 2016 · 4 comments
Closed

In Rails 5 params.merge raises exception #26875

gnodiah opened this issue Oct 24, 2016 · 4 comments

Comments

@gnodiah
Copy link

gnodiah commented Oct 24, 2016

I'm upgrading my app from 4.2.6 to 5.0.0.1, and I noticed that Rails 5 deprecated method params.merge! in controllers. so I use params = params.merge instead, but unfortunately it raises:

NoMethodError (undefined method 'merge' for nil:NilClass)

then I use params = params.to_unsafe_h.merge, again it raises NoMethodError for to_unsafe_h method.

what's wrong? And, in my original 4.2.6 app, I merge params in ApplicationController, and in my BooksController the params is the merged one, but in Rails 5 it seems not the merged one?

Rails: 5.0.0.1
Ruby: 2.3.0

@maclover7
Copy link
Contributor

Can you create an executable test script using one of the templates here, that would demonstrate the problem you are seeing? It seems like there's something weird going on with the params variable.

@gnodiah
Copy link
Author

gnodiah commented Oct 25, 2016

@maclover7 thx, test script:

begin
  require "bundler/inline"
rescue LoadError => e
  $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler"
  raise e
end

gemfile(true) do
  source "https://rubygems.org"
  gem "rails", "~> 5.0.0.1"
end

require "action_controller/railtie"

class TestApp < Rails::Application
  config.root = File.dirname(__FILE__)
  secrets.secret_token    = "secret_token"
  secrets.secret_key_base = "secret_key_base"

  config.logger = Logger.new($stdout)
  Rails.logger  = config.logger

  routes.draw do
    get "/" => "test#index"
  end
end

class ApplicationController < ActionController::Base
  before_action :process_params

  def process_params
    p params
    params = params.merge(age: 11)
  end
end

class TestController < ApplicationController
  include Rails.application.routes.url_helpers

  def index
    p params
    render plain: "Home"
  end
end

require "minitest/autorun"
require "rack/test"

class BugTest < Minitest::Test
  include Rack::Test::Methods

  def test_returns_success
    get "/"
    assert last_response.ok?
  end

  private
  def app
    Rails.application
  end
end

@rafaelfranca
Copy link
Member

This is how ruby works. If you have a method called foo and you defined a local variable called foo assigning the value of foo with something the foo in the right side of the expression will be the local variable in the left side.

$ irb
>> def foo
>>   {}
>>   end
=> :foo
>> foo = foo.merge(a: 1)
NoMethodError: undefined method `merge' for nil:NilClass
    from (irb):4
    from /Users/rafaelfranca/.rbenv/versions/2.3.1/bin/irb:11:in `<main>'
>> exit

@gnodiah
Copy link
Author

gnodiah commented Oct 25, 2016

thx @rafaelfranca , u r right.

in the left side I should use self.params rather than params, the former is actually the params= method, and the latter is just a local variable.

class ApplicationController < ActionController::Base                                 
  before_action :process_params                                                      

  def process_params                                                                                                                                        
    p defined? params     # => "method"                                                           
    params = {age: 11}                                                       
    p defined? params     # => "local-variable"                                                                  
    p self.class.method_defined? :params=       # => true                                                                                                                         
    self.params = self.params.merge(age: 11)      # `self.params` is the right usage                                        
  end                                                                                
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants