From b5c9e62bcba355850d41f9ca16b604b5c6a5ae0e Mon Sep 17 00:00:00 2001 From: Chuck Driesler Date: Fri, 13 Sep 2024 09:30:06 +0100 Subject: [PATCH] chore(workspaces): perform workspace project role update via events (#2980) * chore(workspaces): perform workspace project role update via events * chore(workspaces): commented * fix(workspaces): transactions in events * fix(workspaces): transaction limits --- .../workspaces/events/eventListener.ts | 139 ++++++++++++++---- .../workspaces/graph/resolvers/workspaces.ts | 25 +--- packages/server/modules/workspaces/index.ts | 51 +------ .../modules/workspaces/services/management.ts | 88 +++-------- .../workspaces/tests/helpers/creation.ts | 15 +- .../integration/workspaces.graph.spec.ts | 8 +- .../tests/unit/events/eventListener.spec.ts | 21 ++- .../tests/unit/services/management.spec.ts | 98 +++++++----- .../modules/workspacesCore/domain/events.ts | 5 +- 9 files changed, 219 insertions(+), 231 deletions(-) diff --git a/packages/server/modules/workspaces/events/eventListener.ts b/packages/server/modules/workspaces/events/eventListener.ts index 50707fe69..3aacb0256 100644 --- a/packages/server/modules/workspaces/events/eventListener.ts +++ b/packages/server/modules/workspaces/events/eventListener.ts @@ -3,7 +3,11 @@ import { ProjectEvents, ProjectEventsPayloads } from '@/modules/core/events/projectsEmitter' -import { getStream } from '@/modules/core/repositories/streams' +import { + deleteProjectRoleFactory, + getStream, + upsertProjectRoleFactory +} from '@/modules/core/repositories/streams' import { GetWorkspaceRoles, GetWorkspaceRoleToDefaultProjectRoleMapping, @@ -17,13 +21,27 @@ import { isProjectResourceTarget, resolveTarget } from '@/modules/serverinvites/helpers/core' -import { logger } from '@/logging/logging' +import { logger, moduleLogger } from '@/logging/logging' import { updateWorkspaceRoleFactory } from '@/modules/workspaces/services/management' import { getEventBus } from '@/modules/shared/services/eventBus' import { WorkspaceInviteResourceType } from '@/modules/workspaces/domain/constants' import { Roles, WorkspaceRoles } from '@speckle/shared' -import { UpsertProjectRole } from '@/modules/core/domain/projects/operations' +import { + DeleteProjectRole, + UpsertProjectRole +} from '@/modules/core/domain/projects/operations' import { WorkspaceEvents } from '@/modules/workspacesCore/domain/events' +import { Knex } from 'knex' +import { mapWorkspaceRoleToInitialProjectRole } from '@/modules/workspaces/domain/logic' +import { + getWorkspaceRolesFactory, + getWorkspaceWithDomainsFactory, + upsertWorkspaceRoleFactory +} from '@/modules/workspaces/repositories/workspaces' +import { queryAllWorkspaceProjectsFactory } from '@/modules/workspaces/services/projects' +import { getStreams } from '@/modules/core/services/streams' +import { withTransaction } from '@/modules/shared/helpers/dbHelper' +import { findVerifiedEmailsByUserIdFactory } from '@/modules/core/repositories/userEmails' export const onProjectCreatedFactory = ({ @@ -89,7 +107,7 @@ export const onInviteFinalizedFactory = }) if (!project || !project.role) { deps.logger.warn( - `When handling accepted invite - project not found or useris not a collaborator`, + `When handling accepted invite - project not found or user is not a collaborator`, { invite, project: { id: project?.id, role: project?.role } } ) return @@ -109,39 +127,77 @@ export const onInviteFinalizedFactory = }) } -export const onWorkspaceJoinedFactory = +export const onWorkspaceRoleDeletedFactory = + ({ + queryAllWorkspaceProjects, + deleteProjectRole + }: { + queryAllWorkspaceProjects: QueryAllWorkspaceProjects + deleteProjectRole: DeleteProjectRole + }) => + async ({ userId, workspaceId }: { userId: string; workspaceId: string }) => { + // Delete roles for all workspace projects + for await (const projectsPage of queryAllWorkspaceProjects({ + workspaceId + })) { + await Promise.all( + projectsPage.map(({ id: projectId }) => + deleteProjectRole({ projectId, userId }) + ) + ) + } + } + +export const onWorkspaceRoleUpdatedFactory = ({ getDefaultWorkspaceProjectRoleMapping, queryAllWorkspaceProjects, + deleteProjectRole, upsertProjectRole }: { getDefaultWorkspaceProjectRoleMapping: GetWorkspaceRoleToDefaultProjectRoleMapping queryAllWorkspaceProjects: QueryAllWorkspaceProjects + deleteProjectRole: DeleteProjectRole upsertProjectRole: UpsertProjectRole }) => async ({ userId, role, - workspaceId + workspaceId, + flags }: { userId: string role: WorkspaceRoles workspaceId: string + flags?: { + skipProjectRoleUpdatesFor: string[] + } }) => { - const defaultRoleMapping = await getDefaultWorkspaceProjectRoleMapping({ + const defaultProjectRoleMapping = await getDefaultWorkspaceProjectRoleMapping({ workspaceId }) - const maybeProjectRole = defaultRoleMapping[role] - if (!maybeProjectRole) return + const nextProjectRole = defaultProjectRoleMapping[role] - for await (const projects of queryAllWorkspaceProjects({ workspaceId })) { + for await (const projectsPage of queryAllWorkspaceProjects({ workspaceId })) { await Promise.all( - projects.map(async (project) => { + projectsPage.map(async ({ id: projectId }) => { + if (flags?.skipProjectRoleUpdatesFor.includes(projectId)) { + // Skip assignment (used during invite flow) + // TODO: Can we refactor this special case away? + return + } + + if (!nextProjectRole) { + // User is being demoted to a workspace role without project access + await deleteProjectRole({ projectId, userId }) + return + } + await upsertProjectRole({ - projectId: project.id, + projectId, userId, - role: maybeProjectRole + role: nextProjectRole }) }) ) @@ -149,25 +205,50 @@ export const onWorkspaceJoinedFactory = } export const initializeEventListenersFactory = - ({ - onProjectCreated, - onInviteFinalized, - onWorkspaceJoined - }: { - onProjectCreated: ReturnType - onInviteFinalized: ReturnType - onWorkspaceJoined: ReturnType - }) => + ({ db }: { db: Knex }) => () => { const eventBus = getEventBus() const quitCbs = [ - ProjectsEmitter.listen(ProjectEvents.Created, onProjectCreated), - eventBus.listen(ServerInvitesEvents.Finalized, ({ payload }) => - onInviteFinalized(payload) - ), - eventBus.listen(WorkspaceEvents.JoinedFromDiscovery, ({ payload }) => - onWorkspaceJoined(payload) - ) + ProjectsEmitter.listen(ProjectEvents.Created, async (payload) => { + const onProjectCreated = onProjectCreatedFactory({ + getDefaultWorkspaceProjectRoleMapping: mapWorkspaceRoleToInitialProjectRole, + upsertProjectRole: upsertProjectRoleFactory({ db }), + getWorkspaceRoles: getWorkspaceRolesFactory({ db }) + }) + await onProjectCreated(payload) + }), + eventBus.listen(ServerInvitesEvents.Finalized, async ({ payload }) => { + const onInviteFinalized = onInviteFinalizedFactory({ + getStream, + logger: moduleLogger, + updateWorkspaceRole: updateWorkspaceRoleFactory({ + getWorkspaceWithDomains: getWorkspaceWithDomainsFactory({ db }), + findVerifiedEmailsByUserId: findVerifiedEmailsByUserIdFactory({ db }), + getWorkspaceRoles: getWorkspaceRolesFactory({ db }), + upsertWorkspaceRole: upsertWorkspaceRoleFactory({ db }), + emitWorkspaceEvent: (...args) => getEventBus().emit(...args) + }) + }) + await onInviteFinalized(payload) + }), + eventBus.listen(WorkspaceEvents.RoleDeleted, async ({ payload }) => { + const trx = await db.transaction() + const onWorkspaceRoleDeleted = onWorkspaceRoleDeletedFactory({ + queryAllWorkspaceProjects: queryAllWorkspaceProjectsFactory({ getStreams }), + deleteProjectRole: deleteProjectRoleFactory({ db: trx }) + }) + await withTransaction(onWorkspaceRoleDeleted(payload), trx) + }), + eventBus.listen(WorkspaceEvents.RoleUpdated, async ({ payload }) => { + const trx = await db.transaction() + const onWorkspaceRoleUpdated = onWorkspaceRoleUpdatedFactory({ + getDefaultWorkspaceProjectRoleMapping: mapWorkspaceRoleToInitialProjectRole, + queryAllWorkspaceProjects: queryAllWorkspaceProjectsFactory({ getStreams }), + deleteProjectRole: deleteProjectRoleFactory({ db: trx }), + upsertProjectRole: upsertProjectRoleFactory({ db: trx }) + }) + await withTransaction(onWorkspaceRoleUpdated(payload), trx) + }) ] return () => quitCbs.forEach((quit) => quit()) diff --git a/packages/server/modules/workspaces/graph/resolvers/workspaces.ts b/packages/server/modules/workspaces/graph/resolvers/workspaces.ts index c4741c20e..03a515f00 100644 --- a/packages/server/modules/workspaces/graph/resolvers/workspaces.ts +++ b/packages/server/modules/workspaces/graph/resolvers/workspaces.ts @@ -4,9 +4,7 @@ import { removePrivateFields } from '@/modules/core/helpers/userHelper' import { getStream, getUserStreams, - getUserStreamsCount, - upsertProjectRoleFactory, - deleteProjectRoleFactory + getUserStreamsCount } from '@/modules/core/repositories/streams' import { getUser, getUsers } from '@/modules/core/repositories/users' import { getStreams } from '@/modules/core/services/streams' @@ -122,7 +120,6 @@ import { isUserWorkspaceDomainPolicyCompliantFactory } from '@/modules/workspaces/services/domains' import { getServerInfo } from '@/modules/core/services/generic' -import { mapWorkspaceRoleToInitialProjectRole } from '@/modules/workspaces/domain/logic' import { updateStreamRoleAndNotify } from '@/modules/core/services/streams/management' import { deleteOldAndInsertNewVerificationFactory } from '@/modules/emails/repositories' import { renderEmail } from '@/modules/emails/services/emailRendering' @@ -354,10 +351,6 @@ export = FF_WORKSPACES_MODULE_ENABLED const deleteWorkspaceRole = deleteWorkspaceRoleFactory({ deleteWorkspaceRole: repoDeleteWorkspaceRoleFactory({ db: trx }), getWorkspaceRoles: getWorkspaceRolesFactory({ db: trx }), - deleteProjectRole: deleteProjectRoleFactory({ db: trx }), - queryAllWorkspaceProjects: queryAllWorkspaceProjectsFactory({ - getStreams - }), emitWorkspaceEvent: getEventBus().emit }) @@ -376,13 +369,6 @@ export = FF_WORKSPACES_MODULE_ENABLED db: trx }), getWorkspaceRoles: getWorkspaceRolesFactory({ db: trx }), - getDefaultWorkspaceProjectRoleMapping: - mapWorkspaceRoleToInitialProjectRole, - upsertProjectRole: upsertProjectRoleFactory({ db: trx }), - deleteProjectRole: deleteProjectRoleFactory({ db: trx }), - queryAllWorkspaceProjects: queryAllWorkspaceProjectsFactory({ - getStreams - }), emitWorkspaceEvent: getEventBus().emit }) @@ -465,8 +451,6 @@ export = FF_WORKSPACES_MODULE_ENABLED const deleteWorkspaceRole = deleteWorkspaceRoleFactory({ deleteWorkspaceRole: repoDeleteWorkspaceRoleFactory({ db: trx }), getWorkspaceRoles: getWorkspaceRolesFactory({ db: trx }), - deleteProjectRole: deleteProjectRoleFactory({ db: trx }), - queryAllWorkspaceProjects: queryAllWorkspaceProjectsFactory({ getStreams }), emitWorkspaceEvent: getEventBus().emit }) @@ -580,13 +564,6 @@ export = FF_WORKSPACES_MODULE_ENABLED findVerifiedEmailsByUserId: findVerifiedEmailsByUserIdFactory({ db }), getWorkspaceRoles: getWorkspaceRolesFactory({ db }), upsertWorkspaceRole: upsertWorkspaceRoleFactory({ db }), - upsertProjectRole: upsertProjectRoleFactory({ db }), - getDefaultWorkspaceProjectRoleMapping: - mapWorkspaceRoleToInitialProjectRole, - deleteProjectRole: deleteProjectRoleFactory({ db }), - queryAllWorkspaceProjects: queryAllWorkspaceProjectsFactory({ - getStreams - }), emitWorkspaceEvent: getEventBus().emit }) }), diff --git a/packages/server/modules/workspaces/index.ts b/packages/server/modules/workspaces/index.ts index 43fb1e1ae..8a361f187 100644 --- a/packages/server/modules/workspaces/index.ts +++ b/packages/server/modules/workspaces/index.ts @@ -6,29 +6,8 @@ import { Optional, SpeckleModule } from '@/modules/shared/helpers/typeHelper' import { workspaceRoles } from '@/modules/workspaces/roles' import { workspaceScopes } from '@/modules/workspaces/scopes' import { registerOrUpdateRole } from '@/modules/shared/repositories/roles' -import { - initializeEventListenersFactory, - onInviteFinalizedFactory, - onProjectCreatedFactory, - onWorkspaceJoinedFactory -} from '@/modules/workspaces/events/eventListener' -import { - getWorkspaceRolesFactory, - getWorkspaceWithDomainsFactory, - upsertWorkspaceRoleFactory -} from '@/modules/workspaces/repositories/workspaces' -import { - deleteProjectRoleFactory, - getStream, - upsertProjectRoleFactory -} from '@/modules/core/repositories/streams' -import { updateWorkspaceRoleFactory } from '@/modules/workspaces/services/management' -import { getEventBus } from '@/modules/shared/services/eventBus' -import { getStreams } from '@/modules/core/services/streams' -import { findVerifiedEmailsByUserIdFactory } from '@/modules/core/repositories/userEmails' +import { initializeEventListenersFactory } from '@/modules/workspaces/events/eventListener' import { validateModuleLicense } from '@/modules/gatekeeper/services/validateLicense' -import { queryAllWorkspaceProjectsFactory } from '@/modules/workspaces/services/projects' -import { mapWorkspaceRoleToInitialProjectRole } from '@/modules/workspaces/domain/logic' const { FF_WORKSPACES_MODULE_ENABLED } = getFeatureFlags() @@ -58,33 +37,7 @@ const workspacesModule: SpeckleModule = { moduleLogger.info('⚒️ Init workspaces module') if (isInitial) { - quitListeners = initializeEventListenersFactory({ - onProjectCreated: onProjectCreatedFactory({ - getDefaultWorkspaceProjectRoleMapping: mapWorkspaceRoleToInitialProjectRole, - upsertProjectRole: upsertProjectRoleFactory({ db }), - getWorkspaceRoles: getWorkspaceRolesFactory({ db }) - }), - onWorkspaceJoined: onWorkspaceJoinedFactory({ - getDefaultWorkspaceProjectRoleMapping: mapWorkspaceRoleToInitialProjectRole, - queryAllWorkspaceProjects: queryAllWorkspaceProjectsFactory({ getStreams }), - upsertProjectRole: upsertProjectRoleFactory({ db }) - }), - onInviteFinalized: onInviteFinalizedFactory({ - getStream, - logger: moduleLogger, - updateWorkspaceRole: updateWorkspaceRoleFactory({ - getWorkspaceWithDomains: getWorkspaceWithDomainsFactory({ db }), - findVerifiedEmailsByUserId: findVerifiedEmailsByUserIdFactory({ db }), - getWorkspaceRoles: getWorkspaceRolesFactory({ db }), - upsertWorkspaceRole: upsertWorkspaceRoleFactory({ db }), - getDefaultWorkspaceProjectRoleMapping: mapWorkspaceRoleToInitialProjectRole, - upsertProjectRole: upsertProjectRoleFactory({ db }), - deleteProjectRole: deleteProjectRoleFactory({ db }), - queryAllWorkspaceProjects: queryAllWorkspaceProjectsFactory({ getStreams }), - emitWorkspaceEvent: (...args) => getEventBus().emit(...args) - }) - }) - })() + quitListeners = initializeEventListenersFactory({ db })() } await Promise.all([initScopes(), initRoles()]) }, diff --git a/packages/server/modules/workspaces/services/management.ts b/packages/server/modules/workspaces/services/management.ts index eea98acb1..289c46902 100644 --- a/packages/server/modules/workspaces/services/management.ts +++ b/packages/server/modules/workspaces/services/management.ts @@ -9,7 +9,6 @@ import { UpsertWorkspaceRole, GetWorkspaceWithDomains, GetWorkspaceDomains, - GetWorkspaceRoleToDefaultProjectRoleMapping, UpdateWorkspace } from '@/modules/workspaces/domain/operations' import { @@ -52,10 +51,6 @@ import { DeleteAllResourceInvites } from '@/modules/serverinvites/domain/operati import { WorkspaceInviteResourceType } from '@/modules/workspaces/domain/constants' import { ProjectInviteResourceType } from '@/modules/serverinvites/domain/constants' import { chunk, isEmpty, omit } from 'lodash' -import { - DeleteProjectRole, - UpsertProjectRole -} from '@/modules/core/domain/projects/operations' import { userEmailsCompliantWithWorkspaceDomains } from '@/modules/workspaces/domain/logic' import { workspaceRoles as workspaceRoleDefinitions } from '@/modules/workspaces/roles' import { blockedDomains } from '@speckle/shared' @@ -229,15 +224,11 @@ export const deleteWorkspaceRoleFactory = ({ getWorkspaceRoles, deleteWorkspaceRole, - emitWorkspaceEvent, - queryAllWorkspaceProjects, - deleteProjectRole + emitWorkspaceEvent }: { getWorkspaceRoles: GetWorkspaceRoles deleteWorkspaceRole: DeleteWorkspaceRole emitWorkspaceEvent: EmitWorkspaceEvent - queryAllWorkspaceProjects: QueryAllWorkspaceProjects - deleteProjectRole: DeleteProjectRole }) => async ({ workspaceId, @@ -255,17 +246,6 @@ export const deleteWorkspaceRoleFactory = return null } - // Delete workspace project roles - for await (const projectsPage of queryAllWorkspaceProjects({ - workspaceId - })) { - await Promise.all( - projectsPage.map(({ id: projectId }) => - deleteProjectRole({ projectId, userId }) - ) - ) - } - // Emit deleted role await emitWorkspaceEvent({ eventName: WorkspaceEvents.RoleDeleted, @@ -295,20 +275,12 @@ export const updateWorkspaceRoleFactory = getWorkspaceWithDomains, findVerifiedEmailsByUserId, upsertWorkspaceRole, - upsertProjectRole, - deleteProjectRole, - getDefaultWorkspaceProjectRoleMapping, - queryAllWorkspaceProjects, emitWorkspaceEvent }: { getWorkspaceRoles: GetWorkspaceRoles getWorkspaceWithDomains: GetWorkspaceWithDomains findVerifiedEmailsByUserId: FindVerifiedEmailsByUserId upsertWorkspaceRole: UpsertWorkspaceRole - upsertProjectRole: UpsertProjectRole - deleteProjectRole: DeleteProjectRole - getDefaultWorkspaceProjectRoleMapping: GetWorkspaceRoleToDefaultProjectRoleMapping - queryAllWorkspaceProjects: QueryAllWorkspaceProjects emitWorkspaceEvent: EmitWorkspaceEvent }) => async ({ @@ -327,8 +299,16 @@ export const updateWorkspaceRoleFactory = */ preventRoleDowngrade?: boolean }): Promise => { - // Protect against removing last admin const workspaceRoles = await getWorkspaceRoles({ workspaceId }) + + // Return early if no work required + const previousWorkspaceRole = workspaceRoles.find((acl) => acl.userId === userId) + + if (previousWorkspaceRole?.role === nextWorkspaceRole) { + return + } + + // Protect against removing last admin if ( isUserLastWorkspaceAdmin(workspaceRoles, userId) && nextWorkspaceRole !== Roles.Workspace.Admin @@ -336,8 +316,6 @@ export const updateWorkspaceRoleFactory = throw new WorkspaceAdminRequiredError() } - const previousWorkspaceRole = workspaceRoles.find((acl) => acl.userId === userId) - // prevent role downgrades (used during invite flow) if (preventRoleDowngrade) { if (previousWorkspaceRole) { @@ -369,7 +347,7 @@ export const updateWorkspaceRoleFactory = } } - // Perform upsert + // Perform and emit change await upsertWorkspaceRole({ userId, workspaceId, @@ -377,45 +355,17 @@ export const updateWorkspaceRoleFactory = createdAt: previousWorkspaceRole?.createdAt ?? new Date() }) - // Emit new role await emitWorkspaceEvent({ eventName: WorkspaceEvents.RoleUpdated, - payload: { userId, workspaceId, role: nextWorkspaceRole } + payload: { + userId, + workspaceId, + role: nextWorkspaceRole, + flags: { + skipProjectRoleUpdatesFor: skipProjectRoleUpdatesFor ?? [] + } + } }) - - // Update roles for all workspace projects - const defaultProjectRoleMapping = await getDefaultWorkspaceProjectRoleMapping({ - workspaceId - }) - - for await (const projectsPage of queryAllWorkspaceProjects({ - workspaceId - })) { - await Promise.all( - projectsPage.map(({ id: projectId }) => { - // skip assigning project role implied by workspace role (used during invite flow) - if (skipProjectRoleUpdatesFor?.includes(projectId)) { - return - } - - // no change required - if (previousWorkspaceRole?.role === nextWorkspaceRole) return - - const nextProjectRole = defaultProjectRoleMapping[nextWorkspaceRole] - - // user is being removed from workspace or demoted to workspace guest - if (!nextWorkspaceRole || !nextProjectRole) - return deleteProjectRole({ projectId, userId }) - - // user is being granted a workspace role with new role for given project - return upsertProjectRole({ - projectId, - userId, - role: nextProjectRole - }) - }) - ) - } } export const addDomainToWorkspaceFactory = diff --git a/packages/server/modules/workspaces/tests/helpers/creation.ts b/packages/server/modules/workspaces/tests/helpers/creation.ts index d8521362c..9ad78eb93 100644 --- a/packages/server/modules/workspaces/tests/helpers/creation.ts +++ b/packages/server/modules/workspaces/tests/helpers/creation.ts @@ -1,21 +1,15 @@ import { db } from '@/db/knex' -import { - deleteProjectRoleFactory, - getStream, - upsertProjectRoleFactory -} from '@/modules/core/repositories/streams' +import { getStream } from '@/modules/core/repositories/streams' import { findEmailsByUserIdFactory, findVerifiedEmailsByUserIdFactory } from '@/modules/core/repositories/userEmails' -import { getStreams } from '@/modules/core/services/streams' import { findUserByTargetFactory, insertInviteAndDeleteOldFactory } from '@/modules/serverinvites/repositories/serverInvites' import { createAndSendInviteFactory } from '@/modules/serverinvites/services/creation' import { getEventBus } from '@/modules/shared/services/eventBus' -import { mapWorkspaceRoleToInitialProjectRole } from '@/modules/workspaces/domain/logic' import { getWorkspaceRolesFactory, upsertWorkspaceFactory, @@ -38,7 +32,6 @@ import { updateWorkspaceFactory, addDomainToWorkspaceFactory } from '@/modules/workspaces/services/management' -import { queryAllWorkspaceProjectsFactory } from '@/modules/workspaces/services/projects' import { BasicTestUser } from '@/test/authHelper' import { CreateWorkspaceInviteMutationVariables } from '@/test/graphql/generated/graphql' import { MaybeNullOrUndefined, Roles, WorkspaceRoles } from '@speckle/shared' @@ -138,10 +131,6 @@ export const assignToWorkspace = async ( findVerifiedEmailsByUserId: findVerifiedEmailsByUserIdFactory({ db }), getWorkspaceRoles: getWorkspaceRolesFactory({ db }), upsertWorkspaceRole: upsertWorkspaceRoleFactory({ db }), - getDefaultWorkspaceProjectRoleMapping: mapWorkspaceRoleToInitialProjectRole, - upsertProjectRole: upsertProjectRoleFactory({ db }), - deleteProjectRole: deleteProjectRoleFactory({ db }), - queryAllWorkspaceProjects: queryAllWorkspaceProjectsFactory({ getStreams }), emitWorkspaceEvent: (...args) => getEventBus().emit(...args) }) @@ -159,8 +148,6 @@ export const unassignFromWorkspace = async ( const deleteWorkspaceRole = deleteWorkspaceRoleFactory({ getWorkspaceRoles: getWorkspaceRolesFactory({ db }), deleteWorkspaceRole: dbDeleteWorkspaceRoleFactory({ db }), - deleteProjectRole: deleteProjectRoleFactory({ db }), - queryAllWorkspaceProjects: queryAllWorkspaceProjectsFactory({ getStreams }), emitWorkspaceEvent: (...args) => getEventBus().emit(...args) }) diff --git a/packages/server/modules/workspaces/tests/integration/workspaces.graph.spec.ts b/packages/server/modules/workspaces/tests/integration/workspaces.graph.spec.ts index 3f3a5e9de..65456cd25 100644 --- a/packages/server/modules/workspaces/tests/integration/workspaces.graph.spec.ts +++ b/packages/server/modules/workspaces/tests/integration/workspaces.graph.spec.ts @@ -432,11 +432,9 @@ describe('Workspaces GQL CRUD', () => { // first 10 users await createTestUsers(freeGuests) - await Promise.all( - freeGuests.map((guest) => - assignToWorkspace(workspace, guest, Roles.Workspace.Guest) - ) - ) + for (const guest of freeGuests) { + await assignToWorkspace(workspace, guest, Roles.Workspace.Guest) + } await Promise.all([ createTestUser(member), diff --git a/packages/server/modules/workspaces/tests/unit/events/eventListener.spec.ts b/packages/server/modules/workspaces/tests/unit/events/eventListener.spec.ts index c8282a9a4..6a3d4f182 100644 --- a/packages/server/modules/workspaces/tests/unit/events/eventListener.spec.ts +++ b/packages/server/modules/workspaces/tests/unit/events/eventListener.spec.ts @@ -4,7 +4,7 @@ import { Roles, StreamRoles } from '@speckle/shared' import { StreamAclRecord, StreamRecord } from '@/modules/core/helpers/types' import { onProjectCreatedFactory, - onWorkspaceJoinedFactory + onWorkspaceRoleUpdatedFactory } from '@/modules/workspaces/events/eventListener' import { expect } from 'chai' import { mapWorkspaceRoleToInitialProjectRole } from '@/modules/workspaces/domain/logic' @@ -61,16 +61,22 @@ describe('Event handlers', () => { expect(projectRoles.length).to.equal(2) }) }) - describe('onWorkspaceJoinedFactory creates a function, that', () => { + describe('onWorkspaceRoleUpdatedFactory creates a function, that', () => { it('assigns no project roles if the role mapping returns null', async () => { - await onWorkspaceJoinedFactory({ + let isDeleteCalled = false + + await onWorkspaceRoleUpdatedFactory({ getDefaultWorkspaceProjectRoleMapping: async () => ({ [Roles.Workspace.Admin]: Roles.Stream.Owner, [Roles.Workspace.Member]: Roles.Stream.Contributor, [Roles.Workspace.Guest]: null }), async *queryAllWorkspaceProjects() { - expect.fail() + yield [{ id: 'test' } as StreamRecord] + }, + deleteProjectRole: async () => { + isDeleteCalled = true + return undefined }, upsertProjectRole: async () => { expect.fail() @@ -80,6 +86,8 @@ describe('Event handlers', () => { userId: cryptoRandomString({ length: 10 }), workspaceId: cryptoRandomString({ length: 10 }) }) + + expect(isDeleteCalled).to.be.true }) it('assigns the mapped projects roles to all queried project', async () => { const projectIds = [ @@ -92,7 +100,7 @@ describe('Event handlers', () => { const projectRole = Roles.Stream.Reviewer const storedRoles: { userId: string; role: StreamRoles; projectId: string }[] = [] - await onWorkspaceJoinedFactory({ + await onWorkspaceRoleUpdatedFactory({ getDefaultWorkspaceProjectRoleMapping: async () => ({ [Roles.Workspace.Admin]: Roles.Stream.Owner, [Roles.Workspace.Member]: projectRole, @@ -103,6 +111,9 @@ describe('Event handlers', () => { yield projIds.map((projId) => ({ id: projId } as unknown as StreamRecord)) } }, + deleteProjectRole: async () => { + expect.fail() + }, upsertProjectRole: async (args) => { storedRoles.push(args) return {} as StreamRecord diff --git a/packages/server/modules/workspaces/tests/unit/services/management.spec.ts b/packages/server/modules/workspaces/tests/unit/services/management.spec.ts index b81b9e61c..f3c103569 100644 --- a/packages/server/modules/workspaces/tests/unit/services/management.spec.ts +++ b/packages/server/modules/workspaces/tests/unit/services/management.spec.ts @@ -14,7 +14,10 @@ import { import { Roles } from '@speckle/shared' import { expect } from 'chai' import cryptoRandomString from 'crypto-random-string' -import { WorkspaceEvents } from '@/modules/workspacesCore/domain/events' +import { + WorkspaceEvents, + WorkspaceEventsPayloads +} from '@/modules/workspacesCore/domain/events' import { StreamAclRecord, StreamRecord } from '@/modules/core/helpers/types' import { expectToThrow } from '@/test/assertionHelper' import { createRandomPassword } from '@/modules/core/helpers/testHelpers' @@ -386,17 +389,21 @@ const buildDeleteWorkspaceRoleAndTestContext = ( context.eventData.eventName = eventName context.eventData.payload = payload + switch (eventName) { + case 'workspace.role-deleted': { + const { userId } = + payload as WorkspaceEventsPayloads['workspace.role-deleted'] + for (const project of context.workspaceProjects) { + context.workspaceProjectRoles = context.workspaceProjectRoles.filter( + (role) => role.resourceId !== project.id && role.userId !== userId + ) + } + break + } + } + return [] }, - async *queryAllWorkspaceProjects() { - yield context.workspaceProjects - }, - deleteProjectRole: async ({ projectId, userId }) => { - context.workspaceProjectRoles = context.workspaceProjectRoles.filter( - (role) => role.resourceId !== projectId && role.userId !== userId - ) - return {} as StreamRecord - }, ...dependencyOverrides } @@ -430,32 +437,47 @@ const buildUpdateWorkspaceRoleAndTestContext = ( context.eventData.eventName = eventName context.eventData.payload = payload - return [] - }, - async *queryAllWorkspaceProjects() { - yield context.workspaceProjects - }, - getDefaultWorkspaceProjectRoleMapping: mapWorkspaceRoleToInitialProjectRole, - upsertProjectRole: async (role) => { - const streamAcl: StreamAclRecord = { - userId: role.userId, - role: role.role, - resourceId: role.projectId + switch (eventName) { + case 'workspace.role-deleted': { + const { userId } = + payload as WorkspaceEventsPayloads['workspace.role-deleted'] + for (const project of context.workspaceProjects) { + context.workspaceProjectRoles = context.workspaceProjectRoles.filter( + (role) => role.resourceId !== project.id && role.userId !== userId + ) + } + break + } + case 'workspace.role-updated': { + const workspaceRole = + payload as WorkspaceEventsPayloads['workspace.role-updated'] + const mapping = await mapWorkspaceRoleToInitialProjectRole({ + workspaceId: workspaceRole.workspaceId + }) + + for (const project of context.workspaceProjects) { + const projectRole = mapping[workspaceRole.role] + + if (!projectRole) { + continue + } + + const streamAcl: StreamAclRecord = { + userId: workspaceRole.userId, + role: projectRole, + resourceId: project.id + } + + context.workspaceProjectRoles = context.workspaceProjectRoles.filter( + (acl) => acl.userId !== workspaceRole.userId + ) + context.workspaceProjectRoles.push(streamAcl) + } + break + } } - context.workspaceProjectRoles = context.workspaceProjectRoles.filter( - (acl) => acl.userId !== role.userId - ) - context.workspaceProjectRoles.push(streamAcl) - - return {} as StreamRecord - }, - deleteProjectRole: async ({ userId }) => { - context.workspaceProjectRoles = context.workspaceProjectRoles.filter( - (acl) => acl.userId !== userId - ) - - return {} as StreamRecord + return [] }, ...dependencyOverrides } @@ -589,9 +611,15 @@ describe('Workspace role services', () => { await updateWorkspaceRole(role) + const payload = { + ...(context.eventData + .payload as WorkspaceEventsPayloads['workspace.role-updated']) + } + delete payload.flags + expect(context.eventData.isCalled).to.be.true expect(context.eventData.eventName).to.equal(WorkspaceEvents.RoleUpdated) - expect(context.eventData.payload).to.deep.equal(role) + expect(payload).to.deep.equal(role) }) it('throws if attempting to remove the last admin in a workspace', async () => { const userId = cryptoRandomString({ length: 10 }) diff --git a/packages/server/modules/workspacesCore/domain/events.ts b/packages/server/modules/workspacesCore/domain/events.ts index dd3147d10..03f01ad1a 100644 --- a/packages/server/modules/workspacesCore/domain/events.ts +++ b/packages/server/modules/workspacesCore/domain/events.ts @@ -20,7 +20,10 @@ type WorkspaceCreatedPayload = Workspace & { } type WorkspaceUpdatedPayload = Workspace type WorkspaceRoleDeletedPayload = Pick -type WorkspaceRoleUpdatedPayload = Pick +type WorkspaceRoleUpdatedPayload = Pick< + WorkspaceAcl, + 'userId' | 'workspaceId' | 'role' +> & { flags?: { skipProjectRoleUpdatesFor: string[] } } type WorkspaceJoinedFromDiscoveryPayload = { userId: string workspaceId: string