From d4521a1c0cc721f0911cad8aa9b8c51803561914 Mon Sep 17 00:00:00 2001
From: Youssef <khedher.youssef@hotmail.fr>
Date: Thu, 3 Mar 2022 14:25:14 +0100
Subject: [PATCH] fix: changeRole to assign roles without existing members

Co-authored-by: Christopher Kolstad <chriswk@getunleash.ai>
---
 src/lib/error/project-without-owner-error.ts  | 23 ++++++++++++
 src/lib/routes/util.ts                        |  2 +
 src/lib/services/project-service.ts           |  6 +--
 .../e2e/services/project-service.e2e.test.ts  | 37 +++++++++++++++++++
 4 files changed, 65 insertions(+), 3 deletions(-)
 create mode 100644 src/lib/error/project-without-owner-error.ts

diff --git a/src/lib/error/project-without-owner-error.ts b/src/lib/error/project-without-owner-error.ts
new file mode 100644
index 00000000000..ae9bb6fb092
--- /dev/null
+++ b/src/lib/error/project-without-owner-error.ts
@@ -0,0 +1,23 @@
+export default class ProjectWithoutOwnerError extends Error {
+    constructor() {
+        super();
+        Error.captureStackTrace(this, this.constructor);
+
+        this.name = this.constructor.name;
+        this.message = 'A project must have at least one owner';
+    }
+
+    toJSON(): any {
+        const obj = {
+            isJoi: true,
+            name: this.constructor.name,
+            details: [
+                {
+                    validationErrors: [],
+                    message: this.message,
+                },
+            ],
+        };
+        return obj;
+    }
+}
diff --git a/src/lib/routes/util.ts b/src/lib/routes/util.ts
index 07798cbd208..0b994fa5fa4 100644
--- a/src/lib/routes/util.ts
+++ b/src/lib/routes/util.ts
@@ -66,6 +66,8 @@ export const handleErrors: (
             return res.status(409).json(error).end();
         case 'RoleInUseError':
             return res.status(400).json(error).end();
+        case 'ProjectWithoutOwnerError':
+            return res.status(409).json(error).end();
         default:
             logger.error('Server failed executing request', error);
             return res.status(500).end();
diff --git a/src/lib/services/project-service.ts b/src/lib/services/project-service.ts
index 8281a0fb2e1..5208fb04aa3 100644
--- a/src/lib/services/project-service.ts
+++ b/src/lib/services/project-service.ts
@@ -36,6 +36,7 @@ import NoAccessError from '../error/no-access-error';
 import IncompatibleProjectError from '../error/incompatible-project-error';
 import { DEFAULT_PROJECT } from '../types/project';
 import { IFeatureTagStore } from 'lib/types/stores/feature-tag-store';
+import ProjectWithoutOwnerError from '../error/project-without-owner-error';
 
 const getCreatedBy = (user: User) => user.email || user.username;
 
@@ -349,7 +350,7 @@ export default class ProjectService {
                 projectId,
             );
             if (users.length < 2) {
-                throw new Error('A project must have at least one owner');
+                throw new ProjectWithoutOwnerError();
             }
         }
     }
@@ -360,8 +361,6 @@ export default class ProjectService {
         userId: number,
         createdBy: string,
     ): Promise<void> {
-        const role = await this.findProjectRole(projectId, roleId);
-
         const usersWithRoles = await this.getUsersWithAccess(projectId);
         const user = usersWithRoles.users.find((u) => u.id === userId);
         const currentRole = usersWithRoles.roles.find(
@@ -380,6 +379,7 @@ export default class ProjectService {
             roleId,
             projectId,
         );
+        const role = await this.findProjectRole(projectId, roleId);
 
         await this.eventStore.store(
             new ProjectUserUpdateRoleEvent({
diff --git a/src/test/e2e/services/project-service.e2e.test.ts b/src/test/e2e/services/project-service.e2e.test.ts
index 2f72fc3e1bc..9a13ccf49e5 100644
--- a/src/test/e2e/services/project-service.e2e.test.ts
+++ b/src/test/e2e/services/project-service.e2e.test.ts
@@ -685,6 +685,43 @@ test('should update role for user on project', async () => {
     expect(ownerUsers).toHaveLength(2);
 });
 
+test('should able to assign role without existing members', async () => {
+    const project = {
+        id: 'update-users-test',
+        name: 'New project',
+        description: 'Blah',
+    };
+    await projectService.createProject(project, user);
+
+    const projectMember1 = await stores.userStore.insert({
+        name: 'Some Member',
+        email: 'update1999@getunleash.io',
+    });
+
+    const testRole = await stores.roleStore.create({
+        name: 'Power user',
+        roleType: 'custom',
+        description: 'Grants access to modify all environments',
+    });
+
+    const memberRole = await stores.roleStore.getRoleByName(RoleName.MEMBER);
+
+    await projectService.addUser(project.id, memberRole.id, projectMember1.id);
+    await projectService.changeRole(
+        project.id,
+        testRole.id,
+        projectMember1.id,
+        'test',
+    );
+
+    const { users } = await projectService.getUsersWithAccess(project.id, user);
+    const memberUsers = users.filter((user) => user.roleId === memberRole.id);
+    const testUsers = users.filter((user) => user.roleId === testRole.id);
+
+    expect(memberUsers).toHaveLength(0);
+    expect(testUsers).toHaveLength(1);
+});
+
 test('should not update role for user on project when she is the owner', async () => {
     const project = {
         id: 'update-users-not-allowed',