Further PR comments.

packages/backend-core/src/docIds/ids.ts

@@ -89,6 +89,13 @@ export function generateRoleID(name: string) {
   return `${prefix}${name}`
 }
 
+/**
+ * Utility function to be more verbose.
+ */
+export function prefixRoleID(name: string) {
+  return generateRoleID(name)
+}
+
 /**
  * Generates a new dev info document ID - this is scoped to a user.
  * @returns {string} The new dev info ID which info for dev (like api key) can be stored under.

packages/backend-core/src/security/roles.ts

@@ -1,5 +1,5 @@
 import { BuiltinPermissionID, PermissionLevel } from "./permissions"
-import { generateRoleID, getRoleParams, DocumentType, SEPARATOR } from "../db"
+import { prefixRoleID, getRoleParams, DocumentType, SEPARATOR } from "../db"
 import { getAppDB } from "../context"
 import { doWithDB } from "../db"
 import { Screen, Role as RoleDoc } from "@budibase/types"
@@ -169,7 +169,7 @@ export async function getRole(
     )
   } else {
     // make sure has the prefix (if it has it then it won't be added)
-    roleId = generateRoleID(roleId)
+    roleId = prefixRoleID(roleId)
   }
   try {
     const db = getAppDB()
@@ -402,7 +402,7 @@ export function getDBRoleID(roleName: string) {
   if (roleName?.startsWith(DocumentType.ROLE)) {
     return roleName
   }
-  return generateRoleID(roleName)
+  return prefixRoleID(roleName)
 }
 
 /**

packages/builder/src/components/backend/DataTable/modals/EditRoles.svelte

@@ -109,8 +109,6 @@
       return "Select a unique role name."
     } else if (invalidRoleName) {
       return "Please enter a role name consisting of only alphanumeric symbols and underscores"
-    } else {
-      return null
     }
   }
 

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

@@ -29,10 +29,10 @@ async function updateRolesOnUserTable(
         roleVersion === roles.RoleIDVersion.NAME
           ? roles.getExternalRoleID(roleId, roleVersion)
           : roleId
-      const indexOf = constraints.inclusion.indexOf(updatedRoleId)
-      if (remove && indexOf !== -1) {
-        constraints.inclusion.splice(indexOf, 1)
-      } else if (!remove && indexOf === -1) {
+      const indexOfRoleId = constraints.inclusion.indexOf(updatedRoleId)
+      if (remove && indexOfRoleId !== -1) {
+        constraints.inclusion.splice(indexOfRoleId, 1)
+      } else if (!remove && indexOfRoleId === -1) {
         constraints.inclusion.push(updatedRoleId)
       }
       break