pr comments and updating group check to be more safe

packages/builder/src/pages/builder/portal/_layout.svelte

@@ -45,14 +45,6 @@
         },
       ])
     }
-    if (isEnabled(FEATURE_FLAGS.USER_GROUPS)) {
-      menu = menu.concat([
-        {
-          title: "User Groups",
-          href: "/builder/portal/manage/groups",
-        },
-      ])
-    }
 
     if (admin) {
       menu = menu.concat([
@@ -61,7 +53,10 @@
           href: "/builder/portal/manage/users",
           heading: "Manage",
         },
-
+        isEnabled(FEATURE_FLAGS.USER_GROUPS) && {
+          title: "User Groups",
+          href: "/builder/portal/manage/groups",
+        },
         { title: "Auth", href: "/builder/portal/manage/auth" },
         { title: "Email", href: "/builder/portal/manage/email" },
         {

packages/builder/src/pages/builder/portal/manage/users/_components/AddUserModal.svelte

@@ -6,6 +6,7 @@
     Multiselect,
     InputDropdown,
     Layout,
+    Icon,
   } from "@budibase/bbui"
   import { groups, auth } from "stores/portal"
   import { Constants } from "@budibase/frontend-core"
@@ -15,7 +16,7 @@
   const password = Math.random().toString(36).substring(2, 22)
   let disabled
   let userGroups = []
-
+  $: errors = []
   $: hasGroupsLicense = $auth.user?.license.features.includes(
     Constants.Features.USER_GROUPS
   )
@@ -28,6 +29,10 @@
       forceResetPassword: true,
     },
   ]
+
+  function removeInput(idx) {
+    userData = userData.filter((e, i) => i !== idx)
+  }
   function addNewInput() {
     userData = [
       ...userData,
@@ -40,9 +45,15 @@
     ]
   }
 
-  function validateInput(email) {
+  function validateInput(email, index) {
     if (email) {
-      return emailValidator(email) === true ? null : emailValidator(email)
+      if (emailValidator(email) === true) {
+        errors[index] = true
+        return null
+      } else {
+        errors[index] = false
+        return emailValidator(email)
+      }
     }
   }
 </script>
@@ -56,18 +67,40 @@
   confirmDisabled={disabled}
   cancelText="Cancel"
   showCloseIcon={false}
+  disabled={errors.some(x => x === false) ||
+    userData.some(x => x.email === "" || x.email === null)}
 >
   <Layout noPadding gap="XS">
     <Label>Email Address</Label>
 
     {#each userData as input, index}
-      <InputDropdown
+      <div
-        inputType="email"
+        style="display: flex;
-        bind:inputValue={input.email}
+        align-items: center;
-        bind:dropdownValue={input.role}
+        flex-direction: row;"
-        options={Constants.BbRoles}
+      >
-        error={validateInput(input.email)}
+        <div style="width: 90%">
-      />
+          <InputDropdown
+            inputType="email"
+            bind:inputValue={input.email}
+            bind:dropdownValue={input.role}
+            options={Constants.BbRoles}
+            error={validateInput(input.email, index)}
+          />
+        </div>
+        <div
+          class:fix-height={errors.length && !errors[index]}
+          class:normal-height={errors.length && !!errors[index]}
+          style="width: 10% "
+        >
+          <Icon
+            name="Close"
+            hoverable
+            size="S"
+            on:click={() => removeInput(index)}
+          />
+        </div>
+      </div>
     {/each}
     <div>
       <ActionButton on:click={addNewInput} icon="Add">Add email</ActionButton>
@@ -87,6 +120,14 @@
 </ModalContent>
 
 <style>
+  .fix-height {
+    margin-bottom: 5%;
+  }
+
+  .normal-height {
+    margin-bottom: 0%;
+  }
+
   :global(.spectrum-Picker) {
     border-top-left-radius: 0px;
   }

packages/server/src/utilities/global.js

@@ -32,8 +32,10 @@ exports.updateAppRole = (user, { appId } = {}) => {
   // if a role wasn't found then either set as admin (builder) or public (everyone else)
   if (!user.roleId && user.builder && user.builder.global) {
     user.roleId = BUILTIN_ROLE_IDS.ADMIN
-  } else if (!user.roleId) {
+  } else if (!user.roleId && !user?.userGroups?.length) {
     user.roleId = BUILTIN_ROLE_IDS.PUBLIC
+  } else if (user?.userGroups?.length) {
+    user.roleId = null
   }
 
   delete user.roles
@@ -41,10 +43,8 @@ exports.updateAppRole = (user, { appId } = {}) => {
 }
 
 async function checkGroupRoles(user, { appId } = {}) {
-  if (!user.roleId) {
+  let roleId = await groups.getGroupRoleId(user, appId)
-    let roleId = await groups.getGroupRoleId(user, appId)
+  user.roleId = roleId
-    user.roleId = roleId
-  }
 
   return user
 }
@@ -54,7 +54,7 @@ async function processUser(user, { appId } = {}) {
     delete user.password
   }
   user = await exports.updateAppRole(user, { appId })
-  if (user?.userGroups?.length) {
+  if (!user.roleId && user?.userGroups?.length) {
     user = await checkGroupRoles(user, { appId })
   }
 

packages/worker/src/sdk/users/users.ts

@@ -31,9 +31,7 @@ export const allUsers = async () => {
 }
 
 export const countUsersByApp = async (appId: string) => {
-  let response: any = await usersCore.searchGlobalUsersByApp(appId, {
+  let response: any = await usersCore.searchGlobalUsersByApp(appId)
-    include_docs: true,
-  })
   return {
     userCount: response.length,
   }
@@ -65,7 +63,7 @@ export const paginatedUsers = async ({
     userList = await usersCore.searchGlobalUsersByEmail(email, opts)
     property = "email"
   } else {
-    // no search, query allDcso
+    // no search, query allDocs
     const response = await db.allDocs(dbUtils.getGlobalUserParams(null, opts))
     userList = response.rows.map((row: any) => row.doc)
   }