Merge pull request #4398 from pangratz/feature-overhaul-references

[FEATURE ds-overhaul-references] Fix inconsistencies with Reference#push

FEATURES.md

@@ -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 } }
+        ]
+      }
+      ```
+

addon/-private/system/references/belongs-to.js

@@ -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);
@@ -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);

addon/-private/system/references/has-many.js

@@ -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) {
@@ -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);
 

config/features.json

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

tests/integration/references/belongs-to-test.js

@@ -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;
@@ -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;
@@ -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");