Adding Model#model_name

rubyonrails-core
1

Following up on https://github.com/rails/rails/issues/15428, with adjustments.

Throughout the Rails code there are a few references to object.class.model_name. Instead of always pushing this up to the class, it makes sense to me that we ask the model for its model_name.

This might obviate the need for the dubious ActionController::ModelNaming#model_name_from_record_or_class, https://github.com/rails/rails/blob/master/actionpack/lib/action_controller/model_naming.rb


module ActionController

module ModelNaming

# Converts the given object to an ActiveModel compliant one.

def convert_to_model(object)

object.respond_to?(:to_model) ? object.to_model : object

end

def model_name_from_record_or_class(record_or_class)

(record_or_class.is_a?(Class) ? record_or_class : convert_to_model(record_or_class).class).model_name

end

end

end

Example:

class Foo extend ActiveModel::Naming def model_name class.model_name end end

Foo.respond_to?(:model_name) => true

Foo.new.respond_to?(:model_name) => true

Foo.model_name == Foo.new.model_name => true

2

Yes, please!

3

I wouldn’t be in favour of this - purely because I’m generally against adding instance methods to ActiveRecord::Base in case they override attributes defined on the table. For example, a photography studio may have a photos table which imports from somewhere and has a “model_name” column.

I don’t see the issue with having it on the class myself.

Cheers,

Andy

4

I’m very much in favor of this as well. Please make it so. To address Andy’s concerns, we should have proper error messages if this is overwriting an attribute of the same name. But it’s too convenient not to have.

5

Pull request sent: https://github.com/rails/rails/pull/15871

6

Awesome you guys.

7

Excellent start Yuki!

I started working on this as well on Friday and came to pretty much the same solution.

I would have just left my changes in favor of Yuki’s PR, but I think is misses the point.

The idea is for rails to stop calling x.class.model_name, and only call x.model_name.

I’ve started another PR making those changes: https://github.com/rails/rails/pull/15889

There’s still two things to address:

  1. I’ve left a few REVIEW comments. I’m looking for feedback there, but will remove them and rebase soon enough.
  2. I’m still working on a warning to address the issue that Andy Jeffries brought up in this thread.

I’m thinking it should just be a warning. I’m digging into it now; does anyone know of an example of rails currently doing this?

Thanks!

-Amiel

8

Sorry for replying an old post =(. But, I am moving an app from Rails 4.1.x to Rails 4.2.4 and I am with error wher I use ActiveModel::Erros.new(object) when the object it is not an activemodel. I am looking on the rails source code and the ActiveModel::Error has changed from 4.1.x to 4.2.4 rails/errors.rb at v4.2.4 · rails/rails · GitHub with this PR. This method on the line 437 (rails/errors.rb at v4.2.4 · rails/rails · GitHub) is trying to look for the method model_name from the instance, but in the version 4.1.x it looks from the class. It was changed on this commit: Use #model_name on instances instead of classes · rails/rails@6b0e834 · GitHub.

I know that I can fix it by creating a module that I can use on my class with this method using the ActiveModel::Naming. Am I wrong in use ActiveModel::Error without an activemodel? And the older way is not right in use the method from class?

Thanks so much, and sorry for opening an old topic again =/