From 9dc57c82d1200266e9629f9bdc0baf298879a701 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 24 Jan 2017 13:27:22 -0700 Subject: [PATCH 1/6] Fixes to consts + services --- .travis-yml | 3 --- circle.yml | 1 + graph/loaders/comments.js | 8 -------- graph/loaders/users.js | 5 +---- graph/resolvers/action.js | 12 ------------ graph/resolvers/action_summary.js | 15 +-------------- 6 files changed, 3 insertions(+), 41 deletions(-) delete mode 100644 .travis-yml diff --git a/.travis-yml b/.travis-yml deleted file mode 100644 index 85242354a..000000000 --- a/.travis-yml +++ /dev/null @@ -1,3 +0,0 @@ -language: node_js -node_js: - - 4 diff --git a/circle.yml b/circle.yml index b9cea6fed..20ab7b659 100644 --- a/circle.yml +++ b/circle.yml @@ -3,6 +3,7 @@ machine: version: 7 services: - docker + - redis test: override: diff --git a/graph/loaders/comments.js b/graph/loaders/comments.js index 84c6f7ed0..35463dfd0 100644 --- a/graph/loaders/comments.js +++ b/graph/loaders/comments.js @@ -44,16 +44,8 @@ const getCommentsByStatusAndAssetID = (context, {status = null, asset_id = null} }; const getCommentsByActionTypeAndAssetID = (context, {action_type, asset_id = null}) => { - - // TODO: remove when we move the enum over to the uppercase. - if (action_type) { - action_type = action_type.toLowerCase(); - } - return ActionModel.find({ action_type, - - // TODO: remove when we move the enum over to the uppercase. item_type: 'COMMENTS' }).then((actions) => { let comments = CommentModel.find({ diff --git a/graph/loaders/users.js b/graph/loaders/users.js index 793ecfbf9..90c661f71 100644 --- a/graph/loaders/users.js +++ b/graph/loaders/users.js @@ -4,12 +4,9 @@ const util = require('./util'); const UsersService = require('../../services/users'); -const genUserByIDs = (context, ids) => { - console.log('genUserIds', ids); - return UsersService +const genUserByIDs = (context, ids) => UsersService .findByIdArray(ids) .then(util.singleJoinBy(ids, 'id')); -}; /** * Creates a set of loaders based on a GraphQL context. diff --git a/graph/resolvers/action.js b/graph/resolvers/action.js index f82cefdb1..7a93556e0 100644 --- a/graph/resolvers/action.js +++ b/graph/resolvers/action.js @@ -1,16 +1,4 @@ const Action = { - action_type({action_type}) { - - // FIXME: remove once we cast the data model to have uppercase action - // types. - return action_type.toUpperCase(); - }, - item_type({item_type}) { - - // FIXME: remove once we cast the data model to have uppercase item - // types. - return item_type.toUpperCase(); - }, // This will load the user for the specific action. We'll limit this to the // admin users only. diff --git a/graph/resolvers/action_summary.js b/graph/resolvers/action_summary.js index 5a2ef0994..48eff50c7 100644 --- a/graph/resolvers/action_summary.js +++ b/graph/resolvers/action_summary.js @@ -1,16 +1,3 @@ -const ActionSummary = { - action_type({action_type}) { - - // FIXME: remove once we cast the data model to have uppercase action - // types. - return action_type.toUpperCase(); - }, - item_type({item_type}) { - - // FIXME: remove once we cast the data model to have uppercase item - // types. - return item_type.toUpperCase(); - } -}; +const ActionSummary = {}; module.exports = ActionSummary; From 0a5a23c1f3a9546f6f0405c1e5811c4d09217bfd Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 24 Jan 2017 13:45:06 -0700 Subject: [PATCH 2/6] Added fixes for tests on CI --- bin/cli-settings | 8 +++++++- circle.yml | 3 ++- client/coral-admin/src/containers/Community/Table.js | 4 ++-- client/coral-settings/components/SettingsHeader.js | 3 ++- docs/swagger.yaml | 2 +- graph/resolvers/action.js | 2 +- graph/resolvers/comment.js | 7 ------- graph/resolvers/root_query.js | 4 ++-- graph/resolvers/user.js | 2 +- init.js | 1 - routes/admin/index.js | 2 +- routes/api/comments/index.js | 12 ++++++------ routes/api/index.js | 8 ++++---- routes/api/queue/index.js | 6 +++--- routes/api/users/index.js | 8 ++++---- services/mongoose.js | 9 +++++++++ test/e2e/tests/Admin/LoginTest.js | 2 +- test/e2e/tests/Moderator/LoginTest.js | 2 +- 18 files changed, 47 insertions(+), 38 deletions(-) diff --git a/bin/cli-settings b/bin/cli-settings index b4acfae61..7854942c0 100755 --- a/bin/cli-settings +++ b/bin/cli-settings @@ -22,7 +22,13 @@ program .command('init') .description('initilizes the talk settings') .action(() => { - const defaults = {id: '1', moderation: 'PRE'}; + const defaults = { + moderation: 'PRE', + wordlist: { + banned: [], + suspect: [] + } + }; SettingsService .init(defaults) diff --git a/circle.yml b/circle.yml index 20ab7b659..f7f6aa098 100644 --- a/circle.yml +++ b/circle.yml @@ -7,8 +7,9 @@ machine: test: override: - - MOCHA_FILE=$CIRCLE_TEST_REPORTS/junit/test-results.xml ./node_modules/.bin/mocha tests --reporter mocha-junit-reporter - npm run lint + - ./bin/cli settings init + - MOCHA_FILE=$CIRCLE_TEST_REPORTS/junit/test-results.xml ./node_modules/.bin/mocha tests --reporter mocha-junit-reporter - npm run build deployment: diff --git a/client/coral-admin/src/containers/Community/Table.js b/client/coral-admin/src/containers/Community/Table.js index c146e564a..54aab9879 100644 --- a/client/coral-admin/src/containers/Community/Table.js +++ b/client/coral-admin/src/containers/Community/Table.js @@ -65,8 +65,8 @@ class Table extends Component { label={lang.t('community.role')} onChange={role => this.onRoleChange(row.id, role)}> - - + + diff --git a/client/coral-settings/components/SettingsHeader.js b/client/coral-settings/components/SettingsHeader.js index b169411db..8ed13713f 100644 --- a/client/coral-settings/components/SettingsHeader.js +++ b/client/coral-settings/components/SettingsHeader.js @@ -6,8 +6,9 @@ export default ({userData}) => (

{userData.displayName}

{ + // Hiding display of users ID unless there's a use case for it. - //

{userData.profiles.map(profile => profile.id)}

+ //

{userData.profiles.map(profile => profile.id)}

} ); diff --git a/docs/swagger.yaml b/docs/swagger.yaml index 554afb29e..6f57f3918 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -916,7 +916,7 @@ definitions: type: array items: type: string - description: Roles occupied by the user (e.g. 'admin', 'moderator', etc.) + description: Roles occupied by the user (e.g. 'ADMIN', 'MODERATOR', etc.) status: type: string description: The current status of the user in the system. diff --git a/graph/resolvers/action.js b/graph/resolvers/action.js index 7a93556e0..e6886653e 100644 --- a/graph/resolvers/action.js +++ b/graph/resolvers/action.js @@ -3,7 +3,7 @@ const Action = { // This will load the user for the specific action. We'll limit this to the // admin users only. user({user_id}, _, {loaders, user}) { - if (user.hasRole('admin')) { + if (user.hasRole('ADMIN')) { return loaders.Users.getByID.load(user_id); } } diff --git a/graph/resolvers/comment.js b/graph/resolvers/comment.js index d7d90918b..ce2283cda 100644 --- a/graph/resolvers/comment.js +++ b/graph/resolvers/comment.js @@ -8,13 +8,6 @@ const Comment = { actions({id}, _, {loaders}) { return loaders.Actions.getByItemID.load(id); }, - status({status}) { - - // Because the status can be `null`, we do this check. - if (status) { - return status.toUpperCase(); - } - }, asset({asset_id}, _, {loaders}) { return loaders.Assets.getByID.load(asset_id); } diff --git a/graph/resolvers/root_query.js b/graph/resolvers/root_query.js index 3a2499fd8..5676b4f48 100644 --- a/graph/resolvers/root_query.js +++ b/graph/resolvers/root_query.js @@ -1,6 +1,6 @@ const RootQuery = { assets(_, args, {loaders, user}) { - if (user == null || !user.hasRoles('admin')) { + if (user == null || !user.hasRoles('ADMIN')) { return null; } @@ -25,7 +25,7 @@ const RootQuery = { // This endpoint is used for loading moderation queues, so hide it in the // event that we aren't an admin. comments(_, {query}, {loaders, user}) { - if (user == null || !user.hasRoles('admin')) { + if (user == null || !user.hasRoles('ADMIN')) { return null; } diff --git a/graph/resolvers/user.js b/graph/resolvers/user.js index 3d65416fa..65882feee 100644 --- a/graph/resolvers/user.js +++ b/graph/resolvers/user.js @@ -6,7 +6,7 @@ const User = { // If the user is not an admin, only return comment list for the owner of // the comments. - if (!user.hasRoles('admin') || user.id !== id) { + if (!user.hasRoles('ADMIN') || user.id !== id) { return null; } diff --git a/init.js b/init.js index 7c56d71cc..eb17963a6 100644 --- a/init.js +++ b/init.js @@ -4,7 +4,6 @@ module.exports = () => Promise.all([ // Upsert the settings object. SettingsService.init({ - id: '1', moderation: 'PRE', wordlist: { banned: [], diff --git a/routes/admin/index.js b/routes/admin/index.js index 2a37e8f0f..d9731a1af 100644 --- a/routes/admin/index.js +++ b/routes/admin/index.js @@ -15,7 +15,7 @@ router.get('/login', (req, res, next) => { }); router.get('*', (req, res) => { - res.render('admin', {basePath: '/client/coral-admin'}); + res.render('ADMIN', {basePath: '/client/coral-admin'}); }); module.exports = router; diff --git a/routes/api/comments/index.js b/routes/api/comments/index.js index c73a9f536..19346274d 100644 --- a/routes/api/comments/index.js +++ b/routes/api/comments/index.js @@ -22,13 +22,13 @@ router.get('/', (req, res, next) => { } = req.query; // everything on this route requires admin privileges besides listing comments for owner of said comments - if (!authorization.has(req.user, 'admin') && !user_id) { + if (!authorization.has(req.user, 'ADMIN') && !user_id) { next(errors.ErrNotAuthorized); return; } // if the user is not an admin, only return comment list for the owner of the comments - if (req.user.id !== user_id && !authorization.has(req.user, 'admin')) { + if (req.user.id !== user_id && !authorization.has(req.user, 'ADMIN')) { next(errors.ErrNotAuthorized); return; } @@ -50,7 +50,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 = CommentsService.findByUserId(user_id, authorization.has(req.user, 'admin')); + query = CommentsService.findByUserId(user_id, authorization.has(req.user, 'ADMIN')); } else if (status) { query = assetIDWrap(CommentsService.findByStatus(status === 'NEW' ? null : status)); } else if (action_type) { @@ -85,7 +85,7 @@ router.get('/', (req, res, next) => { }); }); -router.get('/:comment_id', authorization.needed('admin'), (req, res, next) => { +router.get('/:comment_id', authorization.needed('ADMIN'), (req, res, next) => { CommentsService .findById(req.params.comment_id) .then(comment => { @@ -101,7 +101,7 @@ router.get('/:comment_id', authorization.needed('admin'), (req, res, next) => { }); }); -router.delete('/:comment_id', authorization.needed('admin'), (req, res, next) => { +router.delete('/:comment_id', authorization.needed('ADMIN'), (req, res, next) => { CommentsService .removeById(req.params.comment_id) .then(() => { @@ -112,7 +112,7 @@ router.delete('/:comment_id', authorization.needed('admin'), (req, res, next) => }); }); -router.put('/:comment_id/status', authorization.needed('admin'), (req, res, next) => { +router.put('/:comment_id/status', authorization.needed('ADMIN'), (req, res, next) => { const { status } = req.body; diff --git a/routes/api/index.js b/routes/api/index.js index b905e3b27..c40c428b4 100644 --- a/routes/api/index.js +++ b/routes/api/index.js @@ -3,9 +3,9 @@ const authorization = require('../../middleware/authorization'); const router = express.Router(); -router.use('/assets', authorization.needed('admin'), require('./assets')); -router.use('/settings', authorization.needed('admin'), require('./settings')); -router.use('/queue', authorization.needed('admin'), require('./queue')); +router.use('/assets', authorization.needed('ADMIN'), require('./assets')); +router.use('/settings', authorization.needed('ADMIN'), require('./settings')); +router.use('/queue', authorization.needed('ADMIN'), require('./queue')); router.use('/comments', authorization.needed(), require('./comments')); router.use('/actions', authorization.needed(), require('./actions')); @@ -15,6 +15,6 @@ router.use('/users', require('./users')); router.use('/account', require('./account')); // Bind the kue handler to the /kue path. -router.use('/kue', authorization.needed('admin'), require('../../services/kue').kue.app); +router.use('/kue', authorization.needed('ADMIN'), require('../../services/kue').kue.app); module.exports = router; diff --git a/routes/api/queue/index.js b/routes/api/queue/index.js index 6ec5a5b29..c106f17f7 100644 --- a/routes/api/queue/index.js +++ b/routes/api/queue/index.js @@ -27,7 +27,7 @@ function gatherActionsAndUsers (comments) { // depending on the settings. The :moderation overwrites this settings. // Pre-moderation: New comments are shown in the moderator queues immediately. // Post-moderation: New comments do not appear in moderation queues unless they are flagged by other users. -router.get('/comments/pending', authorization.needed('admin'), (req, res, next) => { +router.get('/comments/pending', authorization.needed('ADMIN'), (req, res, next) => { const {asset_id} = req.query; @@ -41,7 +41,7 @@ router.get('/comments/pending', authorization.needed('admin'), (req, res, next) }); }); -router.get('/comments/rejected', authorization.needed('admin'), (req, res, next) => { +router.get('/comments/rejected', authorization.needed('ADMIN'), (req, res, next) => { const {asset_id} = req.query; CommentsService.moderationQueue('REJECTED', asset_id) @@ -54,7 +54,7 @@ router.get('/comments/rejected', authorization.needed('admin'), (req, res, next) }); }); -router.get('/comments/flagged', authorization.needed('admin'), (req, res, next) => { +router.get('/comments/flagged', authorization.needed('ADMIN'), (req, res, next) => { const {asset_id} = req.query; const assetIDWrap = (query) => { diff --git a/routes/api/users/index.js b/routes/api/users/index.js index 130d5bc0b..3ea3f3213 100644 --- a/routes/api/users/index.js +++ b/routes/api/users/index.js @@ -6,7 +6,7 @@ const CommentsService = require('../../../services/comments'); const mailer = require('../../../services/mailer'); const authorization = require('../../../middleware/authorization'); -router.get('/', authorization.needed('admin'), (req, res, next) => { +router.get('/', authorization.needed('ADMIN'), (req, res, next) => { const { value = '', field = 'created_at', @@ -35,7 +35,7 @@ router.get('/', authorization.needed('admin'), (req, res, next) => { .catch(next); }); -router.post('/:user_id/role', authorization.needed('admin'), (req, res, next) => { +router.post('/:user_id/role', authorization.needed('ADMIN'), (req, res, next) => { UsersService .addRoleToUser(req.params.user_id, req.body.role) .then(() => { @@ -44,7 +44,7 @@ router.post('/:user_id/role', authorization.needed('admin'), (req, res, next) => .catch(next); }); -router.post('/:user_id/status', authorization.needed('admin'), (req, res, next) => { +router.post('/:user_id/status', authorization.needed('ADMIN'), (req, res, next) => { UsersService .setStatus(req.params.user_id, req.body.status) .then((status) => { @@ -138,7 +138,7 @@ router.post('/:user_id/actions', authorization.needed(), (req, res, next) => { }); }); -router.post('/:user_id/email/confirm', authorization.needed('admin'), (req, res, next) => { +router.post('/:user_id/email/confirm', authorization.needed('ADMIN'), (req, res, next) => { const { user_id } = req.params; diff --git a/services/mongoose.js b/services/mongoose.js index 1b9097760..c7092326f 100644 --- a/services/mongoose.js +++ b/services/mongoose.js @@ -57,3 +57,12 @@ mongoose.connect(url, (err) => { }); module.exports = mongoose; + +// Here we include all the models that mongoose is used for, this ensures that +// when we import mongoose that we also start up all the indexing opreations +// here. +require('../models/action'); +require('../models/asset'); +require('../models/comment'); +require('../models/setting'); +require('../models/user'); diff --git a/test/e2e/tests/Admin/LoginTest.js b/test/e2e/tests/Admin/LoginTest.js index e75d29903..9babdf29b 100644 --- a/test/e2e/tests/Admin/LoginTest.js +++ b/test/e2e/tests/Admin/LoginTest.js @@ -1,5 +1,5 @@ module.exports = { - '@tags': ['login', 'admin'], + '@tags': ['login', 'ADMIN'], before(client) { const embedStreamPage = client.page.embedStreamPage(); const {launchUrl} = client; diff --git a/test/e2e/tests/Moderator/LoginTest.js b/test/e2e/tests/Moderator/LoginTest.js index 3f1754a50..d04baabac 100644 --- a/test/e2e/tests/Moderator/LoginTest.js +++ b/test/e2e/tests/Moderator/LoginTest.js @@ -1,5 +1,5 @@ module.exports = { - '@tags': ['login', 'moderator'], + '@tags': ['login', 'MODERATOR'], before: client => { const embedStreamPage = client.page.embedStreamPage(); const {launchUrl} = client; From 2ffecbeeffc2bfc29d754de27321a5dfd929d36c Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 24 Jan 2017 13:46:53 -0700 Subject: [PATCH 3/6] Removed build step, is already done via postinstall hook --- circle.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/circle.yml b/circle.yml index f7f6aa098..330f3e4ef 100644 --- a/circle.yml +++ b/circle.yml @@ -10,7 +10,6 @@ test: - npm run lint - ./bin/cli settings init - MOCHA_FILE=$CIRCLE_TEST_REPORTS/junit/test-results.xml ./node_modules/.bin/mocha tests --reporter mocha-junit-reporter - - npm run build deployment: release: From 603854c0bef162a1a19df926fc182251dc26f964 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 24 Jan 2017 14:06:39 -0700 Subject: [PATCH 4/6] Adjusted tests for circle --- circle.yml | 3 ++- package.json | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/circle.yml b/circle.yml index 330f3e4ef..fb598ffe0 100644 --- a/circle.yml +++ b/circle.yml @@ -9,7 +9,8 @@ test: override: - npm run lint - ./bin/cli settings init - - MOCHA_FILE=$CIRCLE_TEST_REPORTS/junit/test-results.xml ./node_modules/.bin/mocha tests --reporter mocha-junit-reporter + - sleep 5 + - MOCHA_FILE=junit/test-results.xml NPM_PACKAGE_CONFIG_MOCHA_REPORTER=mocha-junit-reporter npm run test deployment: release: diff --git a/package.json b/package.json index 5c375a5b4..59b31eb69 100644 --- a/package.json +++ b/package.json @@ -10,7 +10,7 @@ "build-watch": "NODE_ENV=development webpack --config webpack.config.dev.js --watch", "lint": "eslint bin/* .", "lint-fix": "eslint bin/* . --fix", - "test": "TEST_MODE=unit NODE_ENV=test mocha", + "test": "TEST_MODE=unit NODE_ENV=test mocha -R ${NPM_PACKAGE_CONFIG_MOCHA_REPORTER:-spec}", "test-cover": "TEST_MODE=unit NODE_ENV=test istanbul cover _mocha --report text --check-coverage -- -R spec", "pree2e": "NODE_ENV=test scripts/pree2e.sh", "e2e": "NODE_ENV=test nightwatch", From 53dd34f406151693fec9e152e83e10c9b4709476 Mon Sep 17 00:00:00 2001 From: David Erwin Date: Tue, 24 Jan 2017 16:11:17 -0500 Subject: [PATCH 5/6] Updating admin screen for all caps enums --- client/coral-admin/src/containers/Community/Table.js | 8 ++++---- .../containers/ModerationQueue/ModerationContainer.js | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/client/coral-admin/src/containers/Community/Table.js b/client/coral-admin/src/containers/Community/Table.js index c146e564a..254550e74 100644 --- a/client/coral-admin/src/containers/Community/Table.js +++ b/client/coral-admin/src/containers/Community/Table.js @@ -55,8 +55,8 @@ class Table extends Component { className={styles.selectField} label={lang.t('community.status')} onChange={status => this.onCommenterStatusChange(row.id, status)}> - - + + @@ -65,8 +65,8 @@ class Table extends Component { label={lang.t('community.role')} onChange={role => this.onRoleChange(row.id, role)}> - - + + diff --git a/client/coral-admin/src/containers/ModerationQueue/ModerationContainer.js b/client/coral-admin/src/containers/ModerationQueue/ModerationContainer.js index 41988e216..1b23de89a 100644 --- a/client/coral-admin/src/containers/ModerationQueue/ModerationContainer.js +++ b/client/coral-admin/src/containers/ModerationQueue/ModerationContainer.js @@ -75,12 +75,12 @@ class ModerationContainer extends React.Component { render () { const {comments} = this.props; - const premodIds = comments.ids.filter(id => comments.byId[id].status === 'premod'); - const rejectedIds = comments.ids.filter(id => comments.byId[id].status === 'rejected'); + const premodIds = comments.ids.filter(id => comments.byId[id].status === 'PREMOD'); + const rejectedIds = comments.ids.filter(id => comments.byId[id].status === 'REJECTED'); const flaggedIds = comments.ids.filter(id => comments.byId[id].flagged === true && - comments.byId[id].status !== 'rejected' && - comments.byId[id].status !== 'accepted' + comments.byId[id].status !== 'REJECTED' && + comments.byId[id].status !== 'ACCEPTED' ); return ( @@ -112,7 +112,7 @@ const mapDispatchToProps = dispatch => { fetchFlaggedQueue: () => dispatch(fetchFlaggedQueue()), showBanUserDialog: (userId, userName, commentId) => dispatch(showBanUserDialog(userId, userName, commentId)), hideBanUserDialog: () => dispatch(hideBanUserDialog(false)), - banUser: (userId, commentId) => dispatch(userStatusUpdate('banned', userId, commentId)).then(() => { + banUser: (userId, commentId) => dispatch(userStatusUpdate('BANNED', userId, commentId)).then(() => { dispatch(fetchModerationQueueComments()); }), updateStatus: (action, comment) => dispatch(updateStatus(action, comment)) From 050e98f1576b4eeb6b6141869fe5df4d6828de9d Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 24 Jan 2017 14:12:38 -0700 Subject: [PATCH 6/6] Fixed report location --- circle.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/circle.yml b/circle.yml index fb598ffe0..4d1f54286 100644 --- a/circle.yml +++ b/circle.yml @@ -10,7 +10,7 @@ test: - npm run lint - ./bin/cli settings init - sleep 5 - - MOCHA_FILE=junit/test-results.xml NPM_PACKAGE_CONFIG_MOCHA_REPORTER=mocha-junit-reporter npm run test + - MOCHA_FILE=$CIRCLE_TEST_REPORTS/junit/test-results.xml NPM_PACKAGE_CONFIG_MOCHA_REPORTER=mocha-junit-reporter npm run test deployment: release: