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

ActiveRecord::AttributeMethods#column_for_attribute doesn't work with alias_attribute and behaves strangely with nonexistent attribute #15877

Closed
Incanus3 opened this issue Jun 23, 2014 · 4 comments · Fixed by #15878
Assignees

Comments

@Incanus3
Copy link

When model.column_for_attribute is called for aliased or nonexistent attribute name, it returns an instance of ActiveRecord::ConnectionAdapters::NullColumn. For nonexistent attribute this is ok, but column.name shouldn't return the nonexistent name - the caller needs to know the column doesn't exist. For aliased column, this should return real ActiveRecord::ConnectionAdapters::Column with the right column name, type, etc.

I've stumbled upon this when creating a form using simple_form, which autodetects input types to use by calling column_for_attribute on the object.

In rails 4.0 column_for_attribute returns nil in these cases, which also breaks the autodetection for aliased attributes, but at least you notice it when calling .type on it.

I've created a standard test for this:
https://gist.github.com/Incanus3/3210403c996c5de2b297

This is also mentioned in this old, closed issue:
#6177

@sgrif
Copy link
Contributor

sgrif commented Jun 23, 2014

This seemed to be an innocuous change, since there are numerous other places to check if a column exists. If you need a list of existent columns, you should probably go through column_names, columns, columns_hash, etc. However, we should probably go through a deprecation cycle on this method. @senny, WDYT?

@sgrif sgrif self-assigned this Jun 23, 2014
@Incanus3
Copy link
Author

You're probably right. My primary concern was that this doesn't work for aliased attributes, only after creating the test for this and trying it on master (instead of 4.0.5) I found out that now it returns the NullColumn, which returns the nonexistent column name for .name, which seemed strange to me so I included that info.

@sgrif
Copy link
Contributor

sgrif commented Jun 23, 2014

Can you expand upon what you mean by "doesn't work for aliased attributes"?

@Incanus3
Copy link
Author

Well, as the gist shows, when you call model.column_for_attribute(:aliased_attribute_name) it returns NullColumn (or nil in 4.0), whereas it would be nice, if it returned the same thing as model.column_for_attribute(:original_attribute_name).
In my case, I'm building a rails app on legacy db, shared with some other app, so I use alias_attribute a lot. The thing is, I don't want any other code apart from the model itself to know, that the actual column is named differently.
Aliased attribute names work for create, update, etc. They don't work for find, where and pluck but this can be worked around using scopes, custom finders, etc. But I consider column_for_attribute a part of the introspection API, which is why I think it should take attribute aliases into consideration.

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

Successfully merging a pull request may close this issue.

3 participants