From ed27209de98e7ccc1b4765a89383e1f5c19b012d Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 12 May 2017 10:09:25 -0600 Subject: [PATCH] More edit changes --- graph/loaders/metrics.js | 254 +++++++++++----------- graph/mutators/comment.js | 1 + graph/resolvers/asset.js | 26 ++- graph/resolvers/comment.js | 10 +- graph/resolvers/root_query.js | 43 +--- graph/resolvers/user.js | 15 ++ graph/typeDefs.graphql | 6 +- models/comment.js | 11 + services/comments.js | 3 +- test/server/graph/mutations/ignoreUser.js | 45 ++-- 10 files changed, 218 insertions(+), 196 deletions(-) diff --git a/graph/loaders/metrics.js b/graph/loaders/metrics.js index a0408ed4d..e91a1adf4 100644 --- a/graph/loaders/metrics.js +++ b/graph/loaders/metrics.js @@ -54,166 +54,158 @@ const getAssetActivityMetrics = ({loaders: {Assets}}, {from, to, limit}) => { /** * Returns a list of assets with action metadata included on the models. */ -const getAssetMetrics = ({loaders: {Metrics, Assets, Comments}}, {from, to, sort, limit}) => { +const getAssetMetrics = async ({loaders: {Metrics, Assets, Comments}}, {from, to, sort, limit}) => { - let commentMetrics = {}; - let assetMetrics = []; + // Get the recent actions. + let actionSummaries = await Metrics.getRecentActions.load({from, to}); - return Metrics.getRecentActions.load({from, to}) - .then((actionSummaries) => { + let commentMetrics = actionSummaries.reduce((acc, {item_id, action_type, count}) => { + if (action_type !== sort) { + return acc; + } - commentMetrics = actionSummaries.reduce((acc, {item_id, action_type, count}) => { - if (action_type !== sort) { - return acc; - } + if (!(item_id in acc)) { + acc[item_id] = []; + } - if (!(item_id in acc)) { - acc[item_id] = []; - } + acc[item_id].push({action_type, count}); - acc[item_id].push({action_type, count}); + return acc; + }, {}); - return acc; - }, {}); + // Collect just the comment id's. + let commentIDs = Object.keys(commentMetrics); - // Collect just the comment id's. - let commentIDs = Object.keys(commentMetrics); + // Find those comments. + let comments = await Comments.get.loadMany(commentIDs); - // Find those comments. - return Comments.get.loadMany(commentIDs); + let commentResults = _.groupBy(comments, 'asset_id'); + + let assetMetrics = Object.keys(commentResults) + .map((asset_id) => { + let ids = commentResults[asset_id].map((comment) => comment.id); + let summaries = _.groupBy(_.flatten(ids.map((id) => commentMetrics[id])), 'action_type'); + + let action_summaries = Object.keys(summaries).map((action_type) => ({ + action_type, + actionCount: summaries[action_type].reduce((acc, {count}) => acc + count, 0), + actionableItemCount: summaries[action_type].length + })); + + return {action_summaries, id: asset_id}; }) - .then((comments) => { + + .filter((asset) => { + let contextActionSummary = asset.action_summaries.find((({action_type}) => action_type === sort)); + if (contextActionSummary === null || contextActionSummary.actionCount === 0) { + return false; + } - let commentResults = _.groupBy(comments, 'asset_id'); - - assetMetrics = Object.keys(commentResults).map((asset_id) => { - let ids = commentResults[asset_id].map((comment) => comment.id); - let summaries = _.groupBy(_.flatten(ids.map((id) => commentMetrics[id])), 'action_type'); - - let action_summaries = Object.keys(summaries).map((action_type) => ({ - action_type, - actionCount: summaries[action_type].reduce((acc, {count}) => acc + count, 0), - actionableItemCount: summaries[action_type].length - })); - - return {action_summaries, id: asset_id}; - }) - - .filter((asset) => { - let contextActionSummary = asset.action_summaries.find((({action_type}) => action_type === sort)); - if (contextActionSummary === null || contextActionSummary.actionCount === 0) { - return false; - } - - return true; - }) - - // Sort these metrics by the predefined sort order. This will ensure that - // if the action summary does not exist on the object, that it is less - // prefered over the one that does have it. - .sort((a, b) => { - let aActionSummary = a.action_summaries.find((({action_type}) => action_type === sort)); - let bActionSummary = b.action_summaries.find((({action_type}) => action_type === sort)); - - // Both of them had an actionCount, hence we can determine that we could - // compare the actual values directly. - return bActionSummary.actionCount - aActionSummary.actionCount; - }); - - // Only keep the top `limit`. - assetMetrics = assetMetrics.slice(0, limit); - - // Determine the assets that we need to return. - return Assets.getByID.loadMany(assetMetrics.map((asset) => asset.id)); + return true; }) - .then((assets) => { - // Join up the assets that are returned by their id. - let groupedAssets = _.groupBy(assets, 'id'); + // Sort these metrics by the predefined sort order. This will ensure that + // if the action summary does not exist on the object, that it is less + // prefered over the one that does have it. + .sort((a, b) => { + let aActionSummary = a.action_summaries.find((({action_type}) => action_type === sort)); + let bActionSummary = b.action_summaries.find((({action_type}) => action_type === sort)); - // Return from the sorted asset metrics and return their assetes. - return assetMetrics.map(({id, action_summaries}) => { - if (id in groupedAssets) { - let asset = groupedAssets[id][0]; - - // Add the action summaries to the asset. - asset.action_summaries = action_summaries; - - return asset; - } - - return null; - }).filter((asset) => asset != null); + // Both of them had an actionCount, hence we can determine that we could + // compare the actual values directly. + return bActionSummary.actionCount - aActionSummary.actionCount; }); + + // Only keep the top `limit`. + assetMetrics = assetMetrics.slice(0, limit); + + // Determine the assets that we need to return. + let assets = await Assets.getByID.loadMany(assetMetrics.map((asset) => asset.id)); + + // Join up the assets that are returned by their id. + let groupedAssets = _.groupBy(assets, 'id'); + + // Return from the sorted asset metrics and return their assetes. + return assetMetrics.map(({id, action_summaries}) => { + if (id in groupedAssets) { + let asset = groupedAssets[id][0]; + + // Add the action summaries to the asset. + asset.action_summaries = action_summaries; + + return asset; + } + + return null; + }).filter((asset) => asset != null); }; /** * Returns a list of comments that are retrieved based on most activity within * the indicated time range. */ -const getCommentMetrics = ({loaders: {Metrics, Comments}}, {from, to, sort, limit}) => { +const getCommentMetrics = async ({loaders: {Metrics, Comments}}, {from, to, sort, limit}) => { let commentActionSummaries = {}; - return Metrics.getRecentActions.load({from, to}) - .then((actionSummaries) => { + let actionSummaries = await Metrics.getRecentActions.load({from, to}); - actionSummaries.sort((a, b) => { - let aActionSummary = a.action_type === sort ? a : null; - let bActionSummary = b.action_type === sort ? b : null; + actionSummaries.sort((a, b) => { + let aActionSummary = a.action_type === sort ? a : null; + let bActionSummary = b.action_type === sort ? b : null; - // If either a or b don't have this action type, then one of them will - // automatically win. - if (aActionSummary == null || bActionSummary == null) { - if (bActionSummary != null) { - return 1; - } - - if (aActionSummary != null) { - return -1; - } - - return 0; - } - - // Both of them had an actionCount, hence we can determine that we could - // compare the actual values directly. - return bActionSummary.count - aActionSummary.count; - }); - - commentActionSummaries = _.groupBy(actionSummaries, 'item_id'); - - // Grab the comment id's for comment where they have at least one of the - // actions being sorted by. - let commentIDs = Object.keys(commentActionSummaries).filter((item_id) => { - let contextActionSummary = commentActionSummaries[item_id].find(({action_type}) => action_type === sort); - if (contextActionSummary == null) { - return false; - } - - return true; - }); - - // Only keep the top `limit`. - commentIDs = commentIDs.slice(0, limit); - - // If there are no comment's to get, then just continue with an empty - // array. - if (commentIDs.length === 0) { - return []; + // If either a or b don't have this action type, then one of them will + // automatically win. + if (aActionSummary == null || bActionSummary == null) { + if (bActionSummary != null) { + return 1; } - // Find those comments, this is the final stage, so let's get all the - // fields. - return Comments.get.loadMany(commentIDs); - }) - .then((comments) => comments.map((comment) => { + if (aActionSummary != null) { + return -1; + } - // Add in the action summaries genrerated. - comment.action_summaries = commentActionSummaries[comment.id]; + return 0; + } - return comment; - })); + // Both of them had an actionCount, hence we can determine that we could + // compare the actual values directly. + return bActionSummary.count - aActionSummary.count; + }); + + commentActionSummaries = _.groupBy(actionSummaries, 'item_id'); + + // Grab the comment id's for comment where they have at least one of the + // actions being sorted by. + let commentIDs = Object.keys(commentActionSummaries).filter((item_id) => { + let contextActionSummary = commentActionSummaries[item_id].find(({action_type}) => action_type === sort); + if (contextActionSummary == null) { + return false; + } + + return true; + }); + + // Only keep the top `limit`. + commentIDs = commentIDs.slice(0, limit); + + // If there are no comment's to get, then just continue with an empty + // array. + if (commentIDs.length === 0) { + return []; + } + + // Find those comments, this is the final stage, so let's get all the + // fields. + let comments = await Comments.get.loadMany(commentIDs); + + return comments.map((comment) => { + + // Add in the action summaries genrerated. + comment.action_summaries = commentActionSummaries[comment.id]; + + return comment; + }); }; const getRecentActions = (context, {from, to}) => { diff --git a/graph/mutators/comment.js b/graph/mutators/comment.js index 89ecfc026..aa113e54b 100644 --- a/graph/mutators/comment.js +++ b/graph/mutators/comment.js @@ -223,6 +223,7 @@ const edit = async (context, {id, asset_id, edit: {body}}) => { // Determine the new status of the comment. const status = await resolveNewCommentStatus(context, {asset_id, body}, wordlist, settings); + // Execute the edit. await CommentsService.edit(id, context.user.id, {body, status}); return {status}; diff --git a/graph/resolvers/asset.js b/graph/resolvers/asset.js index 96bc8c1bc..41710ba35 100644 --- a/graph/resolvers/asset.js +++ b/graph/resolvers/asset.js @@ -12,6 +12,8 @@ const Asset = { }); }, commentCount({id, commentCount}, {excludeIgnored}, {user, loaders: {Comments}}) { + + // TODO: remove if (user && excludeIgnored) { return Comments.parentCountByAssetIDPersonalized({assetId: id, excludeIgnored}); } @@ -21,6 +23,8 @@ const Asset = { return Comments.parentCountByAssetID.load(id); }, totalCommentCount({id, totalCommentCount}, {excludeIgnored}, {user, loaders: {Comments}}) { + + // TODO: remove if (user && excludeIgnored) { return Comments.countByAssetIDPersonalized({assetId: id, excludeIgnored}); } @@ -29,16 +33,18 @@ const Asset = { } return Comments.countByAssetID.load(id); }, - settings({settings = null}, _, {loaders: {Settings}}) { - return Settings.load() - .then((globalSettings) => { - if (settings) { - settings = Object.assign({}, globalSettings.toObject(), settings); - } else { - settings = globalSettings.toObject(); - } - return settings; - }); + async settings({settings = null}, _, {loaders: {Settings}}) { + + // Load the global settings, and merge them into the asset specific settings + // if we have some. + let globalSettings = await Settings.load(); + if (settings !== null) { + settings = Object.assign({}, globalSettings.toObject(), settings); + } else { + settings = globalSettings.toObject(); + } + + return settings; } }; diff --git a/graph/resolvers/comment.js b/graph/resolvers/comment.js index b52361f40..8d0473900 100644 --- a/graph/resolvers/comment.js +++ b/graph/resolvers/comment.js @@ -1,5 +1,3 @@ -const CommentsService = require('../../services/comments'); - const Comment = { parent({parent_id}, _, {loaders: {Comments}}) { if (parent_id == null) { @@ -24,6 +22,8 @@ const Comment = { }); }, replyCount({id}, {excludeIgnored}, {user, loaders: {Comments}}) { + + // TODO: remove if (user && excludeIgnored) { return Comments.countByParentIDPersonalized({id, excludeIgnored}); } @@ -49,11 +49,9 @@ const Comment = { return Assets.getByID.load(asset_id); }, editing(comment) { - const editableUntil = CommentsService.getEditableUntilDate(comment); - const edited = comment.body_history.length > 1; return { - edited, - editableUntil, + edited: comment.edited, + editableUntil: comment.editableUntil }; } }; diff --git a/graph/resolvers/root_query.js b/graph/resolvers/root_query.js index 7faa8cb1d..ad4ed4b0d 100644 --- a/graph/resolvers/root_query.js +++ b/graph/resolvers/root_query.js @@ -19,16 +19,14 @@ const RootQuery = { // This endpoint is used for loading moderation queues, so hide it in the // event that we aren't an admin. - comments(_, {query: {action_type, statuses, asset_id, parent_id, limit, cursor, sort, excludeIgnored}}, {user, loaders: {Comments, Actions}}) { + async comments(_, {query: {action_type, statuses, asset_id, parent_id, limit, cursor, sort, excludeIgnored}}, {user, loaders: {Comments, Actions}}) { let query = {statuses, asset_id, parent_id, limit, cursor, sort, excludeIgnored}; if (user != null && user.hasRoles('ADMIN') && action_type) { - return Actions.getByTypes({action_type, item_type: 'COMMENTS'}) - .then((ids) => { + let ids = await Actions.getByTypes({action_type, item_type: 'COMMENTS'}); - // Perform the query using the available resolver. - return Comments.getByQuery({ids, statuses, asset_id, parent_id, limit, cursor, sort, excludeIgnored}); - }); + // Perform the query using the available resolver. + return Comments.getByQuery({ids, statuses, asset_id, parent_id, limit, cursor, sort, excludeIgnored}); } return Comments.getByQuery(query); @@ -36,18 +34,16 @@ const RootQuery = { comment(_, {id}, {loaders: {Comments}}) { return Comments.get.load(id); }, - commentCount(_, {query: {action_type, statuses, asset_id, parent_id}}, {user, loaders: {Actions, Comments}}) { + async commentCount(_, {query: {action_type, statuses, asset_id, parent_id}}, {user, loaders: {Actions, Comments}}) { if (user == null || !user.hasRoles('ADMIN')) { return null; } if (action_type) { - return Actions.getByTypes({action_type, item_type: 'COMMENTS'}) - .then((ids) => { + let ids = await Actions.getByTypes({action_type, item_type: 'COMMENTS'}); - // Perform the query using the available resolver. - return Comments.getCountByQuery({ids, statuses, asset_id, parent_id}); - }); + // Perform the query using the available resolver. + return Comments.getCountByQuery({ids, statuses, asset_id, parent_id}); } return Comments.getCountByQuery({statuses, asset_id, parent_id}); @@ -83,22 +79,9 @@ const RootQuery = { return user; }, - myIgnoredUsers: async (_, args, {user, loaders: {Users}}) => { - if (!user) { - return null; - } - - // get currentUser again since context.user was out of date when running test/graph/mutations/ignoreUser - const currentUser = (await Users.getByQuery({ids: [user.id], limit: 1}))[0]; - if (!(currentUser && Array.isArray(currentUser.ignoresUsers) && currentUser.ignoresUsers.length)) { - return []; - } - return await Users.getByQuery({ids: currentUser.ignoresUsers}); - }, - // This endpoint is used for loading the user moderation queues (users whose username has been flagged), // so hide it in the event that we aren't an admin. - users(_, {query: {action_type, limit, cursor, sort}}, {user, loaders: {Users, Actions}}) { + async users(_, {query: {action_type, limit, cursor, sort}}, {user, loaders: {Users, Actions}}) { if (user == null || !user.hasRoles('ADMIN')) { return null; @@ -107,12 +90,10 @@ const RootQuery = { const query = {limit, cursor, sort}; if (action_type) { - return Actions.getByTypes({action_type, item_type: 'USERS'}) - .then((ids) => { + let ids = await Actions.getByTypes({action_type, item_type: 'USERS'}); - // Perform the query using the available resolver. - return Users.getByQuery({ids, limit, cursor, sort}).find({status: 'PENDING'}); - }); + // Perform the query using the available resolver. + return Users.getByQuery({ids, limit, cursor, sort}).find({status: 'PENDING'}); } return Users.getByQuery(query); diff --git a/graph/resolvers/user.js b/graph/resolvers/user.js index d8ed7ee15..9b25c812f 100644 --- a/graph/resolvers/user.js +++ b/graph/resolvers/user.js @@ -20,6 +20,21 @@ const User = { return null; }, + ignoredUsers({id}, args, {user, loaders: {Users}}) { + + // Only allow a logged in user that is either the current user or is a staff + // member to access the ignoredUsers of a given user. + if (!user || ((user.id !== id) && !(user.hasRoles('ADMIN') || user.hasRoles('MODERATOR')))) { + return null; + } + + // Return nothing if there is nothing to query for. + if (!user.ignoresUsers || user.ignoresUsers.length <= 0) { + return []; + } + + return Users.getByQuery({ids: user.ignoresUsers}); + }, roles({id, roles}, _, {user}) { // If the user is not an admin, only return the current user's roles. diff --git a/graph/typeDefs.graphql b/graph/typeDefs.graphql index 40fc2c189..f51258eb1 100644 --- a/graph/typeDefs.graphql +++ b/graph/typeDefs.graphql @@ -42,6 +42,9 @@ type User { # determines whether the user can edit their username canEditName: Boolean + # ignored users. + ignoredUsers: [User!] + # returns all comments based on a query. comments(query: CommentsQuery): [Comment!] @@ -543,9 +546,6 @@ type RootQuery { # role. me: User - # Users that the currently logged in user ignores - myIgnoredUsers: [User] - # Users returned based on a query. users(query: UsersQuery): [User] diff --git a/models/comment.js b/models/comment.js index 68a8f7526..ddd39c68e 100644 --- a/models/comment.js +++ b/models/comment.js @@ -2,6 +2,8 @@ const mongoose = require('../services/mongoose'); const Schema = mongoose.Schema; const uuid = require('uuid'); +const EDIT_WINDOW_MS = 30 * 1000; // 30 seconds + const STATUSES = [ 'ACCEPTED', 'REJECTED', @@ -107,7 +109,16 @@ const CommentSchema = new Schema({ } }); +CommentSchema.virtual('edited').get(function() { + return this.body_history.length > 1; +}); + +CommentSchema.virtual('editableUntil').get(function() { + return new Date(Number(this.created_at) + EDIT_WINDOW_MS); +}); + // Comment model. const Comment = mongoose.model('Comment', CommentSchema); module.exports = Comment; +module.exports.EDIT_WINDOW_MS = EDIT_WINDOW_MS; diff --git a/services/comments.js b/services/comments.js index 25e5cc528..272b19cd0 100644 --- a/services/comments.js +++ b/services/comments.js @@ -1,12 +1,11 @@ const CommentModel = require('../models/comment'); +const EDIT_WINDOW_MS = CommentModel.EDIT_WINDOW_MS; const ActionModel = require('../models/action'); const ActionsService = require('./actions'); const errors = require('../errors'); -const EDIT_WINDOW_MS = 30 * 1000; // 30 seconds - module.exports = class CommentsService { /** diff --git a/test/server/graph/mutations/ignoreUser.js b/test/server/graph/mutations/ignoreUser.js index c4b27d31d..256fd732b 100644 --- a/test/server/graph/mutations/ignoreUser.js +++ b/test/server/graph/mutations/ignoreUser.js @@ -18,9 +18,11 @@ const ignoreUserMutation = ` const getMyIgnoredUsersQuery = ` query myIgnoredUsers { - myIgnoredUsers { - id, - username + me { + ignoredUsers { + id + username + } } } `; @@ -31,25 +33,32 @@ describe('graph.mutations.ignoreUser', () => { }); it('users can ignoreUser', async () => { - const user = await UsersService.createLocalUser('usernameA@example.com', 'password', 'usernameA'); + let currentUser = await UsersService.createLocalUser('usernameA@example.com', 'password', 'usernameA'); const userToIgnore = await UsersService.createLocalUser('usernameB@example.com', 'password', 'usernameB'); - const context = new Context({user}); + let context = new Context({user: currentUser}); + const ignoreUserResponse = await graphql(schema, ignoreUserMutation, {}, context, {id: userToIgnore.id}); if (ignoreUserResponse.errors && ignoreUserResponse.errors.length) { console.error(ignoreUserResponse.errors); } expect(ignoreUserResponse.errors).to.be.empty; + // Refresh the user from the database and create the new context for the + // request. + currentUser = await UsersService.findById(currentUser.id); + context = new Context({user: currentUser}); + // now check my ignored users const myIgnoredUsersResponse = await graphql(schema, getMyIgnoredUsersQuery, {}, context, {}); if (myIgnoredUsersResponse.errors && myIgnoredUsersResponse.errors.length) { console.error(myIgnoredUsersResponse.errors); } expect(myIgnoredUsersResponse.errors).to.be.empty; - const myIgnoredUsers = myIgnoredUsersResponse.data.myIgnoredUsers; - expect(myIgnoredUsers.length).to.equal(1); - expect(myIgnoredUsers[0].id).to.equal(userToIgnore.id); - expect(myIgnoredUsers[0].username).to.equal(userToIgnore.username); + + const ignoredUsers = myIgnoredUsersResponse.data.me.ignoredUsers; + expect(ignoredUsers.length).to.equal(1); + expect(ignoredUsers[0].id).to.equal(userToIgnore.id); + expect(ignoredUsers[0].username).to.equal(userToIgnore.username); }); it('users cannot ignore themselves', async () => { @@ -64,7 +73,7 @@ describe('graph.mutations.ignoreUser', () => { console.error(myIgnoredUsersResponse.errors); } expect(myIgnoredUsersResponse.errors).to.be.empty; - const myIgnoredUsers = myIgnoredUsersResponse.data.myIgnoredUsers; + const myIgnoredUsers = myIgnoredUsersResponse.data.me.ignoredUsers; expect(myIgnoredUsers.length).to.equal(0); }); @@ -80,12 +89,12 @@ describe('graph.mutations.stopIgnoringUser', () => { // We're going to ignore 2 users, // then stopIgnoring 1 of them // then assert myIgnoredUsers only lists the one remaining - const user = await UsersService.createLocalUser('usernameA@example.com', 'password', 'usernameA'); + let currentUser = await UsersService.createLocalUser('usernameA@example.com', 'password', 'usernameA'); const usersToIgnore = await Promise.all([ UsersService.createLocalUser('usernameB@example.com', 'password', 'usernameB'), UsersService.createLocalUser('usernameC@example.com', 'password', 'usernameC'), ]); - const context = new Context({user}); + let context = new Context({user: currentUser}); // ignore two users const ignoreUserResponses = await Promise.all(usersToIgnore.map((u) => graphql(schema, ignoreUserMutation, {}, context, {id: u.id}))); @@ -96,6 +105,11 @@ describe('graph.mutations.stopIgnoringUser', () => { expect(response.errors).to.be.empty; }); + // Refresh the user from the database and create the new context for the + // request. + currentUser = await UsersService.findById(currentUser.id); + context = new Context({user: currentUser}); + const stopIgnoringUserMutation = ` mutation stopIgnoringUser ($id: ID!) { stopIgnoringUser(id:$id) { @@ -113,13 +127,18 @@ describe('graph.mutations.stopIgnoringUser', () => { } expect(stopIgnoringUserResponse.errors).to.be.empty; + // Refresh the user from the database and create the new context for the + // request. + currentUser = await UsersService.findById(currentUser.id); + context = new Context({user: currentUser}); + // now check my ignored users const myIgnoredUsersResponse = await graphql(schema, getMyIgnoredUsersQuery, {}, context, {}); if (myIgnoredUsersResponse.errors && myIgnoredUsersResponse.errors.length) { console.error(myIgnoredUsersResponse.errors); } expect(myIgnoredUsersResponse.errors).to.be.empty; - const myIgnoredUsers = myIgnoredUsersResponse.data.myIgnoredUsers; + const myIgnoredUsers = myIgnoredUsersResponse.data.me.ignoredUsers; expect(myIgnoredUsers.length).to.equal(1); expect(myIgnoredUsers[0].id).to.equal(usersToIgnore[1].id); expect(myIgnoredUsers[0].username).to.equal(usersToIgnore[1].username);