ActiveRecord builds instance of wrong class through a scope targeting an STI class #7021

Closed
steveluscher opened this Issue Jul 10, 2012 · 8 comments

Comments

Projects
None yet
3 participants
@steveluscher

I would like to be able to call the build method on a scope that targets a certain class of model via its STI type, and have ActiveRecord build an instance of the correct class.

class Post < ActiveRecord::Base
  scope :special, where(type: 'SpecialPost')
end

class SpecialPost < Post; end

> Post.special.build # Expect an instance of SpecialPost here
=> #<Post ...>

Here, I expected an instance of SpecialPost, not an instance of Post.

@carlosantoniodasilva

This comment has been minimized.

Show comment
Hide comment
@carlosantoniodasilva

carlosantoniodasilva Jul 10, 2012

Member

This does not work, and I believe it's right the way it is. Setting the type attribute in a scope doesn't mean Active Record will build that particular type object when using STI, it only means it'll set that type attribute when building your post.

So, doing Post.special.build should give you a Post with a SpecialPost type, nothing more. If you save and reload, you'd probably get a SpecialPost instead.. And if you want a new instance of SpecialPost, you'd have to do SpecialPost.build instead.

Thanks!

This does not work, and I believe it's right the way it is. Setting the type attribute in a scope doesn't mean Active Record will build that particular type object when using STI, it only means it'll set that type attribute when building your post.

So, doing Post.special.build should give you a Post with a SpecialPost type, nothing more. If you save and reload, you'd probably get a SpecialPost instead.. And if you want a new instance of SpecialPost, you'd have to do SpecialPost.build instead.

Thanks!

@steveluscher

This comment has been minimized.

Show comment
Hide comment
@steveluscher

steveluscher Jul 10, 2012

I respectfully disagree. Using the exact same code above:

> Post.special.first
=> #<SpecialPost ...>
> Post.special.build
=> #<Post ...>

Surely we can't advocate for a design where the association getter does one thing, and the association builder does another.

I respectfully disagree. Using the exact same code above:

> Post.special.first
=> #<SpecialPost ...>
> Post.special.build
=> #<Post ...>

Surely we can't advocate for a design where the association getter does one thing, and the association builder does another.

@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Jul 10, 2012

Member

special is an association or a scope? if it is a scope this behavior is right.

Member

rafaelfranca commented Jul 10, 2012

special is an association or a scope? if it is a scope this behavior is right.

@steveluscher

This comment has been minimized.

Show comment
Hide comment
@steveluscher

steveluscher Jul 10, 2012

You are correct that I misspoke. I meant to say “scope getter does one thing, and the scope builder does another.”

My claim stands: for Model.scope.first to return an instance of one class and Model.scope.build to return an instance of a different class breaks my expectation of what that scope represents.

You are correct that I misspoke. I meant to say “scope getter does one thing, and the scope builder does another.”

My claim stands: for Model.scope.first to return an instance of one class and Model.scope.build to return an instance of a different class breaks my expectation of what that scope represents.

@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Jul 10, 2012

Member

A scope can't know anything about STI. Scopes always return the caller class instance when you are using the build method, this is expected behavior.

See https://github.com/rails/rails/blob/master/activerecord/lib/active_record/relation.rb#L79

Member

rafaelfranca commented Jul 10, 2012

A scope can't know anything about STI. Scopes always return the caller class instance when you are using the build method, this is expected behavior.

See https://github.com/rails/rails/blob/master/activerecord/lib/active_record/relation.rb#L79

@steveluscher

This comment has been minimized.

Show comment
Hide comment
@steveluscher

steveluscher Jul 12, 2012

So, are we sure that we don't want this to work?

class Citizen
  scope :canadian, where(type: 'CanadianPerson')
  scope :american, where(type: 'AmericanPerson')
  scope :swedish,  where(type: 'SwedishPerson')
end

nationality = [:canadian, :american, :swedish, ...]

p = Citizen(nationality.sample).build
=> #<CanadianCitizen ...>

So, are we sure that we don't want this to work?

class Citizen
  scope :canadian, where(type: 'CanadianPerson')
  scope :american, where(type: 'AmericanPerson')
  scope :swedish,  where(type: 'SwedishPerson')
end

nationality = [:canadian, :american, :swedish, ...]

p = Citizen(nationality.sample).build
=> #<CanadianCitizen ...>
@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Jul 12, 2012

Member

We can make this work. But right now it is not an issue. You can either open a pull request or discuss in the Rails Core mailing list.

Member

rafaelfranca commented Jul 12, 2012

We can make this work. But right now it is not an issue. You can either open a pull request or discuss in the Rails Core mailing list.

@steveluscher

This comment has been minimized.

Show comment
Hide comment
@steveluscher

steveluscher Dec 3, 2012

Rails 4.0 seems to have rolled this in.

Rails 4.0 seems to have rolled this in.

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