Skip to content

Add support for creating resources with polymorphic has one.#274

Closed
barelyknown wants to merge 5 commits intoJSONAPI-Resources:masterfrom
togglepro:polymorphic
Closed

Add support for creating resources with polymorphic has one.#274
barelyknown wants to merge 5 commits intoJSONAPI-Resources:masterfrom
togglepro:polymorphic

Conversation

@barelyknown
Copy link
Copy Markdown
Collaborator

This simply adds to PR https://github.com/cerebris/jsonapi-resources/pull/255 from @ggordon. I added support for creating resources with a polymorphic has_one. I don't like the code, but I wanted to change as little as possible for now since I need this feature ASAP. We should refactor to handle the creation of all has_one relationships the same way (whether or not they're polymorphic), but I think that we can merge prior to that since we'll have good test coverage and the public API feels right.

Comment thread lib/jsonapi/request.rb Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd prefer to see the HasOne check first, then check association.polymorphic? within that block. We could then follow suit for HasMany relationships later.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Ok, I added that.

@dgeb
Copy link
Copy Markdown
Contributor

dgeb commented Jun 30, 2015

I'd eventually like to allow restrictions on possible types in a polymorphic relationship. This doesn't have to make it into this PR, but we should think about the interface. I suppose it could be a separate param from polymorphic: true, such as allowed_types.

@dgeb
Copy link
Copy Markdown
Contributor

dgeb commented Jun 30, 2015

All in all, this looks like a promising start on polymorphism - nice work @ggordon and @barelyknown!

@barelyknown
Copy link
Copy Markdown
Collaborator Author

I'm going to take a crack at support for has_many polymorphism now. We'll see how it goes.

This adds support for the serialization of `has_many` relationships. I'm not sure if it handles every situation that people are imagining but it addresses the primary issue that I saw which was the need to return the correct type for the individual resources. As with the solution for polymorphic `has_one` relationships, you need to add a `Resource` and `Controller` class for each potential type. I didn't address anything new related to creating or updating `has_many` relationships in this commit.
@barelyknown
Copy link
Copy Markdown
Collaborator Author

That last commit adds polymorphism to the "read" side of has_many relationships. I'm not sure why there are merge conflicts - I'm not seeing that on my side.

@ggordon
Copy link
Copy Markdown
Contributor

ggordon commented Jul 2, 2015

@barelyknown I had merge conflicts in #255 with the Readme, from PR #277, those are probably the same.

@barelyknown
Copy link
Copy Markdown
Collaborator Author

Ugh. The conflicts are a mess now. I'll see if I can work it out.

@lgebhardt
Copy link
Copy Markdown
Contributor

Sorry, I know. I'm going through them now. So if you want to hold off I'll see if I can get it all resolved.

@lgebhardt
Copy link
Copy Markdown
Contributor

I pushed the new rebase/274 branch up.

I hope I got it all correct. All the tests pass, and there are the same two lines that are not covered.

@barelyknown
Copy link
Copy Markdown
Collaborator Author

Oh, good. Thanks for doing that - I know that it was a pain!

@lgebhardt
Copy link
Copy Markdown
Contributor

@barelyknown It was my fault for not merging this earlier.

@lgebhardt
Copy link
Copy Markdown
Contributor

Thanks!

Merged by #288

@arcreative
Copy link
Copy Markdown
Contributor

arcreative commented Feb 13, 2017

@lgebhardt I'm having an issue on 0.8.3 where trying to include the polymorphic relationship results in JSONAPI: Could not find resource 'subject'. (Class Api::V1::SubjectResource not found). The resource is supposed to be variable, not understanding why it's looking for SubjectResource when polymorphic is set to true... If I force the type with has_one :subject, polymorphic: true, class_name: 'Feedback', the included record has a blank type (data[0].relationships.subject = {type: "", id: "76"}). Either I'm doing something wrong, or...

Resource definition:
has_one :subject, polymorphic: true
Fails with JSONAPI: Could not find resource 'subject'. (Class Api::V1::SubjectResource not found
has_one :subject, polymorphic: true, class_name: 'Feedback' # Obviously not ideal, just testing
Returns invalid payload when including resource, e.g. data[0].relationships.subject = {type: "", id: "76"}

Model definition:
belongs_to :subject, polymorphic: true
All polymorphic stuff seems to otherwise be working normally

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.

5 participants