[next] Ancestors (#2333)

* feat: simplified ancestor management

* fix: removed old comment

src/core/server/graph/tenant/resolvers/Comment.ts

@@ -14,19 +14,16 @@ import { getLatestRevision } from "coral-server/models/comment";
 import { createConnection } from "coral-server/models/helpers/connection";
 import { getCommentEditableUntilDate } from "coral-server/services/comments";
 
+import { StoryNotFoundError } from "coral-server/errors";
+import { hasAncestors } from "coral-server/models/comment/helpers";
 import TenantContext from "../context";
 import { getURLWithCommentID } from "./util";
 
 const maybeLoadOnlyID = (
   ctx: TenantContext,
   info: GraphQLResolveInfo,
-  id?: string
+  id: string
 ) => {
-  // If there isn't an id, then return nothing!
-  if (!id) {
-    return null;
-  }
-
   // Get the field names of the fields being requested, if it's only the ID,
   // we have that, so no need to make a database request.
   const fields = getRequestedFields<GQLComment>(info);
@@ -62,9 +59,10 @@ export const Comment: GQLCommentTypeResolver<comment.Comment> = {
   replies: (c, input, ctx) =>
     // If there is at least one reply, then use the connection loader, otherwise
     // return a blank connection.
-    c.replyCount > 0
+    c.childCount > 0
       ? ctx.loaders.Comments.forParent(c.storyID, c.id, input)
       : createConnection(),
+  replyCount: ({ childCount }) => childCount || 0,
   // Action Counts are encoded, decode them for use with the GraphQL system.
   actionCounts: c => decodeActionCounts(c.actionCounts),
   flags: ({ id }, { first = 10, after }, ctx) =>
@@ -78,25 +76,25 @@ export const Comment: GQLCommentTypeResolver<comment.Comment> = {
     }),
   viewerActionPresence: (c, input, ctx) =>
     ctx.user ? ctx.loaders.Comments.retrieveMyActionPresence.load(c.id) : null,
-  parentCount: c => (c.parentID ? c.grandparentIDs.length + 1 : 0),
-  depth: c => (c.parentID ? c.grandparentIDs.length + 1 : 0),
+  parentCount: c => (hasAncestors(c) ? c.ancestorIDs.length : 0),
+  depth: c => (hasAncestors(c) ? c.ancestorIDs.length : 0),
   rootParent: (c, input, ctx, info) =>
-    maybeLoadOnlyID(
-      ctx,
-      info,
-      c.grandparentIDs.length > 0 ? c.grandparentIDs[0] : c.parentID
-    ),
-  parent: (c, input, ctx, info) => maybeLoadOnlyID(ctx, info, c.parentID),
+    hasAncestors(c)
+      ? maybeLoadOnlyID(ctx, info, c.ancestorIDs[c.ancestorIDs.length - 1])
+      : null,
+  parent: (c, input, ctx, info) =>
+    hasAncestors(c) ? maybeLoadOnlyID(ctx, info, c.parentID) : null,
   parents: (c, input, ctx) =>
     // Some resolver optimization.
-    c.parentID ? ctx.loaders.Comments.parents(c, input) : createConnection(),
+    hasAncestors(c)
+      ? ctx.loaders.Comments.parents(c, input)
+      : createConnection(),
   story: (c, input, ctx) => ctx.loaders.Stories.story.load(c.storyID),
-  permalink: async (c, input, ctx) => {
-    const story = await ctx.loaders.Stories.story.load(c.storyID);
+  permalink: async ({ id, storyID }, input, ctx) => {
+    const story = await ctx.loaders.Stories.story.load(storyID);
     if (!story) {
-      // TODO: better error reporting?
-      throw new Error("Story not found");
+      throw new StoryNotFoundError(storyID);
     }
-    return getURLWithCommentID(story.url, c.id);
+    return getURLWithCommentID(story.url, id);
   },
 };

src/core/server/graph/tenant/resolvers/CommentCounts.ts

@@ -1,14 +1,14 @@
-import {
-  GQLCOMMENT_STATUS,
-  GQLCommentCountsTypeResolver,
-} from "coral-server/graph/tenant/schema/__generated__/types";
+import { GQLCommentCountsTypeResolver } from "coral-server/graph/tenant/schema/__generated__/types";
+import { VISIBLE_STATUSES } from "coral-server/models/comment/constants";
 import { CommentStatusCounts } from "coral-server/models/story";
 
 export const CommentCounts: GQLCommentCountsTypeResolver<
   CommentStatusCounts
 > = {
   totalVisible: commentCounts =>
-    commentCounts[GQLCOMMENT_STATUS.ACCEPTED] +
-    commentCounts[GQLCOMMENT_STATUS.NONE],
+    VISIBLE_STATUSES.reduce(
+      (total, status) => total + commentCounts[status],
+      0
+    ),
   statuses: commentCounts => commentCounts,
 };

src/core/server/models/comment/constants.ts

@@ -0,0 +1,10 @@
+import { GQLCOMMENT_STATUS } from "coral-server/graph/tenant/schema/__generated__/types";
+
+/**
+ * VISIBLE_STATUSES are the comment statuses that a Comment may have that would
+ * make it visible to readers.
+ */
+export const VISIBLE_STATUSES = [
+  GQLCOMMENT_STATUS.NONE,
+  GQLCOMMENT_STATUS.ACCEPTED,
+];

src/core/server/models/comment/helpers.ts

@@ -0,0 +1,23 @@
+import { Comment } from ".";
+import { VISIBLE_STATUSES } from "./constants";
+
+/**
+ * hasAncestors will check to see if a given comment has any ancestors.
+ *
+ * @param comment the comment to check the ancestors on
+ */
+export function hasAncestors(
+  comment: Pick<Comment, "ancestorIDs" | "parentID">
+): comment is Required<Pick<Comment, "ancestorIDs" | "parentID">> {
+  return Boolean(comment.ancestorIDs.length > 0);
+}
+
+/**
+ * hasVisibleStatus will check to see if the comment has a visibility status
+ * where readers could see it.
+ *
+ * @param comment the comment to check the status on
+ */
+export function hasVisibleStatus(comment: Pick<Comment, "status">): boolean {
+  return VISIBLE_STATUSES.includes(comment.status);
+}

src/core/server/models/comment/index.ts

@@ -27,6 +27,8 @@ import {
 } from "coral-server/models/helpers/indexing";
 import Query from "coral-server/models/helpers/query";
 import { TenantResource } from "coral-server/models/tenant";
+import { VISIBLE_STATUSES } from "./constants";
+import { hasAncestors } from "./helpers";
 import { CommentTag } from "./tag";
 
 function collection(mongo: Db) {
@@ -70,13 +72,19 @@ export interface Comment extends TenantResource {
   readonly id: string;
 
   /**
-   * parentID stores the ID of a parent Comment if this Comment is a reply.
+   * ancestorIDs stores all the ancestor ID's, with the direct parent being
+   * first.
+   */
+  ancestorIDs: string[];
+
+  /**
+   * parentID is the ID of the parent Comment if this Comment is a reply.
    */
   parentID?: string;
 
   /**
-   * parentRevisionID is the ID of the Revision on the parent Comment that this
-   * was a reply to.
+   * parentRevisionID is the ID of the Revision on the Comment referenced by the
+   * `parentID`.
    */
   parentRevisionID?: string;
 
@@ -108,16 +116,9 @@ export interface Comment extends TenantResource {
   actionCounts: EncodedCommentActionCounts;
 
   /**
-   * grandparentIDs stores all the ID's of all the Comment's that came before.
-   * This prevents the need for performing multiple queries to retrieve the
-   * Comment ancestors.
+   * childIDs are the ID's of all the Comment's that are direct replies.
    */
-  grandparentIDs: string[];
-
-  /**
-   * replyIDs are the ID's of all the Comment's that are direct replies.
-   */
-  replyIDs: string[];
+  childIDs: string[];
 
   /**
    * tags are CommentTag's on a specific Comment to be showcased with the
@@ -126,11 +127,11 @@ export interface Comment extends TenantResource {
   tags: CommentTag[];
 
   /**
-   * replyCount is the count of direct replies. It is stored as a separate value
-   * here even though the replyIDs field technically contained the same data in
+   * childCount is the count of direct replies. It is stored as a separate value
+   * here even though the childIDs field technically contained the same data in
    * it's length because we needed to sort by this field sometimes.
    */
-  replyCount: number;
+  childCount: number;
 
   /**
    * metadata stores the deep Comment properties.
@@ -158,7 +159,7 @@ export async function createCommentIndexes(mongo: Db) {
   const variants = createConnectionOrderVariants<Readonly<Comment>>([
     { createdAt: -1 },
     { createdAt: 1 },
-    { replyCount: -1, createdAt: -1 },
+    { childCount: -1, createdAt: -1 },
     { "actionCounts.REACTION": -1, createdAt: -1 },
   ]);
 
@@ -209,8 +210,8 @@ export type CreateCommentInput = Omit<
   | "id"
   | "tenantID"
   | "createdAt"
-  | "replyIDs"
-  | "replyCount"
+  | "childIDs"
+  | "childCount"
   | "actionCounts"
   | "revisions"
   | "deletedAt"
@@ -240,8 +241,8 @@ export async function createComment(
   const defaults: Sub<Comment, CreateCommentInput> = {
     id: uuid.v4(),
     tenantID,
-    replyIDs: [],
-    replyCount: 0,
+    childIDs: [],
+    childCount: 0,
     revisions: [revision],
     createdAt: now,
   };
@@ -279,8 +280,8 @@ export async function pushChildCommentIDOntoParent(
       id: parentID,
     },
     {
-      $push: { replyIDs: childID },
-      $inc: { replyCount: 1 },
+      $push: { childIDs: childID },
+      $inc: { childCount: 1 },
     }
   );
 
@@ -533,7 +534,7 @@ export async function retrieveCommentParentsConnection(
   { last: limit, before: skip = 0 }: { last: number; before?: number }
 ): Promise<Readonly<Connection<Readonly<Comment>>>> {
   // Return nothing if this comment does not have any parents.
-  if (!comment.parentID) {
+  if (!hasAncestors(comment)) {
     return createConnection({
       pageInfo: {
         hasNextPage: false,
@@ -548,41 +549,18 @@ export async function retrieveCommentParentsConnection(
     return createConnection({
       pageInfo: {
         hasNextPage: false,
-        hasPreviousPage: !!comment.parentID,
+        hasPreviousPage: true,
         endCursor: 0,
         startCursor: 0,
       },
     });
   }
 
-  // If the last paramter is 1, and the after paramter is either unset or equal
-  // to zero, then all we have to return is the direct parent.
-  if (limit === 1 && skip <= 0) {
-    const parent = await retrieveComment(mongo, tenantID, comment.parentID);
-    if (!parent) {
-      throw new Error("parent comment not found");
-    }
-
-    return {
-      edges: [{ node: parent, cursor: 1 }],
-      nodes: [parent],
-      pageInfo: {
-        hasNextPage: false,
-        hasPreviousPage: comment.grandparentIDs.length > 0,
-        endCursor: 1,
-        startCursor: 1,
-      },
-    };
-  }
-
-  // Create a list of all the comment parent ids, in reverse order.
-  const parentIDs = [comment.parentID, ...comment.grandparentIDs.reverse()];
-
   // Fetch the subset of the comment id's that we are going to query for.
-  const parentIDSubset = parentIDs.slice(skip, skip + limit);
+  const ancestorIDs = comment.ancestorIDs.slice(skip, skip + limit);
 
   // Retrieve the parents via the subset list.
-  const nodes = await retrieveManyComments(mongo, tenantID, parentIDSubset);
+  const nodes = await retrieveManyComments(mongo, tenantID, ancestorIDs);
 
   // Loop over the list to ensure that none of the entries is null (indicating
   // that there was a misplaced parent). We can assert the type here because we
@@ -604,7 +582,7 @@ export async function retrieveCommentParentsConnection(
     nodes,
     pageInfo: {
       hasNextPage: false,
-      hasPreviousPage: parentIDs.length > limit + skip,
+      hasPreviousPage: comment.ancestorIDs.length > limit + skip,
       startCursor: edges.length > 0 ? edges[0].cursor : null,
       endCursor: edges.length > 0 ? edges[edges.length - 1].cursor : null,
     },
@@ -671,14 +649,7 @@ export const retrieveVisibleCommentConnection = (
   mongo: Db,
   tenantID: string,
   input: CommentConnectionInput
-) =>
-  retrieveStatusCommentConnection(
-    mongo,
-    tenantID,
-    // Only get Comment's that are visible.
-    [GQLCOMMENT_STATUS.NONE, GQLCOMMENT_STATUS.ACCEPTED],
-    input
-  );
+) => retrieveStatusCommentConnection(mongo, tenantID, VISIBLE_STATUSES, input);
 
 /**
  * retrieveStatusCommentConnection will retrieve a connection that contains

src/core/server/services/comments/index.ts

@@ -32,6 +32,10 @@ import {
   CoralError,
   StoryNotFoundError,
 } from "coral-server/errors";
+import {
+  hasAncestors,
+  hasVisibleStatus,
+} from "coral-server/models/comment/helpers";
 import { AugmentedRedis } from "../redis";
 import { addCommentActions, CreateAction } from "./actions";
 import { calculateCounts, calculateCountsDiff } from "./moderation/counts";
@@ -39,7 +43,7 @@ import { PhaseResult, processForModeration } from "./pipeline";
 
 export type CreateComment = Omit<
   CreateCommentInput,
-  "status" | "metadata" | "grandparentIDs" | "actionCounts" | "tags"
+  "status" | "metadata" | "ancestorIDs" | "actionCounts" | "tags"
 >;
 
 export async function create(
@@ -70,7 +74,7 @@ export async function create(
     throw new StoryNotFoundError(input.storyID);
   }
 
-  const grandparentIDs: string[] = [];
+  const ancestorIDs: string[] = [];
   if (input.parentID) {
     // Check to see that the reference parent ID exists.
     const parent = await retrieveComment(mongo, tenant.id, input.parentID);
@@ -78,18 +82,20 @@ export async function create(
       throw new CommentNotFoundError(input.parentID);
     }
 
-    // FIXME: (wyattjoh) Check that the parent comment was visible!
+    // Check that the parent comment was visible.
+    if (!hasVisibleStatus(parent)) {
+      throw new CommentNotFoundError(parent.id);
+    }
 
-    // Push the parent's parent id's into the comment's grandparent id's.
-    grandparentIDs.push(...parent.grandparentIDs);
-    if (parent.parentID) {
-      // If this parent has a parent, push it down as well.
-      grandparentIDs.push(parent.parentID);
+    ancestorIDs.push(input.parentID);
+    if (hasAncestors(parent)) {
+      // Push the parent's ancestors id's into the comment's ancestor id's.
+      ancestorIDs.push(...parent.ancestorIDs);
     }
 
     log.trace(
-      { grandparentIDs: grandparentIDs.length },
-      "pushed grandparent id's into comment creation"
+      { ancestorIDs: ancestorIDs.length },
+      "pushed parent ancestorIDs into comment"
     );
   }
 
@@ -149,7 +155,7 @@ export async function create(
       tags,
       body,
       status,
-      grandparentIDs,
+      ancestorIDs,
       metadata,
       actionCounts,
     },