jsonapi-mapper icon indicating copy to clipboard operation
jsonapi-mapper copied to clipboard

Automatically serialize belongsToMany into relation meta attribute

Open jamesdixon opened this issue 9 years ago • 8 comments

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:

  • Do we want serialization of pivot data to occur automatically? or should we provide an option to disable?
  • If a user wants to override the relationshipMeta function with their own method, do we couple this with the option to enable or disable serialization of belongsToMany pivot data to allow chaining of the provided method and the method we provide so they don't have to provide their own function for serializing belongsToMany relations into metadata?
  • Some modifications to the toJSON method were made to add pivot data to the model that's ultimately passed into the function provided to relationshipMeta. Currently, .serialize({ shallow: true }) will not include pivot data. I looked at the Bookshelf code (https://github.com/tgriesser/bookshelf/blob/9c7b56cb3145930d56c55b07ddf4d36a702e31b3/src/base/model.js#L263) for this and it doesn't appear that there would be an adverse affect for allowing pivot data to be included even when doing a shallow serialization, but for now, I've done the work on our end.

jamesdixon avatar Sep 23 '16 20:09 jamesdixon

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 avatar Sep 23 '16 21:09 jamesdixon

@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.

jamesdixon avatar Sep 27 '16 19:09 jamesdixon

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?

chamini2 avatar Sep 27 '16 20:09 chamini2

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

jamesdixon avatar Sep 27 '16 21:09 jamesdixon

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

jamesdixon avatar Oct 04 '16 23:10 jamesdixon

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.

chamini2 avatar Oct 04 '16 23:10 chamini2

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

jamesdixon avatar Oct 04 '16 23:10 jamesdixon

Maybe rebase from master?

chamini2 avatar May 23 '17 21:05 chamini2