diff --git a/packages/server/modules/core/domain/users/operations.ts b/packages/server/modules/core/domain/users/operations.ts index d6954146e..ef66d7ada 100644 --- a/packages/server/modules/core/domain/users/operations.ts +++ b/packages/server/modules/core/domain/users/operations.ts @@ -1,4 +1,5 @@ import { User, UserWithOptionalRole } from '@/modules/core/domain/users/types' +import { UserUpdateInput } from '@/modules/core/graph/generated/graphql' import { ServerAclRecord } from '@/modules/core/helpers/types' import { Nullable, NullableKeysToOptional, ServerRoles } from '@speckle/shared' @@ -36,6 +37,14 @@ export type StoreUser = (params: { user: Omit, 'suuid' | 'createdAt'> }) => Promise +export type UpdateUser = ( + userId: string, + update: Partial, + options?: Partial<{ + skipClean: boolean + }> +) => Promise> + export type CountAdminUsers = () => Promise export type StoreUserAcl = (params: { @@ -83,3 +92,8 @@ export type FindOrCreateValidatedUser = (params: { email: string isNewUser?: boolean }> + +export type UpdateUserAndNotify = ( + userId: string, + update: UserUpdateInput +) => Promise diff --git a/packages/server/modules/core/graph/resolvers/users.js b/packages/server/modules/core/graph/resolvers/users.js index 94f184ba4..100ab8ae3 100644 --- a/packages/server/modules/core/graph/resolvers/users.js +++ b/packages/server/modules/core/graph/resolvers/users.js @@ -4,7 +4,6 @@ const { searchUsers, changeUserRole } = require('@/modules/core/services/users') -const { updateUserAndNotify } = require('@/modules/core/services/users/management') const { ActionTypes } = require('@/modules/activitystream/helpers/types') const { validateScopes } = require(`@/modules/shared`) const zxcvbn = require('zxcvbn') @@ -16,7 +15,9 @@ const { Roles, Scopes } = require('@speckle/shared') const { markOnboardingComplete, legacyGetUserFactory, - legacyGetUserByEmailFactory + legacyGetUserByEmailFactory, + getUserFactory, + updateUserFactory } = require('@/modules/core/repositories/users') const { UsersMeta } = require('@/modules/core/dbSchema') const { getServerInfo } = require('@/modules/core/services/generic') @@ -29,10 +30,24 @@ const { const db = require('@/db/knex') const { BadRequestError } = require('@/modules/shared/errors') const { saveActivityFactory } = require('@/modules/activitystream/repositories') +const { + updateUserAndNotifyFactory +} = require('@/modules/core/services/users/management') +const { + addUserUpdatedActivityFactory +} = require('@/modules/activitystream/services/userActivity') const getUser = legacyGetUserFactory({ db }) const getUserByEmail = legacyGetUserByEmailFactory({ db }) +const updateUserAndNotify = updateUserAndNotifyFactory({ + getUser: getUserFactory({ db }), + updateUser: updateUserFactory({ db }), + addUserUpdatedActivity: addUserUpdatedActivityFactory({ + saveActivity: saveActivityFactory({ db }) + }) +}) + /** @type {import('@/modules/core/graph/generated/graphql').Resolvers} */ module.exports = { Query: { diff --git a/packages/server/modules/core/repositories/users.ts b/packages/server/modules/core/repositories/users.ts index c5065ac9e..85727330e 100644 --- a/packages/server/modules/core/repositories/users.ts +++ b/packages/server/modules/core/repositories/users.ts @@ -21,7 +21,8 @@ import { LegacyGetUser, LegacyGetUserByEmail, StoreUser, - StoreUserAcl + StoreUserAcl, + UpdateUser } from '@/modules/core/domain/users/operations' export type { UserWithOptionalRole, GetUserParams } @@ -234,29 +235,34 @@ const validateInputRecord = (input: Partial) => { } } -export async function updateUser( - userId: string, - update: Partial, - options?: Partial<{ - skipClean: boolean - }> -) { - if (!options?.skipClean) { - update = cleanInputRecord(update) +export const updateUserFactory = + (deps: { db: Knex }): UpdateUser => + async ( + userId: string, + update: Partial, + options?: Partial<{ + skipClean: boolean + }> + ) => { + if (!options?.skipClean) { + update = cleanInputRecord(update) + } + validateInputRecord(update) + + const [newUser] = await tables + .users(deps.db) + .where(Users.col.id, userId) + .update(update, '*') + + if (update.email) { + await updateUserEmailFactory(deps)({ + query: { userId, primary: true }, + update: { email: update.email } + }) + } + + return newUser as Nullable } - validateInputRecord(update) - - const [newUser] = await Users.knex().where(Users.col.id, userId).update(update, '*') - - if (update.email) { - await updateUserEmailFactory({ db })({ - query: { userId, primary: true }, - update: { email: update.email } - }) - } - - return newUser as Nullable -} export async function getFirstAdmin() { const q = Users.knex() diff --git a/packages/server/modules/core/services/users.js b/packages/server/modules/core/services/users.js index 45f4e501e..7ab319c24 100644 --- a/packages/server/modules/core/services/users.js +++ b/packages/server/modules/core/services/users.js @@ -8,7 +8,6 @@ const { } = require('@/modules/core/dbSchema') const { validateUserPassword, - updateUserAndNotify, MINIMUM_PASSWORD_LENGTH } = require('@/modules/core/services/users/management') @@ -52,13 +51,6 @@ module.exports = { return role }, - /** - * @deprecated {Use updateUserAndNotify() or repo method directly} - */ - async updateUser(id, user) { - return await updateUserAndNotify(id, user) - }, - /** * @deprecated {Use changePassword()} */ diff --git a/packages/server/modules/core/services/users/management.ts b/packages/server/modules/core/services/users/management.ts index a28a8f3a6..7552587d7 100644 --- a/packages/server/modules/core/services/users/management.ts +++ b/packages/server/modules/core/services/users/management.ts @@ -1,18 +1,20 @@ import { db } from '@/db/knex' -import { saveActivityFactory } from '@/modules/activitystream/repositories' import { addUserUpdatedActivityFactory } from '@/modules/activitystream/services/userActivity' import { CountAdminUsers, CreateValidatedUser, FindOrCreateValidatedUser, + GetUser, StoreUser, - StoreUserAcl + StoreUserAcl, + UpdateUser, + UpdateUserAndNotify } from '@/modules/core/domain/users/operations' import { UserUpdateError, UserValidationError } from '@/modules/core/errors/user' import { PasswordTooShortError, UserInputError } from '@/modules/core/errors/userinput' import { UserUpdateInput } from '@/modules/core/graph/generated/graphql' import type { UserRecord } from '@/modules/core/helpers/userHelper' -import { getUserFactory, updateUser } from '@/modules/core/repositories/users' +import { getUserFactory, updateUserFactory } from '@/modules/core/repositories/users' import { getServerInfo } from '@/modules/core/services/generic' import { sanitizeImageUrl } from '@/modules/shared/helpers/sanitization' import { isNullOrUndefined, NullableKeysToOptional, Roles } from '@speckle/shared' @@ -28,43 +30,46 @@ import { UsersEvents, UsersEventsEmitter } from '@/modules/core/events/usersEmit export const MINIMUM_PASSWORD_LENGTH = 8 -export async function updateUserAndNotify(userId: string, update: UserUpdateInput) { - const getUser = getUserFactory({ db }) - const existingUser = await getUser(userId) - if (!existingUser) { - throw new UserUpdateError('Attempting to update a non-existant user') - } - - const filteredUpdate: Partial = {} - for (const entry of Object.entries(update)) { - const key = entry[0] as keyof typeof update - let val = entry[1] - - if (key === 'avatar') { - val = sanitizeImageUrl(val) +export const updateUserAndNotifyFactory = + (deps: { + getUser: GetUser + updateUser: UpdateUser + addUserUpdatedActivity: ReturnType + }): UpdateUserAndNotify => + async (userId: string, update: UserUpdateInput) => { + const existingUser = await deps.getUser(userId) + if (!existingUser) { + throw new UserUpdateError('Attempting to update a non-existant user') } - if (!isNullOrUndefined(val)) { - filteredUpdate[key] = val + const filteredUpdate: Partial = {} + for (const entry of Object.entries(update)) { + const key = entry[0] as keyof typeof update + let val = entry[1] + + if (key === 'avatar') { + val = sanitizeImageUrl(val) + } + + if (!isNullOrUndefined(val)) { + filteredUpdate[key] = val + } } + + const newUser = await deps.updateUser(userId, filteredUpdate) + if (!newUser) { + throw new UserUpdateError("Couldn't update user") + } + + await deps.addUserUpdatedActivity({ + oldUser: existingUser, + update, + updaterId: userId + }) + + return newUser } - const newUser = await updateUser(userId, filteredUpdate) - if (!newUser) { - throw new UserUpdateError("Couldn't update user") - } - - await addUserUpdatedActivityFactory({ - saveActivity: saveActivityFactory({ db }) - })({ - oldUser: existingUser, - update, - updaterId: userId - }) - - return newUser -} - export async function validateUserPassword(params: { user: UserRecord password: string @@ -107,7 +112,7 @@ export async function changePassword( } const passwordDigest = await createPasswordDigest(newPassword) - await updateUser( + await updateUserFactory({ db })( userId, { passwordDigest diff --git a/packages/server/modules/core/tests/integration/updateUser.spec.ts b/packages/server/modules/core/tests/integration/updateUser.spec.ts index 05f989126..72463e316 100644 --- a/packages/server/modules/core/tests/integration/updateUser.spec.ts +++ b/packages/server/modules/core/tests/integration/updateUser.spec.ts @@ -13,7 +13,7 @@ import { legacyGetUserFactory, storeUserAclFactory, storeUserFactory, - updateUser + updateUserFactory } from '@/modules/core/repositories/users' import { expectToThrow } from '@/test/assertionHelper' import { @@ -65,6 +65,7 @@ const createUser = createUserFactory({ }), usersEventsEmitter: UsersEmitter.emit }) +const updateUser = updateUserFactory({ db }) describe('Users @core-users', () => { beforeEach(async () => { diff --git a/packages/server/modules/core/tests/users.spec.js b/packages/server/modules/core/tests/users.spec.js index 35ae80a36..292058262 100644 --- a/packages/server/modules/core/tests/users.spec.js +++ b/packages/server/modules/core/tests/users.spec.js @@ -5,7 +5,6 @@ const assert = require('assert') const { changeUserRole, searchUsers, - updateUser, deleteUser, validatePasssword, updateUserPassword @@ -89,7 +88,8 @@ const { storeUserFactory, countAdminUsersFactory, storeUserAclFactory, - legacyGetUserByEmailFactory + legacyGetUserByEmailFactory, + updateUserFactory } = require('@/modules/core/repositories/users') const { findEmailFactory, @@ -108,7 +108,8 @@ const { renderEmail } = require('@/modules/emails/services/emailRendering') const { sendEmail } = require('@/modules/emails/services/sending') const { createUserFactory, - findOrCreateUserFactory + findOrCreateUserFactory, + updateUserAndNotifyFactory } = require('@/modules/core/services/users/management') const { validateAndCreateUserEmailFactory @@ -117,6 +118,9 @@ const { finalizeInvitedServerRegistrationFactory } = require('@/modules/serverinvites/services/processing') const { UsersEmitter } = require('@/modules/core/events/usersEmitter') +const { + addUserUpdatedActivityFactory +} = require('@/modules/activitystream/services/userActivity') const getUser = legacyGetUserFactory({ db }) const getUsers = getUsersFactory({ db }) @@ -212,6 +216,13 @@ const findOrCreateUser = findOrCreateUserFactory({ findPrimaryEmailForUser: findPrimaryEmailForUserFactory({ db }) }) const getUserByEmail = legacyGetUserByEmailFactory({ db }) +const updateUser = updateUserAndNotifyFactory({ + getUser: getUserFactory({ db }), + updateUser: updateUserFactory({ db }), + addUserUpdatedActivity: addUserUpdatedActivityFactory({ + saveActivity: saveActivityFactory({ db }) + }) +}) describe('Actors & Tokens @user-services', () => { const myTestActor = {