From 4e9b19c7f661192e7476b834d2b41ca735ba55f3 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Wed, 24 Jan 2018 15:00:30 -0700 Subject: [PATCH 01/19] performance related improvements to migraitons --- migrations/1510174676_user_status.js | 37 ++++++++++++++++------- migrations/1511801783_user_roles.js | 44 ++++++++++++++++++++-------- 2 files changed, 58 insertions(+), 23 deletions(-) diff --git a/migrations/1510174676_user_status.js b/migrations/1510174676_user_status.js index b3d7c0984..6ddd1e4b4 100644 --- a/migrations/1510174676_user_status.js +++ b/migrations/1510174676_user_status.js @@ -9,7 +9,19 @@ const getUserBatch = async () => { }; // Find all the users that need migrating. - return UserModel.collection.find(query); + return UserModel.collection.find(query).batchSize(100); +}; + +const processUpdates = async updates => { + // Create a new batch operation. + let bulk = UserModel.collection.initializeUnorderedBulkOp(); + + for (const { query, update } of updates) { + bulk.find(query).updateOne(update); + } + + // Execute the bulk update operation. + await bulk.execute(); }; module.exports = { @@ -19,7 +31,7 @@ module.exports = { // Get the first batch of users. let cursor = await getUserBatch(); - const updates = []; + let updates = []; while (await cursor.hasNext()) { const user = await cursor.next(); @@ -211,18 +223,23 @@ module.exports = { } updates.push({ query: { id }, update }); + + // Process every 1000 users. + if (updates.length > 1000) { + // Process the updates. + await processUpdates(updates); + + // Clear the updates array. + updates = []; + } } if (updates.length > 0) { - // Create a new batch operation. - let bulk = UserModel.collection.initializeUnorderedBulkOp(); + // Process the updates. + await processUpdates(updates); - for (const { query, update } of updates) { - bulk.find(query).updateOne(update); - } - - // Execute the bulk update operation. - await bulk.execute(); + // Clear the updates array. + updates = []; } }, }; diff --git a/migrations/1511801783_user_roles.js b/migrations/1511801783_user_roles.js index 08a0a1abc..aaca89b71 100644 --- a/migrations/1511801783_user_roles.js +++ b/migrations/1511801783_user_roles.js @@ -12,13 +12,27 @@ const findNewRole = roles => { return 'COMMENTER'; }; +const processUpdates = async updates => { + // Create a new batch operation. + const bulk = UserModel.collection.initializeUnorderedBulkOp(); + + for (const { query, update } of updates) { + bulk.find(query).updateOne(update); + } + + // Execute the bulk update operation. + await bulk.execute(); +}; + module.exports = { async up() { - const cursor = await UserModel.collection.find({ - roles: { - $exists: true, - }, - }); + const cursor = await UserModel.collection + .find({ + roles: { + $exists: true, + }, + }) + .batchSize(100); const updates = []; while (await cursor.hasNext()) { @@ -39,18 +53,22 @@ module.exports = { }, }, }); + + if (updates.length > 1000) { + // Process the updates. + await processUpdates(updates); + + // Clear the updates array. + updates = []; + } } if (updates.length > 0) { - // Create a new batch operation. - const bulk = UserModel.collection.initializeUnorderedBulkOp(); + // Process the updates. + await processUpdates(updates); - for (const { query, update } of updates) { - bulk.find(query).updateOne(update); - } - - // Execute the bulk update operation. - await bulk.execute(); + // Clear the updates array. + updates = []; } }, }; From 1fc07e8649cf9b3e72dc284000fcaffa365733a3 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Wed, 24 Jan 2018 23:31:31 +0100 Subject: [PATCH 02/19] Implement activity tracking for flagged accounts --- client/coral-admin/src/actions/community.js | 6 + client/coral-admin/src/components/Header.js | 6 +- client/coral-admin/src/constants/community.js | 2 + client/coral-admin/src/containers/Header.js | 7 +- client/coral-admin/src/graphql/index.js | 16 +- client/coral-admin/src/reducers/community.js | 10 ++ .../Community/containers/FlaggedAccounts.js | 21 ++- .../routes/Community/containers/Indicator.js | 152 +++++++++++++++++- .../src/routes/Community/graphql.js | 24 ++- 9 files changed, 217 insertions(+), 27 deletions(-) diff --git a/client/coral-admin/src/actions/community.js b/client/coral-admin/src/actions/community.js index d2b8f1a3f..82a74daf3 100644 --- a/client/coral-admin/src/actions/community.js +++ b/client/coral-admin/src/actions/community.js @@ -11,6 +11,7 @@ import { HIDE_BANUSER_DIALOG, SHOW_REJECT_USERNAME_DIALOG, HIDE_REJECT_USERNAME_DIALOG, + SET_INDICATOR_TRACK, } from '../constants/community'; import t from 'coral-framework/services/i18n'; @@ -68,3 +69,8 @@ export const showRejectUsernameDialog = user => ({ export const hideRejectUsernameDialog = () => ({ type: HIDE_REJECT_USERNAME_DIALOG, }); + +export const setIndicatorTrack = track => ({ + type: SET_INDICATOR_TRACK, + track, +}); diff --git a/client/coral-admin/src/components/Header.js b/client/coral-admin/src/components/Header.js index e6bd73f52..6b2274d27 100644 --- a/client/coral-admin/src/components/Header.js +++ b/client/coral-admin/src/components/Header.js @@ -15,6 +15,7 @@ const CoralHeader = ({ showShortcuts = () => {}, auth, root, + data, }) => { return (
@@ -31,7 +32,7 @@ const CoralHeader = ({ activeClassName={styles.active} > {t('configure.moderate')} - + )} {t('configure.community')} - + {can(auth.user, 'UPDATE_CONFIG') && ( @@ -119,6 +120,7 @@ CoralHeader.propTypes = { showShortcuts: PropTypes.func, handleLogout: PropTypes.func.isRequired, root: PropTypes.object.isRequired, + data: PropTypes.object.isRequired, }; export default CoralHeader; diff --git a/client/coral-admin/src/constants/community.js b/client/coral-admin/src/constants/community.js index a9cc2f1e3..1ffe3f8d6 100644 --- a/client/coral-admin/src/constants/community.js +++ b/client/coral-admin/src/constants/community.js @@ -18,3 +18,5 @@ export const SHOW_REJECT_USERNAME_DIALOG = `${prefix}_SHOW_REJECT_USERNAME_DIALO export const HIDE_REJECT_USERNAME_DIALOG = `${prefix}_HIDE_REJECT_USERNAME_DIALOG`; export const SET_SEARCH_VALUE = `${prefix}_SET_SEARCH_VALUE`; + +export const SET_INDICATOR_TRACK = `${prefix}_SET_INDICATOR_TRACK`; diff --git a/client/coral-admin/src/containers/Header.js b/client/coral-admin/src/containers/Header.js index dae096517..dd16c17c6 100644 --- a/client/coral-admin/src/containers/Header.js +++ b/client/coral-admin/src/containers/Header.js @@ -13,10 +13,5 @@ export default withQuery( } ${ModerationIndicator.fragments.root} ${CommunityIndicator.fragments.root} - `, - { - options: { - pollInterval: 10000, - }, - } + ` )(Header); diff --git a/client/coral-admin/src/graphql/index.js b/client/coral-admin/src/graphql/index.js index 3a24702cd..95595c265 100644 --- a/client/coral-admin/src/graphql/index.js +++ b/client/coral-admin/src/graphql/index.js @@ -173,13 +173,17 @@ export default { }, updateQueries: { TalkAdmin_Community_FlaggedAccounts: (prev, { mutationResult }) => { + const decrement = { + flaggedUsernamesCount: { $apply: count => count - 1 }, + }; + // Remove from list after the mutation was "really" completed. if (get(mutationResult, 'data.approveUsername.isOptimistic')) { - return prev; + return update(prev, decrement); } const updated = update(prev, { - flaggedUsernamesCount: { $apply: count => count - 1 }, + ...decrement, flaggedUsers: { nodes: { $apply: nodes => nodes.filter(node => node.id !== id) }, }, @@ -227,13 +231,17 @@ export default { }, updateQueries: { TalkAdmin_Community_FlaggedAccounts: (prev, { mutationResult }) => { + const decrement = { + flaggedUsernamesCount: { $apply: count => count - 1 }, + }; + // Remove from list after the mutation was "really" completed. if (get(mutationResult, 'data.rejectUsername.isOptimistic')) { - return prev; + return update(prev, decrement); } const updated = update(prev, { - flaggedUsernamesCount: { $apply: count => count - 1 }, + ...decrement, flaggedUsers: { nodes: { $apply: nodes => nodes.filter(node => node.id !== id), diff --git a/client/coral-admin/src/reducers/community.js b/client/coral-admin/src/reducers/community.js index 9fe17ad8f..68e471935 100644 --- a/client/coral-admin/src/reducers/community.js +++ b/client/coral-admin/src/reducers/community.js @@ -9,6 +9,7 @@ import { HIDE_BANUSER_DIALOG, SHOW_REJECT_USERNAME_DIALOG, HIDE_REJECT_USERNAME_DIALOG, + SET_INDICATOR_TRACK, } from '../constants/community'; const initialState = { @@ -24,6 +25,10 @@ const initialState = { user: {}, banDialog: false, rejectUsernameDialog: false, + // If true the activity indicator will track flagged account changes + // in order to determine the current queue count. Set this to false + // if the queue count is determined by other means. + indicatorTrack: true, }; export default function community(state = initialState, action) { @@ -91,6 +96,11 @@ export default function community(state = initialState, action) { ...state, searchValue: action.value, }; + case SET_INDICATOR_TRACK: + return { + ...state, + indicatorTrack: action.track, + }; default: return state; } diff --git a/client/coral-admin/src/routes/Community/containers/FlaggedAccounts.js b/client/coral-admin/src/routes/Community/containers/FlaggedAccounts.js index 09240b8f3..f5fd36263 100644 --- a/client/coral-admin/src/routes/Community/containers/FlaggedAccounts.js +++ b/client/coral-admin/src/routes/Community/containers/FlaggedAccounts.js @@ -6,12 +6,15 @@ import withQuery from 'coral-framework/hocs/withQuery'; import { Spinner } from 'coral-ui'; import PropTypes from 'prop-types'; import { withApproveUsername } from 'coral-framework/graphql/mutations'; -import { showRejectUsernameDialog } from '../../../actions/community'; +import { + showRejectUsernameDialog, + setIndicatorTrack, +} from '../../../actions/community'; import { viewUserDetail } from '../../../actions/userDetail'; import { getDefinitionName } from 'coral-framework/utils'; import { appendNewNodes } from 'plugin-api/beta/client/utils'; import update from 'immutability-helper'; -import { handleFlaggedUsernameChange } from '../graphql'; +import { handleFlaggedAccountsChange } from '../graphql'; import { notify } from 'coral-framework/actions/notification'; import { isFlaggedUserDangling } from '../utils'; import t from 'coral-framework/services/i18n'; @@ -50,7 +53,7 @@ class FlaggedAccountsContainer extends Component { prev, { subscriptionData: { data: { usernameFlagged: user } } } ) => { - return handleFlaggedUsernameChange(prev, user, () => { + return handleFlaggedAccountsChange(prev, user, () => { const msg = t( 'flagged_usernames.notify_flagged', whoFlagged(user), @@ -66,7 +69,7 @@ class FlaggedAccountsContainer extends Component { prev, { subscriptionData: { data: { usernameApproved: user } } } ) => { - return handleFlaggedUsernameChange(prev, user, () => { + return handleFlaggedAccountsChange(prev, user, () => { const msg = t( 'flagged_usernames.notify_approved', whoChangedTheStatus(user.state.status.username), @@ -82,7 +85,7 @@ class FlaggedAccountsContainer extends Component { prev, { subscriptionData: { data: { usernameRejected: user } } } ) => { - return handleFlaggedUsernameChange(prev, user, () => { + return handleFlaggedAccountsChange(prev, user, () => { const msg = t( 'flagged_usernames.notify_rejected', whoChangedTheStatus(user.state.status.username), @@ -102,7 +105,7 @@ class FlaggedAccountsContainer extends Component { }, } ) => { - return handleFlaggedUsernameChange(prev, user, () => { + return handleFlaggedAccountsChange(prev, user, () => { const msg = t( 'flagged_usernames.notify_changed', previousUsername, @@ -125,10 +128,14 @@ class FlaggedAccountsContainer extends Component { } componentWillMount() { + // Stop activity indicator tracking, as we'll handle it here. + this.props.setIndicatorTrack(false); this.subscribeToUpdates(); } componentWillUnmount() { + // Restart activity indicator tracking. + this.props.setIndicatorTrack(true); this.unsubscribe(); } @@ -197,6 +204,7 @@ FlaggedAccountsContainer.propTypes = { approveUsername: PropTypes.func, data: PropTypes.object, root: PropTypes.object, + setIndicatorTrack: PropTypes.func, }; const LOAD_MORE_QUERY = gql` @@ -288,6 +296,7 @@ const mapDispatchToProps = dispatch => showRejectUsernameDialog, viewUserDetail, notify, + setIndicatorTrack, }, dispatch ); diff --git a/client/coral-admin/src/routes/Community/containers/Indicator.js b/client/coral-admin/src/routes/Community/containers/Indicator.js index 04a351fb6..eeb5e20f7 100644 --- a/client/coral-admin/src/routes/Community/containers/Indicator.js +++ b/client/coral-admin/src/routes/Community/containers/Indicator.js @@ -1,24 +1,164 @@ +import React, { Component } from 'react'; import { compose, gql } from 'react-apollo'; import Indicator from '../../../components/Indicator'; import { withFragments } from 'plugin-api/beta/client/hocs'; -import { branch, renderNothing } from 'recompose'; +import { handleIndicatorChange } from '../graphql'; +import PropTypes from 'prop-types'; +import { connect } from 'react-redux'; -const hideIfNoData = hasNoData => branch(hasNoData, renderNothing); +class IndicatorContainer extends Component { + subscriptions = []; + + subscribeToUpdates() { + const parameters = [ + { + document: USERNAME_FLAGGED_SUBSCRIPTION, + updateQuery: ( + prev, + { subscriptionData: { data: { usernameFlagged: user } } } + ) => { + return handleIndicatorChange(prev, user); + }, + }, + { + document: USERNAME_APPROVED_SUBSCRIPTION, + updateQuery: ( + prev, + { subscriptionData: { data: { usernameApproved: user } } } + ) => { + return handleIndicatorChange(prev, user); + }, + }, + { + document: USERNAME_REJECTED_SUBSCRIPTION, + updateQuery: ( + prev, + { subscriptionData: { data: { usernameRejected: user } } } + ) => { + return handleIndicatorChange(prev, user); + }, + }, + { + document: USERNAME_CHANGED_SUBSCRIPTION, + updateQuery: ( + prev, + { subscriptionData: { data: { usernameChanged: { user } } } } + ) => { + return handleIndicatorChange(prev, user); + }, + }, + ]; + + this.subscriptions = parameters.map(param => + this.props.data.subscribeToMore(param) + ); + } + + unsubscribe() { + this.subscriptions.forEach(unsubscribe => unsubscribe()); + this.subscriptions = []; + } + + componentWillMount() { + if (this.props.track) { + this.subscribeToUpdates(); + } + } + + componentWillUnmount() { + this.unsubscribe(); + } + + componentWillReceiveProps(nextProps) { + if (!this.props.track && nextProps.track) { + this.subscribeToUpdates(); + } + if (this.props.track && !nextProps.track) { + this.unsubscribe(); + } + } + + render() { + if (!this.props.root || !this.props.root.flaggedUsernamesCount) { + return null; + } + + return ; + } +} + +IndicatorContainer.propTypes = { + data: PropTypes.object, + root: PropTypes.object, + track: PropTypes.bool, +}; + +const fields = ` + state { + status { + username { + status + } + } + } +`; + +const USERNAME_FLAGGED_SUBSCRIPTION = gql` + subscription TalkAdmin_CommunityIndicator_UsernameFlagged { + usernameFlagged { + ${fields} + } + } +`; + +const USERNAME_APPROVED_SUBSCRIPTION = gql` + subscription TalkAdmin_ComunityIndicator_UsernameApproved { + usernameApproved { + ${fields} + } + } +`; + +const USERNAME_REJECTED_SUBSCRIPTION = gql` + subscription TalkAdmin_CommunityIndicator_UsernameRejected { + usernameRejected { + ${fields} + } + } +`; + +const USERNAME_CHANGED_SUBSCRIPTION = gql` + subscription TalkAdmin_ComunityIndicator_UsernameChanged { + usernameChanged { + previousUsername + user { + ${fields} + } + } + } +`; + +const mapStateToProps = state => ({ + track: state.community.indicatorTrack, +}); const enhance = compose( + connect(mapStateToProps), withFragments({ root: gql` - fragment TalkAdmin_Community_Indicator_root on RootQuery { + fragment TalkAdmin_CommunityIndicator_root on RootQuery { flaggedUsernamesCount: userCount( query: { action_type: FLAG state: { status: { username: [SET, CHANGED] } } } ) + me { + id + } } `, - }), - hideIfNoData(props => !props.root.flaggedUsernamesCount) + }) ); -export default enhance(Indicator); +export default enhance(IndicatorContainer); diff --git a/client/coral-admin/src/routes/Community/graphql.js b/client/coral-admin/src/routes/Community/graphql.js index af10c8e86..8d6941c41 100644 --- a/client/coral-admin/src/routes/Community/graphql.js +++ b/client/coral-admin/src/routes/Community/graphql.js @@ -49,13 +49,13 @@ function decrementFlaggedUserCount(root) { } /** - * Assimilate flagged user changes into current store. + * Assimilate flagged acount changes into current store. * @param {Object} root current state of the store * @param {Object} user user that was changed * @param {function} notify callback to show notification * @return {Object} next state of the store */ -export function handleFlaggedUsernameChange(root, user, notify) { +export function handleFlaggedAccountsChange(root, user, notify) { if (user.state.status.username.status !== 'SET') { // Check if change came from current user, if so ignore it. const lastChange = @@ -87,7 +87,7 @@ export function handleFlaggedUsernameChange(root, user, notify) { break; case 'APPROVED': case 'REJECTED': - return root; + return decrementFlaggedUserCount(root); default: } } @@ -105,3 +105,21 @@ export function handleFlaggedUsernameChange(root, user, notify) { } } } + +/** + * Track indicator status + * @param {Object} root current state of the store + * @param {Object} user user that was changed + * @return {Object} next state of the store + */ +export function handleIndicatorChange(root, user) { + switch (user.state.status.username.status) { + case 'SET': + case 'CHANGED': + return incrementFlaggedUserCount(root); + case 'APPROVED': + case 'REJECTED': + return decrementFlaggedUserCount(root); + default: + } +} From 2246b16cf41ea2b88ed534dc3abba6c6457512fa Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Wed, 24 Jan 2018 23:34:53 +0100 Subject: [PATCH 03/19] Add some comments --- client/coral-admin/src/actions/community.js | 1 + 1 file changed, 1 insertion(+) diff --git a/client/coral-admin/src/actions/community.js b/client/coral-admin/src/actions/community.js index 82a74daf3..9a0452ea1 100644 --- a/client/coral-admin/src/actions/community.js +++ b/client/coral-admin/src/actions/community.js @@ -70,6 +70,7 @@ export const hideRejectUsernameDialog = () => ({ type: HIDE_REJECT_USERNAME_DIALOG, }); +// Enable or disable the activity indicator subscriptions. export const setIndicatorTrack = track => ({ type: SET_INDICATOR_TRACK, track, From 66934cbb82485a53a7e7ab6b1db4e5c28d026198 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Thu, 25 Jan 2018 16:45:19 +0100 Subject: [PATCH 04/19] Implement indicator subscriptions for moderation --- client/coral-admin/src/actions/moderation.js | 10 +- .../coral-admin/src/constants/moderation.js | 21 ++- client/coral-admin/src/reducers/moderation.js | 14 +- .../Community/containers/FlaggedAccounts.js | 4 - .../routes/Community/containers/Indicator.js | 3 - .../routes/Moderation/containers/Indicator.js | 176 +++++++++++++++++- .../Moderation/containers/Moderation.js | 6 + .../src/routes/Moderation/graphql.js | 59 ++++-- 8 files changed, 258 insertions(+), 35 deletions(-) diff --git a/client/coral-admin/src/actions/moderation.js b/client/coral-admin/src/actions/moderation.js index cd09b04b2..7899b7685 100644 --- a/client/coral-admin/src/actions/moderation.js +++ b/client/coral-admin/src/actions/moderation.js @@ -31,10 +31,16 @@ export const storySearchChange = value => ({ }); export const clearState = () => ({ - type: actions.MODERATION_CLEAR_STATE, + type: actions.CLEAR_STATE, }); export const selectCommentId = id => ({ - type: actions.MODERATION_SELECT_COMMENT, + type: actions.SELECT_COMMENT, id, }); + +// Enable or disable the activity indicator subscriptions. +export const setIndicatorTrack = track => ({ + type: actions.SET_INDICATOR_TRACK, + track, +}); diff --git a/client/coral-admin/src/constants/moderation.js b/client/coral-admin/src/constants/moderation.js index 8eb939d76..c233692dc 100644 --- a/client/coral-admin/src/constants/moderation.js +++ b/client/coral-admin/src/constants/moderation.js @@ -1,9 +1,12 @@ -export const TOGGLE_MODAL = 'TOGGLE_MODAL'; -export const SINGLE_VIEW = 'SINGLE_VIEW'; -export const HIDE_SHORTCUTS_NOTE = 'HIDE_SHORTCUTS_NOTE'; -export const SET_SORT_ORDER = 'MODERATION_SET_SORT_ORDER'; -export const SHOW_STORY_SEARCH = 'SHOW_STORY_SEARCH'; -export const HIDE_STORY_SEARCH = 'HIDE_STORY_SEARCH'; -export const STORY_SEARCH_CHANGE_VALUE = 'STORY_SEARCH_CHANGE_VALUE'; -export const MODERATION_CLEAR_STATE = 'MODERATION_CLEAR_STATE'; -export const MODERATION_SELECT_COMMENT = 'MODERATION_SELECT_COMMENT'; +const prefix = `MODERATION`; + +export const TOGGLE_MODAL = `${prefix}_TOGGLE_MODAL`; +export const SINGLE_VIEW = `${prefix}_SINGLE_VIEW`; +export const HIDE_SHORTCUTS_NOTE = `${prefix}_HIDE_SHORTCUTS_NOTE`; +export const SET_SORT_ORDER = `${prefix}_SET_SORT_ORDER`; +export const SHOW_STORY_SEARCH = `${prefix}_SHOW_STORY_SEARCH`; +export const HIDE_STORY_SEARCH = `${prefix}_HIDE_STORY_SEARCH`; +export const STORY_SEARCH_CHANGE_VALUE = `${prefix}_STORY_SEARCH_CHANGE_VALUE`; +export const CLEAR_STATE = `${prefix}_CLEAR_STATE`; +export const SELECT_COMMENT = `${prefix}_SELECT_COMMENT`; +export const SET_INDICATOR_TRACK = `${prefix}_SET_INDICATOR_TRACK`; diff --git a/client/coral-admin/src/reducers/moderation.js b/client/coral-admin/src/reducers/moderation.js index e2c6a32ff..3dee12785 100644 --- a/client/coral-admin/src/reducers/moderation.js +++ b/client/coral-admin/src/reducers/moderation.js @@ -8,14 +8,19 @@ const initialState = { shortcutsNoteVisible: 'show', sortOrder: 'DESC', selectedCommentId: '', + // If true the activity indicator will turn on subscriptions + // in order to determine queue counts. Set this to false + // if the queue count is determined by other means. + indicatorTrack: true, }; export default function moderation(state = initialState, action) { switch (action.type) { - case actions.MODERATION_CLEAR_STATE: + case actions.CLEAR_STATE: return { ...initialState, shortcutsNoteVisible: state.shortcutsNoteVisible, + indicatorTrack: state.indicatorTrack, }; case actions.TOGGLE_MODAL: return { @@ -52,11 +57,16 @@ export default function moderation(state = initialState, action) { ...state, sortOrder: action.order, }; - case actions.MODERATION_SELECT_COMMENT: + case actions.SELECT_COMMENT: return { ...state, selectedCommentId: action.id, }; + case actions.SET_INDICATOR_TRACK: + return { + ...state, + indicatorTrack: action.track, + }; default: return state; } diff --git a/client/coral-admin/src/routes/Community/containers/FlaggedAccounts.js b/client/coral-admin/src/routes/Community/containers/FlaggedAccounts.js index f5fd36263..ddd684f87 100644 --- a/client/coral-admin/src/routes/Community/containers/FlaggedAccounts.js +++ b/client/coral-admin/src/routes/Community/containers/FlaggedAccounts.js @@ -35,10 +35,6 @@ function whoFlagged(user) { class FlaggedAccountsContainer extends Component { subscriptions = []; - constructor(props) { - super(props); - } - getCountWithoutDangling() { return this.props.root.flaggedUsers.nodes.filter( node => !isFlaggedUserDangling(node) diff --git a/client/coral-admin/src/routes/Community/containers/Indicator.js b/client/coral-admin/src/routes/Community/containers/Indicator.js index eeb5e20f7..9e5be831a 100644 --- a/client/coral-admin/src/routes/Community/containers/Indicator.js +++ b/client/coral-admin/src/routes/Community/containers/Indicator.js @@ -153,9 +153,6 @@ const enhance = compose( state: { status: { username: [SET, CHANGED] } } } ) - me { - id - } } `, }) diff --git a/client/coral-admin/src/routes/Moderation/containers/Indicator.js b/client/coral-admin/src/routes/Moderation/containers/Indicator.js index bd33a58ef..d4fe22851 100644 --- a/client/coral-admin/src/routes/Moderation/containers/Indicator.js +++ b/client/coral-admin/src/routes/Moderation/containers/Indicator.js @@ -1,11 +1,179 @@ +import React, { Component } from 'react'; import { compose, gql } from 'react-apollo'; import Indicator from '../../../components/Indicator'; import { withFragments } from 'plugin-api/beta/client/hocs'; -import { branch, renderNothing } from 'recompose'; +import { handleIndicatorChange } from '../graphql'; +import PropTypes from 'prop-types'; +import { connect } from 'react-redux'; +import withQueueConfig from '../hoc/withQueueConfig'; +import baseQueueConfig from '../queueConfig'; -const hideIfNoData = hasNoData => branch(hasNoData, renderNothing); +class IndicatorContainer extends Component { + subscriptions = []; + + subscribeToUpdates() { + const parameters = [ + { + document: COMMENT_ADDED_SUBSCRIPTION, + updateQuery: ( + prev, + { subscriptionData: { data: { commentAdded: comment } } } + ) => { + return handleIndicatorChange(prev, comment, this.props.queueConfig); + }, + }, + { + document: COMMENT_FLAGGED_SUBSCRIPTION, + updateQuery: ( + prev, + { subscriptionData: { data: { commentFlagged: comment } } } + ) => { + return handleIndicatorChange(prev, comment, this.props.queueConfig); + }, + }, + { + document: COMMENT_ACCEPTED_SUBSCRIPTION, + updateQuery: ( + prev, + { subscriptionData: { data: { commentAccepted: comment } } } + ) => { + return handleIndicatorChange(prev, comment, this.props.queueConfig); + }, + }, + { + document: COMMENT_REJECTED_SUBSCRIPTION, + updateQuery: ( + prev, + { subscriptionData: { data: { commentRejected: { comment } } } } + ) => { + return handleIndicatorChange(prev, comment, this.props.queueConfig); + }, + }, + { + document: COMMENT_RESET_SUBSCRIPTION, + updateQuery: ( + prev, + { subscriptionData: { data: { commentReset: { comment } } } } + ) => { + return handleIndicatorChange(prev, comment, this.props.queueConfig); + }, + }, + ]; + + this.subscriptions = parameters.map(param => + this.props.data.subscribeToMore(param) + ); + } + + unsubscribe() { + this.subscriptions.forEach(unsubscribe => unsubscribe()); + this.subscriptions = []; + } + + componentWillMount() { + if (this.props.track) { + this.subscribeToUpdates(); + } + } + + componentWillUnmount() { + this.unsubscribe(); + } + + componentWillReceiveProps(nextProps) { + if (!this.props.track && nextProps.track) { + this.subscribeToUpdates(); + } + if (this.props.track && !nextProps.track) { + this.unsubscribe(); + } + } + + render() { + if ( + !this.props.root || + (!this.props.root.premodCount && !this.props.root.reportedCount) + ) { + return null; + } + + return ; + } +} + +IndicatorContainer.propTypes = { + data: PropTypes.object, + root: PropTypes.object, + track: PropTypes.bool, + queueConfig: PropTypes.object, +}; + +const fields = ` + status + actions { + __typename + ... on FlagAction { + reason + } + user { + id + role + } + } + status_history { + type + assigned_by { + id + } + } +`; + +const COMMENT_ADDED_SUBSCRIPTION = gql` + subscription TalkAdmin_ModerationIndicator_CommentAdded { + commentAdded { + ${fields} + } + } +`; + +const COMMENT_FLAGGED_SUBSCRIPTION = gql` + subscription TalkAdmin_ModerationIndicator_CommentFlagged { + commentFlagged { + ${fields} + } + } +`; + +const COMMENT_ACCEPTED_SUBSCRIPTION = gql` + subscription TalkAdmin_ModerationIndicator_CommentAccepted { + commentAccepted{ + ${fields} + } + } +`; + +const COMMENT_REJECTED_SUBSCRIPTION = gql` + subscription TalkAdmin_ModerationIndicator_CommentRejected { + commentRejected{ + ${fields} + } + } +`; + +const COMMENT_RESET_SUBSCRIPTION = gql` + subscription TalkAdmin_ModerationIndicator_CommentReset { + commentReset { + ${fields} + } + } +`; + +const mapStateToProps = state => ({ + track: state.moderation.indicatorTrack, +}); const enhance = compose( + connect(mapStateToProps), withFragments({ root: gql` fragment TalkAdmin_Moderation_Indicator_root on RootQuery { @@ -19,7 +187,7 @@ const enhance = compose( } `, }), - hideIfNoData(props => !props.root.premodCount && !props.root.reportedCount) + withQueueConfig(baseQueueConfig) ); -export default enhance(Indicator); +export default enhance(IndicatorContainer); diff --git a/client/coral-admin/src/routes/Moderation/containers/Moderation.js b/client/coral-admin/src/routes/Moderation/containers/Moderation.js index fd96c598b..9790291a8 100644 --- a/client/coral-admin/src/routes/Moderation/containers/Moderation.js +++ b/client/coral-admin/src/routes/Moderation/containers/Moderation.js @@ -27,6 +27,7 @@ import { storySearchChange, clearState, selectCommentId, + setIndicatorTrack, } from 'actions/moderation'; import withQueueConfig from '../hoc/withQueueConfig'; import { notify } from 'coral-framework/actions/notification'; @@ -198,11 +199,15 @@ class ModerationContainer extends Component { } componentWillMount() { + // Stop activity indicator tracking, as we'll handle it here. + this.props.setIndicatorTrack(false); this.props.clearState(); this.subscribeToUpdates(); } componentWillUnmount() { + // Restart activity indicator tracking. + this.props.setIndicatorTrack(true); this.unsubscribe(); } @@ -525,6 +530,7 @@ const mapDispatchToProps = dispatch => ({ clearState, notify, selectCommentId, + setIndicatorTrack, }, dispatch ), diff --git a/client/coral-admin/src/routes/Moderation/graphql.js b/client/coral-admin/src/routes/Moderation/graphql.js index 481c42ab9..ae4bb5e6e 100644 --- a/client/coral-admin/src/routes/Moderation/graphql.js +++ b/client/coral-admin/src/routes/Moderation/graphql.js @@ -91,6 +91,22 @@ function getCommentQueues(comment, queueConfig) { return queues; } +/** + * getPreviousCommentQueues determines queues that this comment previously belonged to. + */ +function getPreviousCommentQueues(comment, queueConfig) { + return comment.status_history.length <= 1 + ? [] + : getCommentQueues( + { + ...comment, + status: + comment.status_history[comment.status_history.length - 2].type, + }, + queueConfig + ); +} + /** * Return whether or not the comment belongs to the queue. */ @@ -203,17 +219,7 @@ export function handleCommentChange( let next = root; // Queues that this comment previously belonged to. - const prevQueues = - comment.status_history.length <= 1 - ? [] - : getCommentQueues( - { - ...comment, - status: - comment.status_history[comment.status_history.length - 2].type, - }, - queueConfig - ); + const prevQueues = getPreviousCommentQueues(comment, queueConfig); // Queues that this comment needs to be placed. const nextQueues = getCommentQueues(comment, queueConfig); @@ -291,3 +297,34 @@ export function handleCommentChange( }); return next; } + +const indicatorQueues = ['premod', 'reported']; + +/** + * Track indicator status + * @param {Object} root current state of the store + * @param {Object} comment comment that was changed + * @return {Object} next state of the store + */ +export function handleIndicatorChange(root, comment, queueConfig) { + let next = root; + + // Queues that this comment previously belonged to. + const prevQueues = getPreviousCommentQueues(comment, queueConfig); + + // Queues that this comment needs to be placed. + const nextQueues = getCommentQueues(comment, queueConfig); + + for (const queue of indicatorQueues) { + if (prevQueues.indexOf(queue) === -1 && nextQueues.indexOf(queue) >= 0) { + next = increaseCommentCount(next, queue); + } else if ( + prevQueues.indexOf(queue) >= 0 && + nextQueues.indexOf(queue) === -1 + ) { + next = decreaseCommentCount(next, queue); + } + } + + return next; +} From 1e517469c3d0b968e743c51fb4d28a8bdbedcb67 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 25 Jan 2018 10:16:14 -0700 Subject: [PATCH 05/19] Fail on plugin installation fail --- bin/cli-plugins | 86 +++++++++++++++++++++++++------------------------ 1 file changed, 44 insertions(+), 42 deletions(-) diff --git a/bin/cli-plugins b/bin/cli-plugins index a99e97649..883682ede 100755 --- a/bin/cli-plugins +++ b/bin/cli-plugins @@ -235,7 +235,7 @@ async function reconcileLocalPlugins({ skipRemote, dryRun }) { if (output.status) { throw new Error( - 'Could not install local plugin dependencies, errors occured during install' + 'Could not install local plugin dependencies, errors occurred during install' ); } @@ -253,59 +253,61 @@ async function reconcilePluginDeps({ dryRun, upgradeRemote, }) { - let startTime = new Date(); + try { + let startTime = new Date(); - // We don't need to do anything if we skip everything.... - if (skipLocal && skipRemote) { - return; - } + // We don't need to do anything if we skip everything.... + if (skipLocal && skipRemote) { + return; + } - // Traverse local plugins and install dependencies if enabled. - if (!skipLocal) { - await reconcileLocalPlugins({ skipRemote, dryRun }); - } + // Traverse local plugins and install dependencies if enabled. + if (!skipLocal) { + await reconcileLocalPlugins({ skipRemote, dryRun }); + } - // Locate any external plugins and install them. - if (!skipRemote) { - let results = []; - try { - results = await reconcileRemotePlugins({ + // Locate any external plugins and install them. + if (!skipRemote) { + const results = await reconcileRemotePlugins({ skipLocal, skipRemote, dryRun, upgradeRemote, }); - } catch (e) { - throw e; + + let status; + if (dryRun) { + status = '[dry-run] success'.green; + } else { + status = 'success'.green; + } + + let message; + if (results.upgradable.length === 0 && results.fetchable.length === 0) { + message = 'Already up-to-date.'; + } else if (results.upgradable.length === 0) { + message = `Fetched ${results.fetchable.length} new plugins.`; + } else if (results.fetchable.length === 0) { + message = `Upgraded ${results.upgradable.length} new plugins.`; + } else { + message = `Fetched ${results.fetchable.length} new plugins, upgraded ${ + results.upgradable.length + } plugins.`; + } + + console.log(`\n${status} ${message}`); } - let status; - if (dryRun) { - status = '[dry-run] success'.green; - } else { - status = 'success'.green; - } + let endTime = new Date(); - let message; - if (results.upgradable.length === 0 && results.fetchable.length === 0) { - message = 'Already up-to-date.'; - } else if (results.upgradable.length === 0) { - message = `Fetched ${results.fetchable.length} new plugins.`; - } else if (results.fetchable.length === 0) { - message = `Upgraded ${results.upgradable.length} new plugins.`; - } else { - message = `Fetched ${results.fetchable.length} new plugins, upgraded ${ - results.upgradable.length - } plugins.`; - } - - console.log(`\n${status} ${message}`); + let totalTime = ((endTime.getTime() - startTime.getTime()) / 1000).toFixed( + 2 + ); + console.log(`✨ Done in ${totalTime}s.`); + } catch (err) { + console.error(err); + process.exit(1); } - - let endTime = new Date(); - - let totalTime = ((endTime.getTime() - startTime.getTime()) / 1000).toFixed(2); - console.log(`✨ Done in ${totalTime}s.`); } async function createSeedPlugin() { From de9383e657c42f83f742e81bd897adc8023e8e17 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 25 Jan 2018 10:17:26 -0700 Subject: [PATCH 06/19] more extreme error exiting for rejected promise that are not handled --- bin/cli | 17 ----------------- bin/util.js | 3 ++- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/bin/cli b/bin/cli index 6237d4740..b61c529c5 100755 --- a/bin/cli +++ b/bin/cli @@ -41,20 +41,3 @@ if (!commands.includes(command)) { } process.exit(1); } - -// /** -// * When this process exists, check to see if we have a running command, if we do -// * check to see if it is still running. If it is, then kill it with a SIGINT -// * signal. This is for the use case where we want to kill the process that is -// * labeled with the PID written out by the parent process. -// */ -// process.once('exit', () => { -// if ( - -// // program.runningCommand && -// program.runningCommand.killed === false && -// program.runningCommand.exitCode === null -// ) { -// program.runningCommand.kill('SIGINT'); -// } -// }); diff --git a/bin/util.js b/bin/util.js index 0a9adb966..e80f123a5 100644 --- a/bin/util.js +++ b/bin/util.js @@ -63,5 +63,6 @@ process.once('SIGUSR2', () => util.shutdown(0, 'SIGUSR2')); // ignoring them. In the future, promise rejections that are not handled will // terminate the Node.js process with a non-zero exit code. process.on('unhandledRejection', err => { - throw err; + console.error(err); + process.exit(1); }); From ef7693afa5ed1ca50c8674932c5d4fa1df858706 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 25 Jan 2018 11:25:32 -0700 Subject: [PATCH 07/19] fixed error in migration --- migrations/1511801783_user_roles.js | 2 +- package.json | 6 +++++- yarn.lock | 24 +++++++++++++++--------- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/migrations/1511801783_user_roles.js b/migrations/1511801783_user_roles.js index aaca89b71..2341463a5 100644 --- a/migrations/1511801783_user_roles.js +++ b/migrations/1511801783_user_roles.js @@ -34,7 +34,7 @@ module.exports = { }) .batchSize(100); - const updates = []; + let updates = []; while (await cursor.hasNext()) { const user = await cursor.next(); diff --git a/package.json b/package.json index 17af19867..a0cc594ab 100644 --- a/package.json +++ b/package.json @@ -222,6 +222,10 @@ }, "pre-commit": { "silent": false, - "run": ["lint", "test:client", "test:server"] + "run": [ + "lint", + "test:client", + "test:server" + ] } } diff --git a/yarn.lock b/yarn.lock index 38f5963ec..d13397fee 100644 --- a/yarn.lock +++ b/yarn.lock @@ -55,8 +55,8 @@ to-fast-properties "^2.0.0" "@coralproject/eslint-config-talk@^0.1.0": - version "0.1.0" - resolved "https://registry.yarnpkg.com/@coralproject/eslint-config-talk/-/eslint-config-talk-0.1.0.tgz#3ddc5f6fb4362a1cd05a5fea56cdb3095afc8cc3" + version "0.1.1" + resolved "https://registry.yarnpkg.com/@coralproject/eslint-config-talk/-/eslint-config-talk-0.1.1.tgz#71991b4937a3ffe657128d7f1170da4b5fb75c9e" dependencies: babel-eslint "^8.0.1" eslint-config-prettier "^2.9.0" @@ -2507,7 +2507,13 @@ dns-prefetch-control@0.1.0: version "0.1.0" resolved "https://registry.yarnpkg.com/dns-prefetch-control/-/dns-prefetch-control-0.1.0.tgz#60ddb457774e178f1f9415f0cabb0e85b0b300b2" -doctrine@^2.0.0, doctrine@^2.0.2: +doctrine@^2.0.0: + version "2.1.0" + resolved "https://registry.yarnpkg.com/doctrine/-/doctrine-2.1.0.tgz#5cd01fc101621b42c4cd7f5d1a66243716d3f39d" + dependencies: + esutils "^2.0.2" + +doctrine@^2.0.2: version "2.0.2" resolved "https://registry.yarnpkg.com/doctrine/-/doctrine-2.0.2.tgz#68f96ce8efc56cc42651f1faadb4f175273b0075" dependencies: @@ -2746,7 +2752,7 @@ error-ex@^1.2.0, error-ex@^1.3.1: dependencies: is-arrayish "^0.2.1" -es-abstract@^1.4.3, es-abstract@^1.7.0: +es-abstract@^1.4.3: version "1.9.0" resolved "https://registry.yarnpkg.com/es-abstract/-/es-abstract-1.9.0.tgz#690829a07cae36b222e7fd9b75c0d0573eb25227" dependencies: @@ -2756,7 +2762,7 @@ es-abstract@^1.4.3, es-abstract@^1.7.0: is-callable "^1.1.3" is-regex "^1.0.4" -es-abstract@^1.6.1: +es-abstract@^1.6.1, es-abstract@^1.7.0: version "1.10.0" resolved "https://registry.yarnpkg.com/es-abstract/-/es-abstract-1.10.0.tgz#1ecb36c197842a00d8ee4c2dfd8646bb97d60864" dependencies: @@ -2873,8 +2879,8 @@ eslint-config-prettier@^2.9.0: get-stdin "^5.0.1" eslint-plugin-jest@^21.6.1: - version "21.6.1" - resolved "https://registry.yarnpkg.com/eslint-plugin-jest/-/eslint-plugin-jest-21.6.1.tgz#adca015bbdb8d23b210438ff9e1cee1dd9ec35df" + version "21.7.0" + resolved "https://registry.yarnpkg.com/eslint-plugin-jest/-/eslint-plugin-jest-21.7.0.tgz#651f1c6ce999af3ac59ab8bf8a376d742fd0fc23" eslint-plugin-mocha@^4.11.0: version "4.11.0" @@ -2883,8 +2889,8 @@ eslint-plugin-mocha@^4.11.0: ramda "^0.24.1" eslint-plugin-prettier@^2.4.0: - version "2.4.0" - resolved "https://registry.yarnpkg.com/eslint-plugin-prettier/-/eslint-plugin-prettier-2.4.0.tgz#85cab0775c6d5e3344ef01e78d960f166fb93aae" + version "2.5.0" + resolved "https://registry.yarnpkg.com/eslint-plugin-prettier/-/eslint-plugin-prettier-2.5.0.tgz#39a91dd7528eaf19cd42c0ee3f2c1f684606a05f" dependencies: fast-diff "^1.1.1" jest-docblock "^21.0.0" From 26ca3622c2c633090f81b92a9368bf9651309333 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Thu, 25 Jan 2018 20:01:25 +0100 Subject: [PATCH 08/19] Integrate featured into activity indicator subscription --- .../routes/Moderation/containers/Indicator.js | 34 +++++-- .../containers/ModIndicatorSubscription.js | 94 +++++++++++++++++++ .../client/index.js | 2 + .../talk-plugin-featured-comments/index.js | 4 +- 4 files changed, 122 insertions(+), 12 deletions(-) create mode 100644 plugins/talk-plugin-featured-comments/client/containers/ModIndicatorSubscription.js diff --git a/client/coral-admin/src/routes/Moderation/containers/Indicator.js b/client/coral-admin/src/routes/Moderation/containers/Indicator.js index d4fe22851..7df710f2f 100644 --- a/client/coral-admin/src/routes/Moderation/containers/Indicator.js +++ b/client/coral-admin/src/routes/Moderation/containers/Indicator.js @@ -7,10 +7,15 @@ import PropTypes from 'prop-types'; import { connect } from 'react-redux'; import withQueueConfig from '../hoc/withQueueConfig'; import baseQueueConfig from '../queueConfig'; +import Slot from 'coral-framework/components/Slot'; class IndicatorContainer extends Component { subscriptions = []; + handleCommentChange = (root, comment) => { + return handleIndicatorChange(root, comment, this.props.queueConfig); + }; + subscribeToUpdates() { const parameters = [ { @@ -19,7 +24,7 @@ class IndicatorContainer extends Component { prev, { subscriptionData: { data: { commentAdded: comment } } } ) => { - return handleIndicatorChange(prev, comment, this.props.queueConfig); + return this.handleCommentChange(prev, comment); }, }, { @@ -28,7 +33,7 @@ class IndicatorContainer extends Component { prev, { subscriptionData: { data: { commentFlagged: comment } } } ) => { - return handleIndicatorChange(prev, comment, this.props.queueConfig); + return this.handleCommentChange(prev, comment); }, }, { @@ -37,25 +42,25 @@ class IndicatorContainer extends Component { prev, { subscriptionData: { data: { commentAccepted: comment } } } ) => { - return handleIndicatorChange(prev, comment, this.props.queueConfig); + return this.handleCommentChange(prev, comment); }, }, { document: COMMENT_REJECTED_SUBSCRIPTION, updateQuery: ( prev, - { subscriptionData: { data: { commentRejected: { comment } } } } + { subscriptionData: { data: { commentRejected: comment } } } ) => { - return handleIndicatorChange(prev, comment, this.props.queueConfig); + return this.handleCommentChange(prev, comment); }, }, { document: COMMENT_RESET_SUBSCRIPTION, updateQuery: ( prev, - { subscriptionData: { data: { commentReset: { comment } } } } + { subscriptionData: { data: { commentReset: comment } } } ) => { - return handleIndicatorChange(prev, comment, this.props.queueConfig); + return this.handleCommentChange(prev, comment); }, }, ]; @@ -97,7 +102,16 @@ class IndicatorContainer extends Component { return null; } - return ; + return ( + + + + + ); } } @@ -146,7 +160,7 @@ const COMMENT_FLAGGED_SUBSCRIPTION = gql` const COMMENT_ACCEPTED_SUBSCRIPTION = gql` subscription TalkAdmin_ModerationIndicator_CommentAccepted { - commentAccepted{ + commentAccepted { ${fields} } } @@ -154,7 +168,7 @@ const COMMENT_ACCEPTED_SUBSCRIPTION = gql` const COMMENT_REJECTED_SUBSCRIPTION = gql` subscription TalkAdmin_ModerationIndicator_CommentRejected { - commentRejected{ + commentRejected { ${fields} } } diff --git a/plugins/talk-plugin-featured-comments/client/containers/ModIndicatorSubscription.js b/plugins/talk-plugin-featured-comments/client/containers/ModIndicatorSubscription.js new file mode 100644 index 000000000..53af12eb4 --- /dev/null +++ b/plugins/talk-plugin-featured-comments/client/containers/ModIndicatorSubscription.js @@ -0,0 +1,94 @@ +import React from 'react'; +import { gql } from 'react-apollo'; + +class ModIndicatorSubscription extends React.Component { + subscriptions = null; + + componentWillMount() { + const configs = [ + { + document: COMMENT_FEATURED_SUBSCRIPTION, + updateQuery: ( + prev, + { subscriptionData: { data: { commentFeatured: { comment } } } } + ) => { + return this.props.handleCommentChange(prev, comment); + }, + }, + { + document: COMMENT_UNFEATURED_SUBSCRIPTION, + updateQuery: ( + prev, + { subscriptionData: { data: { commentUnfeatured: { comment } } } } + ) => { + return this.props.handleCommentChange(prev, comment); + }, + }, + ]; + this.subscriptions = configs.map(config => + this.props.data.subscribeToMore(config) + ); + } + + componentWillUnmount() { + this.subscriptions.forEach(unsubscribe => unsubscribe()); + } + + render() { + return null; + } +} + +const COMMENT_FEATURED_SUBSCRIPTION = gql` + subscription TalkFeaturedComments_Indicator_CommentFeatured { + commentFeatured { + comment { + status + actions { + __typename + ... on FlagAction { + reason + } + user { + id + role + } + } + status_history { + type + assigned_by { + id + } + } + } + } + } +`; + +const COMMENT_UNFEATURED_SUBSCRIPTION = gql` + subscription TalkFeaturedComments_Indicator_CommentUnfeatured { + commentUnfeatured { + comment { + status + actions { + __typename + ... on FlagAction { + reason + } + user { + id + role + } + } + status_history { + type + assigned_by { + id + } + } + } + } + } +`; + +export default ModIndicatorSubscription; diff --git a/plugins/talk-plugin-featured-comments/client/index.js b/plugins/talk-plugin-featured-comments/client/index.js index 1f0bac704..2572a54f4 100644 --- a/plugins/talk-plugin-featured-comments/client/index.js +++ b/plugins/talk-plugin-featured-comments/client/index.js @@ -6,6 +6,7 @@ import update from 'immutability-helper'; import ModTag from './containers/ModTag'; import ModActionButton from './containers/ModActionButton'; import ModSubscription from './containers/ModSubscription'; +import ModIndicatorSubscription from './containers/ModIndicatorSubscription'; import FeaturedDialog from './containers/FeaturedDialog'; import { gql } from 'react-apollo'; import reducer from './reducer'; @@ -23,6 +24,7 @@ export default { moderationActions: [ModActionButton], adminModeration: [ModSubscription, FeaturedDialog], adminCommentInfoBar: [ModTag], + adminModerationIndicator: [ModIndicatorSubscription], }, mutations: { IgnoreUser: ({ variables }) => ({ diff --git a/plugins/talk-plugin-featured-comments/index.js b/plugins/talk-plugin-featured-comments/index.js index f5a9c8ae5..6dd3114e1 100644 --- a/plugins/talk-plugin-featured-comments/index.js +++ b/plugins/talk-plugin-featured-comments/index.js @@ -36,7 +36,7 @@ module.exports = { commentFeatured: (options, args) => ({ commentFeatured: { filter: ({ comment }, { user }) => { - if (args.asset_id === null) { + if (!args.asset_id) { return check(user, ['ADMIN', 'MODERATOR']); } return comment.asset_id === args.asset_id; @@ -46,7 +46,7 @@ module.exports = { commentUnfeatured: (options, args) => ({ commentUnfeatured: { filter: ({ comment }, { user }) => { - if (args.asset_id === null) { + if (!args.asset_id) { return check(user, ['ADMIN', 'MODERATOR']); } return comment.asset_id === args.asset_id; From 6fd4d1576e48f8dc1cd8e3eadc7a1a135c41eba5 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Thu, 25 Jan 2018 20:08:57 +0100 Subject: [PATCH 09/19] Correct logic for disabling activity subscriptions --- .../Moderation/containers/Moderation.js | 31 ++++++++++++++----- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/client/coral-admin/src/routes/Moderation/containers/Moderation.js b/client/coral-admin/src/routes/Moderation/containers/Moderation.js index 9790291a8..481a6d331 100644 --- a/client/coral-admin/src/routes/Moderation/containers/Moderation.js +++ b/client/coral-admin/src/routes/Moderation/containers/Moderation.js @@ -199,25 +199,42 @@ class ModerationContainer extends Component { } componentWillMount() { - // Stop activity indicator tracking, as we'll handle it here. - this.props.setIndicatorTrack(false); + if (!this.props.data.variables.asset_id) { + // Stop activity indicator tracking, as we'll handle it here. + this.props.setIndicatorTrack(false); + } this.props.clearState(); this.subscribeToUpdates(); } componentWillUnmount() { - // Restart activity indicator tracking. - this.props.setIndicatorTrack(true); + if (!this.props.data.variables.asset_id) { + // Restart activity indicator tracking. + this.props.setIndicatorTrack(true); + } this.unsubscribe(); } componentWillReceiveProps(nextProps) { + const currentAssetId = this.props.data.variables.asset_id; + const nextAssetId = nextProps.data.variables.asset_id; + // Resubscribe when we change between assets. - if ( - this.props.data.variables.asset_id !== nextProps.data.variables.asset_id - ) { + if (currentAssetId !== nextAssetId) { this.resubscribe(nextProps.data.variables); } + + // We are only subscribing to a specific asset_id, so activity indicator + // needs to do its own tracking. + if (!currentAssetId && nextAssetId) { + this.props.setIndicatorTrack(true); + } + + // We are subscribing to all comment changes, and as such there is no + // need for the activity indicator to do the same. + if (currentAssetId && !nextAssetId) { + this.props.setIndicatorTrack(false); + } } cleanUpQueue = queue => { From 426025f8c23c0a7c9ac2619b52c7858dc00b6a93 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Thu, 25 Jan 2018 20:49:57 +0100 Subject: [PATCH 10/19] Subscribe to edited + bug fixes --- .../routes/Moderation/containers/Indicator.js | 19 ++++++++++++++++++- graph/resolvers/util.js | 8 ++++++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/client/coral-admin/src/routes/Moderation/containers/Indicator.js b/client/coral-admin/src/routes/Moderation/containers/Indicator.js index 7df710f2f..41c5be990 100644 --- a/client/coral-admin/src/routes/Moderation/containers/Indicator.js +++ b/client/coral-admin/src/routes/Moderation/containers/Indicator.js @@ -36,6 +36,15 @@ class IndicatorContainer extends Component { return this.handleCommentChange(prev, comment); }, }, + { + document: COMMENT_EDITED_SUBSCRIPTION, + updateQuery: ( + prev, + { subscriptionData: { data: { commentEdited: comment } } } + ) => { + return this.handleCommentChange(prev, comment); + }, + }, { document: COMMENT_ACCEPTED_SUBSCRIPTION, updateQuery: ( @@ -144,7 +153,7 @@ const fields = ` const COMMENT_ADDED_SUBSCRIPTION = gql` subscription TalkAdmin_ModerationIndicator_CommentAdded { - commentAdded { + commentAdded(statuses: null) { ${fields} } } @@ -158,6 +167,14 @@ const COMMENT_FLAGGED_SUBSCRIPTION = gql` } `; +const COMMENT_EDITED_SUBSCRIPTION = gql` + subscription TalkAdmin_ModerationIndicator_CommentEdited { + commentEdited { + ${fields} + } + } +`; + const COMMENT_ACCEPTED_SUBSCRIPTION = gql` subscription TalkAdmin_ModerationIndicator_CommentAccepted { commentAccepted { diff --git a/graph/resolvers/util.js b/graph/resolvers/util.js index ff9ab9c95..992148b4f 100644 --- a/graph/resolvers/util.js +++ b/graph/resolvers/util.js @@ -50,8 +50,12 @@ const decorateWithPermissionCheck = (typeResolver, protect) => { */ const decorateUserField = (typeResolver, field) => { // The default resolver for the user decorator is loading the user by id. - let fieldResolver = (obj, args, ctx) => - ctx.loaders.Users.getByID.load(obj[field]); + let fieldResolver = (obj, args, ctx) => { + if (!obj[field]) { + return null; + } + return ctx.loaders.Users.getByID.load(obj[field]); + }; // The resolver can be overridden however. This decorator will simply wrap the // field with a permission check. From e6f796e99c4e8ecb2a208d51f7f85d0bbb23d68d Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 25 Jan 2018 13:07:29 -0700 Subject: [PATCH 11/19] expanded on migration support --- bin/cli-migration | 87 +++++++++++++------ migrations/1496771633_tags.js | 63 +++++++------- migrations/1507310316_flags.js | 11 +-- migrations/1510174676_user_status.js | 40 +++------ migrations/1511801783_user_roles.js | 23 ++--- migrations/utils.js | 19 ++++ services/migration.js | 25 ++++-- .../migrations/1510174676_user_status.js | 15 ++-- 8 files changed, 157 insertions(+), 126 deletions(-) create mode 100644 migrations/utils.js diff --git a/bin/cli-migration b/bin/cli-migration index ca93360ad..24f41ac81 100755 --- a/bin/cli-migration +++ b/bin/cli-migration @@ -5,6 +5,7 @@ */ const util = require('./util'); +const _ = require('lodash'); const program = require('commander'); const inquirer = require('inquirer'); const mongoose = require('../services/mongoose'); @@ -25,46 +26,61 @@ async function createMigration(name) { } } -async function runMigrations() { +async function runMigrations(options) { + const { yes, queryBatchSize, updateBatchSize } = options; + console.log({ yes, queryBatchSize, updateBatchSize }); try { - let { backedUp } = await inquirer.prompt([ - { - type: 'confirm', - name: 'backedUp', - message: 'Did you perform a database backup', - default: false, - }, - ]); + if (!yes) { + const { backedUp } = await inquirer.prompt([ + { + type: 'confirm', + name: 'backedUp', + message: 'Did you perform a database backup', + default: false, + }, + ]); - if (!backedUp) { - throw new Error( - 'Please backup your databases prior to migrations occuring' - ); + if (!backedUp) { + throw new Error( + 'Please backup your databases prior to migrations occuring' + ); + } } // Get the migrations to run. - let migrations = await MigrationService.listPending(); + const migrations = await MigrationService.listPending(); console.log('Now going to run the following migrations:\n'); - for (let { filename } of migrations) { + for (const { filename } of migrations) { console.log(`\tmigrations/${filename}`); } - let { confirm } = await inquirer.prompt([ - { - type: 'confirm', - name: 'confirm', - message: 'Proceed with migrations', - default: false, - }, - ]); + if (!yes) { + const { confirm } = await inquirer.prompt([ + { + type: 'confirm', + name: 'confirm', + message: 'Proceed with migrations', + default: false, + }, + ]); - if (confirm) { - // Run the migrations. - await MigrationService.run(migrations); + if (confirm) { + // Run the migrations. + await MigrationService.run(migrations, { + queryBatchSize, + updateBatchSize, + }); + } else { + console.warn('Skipping migrations'); + } } else { - console.warn('Skipping migrations'); + // Run the migrations. + await MigrationService.run(migrations, { + queryBatchSize, + updateBatchSize, + }); } util.shutdown(); @@ -83,8 +99,25 @@ program .description('creates a new migration') .action(createMigration); +// Bypasses issue that defaults + coercion doesn't work well together. +// Ref: https://github.com/tj/commander.js/issues/400#issuecomment-310860869 +const parse10 = _.ary(_.partialRight(parseInt, 10), 1); + program .command('run') + .option( + '-q, --query-batch-size ', + 'will answer yes to all questions', + parse10, + 100 + ) + .option( + '-u, --update-batch-size ', + 'will answer yes to all questions', + parse10, + 1000 + ) + .option('-y, --yes', 'will answer yes to all questions') .description('runs all pending migrations') .action(runMigrations); diff --git a/migrations/1496771633_tags.js b/migrations/1496771633_tags.js index b21ff671e..142f10fc0 100644 --- a/migrations/1496771633_tags.js +++ b/migrations/1496771633_tags.js @@ -1,34 +1,32 @@ const CommentModel = require('../models/comment'); +const { processUpdates } = require('./utils'); module.exports = { - async up() { + async up({ queryBatchSize, updateBatchSize }) { // Find all comments that have tags. - let comments = await CommentModel.aggregate([ - { - $match: { - tags: { - $exists: true, - $ne: [], + const cursor = await CommentModel.collection + .aggregate([ + { + $match: { + tags: { + $exists: true, + $ne: [], + }, }, }, - }, - { - $project: { - id: true, - tags: true, + { + $project: { + id: true, + tags: true, + }, }, - }, - ]); + ]) + .batchSize(queryBatchSize); - // If no comments were found, nothing needs to be done! - if (comments.length <= 0) { - return; - } + let updates = []; + while (await cursor.hasNext()) { + let { id, tags } = await cursor.next(); - const updates = []; - - // Loop over the comments retrieved, updating the tag structure. - for (let { id, tags } of comments) { // OLD // // [ @@ -75,19 +73,22 @@ module.exports = { })); updates.push({ query: { id }, update: { $set: { tags } } }); + + if (updates.length > updateBatchSize) { + // Process the updates. + await processUpdates(CommentModel, updates); + + // Clear the updates array. + updates = []; + } } if (updates.length > 0) { - // Create a new batch operation. - let batch = CommentModel.collection.initializeUnorderedBulkOp(); + // Process the updates. + await processUpdates(CommentModel, updates); - for (const { query, update } of updates) { - // Execute the batch operation. - batch.find(query).updateOne(update); - } - - // Execute the batch update operation. - await batch.execute(); + // Clear the updates array. + updates = []; } }, }; diff --git a/migrations/1507310316_flags.js b/migrations/1507310316_flags.js index d29a2c25b..3b0277e95 100644 --- a/migrations/1507310316_flags.js +++ b/migrations/1507310316_flags.js @@ -1,4 +1,5 @@ const ActionModel = require('../models/action'); +const { processUpdates } = require('./utils'); const mapping = { COMMENTS: { @@ -44,15 +45,7 @@ module.exports = { } if (updates.length > 0) { - // Setup the batch operation. - const batch = ActionModel.collection.initializeUnorderedBulkOp(); - - for (const { query, update } of updates) { - batch.find(query).update(update); - } - - // Execute the batch update operation. - await batch.execute(); + await processUpdates(ActionModel, updates); } }, }; diff --git a/migrations/1510174676_user_status.js b/migrations/1510174676_user_status.js index 6ddd1e4b4..c8065bd87 100644 --- a/migrations/1510174676_user_status.js +++ b/migrations/1510174676_user_status.js @@ -1,35 +1,19 @@ const UserModel = require('../models/user'); +const { processUpdates } = require('./utils'); const merge = require('lodash/merge'); -const getUserBatch = async () => { - let query = { - status: { - $in: ['ACTIVE', 'BANNED', 'PENDING', 'APPROVED'], - }, - }; - - // Find all the users that need migrating. - return UserModel.collection.find(query).batchSize(100); -}; - -const processUpdates = async updates => { - // Create a new batch operation. - let bulk = UserModel.collection.initializeUnorderedBulkOp(); - - for (const { query, update } of updates) { - bulk.find(query).updateOne(update); - } - - // Execute the bulk update operation. - await bulk.execute(); -}; - module.exports = { - async up() { + async up({ queryBatchSize, updateBatchSize }) { const created_at = Date.now(); // Get the first batch of users. - let cursor = await getUserBatch(); + let cursor = await UserModel.collection + .find({ + status: { + $in: ['ACTIVE', 'BANNED', 'PENDING', 'APPROVED'], + }, + }) + .batchSize(queryBatchSize); let updates = []; while (await cursor.hasNext()) { @@ -225,9 +209,9 @@ module.exports = { updates.push({ query: { id }, update }); // Process every 1000 users. - if (updates.length > 1000) { + if (updates.length > updateBatchSize) { // Process the updates. - await processUpdates(updates); + await processUpdates(UserModel, updates); // Clear the updates array. updates = []; @@ -236,7 +220,7 @@ module.exports = { if (updates.length > 0) { // Process the updates. - await processUpdates(updates); + await processUpdates(UserModel, updates); // Clear the updates array. updates = []; diff --git a/migrations/1511801783_user_roles.js b/migrations/1511801783_user_roles.js index 2341463a5..384047fff 100644 --- a/migrations/1511801783_user_roles.js +++ b/migrations/1511801783_user_roles.js @@ -1,4 +1,5 @@ const UserModel = require('../models/user'); +const { processUpdates } = require('./utils'); const findNewRole = roles => { if (roles.includes('ADMIN')) { @@ -12,27 +13,15 @@ const findNewRole = roles => { return 'COMMENTER'; }; -const processUpdates = async updates => { - // Create a new batch operation. - const bulk = UserModel.collection.initializeUnorderedBulkOp(); - - for (const { query, update } of updates) { - bulk.find(query).updateOne(update); - } - - // Execute the bulk update operation. - await bulk.execute(); -}; - module.exports = { - async up() { + async up({ queryBatchSize, updateBatchSize }) { const cursor = await UserModel.collection .find({ roles: { $exists: true, }, }) - .batchSize(100); + .batchSize(queryBatchSize); let updates = []; while (await cursor.hasNext()) { @@ -54,9 +43,9 @@ module.exports = { }, }); - if (updates.length > 1000) { + if (updates.length > updateBatchSize) { // Process the updates. - await processUpdates(updates); + await processUpdates(UserModel, updates); // Clear the updates array. updates = []; @@ -65,7 +54,7 @@ module.exports = { if (updates.length > 0) { // Process the updates. - await processUpdates(updates); + await processUpdates(UserModel, updates); // Clear the updates array. updates = []; diff --git a/migrations/utils.js b/migrations/utils.js new file mode 100644 index 000000000..3a564ede8 --- /dev/null +++ b/migrations/utils.js @@ -0,0 +1,19 @@ +/** + * processUpdates processes batches of updates on the given model. + * + * @param {Object} model mongoose model that should perform the operations on + * @param {Array} updates array of updates to execute + */ +const processUpdates = async (model, updates) => { + // Create a new batch operation. + const bulk = model.collection.initializeUnorderedBulkOp(); + + for (const { query, update } of updates) { + bulk.find(query).updateOne(update); + } + + // Execute the bulk update operation. + await bulk.execute(); +}; + +module.exports = { processUpdates }; diff --git a/services/migration.js b/services/migration.js index f8f09d469..38ce5339b 100644 --- a/services/migration.js +++ b/services/migration.js @@ -1,17 +1,19 @@ const MigrationModel = require('../models/migration'); const fs = require('fs'); +const ms = require('ms'); const path = require('path'); const Joi = require('joi'); const debug = require('debug')('talk:services:migration'); const sc = require('snake-case'); +const { stripIndent } = require('common-tags'); const { talk: { migration: { minVersion } } } = require('../package.json'); -const migrationTemplate = `module.exports = { - async up() { - - } -}; +const migrationTemplate = stripIndent` + module.exports = { + async up({ queryBatchSize, updateBatchSize }) { + } + }; `; class MigrationService { @@ -61,6 +63,7 @@ class MigrationService { // Parse the migrations from the file listing. let migrations = migrationFiles + .filter(filename => versionRe.test(filename)) .map(filename => { // Parse the version from the filename. let matches = filename.match(versionRe); @@ -109,7 +112,10 @@ class MigrationService { * * @param {Array} migrations a list of migrations returned by `listPending` */ - static async run(migrations) { + static async run( + migrations, + { queryBatchSize = 100, updateBatchSize = 1000 } = {} + ) { if (migrations.length === 0) { console.log('No migrations to run!'); return; @@ -117,9 +123,12 @@ class MigrationService { for (let { filename, version, migration } of migrations) { try { + const startTime = new Date(); console.log(`Starting migration ${filename}`); - await migration.up(); - console.log(`Finished migration ${filename}`); + await migration.up({ queryBatchSize, updateBatchSize }); + const endTime = new Date(); + const totalTime = endTime.getTime() - startTime.getTime(); + console.log(`Finished migration ${filename} in ${ms(totalTime)}`); } catch (e) { console.error(`Migration ${filename} failed`); throw e; diff --git a/test/server/migrations/1510174676_user_status.js b/test/server/migrations/1510174676_user_status.js index 8e1ceca5d..8dbbf543f 100644 --- a/test/server/migrations/1510174676_user_status.js +++ b/test/server/migrations/1510174676_user_status.js @@ -5,6 +5,9 @@ const chai = require('chai'); chai.use(require('chai-datetime')); const { expect } = chai; +const performMigration = () => + migration.up({ queryBatchSize: 100, updateBatchSize: 100 }); + describe('migration.1510174676_user_status', () => { describe('active user', () => { beforeEach(async () => { @@ -24,7 +27,7 @@ describe('migration.1510174676_user_status', () => { expect(user).to.have.property('canEditName', false); // Perform the migration. - await migration.up(); + await performMigration(); user = await UserModel.collection.findOne({ id: '123' }); @@ -54,7 +57,7 @@ describe('migration.1510174676_user_status', () => { expect(user).to.have.property('canEditName', true); // Perform the migration. - await migration.up(); + await performMigration(); user = await UserModel.collection.findOne({ id: '123' }); @@ -85,7 +88,7 @@ describe('migration.1510174676_user_status', () => { expect(user.canEditName).to.equal(true); // Perform the migration. - await migration.up(); + await performMigration(); user = await UserModel.collection.findOne({ id: '123' }); @@ -117,7 +120,7 @@ describe('migration.1510174676_user_status', () => { expect(user.canEditName).to.equal(false); // Perform the migration. - await migration.up(); + await performMigration(); user = await UserModel.collection.findOne({ id: '123' }); @@ -153,7 +156,7 @@ describe('migration.1510174676_user_status', () => { const until = user.suspension.until; // Perform the migration. - await migration.up(); + await performMigration(); user = await UserModel.collection.findOne({ id: '123' }); @@ -187,7 +190,7 @@ describe('migration.1510174676_user_status', () => { expect(user.status).to.equal('BANNED'); // Perform the migration. - await migration.up(); + await performMigration(); user = await UserModel.collection.findOne({ id: '123' }); From e8f73ddb87c5f81a161102abf88497e0d751f3c1 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 25 Jan 2018 13:20:16 -0700 Subject: [PATCH 12/19] changed copy --- bin/cli-migration | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/cli-migration b/bin/cli-migration index 24f41ac81..32c0cb4dd 100755 --- a/bin/cli-migration +++ b/bin/cli-migration @@ -107,13 +107,13 @@ program .command('run') .option( '-q, --query-batch-size ', - 'will answer yes to all questions', + 'change the size of queried documents that are batched at a time', parse10, 100 ) .option( '-u, --update-batch-size ', - 'will answer yes to all questions', + 'change the size of documents that are batched before the update is sent', parse10, 1000 ) From ce25cfb159ceaa163237e2db55f947b3b1a9e102 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Thu, 25 Jan 2018 22:03:37 +0100 Subject: [PATCH 13/19] Reconstruct previous comment state more accurately --- .../src/containers/CommentLabels.js | 1 + .../routes/Moderation/containers/Indicator.js | 11 +-- .../Moderation/containers/Moderation.js | 49 +++++----- .../src/routes/Moderation/graphql.js | 96 ++++++++++++++++--- graph/typeDefs.graphql | 3 + .../containers/ModIndicatorSubscription.js | 53 ++++------ .../client/containers/ModSubscription.js | 27 ++++-- 7 files changed, 153 insertions(+), 87 deletions(-) diff --git a/client/coral-admin/src/containers/CommentLabels.js b/client/coral-admin/src/containers/CommentLabels.js index 4cee8b1cc..ee65eb636 100644 --- a/client/coral-admin/src/containers/CommentLabels.js +++ b/client/coral-admin/src/containers/CommentLabels.js @@ -25,6 +25,7 @@ export default withFragments({ id role } + created_at } ${getSlotFragmentSpreads(slots, 'comment')} } diff --git a/client/coral-admin/src/routes/Moderation/containers/Indicator.js b/client/coral-admin/src/routes/Moderation/containers/Indicator.js index 41c5be990..b5620765e 100644 --- a/client/coral-admin/src/routes/Moderation/containers/Indicator.js +++ b/client/coral-admin/src/routes/Moderation/containers/Indicator.js @@ -135,20 +135,17 @@ const fields = ` status actions { __typename - ... on FlagAction { - reason - } - user { - id - role - } + created_at } status_history { type assigned_by { id } + created_at } + updated_at + created_at `; const COMMENT_ADDED_SUBSCRIPTION = gql` diff --git a/client/coral-admin/src/routes/Moderation/containers/Moderation.js b/client/coral-admin/src/routes/Moderation/containers/Moderation.js index 481a6d331..c654ad6b8 100644 --- a/client/coral-admin/src/routes/Moderation/containers/Moderation.js +++ b/client/coral-admin/src/routes/Moderation/containers/Moderation.js @@ -338,10 +338,30 @@ class ModerationContainer extends Component { ); } } + +const subscriptionFields = ` + status + actions { + __typename + created_at + } + status_history { + type + created_at + assigned_by { + id + username + } + } + created_at + updated_at +`; + const COMMENT_ADDED_SUBSCRIPTION = gql` subscription CommentAdded($asset_id: ID){ commentAdded(asset_id: $asset_id, statuses: null){ ...${getDefinitionName(Comment.fragments.comment)} + ${subscriptionFields} } } ${Comment.fragments.comment} @@ -351,6 +371,7 @@ const COMMENT_EDITED_SUBSCRIPTION = gql` subscription CommentEdited($asset_id: ID){ commentEdited(asset_id: $asset_id){ ...${getDefinitionName(Comment.fragments.comment)} + ${subscriptionFields} } } ${Comment.fragments.comment} @@ -360,6 +381,7 @@ const COMMENT_FLAGGED_SUBSCRIPTION = gql` subscription CommentFlagged($asset_id: ID){ commentFlagged(asset_id: $asset_id){ ...${getDefinitionName(Comment.fragments.comment)} + ${subscriptionFields} } } ${Comment.fragments.comment} @@ -369,14 +391,7 @@ const COMMENT_ACCEPTED_SUBSCRIPTION = gql` subscription CommentAccepted($asset_id: ID){ commentAccepted(asset_id: $asset_id){ ...${getDefinitionName(Comment.fragments.comment)} - status_history { - type - created_at - assigned_by { - id - username - } - } + ${subscriptionFields} } } ${Comment.fragments.comment} @@ -386,14 +401,7 @@ const COMMENT_REJECTED_SUBSCRIPTION = gql` subscription CommentRejected($asset_id: ID){ commentRejected(asset_id: $asset_id){ ...${getDefinitionName(Comment.fragments.comment)} - status_history { - type - created_at - assigned_by { - id - username - } - } + ${subscriptionFields} } } ${Comment.fragments.comment} @@ -403,14 +411,7 @@ const COMMENT_RESET_SUBSCRIPTION = gql` subscription CommentReset($asset_id: ID){ commentReset(asset_id: $asset_id){ ...${getDefinitionName(Comment.fragments.comment)} - status_history { - type - created_at - assigned_by { - id - username - } - } + ${subscriptionFields} } } ${Comment.fragments.comment} diff --git a/client/coral-admin/src/routes/Moderation/graphql.js b/client/coral-admin/src/routes/Moderation/graphql.js index ae4bb5e6e..e113e02e1 100644 --- a/client/coral-admin/src/routes/Moderation/graphql.js +++ b/client/coral-admin/src/routes/Moderation/graphql.js @@ -91,20 +91,87 @@ function getCommentQueues(comment, queueConfig) { return queues; } +function getOlderDate(a, b) { + if (a) { + a = new Date(a); + } + if (b) { + b = new Date(b); + } + + if (!b) { + return a; + } + + if (!a) { + return b; + } + return a < b ? b : a; +} + +function determineLatestChange(comment) { + let lc = null; + + // Skip it when comment itself was not updated. + // We'll get use the one from the status_history instead + // as they might diverge a little bit (status_history item is created + // before the comment itself) + if (comment.createdAt !== comment.updatedAt) { + lc = getOlderDate(lc, comment.createdAt); + lc = getOlderDate(lc, comment.updatedAt); + } + + comment.status_history.forEach(item => { + lc = getOlderDate(lc, item.created_at); + lc = getOlderDate(lc, item.updated_at); + }); + + comment.actions.forEach(item => { + lc = getOlderDate(lc, item.created_at); + lc = getOlderDate(lc, item.updated_at); + }); + + return lc; +} + +function reconstructPreviousCommentState(comment) { + const history = comment.status_history; + const actions = comment.actions; + const lastChangeDate = determineLatestChange(comment); + console.log(lastChangeDate); + const previousComment = { + ...comment, + status_history: history.filter( + item => new Date(item.created_at) < lastChangeDate + ), + actions: actions.filter(item => new Date(item.created_at) < lastChangeDate), + }; + + // Comment did not exist previously. + if (!previousComment.status_history.length) { + return null; + } + + previousComment.status = + previousComment.status_history[ + previousComment.status_history.length - 1 + ].type; + + return previousComment; +} + /** * getPreviousCommentQueues determines queues that this comment previously belonged to. */ function getPreviousCommentQueues(comment, queueConfig) { - return comment.status_history.length <= 1 - ? [] - : getCommentQueues( - { - ...comment, - status: - comment.status_history[comment.status_history.length - 2].type, - }, - queueConfig - ); + const previousCommentState = reconstructPreviousCommentState(comment); + + console.log(previousCommentState, comment); + if (!previousCommentState) { + return []; + } + + return getCommentQueues(previousCommentState, queueConfig); } /** @@ -224,6 +291,8 @@ export function handleCommentChange( // Queues that this comment needs to be placed. const nextQueues = getCommentQueues(comment, queueConfig); + console.log(prevQueues, nextQueues); + let notificationShown = false; const showNotificationOnce = () => { if (notificationShown) { @@ -318,13 +387,12 @@ export function handleIndicatorChange(root, comment, queueConfig) { for (const queue of indicatorQueues) { if (prevQueues.indexOf(queue) === -1 && nextQueues.indexOf(queue) >= 0) { next = increaseCommentCount(next, queue); - } else if ( - prevQueues.indexOf(queue) >= 0 && - nextQueues.indexOf(queue) === -1 - ) { + } + if (prevQueues.indexOf(queue) >= 0 && nextQueues.indexOf(queue) === -1) { next = decreaseCommentCount(next, queue); } } + console.log(prevQueues, nextQueues, next); return next; } diff --git a/graph/typeDefs.graphql b/graph/typeDefs.graphql index 79d1c5b0f..550856082 100644 --- a/graph/typeDefs.graphql +++ b/graph/typeDefs.graphql @@ -502,6 +502,9 @@ type Comment { # The time when the comment was created created_at: Date! + # The time when the comment was updated. + updated_at: Date + # describes how the comment can be edited editing: EditInfo diff --git a/plugins/talk-plugin-featured-comments/client/containers/ModIndicatorSubscription.js b/plugins/talk-plugin-featured-comments/client/containers/ModIndicatorSubscription.js index 53af12eb4..872fe641c 100644 --- a/plugins/talk-plugin-featured-comments/client/containers/ModIndicatorSubscription.js +++ b/plugins/talk-plugin-featured-comments/client/containers/ModIndicatorSubscription.js @@ -39,27 +39,28 @@ class ModIndicatorSubscription extends React.Component { } } +const fields = ` + status + actions { + __typename + created_at + } + status_history { + type + assigned_by { + id + } + created_at + } + updated_at + created_at +`; + const COMMENT_FEATURED_SUBSCRIPTION = gql` subscription TalkFeaturedComments_Indicator_CommentFeatured { commentFeatured { comment { - status - actions { - __typename - ... on FlagAction { - reason - } - user { - id - role - } - } - status_history { - type - assigned_by { - id - } - } + ${fields} } } } @@ -69,23 +70,7 @@ const COMMENT_UNFEATURED_SUBSCRIPTION = gql` subscription TalkFeaturedComments_Indicator_CommentUnfeatured { commentUnfeatured { comment { - status - actions { - __typename - ... on FlagAction { - reason - } - user { - id - role - } - } - status_history { - type - assigned_by { - id - } - } + ${fields} } } } diff --git a/plugins/talk-plugin-featured-comments/client/containers/ModSubscription.js b/plugins/talk-plugin-featured-comments/client/containers/ModSubscription.js index 3e7eb3aaf..da00e8ba7 100644 --- a/plugins/talk-plugin-featured-comments/client/containers/ModSubscription.js +++ b/plugins/talk-plugin-featured-comments/client/containers/ModSubscription.js @@ -74,19 +74,29 @@ class ModSubscription extends React.Component { } } +const fields = ` + status + actions { + __typename + created_at + } + status_history { + type + assigned_by { + id + } + created_at + } + updated_at + created_at +`; + const COMMENT_FEATURED_SUBSCRIPTION = gql` subscription CommentFeatured($assetId: ID){ commentFeatured(asset_id: $assetId) { comment { ...${getDefinitionName(Comment.fragments.comment)} - status_history { - type - created_at - assigned_by { - id - username - } - } + ${fields} } user { id @@ -102,6 +112,7 @@ const COMMENT_UNFEATURED_SUBSCRIPTION = gql` commentUnfeatured(asset_id: $assetId){ comment { ...${getDefinitionName(Comment.fragments.comment)} + ${fields} } user { id From 04390c5acddcdfdaecea8c265028ecb6844be980 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 25 Jan 2018 16:12:26 -0700 Subject: [PATCH 14/19] migration rewrite and removed verifications --- bin/cli | 1 - bin/cli-verify | 58 --- bin/verifications/database/action_counts.js | 194 ---------- bin/verifications/database/comment_replies.js | 140 ------- bin/verifications/database/index.js | 10 - docs/_docs/06-01-migrating-4.md | 22 -- migrations/1496771633_tags.js | 131 +++---- migrations/1507310316_flags.js | 5 +- migrations/1510174676_user_status.js | 349 ++++++++---------- migrations/1511801783_user_roles.js | 44 +-- migrations/1516920154_action_counts.js | 118 ++++++ migrations/1516920160_reply_counts.js | 30 ++ migrations/utils.js | 19 - package.json | 2 +- services/migration/helpers.js | 78 ++++ services/{migration.js => migration/index.js} | 14 +- .../migrations/1510174676_user_status.js | 3 +- 17 files changed, 472 insertions(+), 746 deletions(-) delete mode 100755 bin/cli-verify delete mode 100644 bin/verifications/database/action_counts.js delete mode 100644 bin/verifications/database/comment_replies.js delete mode 100644 bin/verifications/database/index.js create mode 100644 migrations/1516920154_action_counts.js create mode 100644 migrations/1516920160_reply_counts.js delete mode 100644 migrations/utils.js create mode 100644 services/migration/helpers.js rename services/{migration.js => migration/index.js} (92%) diff --git a/bin/cli b/bin/cli index 6237d4740..a0a0a24a0 100755 --- a/bin/cli +++ b/bin/cli @@ -18,7 +18,6 @@ program .command('token', 'work with the access tokens') .command('users', 'work with the application auth') .command('migration', 'provides utilities for migrating the database') - .command('verify', 'provides utilities for performing data verification') .command( 'plugins', 'provides utilities for interacting with the plugin system' diff --git a/bin/cli-verify b/bin/cli-verify deleted file mode 100755 index e32c7d169..000000000 --- a/bin/cli-verify +++ /dev/null @@ -1,58 +0,0 @@ -#!/usr/bin/env node - -/** - * Module dependencies. - */ - -const util = require('./util'); -const program = require('commander'); -const mongoose = require('../services/mongoose'); -const databaseVerifications = require('./verifications/database'); - -// Register the shutdown criteria. -util.onshutdown([() => mongoose.disconnect()]); - -async function database({ fix = false, limit = Infinity, batch = 1000 }) { - try { - for (const verification of databaseVerifications) { - await verification({ fix, limit, batch }); - } - } catch (err) { - console.error( - `Failed to process all the ${databaseVerifications.length} verifications`, - err - ); - util.shutdown(1); - return; - } - - util.shutdown(); -} - -//============================================================================== -// Setting up the program command line arguments. -//============================================================================== - -program - .command('db') - .description('verifies the database integrity') - .option('-f, --fix', 'fix the problems found with database inconsistencies') - .option( - '-l, --limit [size]', - 'limit the amount of documents to process in a single pass, this will ensure only a maximum number of batch operations are issued [default: inf]', - parseInt - ) - .option( - '-b, --batch [size]', - 'batch size to process verifications and repairs of documents [default: 1000]', - parseInt - ) - .action(database); - -program.parse(process.argv); - -// If there is no command listed, output help. -if (!process.argv.slice(2).length) { - program.outputHelp(); - util.shutdown(); -} diff --git a/bin/verifications/database/action_counts.js b/bin/verifications/database/action_counts.js deleted file mode 100644 index 684c17bca..000000000 --- a/bin/verifications/database/action_counts.js +++ /dev/null @@ -1,194 +0,0 @@ -const UserModel = require('../../../models/user'); -const CommentModel = require('../../../models/comment'); -const ActionsService = require('../../../services/actions'); -const { arrayJoinBy } = require('../../../graph/loaders/util'); -const { get } = require('lodash'); -const debug = require('debug')('talk:cli:verify'); - -const MODELS = [UserModel, CommentModel]; - -async function processBatch(Model, documents) { - // Get an array of all the document id's. - const documentIDs = documents.map(({ id }) => id); - - // Store all the operations on this batch in this array that we'll return - // later. - const operations = []; - - // Get the action summaries for this batch. - const totalActionSummaries = await ActionsService.getActionSummaries( - documentIDs - ).then(arrayJoinBy(documentIDs, 'item_id')); - - // Iterate over the documents. - for (let i = 0; i < documents.length; i++) { - const document = documents[i]; - const actionSummaries = totalActionSummaries[i]; - - let ops = []; - - for (const actionSummary of actionSummaries) { - if (actionSummary.group_id === null) { - continue; - } - - // And we generate the group id. - const ACTION_TYPE = actionSummary.action_type.toLowerCase(); - const GROUP_ID = actionSummary.group_id.toLowerCase(); - - if (GROUP_ID.length <= 0) { - continue; - } - - // And we add a new batch operation if the action summary is associated - // with a group. - const ACTION_COUNT_FIELD = `${ACTION_TYPE}_${GROUP_ID}`; - - // Check that the action summaries match the cached counts. - if ( - get(document, ['action_counts', ACTION_COUNT_FIELD]) !== - actionSummary.count - ) { - // Batch updates for those changes. - ops.push({ - [`action_counts.${ACTION_COUNT_FIELD}`]: actionSummary.count, - }); - } - } - - // Group all the action summaries together from all the different group - // ids. - const groupedActionSummaries = actionSummaries.reduce( - (acc, actionSummary) => { - // action_type is already snake cased (as it would have had to be when it - // was inserted in the database). - const ACTION_TYPE = actionSummary.action_type.toLowerCase(); - - if (!(ACTION_TYPE in acc)) { - acc[ACTION_TYPE] = 0; - } - - acc[ACTION_TYPE] += actionSummary.count; - - return acc; - }, - {} - ); - - for (const ACTION_COUNT_FIELD of Object.keys(groupedActionSummaries)) { - const count = groupedActionSummaries[ACTION_COUNT_FIELD]; - - // Check that the action summaries match the cached counts. - if (get(document, ['action_counts', ACTION_COUNT_FIELD]) !== count) { - // Batch updates for those changes. - ops.push({ - [`action_counts.${ACTION_COUNT_FIELD}`]: count, - }); - } - } - - // If this comment has action summaries that should be updated, then - // perform an update! - if (ops.length > 0) { - operations.push({ - updateOne: { - filter: { - id: document.id, - }, - update: { - $set: Object.assign({}, ...ops), - }, - }, - }); - } - } - - return operations; -} - -module.exports = async ({ fix, batch }) => { - for (const Model of MODELS) { - const cursor = Model.collection - .find({}) - .project({ - id: 1, - action_counts: 1, - }) - .sort({ created_at: 1 }); - - let operations = []; - let documents = []; - - // While there are documents to process. - while (await cursor.hasNext()) { - // Load the document. - const document = await cursor.next(); - - // Push the document into the documents array. - documents.push(document); - - // Check to see if the length of the documents array requires us to - // process it. - if (documents.length > batch) { - // Process this batch. - let batchOperations = await processBatch(Model, documents); - - // Push the batch operations into the model operations. - operations.push(...batchOperations); - - // Clear this batch contents. - documents = []; - } - } - - // Check to see if there are any documents left over. - if (documents.length > 0) { - // Process this batch. - let batchOperations = await processBatch(Model, documents); - - // Push the batch operations into the model operations. - operations.push(...batchOperations); - } - - const OPERATIONS_LENGTH = operations.length; - - console.log( - `action_counts.js: ${OPERATIONS_LENGTH} ${ - Model.collection.name - } need their action counts fixed.` - ); - - // If fix was enabled, execute the batch writes. - if (OPERATIONS_LENGTH > 0) { - if (fix) { - debug( - `action_counts.js: fixing ${OPERATIONS_LENGTH} ${ - Model.collection.name - }...` - ); - - while (operations.length) { - let result = await Model.collection.bulkWrite( - operations.splice(0, batch) - ); - - debug( - `action_counts.js: fixed batch of ${result.modifiedCount} ${ - Model.collection.name - }.` - ); - } - - console.log( - `action_counts.js: applied all ${OPERATIONS_LENGTH} fixes to ${ - Model.collection.name - }.` - ); - } else { - console.warn( - 'Skipping fixing, --fix was not enabled, pass --fix to fix these errors' - ); - } - } - } -}; diff --git a/bin/verifications/database/comment_replies.js b/bin/verifications/database/comment_replies.js deleted file mode 100644 index 309023c75..000000000 --- a/bin/verifications/database/comment_replies.js +++ /dev/null @@ -1,140 +0,0 @@ -const CommentModel = require('../../../models/comment'); -const { singleJoinBy } = require('../../../graph/loaders/util'); -const debug = require('debug')('talk:cli:verify'); - -const getBatch = async (limit, offset) => - CommentModel.find({}) - .select({ id: 1, action_counts: 1, reply_count: 1 }) - .limit(limit) - .skip(offset) - .sort('created_at'); - -module.exports = async ({ fix, limit, batch }) => { - let operations = []; - - // Count how many comments there are to process. - const totalCount = await CommentModel.count(); - - let offset = 0; - let comments = []; - let commentIDs = []; - - console.log(`Processing ${totalCount} comments in batches of ${limit}...`); - - // Keep processing documents until there are is none left. - while (offset < totalCount) { - // Get a batch of comments. - comments = await getBatch(batch, offset); - commentIDs = comments.map(({ id }) => id); - - // Get their reply counts. - let allReplyCounts = await CommentModel.aggregate([ - { - $match: { - parent_id: { - $in: commentIDs, - }, - status: { - $in: ['NONE', 'ACCEPTED'], - }, - }, - }, - { - $group: { - _id: '$parent_id', - count: { - $sum: 1, - }, - }, - }, - ]) - .then(singleJoinBy(commentIDs, '_id')) - .then(results => results.map(result => (result ? result.count : 0))); - - // Loop over the comments, with their action summaries. - for (let i = 0; i < comments.length; i++) { - let comment = comments[i]; - let replyCount = allReplyCounts[i]; - - // And check to see if the action summaries we just computed match what is - // currently set for the comments. - let commentOperations = []; - - // If the reply count needs to be updated, then update it! - if (comment.reply_count !== replyCount) { - commentOperations.push({ - reply_count: replyCount, - }); - } - - // If this comment has action summaries that should be updated, then - // perform an update! - if (commentOperations.length > 0) { - operations.push({ - updateOne: { - filter: { - id: comment.id, - }, - update: { - $set: Object.assign({}, ...commentOperations), - }, - }, - }); - } - } - - debug(`Processed batch of ${comments.length} comments.`); - - if (operations.length >= limit) { - debug( - `Queued operations are ${ - operations.length - }, reached limit of ${limit}, not processing any more.` - ); - - if (operations.length > limit) { - debug( - `${operations.length - - limit} operations have been truncated to enforce the limit` - ); - } - - break; - } - - offset += batch; - } - - const OPERATIONS_LENGTH = operations.length; - - if (limit < Infinity && offset + comments.length < totalCount) { - console.log( - `Processed ${offset + - comments.length}/${totalCount} comments because we reached the update limit of ${limit}.` - ); - } else { - console.log(`Processed all ${totalCount} comments.`); - } - - console.log(`${OPERATIONS_LENGTH} documents need fixing.`); - - // If fix was enabled, execute the batch writes. - if (OPERATIONS_LENGTH > 0) { - if (fix) { - debug(`Fixing ${OPERATIONS_LENGTH} documents...`); - - while (operations.length) { - let batchOperations = operations.splice(0, batch); - let result = await CommentModel.collection.bulkWrite(batchOperations); - - debug(`Fixed batch of ${result.modifiedCount} documents.`); - } - - console.log(`Applied all ${OPERATIONS_LENGTH} fixes.`); - } else { - console.warn( - 'Skipping fixing, --fix was not enabled, pass --fix to fix these errors' - ); - } - } -}; diff --git a/bin/verifications/database/index.js b/bin/verifications/database/index.js deleted file mode 100644 index 58a46bd85..000000000 --- a/bin/verifications/database/index.js +++ /dev/null @@ -1,10 +0,0 @@ -// This will import all the verifications that should be run by the: -// -// cli verify database -// -// command. They exist in the form: -// -// async ({fix = false, batch = 1000}) => {} -// -// where their options are derived. -module.exports = [require('./comment_replies'), require('./action_counts')]; diff --git a/docs/_docs/06-01-migrating-4.md b/docs/_docs/06-01-migrating-4.md index b66921cf0..c8af6844d 100644 --- a/docs/_docs/06-01-migrating-4.md +++ b/docs/_docs/06-01-migrating-4.md @@ -40,28 +40,6 @@ documents rather than performing a nice table alter. If the process crashes during the migration, simply re-run it. The migration operations are designed to act atomically, and be idempotent to documents already updated. -## Database Verifications - -In `v3.*`, we introduced the concept of "verifying the database". Some of our -operations update cached values that live along side the original document to -improve performance. Running the cli command for verifying the database's cache -ensures that all the cached values are up to date. - -Running the following will start the database verification process: - -```bash -./bin/cli verify db --fix -``` -You can notice the `--fix` option, without it, the tool should instead perform -a dry run of the operations it intends to perform. -{: .code-aside} - -This process, like the migration process, should take some time to complete on -large databases. - -Once you have updated your databases, that's all you have to do! Talk should now -function even better and faster with all the new features we poured into v4.0.0! - ## Template Change In `v4.0.0`, we introduced extensive support for compressing our javascript diff --git a/migrations/1496771633_tags.js b/migrations/1496771633_tags.js index 142f10fc0..a7155e2d1 100644 --- a/migrations/1496771633_tags.js +++ b/migrations/1496771633_tags.js @@ -1,63 +1,38 @@ const CommentModel = require('../models/comment'); -const { processUpdates } = require('./utils'); -module.exports = { - async up({ queryBatchSize, updateBatchSize }) { - // Find all comments that have tags. - const cursor = await CommentModel.collection - .aggregate([ - { - $match: { - tags: { - $exists: true, - $ne: [], - }, - }, - }, - { - $project: { - id: true, - tags: true, - }, - }, - ]) - .batchSize(queryBatchSize); +// OLD +// +// [ +// { +// name: 'OFF_TOPIC', +// assigned_by: '', +// created_at: new Date() +// } +// ] - let updates = []; - while (await cursor.hasNext()) { - let { id, tags } = await cursor.next(); - - // OLD - // - // [ - // { - // name: 'OFF_TOPIC', - // assigned_by: '', - // created_at: new Date() - // } - // ] - - // NEW - // - // [ - // { - // tag: { - // name: 'OFF_TOPIC', - // permissions: { - // public: true, - // self: false, - // roles: [] - // }, - // models: ['COMMENTS'], - // created_at: new Date() - // }, - // assigned_by: '', - // created_at: new Date() - // } - // ] - - // Remap the tag structure. - tags = tags.map(({ name, assigned_by, created_at }) => ({ +// NEW +// +// [ +// { +// tag: { +// name: 'OFF_TOPIC', +// permissions: { +// public: true, +// self: false, +// roles: [] +// }, +// models: ['COMMENTS'], +// created_at: new Date() +// }, +// assigned_by: '', +// created_at: new Date() +// } +// ] +const transformTags = ({ id, tags }) => ({ + query: { id }, + update: { + $set: { + tags: tags.map(({ name, assigned_by, created_at }) => ({ tag: { name, permissions: { @@ -70,25 +45,31 @@ module.exports = { }, assigned_by, created_at, - })); + })), + }, + }, +}); - updates.push({ query: { id }, update: { $set: { tags } } }); +module.exports = { + async up({ transformSingleWithCursor }) { + // Find all comments that have tags. + const cursor = CommentModel.collection.aggregate([ + { + $match: { + tags: { + $exists: true, + $ne: [], + }, + }, + }, + { + $project: { + id: true, + tags: true, + }, + }, + ]); - if (updates.length > updateBatchSize) { - // Process the updates. - await processUpdates(CommentModel, updates); - - // Clear the updates array. - updates = []; - } - } - - if (updates.length > 0) { - // Process the updates. - await processUpdates(CommentModel, updates); - - // Clear the updates array. - updates = []; - } + await transformSingleWithCursor(cursor, transformTags, CommentModel); }, }; diff --git a/migrations/1507310316_flags.js b/migrations/1507310316_flags.js index 3b0277e95..1ff3b4248 100644 --- a/migrations/1507310316_flags.js +++ b/migrations/1507310316_flags.js @@ -1,5 +1,4 @@ const ActionModel = require('../models/action'); -const { processUpdates } = require('./utils'); const mapping = { COMMENTS: { @@ -13,7 +12,7 @@ const mapping = { }; module.exports = { - async up() { + async up({ processManyUpdates }) { const updates = []; for (const item_type in mapping) { const mappings = mapping[item_type]; @@ -45,7 +44,7 @@ module.exports = { } if (updates.length > 0) { - await processUpdates(ActionModel, updates); + await processManyUpdates(ActionModel, updates); } }, }; diff --git a/migrations/1510174676_user_status.js b/migrations/1510174676_user_status.js index c8065bd87..80271557b 100644 --- a/migrations/1510174676_user_status.js +++ b/migrations/1510174676_user_status.js @@ -1,56 +1,132 @@ const UserModel = require('../models/user'); -const { processUpdates } = require('./utils'); const merge = require('lodash/merge'); -module.exports = { - async up({ queryBatchSize, updateBatchSize }) { - const created_at = Date.now(); +const transformUser = user => { + const created_at = Date.now(); - // Get the first batch of users. - let cursor = await UserModel.collection - .find({ + const { id, status, canEditName, suspension, disabled } = user; + + let update = { + $unset: { + canEditName: '', + suspension: '', + disabled: '', + }, + $set: { + status: { + // The username status is specific to each case. + username: { + history: [], + }, + + // The user is not banned by default. + banned: { + status: false, + history: [], + }, + + // The user is not suspended by default. + suspension: { + until: null, + history: [], + }, + }, + updated_at: created_at, + }, + }; + + if (disabled) { + update = merge(update, { + $set: { status: { - $in: ['ACTIVE', 'BANNED', 'PENDING', 'APPROVED'], + banned: { + status: true, + history: [ + { + status: true, + created_at, + }, + ], + }, }, - }) - .batchSize(queryBatchSize); + }, + }); + } - let updates = []; - while (await cursor.hasNext()) { - const user = await cursor.next(); - - const { id, status, canEditName, suspension, disabled } = user; - - let update = { - $unset: { - canEditName: '', - suspension: '', - disabled: '', + // If the user has an "until" property of their suspension, then we need + // to reflect that in the new status object. + if (suspension && suspension.until !== null) { + update = merge(update, { + $set: { + status: { + suspension: { + until: suspension.until, + history: [ + { + until: suspension.until, + created_at, + }, + ], + }, }, - $set: { - status: { - // The username status is specific to each case. - username: { - history: [], - }, + }, + }); + } - // The user is not banned by default. - banned: { - status: false, - history: [], - }, - - // The user is not suspended by default. - suspension: { - until: null, - history: [], + switch (status) { + case 'ACTIVE': + if (canEditName) { + update = merge(update, { + $set: { + status: { + username: { + status: 'UNSET', + history: [ + { + status: 'UNSET', + created_at, + }, + ], + }, }, }, - updated_at: created_at, - }, - }; - - if (disabled) { + }); + } else { + update = merge(update, { + $set: { + status: { + username: { + status: 'SET', + history: [ + { + status: 'SET', + created_at, + }, + ], + }, + }, + }, + }); + } + break; + case 'BANNED': + if (canEditName) { + update = merge(update, { + $set: { + status: { + username: { + status: 'REJECTED', + history: [ + { + status: 'REJECTED', + created_at, + }, + ], + }, + }, + }, + }); + } else { update = merge(update, { $set: { status: { @@ -63,22 +139,11 @@ module.exports = { }, ], }, - }, - }, - }); - } - - // If the user has an "until" property of their suspension, then we need - // to reflect that in the new status object. - if (suspension && suspension.until !== null) { - update = merge(update, { - $set: { - status: { - suspension: { - until: suspension.until, + username: { + status: 'SET', history: [ { - until: suspension.until, + status: 'SET', created_at, }, ], @@ -87,143 +152,57 @@ module.exports = { }, }); } - - switch (status) { - case 'ACTIVE': - if (canEditName) { - update = merge(update, { - $set: { - status: { - username: { - status: 'UNSET', - history: [ - { - status: 'UNSET', - created_at, - }, - ], - }, - }, - }, - }); - } else { - update = merge(update, { - $set: { - status: { - username: { - status: 'SET', - history: [ - { - status: 'SET', - created_at, - }, - ], - }, - }, - }, - }); - } - break; - case 'BANNED': - if (canEditName) { - update = merge(update, { - $set: { - status: { - username: { - status: 'REJECTED', - history: [ - { - status: 'REJECTED', - created_at, - }, - ], - }, - }, - }, - }); - } else { - update = merge(update, { - $set: { - status: { - banned: { - status: true, - history: [ - { - status: true, - created_at, - }, - ], - }, - username: { - status: 'SET', - history: [ - { - status: 'SET', - created_at, - }, - ], - }, - }, - }, - }); - } - break; - case 'PENDING': - update = merge(update, { - $set: { - status: { - username: { + break; + case 'PENDING': + update = merge(update, { + $set: { + status: { + username: { + status: 'CHANGED', + history: [ + { status: 'CHANGED', - history: [ - { - status: 'CHANGED', - created_at, - }, - ], + created_at, }, - }, + ], }, - }); - break; - case 'APPROVED': - update = merge(update, { - $set: { - status: { - username: { + }, + }, + }); + break; + case 'APPROVED': + update = merge(update, { + $set: { + status: { + username: { + status: 'APPROVED', + history: [ + { status: 'APPROVED', - history: [ - { - status: 'APPROVED', - created_at, - }, - ], + created_at, }, - }, + ], }, - }); - break; - default: - throw new Error(`${status} is an invalid status`); - } + }, + }, + }); + break; + default: + throw new Error(`${status} is an invalid status`); + } - updates.push({ query: { id }, update }); + return { query: { id }, update }; +}; - // Process every 1000 users. - if (updates.length > updateBatchSize) { - // Process the updates. - await processUpdates(UserModel, updates); +module.exports = { + async up({ transformSingleWithCursor }) { + // Get the first batch of users. + const cursor = UserModel.collection.find({ + status: { + $in: ['ACTIVE', 'BANNED', 'PENDING', 'APPROVED'], + }, + }); - // Clear the updates array. - updates = []; - } - } - - if (updates.length > 0) { - // Process the updates. - await processUpdates(UserModel, updates); - - // Clear the updates array. - updates = []; - } + await transformSingleWithCursor(cursor, transformUser, UserModel); }, }; diff --git a/migrations/1511801783_user_roles.js b/migrations/1511801783_user_roles.js index 384047fff..961a696e2 100644 --- a/migrations/1511801783_user_roles.js +++ b/migrations/1511801783_user_roles.js @@ -1,5 +1,4 @@ const UserModel = require('../models/user'); -const { processUpdates } = require('./utils'); const findNewRole = roles => { if (roles.includes('ADMIN')) { @@ -14,20 +13,16 @@ const findNewRole = roles => { }; module.exports = { - async up({ queryBatchSize, updateBatchSize }) { - const cursor = await UserModel.collection - .find({ - roles: { - $exists: true, - }, - }) - .batchSize(queryBatchSize); + async up({ transformSingleWithCursor }) { + const cursor = UserModel.collection.find({ + roles: { + $exists: true, + }, + }); - let updates = []; - while (await cursor.hasNext()) { - const user = await cursor.next(); - - updates.push({ + await transformSingleWithCursor( + cursor, + user => ({ query: { id: user.id, }, @@ -41,23 +36,8 @@ module.exports = { roles: '', }, }, - }); - - if (updates.length > updateBatchSize) { - // Process the updates. - await processUpdates(UserModel, updates); - - // Clear the updates array. - updates = []; - } - } - - if (updates.length > 0) { - // Process the updates. - await processUpdates(UserModel, updates); - - // Clear the updates array. - updates = []; - } + }), + UserModel + ); }, }; diff --git a/migrations/1516920154_action_counts.js b/migrations/1516920154_action_counts.js new file mode 100644 index 000000000..5aa95614c --- /dev/null +++ b/migrations/1516920154_action_counts.js @@ -0,0 +1,118 @@ +const ActionModel = require('../models/action'); +const UserModel = require('../models/user'); +const CommentModel = require('../models/comment'); + +module.exports = { + async up({ transformSingleWithCursor }) { + const models = [ + { Model: CommentModel, item_type: 'COMMENTS' }, + { Model: UserModel, item_type: 'USERS' }, + ]; + for (const { Model, item_type } of models) { + let cursor = ActionModel.collection.aggregate([ + { + $match: { + group_id: { $ne: null }, + item_type, + }, + }, + { + $group: { + // group unique documents by these properties, we are leveraging the + // fact that each uuid is completely unique. + _id: { + item_id: '$item_id', + action_type: '$action_type', + group_id: '$group_id', + }, + + // and sum up all actions matching the above grouping criteria + count: { + $sum: 1, + }, + }, + }, + { + $project: { + // suppress the _id field + _id: false, + + // map the fields from the _id grouping down a level + item_id: '$_id.item_id', + action_type: { $toLower: '$_id.action_type' }, + group_id: { $toLower: '$_id.group_id' }, + + // map the field directly + count: '$count', + }, + }, + ]); + + // Transform those documents. + await transformSingleWithCursor( + cursor, + ({ item_id, action_type, group_id, count }) => ({ + query: { id: item_id }, + update: { + $set: { + [`action_counts.${action_type}_${group_id}`]: count, + }, + }, + }), + Model + ); + + // Secondly, we'll collect the group group id's (all the actions for a + // specific action type) to update counts of. + cursor = ActionModel.collection.aggregate([ + { + $match: { + item_type, + }, + }, + { + $group: { + // group unique documents by these properties, we are leveraging the + // fact that each uuid is completely unique. + _id: { + item_id: '$item_id', + action_type: '$action_type', + }, + + // and sum up all actions matching the above grouping criteria + count: { + $sum: 1, + }, + }, + }, + { + $project: { + // suppress the _id field + _id: false, + + // map the fields from the _id grouping down a level + item_id: '$_id.item_id', + action_type: { $toLower: '$_id.action_type' }, + + // map the field directly + count: '$count', + }, + }, + ]); + + // Transform those documents. + await transformSingleWithCursor( + cursor, + ({ item_id, action_type, count }) => ({ + query: { id: item_id }, + update: { + $set: { + [`action_counts.${action_type}`]: count, + }, + }, + }), + Model + ); + } + }, +}; diff --git a/migrations/1516920160_reply_counts.js b/migrations/1516920160_reply_counts.js new file mode 100644 index 000000000..7dd18e8f8 --- /dev/null +++ b/migrations/1516920160_reply_counts.js @@ -0,0 +1,30 @@ +const CommentModel = require('../models/comment'); + +const transformComments = ({ _id: parent_id, reply_count }) => ({ + query: { id: parent_id, reply_count: { $ne: reply_count } }, + update: { $set: { reply_count } }, +}); + +module.exports = { + async up({ transformSingleWithCursor }) { + const cursor = CommentModel.collection.aggregate([ + { + $match: { + parent_id: { $ne: null }, + status: { $in: ['NONE', 'ACCEPTED'] }, + }, + }, + { + $group: { + _id: '$parent_id', + reply_count: { + $sum: 1, + }, + }, + }, + ]); + + // Transform those documents. + await transformSingleWithCursor(cursor, transformComments, CommentModel); + }, +}; diff --git a/migrations/utils.js b/migrations/utils.js deleted file mode 100644 index 3a564ede8..000000000 --- a/migrations/utils.js +++ /dev/null @@ -1,19 +0,0 @@ -/** - * processUpdates processes batches of updates on the given model. - * - * @param {Object} model mongoose model that should perform the operations on - * @param {Array} updates array of updates to execute - */ -const processUpdates = async (model, updates) => { - // Create a new batch operation. - const bulk = model.collection.initializeUnorderedBulkOp(); - - for (const { query, update } of updates) { - bulk.find(query).updateOne(update); - } - - // Execute the bulk update operation. - await bulk.execute(); -}; - -module.exports = { processUpdates }; diff --git a/package.json b/package.json index a0cc594ab..b3afdc9b6 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,7 @@ }, "talk": { "migration": { - "minVersion": 1511801783 + "minVersion": 1516920160 } }, "repository": { diff --git a/services/migration/helpers.js b/services/migration/helpers.js new file mode 100644 index 000000000..38df5c128 --- /dev/null +++ b/services/migration/helpers.js @@ -0,0 +1,78 @@ +/** + * processUpdates processes batches of updates on the given model. + * + * @param {Object} model mongoose model that should perform the operations on + * @param {Array} updates array of updates to execute + */ +const processUpdates = async (model, updates) => { + // Create a new batch operation. + const bulk = model.collection.initializeUnorderedBulkOp(); + + for (const { query, update } of updates) { + bulk.find(query).updateOne(update); + } + + // Execute the bulk update operation. + await bulk.execute(); +}; + +const transformSingleWithCursor = ({ + queryBatchSize, + updateBatchSize, +}) => async (cursor, process, Model) => { + // We'll manage the updates that we store inside this object. + let updates = []; + + // First we'll collect all the individual actions with specific group id's. + cursor = await cursor.batchSize(queryBatchSize); + + while (await cursor.hasNext()) { + const result = await cursor.next(); + + const transformed = await process(result); + if (transformed) { + updates.push(transformed); + } + + if (updates.length > updateBatchSize) { + // Process the updates. + await processUpdates(Model, updates); + + // Clear the updates array. + updates = []; + } + } + + if (updates.length > 0) { + // Process the updates. + await processUpdates(Model, updates); + + // Clear the updates array. + updates = []; + } +}; + +/** + * processManyUpdates processes batches of updates on many models with the given + * model. + * + * @param {Object} model mongoose model that should perform the operations on + * @param {Array} updates array of updates to execute + */ +const processManyUpdates = async (model, updates) => { + // Create a new batch operation. + const bulk = model.collection.initializeUnorderedBulkOp(); + + for (const { query, update } of updates) { + bulk.find(query).update(update); + } + + // Execute the bulk update operation. + await bulk.execute(); +}; + +module.exports = ctx => ({ + processManyUpdates, + processUpdates, + transformSingleWithCursor: transformSingleWithCursor(ctx), +}); diff --git a/services/migration.js b/services/migration/index.js similarity index 92% rename from services/migration.js rename to services/migration/index.js index 38ce5339b..312282666 100644 --- a/services/migration.js +++ b/services/migration/index.js @@ -1,12 +1,13 @@ -const MigrationModel = require('../models/migration'); +const MigrationModel = require('../../models/migration'); const fs = require('fs'); const ms = require('ms'); const path = require('path'); const Joi = require('joi'); const debug = require('debug')('talk:services:migration'); const sc = require('snake-case'); +const helpers = require('./helpers'); const { stripIndent } = require('common-tags'); -const { talk: { migration: { minVersion } } } = require('../package.json'); +const { talk: { migration: { minVersion } } } = require('../../package.json'); const migrationTemplate = stripIndent` module.exports = { @@ -46,7 +47,7 @@ class MigrationService { static async listPending() { // Get all the migration files. let migrationFiles = fs.readdirSync( - path.join(__dirname, '..', 'migrations') + path.join(__dirname, '..', '..', 'migrations') ); // Ensure that all migrations follow this format. @@ -78,7 +79,7 @@ class MigrationService { } // Read the migration from the filesystem. - let migration = require(`../migrations/${filename}`); + let migration = require(`../../migrations/${filename}`); Joi.assert( migration, migrationSchema, @@ -121,11 +122,14 @@ class MigrationService { return; } + // Create the context helpers. + const ctx = helpers({ queryBatchSize, updateBatchSize }); + for (let { filename, version, migration } of migrations) { try { const startTime = new Date(); console.log(`Starting migration ${filename}`); - await migration.up({ queryBatchSize, updateBatchSize }); + await migration.up(ctx); const endTime = new Date(); const totalTime = endTime.getTime() - startTime.getTime(); console.log(`Finished migration ${filename} in ${ms(totalTime)}`); diff --git a/test/server/migrations/1510174676_user_status.js b/test/server/migrations/1510174676_user_status.js index 8dbbf543f..f9af655d8 100644 --- a/test/server/migrations/1510174676_user_status.js +++ b/test/server/migrations/1510174676_user_status.js @@ -1,12 +1,13 @@ const migration = require('../../../migrations/1510174676_user_status'); const UserModel = require('../../../models/user'); +const helpers = require('../../../services/migration/helpers'); const chai = require('chai'); chai.use(require('chai-datetime')); const { expect } = chai; const performMigration = () => - migration.up({ queryBatchSize: 100, updateBatchSize: 100 }); + migration.up(helpers({ queryBatchSize: 100, updateBatchSize: 100 })); describe('migration.1510174676_user_status', () => { describe('active user', () => { From 75751285febc7fa99b54ee27afbbf0603970b478 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 25 Jan 2018 16:18:05 -0700 Subject: [PATCH 15/19] fixed default setup --- bin/cli-setup | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/bin/cli-setup b/bin/cli-setup index b0f5e2efa..282a8a2d7 100755 --- a/bin/cli-setup +++ b/bin/cli-setup @@ -13,6 +13,7 @@ const MODERATION_OPTIONS = require('../models/enum/moderation_options'); const SettingsService = require('../services/settings'); const SetupService = require('../services/setup'); const UsersService = require('../services/users'); +const MigrationService = require('../services/migration'); const errors = require('../errors'); // Register the shutdown criteria. @@ -51,6 +52,12 @@ const performSetup = async () => { if (program.defaults) { await SettingsService.init(); + // Get the migrations to run. + let migrations = await MigrationService.listPending(); + + // Perform all migrations. + await MigrationService.run(migrations); + console.log('Settings created.'); console.log('\nTalk is now installed!'); @@ -194,7 +201,7 @@ const performSetup = async () => { ); }; -// Start tthe setup process. +// Start the setup process. performSetup() .then(() => { util.shutdown(); From b638b783febc99eb22920628a64749e5eb6e9082 Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 26 Jan 2018 01:42:46 +0100 Subject: [PATCH 16/19] Refactor --- .../routes/Moderation/containers/Indicator.js | 31 +++++-------------- .../Moderation/containers/Moderation.js | 19 +----------- .../src/routes/Moderation/graphql.js | 24 ++++++++++---- .../containers/ModIndicatorSubscription.js | 22 ++----------- .../client/containers/ModSubscription.js | 22 ++----------- 5 files changed, 32 insertions(+), 86 deletions(-) diff --git a/client/coral-admin/src/routes/Moderation/containers/Indicator.js b/client/coral-admin/src/routes/Moderation/containers/Indicator.js index b5620765e..359a2cd5d 100644 --- a/client/coral-admin/src/routes/Moderation/containers/Indicator.js +++ b/client/coral-admin/src/routes/Moderation/containers/Indicator.js @@ -2,7 +2,7 @@ import React, { Component } from 'react'; import { compose, gql } from 'react-apollo'; import Indicator from '../../../components/Indicator'; import { withFragments } from 'plugin-api/beta/client/hocs'; -import { handleIndicatorChange } from '../graphql'; +import { handleIndicatorChange, subscriptionFields } from '../graphql'; import PropTypes from 'prop-types'; import { connect } from 'react-redux'; import withQueueConfig from '../hoc/withQueueConfig'; @@ -131,27 +131,10 @@ IndicatorContainer.propTypes = { queueConfig: PropTypes.object, }; -const fields = ` - status - actions { - __typename - created_at - } - status_history { - type - assigned_by { - id - } - created_at - } - updated_at - created_at -`; - const COMMENT_ADDED_SUBSCRIPTION = gql` subscription TalkAdmin_ModerationIndicator_CommentAdded { commentAdded(statuses: null) { - ${fields} + ${subscriptionFields} } } `; @@ -159,7 +142,7 @@ const COMMENT_ADDED_SUBSCRIPTION = gql` const COMMENT_FLAGGED_SUBSCRIPTION = gql` subscription TalkAdmin_ModerationIndicator_CommentFlagged { commentFlagged { - ${fields} + ${subscriptionFields} } } `; @@ -167,7 +150,7 @@ const COMMENT_FLAGGED_SUBSCRIPTION = gql` const COMMENT_EDITED_SUBSCRIPTION = gql` subscription TalkAdmin_ModerationIndicator_CommentEdited { commentEdited { - ${fields} + ${subscriptionFields} } } `; @@ -175,7 +158,7 @@ const COMMENT_EDITED_SUBSCRIPTION = gql` const COMMENT_ACCEPTED_SUBSCRIPTION = gql` subscription TalkAdmin_ModerationIndicator_CommentAccepted { commentAccepted { - ${fields} + ${subscriptionFields} } } `; @@ -183,7 +166,7 @@ const COMMENT_ACCEPTED_SUBSCRIPTION = gql` const COMMENT_REJECTED_SUBSCRIPTION = gql` subscription TalkAdmin_ModerationIndicator_CommentRejected { commentRejected { - ${fields} + ${subscriptionFields} } } `; @@ -191,7 +174,7 @@ const COMMENT_REJECTED_SUBSCRIPTION = gql` const COMMENT_RESET_SUBSCRIPTION = gql` subscription TalkAdmin_ModerationIndicator_CommentReset { commentReset { - ${fields} + ${subscriptionFields} } } `; diff --git a/client/coral-admin/src/routes/Moderation/containers/Moderation.js b/client/coral-admin/src/routes/Moderation/containers/Moderation.js index c654ad6b8..b5820287b 100644 --- a/client/coral-admin/src/routes/Moderation/containers/Moderation.js +++ b/client/coral-admin/src/routes/Moderation/containers/Moderation.js @@ -15,6 +15,7 @@ import { handleCommentChange, commentBelongToQueue, cleanUpQueue, + subscriptionFields, } from '../graphql'; import { viewUserDetail } from '../../../actions/userDetail'; @@ -339,24 +340,6 @@ class ModerationContainer extends Component { } } -const subscriptionFields = ` - status - actions { - __typename - created_at - } - status_history { - type - created_at - assigned_by { - id - username - } - } - created_at - updated_at -`; - const COMMENT_ADDED_SUBSCRIPTION = gql` subscription CommentAdded($asset_id: ID){ commentAdded(asset_id: $asset_id, statuses: null){ diff --git a/client/coral-admin/src/routes/Moderation/graphql.js b/client/coral-admin/src/routes/Moderation/graphql.js index e113e02e1..44b4f6108 100644 --- a/client/coral-admin/src/routes/Moderation/graphql.js +++ b/client/coral-admin/src/routes/Moderation/graphql.js @@ -113,7 +113,7 @@ function determineLatestChange(comment) { let lc = null; // Skip it when comment itself was not updated. - // We'll get use the one from the status_history instead + // We'll use the one from the status_history instead // as they might diverge a little bit (status_history item is created // before the comment itself) if (comment.createdAt !== comment.updatedAt) { @@ -138,7 +138,6 @@ function reconstructPreviousCommentState(comment) { const history = comment.status_history; const actions = comment.actions; const lastChangeDate = determineLatestChange(comment); - console.log(lastChangeDate); const previousComment = { ...comment, status_history: history.filter( @@ -166,7 +165,6 @@ function reconstructPreviousCommentState(comment) { function getPreviousCommentQueues(comment, queueConfig) { const previousCommentState = reconstructPreviousCommentState(comment); - console.log(previousCommentState, comment); if (!previousCommentState) { return []; } @@ -291,8 +289,6 @@ export function handleCommentChange( // Queues that this comment needs to be placed. const nextQueues = getCommentQueues(comment, queueConfig); - console.log(prevQueues, nextQueues); - let notificationShown = false; const showNotificationOnce = () => { if (notificationShown) { @@ -393,6 +389,22 @@ export function handleIndicatorChange(root, comment, queueConfig) { } } - console.log(prevQueues, nextQueues, next); return next; } + +export const subscriptionFields = ` + status + actions { + __typename + created_at + } + status_history { + type + assigned_by { + id + } + created_at + } + updated_at + created_at +`; diff --git a/plugins/talk-plugin-featured-comments/client/containers/ModIndicatorSubscription.js b/plugins/talk-plugin-featured-comments/client/containers/ModIndicatorSubscription.js index 872fe641c..86ae6bb2c 100644 --- a/plugins/talk-plugin-featured-comments/client/containers/ModIndicatorSubscription.js +++ b/plugins/talk-plugin-featured-comments/client/containers/ModIndicatorSubscription.js @@ -1,5 +1,6 @@ import React from 'react'; import { gql } from 'react-apollo'; +import { subscriptionFields } from 'coral-admin/src/routes/Moderation/graphql'; class ModIndicatorSubscription extends React.Component { subscriptions = null; @@ -39,28 +40,11 @@ class ModIndicatorSubscription extends React.Component { } } -const fields = ` - status - actions { - __typename - created_at - } - status_history { - type - assigned_by { - id - } - created_at - } - updated_at - created_at -`; - const COMMENT_FEATURED_SUBSCRIPTION = gql` subscription TalkFeaturedComments_Indicator_CommentFeatured { commentFeatured { comment { - ${fields} + ${subscriptionFields} } } } @@ -70,7 +54,7 @@ const COMMENT_UNFEATURED_SUBSCRIPTION = gql` subscription TalkFeaturedComments_Indicator_CommentUnfeatured { commentUnfeatured { comment { - ${fields} + ${subscriptionFields} } } } diff --git a/plugins/talk-plugin-featured-comments/client/containers/ModSubscription.js b/plugins/talk-plugin-featured-comments/client/containers/ModSubscription.js index da00e8ba7..6d0fdd811 100644 --- a/plugins/talk-plugin-featured-comments/client/containers/ModSubscription.js +++ b/plugins/talk-plugin-featured-comments/client/containers/ModSubscription.js @@ -5,6 +5,7 @@ import Comment from 'coral-admin/src/routes/Moderation/containers/Comment'; import { getDefinitionName } from 'coral-framework/utils'; import truncate from 'lodash/truncate'; import t from 'coral-framework/services/i18n'; +import { subscriptionFields } from 'coral-admin/src/routes/Moderation/graphql'; function prepareNotificationText(text) { return truncate(text, { length: 50 }).replace('\n', ' '); @@ -74,29 +75,12 @@ class ModSubscription extends React.Component { } } -const fields = ` - status - actions { - __typename - created_at - } - status_history { - type - assigned_by { - id - } - created_at - } - updated_at - created_at -`; - const COMMENT_FEATURED_SUBSCRIPTION = gql` subscription CommentFeatured($assetId: ID){ commentFeatured(asset_id: $assetId) { comment { ...${getDefinitionName(Comment.fragments.comment)} - ${fields} + ${subscriptionFields} } user { id @@ -112,7 +96,7 @@ const COMMENT_UNFEATURED_SUBSCRIPTION = gql` commentUnfeatured(asset_id: $assetId){ comment { ...${getDefinitionName(Comment.fragments.comment)} - ${fields} + ${subscriptionFields} } user { id From 6caae2df9734f46528d34d616064548ee7673a5a Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 25 Jan 2018 18:17:04 -0700 Subject: [PATCH 17/19] applied migration performance improvements --- bin/cli-migration | 5 +- migrations/1516920154_action_counts.js | 136 +++++++++++++------------ migrations/1516920160_reply_counts.js | 31 +++--- services/migration/helpers.js | 35 ++++++- services/migration/index.js | 2 +- 5 files changed, 123 insertions(+), 86 deletions(-) diff --git a/bin/cli-migration b/bin/cli-migration index 32c0cb4dd..63e16f97f 100755 --- a/bin/cli-migration +++ b/bin/cli-migration @@ -28,7 +28,6 @@ async function createMigration(name) { async function runMigrations(options) { const { yes, queryBatchSize, updateBatchSize } = options; - console.log({ yes, queryBatchSize, updateBatchSize }); try { if (!yes) { const { backedUp } = await inquirer.prompt([ @@ -109,13 +108,13 @@ program '-q, --query-batch-size ', 'change the size of queried documents that are batched at a time', parse10, - 100 + 10000 ) .option( '-u, --update-batch-size ', 'change the size of documents that are batched before the update is sent', parse10, - 1000 + 20000 ) .option('-y, --yes', 'will answer yes to all questions') .description('runs all pending migrations') diff --git a/migrations/1516920154_action_counts.js b/migrations/1516920154_action_counts.js index 5aa95614c..7558b5eb0 100644 --- a/migrations/1516920154_action_counts.js +++ b/migrations/1516920154_action_counts.js @@ -9,44 +9,47 @@ module.exports = { { Model: UserModel, item_type: 'USERS' }, ]; for (const { Model, item_type } of models) { - let cursor = ActionModel.collection.aggregate([ - { - $match: { - group_id: { $ne: null }, - item_type, - }, - }, - { - $group: { - // group unique documents by these properties, we are leveraging the - // fact that each uuid is completely unique. - _id: { - item_id: '$item_id', - action_type: '$action_type', - group_id: '$group_id', - }, - - // and sum up all actions matching the above grouping criteria - count: { - $sum: 1, + let cursor = ActionModel.collection.aggregate( + [ + { + $match: { + group_id: { $ne: null }, + item_type, }, }, - }, - { - $project: { - // suppress the _id field - _id: false, + { + $group: { + // group unique documents by these properties, we are leveraging the + // fact that each uuid is completely unique. + _id: { + item_id: '$item_id', + action_type: '$action_type', + group_id: '$group_id', + }, - // map the fields from the _id grouping down a level - item_id: '$_id.item_id', - action_type: { $toLower: '$_id.action_type' }, - group_id: { $toLower: '$_id.group_id' }, - - // map the field directly - count: '$count', + // and sum up all actions matching the above grouping criteria + count: { + $sum: 1, + }, + }, }, - }, - ]); + { + $project: { + // suppress the _id field + _id: false, + + // map the fields from the _id grouping down a level + item_id: '$_id.item_id', + action_type: { $toLower: '$_id.action_type' }, + group_id: { $toLower: '$_id.group_id' }, + + // map the field directly + count: '$count', + }, + }, + ], + { allowDiskUse: true } + ); // Transform those documents. await transformSingleWithCursor( @@ -64,41 +67,44 @@ module.exports = { // Secondly, we'll collect the group group id's (all the actions for a // specific action type) to update counts of. - cursor = ActionModel.collection.aggregate([ - { - $match: { - item_type, - }, - }, - { - $group: { - // group unique documents by these properties, we are leveraging the - // fact that each uuid is completely unique. - _id: { - item_id: '$item_id', - action_type: '$action_type', - }, - - // and sum up all actions matching the above grouping criteria - count: { - $sum: 1, + cursor = ActionModel.collection.aggregate( + [ + { + $match: { + item_type, }, }, - }, - { - $project: { - // suppress the _id field - _id: false, + { + $group: { + // group unique documents by these properties, we are leveraging the + // fact that each uuid is completely unique. + _id: { + item_id: '$item_id', + action_type: '$action_type', + }, - // map the fields from the _id grouping down a level - item_id: '$_id.item_id', - action_type: { $toLower: '$_id.action_type' }, - - // map the field directly - count: '$count', + // and sum up all actions matching the above grouping criteria + count: { + $sum: 1, + }, + }, }, - }, - ]); + { + $project: { + // suppress the _id field + _id: false, + + // map the fields from the _id grouping down a level + item_id: '$_id.item_id', + action_type: { $toLower: '$_id.action_type' }, + + // map the field directly + count: '$count', + }, + }, + ], + { allowDiskUse: true } + ); // Transform those documents. await transformSingleWithCursor( diff --git a/migrations/1516920160_reply_counts.js b/migrations/1516920160_reply_counts.js index 7dd18e8f8..fc81bd912 100644 --- a/migrations/1516920160_reply_counts.js +++ b/migrations/1516920160_reply_counts.js @@ -7,22 +7,25 @@ const transformComments = ({ _id: parent_id, reply_count }) => ({ module.exports = { async up({ transformSingleWithCursor }) { - const cursor = CommentModel.collection.aggregate([ - { - $match: { - parent_id: { $ne: null }, - status: { $in: ['NONE', 'ACCEPTED'] }, - }, - }, - { - $group: { - _id: '$parent_id', - reply_count: { - $sum: 1, + const cursor = CommentModel.collection.aggregate( + [ + { + $match: { + parent_id: { $ne: null }, + status: { $in: ['NONE', 'ACCEPTED'] }, }, }, - }, - ]); + { + $group: { + _id: '$parent_id', + reply_count: { + $sum: 1, + }, + }, + }, + ], + { allowDiskUse: true } + ); // Transform those documents. await transformSingleWithCursor(cursor, transformComments, CommentModel); diff --git a/services/migration/helpers.js b/services/migration/helpers.js index 38df5c128..f2d959ca4 100644 --- a/services/migration/helpers.js +++ b/services/migration/helpers.js @@ -1,3 +1,5 @@ +const debug = require('debug')('talk:services:migration'); + /** * processUpdates processes batches of updates on the given model. * @@ -16,16 +18,37 @@ const processUpdates = async (model, updates) => { await bulk.execute(); }; +const debugProcessStatistics = (count, totalCount) => { + if (totalCount > 0) { + debug( + `processed ${(count / totalCount * 100).toFixed( + 2 + )}% (${count}/${totalCount}) update` + ); + } else { + debug(`processed ${count} updates`); + } +}; + const transformSingleWithCursor = ({ queryBatchSize, updateBatchSize, -}) => async (cursor, process, Model) => { +}) => async (query, process, Model) => { + debug('starting transform'); + // We'll manage the updates that we store inside this object. let updates = []; - // First we'll collect all the individual actions with specific group id's. - cursor = await cursor.batchSize(queryBatchSize); + // Count the elements in the transformation. + let totalCount = 0; + try { + totalCount = await query.count(); + } catch (err) {} + // First we'll collect all the individual actions with specific group id's. + const cursor = await query.batchSize(queryBatchSize); + + let count = 0; while (await cursor.hasNext()) { const result = await cursor.next(); @@ -37,6 +60,8 @@ const transformSingleWithCursor = ({ if (updates.length > updateBatchSize) { // Process the updates. await processUpdates(Model, updates); + count += updates.length; + debugProcessStatistics(count, totalCount); // Clear the updates array. updates = []; @@ -46,10 +71,14 @@ const transformSingleWithCursor = ({ if (updates.length > 0) { // Process the updates. await processUpdates(Model, updates); + count += updates.length; + debugProcessStatistics(count, totalCount); // Clear the updates array. updates = []; } + + debug('finished transform'); }; /** diff --git a/services/migration/index.js b/services/migration/index.js index 312282666..e0da3cddf 100644 --- a/services/migration/index.js +++ b/services/migration/index.js @@ -115,7 +115,7 @@ class MigrationService { */ static async run( migrations, - { queryBatchSize = 100, updateBatchSize = 1000 } = {} + { queryBatchSize = 10000, updateBatchSize = 20000 } = {} ) { if (migrations.length === 0) { console.log('No migrations to run!'); From 57a77a7edff08a0cb60ef74de23e4c127048d9d0 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 25 Jan 2018 19:21:00 -0700 Subject: [PATCH 18/19] enhanced flag migration --- migrations/1496771633_tags.js | 29 ++++++++++++++++------------- services/migration/helpers.js | 2 +- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/migrations/1496771633_tags.js b/migrations/1496771633_tags.js index a7155e2d1..6893a5c47 100644 --- a/migrations/1496771633_tags.js +++ b/migrations/1496771633_tags.js @@ -53,22 +53,25 @@ const transformTags = ({ id, tags }) => ({ module.exports = { async up({ transformSingleWithCursor }) { // Find all comments that have tags. - const cursor = CommentModel.collection.aggregate([ - { - $match: { - tags: { - $exists: true, - $ne: [], + const cursor = CommentModel.collection.aggregate( + [ + { + $match: { + tags: { + $exists: true, + $ne: [], + }, }, }, - }, - { - $project: { - id: true, - tags: true, + { + $project: { + id: true, + tags: true, + }, }, - }, - ]); + ], + { allowDiskUse: true } + ); await transformSingleWithCursor(cursor, transformTags, CommentModel); }, diff --git a/services/migration/helpers.js b/services/migration/helpers.js index f2d959ca4..5eb33aadf 100644 --- a/services/migration/helpers.js +++ b/services/migration/helpers.js @@ -23,7 +23,7 @@ const debugProcessStatistics = (count, totalCount) => { debug( `processed ${(count / totalCount * 100).toFixed( 2 - )}% (${count}/${totalCount}) update` + )}% (${count}/${totalCount}) updates` ); } else { debug(`processed ${count} updates`); From 3fa2af975c327ec21832d80489a91356911b3f6b Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Fri, 26 Jan 2018 17:28:21 +0100 Subject: [PATCH 19/19] Bug fixes --- client/coral-admin/src/containers/Header.js | 9 +++++++-- .../src/routes/Moderation/containers/Indicator.js | 5 ++++- client/coral-admin/src/routes/Moderation/graphql.js | 11 ----------- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/client/coral-admin/src/containers/Header.js b/client/coral-admin/src/containers/Header.js index dd16c17c6..d3be66764 100644 --- a/client/coral-admin/src/containers/Header.js +++ b/client/coral-admin/src/containers/Header.js @@ -7,11 +7,16 @@ import { getDefinitionName } from 'coral-framework/utils'; export default withQuery( gql` - query TalkAdmin_Header { + query TalkAdmin_Header($nullID: ID) { ...${getDefinitionName(ModerationIndicator.fragments.root)} ...${getDefinitionName(CommunityIndicator.fragments.root)} } ${ModerationIndicator.fragments.root} ${CommunityIndicator.fragments.root} - ` + `, + { + options: { + variables: { nullID: null }, + }, + } )(Header); diff --git a/client/coral-admin/src/routes/Moderation/containers/Indicator.js b/client/coral-admin/src/routes/Moderation/containers/Indicator.js index 359a2cd5d..1475b6e2d 100644 --- a/client/coral-admin/src/routes/Moderation/containers/Indicator.js +++ b/client/coral-admin/src/routes/Moderation/containers/Indicator.js @@ -188,11 +188,14 @@ const enhance = compose( withFragments({ root: gql` fragment TalkAdmin_Moderation_Indicator_root on RootQuery { - premodCount: commentCount(query: { statuses: [PREMOD] }) + premodCount: commentCount( + query: { statuses: [PREMOD], asset_id: $nullID } + ) reportedCount: commentCount( query: { statuses: [NONE, PREMOD, SYSTEM_WITHHELD] action_type: FLAG + asset_id: $nullID } ) } diff --git a/client/coral-admin/src/routes/Moderation/graphql.js b/client/coral-admin/src/routes/Moderation/graphql.js index 44b4f6108..c542b8f84 100644 --- a/client/coral-admin/src/routes/Moderation/graphql.js +++ b/client/coral-admin/src/routes/Moderation/graphql.js @@ -112,23 +112,12 @@ function getOlderDate(a, b) { function determineLatestChange(comment) { let lc = null; - // Skip it when comment itself was not updated. - // We'll use the one from the status_history instead - // as they might diverge a little bit (status_history item is created - // before the comment itself) - if (comment.createdAt !== comment.updatedAt) { - lc = getOlderDate(lc, comment.createdAt); - lc = getOlderDate(lc, comment.updatedAt); - } - comment.status_history.forEach(item => { lc = getOlderDate(lc, item.created_at); - lc = getOlderDate(lc, item.updated_at); }); comment.actions.forEach(item => { lc = getOlderDate(lc, item.created_at); - lc = getOlderDate(lc, item.updated_at); }); return lc;