Allow to override the full_message error format#32956
Allow to override the full_message error format#32956rafaelfranca merged 2 commits intorails:masterfrom
Conversation
daf2453 to
4b2b291
Compare
sikachu
left a comment
There was a problem hiding this comment.
I think this is a good patch. I personally found myself having to find way to workaround this a few times. It'd be nice to be able to do this in i18n config.
wdyt @rafaelfranca @matthewd ?
There was a problem hiding this comment.
Weird. Why did you need to add this? Active Model is not a ORM.
There was a problem hiding this comment.
Removed.
Looking the active_record and http://api.rubyonrails.org/classes/Rails/Railtie.html as examples, I though it was needed.
There was a problem hiding this comment.
Let's document this option in the configuring guide https://github.com/rails/rails/blob/master/guides/source/configuring.md.
4b2b291 to
32513c4
Compare
| class << self | ||
| attr_accessor :i18n_full_message # :nodoc: | ||
| end | ||
| self.i18n_full_message = false |
There was a problem hiding this comment.
Is it added intentionally?
It seems extra since we set default value of i18n_full_message in activemodel/lib/active_model/railtie.rb.
diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb
index 56404a036c..7400bb5a6c 100644
--- a/activemodel/lib/active_model/errors.rb
+++ b/activemodel/lib/active_model/errors.rb
@@ -65,7 +65,6 @@ class Errors
class << self
attr_accessor :i18n_full_message # :nodoc:
end
- self.i18n_full_message = false
attr_reader :messages, :details
diff --git a/activemodel/lib/active_model/railtie.rb b/activemodel/lib/active_model/railtie.rb
index 0ed70bd473..ed70d65c8e 100644
--- a/activemodel/lib/active_model/railtie.rb
+++ b/activemodel/lib/active_model/railtie.rb
@@ -8,13 +8,14 @@ class Railtie < Rails::Railtie # :nodoc:
config.eager_load_namespaces << ActiveModel
config.active_model = ActiveSupport::OrderedOptions.new
+ config.active_model.i18n_full_message = false
initializer "active_model.secure_password" do
ActiveModel::SecurePassword.min_cost = Rails.env.test?
end
initializer "active_model.i18n_full_message" do
- ActiveModel::Errors.i18n_full_message = config.active_model.delete(:i18n_full_message) || false
+ ActiveModel::Errors.i18n_full_message = config.active_model.delete(:i18n_full_message)
end
end
endThere was a problem hiding this comment.
Yes, it is intentional. We want a default value if the railtie is not loaded, for example if you use Active Model without Rails.
There was a problem hiding this comment.
It makes sense then. Thanks for explanation.
What do you think about making ActiveModel::Errors.i18n_full_message as public API in order to allow users use this new feature if use Active Model without Rails?
- Fix indentation. - Add a missing dot to the end of the sentence. Related to rails#32956
Add mention about default value of `config.active_model.i18n_full_message`.
Add changelog for #32956 [ci skip]
…age` - I feel `i18n_customize_full_messages` explains the meaning of the config better. - Followup of rails#32956
Follow-up to rails#32956 and rails#35789 The introduction of this configuration in rails#32956 stated that: > The goal of this PR is to make it easier for an app to transition from a #{attribute} #{message} to a #{message}, full_message error format. However, by setting the default value to `false`, this feature was difficult to discover.
Follow-up to rails#32956 and rails#35789 The introduction of this configuration in rails#32956 stated that: > The goal of this PR is to make it easier for an app to transition from a #{attribute} #{message} to a #{message}, full_message error format. However, by setting the default value to `false`, this feature was difficult to discover.
Follow-up to rails#32956 and rails#35789 The introduction of this configuration in rails#32956 stated that: > The goal of this PR is to make it easier for an app to transition from a #{attribute} #{message} to a #{message}, full_message error format. However, by setting the default value to `false`, this feature was difficult to discover.
Follow-up to rails#32956 and rails#35789 The introduction of this configuration in rails#32956 stated that: > The goal of this PR is to make it easier for an app to transition from a #{attribute} #{message} to a #{message}, full_message error format. However, by setting the default value to `false`, this feature was difficult to discover.
Follow-up to rails#32956 and rails#35789 The introduction of this configuration in rails#32956 stated that: > The goal of this PR is to make it easier for an app to transition from a #{attribute} #{message} to a #{message}, full_message error format. However, by setting the default value to `false`, this feature was difficult to discover.
…`false` to `true` Follow-up to rails#32956 and rails#35789 The introduction of this configuration in rails#32956 stated that: > The goal of this PR is to make it easier for an app to transition from a #{attribute} #{message} to a #{message}, full_message error format. However, by setting the default value to `false`, this feature was difficult to discover.
…` to `true` Follow-up to rails#32956 and rails#35789 The introduction of this configuration in rails#32956 stated that: > The goal of this PR is to make it easier for an app to transition from a #{attribute} #{message} to a #{message}, full_message error format. However, by setting the default value to `false`, this feature was difficult to discover.
…` to `true` Follow-up to rails#32956 and rails#35789 The introduction of this configuration in rails#32956 stated that: > The goal of this PR is to make it easier for an app to transition from a #{attribute} #{message} to a #{message}, full_message error format. However, by setting the default value to `false`, this feature was difficult to discover. `config.active_model.i18n_customize_full_message` defaults to `true` for `config.load_defaults 7.2` and above.
…` to `true` Follow-up to rails#32956 and rails#35789 The introduction of this configuration in rails#32956 stated that: > The goal of this PR is to make it easier for an app to transition from a #{attribute} #{message} to a #{message}, full_message error format. However, because the default value was set to `false`, this feature was difficult to discover. `config.active_model.i18n_customize_full_message` defaults to `true` for `config.load_defaults 7.2` and above.
…` to `true` Follow-up to rails#32956 and rails#35789 The introduction of this configuration in rails#32956 stated that: > The goal of this PR is to make it easier for an app to transition from a #{attribute} #{message} to a #{message}, full_message error format. However, because the default value was set to `false`, this feature was difficult to discover. `config.active_model.i18n_customize_full_message` defaults to `true` for `config.load_defaults 7.2` and above.
…` to `true` Follow-up to rails#32956 and rails#35789 The introduction of this configuration in rails#32956 stated that: > The goal of this PR is to make it easier for an app to transition from a #{attribute} #{message} to a #{message}, full_message error format. However, because the default value was set to `false`, this feature was difficult to discover. `config.active_model.i18n_customize_full_message` defaults to `true` for `config.load_defaults 7.2` and above.
Summary
The goal of this PR is to make it easier for an app to transition from a
#{attribute} #{message}to a#{message},full_messageerror format.full_messageformats error messages with a#{attribute} #{message}format which generates error messages as “name cannot be nil”.It is possible to override the format with
:"errors.format"but only language wide. https://github.com/rails/rails/blob/master/activemodel/lib/active_model/errors.rb#L371 allows a language to define a custom format, but changing it forces to extract all messages, including the implicit ones as:blankThe
#{attribute} #{message}format prevents languages to move the attribute name to somewhere else within the message, for example, in some cases it can be preferred to have error messages as“The person's name cannot be blank”, so changing the format to#{message}and including the attribute name in the message itself can be preferable.To make such transition easier, this PR allows to override
errors.formatat the attribute or model level, so an app can use either:How
This is based on
active_model/translation.rb#human_attribute_namehttps://github.com/rails/rails/blob/master/activemodel/lib/active_model/translation.rb#L44
And
active_model/errors.rb#generate_messagehttps://github.com/rails/rails/blob/master/activemodel/lib/active_model/errors.rb#L401
@base.class.respond_to?(:i18n_scope)is needed for cases whereActiveModel::Errorsis used without including theTranslationmoduleBenchmark
Results
The result with the variance between runs