refactor: move project membership check from access to project (#3322)

src/lib/services/access-service.ts

@@ -25,13 +25,12 @@ import NameExistsError from '../error/name-exists-error';
 import { IEnvironmentStore } from 'lib/types/stores/environment-store';
 import RoleInUseError from '../error/role-in-use-error';
 import { roleSchema } from '../schema/role-schema';
-import { CUSTOM_ROLE_TYPE, ALL_PROJECTS, ALL_ENVS } from '../util/constants';
+import { ALL_ENVS, ALL_PROJECTS, CUSTOM_ROLE_TYPE } from '../util/constants';
 import { DEFAULT_PROJECT } from '../types/project';
 import InvalidOperationError from '../error/invalid-operation-error';
 import BadDataError from '../error/bad-data-error';
 import { IGroupModelWithProjectRole } from '../types/group';
 import { GroupService } from './group-service';
-import { uniqueByKey } from '../util/unique';
 
 const { ADMIN } = permissions;
 
@@ -410,31 +409,6 @@ export class AccessService {
         return [roles, users.flat(), groups];
     }
 
-    async getProjectMembers(
-        projectId: string,
-    ): Promise<Array<Pick<IUser, 'id' | 'email' | 'username'>>> {
-        const [, users, groups] = await this.getProjectRoleAccess(projectId);
-        const actualUsers = users.map((user) => ({
-            id: user.id,
-            email: user.email,
-            username: user.username,
-        }));
-        const actualGroupUsers = groups
-            .flatMap((group) => group.users)
-            .map((user) => user.user)
-            .map((user) => ({
-                id: user.id,
-                email: user.email,
-                username: user.username,
-            }));
-        return uniqueByKey([...actualUsers, ...actualGroupUsers], 'id');
-    }
-
-    async isProjectMember(userId: number, projectId: string): Promise<boolean> {
-        const users = await this.getProjectMembers(projectId);
-        return Boolean(users.find((user) => user.id === userId));
-    }
-
     async createDefaultProjectRoles(
         owner: IUser,
         projectId: string,

src/lib/services/project-service.ts

@@ -50,6 +50,7 @@ import { IGroupModelWithProjectRole, IGroupRole } from 'lib/types/group';
 import { FavoritesService } from './favorites-service';
 import { TimeToProduction } from '../read-models/time-to-production/time-to-production';
 import { IProjectStatsStore } from 'lib/types/stores/project-stats-store-type';
+import { uniqueByKey } from '../util/unique';
 
 const getCreatedBy = (user: IUser) => user.email || user.username || 'unknown';
 
@@ -623,6 +624,33 @@ export default class ProjectService {
         return this.store.getMembersCountByProject(projectId);
     }
 
+    async getProjectUsers(
+        projectId: string,
+    ): Promise<Array<Pick<IUser, 'id' | 'email' | 'username'>>> {
+        const [, users, groups] = await this.accessService.getProjectRoleAccess(
+            projectId,
+        );
+        const actualUsers = users.map((user) => ({
+            id: user.id,
+            email: user.email,
+            username: user.username,
+        }));
+        const actualGroupUsers = groups
+            .flatMap((group) => group.users)
+            .map((user) => user.user)
+            .map((user) => ({
+                id: user.id,
+                email: user.email,
+                username: user.username,
+            }));
+        return uniqueByKey([...actualUsers, ...actualGroupUsers], 'id');
+    }
+
+    async isProjectUser(userId: number, projectId: string): Promise<boolean> {
+        const users = await this.getProjectUsers(projectId);
+        return Boolean(users.find((user) => user.id === userId));
+    }
+
     async getProjectsByUser(userId: number): Promise<string[]> {
         return this.store.getProjectsByUser(userId);
     }

src/test/e2e/services/access-service.e2e.test.ts

@@ -6,7 +6,7 @@ import { AccessService } from '../../../lib/services/access-service';
 
 import * as permissions from '../../../lib/types/permissions';
 import { RoleName } from '../../../lib/types/model';
-import { IUnleashStores, IUser } from '../../../lib/types';
+import { IUnleashStores } from '../../../lib/types';
 import FeatureToggleService from '../../../lib/services/feature-toggle-service';
 import ProjectService from '../../../lib/services/project-service';
 import { createTestConfig } from '../../config/test-config';
@@ -43,16 +43,6 @@ const createUserViewerAccess = async (name, email) => {
     return user;
 };
 
-const isProjectMember = async (
-    user: Pick<IUser, 'id' | 'permissions' | 'isAPI'>,
-    projectName: string,
-    condition: boolean,
-) => {
-    expect(await accessService.isProjectMember(user.id, projectName)).toBe(
-        condition,
-    );
-};
-
 const hasCommonProjectAccess = async (user, projectName, condition) => {
     const defaultEnv = 'default';
     const developmentEnv = 'development';
@@ -415,13 +405,6 @@ test('should grant user access to project', async () => {
 
     // // Should be able to update feature toggles inside the project
     await hasCommonProjectAccess(sUser, project, true);
-    await isProjectMember(sUser, project, true);
-    await isProjectMember(user, project, true);
-    // should list project members
-    expect(await accessService.getProjectMembers(project)).toStrictEqual([
-        { email: user.email, id: user.id, username: user.username },
-        { email: sUser.email, id: sUser.id, username: sUser.username },
-    ]);
 
     // Should not be able to admin the project itself.
     expect(
@@ -912,7 +895,6 @@ test('Should be allowed move feature toggle to project when given access through
     const projectRole = await accessService.getRoleByName(RoleName.MEMBER);
 
     await hasCommonProjectAccess(viewerUser, project.id, false);
-    await isProjectMember(viewerUser, project.id, false);
 
     await accessService.addGroupToRole(
         groupWithProjectAccess.id!,
@@ -922,7 +904,6 @@ test('Should be allowed move feature toggle to project when given access through
     );
 
     await hasCommonProjectAccess(viewerUser, project.id, true);
-    await isProjectMember(viewerUser, project.id, true);
 });
 
 test('Should not lose user role access when given permissions from a group', async () => {

src/test/e2e/services/project-service.e2e.test.ts

@@ -26,6 +26,16 @@ let featureToggleService: FeatureToggleService;
 let favoritesService: FavoritesService;
 let user;
 
+const isProjectUser = async (
+    userId: number,
+    projectName: string,
+    condition: boolean,
+) => {
+    expect(await projectService.isProjectUser(userId, projectName)).toBe(
+        condition,
+    );
+};
+
 beforeAll(async () => {
     db = await dbInit('project_service_serial', getLogger);
     stores = db.stores;
@@ -243,6 +253,8 @@ test('should get list of users with access to project', async () => {
     expect(users[0].name).toBe(user.name);
     expect(users[0].roleId).toBe(owner.id);
     expect(member).toBeTruthy();
+
+    await isProjectUser(users[0].id, project.id, true);
 });
 
 test('should add a member user to the project', async () => {
@@ -285,6 +297,19 @@ test('should add a member user to the project', async () => {
     expect(memberUsers[0].name).toBe(projectMember1.name);
     expect(memberUsers[1].id).toBe(projectMember2.id);
     expect(memberUsers[1].name).toBe(projectMember2.name);
+    expect(await projectService.getProjectUsers(project.id)).toStrictEqual([
+        { email: user.email, id: user.id, username: user.username },
+        {
+            email: projectMember1.email,
+            id: projectMember1.id,
+            username: projectMember1.username,
+        },
+        {
+            email: projectMember2.email,
+            id: projectMember2.id,
+            username: projectMember2.username,
+        },
+    ]);
 });
 
 test('should add admin users to the project', async () => {
@@ -328,6 +353,9 @@ test('should add admin users to the project', async () => {
     expect(adminUsers[1].name).toBe(projectAdmin1.name);
     expect(adminUsers[2].id).toBe(projectAdmin2.id);
     expect(adminUsers[2].name).toBe(projectAdmin2.name);
+    await isProjectUser(adminUsers[0].id, project.id, true);
+    await isProjectUser(adminUsers[1].id, project.id, true);
+    await isProjectUser(adminUsers[2].id, project.id, true);
 });
 
 test('add user should fail if user already have access', async () => {