Automatically serialize belongsToMany into relation meta attribute#74
Automatically serialize belongsToMany into relation meta attribute#74jamesdixon wants to merge 2 commits intoscoutforpets:masterfrom
Conversation
|
Another to point to mention: Currently, the attributes returned as part of meta are not affected by the serializer's |
|
@chamini2 to clarify, the PR allows for pivot data (data contained in join tables) to be automatically serialized into Imagine that I have a three tables: |
|
So it's for |
|
Correct. The description in the test is a typo :) |
|
@chamini2 any comment on this other than the typo in the test name? |
|
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. |
|
No rush on it at all, so please take your time. I'd prioritize the plugin-related tickets before this. |
chamini2
left a comment
chamini2
left a comment
There was a problem hiding this comment.
Add documentation for the new option
| return template; | ||
| } | ||
|
|
||
| function relationshipMeta(relation, models) { |
There was a problem hiding this comment.
I would not declare this relationshipMeta function here, because it is just the default function to use in case none was passed.
I would declare it where it is being used as a default.
| let { bookOpts, linkOpts }: Information = info; | ||
| let { bookOpts, linkOpts, serialOpts }: Information = info; | ||
| let { enableLinks }: BookOpts = bookOpts; | ||
|
|
There was a problem hiding this comment.
Adding it here, like:
const defaultRelationshipMeta = function (relation, models) {
if (isArray(models)) { ... }
}
const { relationshipMeta = defaultRelationshipMeta } = seralOpts;| expect(_.matches(expected)(result)).toBe(true); | ||
|
|
||
| }); | ||
|
|
There was a problem hiding this comment.
Add a test for the attribute omission done
|
Maybe rebase from master? |
2 participants
This PR adds the ability to automatically serialize pivot/join table data returned by Bookshelf into a relationships'
metaattribute forbelongsToManyrelations.The serializer already has a
relationshipMetaoption that accepts an object containing a string or function. This PR adds a function that is passed torelationshipMetaby default that will serialize pivot data into the relationships'metaattribute. This function can be overridden by passing your own function torelationshipMetaas an option.A few questions/comments:
relationshipMetafunction 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?toJSONmethod were made to add pivot data to the model that's ultimately passed into the function provided torelationshipMeta. 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.