Skip to content

Commit

Permalink
[FEATURE ds-overhaul-references] Fix inconsistencies with Reference#push
Browse files Browse the repository at this point in the history
`push()` for a belongs-to and has-many reference accepts weird formats,
where it basically should only support a JSON-API document. If this
feature is enabled, it adds deprecation messages if something different
than a JSON-API document is pushed.
  • Loading branch information
pangratz committed May 24, 2016
1 parent 4fb5bb4 commit 960f837
Show file tree
Hide file tree
Showing 6 changed files with 233 additions and 29 deletions.
19 changes: 19 additions & 0 deletions FEATURES.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,22 @@ entry in `config/features.json`.
jsonApiSerializer.modelNameFromPayloadType("api::v1::administrator");
jsonApiSerializer.modelNameFromPayloadType("api::v1::super-user");
```
- `ds-overhaul-references` [#4398](https://github.com/emberjs/data/pull/4398)
This tackles some inconsistencies within `push()` on references. It should
only be used to push a JSON-API payload. The following use cases are
addressed and deprecated:
- `BelongsToReference#push()` accepts a `DS.Model`
- `HasManyReference#push()` accepts a plain array
- `HasManyReference#push()` accepts a pseudo-JSON-API format:
```js
{
data: [
{ data: { type: 'model', id: 1 } }
]
}
```
9 changes: 8 additions & 1 deletion addon/-private/system/references/belongs-to.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import Model from 'ember-data/model';
import Ember from 'ember';
import Reference from './reference';

import { assertPolymorphicType } from "ember-data/-private/debug";
import isEnabled from 'ember-data/-private/features';
import { assertPolymorphicType, deprecate } from "ember-data/-private/debug";

var BelongsToReference = function(store, parentInternalModel, belongsToRelationship) {
this._super$constructor(store, parentInternalModel);
Expand Down Expand Up @@ -43,6 +44,12 @@ BelongsToReference.prototype.push = function(objectOrPromise) {
var record;

if (data instanceof Model) {
if (isEnabled('ds-overhaul-references')) {
deprecate("BelongsToReference#push(DS.Model) is deprecated. Update relationship via `model.set('relationshipName', value)` instead.", false, {
id: 'ds.references.belongs-to.push-record',
until: '3.0'
});
}
record = data;
} else {
record = this.store.push(data);
Expand Down
51 changes: 44 additions & 7 deletions addon/-private/system/references/has-many.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@ import Ember from 'ember';
import Reference from './reference';
import {
assertPolymorphicType,
deprecate,
runInDebug
} from 'ember-data/-private/debug';

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

const get = Ember.get;

var HasManyReference = function(store, parentInternalModel, hasManyRelationship) {
Expand Down Expand Up @@ -48,20 +51,54 @@ HasManyReference.prototype.meta = function() {
HasManyReference.prototype.push = function(objectOrPromise) {
return Ember.RSVP.resolve(objectOrPromise).then((payload) => {
var array = payload;

if (isEnabled("ds-overhaul-references")) {
deprecate("HasManyReference#push(array) is deprecated. Push a JSON-API document instead.", !Array.isArray(payload), {
id: 'ds.references.has-many.push-array',
until: '3.0'
});
}

let useLegacyArrayPush = true;
if (typeof payload === "object" && payload.data) {
array = payload.data;
useLegacyArrayPush = array.length && array[0].data;

if (isEnabled('ds-overhaul-references')) {
deprecate("HasManyReference#push() expects a valid JSON-API document.", !useLegacyArrayPush, {
id: 'ds.references.has-many.push-invalid-json-api',
until: '3.0'
});
}
}

if (!isEnabled('ds-overhaul-references')) {
useLegacyArrayPush = true;
}

var internalModels = array.map((obj) => {
var record = this.store.push(obj);
let internalModels;
if (useLegacyArrayPush) {
internalModels = array.map((obj) => {
var record = this.store.push(obj);

runInDebug(() => {
var relationshipMeta = this.hasManyRelationship.relationshipMeta;
assertPolymorphicType(this.internalModel, relationshipMeta, record._internalModel);
runInDebug(() => {
var relationshipMeta = this.hasManyRelationship.relationshipMeta;
assertPolymorphicType(this.internalModel, relationshipMeta, record._internalModel);
});

return record._internalModel;
});
} else {
let records = this.store.push(payload);
internalModels = Ember.A(records).mapBy('_internalModel');

return record._internalModel;
});
runInDebug(() => {
internalModels.forEach((internalModel) => {
var relationshipMeta = this.hasManyRelationship.relationshipMeta;
assertPolymorphicType(this.internalModel, relationshipMeta, internalModel);
});
});
}

this.hasManyRelationship.computeChanges(internalModels);

Expand Down
1 change: 1 addition & 0 deletions config/features.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@
"ds-serialize-ids-and-types": true,
"ds-extended-errors": null,
"ds-links-in-record-array": null,
"ds-overhaul-references": null,
"ds-payload-type-hooks": null
}
7 changes: 6 additions & 1 deletion tests/integration/references/belongs-to-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import DS from 'ember-data';
import Ember from 'ember';
import setupStore from 'dummy/tests/helpers/store';
import testInDebug from 'dummy/tests/helpers/test-in-debug';
import isEnabled from 'ember-data/-private/features';
import { module, test } from 'qunit';

var get = Ember.get;
Expand Down Expand Up @@ -163,7 +164,7 @@ test("push(object)", function(assert) {
});
});

test("push(record)", function(assert) {
testInDebug("push(record)", function(assert) {
var done = assert.async();

var person, family;
Expand Down Expand Up @@ -193,6 +194,10 @@ test("push(record)", function(assert) {
var familyReference = person.belongsTo('family');

run(function() {
if (isEnabled('ds-overhaul-references')) {
assert.expectDeprecation("BelongsToReference#push(DS.Model) is deprecated. Update relationship via `model.set('relationshipName', value)` instead.");
}

familyReference.push(family).then(function(record) {
assert.ok(Family.detectInstance(record), "push resolves with the referenced record");
assert.equal(get(record, 'name'), "Coreleone", "name is set");
Expand Down
Loading

0 comments on commit 960f837

Please sign in to comment.