Merge pull request spine#529 from spine/model-refresh-bug

Fix for spine#527

src/ajax.coffee

@@ -9,13 +9,13 @@ Ajax =
       @generateURL(object)
     else
       @generateURL(object, encodeURIComponent(object.id))
-    
+
   getCollectionURL: (object) ->
     @generateURL(object)
-  
+
   getScope: (object) ->
     object.scope?() or object.scope
-  
+
   getCollection: (object) ->
     if object.url isnt object.generateURL
       if typeof object.url is 'function'
@@ -104,7 +104,7 @@ class Base
         [promise, statusText, '']
       )
       promise
-    
+
     @queue request
     promise
 
@@ -233,16 +233,16 @@ GenerateURL =
 
 Include =
   ajax: -> new Singleton(this)
-  
+
   generateURL: GenerateURL.include
-  
+
   url: GenerateURL.include
-  
+
 Extend =
   ajax: -> new Collection(this)
-  
+
   generateURL: GenerateURL.extend
-  
+
   url: GenerateURL.extend
 
 Model.Ajax =

src/spine.coffee

@@ -157,21 +157,21 @@ class Model extends Module
   @exists: (id) ->
     return if @irecords[id] then true else false
 
-  @addRecord: (record) ->
+  @addRecord: (record, options = {}) ->
     if record.id and @irecords[record.id]
-      @irecords[record.id].remove()
-
+      @irecords[record.id].remove(options)
+      @irecords[record.id].load record unless options.clear
     record.id or= record.cid
+    @irecords[record.id]  ?= record
+    @irecords[record.cid] ?= record
     @records.push(record)
-    @irecords[record.id]  = record
-    @irecords[record.cid] = record
 
   @refresh: (values, options = {}) ->
     @deleteAll() if options.clear
 
     records = @fromJSON(values)
     records = [records] unless isArray(records)
-    @addRecord(record) for record in records
+    @addRecord(record, options) for record in records
     @sort()
 
     result = @cloneArray(records)
@@ -252,8 +252,13 @@ class Model extends Module
     if typeof objects is 'string'
       objects = JSON.parse(objects)
     if isArray(objects)
-      (new @(value) for value in objects)
+      for value in objects
+        if value instanceof this
+          value
+        else
+          new @(value)
     else
+      return objects if objects instanceof this
       new @(objects)
 
   @fromForm: ->
@@ -298,7 +303,8 @@ class Model extends Module
   load: (atts) ->
     if atts.id then @id = atts.id
     for key, value of atts
-      if atts.hasOwnProperty(key) and typeof @[key] is 'function'
+      if typeof @[key] is 'function'
+        continue if typeof value is 'function'
         @[key](value)
       else
         @[key] = value
@@ -354,20 +360,22 @@ class Model extends Module
     @id = id
     @save()
 
-  remove: ->
+  remove: (options = {}) ->
     # Remove record from model
     records = @constructor.records.slice(0)
     for record, i in records when @eql(record)
       records.splice(i, 1)
       break
     @constructor.records = records
-    # Remove the ID and CID
-    delete @constructor.irecords[@id]
-    delete @constructor.irecords[@cid]
+    if options.clear
+      # Remove the ID and CID indexes
+      delete @constructor.irecords[@id]
+      delete @constructor.irecords[@cid]
 
   destroy: (options = {}) ->
+    options.clear ?= true
     @trigger('beforeDestroy', options)
-    @remove()
+    @remove(options)
     @destroyed = true
     # handle events
     @trigger('destroy', options)

test/specs/model.js

@@ -34,10 +34,23 @@ describe("Model", function(){
     expect(Asset.first().name).toEqual("wem.pdf");
   });
 
-  it("can destroy records", function(){
+  it("can keep record clones in sync after refreshing the record", function(){
     var asset = Asset.create({name: "test.pdf"});
+    expect(asset.name).toEqual("test.pdf");
+    expect(asset.__proto__).toEqual(Asset.irecords[asset.id]);
+
+    var changedAsset = asset.toJSON();
+    changedAsset.name = "wem.pdf";
+    Asset.refresh(changedAsset);
 
+    expect(asset.name).toEqual("wem.pdf");
+    expect(asset.__proto__).toEqual(Asset.irecords[asset.id]);
+  });
+
+  it("can destroy records", function(){
+    var asset = Asset.create({name: "test.pdf"});
     expect(Asset.first()).toEqual(asset);
+
     asset.destroy();
     expect(Asset.first()).toBeFalsy();
   });
@@ -49,15 +62,15 @@ describe("Model", function(){
     asset.destroy();
     expect(Asset.find(asset.id)).toBeFalsy();
   });
-  
+
   it("can use notFound fallback function if records are not found", function(){
     var asset = Asset.create({name: "test.pdf"});
     expect(Asset.find(asset.id)).toBeTruthy();
     // defauly notFound simply returns null
     asset.destroy();
     expect(Asset.find(asset.id)).toBeFalsy();
     // a custom notFound fallback can be added to the find
-    var customfallback = function(id){ 
+    var customfallback = function(id){
       sessionStorage.fallbackRan = true
       sessionStorage.fallbackReceivedId = id
       return Asset.create({name: 'test2.pdf', id:id})
@@ -67,7 +80,7 @@ describe("Model", function(){
     expect(foundAsset.id).toBe(asset.id);
     expect(sessionStorage.fallbackRan).toBe('true');
     expect(sessionStorage.fallbackReceivedId).toBe(asset.id);
-    // notFound can be customized on the model 
+    // notFound can be customized on the model
     asset.destroy(); //reset
     expect(Asset.find(asset.id)).toBeFalsy(); // test reset worked
     Asset.notFound = function(id){
@@ -80,7 +93,7 @@ describe("Model", function(){
     expect(foundAsset2.name).toBe('test3.pdf');
     expect(sessionStorage.fallback2Ran).toBe('true');
     expect(sessionStorage.fallback2ReceivedId).toBe(asset.id);
-    
+
     sessionStorage.clear()
   });
 
@@ -306,7 +319,7 @@ describe("Model", function(){
     expect(asset.attributes()).toEqual({});
   });
 
-  it("can load attributes()", function(){
+  it("can load() attributes", function(){
     var asset = new Asset();
     var result = asset.load({name: "In da' house"});
     expect(result).toBe(asset);
@@ -328,6 +341,24 @@ describe("Model", function(){
     expect(asset.last_name).toEqual("MacCaw");
   });
 
+  it("can load() attributes from model instances respecting getters/setters", function(){
+    spy = jasmine.createSpy();
+    Asset.include({spy: spy});
+    var asset     = Asset.create({name: "test.pdf"});
+    var assetDupe = new Asset(asset.attributes());
+
+    assetDupe.spy  = spy; // Simulate instance method using CoffeeScript fat-arrow
+    assetDupe.name = "wem.pdf";
+
+    asset.load(assetDupe);
+    expect(spy).not.toHaveBeenCalled();
+    expect(asset.name).toEqual("wem.pdf");
+
+    assetDupe.spy = "setter value";
+    asset.load(assetDupe);
+    expect(spy).toHaveBeenCalledWith("setter value");
+  });
+
   it("attributes() respects getters/setters", function(){
     Asset.include({
       name: function(){
@@ -513,9 +544,7 @@ describe("Model", function(){
     var spy;
 
     beforeEach(function(){
-      var noop = {spy: function(){}};
-      spyOn(noop, "spy");
-      spy = noop.spy;
+      spy = jasmine.createSpy();
     });
 
     it("can interate over records", function(){
@@ -556,14 +585,14 @@ describe("Model", function(){
       Asset.bind("destroy", spy);
       var asset = Asset.create({name: "cartoon world.png"});
       asset.destroy();
-      expect(spy).toHaveBeenCalledWith(asset, {});
+      expect(spy).toHaveBeenCalledWith(asset, {clear: true});
     });
 
     it("can fire destroy events when destroy all record with options", function(){
       Asset.bind("destroy", spy);
       var asset = Asset.create({name: "screaming goats.png"});
       Asset.destroyAll({ajax: false});
-      expect(spy).toHaveBeenCalledWith(asset, {ajax: false});
+      expect(spy).toHaveBeenCalledWith(asset, {ajax: false, clear: true});
     });
 
     it("can fire refresh events", function(){
@@ -602,7 +631,7 @@ describe("Model", function(){
       expect(spy).toHaveBeenCalledWith(asset, "update", {});
 
       asset.destroy();
-      expect(spy).toHaveBeenCalledWith(asset, "destroy", {});
+      expect(spy).toHaveBeenCalledWith(asset, "destroy", {clear: true});
     });
 
     it("can fire error events", function(){