From 0c7b35ffde9646e046cdba7f39ff7f2a2681cc97 Mon Sep 17 00:00:00 2001 From: Daniel Janeiro Date: Sat, 7 Oct 2017 21:51:58 +0100 Subject: [PATCH 1/5] Add cannot ignore staff error --- errors.js | 9 ++++++++- locales/en.yml | 1 + 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/errors.js b/errors.js index ab1667e07..dca3f0511 100644 --- a/errors.js +++ b/errors.js @@ -195,6 +195,12 @@ const ErrAssetURLAlreadyExists = new APIError('Asset URL already exists, cannot status: 409 }); +// ErrCannotIgnoreStaff is returned when a user tries to ignore a staff member. +const ErrCannotIgnoreStaff = new APIError('Cannot ignore staff members.', { + translation_key: 'CANNOT_IGNORE_STAFF', + status: 400 +}); + module.exports = { ExtendableError, APIError, @@ -221,5 +227,6 @@ module.exports = { ErrLoginAttemptMaximumExceeded, ErrEditWindowHasEnded, ErrCommentTooShort, - ErrAssetURLAlreadyExists + ErrAssetURLAlreadyExists, + ErrCannotIgnoreStaff }; diff --git a/locales/en.yml b/locales/en.yml index 03a445940..1687ab1df 100644 --- a/locales/en.yml +++ b/locales/en.yml @@ -206,6 +206,7 @@ en: NOT_FOUND: "Resource not found" ALREADY_EXISTS: "Resource already exists" INVALID_ASSET_URL: "Assert URL is invalid" + CANNOT_IGNORE_STAFF: "Cannot ignore Staff members." email: "Not a valid E-Mail" confirm_password: "Passwords don't match. Please check again" network_error: "Failed to connect to server. Check your internet connection and try again." From 7eaad1541b8bb24b942b91dfbc9b9978f7e464c8 Mon Sep 17 00:00:00 2001 From: Daniel Janeiro Date: Sat, 7 Oct 2017 22:35:54 +0100 Subject: [PATCH 2/5] Implement cannot ignore staff in users service `UsersService.ignoreUsers` now checks if any user provided is a staff member. In case it is it throws a ErrCannotIgnoreStaff error. --- services/users.js | 8 +++++++- test/server/services/users.js | 13 +++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/services/users.js b/services/users.js index 1dc9bceb6..4a2b6a0ee 100644 --- a/services/users.js +++ b/services/users.js @@ -2,6 +2,7 @@ const assert = require('assert'); const uuid = require('uuid'); const bcrypt = require('bcryptjs'); const errors = require('../errors'); +const some = require('lodash/some'); const { ROOT_URL @@ -879,13 +880,18 @@ module.exports = class UsersService { * @param {String} id the id of the user that is ignoring another users * @param {Array} usersToIgnore Array of user IDs to ignore */ - static ignoreUsers(id, usersToIgnore) { + static async ignoreUsers(id, usersToIgnore) { assert(Array.isArray(usersToIgnore), 'usersToIgnore is an array'); assert(usersToIgnore.every((u) => typeof u === 'string'), 'usersToIgnore is an array of string user IDs'); if (usersToIgnore.includes(id)) { throw new Error('Users cannot ignore themselves'); } + const users = await UsersService.findByIdArray(usersToIgnore); + if (some(users, (user) => user.isStaff())) { + throw errors.ErrCannotIgnoreStaff; + } + // TODO: For each usersToIgnore, make sure they exist? return UserModel.update({id}, { $addToSet: { diff --git a/test/server/services/users.js b/test/server/services/users.js index 57d0f3815..3efaf8f20 100644 --- a/test/server/services/users.js +++ b/test/server/services/users.js @@ -164,6 +164,19 @@ describe('services.UsersService', () => { const userAfterIgnoring2 = await UsersService.findById(user.id); expect(userAfterIgnoring2.ignoresUsers.length).to.equal(2); }); + + it('should not ignore a staff member', async () => { + const user = mockUsers[0]; + const usersToIgnore = [mockUsers[1]]; + await UsersService.addRoleToUser(usersToIgnore[0].id, 'STAFF'); + + try { + await UsersService.ignoreUsers(user.id, usersToIgnore.map((u) => u.id)); + } catch (err) { + expect(err.status).to.equal(400); + expect(err.translation_key).to.equal('CANNOT_IGNORE_STAFF'); + } + }); }); describe('#ban', () => { From efdf7a6b25a11704b93052e4d324b4d82b943fae Mon Sep 17 00:00:00 2001 From: Daniel Janeiro Date: Sat, 7 Oct 2017 22:44:23 +0100 Subject: [PATCH 3/5] Handle IgnoreUser mutation errors We now check if the IgnoreUser mutation returned an error before updating the apollo store. If the mutation returns an error we keep the store as it is, otherwise the user is added to the ignore list. --- plugins/talk-plugin-ignore-user/client/index.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/plugins/talk-plugin-ignore-user/client/index.js b/plugins/talk-plugin-ignore-user/client/index.js index 6583f68e6..efec73572 100644 --- a/plugins/talk-plugin-ignore-user/client/index.js +++ b/plugins/talk-plugin-ignore-user/client/index.js @@ -3,6 +3,7 @@ import IgnoreUserConfirmation from './containers/IgnoreUserConfirmation'; import IgnoredUserSection from './containers/IgnoredUserSection'; import translations from './translations.yml'; import update from 'immutability-helper'; +import get from 'lodash/get'; export default { slots: { @@ -14,7 +15,11 @@ export default { mutations: { IgnoreUser: ({variables}) => ({ updateQueries: { - CoralEmbedStream_Embed: (previousData) => { + CoralEmbedStream_Embed: (previousData, {mutationResult: {data: {ignoreUser}}}) => { + if (get(ignoreUser, 'errors.length')) { + return previousData; + } + const ignoredUserId = variables.id; const updated = update(previousData, {me: {ignoredUsers: {$push: [{ id: ignoredUserId, From 27c6d830d948d066290547c309948739c8ecac20 Mon Sep 17 00:00:00 2001 From: Daniel Janeiro Date: Fri, 13 Oct 2017 01:13:53 +0100 Subject: [PATCH 4/5] Remove error handling code Remove error handling code since the withMutation HoC already handles errors for us. --- plugins/talk-plugin-ignore-user/client/index.js | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/plugins/talk-plugin-ignore-user/client/index.js b/plugins/talk-plugin-ignore-user/client/index.js index efec73572..6583f68e6 100644 --- a/plugins/talk-plugin-ignore-user/client/index.js +++ b/plugins/talk-plugin-ignore-user/client/index.js @@ -3,7 +3,6 @@ import IgnoreUserConfirmation from './containers/IgnoreUserConfirmation'; import IgnoredUserSection from './containers/IgnoredUserSection'; import translations from './translations.yml'; import update from 'immutability-helper'; -import get from 'lodash/get'; export default { slots: { @@ -15,11 +14,7 @@ export default { mutations: { IgnoreUser: ({variables}) => ({ updateQueries: { - CoralEmbedStream_Embed: (previousData, {mutationResult: {data: {ignoreUser}}}) => { - if (get(ignoreUser, 'errors.length')) { - return previousData; - } - + CoralEmbedStream_Embed: (previousData) => { const ignoredUserId = variables.id; const updated = update(previousData, {me: {ignoredUsers: {$push: [{ id: ignoredUserId, From 2fd7829b882442458df84701008661b5b061c6bb Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 13 Oct 2017 17:33:59 +0700 Subject: [PATCH 5/5] Prevent calling updators when there is an error --- client/coral-framework/hocs/withMutation.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/client/coral-framework/hocs/withMutation.js b/client/coral-framework/hocs/withMutation.js index f16809322..cb6bc3789 100644 --- a/client/coral-framework/hocs/withMutation.js +++ b/client/coral-framework/hocs/withMutation.js @@ -108,7 +108,7 @@ export default (document, config = {}) => hoistStatics((WrappedComponent) => { res[key] = (prev, result) => { if (getResponseErrors(result.mutationResult)) { - // Do not run updates when we have mutation errors. + // Do not run updates when we have mutation errors. return prev; } return map[key](prev, result) || prev; @@ -116,6 +116,11 @@ export default (document, config = {}) => hoistStatics((WrappedComponent) => { } else { const existing = res[key]; res[key] = (prev, result) => { + if (getResponseErrors(result.mutationResult)) { + + // Do not run updates when we have mutation errors. + return prev; + } const next = existing(prev, result); return map[key](next, result) || next; };