Don't encourage usage of columns_hash
#15976
Conversation
As discussed in heartcombo/simple_form#1094, we should not encourage usage of `columns_hash`, and instead provide an alternate method to determine whether or not an attribute exists. The language `attribute` was chosen over `column` since these are in the `AttributeMethods` module.
| @@ -202,12 +202,17 @@ def column_for_attribute(name) | |||
| if column.nil? | |||
| ActiveSupport::Deprecation.warn(<<-MESSAGE.strip_heredoc) | |||
| `column_for_attribute` will return a null object for non-existent columns | |||
There was a problem hiding this comment.
This message might be too technical for the average user, with terms like "null object" and such. (The ordinary developer would probably just read it as "returning nil" here.)
Maybe something like...
You are seeing this deprecation message because you called `column_for_attribute` with the attribute
name "#{name}", which does not have a corresponding database column. In Rails 5.0, calling
`column_for_attribute` with a non-existent column name will return a special column object instead of
`nil`. If you are using this method to check for an attribute's existence, you should use
`attribute_exists?` instead.
It could be done in a separate PR though. @sgrif does that seem good to you?
There was a problem hiding this comment.
Haven't we referred to the null object pattern elsewhere (e.g. NullRelation)? That seems fine, though.
There was a problem hiding this comment.
As a general principle, I think we should aim to require the least amount technical literacy required to comprehend these messages, because people need to understand them to take action (when I added this message @jeremy made me add an example of "circular reference").
In this case though, my main concern is that "return a null object" sounds too similar to "returning nil", so it might be extra confusing in this context :)
Don't encourage usage of `columns_hash`
As discussed in heartcombo/simple_form#1094,
we should not encourage usage of
columns_hash, and instead provide analternate method to determine whether or not an attribute exists.
The language
attributewas chosen overcolumnsince these are in theAttributeMethodsmodule.