From c110e5c2a7c4e4e3ca431ca4591760f2c11335fd Mon Sep 17 00:00:00 2001 From: Carson Full Date: Tue, 17 Jun 2025 12:52:23 -0500 Subject: [PATCH] When swapping directors, keep members active that are filling other roles --- .../project-member.gel.repository.ts | 12 +++- .../project-member.repository.ts | 49 ++++++++++++--- src/core/database/query/cypher-functions.ts | 5 ++ .../query/properties/update-property.ts | 2 +- ...s-memberships-on-open-projects.e2e-spec.ts | 60 ++++++++++--------- 5 files changed, 90 insertions(+), 38 deletions(-) diff --git a/src/components/project/project-member/project-member.gel.repository.ts b/src/components/project/project-member/project-member.gel.repository.ts index 96f17bd596..47de90eb49 100644 --- a/src/components/project/project-member/project-member.gel.repository.ts +++ b/src/components/project/project-member/project-member.gel.repository.ts @@ -104,12 +104,20 @@ export class ProjectMemberGelRepository ), ), })); - const inactivated = e.update(members, () => ({ + const inactivated = e.update(members, (member) => ({ + filter: e.op(e.count(member.roles), '=', 1), set: { inactiveAt: e.datetime_of_transaction(), }, })); - const replacements = e.for(inactivated.project, (project) => + const fillingOtherRoles = e.update(members, (member) => ({ + filter: e.op(e.count(member.roles), '>', 1), + set: { + roles: { '-=': $.role }, + }, + })); + const projects = e.op(inactivated, 'union', fillingOtherRoles).project; + const replacements = e.for(projects, (project) => e .insert(e.Project.Member, { project, diff --git a/src/components/project/project-member/project-member.repository.ts b/src/components/project/project-member/project-member.repository.ts index 5bc4df66d1..56e866e2c7 100644 --- a/src/components/project/project-member/project-member.repository.ts +++ b/src/components/project/project-member/project-member.repository.ts @@ -34,6 +34,7 @@ import { variable, } from '~/core/database/query'; import { type FilterFn } from '~/core/database/query/filters'; +import { conditionalOn } from '~/core/database/query/properties/update-property'; import { userFilters, UserRepository } from '../../user/user.repository'; import { type ProjectFilters } from '../dto'; import { projectFilters } from '../project-filters.query'; @@ -235,15 +236,47 @@ export class ProjectMemberRepository extends DtoRepository(ProjectMember) { project: { status: ['Active', 'InDevelopment'] }, }), ) - .apply( - updateProperty({ - resource: ProjectMember, - key: 'inactiveAt', - value: now, - permanentAfter: 0, - }), + .subQuery('node', (sub) => + sub + .match([ + node('node'), + relation('out', '', 'roles', ACTIVE), + node('roles', 'Property'), + ]) + .apply( + conditionalOn( + 'size(roles.value) > 1', + ['node'], + // If there are other roles, remove this role from the membership & keep it active + (q) => + q + .apply( + updateProperty({ + resource: ProjectMember, + key: 'roles', + value: variable( + apoc.coll.disjunction('roles.value', [`"${role}"`]), + ), + permanentAfter: 0, + }), + ) + .return('stats'), + // Else then mark the membership inactive & maintain the role + (q) => + q + .apply( + updateProperty({ + resource: ProjectMember, + key: 'inactiveAt', + value: now, + permanentAfter: 0, + }), + ) + .return('stats'), + ), + ) + .return('stats as oldMemberStats'), ) - .with('project') .subQuery('project', (sub) => sub .match([ diff --git a/src/core/database/query/cypher-functions.ts b/src/core/database/query/cypher-functions.ts index 71d905a018..b7432d11d0 100644 --- a/src/core/database/query/cypher-functions.ts +++ b/src/core/database/query/cypher-functions.ts @@ -82,6 +82,11 @@ export const apoc = { coll: { flatten: fn1('apoc.coll.flatten'), indexOf: fn('apoc.coll.indexOf'), + /** + * Returns the items in the first list that aren't in the second list. + * @see https://neo4j.com/docs/apoc/current/overview/apoc.coll/apoc.coll.disjunction/ + */ + disjunction: fn2('apoc.coll.disjunction'), /** * Returns the distinct union of the two given LIST values. * @see https://neo4j.com/docs/apoc/current/overview/apoc.coll/apoc.coll.union/ diff --git a/src/core/database/query/properties/update-property.ts b/src/core/database/query/properties/update-property.ts index 02e14b2b20..2e0ac5d237 100644 --- a/src/core/database/query/properties/update-property.ts +++ b/src/core/database/query/properties/update-property.ts @@ -206,7 +206,7 @@ export const conditionalOn = ( trueQuery: QueryFragment, falseQuery: QueryFragment, ): QueryFragment => { - const imports = [...new Set([conditionVar, ...scope])]; + const imports = [...new Set([varInExp(conditionVar), ...scope])]; return (query) => query.subQuery((sub) => sub diff --git a/test/features/director-change-replaces-memberships-on-open-projects.e2e-spec.ts b/test/features/director-change-replaces-memberships-on-open-projects.e2e-spec.ts index 5c82ce817f..8996a62d81 100644 --- a/test/features/director-change-replaces-memberships-on-open-projects.e2e-spec.ts +++ b/test/features/director-change-replaces-memberships-on-open-projects.e2e-spec.ts @@ -1,3 +1,4 @@ +import { entries, mapEntries } from '@seedcompany/common'; import { DateTime } from 'luxon'; import { type ID, Role } from '~/common'; import { graphql } from '~/graphql'; @@ -66,6 +67,15 @@ it('director change replaces memberships on open projects', async () => { }); return project; })(), + hasMemberIncludingOtherRoles: await (async () => { + const project = await createProject(app); + await createProjectMember(app, { + projectId: project.id, + userId: directors.old.id, + roles: [Role.RegionalDirector, Role.ProjectManager], + }); + return project; + })(), alreadyHasRoleFilled: await (async () => { const project = await createProject(app); await createProjectMember(app, { @@ -138,33 +148,13 @@ it('director change replaces memberships on open projects', async () => { }; const getResults = async () => { - const results = { - needsSwapA: await fetchMembers(app, projects.needsSwapA.id), - needsSwapB: await fetchMembers(app, projects.needsSwapB.id), - doesNotHaveMember: await fetchMembers(app, projects.doesNotHaveMember.id), - hasMemberButInactive: await fetchMembers( - app, - projects.hasMemberButInactive.id, - ), - alreadyHasRoleFilled: await fetchMembers( - app, - projects.alreadyHasRoleFilled.id, - ), - alreadyHasNewDirectorActive: await fetchMembers( - app, - projects.alreadyHasNewDirectorActive.id, - ), - alreadyHasNewDirectorInactive: await fetchMembers( - app, - projects.alreadyHasNewDirectorInactive.id, - ), - alreadyHasNewDirectorWithoutRole: await fetchMembers( - app, - projects.alreadyHasNewDirectorWithoutRole.id, - ), - closed: await fetchMembers(app, projects.closed.id), - }; - + const resultList = await Promise.all( + entries(projects).map(async ([key, project]) => { + const results = await fetchMembers(app, project.id); + return [key, results] as const; + }), + ); + const results = mapEntries(resultList, (i) => i).asRecord; return { get: (project: keyof typeof results, key: keyof typeof directors) => { const member = results[project].find( @@ -200,6 +190,14 @@ it('director change replaces memberships on open projects', async () => { expect(before.get('hasMemberButInactive', 'old')).toEqual(InactiveRD); expect(before.get('hasMemberButInactive', 'new')).toBeUndefined(); expect(before.get('hasMemberButInactive', 'unrelated')).toBeUndefined(); + expect(before.get('hasMemberIncludingOtherRoles', 'old')).toEqual({ + active: true, + roles: [Role.RegionalDirector, Role.ProjectManager], + }); + expect(before.get('hasMemberIncludingOtherRoles', 'new')).toBeUndefined(); + expect( + before.get('hasMemberIncludingOtherRoles', 'unrelated'), + ).toBeUndefined(); expect(before.get('alreadyHasRoleFilled', 'old')).toBeUndefined(); expect(before.get('alreadyHasRoleFilled', 'new')).toBeUndefined(); expect(before.get('alreadyHasRoleFilled', 'unrelated')).toEqual(ActiveRD); @@ -264,6 +262,14 @@ it('director change replaces memberships on open projects', async () => { expect(after.get('hasMemberButInactive', 'old')).toEqual(InactiveRD); expect(after.get('hasMemberButInactive', 'new')).toBeUndefined(); expect(after.get('hasMemberButInactive', 'unrelated')).toBeUndefined(); + expect(after.get('hasMemberIncludingOtherRoles', 'old')).toEqual({ + active: true, // still active + roles: [Role.ProjectManager], // but without RD + }); + expect(after.get('hasMemberIncludingOtherRoles', 'new')).toEqual(ActiveRD); + expect( + after.get('hasMemberIncludingOtherRoles', 'unrelated'), + ).toBeUndefined(); expect(after.get('alreadyHasRoleFilled', 'old')).toBeUndefined(); expect(after.get('alreadyHasRoleFilled', 'new')).toBeUndefined(); expect(after.get('alreadyHasRoleFilled', 'unrelated')).toEqual(ActiveRD);