Refactor to remove author.email from API

refs #2330

- Pass through `options` to all toJSON calls on posts, tags, and users
- Use options.context.user to determine whether it's OK to return user.email
- Remove author.email handling code from frontend.js
This commit is contained in:
Hannah Wolfe 2015-04-17 22:27:04 +01:00
parent ede50f38a3
commit e26e83d40a
14 changed files with 61 additions and 77 deletions

View File

@ -228,7 +228,7 @@ authentication = {
userSettings.push({key: 'title', value: setupUser.blogTitle});
userSettings.push({key: 'description', value: 'Thoughts, stories and ideas.'});
}
setupUser = user.toJSON();
setupUser = user.toJSON(internal);
return settings.edit({settings: userSettings}, {context: {user: setupUser.id}});
}).then(function () {
var data = {

View File

@ -73,6 +73,7 @@ posts = {
read: function read(options) {
var attrs = ['id', 'slug', 'status'],
data = _.pick(options, attrs);
options = _.omit(options, attrs);
// only published posts if no user is present
@ -86,7 +87,7 @@ posts = {
return dataProvider.Post.findOne(data, options).then(function (result) {
if (result) {
return {posts: [result.toJSON()]};
return {posts: [result.toJSON(options)]};
}
return Promise.reject(new errors.NotFoundError('Post not found.'));
@ -112,7 +113,7 @@ posts = {
return dataProvider.Post.edit(checkedPostData.posts[0], options);
}).then(function (result) {
if (result) {
var post = result.toJSON();
var post = result.toJSON(options);
// If previously was not published and now is (or vice versa), signal the change
post.statusChanged = false;
@ -149,7 +150,7 @@ posts = {
return dataProvider.Post.add(checkedPostData.posts[0], options);
}).then(function (result) {
var post = result.toJSON();
var post = result.toJSON(options);
if (post.status === 'published') {
// When creating a new post that is published right now, signal the change

View File

@ -62,7 +62,7 @@ tags = {
return dataProvider.Tag.findOne(data, options).then(function (result) {
if (result) {
return {tags: [result.toJSON()]};
return {tags: [result.toJSON(options)]};
}
return Promise.reject(new errors.NotFoundError('Tag not found.'));
@ -88,7 +88,7 @@ tags = {
return utils.checkObject(object, docName).then(function (checkedTagData) {
return dataProvider.Tag.add(checkedTagData.tags[0], options);
}).then(function (result) {
var tag = result.toJSON();
var tag = result.toJSON(options);
return {tags: [tag]};
});
@ -117,7 +117,7 @@ tags = {
return dataProvider.Tag.edit(checkedTagData.tags[0], options);
}).then(function (result) {
if (result) {
var tag = result.toJSON();
var tag = result.toJSON(options);
return {tags: [tag]};
}

View File

@ -112,7 +112,7 @@ users = {
return dataProvider.User.findOne(data, options).then(function (result) {
if (result) {
return {users: [result.toJSON()]};
return {users: [result.toJSON(options)]};
}
return Promise.reject(new errors.NotFoundError('User not found.'));
@ -141,7 +141,7 @@ users = {
return dataProvider.User.edit(data.users[0], options)
.then(function (result) {
if (result) {
return {users: [result.toJSON()]};
return {users: [result.toJSON(options)]};
}
return Promise.reject(new errors.NotFoundError('User not found.'));
@ -162,7 +162,7 @@ users = {
return dataProvider.User.findOne(
{id: options.context.user, status: 'all'}, {include: ['roles']}
).then(function (contextUser) {
var contextRoleId = contextUser.related('roles').toJSON()[0].id;
var contextRoleId = contextUser.related('roles').toJSON(options)[0].id;
if (roleId !== contextRoleId && editedUserId === contextUser.id) {
return Promise.reject(new errors.NoPermissionError('You cannot change your own role.'));
@ -231,7 +231,7 @@ users = {
}
}
}).then(function (invitedUser) {
user = invitedUser.toJSON();
user = invitedUser.toJSON(options);
return sendInviteEmail(user);
}).then(function () {
// If status was invited-pending and sending the invitation succeeded, set status to invited.
@ -239,7 +239,7 @@ users = {
return dataProvider.User.edit(
{status: 'invited'}, _.extend({}, options, {id: user.id})
).then(function (editedUser) {
user = editedUser.toJSON();
user = editedUser.toJSON(options);
});
}
}).then(function () {

View File

@ -41,14 +41,6 @@ function getPostPage(options) {
* @return {Object} containing page variables
*/
function formatPageResponse(posts, page, extraValues) {
// Delete email from author for frontend output
// TODO: do this on API level if no context is available
posts = _.each(posts, function (post) {
if (post.author) {
delete post.author.email;
}
return post;
});
extraValues = extraValues || {};
var resp = {
@ -63,12 +55,6 @@ function formatPageResponse(posts, page, extraValues) {
* @return {Object} containing page variables
*/
function formatResponse(post) {
// Delete email from author for frontend output
// TODO: do this on API level if no context is available
if (post.author) {
delete post.author.email;
}
return {
post: post
};

View File

@ -3,6 +3,8 @@ var Promise = require('bluebird'),
models = require('../../models'),
utils = require('./utils'),
internal = utils.internal,
DataImporter;
DataImporter = function () {};
@ -12,13 +14,14 @@ DataImporter.prototype.importData = function (data) {
};
DataImporter.prototype.loadUsers = function () {
var users = {all: {}};
var users = {all: {}},
options = _.extend({}, {include: ['roles']}, internal);
return models.User.findAll({include: ['roles']}).then(function (_users) {
return models.User.findAll(options).then(function (_users) {
_users.forEach(function (user) {
users.all[user.get('email')] = {realId: user.get('id')};
if (user.related('roles').toJSON()[0] && user.related('roles').toJSON()[0].name === 'Owner') {
users.owner = user.toJSON();
if (user.related('roles').toJSON(options)[0] && user.related('roles').toJSON(options)[0].name === 'Owner') {
users.owner = user.toJSON(options);
}
});
@ -47,7 +50,7 @@ DataImporter.prototype.doUserImport = function (t, tableData, users, errors) {
if (d.isRejected()) {
errors = errors.concat(d.reason());
} else {
imported.push(d.value().toJSON());
imported.push(d.value().toJSON(internal));
}
});

View File

@ -35,6 +35,8 @@ stripProperties = function stripProperties(properties, data) {
};
utils = {
internal: internal,
processUsers: function preProcessUsers(tableData, owner, existingUsers, objs) {
// We need to:
// 1. figure out who the owner of the blog is
@ -164,7 +166,7 @@ utils = {
ops.push(models.Tag.findOne({name: tag.name}, {transacting: transaction}).then(function (_tag) {
if (!_tag) {
return models.Tag.add(tag, _.extend(internal, {transacting: transaction}))
return models.Tag.add(tag, _.extend({}, internal, {transacting: transaction}))
.catch(function (error) {
return Promise.reject({raw: error, model: 'tag', data: tag});
});
@ -196,7 +198,7 @@ utils = {
post.created_at = Date.now();
}
ops.push(models.Post.add(post, _.extend(internal, {transacting: transaction, importing: true}))
ops.push(models.Post.add(post, _.extend({}, internal, {transacting: transaction, importing: true}))
.catch(function (error) {
return Promise.reject({raw: error, model: 'post', data: post});
})
@ -225,7 +227,7 @@ utils = {
user.password = globalUtils.uid(50);
user.status = 'locked';
ops.push(models.User.add(user, _.extend(internal, {transacting: transaction}))
ops.push(models.User.add(user, _.extend({}, internal, {transacting: transaction}))
.catch(function (error) {
return Promise.reject({raw: error, model: 'user', data: user});
}));
@ -255,7 +257,7 @@ utils = {
datum.key = updatedSettingKeys[datum.key] || datum.key;
});
ops.push(models.Settings.edit(tableData, _.extend(internal, {transacting: transaction})).catch(function (error) {
ops.push(models.Settings.edit(tableData, _.extend({}, internal, {transacting: transaction})).catch(function (error) {
// Ignore NotFound errors
if (!(error instanceof errors.NotFoundError)) {
return Promise.reject({raw: error, model: 'setting', data: tableData});
@ -278,7 +280,7 @@ utils = {
// Avoid duplicates
ops.push(models.App.findOne({name: app.name}, {transacting: transaction}).then(function (_app) {
if (!_app) {
return models.App.add(app, _.extend(internal, {transacting: transaction}))
return models.App.add(app, _.extend({}, internal, {transacting: transaction}))
.catch(function (error) {
return Promise.reject({raw: error, model: 'app', data: app});
});

View File

@ -152,7 +152,7 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
// if include is set, expand to full object
var fullKey = _.isEmpty(options.name) ? key : options.name + '.' + key;
if (_.contains(self.include, fullKey)) {
attrs[key] = relation.toJSON({name: fullKey, include: self.include});
attrs[key] = relation.toJSON(_.extend({}, options, {name: fullKey, include: self.include}));
}
}
});

View File

@ -26,7 +26,7 @@ getPermalinkSetting = function (model, attributes, options) {
}
return ghostBookshelf.model('Settings').findOne({key: 'permalinks'}).then(function (response) {
if (response) {
response = response.toJSON();
response = response.toJSON(options);
permalinkSetting = response.hasOwnProperty('value') ? response.value : '';
}
});
@ -204,7 +204,7 @@ Post = ghostBookshelf.Model.extend({
var doNotExist = [],
sequenceTasks = [];
existingTags = existingTags.toJSON();
existingTags = existingTags.toJSON(options);
doNotExist = _.reject(self.myTags, function (tag) {
return _.any(existingTags, function (existingTag) {
@ -489,14 +489,7 @@ Post = ghostBookshelf.Model.extend({
pagination.next = null;
pagination.prev = null;
// Pass include to each model so that toJSON works correctly
if (options.include) {
_.each(postCollection.models, function (item) {
item.include = options.include;
});
}
data.posts = postCollection.toJSON();
data.posts = postCollection.toJSON(options);
data.meta = meta;
meta.pagination = pagination;
@ -514,14 +507,14 @@ Post = ghostBookshelf.Model.extend({
if (tagInstance) {
meta.filters = {};
if (!tagInstance.isNew()) {
meta.filters.tags = [tagInstance.toJSON()];
meta.filters.tags = [tagInstance.toJSON(options)];
}
}
if (authorInstance) {
meta.filters = {};
if (!authorInstance.isNew()) {
meta.filters.author = authorInstance.toJSON();
meta.filters.author = authorInstance.toJSON(options);
}
}
@ -554,7 +547,7 @@ Post = ghostBookshelf.Model.extend({
return ghostBookshelf.Model.findOne.call(this, data, options).then(function (post) {
if ((withNext || withPrev) && post && !post.page) {
var postData = post.toJSON(),
var postData = post.toJSON(options),
publishedAt = postData.published_at,
prev,
next;

View File

@ -163,7 +163,7 @@ Tag = ghostBookshelf.Model.extend({
pagination.next = null;
pagination.prev = null;
data.tags = tagCollection.toJSON();
data.tags = tagCollection.toJSON(options);
data.meta = meta;
meta.pagination = pagination;

View File

@ -124,6 +124,10 @@ User = ghostBookshelf.Model.extend({
// remove password hash for security reasons
delete attrs.password;
if (!options || !options.context || (!options.context.user && !options.context.internal)) {
delete attrs.email;
}
return attrs;
},
@ -340,14 +344,7 @@ User = ghostBookshelf.Model.extend({
pagination.next = null;
pagination.prev = null;
// Pass include to each model so that toJSON works correctly
if (options.include) {
_.each(userCollection.models, function (item) {
item.include = options.include;
});
}
data.users = userCollection.toJSON();
data.users = userCollection.toJSON(options);
data.meta = meta;
meta.pagination = pagination;
@ -365,7 +362,7 @@ User = ghostBookshelf.Model.extend({
if (roleInstance) {
meta.filters = {};
if (!roleInstance.isNew()) {
meta.filters.roles = [roleInstance.toJSON()];
meta.filters.roles = [roleInstance.toJSON(options)];
}
}
@ -873,7 +870,7 @@ User = ghostBookshelf.Model.extend({
contextUser = results[1];
// check if user has the owner role
var currentRoles = contextUser.toJSON().roles;
var currentRoles = contextUser.toJSON(options).roles;
if (!_.any(currentRoles, {id: ownerRole.id})) {
return Promise.reject(new errors.NoPermissionError('Only owners are able to transfer the owner role.'));
}
@ -883,7 +880,7 @@ User = ghostBookshelf.Model.extend({
}).then(function (results) {
var adminRole = results[0],
user = results[1],
currentRoles = user.toJSON().roles;
currentRoles = user.toJSON(options).roles;
if (!_.any(currentRoles, {id: adminRole.id})) {
return Promise.reject(new errors.ValidationError('Only administrators can be assigned the owner role.'));
@ -898,7 +895,8 @@ User = ghostBookshelf.Model.extend({
.query('whereIn', 'id', [contextUser.id, results[2]])
.fetch({withRelated: ['roles']});
}).then(function (users) {
return users.toJSON({include: ['roles']});
options.include = ['roles'];
return users.toJSON(options);
});
},

View File

@ -139,12 +139,19 @@ describe('Users API', function () {
});
describe('Read', function () {
function checkReadResponse(response) {
function checkReadResponse(response, noEmail) {
should.exist(response);
should.not.exist(response.meta);
should.exist(response.users);
response.users[0].id.should.eql(1);
testUtils.API.checkResponse(response.users[0], 'user');
if (noEmail) {
// Email should be missing
testUtils.API.checkResponse(response.users[0], 'user', [], ['email']);
should.not.exist(response.users[0].email);
} else {
testUtils.API.checkResponse(response.users[0], 'user');
}
response.users[0].created_at.should.be.a.Date;
}
@ -180,7 +187,7 @@ describe('Users API', function () {
it('No-auth can read', function (done) {
UserAPI.read({id: userIdFor.owner}).then(function (response) {
checkReadResponse(response);
checkReadResponse(response, true);
done();
}).catch(done);
});

View File

@ -311,8 +311,7 @@ describe('Frontend Controller', function () {
published_at: new Date('2014/1/2').getTime(),
author: {
id: 1,
name: 'Test User',
email: 'test@ghost.org'
name: 'Test User'
}
}],
mockTags = [{
@ -401,7 +400,6 @@ describe('Frontend Controller', function () {
render: function (view, context) {
view.should.equal('tag');
context.tag.should.equal(mockTags[0]);
should.not.exist(context.posts[0].author.email);
done();
}
};
@ -633,7 +631,6 @@ describe('Frontend Controller', function () {
render: function (view, context) {
view.should.equal('page-' + mockPosts[2].posts[0].slug);
context.post.should.equal(mockPosts[2].posts[0]);
should.not.exist(context.post.author.email);
done();
}
};
@ -663,7 +660,6 @@ describe('Frontend Controller', function () {
render: function (view, context) {
view.should.equal('page');
context.post.should.equal(mockPosts[0].posts[0]);
should.not.exist(context.post.author.email);
done();
}
};
@ -856,7 +852,6 @@ describe('Frontend Controller', function () {
view.should.equal('post');
context.post.should.exist;
context.post.should.equal(mockPosts[1].posts[0]);
should.not.exist(context.post.author.email);
done();
}
};
@ -971,7 +966,6 @@ describe('Frontend Controller', function () {
view.should.equal('post');
context.post.should.exist;
context.post.should.equal(mockPosts[1].posts[0]);
should.not.exist(context.post.author.email);
done();
}
};
@ -1102,7 +1096,6 @@ describe('Frontend Controller', function () {
view.should.equal('post');
should.exist(context.post);
context.post.should.equal(mockPosts[1].posts[0]);
should.not.exist(context.post.author.email);
done();
}
};
@ -1234,7 +1227,6 @@ describe('Frontend Controller', function () {
view.should.equal('post');
should.exist(context.post);
context.post.should.equal(mockPosts[1].posts[0]);
should.not.exist(context.post.author.email);
done();
}
};

View File

@ -1,4 +1,5 @@
var url = require('url'),
var _ = require('lodash'),
url = require('url'),
moment = require('moment'),
config = require('../../server/config'),
ApiRouteBase = '/ghost/api/v0.1/',
@ -62,9 +63,10 @@ function checkResponseValue(jsonResponse, properties) {
Object.keys(jsonResponse).length.should.eql(properties.length);
}
function checkResponse(jsonResponse, objectType, additionalProperties) {
function checkResponse(jsonResponse, objectType, additionalProperties, missingProperties) {
var checkProperties = expectedProperties[objectType];
checkProperties = additionalProperties ? checkProperties.concat(additionalProperties) : checkProperties;
checkProperties = missingProperties ? _.xor(checkProperties, missingProperties) : checkProperties;
checkResponseValue(jsonResponse, checkProperties);
}