From dca37a61da00bc57995c488a9526b104b25937e7 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Wed, 27 Jul 2022 10:20:39 +0100 Subject: [PATCH] adding tests and pr comments --- .../src/events/publishers/group.ts | 8 +- .../tests/utilities/mocks/events.js | 8 ++ .../portal/manage/groups/[groupId].svelte | 16 +++- .../overview/_components/AccessTab.svelte | 6 +- packages/server/src/utilities/global.js | 1 - packages/types/src/documents/global/user.ts | 1 - .../types/src/documents/global/userGroup.ts | 8 +- packages/types/src/sdk/events/event.ts | 6 +- .../src/api/controllers/global/users.ts | 3 +- .../src/api/routes/tests/groups.spec.js | 95 +++++++++++++++++++ .../worker/src/api/routes/tests/users.spec.js | 85 +++++++++++++++-- packages/worker/src/sdk/users/users.ts | 21 ++-- .../worker/src/tests/TestConfiguration.js | 18 +++- .../worker/src/tests/structures/groups.ts | 11 +++ packages/worker/src/tests/structures/index.js | 2 + 15 files changed, 250 insertions(+), 39 deletions(-) create mode 100644 packages/worker/src/api/routes/tests/groups.spec.js create mode 100644 packages/worker/src/tests/structures/groups.ts diff --git a/packages/backend-core/src/events/publishers/group.ts b/packages/backend-core/src/events/publishers/group.ts index ab805c5610..d300873725 100644 --- a/packages/backend-core/src/events/publishers/group.ts +++ b/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) } diff --git a/packages/backend-core/tests/utilities/mocks/events.js b/packages/backend-core/tests/utilities/mocks/events.js index a4055cc5ea..415d59019d 100644 --- a/packages/backend-core/tests/utilities/mocks/events.js +++ b/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") diff --git a/packages/builder/src/pages/builder/portal/manage/groups/[groupId].svelte b/packages/builder/src/pages/builder/portal/manage/groups/[groupId].svelte index bbb7e1491d..2bcfd85cb6 100644 --- a/packages/builder/src/pages/builder/portal/manage/groups/[groupId].svelte +++ b/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 @@ {#if group?.users.length} {#each group.users as user} - removeUser(user?._id)} hoverable @@ -167,8 +175,8 @@ - {#if group?.apps.length} - {#each group.apps as app} + {#if groupApps.length} + {#each groupApps as app} { - 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) diff --git a/packages/server/src/utilities/global.js b/packages/server/src/utilities/global.js index abf10aff20..b277b1aad9 100644 --- a/packages/server/src/utilities/global.js +++ b/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)) } diff --git a/packages/types/src/documents/global/user.ts b/packages/types/src/documents/global/user.ts index 41fcc4aa7b..c9255a1bb1 100644 --- a/packages/types/src/documents/global/user.ts +++ b/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[] } diff --git a/packages/types/src/documents/global/userGroup.ts b/packages/types/src/documents/global/userGroup.ts index e37286222e..b822e5bb7b 100644 --- a/packages/types/src/documents/global/userGroup.ts +++ b/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 } diff --git a/packages/types/src/sdk/events/event.ts b/packages/types/src/sdk/events/event.ts index 7ec1bf157e..f0e023df51 100644 --- a/packages/types/src/sdk/events/event.ts +++ b/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 diff --git a/packages/worker/src/api/controllers/global/users.ts b/packages/worker/src/api/controllers/global/users.ts index a6458e0432..ff630efae8 100644 --- a/packages/worker/src/api/controllers/global/users.ts +++ b/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`, diff --git a/packages/worker/src/api/routes/tests/groups.spec.js b/packages/worker/src/api/routes/tests/groups.spec.js new file mode 100644 index 0000000000..db3b1052e7 --- /dev/null +++ b/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) + }) + }) +}) diff --git a/packages/worker/src/api/routes/tests/users.spec.js b/packages/worker/src/api/routes/tests/users.spec.js index bf71c9df8c..f802f701f5 100644 --- a/packages/worker/src/api/routes/tests/users.spec.js +++ b/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", () => { @@ -23,9 +24,9 @@ describe("/api/global/users", () => { .set(config.defaultHeaders()) .expect("Content-Type", /json/) .expect(200) - - const emailCall = sendMailMock.mock.calls[0][0] - // after this URL there should be a code + + const emailCall = sendMailMock.mock.calls[0][0] + // after this URL there should be a code const parts = emailCall.html.split("http://localhost:10000/builder/invite?code=") const code = parts[1].split("\"")[0].split("&")[0] return { code, res } @@ -59,7 +60,7 @@ describe("/api/global/users", () => { expect(events.user.inviteAccepted).toBeCalledWith(user) }) - const createUser = async (user) => { + const createUser = async (user) => { const existing = await config.getUser(user.email) if (existing) { await deleteUser(existing._id) @@ -83,14 +84,37 @@ 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) { await request - .delete(`/api/global/users/${user._id}`) - .set(config.defaultHeaders()) - .expect("Content-Type", /json/) - .expect(200) + .delete(`/api/global/users/${user._id}`) + .set(config.defaultHeaders()) + .expect("Content-Type", /json/) + .expect(200) } } @@ -106,10 +130,25 @@ 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" }) - await createUser(user) + await createUser(user) expect(events.user.created).toBeCalledTimes(1) expect(events.user.updated).not.toBeCalled() @@ -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) + + }) + }) }) \ No newline at end of file diff --git a/packages/worker/src/sdk/users/users.ts b/packages/worker/src/sdk/users/users.ts index 899c9f9a0a..7e8d2cc0cd 100644 --- a/packages/worker/src/sdk/users/users.ts +++ b/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) => { diff --git a/packages/worker/src/tests/TestConfiguration.js b/packages/worker/src/tests/TestConfiguration.js index 9c45217e3e..694c56bfcd 100644 --- a/packages/worker/src/tests/TestConfiguration.js +++ b/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) { diff --git a/packages/worker/src/tests/structures/groups.ts b/packages/worker/src/tests/structures/groups.ts new file mode 100644 index 0000000000..874d1b6a10 --- /dev/null +++ b/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 +} diff --git a/packages/worker/src/tests/structures/index.js b/packages/worker/src/tests/structures/index.js index 115d22e731..3212ae606d 100644 --- a/packages/worker/src/tests/structures/index.js +++ b/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, }