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

[BUGFIX] EmbeddedRecordMixin should include the type serializing hasMany as ids #3848

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions FEATURES.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,27 @@ entry in `config/features.json`.

Enables `pushPayload` to return the model(s) that are created or
updated via the internal `store.push`. [PR 4110](https://github.com/emberjs/data/pull/4110)

- `ds-serialize-ids-and-types`

Enables a new `ids-and-type` strategy (in addition to the already existing `ids` and `records`) for
serializing has many relationships using the `DS.EmbeddedRecordsMixin` that will include both
`id` and `type` of each model as an object.

For instance, if a use has many pets, which is a polymorphic relationship, the generated payload would be:

```js
{
"user": {
"id": "1"
"name": "Bertin Osborne",
"pets": [
{ "id": "1", "type": "Cat" },
{ "id": "2", "type": "Parrot"}
]
}
}
```

This is particularly useful for polymorphic relationships not backed by STI when just including the id
of the records is not enough.
97 changes: 91 additions & 6 deletions addon/serializers/embedded-records-mixin.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Ember from 'ember';
import { warn } from "ember-data/-private/debug";
import isEnabled from 'ember-data/-private/features';

var get = Ember.get;
var set = Ember.set;
Expand Down Expand Up @@ -235,7 +236,7 @@ export default Ember.Mixin.create({
},

/**
Serialize `hasMany` relationship when it is configured as embedded objects.
Serializes `hasMany` relationships when it is configured as embedded objects.

This example of a post model has many comments:

Expand Down Expand Up @@ -285,7 +286,7 @@ export default Ember.Mixin.create({
```

The attrs options object can use more specific instruction for extracting and
serializing. When serializing, an option to embed `ids` or `records` can be set.
serializing. When serializing, an option to embed `ids`, `ids-and-types` or `records` can be set.
When extracting the only option is `records`.

So `{ embedded: 'always' }` is shorthand for:
Expand Down Expand Up @@ -314,6 +315,58 @@ export default Ember.Mixin.create({
}
```

To embed the relationship as a collection of objects with `id` and `type` keys, set
`ids-and-types` for the related object.

This is particularly useful for polymorphic relationships where records don't share
the same table and the `id` is not enough information.

By example having a user that has many pets:

```js
User = DS.Model.extend({
name: DS.attr('string'),
pets: DS.hasMany('pet', { polymorphic: true })
});

Pet = DS.Model.extend({
name: DS.attr('string'),
});

Cat = Pet.extend({
// ...
});

Parrot = Pet.extend({
// ...
});
```

```app/serializers/user.js
import DS from 'ember-data;

export default DS.RESTSerializer.extend(DS.EmbeddedRecordsMixin, {
attrs: {
pets: { serialize: 'ids-and-types', deserialize: 'records' }
}
});
```

```js
{
"user": {
"id": "1"
"name": "Bertin Osborne",
"pets": [
{ "id": "1", "type": "Cat" },
{ "id": "1", "type": "Parrot"}
]
}
}
```

Note that the `ids-and-types` strategy is still behind the `ds-serialize-ids-and-types` feature flag.

@method serializeHasMany
@param {DS.Snapshot} snapshot
@param {Object} json
Expand All @@ -325,16 +378,42 @@ export default Ember.Mixin.create({
this._super(snapshot, json, relationship);
return;
}
var includeIds = this.hasSerializeIdsOption(attr);
var includeRecords = this.hasSerializeRecordsOption(attr);
if (includeIds) {

if (this.hasSerializeIdsOption(attr)) {
let serializedKey = this.keyForRelationship(attr, relationship.kind, 'serialize');
json[serializedKey] = snapshot.hasMany(attr, { ids: true });
} else if (includeRecords) {
} else if (this.hasSerializeRecordsOption(attr)) {
this._serializeEmbeddedHasMany(snapshot, json, relationship);
} else {
if (isEnabled("ds-serialize-ids-and-types")) {
if (this.hasSerializeIdsAndTypesOption(attr)) {
this._serializeHasManyAsIdsAndTypes(snapshot, json, relationship);
}
}
}
},

/**
Serializes a hasMany relationship as an array of objects containing only `id` and `type`
keys.
This has its use case on polymorphic hasMany relationships where the server is not storing
all records in the same table using STI, and therefore the `id` is not enough information

TODO: Make the default in Ember-data 3.0??
*/
_serializeHasManyAsIdsAndTypes(snapshot, json, relationship) {
var serializedKey = this.keyForAttribute(relationship.key, 'serialize');
var hasMany = snapshot.hasMany(relationship.key);

json[serializedKey] = Ember.A(hasMany).map(function (recordSnapshot) {
//
// I'm sure I'm being utterly naive here. Propably id is a configurate property and
// type too, and the modelName has to be normalized somehow.
//
return { id: recordSnapshot.id, type: recordSnapshot.modelName };
});
},

_serializeEmbeddedHasMany(snapshot, json, relationship) {
var serializedKey = this._getMappedKey(relationship.key, snapshot.type);
if (serializedKey === relationship.key && this.keyForRelationship) {
Expand Down Expand Up @@ -420,6 +499,12 @@ export default Ember.Mixin.create({
return option && (option.serialize === 'ids' || option.serialize === 'id');
},

// checks config for attrs option to serialize records as objects containing id and types
hasSerializeIdsAndTypesOption(attr) {
var option = this.attrsOption(attr);
return option && (option.serialize === 'ids-and-types' || option.serialize === 'id-and-type');
},

// checks config for attrs option to serialize records
noSerializeOptionSpecified(attr) {
var option = this.attrsOption(attr);
Expand Down
3 changes: 2 additions & 1 deletion config/features.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@
"ds-finder-include": null,
"ds-references": null,
"ds-transform-pass-options": null,
"ds-pushpayload-return": null
"ds-pushpayload-return": null,
"ds-serialize-ids-and-types": null
}
69 changes: 58 additions & 11 deletions tests/integration/serializers/embedded-records-mixin-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ import Ember from 'ember';
import {module, test} from 'qunit';

import DS from 'ember-data';
import isEnabled from 'ember-data/-private/features';

var get = Ember.get;
var HomePlanet, SuperVillain, EvilMinion, SecretLab, SecretWeapon, BatCave, Comment,
league, superVillain, evilMinion, secretWeapon, homePlanet, secretLab, env;
var HomePlanet, SuperVillain, CommanderVillain, NormalMinion, EvilMinion, YellowMinion, RedMinion, SecretLab, SecretWeapon, BatCave, Comment,
league, superVillain, commanderVillain, evilMinion, yellowMinion, redMinion, secretWeapon, homePlanet, secretLab, env;
var run = Ember.run;
var LightSaber;

Expand Down Expand Up @@ -44,22 +45,36 @@ module("integration/embedded_records_mixin - EmbeddedRecordsMixin", {
superVillain: DS.belongsTo('super-villain', { async: false }),
name: DS.attr('string')
});
NormalMinion = DS.Model.extend({
name: DS.attr('string')
});
YellowMinion = NormalMinion.extend();
RedMinion = NormalMinion.extend();
CommanderVillain = DS.Model.extend({
name: DS.attr('string'),
minions: DS.hasMany('normal-minion', { polymorphic: true })
});
Comment = DS.Model.extend({
body: DS.attr('string'),
root: DS.attr('boolean'),
children: DS.hasMany('comment', { inverse: null, async: false })
});
env = setupStore({
superVillain: SuperVillain,
homePlanet: HomePlanet,
secretLab: SecretLab,
batCave: BatCave,
secretWeapon: SecretWeapon,
lightSaber: LightSaber,
evilMinion: EvilMinion,
comment: Comment
superVillain: SuperVillain,
commanderVillain: CommanderVillain,
homePlanet: HomePlanet,
secretLab: SecretLab,
batCave: BatCave,
secretWeapon: SecretWeapon,
lightSaber: LightSaber,
evilMinion: EvilMinion,
normalMinion: NormalMinion,
yellowMinion: YellowMinion,
redMinion: RedMinion,
comment: Comment
});
env.store.modelFor('super-villain');
env.store.modelFor('commander-villain');
env.store.modelFor('home-planet');
env.store.modelFor('secret-lab');
env.store.modelFor('bat-cave');
Expand Down Expand Up @@ -1058,6 +1073,39 @@ test("serialize with embedded objects (hasMany relationships, including related
});
});

if (isEnabled("ds-serialize-ids-and-types")) {
test("serialize has many relationship using the `ids-and-types` strategy", function(assert) {
run(function() {
yellowMinion = env.store.createRecord('yellow-minion', { id: 1, name: "Yellowy" });
redMinion = env.store.createRecord('red-minion', { id: 1, name: "Reddy" });
commanderVillain = env.store.createRecord('commander-villain', { id: 1, name: "Jeff", minions: [yellowMinion, redMinion] });
});

env.registry.register('serializer:commander-villain', DS.RESTSerializer.extend(DS.EmbeddedRecordsMixin, {
attrs: {
minions: { serialize: 'ids-and-types' }
}
}));
var serializer, json;
run(function() {
serializer = env.container.lookup("serializer:commander-villain");
var snapshot = commanderVillain._createSnapshot();
json = serializer.serialize(snapshot);
});

assert.deepEqual(json, {
name: 'Jeff',
minions: [{
id: '1',
type: 'yellow-minion'
}, {
id: '1',
type: 'red-minion'
}]
});
});
}

test("normalizeResponse with embedded object (belongsTo relationship)", function(assert) {
env.registry.register('serializer:super-villain', DS.RESTSerializer.extend(DS.EmbeddedRecordsMixin, {
attrs: {
Expand Down Expand Up @@ -1949,7 +1997,6 @@ test("normalizeResponse with polymorphic belongsTo and custom primary key", func
}
]
}, "Custom primary key is correctly normalized");

});

test("Mixin can be used with RESTSerializer which does not define keyForAttribute", function(assert) {
Expand Down