Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow definition of nested model relationships with dotted notation #91

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 50 additions & 4 deletions agility.js
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,37 @@
this.model._data = $.extend({}, arg); // erases previous model attributes without pointing to object
}
else {
//iterate through properties and find nested declarations
for (var prop in arg){
if (prop.indexOf('.') > -1){
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems that this would allow setting a property referenced by an empty string:

data-bind=".oops"

which will try to generate a structure of:

model._data[ '' ] = { oops : 'value' };

This is not a bad thing in itself, and perhaps some future convention can have special meaning for variables ".myFoo" as a result of that, just thinking out loud here.

var path = prop.split('.');
var current_node = this.model._data[path[0]];
if (!current_node){
current_node = this.model._data[path[0]] = {};
}
var next_node;
for (var i = 1; i < path.length - 1; i++){
next_node = current_node[path[i]];
if ($.isPlainObject(next_node)){
current_node = next_node;
}else{
current_node[path[i]] = {};
current_node = current_node[path[i]];
}
}
var last_property = path[path.length - 1];
if ($.isPlainObject(arg[key]) && $.isPlainObject(current_node[last_property])){
//if we're assigning objects, extend rather than replace
$.extend(current_node[last_property], arg[prop]);
}else{
current_node[last_property] = arg[prop];
}

delete _clone[ prop ]; // no need to fire change twice
modified.push(prop);
delete arg[prop];
}
}
$.extend(this.model._data, arg); // default is extend
}
for (var key in arg) {
Expand Down Expand Up @@ -373,8 +404,23 @@
return this.model._data;
}
// Attribute getter
if (typeof arg === 'string') {
return this.model._data[arg];
if (typeof arg === 'string') {
var paths = arg.split('.');
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, we seem to have some indentation issues here - perhaps you're using tabs and not spaces? we use spaces in Agility

var value = this.model._data[paths[0]];
//check for nested objects
if ($.isPlainObject(value)){
for (var i = 1; i < paths.length; i++){
if ($.isPlainObject(value) && value[paths[i]]){
value = value[paths[i]];
}else{
value = value[paths.splice(i).join('.')];
}
}
}else{
//direct key access
value = this.model._data[arg];
}
return value;
}
throw 'agility.js: unknown argument for getter';
},
Expand Down Expand Up @@ -444,7 +490,7 @@
_parseBindStr: function(str){
var obj = {key:null, attr:[]},
pairs = str.split(','),
regex = /([a-zA-Z0-9_\-]+)(?:[\s=]+([a-zA-Z0-9_\-]+))?/,
regex = /([a-zA-Z0-9_\-\.]+)(?:[\s=]+([a-zA-Z0-9_\-]+))?/,
keyAssigned = false,
matched;

Expand Down Expand Up @@ -1009,7 +1055,7 @@
// parse id from Location
self.model.set({ id: jqXHR.getResponseHeader('Location').match(/\/([0-9]+)$/)[1] }, {silent:true});
}
self.trigger('persist:save:success');
self.trigger('persist:save:success', {data:data});
},
error: function(){
self.trigger('persist:error');
Expand Down
33 changes: 33 additions & 0 deletions test/public/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -955,6 +955,39 @@
equals(obj1.model.get('text'), undefined, 'obj.model.reset() erases non-original attributes');
});

test("Nested Model calls", function(){
var a_change = false;
var t = false;
var obj1 = $$({a:1}, '<div data-bind="text.name"></div>', {
'change:a': function(){
a_change = true;
},
'change:text.name': function(){
t = true;
}
});
obj1.model.set({text:{name:'Joe Doe'}});
equals(obj1.model.get('a'), 1, 'obj.model.set() extends by default');
equals(obj1.view.$().text(), 'Joe Doe', 'obj.model.set() fires view change');
equals(t, true, 'obj.model.set() fires change:var');
a_change = false;
obj1.model.set({'text.name':'New Text'}, {reset:true});
equals(obj1.model.get('a'), undefined, 'obj.model.set() resets OK');
equals(a_change, true, 'obj.model.set() with reset=true fires change:a');

obj1.model.reset();
equals(obj1.model.get('a'), 1, 'obj.model.reset() brings back original attribute');
equals(obj1.model.get('text.name'), undefined, 'obj.model.reset() erases non-original attributes');

obj1.model.set({text:{name:'Test'}});
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

more indentation issues

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yikes, apologies. Will fix up indentation asap

equals(obj1.model.get('text.name'), 'Test', 'obj.model.set() nested model set/get');
equals(obj1.view.$().text(), 'Test', 'nested obj.model.set() fires view change');

obj1.model.set({person:{'address.street':'123 fake street'}});
equals(obj1.model.get('person.address.street'), '123 fake street', 'obj.model.set() nested model set/get');
equals(obj1.model.get('person')['address.street'], '123 fake street', 'obj.model.set() nested model set/get with dotted key');
});

test("Chainable calls", function(){
t = false;
var obj = $$().model.set({text:'Joe Doe'}).bind('click &', function(){ t = true; }).trigger('click &');
Expand Down