heartcombo / devise

Flexible authentication solution for Rails with Warden.
http://blog.plataformatec.com.br/tag/devise/
MIT License
23.89k stars 5.54k forks source link

Fix deprecation warning in ParameterSanitizer#permit #5551

Open j-sieg opened 1 year ago

j-sieg commented 1 year ago

Hello first time here

I'm using Devise 4.8.1 with Rails and I'm upgrading my Rails version to 7.0.4 When I ran the tests on my app, I get these deprecation warnings:

app/models/user/parameter_sanitizer.rb:24: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
.rbenv/versions/2.7.4/lib/ruby/gems/2.7.0/gems/devise-4.8.1/lib/devise/parameter_sanitizer.rb:110: warning: The called method `permit' is defined here

There were plenty of them, so I decided to bundle open devise. I edited the file, ran the tests on my app, and the deprecation warnings about the ParameterSanitizer were gone. I put what I wrote on this PR.

carlosantoniodasilva commented 1 year ago

@j-sieg I don't see those warnings when running Devise tests on Ruby 2.7, can you show the code in your custom parameter sanitizer: app/models/user/parameter_sanitizer.rb?

Maybe that's implemented in a way that's delegating as non-kwargs to Devise or something. Thanks!

j-sieg commented 1 year ago

@carlosantoniodasilva here's the code in app/models/user/parameter_sanitizer.rb

class User::ParameterSanitizer < Devise::ParameterSanitizer
  def initialize(params, resource = nil)
    # resource's class, resource's name, params
    super(User, :user, params)

    sign_up_defaults = [:name, :mobile_number]
    account_update_defaults = [
      :name,
      :date_of_birth,
      :mobile_number,
      :avatar_url
    ]

    # Omitted code
    # I use `resource` here to push some fields depending on biz logic

    permit(:sign_up, keys: sign_up_defaults)
    permit(:account_update, keys: account_update_defaults)
  end
end
carlosantoniodasilva commented 1 year ago

@j-sieg thanks. I don't see anything wrong at a glance, I thought maybe the calling code could be passing a hash like this:

permit_options = { keys: [:foo, :bar] }

...

# permit_options is a hash in this sample
permit(:sign_up, permit_options)

If that was the case, then the calling code would have to be changed to actual kwargs:

permit(:sign_up, **permit_options)

But it doesn't seem to be the case... what's interesting is that this code from Devise hasn't changed since 2015 so I'm not sure I want to switch it back to a hash of options when kwargs are much clearer there. 🤔