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] Add support for polymorphic associations. #1453

Closed
wants to merge 1 commit into from

Conversation

beauby
Copy link
Contributor

@beauby beauby commented Jan 20, 2016

Properly choose the association key for polymorphic associations.

@joaomdmoura
Copy link
Member

I know this seems a [WIP] but I'll point it out just to make sure we don't forget: tests are missing
Updated the title to add WIP flag

@@ -78,6 +78,17 @@ def build_association(subject, parent_serializer_options)

private

def polymorphic_key(assoc_value)
reflection_key = assoc_value.class.model_name
assoc_value.respond_to?(:each) ? reflection_key.plural : reflection_key.singular
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😘 what I need, I'm merging this in a branch of mine 😁

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, tell me whether this worked! (usage is has_one :addressable, polymorphic: true)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works like a charm ;)

@joaomdmoura joaomdmoura changed the title Add support for polymorphic associations. [WIP] Add support for polymorphic associations. Jan 20, 2016

def reflection_options(assoc_value, options)
reflection_options = options.dup
reflection_options[:key] = polymorphic_key(assoc_value) if options[:polymorphic] && !options.key?(:key)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: We might want to make it a ||= in order not to override user-provided options (instead of testing for !options.key?(:key)).

@beauby
Copy link
Contributor Author

beauby commented Jan 22, 2016

After giving it a thought, I'm not sure this feature really makes sense (especially in the case of to-many relationships and empty to-ones).

@beauby
Copy link
Contributor Author

beauby commented Mar 17, 2016

Closing, this does not make any sense.

@beauby beauby closed this Mar 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants