From b646d99253657d06986dd8aa2ce0a6c2697e78f7 Mon Sep 17 00:00:00 2001 From: Amar Trebinjac <36768584+AmarTrebinjac@users.noreply.github.com> Date: Thu, 6 Feb 2025 13:55:36 +0100 Subject: [PATCH 1/2] feat: multi squad moderation (#2617) Enables a moderator to view moderation items from multiple squads at once, as well as bulk approve for multiple squads at once. --- __tests__/notifications/index.ts | 4 +- __tests__/posts.ts | 98 +++++++++++++----- src/common/post.ts | 164 ++++++++++++++++++++++++++++++- src/notifications/builder.ts | 6 +- src/notifications/generate.ts | 2 +- src/schema/posts.ts | 142 ++++---------------------- src/schema/sources.ts | 32 ++++++ 7 files changed, 299 insertions(+), 149 deletions(-) diff --git a/__tests__/notifications/index.ts b/__tests__/notifications/index.ts index 108078be9..ac8e83671 100644 --- a/__tests__/notifications/index.ts +++ b/__tests__/notifications/index.ts @@ -1295,7 +1295,7 @@ describe('storeNotificationBundle', () => { ); expect(actual.notification.description).toBeFalsy(); expect(actual.notification.targetUrl).toEqual( - 'http://localhost:5002/squads/a/moderate', + 'http://localhost:5002/squads/moderate', ); expect(actual.avatars).toEqual([ { @@ -1352,7 +1352,7 @@ describe('storeNotificationBundle', () => { ); expect(actual.notification.description).toEqual('Lacks value.'); expect(actual.notification.targetUrl).toEqual( - 'http://localhost:5002/squads/a/moderate', + 'http://localhost:5002/squads/moderate?handle=a', ); expect(actual.avatars).toEqual([ { diff --git a/__tests__/posts.ts b/__tests__/posts.ts index dd54ccf4a..a900fb2aa 100644 --- a/__tests__/posts.ts +++ b/__tests__/posts.ts @@ -153,18 +153,33 @@ beforeEach(async () => { name: 'Joanna Deer', }, ]); - await con.getRepository(SquadSource).save({ - id: 'm', - name: 'Moderated Squad', - image: 'http//image.com/m', - handle: 'moderatedSquad', - type: SourceType.Squad, - active: true, - private: false, - moderationRequired: true, - memberPostingRank: sourceRoleRank[SourceMemberRoles.Member], - memberInviteRank: sourceRoleRank[SourceMemberRoles.Member], - }); + await con.getRepository(SquadSource).save([ + { + id: 'm', + name: 'Moderated Squad', + image: 'http//image.com/m', + handle: 'moderatedSquad', + type: SourceType.Squad, + active: true, + private: false, + moderationRequired: true, + memberPostingRank: sourceRoleRank[SourceMemberRoles.Member], + memberInviteRank: sourceRoleRank[SourceMemberRoles.Member], + }, + { + id: 'm2', + name: 'Second Moderated Squad', + image: 'http//image.com/m2', + handle: 'moderatedSquad2', + type: SourceType.Squad, + active: true, + private: false, + moderationRequired: true, + memberPostingRank: sourceRoleRank[SourceMemberRoles.Member], + memberInviteRank: sourceRoleRank[SourceMemberRoles.Member], + }, + ]); + await con.getRepository(SourceMember).save([ { userId: '3', @@ -184,6 +199,12 @@ beforeEach(async () => { role: SourceMemberRoles.Member, referralToken: randomUUID(), }, + { + userId: '2', + sourceId: 'm2', + role: SourceMemberRoles.Moderator, + referralToken: randomUUID(), + }, ]); await deleteKeysByPattern(`${rateLimiterName}:*`); }); @@ -6361,7 +6382,7 @@ describe('query postCodeSnippets', () => { }); describe('Source post moderation approve/reject', () => { - const [pendingId, rejectedId] = Array.from({ length: 2 }, () => + const [pendingId, pendingId2, rejectedId] = Array.from({ length: 2 }, () => generateUUID(), ); beforeEach(async () => { @@ -6376,6 +6397,15 @@ describe('Source post moderation approve/reject', () => { status: SourcePostModerationStatus.Pending, type: PostType.Article, }, + { + id: pendingId2, + sourceId: 'm2', + createdById: '4', + title: 'Title', + content: 'Content', + status: SourcePostModerationStatus.Pending, + type: PostType.Article, + }, { id: rejectedId, sourceId: 'm', @@ -6395,7 +6425,7 @@ describe('Source post moderation approve/reject', () => { mutation ModerateSourcePost( $postIds: [ID]!, $status: String, - $sourceId: ID!, + $sourceId: ID, $rejectionReason: String, $moderatorMessage: String ) { @@ -6413,7 +6443,6 @@ describe('Source post moderation approve/reject', () => { mutation: MUTATION, variables: { postIds: [pendingId], - sourceId: 'm', status: SourcePostModerationStatus.Approved, }, }, @@ -6429,7 +6458,6 @@ describe('Source post moderation approve/reject', () => { mutation: MUTATION, variables: { postIds: [pendingId], - sourceId: 'm', status: SourcePostModerationStatus.Approved, }, }, @@ -6445,7 +6473,6 @@ describe('Source post moderation approve/reject', () => { mutation: MUTATION, variables: { postIds: [pendingId], - sourceId: 'm', status: SourcePostModerationStatus.Approved, }, }, @@ -6461,7 +6488,6 @@ describe('Source post moderation approve/reject', () => { }> = await client.mutate(MUTATION, { variables: { postIds: [pendingId], - sourceId: 'm', status: SourcePostModerationStatus.Approved, }, }); @@ -6476,6 +6502,38 @@ describe('Source post moderation approve/reject', () => { expect(post.moderatedById).toEqual('3'); }); + it('should not approve posts in sources where user is not moderator', async () => { + loggedUser = '2'; // Not moderator of "m" squad, which "pendingId" post belongs to. + + await testMutationErrorCode( + client, + { + mutation: MUTATION, + variables: { + postIds: [pendingId], + status: SourcePostModerationStatus.Approved, + }, + }, + 'FORBIDDEN', + ); + }); + + it('should throw error when one or more posts in postIds is from a source where user is not moderator', async () => { + loggedUser = '2'; // Not moderator of "m" squad, which "pendingId" post belongs to. + + await testMutationErrorCode( + client, + { + mutation: MUTATION, + variables: { + postIds: [pendingId, pendingId2], + status: SourcePostModerationStatus.Approved, + }, + }, + 'FORBIDDEN', + ); + }); + it('should reject pending posts', async () => { loggedUser = '3'; // Moderator level @@ -6484,7 +6542,6 @@ describe('Source post moderation approve/reject', () => { }> = await client.mutate(MUTATION, { variables: { postIds: [pendingId], - sourceId: 'm', status: SourcePostModerationStatus.Rejected, rejectionReason: 'Spam', moderatorMessage: 'This is spam', @@ -6512,7 +6569,6 @@ describe('Source post moderation approve/reject', () => { mutation: MUTATION, variables: { postIds: [pendingId], - sourceId: 'm', status: SourcePostModerationStatus.Rejected, moderatorMessage: 'This is spam', }, @@ -6530,7 +6586,6 @@ describe('Source post moderation approve/reject', () => { mutation: MUTATION, variables: { postIds: [pendingId], - sourceId: 'm', status: SourcePostModerationStatus.Rejected, rejectionReason: 'Other', }, @@ -6547,7 +6602,6 @@ describe('Source post moderation approve/reject', () => { }> = await client.mutate(MUTATION, { variables: { postIds: [pendingId, rejectedId], - sourceId: 'm', status: SourcePostModerationStatus.Approved, }, }); diff --git a/src/common/post.ts b/src/common/post.ts index 03dc00217..b3953c871 100644 --- a/src/common/post.ts +++ b/src/common/post.ts @@ -10,6 +10,8 @@ import { Post, PostOrigin, PostType, + Source, + SourceMember, SquadSource, User, validateCommentary, @@ -24,7 +26,7 @@ import { GQLPost } from '../schema/posts'; import { FileUpload } from 'graphql-upload/GraphQLUpload.js'; import { HttpError, retryFetchParse } from '../integrations/retry'; import { checkWithVordr, VordrFilterType } from './vordr'; -import { AuthContext } from '../Context'; +import { AuthContext, type Context } from '../Context'; import { createHash } from 'node:crypto'; import { PostCodeSnippet } from '../entity/posts/PostCodeSnippet'; import { logger } from '../logger'; @@ -42,6 +44,36 @@ import { } from '../entity/SourcePostModeration'; import { mapCloudinaryUrl, uploadPostFile, UploadPreset } from './cloudinary'; import { getMentions } from '../schema/comments'; +import type { ConnectionArguments } from 'graphql-relay'; +import graphorm from '../graphorm'; +import type { GraphQLResolveInfo } from 'graphql'; +import { offsetPageGenerator } from '../schema/common'; +import { SourceMemberRoles } from '../roles'; + +export type SourcePostModerationArgs = ConnectionArguments & { + sourceId: string; + status: SourcePostModerationStatus[]; +}; + +export interface GQLSourcePostModeration { + id: string; + title?: string; + content?: string; + contentHtml?: string; + image?: string; + sourceId: string; + sharedPostId?: string; + status: SourcePostModerationStatus; + createdAt: Date; + updatedAt: Date; + source: Source; + post?: Post; + postId?: string; +} + +const POST_MODERATION_PAGE_SIZE = 15; +const sourcePostModerationPageGenerator = + offsetPageGenerator(POST_MODERATION_PAGE_SIZE, 50); export const defaultImage = { urls: @@ -693,3 +725,133 @@ export const getPostSmartTitle = ( contentLanguage, (post.contentMeta as PostContentMeta)?.alt_title?.translations, ) || getPostTranslatedTitle(post, contentLanguage); + +export const getModerationItemsAsAdminForSource = async ( + ctx: Context, + info: GraphQLResolveInfo, + args: SourcePostModerationArgs, +) => { + const page = sourcePostModerationPageGenerator.connArgsToPage(args); + const statuses = Array.from(new Set(args.status)); + + return graphorm.queryPaginated( + ctx, + info, + (nodeSize) => + sourcePostModerationPageGenerator.hasPreviousPage(page, nodeSize), + (nodeSize) => sourcePostModerationPageGenerator.hasNextPage(page, nodeSize), + (node, index) => + sourcePostModerationPageGenerator.nodeToCursor(page, args, node, index), + (builder) => { + builder.queryBuilder + .where(`"${builder.alias}"."sourceId" = :sourceId`, { + sourceId: args.sourceId, + }) + .andWhere(`("${builder.alias}"."flags"->>'vordr')::boolean IS NOT TRUE`) + .orderBy(`${builder.alias}.updatedAt`, 'DESC') + .limit(page.limit) + .offset(page.offset); + + if (statuses.length) { + builder.queryBuilder.andWhere( + `"${builder.alias}"."status" IN (:...status)`, + { + status: statuses, + }, + ); + } + + return builder; + }, + undefined, + true, + ); +}; + +export const getModerationItemsByUserForSource = async ( + ctx: Context, + info: GraphQLResolveInfo, + args: SourcePostModerationArgs, +) => { + const page = sourcePostModerationPageGenerator.connArgsToPage(args); + const { userId } = ctx; + const statuses = Array.from(new Set(args.status)); + + return graphorm.queryPaginated( + ctx, + info, + (nodeSize) => + sourcePostModerationPageGenerator.hasPreviousPage(page, nodeSize), + (nodeSize) => sourcePostModerationPageGenerator.hasNextPage(page, nodeSize), + (node, index) => + sourcePostModerationPageGenerator.nodeToCursor(page, args, node, index), + (builder) => { + builder.queryBuilder + .where(`"${builder.alias}"."sourceId" = :sourceId`, { + sourceId: args.sourceId, + }) + .andWhere(`"${builder.alias}"."createdById" = :userId`, { + userId, + }) + .orderBy(`${builder.alias}.updatedAt`, 'DESC') + .limit(page.limit) + .offset(page.offset); + + if (statuses.length) { + builder.queryBuilder.andWhere( + `"${builder.alias}"."status" IN (:...status)`, + { + status: statuses, + }, + ); + } + + return builder; + }, + undefined, + true, + ); +}; + +export const getAllModerationItemsAsAdmin = async ( + ctx: Context, + info: GraphQLResolveInfo, + args: SourcePostModerationArgs, +) => { + const page = sourcePostModerationPageGenerator.connArgsToPage(args); + const { userId } = ctx; + const statuses = Array.from(new Set(args.status)); + + return graphorm.queryPaginated( + ctx, + info, + (nodeSize) => + sourcePostModerationPageGenerator.hasPreviousPage(page, nodeSize), + (nodeSize) => sourcePostModerationPageGenerator.hasNextPage(page, nodeSize), + (node, index) => + sourcePostModerationPageGenerator.nodeToCursor(page, args, node, index), + (builder) => { + builder.queryBuilder + .innerJoin(SourceMember, 'sm', 'sm.userId = :userId', { userId }) + .where('sm.role IN (:...roles)', { + roles: [SourceMemberRoles.Admin, SourceMemberRoles.Moderator], + }) + .andWhere(`"${builder.alias}"."sourceId" = "sm"."sourceId"`) + .orderBy(`${builder.alias}.updatedAt`, 'DESC') + .limit(page.limit) + .offset(page.offset); + + if (statuses.length) { + builder.queryBuilder.andWhere( + `"${builder.alias}"."status" IN (:...status)`, + { + status: statuses, + }, + ); + } + return builder; + }, + undefined, + true, + ); +}; diff --git a/src/notifications/builder.ts b/src/notifications/builder.ts index 2752bfd8e..34dcedabd 100644 --- a/src/notifications/builder.ts +++ b/src/notifications/builder.ts @@ -212,9 +212,11 @@ export class NotificationBuilder { }); } - targetSourceModeration(source: Reference): NotificationBuilder { + targetSourceModeration(source?: Reference): NotificationBuilder { return this.enrichNotification({ - targetUrl: `${getSourceLink(source)}/moderate`, + targetUrl: source + ? `${process.env.COMMENTS_PREFIX}/squads/moderate?handle=${source.handle}` + : `${process.env.COMMENTS_PREFIX}/squads/moderate`, }); } diff --git a/src/notifications/generate.ts b/src/notifications/generate.ts index d887d704c..f8b6e1e63 100644 --- a/src/notifications/generate.ts +++ b/src/notifications/generate.ts @@ -183,7 +183,7 @@ export const generateNotificationMap: Record< .avatarSource(ctx.source) .avatarUser(ctx.user) .referencePostModeration(ctx.post) - .targetSourceModeration(ctx.source), + .targetSourceModeration(), community_picks_failed: (builder, ctx: NotificationSubmissionContext) => builder.systemNotification().referenceSubmission(ctx.submission), community_picks_succeeded: (builder, ctx: NotificationPostContext) => diff --git a/src/schema/posts.ts b/src/schema/posts.ts index ab06597d4..ab6607087 100644 --- a/src/schema/posts.ts +++ b/src/schema/posts.ts @@ -9,6 +9,7 @@ import { ensureSourcePermissions, GQLSource, isPrivilegedMember, + canModeratePosts, SourcePermissions, sourceTypesWithMembers, } from './sources'; @@ -40,6 +41,11 @@ import { validateSourcePostModeration, getPostTranslatedTitle, getPostSmartTitle, + getModerationItemsAsAdminForSource, + getModerationItemsByUserForSource, + type GQLSourcePostModeration, + type SourcePostModerationArgs, + getAllModerationItemsAsAdmin, } from '../common'; import { ArticlePost, @@ -108,28 +114,11 @@ import { SourcePostModerationStatus, } from '../entity/SourcePostModeration'; import { logger } from '../logger'; -import { Source } from '@dailydotdev/schema'; import { queryReadReplica } from '../common/queryReadReplica'; import { remoteConfig } from '../remoteConfig'; import { ensurePostRateLimit } from '../common/rateLimit'; import { whereNotUserBlocked } from '../common/contentPreference'; -export interface GQLSourcePostModeration { - id: string; - title?: string; - content?: string; - contentHtml?: string; - image?: string; - sourceId: string; - sharedPostId?: string; - status: SourcePostModerationStatus; - createdAt: Date; - updatedAt: Date; - source: Source; - post?: Post; - postId?: string; -} - export interface GQLPost { id: string; type: PostType; @@ -197,15 +186,7 @@ export type GQLPostSmartTitle = { title: string; }; -const POST_MODERATION_PAGE_SIZE = 15; const POST_MODERATION_LIMIT_FOR_MUTATION = 50; -const sourcePostModerationPageGenerator = - offsetPageGenerator(POST_MODERATION_PAGE_SIZE, 50); - -type SourcePostModerationArgs = ConnectionArguments & { - sourceId: string; - status: SourcePostModerationStatus[]; -}; export interface GQLPostUpvote { createdAt: Date; @@ -813,7 +794,7 @@ export const typeDefs = /* GraphQL */ ` """ Id of the source """ - sourceId: ID! + sourceId: ID """ Status of the moderation """ @@ -1246,7 +1227,7 @@ export const typeDefs = /* GraphQL */ ` """ Source to moderate the post in """ - sourceId: ID! + sourceId: ID """ Rejection reason for the post """ @@ -1458,95 +1439,14 @@ export const resolvers: IResolvers = traceResolvers< ctx: Context, info, ): Promise> => { - const isModerator = await isPrivilegedMember(ctx, args.sourceId); - const page = sourcePostModerationPageGenerator.connArgsToPage(args); - - const statuses = Array.from(new Set(args.status)); - - if (isModerator) { - return graphorm.queryPaginated( - ctx, - info, - (nodeSize) => - sourcePostModerationPageGenerator.hasPreviousPage(page, nodeSize), - (nodeSize) => - sourcePostModerationPageGenerator.hasNextPage(page, nodeSize), - (node, index) => - sourcePostModerationPageGenerator.nodeToCursor( - page, - args, - node, - index, - ), - (builder) => { - builder.queryBuilder - .where(`"${builder.alias}"."sourceId" = :sourceId`, { - sourceId: args.sourceId, - }) - .andWhere( - `("${builder.alias}"."flags"->>'vordr')::boolean IS NOT TRUE`, - ) - .orderBy(`${builder.alias}.updatedAt`, 'DESC') - .limit(page.limit) - .offset(page.offset); - - if (statuses.length) { - builder.queryBuilder.andWhere( - `"${builder.alias}"."status" IN (:...status)`, - { - status: statuses, - }, - ); - } - - return builder; - }, - undefined, - true, - ); + if (args?.sourceId) { + const isModerator = await isPrivilegedMember(ctx, args.sourceId); + if (isModerator) { + return getModerationItemsAsAdminForSource(ctx, info, args); + } + return getModerationItemsByUserForSource(ctx, info, args); } - const { userId } = ctx; - - return graphorm.queryPaginated( - ctx, - info, - (nodeSize) => - sourcePostModerationPageGenerator.hasPreviousPage(page, nodeSize), - (nodeSize) => - sourcePostModerationPageGenerator.hasNextPage(page, nodeSize), - (node, index) => - sourcePostModerationPageGenerator.nodeToCursor( - page, - args, - node, - index, - ), - (builder) => { - builder.queryBuilder - .where(`"${builder.alias}"."sourceId" = :sourceId`, { - sourceId: args.sourceId, - }) - .andWhere(`"${builder.alias}"."createdById" = :userId`, { - userId, - }) - .orderBy(`${builder.alias}.updatedAt`, 'DESC') - .limit(page.limit) - .offset(page.offset); - - if (statuses.length) { - builder.queryBuilder.andWhere( - `"${builder.alias}"."status" IN (:...status)`, - { - status: statuses, - }, - ); - } - - return builder; - }, - undefined, - true, - ); + return getAllModerationItemsAsAdmin(ctx, info, args); }, post: async ( source, @@ -2442,7 +2342,6 @@ export const resolvers: IResolvers = traceResolvers< moderateSourcePosts: async ( _, { - sourceId, postIds, status, rejectionReason = null, @@ -2480,16 +2379,17 @@ export const resolvers: IResolvers = traceResolvers< throw new ValidationError('Invalid status provided'); } - const isModerator = await isPrivilegedMember(ctx, sourceId); + const canModerate = await canModeratePosts(ctx, uniquePostIds); + + if (!canModerate) { + throw new ForbiddenError('Access denied!'); + } + const update: Partial = { status, moderatedById: ctx.userId, }; - if (!isModerator) { - throw new ForbiddenError('Access denied!'); - } - if (status === SourcePostModerationStatus.Rejected) { if (!rejectionReason) { throw new ValidationError('Rejection reason is required'); diff --git a/src/schema/sources.ts b/src/schema/sources.ts index ee098079d..aad69ad7d 100644 --- a/src/schema/sources.ts +++ b/src/schema/sources.ts @@ -982,6 +982,38 @@ export const canAccessSource = async ( return hasPermissionCheck(source, member, permission, validateRankAgainst); }; +export const canModeratePosts = async ( + ctx: Context, + moderationIds: string[], +): Promise => { + const posts = await ctx.con.getRepository(SourcePostModeration).find({ + where: { id: In(moderationIds) }, + select: ['sourceId'], + }); + + if (!posts.length) { + return false; + } + + const sourceIds = Array.from(new Set(posts.map((p) => p.sourceId))); + + const memberships = await ctx.con.getRepository(SourceMember).find({ + where: { + sourceId: In(sourceIds), + userId: ctx.userId, + }, + select: ['role'], + }); + + if (!memberships.length || memberships.length !== sourceIds.length) { + return false; + } + + return memberships.every( + (m) => sourceRoleRank[m.role] >= sourceRoleRank.moderator, + ); +}; + export const isPrivilegedMember = async ( ctx: Context, sourceId: string, From 378313fd214f7d40b3fc2adcbb93dda20a00d1cc Mon Sep 17 00:00:00 2001 From: Amar Trebinjac <36768584+AmarTrebinjac@users.noreply.github.com> Date: Thu, 6 Feb 2025 14:27:59 +0100 Subject: [PATCH 2/2] fix: add vordr check to common moderation query (#2644) Added vordr check to the common moderation fetch logic, so that it's the same as for just a single squad --- src/common/post.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/common/post.ts b/src/common/post.ts index b3953c871..1867237b3 100644 --- a/src/common/post.ts +++ b/src/common/post.ts @@ -836,6 +836,7 @@ export const getAllModerationItemsAsAdmin = async ( .where('sm.role IN (:...roles)', { roles: [SourceMemberRoles.Admin, SourceMemberRoles.Moderator], }) + .andWhere(`("${builder.alias}"."flags"->>'vordr')::boolean IS NOT TRUE`) .andWhere(`"${builder.alias}"."sourceId" = "sm"."sourceId"`) .orderBy(`${builder.alias}.updatedAt`, 'DESC') .limit(page.limit)