Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve handling of users and roles in admin #3388

Merged
merged 1 commit into from
Jul 24, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 40 additions & 22 deletions core/client/controllers/modals/invite-new-user.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,47 +9,65 @@ var InviteNewUserController = Ember.Controller.extend({
}
},

// @TODO: replace with roles from server - see issue #3196
roles: [
{
id: 3,
name: 'Author'
}
],
roles: Ember.computed(function () {
var roles = {},
self = this;

roles.promise = this.store.find('role', { permissions: 'assign' }).then(function (roles) {
return roles.rejectBy('name', 'Owner').sortBy('name');
}).then(function (roles) {
// After the promise containing the roles has been resolved and the array
// has been sorted, explicitly set the selectedRole for the Ember.Select.
// The explicit set is needed because the data-select-text attribute is
// not being set until a change is made in the dropdown list.
// This is only required with Ember.Select when it is bound to async data.
self.set('selectedRole', roles.get('firstObject'));

return roles;
});

return Ember.ArrayProxy.extend(Ember.PromiseProxyMixin).create(roles);
}),

actions: {
confirmAccept: function () {
var email = this.get('email'),
role_id = this.get('role'),
self = this,
newUser;
newUser,
role;

this.notifications.closePassive();

newUser = this.store.createRecord('user', {
newUser = self.store.createRecord('user', {
email: email,
role: role_id,
status: 'invited'
});

// no need to make an API request, the store will already have this role
role = self.store.getById('role', role_id);

newUser.get('roles').pushObject(role);

newUser.save().then(function () {
var notificationText = 'Invitation sent! (' + email + ')';

self.notifications.showSuccess(notificationText, false);
}).catch(function (errors) {
if (errors[0].message.indexOf('Email Error:') === -1) {
newUser.deleteRecord();
} else {
newUser.set('status', 'invited-pending');
}
self.notifications.closePassive();

// If sending the invitation email fails, the API will still return a status of 201
// but the user's status in the response object will be 'invited-pending'.
if (newUser.get('status') === 'invited-pending') {
self.notifications.showWarn('Invitation email was not sent. Please try resending.');
}
else {
self.notifications.showSuccess(notificationText, false);
}
}).catch(function (errors) {
newUser.deleteRecord();
self.notifications.closePassive();
self.notifications.showErrors(errors);
});

this.set('email', null);
this.set('role', null);

self.set('email', null);
self.set('role', null);
},

confirmReject: function () {
Expand Down
12 changes: 9 additions & 3 deletions core/client/controllers/settings/users/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,21 @@ import PaginationControllerMixin from 'ghost/mixins/pagination-controller';
var UsersIndexController = Ember.ArrayController.extend(PaginationControllerMixin, {
init: function () {
//let the PaginationControllerMixin know what type of model we will be paginating
//this is necesariy because we do not have access to the model inside the Controller::init method
//this is necessary because we do not have access to the model inside the Controller::init method
this._super({'modelType': 'user'});
},

users: Ember.computed.alias('model'),

activeUsers: Ember.computed.filterBy('users', 'active', true).property('users'),
activeUsers: Ember.computed.filter('users', function (user) {
return /^active|warn-[1-4]|locked$/.test(user.get('status'));
}),

invitedUsers: Ember.computed.filterBy('users', 'invited', true).property('users')
invitedUsers: Ember.computed.filter('users', function (user) {
var status = user.get('status');

return status === 'invited' || status === 'invited-pending';
})
});

export default UsersIndexController;
3 changes: 3 additions & 0 deletions core/client/models/role.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ var Role = DS.Model.extend({
uuid: DS.attr('string'),
name: DS.attr('string'),
description: DS.attr('string'),
created_at: DS.attr('moment-date'),
updated_at: DS.attr('moment-date'),

lowerCaseName: Ember.computed('name', function () {
return this.get('name').toLocaleLowerCase();
})
Expand Down
3 changes: 3 additions & 0 deletions core/client/models/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ var User = DS.Model.extend(NProgressSaveMixin, ValidationEngine, {
meta_description: DS.attr('string'),
last_login: DS.attr('moment-date'),
created_at: DS.attr('moment-date'),
created_by: DS.attr('number'),
updated_at: DS.attr('moment-date'),
updated_by: DS.attr('number'),
roles: DS.hasMany('role', { embedded: 'always' }),


Expand Down
27 changes: 7 additions & 20 deletions core/client/routes/settings/users/index.js
Original file line number Diff line number Diff line change
@@ -1,33 +1,20 @@
import PaginationRouteMixin from 'ghost/mixins/pagination-route';

var activeUsersPaginationSettings = {
include: 'roles',
var paginationSettings = {
page: 1,
limit: 20
};

var invitedUsersPaginationSettings = {
include: 'roles',
limit: 'all',
status: 'invited'
limit: 20,
status: 'all'
};

var UsersIndexRoute = Ember.Route.extend(Ember.SimpleAuth.AuthenticatedRouteMixin, PaginationRouteMixin, {
setupController: function (controller, model) {
this._super(controller, model.active);
this.setupPagination(activeUsersPaginationSettings);
this._super(controller, model);
this.setupPagination(paginationSettings);
},

model: function () {
// using `.filter` allows the template to auto-update when new models are pulled in from the server.
// we just need to 'return true' to allow all models by default.
return Ember.RSVP.hash({
inactive: this.store.filter('user', invitedUsersPaginationSettings, function () {
return true;
}),
active: this.store.filter('user', activeUsersPaginationSettings, function () {
return true;
})
return this.store.filter('user', paginationSettings, function () {
return true;
});
}
});
Expand Down
6 changes: 3 additions & 3 deletions core/client/templates/modals/invite-new-user.hbs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
{{#gh-modal-dialog action="closeModal" showClose=true type="action" animation="fade"
title="Invite a New User" confirm=confirm class="invite-new-user" }}
title="Invite a New User" confirm=confirm class="invite-new-user"}}

<fieldset>
<div class="form-group">
<label for="new-user-email"">Email Address</label>
<label for="new-user-email">Email Address</label>
{{input class="email" id="new-user-email" type="email" placeholder="Email Address" name="email" autofocus="autofocus"
autocapitalize="off" autocorrect="off" value=email }}
autocapitalize="off" autocorrect="off" value=email}}
</div>

<div class="form-group for-select">
Expand Down
4 changes: 2 additions & 2 deletions core/client/validators/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ var UserValidator = Ember.Object.create({
invited: function (model) {
var validationErrors = [],
email = model.get('email'),
role = model.get('role');
roles = model.get('roles');

if (!validator.isEmail(email)) {
validationErrors.push({ message: 'Please supply a valid email address' });
}

if (!validator.isLength(role, 1)) {
if (roles.length < 1) {
validationErrors.push({ message: 'Please select a role' });
}

Expand Down
17 changes: 9 additions & 8 deletions core/server/api/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,17 +107,18 @@ cacheInvalidationHeader = function (req, result) {
locationHeader = function (req, result) {
var apiRoot = config.urlFor('api'),
location,
post,
notification,
endpoint = req._parsedUrl.pathname;
newObject;

if (req.method === 'POST') {
if (result.hasOwnProperty('posts')) {
post = result.posts[0];
location = apiRoot + '/posts/' + post.id + '/?status=' + post.status;
} else if (endpoint === '/notifications/') {
notification = result.notifications;
location = apiRoot + endpoint + notification[0].id;
newObject = result.posts[0];
location = apiRoot + '/posts/' + newObject.id + '/?status=' + newObject.status;
} else if (result.hasOwnProperty('notifications')) {
newObject = result.notifications[0];
location = apiRoot + '/notifications/' + newObject.id;
} else if (result.hasOwnProperty('users')) {
newObject = result.users[0];
location = apiRoot + '/users/' + newObject.id;
}
}

Expand Down
11 changes: 7 additions & 4 deletions core/server/api/roles.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// # Posts API
// RESTful API for the Post resource
// # Roles API
// RESTful API for the Role resource
var when = require('when'),
_ = require('lodash'),
canThis = require('../permissions').canThis,
Expand All @@ -18,11 +18,13 @@ roles = {
* ### Browse
* Find all roles
*
* Will return all roles that the current user is able to assign
* If a 'permissions' property is passed in the options object then
* the results will be filtered based on whether or not the context user has the given
* permission on a role.
*
*
* @public
* @param {{context, page, limit, status, staticPages, tag}} options (optional)
* @param {{context, permissions}} options (optional)
* @returns {Promise(Roles)} Roles Collection
*/
browse: function browse(options) {
Expand All @@ -32,6 +34,7 @@ roles = {
return canThis(options.context).browse.role().then(function () {
return dataProvider.Role.findAll(options).then(function (foundRoles) {
if (options.permissions === 'assign') {

// Hacky implementation of filtering because when.filter is only available in when 3.4.0,
// but that's buggy and kills other tests and introduces Heisenbugs. Until we turn everything
// to Bluebird, this works. Sorry.
Expand Down
35 changes: 23 additions & 12 deletions core/server/api/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ var when = require('when'),
globalUtils = require('../utils'),
config = require('../config'),
mail = require('./mail'),
rolesAPI = require('./roles'),

docName = 'users',
ONE_DAY = 60 * 60 * 24 * 1000,
Expand Down Expand Up @@ -107,7 +108,7 @@ users = {
return when.reject(new errors.NotFoundError('User not found.'));
});
}, function () {
return when.reject(new errors.NoPermissionError('You do not have permission to edit this users.'));
return when.reject(new errors.NoPermissionError('You do not have permission to edit this user.'));
});
},

Expand Down Expand Up @@ -145,16 +146,22 @@ users = {
}

newUser = checkedUserData.users[0];
newUser.role = parseInt(newUser.roles[0].id || newUser.roles[0], 10);

if (newUser.email) {
newUser.name = object.users[0].email.substring(0, newUser.email.indexOf('@'));
newUser.password = globalUtils.uid(50);
newUser.status = 'invited';
// TODO: match user role with db and enforce permissions
newUser.role = 3;
} else {
return when.reject(new errors.BadRequestError('No email provided.'));
}
return rolesAPI.browse({ context: options.context, permissions: 'assign' }).then(function (results) {
// Make sure user is allowed to add a user with this role
if (!_.any(results.roles, { id: newUser.role })) {
return when.reject(new errors.NoPermissionError('Not allowed to create user with that role.'));
}

if (newUser.email) {
newUser.name = object.users[0].email.substring(0, newUser.email.indexOf('@'));
newUser.password = globalUtils.uid(50);
newUser.status = 'invited';
} else {
return when.reject(new errors.BadRequestError('No email provided.'));
}
});
}).then(function () {
return dataProvider.User.getByEmail(newUser.email);
}).then(function (foundUser) {
Expand Down Expand Up @@ -204,9 +211,13 @@ users = {
}).otherwise(function (error) {
if (error && error.type === 'EmailError') {
error.message = 'Error sending email: ' + error.message + ' Please check your email settings and resend the invitation.';
errors.logWarn(error.message);

// If sending the invitation failed, set status to invited-pending
return dataProvider.User.edit({status: 'invited-pending'}, {id: user.id}).then(function () {
return when.reject(error);
return dataProvider.User.edit({status: 'invited-pending'}, {id: user.id}).then(function (user) {
return dataProvider.User.findOne({ id: user.id }, options).then(function (user) {
return { users: [user] };
});
});
}
return when.reject(error);
Expand Down
5 changes: 2 additions & 3 deletions core/server/models/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,8 @@ User = ghostBookshelf.Model.extend({
//TODO (cont'd from above): * valid "active" statuses: active, warn-1, warn-2, warn-3, warn-4, locked
//TODO (cont'd from above): * valid "invited" statuses" invited, invited-pending

// the status provided.
if (options.status) {
// Filter on the status. A status of 'all' translates to no filter since we want all statuses
if (options.status && options.status !== 'all') {
// make sure that status is valid
//TODO: need a better way of getting a list of statuses other than hard-coding them...
options.status = _.indexOf(['active', 'warn-1', 'warn-2', 'warn-3', 'locked', 'invited'], options.status) !== -1 ? options.status : 'active';
Expand Down Expand Up @@ -390,7 +390,6 @@ User = ghostBookshelf.Model.extend({
// Save the user with the hashed password
return ghostBookshelf.Model.add.call(self, userData, options);
}).then(function (addedUser) {

// Assign the userData to our created user so we can pass it back
userData = addedUser;
if (!data.role) {
Expand Down