Test fixes, as well as issue discovered by test around _id generation and event triggering.

packages/server/src/api/controllers/role.ts

@@ -55,11 +55,19 @@ export async function save(ctx: UserCtx) {
   const db = context.getAppDB()
   let { _id, name, inherits, permissionId, version } = ctx.request.body
   let isCreate = false
-  if (!_id || version === roles.RoleVersion.VERSION_2) {
+
+  if (_id && roles.isBuiltin(_id)) {
+    ctx.throw(400, "Cannot update builtin roles.")
+  }
+
+  // if not id found, then its creation
+  if (!_id) {
     _id = generateRoleID(name)
     isCreate = true
-  } else if (roles.isBuiltin(_id)) {
-    ctx.throw(400, "Cannot update builtin roles.")
+  }
+  // version 2 roles need updated to add back role_
+  else if (version === roles.RoleVersion.VERSION_2) {
+    _id = generateRoleID(name)
   }
 
   const role = new roles.Role(_id, name, permissionId).addInheritance(inherits)

packages/server/src/api/routes/tests/role.spec.js

@@ -29,10 +29,11 @@ describe("/roles", () => {
 
   describe("create", () => {
     it("returns a success message when role is successfully created", async () => {
-      const res = await createRole()
+      const role = basicRole()
+      const res = await createRole(role)
 
       expect(res.res.statusMessage).toEqual(
-        "Role 'NewRole' created successfully."
+        `Role '${role.name}' created successfully.`
       )
       expect(res.body._id).toBeDefined()
       expect(res.body._rev).toBeDefined()
@@ -44,12 +45,13 @@ describe("/roles", () => {
 
   describe("update", () => {
     it("updates a role", async () => {
-      let res = await createRole()
+      const role = basicRole()
+      let res = await createRole(role)
       jest.clearAllMocks()
       res = await createRole(res.body)
 
       expect(res.res.statusMessage).toEqual(
-        "Role 'NewRole' created successfully."
+        `Role '${role.name}' created successfully.`
       )
       expect(res.body._id).toBeDefined()
       expect(res.body._rev).toBeDefined()
@@ -86,7 +88,7 @@ describe("/roles", () => {
       expect(powerUserRole.inherits).toEqual(BUILTIN_ROLE_IDS.BASIC)
       expect(powerUserRole.permissionId).toEqual(BuiltinPermissionID.POWER)
 
-      const customRoleFetched = res.body.find(r => r._id === customRole._id)
+      const customRoleFetched = res.body.find(r => r._id === customRole.name)
       expect(customRoleFetched).toBeDefined()
       expect(customRoleFetched.inherits).toEqual(BUILTIN_ROLE_IDS.BASIC)
       expect(customRoleFetched.permissionId).toEqual(

packages/server/src/tests/utilities/structures.ts

@@ -266,6 +266,7 @@ export function basicRole() {
     name: `NewRole_${utils.newid()}`,
     inherits: roles.BUILTIN_ROLE_IDS.BASIC,
     permissionId: permissions.BuiltinPermissionID.READ_ONLY,
+    version: 2,
   }
 }