From b951d71f9ce86861ca8983f7e764ccaee628faa0 Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Fri, 11 Apr 2025 14:51:13 +0100 Subject: [PATCH 01/36] chore(logging): operations logging around access request mutations --- .../accessrequests/graph/resolvers/index.ts | 67 +++++++++++++++---- 1 file changed, 53 insertions(+), 14 deletions(-) diff --git a/packages/server/modules/accessrequests/graph/resolvers/index.ts b/packages/server/modules/accessrequests/graph/resolvers/index.ts index 5975cb751..eeea29548 100644 --- a/packages/server/modules/accessrequests/graph/resolvers/index.ts +++ b/packages/server/modules/accessrequests/graph/resolvers/index.ts @@ -31,6 +31,7 @@ import { import { authorizeResolver } from '@/modules/shared' import { LogicError } from '@/modules/shared/errors' import { getEventBus } from '@/modules/shared/services/eventBus' +import { withOperationLogging } from '@/observability/domain/businessLogging' const getUser = getUserFactory({ db }) const getStream = getStreamFactory({ db }) @@ -103,7 +104,20 @@ const resolvers: Resolvers = { if (!userId) throw new LogicError('User ID unexpectedly false') const { streamId } = args - return await requestStreamAccess(userId, streamId) + const logger = ctx.log.child({ + streamId, + projectId: streamId + }) + const result = await withOperationLogging( + async () => await requestStreamAccess(userId, streamId), + { + logger, + operationName: 'requestStreamAccess', + operationDescription: 'Request for stream access' + } + ) + if (!result) throw new LogicError('Unable to create stream access request') // should have already thrown by this point + return result } }, ProjectMutations: { @@ -113,25 +127,50 @@ const resolvers: Resolvers = { async create(_parent, args, ctx) { const { userId } = ctx const { projectId } = args - return await requestProjectAccess(userId!, projectId) + const logger = ctx.log.child({ + projectId, + streamId: projectId // for legacy compatibility + }) + const result = await withOperationLogging( + async () => await requestProjectAccess(userId!, projectId), + { + logger, + operationName: 'CreateProjectAccessRequest', + operationDescription: 'Create a request for project access' + } + ) + if (!result) throw new LogicError('Unable to create project access request') // should have already thrown by this point + return result }, async use(_parent, args, ctx) { const { userId, resourceAccessRules } = ctx const { requestId, accept, role } = args + const logger = ctx.log - const usedReq = await processPendingProjectRequest( - userId!, - requestId, - accept, - mapStreamRoleToValue(role), - resourceAccessRules + const project = await withOperationLogging( + async () => { + const usedReq = await processPendingProjectRequest( + userId!, + requestId, + accept, + mapStreamRoleToValue(role), + resourceAccessRules + ) + + const project = await ctx.loaders.streams.getStream.load(usedReq.resourceId) + if (!project) { + throw new LogicError('Unexpectedly unable to find request project') + } + + return project + }, + { + logger, + operationName: 'ProcessProjectAccessRequest', + operationDescription: 'Use a request for project access' + } ) - - const project = await ctx.loaders.streams.getStream.load(usedReq.resourceId) - if (!project) { - throw new LogicError('Unexpectedly unable to find request project') - } - + if (!project) throw new LogicError('Unable to user project access request') // should have already thrown by this point return project } }, From c937c4c30cedcb6885e77de20a790de2ae016bab Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Mon, 14 Apr 2025 13:49:36 +0100 Subject: [PATCH 02/36] Improve error handling --- .../accessrequests/graph/resolvers/index.ts | 30 ++++++++----------- .../server/modules/gatekeeper/rest/billing.ts | 2 +- .../observability/domain/businessLogging.ts | 6 ++-- 3 files changed, 16 insertions(+), 22 deletions(-) diff --git a/packages/server/modules/accessrequests/graph/resolvers/index.ts b/packages/server/modules/accessrequests/graph/resolvers/index.ts index eeea29548..07825f5b8 100644 --- a/packages/server/modules/accessrequests/graph/resolvers/index.ts +++ b/packages/server/modules/accessrequests/graph/resolvers/index.ts @@ -108,7 +108,7 @@ const resolvers: Resolvers = { streamId, projectId: streamId }) - const result = await withOperationLogging( + return await withOperationLogging( async () => await requestStreamAccess(userId, streamId), { logger, @@ -116,8 +116,6 @@ const resolvers: Resolvers = { operationDescription: 'Request for stream access' } ) - if (!result) throw new LogicError('Unable to create stream access request') // should have already thrown by this point - return result } }, ProjectMutations: { @@ -131,7 +129,7 @@ const resolvers: Resolvers = { projectId, streamId: projectId // for legacy compatibility }) - const result = await withOperationLogging( + return await withOperationLogging( async () => await requestProjectAccess(userId!, projectId), { logger, @@ -139,38 +137,34 @@ const resolvers: Resolvers = { operationDescription: 'Create a request for project access' } ) - if (!result) throw new LogicError('Unable to create project access request') // should have already thrown by this point - return result }, async use(_parent, args, ctx) { const { userId, resourceAccessRules } = ctx const { requestId, accept, role } = args const logger = ctx.log - const project = await withOperationLogging( - async () => { - const usedReq = await processPendingProjectRequest( + const usedReq = await withOperationLogging( + async () => + await processPendingProjectRequest( userId!, requestId, accept, mapStreamRoleToValue(role), resourceAccessRules - ) + ), - const project = await ctx.loaders.streams.getStream.load(usedReq.resourceId) - if (!project) { - throw new LogicError('Unexpectedly unable to find request project') - } - - return project - }, { logger, operationName: 'ProcessProjectAccessRequest', operationDescription: 'Use a request for project access' } ) - if (!project) throw new LogicError('Unable to user project access request') // should have already thrown by this point + + const project = await ctx.loaders.streams.getStream.load(usedReq.resourceId) + if (!project) { + throw new LogicError('Unexpectedly unable to find request project') + } + return project } }, diff --git a/packages/server/modules/gatekeeper/rest/billing.ts b/packages/server/modules/gatekeeper/rest/billing.ts index 6754fa96e..f048386ad 100644 --- a/packages/server/modules/gatekeeper/rest/billing.ts +++ b/packages/server/modules/gatekeeper/rest/billing.ts @@ -153,7 +153,7 @@ export const getBillingRouter = (): Router => { operationName: 'completeCheckoutSession', operationDescription: 'Payment succeeded or Stripe session completed, and payment was paid', - errorHandler: (err, logger) => { + errorHandler: async (err, logger) => { if (err instanceof WorkspaceAlreadyPaidError) { // ignore the request, this is prob a replay from stripe logger.info('Workspace is already paid, ignoring') diff --git a/packages/server/observability/domain/businessLogging.ts b/packages/server/observability/domain/businessLogging.ts index 145f1f52a..1e545bc84 100644 --- a/packages/server/observability/domain/businessLogging.ts +++ b/packages/server/observability/domain/businessLogging.ts @@ -18,9 +18,9 @@ export const withOperationLogging = async ( logger: Logger operationName: string operationDescription?: string - errorHandler?: (err: unknown, logger: Logger) => MaybeAsync + errorHandler?: (err: unknown, logger: Logger) => MaybeAsync } -): Promise => { +): Promise => { const { operationName, operationDescription } = params const errorHandler = params.errorHandler || logErrorThenThrow const logger = params.logger.child(OperationName(operationName)) @@ -36,6 +36,6 @@ export const withOperationLogging = async ( logger.info(OperationStatus.success, OperationLogLinePrefix) return results } catch (err) { - await errorHandler(err, logger) + return await errorHandler(err, logger) } } From 1c17d601d83a1cffac3d33df697b2e1e3a28beae Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Mon, 14 Apr 2025 17:07:06 +0100 Subject: [PATCH 03/36] feat(server/logging): add operations logging to comment mutations --- .../comments/graph/resolvers/comments.ts | 179 ++++++++++++++---- .../observability/domain/businessLogging.ts | 6 +- 2 files changed, 143 insertions(+), 42 deletions(-) diff --git a/packages/server/modules/comments/graph/resolvers/comments.ts b/packages/server/modules/comments/graph/resolvers/comments.ts index ac114092f..b278e6c90 100644 --- a/packages/server/modules/comments/graph/resolvers/comments.ts +++ b/packages/server/modules/comments/graph/resolvers/comments.ts @@ -91,6 +91,7 @@ import { getProjectDbClient } from '@/modules/multiregion/utils/dbSelector' import { Knex } from 'knex' import { getEventBus } from '@/modules/shared/services/eventBus' import { throwIfAuthNotOk } from '@/modules/shared/helpers/errorHelper' +import { withOperationLogging } from '@/observability/domain/businessLogging' // We can use the main DB for these const getStream = getStreamFactory({ db }) @@ -495,6 +496,11 @@ export = { }) throwIfAuthNotOk(canCreate) + const logger = ctx.log.child({ + projectId, + streamId: projectId //legacy + }) + const projectDb = await getProjectDbClient({ projectId }) const getViewerResourceItemsUngrouped = buildGetViewerResourceItemsUngrouped({ @@ -517,16 +523,28 @@ export = { emitEvent: getEventBus().emit }) - return await createCommentThreadAndNotify(args.input, ctx.userId!) + return await withOperationLogging( + async () => await createCommentThreadAndNotify(args.input, ctx.userId!), + { + operationName: 'createCommentThread', + operationDescription: 'Create comment thread', + logger + } + ) }, async reply(_parent, args, ctx) { + const projectId = args.input.projectId const canCreateComment = await ctx.authPolicies.project.comment.canCreate({ userId: ctx.userId, - projectId: args.input.projectId + projectId }) throwIfAuthNotOk(canCreateComment) + const logger = ctx.log.child({ + projectId, + streamId: projectId //legacy + }) - const projectDb = await getProjectDbClient({ projectId: args.input.projectId }) + const projectDb = await getProjectDbClient({ projectId }) const getComment = getCommentFactory({ db: projectDb }) const validateInputAttachments = validateInputAttachmentsFactory({ getBlobs: getBlobsFactory({ db: projectDb }) @@ -549,18 +567,33 @@ export = { }) }) - return await createCommentReplyAndNotify(args.input, ctx.userId!) + return await withOperationLogging( + async () => await createCommentReplyAndNotify(args.input, ctx.userId!), + { + operationName: 'replyToComment', + operationDescription: 'Reply to comment', + logger + } + ) }, async edit(_parent, args, ctx) { + const projectId = args.input.projectId + const commentId = args.input.commentId const canEditComment = await ctx.authPolicies.project.comment.canEdit({ - projectId: args.input.projectId, + projectId, userId: ctx.userId, - commentId: args.input.commentId + commentId }) throwIfAuthNotOk(canEditComment) + const logger = ctx.log.child({ + projectId, + streamId: projectId, //legacy + commentId + }) + const projectDb = await getProjectDbClient({ - projectId: args.input.projectId + projectId }) const getComment = getCommentFactory({ db: projectDb }) const validateInputAttachments = validateInputAttachmentsFactory({ @@ -575,18 +608,28 @@ export = { emitEvent: getEventBus().emit }) - return await editCommentAndNotify(args.input, ctx.userId!) + return await withOperationLogging( + async () => await editCommentAndNotify(args.input, ctx.userId!), + { logger, operationName: 'editComment', operationDescription: 'Edit comment' } + ) }, async archive(_parent, args, ctx) { + const projectId = args.input.projectId + const commentId = args.input.commentId const canArchive = await ctx.authPolicies.project.comment.canArchive({ userId: ctx.userId, - projectId: args.input.projectId, - commentId: args.input.commentId + projectId, + commentId }) throwIfAuthNotOk(canArchive) + const logger = ctx.log.child({ + projectId, + streamId: projectId, //legacy + commentId + }) const projectDb = await getProjectDbClient({ - projectId: args.input.projectId + projectId }) const getComment = getCommentFactory({ db: projectDb }) const getStream = getStreamFactory({ db: projectDb }) @@ -605,10 +648,14 @@ export = { emitEvent: getEventBus().emit }) - await archiveCommentAndNotify( - args.input.commentId, - ctx.userId!, - args.input.archived + await withOperationLogging( + async () => + await archiveCommentAndNotify(commentId, ctx.userId!, args.input.archived), + { + logger, + operationName: 'archiveComment', + operationDescription: 'Archive comment' + } ) return true } @@ -675,13 +722,19 @@ export = { }, async commentCreate(_parent, args, context) { + const projectId = args.input.streamId const canCreate = await context.authPolicies.project.comment.canCreate({ userId: context.userId, - projectId: args.input.streamId + projectId }) throwIfAuthNotOk(canCreate) - const projectDb = await getProjectDbClient({ projectId: args.input.streamId }) + const logger = context.log.child({ + projectId, + streamId: projectId //legacy + }) + + const projectDb = await getProjectDbClient({ projectId }) const getViewerResourcesFromLegacyIdentifiers = buildGetViewerResourcesFromLegacyIdentifiers({ db: projectDb }) @@ -699,23 +752,39 @@ export = { emitEvent: getEventBus().emit, getViewerResourcesFromLegacyIdentifiers }) - const comment = await createComment({ - userId: context.userId!, - input: args.input - }) + const comment = await withOperationLogging( + async () => + await createComment({ + userId: context.userId!, + input: args.input + }), + { + operationName: 'createComment', + operationDescription: 'Create comment', + logger + } + ) return comment.id }, async commentEdit(_parent, args, context) { + const projectId = args.input.streamId + const commentId = args.input.id const canEdit = await context.authPolicies.project.comment.canEdit({ userId: context.userId, - projectId: args.input.streamId, - commentId: args.input.id + projectId, + commentId }) throwIfAuthNotOk(canEdit) - const projectDb = await getProjectDbClient({ projectId: args.input.streamId }) + const logger = context.log.child({ + projectId, + streamId: projectId, //legacy + commentId + }) + + const projectDb = await getProjectDbClient({ projectId }) const editComment = editCommentFactory({ getComment: getCommentFactory({ db: projectDb }), validateInputAttachments: validateInputAttachmentsFactory({ @@ -725,7 +794,10 @@ export = { emitEvent: getEventBus().emit }) - await editComment({ userId: context.userId!, input: args.input }) + await withOperationLogging( + async () => await editComment({ userId: context.userId!, input: args.input }), + { operationName: 'editComment', operationDescription: 'Edit comment', logger } + ) return true }, @@ -745,38 +817,59 @@ export = { }, async commentArchive(_parent, args, context) { + const projectId = args.streamId + const commentId = args.commentId const canArchive = await context.authPolicies.project.comment.canArchive({ userId: context.userId, - projectId: args.streamId, - commentId: args.commentId + projectId, + commentId }) throwIfAuthNotOk(canArchive) - const projectDb = await getProjectDbClient({ projectId: args.streamId }) + const logger = context.log.child({ + projectId, + streamId: projectId, //legacy + commentId + }) + + const projectDb = await getProjectDbClient({ projectId }) const archiveComment = archiveCommentFactory({ getComment: getCommentFactory({ db: projectDb }), getStream, updateComment: updateCommentFactory({ db: projectDb }), emitEvent: getEventBus().emit }) - await archiveComment({ ...args, userId: context.userId! }) // NOTE: permissions check inside service + await withOperationLogging( + async () => await archiveComment({ ...args, userId: context.userId! }), // NOTE: permissions check inside service + { + logger, + operationName: 'archiveComment', + operationDescription: 'Archive comment' + } + ) return true }, async commentReply(_parent, args, context) { + const projectId = args.input.streamId if (!context.userId) throw new ForbiddenError('Only registered users can comment.') + const logger = context.log.child({ + projectId, + streamId: projectId //legacy + }) + const stream = await getStream({ - streamId: args.input.streamId, + streamId: projectId, userId: context.userId }) if (!stream?.allowPublicComments && !stream?.role) throw new ForbiddenError('You are not authorized.') - const projectDb = await getProjectDbClient({ projectId: args.input.streamId }) + const projectDb = await getProjectDbClient({ projectId }) const createCommentReply = createCommentReplyFactory({ validateInputAttachments: validateInputAttachmentsFactory({ @@ -796,14 +889,22 @@ export = { buildGetViewerResourcesFromLegacyIdentifiers({ db: projectDb }) }) }) - const reply = await createCommentReply({ - authorId: context.userId, - parentCommentId: args.input.parentComment, - streamId: args.input.streamId, - text: args.input.text as SmartTextEditorValueSchema, - data: args.input.data ?? null, - blobIds: args.input.blobIds - }) + const reply = await withOperationLogging( + async () => + await createCommentReply({ + authorId: context.userId, + parentCommentId: args.input.parentComment, + streamId: args.input.streamId, + text: args.input.text as SmartTextEditorValueSchema, + data: args.input.data ?? null, + blobIds: args.input.blobIds + }), + { + logger, + operationName: 'createCommentReply', + operationDescription: 'Create comment reply' + } + ) return reply.id } diff --git a/packages/server/observability/domain/businessLogging.ts b/packages/server/observability/domain/businessLogging.ts index 145f1f52a..1e545bc84 100644 --- a/packages/server/observability/domain/businessLogging.ts +++ b/packages/server/observability/domain/businessLogging.ts @@ -18,9 +18,9 @@ export const withOperationLogging = async ( logger: Logger operationName: string operationDescription?: string - errorHandler?: (err: unknown, logger: Logger) => MaybeAsync + errorHandler?: (err: unknown, logger: Logger) => MaybeAsync } -): Promise => { +): Promise => { const { operationName, operationDescription } = params const errorHandler = params.errorHandler || logErrorThenThrow const logger = params.logger.child(OperationName(operationName)) @@ -36,6 +36,6 @@ export const withOperationLogging = async ( logger.info(OperationStatus.success, OperationLogLinePrefix) return results } catch (err) { - await errorHandler(err, logger) + return await errorHandler(err, logger) } } From 7deb4554c6836ebc88ca68004b0c87ecaff64c5e Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Mon, 14 Apr 2025 17:20:07 +0100 Subject: [PATCH 04/36] chore(server/logging): add operation logs to email module --- .../modules/emails/graph/resolvers/index.ts | 18 +++++++++++++++--- packages/server/modules/emails/rest/index.ts | 13 +++++++++++-- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/packages/server/modules/emails/graph/resolvers/index.ts b/packages/server/modules/emails/graph/resolvers/index.ts index cc31f05fb..848a8ce5e 100644 --- a/packages/server/modules/emails/graph/resolvers/index.ts +++ b/packages/server/modules/emails/graph/resolvers/index.ts @@ -13,6 +13,7 @@ import { import { renderEmail } from '@/modules/emails/services/emailRendering' import { sendEmail } from '@/modules/emails/services/sending' import { requestEmailVerificationFactory } from '@/modules/emails/services/verification/request' +import { withOperationLogging } from '@/observability/domain/businessLogging' const getUser = getUserFactory({ db }) const requestEmailVerification = requestEmailVerificationFactory({ @@ -38,14 +39,25 @@ export = { Mutation: { async requestVerification(_parent, _args, ctx) { const { userId } = ctx - await requestEmailVerification(userId || '') + await withOperationLogging( + async () => await requestEmailVerification(userId || ''), + { + logger: ctx.log, + operationName: 'requestEmailVerification', + operationDescription: 'Request email verification' + } + ) return true }, - async requestVerificationByEmail(_parent, args) { + async requestVerificationByEmail(_parent, args, ctx) { const { email } = args const user = await getUserByEmail(email) if (!user?.email || user.verified) return false - await requestEmailVerification(user.id) + await withOperationLogging(async () => await requestEmailVerification(user.id), { + logger: ctx.log, + operationName: 'requestEmailVerificationFromEmail', + operationDescription: `Request verification by email` + }) return true } } diff --git a/packages/server/modules/emails/rest/index.ts b/packages/server/modules/emails/rest/index.ts index b02f83910..b0d44b050 100644 --- a/packages/server/modules/emails/rest/index.ts +++ b/packages/server/modules/emails/rest/index.ts @@ -9,9 +9,11 @@ import { } from '@/modules/emails/repositories' import { db } from '@/db/knex' import { markUserAsVerifiedFactory } from '@/modules/core/repositories/users' +import { withOperationLogging } from '@/observability/domain/businessLogging' export = (app: Express) => { app.get('/auth/verifyemail', async (req, res) => { + const logger = req.log try { const finalizeEmailVerification = finalizeEmailVerificationFactory({ getPendingToken: getPendingTokenFactory({ db }), @@ -19,7 +21,14 @@ export = (app: Express) => { deleteVerifications: deleteVerificationsFactory({ db }) }) - await finalizeEmailVerification(req.query.t as Optional) + await withOperationLogging( + async () => await finalizeEmailVerification(req.query.t as Optional), + { + logger, + operationName: 'finalizeEmailVerification', + operationDescription: 'Finalize email verification' + } + ) return res.redirect( new URL('/?emailverifiedstatus=true', getFrontendOrigin()).toString() ) @@ -28,7 +37,7 @@ export = (app: Express) => { error instanceof EmailVerificationFinalizationError ? error.message : 'Email verification unexpectedly failed' - req.log.info({ err: error }, 'Email verification failed.') + logger.info({ err: error }, 'Email verification failed.') return res.redirect( new URL(`/?emailverifiederror=${msg}`, getFrontendOrigin()).toString() From 9c5c119f191e4560482f95229b4ffbf0491ecfc4 Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Mon, 14 Apr 2025 20:18:37 +0100 Subject: [PATCH 05/36] fix bug --- packages/server/modules/comments/graph/resolvers/comments.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/modules/comments/graph/resolvers/comments.ts b/packages/server/modules/comments/graph/resolvers/comments.ts index b278e6c90..eec6f13b7 100644 --- a/packages/server/modules/comments/graph/resolvers/comments.ts +++ b/packages/server/modules/comments/graph/resolvers/comments.ts @@ -892,7 +892,7 @@ export = { const reply = await withOperationLogging( async () => await createCommentReply({ - authorId: context.userId, + authorId: context.userId!, parentCommentId: args.input.parentComment, streamId: args.input.streamId, text: args.input.text as SmartTextEditorValueSchema, From 681b065790a8e261d6a5258fc5d4a344a558afc7 Mon Sep 17 00:00:00 2001 From: andrewwallacespeckle Date: Mon, 14 Apr 2025 23:31:37 +0100 Subject: [PATCH 06/36] Fix move permissions --- .../projects/ProjectDashboardCard.vue | 8 ++- .../components/workspace/ProjectList.vue | 22 +++----- .../workspace/header/AddProjectMenu.vue | 6 ++- .../components/workspace/header/Header.vue | 17 +++---- .../workspace/moveProject/Manager.vue | 5 ++ .../workspace/moveProject/SelectProject.vue | 31 ++++++------ .../workspace/moveProject/SelectWorkspace.vue | 50 ++++++++++++------- .../lib/common/generated/gql/gql.ts | 12 ++--- .../lib/common/generated/gql/graphql.ts | 32 ++++++------ .../frontend-2/pages/projects/[id]/index.vue | 4 +- 10 files changed, 100 insertions(+), 87 deletions(-) diff --git a/packages/frontend-2/components/projects/ProjectDashboardCard.vue b/packages/frontend-2/components/projects/ProjectDashboardCard.vue index e3889b2e1..4f36911f7 100644 --- a/packages/frontend-2/components/projects/ProjectDashboardCard.vue +++ b/packages/frontend-2/components/projects/ProjectDashboardCard.vue @@ -64,7 +64,12 @@ }} diff --git a/packages/frontend-2/components/workspace/moveProject/SelectWorkspace.vue b/packages/frontend-2/components/workspace/moveProject/SelectWorkspace.vue index 0423f2235..deb45dd18 100644 --- a/packages/frontend-2/components/workspace/moveProject/SelectWorkspace.vue +++ b/packages/frontend-2/components/workspace/moveProject/SelectWorkspace.vue @@ -26,7 +26,7 @@ From 0c18acc4527d1b986e48a9f8786edd32fe6180fa Mon Sep 17 00:00:00 2001 From: Alessandro Magionami Date: Tue, 15 Apr 2025 10:44:12 +0200 Subject: [PATCH 07/36] Alessandro/web 2945 comments hide body (#4385) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore(core): move limits logic into shared * feat(comments): limit text and rawText for comments * chore(core): removed test moved to shared * chore(comments): generate gql types * feat(comments): rework comment history limits * chore(comments): fix tests * chore(shared): add dayjs as dependency --------- Co-authored-by: Gergő Jedlicska --- .../assets/comments/typedefs/comments.gql | 4 +- .../comments/graph/resolvers/comments.ts | 51 +++++- .../modules/comments/tests/comments.spec.ts | 29 +++- .../comments/tests/projectComments.spec.ts | 6 +- .../modules/core/graph/generated/graphql.ts | 8 +- .../modules/core/graph/resolvers/commits.ts | 45 ++++-- .../modules/core/graph/resolvers/versions.ts | 27 ++-- .../modules/core/helpers/testHelpers.ts | 3 + .../modules/core/services/versions/limits.ts | 68 -------- .../tests/integration/versions.graph.spec.ts | 19 +-- .../core/tests/unit/services/versions.spec.ts | 146 ------------------ .../graph/generated/graphql.ts | 8 +- .../cross-server-sync/services/commit.ts | 8 +- .../server/test/graphql/generated/graphql.ts | 18 +-- packages/server/test/mocks/global.ts | 4 + packages/shared/package.json | 1 + .../src/authz/domain/projects/limits.ts | 12 ++ packages/shared/src/authz/index.ts | 1 + .../project/canMoveToWorkspace.spec.ts | 6 +- .../policies/project/model/canCreate.spec.ts | 6 +- .../canCreateWorkspaceProject.spec.ts | 12 +- packages/shared/src/index.ts | 1 + packages/shared/src/limit/domain.ts | 13 ++ packages/shared/src/limit/utils.spec.ts | 95 ++++++++++++ packages/shared/src/limit/utils.ts | 53 +++++++ packages/shared/src/tests/helpers/utils.ts | 5 + .../shared/src/workspaces/helpers/features.ts | 18 ++- .../shared/src/workspaces/helpers/limits.ts | 5 +- yarn.lock | 8 + 29 files changed, 372 insertions(+), 308 deletions(-) delete mode 100644 packages/server/modules/core/services/versions/limits.ts delete mode 100644 packages/server/modules/core/tests/unit/services/versions.spec.ts create mode 100644 packages/shared/src/authz/domain/projects/limits.ts create mode 100644 packages/shared/src/limit/domain.ts create mode 100644 packages/shared/src/limit/utils.spec.ts create mode 100644 packages/shared/src/limit/utils.ts create mode 100644 packages/shared/src/tests/helpers/utils.ts diff --git a/packages/server/assets/comments/typedefs/comments.gql b/packages/server/assets/comments/typedefs/comments.gql index 30546a932..dbfe6c7e8 100644 --- a/packages/server/assets/comments/typedefs/comments.gql +++ b/packages/server/assets/comments/typedefs/comments.gql @@ -139,11 +139,11 @@ type Comment { authorId: String! archived: Boolean! screenshot: String - text: SmartTextEditorValue! + text: SmartTextEditorValue """ Plain-text version of the comment text, ideal for previews """ - rawText: String! + rawText: String """ Resources that this comment targets. Can be a mixture of either one stream, or multiple commits and objects. """ diff --git a/packages/server/modules/comments/graph/resolvers/comments.ts b/packages/server/modules/comments/graph/resolvers/comments.ts index ac114092f..b1c780efd 100644 --- a/packages/server/modules/comments/graph/resolvers/comments.ts +++ b/packages/server/modules/comments/graph/resolvers/comments.ts @@ -81,6 +81,7 @@ import { getCommitsAndTheirBranchIdsFactory, getSpecificBranchCommitsFactory } from '@/modules/core/repositories/commits' +import { getFeatureFlags } from '@/modules/shared/helpers/envHelper' import { getBranchLatestCommitsFactory, getStreamBranchesByNameFactory @@ -90,7 +91,15 @@ import { getStreamFactory } from '@/modules/core/repositories/streams' import { getProjectDbClient } from '@/modules/multiregion/utils/dbSelector' import { Knex } from 'knex' import { getEventBus } from '@/modules/shared/services/eventBus' +import { StreamNotFoundError } from '@/modules/core/errors/stream' +import { isCreatedBeyondHistoryLimitCutoff, getProjectLimitDate } from '@speckle/shared' import { throwIfAuthNotOk } from '@/modules/shared/helpers/errorHelper' +import { Authz } from '@speckle/shared' + +const { FF_FORCE_PERSONAL_PROJECTS_LIMITS_ENABLED } = getFeatureFlags() +const getPersonalProjectLimits = FF_FORCE_PERSONAL_PROJECTS_LIMITS_ENABLED + ? () => Promise.resolve(Authz.PersonalProjectsLimits) + : () => Promise.resolve(null) // We can use the main DB for these const getStream = getStreamFactory({ db }) @@ -203,15 +212,49 @@ export = { /** * Format comment.text for output, since it can have multiple formats */ - text(parent) { - const commentText = parent?.text || '' + async text(parent, _args, ctx) { + const project = await ctx.loaders.streams.getStream.load(parent.streamId) + + if (!project) { + throw new StreamNotFoundError('Project not found', { + info: { streamId: parent.streamId } + }) + } + + const isBeyondLimit = await isCreatedBeyondHistoryLimitCutoff({ + getProjectLimitDate: getProjectLimitDate({ + getWorkspaceLimits: ctx.authLoaders.getWorkspaceLimits, + getPersonalProjectLimits + }) + })({ entity: parent, limitType: 'commentHistory', project }) + // null is for out of limits + if (isBeyondLimit) return null + // why is the text nullable in the DB record? + if (!parent.text) return '' return { - ...ensureCommentSchema(commentText), + ...ensureCommentSchema(parent.text), projectId: parent.streamId } }, - rawText(parent) { + async rawText(parent, _args, ctx) { + const project = await ctx.loaders.streams.getStream.load(parent.streamId) + + if (!project) { + throw new StreamNotFoundError('Project not found', { + info: { streamId: parent.streamId } + }) + } + + const isBeyondLimit = await isCreatedBeyondHistoryLimitCutoff({ + getProjectLimitDate: getProjectLimitDate({ + getWorkspaceLimits: ctx.authLoaders.getWorkspaceLimits, + getPersonalProjectLimits + }) + })({ entity: parent, limitType: 'commentHistory', project }) + // null is for out of limits + if (isBeyondLimit) return null + // why us the text nullable in the DB? const { doc } = ensureCommentSchema(parent.text || '') return documentToBasicString(doc) }, diff --git a/packages/server/modules/comments/tests/comments.spec.ts b/packages/server/modules/comments/tests/comments.spec.ts index 199aa4763..eaa30649e 100644 --- a/packages/server/modules/comments/tests/comments.spec.ts +++ b/packages/server/modules/comments/tests/comments.spec.ts @@ -30,7 +30,11 @@ import { purgeNotifications } from '@/test/notificationsHelper' import { NotificationType } from '@/modules/notifications/helpers/types' -import { EmailSendingServiceMock, CommentsRepositoryMock } from '@/test/mocks/global' +import { + EmailSendingServiceMock, + CommentsRepositoryMock, + StreamsRepositoryMock +} from '@/test/mocks/global' import { createAuthedTestContext, ServerAndContext } from '@/test/graphqlHelper' import { checkStreamResourceAccessFactory, @@ -125,6 +129,7 @@ import { getViewerResourcesForCommentsFactory, getViewerResourcesFromLegacyIdentifiersFactory } from '@/modules/core/services/commit/viewerResources' +import { StreamRecord } from '@/modules/core/helpers/types' type LegacyCommentRecord = CommentRecord & { total_count: string @@ -287,6 +292,7 @@ function generateRandomCommentText() { const mailerMock = EmailSendingServiceMock const commentRepoMock = CommentsRepositoryMock +const streamsRepoMock = StreamsRepositoryMock describe('Comments @comments', () => { let app: express.Express @@ -366,6 +372,7 @@ describe('Comments @comments', () => { after(() => { notificationsState.destroy() commentRepoMock.destroy() + streamsRepoMock.destroy() }) afterEach(() => { @@ -1688,8 +1695,8 @@ describe('Comments @comments', () => { expect(errors?.length || 0).to.eq(0) expect(data?.comment).to.be.ok - expect(data?.comment?.text.doc).to.be.null - expect(data?.comment?.text.attachments?.length).to.be.greaterThan(0) + expect(data?.comment?.text?.doc).to.be.null + expect(data?.comment?.text?.attachments?.length).to.be.greaterThan(0) }) const unexpectedValDataset = [ @@ -1698,9 +1705,18 @@ describe('Comments @comments', () => { ] unexpectedValDataset.forEach(({ display, value }) => { it(`unexpected text value (${display}) in DB throw sanitized errors`, async () => { + streamsRepoMock.enable() + streamsRepoMock.mockFunction('getStreamsFactory', () => async () => [ + { + id: stream.id, + workspaceId: '' + } as unknown as StreamRecord + ]) const item = { id: '1', - text: value + text: value, + streamId: stream.id, + createdAt: new Date() } as unknown as LegacyCommentRecord commentRepoMock.enable() @@ -1710,12 +1726,13 @@ describe('Comments @comments', () => { totalCount: 1 })) - const { data, errors } = await readComments() + const { errors } = await readComments() - expect(data?.comments).to.not.be.ok expect((errors || []).map((e) => e.message).join(';')).to.contain( 'Unexpected comment schema format' ) + streamsRepoMock.disable() + streamsRepoMock.resetMockedFunctions() }) }) }) diff --git a/packages/server/modules/comments/tests/projectComments.spec.ts b/packages/server/modules/comments/tests/projectComments.spec.ts index a7a8dfff2..d2047bf91 100644 --- a/packages/server/modules/comments/tests/projectComments.spec.ts +++ b/packages/server/modules/comments/tests/projectComments.spec.ts @@ -112,7 +112,7 @@ describe('Project Comments', () => { expect(res1).to.not.haveGraphQLErrors() expect(threadId).to.be.ok expect(res1.data?.commentMutations.create.rawText).to.equal(parentText) - expect(res1.data?.commentMutations.create.text.doc).to.be.ok + expect(res1.data?.commentMutations.create.text?.doc).to.be.ok expect(res1.data?.commentMutations.create.authorId).to.equal(me.id) expect(createEventFired).to.be.true }) @@ -161,7 +161,7 @@ describe('Project Comments', () => { expect(res2).to.not.haveGraphQLErrors() expect(res2.data?.commentMutations.reply.rawText).to.equal(replyText) - expect(res2.data?.commentMutations.reply.text.doc).to.be.ok + expect(res2.data?.commentMutations.reply.text?.doc).to.be.ok expect(res2.data?.commentMutations.reply.authorId).to.equal(me.id) expect(replyEventFired).to.be.true }) @@ -190,7 +190,7 @@ describe('Project Comments', () => { expect(res).to.not.haveGraphQLErrors() expect(res.data?.commentMutations.edit.rawText).to.equal(newText) - expect(res.data?.commentMutations.edit.text.doc).to.be.ok + expect(res.data?.commentMutations.edit.text?.doc).to.be.ok expect(res.data?.commentMutations.edit.authorId).to.equal(me.id) expect(editEventFired).to.be.true }) diff --git a/packages/server/modules/core/graph/generated/graphql.ts b/packages/server/modules/core/graph/generated/graphql.ts index 9cb817cf3..01e4df22c 100644 --- a/packages/server/modules/core/graph/generated/graphql.ts +++ b/packages/server/modules/core/graph/generated/graphql.ts @@ -634,7 +634,7 @@ export type Comment = { parent?: Maybe; permissions: CommentPermissionChecks; /** Plain-text version of the comment text, ideal for previews */ - rawText: Scalars['String']['output']; + rawText?: Maybe; /** @deprecated Not actually implemented */ reactions?: Maybe>>; /** Gets the replies to this comment. */ @@ -644,7 +644,7 @@ export type Comment = { /** Resources that this comment targets. Can be a mixture of either one stream, or multiple commits and objects. */ resources: Array; screenshot?: Maybe; - text: SmartTextEditorValue; + text?: Maybe; /** The time this comment was last updated. Corresponds also to the latest reply to this comment, if any. */ updatedAt: Scalars['DateTime']['output']; /** The last time you viewed this comment. Present only if an auth'ed request. Relevant only if a top level commit. */ @@ -6153,13 +6153,13 @@ export type CommentResolvers; parent?: Resolver, ParentType, ContextType>; permissions?: Resolver; - rawText?: Resolver; + rawText?: Resolver, ParentType, ContextType>; reactions?: Resolver>>, ParentType, ContextType>; replies?: Resolver>; replyAuthors?: Resolver>; resources?: Resolver, ParentType, ContextType>; screenshot?: Resolver, ParentType, ContextType>; - text?: Resolver; + text?: Resolver, ParentType, ContextType>; updatedAt?: Resolver; viewedAt?: Resolver, ParentType, ContextType>; viewerResources?: Resolver, ParentType, ContextType>; diff --git a/packages/server/modules/core/graph/resolvers/commits.ts b/packages/server/modules/core/graph/resolvers/commits.ts index e35f6da92..2aaed6f48 100644 --- a/packages/server/modules/core/graph/resolvers/commits.ts +++ b/packages/server/modules/core/graph/resolvers/commits.ts @@ -27,8 +27,10 @@ import { batchDeleteCommitsFactory, batchMoveCommitsFactory } from '@/modules/core/services/commit/batchCommitActions' -import { StreamInvalidAccessError } from '@/modules/core/errors/stream' -import { isNonNullable, MaybeNullOrUndefined, Roles } from '@speckle/shared' +import { + StreamInvalidAccessError, + StreamNotFoundError +} from '@/modules/core/errors/stream' import { throwIfResourceAccessNotAllowed, toProjectIdWhitelist @@ -81,9 +83,18 @@ import { getEventBus } from '@/modules/shared/services/eventBus' import { TokenResourceIdentifierType } from '@/modules/core/domain/tokens/types' import { throwIfAuthNotOk } from '@/modules/shared/helpers/errorHelper' import { getFeatureFlags } from '@/modules/shared/helpers/envHelper' -import { getDateFromLimitsFactory } from '@/modules/core/services/versions/limits' +import { + Authz, + getProjectLimitDate, + isNonNullable, + MaybeNullOrUndefined, + Roles +} from '@speckle/shared' const { FF_FORCE_PERSONAL_PROJECTS_LIMITS_ENABLED } = getFeatureFlags() +const getPersonalProjectLimits = FF_FORCE_PERSONAL_PROJECTS_LIMITS_ENABLED + ? () => Promise.resolve(Authz.PersonalProjectsLimits) + : () => Promise.resolve(null) const getStreams = getStreamsFactory({ db }) @@ -209,12 +220,10 @@ export = { async commits(parent, args, ctx) { const projectDB = await getProjectDbClient({ projectId: parent.id }) - const limitsDate = await getDateFromLimitsFactory({ - environment: { - personalProjectsLimitEnabled: FF_FORCE_PERSONAL_PROJECTS_LIMITS_ENABLED - }, - getWorkspaceLimits: ctx.authLoaders.getWorkspaceLimits - })({ workspaceId: parent.workspaceId }) + const limitsDate = await getProjectLimitDate({ + getWorkspaceLimits: ctx.authLoaders.getWorkspaceLimits, + getPersonalProjectLimits + })({ limitType: 'versionsHistory', project: parent }) const getCommitsByStreamId = legacyGetPaginatedStreamCommitsPageFactory({ db: projectDB, @@ -330,13 +339,17 @@ export = { } } - const stream = await ctx.loaders.streams.getStream.load(parent.streamId) - const limitsDate = await getDateFromLimitsFactory({ - environment: { - personalProjectsLimitEnabled: FF_FORCE_PERSONAL_PROJECTS_LIMITS_ENABLED - }, - getWorkspaceLimits: ctx.authLoaders.getWorkspaceLimits - })({ workspaceId: stream?.workspaceId }) + const project = await ctx.loaders.streams.getStream.load(parent.streamId) + if (!project) { + throw new StreamNotFoundError('Project not found', { + info: { streamId: parent.streamId } + }) + } + + const limitsDate = await getProjectLimitDate({ + getWorkspaceLimits: ctx.authLoaders.getWorkspaceLimits, + getPersonalProjectLimits + })({ limitType: 'versionsHistory', project }) const getPaginatedBranchCommits = getPaginatedBranchCommitsFactory({ getSpecificBranchCommits: getSpecificBranchCommitsFactory({ db: projectDB }), diff --git a/packages/server/modules/core/graph/resolvers/versions.ts b/packages/server/modules/core/graph/resolvers/versions.ts index 51f4364b7..049d18c83 100644 --- a/packages/server/modules/core/graph/resolvers/versions.ts +++ b/packages/server/modules/core/graph/resolvers/versions.ts @@ -48,14 +48,21 @@ import { getProjectDbClient } from '@/modules/multiregion/utils/dbSelector' import coreModule from '@/modules/core' import { getEventBus } from '@/modules/shared/services/eventBus' import { StreamNotFoundError } from '@/modules/core/errors/stream' -import { getLimitedReferencedObjectFactory } from '@/modules/core/services/versions/limits' import { throwIfResourceAccessNotAllowed } from '@/modules/core/helpers/token' import { TokenResourceIdentifierType } from '@/modules/core/domain/tokens/types' import { throwIfAuthNotOk } from '@/modules/shared/helpers/errorHelper' import { Version } from '@/modules/core/domain/commits/types' import { GraphQLResolveInfo } from 'graphql' +import { + Authz, + getProjectLimitDate, + isCreatedBeyondHistoryLimitCutoff +} from '@speckle/shared' const { FF_FORCE_PERSONAL_PROJECTS_LIMITS_ENABLED } = getFeatureFlags() +const getPersonalProjectLimits = FF_FORCE_PERSONAL_PROJECTS_LIMITS_ENABLED + ? () => Promise.resolve(Authz.PersonalProjectsLimits) + : () => Promise.resolve(null) /** * Simple utility to check if version is inside a Model or a Project @@ -126,12 +133,12 @@ export = { }) } - const getLimitedReferencedObject = getLimitedReferencedObjectFactory({ - environment: { - personalProjectsLimitEnabled: FF_FORCE_PERSONAL_PROJECTS_LIMITS_ENABLED - }, - getWorkspaceLimits: ctx.authLoaders.getWorkspaceLimits - }) + const isBeyondLimit = await isCreatedBeyondHistoryLimitCutoff({ + getProjectLimitDate: getProjectLimitDate({ + getWorkspaceLimits: ctx.authLoaders.getWorkspaceLimits, + getPersonalProjectLimits + }) + })({ entity: parent, limitType: 'versionsHistory', project }) let lastVersion: Version | null if (getTypeFromPath(info) === 'Model') { lastVersion = await ctx.loaders @@ -143,10 +150,8 @@ export = { .streams.getLastVersion.load(parent.streamId) } if (lastVersion?.id === parent.id) return parent.referencedObject - return await getLimitedReferencedObject({ - version: parent, - workspaceId: project.workspaceId - }) + if (isBeyondLimit) return null + return parent.referencedObject } }, Mutation: { diff --git a/packages/server/modules/core/helpers/testHelpers.ts b/packages/server/modules/core/helpers/testHelpers.ts index 1b771176b..0404b06c4 100644 --- a/packages/server/modules/core/helpers/testHelpers.ts +++ b/packages/server/modules/core/helpers/testHelpers.ts @@ -8,6 +8,9 @@ export function createRandomPassword(length?: number) { return crs({ length: length ?? 10 }) } +/** + * @deprecated use the one in shared + */ export function createRandomString(length?: number) { return crs({ length: length ?? 10 }) } diff --git a/packages/server/modules/core/services/versions/limits.ts b/packages/server/modules/core/services/versions/limits.ts deleted file mode 100644 index 7f79958e6..000000000 --- a/packages/server/modules/core/services/versions/limits.ts +++ /dev/null @@ -1,68 +0,0 @@ -import { Version } from '@/modules/core/domain/commits/types' -import { GetWorkspaceLimits } from '@speckle/shared/dist/commonjs/authz/domain/workspaces/operations' -import dayjs from 'dayjs' - -export const PersonalProjectsLimits: { - versionHistory: { value: number; unit: 'week' } -} = { - versionHistory: { - value: 1, - unit: 'week' - } -} - -export const getLimitedReferencedObjectFactory = - ({ - environment: { personalProjectsLimitEnabled }, - getWorkspaceLimits - }: { - environment: { personalProjectsLimitEnabled: boolean } - getWorkspaceLimits: GetWorkspaceLimits - }) => - async ({ - version, - workspaceId - }: { - version: Pick - workspaceId?: string | null - }) => { - const limitDate = await getDateFromLimitsFactory({ - environment: { personalProjectsLimitEnabled }, - getWorkspaceLimits - })({ workspaceId }) - - if (dayjs(limitDate).isAfter(version.createdAt)) return null - return version.referencedObject - } - -export const getDateFromLimitsFactory = - ({ - getWorkspaceLimits, - environment: { personalProjectsLimitEnabled } - }: { - getWorkspaceLimits: GetWorkspaceLimits - environment: { personalProjectsLimitEnabled: boolean } - }) => - async ({ workspaceId }: { workspaceId?: string | null }) => { - if (workspaceId) { - const limits = await getWorkspaceLimits({ workspaceId }) - if (!limits?.versionsHistory) { - return null - } - - return dayjs() - .subtract(limits.versionsHistory.value, limits.versionsHistory.unit) - .toDate() - } - - if (!personalProjectsLimitEnabled) { - return null - } - - return dayjs() - .subtract( - PersonalProjectsLimits.versionHistory.value, - PersonalProjectsLimits.versionHistory.unit - ) - .toDate() - } diff --git a/packages/server/modules/core/tests/integration/versions.graph.spec.ts b/packages/server/modules/core/tests/integration/versions.graph.spec.ts index 4c49129df..3bd5a1803 100644 --- a/packages/server/modules/core/tests/integration/versions.graph.spec.ts +++ b/packages/server/modules/core/tests/integration/versions.graph.spec.ts @@ -83,7 +83,7 @@ const createUser = createUserFactory({ emitEvent: getEventBus().emit }) -const { FF_BILLING_INTEGRATION_ENABLED, FF_WORKSPACES_MODULE_ENABLED } = +const { FF_BILLING_INTEGRATION_ENABLED, FF_FORCE_PERSONAL_PROJECTS_LIMITS_ENABLED } = getFeatureFlags() describe('Versions graphql @core', () => { @@ -136,11 +136,11 @@ describe('Versions graphql @core', () => { } ) }) - ;(FF_WORKSPACES_MODULE_ENABLED ? describe : describe.skip)( + ;(FF_FORCE_PERSONAL_PROJECTS_LIMITS_ENABLED ? describe : describe.skip)( 'Version.referencedObject', () => { + const tenDaysAgo = dayjs().subtract(10, 'day').toDate() it('should return version referencedObject if version is the last model version', async () => { - const tenDaysAgo = dayjs().subtract(10, 'day').toDate() const user = await createTestUser({ name: createRandomString(), email: createRandomEmail() @@ -235,25 +235,14 @@ describe('Versions graphql @core', () => { }) }) it('should return version referencedObject if version is the last project version', async () => { - const tenDaysAgo = dayjs().subtract(10, 'day').toDate() const user = await createTestUser({ name: createRandomString(), email: createRandomEmail() }) - const workspace = { - id: createRandomString(), - name: createRandomString(), - slug: createRandomString(), - ownerId: user.id - } - await createTestWorkspace(workspace, user, { - addPlan: { name: 'free', status: 'valid' } - }) const project1 = { id: '', - name: createRandomString(), - workspaceId: workspace.id + name: createRandomString() } await createTestStream(project1, user) diff --git a/packages/server/modules/core/tests/unit/services/versions.spec.ts b/packages/server/modules/core/tests/unit/services/versions.spec.ts deleted file mode 100644 index b9d3fe21f..000000000 --- a/packages/server/modules/core/tests/unit/services/versions.spec.ts +++ /dev/null @@ -1,146 +0,0 @@ -import { createRandomString } from '@/modules/core/helpers/testHelpers' -import { - getDateFromLimitsFactory, - getLimitedReferencedObjectFactory -} from '@/modules/core/services/versions/limits' -import { GetWorkspaceLimits } from '@speckle/shared/dist/commonjs/authz/domain/workspaces/operations' -import { expect } from 'chai' -import dayjs from 'dayjs' - -describe('Module @core', () => { - describe('Services versions', () => { - describe('getDateFromLimits', () => { - it('should return null if workspace has no versionHistory limits', async () => { - const getWorkspaceLimits = async () => null - const workspaceId = createRandomString() - - expect( - await getDateFromLimitsFactory({ - getWorkspaceLimits, - environment: { personalProjectsLimitEnabled: false } - })({ workspaceId }) - ).to.eq(null) - }) - it('should return date in workspace versionHistory limits', async () => { - const getWorkspaceLimits = async () => ({ - projectCount: null, - modelCount: null, - versionsHistory: { value: 1, unit: 'month' as const } - }) - const workspaceId = createRandomString() - - expect( - ( - await getDateFromLimitsFactory({ - getWorkspaceLimits, - environment: { personalProjectsLimitEnabled: false } - })({ workspaceId }) - ) - ?.toISOString() - .slice(0, -5) - ).to.eq(dayjs().subtract(1, 'month').toDate().toISOString().slice(0, -5)) - }) - }) - describe('getLimitedReferencedObjectFactory returns a function that, ', () => { - it('should return the version referencedObject if project workspace has no limits', async () => { - const getWorkspaceLimits = (() => null) as unknown as GetWorkspaceLimits - const workspaceId = createRandomString() - const version = { - referencedObject: createRandomString(), - createdAt: new Date() - } - expect( - await getLimitedReferencedObjectFactory({ - environment: { personalProjectsLimitEnabled: false }, - getWorkspaceLimits - })({ version, workspaceId }) - ).to.eq(version.referencedObject) - }) - it('should return null if version is outside of workspace limit', async () => { - const getWorkspaceLimits = (() => ({ - versionsHistory: { value: 7, unit: 'day' } - })) as unknown as GetWorkspaceLimits - const workspaceId = createRandomString() - const tenDaysAgo = new Date() - tenDaysAgo.setDate(new Date().getDate() - 10) - const version = { - referencedObject: createRandomString(), - createdAt: tenDaysAgo - } - expect( - await getLimitedReferencedObjectFactory({ - environment: { personalProjectsLimitEnabled: false }, - getWorkspaceLimits - })({ version, workspaceId }) - ).to.eq(null) - }) - it('should return version referencedObject if version is inside of workspace limit', async () => { - const getWorkspaceLimits = (() => ({ - versionsHistory: { value: 7, unit: 'day' } - })) as unknown as GetWorkspaceLimits - const workspaceId = createRandomString() - const twoDaysAgo = new Date() - twoDaysAgo.setDate(new Date().getDate() - 2) - const version = { - referencedObject: createRandomString(), - createdAt: twoDaysAgo - } - expect( - await getLimitedReferencedObjectFactory({ - environment: { personalProjectsLimitEnabled: false }, - getWorkspaceLimits - })({ version, workspaceId }) - ).to.eq(version.referencedObject) - }) - it('should return version referencedObject if project is not in a workspace and personalProjectsLimits is not enabled', async () => { - const getWorkspaceLimits = (() => - expect.fail()) as unknown as GetWorkspaceLimits - const workspaceId = null - const tenDaysAgo = new Date() - tenDaysAgo.setDate(new Date().getDate() - 10) - const version = { - referencedObject: createRandomString(), - createdAt: tenDaysAgo - } - expect( - await getLimitedReferencedObjectFactory({ - environment: { personalProjectsLimitEnabled: false }, - getWorkspaceLimits - })({ version, workspaceId }) - ).to.eq(version.referencedObject) - }) - it('should return null if project is not in a workspace and personalProjectsLimits is enabled and version is outside of limits', async () => { - const getWorkspaceLimits = (() => - expect.fail()) as unknown as GetWorkspaceLimits - const workspaceId = null - const tenDaysAgo = new Date() - tenDaysAgo.setDate(new Date().getDate() - 10) - const version = { - referencedObject: createRandomString(), - createdAt: tenDaysAgo - } - expect( - await getLimitedReferencedObjectFactory({ - environment: { personalProjectsLimitEnabled: true }, - getWorkspaceLimits - })({ version, workspaceId }) - ).to.eq(null) - }) - it('should return version referencedObject if project is not in a workspace and personalProjectsLimits is enabled and version is inside limits', async () => { - const getWorkspaceLimits = (() => - expect.fail()) as unknown as GetWorkspaceLimits - const workspaceId = null - const version = { - referencedObject: createRandomString(), - createdAt: new Date() - } - expect( - await getLimitedReferencedObjectFactory({ - environment: { personalProjectsLimitEnabled: true }, - getWorkspaceLimits - })({ version, workspaceId }) - ).to.eq(version.referencedObject) - }) - }) - }) -}) diff --git a/packages/server/modules/cross-server-sync/graph/generated/graphql.ts b/packages/server/modules/cross-server-sync/graph/generated/graphql.ts index 6722d303a..1ec3c8884 100644 --- a/packages/server/modules/cross-server-sync/graph/generated/graphql.ts +++ b/packages/server/modules/cross-server-sync/graph/generated/graphql.ts @@ -614,7 +614,7 @@ export type Comment = { parent?: Maybe; permissions: CommentPermissionChecks; /** Plain-text version of the comment text, ideal for previews */ - rawText: Scalars['String']['output']; + rawText?: Maybe; /** @deprecated Not actually implemented */ reactions?: Maybe>>; /** Gets the replies to this comment. */ @@ -624,7 +624,7 @@ export type Comment = { /** Resources that this comment targets. Can be a mixture of either one stream, or multiple commits and objects. */ resources: Array; screenshot?: Maybe; - text: SmartTextEditorValue; + text?: Maybe; /** The time this comment was last updated. Corresponds also to the latest reply to this comment, if any. */ updatedAt: Scalars['DateTime']['output']; /** The last time you viewed this comment. Present only if an auth'ed request. Relevant only if a top level commit. */ @@ -5114,9 +5114,9 @@ export type CrossSyncDownloadableCommitViewerThreadsQueryVariables = Exact<{ }>; -export type CrossSyncDownloadableCommitViewerThreadsQuery = { __typename?: 'Query', project: { __typename?: 'Project', id: string, commentThreads: { __typename?: 'ProjectCommentCollection', totalCount: number, totalArchivedCount: number, items: Array<{ __typename?: 'Comment', id: string, viewerState?: Record | null, screenshot?: string | null, replies: { __typename?: 'CommentCollection', items: Array<{ __typename?: 'Comment', id: string, viewerState?: Record | null, screenshot?: string | null, text: { __typename?: 'SmartTextEditorValue', doc?: Record | null } }> }, text: { __typename?: 'SmartTextEditorValue', doc?: Record | null } }> } } }; +export type CrossSyncDownloadableCommitViewerThreadsQuery = { __typename?: 'Query', project: { __typename?: 'Project', id: string, commentThreads: { __typename?: 'ProjectCommentCollection', totalCount: number, totalArchivedCount: number, items: Array<{ __typename?: 'Comment', id: string, viewerState?: Record | null, screenshot?: string | null, replies: { __typename?: 'CommentCollection', items: Array<{ __typename?: 'Comment', id: string, viewerState?: Record | null, screenshot?: string | null, text?: { __typename?: 'SmartTextEditorValue', doc?: Record | null } | null }> }, text?: { __typename?: 'SmartTextEditorValue', doc?: Record | null } | null }> } } }; -export type DownloadbleCommentMetadataFragment = { __typename?: 'Comment', id: string, viewerState?: Record | null, screenshot?: string | null, text: { __typename?: 'SmartTextEditorValue', doc?: Record | null } }; +export type DownloadbleCommentMetadataFragment = { __typename?: 'Comment', id: string, viewerState?: Record | null, screenshot?: string | null, text?: { __typename?: 'SmartTextEditorValue', doc?: Record | null } | null }; export type CrossSyncProjectMetadataQueryVariables = Exact<{ id: Scalars['String']['input']; diff --git a/packages/server/modules/cross-server-sync/services/commit.ts b/packages/server/modules/cross-server-sync/services/commit.ts index 02710b2b2..0d48a226c 100644 --- a/packages/server/modules/cross-server-sync/services/commit.ts +++ b/packages/server/modules/cross-server-sync/services/commit.ts @@ -393,13 +393,13 @@ const saveNewThreadsFactory = const threadInputs: { originalComment: ViewerThread; input: CreateCommentInput }[] = threads - .filter((t) => !!t.text.doc) + .filter((t) => !!t.text?.doc) .map((t) => ({ originalComment: t, input: { projectId: targetStream.id, content: { - doc: t.text.doc, + doc: t.text?.doc, blobIds: [] // TODO: Currently not supported }, viewerState: t.viewerState @@ -436,12 +436,12 @@ const saveNewThreadsFactory = ) await Promise.all( replies.items - .filter((i) => !!i.text.doc) + .filter((i) => !!i.text?.doc) .map((r) => deps.createCommentReplyAndNotify( { content: { - doc: r.text.doc, + doc: r.text?.doc, blobIds: [] }, threadId: newComment.id, diff --git a/packages/server/test/graphql/generated/graphql.ts b/packages/server/test/graphql/generated/graphql.ts index 584761421..8713b5f7e 100644 --- a/packages/server/test/graphql/generated/graphql.ts +++ b/packages/server/test/graphql/generated/graphql.ts @@ -615,7 +615,7 @@ export type Comment = { parent?: Maybe; permissions: CommentPermissionChecks; /** Plain-text version of the comment text, ideal for previews */ - rawText: Scalars['String']['output']; + rawText?: Maybe; /** @deprecated Not actually implemented */ reactions?: Maybe>>; /** Gets the replies to this comment. */ @@ -625,7 +625,7 @@ export type Comment = { /** Resources that this comment targets. Can be a mixture of either one stream, or multiple commits and objects. */ resources: Array; screenshot?: Maybe; - text: SmartTextEditorValue; + text?: Maybe; /** The time this comment was last updated. Corresponds also to the latest reply to this comment, if any. */ updatedAt: Scalars['DateTime']['output']; /** The last time you viewed this comment. Present only if an auth'ed request. Relevant only if a top level commit. */ @@ -5507,7 +5507,7 @@ export type AutomateValidateAuthCodeQueryVariables = Exact<{ export type AutomateValidateAuthCodeQuery = { __typename?: 'Query', automateValidateAuthCode: boolean }; -export type CommentWithRepliesFragment = { __typename?: 'Comment', id: string, rawText: string, text: { __typename?: 'SmartTextEditorValue', doc?: Record | null, attachments?: Array<{ __typename?: 'BlobMetadata', id: string, fileName: string, streamId: string }> | null }, replies: { __typename?: 'CommentCollection', items: Array<{ __typename?: 'Comment', id: string, text: { __typename?: 'SmartTextEditorValue', doc?: Record | null, attachments?: Array<{ __typename?: 'BlobMetadata', id: string, fileName: string, streamId: string }> | null } }> } }; +export type CommentWithRepliesFragment = { __typename?: 'Comment', id: string, rawText?: string | null, text?: { __typename?: 'SmartTextEditorValue', doc?: Record | null, attachments?: Array<{ __typename?: 'BlobMetadata', id: string, fileName: string, streamId: string }> | null } | null, replies: { __typename?: 'CommentCollection', items: Array<{ __typename?: 'Comment', id: string, text?: { __typename?: 'SmartTextEditorValue', doc?: Record | null, attachments?: Array<{ __typename?: 'BlobMetadata', id: string, fileName: string, streamId: string }> | null } | null }> } }; export type CreateCommentMutationVariables = Exact<{ input: CommentCreateInput; @@ -5529,7 +5529,7 @@ export type GetCommentQueryVariables = Exact<{ }>; -export type GetCommentQuery = { __typename?: 'Query', comment?: { __typename?: 'Comment', id: string, rawText: string, text: { __typename?: 'SmartTextEditorValue', doc?: Record | null, attachments?: Array<{ __typename?: 'BlobMetadata', id: string, fileName: string, streamId: string }> | null }, replies: { __typename?: 'CommentCollection', items: Array<{ __typename?: 'Comment', id: string, text: { __typename?: 'SmartTextEditorValue', doc?: Record | null, attachments?: Array<{ __typename?: 'BlobMetadata', id: string, fileName: string, streamId: string }> | null } }> } } | null }; +export type GetCommentQuery = { __typename?: 'Query', comment?: { __typename?: 'Comment', id: string, rawText?: string | null, text?: { __typename?: 'SmartTextEditorValue', doc?: Record | null, attachments?: Array<{ __typename?: 'BlobMetadata', id: string, fileName: string, streamId: string }> | null } | null, replies: { __typename?: 'CommentCollection', items: Array<{ __typename?: 'Comment', id: string, text?: { __typename?: 'SmartTextEditorValue', doc?: Record | null, attachments?: Array<{ __typename?: 'BlobMetadata', id: string, fileName: string, streamId: string }> | null } | null }> } } | null }; export type GetCommentsQueryVariables = Exact<{ streamId: Scalars['String']['input']; @@ -5537,7 +5537,7 @@ export type GetCommentsQueryVariables = Exact<{ }>; -export type GetCommentsQuery = { __typename?: 'Query', comments?: { __typename?: 'CommentCollection', totalCount: number, cursor?: string | null, items: Array<{ __typename?: 'Comment', id: string, rawText: string, text: { __typename?: 'SmartTextEditorValue', doc?: Record | null, attachments?: Array<{ __typename?: 'BlobMetadata', id: string, fileName: string, streamId: string }> | null }, replies: { __typename?: 'CommentCollection', items: Array<{ __typename?: 'Comment', id: string, text: { __typename?: 'SmartTextEditorValue', doc?: Record | null, attachments?: Array<{ __typename?: 'BlobMetadata', id: string, fileName: string, streamId: string }> | null } }> } }> } | null }; +export type GetCommentsQuery = { __typename?: 'Query', comments?: { __typename?: 'CommentCollection', totalCount: number, cursor?: string | null, items: Array<{ __typename?: 'Comment', id: string, rawText?: string | null, text?: { __typename?: 'SmartTextEditorValue', doc?: Record | null, attachments?: Array<{ __typename?: 'BlobMetadata', id: string, fileName: string, streamId: string }> | null } | null, replies: { __typename?: 'CommentCollection', items: Array<{ __typename?: 'Comment', id: string, text?: { __typename?: 'SmartTextEditorValue', doc?: Record | null, attachments?: Array<{ __typename?: 'BlobMetadata', id: string, fileName: string, streamId: string }> | null } | null }> } }> } | null }; export type BaseCommitFieldsFragment = { __typename?: 'Commit', id: string, authorName?: string | null, authorId?: string | null, authorAvatar?: string | null, streamId?: string | null, streamName?: string | null, sourceApplication?: string | null, message?: string | null, referencedObject: string, createdAt?: string | null, commentCount: number }; @@ -5732,28 +5732,28 @@ export type UseProjectAccessRequestMutationVariables = Exact<{ export type UseProjectAccessRequestMutation = { __typename?: 'Mutation', projectMutations: { __typename?: 'ProjectMutations', accessRequestMutations: { __typename?: 'ProjectAccessRequestMutations', use: { __typename?: 'Project', id: string } } } }; -export type BasicProjectCommentFragment = { __typename?: 'Comment', id: string, rawText: string, authorId: string, text: { __typename?: 'SmartTextEditorValue', doc?: Record | null } }; +export type BasicProjectCommentFragment = { __typename?: 'Comment', id: string, rawText?: string | null, authorId: string, text?: { __typename?: 'SmartTextEditorValue', doc?: Record | null } | null }; export type CreateProjectCommentMutationVariables = Exact<{ input: CreateCommentInput; }>; -export type CreateProjectCommentMutation = { __typename?: 'Mutation', commentMutations: { __typename?: 'CommentMutations', create: { __typename?: 'Comment', id: string, rawText: string, authorId: string, text: { __typename?: 'SmartTextEditorValue', doc?: Record | null } } } }; +export type CreateProjectCommentMutation = { __typename?: 'Mutation', commentMutations: { __typename?: 'CommentMutations', create: { __typename?: 'Comment', id: string, rawText?: string | null, authorId: string, text?: { __typename?: 'SmartTextEditorValue', doc?: Record | null } | null } } }; export type CreateProjectCommentReplyMutationVariables = Exact<{ input: CreateCommentReplyInput; }>; -export type CreateProjectCommentReplyMutation = { __typename?: 'Mutation', commentMutations: { __typename?: 'CommentMutations', reply: { __typename?: 'Comment', id: string, rawText: string, authorId: string, text: { __typename?: 'SmartTextEditorValue', doc?: Record | null } } } }; +export type CreateProjectCommentReplyMutation = { __typename?: 'Mutation', commentMutations: { __typename?: 'CommentMutations', reply: { __typename?: 'Comment', id: string, rawText?: string | null, authorId: string, text?: { __typename?: 'SmartTextEditorValue', doc?: Record | null } | null } } }; export type EditProjectCommentMutationVariables = Exact<{ input: EditCommentInput; }>; -export type EditProjectCommentMutation = { __typename?: 'Mutation', commentMutations: { __typename?: 'CommentMutations', edit: { __typename?: 'Comment', id: string, rawText: string, authorId: string, text: { __typename?: 'SmartTextEditorValue', doc?: Record | null } } } }; +export type EditProjectCommentMutation = { __typename?: 'Mutation', commentMutations: { __typename?: 'CommentMutations', edit: { __typename?: 'Comment', id: string, rawText?: string | null, authorId: string, text?: { __typename?: 'SmartTextEditorValue', doc?: Record | null } | null } } }; export type BasicProjectFieldsFragment = { __typename?: 'Project', id: string, name: string, description?: string | null, visibility: SimpleProjectVisibility, allowPublicComments: boolean, role?: string | null, createdAt: string, updatedAt: string }; diff --git a/packages/server/test/mocks/global.ts b/packages/server/test/mocks/global.ts index 2bb09e5f5..5f32df19c 100644 --- a/packages/server/test/mocks/global.ts +++ b/packages/server/test/mocks/global.ts @@ -38,6 +38,10 @@ export const EnvHelperMock = mockRequireModule< ['@/modules/shared/index'] ) +export const StreamsRepositoryMock = mockRequireModule< + typeof import('@/modules/core/repositories/streams') +>(['@/modules/core/repositories/streams']) + export const mockAdminOverride = () => { const enable = (enabled: boolean) => { EnvHelperMock.mockFunction('adminOverrideEnabled', () => enabled) diff --git a/packages/shared/package.json b/packages/shared/package.json index 264f1b06d..1b0890c6d 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -36,6 +36,7 @@ "3d" ], "dependencies": { + "dayjs": "^1.11.13", "lodash": "^4.17.21", "lodash-es": "^4.17.21", "nanoid": "^5.1.5", diff --git a/packages/shared/src/authz/domain/projects/limits.ts b/packages/shared/src/authz/domain/projects/limits.ts new file mode 100644 index 000000000..357720396 --- /dev/null +++ b/packages/shared/src/authz/domain/projects/limits.ts @@ -0,0 +1,12 @@ +import { HistoryLimits } from '../../../limit/domain.js' + +export const PersonalProjectsLimits: HistoryLimits = { + versionsHistory: { + value: 1, + unit: 'week' + }, + commentHistory: { + value: 1, + unit: 'week' + } +} diff --git a/packages/shared/src/authz/index.ts b/packages/shared/src/authz/index.ts index 4fbbd743e..ed7c4aa17 100644 --- a/packages/shared/src/authz/index.ts +++ b/packages/shared/src/authz/index.ts @@ -7,3 +7,4 @@ export { export * from './helpers/graphql.js' export * from './domain/authErrors.js' export { AuthPolicyResult } from './domain/policies.js' +export { PersonalProjectsLimits } from './domain/projects/limits.js' diff --git a/packages/shared/src/authz/policies/project/canMoveToWorkspace.spec.ts b/packages/shared/src/authz/policies/project/canMoveToWorkspace.spec.ts index 82d7fbcdc..5a305f9d9 100644 --- a/packages/shared/src/authz/policies/project/canMoveToWorkspace.spec.ts +++ b/packages/shared/src/authz/policies/project/canMoveToWorkspace.spec.ts @@ -51,7 +51,8 @@ const buildCanMoveToWorkspace = ( return { modelCount: 5, projectCount: 5, - versionsHistory: null + versionsHistory: null, + commentHistory: null } }, getWorkspaceProjectCount: async () => { @@ -131,7 +132,8 @@ describe('canMoveToWorkspacePolicy returns a function, that', () => { return { projectCount: 1, modelCount: 5, - versionsHistory: null + versionsHistory: null, + commentHistory: null } }, getWorkspaceProjectCount: async () => { diff --git a/packages/shared/src/authz/policies/project/model/canCreate.spec.ts b/packages/shared/src/authz/policies/project/model/canCreate.spec.ts index eb11989f2..3727f436a 100644 --- a/packages/shared/src/authz/policies/project/model/canCreate.spec.ts +++ b/packages/shared/src/authz/policies/project/model/canCreate.spec.ts @@ -54,7 +54,8 @@ const buildCanCreateModelPolicy = ( return { modelCount: 5, projectCount: 1, - versionsHistory: null + versionsHistory: null, + commentHistory: null } }, getWorkspaceModelCount: async () => { @@ -143,7 +144,8 @@ describe('canCreateModelPolicy returns a function, that', () => { return { projectCount: 1, modelCount: 5, - versionsHistory: null + versionsHistory: null, + commentHistory: null } }, getWorkspaceModelCount: async () => { diff --git a/packages/shared/src/authz/policies/workspace/canCreateWorkspaceProject.spec.ts b/packages/shared/src/authz/policies/workspace/canCreateWorkspaceProject.spec.ts index ee3c94c86..f8cb27bd0 100644 --- a/packages/shared/src/authz/policies/workspace/canCreateWorkspaceProject.spec.ts +++ b/packages/shared/src/authz/policies/workspace/canCreateWorkspaceProject.spec.ts @@ -477,7 +477,8 @@ describe('canCreateWorkspaceProjectPolicy creates a function, that handles', () return { projectCount: null, modelCount: null, - versionsHistory: null + versionsHistory: null, + commentHistory: null } }, getWorkspaceProjectCount: async () => { @@ -517,7 +518,8 @@ describe('canCreateWorkspaceProjectPolicy creates a function, that handles', () return { projectCount: 10, modelCount: 50, - versionsHistory: null + versionsHistory: null, + commentHistory: null } }, getWorkspaceProjectCount: async () => { @@ -559,7 +561,8 @@ describe('canCreateWorkspaceProjectPolicy creates a function, that handles', () return { projectCount: 10, modelCount: 50, - versionsHistory: null + versionsHistory: null, + commentHistory: null } }, getWorkspaceProjectCount: async () => { @@ -599,7 +602,8 @@ describe('canCreateWorkspaceProjectPolicy creates a function, that handles', () return { projectCount: 10, modelCount: 50, - versionsHistory: null + versionsHistory: null, + commentHistory: null } }, getWorkspaceProjectCount: async () => { diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 30a862992..6d42eb36e 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -7,3 +7,4 @@ export * as Authz from './authz/index.js' export * from './core/index.js' export * from './workspaces/index.js' export * from './onboarding/index.js' +export * from './limit/utils.js' diff --git a/packages/shared/src/limit/domain.ts b/packages/shared/src/limit/domain.ts new file mode 100644 index 000000000..b6dcffa1f --- /dev/null +++ b/packages/shared/src/limit/domain.ts @@ -0,0 +1,13 @@ +export type HistoryLimit = { value: number; unit: 'day' | 'week' | 'month' } + +export type HistoryLimits = Record + +export const HistoryLimitTypes = { + versionsHistory: 'versionsHistory', + commentHistory: 'commentHistory' +} as const + +export type HistoryLimitTypes = + (typeof HistoryLimitTypes)[keyof typeof HistoryLimitTypes] + +export type GetHistoryLimits = () => Promise diff --git a/packages/shared/src/limit/utils.spec.ts b/packages/shared/src/limit/utils.spec.ts new file mode 100644 index 000000000..dde83da87 --- /dev/null +++ b/packages/shared/src/limit/utils.spec.ts @@ -0,0 +1,95 @@ +import { describe, expect, it } from 'vitest' +import { + calculateLimitCutoffDate, + getProjectLimitDate, + isCreatedBeyondHistoryLimitCutoff +} from './utils.js' +import dayjs from 'dayjs' + +describe('Limits utils', () => { + describe('calculateLimitCutoffDate', () => { + it('returns null if historyLimits is null', () => { + const cutoffDate = calculateLimitCutoffDate(null, 'commentHistory') + expect(cutoffDate).toBeNull() + }) + it('returns null if limitType is null on the historyLimit', () => { + const cutoffDate = calculateLimitCutoffDate( + { commentHistory: { value: 1, unit: 'day' }, versionsHistory: null }, + 'versionsHistory' + ) + expect(cutoffDate).toBeNull() + }) + it('returns cutoff date from limits', () => { + const cutoffDate = calculateLimitCutoffDate( + { commentHistory: { value: 1, unit: 'day' }, versionsHistory: null }, + 'commentHistory' + ) + const now = dayjs() + expect(now.diff(cutoffDate, 'days')).to.equal(1) + }) + }) + describe('getProjectLimitDate', () => { + it('returns workspaceLimits for workspace projects', async () => { + const cutoffDate = await getProjectLimitDate({ + getPersonalProjectLimits: () => { + expect.fail() + }, + getWorkspaceLimits: async () => null + })({ limitType: 'commentHistory', project: { workspaceId: 'asdfg12345' } }) + expect(cutoffDate).toBeNull() + }) + it('returns projectLimits for non workspaceProjects', async () => { + const cutoffDate = await getProjectLimitDate({ + getPersonalProjectLimits: async () => null, + getWorkspaceLimits: async () => { + expect.fail() + } + })({ limitType: 'commentHistory', project: { workspaceId: null } }) + expect(cutoffDate).toBeNull() + }) + }) + describe('isCreatedBeyondHistoryLimitCutoff', () => { + it('returns false if there are no limits', async () => { + const isCreatedBeyondHistoryLimit = await isCreatedBeyondHistoryLimitCutoff({ + getProjectLimitDate: async () => null + })({ + entity: { createdAt: new Date() }, + limitType: 'commentHistory', + project: { workspaceId: null } + }) + expect(isCreatedBeyondHistoryLimit).to.be.toBeFalsy() + }) + it('returns false if entity is newer than the limit cutoff date', async () => { + const isCreatedBeyondHistoryLimit = await isCreatedBeyondHistoryLimitCutoff({ + getProjectLimitDate: async () => new Date(1999) + })({ + entity: { createdAt: new Date() }, + limitType: 'commentHistory', + project: { workspaceId: null } + }) + expect(isCreatedBeyondHistoryLimit).to.be.toBeFalsy() + }) + it('returns false if entity is right on the limit cutoff date', async () => { + const date = new Date() + const isCreatedBeyondHistoryLimit = await isCreatedBeyondHistoryLimitCutoff({ + getProjectLimitDate: async () => date + })({ + entity: { createdAt: date }, + limitType: 'commentHistory', + project: { workspaceId: null } + }) + expect(isCreatedBeyondHistoryLimit).to.be.toBeFalsy() + }) + }) + it('returns true of entity is older than the limit cutoff date', async () => { + const date = new Date() + const isCreatedBeyondHistoryLimit = await isCreatedBeyondHistoryLimitCutoff({ + getProjectLimitDate: async () => date + })({ + entity: { createdAt: new Date(1999) }, + limitType: 'commentHistory', + project: { workspaceId: null } + }) + expect(isCreatedBeyondHistoryLimit).to.be.toBeTruthy() + }) +}) diff --git a/packages/shared/src/limit/utils.ts b/packages/shared/src/limit/utils.ts new file mode 100644 index 000000000..9c8074c8c --- /dev/null +++ b/packages/shared/src/limit/utils.ts @@ -0,0 +1,53 @@ +import dayjs from 'dayjs' +import { GetWorkspaceLimits } from '../authz/domain/workspaces/operations.js' +import { GetHistoryLimits, HistoryLimitTypes, HistoryLimits } from './domain.js' +import { Project } from '../authz/domain/projects/types.js' + +export const isCreatedBeyondHistoryLimitCutoff = + ({ getProjectLimitDate }: { getProjectLimitDate: GetProjectLimitDate }) => + async ({ + entity, + project, + limitType + }: { + entity: { createdAt: Date } + project: Pick + limitType: HistoryLimitTypes + }): Promise => { + const limitDate = await getProjectLimitDate({ + project, + limitType + }) + return limitDate ? dayjs(limitDate).isAfter(entity.createdAt) : false + } + +export const calculateLimitCutoffDate = ( + historyLimits: HistoryLimits | null, + limitType: HistoryLimitTypes +): Date | null => { + if (!historyLimits) return null + if (!historyLimits[limitType]) return null + return dayjs() + .subtract(historyLimits[limitType].value, historyLimits[limitType].unit) + .toDate() +} + +type GetProjectLimitDate = (args: { + project: Pick + limitType: HistoryLimitTypes +}) => Promise + +export const getProjectLimitDate = + ({ + getWorkspaceLimits, + getPersonalProjectLimits + }: { + getWorkspaceLimits: GetWorkspaceLimits + getPersonalProjectLimits: GetHistoryLimits + }): GetProjectLimitDate => + async ({ project, limitType }) => { + const limits = project.workspaceId + ? await getWorkspaceLimits({ workspaceId: project.workspaceId }) + : await getPersonalProjectLimits() + return calculateLimitCutoffDate(limits, limitType) + } diff --git a/packages/shared/src/tests/helpers/utils.ts b/packages/shared/src/tests/helpers/utils.ts new file mode 100644 index 000000000..856771ab6 --- /dev/null +++ b/packages/shared/src/tests/helpers/utils.ts @@ -0,0 +1,5 @@ +import cryptoRandomString from 'crypto-random-string' + +export function createRandomString(length?: number) { + return cryptoRandomString({ length: length ?? 10 }) +} diff --git a/packages/shared/src/workspaces/helpers/features.ts b/packages/shared/src/workspaces/helpers/features.ts index f61799b6a..995e4ae35 100644 --- a/packages/shared/src/workspaces/helpers/features.ts +++ b/packages/shared/src/workspaces/helpers/features.ts @@ -66,7 +66,8 @@ export type WorkspacePlanPriceStructure = { const unlimited: WorkspaceLimits = { projectCount: null, modelCount: null, - versionsHistory: null + versionsHistory: null, + commentHistory: null } export type WorkspacePlanConfig = { @@ -109,7 +110,8 @@ export const WorkspacePaidPlanConfigs: { limits: { projectCount: 5, modelCount: 25, - versionsHistory: { value: 30, unit: 'day' } + versionsHistory: { value: 30, unit: 'day' }, + commentHistory: { value: 30, unit: 'day' } } }, // New @@ -119,7 +121,8 @@ export const WorkspacePaidPlanConfigs: { limits: { projectCount: null, modelCount: null, - versionsHistory: { value: 30, unit: 'day' } + versionsHistory: { value: 30, unit: 'day' }, + commentHistory: { value: 30, unit: 'day' } } }, [PaidWorkspacePlans.Pro]: { @@ -133,7 +136,8 @@ export const WorkspacePaidPlanConfigs: { limits: { projectCount: 10, modelCount: 50, - versionsHistory: null + versionsHistory: null, + commentHistory: null } }, [PaidWorkspacePlans.ProUnlimited]: { @@ -147,7 +151,8 @@ export const WorkspacePaidPlanConfigs: { limits: { projectCount: null, modelCount: null, - versionsHistory: null + versionsHistory: null, + commentHistory: null } } } @@ -203,7 +208,8 @@ export const WorkspaceUnpaidPlanConfigs: { limits: { projectCount: 1, modelCount: 5, - versionsHistory: { value: 7, unit: 'day' } + versionsHistory: { value: 7, unit: 'day' }, + commentHistory: { value: 7, unit: 'day' } } } } diff --git a/packages/shared/src/workspaces/helpers/limits.ts b/packages/shared/src/workspaces/helpers/limits.ts index ea38e75f7..d2647af7d 100644 --- a/packages/shared/src/workspaces/helpers/limits.ts +++ b/packages/shared/src/workspaces/helpers/limits.ts @@ -1,5 +1,6 @@ +import { HistoryLimits } from '../../limit/domain.js' + export type WorkspaceLimits = { projectCount: number | null modelCount: number | null - versionsHistory: { value: number; unit: 'day' | 'week' | 'month' } | null -} +} & HistoryLimits diff --git a/yarn.lock b/yarn.lock index 26ff24cc5..7f96b9b2c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -16986,6 +16986,7 @@ __metadata: "@vitest/coverage-v8": "npm:^3.0.9" "@vitest/ui": "npm:^3.0.9" crypto-random-string: "npm:^5.0.0" + dayjs: "npm:^1.11.13" eslint: "npm:^9.4.0" eslint-config-prettier: "npm:^9.1.0" knex: "npm:^2.5.1" @@ -27572,6 +27573,13 @@ __metadata: languageName: node linkType: hard +"dayjs@npm:^1.11.13": + version: 1.11.13 + resolution: "dayjs@npm:1.11.13" + checksum: 10/7374d63ab179b8d909a95e74790def25c8986e329ae989840bacb8b1888be116d20e1c4eee75a69ea0dfbae13172efc50ef85619d304ee7ca3c01d5878b704f5 + languageName: node + linkType: hard + "dayjs@npm:^1.11.5": version: 1.11.5 resolution: "dayjs@npm:1.11.5" From a50e053096ccfbde3fe1ba2477cffa70f9c22601 Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Tue, 15 Apr 2025 11:37:06 +0100 Subject: [PATCH 08/36] chore(server/logging): add operation logging to automate module - tidy up some passing of loggers to automate - do not use console.log, instead use @/observability/logging --- .prettierignore | 2 +- .../automate/clients/executionEngine.ts | 35 ++- .../automate/graph/resolvers/automate.ts | 296 +++++++++++++----- .../automate/services/functionManagement.ts | 14 +- .../automate/services/runsManagement.ts | 14 +- packages/server/test/hooks.ts | 2 +- 6 files changed, 255 insertions(+), 108 deletions(-) diff --git a/.prettierignore b/.prettierignore index a7733da94..242072ef1 100644 --- a/.prettierignore +++ b/.prettierignore @@ -6,7 +6,7 @@ dist-* coverage .nyc_output packages/server/reports* -packages/preview-service/public/render/**/* +packages/preview-service/public/**/* packages/objectloader/examples/browser/objectloader.web.js packages/viewer/example/speckleviewer.web.js diff --git a/packages/server/modules/automate/clients/executionEngine.ts b/packages/server/modules/automate/clients/executionEngine.ts index 1248629ec..fc6890f3a 100644 --- a/packages/server/modules/automate/clients/executionEngine.ts +++ b/packages/server/modules/automate/clients/executionEngine.ts @@ -1,4 +1,3 @@ -import { automateLogger } from '@/observability/logging' import { ExecutionEngineBadResponseBodyError, type ExecutionEngineErrorResponse, @@ -29,8 +28,9 @@ import { timeoutAt } from '@speckle/shared' import { randomUUID } from 'crypto' -import { Logger } from 'pino' +import { automateLogger, type Logger } from '@/observability/logging' import { has, isObjectLike, isEmpty } from 'lodash' +import { getRequestLogger } from '@/observability/components/express/requestContext' export type AuthCodePayloadWithOrigin = AuthCodePayload & { origin: string } @@ -51,8 +51,10 @@ const getApiUrl = ( path?: string, options?: Partial<{ query: Record - }> + }> & { logger?: Logger } ) => { + const logger = options?.logger || getRequestLogger() || automateLogger + const automateUrl = speckleAutomateUrl() if (!automateUrl) throw new MisconfiguredEnvironmentError( @@ -70,7 +72,8 @@ const getApiUrl = ( const urlValue = typeof val === 'object' ? val.join(',') : val.toString() url.searchParams.append(key, urlValue) } catch { - console.log({ val }) + logger.warn({ automateUrl: val }, 'Failed to parse query param') + //ignore } }) } @@ -119,6 +122,7 @@ const invokeRequest = async (params: { retry?: boolean }) => { const { url, method = 'get', body, token, requestId } = params + const logger = getRequestLogger() || automateLogger const response = await retry( async () => @@ -138,7 +142,7 @@ const invokeRequest = async (params: { ]), params.retry !== false ? 3 : 1, (i, error) => { - automateLogger.warn( + logger.warn( { url, method, err: error }, 'Automate Execution Engine API call failed, retrying...' ) @@ -408,7 +412,8 @@ export const getFunctionFactory = : undefined const url = getApiUrl(`/api/v1/functions/${functionId}`, { - query + query, + logger }) return await invokeSafeJsonRequestFactory({ @@ -462,7 +467,10 @@ export const getFunctionReleaseFactory = const { logger } = deps const { functionId, functionReleaseId } = params const url = getApiUrl( - `/api/v1/functions/${functionId}/versions/${functionReleaseId}` + `/api/v1/functions/${functionId}/versions/${functionReleaseId}`, + { + logger + } ) const result = await invokeSafeJsonRequestFactory({ @@ -507,7 +515,8 @@ export const getFunctionsFactory = query: { requireRelease: true, ...params.filters - } + }, + logger }) const authToken = params.auth @@ -548,7 +557,8 @@ export const getPublicFunctionsFactory = query: { ...query, featuredFunctionsOnly: true - } + }, + logger }) return await invokeSafeJsonRequestFactory({ @@ -578,7 +588,7 @@ export const getUserFunctionsFactory = }) => { const { logger } = deps const { userId, query, body } = params - const url = getApiUrl(`/api/v2/users/${userId}/functions`, { query }) + const url = getApiUrl(`/api/v2/users/${userId}/functions`, { query, logger }) return await invokeSafeJsonRequestFactory({ logger @@ -609,7 +619,10 @@ export const getWorkspaceFunctionsFactory = }) => { const { logger } = deps const { workspaceId, query, body } = params - const url = getApiUrl(`/api/v2/workspaces/${workspaceId}/functions`, { query }) + const url = getApiUrl(`/api/v2/workspaces/${workspaceId}/functions`, { + query, + logger + }) return await invokeSafeJsonRequestFactory({ logger diff --git a/packages/server/modules/automate/graph/resolvers/automate.ts b/packages/server/modules/automate/graph/resolvers/automate.ts index ea9fb53b6..1983ec04a 100644 --- a/packages/server/modules/automate/graph/resolvers/automate.ts +++ b/packages/server/modules/automate/graph/resolvers/automate.ts @@ -73,10 +73,7 @@ import { manuallyTriggerAutomationFactory, triggerAutomationRevisionRunFactory } from '@/modules/automate/services/trigger' -import { - reportFunctionRunStatusFactory, - ReportFunctionRunStatusDeps -} from '@/modules/automate/services/runsManagement' +import { reportFunctionRunStatusFactory } from '@/modules/automate/services/runsManagement' import { AutomationNotFoundError, FunctionNotFoundError @@ -125,6 +122,7 @@ import { import { getEventBus } from '@/modules/shared/services/eventBus' import { getProjectDbClient } from '@/modules/multiregion/utils/dbSelector' import { BranchNotFoundError } from '@/modules/core/errors/branch' +import { withOperationLogging } from '@/observability/domain/businessLogging' const { FF_AUTOMATE_MODULE_ENABLED } = getFeatureFlags() @@ -533,54 +531,90 @@ export = (FF_AUTOMATE_MODULE_ENABLED }, AutomateMutations: { async createFunction(_parent, args, ctx) { + const logger = ctx.log const create = createFunctionFromTemplateFactory({ createExecutionEngineFn: createFunction, getUser: getUserFactory({ db }), createStoredAuthCode: createStoredAuthCodeFactory({ redis: getGenericRedis() - }) + }), + logger }) - return (await create({ input: args.input, userId: ctx.userId! })) - .graphqlReturn + const { graphqlReturn } = await withOperationLogging( + async () => await create({ input: args.input, userId: ctx.userId! }), + { + logger, + operationName: 'createFunction', + operationDescription: 'Create a new Automate function' + } + ) + return graphqlReturn }, async createFunctionWithoutVersion(_parent, args, ctx) { + const logger = ctx.log + const authCode = await createStoredAuthCodeFactory({ redis: getGenericRedis() })({ userId: ctx.userId!, action: AuthCodePayloadAction.CreateFunction }) - return await createFunctionWithoutVersion({ - body: { - speckleServerAuthenticationPayload: { - ...authCode, - origin: getServerOrigin() - }, - functionName: args.input.name, - description: args.input.description, - repositoryUrl: - 'https://github.com/specklesystems/speckle_automate_python_example', - supportedSourceApps: [], - tags: [] + return await withOperationLogging( + async () => + await createFunctionWithoutVersion({ + body: { + speckleServerAuthenticationPayload: { + ...authCode, + origin: getServerOrigin() + }, + functionName: args.input.name, + description: args.input.description, + repositoryUrl: + 'https://github.com/specklesystems/speckle_automate_python_example', + supportedSourceApps: [], + tags: [] + } + }), + { + logger, + operationName: 'createFunctionWithoutVersion', + operationDescription: 'Create a new Automate function without version' } - }) + ) }, async updateFunction(_parent, args, ctx) { + const functionId = args.input.id + const logger = ctx.log.child({ + functionId + }) const update = updateFunctionFactory({ updateFunction: execEngineUpdateFunction, - getFunction: getFunctionFactory({ logger: ctx.log }), + getFunction: getFunctionFactory({ logger }), createStoredAuthCode: createStoredAuthCodeFactory({ redis: getGenericRedis() - }), - logger: ctx.log + }) }) - return await update({ input: args.input, userId: ctx.userId! }) + return await withOperationLogging( + async () => await update({ input: args.input, userId: ctx.userId! }), + { + logger, + operationName: 'updateFunction', + operationDescription: 'Update an Automate function' + } + ) } }, ProjectAutomationMutations: { async create(parent, { input }, ctx) { - const projectDb = await getProjectDbClient({ projectId: parent.projectId }) + const projectId = parent.projectId + + const logger = ctx.log.child({ + projectId, + streamId: projectId //legacy + }) + + const projectDb = await getProjectDbClient({ projectId }) const create = createAutomationFactory({ createAuthCode: createStoredAuthCodeFactory({ redis: getGenericRedis() }), @@ -591,17 +625,34 @@ export = (FF_AUTOMATE_MODULE_ENABLED eventEmit: getEventBus().emit }) - return ( - await create({ - input, - userId: ctx.userId!, - projectId: parent.projectId, - userResourceAccessRules: ctx.resourceAccessRules - }) - ).automation + const { automation } = await withOperationLogging( + async () => + await create({ + input, + userId: ctx.userId!, + projectId, + userResourceAccessRules: ctx.resourceAccessRules + }), + { + logger, + operationName: 'createProjectAutomation', + operationDescription: 'Create a new Automation attached to a project' + } + ) + + return automation }, async update(parent, { input }, ctx) { - const projectDb = await getProjectDbClient({ projectId: parent.projectId }) + const projectId = parent.projectId + const automationId = input.id + + const logger = ctx.log.child({ + projectId, + streamId: projectId, //legacy + automationId + }) + + const projectDb = await getProjectDbClient({ projectId }) const update = validateAndUpdateAutomationFactory({ getAutomation: getAutomationFactory({ db: projectDb }), @@ -610,15 +661,32 @@ export = (FF_AUTOMATE_MODULE_ENABLED eventEmit: getEventBus().emit }) - return await update({ - input, - userId: ctx.userId!, - projectId: parent.projectId, - userResourceAccessRules: ctx.resourceAccessRules - }) + return await withOperationLogging( + async () => + await update({ + input, + userId: ctx.userId!, + projectId, + userResourceAccessRules: ctx.resourceAccessRules + }), + { + logger, + operationName: 'updateProjectAutomation', + operationDescription: 'Update an Automation attached to a project' + } + ) }, async createRevision(parent, { input }, ctx) { - const projectDb = await getProjectDbClient({ projectId: parent.projectId }) + const projectId = parent.projectId + const automationId = input.automationId + + const logger = ctx.log.child({ + projectId, + streamId: projectId, //legacy + automationId + }) + + const projectDb = await getProjectDbClient({ projectId }) const create = createAutomationRevisionFactory({ getAutomation: getAutomationFactory({ db: projectDb }), @@ -634,15 +702,31 @@ export = (FF_AUTOMATE_MODULE_ENABLED validateStreamAccess }) - return await create({ - input, - projectId: parent.projectId, - userId: ctx.userId!, - userResourceAccessRules: ctx.resourceAccessRules - }) + return await withOperationLogging( + async () => + await create({ + input, + projectId, + userId: ctx.userId!, + userResourceAccessRules: ctx.resourceAccessRules + }), + { + logger, + operationName: 'createAutomationRevision', + operationDescription: 'Create a new Automation revision' + } + ) }, async trigger(parent, { automationId }, ctx) { - const projectDb = await getProjectDbClient({ projectId: parent.projectId }) + const projectId = parent.projectId + + const logger = ctx.log.child({ + projectId, + streamId: projectId, //legacy + automationId + }) + + const projectDb = await getProjectDbClient({ projectId }) const trigger = manuallyTriggerAutomationFactory({ getAutomationTriggerDefinitions: getAutomationTriggerDefinitionsFactory({ @@ -668,17 +752,32 @@ export = (FF_AUTOMATE_MODULE_ENABLED validateStreamAccess }) - const { automationRunId } = await trigger({ - automationId, - userId: ctx.userId!, - userResourceAccessRules: ctx.resourceAccessRules, - projectId: parent.projectId - }) + const { automationRunId } = await withOperationLogging( + async () => + await trigger({ + automationId, + userId: ctx.userId!, + userResourceAccessRules: ctx.resourceAccessRules, + projectId + }), + { + logger, + operationName: 'triggerProjectAutomation', + operationDescription: 'Trigger an Automation' + } + ) return automationRunId }, async createTestAutomation(parent, { input }, ctx) { - const projectDb = await getProjectDbClient({ projectId: parent.projectId }) + const projectId = parent.projectId + + const logger = ctx.log.child({ + projectId, + streamId: projectId //legacy + }) + + const projectDb = await getProjectDbClient({ projectId }) const create = createTestAutomationFactory({ getEncryptionKeyPair, @@ -689,14 +788,30 @@ export = (FF_AUTOMATE_MODULE_ENABLED eventEmit: getEventBus().emit }) - return await create({ - input, - projectId: parent.projectId, - userId: ctx.userId!, - userResourceAccessRules: ctx.resourceAccessRules - }) + return await withOperationLogging( + async () => + await create({ + input, + projectId, + userId: ctx.userId!, + userResourceAccessRules: ctx.resourceAccessRules + }), + { + logger, + operationName: 'createTestAutomation', + operationDescription: 'Create a new test Automation' + } + ) }, async createTestAutomationRun(parent, { automationId }, ctx) { + const projectId = parent.projectId + + const logger = ctx.log.child({ + projectId, + streamId: projectId, //legacy + automationId + }) + const projectDb = await getProjectDbClient({ projectId: parent.projectId }) const create = createTestAutomationRunFactory({ @@ -725,11 +840,19 @@ export = (FF_AUTOMATE_MODULE_ENABLED validateStreamAccess }) - return await create({ - projectId: parent.projectId, - automationId, - userId: ctx.userId! - }) + return await withOperationLogging( + async () => + await create({ + projectId: parent.projectId, + automationId, + userId: ctx.userId! + }), + { + logger, + operationName: 'createTestAutomationRun', + operationDescription: 'Create a new test Automation run' + } + ) } }, Query: { @@ -928,9 +1051,17 @@ export = (FF_AUTOMATE_MODULE_ENABLED } }, Mutation: { - async automateFunctionRunStatusReport(_parent, { input }) { + async automateFunctionRunStatusReport(_parent, { input }, ctx) { + const projectId = input.projectId + const functionRunId = input.functionRunId + const logger = ctx.log.child({ + projectId, + streamId: projectId, //legacy + functionRunId + }) + const projectDb = await getProjectDbClient({ projectId: input.projectId }) - const deps: ReportFunctionRunStatusDeps = { + const reportFunctionRunStatus = reportFunctionRunStatusFactory({ getAutomationFunctionRunRecord: getFunctionRunFactory({ db: projectDb }), @@ -941,18 +1072,25 @@ export = (FF_AUTOMATE_MODULE_ENABLED db: projectDb }), emitEvent: getEventBus().emit - } + }) - const payload = { - ...input, - contextView: input.contextView ?? null, - results: (input.results as Automate.AutomateTypes.ResultsSchema) ?? null, - runId: input.functionRunId, - status: mapGqlStatusToDbStatus(input.status), - statusMessage: input.statusMessage ?? null - } - - const result = await reportFunctionRunStatusFactory(deps)(payload) + const result = await withOperationLogging( + async () => + await reportFunctionRunStatus({ + ...input, + contextView: input.contextView ?? null, + results: + (input.results as Automate.AutomateTypes.ResultsSchema) ?? null, + runId: input.functionRunId, + status: mapGqlStatusToDbStatus(input.status), + statusMessage: input.statusMessage ?? null + }), + { + logger, + operationName: 'automateFunctionRunStatusReport', + operationDescription: 'Report the status of a function run' + } + ) return result }, diff --git a/packages/server/modules/automate/services/functionManagement.ts b/packages/server/modules/automate/services/functionManagement.ts index 907c4255a..40ba3e02d 100644 --- a/packages/server/modules/automate/services/functionManagement.ts +++ b/packages/server/modules/automate/services/functionManagement.ts @@ -43,7 +43,7 @@ import { speckleAutomateUrl } from '@/modules/shared/helpers/envHelper' import { getFunctionsMarketplaceUrl } from '@/modules/core/helpers/routeHelper' -import { automateLogger, Logger } from '@/observability/logging' +import type { Logger } from '@/observability/logging' import { CreateStoredAuthCode } from '@/modules/automate/domain/operations' import { GetUser } from '@/modules/core/domain/users/operations' import { noop } from 'lodash' @@ -132,13 +132,14 @@ export type CreateFunctionDeps = { createStoredAuthCode: CreateStoredAuthCode createExecutionEngineFn: typeof createFunction getUser: GetUser + logger: Logger } export const createFunctionFromTemplateFactory = (deps: CreateFunctionDeps) => async (params: { input: CreateAutomateFunctionInput; userId: string }) => { const { input, userId } = params - const { createExecutionEngineFn, getUser, createStoredAuthCode } = deps + const { createExecutionEngineFn, getUser, createStoredAuthCode, logger } = deps // Validate user const user = await getUser(userId) @@ -163,7 +164,7 @@ export const createFunctionFromTemplateFactory = const created = await createExecutionEngineFn({ body }) if (isDevEnv() && created) { - automateLogger.info({ created }, `[dev] Created function #${created.functionId}`) + logger.info({ created }, `[dev] Created function #${created.functionId}`) } // Don't want to pull the function w/ another req, so we'll just return the input @@ -197,16 +198,15 @@ export type UpdateFunctionDeps = { updateFunction: typeof updateExecEngineFunction getFunction: ReturnType createStoredAuthCode: CreateStoredAuthCode - logger: Logger } export const updateFunctionFactory = (deps: UpdateFunctionDeps) => async (params: { input: UpdateAutomateFunctionInput; userId: string }) => { - const { updateFunction, createStoredAuthCode, logger } = deps + const { getFunction, updateFunction, createStoredAuthCode } = deps const { input, userId } = params - const existingFn = await getFunctionFactory({ logger })({ functionId: input.id }) + const existingFn = await getFunction({ functionId: input.id }) if (!existingFn) { throw new AutomateFunctionUpdateError('Function not found') } @@ -239,8 +239,6 @@ export const updateFunctionFactory = } }) - console.log(JSON.stringify(apiResult, null, 2)) - return convertFunctionToGraphQLReturn(apiResult) } diff --git a/packages/server/modules/automate/services/runsManagement.ts b/packages/server/modules/automate/services/runsManagement.ts index 9dc6e0902..65daf91ba 100644 --- a/packages/server/modules/automate/services/runsManagement.ts +++ b/packages/server/modules/automate/services/runsManagement.ts @@ -85,15 +85,13 @@ export const resolveStatusFromFunctionRunStatuses = ( return AutomationRunStatuses.succeeded } -export type ReportFunctionRunStatusDeps = { - getAutomationFunctionRunRecord: GetFunctionRun - upsertAutomationFunctionRunRecord: UpsertAutomationFunctionRun - automationRunUpdater: UpdateAutomationRun - emitEvent: EventBusEmit -} - export const reportFunctionRunStatusFactory = - (deps: ReportFunctionRunStatusDeps) => + (deps: { + getAutomationFunctionRunRecord: GetFunctionRun + upsertAutomationFunctionRunRecord: UpsertAutomationFunctionRun + automationRunUpdater: UpdateAutomationRun + emitEvent: EventBusEmit + }) => async ( params: Pick< AutomationFunctionRunRecord, diff --git a/packages/server/test/hooks.ts b/packages/server/test/hooks.ts index 3bb73a083..03697c1ba 100644 --- a/packages/server/test/hooks.ts +++ b/packages/server/test/hooks.ts @@ -354,7 +354,7 @@ let graphqlServer: Optional> = undefined export const mochaHooks: mocha.RootHookObject = { beforeAll: async () => { if (isMultiRegionTestMode()) { - console.log('Running tests in multi-region mode...') + logger.info('Running tests in multi-region mode...') } logger.info('running before all') From 593cfa9c8064b1b5adceeadc29dd6d85565a2529 Mon Sep 17 00:00:00 2001 From: Benjamin Ottensten Date: Tue, 15 Apr 2025 13:01:21 +0200 Subject: [PATCH 09/36] Feat: Update legacy projects announcement (#4428) * Update legacy projects announcement * Remove ref import --------- Co-authored-by: Mike Tasset --- .../projects/MoveToWorkspaceAlert.vue | 82 +++++++++++++------ .../frontend-2/lib/common/helpers/route.ts | 2 +- 2 files changed, 60 insertions(+), 24 deletions(-) diff --git a/packages/frontend-2/components/projects/MoveToWorkspaceAlert.vue b/packages/frontend-2/components/projects/MoveToWorkspaceAlert.vue index a7627d27a..195e5a30b 100644 --- a/packages/frontend-2/components/projects/MoveToWorkspaceAlert.vue +++ b/packages/frontend-2/components/projects/MoveToWorkspaceAlert.vue @@ -1,42 +1,75 @@ @@ -76,11 +85,13 @@ import { useDebouncedTextInput } from '@speckle/ui-components' import type { + PermissionCheckResult, WorkspaceMoveProjectManager_ProjectFragment, - WorkspacePermissionChecks + WorkspaceMoveProjectManager_WorkspaceFragment } from '~~/lib/common/generated/gql/graphql' import { usePaginatedQuery } from '~/lib/common/composables/graphql' import { workspaceMoveProjectManagerUserQuery } from '~/lib/workspaces/graphql/queries' +import { formatName } from '~/lib/billing/helpers/plan' const search = defineModel('search') const { on, bind } = useDebouncedTextInput({ model: search }) @@ -91,7 +102,7 @@ const emit = defineEmits<{ const props = defineProps<{ workspaceSlug?: string - workspacePermissions?: WorkspacePermissionChecks + projectPermissions?: PermissionCheckResult }>() const { @@ -117,6 +128,9 @@ const { }) const showLimitDialog = ref(false) +const limitReachedWorkspace = ref( + null +) const userProjects = computed(() => result.value?.activeUser?.projects.items || []) const moveableProjects = computed(() => userProjects.value) @@ -142,6 +156,9 @@ const getProjectTooltip = computed( const onMoveClick = (project: WorkspaceMoveProjectManager_ProjectFragment) => { if (props.workspaceSlug) { + limitReachedWorkspace.value = { + name: props.workspaceSlug + } as WorkspaceMoveProjectManager_WorkspaceFragment showLimitDialog.value = true return } diff --git a/packages/frontend-2/components/workspace/moveProject/SelectWorkspace.vue b/packages/frontend-2/components/workspace/moveProject/SelectWorkspace.vue index deb45dd18..f7a25f2fc 100644 --- a/packages/frontend-2/components/workspace/moveProject/SelectWorkspace.vue +++ b/packages/frontend-2/components/workspace/moveProject/SelectWorkspace.vue @@ -80,9 +80,9 @@ diff --git a/packages/frontend-2/components/settings/workspaces/members/actions/Menu.vue b/packages/frontend-2/components/settings/workspaces/members/actions/Menu.vue index ea8a3d31a..e3ac46a2b 100644 --- a/packages/frontend-2/components/settings/workspaces/members/actions/Menu.vue +++ b/packages/frontend-2/components/settings/workspaces/members/actions/Menu.vue @@ -55,11 +55,10 @@ @success="onDialogSuccess" /> - Date: Tue, 15 Apr 2025 16:58:00 +0300 Subject: [PATCH 15/36] fix(shared): not using #lodash alias (#4440) --- packages/shared/src/core/helpers/timeConstants.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared/src/core/helpers/timeConstants.ts b/packages/shared/src/core/helpers/timeConstants.ts index ac109941e..a394e1658 100644 --- a/packages/shared/src/core/helpers/timeConstants.ts +++ b/packages/shared/src/core/helpers/timeConstants.ts @@ -1,4 +1,4 @@ -import { mapValues } from 'lodash' +import { mapValues } from '#lodash' /* * Time with seconds as the base unit From bee73b6f21c350ee1cedd93570ab73b0e85d350b Mon Sep 17 00:00:00 2001 From: andrewwallacespeckle Date: Tue, 15 Apr 2025 14:58:45 +0100 Subject: [PATCH 16/36] Add line lamp to workspace card description --- .../components/workspace/discoverableWorkspaces/Card.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/frontend-2/components/workspace/discoverableWorkspaces/Card.vue b/packages/frontend-2/components/workspace/discoverableWorkspaces/Card.vue index d743f3a79..8ea9ff3ff 100644 --- a/packages/frontend-2/components/workspace/discoverableWorkspaces/Card.vue +++ b/packages/frontend-2/components/workspace/discoverableWorkspaces/Card.vue @@ -2,7 +2,7 @@