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

NumericalityValidator allow_nil option doesn't work anymore for params coming from a controller #40750

Closed
filippoliverani opened this issue Dec 4, 2020 · 1 comment · Fixed by #40765
Milestone

Comments

@filippoliverani
Copy link
Contributor

numericality validation helper behaviour on attributes of an ActiveRecord object created using ActionController::Parameters has changed in Rails 6.1.0.rc2. It could be related to this refactoring

Steps to reproduce

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "rails", '6.1.0.rc2'
  gem "sqlite3"
end

require "action_controller"
require "active_record"
require "minitest/autorun"
require "logger"

ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :comments, force: true do |t|
     t.integer :priority
  end
end

class Comment < ActiveRecord::Base
  validates :priority, numericality: { greater_than: 0 }, allow_nil: true
end

class BugTest < Minitest::Test
  def test_numericality_validation
    params = ActionController::Parameters.new({ comment: { priority: '' } })
    comment = Comment.new(params.require(:comment).permit!)

    assert comment.valid?
  end
end

Expected behavior

allow_nil option should allow to skip numericality validation for empty ActionController::Parameters fields. Having to switch to allow_blank option would be a breaking change.

Actual behavior

numericality validation fails with empty fields even if allow_nil option is specified.

System configuration

Rails version: 6.1.0.rc2

Ruby version: 2.7.2

@eugeneius
Copy link
Member

This seems to be the intended effect of 4cc438a: numericality validations apply to the value before typecasting, which wasn't nil, so allow_nil shouldn't apply.

If we want to keep the new behaviour, it should have tests and a changelog entry though.

@eugeneius eugeneius added this to the 6.1.0 milestone Dec 7, 2020
kamipo added a commit to kamipo/rails that referenced this issue Dec 8, 2020
It is a regression for 4cc438a.

`NumericalityValidator` basically takes the value before typecasting,
but `allow_nil` should work for the typecasted value for the
compatibility.

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

Successfully merging a pull request may close this issue.

2 participants