replace exception thrown for invalid dates #1339
Conversation
lib/model-builder.js
Outdated
| @@ -426,6 +426,9 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett | |||
| var requiredOptions = typeof prop.required === 'object' ? prop.required : undefined; | |||
| ModelClass.validatesPresenceOf(propertyName, requiredOptions); | |||
| } | |||
| if (DataType === Date) { | |||
There was a problem hiding this comment.
Can we do a one liner if statement here? if (DataType === Date) ModelClass.validatesDateOf(propertyName);
lib/model-builder.js
Outdated
| var d = new Date(arg); | ||
| if (isNaN(d.getTime())) { | ||
| throw new Error(g.f('Invalid date: %s', arg)); | ||
| if (arg === null) { |
There was a problem hiding this comment.
Same here. One liner if statement.
| /*! | ||
| * Date validator | ||
| */ | ||
| function validateDate(attr, conf, err) { |
There was a problem hiding this comment.
What is the need of conf here?
There was a problem hiding this comment.
It turns out that conf is needed, otherwise the tests failed.
It's a required attribute for similar functions defined inside validators (in line397), even though it's not being used.
There was a problem hiding this comment.
@dhmlau I am not sure what you mean
There was a problem hiding this comment.
the validators are defined in the block around line 397. There are validatePresence, validateAbsence and also the newly added validateDate. If you go to those validate* functions, they all have conf. This is needed when those functions are being called.
In fact, without conf, tests fail because it expects the 2nd parameter in validate* function to be a function.
lib/validations.js
Outdated
| if (this[attr] === null || this[attr] === undefined) return; | ||
|
|
||
| var date = new Date(this[attr]); | ||
| if (isNaN(date.getTime())) { |
There was a problem hiding this comment.
One liner if statement please :)
lib/model-builder.js
Outdated
| } | ||
| var d = new Date(arg); |
There was a problem hiding this comment.
I'd wrap this with a try catch block in case there is an error here
Description
Originated from PR #718
There's no response from the user to grant me access to his fork, so I've created a separate PR. User @hotaru355 has signed CLA.
Excerpt from original PR:
2 tests are removed because they are no longer valid because no exceptions are thrown when instantiating the model.
should fail if field validation failsin manipuliation.test.jsthrows an error when property of type Date is set to an invalid valuein datatype.test.jsRelated issues
#718
Checklist
guide