adding tests and pr comments

packages/backend-core/src/events/publishers/group.ts

@@ -32,9 +32,9 @@ export async function deleted(group: UserGroup) {
   await publishEvent(Event.USER_GROUP_DELETED, properties)
 }
 
-export async function usersAdded(emails: string[], group: UserGroup) {
+export async function usersAdded(count: number, group: UserGroup) {
   const properties: GroupUsersAddedEvent = {
-    count: emails.length,
+    count,
     groupId: group._id as string,
   }
   await publishEvent(Event.USER_GROUP_USERS_ADDED, properties)
@@ -57,6 +57,8 @@ export async function createdOnboarding(groupId: string) {
 }
 
 export async function permissionsEdited(roles: UserGroupRoles) {
-  const properties: UserGroupRoles = roles
+  const properties: UserGroupRoles = {
+    ...roles,
+  }
   await publishEvent(Event.USER_GROUP_PERMISSIONS_EDITED, properties)
 }

packages/backend-core/tests/utilities/mocks/events.js

@@ -89,6 +89,14 @@ jest.spyOn(events.user, "passwordUpdated")
 jest.spyOn(events.user, "passwordResetRequested")
 jest.spyOn(events.user, "passwordReset")
 
+jest.spyOn(events.group, "created")
+jest.spyOn(events.group, "updated")
+jest.spyOn(events.group, "deleted")
+jest.spyOn(events.group, "usersAdded")
+jest.spyOn(events.group, "usersDeleted")
+jest.spyOn(events.group, "createdOnboarding")
+jest.spyOn(events.group, "permissionsEdited")
+
 jest.spyOn(events.serve, "servedBuilder")
 jest.spyOn(events.serve, "servedApp")
 jest.spyOn(events.serve, "servedAppPreview")

packages/builder/src/pages/builder/portal/manage/groups/[groupId].svelte

@@ -39,12 +39,19 @@
 
   async function selectUser(id) {
     let selectedUser = selectedUsers.includes(id)
-    let enrichedUser = $users.data.find(user => user._id === id)
     if (selectedUser) {
       selectedUsers = selectedUsers.filter(id => id !== selectedUser)
       let newUsers = group.users.filter(user => user._id !== id)
       group.users = newUsers
     } else {
+      let enrichedUser = $users.data
+        .filter(user => user._id === id)
+        .map(u => {
+          return {
+            _id: u._id,
+            email: u.email,
+          }
+        })[0]
       selectedUsers = [...selectedUsers, id]
       group.users.push(enrichedUser)
     }
@@ -64,6 +71,7 @@
     $users.data?.filter(x => !group?.users.map(y => y._id).includes(x._id)) ||
     []
 
+  $: groupApps = $apps.filter(x => group.apps.includes(x.appId))
   async function removeUser(id) {
     let newUsers = group.users.filter(user => user._id !== id)
     group.users = newUsers
@@ -142,7 +150,7 @@
   <List>
     {#if group?.users.length}
       {#each group.users as user}
-        <ListItem subtitle={user?.access} title={user?.email} avatar
+        <ListItem title={user?.email} avatar
           ><Icon
             on:click={() => removeUser(user?._id)}
             hoverable
@@ -167,8 +175,8 @@
   </div>
 
   <List>
-    {#if group?.apps.length}
-      {#each group.apps as app}
+    {#if groupApps.length}
+      {#each groupApps as app}
         <ListItem
           title={app.name}
           icon={app?.icon?.name || "Apps"}

packages/builder/src/pages/builder/portal/overview/_components/AccessTab.svelte

@@ -44,9 +44,7 @@
       })
     }) || []
   $: appGroups = $groups.filter(x => {
-    return x.apps.find(y => {
-      return y.appId === app.appId
-    })
+    return x.apps.includes(app.appId)
   })
 
   async function addData(appData) {
@@ -57,7 +55,7 @@
         let matchedGroup = $groups.find(group => {
           return group._id === data.id
         })
-        matchedGroup.apps.push(app)
+        matchedGroup.apps.push(app.appId)
         matchedGroup.roles[fixedAppId] = data.role
 
         groups.actions.save(matchedGroup)

packages/server/src/utilities/global.js

@@ -106,7 +106,6 @@ exports.getGlobalUsers = async (users = null) => {
   if (!appId) {
     return globalUsers
   }
-  console.log("maybe??")
 
   return globalUsers.map(user => exports.updateAppRole(user))
 }

packages/types/src/documents/global/user.ts

@@ -13,7 +13,6 @@ export interface User extends Document {
   providerType?: string
   password?: string
   status?: string
-
   createdAt?: number // override the default createdAt behaviour - users sdk historically set this to Date.now()
   userGroups?: string[]
 }

packages/types/src/documents/global/userGroup.ts

@@ -4,12 +4,16 @@ export interface UserGroup extends Document {
   name: string
   icon: string
   color: string
-  users: User[]
-  apps: any[]
+  users: groupUser[]
+  apps: string[]
   roles: UserGroupRoles
   createdAt?: number
 }
 
+export interface groupUser {
+  _id: string
+  email: string[]
+}
 export interface UserGroupRoles {
   [key: string]: string
 }

packages/types/src/sdk/events/event.ts

@@ -156,9 +156,9 @@ export enum Event {
   USER_GROUP_UPDATED = "user_group:updated",
   USER_GROUP_DELETED = "user_group:deleted",
   USER_GROUP_USERS_ADDED = "user_group:user_added",
-  USER_GROUP_USERS_REMOVED = "user_group_:users_deleted",
-  USER_GROUP_PERMISSIONS_EDITED = "user_group_:permissions_edited",
-  USER_GROUP_ONBOARDING = "user_group_:onboarding_added",
+  USER_GROUP_USERS_REMOVED = "user_group:users_deleted",
+  USER_GROUP_PERMISSIONS_EDITED = "user_group:permissions_edited",
+  USER_GROUP_ONBOARDING = "user_group:onboarding_added",
 }
 
 // properties added at the final stage of the event pipeline

packages/worker/src/api/controllers/global/users.ts

@@ -127,8 +127,7 @@ export const destroy = async (ctx: any) => {
 export const bulkDelete = async (ctx: any) => {
   const { userIds } = ctx.request.body
   try {
-    let { groupsToModify, usersResponse } = await users.bulkDelete(userIds)
-    await groupUtils.bulkDeleteGroupUsers(groupsToModify)
+    let usersResponse = await users.bulkDelete(userIds)
 
     ctx.body = {
       message: `${usersResponse.length} user(s) deleted`,

packages/worker/src/api/routes/tests/groups.spec.js

@@ -0,0 +1,95 @@
+const { config, request, structures } = require("../../../tests")
+const { events } = require("@budibase/backend-core")
+describe("/api/global/groups", () => {
+
+    beforeAll(async () => {
+        await config.beforeAll()
+    })
+
+    afterAll(async () => {
+        await config.afterAll()
+    })
+
+    const createGroup = async (group) => {
+        const existing = await config.getGroup(group.name)
+        if (existing) {
+            await deleteGroup(existing._id)
+        }
+        return config.saveGroup(group)
+    }
+
+    const updateGroup = async (group) => {
+        const existing = await config.getGroup(group._id)
+        group._id = existing._id
+        return config.saveGroup(group)
+    }
+
+
+    const deleteGroup = async (group) => {
+        const oldGroup = await config.getGroup(group._id)
+        if (oldGroup) {
+            await request
+                .delete(`/api/global/users/${oldGroup._id}`)
+                .set(config.defaultHeaders())
+                .expect("Content-Type", /json/)
+                .expect(200)
+        }
+    }
+
+
+
+    describe("create", () => {
+        it("should be able to create a basic group", async () => {
+            jest.clearAllMocks()
+            const group = structures.groups.UserGroup()
+            await createGroup(group)
+
+            expect(events.group.created).toBeCalledTimes(1)
+            expect(events.group.updated).not.toBeCalled()
+            expect(events.group.permissionsEdited).not.toBeCalled()
+        })
+    })
+    describe("update", () => {
+        it("should be able to update a basic group", async () => {
+            jest.clearAllMocks()
+            const group = structures.groups.UserGroup()
+            let oldGroup = await createGroup(group)
+
+            let groupToSend = {
+                ...group,
+                ...oldGroup,
+                name: "New Name"
+            }
+            await updateGroup(groupToSend)
+
+            expect(events.group.updated).toBeCalledTimes(1)
+            expect(events.group.permissionsEdited).not.toBeCalled()
+        })
+        it("should be able to update permissions on a group", async () => {
+            jest.clearAllMocks()
+            const group = structures.groups.UserGroup()
+            let oldGroup = await createGroup(group)
+
+            let groupToSend = {
+                ...group,
+                ...oldGroup,
+                roles: { app_1234345345: "BASIC" }
+            }
+            await updateGroup(groupToSend)
+
+            expect(events.group.updated).toBeCalledTimes(1)
+            expect(events.group.permissionsEdited).toBeCalledTimes(1)
+        })
+    })
+
+    describe("destroy", () => {
+        it("should be able to destroy a basic group", async () => {
+            const group = structures.groups.UserGroup()
+            let oldGroup = await createGroup(group)
+            jest.clearAllMocks()
+            await deleteGroup(oldGroup)
+
+            expect(events.user.deleted).toBeCalledTimes(1)
+        })
+    })
+})

packages/worker/src/api/routes/tests/users.spec.js

@@ -1,5 +1,6 @@
 jest.mock("nodemailer")
 const { config, request, mocks, structures } = require("../../../tests")
+const { cr } = require("./groups.spec")
 const sendMailMock = mocks.email.mock()
 const { events } = require("@budibase/backend-core")
 describe("/api/global/users", () => {
@@ -83,6 +84,29 @@ describe("/api/global/users", () => {
     return res.body
   }
 
+
+  const bulkCreateUsers = async (users) => {
+    const res = await request
+      .post(`/api/global/users/bulkCreate`)
+      .send(users)
+      .set(config.defaultHeaders())
+      .expect("Content-Type", /json/)
+      .expect(200)
+    return res.body
+  }
+
+  const bulkDeleteUsers = async (users) => {
+    const res = await request
+      .post(`/api/global/users/bulkDelete`)
+      .send(users)
+      .set(config.defaultHeaders())
+      .expect("Content-Type", /json/)
+      .expect(200)
+    return res.body
+  }
+
+
+
   const deleteUser = async (email) => {
     const user = await config.getUser(email)
     if (user) {
@@ -106,6 +130,21 @@ describe("/api/global/users", () => {
       expect(events.user.permissionAdminAssigned).not.toBeCalled()
     })
 
+    it("should be able to bulkCreate users with different permissions", async () => {
+      jest.clearAllMocks()
+      const builder = structures.users.builderUser({ email: "basic@test.com" })
+      const admin = structures.users.adminUser({ email: "admin@test.com" })
+      const user = structures.users.user({ email: "user@test.com" })
+
+      let toCreate = { users: [builder, admin, user], groups: [] }
+      await bulkCreateUsers(toCreate)
+
+      expect(events.user.created).toBeCalledTimes(3)
+      expect(events.user.permissionAdminAssigned).toBeCalledTimes(1)
+      expect(events.user.permissionBuilderAssigned).toBeCalledTimes(1)
+    })
+
+
     it("should be able to create an admin user", async () => {
       jest.clearAllMocks()
       const user = structures.users.adminUser({ email: "admin@test.com" })
@@ -117,6 +156,18 @@ describe("/api/global/users", () => {
       expect(events.user.permissionAdminAssigned).toBeCalledTimes(1)
     })
 
+    it("should be able to create an admin user", async () => {
+      jest.clearAllMocks()
+      const user = structures.users.adminUser({ email: "admin@test.com" })
+      await createUser(user)
+
+      expect(events.user.created).toBeCalledTimes(1)
+      expect(events.user.updated).not.toBeCalled()
+      expect(events.user.permissionBuilderAssigned).not.toBeCalled()
+      expect(events.user.permissionAdminAssigned).toBeCalledTimes(1)
+    })
+
+
     it("should be able to create a builder user", async () => {
       jest.clearAllMocks()
       const user = structures.users.builderUser({ email: "builder@test.com" })
@@ -332,5 +383,21 @@ describe("/api/global/users", () => {
       expect(events.user.permissionBuilderRemoved).toBeCalledTimes(1)
       expect(events.user.permissionAdminRemoved).not.toBeCalled()
     })
+
+    it("should be able to bulk delete users with different permissions", async () => {
+      jest.clearAllMocks()
+      const builder = structures.users.builderUser({ email: "basic@test.com" })
+      const admin = structures.users.adminUser({ email: "admin@test.com" })
+      const user = structures.users.user({ email: "user@test.com" })
+
+      let toCreate = { users: [builder, admin, user], groups: [] }
+      let createdUsers = await bulkCreateUsers(toCreate)
+      await bulkDeleteUsers({ userIds: [createdUsers[0]._id, createdUsers[1]._id, createdUsers[2]._id] })
+      expect(events.user.deleted).toBeCalledTimes(3)
+      expect(events.user.permissionAdminRemoved).toBeCalledTimes(1)
+      expect(events.user.permissionBuilderRemoved).toBeCalledTimes(1)
+
+    })
+
   })
 })

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

@@ -16,7 +16,7 @@ import {
   migrations,
 } from "@budibase/backend-core"
 import { MigrationType, User } from "@budibase/types"
-import { groups as groupUtils } from "@budibase/pro/"
+import { groups as groupUtils } from "@budibase/pro"
 
 const PAGE_LIMIT = 8
 
@@ -201,7 +201,7 @@ export const save = async (
     const putUserFn = () => {
       return db.put(builtUser)
     }
-    console.log(builtUser)
+
     if (eventHelpers.isAddingBuilder(builtUser, dbUser)) {
       response = await quotas.addDeveloper(putUserFn)
     } else {
@@ -294,19 +294,20 @@ export const bulkCreate = async (
   })
 
   const usersToBulkSave = await Promise.all(usersToSave)
-  const response = await quotas.addDevelopers(
-    () => db.bulkDocs(usersToBulkSave),
-    builderCount
-  )
+  await quotas.addDevelopers(() => db.bulkDocs(usersToBulkSave), builderCount)
 
   // Post processing of bulk added users, i.e events and cache operations
   for (const user of usersToBulkSave) {
-    delete user.password
     await eventHelpers.handleSaveEvents(user, null)
     await apps.syncUserInApps(user._id)
   }
 
-  return response
+  return usersToBulkSave.map(user => {
+    return {
+      _id: user._id,
+      email: user.email,
+    }
+  })
 }
 
 export const bulkDelete = async (userIds: any) => {
@@ -349,6 +350,8 @@ export const bulkDelete = async (userIds: any) => {
     }))
   )
 
+  await groupUtils.bulkDeleteGroupUsers(groupsToModify)
+
   //Deletion post processing
   for (let user of usersToDelete) {
     await bulkDeleteProcessing(user)
@@ -356,7 +359,7 @@ export const bulkDelete = async (userIds: any) => {
 
   await quotas.removeDevelopers(builderCount)
 
-  return { groupsToModify, usersResponse: response }
+  return response
 }
 
 export const destroy = async (id: string, currentUser: any) => {

packages/worker/src/tests/TestConfiguration.js

@@ -11,7 +11,7 @@ const { createASession } = require("@budibase/backend-core/sessions")
 const { TENANT_ID, CSRF_TOKEN } = require("./structures")
 const structures = require("./structures")
 const { doInTenant } = require("@budibase/backend-core/tenancy")
-
+const { groups } = require("@budibase/pro")
 class TestConfiguration {
   constructor(openServer = true) {
     if (openServer) {
@@ -116,6 +116,22 @@ class TestConfiguration {
     })
   }
 
+  async getGroup(id) {
+    return doInTenant(TENANT_ID, () => {
+      return groups.get(id)
+    })
+  }
+
+  async saveGroup(group) {
+    const res = await this.getRequest()
+      .post(`/api/global/groups`)
+      .send(group)
+      .set(this.defaultHeaders())
+      .expect("Content-Type", /json/)
+      .expect(200)
+    return res.body
+  }
+
   async createUser(email, password) {
     const user = await this.getUser(structures.users.email)
     if (user) {

packages/worker/src/tests/structures/groups.ts

@@ -0,0 +1,11 @@
+export const UserGroup = () => {
+  let group = {
+    apps: [],
+    color: "var(--spectrum-global-color-blue-600)",
+    icon: "UserGroup",
+    name: "New group",
+    roles: {},
+    users: [],
+  }
+  return group
+}

packages/worker/src/tests/structures/index.js

@@ -1,5 +1,6 @@
 const configs = require("./configs")
 const users = require("./users")
+const groups = require("./groups")
 
 const TENANT_ID = "default"
 const CSRF_TOKEN = "e3727778-7af0-4226-b5eb-f43cbe60a306"
@@ -9,4 +10,5 @@ module.exports = {
   users,
   TENANT_ID,
   CSRF_TOKEN,
+  groups,
 }