When swapping directors, keep members active that are filling other roles (#3501)

Author: CarsonF

src/components/project/project-member/project-member.gel.repository.ts

@@ -116,12 +116,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,

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';
@@ -240,15 +241,47 @@ export class ProjectMemberRepository extends DtoRepository(ProjectMember) {
           },
         }),
       )
-      .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([

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<ANY> values.
      * @see https://neo4j.com/docs/apoc/current/overview/apoc.coll/apoc.coll.union/

src/core/database/query/properties/update-property.ts

@@ -206,7 +206,7 @@ export const conditionalOn = <R>(
   trueQuery: QueryFragment<unknown, R>,
   falseQuery: QueryFragment<unknown, R>,
 ): QueryFragment<unknown, R> => {
-  const imports = [...new Set([conditionVar, ...scope])];
+  const imports = [...new Set([varInExp(conditionVar), ...scope])];
   return (query) =>
     query.subQuery((sub) =>
       sub

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);