From 6db72db8373ac40ffc41bd015e6d68269276834a Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Thu, 19 Mar 2020 18:59:34 +0000 Subject: [PATCH] [CORL-963] Story sorting fix (#2887) * fix: fixed sorting issues with queries * review: naming touchups --- src/core/server/graph/loaders/Stories.ts | 83 ++++++++++++++++++- src/core/server/graph/resolvers/Story.ts | 17 ++-- src/core/server/models/story/helpers.ts | 15 +++- .../comments/pipeline/phases/storyClosed.ts | 5 +- 4 files changed, 100 insertions(+), 20 deletions(-) diff --git a/src/core/server/graph/loaders/Stories.ts b/src/core/server/graph/loaders/Stories.ts index 48f6725ac..fa689d361 100644 --- a/src/core/server/graph/loaders/Stories.ts +++ b/src/core/server/graph/loaders/Stories.ts @@ -1,8 +1,10 @@ import DataLoader from "dataloader"; import { defaultTo } from "lodash"; +import { DateTime } from "luxon"; import GraphContext from "coral-server/graph/context"; import { Connection } from "coral-server/models/helpers"; +import { CloseCommenting } from "coral-server/models/settings"; import { retrieveActiveStories, retrieveManyStories, @@ -26,16 +28,89 @@ import { import { createManyBatchLoadFn } from "./util"; const statusFilter = ( - status?: GQLSTORY_STATUS + closeCommenting: CloseCommenting, + status?: GQLSTORY_STATUS, + now = new Date() ): StoryConnectionInput["filter"] => { switch (status) { case GQLSTORY_STATUS.OPEN: + if (closeCommenting.auto) { + // Automatic story closing has been enabled. Stories will be considered + // open if they have a ${closedAt} date in the future, if they've been + // forced open (where ${closedAt} is set to false), or they haven't been + // closed and their ${createdAt} date is after ${now - close.timeout}. + + // Calculate the cutoff time for createdAt. + const consideredClosedAt = DateTime.fromJSDate(now) + .plus({ + seconds: -closeCommenting.timeout, + }) + .toJSDate(); + + return { + $or: [ + // The story will be open if the close date is in the future... + { + closedAt: { $gt: now }, + }, + // Or the story has been forced open by setting closedAt to false... + { + closedAt: false, + }, + // Or the closed at date isn't set and the createdAt date is after + // the cutoff date. + { + closedAt: null, + createdAt: { $gt: consideredClosedAt }, + }, + ], + }; + } + + // Automatic story closing is not enabled. Stories will be considered open + // if they do not have a ${closedAt}, it has been forced open (where + // ${closedAt} is set to false), or the ${closedAt} date is in the future. return { - closedAt: { $in: [null, false] }, + $or: [ + // A story is open if the closedAt date is in the future... + { closedAt: { $gt: now } }, + // Or the closedAt date is not set. + { closedAt: { $in: [null, false] } }, + ], }; case GQLSTORY_STATUS.CLOSED: + if (closeCommenting.auto) { + // Automatic story closing has been enabled. Stories will be considered + // closed if they have a ${closedAt} date before the current date or + // they do not have a ${closedAt} date set and the ${createdAt} date is + // before ${now - close.timeout} (implying that it's close window is + // up). + + // Calculate the cutoff time for createdAt. + const consideredClosedAt = DateTime.fromJSDate(now) + .plus({ + seconds: -closeCommenting.timeout, + }) + .toJSDate(); + + return { + $or: [ + // The story will be closed if the closedAt date is in the past... + { closedAt: { $lte: now } }, + // Or the closedAt date isn't set and the createdAt date is before + // the cutoff date. + { + closedAt: null, + createdAt: { $lte: consideredClosedAt }, + }, + ], + }; + } + + // Automatic story closing is not enabled. Stories will be considered + // closed if the ${closedAt} date is before ${now}. return { - closedAt: { $lte: new Date() }, + closedAt: { $lte: now }, }; default: return {}; @@ -126,7 +201,7 @@ export default (ctx: GraphContext) => ({ // Merge the site filter into the connection filter. ...siteFilter(siteID), // Merge the status filter into the connection filter. - ...statusFilter(status), + ...statusFilter(ctx.tenant.closeCommenting, status, ctx.now), // Merge the query filters into the query. ...queryFilter(query), }, diff --git a/src/core/server/graph/resolvers/Story.ts b/src/core/server/graph/resolvers/Story.ts index cef8ef679..3b9df834c 100644 --- a/src/core/server/graph/resolvers/Story.ts +++ b/src/core/server/graph/resolvers/Story.ts @@ -1,29 +1,26 @@ import { defaultsDeep } from "lodash"; +import { decodeActionCounts } from "coral-server/models/action/comment"; +import * as story from "coral-server/models/story"; + import { GQLSTORY_STATUS, GQLStoryTypeResolver, GQLTAG, } from "coral-server/graph/schema/__generated__/types"; -import { decodeActionCounts } from "coral-server/models/action/comment"; -import * as story from "coral-server/models/story"; -import GraphContext from "../context"; import { CommentCountsInput } from "./CommentCounts"; import { storyModerationInputResolver } from "./ModerationQueues"; -const isStoryClosed = (s: story.Story, ctx: GraphContext) => { - const closedAt = story.getStoryClosedAt(ctx.tenant, s) || null; - return !!closedAt && new Date() >= closedAt; -}; - export const Story: GQLStoryTypeResolver = { comments: (s, input, ctx) => ctx.loaders.Comments.forStory(s.id, input), featuredComments: (s, input, ctx) => ctx.loaders.Comments.taggedForStory(s.id, GQLTAG.FEATURED, input), status: (s, input, ctx) => - isStoryClosed(s, ctx) ? GQLSTORY_STATUS.CLOSED : GQLSTORY_STATUS.OPEN, - isClosed: (s, input, ctx) => isStoryClosed(s, ctx), + story.isStoryClosed(ctx.tenant, s, ctx.now) + ? GQLSTORY_STATUS.CLOSED + : GQLSTORY_STATUS.OPEN, + isClosed: (s, input, ctx) => story.isStoryClosed(ctx.tenant, s, ctx.now), closedAt: (s, input, ctx) => story.getStoryClosedAt(ctx.tenant, s) || null, commentActionCounts: s => decodeActionCounts(s.commentCounts.action), commentCounts: (s): CommentCountsInput => s, diff --git a/src/core/server/models/story/helpers.ts b/src/core/server/models/story/helpers.ts index 4c7f4a5c3..6bca88022 100644 --- a/src/core/server/models/story/helpers.ts +++ b/src/core/server/models/story/helpers.ts @@ -26,10 +26,19 @@ export function getStoryTitle(story: Pick) { : story.url; } +export function isStoryClosed( + tenant: Pick, + story: Pick, + now = new Date() +) { + const closedAt = getStoryClosedAt(tenant, story); + return closedAt && closedAt <= now; +} + export function getStoryClosedAt( tenant: Pick, story: Pick -): Story["closedAt"] { +): Date | null { // Try to get the closedAt time from the story. if (story.closedAt) { return story.closedAt; @@ -37,7 +46,7 @@ export function getStoryClosedAt( // Check to see if the story has been forced open again. if (story.closedAt === false) { - return false; + return null; } // If the story hasn't already been closed, then check to see if the Tenant @@ -50,5 +59,5 @@ export function getStoryClosedAt( .toJSDate(); } - return; + return null; } diff --git a/src/core/server/services/comments/pipeline/phases/storyClosed.ts b/src/core/server/services/comments/pipeline/phases/storyClosed.ts index 05bdb7a16..b114572ae 100644 --- a/src/core/server/services/comments/pipeline/phases/storyClosed.ts +++ b/src/core/server/services/comments/pipeline/phases/storyClosed.ts @@ -1,5 +1,5 @@ import { StoryClosedError } from "coral-server/errors"; -import { getStoryClosedAt } from "coral-server/models/story"; +import { isStoryClosed } from "coral-server/models/story"; import { IntermediatePhaseResult, ModerationPhaseContext, @@ -14,8 +14,7 @@ export const storyClosed = ({ ModerationPhaseContext, "story" | "tenant" | "now" >): IntermediatePhaseResult | void => { - const closedAt = getStoryClosedAt(tenant, story); - if (closedAt && closedAt <= now) { + if (isStoryClosed(tenant, story, now)) { throw new StoryClosedError(); } };