From 89f6f1174b7d3460f886fec40f04fe360362eb5b Mon Sep 17 00:00:00 2001 From: Michael Drury Date: Fri, 23 Sep 2022 22:21:51 +0100 Subject: [PATCH] PR comments. --- packages/frontend-core/src/api/user.js | 1 - .../worker/src/api/controllers/global/auth.ts | 16 ++++++-- .../worker/src/api/controllers/global/self.ts | 4 +- .../src/api/controllers/global/users.ts | 40 +++++++------------ .../src/api/controllers/system/accounts.ts | 10 ++--- .../api/routes/system/tests/accounts.spec.ts | 8 ++-- .../functions/globalInfoSyncUsers.ts | 2 +- packages/worker/src/sdk/index.ts | 9 ++++- packages/worker/src/sdk/users/users.ts | 12 +++++- 9 files changed, 56 insertions(+), 46 deletions(-) diff --git a/packages/frontend-core/src/api/user.js b/packages/frontend-core/src/api/user.js index 9a02ef757c..39d9359e91 100644 --- a/packages/frontend-core/src/api/user.js +++ b/packages/frontend-core/src/api/user.js @@ -98,7 +98,6 @@ export const buildUserEndpoints = API => ({ }, }, }) - console.log(res) return res.created }, diff --git a/packages/worker/src/api/controllers/global/auth.ts b/packages/worker/src/api/controllers/global/auth.ts index 834531cd78..c27fe17ee7 100644 --- a/packages/worker/src/api/controllers/global/auth.ts +++ b/packages/worker/src/api/controllers/global/auth.ts @@ -8,7 +8,7 @@ const { checkResetPasswordCode } = require("../../../utilities/redis") const { getGlobalDB } = require("@budibase/backend-core/tenancy") const env = require("../../../environment") import { events, users as usersCore, context } from "@budibase/backend-core" -import { users } from "../../../sdk" +import sdk from "../../../sdk" import { User } from "@budibase/types" export const googleCallbackUrl = async (config: any) => { @@ -167,7 +167,11 @@ export const googlePreAuth = async (ctx: any, next: any) => { workspace: ctx.query.workspace, }) let callbackUrl = await exports.googleCallbackUrl(config) - const strategy = await google.strategyFactory(config, callbackUrl, users.save) + const strategy = await google.strategyFactory( + config, + callbackUrl, + sdk.users.save + ) return passport.authenticate(strategy, { scope: ["profile", "email"], @@ -184,7 +188,11 @@ export const googleAuth = async (ctx: any, next: any) => { workspace: ctx.query.workspace, }) const callbackUrl = await exports.googleCallbackUrl(config) - const strategy = await google.strategyFactory(config, callbackUrl, users.save) + const strategy = await google.strategyFactory( + config, + callbackUrl, + sdk.users.save + ) return passport.authenticate( strategy, @@ -214,7 +222,7 @@ export const oidcStrategyFactory = async (ctx: any, configId: any) => { chosenConfig, callbackUrl ) - return oidc.strategyFactory(enrichedConfig, users.save) + return oidc.strategyFactory(enrichedConfig, sdk.users.save) } /** diff --git a/packages/worker/src/api/controllers/global/self.ts b/packages/worker/src/api/controllers/global/self.ts index 6eb123d768..685e2c8243 100644 --- a/packages/worker/src/api/controllers/global/self.ts +++ b/packages/worker/src/api/controllers/global/self.ts @@ -1,4 +1,4 @@ -import { users } from "../../../sdk" +import sdk from "../../../sdk" import { events, featureFlags, @@ -116,7 +116,7 @@ export async function getSelf(ctx: any) { checkCurrentApp(ctx) // get the main body of the user - const user = await users.getUser(userId) + const user = await sdk.users.getUser(userId) ctx.body = await groups.enrichUserRolesFromGroups(user) // add the feature flags for this tenant diff --git a/packages/worker/src/api/controllers/global/users.ts b/packages/worker/src/api/controllers/global/users.ts index 1914ea15ee..8894330f67 100644 --- a/packages/worker/src/api/controllers/global/users.ts +++ b/packages/worker/src/api/controllers/global/users.ts @@ -1,5 +1,5 @@ import { checkInviteCode } from "../../../utilities/redis" -import { users as userSdk } from "../../../sdk" +import sdk from "../../../sdk" import env from "../../../environment" import { BulkUserRequest, @@ -17,13 +17,12 @@ import { tenancy, } from "@budibase/backend-core" import { checkAnyUserExists } from "../../../utilities/users" -import { groups } from "@budibase/pro" const MAX_USERS_UPLOAD_LIMIT = 1000 export const save = async (ctx: any) => { try { - ctx.body = await userSdk.save(ctx.request.body) + ctx.body = await sdk.users.save(ctx.request.body) } catch (err: any) { ctx.throw(err.status || 400, err) } @@ -33,7 +32,7 @@ const bulkDelete = async (userIds: string[], currentUserId: string) => { if (userIds?.indexOf(currentUserId) !== -1) { throw new Error("Unable to delete self.") } - return await userSdk.bulkDelete(userIds) + return await sdk.users.bulkDelete(userIds) } const bulkCreate = async (users: User[], groupIds: string[]) => { @@ -42,18 +41,7 @@ const bulkCreate = async (users: User[], groupIds: string[]) => { "Max limit for upload is 1000 users. Please reduce file size and try again." ) } - const created = await userSdk.bulkCreate(users, groupIds) - const success = created?.successful - // now update the groups - if (Array.isArray(success) && groupIds) { - const groupPromises = [] - const createdUserIds = success.map(user => user._id) - for (let groupId of groupIds) { - groupPromises.push(groups.addUsers(groupId, createdUserIds)) - } - await Promise.all(groupPromises) - } - return created + return await sdk.users.bulkCreate(users, groupIds) } export const bulkUpdate = async (ctx: any) => { @@ -68,7 +56,7 @@ export const bulkUpdate = async (ctx: any) => { deleted = await bulkDelete(input.delete.userIds, currentUserId) } } catch (err: any) { - ctx.throw(400, err?.message || err) + ctx.throw(err.status || 400, err?.message || err) } ctx.body = { created, deleted } as BulkUserResponse } @@ -114,7 +102,7 @@ export const adminUser = async (ctx: any) => { // always bust checklist beforehand, if an error occurs but can proceed, don't get // stuck in a cycle await cache.bustCache(cache.CacheKeys.CHECKLIST) - const finalUser = await userSdk.save(user, { + const finalUser = await sdk.users.save(user, { hashPassword, requirePassword, }) @@ -136,7 +124,7 @@ export const adminUser = async (ctx: any) => { export const countByApp = async (ctx: any) => { const appId = ctx.params.appId try { - ctx.body = await userSdk.countUsersByApp(appId) + ctx.body = await sdk.users.countUsersByApp(appId) } catch (err: any) { ctx.throw(err.status || 400, err) } @@ -148,7 +136,7 @@ export const destroy = async (ctx: any) => { ctx.throw(400, "Unable to delete self.") } - await userSdk.destroy(id, ctx.user) + await sdk.users.destroy(id, ctx.user) ctx.body = { message: `User ${id} deleted.`, @@ -156,7 +144,7 @@ export const destroy = async (ctx: any) => { } export const search = async (ctx: any) => { - const paginated = await userSdk.paginatedUsers(ctx.request.body) + const paginated = await sdk.users.paginatedUsers(ctx.request.body) // user hashed password shouldn't ever be returned for (let user of paginated.data) { if (user) { @@ -168,7 +156,7 @@ export const search = async (ctx: any) => { // called internally by app server user fetch export const fetch = async (ctx: any) => { - const all = await userSdk.allUsers() + const all = await sdk.users.allUsers() // user hashed password shouldn't ever be returned for (let user of all) { if (user) { @@ -180,7 +168,7 @@ export const fetch = async (ctx: any) => { // called internally by app server user find export const find = async (ctx: any) => { - ctx.body = await userSdk.getUser(ctx.params.id) + ctx.body = await sdk.users.getUser(ctx.params.id) } export const tenantUserLookup = async (ctx: any) => { @@ -195,7 +183,7 @@ export const tenantUserLookup = async (ctx: any) => { export const invite = async (ctx: any) => { const request = ctx.request.body as InviteUserRequest - const response = await userSdk.invite([request]) + const response = await sdk.users.invite([request]) // explicitly throw for single user invite if (response.unsuccessful.length) { @@ -214,7 +202,7 @@ export const invite = async (ctx: any) => { export const inviteMultiple = async (ctx: any) => { const request = ctx.request.body as InviteUsersRequest - ctx.body = await userSdk.invite(request) + ctx.body = await sdk.users.invite(request) } export const inviteAccept = async (ctx: any) => { @@ -223,7 +211,7 @@ export const inviteAccept = async (ctx: any) => { // info is an extension of the user object that was stored by global const { email, info }: any = await checkInviteCode(inviteCode) ctx.body = await tenancy.doInTenant(info.tenantId, async () => { - const saved = await userSdk.save({ + const saved = await sdk.users.save({ firstName, lastName, password, diff --git a/packages/worker/src/api/controllers/system/accounts.ts b/packages/worker/src/api/controllers/system/accounts.ts index 5e72f35bab..0aa5f25785 100644 --- a/packages/worker/src/api/controllers/system/accounts.ts +++ b/packages/worker/src/api/controllers/system/accounts.ts @@ -1,21 +1,21 @@ import { Account, AccountMetadata } from "@budibase/types" -import { accounts } from "../../../sdk" +import sdk from "../../../sdk" export const save = async (ctx: any) => { const account = ctx.request.body as Account let metadata: AccountMetadata = { - _id: accounts.formatAccountMetadataId(account.accountId), + _id: sdk.accounts.formatAccountMetadataId(account.accountId), email: account.email, } - metadata = await accounts.saveMetadata(metadata) + metadata = await sdk.accounts.saveMetadata(metadata) ctx.body = metadata ctx.status = 200 } export const destroy = async (ctx: any) => { - const accountId = accounts.formatAccountMetadataId(ctx.params.accountId) - await accounts.destroyMetadata(accountId) + const accountId = sdk.accounts.formatAccountMetadataId(ctx.params.accountId) + await sdk.accounts.destroyMetadata(accountId) ctx.status = 204 } diff --git a/packages/worker/src/api/routes/system/tests/accounts.spec.ts b/packages/worker/src/api/routes/system/tests/accounts.spec.ts index e3a6141cb7..f977d22cd9 100644 --- a/packages/worker/src/api/routes/system/tests/accounts.spec.ts +++ b/packages/worker/src/api/routes/system/tests/accounts.spec.ts @@ -1,4 +1,4 @@ -import { accounts } from "../../../../sdk" +import sdk from "../../../../sdk" import { TestConfiguration, structures, API } from "../../../../tests" import { v4 as uuid } from "uuid" @@ -25,8 +25,8 @@ describe("accounts", () => { const response = await api.accounts.saveMetadata(account) - const id = accounts.formatAccountMetadataId(account.accountId) - const metadata = await accounts.getMetadata(id) + const id = sdk.accounts.formatAccountMetadataId(account.accountId) + const metadata = await sdk.accounts.getMetadata(id) expect(response).toStrictEqual(metadata) }) }) @@ -38,7 +38,7 @@ describe("accounts", () => { await api.accounts.destroyMetadata(account.accountId) - const deleted = await accounts.getMetadata(account.accountId) + const deleted = await sdk.accounts.getMetadata(account.accountId) expect(deleted).toBe(undefined) }) diff --git a/packages/worker/src/migrations/functions/globalInfoSyncUsers.ts b/packages/worker/src/migrations/functions/globalInfoSyncUsers.ts index cae6c6af51..941791fe93 100644 --- a/packages/worker/src/migrations/functions/globalInfoSyncUsers.ts +++ b/packages/worker/src/migrations/functions/globalInfoSyncUsers.ts @@ -1,5 +1,5 @@ import { User } from "@budibase/types" -import * as sdk from "../../sdk" +import sdk from "../../sdk" /** * Date: diff --git a/packages/worker/src/sdk/index.ts b/packages/worker/src/sdk/index.ts index fdc1098361..5febb7ba3c 100644 --- a/packages/worker/src/sdk/index.ts +++ b/packages/worker/src/sdk/index.ts @@ -1,2 +1,7 @@ -export * as users from "./users" -export * as accounts from "./accounts" +import * as users from "./users" +import * as accounts from "./accounts" + +export default { + users, + accounts, +} diff --git a/packages/worker/src/sdk/users/users.ts b/packages/worker/src/sdk/users/users.ts index ece2d76082..775514ea5e 100644 --- a/packages/worker/src/sdk/users/users.ts +++ b/packages/worker/src/sdk/users/users.ts @@ -31,6 +31,7 @@ import { } from "@budibase/types" import { sendEmail } from "../../utilities/email" import { EmailTemplatePurpose } from "../../constants" +import { groups as groupsSdk } from "@budibase/pro" const PAGE_LIMIT = 8 @@ -347,7 +348,6 @@ export const bulkCreate = async ( newUsersRequested: User[], groups: string[] ): Promise => { - const db = tenancy.getGlobalDB() const tenantId = tenancy.getTenantId() let usersToSave: any[] = [] @@ -407,6 +407,16 @@ export const bulkCreate = async ( } }) + // now update the groups + if (Array.isArray(saved) && groups) { + const groupPromises = [] + const createdUserIds = saved.map(user => user._id) + for (let groupId of groups) { + groupPromises.push(groupsSdk.addUsers(groupId, createdUserIds)) + } + await Promise.all(groupPromises) + } + return { successful: saved, unsuccessful,