Allow for unnamed attributes #125
Conversation
| for(var name in model) { | ||
| if(!model.hasOwnProperty(name)){ | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Needed to add this continue in order to avoid trying to turn schema, options, ..., etcetera into Attribute objects that would attempt to be persisted.
| dynamoObj[attr.name] = dynamoAttr; | ||
|
|
||
| if(!attr & this.options.saveUnknown) { | ||
| attr = Attribute.create(this, name, typeof model[name]); |
There was a problem hiding this comment.
I think the typeof here works better than always assuming Object. JSON.stringify doesn't always give you what you'd want.
> JSON.stringify("astring")
'"astring"'
| for(var name in model) { | ||
| if(!model.hasOwnProperty(name)){ | ||
| continue; | ||
| } |
There was a problem hiding this comment.
think you can just use Object.getOwnPropertyNames(),
There was a problem hiding this comment.
tried that and had a bunch of test failures that didn't seem worth tracking down since the solution above worked.
| @@ -254,7 +254,7 @@ describe('Query', function (){ | |||
| Dog.query('breed').eq('unknown') | |||
| .filter('color').not().null().exec() | |||
| .then(function (dogs) { | |||
| dogs.length.should.eql(5); | |||
| dogs.length.should.eql(4); | |||
There was a problem hiding this comment.
based on the test data setup, this assertion was wrong
| @@ -268,7 +268,7 @@ describe('Query', function (){ | |||
| .filter('color').not().eq('Brown') | |||
| .exec() | |||
| .then(function (dogs) { | |||
| dogs.length.should.eql(1); | |||
| dogs.length.should.eql(2); | |||
There was a problem hiding this comment.
based on the test data setup, this assertion was wrong
|
Thanks for the PR. I know several people will be happy to get this. The code and test cases look good (I wonder how the Query tests have been passing - thanks for fixing that). Could you add a few quick line to the schema options section of the README? - |
|
Thanks a bunch. |
This is an attempt to address issue #107
I pulled the code out of the conversation, modified slightly to get it to work, and wrote a couple of tests against it. I'm looking for some validation that this is on the right track and what else you'd like to see in terms of test coverage and documentation in order to accept this pull request.