From 9dfd36431f6a79d548032ca0ce8ab400c2b69c76 Mon Sep 17 00:00:00 2001 From: Chuck Driesler Date: Fri, 18 Apr 2025 10:04:07 +0100 Subject: [PATCH] fix(invites): projectinvitablecollaborators should only return members (#4493) --- .../modules/workspaces/repositories/users.ts | 6 +- .../integration/repositories/users.spec.ts | 307 +++++------------- .../tests/integration/users.graph.spec.ts | 131 ++++---- 3 files changed, 164 insertions(+), 280 deletions(-) diff --git a/packages/server/modules/workspaces/repositories/users.ts b/packages/server/modules/workspaces/repositories/users.ts index 7a09b9f0b..d298d1853 100644 --- a/packages/server/modules/workspaces/repositories/users.ts +++ b/packages/server/modules/workspaces/repositories/users.ts @@ -41,8 +41,8 @@ const buildInvitableCollaboratorsByProjectIdQueryFactory = Users.col.id, tables .streamAcl(db) - .select(StreamAcl.col.resourceId) - .whereNot(StreamAcl.col.resourceId, projectId) + .select(StreamAcl.col.userId) + .where(StreamAcl.col.resourceId, projectId) ) if (search) { query @@ -104,5 +104,5 @@ export const countInvitableCollaboratorsByProjectIdFactory = search }) const [res] = await query.count() - return parseInt(res.count.toString()) + return parseInt(res?.count?.toString() ?? '0') } diff --git a/packages/server/modules/workspaces/tests/integration/repositories/users.spec.ts b/packages/server/modules/workspaces/tests/integration/repositories/users.spec.ts index c1c3f1cea..cc1451fc1 100644 --- a/packages/server/modules/workspaces/tests/integration/repositories/users.spec.ts +++ b/packages/server/modules/workspaces/tests/integration/repositories/users.spec.ts @@ -6,275 +6,140 @@ import { import { getInvitableCollaboratorsByProjectIdFactory } from '@/modules/workspaces/repositories/users' import { assignToWorkspace, + BasicTestWorkspace, createTestWorkspace } from '@/modules/workspaces/tests/helpers/creation' -import { createTestUser } from '@/test/authHelper' -import { createTestStream } from '@/test/speckle-helpers/streamHelper' -import { Roles } from '@speckle/shared' +import { BasicTestUser, createTestUser, createTestUsers } from '@/test/authHelper' +import { BasicTestStream, createTestStream } from '@/test/speckle-helpers/streamHelper' import { expect } from 'chai' import { pick } from 'lodash' describe('Workspace repositories', () => { describe('users repository', () => { - describe('getInvitableCollaboratorsByProjectIdFactory returns a function that ', () => { + describe('getInvitableCollaboratorsByProjectIdFactory returns a function, that', () => { const getInvitableCollaboratorsByProjectId = getInvitableCollaboratorsByProjectIdFactory({ db }) + const adminUser: BasicTestUser = { + id: '', + name: createRandomString(), + email: createRandomEmail() + } + const workspaceMemberA: BasicTestUser = { + id: '', + name: createRandomString() + 'foo', + email: 'baz' + createRandomEmail() + } + const workspaceMemberB: BasicTestUser = { + id: '', + name: createRandomString() + 'baz', + email: 'bar' + createRandomEmail() + } + const nonWorkspaceMember: BasicTestUser = { + id: '', + name: createRandomString(), + email: createRandomEmail() + } + + const testWorkspace: BasicTestWorkspace = { + id: createRandomString(), + name: createRandomString(), + slug: createRandomString(), + ownerId: '' + } + + // The project we will run the test suite search against + const testProject: BasicTestStream = { + id: '', + ownerId: '', + name: createRandomString(), + isPublic: true, + workspaceId: '' + } + // An extra project for test comprehensiveness + const testOtherProject: BasicTestStream = { + id: '', + ownerId: '', + name: createRandomString(), + isPublic: true, + workspaceId: '' + } + + before(async () => { + await createTestUser(adminUser) + await createTestUsers([workspaceMemberA, workspaceMemberB, nonWorkspaceMember]) + + await createTestWorkspace(testWorkspace, adminUser, { + addPlan: { + name: 'unlimited', + status: 'valid' + } + }) + await assignToWorkspace(testWorkspace, workspaceMemberA) + await assignToWorkspace(testWorkspace, workspaceMemberB) + + testProject.workspaceId = testWorkspace.id + testOtherProject.workspaceId = testWorkspace.id + + await createTestStream(testProject, adminUser) + await createTestStream(testOtherProject, workspaceMemberA) + }) + it('should return all workspace collaborators not members of the project', async () => { - const admin = await createTestUser({ - name: createRandomString(), - email: createRandomEmail(), - role: Roles.Server.User, - verified: true - }) - const workspace = { - id: createRandomString(), - name: createRandomString(), - slug: createRandomString(), - ownerId: admin.id - } - await createTestWorkspace(workspace, admin) - - const member = await createTestUser({ - name: createRandomString(), - email: createRandomEmail(), - role: Roles.Server.User, - verified: true - }) - await assignToWorkspace(workspace, member, Roles.Workspace.Member) - - // Non workspace member - await createTestUser({ - name: createRandomString(), - email: createRandomEmail(), - role: Roles.Server.User, - verified: true - }) - - const projectMember = await createTestUser({ - name: createRandomString(), - email: createRandomEmail(), - role: Roles.Server.User, - verified: true - }) - - const project = { - id: createRandomString(), - workspaceId: workspace.id - } - await createTestStream(project, projectMember) - - // User in another project should still be invitable - const otherProject = { - id: createRandomString(), - workspaceId: workspace.id - } - await createTestStream(otherProject, admin) - const invitable = await getInvitableCollaboratorsByProjectId({ filter: { - workspaceId: workspace.id, - projectId: project.id + workspaceId: testWorkspace.id, + projectId: testProject.id }, limit: 10 }) expect(invitable).to.have.length(2) expect(invitable.map((i) => pick(i, ['id', 'name']))).to.deep.equalInAnyOrder([ - { id: admin.id, name: admin.name }, - { id: member.id, name: member.name } + { id: workspaceMemberA.id, name: workspaceMemberA.name }, + { id: workspaceMemberB.id, name: workspaceMemberB.name } ]) }) it('should should filter by user name', async () => { - const admin = await createTestUser({ - name: createRandomString() + 'fixed' + createRandomString(), - email: createRandomEmail(), - role: Roles.Server.User, - verified: true - }) - const workspace = { - id: createRandomString(), - name: createRandomString(), - slug: createRandomString(), - ownerId: admin.id - } - await createTestWorkspace(workspace, admin) - - const member = await createTestUser({ - name: createRandomString(), - email: createRandomEmail(), - role: Roles.Server.User, - verified: true - }) - await assignToWorkspace(workspace, member, Roles.Workspace.Member) - - // Non workspace member - await createTestUser({ - name: createRandomString(), - email: createRandomEmail(), - role: Roles.Server.User, - verified: true - }) - - const projectMember = await createTestUser({ - name: createRandomString(), - email: createRandomEmail(), - role: Roles.Server.User, - verified: true - }) - - const project = { - id: createRandomString(), - workspaceId: workspace.id - } - await createTestStream(project, projectMember) - - // User in another project should still be invitable - const otherProject = { - id: createRandomString(), - workspaceId: workspace.id - } - await createTestStream(otherProject, admin) - const invitable = await getInvitableCollaboratorsByProjectId({ filter: { - workspaceId: workspace.id, - projectId: project.id, - search: 'fixed' + workspaceId: testWorkspace.id, + projectId: testProject.id, + search: 'foo' }, limit: 10 }) expect(invitable).to.have.length(1) expect(invitable.map((i) => pick(i, ['id', 'name']))).to.deep.equalInAnyOrder([ - { id: admin.id, name: admin.name } + { id: workspaceMemberA.id, name: workspaceMemberA.name } ]) }) it('should should filter by user email', async () => { - const admin = await createTestUser({ - name: createRandomString(), - email: createRandomString() + 'fixed' + createRandomString(), - role: Roles.Server.User, - verified: true - }) - const workspace = { - id: createRandomString(), - name: createRandomString(), - slug: createRandomString(), - ownerId: admin.id - } - await createTestWorkspace(workspace, admin) - - const member = await createTestUser({ - name: createRandomString(), - email: createRandomEmail(), - role: Roles.Server.User, - verified: true - }) - await assignToWorkspace(workspace, member, Roles.Workspace.Member) - - // Non workspace member - await createTestUser({ - name: createRandomString(), - email: createRandomEmail(), - role: Roles.Server.User, - verified: true - }) - - const projectMember = await createTestUser({ - name: createRandomString(), - email: createRandomEmail(), - role: Roles.Server.User, - verified: true - }) - - const project = { - id: createRandomString(), - workspaceId: workspace.id - } - await createTestStream(project, projectMember) - - // User in another project should still be invitable - const otherProject = { - id: createRandomString(), - workspaceId: workspace.id - } - await createTestStream(otherProject, admin) - const invitable = await getInvitableCollaboratorsByProjectId({ filter: { - workspaceId: workspace.id, - projectId: project.id, - search: 'fixed' + workspaceId: testWorkspace.id, + projectId: testProject.id, + search: 'bar' }, limit: 10 }) expect(invitable).to.have.length(1) expect(invitable.map((i) => pick(i, ['id', 'name']))).to.deep.equalInAnyOrder([ - { id: admin.id, name: admin.name } + { id: workspaceMemberB.id, name: workspaceMemberB.name } ]) }) it('should should filter by user name and email', async () => { - const admin = await createTestUser({ - name: createRandomString(), - email: createRandomString() + 'fixed' + createRandomString(), - role: Roles.Server.User, - verified: true - }) - const workspace = { - id: createRandomString(), - name: createRandomString(), - slug: createRandomString(), - ownerId: admin.id - } - await createTestWorkspace(workspace, admin) - - const member = await createTestUser({ - name: createRandomString() + 'fixed' + createRandomString(), - email: createRandomEmail(), - role: Roles.Server.User, - verified: true - }) - await assignToWorkspace(workspace, member, Roles.Workspace.Member) - - // Non workspace member - await createTestUser({ - name: createRandomString(), - email: createRandomEmail(), - role: Roles.Server.User, - verified: true - }) - - const projectMember = await createTestUser({ - name: createRandomString(), - email: createRandomEmail(), - role: Roles.Server.User, - verified: true - }) - - const project = { - id: createRandomString(), - workspaceId: workspace.id - } - await createTestStream(project, projectMember) - - // User in another project should still be invitable - const otherProject = { - id: createRandomString(), - workspaceId: workspace.id - } - await createTestStream(otherProject, admin) - const invitable = await getInvitableCollaboratorsByProjectId({ filter: { - workspaceId: workspace.id, - projectId: project.id, - search: 'fixed' + workspaceId: testWorkspace.id, + projectId: testProject.id, + search: 'baz' }, limit: 10 }) expect(invitable).to.have.length(2) expect(invitable.map((i) => pick(i, ['id', 'name']))).to.deep.equalInAnyOrder([ - { id: admin.id, name: admin.name }, - { id: member.id, name: member.name } + { id: workspaceMemberA.id, name: workspaceMemberA.name }, + { id: workspaceMemberB.id, name: workspaceMemberB.name } ]) }) }) diff --git a/packages/server/modules/workspaces/tests/integration/users.graph.spec.ts b/packages/server/modules/workspaces/tests/integration/users.graph.spec.ts index 941edf05f..bfe492979 100644 --- a/packages/server/modules/workspaces/tests/integration/users.graph.spec.ts +++ b/packages/server/modules/workspaces/tests/integration/users.graph.spec.ts @@ -7,7 +7,12 @@ import { BasicTestWorkspace, createTestWorkspace } from '@/modules/workspaces/tests/helpers/creation' -import { BasicTestUser, createTestUser, login } from '@/test/authHelper' +import { + BasicTestUser, + createTestUser, + createTestUsers, + login +} from '@/test/authHelper' import { GetProjectInvitableCollaboratorsDocument, SetUserActiveWorkspaceDocument, @@ -16,7 +21,6 @@ import { import { testApolloServer, TestApolloServer } from '@/test/graphqlHelper' import { beforeEachContext } from '@/test/hooks' import { BasicTestStream, createTestStream } from '@/test/speckle-helpers/streamHelper' -import { Roles } from '@speckle/shared' import { expect } from 'chai' import cryptoRandomString from 'crypto-random-string' @@ -105,61 +109,76 @@ describe('ActiveUserMutations.setActiveWorkspace', () => { }) describe('Project.invitableCollaborators', () => { + const adminUser: BasicTestUser = { + id: '', + name: createRandomString(), + email: createRandomEmail() + } + const workspaceMemberA: BasicTestUser = { + id: '', + name: createRandomString() + 'foo', + email: 'baz' + createRandomEmail() + } + const workspaceMemberB: BasicTestUser = { + id: '', + name: createRandomString() + 'baz', + email: 'bar' + createRandomEmail() + } + const nonWorkspaceMember: BasicTestUser = { + id: '', + name: createRandomString(), + email: createRandomEmail() + } + + const testWorkspace: BasicTestWorkspace = { + id: createRandomString(), + name: createRandomString(), + slug: createRandomString(), + ownerId: '' + } + + // The project we will run the test suite search against + const testProject: BasicTestStream = { + id: '', + ownerId: '', + name: createRandomString(), + isPublic: true, + workspaceId: '' + } + // An extra project for test comprehensiveness + const testOtherProject: BasicTestStream = { + id: '', + ownerId: '', + name: createRandomString(), + isPublic: true, + workspaceId: '' + } + + before(async () => { + await createTestUser(adminUser) + await createTestUsers([workspaceMemberA, workspaceMemberB, nonWorkspaceMember]) + + await createTestWorkspace(testWorkspace, adminUser, { + addPlan: { + name: 'unlimited', + status: 'valid' + } + }) + await assignToWorkspace(testWorkspace, workspaceMemberA) + await assignToWorkspace(testWorkspace, workspaceMemberB) + + testProject.workspaceId = testWorkspace.id + testOtherProject.workspaceId = testWorkspace.id + + await createTestStream(testProject, adminUser) + await createTestStream(testOtherProject, workspaceMemberA) + }) + it('should return invitable collaborators', async () => { - const admin = await createTestUser({ - name: createRandomString(), - email: createRandomEmail(), - role: Roles.Server.User, - verified: true - }) - const workspace = { - id: createRandomString(), - name: createRandomString(), - slug: createRandomString(), - ownerId: admin.id - } - await createTestWorkspace(workspace, admin) - - const member = await createTestUser({ - name: createRandomString(), - email: createRandomEmail(), - role: Roles.Server.User, - verified: true - }) - await assignToWorkspace(workspace, member, Roles.Workspace.Member) - - // Non workspace member - await createTestUser({ - name: createRandomString(), - email: createRandomEmail(), - role: Roles.Server.User, - verified: true - }) - - const projectMember = await createTestUser({ - name: createRandomString(), - email: createRandomEmail(), - role: Roles.Server.User, - verified: true - }) - - const project = { - id: createRandomString(), - workspaceId: workspace.id - } - await createTestStream(project, projectMember) - - // User in another project should still be invitable - const otherProject = { - id: createRandomString(), - workspaceId: workspace.id - } - await createTestStream(otherProject, admin) - - const session = await login(admin) + const session = await login(adminUser) const res = await session.execute(GetProjectInvitableCollaboratorsDocument, { - projectId: project.id + projectId: testProject.id }) expect(res).not.haveGraphQLErrors() @@ -167,8 +186,8 @@ describe('Project.invitableCollaborators', () => { expect(invitable?.totalCount).to.eq(2) expect(invitable?.items).to.have.length(2) expect(invitable?.items).to.deep.equalInAnyOrder([ - { id: admin.id, user: { name: admin.name } }, - { id: member.id, user: { name: member.name } } + { id: workspaceMemberA.id, user: { name: workspaceMemberA.name } }, + { id: workspaceMemberB.id, user: { name: workspaceMemberB.name } } ]) }) })