scoutforpets / jsonapi-mapper

JSON API-Compliant Serialization for your Node ORM
The Unlicense
42 stars 24 forks source link

Automatically serialize belongsToMany into relation meta attribute #74

Open jamesdixon opened 8 years ago

jamesdixon commented 8 years ago

This PR adds the ability to automatically serialize pivot/join table data returned by Bookshelf into a relationships' meta attribute for belongsToMany relations.

The serializer already has a relationshipMeta option that accepts an object containing a string or function. This PR adds a function that is passed to relationshipMeta by default that will serialize pivot data into the relationships' meta attribute. This function can be overridden by passing your own function to relationshipMeta as an option.

A few questions/comments:

jamesdixon commented 8 years ago

Another to point to mention:

Currently, the attributes returned as part of meta are not affected by the serializer's typeForAttribute option. In my case, everything should be camelCased, but those attributes are snake_case.

jamesdixon commented 8 years ago

@chamini2 to clarify, the PR allows for pivot data (data contained in join tables) to be automatically serialized into meta data on a relationship.

Imagine that I have a three tables: appointment, appointment_service and appointment_pet. The latter two tables are join tables that associate services and pets with an appointment. However, what if I needed to specify that certain pets are only applicable to a specific service? For example, I have a feeding service that should only apply to the dog, Max. The place to specify that information would be another field in appointment_service -- let's call it specific_pets. According to the JSON API folks, the place to put this type of information would be the meta attr of the relation. This PR enables serialization of those pivot/join properties into the meta attr of the relation.

chamini2 commented 8 years ago

So it's for belongsToMany relations. But the test I see in this PR talks about a belongsTo relation, should it be changed to belongsToMany then?

jamesdixon commented 8 years ago

Correct. The description in the test is a typo :)

jamesdixon commented 8 years ago

@chamini2 any comment on this other than the typo in the test name?

chamini2 commented 8 years ago

Actually, I haven't reviewed it yet! I tried to understand the feature with the spec and since I'm not entirely sure how pivot tables work I can't really review it right now. I was waiting for some time available to understand what's accomplished here! 😄

Are you needing this at the moment? I can take a look for next monday for sure.

jamesdixon commented 8 years ago

No rush on it at all, so please take your time. I'd prioritize the plugin-related tickets before this.

chamini2 commented 7 years ago

Maybe rebase from master?