Skip to content

Commit

Permalink
Fix validations on user settings page
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jaswilli committed Jul 14, 2014
1 parent 6cdf9e5 commit ef1858b
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 23 deletions.
22 changes: 8 additions & 14 deletions core/client/controllers/settings/users/user.js
Expand Up @@ -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: {
Expand Down Expand Up @@ -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);
});
},
Expand Down
1 change: 1 addition & 0 deletions core/client/models/user.js
Expand Up @@ -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'),
Expand Down
11 changes: 7 additions & 4 deletions core/client/validators/user.js
Expand Up @@ -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' });
}
Expand All @@ -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;
}
}
});
Expand Down
2 changes: 1 addition & 1 deletion core/server/data/schema.js
Expand Up @@ -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'},
Expand Down
6 changes: 5 additions & 1 deletion core/server/data/validation/index.js
Expand Up @@ -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
Expand Down Expand Up @@ -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.
//
Expand Down
2 changes: 1 addition & 1 deletion core/test/functional/routes/api/users_test.js
Expand Up @@ -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']);
Expand Down
4 changes: 2 additions & 2 deletions core/test/integration/model/model_users_spec.js
Expand Up @@ -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();

Expand Down

0 comments on commit ef1858b

Please sign in to comment.