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

Add error message value formatter to the numericality validator #41482

Closed
wants to merge 3 commits into from

Conversation

joaomangilli
Copy link

Summary

Sometimes, we need to format the value being shown in an error message in the numericality validator (like greater_than_or_equal_to: 25, we want to format the number 25 to something like 25.00) to match a monetary value or something like that.

This PR opens an option to format that value the way you need it.

@rafaelfranca
Copy link
Member

Makes sense. Can you add a CHANGELOG entry?

@rails-bot rails-bot bot added the docs label Feb 20, 2021
@joaomangilli
Copy link
Author

@rafaelfranca done

@jonathanhefner
Copy link
Member

@joaomangilli Do you have a use-case in mind that would benefit from :value_format instead of specifying a :message Proc? A :message Proc can access the validation value via :count, e.g.:

validates_numericality_of :approved, greater_than: 50000, 
  message: -> (record, data) { "must be greater than #{do_something(data[:count])}" }

Perhaps this could be improved by allowing the :message Proc to modify and return the data Hash, thus still relying on the framework to perform interpolation? e.g.:

validates_numericality_of :approved, greater_than: 50000, 
  message: -> (record, data) { data.merge(count: do_something(data[:count])) }

@joaomangilli
Copy link
Author

@jonathanhefner thanks! I think this is a best way to solve the problem. This can pave the way for solving more problems. I'll make the change.

@jonathanhefner
Copy link
Member

@joaomangilli To clarify: I'm not sure if returning a Hash from a :message Proc is the best way to solve this. I was just brainstorming out loud. 😃

Another possibility might be using Errors#generate_message or a similar method. For example, if the data Hash included the non-humanized attribute name and the error type, you could write something like:

validates_numericality_of :approved, greater_than: 50000, message: -> (record, data) do 
  data[:count] = do_something(data[:count])
  record.errors.generate_message(data[:attribute_symbol], data[:type], data)
end

@joaomangilli
Copy link
Author

joaomangilli commented Feb 23, 2021

@jonathanhefner sorry, I wanted to say that it looks like a better (not the best) solution.

But the more I think about it the more I think that this validator should offer a native option to format the count value, without the need to deal with the message.

I know that this value is part of the message, but I think it makes sense to pass this value already formatted, so we don't have to deal with it in the message.

Because to do this in the message, we need to change the method of the message to offer more options or flexibility and it still won't solve the problem, as we will still need to use this method to "interpolate" the formatted value in the message.

What do you think?

@joaomangilli
Copy link
Author

@rafaelfranca can we go ahead with this?

@rails-bot
Copy link

rails-bot bot commented Jun 30, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

@rails-bot rails-bot bot added the stale label Jun 30, 2021
@rails-bot rails-bot bot removed the stale label Jul 1, 2021
@zzak zzak requested a review from rafaelfranca July 1, 2021 23:22
@rails-bot
Copy link

rails-bot bot commented Sep 30, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

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