From 88f6cad840a1c35ea12bae4cddaf5a59952bbb43 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 6 Dec 2019 18:46:54 +0000 Subject: [PATCH] fix: resolved issue with incorrect moderation url being generated (#2743) --- src/core/server/app/url.ts | 18 +++++ src/core/server/graph/tenant/context.ts | 6 +- .../server/graph/tenant/resolvers/util.ts | 16 +--- src/core/server/services/slack/context.ts | 23 ++++-- src/core/server/services/slack/publisher.ts | 74 ++++++++++--------- 5 files changed, 82 insertions(+), 55 deletions(-) diff --git a/src/core/server/app/url.ts b/src/core/server/app/url.ts index 25bdc8e0e..32402f859 100644 --- a/src/core/server/app/url.ts +++ b/src/core/server/app/url.ts @@ -29,6 +29,24 @@ export function constructTenantURL( return url.href; } +export function reconstructTenantURL( + config: Config, + tenant: Pick, + req?: Request, + path = "/" +) { + // If the request is available, then prefer it over building from the tenant + // as the tenant does not include the port number. This should only really + // be a problem if the graph API is called internally. + if (req) { + return reconstructURL(req, path); + } + + // Note that when constructing the callback url with the tenant, the port + // information is lost. + return constructTenantURL(config, tenant, path); +} + export function doesRequireSchemePrefixing(url: string) { return !url.startsWith("http"); } diff --git a/src/core/server/graph/tenant/context.ts b/src/core/server/graph/tenant/context.ts index ed46f9de3..a9e16d650 100644 --- a/src/core/server/graph/tenant/context.ts +++ b/src/core/server/graph/tenant/context.ts @@ -61,7 +61,11 @@ export default class TenantContext extends CommonContext { this.clientID = options.clientID; this.publisher = createPublisher({ pubsub: this.pubsub, - slackPublisher: createSlackPublisher(this.mongo, this.tenant), + slackPublisher: createSlackPublisher( + this.mongo, + this.config, + this.tenant + ), notifierQueue, tenantID: this.tenant.id, clientID: this.clientID, diff --git a/src/core/server/graph/tenant/resolvers/util.ts b/src/core/server/graph/tenant/resolvers/util.ts index f4dba3138..23c914c5c 100644 --- a/src/core/server/graph/tenant/resolvers/util.ts +++ b/src/core/server/graph/tenant/resolvers/util.ts @@ -2,7 +2,7 @@ import { GraphQLResolveInfo } from "graphql"; import graphqlFields from "graphql-fields"; import { pull } from "lodash"; -import { constructTenantURL, reconstructURL } from "coral-server/app/url"; +import { reconstructTenantURL } from "coral-server/app/url"; import TenantContext from "../context"; @@ -16,16 +16,6 @@ export function getRequestedFields(info: GraphQLResolveInfo) { } export function reconstructTenantURLResolver(path: string) { - return (parent: T, args: {}, ctx: TenantContext) => { - // If the request is available, then prefer it over building from the tenant - // as the tenant does not include the port number. This should only really - // be a problem if the graph API is called internally. - if (ctx.req) { - return reconstructURL(ctx.req, path); - } - - // Note that when constructing the callback url with the tenant, the port - // information is lost. - return constructTenantURL(ctx.config, ctx.tenant, path); - }; + return (parent: T, args: {}, ctx: TenantContext) => + reconstructTenantURL(ctx.config, ctx.tenant, ctx.req, path); } diff --git a/src/core/server/services/slack/context.ts b/src/core/server/services/slack/context.ts index 24acfc1de..6122b0d99 100644 --- a/src/core/server/services/slack/context.ts +++ b/src/core/server/services/slack/context.ts @@ -2,44 +2,53 @@ import Logger from "bunyan"; import DataLoader from "dataloader"; import { Db } from "mongodb"; +import { Config } from "coral-server/config"; import { Comment, retrieveManyComments } from "coral-server/models/comment"; import { retrieveManyStories, Story } from "coral-server/models/story"; +import { Tenant } from "coral-server/models/tenant"; import { retrieveManyUsers, User } from "coral-server/models/user"; +import { Request } from "coral-server/types/express"; interface Options { mongo: Db; - tenantID: string; + tenant: Pick; + config: Config; + req?: Request; } class SlackContext { public readonly mongo: Db; - public readonly tenantID: string; + public readonly tenant: Pick; public readonly logger: Logger; + public readonly config: Config; + public readonly req?: Request; public readonly comments: DataLoader< string, Readonly | null > = new DataLoader(commentIDs => - retrieveManyComments(this.mongo, this.tenantID, commentIDs) + retrieveManyComments(this.mongo, this.tenant.id, commentIDs) ); public readonly stories: DataLoader< string, Readonly | null > = new DataLoader(storyIDs => - retrieveManyStories(this.mongo, this.tenantID, storyIDs) + retrieveManyStories(this.mongo, this.tenant.id, storyIDs) ); public readonly users: DataLoader< string, Readonly | null > = new DataLoader(userIDs => - retrieveManyUsers(this.mongo, this.tenantID, userIDs) + retrieveManyUsers(this.mongo, this.tenant.id, userIDs) ); - constructor({ mongo, tenantID }: Options) { + constructor({ mongo, tenant, config, req }: Options) { this.mongo = mongo; - this.tenantID = tenantID; + this.tenant = tenant; + this.config = config; + this.req = req; } } diff --git a/src/core/server/services/slack/publisher.ts b/src/core/server/services/slack/publisher.ts index 9501c8310..68d1c7a7f 100644 --- a/src/core/server/services/slack/publisher.ts +++ b/src/core/server/services/slack/publisher.ts @@ -1,3 +1,7 @@ +import { Db } from "mongodb"; + +import { reconstructTenantURL } from "coral-server/app/url"; +import { Config } from "coral-server/config"; import { CommentCreatedInput } from "coral-server/graph/tenant/resolvers/Subscription/commentCreated"; import { CommentEnteredModerationQueueInput } from "coral-server/graph/tenant/resolvers/Subscription/commentEnteredModerationQueue"; import { CommentFeaturedInput } from "coral-server/graph/tenant/resolvers/Subscription/commentFeatured"; @@ -7,11 +11,15 @@ import { CommentReplyCreatedInput } from "coral-server/graph/tenant/resolvers/Su import { CommentStatusUpdatedInput } from "coral-server/graph/tenant/resolvers/Subscription/commentStatusUpdated"; import { SUBSCRIPTION_CHANNELS } from "coral-server/graph/tenant/resolvers/Subscription/types"; import logger from "coral-server/logger"; +import { getLatestRevision } from "coral-server/models/comment/helpers"; +import { + getStoryTitle, + getURLWithCommentID, +} from "coral-server/models/story/helpers"; +import { Tenant } from "coral-server/models/tenant"; import { GQLMODERATION_QUEUE } from "coral-server/graph/tenant/schema/__generated__/types"; -import { Tenant } from "coral-server/models/tenant"; -import { Db } from "mongodb"; import SlackContext from "./context"; type Payload = @@ -55,21 +63,19 @@ function isPending(channel: SUBSCRIPTION_CHANNELS, payload: Payload) { ); } -function createModerationLink(rootURL: string, commentID: string) { - return `${rootURL}/admin/moderate/comment/${commentID}`; -} - -function createCommentLink(storyURL: string, commentID: string) { - const urlBuilder = new URL(storyURL); - urlBuilder.searchParams.set("commentID", commentID); - return urlBuilder.href; +function createModerationLink(ctx: SlackContext, commentID: string) { + return reconstructTenantURL( + ctx.config, + ctx.tenant, + ctx.req, + `/admin/moderate/comment/${commentID}` + ); } async function postCommentToSlack( ctx: SlackContext, - orgURL: string, commentID: string, - webhookURL: string + hookURL: string ) { const comment = await ctx.comments.load(commentID); if (comment === null) { @@ -84,14 +90,14 @@ async function postCommentToSlack( return; } - const storyTitle = story.metadata ? story.metadata.title : ""; - const commentBody = - comment.revisions.length > 0 - ? comment.revisions[comment.revisions.length - 1].body - : ""; + // Get some properties about the event. + const storyTitle = getStoryTitle(story); + const commentBody = getLatestRevision(comment).body; + const moderateLink = createModerationLink(ctx, commentID); + const commentLink = getURLWithCommentID(story.url, comment.id); + + // Replace HTML link breaks with newlines. const body = commentBody.replace(//g, "\n"); - const moderateLink = createModerationLink(orgURL, comment.id); - const commentLink = createCommentLink(story.url, comment.id); const data = { text: `${author.username} commented on: ${storyTitle}`, @@ -110,9 +116,7 @@ async function postCommentToSlack( text: `<${moderateLink}|Go to Moderation> | <${commentLink}|See Comment>`, }, }, - { - type: "divider", - }, + { type: "divider" }, { type: "section", text: { @@ -124,17 +128,17 @@ async function postCommentToSlack( }; try { - const response = await fetch(webhookURL, { + // Send the post to the Slack URL. + const res = await fetch(hookURL, { method: "POST", - cache: "no-cache", headers: { "Content-Type": "application/json", }, body: JSON.stringify(data), }); - if (response.ok) { - logger.error({ response }, "error sending Slack comment"); + if (!res.ok) { + logger.error({ res }, "error sending Slack comment"); } } catch (err) { logger.error({ err }, "error sending Slack comment"); @@ -146,7 +150,11 @@ export type SlackPublisher = ( payload: Payload ) => Promise; -function createSlackPublisher(mongo: Db, tenant: Tenant): SlackPublisher { +function createSlackPublisher( + mongo: Db, + config: Config, + tenant: Tenant +): SlackPublisher { if ( !tenant.slack || !tenant.slack.channels || @@ -160,7 +168,7 @@ function createSlackPublisher(mongo: Db, tenant: Tenant): SlackPublisher { const { channels } = tenant.slack; return async (channel: SUBSCRIPTION_CHANNELS, payload: Payload) => { - const ctx = new SlackContext({ mongo, tenantID: tenant.id }); + const ctx = new SlackContext({ mongo, config, tenant }); try { const inModeration = enteredModeration(channel, payload); @@ -186,19 +194,17 @@ function createSlackPublisher(mongo: Db, tenant: Tenant): SlackPublisher { return; } - const orgURL = tenant.organization.url; - if ( triggers.allComments && (reported || pending || featured || inModeration) ) { - await postCommentToSlack(ctx, orgURL, commentID, hookURL); + await postCommentToSlack(ctx, commentID, hookURL); } else if (triggers.reportedComments && reported) { - await postCommentToSlack(ctx, orgURL, commentID, hookURL); + await postCommentToSlack(ctx, commentID, hookURL); } else if (triggers.pendingComments && pending) { - await postCommentToSlack(ctx, orgURL, commentID, hookURL); + await postCommentToSlack(ctx, commentID, hookURL); } else if (triggers.featuredComments && featured) { - await postCommentToSlack(ctx, orgURL, commentID, hookURL); + await postCommentToSlack(ctx, commentID, hookURL); } } } catch (err) {