fix: fetching user root roles include custom ones #4068
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
| @@ -45,7 +45,7 @@ export const createFakeAccessService = ( | |||
| const accountStore = new FakeAccountStore(); | |||
| const roleStore = new FakeRoleStore(); | |||
| const environmentStore = new FakeEnvironmentStore(); | |||
| const accessStore = new FakeAccessStore(); | |||
| const accessStore = new FakeAccessStore(roleStore); | |||
There was a problem hiding this comment.
@kwasniew I'm reusing your createAccessService method which simplifies the test setup, but note I'm explicitly adding this dependency because access-store directly queries roles table:
unleash/src/lib/db/access-store.ts
Line 23 in 0457ae4
To simulate this DB coupling I'm adding this dependency.
I couldn't find a better way which is why I'm tagging you in case you have some ideas
src/lib/services/access-service.ts
Outdated
| @@ -275,7 +275,9 @@ export class AccessService { | |||
|
|
|||
| async getUserRootRoles(userId: number): Promise<IRoleWithProject[]> { | |||
| const userRoles = await this.store.getRolesForUserId(userId); | |||
| return userRoles.filter((r) => r.type === RoleType.ROOT); | |||
| return userRoles.filter( | |||
| (r) => r.type === RoleType.ROOT || r.type === RoleType.ROOT_CUSTOM, | |||
There was a problem hiding this comment.
This is the fix
There was a problem hiding this comment.
Maybe consider ({type}) => ROOT_ROLE_TYPES.includes(type) in case it's cleaner.
There was a problem hiding this comment.
Addressed in 9956e9e
| name: string; | ||
| /** | ||
| * Extends IRole making description mandatory | ||
| */ |
There was a problem hiding this comment.
I would still like to drop ICustomRole and only have IRole. Description is already required in the frontend interface frontend/src/interfaces/role.ts and AFAIK there's always a description.
Could be out of scope for this PR and maybe belong in a separate one though.
There was a problem hiding this comment.
👍 but I thought chasing down why the description was optional required a deeper investigation and analyze potential backward incompatibility problems as I see that some project role in our sandbox instance were created without a description
Co-authored-by: Nuno Góis <github@nunogois.com>
About the changes
getUserRootRolesshould also consider custom root rolesThis introduces test cases that unveiled a dependency between stores (this happens actually at the DB layer having access-service access tables from two different stores but skipping the store layer).
https://linear.app/unleash/issue/2-1161/a-user-with-custom-root-role-and-permission-to-create-client-api