Skip to content

Commit

Permalink
fix: changeRole to assign roles without existing members
Browse files Browse the repository at this point in the history
Co-authored-by: Christopher Kolstad <chriswk@getunleash.ai>
  • Loading branch information
ykhedher and Christopher Kolstad committed Mar 3, 2022
1 parent d264f30 commit d4521a1
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 3 deletions.
23 changes: 23 additions & 0 deletions 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;
}
}
2 changes: 2 additions & 0 deletions src/lib/routes/util.ts
Expand Up @@ -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();
Expand Down
6 changes: 3 additions & 3 deletions src/lib/services/project-service.ts
Expand Up @@ -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;

Expand Down Expand Up @@ -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();
}
}
}
Expand All @@ -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(
Expand All @@ -380,6 +379,7 @@ export default class ProjectService {
roleId,
projectId,
);
const role = await this.findProjectRole(projectId, roleId);

await this.eventStore.store(
new ProjectUserUpdateRoleEvent({
Expand Down
37 changes: 37 additions & 0 deletions src/test/e2e/services/project-service.e2e.test.ts
Expand Up @@ -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',
Expand Down

0 comments on commit d4521a1

Please sign in to comment.