From fa00862777b9f70f833a510eb12fcf00d45db12d Mon Sep 17 00:00:00 2001 From: nick-funk Date: Tue, 7 Mar 2023 13:56:24 -0700 Subject: [PATCH 01/17] update story url if valid id is provided During the retrieval of a comment stream via the createStreamEmbed, if the site provides a valid storyID that matches a real story but the url it is coming from differs from the one on record, we update the story url. This prevents numerous "DUPLICATE_STORY_ID" errors when newsrooms modify their story URL multiple times in their CMS due to article title changes, slug changes, etc. --- src/core/common/errors.ts | 6 ++++ src/core/server/errors/index.ts | 11 +++++++ src/core/server/models/story/story.ts | 40 +++++++++++++++++++++-- src/core/server/services/stories/index.ts | 10 +++++- 4 files changed, 63 insertions(+), 4 deletions(-) diff --git a/src/core/common/errors.ts b/src/core/common/errors.ts index 930f8c23d2..30129f35e3 100644 --- a/src/core/common/errors.ts +++ b/src/core/common/errors.ts @@ -419,4 +419,10 @@ export enum ERROR_CODES { * their username and the provided username has already been taken. */ USERNAME_ALREADY_EXISTS = "USERNAME_ALREADY_EXISTS", + + /** + * UNABLE_TO_UPDATE_STORY_URL is thrown when a story already exists for + * a storyID and we were unable to update the url for that story ID. + */ + UNABLE_TO_UPDATE_STORY_URL = "UNABLE_TO_UPDATE_STORY_URL", } diff --git a/src/core/server/errors/index.ts b/src/core/server/errors/index.ts index 1430e1fa47..99e8925d78 100644 --- a/src/core/server/errors/index.ts +++ b/src/core/server/errors/index.ts @@ -970,3 +970,14 @@ export class UsernameAlreadyExists extends CoralError { }); } } + +export class UnableToUpdateStoryURL extends CoralError { + constructor(cause: MongoError, id: string, oldUrl: string, url: string) { + super({ + cause, + reportable: true, + code: ERROR_CODES.UNABLE_TO_UPDATE_STORY_URL, + context: { pvt: { id, oldUrl, url } }, + }); + } +} diff --git a/src/core/server/models/story/story.ts b/src/core/server/models/story/story.ts index 3eabf164e9..57ae03267e 100644 --- a/src/core/server/models/story/story.ts +++ b/src/core/server/models/story/story.ts @@ -8,6 +8,7 @@ import { DuplicateStoryIDError, DuplicateStoryURLError, StoryNotFoundError, + UnableToUpdateStoryURL, } from "coral-server/errors"; import { Connection, @@ -125,13 +126,46 @@ export interface UpsertStoryResult { export async function upsertStory( mongo: MongoContext, tenantID: string, - { id = uuid(), url, mode, siteID }: UpsertStoryInput, + { id, url, mode, siteID }: UpsertStoryInput, now = new Date() ): Promise { + if (id) { + const existingStory = await mongo + .stories() + .findOne({ tenantID, siteID, id }); + if (existingStory && url && existingStory.url !== url) { + const result = await mongo.stories().findOneAndUpdate( + { tenantID, siteID, id }, + { + $set: { + url, + }, + }, + { returnOriginal: false } + ); + + if (!result.ok || !result.value) { + throw new UnableToUpdateStoryURL( + result.lastErrorObject, + id, + existingStory.url, + url + ); + } + + return { + story: result.value, + wasUpserted: true, + }; + } + } + + const newID = id ? id : uuid(); + // Create the story, optionally sourcing the id from the input, additionally // porting in the tenantID. const story: Story = { - id, + id: newID, url, tenantID, siteID, @@ -176,7 +210,7 @@ export async function upsertStory( // Evaluate the error, if it is in regards to violating the unique index, // then return a duplicate Story error. if (err instanceof MongoError && err.code === 11000) { - throw new DuplicateStoryIDError(err, id, url); + throw new DuplicateStoryIDError(err, newID, url); } throw err; diff --git a/src/core/server/services/stories/index.ts b/src/core/server/services/stories/index.ts index 760e3c322f..80be87b81f 100644 --- a/src/core/server/services/stories/index.ts +++ b/src/core/server/services/stories/index.ts @@ -7,6 +7,7 @@ import { MongoContext } from "coral-server/data/context"; import { CannotMergeAnArchivedStory, CannotOpenAnArchivedStory, + StoryNotFoundError, StoryURLInvalidError, UserNotFoundError, } from "coral-server/errors"; @@ -94,7 +95,14 @@ export async function findOrCreate( } let siteID = null; - if (input.url) { + if (input.id) { + const story = await findStory(mongo, tenant.id, { id: input.id }); + if (story) { + siteID = story.siteID; + } + } + + if (input.url && siteID === null) { const site = await findSiteByURL(mongo, tenant.id, input.url); // If the URL is provided, and the url is not associated with a site, then refuse // to create the Asset. From 8fe87a70eb05f6466ae7a3d9bab2bbcac2f45b21 Mon Sep 17 00:00:00 2001 From: nick-funk Date: Tue, 7 Mar 2023 14:09:57 -0700 Subject: [PATCH 02/17] remove unused import --- src/core/server/services/stories/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/core/server/services/stories/index.ts b/src/core/server/services/stories/index.ts index 80be87b81f..eb6491b947 100644 --- a/src/core/server/services/stories/index.ts +++ b/src/core/server/services/stories/index.ts @@ -7,7 +7,6 @@ import { MongoContext } from "coral-server/data/context"; import { CannotMergeAnArchivedStory, CannotOpenAnArchivedStory, - StoryNotFoundError, StoryURLInvalidError, UserNotFoundError, } from "coral-server/errors"; From b55d104f6de9d54522dc8d935a8dfb4018eac7b8 Mon Sep 17 00:00:00 2001 From: nick-funk Date: Tue, 7 Mar 2023 14:11:46 -0700 Subject: [PATCH 03/17] update error translations: new UNABLE_TO_UPDATE_STORY_URL code --- src/core/server/errors/translations.ts | 1 + src/core/server/locales/en-US/errors.ftl | 1 + 2 files changed, 2 insertions(+) diff --git a/src/core/server/errors/translations.ts b/src/core/server/errors/translations.ts index 4cd63fb8e3..a5e3c16c4c 100644 --- a/src/core/server/errors/translations.ts +++ b/src/core/server/errors/translations.ts @@ -76,4 +76,5 @@ export const ERROR_TRANSLATIONS: Record = { CANNOT_OPEN_AN_ARCHIVED_STORY: "error-cannotOpenAnArchivedStory", CANNOT_MERGE_AN_ARCHIVED_STORY: "error-cannotMergeAnArchivedStory", USERNAME_ALREADY_EXISTS: "error-usernameAlreadyExists", + UNABLE_TO_UPDATE_STORY_URL: "error-unableToUpdateStoryURL", }; diff --git a/src/core/server/locales/en-US/errors.ftl b/src/core/server/locales/en-US/errors.ftl index b710851346..ffb272a668 100644 --- a/src/core/server/locales/en-US/errors.ftl +++ b/src/core/server/locales/en-US/errors.ftl @@ -76,3 +76,4 @@ error-cannotCreateCommentOnArchivedStory = Cannot create a comment on an archive error-cannotOpenAnArchivedStory = Cannot open an archived story. The story must be unarchived first. error-cannotMergeAnArchivedStory = Cannot merge an archived story. The story must be unarchived first. error-usernameAlreadyExists = This username already exists. Please choose another. +error-unableToUpdateStoryURL = Unable to update the story URL. From ef45709710246087c976873bf91d9c20eb97c220 Mon Sep 17 00:00:00 2001 From: nick-funk Date: Thu, 16 Mar 2023 09:39:14 -0600 Subject: [PATCH 04/17] allow admins to demote other admins --- src/core/client/admin/permissions/user.ts | 2 +- src/core/common/permissions/rules.ts | 2 +- src/core/common/permissions/validateRoleChange.ts | 5 ----- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/core/client/admin/permissions/user.ts b/src/core/client/admin/permissions/user.ts index 20bfd41212..649823bb0d 100644 --- a/src/core/client/admin/permissions/user.ts +++ b/src/core/client/admin/permissions/user.ts @@ -17,7 +17,7 @@ interface PermissionContext { const permissionMap: PermissionMap = { CHANGE_ROLE: { - [GQLUSER_ROLE.ADMIN]: (ctx) => ctx?.user.role !== GQLUSER_ROLE.ADMIN, + [GQLUSER_ROLE.ADMIN]: () => true, [GQLUSER_ROLE.MODERATOR]: (ctx) => { if (!ctx) { return true; diff --git a/src/core/common/permissions/rules.ts b/src/core/common/permissions/rules.ts index dfcf840485..c1135952b0 100644 --- a/src/core/common/permissions/rules.ts +++ b/src/core/common/permissions/rules.ts @@ -6,7 +6,7 @@ import { } from "./types"; // admins can do whatever they want to anyone except other admins const adminsArePowerful: PermissionsActionRuleTest = ({ viewer, user }) => ({ - applies: viewer.role === "ADMIN" && user.role !== "ADMIN", + applies: viewer.role === "ADMIN", reason: "Admins may change any non admin's role or scopes", }); diff --git a/src/core/common/permissions/validateRoleChange.ts b/src/core/common/permissions/validateRoleChange.ts index 84622f555d..c1eb0bb190 100644 --- a/src/core/common/permissions/validateRoleChange.ts +++ b/src/core/common/permissions/validateRoleChange.ts @@ -48,11 +48,6 @@ export const validateRoleChange = ( */ scoped = false ): boolean => { - // User is admin - if (user.role === "ADMIN") { - return false; - } - // Viewer is changing their own role if (user.id === viewer.id) { return false; From cedf1084e73190bcd6767c02a4100611a9007661 Mon Sep 17 00:00:00 2001 From: nick-funk Date: Thu, 16 Mar 2023 10:16:44 -0600 Subject: [PATCH 05/17] add tests around new admin demotion functionality --- .../test/community/community-rtl.spec.tsx | 133 ++++++++++++++---- src/core/client/admin/test/fixtures.ts | 8 ++ 2 files changed, 114 insertions(+), 27 deletions(-) diff --git a/src/core/client/admin/test/community/community-rtl.spec.tsx b/src/core/client/admin/test/community/community-rtl.spec.tsx index 75cad75f93..f34810c250 100644 --- a/src/core/client/admin/test/community/community-rtl.spec.tsx +++ b/src/core/client/admin/test/community/community-rtl.spec.tsx @@ -12,12 +12,10 @@ import { pureMerge } from "coral-common/utils"; import { GQLResolver, GQLUSER_ROLE, - GQLUsersConnection, QueryToSettingsResolver, QueryToUsersResolver, } from "coral-framework/schema"; import { - createFixture, createQueryResolverStub, createResolversStub, CreateTestRendererParams, @@ -260,31 +258,6 @@ it("change user role", async () => { expect(resolvers.Mutation!.updateUserRole!.called).toBe(true); }); -it("no one may change an admins role", async () => { - const resolvers = createResolversStub({ - Query: { - users: createQueryResolverStub(() => - createFixture({ - edges: users.admins, - nodes: users.admins, - }) - ), - }, - }); - await createTestRenderer({ - resolvers, - }); - - const container = await screen.findByTestId("community-container"); - expect(container).toBeDefined(); - expect(within(container).getAllByRole("row").length).toEqual( - users.admins.length - ); - expect( - within(container).queryByLabelText("Change role") - ).not.toBeInTheDocument(); -}); - it("org mods may allocate site mods", async () => { const orgModerator = users.moderators[0]; const commenter = users.commenters[0]; @@ -625,6 +598,112 @@ it("allows admins to promote site mods to org mod", async () => { ); }); +it("change user role", async () => { + const user = users.commenters[0]; + const resolvers = createResolversStub({ + Mutation: { + updateUserRole: ({ variables }) => { + expectAndFail(variables).toMatchObject({ + userID: user.id, + role: GQLUSER_ROLE.STAFF, + }); + const userRecord = pureMerge(user, { + role: variables.role, + }); + return { + user: userRecord, + }; + }, + }, + }); + await createTestRenderer({ + resolvers, + }); + + const userRow = await screen.findByTestId(`community-row-${user.id}`); + const changeRoleButton = within(userRow).getByLabelText("Change role"); + userEvent.click(changeRoleButton); + + const popup = within(userRow).getByLabelText( + "A dropdown to change the user role" + ); + const staffButton = await within(popup).findByRole("button", { + name: "Staff", + }); + fireEvent.click(staffButton); + + expect(resolvers.Mutation!.updateUserRole!.called).toBe(true); +}); + +it("only admins can demote other admins", async () => { + const viewer = users.moderators[0]; + const adminUser = users.admins[1]; + + const resolvers = createResolversStub({ + Query: { + viewer: () => viewer, + settings: () => settingsWithMultisite, + } + }); + await createTestRenderer({ + resolvers, + }); + + const userRow = await screen.findByTestId( + `community-row-${adminUser.id}` + ); + + const changeRoleButton = within(userRow).queryByLabelText("Change role"); + expect(changeRoleButton).toBeNull(); +}); + +it("allow admins to demote other admins", async () => { + const viewer = users.admins[0]; + const adminUser = users.admins[1]; + + const resolvers = createResolversStub({ + Query: { + viewer: () => viewer, + settings: () => settingsWithMultisite, + }, + Mutation: { + updateUserRole: () => { + const userRecord = pureMerge( + adminUser, + { + role: GQLUSER_ROLE.MODERATOR, + moderationScopes: undefined, + } + ); + return { + user: userRecord, + }; + }, + }, + }); + await createTestRenderer({ + resolvers, + }); + + const userRow = await screen.findByTestId( + `community-row-${adminUser.id}` + ); + const changeRoleButton = within(userRow).getByLabelText("Change role"); + userEvent.click(changeRoleButton); + + const popup = within(userRow).getByLabelText( + "A dropdown to change the user role" + ); + const orgModButton = within(popup).getByRole("button", { + name: "Organization Moderator", + }); + fireEvent.click(orgModButton); + + await waitFor(() => + expect(resolvers.Mutation!.updateUserRole!.called).toBe(true) + ); +}); + // BOOKMARK: marcus, i think we just need to add a new field to these fixtures, maybe scopes.sites.id? it("load more", async () => { await createTestRenderer({ diff --git a/src/core/client/admin/test/fixtures.ts b/src/core/client/admin/test/fixtures.ts index b9ad89a357..b7e1a52b82 100644 --- a/src/core/client/admin/test/fixtures.ts +++ b/src/core/client/admin/test/fixtures.ts @@ -447,6 +447,13 @@ export const users = { role: GQLUSER_ROLE.ADMIN, ignoreable: false, }, + { + id: "user-admin-1", + username: "Not Markus", + email: "not-markus@test.com", + role: GQLUSER_ROLE.ADMIN, + ignoreable: false, + }, ], baseUser ), @@ -1273,6 +1280,7 @@ export const commentFlagsNoDetails = createFixtures([ export const communityUsers = createFixture({ edges: [ { node: users.admins[0], cursor: users.admins[0].createdAt }, + { node: users.admins[1], cursor: users.admins[1].createdAt }, { node: users.moderators[0], cursor: users.moderators[0].createdAt }, { node: users.moderators[1], cursor: users.moderators[1].createdAt }, { node: users.moderators[2], cursor: users.moderators[2].createdAt }, From 27336e68b5827ea0fbbbb22566f757244993242e Mon Sep 17 00:00:00 2001 From: nick-funk Date: Fri, 17 Mar 2023 09:54:42 -0600 Subject: [PATCH 06/17] fix linting --- .../test/community/community-rtl.spec.tsx | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/src/core/client/admin/test/community/community-rtl.spec.tsx b/src/core/client/admin/test/community/community-rtl.spec.tsx index f34810c250..7a1152ad2e 100644 --- a/src/core/client/admin/test/community/community-rtl.spec.tsx +++ b/src/core/client/admin/test/community/community-rtl.spec.tsx @@ -643,15 +643,13 @@ it("only admins can demote other admins", async () => { Query: { viewer: () => viewer, settings: () => settingsWithMultisite, - } + }, }); await createTestRenderer({ resolvers, }); - const userRow = await screen.findByTestId( - `community-row-${adminUser.id}` - ); + const userRow = await screen.findByTestId(`community-row-${adminUser.id}`); const changeRoleButton = within(userRow).queryByLabelText("Change role"); expect(changeRoleButton).toBeNull(); @@ -668,13 +666,10 @@ it("allow admins to demote other admins", async () => { }, Mutation: { updateUserRole: () => { - const userRecord = pureMerge( - adminUser, - { - role: GQLUSER_ROLE.MODERATOR, - moderationScopes: undefined, - } - ); + const userRecord = pureMerge(adminUser, { + role: GQLUSER_ROLE.MODERATOR, + moderationScopes: undefined, + }); return { user: userRecord, }; @@ -685,9 +680,7 @@ it("allow admins to demote other admins", async () => { resolvers, }); - const userRow = await screen.findByTestId( - `community-row-${adminUser.id}` - ); + const userRow = await screen.findByTestId(`community-row-${adminUser.id}`); const changeRoleButton = within(userRow).getByLabelText("Change role"); userEvent.click(changeRoleButton); From 43248e126c90c85f24c024e5e30b11313cf4f1cd Mon Sep 17 00:00:00 2001 From: nick-funk Date: Tue, 21 Mar 2023 09:38:34 -0600 Subject: [PATCH 07/17] update comment on admin permissions --- src/core/common/permissions/rules.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/common/permissions/rules.ts b/src/core/common/permissions/rules.ts index c1135952b0..196c55566f 100644 --- a/src/core/common/permissions/rules.ts +++ b/src/core/common/permissions/rules.ts @@ -4,7 +4,7 @@ import { isSiteModerator, PermissionsActionRuleTest, } from "./types"; -// admins can do whatever they want to anyone except other admins +// admins can perform any action, even demoting other admins const adminsArePowerful: PermissionsActionRuleTest = ({ viewer, user }) => ({ applies: viewer.role === "ADMIN", reason: "Admins may change any non admin's role or scopes", From cad9533848026ce067397bf11cf68a661b685d2f Mon Sep 17 00:00:00 2001 From: nick-funk Date: Tue, 21 Mar 2023 09:40:01 -0600 Subject: [PATCH 08/17] update reason text for admin's performing a role change --- src/core/common/permissions/rules.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/common/permissions/rules.ts b/src/core/common/permissions/rules.ts index 196c55566f..a5b29990ba 100644 --- a/src/core/common/permissions/rules.ts +++ b/src/core/common/permissions/rules.ts @@ -7,7 +7,7 @@ import { // admins can perform any action, even demoting other admins const adminsArePowerful: PermissionsActionRuleTest = ({ viewer, user }) => ({ applies: viewer.role === "ADMIN", - reason: "Admins may change any non admin's role or scopes", + reason: "Admins may change any user's role or scopes", }); // org mods can promote anyone < site mod to be an unscoped member From d48d2440a8e04131a05d38ff00ce7a52f76b2d97 Mon Sep 17 00:00:00 2001 From: nick-funk Date: Tue, 21 Mar 2023 10:21:15 -0600 Subject: [PATCH 09/17] localize user fixtures for community tests --- .../admin/test/community/community-rtl.spec.tsx | 16 +++++++++++++++- src/core/client/admin/test/fixtures.ts | 1 - 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/core/client/admin/test/community/community-rtl.spec.tsx b/src/core/client/admin/test/community/community-rtl.spec.tsx index 7a1152ad2e..0d3a3210af 100644 --- a/src/core/client/admin/test/community/community-rtl.spec.tsx +++ b/src/core/client/admin/test/community/community-rtl.spec.tsx @@ -11,6 +11,7 @@ import customRenderAppWithContext from "coral-admin/test/customRenderAppWithCont import { pureMerge } from "coral-common/utils"; import { GQLResolver, + GQLUsersConnection, GQLUSER_ROLE, QueryToSettingsResolver, QueryToUsersResolver, @@ -24,7 +25,6 @@ import { import { createContext } from "../create"; import { - communityUsers, disabledEmail, disabledLocalAuth, disabledLocalAuthAdminTargetFilter, @@ -36,6 +36,20 @@ import { sites, users, } from "../fixtures"; +import { createFixture } from "coral-framework/testHelpers"; + +export const communityUsers = createFixture({ + edges: [ + { node: users.admins[0], cursor: users.admins[0].createdAt }, + { node: users.admins[1], cursor: users.admins[1].createdAt }, + { node: users.moderators[0], cursor: users.moderators[0].createdAt }, + { node: users.moderators[1], cursor: users.moderators[1].createdAt }, + { node: users.moderators[2], cursor: users.moderators[2].createdAt }, + { node: users.staff[0], cursor: users.staff[0].createdAt }, + { node: users.commenters[0], cursor: users.commenters[0].createdAt }, + ], + pageInfo: { endCursor: null, hasNextPage: false }, +}); const adminViewer = users.admins[0]; diff --git a/src/core/client/admin/test/fixtures.ts b/src/core/client/admin/test/fixtures.ts index b7e1a52b82..a9a5e87fff 100644 --- a/src/core/client/admin/test/fixtures.ts +++ b/src/core/client/admin/test/fixtures.ts @@ -1280,7 +1280,6 @@ export const commentFlagsNoDetails = createFixtures([ export const communityUsers = createFixture({ edges: [ { node: users.admins[0], cursor: users.admins[0].createdAt }, - { node: users.admins[1], cursor: users.admins[1].createdAt }, { node: users.moderators[0], cursor: users.moderators[0].createdAt }, { node: users.moderators[1], cursor: users.moderators[1].createdAt }, { node: users.moderators[2], cursor: users.moderators[2].createdAt }, From c9115f1eec30e8f66fa54a4bf6b76561a546c058 Mon Sep 17 00:00:00 2001 From: nick-funk Date: Tue, 21 Mar 2023 11:55:02 -0600 Subject: [PATCH 10/17] sort imports --- src/core/client/admin/test/community/community-rtl.spec.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/client/admin/test/community/community-rtl.spec.tsx b/src/core/client/admin/test/community/community-rtl.spec.tsx index 0d3a3210af..99972c5d57 100644 --- a/src/core/client/admin/test/community/community-rtl.spec.tsx +++ b/src/core/client/admin/test/community/community-rtl.spec.tsx @@ -11,12 +11,13 @@ import customRenderAppWithContext from "coral-admin/test/customRenderAppWithCont import { pureMerge } from "coral-common/utils"; import { GQLResolver, - GQLUsersConnection, GQLUSER_ROLE, + GQLUsersConnection, QueryToSettingsResolver, QueryToUsersResolver, } from "coral-framework/schema"; import { + createFixture, createQueryResolverStub, createResolversStub, CreateTestRendererParams, @@ -36,7 +37,6 @@ import { sites, users, } from "../fixtures"; -import { createFixture } from "coral-framework/testHelpers"; export const communityUsers = createFixture({ edges: [ From 8e1432db9cdcaabaa864d641aa0e1aacf4958778 Mon Sep 17 00:00:00 2001 From: nick-funk Date: Tue, 21 Mar 2023 14:22:58 -0600 Subject: [PATCH 11/17] add `sectionOverride` option to scraper --- src/core/server/services/stories/scraper/rules/section.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/core/server/services/stories/scraper/rules/section.ts b/src/core/server/services/stories/scraper/rules/section.ts index 28ba158895..4edc2c3424 100644 --- a/src/core/server/services/stories/scraper/rules/section.ts +++ b/src/core/server/services/stories/scraper/rules/section.ts @@ -5,6 +5,9 @@ import { wrap } from "./helpers"; export const sectionScraper = (): RuleBundle => ({ section: [ + // an override for newsrooms using section differently than + // the default way + wrap(($) => $(`meta[property="sectionOverride"]`).attr("content")), // From: http://ogp.me/#type_article wrap($jsonld("articleSection")), wrap(($) => $('meta[itemprop="articleSection"]').attr("content")), From 3e4d0e26da511b55729f61e1189bb25a45602829 Mon Sep 17 00:00:00 2001 From: nick-funk Date: Tue, 18 Apr 2023 11:57:41 -0600 Subject: [PATCH 12/17] define DATA_CACHE feature flag --- src/core/server/graph/schema/schema.graphql | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/core/server/graph/schema/schema.graphql b/src/core/server/graph/schema/schema.graphql index 6b43969552..f37f549cb6 100644 --- a/src/core/server/graph/schema/schema.graphql +++ b/src/core/server/graph/schema/schema.graphql @@ -561,6 +561,12 @@ enum FEATURE_FLAG { an external profile for a user. """ CONFIGURE_PUBLIC_PROFILE_URL + + """ + DATA_CACHE will allow the use of the data cache to store and load comments, + users, and commentActions more quickly. It is disabled by default. + """ + DATA_CACHE } # The moderation mode of the site. From ec05e3a4e9988cd2d242763332ac032113e070e2 Mon Sep 17 00:00:00 2001 From: nick-funk Date: Tue, 18 Apr 2023 12:15:10 -0600 Subject: [PATCH 13/17] add availability check to data caches --- .../server/data/cache/commentActionsCache.ts | 11 +++++- .../server/data/cache/commentCache.spec.ts | 1 + src/core/server/data/cache/commentCache.ts | 12 +++++- src/core/server/data/cache/dataCache.ts | 38 ++++++++++++++++++- src/core/server/data/cache/userCache.ts | 11 +++++- src/core/server/graph/context.ts | 1 + src/core/server/queue/tasks/loadCache.ts | 1 + src/core/server/queue/tasks/rejector.ts | 1 + 8 files changed, 71 insertions(+), 5 deletions(-) diff --git a/src/core/server/data/cache/commentActionsCache.ts b/src/core/server/data/cache/commentActionsCache.ts index 51fc1eefc6..62ac41c637 100644 --- a/src/core/server/data/cache/commentActionsCache.ts +++ b/src/core/server/data/cache/commentActionsCache.ts @@ -1,14 +1,17 @@ import { Logger } from "coral-server/logger"; import { CommentAction } from "coral-server/models/action/comment"; import { AugmentedRedis } from "coral-server/services/redis"; +import { TenantCache } from "coral-server/services/tenant/cache"; import { MongoContext } from "../context"; +import { dataCacheAvailable, IDataCache } from "./dataCache"; -export class CommentActionsCache { +export class CommentActionsCache implements IDataCache { private expirySeconds: number; private mongo: MongoContext; private redis: AugmentedRedis; + private tenantCache: TenantCache | null; private logger: Logger; private commentActionsByKey: Map>; @@ -16,11 +19,13 @@ export class CommentActionsCache { constructor( mongo: MongoContext, redis: AugmentedRedis, + tenantCache: TenantCache | null, logger: Logger, expirySeconds: number ) { this.mongo = mongo; this.redis = redis; + this.tenantCache = tenantCache; this.logger = logger.child({ dataCache: "CommentActionsCache" }); this.expirySeconds = expirySeconds; @@ -28,6 +33,10 @@ export class CommentActionsCache { this.commentActionsByKey = new Map>(); } + public async available(tenantID: string): Promise { + return dataCacheAvailable(this.tenantCache, tenantID); + } + private computeDataKey(tenantID: string, id: string) { const key = `${tenantID}:${id}:commentActionData`; return key; diff --git a/src/core/server/data/cache/commentCache.spec.ts b/src/core/server/data/cache/commentCache.spec.ts index a04400d0dd..15b07d3ece 100644 --- a/src/core/server/data/cache/commentCache.spec.ts +++ b/src/core/server/data/cache/commentCache.spec.ts @@ -44,6 +44,7 @@ const createFixtures = async ( const commentCache = new CommentCache( mongo, redis, + null, logger, false, options?.expirySeconds ? options.expirySeconds : 5 * 60 diff --git a/src/core/server/data/cache/commentCache.ts b/src/core/server/data/cache/commentCache.ts index 6af9325449..5215ea1854 100644 --- a/src/core/server/data/cache/commentCache.ts +++ b/src/core/server/data/cache/commentCache.ts @@ -3,6 +3,7 @@ import { Logger } from "coral-server/logger"; import { Comment } from "coral-server/models/comment"; import { PUBLISHED_STATUSES } from "coral-server/models/comment/constants"; import { AugmentedRedis } from "coral-server/services/redis"; +import { TenantCache } from "coral-server/services/tenant/cache"; import { GQLCOMMENT_SORT, @@ -10,6 +11,8 @@ import { GQLTAG, } from "coral-server/graph/schema/__generated__/types"; +import { dataCacheAvailable, IDataCache } from "./dataCache"; + const REDIS_SUPPORTED_SORTS = [ GQLCOMMENT_SORT.CREATED_AT_ASC, GQLCOMMENT_SORT.CREATED_AT_DESC, @@ -21,7 +24,7 @@ export interface Filter { statuses?: GQLCOMMENT_STATUS[]; } -export class CommentCache { +export class CommentCache implements IDataCache { private disableLocalCaching: boolean; private expirySeconds: number; @@ -31,16 +34,19 @@ export class CommentCache { private commentsByKey: Map>; private membersLookup: Map; + private tenantCache: TenantCache | null; constructor( mongo: MongoContext, redis: AugmentedRedis, + tenantCache: TenantCache | null, logger: Logger, disableLocalCaching: boolean, expirySeconds: number ) { this.mongo = mongo; this.redis = redis; + this.tenantCache = tenantCache; this.logger = logger.child({ dataCache: "CommentCache" }); @@ -51,6 +57,10 @@ export class CommentCache { this.membersLookup = new Map(); } + public async available(tenantID: string): Promise { + return dataCacheAvailable(this.tenantCache, tenantID); + } + public computeLockKey(tenantID: string, storyID: string) { const key = `${tenantID}:${storyID}:lock`; return key; diff --git a/src/core/server/data/cache/dataCache.ts b/src/core/server/data/cache/dataCache.ts index ad37f72ce4..471e0dc539 100644 --- a/src/core/server/data/cache/dataCache.ts +++ b/src/core/server/data/cache/dataCache.ts @@ -1,16 +1,41 @@ import { MongoContext } from "coral-server/data/context"; import { Logger } from "coral-server/logger"; +import { hasFeatureFlag } from "coral-server/models/tenant"; import { AugmentedRedis } from "coral-server/services/redis"; -import { CommentActionsCache } from "./commentActionsCache"; +import { TenantCache } from "coral-server/services/tenant/cache"; + +import { GQLFEATURE_FLAG } from "core/client/framework/schema/__generated__/types"; +import { CommentActionsCache } from "./commentActionsCache"; import { CommentCache } from "./commentCache"; import { UserCache } from "./userCache"; export const DEFAULT_DATA_EXPIRY_SECONDS = 24 * 60 * 60; -export class DataCache { +export const dataCacheAvailable = async ( + tenantCache: TenantCache | null, + tenantID: string +): Promise => { + if (!tenantCache) { + return false; + } + + const tenant = await tenantCache.retrieveByID(tenantID); + if (!tenant) { + return false; + } + + return hasFeatureFlag(tenant, GQLFEATURE_FLAG.DATA_CACHE); +}; + +export interface IDataCache { + available(tenantID: string): Promise; +} + +export class DataCache implements IDataCache { private mongo: MongoContext; private redis: AugmentedRedis; + private tenantCache: TenantCache; private logger: Logger; private expirySeconds: number; @@ -24,18 +49,21 @@ export class DataCache { constructor( mongo: MongoContext, redis: AugmentedRedis, + tenantCache: TenantCache, logger: Logger, disableCaching?: boolean, expirySeconds: number = DEFAULT_DATA_EXPIRY_SECONDS ) { this.mongo = mongo; this.redis = redis; + this.tenantCache = tenantCache; this.logger = logger.child({ traceID: this.traceID }); this.expirySeconds = expirySeconds; this.comments = new CommentCache( this.mongo, this.redis, + this.tenantCache, this.logger, Boolean(disableCaching), this.expirySeconds @@ -43,14 +71,20 @@ export class DataCache { this.commentActions = new CommentActionsCache( this.mongo, this.redis, + this.tenantCache, this.logger, this.expirySeconds ); this.users = new UserCache( this.mongo, this.redis, + this.tenantCache, this.logger, this.expirySeconds ); } + + public async available(tenantID: string): Promise { + return dataCacheAvailable(this.tenantCache, tenantID); + } } diff --git a/src/core/server/data/cache/userCache.ts b/src/core/server/data/cache/userCache.ts index 5716b06bcf..66d492415e 100644 --- a/src/core/server/data/cache/userCache.ts +++ b/src/core/server/data/cache/userCache.ts @@ -2,14 +2,17 @@ import { UserNotFoundError } from "coral-server/errors"; import { Logger } from "coral-server/logger"; import { User } from "coral-server/models/user"; import { AugmentedRedis } from "coral-server/services/redis"; +import { TenantCache } from "coral-server/services/tenant/cache"; import { MongoContext } from "../context"; +import { dataCacheAvailable, IDataCache } from "./dataCache"; -export class UserCache { +export class UserCache implements IDataCache { private expirySeconds: number; private mongo: MongoContext; private redis: AugmentedRedis; + private tenantCache: TenantCache | null; private logger: Logger; private usersByKey: Map>; @@ -17,11 +20,13 @@ export class UserCache { constructor( mongo: MongoContext, redis: AugmentedRedis, + tenantCache: TenantCache | null, logger: Logger, expirySeconds: number ) { this.mongo = mongo; this.redis = redis; + this.tenantCache = tenantCache; this.logger = logger.child({ dataCache: "UserCache" }); this.expirySeconds = expirySeconds; @@ -29,6 +34,10 @@ export class UserCache { this.usersByKey = new Map>(); } + public async available(tenantID: string): Promise { + return dataCacheAvailable(this.tenantCache, tenantID); + } + private computeDataKey(tenantID: string, id: string) { const key = `${tenantID}:${id}:userData`; return key; diff --git a/src/core/server/graph/context.ts b/src/core/server/graph/context.ts index 2c9424669c..5fc0f6c074 100644 --- a/src/core/server/graph/context.ts +++ b/src/core/server/graph/context.ts @@ -140,6 +140,7 @@ export default class GraphContext { this.cache = new DataCache( this.mongo, this.redis, + this.tenantCache, this.logger, this.disableCaching, this.config.get("redis_cache_expiry") / 1000 diff --git a/src/core/server/queue/tasks/loadCache.ts b/src/core/server/queue/tasks/loadCache.ts index d1e5bf125c..c9232d0603 100644 --- a/src/core/server/queue/tasks/loadCache.ts +++ b/src/core/server/queue/tasks/loadCache.ts @@ -46,6 +46,7 @@ const createJobProcessor = const { comments, commentActions, users } = new DataCache( mongo, redis, + tenantCache, log, false, config.get("redis_cache_expiry") / 1000 diff --git a/src/core/server/queue/tasks/rejector.ts b/src/core/server/queue/tasks/rejector.ts index 9b5b4246ea..08967ca444 100644 --- a/src/core/server/queue/tasks/rejector.ts +++ b/src/core/server/queue/tasks/rejector.ts @@ -223,6 +223,7 @@ const createJobProcessor = const cache = new DataCache( mongo, redis, + tenantCache, log, false, config.get("redis_cache_expiry") / 1000 From 27f0563991bcc6da772b224ed97acc2a7c19200a Mon Sep 17 00:00:00 2001 From: nick-funk Date: Tue, 18 Apr 2023 12:45:27 -0600 Subject: [PATCH 14/17] prevent usage of data cache unless it is enabled on tenant --- src/core/server/graph/loaders/Comments.ts | 19 +++ src/core/server/graph/resolvers/Comment.ts | 116 ++++++++++++------ .../models/seenComments/seenComments.ts | 12 +- src/core/server/queue/tasks/loadCache.ts | 14 ++- src/core/server/services/comments/actions.ts | 20 ++- src/core/server/services/stories/index.ts | 10 +- src/core/server/stacks/approveComment.ts | 9 +- src/core/server/stacks/createComment.ts | 19 +-- src/core/server/stacks/editComment.ts | 9 +- src/core/server/stacks/rejectComment.ts | 5 +- 10 files changed, 171 insertions(+), 62 deletions(-) diff --git a/src/core/server/graph/loaders/Comments.ts b/src/core/server/graph/loaders/Comments.ts index d36396145e..176fde3447 100644 --- a/src/core/server/graph/loaders/Comments.ts +++ b/src/core/server/graph/loaders/Comments.ts @@ -19,6 +19,7 @@ import { hasFeatureFlag, Tenant } from "coral-server/models/tenant"; import { User } from "coral-server/models/user"; import { retrieveAllCommentsUserConnection, + retrieveChildrenForParentConnection, retrieveCommentConnection, retrieveCommentParentsConnection, retrieveCommentRepliesConnection, @@ -325,8 +326,10 @@ export default (ctx: GraphContext) => ({ } const isArchived = !!(story.isArchived || story.isArchiving); + const cacheAvailable = await ctx.cache.available(ctx.tenant.id); if ( + !cacheAvailable || isRatingsAndReviews(ctx.tenant, story) || isQA(ctx.tenant, story) || isArchived @@ -386,8 +389,10 @@ export default (ctx: GraphContext) => ({ } const isArchived = !!(story.isArchived || story.isArchiving); + const cacheAvailable = await ctx.cache.available(ctx.tenant.id); if ( + !cacheAvailable || isRatingsAndReviews(ctx.tenant, story) || isQA(ctx.tenant, story) || isArchived @@ -456,6 +461,20 @@ export default (ctx: GraphContext) => ({ throw new StoryNotFoundError(comment.storyID); } + const cacheAvailable = await ctx.cache.available(ctx.tenant.id); + if (!cacheAvailable) { + return retrieveChildrenForParentConnection( + ctx.mongo, + ctx.tenant.id, + comment, + { + first: 9999, + orderBy: defaultTo(orderBy, GQLCOMMENT_SORT.CREATED_AT_ASC), + }, + story.isArchived + ).then(primeCommentsFromConnection(ctx)); + } + const conn = await ctx.cache.comments.allChildComments( ctx.tenant.id, story.id, diff --git a/src/core/server/graph/resolvers/Comment.ts b/src/core/server/graph/resolvers/Comment.ts index 40043e744d..94c5370cec 100644 --- a/src/core/server/graph/resolvers/Comment.ts +++ b/src/core/server/graph/resolvers/Comment.ts @@ -15,11 +15,15 @@ import { getDepth, getLatestRevision, hasAncestors, + hasPublishedStatus, } from "coral-server/models/comment/helpers"; import { createConnection } from "coral-server/models/helpers"; import { getURLWithCommentID } from "coral-server/models/story"; import { canModerate } from "coral-server/models/user/helpers"; -import { getCommentEditableUntilDate } from "coral-server/services/comments"; +import { + getCommentEditableUntilDate, + hasRejectedAncestors, +} from "coral-server/services/comments"; import { GQLComment, @@ -45,18 +49,21 @@ export const maybeLoadOnlyID = async ( }; } - const cachedComment = await ctx.cache.comments.find( - ctx.tenant.id, - storyID, - id - ); - if ( - cachedComment !== null && - PUBLISHED_STATUSES.includes(cachedComment.status) - ) { - return cachedComment; - } else if (cachedComment !== null) { - return null; + const cacheAvailable = await ctx.cache.available(ctx.tenant.id); + if (cacheAvailable) { + const cachedComment = await ctx.cache.comments.find( + ctx.tenant.id, + storyID, + id + ); + if ( + cachedComment !== null && + PUBLISHED_STATUSES.includes(cachedComment.status) + ) { + return cachedComment; + } else if (cachedComment !== null) { + return null; + } } // We want more than the ID! Get the comment! @@ -81,16 +88,33 @@ export const Comment: GQLCommentTypeResolver = { return canModerate(ctx.user, { siteID: c.siteID }); }, canReply: async (c, input, ctx) => { - const ancestors = await ctx.cache.comments.findAncestors( - c.tenantID, - c.storyID, - c.id - ); - const rejected = ancestors.filter( - (a) => a.status === GQLCOMMENT_STATUS.REJECTED - ); + const cacheAvailable = await ctx.cache.available(ctx.tenant.id); + if (cacheAvailable) { + const ancestors = await ctx.cache.comments.findAncestors( + c.tenantID, + c.storyID, + c.id + ); + const rejected = ancestors.filter( + (a) => a.status === GQLCOMMENT_STATUS.REJECTED + ); + + return rejected.length === 0; + } else { + const story = await ctx.loaders.Stories.find.load({ id: c.storyID }); + if (!story || story.isArchived || story.isArchiving) { + return false; + } - return rejected.length === 0; + const rejectedAncestors = await hasRejectedAncestors( + ctx.mongo, + ctx.tenant.id, + c.id, + story.isArchived || story.isArchiving + ); + + return !rejectedAncestors; + } }, deleted: ({ deletedAt }) => !!deletedAt, revisionHistory: (c) => @@ -107,12 +131,17 @@ export const Comment: GQLCommentTypeResolver = { ? getCommentEditableUntilDate(ctx.tenant, createdAt) : null, }), - author: (c, input, ctx) => { + author: async (c, input, ctx) => { if (!c.authorID) { return null; } - return ctx.cache.users.findUser(c.tenantID, c.authorID); + const cacheAvailable = await ctx.cache.available(ctx.tenant.id); + if (cacheAvailable) { + return ctx.cache.users.findUser(c.tenantID, c.authorID); + } else { + return ctx.loaders.Users.user.load(c.authorID); + } }, statusHistory: ({ id }, input, ctx) => ctx.loaders.CommentModerationActions.forComment(input, id), @@ -128,14 +157,24 @@ export const Comment: GQLCommentTypeResolver = { return 0; } - const children = await ctx.cache.comments.findMany( - ctx.tenant.id, - storyID, - childIDs - ); - const count = children.filter((c) => c !== null).length; + const cacheAvailable = await ctx.cache.available(ctx.tenant.id); + + if (cacheAvailable) { + const children = await ctx.cache.comments.findMany( + ctx.tenant.id, + storyID, + childIDs + ); + const count = children.filter((c) => c !== null).length; - return count; + return count; + } else { + const children = await ctx.loaders.Comments.visible.loadMany(childIDs); + return children.reduce( + (sum: number, c: any) => (c && hasPublishedStatus(c) ? sum + 1 : sum), + 0 + ); + } }, // Action Counts are encoded, decode them for use with the GraphQL system. actionCounts: (c) => decodeActionCounts(c.actionCounts), @@ -181,10 +220,17 @@ export const Comment: GQLCommentTypeResolver = { ) : null, parent: async (c, input, ctx, info) => { - const parent = c.parentID - ? await ctx.cache.comments.find(c.tenantID, c.storyID, c.parentID) - : null; - return parent; + const cacheAvailable = await ctx.cache.available(ctx.tenant.id); + if (cacheAvailable) { + const parent = c.parentID + ? await ctx.cache.comments.find(c.tenantID, c.storyID, c.parentID) + : null; + return parent; + } else { + return hasAncestors(c) + ? maybeLoadOnlyID(ctx, info, c.storyID, c.parentID) + : null; + } }, parents: (c, input, ctx) => // Some resolver optimization. diff --git a/src/core/server/models/seenComments/seenComments.ts b/src/core/server/models/seenComments/seenComments.ts index 07cb9ace3e..d4568f364c 100644 --- a/src/core/server/models/seenComments/seenComments.ts +++ b/src/core/server/models/seenComments/seenComments.ts @@ -90,12 +90,22 @@ export async function markSeenComments( now: Date, markAllAsSeen?: boolean ) { + const cacheAvailable = await cache.available(tenantID); + let comments; - if (markAllAsSeen) { + if (markAllAsSeen && cacheAvailable) { const markAllCommentIDs = ( await cache.comments.findAllCommentsForStory(tenantID, storyID, false) ).map((comment) => comment.id); comments = reduceCommentIDs(markAllCommentIDs, now); + } else if (markAllAsSeen) { + const markAllCommentIDs = ( + await mongo + .comments() + .find({ tenantID, storyID }, { projection: { id: 1 } }) + .toArray() + ).map((comment) => comment.id); + comments = reduceCommentIDs(markAllCommentIDs, now); } else { comments = reduceCommentIDs(commentIDs, now); } diff --git a/src/core/server/queue/tasks/loadCache.ts b/src/core/server/queue/tasks/loadCache.ts index c9232d0603..96274409e3 100644 --- a/src/core/server/queue/tasks/loadCache.ts +++ b/src/core/server/queue/tasks/loadCache.ts @@ -1,7 +1,10 @@ import { QueueOptions } from "bull"; import { Config } from "coral-server/config"; -import { DataCache } from "coral-server/data/cache/dataCache"; +import { + DataCache, + dataCacheAvailable, +} from "coral-server/data/cache/dataCache"; import { MongoContext } from "coral-server/data/context"; import { createTimer } from "coral-server/helpers"; import logger from "coral-server/logger"; @@ -42,6 +45,15 @@ const createJobProcessor = true ); + const cacheAvailable = await dataCacheAvailable(tenantCache, tenantID); + if (!cacheAvailable) { + log.info( + { tenantID }, + "skipping load cache as caching is not enabled on tenant" + ); + return; + } + const timer = createTimer(); const { comments, commentActions, users } = new DataCache( mongo, diff --git a/src/core/server/services/comments/actions.ts b/src/core/server/services/comments/actions.ts index f73057c820..e5571e3588 100644 --- a/src/core/server/services/comments/actions.ts +++ b/src/core/server/services/comments/actions.ts @@ -240,8 +240,11 @@ export async function removeCommentAction( throw new Error("could not update comment action counts"); } - await cache.commentActions.remove(action); - await cache.comments.update(updatedComment); + const cacheAvailable = await cache.available(tenant.id); + if (cacheAvailable) { + await cache.commentActions.remove(action); + await cache.comments.update(updatedComment); + } // Update the comment counts onto other documents. const counts = await updateAllCommentCounts(mongo, redis, { @@ -294,8 +297,11 @@ export async function createReaction( now ); if (action) { - await cache.commentActions.add(action); - await cache.comments.update(comment); + const cacheAvailable = await cache.available(tenant.id); + if (cacheAvailable) { + await cache.commentActions.add(action); + await cache.comments.update(comment); + } // A comment reaction was created! Publish it. publishCommentReactionCreated( @@ -363,7 +369,8 @@ export async function createDontAgree( now ); - if (action) { + const cacheAvailable = await commentActionsCache.available(tenant.id); + if (action && cacheAvailable) { await commentActionsCache.add(action); } @@ -426,7 +433,8 @@ export async function createFlag( now ); if (action) { - if (action) { + const cacheAvailable = await commentActionsCache.available(tenant.id); + if (cacheAvailable) { await commentActionsCache.add(action); } diff --git a/src/core/server/services/stories/index.ts b/src/core/server/services/stories/index.ts index 3941e4abac..6941101080 100644 --- a/src/core/server/services/stories/index.ts +++ b/src/core/server/services/stories/index.ts @@ -147,10 +147,12 @@ export async function findOrCreate( }); } - const storyIsCached = await cache.isCached(tenant.id, story.id); - - if (!storyIsCached && !story.isArchived && !story.isArchiving) { - await queue.add({ tenantID: tenant.id, storyID: story.id }); + const cacheAvailable = await cache.available(tenant.id); + if (cacheAvailable) { + const storyIsCached = await cache.isCached(tenant.id, story.id); + if (!storyIsCached && !story.isArchived && !story.isArchiving) { + await queue.add({ tenantID: tenant.id, storyID: story.id }); + } } if (tenant.stories.scraping.enabled && !story.metadata && !story.scrapedAt) { diff --git a/src/core/server/stacks/approveComment.ts b/src/core/server/stacks/approveComment.ts index f6dc4d65fe..28b230f182 100644 --- a/src/core/server/stacks/approveComment.ts +++ b/src/core/server/stacks/approveComment.ts @@ -67,9 +67,12 @@ const approveComment = async ( }); } - await cache.comments.update(result.after); - if (result.after.authorID) { - await cache.users.populateUsers(tenant.id, [result.after.authorID]); + const cacheAvailable = await cache.available(tenant.id); + if (cacheAvailable) { + await cache.comments.update(result.after); + if (result.after.authorID) { + await cache.users.populateUsers(tenant.id, [result.after.authorID]); + } } // Return the resulting comment. diff --git a/src/core/server/stacks/createComment.ts b/src/core/server/stacks/createComment.ts index fcc28f5ec5..8989bd46ee 100644 --- a/src/core/server/stacks/createComment.ts +++ b/src/core/server/stacks/createComment.ts @@ -443,16 +443,19 @@ export default async function create( after: comment, }); - await cache.comments.update(comment); - if (comment.authorID) { - await cache.users.populateUsers(tenant.id, [comment.authorID]); - } + const cacheAvailable = await cache.available(tenant.id); + if (cacheAvailable) { + await cache.comments.update(comment); + if (comment.authorID) { + await cache.users.populateUsers(tenant.id, [comment.authorID]); + } - if (parent) { - await cache.comments.update(parent); + if (parent) { + await cache.comments.update(parent); - if (parent.authorID) { - await cache.users.populateUsers(tenant.id, [parent.authorID]); + if (parent.authorID) { + await cache.users.populateUsers(tenant.id, [parent.authorID]); + } } } diff --git a/src/core/server/stacks/editComment.ts b/src/core/server/stacks/editComment.ts index ad8b3a1926..a94fd7270f 100644 --- a/src/core/server/stacks/editComment.ts +++ b/src/core/server/stacks/editComment.ts @@ -273,9 +273,12 @@ export default async function edit( ...result, }); - await cache.comments.update(result.after); - if (result.after.authorID) { - await cache.users.populateUsers(tenant.id, [result.after.authorID]); + const cacheAvailable = await cache.available(tenant.id); + if (cacheAvailable) { + await cache.comments.update(result.after); + if (result.after.authorID) { + await cache.users.populateUsers(tenant.id, [result.after.authorID]); + } } // Publish changes to the event publisher. diff --git a/src/core/server/stacks/rejectComment.ts b/src/core/server/stacks/rejectComment.ts index fab6c77244..51608f5261 100644 --- a/src/core/server/stacks/rejectComment.ts +++ b/src/core/server/stacks/rejectComment.ts @@ -101,7 +101,10 @@ const rejectComment = async ( return tagResult; } - await cache.comments.remove(result.after); + const cacheAvailable = await cache.available(tenant.id); + if (cacheAvailable) { + await cache.comments.remove(result.after); + } // Return the resulting comment. return result.after; From 76b8bed8a29dffb1e5d8fc7d178c45282564fa3a Mon Sep 17 00:00:00 2001 From: nick-funk Date: Wed, 19 Apr 2023 10:17:08 -0600 Subject: [PATCH 15/17] block cache mutations when not available on tenant --- src/core/common/errors.ts | 6 ++++++ src/core/server/errors/index.ts | 9 +++++++++ src/core/server/errors/translations.ts | 1 + src/core/server/graph/mutators/Stories.ts | 15 ++++++++++++++- src/core/server/locales/en-US/errors.ftl | 1 + 5 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/core/common/errors.ts b/src/core/common/errors.ts index 4b83378796..107a84436c 100644 --- a/src/core/common/errors.ts +++ b/src/core/common/errors.ts @@ -425,4 +425,10 @@ export enum ERROR_CODES { * their username and the provided username has already been taken. */ USERNAME_ALREADY_EXISTS = "USERNAME_ALREADY_EXISTS", + + /** + * DATA_CACHING_NOT_AVAILABLE is thrown when someone tries to enact a data + * caching action when it is not available for that tenant. + */ + DATA_CACHING_NOT_AVAILABLE = "DATA_CACHING_NOT_AVAILABLE", } diff --git a/src/core/server/errors/index.ts b/src/core/server/errors/index.ts index fff4c570ff..9d31456322 100644 --- a/src/core/server/errors/index.ts +++ b/src/core/server/errors/index.ts @@ -999,3 +999,12 @@ export class UsernameAlreadyExists extends CoralError { }); } } + +export class DataCachingNotAvailableError extends CoralError { + constructor(tenantID: string) { + super({ + code: ERROR_CODES.DATA_CACHING_NOT_AVAILABLE, + context: { pub: { tenantID } }, + }); + } +} diff --git a/src/core/server/errors/translations.ts b/src/core/server/errors/translations.ts index 9afffc8459..06afe3da6d 100644 --- a/src/core/server/errors/translations.ts +++ b/src/core/server/errors/translations.ts @@ -77,4 +77,5 @@ export const ERROR_TRANSLATIONS: Record = { CANNOT_OPEN_AN_ARCHIVED_STORY: "error-cannotOpenAnArchivedStory", CANNOT_MERGE_AN_ARCHIVED_STORY: "error-cannotMergeAnArchivedStory", USERNAME_ALREADY_EXISTS: "error-usernameAlreadyExists", + DATA_CACHING_NOT_AVAILABLE: "error-dataCachingNotAvailable", }; diff --git a/src/core/server/graph/mutators/Stories.ts b/src/core/server/graph/mutators/Stories.ts index ab6acc7fb3..300c5d4a3e 100644 --- a/src/core/server/graph/mutators/Stories.ts +++ b/src/core/server/graph/mutators/Stories.ts @@ -1,7 +1,10 @@ import { isNull, omitBy } from "lodash"; import { ERROR_CODES } from "coral-common/errors"; -import { StoryNotFoundError } from "coral-server/errors"; +import { + DataCachingNotAvailableError, + StoryNotFoundError, +} from "coral-server/errors"; import GraphContext from "coral-server/graph/context"; import { mapFieldsetToErrorCodes } from "coral-server/graph/errors"; import { initializeCommentTagCountsForStory } from "coral-server/models/comment"; @@ -195,6 +198,11 @@ export const Stories = (ctx: GraphContext) => ({ } }, cacheStory: async (input: GQLCacheStoryInput) => { + const cacheAvailable = await ctx.cache.available(ctx.tenant.id); + if (!cacheAvailable) { + throw new DataCachingNotAvailableError(ctx.tenant.id); + } + const story = await ctx.loaders.Stories.find.load({ id: input.id }); if (!story) { throw new StoryNotFoundError(input.id); @@ -208,6 +216,11 @@ export const Stories = (ctx: GraphContext) => ({ return story; }, invalidateCachedStory: async (input: GQLCacheStoryInput) => { + const cacheAvailable = await ctx.cache.available(ctx.tenant.id); + if (!cacheAvailable) { + throw new DataCachingNotAvailableError(ctx.tenant.id); + } + const story = await ctx.loaders.Stories.find.load({ id: input.id }); if (!story) { throw new StoryNotFoundError(input.id); diff --git a/src/core/server/locales/en-US/errors.ftl b/src/core/server/locales/en-US/errors.ftl index a3affd20d3..d0b00c5b67 100644 --- a/src/core/server/locales/en-US/errors.ftl +++ b/src/core/server/locales/en-US/errors.ftl @@ -77,3 +77,4 @@ error-cannotCreateCommentOnArchivedStory = Cannot create a comment on an archive error-cannotOpenAnArchivedStory = Cannot open an archived story. The story must be unarchived first. error-cannotMergeAnArchivedStory = Cannot merge an archived story. The story must be unarchived first. error-usernameAlreadyExists = This username already exists. Please choose another. +error-dataCachingNotAvailable = Data caching is not available at this time. From 9ebc22174f649e9f7cb791125bd5c7850d845c8d Mon Sep 17 00:00:00 2001 From: Kathryn Beaty Date: Wed, 19 Apr 2023 13:05:39 -0400 Subject: [PATCH 16/17] update commentslinks to better work with customScrollContainer --- src/core/client/stream/common/scrollToBeginning.ts | 9 +++++---- .../tabs/Comments/Stream/CommentsLinks/CommentsLinks.tsx | 5 ++++- src/core/client/ui/helpers/getElementWindowTopOffset.ts | 6 +----- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/core/client/stream/common/scrollToBeginning.ts b/src/core/client/stream/common/scrollToBeginning.ts index f3bf2c4299..f173fa3612 100644 --- a/src/core/client/stream/common/scrollToBeginning.ts +++ b/src/core/client/stream/common/scrollToBeginning.ts @@ -6,11 +6,12 @@ function scrollToBeginning( customScrollContainer?: HTMLElement ) { const tab = root.getElementById("tab-COMMENTS"); - const scrollContainer = customScrollContainer ?? window; if (tab) { - scrollContainer.scrollTo({ - top: getElementWindowTopOffset(scrollContainer, tab), - }); + if (customScrollContainer) { + tab.scrollIntoView(); + } else { + window.scrollTo({ top: getElementWindowTopOffset(window, tab) }); + } } } diff --git a/src/core/client/stream/tabs/Comments/Stream/CommentsLinks/CommentsLinks.tsx b/src/core/client/stream/tabs/Comments/Stream/CommentsLinks/CommentsLinks.tsx index 1c0655bc60..0c8202676d 100644 --- a/src/core/client/stream/tabs/Comments/Stream/CommentsLinks/CommentsLinks.tsx +++ b/src/core/client/stream/tabs/Comments/Stream/CommentsLinks/CommentsLinks.tsx @@ -48,8 +48,11 @@ const CommentsLinks: FunctionComponent = ({ const { renderWindow, customScrollContainer } = useCoralContext(); const root = useShadowRootOrDocument(); const onGoToArticleTop = useCallback(() => { + if (customScrollContainer) { + customScrollContainer.scrollTo({ top: 0 }); + } renderWindow.scrollTo({ top: 0 }); - }, [renderWindow]); + }, [renderWindow, customScrollContainer]); const onGoToCommentsTop = useCallback(() => { scrollToBeginning(root, renderWindow, customScrollContainer); }, [root, renderWindow, customScrollContainer]); diff --git a/src/core/client/ui/helpers/getElementWindowTopOffset.ts b/src/core/client/ui/helpers/getElementWindowTopOffset.ts index 5229a184ed..fdb0eb5b3c 100644 --- a/src/core/client/ui/helpers/getElementWindowTopOffset.ts +++ b/src/core/client/ui/helpers/getElementWindowTopOffset.ts @@ -1,11 +1,7 @@ /** * Get elements top offset relative to the window. */ -function getElementWindowTopOffset( - window: Window | React.RefObject["current"], - element: Element -) { - // eslint-disable-next-line @typescript-eslint/restrict-plus-operands +function getElementWindowTopOffset(window: Window, element: Element) { return element.getBoundingClientRect().top + window.scrollY; } From ce02bc8d3765ef9d750ae1d7008480ec8972e8ad Mon Sep 17 00:00:00 2001 From: Tessa Thornton Date: Fri, 21 Apr 2023 10:27:32 -0400 Subject: [PATCH 17/17] update to 8.0.3 --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index a43fdbf211..9e5d89871d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@coralproject/talk", - "version": "8.0.2", + "version": "8.0.3", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@coralproject/talk", - "version": "8.0.2", + "version": "8.0.3", "license": "Apache-2.0", "dependencies": { "@ampproject/toolbox-cache-url": "^2.9.0", diff --git a/package.json b/package.json index 87c2eab445..11c9db3a4f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@coralproject/talk", - "version": "8.0.2", + "version": "8.0.3", "author": "The Coral Project", "homepage": "https://coralproject.net/", "sideEffects": [