From b6f269b8ea2dee3eebada09b67a77ac3c4f863ec Mon Sep 17 00:00:00 2001 From: Alessandro Magionami Date: Fri, 28 Feb 2025 11:14:28 +0100 Subject: [PATCH] feat(workspaces): code review changes --- .../modules/gatekeeper/domain/operations.ts | 2 +- .../gatekeeper/repositories/workspaceSeat.ts | 14 ++++-- .../modules/workspaces/domain/operations.ts | 2 +- .../workspaces/errors/workspaceSeat.ts | 4 +- .../workspaces/services/workspaceSeat.ts | 4 +- .../tests/integration/workspaceSeat.spec.ts | 48 ++++++++++++++++++- 6 files changed, 61 insertions(+), 13 deletions(-) diff --git a/packages/server/modules/gatekeeper/domain/operations.ts b/packages/server/modules/gatekeeper/domain/operations.ts index 981056d6b..c6ee84fcd 100644 --- a/packages/server/modules/gatekeeper/domain/operations.ts +++ b/packages/server/modules/gatekeeper/domain/operations.ts @@ -34,4 +34,4 @@ export type GetWorkspacePlanByProjectId = ({ export type CreateWorkspaceSeat = ( args: Pick -) => Promise +) => Promise diff --git a/packages/server/modules/gatekeeper/repositories/workspaceSeat.ts b/packages/server/modules/gatekeeper/repositories/workspaceSeat.ts index a60c160d4..d22b2233f 100644 --- a/packages/server/modules/gatekeeper/repositories/workspaceSeat.ts +++ b/packages/server/modules/gatekeeper/repositories/workspaceSeat.ts @@ -22,9 +22,13 @@ export const countSeatsByTypeInWorkspaceFactory = export const createWorkspaceSeatFactory = ({ db }: { db: Knex }): CreateWorkspaceSeat => async ({ userId, workspaceId, type }) => { - return await tables.workspaceSeats(db).insert({ - workspaceId, - userId, - type - }) + await tables + .workspaceSeats(db) + .insert({ + workspaceId, + userId, + type + }) + .onConflict(['workspaceId', 'userId']) + .merge() } diff --git a/packages/server/modules/workspaces/domain/operations.ts b/packages/server/modules/workspaces/domain/operations.ts index 83680afe8..1835511fe 100644 --- a/packages/server/modules/workspaces/domain/operations.ts +++ b/packages/server/modules/workspaces/domain/operations.ts @@ -389,7 +389,7 @@ export type CopyProjectAutomations = (params: { export type AssignWorkspaceSeat = ( params: Pick & { type?: WorkspaceSeatType } -) => Promise +) => Promise export type CopyProjectComments = (params: { projectIds: string[] }) => Promise> diff --git a/packages/server/modules/workspaces/errors/workspaceSeat.ts b/packages/server/modules/workspaces/errors/workspaceSeat.ts index 3c4ecc11a..10aeeff88 100644 --- a/packages/server/modules/workspaces/errors/workspaceSeat.ts +++ b/packages/server/modules/workspaces/errors/workspaceSeat.ts @@ -1,7 +1,7 @@ import { BaseError } from '@/modules/shared/errors' -export class InvalidWorkspaceSeatType extends BaseError { +export class InvalidWorkspaceSeatTypeError extends BaseError { static defaultMessage = 'Workspace seat type is invalid' - static code = 'INDALID_WORKSPACE_SEAT_TYPE' + static code = 'INDALID_WORKSPACE_SEAT_TYPE_ERROR' static statusCode = 400 } diff --git a/packages/server/modules/workspaces/services/workspaceSeat.ts b/packages/server/modules/workspaces/services/workspaceSeat.ts index 82be5226e..6d1605912 100644 --- a/packages/server/modules/workspaces/services/workspaceSeat.ts +++ b/packages/server/modules/workspaces/services/workspaceSeat.ts @@ -5,7 +5,7 @@ import { AssignWorkspaceSeat, GetWorkspaceRoleForUser } from '@/modules/workspaces/domain/operations' -import { InvalidWorkspaceSeatType } from '@/modules/workspaces/errors/workspaceSeat' +import { InvalidWorkspaceSeatTypeError } from '@/modules/workspaces/errors/workspaceSeat' import { Roles, WorkspaceRoles } from '@speckle/shared' import { z } from 'zod' @@ -82,7 +82,7 @@ export const assignWorkspaceSeatFactory = workspaceSeatType: type }) ) { - throw new InvalidWorkspaceSeatType( + throw new InvalidWorkspaceSeatTypeError( `User with workspace role ${workspaceAcl.role} cannot have a seat of type ${type}`, { info: { diff --git a/packages/server/modules/workspaces/tests/integration/workspaceSeat.spec.ts b/packages/server/modules/workspaces/tests/integration/workspaceSeat.spec.ts index 86bd30757..02afd5d51 100644 --- a/packages/server/modules/workspaces/tests/integration/workspaceSeat.spec.ts +++ b/packages/server/modules/workspaces/tests/integration/workspaceSeat.spec.ts @@ -5,7 +5,7 @@ import { } from '@/modules/core/helpers/testHelpers' import { createWorkspaceSeatFactory } from '@/modules/gatekeeper/repositories/workspaceSeat' import { NotFoundError } from '@/modules/shared/errors' -import { InvalidWorkspaceSeatType } from '@/modules/workspaces/errors/workspaceSeat' +import { InvalidWorkspaceSeatTypeError } from '@/modules/workspaces/errors/workspaceSeat' import { getWorkspaceRoleForUserFactory } from '@/modules/workspaces/repositories/workspaces' import { assignWorkspaceSeatFactory } from '@/modules/workspaces/services/workspaceSeat' import { @@ -177,7 +177,51 @@ describe('Workspace workspaceSeat services', () => { })({ userId: user.id, workspaceId: workspace.id, type: 'viewer' }) ) - expect(err.name).to.eq(InvalidWorkspaceSeatType.name) + expect(err.name).to.eq(InvalidWorkspaceSeatTypeError.name) + }) + it('should update seat type on role change', async () => { + const workspaceAdmin: BasicTestUser = { + id: createRandomString(), + name: createRandomString(), + email: createRandomEmail(), + role: Roles.Server.Admin, + verified: true + } + await createTestUser(workspaceAdmin) + + const workspace: BasicTestWorkspace = { + id: createRandomString(), + slug: createRandomString(), + ownerId: workspaceAdmin.id, + name: cryptoRandomString({ length: 6 }), + description: cryptoRandomString({ length: 12 }) + } + await createTestWorkspace(workspace, workspaceAdmin) + + const user: BasicTestUser = { + id: createRandomString(), + name: createRandomString(), + email: createRandomEmail(), + role: Roles.Server.User, + verified: true + } + await createTestUser(user) + + await assignToWorkspace(workspace, user, Roles.Workspace.Member) + const workspaceSeat = await db('workspace_seats') + .where({ userId: user.id, workspaceId: workspace.id }) + .first() + + expect(workspaceSeat.type).to.eq('viewer') + + // Change workspace role + await assignToWorkspace(workspace, user, Roles.Workspace.Admin) + + const workspaceSeatUpdated = await db('workspace_seats') + .where({ userId: user.id, workspaceId: workspace.id }) + .first() + + expect(workspaceSeatUpdated.type).to.eq('editor') }) }) })