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

Hydrate polymorphic relationships #35

Closed
jstoone opened this issue Dec 20, 2016 · 9 comments
Closed

Hydrate polymorphic relationships #35

jstoone opened this issue Dec 20, 2016 · 9 comments
Milestone

Comments

@jstoone
Copy link
Contributor

jstoone commented Dec 20, 2016

I've recently had to hydrate a polymorphic relationship, which at least to my knowledge isn't supported natively in laravel-json-api, so after some tinkering I've written down some implementation ideas which I'd gladly compile down to a PR.

But I see that there is a lot of pending work pending regarding relationships. So I was wondering if there is anything that you see as a higher priority task, that I can help you get started with?

I use your library every day at work, and I would love to help keep the ball rolling, and improve on this already amazing product. As that would benefit everyone.

So in a more informal sense I'm saying this: jstoone at your service, what can I help you with?

@lindyhopchris
Copy link
Member

Ah, great glad to hear you're using it on a daily basis too! I'm also using it on a daily basis, both in work and a personal project. But the problem I'm having as a result is I haven't had much time to work on taking it forward and plugging the gaps it's got at the moment. Your help would be very much appreciated!!

There's a number of things I had in mind for improving the relationships stuff - a lot around supporting the various JSON API endpoints for the relationships. I'll jot those down on a different issue or milestone.

In the meantime what's your implementation idea for hydrating polymorphic relationships? It would be nice to add that in to the current EloquentHydrator, so would be interested to hear your thoughts.

@jstoone
Copy link
Contributor Author

jstoone commented Dec 22, 2016

It would be really neat to have a milestone or issue that lists which features and enhancements that are scheduled for v0.6. In that way we can more easily scope out implementation suggestions in separate issues which we then reference in the milestone or issue.

I've caught a gotcha regarding the polymorphic relationships, which relates to the general order in which relationships are handled, as mentioned in #29 (when creating a new resource).
As you mention, there are a few gaps when it comes to relationships, and maybe it's worth considering that a (big) refactor of everything "relationship" may be needed to get everything to "click, so to say.

But as of now I'd need to do more source diving to become more familiar with the core of the project. And that's why I'm looking forward to look more into solving hydrating polymorphic relationships. And yes my tinkering has been in the EloquentHydrator.

@lindyhopchris
Copy link
Member

@jstoone Happy New Year!

Sorry for lack of response - I've (intentionally) been offline over Christmas and the New Year.

I do have a milestone here for the next version, though it's just things I've thought of so far:
https://github.com/cloudcreativity/laravel-json-api/milestone/2

Lets use this issue here to discuss what needs refactoring to support relationships a lot better, then from that we can create separate issues for each of the bits of work and update the above milestone.

Here's what I think are the requirements for each of the controller "relationship" end-points on the EloquentController. All the following need to support "normal" and "polymorphic" relationships.

readRelatedResource

GET /posts/{resource_id}/{relationship_name}

  • Has-one: returns related resource or null; can do this from the primary resource without any need to "search".
  • Has-many: hands-off to search class to allow relationship to be paginated, filtered, etc. I.e. the search class needs to have a method for dealing with relationships.
  • Has-one test helpers
  • Has-many test helpers

readRelationship

GET /posts/{resource_id}/relationships/{relationship_name}

Same as previous, but the end-point returns the relationship identifiers rather than the actual related resources. I.e. this uses the same functionality as the previous one but passes the results to a different response helper to generate identifiers instead of resources.

  • Has-one identifiers test helpers
  • Has-many identifiers test helpers

replaceRelationship

PATCH /posts/{resource_id}/relationships/{relationship_name}

  • Has one: validates incoming request, hydrates the relationship and then returns the new value.
  • Has many: validates incoming request, hydrates the entire relationship, i.e. replaces it entirely.
  • Before/after hooks
  • Has one replacement test helpers
  • Has many replacement test helpers

addToRelationship

POST /posts/{resource_id}/relationships/{relationship_name}

  • Has one: rejects as invalid
  • Has many: validates incoming request, adds specified resources to the relationship
  • Before/after hooks
  • Has many addition test helpers

removeFromRelationship

DELETE /posts/{resource_id}/relationships/{relationship_name}

  • Has one: rejects as invalid
  • Has many: validates incoming request, removes specified resources from the relationship.
  • Before/after hooks
  • Has many removal test helpers

General ambition...

I'd quite like to make the SearchInterface non-Eloquent specific, and then have an EloquentSearch class instance. This would bring it into line with e.g. the hydrator, where the generic interface is the "process" and then there's a specific Eloquent implementation. At the moment, the "search" process is the only process that is specifically tied to Eloquent.

Let me know your thoughts!

@danherd
Copy link

danherd commented Sep 7, 2017

Hi,

Sorry to be that guy but I was just wondering how much progress has been made on this particular feature of the 1.0 roadmap? It would seem to be to be the biggest and most complex issue (and the biggest loss with regards to adherence to the JSON API specification).

If there's anything I can do to help, please let me know. I've been using this package so much I reckon I'll be comfortable enough digging into the internals now!

Cheers.

@lindyhopchris
Copy link
Member

@danherd no problem asking!

I'm going to have another push on the 1.0 roadmap late September/early October. The main thing I need to sort out is how to handle querying relationships and handing off querying and validation between different resource types. I.e. for a comments relationship on a posts resource, I need to hand the query off to the comments adapter and validator.

Working out how to do that, and updating hydration logic, has been the most complex stuff to work out. But I've got a design concept now and will start on it later this month. The plan is to sort out polymorphism at the same time (or immediately after).

Once I've got the design of that done I might take up your offer to help implement it. Will let you know.

@GregPeden
Copy link
Contributor

GregPeden commented Sep 11, 2017

In the meantime, I have been working around this by including API models for what is really just the polymorphic relationship table. One can create objects on the relationship table and then they function as polymorphic relationships.

@lindyhopchris
Copy link
Member

That sounds like a neat work around for the mo.

@lindyhopchris
Copy link
Member

Closing this as hydration of polymorphic relationships is now on the 1.x-dev branch.

@peripteraptos
Copy link

peripteraptos commented Feb 14, 2018

@SirLamer could you explain a little bit more specific how you did this, maybe with an example? That would be really nice, because i'm also working on polymorphic relationships (comments) and can't get it to work.

Edit: Ahh, you meant filling the e.g "commentable_id" and "commentable_type" by hand

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

No branches or pull requests

5 participants