clarkie/dynogels

Timestamp fields should be `Joi.string().isoDate()` and not `Joi.date()`

Opened this issue · 1 comments

First off, thanks so much for dynogels. It's awesome.

So I noticed some strange behavior when I was trying to run items through the validator on item.table.schema.validate.

Let's say you have a model that has timestamps on it. When you do item.toJSON() on it you'd get something that looks like this:

{
  /* fields */
  createdAt: "2017-09-26T05:47:07.369Z",
  /* more fields */
}

Now let's say you run item.table.schema.validate(item.toJSON()) what you'd get back is:

{
  /* fields */
  createdAt: Mon Sep 25 2017 22:47:07 GMT-0700 (MST), // createdAt instanceof Date === true
  /* more fields */
}

Which doesn't really make sense why it got changed from an ISO Date String into a Date class. Turns out that Joi.date() will cast your ISO Date String into an instance of a Date class when you validate with it.

The thing that sucks about this though is that DynamoDB stores and sends back ISO Date Strings so it would make sense that your code after that point is expecting a string and not an instance of Date. It would make sense then (IMO) to change the validation schema to use Joi.string().isoDate() instead of Joi.date().

Would you agree? Also, would it be just as simple as changing this line?

I agree with you that it seems inconsistent. If the timestamps get put in as ISO Date Strings then the validator should also look for ISO Date Strings. This seems like a pretty straightforward fix but I wonder if this would have any unintended consequences. @cdhowie, @clarkie any thoughts on this?

It looks like the line you mention is the line to change. Might as well change the updatedAt date too I think. @johnhaley81 would you mind doing a pull request?