diff --git a/client/coral-embed-stream/src/containers/Comment.js b/client/coral-embed-stream/src/containers/Comment.js index 005a24ae1..bbf43bd4a 100644 --- a/client/coral-embed-stream/src/containers/Comment.js +++ b/client/coral-embed-stream/src/containers/Comment.js @@ -103,7 +103,7 @@ const withCommentFragments = withFragments({ fragment CoralEmbedStream_Comment_comment on Comment { ...CoralEmbedStream_Comment_SingleComment ${nest(` - replies(limit: 3, excludeIgnored: $excludeIgnored) { + replies(query: {limit: 3, excludeIgnored: $excludeIgnored}) { nodes { ...CoralEmbedStream_Comment_SingleComment ...nest diff --git a/client/coral-embed-stream/src/containers/Stream.js b/client/coral-embed-stream/src/containers/Stream.js index be6c000e0..8b02e793f 100644 --- a/client/coral-embed-stream/src/containers/Stream.js +++ b/client/coral-embed-stream/src/containers/Stream.js @@ -253,9 +253,9 @@ const fragments = { charCount requireEmailConfirmation } - commentCount(excludeIgnored: $excludeIgnored) @skip(if: $hasComment) - totalCommentCount(excludeIgnored: $excludeIgnored) @skip(if: $hasComment) - comments(limit: 10, excludeIgnored: $excludeIgnored) @skip(if: $hasComment) { + commentCount @skip(if: $hasComment) + totalCommentCount @skip(if: $hasComment) + comments(query: {limit: 10, excludeIgnored: $excludeIgnored}) @skip(if: $hasComment) { nodes { ...CoralEmbedStream_Stream_comment } diff --git a/graph/loaders/comments.js b/graph/loaders/comments.js index 0df73e970..56b389528 100644 --- a/graph/loaders/comments.js +++ b/graph/loaders/comments.js @@ -1,7 +1,6 @@ const { SharedCounterDataLoader, singleJoinBy, - arrayJoinBy } = require('./util'); const DataLoader = require('dataloader'); const { @@ -14,7 +13,6 @@ const { const ms = require('ms'); const CommentModel = require('../../models/comment'); -const UsersService = require('../../services/users'); /** * Returns the comment count for all comments that are public based on their @@ -48,38 +46,6 @@ const getCountsByAssetID = (context, asset_ids) => { .then((results) => results.map((result) => result ? result.count : 0)); }; -/** - * Returns the count of all public comments on an asset id, also filtering by personalization options. - * - * @param {Array} id The ID of the asset - * @param {Array} excludeIgnored Exclude comments ignored by the requesting user - */ -const getCountsByAssetIDPersonalized = async (context, {assetId, excludeIgnored, tags}) => { - const query = { - asset_id: assetId, - status: { - $in: ['NONE', 'ACCEPTED'], - }, - }; - - if (tags) { - query['tags.tag.name'] = { - $in: tags, - }; - } - - const user = context.user; - if (excludeIgnored && user) { - - // load afresh, as `user` may be from cache and not have recent ignores - const freshUser = await UsersService.findById(user.id); - const ignoredUsers = freshUser.ignoresUsers; - query.author_id = {$nin: ignoredUsers}; - } - const count = await CommentModel.where(query).count(); - return count; -}; - /** * Returns the comment count for all comments that are public based on their * asset ids. @@ -113,39 +79,6 @@ const getParentCountsByAssetID = (context, asset_ids) => { .then((results) => results.map((result) => result ? result.count : 0)); }; -/** - * Returns the count of top-level comments on an asset id, also filtering by personalization options. - * - * @param {Array} id The ID of the asset - * @param {Array} excludeIgnored Exclude comments ignored by the requesting user - */ -const getParentCountByAssetIDPersonalized = async (context, {assetId, excludeIgnored, tags}) => { - const query = { - asset_id: assetId, - parent_id: null, - status: { - $in: ['NONE', 'ACCEPTED'], - }, - }; - - if (tags) { - query['tags.tag.name'] = { - $in: tags, - }; - } - - const user = context.user; - if (excludeIgnored && user) { - - // load afresh, as `user` may be from cache and not have recent ignores - const freshUser = await UsersService.findById(user.id); - const ignoredUsers = freshUser.ignoresUsers; - query.author_id = {$nin: ignoredUsers}; - } - - return CommentModel.where(query).count(); -}; - /** * Returns the comment count for all comments that are public based on their * parent ids. @@ -178,33 +111,6 @@ const getCountsByParentID = (context, parent_ids) => { .then((results) => results.map((result) => result ? result.count : 0)); }; -/** - * Returns the count of comments for the provided parent_id, also filtering by personalization options. - * - * @param {Array} id The ID of the parent comment - * @param {Array} excludeIgnored Exclude comments ignored by context.user - */ -const getCountByParentIDPersonalized = async (context, {id, excludeIgnored}) => { - const query = { - parent_id: { - $in: [id] - }, - status: { - $in: ['NONE', 'ACCEPTED'] - } - }; - const user = context.user; - if (excludeIgnored && user) { - - // load afresh, as `user` may be from cache and not have recent ignores - const freshUser = await UsersService.findById(user.id); - const ignoredUsers = freshUser.ignoresUsers; - query.author_id = {$nin: ignoredUsers}; - } - const count = await CommentModel.where(query).count(); - return count; -}; - /** * Retrieves the count of comments based on the passed in query. * @param {Object} context graph context @@ -260,7 +166,7 @@ const getCommentsByQuery = async ({user}, {ids, statuses, asset_id, parent_id, a // Only administrators can search for comments with statuses that are not // `null`, or `'ACCEPTED'`. - if (user != null && user.can(SEARCH_NON_NULL_OR_ACCEPTED_COMMENTS) && statuses) { + if (user != null && user.can(SEARCH_NON_NULL_OR_ACCEPTED_COMMENTS) && statuses && statuses.length > 0) { comments = comments.where({ status: { $in: statuses @@ -305,7 +211,7 @@ const getCommentsByQuery = async ({user}, {ids, statuses, asset_id, parent_id, a comments = comments.where({parent_id}); } - if (excludeIgnored && user && user.ignoresUsers) { + if (excludeIgnored && user && user.ignoresUsers && user.ignoresUsers.length > 0) { comments = comments.where({ author_id: {$nin: user.ignoresUsers} }); @@ -347,107 +253,6 @@ const getCommentsByQuery = async ({user}, {ids, statuses, asset_id, parent_id, a }); }; -/** - * Gets the recent replies. - * @param {Object} context graph context - * @param {Array} ids ids of parent ids - * @return {Promise} resolves to recent replies - */ -const genRecentReplies = (context, ids) => { - return CommentModel.aggregate([ - - // get all the replies for the comments in question - {$match: { - parent_id: { - $in: ids - } - }}, - - // sort these by their created at timestamp, ASC'y as these are - // replies - {$sort: { - created_at: 1 - }}, - - // group these replies by their parent_id - {$group: { - _id: '$parent_id', - replies: { - $push: '$$ROOT' - } - }}, - - // project it so that we only retain the first 3 replies of each parent - // comment - {$project: { - _id: '$_id', - replies: { - $slice: [ - '$replies', - 0, - 3 - ] - } - }}, - - {$unwind: '$replies'}, - - ]) - .then((replies) => replies.map((reply) => reply.replies)) - .then(arrayJoinBy(ids, 'parent_id')); -}; - -/** - * Gets the recent comments. - * @param {Object} context graph context - * @param {Array} ids ids of asset ids - * @return {Promise} resolves to recent comments from assets - */ -const genRecentComments = (_, ids) => { - return CommentModel.aggregate([ - - // get all the replies for the comments in question - {$match: { - asset_id: { - $in: ids - } - }}, - - // sort these by their created at timestamp, ASC'y as these are - // replies - {$sort: { - created_at: 1 - }}, - - // group these replies by their parent_id - {$group: { - _id: '$asset_id', - comments: { - $push: '$$ROOT' - } - }}, - - // project it so that we only retain the first 3 replies of each parent - // comment - {$project: { - _id: '$_id', - comments: { - $slice: [ - '$comments', - 0, - 3 - ] - } - }}, - - // Unwind these comments. - {$unwind: '$comments'}, - - ]) - .then((replies) => replies.map((reply) => reply.comments)) - .then(arrayJoinBy(ids, 'asset_id')); -}; - /** * getComments returns the comments by the id's. Only admins can see non-public comments. * @param {Object} context graph context @@ -486,12 +291,7 @@ module.exports = (context) => ({ getByQuery: (query) => getCommentsByQuery(context, query), getCountByQuery: (query) => getCommentCountByQuery(context, query), countByAssetID: new SharedCounterDataLoader('Comments.totalCommentCount', ms(CACHE_EXPIRY_COMMENT_COUNT), (ids) => getCountsByAssetID(context, ids)), - countByAssetIDPersonalized: (query) => getCountsByAssetIDPersonalized(context, query), parentCountByAssetID: new SharedCounterDataLoader('Comments.countByAssetID', ms(CACHE_EXPIRY_COMMENT_COUNT), (ids) => getParentCountsByAssetID(context, ids)), - parentCountByAssetIDPersonalized: (query) => getParentCountByAssetIDPersonalized(context, query), countByParentID: new SharedCounterDataLoader('Comments.countByParentID', ms(CACHE_EXPIRY_COMMENT_COUNT), (ids) => getCountsByParentID(context, ids)), - countByParentIDPersonalized: (query) => getCountByParentIDPersonalized(context, query), - genRecentReplies: new DataLoader((ids) => genRecentReplies(context, ids)), - genRecentComments: new DataLoader((ids) => genRecentComments(context, ids)) } }); diff --git a/graph/resolvers/asset.js b/graph/resolvers/asset.js index fb2f6cac5..f5b3f04bc 100644 --- a/graph/resolvers/asset.js +++ b/graph/resolvers/asset.js @@ -4,9 +4,6 @@ const { } = require('../../perms/constants'); const Asset = { - recentComments({id}, _, {loaders: {Comments}}) { - return Comments.genRecentComments.load(id); - }, async comment({id}, {id: commentId}, {loaders: {Comments}, user}) { const statuses = user && user.can(SEARCH_NON_NULL_OR_ACCEPTED_COMMENTS) ? ['NONE', 'ACCEPTED', 'PREMOD', 'REJECTED'] @@ -20,7 +17,7 @@ const Asset = { return comments.nodes[0]; }, - comments({id}, {sort, limit, deep, excludeIgnored, tags}, {loaders: {Comments}}) { + comments({id}, {query: {sort, limit, excludeIgnored, tags}, deep}, {loaders: {Comments}}) { return Comments.getByQuery({ asset_id: id, sort, @@ -30,26 +27,37 @@ const Asset = { excludeIgnored, }); }, - commentCount({id, commentCount}, {excludeIgnored, tags}, {user, loaders: {Comments}}) { - - // TODO: remove - if ((user && excludeIgnored) || tags) { - return Comments.parentCountByAssetIDPersonalized({assetId: id, excludeIgnored, tags}); - } + commentCount({id, commentCount}, {tags}, {loaders: {Comments}}) { if (commentCount != null) { return commentCount; } + + // If we are filtering by a tag. + if (tags && tags.length > 0) { + + // Then count the comments with those tags. + return Comments.getCountByQuery({ + tags, + asset_id: id, + parent_id: id, + statuses: ['NONE', 'ACCEPTED'], + }); + } + return Comments.parentCountByAssetID.load(id); }, - totalCommentCount({id, totalCommentCount}, {excludeIgnored, tags}, {user, loaders: {Comments}}) { - - // TODO: remove - if ((user && excludeIgnored) || tags) { - return Comments.countByAssetIDPersonalized({assetId: id, excludeIgnored, tags}); - } + totalCommentCount({id, totalCommentCount}, {tags}, {loaders: {Comments}}) { if (totalCommentCount != null) { return totalCommentCount; } + + // If we are filtering by a tag. + if (tags && tags.length > 0) { + + // Then count the comments with those tags. + return Comments.getCountByQuery({tags, asset_id: id, statuses: ['NONE', 'ACCEPTED']}); + } + return Comments.countByAssetID.load(id); }, async settings({settings = null}, _, {loaders: {Settings}}) { diff --git a/graph/resolvers/comment.js b/graph/resolvers/comment.js index 0fd3a730c..f9924f90c 100644 --- a/graph/resolvers/comment.js +++ b/graph/resolvers/comment.js @@ -11,10 +11,16 @@ const Comment = { user({author_id}, _, {loaders: {Users}}) { return Users.getByID.load(author_id); }, - recentReplies({id}, _, {loaders: {Comments}}) { - return Comments.genRecentReplies.load(id); - }, - replies({id, asset_id}, {sort, limit, excludeIgnored}, {loaders: {Comments}}) { + replies({id, asset_id, reply_count}, {query: {sort, limit, excludeIgnored}}, {loaders: {Comments}}) { + + // Don't bother looking up replies if there aren't any there! + if (reply_count === 0) { + return { + nodes: [], + hasNextPage: false, + }; + } + return Comments.getByQuery({ asset_id, parent_id: id, @@ -23,21 +29,17 @@ const Comment = { excludeIgnored, }); }, - replyCount({id}, {excludeIgnored}, {user, loaders: {Comments}}) { + replyCount({reply_count}) { - // TODO: remove - if (user && excludeIgnored) { - return Comments.countByParentIDPersonalized({id, excludeIgnored}); - } - return Comments.countByParentID.load(id); + // A simple remap from the underlying database model to the graph model. + return reply_count; }, actions({id}, _, {user, loaders: {Actions}}) { - - if (user && user.can('SEARCH_ACTIONS')) { - return Actions.getByID.load(id); + if (!user || !user.can('SEARCH_ACTIONS')) { + return null; } - return null; + return Actions.getByID.load(id); }, action_summaries({id, action_summaries}, _, {loaders: {Actions}}) { if (action_summaries) { diff --git a/graph/typeDefs.graphql b/graph/typeDefs.graphql index 505026389..4555131f4 100644 --- a/graph/typeDefs.graphql +++ b/graph/typeDefs.graphql @@ -20,7 +20,7 @@ type Reliability { # `null` if the reliability cannot be determined. flagger: Boolean - # commenter will be `true` when the commenter is reliable, `false` if not, or + # Commenter will be `true` when the commenter is reliable, `false` if not, or # `null` if the reliability cannot be determined. commenter: Boolean } @@ -211,7 +211,7 @@ enum COMMENT_STATUS { PREMOD } -# The types of action there are as enum's. +# The types of action there are as enums. enum ACTION_TYPE { # Represents a FlagAction. @@ -246,7 +246,7 @@ input CommentsQuery { # Skip results from the last created_at timestamp. cursor: Cursor - # Sort the results by created_at. + # Sort the results by from largest first. sort: SORT_ORDER = DESC # Filter by a specific tag name. @@ -256,6 +256,18 @@ input CommentsQuery { excludeIgnored: Boolean } +input RepliesQuery { + + # Sort the results by from smallest first. + sort: SORT_ORDER = ASC + + # Limit the number of results to be returned. + limit: Int = 3 + + # Exclude comments ignored by the requesting user + excludeIgnored: Boolean +} + # CommentCountQuery allows the ability to query comment counts by specific # methods. input CommentCountQuery { @@ -313,14 +325,11 @@ type Comment { # the user who authored the comment. user: User - # the recent replies made against this comment. - recentReplies: [Comment!] - # the replies that were made to the comment. - replies(sort: SORT_ORDER = ASC, limit: Int = 3, excludeIgnored: Boolean): CommentConnection! + replies(query: RepliesQuery!): CommentConnection! # The count of replies on a comment. - replyCount(excludeIgnored: Boolean): Int + replyCount: Int # Actions completed on the parent. Requires the `ADMIN` role. actions: [Action] @@ -569,28 +578,18 @@ type Asset { # The URL that the asset is located on. url: String - # Returns recent comments - recentComments: [Comment!] - - # The comments that are attached to the asset. - # If `deep` is true, it will return comments of all depths, - # otherwise only top-level comments are returned. - comments( - sort: SORT_ORDER = DESC, - limit: Int = 10, - excludeIgnored: Boolean, - tags: [String!] - deep: Boolean, - ): CommentConnection! + # The comments that are attached to the asset. When `deep` is true, the + # comments returned will be at all depths. + comments(query: CommentsQuery!, deep: Boolean = false): CommentConnection! # A Comment from the Asset by comment's ID comment(id: ID!): Comment # The count of top level comments on the asset. - commentCount(excludeIgnored: Boolean, tags: [String!]): Int + commentCount(tags: [String!]): Int # The total count of all comments made on the asset. - totalCommentCount(excludeIgnored: Boolean, tags: [String!]): Int + totalCommentCount(tags: [String!]): Int # The settings (rectified with the global settings) that should be applied to # this asset. @@ -921,19 +920,19 @@ input ModifyTagInput { # Response to the addTag or removeTag mutations. type ModifyTagResponse implements Response { - # An array of errors relating to the mutation that occured. + # An array of errors relating to the mutation that occurred. errors: [UserError!] } # Response to ignoreUser mutation type IgnoreUserResponse implements Response { - # An array of errors relating to the mutation that occured. + # An array of errors relating to the mutation that occurred. errors: [UserError!] } # Response to stopIgnoringUser mutation type StopIgnoringUserResponse implements Response { - # An array of errors relating to the mutation that occured. + # An array of errors relating to the mutation that occurred. errors: [UserError!] } @@ -944,13 +943,13 @@ input EditCommentInput { body: String! } -# EditCommentResponse contains the updated comment and any errors that occured. +# EditCommentResponse contains the updated comment and any errors that occurred. type EditCommentResponse implements Response { # The edited comment. comment: Comment - # An array of errors relating to the mutation that occured. + # An array of errors relating to the mutation that occurred. errors: [UserError!] } @@ -967,7 +966,7 @@ type CreateTokenResponse implements Response { # Token is the Token that was created, or null if it failed. token: Token - # An array of errors relating to the mutation that occured. + # An array of errors relating to the mutation that occurred. errors: [UserError!] } @@ -981,7 +980,7 @@ input RevokeTokenInput { # RevokeTokenResponse contains the errors related to revoking a token. type RevokeTokenResponse implements Response { - # An array of errors relating to the mutation that occured. + # An array of errors relating to the mutation that occurred. errors: [UserError!] } diff --git a/models/comment.js b/models/comment.js index ac1f1bcb1..73aa22d0b 100644 --- a/models/comment.js +++ b/models/comment.js @@ -55,7 +55,7 @@ const CommentSchema = new Schema({ body: { type: String, required: [true, 'The body is required.'], - minlength: 2 + minlength: 2, }, body_history: [BodyHistoryItemSchema], asset_id: String, diff --git a/plugins/talk-plugin-featured-comments/client/containers/Tab.js b/plugins/talk-plugin-featured-comments/client/containers/Tab.js index fee7f5da6..cbe430be2 100644 --- a/plugins/talk-plugin-featured-comments/client/containers/Tab.js +++ b/plugins/talk-plugin-featured-comments/client/containers/Tab.js @@ -17,7 +17,7 @@ const enhance = compose( withFragments({ asset: gql` fragment TalkFeaturedComments_Tab_asset on Asset { - featuredCommentsCount: totalCommentCount(tags: ["FEATURED"], excludeIgnored: $excludeIgnored) @skip(if: $hasComment) + featuredCommentsCount: totalCommentCount(tags: ["FEATURED"]) @skip(if: $hasComment) }`, }), excludeIf((props) => props.asset.featuredCommentsCount === 0), diff --git a/plugins/talk-plugin-featured-comments/client/containers/TabPane.js b/plugins/talk-plugin-featured-comments/client/containers/TabPane.js index a5fd3b7e8..31f2769a0 100644 --- a/plugins/talk-plugin-featured-comments/client/containers/TabPane.js +++ b/plugins/talk-plugin-featured-comments/client/containers/TabPane.js @@ -79,7 +79,7 @@ const enhance = compose( asset: gql` fragment TalkFeaturedComments_TabPane_asset on Asset { id - featuredComments: comments(tags: ["FEATURED"], excludeIgnored: $excludeIgnored, deep: true) @skip(if: $hasComment) { + featuredComments: comments(query: {tags: ["FEATURED"]}, deep: true) @skip(if: $hasComment) { nodes { ...${getDefinitionName(Comment.fragments.comment)} } diff --git a/services/mongoose.js b/services/mongoose.js index 07f3b2189..43966eb0d 100644 --- a/services/mongoose.js +++ b/services/mongoose.js @@ -1,7 +1,7 @@ const mongoose = require('mongoose'); const debug = require('debug')('talk:db'); const enabled = require('debug').enabled; -const queryDebuger = require('debug')('talk:db:query'); +const queryDebugger = require('debug')('talk:db:query'); const { MONGO_URL, @@ -27,14 +27,14 @@ function debugQuery(name, i, ...args) { let params = `(${_args.join(', ')})`; - queryDebuger(functionCall + params); + queryDebugger(functionCall + params); } // Use native promises mongoose.Promise = global.Promise; // Check if debugging is enabled on the talk:db prefix. -if (enabled('talk:db')) { +if (enabled('talk:db:query')) { // Enable the mongoose debugger, here we wrap the similar print function // provided by setting the debug parameter. @@ -53,14 +53,16 @@ if (WEBPACK) { } else { // Connect to the Mongo instance. - mongoose.connect(MONGO_URL, (err) => { - if (err) { + mongoose + .connect(MONGO_URL, { + useMongoClient: true, + }) + .then(() => { + debug('connection established'); + }) + .catch((err) => { throw err; - } - - debug('connection established'); - }); - + }); } module.exports = mongoose; diff --git a/test/server/graph/queries/asset.js b/test/server/graph/queries/asset.js index 0b837a181..68270f9ba 100644 --- a/test/server/graph/queries/asset.js +++ b/test/server/graph/queries/asset.js @@ -45,7 +45,7 @@ describe('graph.queries.asset', () => { const assetCommentsQuery = ` query assetCommentsQuery($id: ID!) { asset(id: $id) { - comments(limit: 10) { + comments(query: {limit: 10}) { nodes { id body @@ -59,7 +59,7 @@ describe('graph.queries.asset', () => { } `; const res = await graphql(schema, assetCommentsQuery, {}, context, {id: asset.id}); - expect(res.erros).is.empty; + expect(res.errors).is.empty; const {nodes, startCursor, endCursor, hasNextPage} = res.data.asset.comments; expect(nodes.length).to.equal(2); expect(startCursor).to.equal(nodes[0].created_at); @@ -82,7 +82,7 @@ describe('graph.queries.asset', () => { const query = ` query assetCommentsQuery($id: ID!, $url: String!, $excludeIgnored: Boolean!) { asset(id: $id, url: $url) { - comments(limit: 10, excludeIgnored: $excludeIgnored) { + comments(query: {limit: 10, excludeIgnored: $excludeIgnored}) { nodes { id body