From ef1858b6c79a209b822ce42bb0f81270234ea172 Mon Sep 17 00:00:00 2001 From: Jason Williams Date: Mon, 14 Jul 2014 16:32:55 +0000 Subject: [PATCH] Fix validations on user settings page Closes #3271 - Change validations on both server and client to allow the Website field to be empty or a valid URL. - Add new schema validation helper isEmptyOrURL. - Remove duplicate call to UserValidator in the save action of the SettingsUser controller. - User.last_login and User.created_at are already Moment objects so Moment#fromNow can be called on them directly. --- .../client/controllers/settings/users/user.js | 22 +++++++------------ core/client/models/user.js | 1 + core/client/validators/user.js | 11 ++++++---- core/server/data/schema.js | 2 +- core/server/data/validation/index.js | 6 ++++- core/test/functional/routes/api/users_test.js | 2 +- .../integration/model/model_users_spec.js | 4 ++-- 7 files changed, 25 insertions(+), 23 deletions(-) diff --git a/core/client/controllers/settings/users/user.js b/core/client/controllers/settings/users/user.js index 47b824afdae6..d74b7607b9ae 100644 --- a/core/client/controllers/settings/users/user.js +++ b/core/client/controllers/settings/users/user.js @@ -33,11 +33,11 @@ var SettingsUserController = Ember.ObjectController.extend({ }.property('user.image'), last_login: function () { - return moment(this.get('user.last_login')).fromNow(); + return this.get('user.last_login').fromNow(); }.property('user.last_login'), created_at: function () { - return moment(this.get('user.created_at')).fromNow(); + return this.get('user.created_at').fromNow(); }.property('user.created_at'), actions: { @@ -70,19 +70,13 @@ var SettingsUserController = Ember.ObjectController.extend({ var user = this.get('user'), self = this; - self.notifications.closePassive(); - - user.validate({format: false}).then(function () { - user.save().then(function (model) { - self.notifications.closePassive(); - self.notifications.showSuccess('Settings successfully saved.'); + user.save({ format: false }).then(function (model) { + self.notifications.closePassive(); + self.notifications.showSuccess('Settings successfully saved.'); - return model; - }).catch(function (errors) { - self.notifications.closePassive(); - self.notifications.showErrors(errors); - }); - }, function (errors) { + return model; + }).catch(function (errors) { + self.notifications.closePassive(); self.notifications.showErrors(errors); }); }, diff --git a/core/client/models/user.js b/core/client/models/user.js index 55a5814e9a31..86cf4a188a0c 100644 --- a/core/client/models/user.js +++ b/core/client/models/user.js @@ -3,6 +3,7 @@ import NProgressSaveMixin from 'ghost/mixins/nprogress-save'; var User = DS.Model.extend(NProgressSaveMixin, ValidationEngine, { validationType: 'user', + uuid: DS.attr('string'), name: DS.attr('string'), slug: DS.attr('string'), diff --git a/core/client/validators/user.js b/core/client/validators/user.js index dc19d6633533..e7b01695f8b3 100644 --- a/core/client/validators/user.js +++ b/core/client/validators/user.js @@ -34,7 +34,6 @@ var UserValidator = Ember.Object.create({ location = model.get('location'), website = model.get('website'); - if (!validator.isLength(name, 0, 150)) { validationErrors.push({ message: 'Name is too long' }); } @@ -51,10 +50,14 @@ var UserValidator = Ember.Object.create({ validationErrors.push({ message: 'Location is too long' }); } - if (!validator.isURL(website, { protocols: ['http', 'https'], require_protocol: true }) || - !validator.isLength(website, 0, 2000)) { - validationErrors.push({ message: 'Please use a valid url' }); + if (!_.isEmpty(website) && + (!validator.isURL(website, { protocols: ['http', 'https'], require_protocol: true }) || + !validator.isLength(website, 0, 2000))) { + + validationErrors.push({ message: 'Website is not a valid url' }); } + + return validationErrors; } } }); diff --git a/core/server/data/schema.js b/core/server/data/schema.js index f03ce482987b..633e538b9d83 100644 --- a/core/server/data/schema.js +++ b/core/server/data/schema.js @@ -31,7 +31,7 @@ var db = { image: {type: 'text', maxlength: 2000, nullable: true}, cover: {type: 'text', maxlength: 2000, nullable: true}, bio: {type: 'string', maxlength: 200, nullable: true}, - website: {type: 'text', maxlength: 2000, nullable: true, validations: {'isURL': true}}, + website: {type: 'text', maxlength: 2000, nullable: true, validations: {'isEmptyOrURL': true}}, location: {type: 'text', maxlength: 65535, nullable: true}, accessibility: {type: 'text', maxlength: 65535, nullable: true}, status: {type: 'string', maxlength: 150, nullable: false, defaultTo: 'active'}, diff --git a/core/server/data/validation/index.js b/core/server/data/validation/index.js index 315349017136..85f57f2d4cda 100644 --- a/core/server/data/validation/index.js +++ b/core/server/data/validation/index.js @@ -23,6 +23,10 @@ validator.extend('notContains', function (str, badString) { return !_.contains(str, badString); }); +validator.extend('isEmptyOrURL', function (str) { + return (_.isEmpty(str) || validator.isURL(str, { protocols: ['http', 'https'], require_protocol: true })); +}); + // Validation validation against schema attributes // values are checked against the validation objects // form schema.js @@ -119,7 +123,7 @@ validateActiveTheme = function (themeName) { // Each validation's key is a method name and its value is an array of options // // eg: -// validations: { isUrl: true, isLength: [20, 40] } +// validations: { isURL: true, isLength: [20, 40] } // // will validate that a setting's length is a URL between 20 and 40 chars. // diff --git a/core/test/functional/routes/api/users_test.js b/core/test/functional/routes/api/users_test.js index c9c8b7269407..5e65d570c91f 100644 --- a/core/test/functional/routes/api/users_test.js +++ b/core/test/functional/routes/api/users_test.js @@ -306,7 +306,7 @@ describe('User API', function () { } var jsonResponse = res.body, - changedValue = 'joe-bloggs.ghost.org', + changedValue = 'http://joe-bloggs.ghost.org', dataToSend; jsonResponse.users[0].should.exist; testUtils.API.checkResponse(jsonResponse.users[0], 'user', ['roles']); diff --git a/core/test/integration/model/model_users_spec.js b/core/test/integration/model/model_users_spec.js index bedc90e41e85..7d7c09a247c8 100644 --- a/core/test/integration/model/model_users_spec.js +++ b/core/test/integration/model/model_users_spec.js @@ -215,10 +215,10 @@ describe('User Model', function run() { user.id.should.equal(firstUser); should.equal(user.website, null); - return UserModel.edit({website: 'some.newurl.com'}, {id: firstUser}); + return UserModel.edit({website: 'http://some.newurl.com'}, {id: firstUser}); }).then(function (edited) { should.exist(edited); - edited.attributes.website.should.equal('some.newurl.com'); + edited.attributes.website.should.equal('http://some.newurl.com'); done();