diff --git a/packages/server/modules/comments/graph/resolvers/comments.ts b/packages/server/modules/comments/graph/resolvers/comments.ts index fa09b0e7e..070564233 100644 --- a/packages/server/modules/comments/graph/resolvers/comments.ts +++ b/packages/server/modules/comments/graph/resolvers/comments.ts @@ -3,7 +3,6 @@ import { ForbiddenError } from '@/modules/shared/errors' import { getStream } from '@/modules/core/services/streams' import { Roles } from '@/modules/core/helpers/mainConstants' import { - getComments, getResourceCommentCount, streamResourceCheckFactory, createCommentFactory, @@ -15,6 +14,7 @@ import { checkStreamResourceAccessFactory, deleteCommentFactory, getCommentFactory, + getCommentsLegacyFactory, insertCommentLinksFactory, insertCommentsFactory, markCommentUpdatedFactory, @@ -151,6 +151,7 @@ export = { projectId: args.streamId, authCtx: context }) + const getComments = getCommentsLegacyFactory({ db }) return { ...(await getComments({ ...args, @@ -172,6 +173,7 @@ export = { } const resources = [{ resourceId: parent.id, resourceType: ResourceType.Comment }] + const getComments = getCommentsLegacyFactory({ db }) return await getComments({ resources, replies: true, diff --git a/packages/server/modules/comments/repositories/comments.ts b/packages/server/modules/comments/repositories/comments.ts index 6f6921611..e8e809d3d 100644 --- a/packages/server/modules/comments/repositories/comments.ts +++ b/packages/server/modules/comments/repositories/comments.ts @@ -786,3 +786,99 @@ export const deleteCommentFactory = async ({ commentId }) => { return !!(await tables.comments(deps.db).where(Comments.col.id, commentId).del()) } + +/** + * One of `streamId` or `resources` expected. If both are provided, then + * `resources` takes precedence. + */ +type GetCommentsLegacyParams = { + limit?: number | null + cursor?: string | null + userId?: string | null + replies?: boolean | null + archived?: boolean | null +} & ( + | { + resources: ResourceIdentifier[] + streamId?: null + } + | { + resources?: ResourceIdentifier[] | null + streamId: string + } +) + +/** + * @deprecated Use `getPaginatedProjectComments()` instead + */ +export const getCommentsLegacyFactory = + (deps: { db: Knex }) => + async ({ + resources, + limit, + cursor, + userId = null, + replies = false, + streamId, + archived = false + }: GetCommentsLegacyParams) => { + const query = deps.db().with('comms', (cte) => { + cte.select().distinctOn('id').from('comments') + cte.join('comment_links', 'comments.id', '=', 'commentId') + + if (userId) { + // link viewed At + cte.leftOuterJoin('comment_views', (b) => { + b.on('comment_views.commentId', '=', 'comments.id') + b.andOn('comment_views.userId', '=', knex.raw('?', userId)) + }) + } + + if (resources && resources.length !== 0) { + cte.where((q) => { + // link resources + for (const res of resources) { + q.orWhere('comment_links.resourceId', '=', res.resourceId) + } + }) + } else { + cte.where({ streamId }) + } + if (!replies) { + cte.whereNull('parentComment') + } + cte.where('archived', '=', archived) + }) + + query.select().from('comms') + + // total count coming from our cte + query.joinRaw('right join (select count(*) from comms) c(total_count) on true') + + // get comment's all linked resources + query.joinRaw(` + join( + select cl."commentId" as id, JSON_AGG(json_build_object('resourceId', cl."resourceId", 'resourceType', cl."resourceType")) as resources + from comment_links cl + join comms on comms.id = cl."commentId" + group by cl."commentId" + ) res using(id)`) + + if (cursor) { + query.where('createdAt', '<', cursor) + } + + limit = clamp(limit ?? 10, 0, 100) + query.orderBy('createdAt', 'desc') + query.limit(limit || 1) // need at least 1 row to get totalCount + + const rows = await query + const totalCount = rows && rows.length > 0 ? parseInt(rows[0].total_count) : 0 + const nextCursor = rows && rows.length > 0 ? rows[rows.length - 1].createdAt : null + + return { + items: !limit ? [] : rows, + cursor: nextCursor ? nextCursor.toISOString() : null, + totalCount + } + } diff --git a/packages/server/modules/comments/services/index.ts b/packages/server/modules/comments/services/index.ts index 25fd6633d..0d360e71b 100644 --- a/packages/server/modules/comments/services/index.ts +++ b/packages/server/modules/comments/services/index.ts @@ -4,7 +4,6 @@ import { ForbiddenError } from '@/modules/shared/errors' import { buildCommentTextFromInput } from '@/modules/comments/services/commentTextService' import { CommentsEvents, CommentsEventsEmit } from '@/modules/comments/events/emitter' import { getStreamCommentCount as repoGetStreamCommentCount } from '@/modules/comments/repositories/comments' -import { clamp } from 'lodash' import { isNonNullable, Roles } from '@speckle/shared' import { ResourceIdentifier, @@ -273,100 +272,6 @@ export const archiveCommentFactory = return updatedComment! } -/** - * One of `streamId` or `resources` expected. If both are provided, then - * `resources` takes precedence. - */ -type GetCommentsParams = { - limit?: number | null - cursor?: string | null - userId?: string | null - replies?: boolean | null - archived?: boolean | null -} & ( - | { - resources: ResourceIdentifier[] - streamId?: null - } - | { - resources?: ResourceIdentifier[] | null - streamId: string - } -) - -/** - * @deprecated Use `getPaginatedProjectComments()` instead - */ -export async function getComments({ - resources, - limit, - cursor, - userId = null, - replies = false, - streamId, - archived = false -}: GetCommentsParams) { - const query = knex.with('comms', (cte) => { - cte.select().distinctOn('id').from('comments') - cte.join('comment_links', 'comments.id', '=', 'commentId') - - if (userId) { - // link viewed At - cte.leftOuterJoin('comment_views', (b) => { - b.on('comment_views.commentId', '=', 'comments.id') - b.andOn('comment_views.userId', '=', knex.raw('?', userId)) - }) - } - - if (resources && resources.length !== 0) { - cte.where((q) => { - // link resources - for (const res of resources) { - q.orWhere('comment_links.resourceId', '=', res.resourceId) - } - }) - } else { - cte.where({ streamId }) - } - if (!replies) { - cte.whereNull('parentComment') - } - cte.where('archived', '=', archived) - }) - - query.select().from('comms') - - // total count coming from our cte - query.joinRaw('right join (select count(*) from comms) c(total_count) on true') - - // get comment's all linked resources - query.joinRaw(` - join( - select cl."commentId" as id, JSON_AGG(json_build_object('resourceId', cl."resourceId", 'resourceType', cl."resourceType")) as resources - from comment_links cl - join comms on comms.id = cl."commentId" - group by cl."commentId" - ) res using(id)`) - - if (cursor) { - query.where('createdAt', '<', cursor) - } - - limit = clamp(limit ?? 10, 0, 100) - query.orderBy('createdAt', 'desc') - query.limit(limit || 1) // need at least 1 row to get totalCount - - const rows = await query - const totalCount = rows && rows.length > 0 ? parseInt(rows[0].total_count) : 0 - const nextCursor = rows && rows.length > 0 ? rows[rows.length - 1].createdAt : null - - return { - items: !limit ? [] : rows, - cursor: nextCursor ? nextCursor.toISOString() : null, - totalCount - } -} - export async function getResourceCommentCount({ resourceId }: { resourceId: string }) { const [res] = await CommentLinks() .count('commentId') diff --git a/packages/server/modules/comments/tests/comments.spec.js b/packages/server/modules/comments/tests/comments.spec.js index 840bbb194..7b98f1ad5 100644 --- a/packages/server/modules/comments/tests/comments.spec.js +++ b/packages/server/modules/comments/tests/comments.spec.js @@ -1,10 +1,3 @@ -// Hooking up comments/services/index.js mock -const { mockRequireModule } = require('@/test/mockHelper') -const commentsServiceMock = mockRequireModule( - ['@/modules/comments/services/index', require.resolve('../services/index')], - ['@/modules/comments/graph/resolvers/comments'] -) - const path = require('path') const { packageRoot } = require('@/bootstrap') const expect = require('chai').expect @@ -16,7 +9,6 @@ const { createCommitByBranchName } = require('@/modules/core/services/commits') const { createObject } = require('@/modules/core/services/objects') const { - getComments, getResourceCommentCount, getStreamCommentCount, streamResourceCheckFactory, @@ -45,7 +37,10 @@ const { purgeNotifications } = require('@/test/notificationsHelper') const { NotificationType } = require('@/modules/notifications/helpers/types') -const { EmailSendingServiceMock } = require('@/test/mocks/global') +const { + EmailSendingServiceMock, + CommentsRepositoryMock +} = require('@/test/mocks/global') const { createAuthedTestContext } = require('@/test/graphqlHelper') const { checkStreamResourceAccessFactory, @@ -55,7 +50,8 @@ const { deleteCommentFactory, markCommentUpdatedFactory, getCommentFactory, - updateCommentFactory + updateCommentFactory, + getCommentsLegacyFactory } = require('@/modules/comments/repositories/comments') const { db } = require('@/db/knex') const { getBlobsFactory } = require('@/modules/blobstorage/repositories') @@ -103,6 +99,7 @@ const archiveComment = archiveCommentFactory({ getStream, updateComment }) +const getComments = getCommentsLegacyFactory({ db }) function buildCommentInputFromString(textString) { return convertBasicStringToDocument(textString) @@ -113,6 +110,7 @@ function generateRandomCommentText() { } const mailerMock = EmailSendingServiceMock +const commentRepoMock = CommentsRepositoryMock describe('Comments @comments', () => { /** @type {import('express').Express} */ @@ -184,12 +182,12 @@ describe('Comments @comments', () => { after(() => { notificationsState.destroy() - commentsServiceMock.destroy() + commentRepoMock.destroy() }) afterEach(() => { - commentsServiceMock.disable() - commentsServiceMock.resetMockedFunctions() + commentRepoMock.disable() + commentRepoMock.resetMockedFunctions() }) it('Should not be allowed to comment without specifying at least one target resource', async () => { @@ -1149,9 +1147,9 @@ describe('Comments @comments', () => { }) it('both legacy (string) comments and new (ProseMirror) documents are formatted as SmartTextEditorValue values', async () => { - commentsServiceMock.enable() - commentsServiceMock.mockFunction('getComments', () => { - return { + commentRepoMock.enable() + commentRepoMock.mockFunction('getCommentsLegacyFactory', () => { + return () => ({ items: [ // Legacy { @@ -1177,7 +1175,7 @@ describe('Comments @comments', () => { ], cursor: new Date().toISOString(), totalCount: 3 - } + }) }) const { data, errors } = await readComments() @@ -1192,8 +1190,8 @@ describe('Comments @comments', () => { text: 'https://aaa.com:3000/h3ll0-world/_?a=1&b=2#aaa' } - commentsServiceMock.enable() - commentsServiceMock.mockFunction('getComments', () => ({ + commentRepoMock.enable() + commentRepoMock.mockFunction('getCommentsLegacyFactory', () => () => ({ items: [item], cursor: new Date().toISOString(), totalCount: 1 @@ -1231,8 +1229,8 @@ describe('Comments @comments', () => { text: textParts.join('') } - commentsServiceMock.enable() - commentsServiceMock.mockFunction('getComments', () => ({ + commentRepoMock.enable() + commentRepoMock.mockFunction('getCommentsLegacyFactory', () => () => ({ items: [item], cursor: new Date().toISOString(), totalCount: 1 @@ -1350,8 +1348,8 @@ describe('Comments @comments', () => { text: value } - commentsServiceMock.enable() - commentsServiceMock.mockFunction('getComments', () => ({ + commentRepoMock.enable() + commentRepoMock.mockFunction('getCommentsLegacyFactory', () => () => ({ items: [item], cursor: new Date().toISOString(), totalCount: 1 diff --git a/packages/server/test/mocks/global.ts b/packages/server/test/mocks/global.ts index 54fe13856..319dff66d 100644 --- a/packages/server/test/mocks/global.ts +++ b/packages/server/test/mocks/global.ts @@ -1,10 +1,13 @@ -import { createGlobalMock } from '@/test/mockHelper' +import { createGlobalMock, mockRequireModule } from '@/test/mockHelper' /** - * Global mocks that can be re-used. Remember to .enable() before use and .disable() - * after use to ensure that other tests work with the real module. + * Global mocks that can be re-used. Early setup ensures that mocks work. */ export const EmailSendingServiceMock = createGlobalMock< typeof import('@/modules/emails/services/sending') >('@/modules/emails/services/sending') + +export const CommentsRepositoryMock = mockRequireModule< + typeof import('@/modules/comments/repositories/comments') +>(['@/modules/comments/repositories/comments'])