Skip to content

Commit da2142d

Browse files
dxgdxg
committed
Fix hasOne required validation.
1 parent cf20a20 commit da2142d

File tree

8 files changed

+98
-84
lines changed

8 files changed

+98
-84
lines changed

‎Changelog.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
### v2.1.16 - 15 Jul 2014
2+
- Fix Model.create missing properties bug
3+
- Add missing `var` (#523)
4+
- Fix hasOne `required: true` when `instance.returnAllErrors` is true.
5+
This also makes hasOne required validations messages consistent with other validation messages.
6+
17
### v2.1.15 - 05 Jun 2014
28
- Feature: Enable plugging in custom third-party drivers (now known as adapters) (#512)
39
- Add Instance.set() so that properties of type object can have their properties set and mark model as dirty (#517)

‎lib/Associations/Extend.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ var Settings = require("../Settings");
44
var Singleton = require("../Singleton");
55
var util = require("../Utilities");
66

7-
exports.prepare = function (db, Model, associations, association_properties, model_fields) {
7+
exports.prepare = function (db, Model, associations) {
88
Model.extendsTo = function (name, properties, opts) {
99
opts = opts || {};
1010

‎lib/Associations/One.js‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ var util = require("../Utilities");
33
var ORMError = require("../Error");
44
var Accessors = { "get": "get", "set": "set", "has": "has", "del": "remove" };
55

6-
exports.prepare = function (Model, associations, association_properties, model_fields) {
6+
exports.prepare = function (Model, associations) {
77
Model.hasOne = function () {
88
var assocName;
99
var assocTemplateName;
@@ -41,7 +41,11 @@ exports.prepare = function (Model, associations, association_properties, model_f
4141
} else if(!association.extension) {
4242
association.field = util.wrapFieldObject(association.field, Model, Model.table, Model.properties);
4343
}
44-
util.convertPropToJoinKeyProp(association.field, { makeKey: false, required: false });
44+
45+
util.convertPropToJoinKeyProp(association.field, {
46+
makeKey: false, required: association.required
47+
});
48+
4549
for (var k in Accessors) {
4650
if (!association.hasOwnProperty(k + "Accessor")) {
4751
association[k + "Accessor"] = Accessors[k] + assocTemplateName;
@@ -53,11 +57,11 @@ exports.prepare = function (Model, associations, association_properties, model_f
5357
if (!association.field.hasOwnProperty(k)) {
5458
continue;
5559
}
56-
association_properties.push(k);
5760
if (!association.reversed) {
58-
Model.allProperties[k] = _.omit(association.field[k], 'klass');
59-
Model.allProperties[k].klass = 'hasOne';
60-
model_fields.push(k);
61+
Model.addProperty(
62+
_.extend({}, association.field[k], { klass: 'hasOne' }),
63+
false
64+
);
6165
}
6266
}
6367

‎lib/Instance.js‎

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -45,35 +45,15 @@ function Instance(Model, opts) {
4545
return saveError(cb, err);
4646
}
4747

48-
for (i = 0; i < opts.one_associations.length; i++) {
49-
for (k in opts.one_associations[i].field) {
50-
if (opts.one_associations[i].required && opts.data[k] === null) {
51-
var err = new Error("Property required");
52-
53-
err.field = k;
54-
err.value = opts.data[k];
55-
err.msg = "Property required";
56-
err.type = "validation";
57-
err.model = Model.table;
58-
59-
if (!Model.settings.get("instance.returnAllErrors")) {
60-
return cb(err);
61-
}
62-
63-
errors.push(err);
64-
}
65-
}
66-
}
67-
6848
var checks = new enforce.Enforce({
6949
returnAllErrors : Model.settings.get("instance.returnAllErrors")
7050
});
7151

7252
for (k in opts.validations) {
7353
required = false;
7454

75-
if (Model.properties[k]) {
76-
required = Model.properties[k].required;
55+
if (Model.allProperties[k]) {
56+
required = Model.allProperties[k].required;
7757
} else {
7858
for (i = 0; i < opts.one_associations.length; i++) {
7959
if (opts.one_associations[i].field === k) {
@@ -82,6 +62,7 @@ function Instance(Model, opts) {
8262
}
8363
}
8464
}
65+
8566
if (!required && instance[k] == null) {
8667
continue; // avoid validating if property is not required and is "empty"
8768
}

‎lib/Model.js‎

Lines changed: 68 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,7 @@ function Model(opts) {
199199
model.allProperties = allProperties;
200200
model.properties = opts.properties;
201201
model.settings = opts.settings;
202+
model.keys = opts.keys;
202203

203204
model.drop = function (cb) {
204205
if (arguments.length === 0) {
@@ -619,6 +620,68 @@ function Model(opts) {
619620
return this;
620621
};
621622

623+
model.prependValidation = function (key, validation) {
624+
if(opts.validations.hasOwnProperty(key)) {
625+
opts.validations[key].splice(0, 0, validation);
626+
} else {
627+
opts.validations[key] = [validation];
628+
}
629+
};
630+
631+
var currFields = {};
632+
633+
model.addProperty = function (propIn, options) {
634+
var cType;
635+
var prop = Property.normalize({
636+
prop: propIn, name: (options && options.name || propIn.name),
637+
customTypes: opts.db.customTypes, settings: opts.settings
638+
});
639+
640+
// Maintains backwards compatibility
641+
if (opts.keys.indexOf(k) != -1) {
642+
prop.key = true;
643+
} else if (prop.key) {
644+
opts.keys.push(k);
645+
}
646+
647+
if (options && options.klass) {
648+
prop.klass = options.klass;
649+
}
650+
651+
switch (prop.klass) {
652+
case 'primary':
653+
opts.properties[prop.name] = prop;
654+
break;
655+
case 'hasOne':
656+
association_properties.push(prop.name)
657+
break;
658+
}
659+
660+
allProperties[prop.name] = prop;
661+
fieldToPropertyMap[prop.mapsTo] = prop;
662+
663+
if (prop.required) {
664+
model.prependValidation(prop.name, Validators.required());
665+
}
666+
667+
if (prop.key && prop.klass == 'primary') {
668+
keyProperties.push(prop);
669+
}
670+
671+
if (prop.lazyload !== true && !currFields[prop.name]) {
672+
currFields[prop.name] = true;
673+
if ((cType = opts.db.customTypes[prop.type]) && cType.datastoreGet) {
674+
model_fields.push({
675+
a: prop.mapsTo, sql: cType.datastoreGet(prop, opts.db.driver.query)
676+
});
677+
} else {
678+
model_fields.push(prop.mapsTo);
679+
}
680+
}
681+
682+
return prop;
683+
};
684+
622685
Object.defineProperty(model, "table", {
623686
value: opts.table,
624687
enumerable: false
@@ -627,14 +690,6 @@ function Model(opts) {
627690
value: opts.keys,
628691
enumerable: false
629692
});
630-
Object.defineProperty(model, "keys", {
631-
value: opts.keys,
632-
enumerable: false
633-
});
634-
Object.defineProperty(model, "properties", {
635-
value: opts.properties,
636-
enumerable: false
637-
});
638693
Object.defineProperty(model, "uid", {
639694
value: opts.driver.uid + "/" + opts.table + "/" + opts.keys.join("/"),
640695
enumerable: false
@@ -647,55 +702,16 @@ function Model(opts) {
647702
}
648703
}
649704

650-
var currFields = {}, cType, name;
651-
652705
// If no keys are defined add the default one
653706
if (opts.keys.length == 0 && !_.any(opts.properties, { key: true })) {
654-
name = opts.settings.get("properties.primary_key");
655-
656-
opts.properties[name] = Property.normalize({
657-
prop: { type: 'serial', key: true, required: false, klass: 'key' },
658-
name: name, customTypes: opts.db.customTypes, settings: opts.settings
659-
});
707+
opts.properties[opts.settings.get("properties.primary_key")] = {
708+
type: 'serial', key: true, required: false, klass: 'primary'
709+
};
660710
}
661711

662712
// standardize properties
663713
for (k in opts.properties) {
664-
var prop = opts.properties[k] = Property.normalize({
665-
prop: opts.properties[k], name: k,
666-
customTypes: opts.db.customTypes, settings: opts.settings
667-
});
668-
prop.klass = 'primary';
669-
allProperties[k] = prop;
670-
fieldToPropertyMap[prop.mapsTo] = prop;
671-
672-
if (opts.keys.indexOf(k) != -1) {
673-
prop.key = true;
674-
} else if (prop.key) {
675-
opts.keys.push(k);
676-
}
677-
if (prop.key) {
678-
keyProperties.push(prop);
679-
}
680-
681-
if (prop.lazyload !== true && !currFields[k]) {
682-
currFields[k] = true;
683-
if ((cType = opts.db.customTypes[prop.type]) && cType.datastoreGet) {
684-
model_fields.push({
685-
a: prop.mapsTo, sql: cType.datastoreGet(prop, opts.db.driver.query)
686-
});
687-
} else {
688-
model_fields.push(prop.mapsTo);
689-
}
690-
}
691-
if (prop.required) {
692-
// Prepend `required` validation
693-
if(opts.validations.hasOwnProperty(k)) {
694-
opts.validations[k].splice(0, 0, Validators.required());
695-
} else {
696-
opts.validations[k] = [Validators.required()];
697-
}
698-
}
714+
model.addProperty(opts.properties[k], { name: k, klass: 'primary' });
699715
}
700716

701717
if (keyProperties.length == 0) {
@@ -707,7 +723,7 @@ function Model(opts) {
707723
model[AvailableHooks[k]] = createHookHelper(AvailableHooks[k]);
708724
}
709725

710-
OneAssociation.prepare(model, one_associations, association_properties, model_fields);
726+
OneAssociation.prepare(model, one_associations);
711727
ManyAssociation.prepare(opts.db, model, many_associations);
712728
ExtendAssociation.prepare(opts.db, model, extend_associations);
713729

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
"sqlite",
1313
"mongodb"
1414
],
15-
"version" : "2.1.15",
15+
"version" : "2.1.16",
1616
"license" : "MIT",
1717
"homepage" : "http://dresende.github.io/node-orm2",
1818
"repository" : "http://github.com/dresende/node-orm2.git",

‎test/integration/association-extend.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ describe("Model.extendsTo()", function() {
2323
Person.create({
2424
name: "John Doe"
2525
}, function (err, person) {
26+
should.not.exist(err);
27+
2628
return person.setAddress(new PersonAddress({
2729
street : "Liberty",
2830
number : 123

‎test/integration/association-hasone-required.js‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ describe("hasOne", function() {
1111
var setup = function (required) {
1212
return function (done) {
1313
db.settings.set('instance.cache', false);
14+
db.settings.set('instance.returnAllErrors', true);
1415

1516
Person = db.define('person', {
1617
name : String
@@ -39,8 +40,12 @@ describe("hasOne", function() {
3940
name : "John",
4041
parentId : null
4142
});
42-
John.save(function (err) {
43-
should.exist(err);
43+
John.save(function (errors) {
44+
should.exist(errors);
45+
should.equal(errors.length, 1);
46+
should.equal(errors[0].type, 'validation');
47+
should.equal(errors[0].msg, 'required');
48+
should.equal(errors[0].property, 'parentId');
4449
return done();
4550
});
4651
});

0 commit comments

Comments
 (0)