Only save id/type of clean records

Previously we were not sending clean records down to the server, which
means ember-data would forget about then after save(). Now, we do send
clean records down to the server, but only their resource identifiers
(avoiding concurrency issues with attributes).

Author: Lee Richmond

addon/mixins/model.js

@@ -1,11 +1,16 @@
 import Ember from 'ember';
 
-const resetRelations = function(record) {
+const resetRelations = function(record, relationshipsDirective) {
   record.eachRelationship((relationName, meta) => {
     if (meta.kind === 'hasMany') {
-      record.set(relationName, Ember.A());
-    } else {
-      record.set(relationName, null);
+      if (relationshipsDirective.hasOwnProperty(relationName)) {
+        let filtered = record.get(relationName).filter((r) => {
+          return r.get('isNew') ||
+            r.get('markedForDestruction') ||
+            r.get('markedForDeletion');
+        });
+        record.get(relationName).removeObjects(filtered);
+      }
     }
   });
 };
@@ -45,9 +50,10 @@ export default Ember.Mixin.create({
     defaultOptions(options);
     let promise = this._super(...arguments);
 
-    if (options.resetRelations) {
-      promise.then(resetRelations);
-    }
+    let directive = options.adapterOptions.relationships;
+    promise.then((record) => {
+      resetRelations(record, directive);
+    });
 
     return promise;
   }

addon/mixins/nested-relations.js

@@ -68,21 +68,17 @@ const jsonapiPayload = function(record) {
 const hasManyData = function(relatedRecords, subRelations) {
   let payloads = [];
   relatedRecords.forEach((relatedRecord) => {
-    if (relatedRecord.get('hasDirtyAttributes')) {
-      let payload = jsonapiPayload(relatedRecord);
-      processRelationships(subRelations, payload, relatedRecord);
-      payloads.push(payload);
-    }
+    let payload = jsonapiPayload(relatedRecord);
+    processRelationships(subRelations, payload, relatedRecord);
+    payloads.push(payload);
   });
   return { data: payloads };
 };
 
 const belongsToData = function(relatedRecord, subRelations) {
-  if (isPresentObject(subRelations) || relatedRecord.get('hasDirtyAttributes')) {
-    let payload = jsonapiPayload(relatedRecord);
-    processRelationships(subRelations, payload, relatedRecord);
-    return { data: payload };
-  }
+  let payload = jsonapiPayload(relatedRecord);
+  processRelationships(subRelations, payload, relatedRecord);
+  return { data: payload };
 };
 
 const processRelationship = function(kind, relationData, subRelations, callback) {

tests/acceptance/delete-nested-object-test.js

@@ -7,22 +7,31 @@ moduleForAcceptance('Acceptance | delete nested objects');
 
 test('deleting a nested object', function(assert) {
   page.visit();
-  page.addTag().tags(0).setName('new tag 1');
+  page.addTag().tags(0).setName('a');
   page.submit();
 
   andThen(function() {
-    assert.equal(detailPage.tagList, 'new tag 1');
+    assert.equal(detailPage.tagList, 'a');
     detailPage.edit();
 
     andThen(function() {
-      page.tags(0).remove();
+      page.addTag().tags(1).setName('b');
+      page.submit();
 
       andThen(function() {
-        assert.equal(page.tags().count, 0, 'should not show removed tag');
-        page.submit();
+        detailPage.edit();
 
         andThen(function() {
-          assert.equal(detailPage.tagList, '', 'should delete removed tag');
+          page.tags(0).remove();
+
+          andThen(function() {
+            assert.equal(page.tags().count, 1, 'should not show removed tag');
+            page.submit();
+
+            andThen(function() {
+              assert.equal(detailPage.tagList, 'b', 'should delete removed tag');
+            });
+          });
         });
       });
     });

tests/acceptance/update-nested-relations-test.js

@@ -5,9 +5,6 @@ import detailPage from 'dummy/tests/pages/show-post';
 
 moduleForAcceptance('Acceptance | update nested relations');
 
-// Todo test ideally
-// * delete
-// * disassociate
 test('updating nested relations', function(assert) {
   let author = server.create('author', { name: 'Joe Author' });
   let tag1 = server.create('tag', { name: 'tag1' });
@@ -40,3 +37,23 @@ test('updating nested relations', function(assert) {
     });
   });
 });
+
+test('updating only one member of a hasMany relation', function(assert) {
+  server.foo = 'bar';
+  let tag1 = server.create('tag', { name: 'tag1' });
+  let tag2 = server.create('tag', { name: 'tag2' });
+  let post = server.create('post', {
+    tags: [tag1, tag2],
+  });
+  visit(`/posts/${post.id}/edit`);
+
+  andThen(function() {
+    assert.equal(page.tags().count, 2);
+    page.tags(0).setName('tag1 changed');
+    page.submit();
+
+    andThen(function() {
+      assert.equal(detailPage.tagList, 'tag1 changed, tag2');
+    });
+  });
+});

tests/dummy/app/post/route.js

@@ -2,9 +2,6 @@ import Ember from 'ember';
 
 export default Ember.Route.extend({
   model(params) {
-    return this.store.findRecord('post', params.postId, {
-      reload: true,
-      include: 'author,tags'
-    });
+    return this.store.peekRecord('post', params.postId);
   }
 });

tests/unit/mixins/model-test.js

@@ -12,8 +12,13 @@ const Author = DS.Model.extend(ModelMixin, {
 });
 
 const Post = DS.Model.extend(ModelMixin, {
-  title: DS.attr('string'),
-  author: DS.belongsTo({ async: false })
+  title:  DS.attr('string'),
+  author: DS.belongsTo({ async: false }),
+  tags:  DS.hasMany({ async: true })
+});
+
+const Tag = DS.Model.extend(ModelMixin, {
+  name: DS.attr('string')
 });
 
 moduleForAcceptance('Unit | Mixin | model', {
@@ -22,6 +27,7 @@ moduleForAcceptance('Unit | Mixin | model', {
     store = getOwner(this).lookup('service:store');
     getOwner(this).register('model:post', Post);
     getOwner(this).register('model:author', Author);
+    getOwner(this).register('model:tag', Tag);
   }
 });
 
@@ -54,29 +60,65 @@ test('#hasDirtyAttributes', function(assert) {
   });
 });
 
-test('#save when resetRelations: false', function(assert) {
-  Ember.run(() => {
-    let author = store.createRecord('author', { name: 'Joe Author' });
-    let post = store.createRecord('post', { title: 'title', author: author });
-
-    let done = assert.async();
-    post.save({ resetRelations: false }).then((p) => {
-      assert.equal(p.get('author'), author);
-      done();
+// note tag with id 3 does not send attributes
+test('resetting relations when only sending dirty relations', function(assert) {
+  let done = assert.async();
+  server.patch('/posts/:id', (db, request) => {
+    let relationships = JSON.parse(request.requestBody).data.relationships;
+    assert.deepEqual(relationships, {
+      tags: {
+        data: [
+          {
+            id: '2',
+            type: 'tags',
+            attributes: { name: 'tag1 changed' }
+          },
+          {
+            id: '3',
+            type: 'tags'
+          }
+        ]
+      }
     });
+    done();
+    let post = db.posts.find(request.params.id);
+    post.tags.models[0].update({ name: 'tag1 changed' });
+    return post;
   });
-});
 
-test('#save when resetRelations: true', function(assert) {
+  let post = server.create('post');
+  post.createTag({ name: 'tag1' });
+  post.createTag({ name: 'tag2' });
   Ember.run(() => {
-    let author = store.createRecord('author', { name: 'Joe Author' });
-    let post = store.createRecord('post', { title: 'title', author: author });
+    store.pushPayload({
+      data: {
+        type: 'posts',
+        id: 1,
+        relationships: {
+          tags: {
+            data: [
+              { type: 'tags', id: '2' },
+              { type: 'tags', id: '3' }
+            ]
+          }
+        }
+      },
+      included: [
+        { type: 'tags', id: '2', attributes: { name: 'tag1' } },
+        { type: 'tags', id: '3', attributes: { name: 'tag2' } }
+      ]
+    });
+  });
 
-    let done = assert.async();
-    post.save().then((p) => {
-      let author = p.get('author');
-      assert.equal(author, null);
-      done();
+  post = store.peekRecord('post', 1);
+  assert.equal(post.get('tags.length'), 2);
+  post.set('tags.firstObject.name', 'tag1 changed');
+
+  let done2 = assert.async();
+  Ember.run(() => {
+    post.save({ adapterOptions: { relationships: 'tags' } }).then((p) => {
+      assert.equal(p.get('tags.length'), 2);
+      done2();
     });
   });
 });

tests/unit/mixins/nested-relations-test.js

@@ -293,6 +293,7 @@ test('it serializes one-to-many correctly', function(assert) {
   assert.deepEqual(json, expectedJSON, 'has correct json');
 });
 
+// note tag 2 does not pass attributes
 test('one-to-many deletion/destruction', function(assert) {
   seedPostWithTags();
   let post = store.peekRecord('post', 1);
@@ -306,6 +307,7 @@ test('one-to-many deletion/destruction', function(assert) {
       relationships: {
         tags: {
           data: [
+            { type: 'tags', id: '2' },
             { type: 'tags', id: '3', attributes: { _delete: true } },
             { type: 'tags', id: '4', attributes: { _destroy: true } },
           ]
@@ -328,7 +330,7 @@ test('relationship specified but not present', function(assert) {
   assert.deepEqual(json, expectedJSON, 'it does not blow up');
 });
 
-test('does not serialize non-dirty relations', function(assert) {
+test('does not serialize attributes of non-dirty relations', function(assert) {
   store.pushPayload({
     data: {
       id: '1',
@@ -376,6 +378,12 @@ test('does not serialize non-dirty relations', function(assert) {
       id: '1',
       type: 'posts',
       relationships: {
+        author: {
+          data: {
+            id: '99',
+            type: 'authors'
+          }
+        },
         genre: {
           data: {
             type: 'genres',
@@ -386,6 +394,7 @@ test('does not serialize non-dirty relations', function(assert) {
         tags: {
           data: [
             { type: 'tags', id: '2', attributes: { name: 'tag1 change' } },
+            { type: 'tags', id: '3' },
             { type: 'tags', attributes: { name: 'new tag' } }
           ]
         }