Skip to content

Loading…

Always assign record IDs before other attributes in Spine.Model::load() #435

Merged
merged 3 commits into from

2 participants

@adambiggs
Spine JS Project member

This is basically a redo of the #171 PR which is now a bit stale. I also included more tests for relation.coffee and another bug fix for when model instances are passed into .load().

The original bug:

Album.hasMany 'photos', Photo
Photo.belongsTo 'album', Album

album = new Album();
album.load
    name: 'My Album',
    photos: [{
        id: '123',
        name: 'Picture of a cat'
    }],
    id: '456' # <= Triggers Bug!

Because id comes after photos, there's no parent record ID assigned yet to resolve the relation.

The other bug fix is exactly the same as #423

@aeischeid aeischeid was assigned
@aeischeid aeischeid merged commit f1e1e06 into spine:master
@aeischeid
Spine JS Project member

I was holding off on #423 has own property thing because I was a little nervous how it might affect some of the relation stuff, but since you know that pretty well and are submitting this I am going to assume that should be okay, or that if it causes problems you won't blame me ;)

@aeischeid
Spine JS Project member

As always, thanks for the pull request!

@adambiggs adambiggs deleted the adambiggs:assign-record-ids-first branch
@adambiggs adambiggs restored the adambiggs:assign-record-ids-first branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 85 additions and 9 deletions.
  1. +4 −0 lib/spine.js
  2. +2 −1 src/spine.coffee
  3. +79 −8 test/specs/model.relation.js
View
4 lib/spine.js
@@ -457,7 +457,11 @@
Model.prototype.load = function(atts) {
var key, value;
+ if (atts.id) {
+ this.id = atts.id;
+ }
for (key in atts) {
+ if (!__hasProp.call(atts, key)) continue;
value = atts[key];
if (typeof this[key] === 'function') {
this[key](value);
View
3 src/spine.coffee
@@ -254,7 +254,8 @@ class Model extends Module
validate: ->
load: (atts) ->
- for key, value of atts
+ if atts.id then @id = atts.id
+ for own key, value of atts
if typeof @[key] is 'function'
@[key](value)
else
View
87 test/specs/model.relation.js
@@ -37,32 +37,103 @@ describe("Model.Relation", function(){
expect( photo.album().exists() ).toEqual(true);
});
- it("should load nested Singleton record", function(){
+ it("should associate an existing Singleton record", function(){
Album.hasOne("photo", Photo);
Photo.belongsTo("album", Album);
var album = new Album();
- album.load({id: "1", name: "Beautiful album",
- photo: {id: "2", name: "Beautiful photo", album_id: "1"}});
+ album.load({
+ id: "1",
+ name: "Beautiful album",
+ });
+
+ var photo = new Photo();
+ photo.load({
+ id: "2",
+ name: "Beautiful photo"
+ });
+
+ album.photo(photo);
expect( album.photo() ).toBeTruthy();
+ expect( album.photo().album_id ).toBe("1");
expect( album.photo().name ).toBe("Beautiful photo");
});
- it("should load nested Collection records", function(){
+ it("should create a new related Singleton record", function(){
+ Album.hasOne("photo", Photo);
+ Photo.belongsTo("album", Album);
+
+ var album = new Album();
+ album.load({
+ name: "Beautiful album",
+ photo: {
+ name: "Beautiful photo"
+ },
+ id: "1"
+ });
+
+ expect( album.photo() ).toBeTruthy();
+ expect( album.photo().album_id ).toBe("1");
+ expect( album.photo().name ).toBe("Beautiful photo");
+ });
+
+ it("should associate existing Collection records", function(){
Album.hasMany("photos", Photo);
Photo.belongsTo("album", Album);
var album = new Album();
album.load({
- id: "1", name: "Beautiful album",
- photos: [{id: "1", name: "Beautiful photo 1", album_id: "1"},
- {id: "2", name: "Beautiful photo 2", album_id: "1"}]
- });
+ name: "Beautiful album",
+ id: "1"
+ });
+
+ var photo1 = new Photo();
+ photo1.load({
+ id: "1",
+ name: "Beautiful photo 1"
+ });
+
+ var photo2 = new Photo();
+ photo2.load({
+ id: "2",
+ name: "Beautiful photo 2"
+ });
+
+ album.photos([ photo1, photo2 ]);
+
+ expect( album.photos() ).toBeTruthy();
+ expect( album.photos().all().length ).toBe(2);
+ expect( album.photos().first().album_id ).toBe("1");
+ expect( album.photos().last().album_id ).toBe("1");
+ expect( album.photos().first().name ).toBe("Beautiful photo 1");
+ expect( album.photos().last().name ).toBe("Beautiful photo 2");
+ });
+
+ it("should create new related Collection records", function(){
+ Album.hasMany("photos", Photo);
+ Photo.belongsTo("album", Album);
+
+ var album = new Album();
+ album.load({
+ name: "Beautiful album",
+ photos: [{
+ id: "1",
+ name: "Beautiful photo 1"
+ },
+ {
+ id: "2",
+ name: "Beautiful photo 2"
+ }],
+ id: "1"
+ });
expect( album.photos() ).toBeTruthy();
expect( album.photos().all().length ).toBe(2);
+ expect( album.photos().first().album_id ).toBe("1");
+ expect( album.photos().last().album_id ).toBe("1");
expect( album.photos().first().name ).toBe("Beautiful photo 1");
expect( album.photos().last().name ).toBe("Beautiful photo 2");
});
+
});
Something went wrong with that request. Please try again.