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.
TryGhost · Jul 5, 2014 · 05d199f · 05d199f
1 parent d5c2e1b
commit 05d199f
Show file tree

Hide file tree

Showing 19 changed files with 122 additions and 4 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
 };