diff --git a/src/core/server/graph/tenant/resolvers/Comment.ts b/src/core/server/graph/tenant/resolvers/Comment.ts index d65cdffcc..951999e86 100644 --- a/src/core/server/graph/tenant/resolvers/Comment.ts +++ b/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(info); @@ -62,9 +59,10 @@ export const Comment: GQLCommentTypeResolver = { 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 = { }), 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); }, }; diff --git a/src/core/server/graph/tenant/resolvers/CommentCounts.ts b/src/core/server/graph/tenant/resolvers/CommentCounts.ts index cc6200dfb..c41fb4d9c 100644 --- a/src/core/server/graph/tenant/resolvers/CommentCounts.ts +++ b/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, }; diff --git a/src/core/server/models/comment/constants.ts b/src/core/server/models/comment/constants.ts new file mode 100644 index 000000000..fa845e7a0 --- /dev/null +++ b/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, +]; diff --git a/src/core/server/models/comment/helpers.ts b/src/core/server/models/comment/helpers.ts new file mode 100644 index 000000000..8ae43f86d --- /dev/null +++ b/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 is Required> { + 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): boolean { + return VISIBLE_STATUSES.includes(comment.status); +} diff --git a/src/core/server/models/comment/index.ts b/src/core/server/models/comment/index.ts index 4601952d0..2b3ea4264 100644 --- a/src/core/server/models/comment/index.ts +++ b/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>([ { 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 = { 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>>> { // 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 diff --git a/src/core/server/services/comments/index.ts b/src/core/server/services/comments/index.ts index 84381c5b2..c3cf1dead 100644 --- a/src/core/server/services/comments/index.ts +++ b/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, },