Repository navigation
[BUG] Nested models are not correctly extracted #229
Description
When you have a response looking like:
data => [
[
id: 1,
type: returnables,
relationships: {
parent: [
id: 2,
type: returnables,
]
}
],
[
id: 2,
type: returnables,
relationships: {
childs: [
[
id: 1,
type: returnables,
]
]
}
]
],
included: []
The relations are not correctly extracted because in json-api-datastore.service::extractQueryData() it processes only the body.included but in above example some of the relations are already in body.data. The json-api specs write:
A compound document MUST NOT include more than one resource object for each type and id pair.
So in my opinion the response is ok, and extractQueryData() should also take body.data into account.
Our current workarround is to override extractQueryData in our own datastore:
protected extractQueryData<T extends JsonApiModel>(
body: any,
modelType: ModelType<T>,
withMeta = false
): T[] | JsonApiQueryData<T> {
const models: T[] = [];
const allRawResources = body.data;
if (body.included) {
allRawResources = allRawResources.concat(data.included);
}
body.data.forEach((data: any) => {
const model: T = this.deserializeModel(modelType, data);
model.syncRelationships(data, allRawResources);
this.addToStore(model);
models.push(model);
});
if (withMeta && withMeta === true) {
return new JsonApiQueryData(models, this.parseMeta(body, modelType));
}
return models;
}
Reacted by Alexis Zucco
Activity
Alxzu commentedon Jul 4, 2019
@frans-beech-it
Hey, the code above didn't work for me, can you share how did you configure your
DatastoreConfig[models]?fsaris commentedon Jul 4, 2019
Ah sorry. I patched an older release.
This should work, not tested to be honest
safo6m commentedon Jul 18, 2019
This should be fixed in version v7.1.0
@frans-beech-it can you verify?
hpawe01 commentedon Jan 20, 2021
@safo6m This is not fixed. Or can you link a pull request?
@fsaris, the JSON:API spec also states, that
For me, this sounds like even if the related resource is already in the primary
data, it should also be in theincludedmember.But they are not so sure about it, because in the next section they state, that
and of course later the line you cited in your first post.
So I think this library should support this scenario (and it does already partly since #241 was merged) and I will provide a pull request for it.