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

WIP: make belongs_to polymorphic use id_method_name #384

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

doits
Copy link

@doits doits commented Jan 26, 2019

fixes #382

This is more or less a WIP to see what you guys say about this.

Some remarks:

  • There could be a short cut to not fetch the associated record in case of belongs_to without a object block. In this specific case it might be enough to use the xxx_type column and &:id_method_name on the parent. This would remove the need to instantiate the associated record in that case. Though then the fetch_id method could not be used and the belongs_to case would be a completely separated case.
  • run_key_transform is not yet cached (like in other methods)

What do you say about this in general?

I could think of a second way to make belongs_to have a complete different logic, which might speed belongs_to up though (if there is no need to fetch the association), but not sure if this is the way to go or if this makes the class too complex and if the xxx_type column is enough to get the class (or if there are some edge cases where this does not work).

@doits doits force-pushed the polymorphic_belongs_to_id_method_name branch from 29b2e83 to 75ba64a Compare January 26, 2019 13:26
@davidcelis
Copy link

This should probably also be updated to work with has_one

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

Successfully merging this pull request may close these issues.

id_method_name not respected with polymorphic association
2 participants