Papertrail is using super class instead of class #146

Closed
tscolari opened this Issue Mar 26, 2012 · 5 comments

Comments

Projects
None yet
4 participants
@tscolari

Hello,

I have a module named Page
and a Article that inherits from Page.

I inserted papartrail on the Articles module, but, when the version is created, it does not use 'Article' as type, but 'Page'.

Is there any work around for this?
I believe that the correct behavior would be storing 'Article' in the column in this situation.

@airblade

This comment has been minimized.

Show comment
Hide comment
@airblade

airblade Mar 27, 2012

Collaborator

Is Page a subclass of ActiveRecord::Base? And is Article a subclass of Page?

(I'm not sure whether you're talking about modules or classes.)

Collaborator

airblade commented Mar 27, 2012

Is Page a subclass of ActiveRecord::Base? And is Article a subclass of Page?

(I'm not sure whether you're talking about modules or classes.)

@tscolari

This comment has been minimized.

Show comment
Hide comment
@tscolari

tscolari Mar 29, 2012

Everything is a model :)

I think it's more an active_record issue, I believe it calls for class.super_class.name and not class.name when creating polymorphics. It just seems an incorrect behavior for me, in this case at least.

Everything is a model :)

I think it's more an active_record issue, I believe it calls for class.super_class.name and not class.name when creating polymorphics. It just seems an incorrect behavior for me, in this case at least.

@airblade

This comment has been minimized.

Show comment
Hide comment
@airblade

airblade Mar 30, 2012

Collaborator

PaperTrail should handle STI. When you reify a version of an article, what class is the reified object?

Collaborator

airblade commented Mar 30, 2012

PaperTrail should handle STI. When you reify a version of an article, what class is the reified object?

@solilin

This comment has been minimized.

Show comment
Hide comment
@solilin

solilin Dec 17, 2012

Hm... and what should do if event == 'create' and object is nil? And by the way reify return redundant db query I think it is not good. Why we can't keep STI class in item_type column?

solilin commented Dec 17, 2012

Hm... and what should do if event == 'create' and object is nil? And by the way reify return redundant db query I think it is not good. Why we can't keep STI class in item_type column?

@batter

This comment has been minimized.

Show comment
Hide comment
@batter

batter Feb 28, 2013

Collaborator

This is how ActiveRecord behaves, and PaperTrail uses ActiveRecord for it's association handling. If you have an objection with how ActiveRecord handles associations in conjunction with STI, please file it against the ActiveRecord repo.

For the record, I don't disagree with your assertion, but I believe that the rationale behind the current implementation is that since all the records for a STI object are held in a single table, the optimal method of querying for the associated objects is simply to select by id, since the id attribute is indexed, and I would imagine that adding a type restrictor to the query probably doesn't actually optimize the query (or at least not enough to make it worth it).

Collaborator

batter commented Feb 28, 2013

This is how ActiveRecord behaves, and PaperTrail uses ActiveRecord for it's association handling. If you have an objection with how ActiveRecord handles associations in conjunction with STI, please file it against the ActiveRecord repo.

For the record, I don't disagree with your assertion, but I believe that the rationale behind the current implementation is that since all the records for a STI object are held in a single table, the optimal method of querying for the associated objects is simply to select by id, since the id attribute is indexed, and I would imagine that adding a type restrictor to the query probably doesn't actually optimize the query (or at least not enough to make it worth it).

@batter batter closed this Feb 28, 2013

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