DavyJonesLocker / client_side_validations

Client Side Validations made easy for Ruby on Rails
MIT License
2.69k stars 403 forks source link

validates numericality does not run on number_field when allow_nil is true #682

Closed kingdonb closed 7 years ago

kingdonb commented 8 years ago

I tried using validates: numericality with allow_nil and it works great, except for one thing. The only alphabetic keystroke that is allowed is the letter 'e'. I assume this is because of scientific notation.

The field will not allow you to submit a number in scientific notation, so I think this allowance is a bug. Looking for feedback about this issue before I attempt to make a PR to fix it.

tagliala commented 8 years ago

The just released 4.2.9 version addressed some issues in numericality validator.

Maybe this is a regression

I will take a look tomorroe

tagliala commented 8 years ago

Sorry, cannot confirm.

Gemfile

gem 'client_side_validations'
gem 'rails', '4.2.7.1'
gem 'turbolinks'

Gemfile.lock

    rails (4.2.7.1)
      actionmailer (= 4.2.7.1)
      actionpack (= 4.2.7.1)
      actionview (= 4.2.7.1)
      activejob (= 4.2.7.1)
      activemodel (= 4.2.7.1)
      activerecord (= 4.2.7.1)
      activesupport (= 4.2.7.1)
      bundler (>= 1.3.0, < 2.0)
      railties (= 4.2.7.1)
      sprockets-rails
    client_side_validations (4.2.9)
      jquery-rails (>= 3.1.2, < 5.0.0)
      js_regex (~> 1.0, >= 1.0.9)
      rails (>= 4.0.0, < 4.3.0)
    turbolinks (5.0.1)
      turbolinks-source (~> 5)

Model

class Post < ActiveRecord::Base
  validates :players, numericality: true
end

application.js

//= require jquery
//= require jquery_ujs
//= require turbolinks
//= require rails.validations
//= require_tree .

/app/views/posts/new.html.erb

<%= form_for @post, validate: true do |f| %>
  <%= f.label :players %>
  <%= f.text_field :players %>

  <%= f.submit %>
<% end %>
  1. insert e: image
  2. insert 1: image
kingdonb commented 8 years ago

Try with :allow_nil

The behavior is not correct. User is led to believe they may save "e" but they save nil. There is no value of /\d+e\d/ that results in a number being saved, that I could divine...

What is the meaning of allowing "e"? Is there a value for :numericality that allows the user to express in scientific notation?

(Can you use it for BigDecimal?)

Result I am looking for is as in your step 1. insert e, but with :allow_nil enabled. Your test did not enable it so your submit did (as I would prefer for the mode :allow_nil) result in failure and a field with errors. With :allow_nil test result is less than optimal.

kingdonb commented 8 years ago

Please reopen

tagliala commented 8 years ago

Still cannot confirm.

Please provide me a reduced test case

Model

class Post < ActiveRecord::Base
  validates :players, numericality: true, allow_nil: true
end

generated script


//<![CDATA[
if(window.ClientSideValidations===undefined)window.ClientSideValidations={};window.ClientSideValidations.disabled_validators=[];window.ClientSideValidations.number_format={"separator":".","delimiter":","};if(window.ClientSideValidations.patterns===undefined)window.ClientSideValidations.patterns = {};window.ClientSideValidations.patterns.numericality=/^(-|\+)?(?:\d+|\d{1,3}(?:\,\d{3})+)(?:\.\d*)?$/;if(window.ClientSideValidations.forms===undefined)window.ClientSideValidations.forms={};window.ClientSideValidations.forms['new_post'] = {"type":"ActionView::Helpers::FormBuilder","input_tag":"\u003cdiv class=\"field_with_errors\"\u003e\u003cspan id=\"input_tag\" /\u003e\u003clabel for=\"\" class=\"message\"\u003e\u003c/label\u003e\u003c/div\u003e","label_tag":"\u003cdiv class=\"field_with_errors\"\u003e\u003clabel id=\"label_tag\" /\u003e\u003c/div\u003e","validators":{"post[players]":{"numericality":[{"messages":{"numericality":"is not a number"},"allow_blank":true}]}}};
//]]>

(at the very end you can see there is "allow_blank": true)

Result: image

Also, I'm not finding results for \d+e in CSV's code

kingdonb commented 8 years ago

Sorry for not providing more details upfront!

Here is an example that shows the failure:

https://github.com/kingdonb/example/commit/a28ad6b640e0e3754d83a0d6c7d151f5160a7a8a

In brief, try f.number_field :players instead of f.text_field :players

I use a custom numericality setting {greater_than_or_equal_to: -240, less_than_or_equal_to: 240} but that isn't what makes my test case different from yours. It's this number_field.

The input restrictions I saw actually do not come from client_side_validations. I see they come from using number_field. The number_field is a built-in of Rails (and browsers, input type="numbers") that restricts the allowed input. I'm not sure if it uses any javascript to do this, or only browser support built-ins...

That number_field where the spurious "e" is allowed and other input is not allowed (because the spec says it should.) The client_side_validation rejects any numbers with the letter e in them, but in this case you are allowed to submit any number of letters e as I described in the prior note and maybe depending on your model datatype or string constructor, they would be erased or not.

That also seems to be the trick to trigger different behavior of client_side_validations, this is the info you needed to be able to repro the issue. I see now there are more moving parts than I thought, this may be tricky to fix...

tagliala commented 8 years ago

@kingdonb thanks.

I confirm there is an issue and it is worse on firefox because it allows all letters.

I will investigate

tagliala commented 8 years ago

Apparently I can't fix this on CSV side http://stackoverflow.com/a/18853513

image

I've tried something, like converting the number field in a text field on the fly and get the value, but doesn't work

Number to text:

// Number field contains `4`
> element.val()
"4"
> element.attr('type')
"number"
> element.clone().attr('type', 'text').val()
"4"
// Number field contains `e`
> element.val()
""
> element.attr('type')
"number"
> element.clone().attr('type', 'text').val()
""

The other way (text to number):

// Text field contains `e`
> element.val()
"e"
> element.attr('type')
"text"
> element.clone().attr('type', 'number').val()
""

At the moment you can either witch to a text_field or treat 0 as a nil value

kingdonb commented 8 years ago

I think my clients can live with this minor minor misbehavior, the good news is there's no reason for me to remove client_side_validations because it's apparently not the software at fault.

I thought I might be able to solve it by setting pattern="something" but I guess pattern is only allowed on text and textarea, not on number type. Also I guess pattern doesn't actually validate input characters, it just validates the value on submit.

Thanks for following up, and for showing all this!

tagliala commented 8 years ago

I thought I might be able to solve it by setting pattern="something" but I guess pattern is only allowed on text and textarea, not on number type.

I've tried that too, but I'm not able to make it work. Also, it is not supported by Safari so I gave up

I will leave this open

tagliala commented 7 years ago

I'm closing this as wontfix at the moment