From 05d199f9b4e58f070c5c7c5812cf9aefc2029b24 Mon Sep 17 00:00:00 2001 From: Jason Williams Date: Sat, 5 Jul 2014 14:57:56 +0000 Subject: [PATCH] Check datatype for date format conversion Closes #3199 -If datatype is dateTime convert to javascript Date object when retrieved from the database. -Add tests to make sure models and internal API are using Date objects for dateTime fields. -Add tests to make sure the HTTP API is returning ISO 8601 date strings for dateTime fields. --- core/server/models/base.js | 7 ++++-- core/test/functional/routes/api/posts_test.js | 3 +++ .../functional/routes/api/settings_test.js | 1 + core/test/functional/routes/api/tags_test.js | 2 ++ core/test/functional/routes/api/users_test.js | 25 +++++++++++++++++++ core/test/integration/api/api_posts_spec.js | 2 ++ .../test/integration/api/api_settings_spec.js | 10 ++++++++ core/test/integration/api/api_tags_spec.js | 2 ++ core/test/integration/api/api_users_spec.js | 21 +++++++++++++++- .../model/model_app_fields_spec.js | 2 ++ .../model/model_app_settings_spec.js | 2 ++ .../test/integration/model/model_apps_spec.js | 2 ++ .../model/model_permissions_spec.js | 1 + .../integration/model/model_posts_spec.js | 1 + .../integration/model/model_roles_spec.js | 1 + .../integration/model/model_settings_spec.js | 1 + .../test/integration/model/model_tags_spec.js | 11 ++++++++ .../integration/model/model_users_spec.js | 24 ++++++++++++++++++ core/test/utils/api.js | 8 +++++- 19 files changed, 122 insertions(+), 4 deletions(-) diff --git a/core/server/models/base.js b/core/server/models/base.js index e71be8f8ad..91fa6f6535 100644 --- a/core/server/models/base.js +++ b/core/server/models/base.js @@ -83,9 +83,12 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({ // Base prototype properties will go here // Fix problems with dates fixDates: function (attrs) { + var self = this; + _.each(attrs, function (value, key) { - if (key.substr(-3) === '_at' && value !== null) { - attrs[key] = moment(attrs[key]).toDate(); + if (value !== null && schema.tables[self.tableName][key].type === 'dateTime') { + // convert dateTime value into a native javascript Date object + attrs[key] = moment(value).toDate(); } }); diff --git a/core/test/functional/routes/api/posts_test.js b/core/test/functional/routes/api/posts_test.js index 44be574ae4..2e3bfe623c 100644 --- a/core/test/functional/routes/api/posts_test.js +++ b/core/test/functional/routes/api/posts_test.js @@ -193,6 +193,7 @@ describe('Post API', function () { _.isBoolean(jsonResponse.posts[0].featured).should.eql(true); _.isBoolean(jsonResponse.posts[0].page).should.eql(true); jsonResponse.posts[0].author.should.be.a.Number; + testUtils.API.isISO8601(jsonResponse.posts[0].created_at).should.be.true; jsonResponse.posts[0].created_by.should.be.a.Number; jsonResponse.posts[0].tags[0].should.be.a.Number; done(); @@ -406,6 +407,8 @@ describe('Post API', function () { updatedPost.posts.should.exist; updatedPost.posts.length.should.be.above(0); updatedPost.posts[0].title.should.eql(newTitle); + testUtils.API.isISO8601(updatedPost.posts[0].created_at).should.be.true; + testUtils.API.isISO8601(updatedPost.posts[0].updated_at).should.be.true; testUtils.API.checkResponse(updatedPost.posts[0], 'post'); updatedPost.posts[0].tags.should.exist; diff --git a/core/test/functional/routes/api/settings_test.js b/core/test/functional/routes/api/settings_test.js index 103089eab0..7a77c1102a 100644 --- a/core/test/functional/routes/api/settings_test.js +++ b/core/test/functional/routes/api/settings_test.js @@ -93,6 +93,7 @@ describe('Settings API', function () { testUtils.API.checkResponseValue(jsonResponse.settings[0], ['id','uuid','key','value','type','created_at','created_by','updated_at','updated_by']); jsonResponse.settings[0].key.should.eql('title'); + testUtils.API.isISO8601(jsonResponse.settings[0].created_at).should.be.true; done(); }); }); diff --git a/core/test/functional/routes/api/tags_test.js b/core/test/functional/routes/api/tags_test.js index 86f4238c97..b629d1df56 100644 --- a/core/test/functional/routes/api/tags_test.js +++ b/core/test/functional/routes/api/tags_test.js @@ -71,6 +71,8 @@ describe('Tag API', function () { jsonResponse.tags.should.exist; jsonResponse.tags.should.have.length(6); testUtils.API.checkResponse(jsonResponse.tags[0], 'tag'); + testUtils.API.isISO8601(jsonResponse.tags[0].created_at).should.be.true; + done(); }); }); diff --git a/core/test/functional/routes/api/users_test.js b/core/test/functional/routes/api/users_test.js index 1dd6080ea1..09fe9e05f0 100644 --- a/core/test/functional/routes/api/users_test.js +++ b/core/test/functional/routes/api/users_test.js @@ -53,6 +53,31 @@ describe('User API', function () { httpServer.close(); }); + it('returns dates in ISO 8601 format', function (done) { + request.get(testUtils.API.getApiQuery('users/')) + .set('Authorization', 'Bearer ' + accesstoken) + .expect('Content-Type', /json/) + .expect(200) + .end(function (err, res) { + if (err) { + return done(err); + } + + var jsonResponse = res.body; + jsonResponse.users.should.exist; + testUtils.API.checkResponse(jsonResponse, 'users'); + + jsonResponse.users.should.have.length(1); + testUtils.API.checkResponse(jsonResponse.users[0], 'user'); + + testUtils.API.isISO8601(jsonResponse.users[0].last_login).should.be.true; + testUtils.API.isISO8601(jsonResponse.users[0].created_at).should.be.true; + testUtils.API.isISO8601(jsonResponse.users[0].updated_at).should.be.true; + + done(); + }); + }); + it('can retrieve all users', function (done) { request.get(testUtils.API.getApiQuery('users/')) .set('Authorization', 'Bearer ' + accesstoken) diff --git a/core/test/integration/api/api_posts_spec.js b/core/test/integration/api/api_posts_spec.js index 3203520021..a5e4871375 100644 --- a/core/test/integration/api/api_posts_spec.js +++ b/core/test/integration/api/api_posts_spec.js @@ -58,6 +58,8 @@ describe('Post API', function () { post = found.posts[0]; + post.created_at.should.be.an.instanceof(Date); + should.exist(post.tags); post.tags.length.should.be.above(0); testUtils.API.checkResponse(post.tags[0], 'tag'); diff --git a/core/test/integration/api/api_settings_spec.js b/core/test/integration/api/api_settings_spec.js index f30b300b99..e70087cfdc 100644 --- a/core/test/integration/api/api_settings_spec.js +++ b/core/test/integration/api/api_settings_spec.js @@ -64,6 +64,16 @@ describe('Settings API', function () { }).catch(getErrorDetails(done)); }); + it('uses Date objects for dateTime fields', function (done) { + return callApiWithContext(defaultContext, 'browse', {}).then(function (results) { + should.exist(results); + + results.settings[0].created_at.should.be.an.instanceof(Date); + + done(); + }).catch(getErrorDetails(done)); + }); + it('can browse', function (done) { return callApiWithContext(defaultContext, 'browse', {}).then(function (results) { should.exist(results); diff --git a/core/test/integration/api/api_tags_spec.js b/core/test/integration/api/api_tags_spec.js index 489f1b10c5..bfadb3c95a 100644 --- a/core/test/integration/api/api_tags_spec.js +++ b/core/test/integration/api/api_tags_spec.js @@ -36,6 +36,8 @@ describe('Tags API', function () { should.exist(results.tags); results.tags.length.should.be.above(0); testUtils.API.checkResponse(results.tags[0], 'tag'); + results.tags[0].created_at.should.be.an.instanceof(Date); + done(); }).catch(done); }); diff --git a/core/test/integration/api/api_users_spec.js b/core/test/integration/api/api_users_spec.js index 87bfbf0975..e2eb977897 100644 --- a/core/test/integration/api/api_users_spec.js +++ b/core/test/integration/api/api_users_spec.js @@ -3,6 +3,7 @@ var testUtils = require('../../utils'), should = require('should'), permissions = require('../../../server/permissions'), + UserModel = require('../../../server/models').User; // Stuff we are testing UsersAPI = require('../../../server/api/users'); @@ -62,6 +63,20 @@ describe('Users API', function () { }).catch(done); }); + it('dateTime fields are returned as Date objects', function (done) { + var userData = testUtils.DataGenerator.forModel.users[0]; + + UserModel.check({ email: userData.email, password: userData.password }).then(function (user) { + return UsersAPI.read({ id: user.id }); + }).then(function (results) { + results.users[0].created_at.should.be.an.instanceof(Date); + results.users[0].updated_at.should.be.an.instanceof(Date); + results.users[0].last_login.should.be.an.instanceof(Date); + + done(); + }).catch(done); + }); + it('admin can browse', function (done) { UsersAPI.browse({context: {user: 1}}).then(function (results) { should.exist(results); @@ -71,6 +86,7 @@ describe('Users API', function () { testUtils.API.checkResponse(results.users[0], 'user'); testUtils.API.checkResponse(results.users[1], 'user'); testUtils.API.checkResponse(results.users[2], 'user'); + done(); }).catch(done); }); @@ -115,6 +131,9 @@ describe('Users API', function () { testUtils.API.checkResponse(results, 'users'); results.users[0].id.should.eql(1); testUtils.API.checkResponse(results.users[0], 'user'); + + results.users[0].created_at.should.be.a.Date; + done(); }).catch(done); }); @@ -156,7 +175,7 @@ describe('Users API', function () { response.users.should.have.length(1); testUtils.API.checkResponse(response.users[0], 'user'); response.users[0].name.should.equal('Joe Blogger'); - + response.users[0].updated_at.should.be.a.Date; done(); }).catch(done); }); diff --git a/core/test/integration/model/model_app_fields_spec.js b/core/test/integration/model/model_app_fields_spec.js index 5d3f276b9e..412a892f1b 100644 --- a/core/test/integration/model/model_app_fields_spec.js +++ b/core/test/integration/model/model_app_fields_spec.js @@ -54,6 +54,8 @@ describe('App Fields Model', function () { AppFieldsModel.findOne({id: 1}).then(function (foundAppField) { should.exist(foundAppField); + foundAppField.get('created_at').should.be.an.instanceof(Date); + done(); }).catch(done); }); diff --git a/core/test/integration/model/model_app_settings_spec.js b/core/test/integration/model/model_app_settings_spec.js index 140983bb2f..6fda311b3e 100644 --- a/core/test/integration/model/model_app_settings_spec.js +++ b/core/test/integration/model/model_app_settings_spec.js @@ -54,6 +54,8 @@ describe('App Setting Model', function () { AppSettingModel.findOne({id: 1}).then(function (foundAppSetting) { should.exist(foundAppSetting); + foundAppSetting.get('created_at').should.be.an.instanceof(Date); + done(); }).catch(done); }); diff --git a/core/test/integration/model/model_apps_spec.js b/core/test/integration/model/model_apps_spec.js index 8644be0f03..dcd537b0f6 100644 --- a/core/test/integration/model/model_apps_spec.js +++ b/core/test/integration/model/model_apps_spec.js @@ -55,6 +55,8 @@ describe('App Model', function () { AppModel.findOne({id: 1}).then(function (foundApp) { should.exist(foundApp); + foundApp.get('created_at').should.be.an.instanceof(Date); + done(); }).catch(done); }); diff --git a/core/test/integration/model/model_permissions_spec.js b/core/test/integration/model/model_permissions_spec.js index 192b9e4038..c715cee441 100644 --- a/core/test/integration/model/model_permissions_spec.js +++ b/core/test/integration/model/model_permissions_spec.js @@ -43,6 +43,7 @@ describe('Permission Model', function () { it('can findOne', function (done) { PermissionModel.findOne({id: 1}).then(function (foundPermission) { should.exist(foundPermission); + foundPermission.get('created_at').should.be.an.instanceof(Date); done(); }).catch(done); diff --git a/core/test/integration/model/model_posts_spec.js b/core/test/integration/model/model_posts_spec.js index 02d4b4e499..2c87795f9e 100644 --- a/core/test/integration/model/model_posts_spec.js +++ b/core/test/integration/model/model_posts_spec.js @@ -42,6 +42,7 @@ describe('Post Model', function () { firstPost.tags.should.be.an.Array; firstPost.author.name.should.equal(DataGenerator.Content.users[0].name); firstPost.fields[0].key.should.equal(DataGenerator.Content.app_fields[0].key); + firstPost.created_at.should.be.an.instanceof(Date); firstPost.created_by.should.be.an.Object; firstPost.updated_by.should.be.an.Object; firstPost.published_by.should.be.an.Object; diff --git a/core/test/integration/model/model_roles_spec.js b/core/test/integration/model/model_roles_spec.js index d39ffe7036..a82ea02fce 100644 --- a/core/test/integration/model/model_roles_spec.js +++ b/core/test/integration/model/model_roles_spec.js @@ -42,6 +42,7 @@ describe('Role Model', function () { it('can findOne', function (done) { RoleModel.findOne({id: 1}).then(function (foundRole) { should.exist(foundRole); + foundRole.get('created_at').should.be.an.instanceof(Date); done(); }).catch(done); diff --git a/core/test/integration/model/model_settings_spec.js b/core/test/integration/model/model_settings_spec.js index 93be546d91..76b9baab5a 100644 --- a/core/test/integration/model/model_settings_spec.js +++ b/core/test/integration/model/model_settings_spec.js @@ -66,6 +66,7 @@ describe('Settings Model', function () { should.exist(found); found.get('value').should.equal(firstSetting.attributes.value); + found.get('created_at').should.be.an.instanceof(Date); done(); diff --git a/core/test/integration/model/model_tags_spec.js b/core/test/integration/model/model_tags_spec.js index 9e62751ab1..4b8c90a01d 100644 --- a/core/test/integration/model/model_tags_spec.js +++ b/core/test/integration/model/model_tags_spec.js @@ -31,6 +31,17 @@ describe('Tag Model', function () { }).catch(done); }); + it('uses Date objects for dateTime fields', function (done) { + TagModel.add(testUtils.DataGenerator.forModel.tags[0], { user: 1 }).then(function (tag) { + return TagModel.findOne({ id: tag.id }); + }).then(function (tag) { + should.exist(tag); + tag.get('created_at').should.be.an.instanceof(Date); + + done(); + }).catch(done); + }); + describe('a Post', function () { var PostModel = Models.Post; diff --git a/core/test/integration/model/model_users_spec.js b/core/test/integration/model/model_users_spec.js index 54bd2cfb60..bedc90e41e 100644 --- a/core/test/integration/model/model_users_spec.js +++ b/core/test/integration/model/model_users_spec.js @@ -150,6 +150,30 @@ describe('User Model', function run() { }).catch(done); }); + it('converts fetched dateTime fields to Date objects', function (done) { + var userData = testUtils.DataGenerator.forModel.users[0]; + + UserModel.check({ email: userData.email, password: userData.password }).then(function (user) { + return UserModel.findOne({ id: user.id }); + }).then(function (user) { + var last_login, + created_at, + updated_at; + + should.exist(user); + + last_login = user.get('last_login'); + created_at = user.get('created_at'); + updated_at = user.get('updated_at'); + + last_login.should.be.an.instanceof(Date); + created_at.should.be.an.instanceof(Date); + updated_at.should.be.an.instanceof(Date); + + done(); + }).catch(done); + }); + it('can findAll', function (done) { UserModel.findAll().then(function (results) { diff --git a/core/test/utils/api.js b/core/test/utils/api.js index 9dbd44f9ea..2f44eeaf72 100644 --- a/core/test/utils/api.js +++ b/core/test/utils/api.js @@ -1,4 +1,5 @@ var url = require('url'), + moment= require('moment'), ApiRouteBase = '/ghost/api/v0.1/', host = 'localhost', port = '2369', @@ -55,11 +56,16 @@ function checkResponse(jsonResponse, objectType) { checkResponseValue(jsonResponse, expectedProperties[objectType]); } +function isISO8601(date) { + return moment(date).parsingFlags().iso; +} + module.exports = { getApiURL: getApiURL, getApiQuery: getApiQuery, getSigninURL: getSigninURL, getAdminURL: getAdminURL, checkResponse: checkResponse, - checkResponseValue: checkResponseValue + checkResponseValue: checkResponseValue, + isISO8601: isISO8601 };