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

format the numbers in comparison validator error messages #44076

Closed
wants to merge 1 commit into from

Conversation

gregmolnar
Copy link
Member

Summary

Currently, the comparison validator error messages are not formatting the numbers, so one can end up with messages like must be greater than or equal to 1000000. This PR changes the error messages to format the numbers with number_to_delimited in validation errors which results in a more readable message like must be greater than 1,000,000

@rails-bot rails-bot bot added the activemodel label Jan 5, 2022
@rafaelfranca
Copy link
Member

Thank you for the pull request but this will break a bunch of applications for really small gain, and also we can't assume people want numbers to be formatted as number because maybe they want number formatted as currency. You are able to customize the message when defining the exception to use the format.

@jclusso
Copy link

jclusso commented Jan 5, 2022

@rafaelfranca what if the validator took an option called formatter which was a proc. Would that be acceptable?

@rafaelfranca
Copy link
Member

What is the difference for the ):message option given it can take a proc as well?

@jclusso
Copy link

jclusso commented Jan 5, 2022

@rafaelfranca if you're using I18n you'd have to manually specify the translation in the proc. I'm not 100% familiar with how the translations work so maybe I'm missing something here. If a helper method exists for getting these translation keys I suppose it wouldn't be that bad.

@rafaelfranca
Copy link
Member

It doesn't, but I don't think it is worth adding this new feature just to avoid writing a i18n call for the validators where you care about the number being formatted.

@gregmolnar gregmolnar deleted the validation-message branch January 5, 2022 19:02
@jclusso
Copy link

jclusso commented Jan 5, 2022

@rafaelfranca it's more complex than an I18n call because data doesn't expose the validator that ran. Unless I'm not understanding something you're saying this is all the code required to make this work in my specific scenario. At this point I might as well just write a custom validator it feels like.

validates :threshold, numericality: { greater_than_or_equal_to: 500,
    less_than_or_equal_to: 1, message: ->(object, data) do
      translation_key =
        if data[:value] >= data[:count]
          'less_than_or_equal_to'
        elsif data[:value] <= data[:count]
          'greater_than_or_equal_to'
        end
      I18n.t(
        "errors.messages.#{translation_key}",
        count: ActiveSupport::NumberHelper.number_to_delimited(data[:count])
      )
    end }

@jonathanhefner
Copy link
Member

For reference, #41482 was an attempt at a generalized solution, with some discussion about different approaches.

@gregmolnar
Copy link
Member Author

For reference, #41482 was an attempt at a generalized solution, with some discussion about different approaches.

That looks like a lot better one than this one.

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

Successfully merging this pull request may close these issues.

None yet

4 participants