chore: Small code cleanups

src/lib/db/access-store.ts

@@ -224,10 +224,10 @@ export class AccessStore implements IAccessStore {
         return rows.map((r) => r.user_id);
     }
 
-    async addUserToProjectRole(
+    async addUserToRole(
         userId: number,
         roleId: number,
-        projecId: string,
+        projecId?: string,
     ): Promise<void> {
         return this.db(T.ROLE_USER).insert({
             user_id: userId,
@@ -236,10 +236,10 @@ export class AccessStore implements IAccessStore {
         });
     }
 
-    async removeUserFromProjectRole(
+    async removeUserFromRole(
         userId: number,
         roleId: number,
-        projectId: string,
+        projectId?: string,
     ): Promise<void> {
         return this.db(T.ROLE_USER)
             .where({

src/lib/services/access-service.ts

@@ -170,12 +170,12 @@ export class AccessService {
         };
     }
 
-    async addUserToProjectRole(
+    async addUserToRole(
         userId: number,
         roleId: number,
         projectId: string,
     ): Promise<void> {
-        return this.store.addUserToProjectRole(userId, roleId, projectId);
+        return this.store.addUserToRole(userId, roleId, projectId);
     }
 
     async getRoleByName(roleName: string): Promise<IRole> {
@@ -194,7 +194,7 @@ export class AccessService {
                     RoleType.ROOT,
                 );
 
-                await this.store.addUserToProjectRole(
+                await this.store.addUserToRole(
                     userId,
                     newRootRole.id,
                     ALL_PROJECTS,
@@ -214,14 +214,15 @@ export class AccessService {
         return userRoles.filter((r) => r.type === RoleType.ROOT);
     }
 
-    async removeUserFromProjectRole(
+    async removeUserFromRole(
         userId: number,
         roleId: number,
         projectId: string,
     ): Promise<void> {
-        return this.store.removeUserFromProjectRole(userId, roleId, projectId);
+        return this.store.removeUserFromRole(userId, roleId, projectId);
     }
 
+    //This actually only exists for testing purposes
     async addPermissionToRole(
         roleId: number,
         permission: string,
@@ -239,6 +240,7 @@ export class AccessService {
         );
     }
 
+    //This actually only exists for testing purposes
     async removePermissionFromRole(
         roleId: number,
         permission: string,
@@ -324,11 +326,11 @@ export class AccessService {
 
         const users = await Promise.all(
             roles.map(async (role) => {
-                const usrs = await this.getProjectUsersForRole(
+                const projectUsers = await this.getProjectUsersForRole(
                     role.id,
                     projectId,
                 );
-                return usrs.map((u) => ({ ...u, roleId: role.id }));
+                return projectUsers.map((u) => ({ ...u, roleId: role.id }));
             }),
         );
         return [roles, users.flat()];
@@ -349,11 +351,7 @@ export class AccessService {
             this.logger.info(
                 `Making ${owner.id} admin of ${projectId} via roleId=${ownerRole.id}`,
             );
-            await this.store.addUserToProjectRole(
-                owner.id,
-                ownerRole.id,
-                projectId,
-            );
+            await this.store.addUserToRole(owner.id, ownerRole.id, projectId);
         }
     }
 

src/lib/services/project-service.ts

@@ -300,11 +300,7 @@ export default class ProjectService {
             throw new Error(`User already has access to project=${projectId}`);
         }
 
-        await this.accessService.addUserToProjectRole(
-            userId,
-            role.id,
-            projectId,
-        );
+        await this.accessService.addUserToRole(userId, role.id, projectId);
     }
 
     // TODO: should be an event too
@@ -328,7 +324,7 @@ export default class ProjectService {
             }
         }
 
-        await this.accessService.removeUserFromProjectRole(
+        await this.accessService.removeUserFromRole(
             userId,
             role.id,
             projectId,

src/lib/types/stores/access-store.ts

@@ -41,15 +41,15 @@ export interface IAccessStore extends Store<IRole, number> {
         role_id: number,
         permissions: IPermission[],
     ): Promise<void>;
-    addUserToProjectRole(
+    addUserToRole(
         userId: number,
         roleId: number,
-        projectId: string,
+        projectId?: string,
     ): Promise<void>;
-    removeUserFromProjectRole(
+    removeUserFromRole(
         userId: number,
         roleId: number,
-        projectId: string,
+        projectId?: string,
     ): Promise<void>;
     removeRolesOfTypeForUser(userId: number, roleType: string): Promise<void>;
     addPermissionsToRole(

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

@@ -28,14 +28,14 @@ let readRole;
 const createUserEditorAccess = async (name, email) => {
     const { userStore } = stores;
     const user = await userStore.insert({ name, email });
-    await accessService.addUserToProjectRole(user.id, editorRole.id, 'default');
+    await accessService.addUserToRole(user.id, editorRole.id, 'default');
     return user;
 };
 
 const createUserViewerAccess = async (name, email) => {
     const { userStore } = stores;
     const user = await userStore.insert({ name, email });
-    await accessService.addUserToProjectRole(
+    await accessService.addUserToRole(
         user.id,
         readRole.id,
         ALL_PROJECTS,
@@ -182,7 +182,7 @@ const createSuperUser = async () => {
         name: 'Alice Admin',
         email: 'admin@getunleash.io',
     });
-    await accessService.addUserToProjectRole(
+    await accessService.addUserToRole(
         user.id,
         adminRole.id,
         ALL_PROJECTS,
@@ -380,7 +380,7 @@ test('should grant user access to project', async () => {
     await accessService.createDefaultProjectRoles(user, project);
 
     const projectRole = await accessService.getRoleByName(RoleName.MEMBER);
-    await accessService.addUserToProjectRole(sUser.id, projectRole.id, project);
+    await accessService.addUserToRole(sUser.id, projectRole.id, project);
 
     // // Should be able to update feature toggles inside the project
     hasCommonProjectAccess(sUser, project, true);
@@ -405,7 +405,7 @@ test('should not get access if not specifying project', async () => {
 
     const projectRole = await accessService.getRoleByName(RoleName.MEMBER);
 
-    await accessService.addUserToProjectRole(sUser.id, projectRole.id, project);
+    await accessService.addUserToRole(sUser.id, projectRole.id, project);
 
     // Should not be able to update feature toggles outside project
     hasCommonProjectAccess(sUser, undefined, false);
@@ -418,14 +418,14 @@ test('should remove user from role', async () => {
         email: 'random123@getunleash.io',
     });
 
-    await accessService.addUserToProjectRole(user.id, editorRole.id, 'default');
+    await accessService.addUserToRole(user.id, editorRole.id, 'default');
 
     // check user has one role
     const userRoles = await accessService.getRolesForUser(user.id);
     expect(userRoles.length).toBe(1);
     expect(userRoles[0].name).toBe(RoleName.EDITOR);
 
-    await accessService.removeUserFromProjectRole(
+    await accessService.removeUserFromRole(
         user.id,
         editorRole.id,
         'default',
@@ -441,7 +441,7 @@ test('should return role with users', async () => {
         email: 'random2223@getunleash.io',
     });
 
-    await accessService.addUserToProjectRole(user.id, editorRole.id, 'default');
+    await accessService.addUserToRole(user.id, editorRole.id, 'default');
 
     const roleWithUsers = await accessService.getRoleData(editorRole.id);
     expect(roleWithUsers.role.name).toBe(RoleName.EDITOR);
@@ -459,7 +459,7 @@ test('should return role with permissions and users', async () => {
         email: 'random2244@getunleash.io',
     });
 
-    await accessService.addUserToProjectRole(user.id, editorRole.id, 'default');
+    await accessService.addUserToRole(user.id, editorRole.id, 'default');
 
     const roleWithPermission = await accessService.getRoleData(editorRole.id);
 
@@ -548,11 +548,7 @@ test('should support permission with "ALL" environment requirement', async () =>
         [CREATE_FEATURE_STRATEGY],
         'production',
     );
-    await accessStore.addUserToProjectRole(
-        user.id,
-        customRole.id,
-        ALL_PROJECTS,
-    );
+    await accessStore.addUserToRole(user.id, customRole.id, ALL_PROJECTS);
 
     const hasAccess = await accessService.hasPermission(
         user,

src/test/fixtures/access-service-mock.ts

@@ -35,7 +35,7 @@ class AccessServiceMock extends AccessService {
         throw new Error('Method not implemented.');
     }
 
-    addUserToProjectRole(userId: number, roleId: number): Promise<void> {
+    addUserToRole(userId: number, roleId: number): Promise<void> {
         throw new Error('Method not implemented.');
     }
 

src/test/fixtures/fake-access-store.ts

@@ -9,7 +9,7 @@ import {
 import { IAvailablePermissions, IPermission } from 'lib/types/model';
 
 class AccessStoreMock implements IAccessStore {
-    removeUserFromProjectRole(
+    removeUserFromRole(
         userId: number,
         roleId: number,
         projectId: string,
@@ -87,7 +87,7 @@ class AccessStoreMock implements IAccessStore {
         throw new Error('Method not implemented.');
     }
 
-    addUserToProjectRole(userId: number, roleId: number): Promise<void> {
+    addUserToRole(userId: number, roleId: number): Promise<void> {
         throw new Error('Method not implemented.');
     }