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

Numericality validator against other fields doesn't work as expected #687

Open
varvaray opened this issue Dec 22, 2016 · 7 comments
Open

Comments

@varvaray
Copy link

When using numericality: greater_than with other model attributes, if i remove 1 of the fields, it doesn't show any messages anymore, but when i try to submit, i'm getting server validation fail. I suppose it's better to reproduce server side behaviour for that case

validates :maximum_players,
allow_nil: true,
numericality: { only_integer: true, less_than_or_equal_to: 2147483647, greater_than_or_equal_to: :minimum_players }

validates :minimum_players,
allow_nil: true,
numericality: { only_integer: true, greater_than_or_equal_to: 0, less_than_or_equal_to: :maximum_players }

8565d634-be36-11e6-8020-e983915200e4

@tagliala
Copy link
Contributor

tagliala commented Dec 22, 2016

@varvaray thanks for splitting this bug report

I'm having troubles in understanding this step: " if i remove 1 of the fields"

Could you please provide me something like

Initial state:

model

validates :maximum_players, 
  allow_nil: true,
  numericality: { only_integer: true, less_than_or_equal_to: 2147483647, greater_than_or_equal_to: :minimum_players }

validates :minimum_players,
  allow_nil: true,
  numericality: { only_integer: true, greater_than_or_equal_to: 0, less_than_or_equal_to: :maximum_players }

view

= f.text_field :minimum_players
= f.text_field :maximum_players

Steps to reproduce:

  1. Insert X in :minimum_players
  2. Insert Y in :maximum_players
  3. Remove ... from ... field

Expected behaviour

  • Minimum players should...
  • Maximum players...

Actual behaviour

No validation messages

@varvaray
Copy link
Author

varvaray commented Jan 9, 2017

@tagliala Sorry for the late answer.

I will point out only the things that are missing.

Steps to reproduce:

Insert 5 in :minimum_players
/got the message "Must be less than or equal to maximum_players"/
Insert 10 in :maximum_players
/the message is still there/
Insert 2 in :minimum_players
/the message is removed/
Remove10 from maximum_players field
/no messages anywhere/
Submit

Expected behaviour

Minimum players should have error message like "Can't compare with empty field"
Maximum players should stay empty, i guess

Actual behaviour

No validation messages

ArgumentError in DFS::TournamentSchedulesController#create
comparison of Fixnum with nil failed

Extracted source (around line #58):

        end

        unless value.send(CHECKS[option], option_value)
          record.errors.add(attr_name, option, filtered_options(value).merge!(count: option_value))
        end
      end

@varvaray
Copy link
Author

varvaray commented Jan 9, 2017

@tagliala another thing i wanted to discuss is the whole concept of 2 fields comparing.

In the example above, when i'm performing step

Insert 10 in :maximum_players

I should have the message in the minimum_players field to be removed already. I did it myself in a bit of hacky way

  = f.number_field :minimum_players, min: 0, step: 1, validate: true,
          data: { compare_with: :maximum_players, compare: :minimum_players }
  = f.number_field :maximum_players, min: 1, step: 1, validate: true,
          data: { compare_with: :minimum_players, compare: :maximum_players }

and in .js

$ ->
  $('form').each ->
    form = $(@)
    form.find('input[data-compare-with]').each ->
      input = $(@)
      compare_with = $('#' + input.attr('id').replace(input.data('compare'), input.data('compare-with')))
      input.on 'focusout', (e) ->
        compare_with.trigger('change.ClientSideValidations')
        compare_with.isValid(ClientSideValidations.forms[form.attr('id')].validators)

So that if in any of 2 fields which are compared value changes, it triggers validation in another one, but i guess this can be implemented in the gem in that way or another.

@tagliala
Copy link
Contributor

Steps to reproduce:

Insert 5 in :minimum_players
/got the message "Must be less than or equal to maximum_players"/

Nope, didn't get that message

image

Unable to replicate, closing here

@tagliala
Copy link
Contributor

tagliala commented Jan 22, 2017

Steps to reproduce:

  1. Insert 1 in minimum_players
  2. Use TAB to focus maximum_players
  3. Insert 2 in maximum_players
  4. Use SHIFT+TAB to focus minimum_players
  5. Insert 3 in minimum_players
  6. Use TAB to focus maximum_players
  7. "Must be less than or equal to maximum_players" message appears
  8. Insert 4 in maximum_players

Expected behaviour:

Validation on minimum_players passes

Actual behaviour:

Validation on minimum_players still fails and it is hard to make it pass (focus + blur doesn't work, inserting 3 again doesn't work. The field should be wiped out, blurred, focused and set again)

@tagliala
Copy link
Contributor

tagliala commented Jan 22, 2017

Unfortunately this could not be solved with the actual implementation of CSV or the solution is not naive.

Example:

Model

  attr_accessor :points_1, :points_2

  validates :points_2, numericality: { greather_than: :points_1 }
  1. Insert 3 in points_1
  2. Use TAB to focus points_2
  3. Insert 2 in points_2
  4. Use SHIFT+TAB to focus points_1
  5. "Must be greater than points_1" message appears
  6. Insert 1 in points_1

There is no validation on points_1 field, so CSV doesn't know that there is the need of running the validations again on another field. Your workaround is the way to go for now.

Maybe this will be easier to fix if we switch to a model based / promise based implementation.

@varvaray
Copy link
Author

@tagliala thanks for the explanation

@tagliala tagliala changed the title Java script doesn't show error message while validating numericality: greater_than with other model attribute when 1 of the fields is empty Numericality validator against other fields doesn't work as expected Jan 24, 2017
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

2 participants