comment count optim, prep for sortBy

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

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
           }

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<String>} id The ID of the asset
- * @param {Array<String>} 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<String>} id The ID of the asset
- * @param {Array<String>} 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<String>} id The ID of the parent comment
- * @param {Array<String>} 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<String>} 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<String>} 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))
   }
 });

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}}) {

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) {

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!]
 }
 

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,

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),

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)}
           }

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;

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