Adri PR comments.

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

@@ -72,9 +72,8 @@ export async function find(ctx: UserCtx<void, FindRoleResponse>) {
   const role = await roles.getRole(ctx.params.roleId)
   if (!role) {
     ctx.throw(404, { message: "Role not found" })
-  } else {
-    ctx.body = externalRole(role)
   }
+  ctx.body = externalRole(role)
 }
 
 export async function save(ctx: UserCtx<SaveRoleRequest, SaveRoleResponse>) {
@@ -82,6 +81,9 @@ export async function save(ctx: UserCtx<SaveRoleRequest, SaveRoleResponse>) {
   let { _id, name, inherits, permissionId, version, uiMetadata } =
     ctx.request.body
   let isCreate = false
+  if (!ctx.request.body._rev && !version) {
+    version = roles.RoleIDVersion.NAME
+  }
   const isNewVersion = version === roles.RoleIDVersion.NAME
 
   if (_id && roles.isBuiltin(_id)) {

packages/server/src/api/routes/tests/permissions.spec.ts

@@ -328,7 +328,7 @@ describe("/permission", () => {
     })
 
     it("should be unable to search for table 2 using role 1", async () => {
-      await config.setRole(role1._id!, async () => {
+      await config.loginAsRole(role1._id!, async () => {
         const response2 = await config.api.row.search(
           table2._id!,
           {
@@ -347,7 +347,7 @@ describe("/permission", () => {
         inherits: [role1._id!, role2._id!],
       })
 
-      await config.setRole(role3._id!, async () => {
+      await config.loginAsRole(role3._id!, async () => {
         const response1 = await config.api.row.search(
           table1._id!,
           {

packages/server/src/api/routes/tests/screen.spec.ts

@@ -79,7 +79,7 @@ describe("/screens", () => {
     })
 
     async function checkScreens(roleId: string, screenIds: string[]) {
-      await config.setRole(roleId, async () => {
+      await config.loginAsRole(roleId, async () => {
         const res = await config.api.application.getDefinition(
           config.prodAppId!,
           {

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

@@ -429,7 +429,7 @@ export default class TestConfiguration {
   // HEADERS
 
   // sets the role for the headers, for the period of a callback
-  async setRole(roleId: string, cb: () => Promise<unknown>) {
+  async loginAsRole(roleId: string, cb: () => Promise<unknown>) {
     const roleUser = await this.createUser({
       roles: {
         [this.prodAppId!]: roleId,

packages/server/src/tests/utilities/api/role.ts

@@ -22,10 +22,6 @@ export class RoleAPI extends TestAPI {
   }
 
   save = async (body: SaveRoleRequest, expectations?: Expectations) => {
-    // the tests should always be creating the "new" version of roles
-    if (body.version === undefined) {
-      body.version = "name"
-    }
     return await this._post<SaveRoleResponse>(`/api/roles`, {
       body,
       expectations,