From d9ea5977cd9cd596e81c4b9282512d04d8f22f1f Mon Sep 17 00:00:00 2001 From: gaba Date: Thu, 5 Jan 2017 18:24:05 -0300 Subject: [PATCH 01/11] Unique displayName for the User. --- models/user.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/models/user.js b/models/user.js index 7df713337..f4ddbd895 100644 --- a/models/user.js +++ b/models/user.js @@ -46,7 +46,11 @@ const UserSchema = new mongoose.Schema({ // This is sourced from the social provider or set manually during user setup // and simply provides a name to display for the given user. - displayName: String, + displayName: { + type: String, + unique: true, + required: true + }, // This is true when the user account is disabled, no action should be // acknowledged when they are disabled. Logins are also prevented. From 712e436f584f4a7ba861fae0e8b00dfe16caee80 Mon Sep 17 00:00:00 2001 From: gaba Date: Fri, 6 Jan 2017 16:38:44 -0300 Subject: [PATCH 02/11] Cleans error on email address or name. We need to go through all the errors and manage them better with translations. --- client/coral-framework/actions/auth.js | 4 +++- client/coral-framework/translations.json | 6 ++++-- client/coral-sign-in/translations.js | 2 ++ models/user.js | 2 +- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/client/coral-framework/actions/auth.js b/client/coral-framework/actions/auth.js index 106224c25..f06ea7526 100644 --- a/client/coral-framework/actions/auth.js +++ b/client/coral-framework/actions/auth.js @@ -82,7 +82,9 @@ export const fetchSignUp = formData => (dispatch) => { dispatch(changeView('SIGNIN')); }, 3000); }) - .catch(() => dispatch(signUpFailure(lang.t('error.emailInUse')))); // We need to inprove error handling. TODO (bc) + .catch(() => { + dispatch(signUpFailure(lang.t('error.emailORusernameInUse'))); + }); // We need to improve error handling. TODO (bc) }; // Forgot Password Actions diff --git a/client/coral-framework/translations.json b/client/coral-framework/translations.json index 06a39944a..99d3341b1 100644 --- a/client/coral-framework/translations.json +++ b/client/coral-framework/translations.json @@ -10,7 +10,8 @@ "displayName": "Display name is too short", "confirmPassword": "Passwords don`t match. Please, check again", "emailPasswordError": "Email and/or password combination incorrect.", - "emailInUse": "Email address already in use" + "emailInUse": "Email address already in use", + "emailORusernameInUse": "Email address or Username already in use" } }, "es": { @@ -24,7 +25,8 @@ "displayName": "El nombre es muy corto", "confirmPassword": "Las contraseñas no coinciden", "emailPasswordError": "Email y/o contraseña incorrecta.", - "emailInUse": "Email address already in use" + "emailInUse": "Correo electronico en uso.", + "emailORusernameInUse": "Correo o Nombre en uso." } } } diff --git a/client/coral-sign-in/translations.js b/client/coral-sign-in/translations.js index 200cb91f1..a50ad6f54 100644 --- a/client/coral-sign-in/translations.js +++ b/client/coral-sign-in/translations.js @@ -19,6 +19,7 @@ export default { alreadyHaveAnAccount: 'Already have an account?', recoverPassword: 'Recover password', emailInUse: 'Email address already in use', + emailORusernameInUse: 'Email address or Username already in use', requiredField: 'This field is required', passwordsDontMatch: 'Passwords don\'t match.', checkTheForm: 'Invalid Form. Please, check the fields' @@ -44,6 +45,7 @@ export default { alreadyHaveAnAccount: 'Ya tienes una cuenta?', recoverPassword: 'Recuperar contraseña', emailInUse: 'Este email se encuentra en uso', + emailORusernameInUse: 'Este email ó nombre se encuentran en uso', requiredField: 'Este campo es requerido', passwordsDontMatch: 'Las contraseñas no coinciden', checkTheForm: 'Formulario Inválido. Por favor, completa los campos' diff --git a/models/user.js b/models/user.js index d8a116ae5..bcc22d895 100644 --- a/models/user.js +++ b/models/user.js @@ -356,7 +356,7 @@ UserService.createLocalUser = (email, password, displayName) => { user.save((err) => { if (err) { if (err.code === 11000) { - return reject('Email address already in use'); + return reject(new Error('Email address or Name already in use')); } return reject(err); } From 4dbc73f9abf77af12e2f5080520405a98e548694 Mon Sep 17 00:00:00 2001 From: gaba Date: Fri, 6 Jan 2017 16:57:25 -0300 Subject: [PATCH 03/11] Add a test on already used display name. --- tests/models/user.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/models/user.js b/tests/models/user.js index 7484880bf..46584a9dc 100644 --- a/tests/models/user.js +++ b/tests/models/user.js @@ -80,6 +80,22 @@ describe('models.User', () => { }); + describe('#createLocalUser', () => { + it('should not create a user with duplicate display name', () => { + return User.createLocalUsers([{ + email: 'otrostampi@gmail.com', + displayName: 'Stampi', + password: '1Coralito!' + }]) + .then((user) => { + expect(user).to.be.null; + }) + .catch((error) => { + expect(error).to.not.be.null; + }); + }); + }); + describe('#createEmailConfirmToken', () => { it('should create a token for a valid user', () => { From 15e7ef6368f89fda4d9a7e2254a1ebe259f65110 Mon Sep 17 00:00:00 2001 From: gaba Date: Thu, 12 Jan 2017 10:46:18 -0800 Subject: [PATCH 04/11] Adds an error const. --- errors.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/errors.js b/errors.js index 0651b4fff..68509520d 100644 --- a/errors.js +++ b/errors.js @@ -15,6 +15,10 @@ const ErrEmailTaken = new Error('Email address already in use'); ErrEmailTaken.translation_key = 'EMAIL_IN_USE'; ErrEmailTaken.status = 400; +const ErrEmailDisplaynameTaken = new Error('Email or Display name already in use'); +ErrEmailDisplaynameTaken.translation_key = 'EMAIL_DISPLAYNAME_IN_USE'; +ErrEmailDisplaynameTaken.status = 400; + const ErrSpecialChars = new Error('No special characters are allowed in a display name'); ErrSpecialChars.translation_key = 'NO_SPECIAL_CHARACTERS'; ErrSpecialChars.status = 400; From acf3ad5e28ccf2d24fa2e9451e52480c275bae3b Mon Sep 17 00:00:00 2001 From: gaba Date: Thu, 12 Jan 2017 11:10:07 -0800 Subject: [PATCH 05/11] Some conflicts were added. --- client/coral-framework/translations.json | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/client/coral-framework/translations.json b/client/coral-framework/translations.json index 08afa8d9a..8015d71e5 100644 --- a/client/coral-framework/translations.json +++ b/client/coral-framework/translations.json @@ -31,10 +31,8 @@ "displayName": "Los nombres pueden contener letras, números y _", "confirmPassword": "Las contraseñas no coinciden", "emailPasswordError": "Email y/o contraseña incorrecta.", -<<<<<<< HEAD "emailInUse": "Correo electronico en uso.", - "emailORusernameInUse": "Correo o Nombre en uso." -======= + "emailORusernameInUse": "Correo o Nombre en uso.", "EMAIL_REQUIRED": "Se requiere una dirección de correo electrónico", "PASSWORD_REQUIRED": "Debe ingresar una contraseña", "PASSWORD_LENGTH": "La contraseña es muy corta", @@ -42,7 +40,6 @@ "DISPLAY_NAME_REQUIRED": "Debe ingresar un nombre", "NO_SPECIAL_CHARACTERS": "Los nombres pueden contener letras, números y _", "PROFANITY_ERROR": "Los nombres no pueden contener blasfemias. Por favor contacte al administrador si cree que esto es un error" ->>>>>>> master } } } From 1027b19a499ad47e812b54eced7f40c0ba1b769e Mon Sep 17 00:00:00 2001 From: gaba Date: Thu, 12 Jan 2017 14:43:36 -0800 Subject: [PATCH 06/11] Added regex to look for the field that is duplicated. Not sure if there is a better way to do it. --- client/coral-framework/translations.json | 7 ++++--- errors.js | 9 +++++---- models/user.js | 6 +++++- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/client/coral-framework/translations.json b/client/coral-framework/translations.json index 8015d71e5..5bf9b40a9 100644 --- a/client/coral-framework/translations.json +++ b/client/coral-framework/translations.json @@ -14,7 +14,8 @@ "PASSWORD_REQUIRED": "Must input a password", "PASSWORD_LENGTH": "Password is too short", "EMAIL_IN_USE": "Email address already in use", - "EMAIL_DISPLAYNAME_IN_USE": "Email address or display name already in use", + "EMAIL_DISPLAY_NAME_IN_USE": "Email address or display name already in use", + "DISPLAYNAME_IN_USE": "Display name already in use", "DISPLAY_NAME_REQUIRED": "Must input a display name", "NO_SPECIAL_CHARACTERS": "Display names can contain letters, numbers and _ only", "PROFANITY_ERROR": "Display names must not contain profanity. Please contact the administrator if you believe this to be in error." @@ -31,12 +32,12 @@ "displayName": "Los nombres pueden contener letras, números y _", "confirmPassword": "Las contraseñas no coinciden", "emailPasswordError": "Email y/o contraseña incorrecta.", - "emailInUse": "Correo electronico en uso.", - "emailORusernameInUse": "Correo o Nombre en uso.", "EMAIL_REQUIRED": "Se requiere una dirección de correo electrónico", "PASSWORD_REQUIRED": "Debe ingresar una contraseña", "PASSWORD_LENGTH": "La contraseña es muy corta", "EMAIL_IN_USE": "La dirección de correo electrónico se encuentra en uso", + "EMAIL_DISPLAY_NAME_IN_USE": "Correo o Nombre en uso.", + "DISPLAYNAME_IN_USE": "Nombre en uso.", "DISPLAY_NAME_REQUIRED": "Debe ingresar un nombre", "NO_SPECIAL_CHARACTERS": "Los nombres pueden contener letras, números y _", "PROFANITY_ERROR": "Los nombres no pueden contener blasfemias. Por favor contacte al administrador si cree que esto es un error" diff --git a/errors.js b/errors.js index 68509520d..0bd8711c6 100644 --- a/errors.js +++ b/errors.js @@ -15,9 +15,9 @@ const ErrEmailTaken = new Error('Email address already in use'); ErrEmailTaken.translation_key = 'EMAIL_IN_USE'; ErrEmailTaken.status = 400; -const ErrEmailDisplaynameTaken = new Error('Email or Display name already in use'); -ErrEmailDisplaynameTaken.translation_key = 'EMAIL_DISPLAYNAME_IN_USE'; -ErrEmailDisplaynameTaken.status = 400; +const ErrDisplayTaken = new Error('Display name already in use'); +ErrDisplayTaken.translation_key = 'DISPLAYNAME_IN_USE'; +ErrDisplayTaken.status = 400; const ErrSpecialChars = new Error('No special characters are allowed in a display name'); ErrSpecialChars.translation_key = 'NO_SPECIAL_CHARACTERS'; @@ -45,5 +45,6 @@ module.exports = { ErrEmailTaken, ErrSpecialChars, ErrMissingDisplay, - ErrContainsProfanity + ErrContainsProfanity, + ErrDisplayTaken }; diff --git a/models/user.js b/models/user.js index 0b6241dfe..b0ead1c5a 100644 --- a/models/user.js +++ b/models/user.js @@ -383,7 +383,11 @@ UserService.createLocalUser = (email, password, displayName) => { user.save((err) => { if (err) { if (err.code === 11000) { - return reject(errors.ErrEmailDisplaynameTaken); + const regex = new RegExp('displayName', 'g'); + if (regex.exec(err.message)[0] === 'displayName') { + return reject(errors.ErrDisplayTaken); + } + return reject(errors.ErrEmailTaken); } return reject(err); } From ad0f530fd3c68b7b8d53b02c68575f5394494c3e Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Thu, 12 Jan 2017 17:07:35 -0700 Subject: [PATCH 07/11] don't return unpublished comments in My Comments section. add tests --- models/comment.js | 12 ++++++++++-- routes/api/comments/index.js | 2 +- tests/models/comment.js | 17 +++++++++++++++++ tests/routes/api/comments/index.js | 5 ++--- 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/models/comment.js b/models/comment.js index 9bd9163c4..d99cbed06 100644 --- a/models/comment.js +++ b/models/comment.js @@ -282,8 +282,16 @@ CommentSchema.statics.all = () => Comment.find(); * probably to be paginated at some point in the future * @return {Promise} array resolves to an array of comments by that user */ -CommentSchema.statics.findByUserId = function (author_id) { - return Comment.find({author_id}); +CommentSchema.statics.findByUserId = function (author_id, admin = false) { + + // do not return un-published comments for non-admins + let query = {author_id}; + + if (!admin) { + query.$nor = [{status: 'premod'}, {status: 'rejected'}]; + } + + return Comment.find(query); }; // Comment model. diff --git a/routes/api/comments/index.js b/routes/api/comments/index.js index 8bf3616eb..1d5d73ea0 100644 --- a/routes/api/comments/index.js +++ b/routes/api/comments/index.js @@ -48,7 +48,7 @@ router.get('/', (req, res, next) => { // otherwise this will be a vulnerability if you pass user_id and something else, // the app will return admin-level data without the proper checks if (user_id) { - query = Comment.findByUserId(user_id); + query = Comment.findByUserId(user_id, authorization.has(req.user, 'admin')); } else if (status) { query = assetIDWrap(Comment.findByStatus(status === 'new' ? null : status)); } else if (action_type) { diff --git a/tests/models/comment.js b/tests/models/comment.js index 20590dc3d..419fb4d79 100644 --- a/tests/models/comment.js +++ b/tests/models/comment.js @@ -209,6 +209,23 @@ describe('models.Comment', () => { }); }); + describe('#findByUserId', () => { + it('should return all comments if admin', () => { + return Comment.findByUserId('456', true) + .then(comments => { + expect(comments).to.have.length(4); + }); + }); + + it('should not return premod and rejected comments if not admin', () => { + return Comment.findByUserId('456') + .then(comments => { + expect(comments).to.have.length(1); + }); + }); + + }); + describe('#changeStatus', () => { it('should change the status of a comment from no status', () => { diff --git a/tests/routes/api/comments/index.js b/tests/routes/api/comments/index.js index 5edb52099..ed1f3d22f 100644 --- a/tests/routes/api/comments/index.js +++ b/tests/routes/api/comments/index.js @@ -83,15 +83,14 @@ describe('/api/v1/comments', () => { ]); }); - it('should return only the owner’s comments if the user is not an admin', () => { + it('should return only the owner’s published comments if the user is not an admin', () => { return chai.request(app) .get('/api/v1/comments?user_id=456') .set(passport.inject({id: '456', roles: []})) .then(res => { expect(res).to.have.status(200); - expect(res.body.comments).to.have.length(2); + expect(res.body.comments).to.have.length(1); expect(res.body.comments[0]).to.have.property('author_id', '456'); - expect(res.body.comments[1]).to.have.property('author_id', '456'); }); }); From c90230c115f9684ffe1f20d04fd230835305537b Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 12 Jan 2017 17:55:08 -0700 Subject: [PATCH 08/11] Adjusted stream to preserve comment view state --- models/comment.js | 46 ++++------ routes/api/stream/index.js | 11 +-- tests/models/comment.js | 22 ----- tests/routes/api/comments/index.js | 23 +++++ tests/routes/api/stream/index.js | 137 ++++++++++++++++++++++------- util.js | 5 +- 6 files changed, 147 insertions(+), 97 deletions(-) diff --git a/models/comment.js b/models/comment.js index 9bd9163c4..3f8156b24 100644 --- a/models/comment.js +++ b/models/comment.js @@ -137,27 +137,18 @@ CommentSchema.statics.findByAssetId = (asset_id) => Comment.find({ }); /** - * Finds the accepted comments by the asset_id get the comments that are - * accepted. - * @param {String} asset_id identifier of the asset which owns the comments (uuid) - * @return {Promise} + * findByAssetIdWithStatuses finds all the comments where the asset id matches + * what's provided and the status is one of the ones listed in the statuses + * array. + * @param {String} asset_id the asset id to search by + * @param {Array} [statuses=[]] the array of statuses to search by + * @return {Promise} resolves to an array of comments */ -CommentSchema.statics.findAcceptedByAssetId = (asset_id) => Comment.find({ +CommentSchema.statics.findByAssetIdWithStatuses = (asset_id, statuses = []) => Comment.find({ asset_id, - status: 'accepted' -}); - -/** - * Finds the new and accepted comments by the asset_id. - * @param {String} asset_id identifier of the asset which owns the comments (uuid) - * @return {Promise} - */ -CommentSchema.statics.findAcceptedAndNewByAssetId = (asset_id) => Comment.find({ - asset_id, - $or: [ - {status: 'accepted'}, - {status: null} - ] + status: { + $in: statuses + } }); /** @@ -198,19 +189,12 @@ CommentSchema.statics.findByStatus = (status = null) => { */ CommentSchema.statics.moderationQueue = (status, asset_id = null) => { - /** - * This adds the asset_id requirement to the query if the asset_id is defined. - */ - const assetIDWrap = (query) => { - if (asset_id) { - query = query.where('asset_id', asset_id); - } + // Fetch the comments with statuses. + let comments = Comment.findByStatus(status); - return query; - }; - - // Pre-moderation: New comments are shown in the moderator queues immediately. - let comments = assetIDWrap(Comment.findByStatus(status)); + if (asset_id) { + comments = comments.where('asset_id', asset_id); + } return comments; }; diff --git a/routes/api/stream/index.js b/routes/api/stream/index.js index 540e36e52..06845a21a 100644 --- a/routes/api/stream/index.js +++ b/routes/api/stream/index.js @@ -48,20 +48,11 @@ router.get('/', (req, res, next) => { settings.merge(asset.settings); } - // Fetch the appropriate comments stream. - let comments; - - if (settings.moderation === 'pre') { - comments = Comment.findAcceptedByAssetId(asset.id); - } else { - comments = Comment.findAcceptedAndNewByAssetId(asset.id); - } - return Promise.all([ // This is the promised component... Fetch the comments based on the // moderation settings. - comments, + Comment.findByAssetIdWithStatuses(asset.id, [null, 'accepted']), // Send back the reference to the asset. asset, diff --git a/tests/models/comment.js b/tests/models/comment.js index 20590dc3d..455b937f3 100644 --- a/tests/models/comment.js +++ b/tests/models/comment.js @@ -161,28 +161,6 @@ describe('models.Comment', () => { expect(result[2]).to.have.property('body', 'comment 40'); }); }); - - it('should find an array of accepted comments by asset id', () => { - return Comment.findAcceptedByAssetId('123').then((result) => { - expect(result).to.have.length(1); - result.sort((a, b) => { - if (a.body < b.body) {return -1;} - else {return 1;} - }); - expect(result[0]).to.have.property('body', 'comment 20'); - }); - }); - - it('should find an array of new and accepted comments by asset id', () => { - return Comment.findAcceptedAndNewByAssetId('123').then((result) => { - expect(result).to.have.length(2); - result.sort((a, b) => { - if (a.body < b.body) {return -1;} - else {return 1;} - }); - expect(result[0]).to.have.property('body', 'comment 10'); - }); - }); }); describe('#moderationQueue()', () => { diff --git a/tests/routes/api/comments/index.js b/tests/routes/api/comments/index.js index 5edb52099..7a3a2c592 100644 --- a/tests/routes/api/comments/index.js +++ b/tests/routes/api/comments/index.js @@ -192,6 +192,7 @@ describe('/api/v1/comments', () => { .then((res) => { expect(res).to.have.status(201); expect(res.body).to.have.property('id'); + expect(res.body).to.have.property('status', 'premod'); }); }); @@ -214,6 +215,7 @@ describe('/api/v1/comments', () => { expect(res).to.have.status(201); expect(res.body).to.have.property('id'); expect(res.body).to.have.property('status', null); + return Promise.all([ res.body, Action.findByType('flag', 'comments') @@ -252,6 +254,27 @@ describe('/api/v1/comments', () => { }); }); + it('should create a comment with null status if it\'s asset is has post-moderation enabled', () => { + return Asset + .findOrCreateByUrl('https://coralproject.net/article1') + .then((asset) => { + return Asset + .overrideSettings(asset.id, {moderation: 'post'}) + .then(() => asset); + }) + .then((asset) => { + return chai.request(app).post('/api/v1/comments') + .set(passport.inject({roles: []})) + .send({'body': 'Something body.', 'author_id': '123', 'asset_id': asset.id, 'parent_id': ''}); + }) + .then((res) => { + expect(res).to.have.status(201); + expect(res.body).to.have.property('id'); + expect(res.body).to.have.property('asset_id'); + expect(res.body).to.have.property('status', null); + }); + }); + it('should create a rejected comment if the body is above the character count', () => { return Asset .findOrCreateByUrl('https://coralproject.net/article1') diff --git a/tests/routes/api/stream/index.js b/tests/routes/api/stream/index.js index d9b59d6fe..104801996 100644 --- a/tests/routes/api/stream/index.js +++ b/tests/routes/api/stream/index.js @@ -24,6 +24,21 @@ describe('/api/v1/stream', () => { } }; + const assets = [ + { + url: 'https://example.com/article/1' + }, + { + url: 'https://example.com/article/2', + settings: { + moderation: 'pre' + } + }, + { + url: 'https://example.com/article/3' + } + ]; + const comments = [{ id: 'abc', body: 'comment 10', @@ -58,6 +73,28 @@ describe('/api/v1/stream', () => { status_history: [{ type: 'rejected' }] + }, { + body: 'comment 50', + status: 'premod', + status_history: [{ + type: 'premod' + }] + }, { + body: 'comment 60', + status: 'accepted', + status_history: [{ + type: 'accepted' + }] + }, { + body: 'comment 70', + status: 'rejected', + status_history: [{ + type: 'rejected' + }] + }, { + body: 'comment 70', + status: null, + status_history: [] }]; const users = [{ @@ -79,42 +116,46 @@ describe('/api/v1/stream', () => { }]; beforeEach(() => { - return Setting.init(settings).then(() => { - return Promise.all([ - User.createLocalUsers(users), - Asset.findOrCreateByUrl('http://test.com'), - Asset - .findOrCreateByUrl('http://coralproject.net/asset2') - .then((asset) => { - return Asset - .overrideSettings(asset.id, {moderation: 'pre'}) - .then(() => asset); - }) - ]) - .then(([users, asset1, asset2]) => { + return Setting.init(settings) + .then(() => Promise.all([ + User.createLocalUsers(users), + Promise.all(assets.map((asset) => Asset.create(asset))) + ])) + .then(([mockUsers, mockAssets]) => { - comments[0].author_id = users[0].id; - comments[1].author_id = users[1].id; - comments[2].author_id = users[0].id; - comments[3].author_id = users[1].id; - - comments[0].asset_id = asset1.id; - comments[1].asset_id = asset1.id; - comments[2].asset_id = asset2.id; - comments[3].asset_id = asset2.id; - - return Promise.all([ - Comment.create(comments), - Action.create(actions) - ]); + // Map the id's over. + mockAssets.forEach((asset, i) => { + assets[i].id = asset.id; }); + + mockUsers.forEach((user, i) => { + users[i].id = user.id; + }); + + comments.forEach((comment, i) => { + comments[i].author_id = users[(i % 2) === 0 ? 0 : 1].id; + }); + + comments[0].asset_id = assets[0].id; + comments[1].asset_id = assets[0].id; + comments[2].asset_id = assets[1].id; + comments[3].asset_id = assets[1].id; + comments[4].asset_id = assets[2].id; + comments[5].asset_id = assets[2].id; + comments[6].asset_id = assets[2].id; + comments[7].asset_id = assets[2].id; + + return Promise.all([ + Comment.create(comments), + Action.create(actions) + ]); }); }); it('should return a stream with comments, users and actions for an existing asset', () => { return chai.request(app) .get('/api/v1/stream') - .query({'asset_url': 'http://test.com'}) + .query({asset_url: assets[0].url}) .then(res => { expect(res).to.have.status(200); expect(res.body.assets.length).to.equal(1); @@ -138,15 +179,47 @@ describe('/api/v1/stream', () => { it('should merge the settings when the asset contains settings to override it with', () => { return chai.request(app) .get('/api/v1/stream') - .query({'asset_url': 'http://coralproject.net/asset2'}) + .query({asset_url: assets[1].url}) .then((res) => { expect(res).to.have.status(200); - expect(res.body.assets.length).to.equal(1); - expect(res.body.comments.length).to.equal(1); - expect(res.body.users.length).to.equal(1); + expect(res.body.assets).to.have.length(1); + expect(res.body.comments).to.have.length(1); + expect(res.body.users).to.have.length(1); expect(res.body.settings).to.have.property('moderation', 'pre'); expect(res.body.settings).to.not.have.property('wordlist'); }); }); + + it('should not change the previously displayed comments based on moderation state changes', () => { + + let preComments, postComments; + + return chai.request(app) + .get('/api/v1/stream') + .query({asset_url: assets[2].url}) + .then((res) => { + expect(res).to.have.status(200); + expect(res.body.comments.length).to.equal(2); + expect(res.body.settings).to.have.property('moderation', 'post'); + + preComments = res.body.comments; + + return Asset.overrideSettings(assets[2].id, {moderation: 'pre'}); + }) + .then(() => { + return chai.request(app) + .get('/api/v1/stream') + .query({asset_url: assets[2].url}); + }) + .then((res) => { + expect(res).to.have.status(200); + expect(res.body.comments.length).to.equal(2); + expect(res.body.settings).to.have.property('moderation', 'pre'); + + postComments = res.body.comments; + + expect(preComments).to.deep.equal(postComments); + }); + }); }); }); diff --git a/util.js b/util.js index 6907d90cd..04c822867 100644 --- a/util.js +++ b/util.js @@ -38,5 +38,6 @@ util.onshutdown = (jobs) => { // Attach to the SIGTERM + SIGINT handles to ensure a clean shutdown in the // event that we have an external event. -process.on('SIGTERM', () => util.shutdown()); -process.on('SIGINT', () => util.shutdown()); +process.on('SIGTERM', () => util.shutdown()); +process.on('SIGINT', () => util.shutdown()); +process.once('SIGUSR2', () => util.shutdown()); From 44a06aef46c41f11ccb4251adb426d31c23a5241 Mon Sep 17 00:00:00 2001 From: Riley Davis Date: Fri, 13 Jan 2017 14:33:01 -0700 Subject: [PATCH 09/11] do not render tab content unless they're active --- client/coral-admin/src/actions/comments.js | 6 +- .../ModerationQueue/ModerationQueue.js | 90 +++++++++++-------- 2 files changed, 54 insertions(+), 42 deletions(-) diff --git a/client/coral-admin/src/actions/comments.js b/client/coral-admin/src/actions/comments.js index f37c79b4f..fe4895ed3 100644 --- a/client/coral-admin/src/actions/comments.js +++ b/client/coral-admin/src/actions/comments.js @@ -55,9 +55,9 @@ export const fetchFlaggedQueue = () => { dispatch({type: commentTypes.COMMENTS_MODERATION_QUEUE_FETCH_REQUEST}); return coralApi('/queue/comments/flagged') - .then(comments => { - comments.forEach(comment => comment.flagged = true); - return comments; + .then(results => { + results.comments.forEach(comment => comment.flagged = true); + return results; }) .then(addUsersCommentsActions.bind(this, dispatch)); }; diff --git a/client/coral-admin/src/containers/ModerationQueue/ModerationQueue.js b/client/coral-admin/src/containers/ModerationQueue/ModerationQueue.js index d46a9b35d..6121ca307 100644 --- a/client/coral-admin/src/containers/ModerationQueue/ModerationQueue.js +++ b/client/coral-admin/src/containers/ModerationQueue/ModerationQueue.js @@ -22,49 +22,61 @@ export default ({onTabClick, ...props}) => ( className={`mdl-tabs__tab ${styles.tab}`}>{lang.t('modqueue.flagged')}
- - + { + props.activeTab === 'pending' + ?
+ + +
+ : null + }
- + { + props.activeTab === 'rejected' + ? + : null + }
- -
+ { + props.activeTab === 'flagged' + ? + : null + } + From 06ab33343f3b29799c7051ca0043cb3d97083d05 Mon Sep 17 00:00:00 2001 From: gaba Date: Fri, 13 Jan 2017 14:15:31 -0800 Subject: [PATCH 10/11] Errors needs to use APIError. --- errors.js | 2 +- models/user.js | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/errors.js b/errors.js index 6cdc7425a..1b26e7170 100644 --- a/errors.js +++ b/errors.js @@ -54,7 +54,7 @@ const ErrEmailTaken = new APIError('Email address already in use', { status: 400 }); -const ErrDisplayTaken = new Error('Display name already in use', { +const ErrDisplayTaken = new APIError('Display name already in use', { translation_key: 'DISPLAYNAME_IN_USE', status: 400 }); diff --git a/models/user.js b/models/user.js index b0ead1c5a..a0ca34d7a 100644 --- a/models/user.js +++ b/models/user.js @@ -383,8 +383,7 @@ UserService.createLocalUser = (email, password, displayName) => { user.save((err) => { if (err) { if (err.code === 11000) { - const regex = new RegExp('displayName', 'g'); - if (regex.exec(err.message)[0] === 'displayName') { + if (err.message.match('displayName')) { return reject(errors.ErrDisplayTaken); } return reject(errors.ErrEmailTaken); From da0ca97483515c1d1290d2ebce63decc4fa29fc2 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 13 Jan 2017 16:02:49 -0700 Subject: [PATCH 11/11] Added comment for SIGUSR2 --- util.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/util.js b/util.js index 04c822867..b081078e7 100644 --- a/util.js +++ b/util.js @@ -37,7 +37,8 @@ util.onshutdown = (jobs) => { }; // Attach to the SIGTERM + SIGINT handles to ensure a clean shutdown in the -// event that we have an external event. +// event that we have an external event. SIGUSR2 is called when the app is asked +// to be 'killed', same procedure here. process.on('SIGTERM', () => util.shutdown()); process.on('SIGINT', () => util.shutdown()); process.once('SIGUSR2', () => util.shutdown());