chore(server): refactor activityStream invocations - batch #1 - user (#3845)

* chore(server): refactor activityStream invocations - batch #1 - user

* minor cleanup

* test fix + swap to TS

* move listeners to new loc

packages/server/modules/activitystream/events/userListeners.ts

@@ -0,0 +1,77 @@
+import { UserUpdateInput } from '@/modules/core/graph/generated/graphql'
+import { UserRecord } from '@/modules/core/helpers/types'
+import { ActionTypes, ResourceTypes } from '@/modules/activitystream/helpers/types'
+import { SaveActivity } from '@/modules/activitystream/domain/operations'
+import { EventBusListen, EventPayload } from '@/modules/shared/services/eventBus'
+import { UserEvents } from '@/modules/core/domain/users/events'
+
+const addUserCreatedActivityFactory =
+  ({ saveActivity }: { saveActivity: SaveActivity }) =>
+  async (payload: EventPayload<typeof UserEvents.Created>) => {
+    const { user } = payload.payload
+
+    await saveActivity({
+      streamId: null,
+      resourceType: ResourceTypes.User,
+      resourceId: user.id,
+      actionType: ActionTypes.User.Create,
+      userId: user.id,
+      info: { user },
+      message: 'User created'
+    })
+  }
+
+const addUserUpdatedActivityFactory =
+  ({ saveActivity }: { saveActivity: SaveActivity }) =>
+  async (params: {
+    oldUser: UserRecord
+    update: UserUpdateInput
+    updaterId: string
+  }) => {
+    const { oldUser, update, updaterId } = params
+
+    await saveActivity({
+      streamId: null,
+      resourceType: ResourceTypes.User,
+      resourceId: oldUser.id,
+      actionType: ActionTypes.User.Update,
+      userId: updaterId,
+      info: { old: oldUser, new: update },
+      message: 'User updated'
+    })
+  }
+
+const addUserDeletedActivityFactory =
+  (deps: { saveActivity: SaveActivity }) =>
+  async (params: { targetUserId: string; invokerUserId: string }) => {
+    const { targetUserId, invokerUserId } = params
+
+    await deps.saveActivity({
+      streamId: null,
+      resourceType: 'user',
+      resourceId: targetUserId,
+      actionType: ActionTypes.User.Delete,
+      userId: invokerUserId,
+      info: {},
+      message: 'User deleted'
+    })
+  }
+
+export const reportUserActivityFactory =
+  (deps: { eventListen: EventBusListen; saveActivity: SaveActivity }) => () => {
+    const addUserDeletedActivity = addUserDeletedActivityFactory(deps)
+    const addUserUpdatedActivity = addUserUpdatedActivityFactory(deps)
+    const addUserCreatedActivity = addUserCreatedActivityFactory(deps)
+
+    const quitters = [
+      deps.eventListen(UserEvents.Deleted, async ({ payload }) => {
+        await addUserDeletedActivity(payload)
+      }),
+      deps.eventListen(UserEvents.Created, addUserCreatedActivity),
+      deps.eventListen(UserEvents.Updated, async ({ payload }) => {
+        await addUserUpdatedActivity(payload)
+      })
+    ]
+
+    return () => quitters.forEach((q) => q())
+  }

packages/server/modules/activitystream/index.ts

@@ -28,16 +28,15 @@ import { Knex } from 'knex'
 import {
   onServerAccessRequestCreatedFactory,
   onServerAccessRequestFinalizedFactory,
-  onServerInviteCreatedFactory,
-  onUserCreatedFactory
+  onServerInviteCreatedFactory
 } from '@/modules/activitystream/services/eventListener'
 import { isProjectResourceTarget } from '@/modules/serverinvites/helpers/core'
 import { publish } from '@/modules/shared/utils/subscriptions'
 import { isStreamAccessRequest } from '@/modules/accessrequests/repositories'
 import { ServerInvitesEvents } from '@/modules/serverinvites/domain/events'
 import { ProjectEvents } from '@/modules/core/domain/projects/events'
-import { UserEvents } from '@/modules/core/domain/users/events'
 import { AccessRequestEvents } from '@/modules/accessrequests/domain/events'
+import { reportUserActivityFactory } from '@/modules/activitystream/events/userListeners'
 
 let scheduledTask: ReturnType<ScheduleExecution> | null = null
 let quitEventListeners: Optional<() => void> = undefined
@@ -53,12 +52,13 @@ const initializeEventListeners = ({
   eventBus: EventBus
   db: Knex
 }) => {
+  const saveActivity = saveActivityFactory({ db })
+  const reportUserActivity = reportUserActivityFactory({
+    eventListen: eventBus.listen,
+    saveActivity
+  })
   const quitCbs = [
-    eventBus.listen(
-      UserEvents.Created,
-      // this activity will always go in the main DB
-      onUserCreatedFactory({ saveActivity: saveActivityFactory({ db }) })
-    ),
+    reportUserActivity(),
     eventBus.listen(AccessRequestEvents.Created, async (payload) => {
       if (!isStreamAccessRequest(payload.payload.request)) return
       return await onServerAccessRequestCreatedFactory({

packages/server/modules/activitystream/services/eventListener.ts

@@ -7,11 +7,9 @@ import {
 import {
   AddStreamAccessRequestDeclinedActivity,
   AddStreamAccessRequestedActivity,
-  AddStreamInviteSentOutActivity,
-  SaveActivity
+  AddStreamInviteSentOutActivity
 } from '@/modules/activitystream/domain/operations'
 import { GetStream } from '@/modules/core/domain/streams/operations'
-import { UserEvents } from '@/modules/core/domain/users/events'
 import {
   ServerInvitesEvents,
   ServerInvitesEventsPayloads
@@ -22,22 +20,6 @@ import {
 } from '@/modules/serverinvites/helpers/core'
 import { EventPayload } from '@/modules/shared/services/eventBus'
 
-export const onUserCreatedFactory =
-  ({ saveActivity }: { saveActivity: SaveActivity }) =>
-  async (payload: EventPayload<typeof UserEvents.Created>) => {
-    const { user } = payload.payload
-
-    await saveActivity({
-      streamId: null,
-      resourceType: 'user',
-      resourceId: user.id,
-      actionType: 'user_create',
-      userId: user.id,
-      info: { user },
-      message: 'User created'
-    })
-  }
-
 export const onServerAccessRequestCreatedFactory =
   ({
     addStreamAccessRequestedActivity

packages/server/modules/activitystream/services/userActivity.ts

@@ -1,24 +0,0 @@
-import { UserUpdateInput } from '@/modules/core/graph/generated/graphql'
-import { UserRecord } from '@/modules/core/helpers/types'
-import { ActionTypes, ResourceTypes } from '@/modules/activitystream/helpers/types'
-import { SaveActivity } from '@/modules/activitystream/domain/operations'
-
-export const addUserUpdatedActivityFactory =
-  ({ saveActivity }: { saveActivity: SaveActivity }) =>
-  async (params: {
-    oldUser: UserRecord
-    update: UserUpdateInput
-    updaterId: string
-  }) => {
-    const { oldUser, update, updaterId } = params
-
-    await saveActivity({
-      streamId: null,
-      resourceType: ResourceTypes.User,
-      resourceId: oldUser.id,
-      actionType: ActionTypes.User.Update,
-      userId: updaterId,
-      info: { old: oldUser, new: update },
-      message: 'User updated'
-    })
-  }

packages/server/modules/core/domain/users/events.ts

@@ -1,10 +1,13 @@
 import { User, UserSignUpContext } from '@/modules/core/domain/users/types'
+import { UserUpdateInput } from '@/modules/core/graph/generated/graphql'
 import { Optional } from '@speckle/shared'
 
 export const userEventsNamespace = 'users' as const
 
 export const UserEvents = {
-  Created: `${userEventsNamespace}.created`
+  Created: `${userEventsNamespace}.created`,
+  Deleted: `${userEventsNamespace}.deleted`,
+  Updated: `${userEventsNamespace}.updated`
 } as const
 
 export type UserEventsPayloads = {
@@ -15,4 +18,13 @@ export type UserEventsPayloads = {
      */
     signUpCtx: Optional<UserSignUpContext>
   }
+  [UserEvents.Deleted]: {
+    targetUserId: string
+    invokerUserId: string
+  }
+  [UserEvents.Updated]: {
+    oldUser: User
+    update: UserUpdateInput
+    updaterId: string
+  }
 }

packages/server/modules/core/domain/users/operations.ts

@@ -159,7 +159,7 @@ export type ValidateUserPassword = (params: {
   password: string
 }) => Promise<boolean>
 
-export type DeleteUser = (id: string) => Promise<boolean>
+export type DeleteUser = (id: string, invokerId?: string) => Promise<boolean>
 
 export type ChangeUserRole = (params: { userId: string; role: string }) => Promise<void>
 

packages/server/modules/core/graph/resolvers/users.ts

@@ -1,4 +1,3 @@
-import { ActionTypes } from '@/modules/activitystream/helpers/types'
 import { validateScopes } from '@/modules/shared'
 import zxcvbn from 'zxcvbn'
 import { Roles, Scopes } from '@speckle/shared'
@@ -27,13 +26,11 @@ import {
 } from '@/modules/serverinvites/repositories/serverInvites'
 import db from '@/db/knex'
 import { BadRequestError } from '@/modules/shared/errors'
-import { saveActivityFactory } from '@/modules/activitystream/repositories'
 import {
   updateUserAndNotifyFactory,
   deleteUserFactory,
   changeUserRoleFactory
 } from '@/modules/core/services/users/management'
-import { addUserUpdatedActivityFactory } from '@/modules/activitystream/services/userActivity'
 import {
   deleteStreamFactory,
   getUserDeletableStreamsFactory
@@ -42,6 +39,7 @@ import { dbLogger } from '@/logging/logging'
 import { getAdminUsersListCollectionFactory } from '@/modules/core/services/users/legacyAdminUsersList'
 import { Resolvers } from '@/modules/core/graph/generated/graphql'
 import { getServerInfoFactory } from '@/modules/core/repositories/server'
+import { getEventBus } from '@/modules/shared/services/eventBus'
 
 const getUser = legacyGetUserFactory({ db })
 const getUserByEmail = legacyGetUserByEmailFactory({ db })
@@ -49,9 +47,7 @@ const getUserByEmail = legacyGetUserByEmailFactory({ db })
 const updateUserAndNotify = updateUserAndNotifyFactory({
   getUser: getUserFactory({ db }),
   updateUser: updateUserFactory({ db }),
-  addUserUpdatedActivity: addUserUpdatedActivityFactory({
-    saveActivity: saveActivityFactory({ db })
-  })
+  emitEvent: getEventBus().emit
 })
 
 const getServerInfo = getServerInfoFactory({ db })
@@ -61,7 +57,8 @@ const deleteUser = deleteUserFactory({
   isLastAdminUser: isLastAdminUserFactory({ db }),
   getUserDeletableStreams: getUserDeletableStreamsFactory({ db }),
   deleteAllUserInvites: deleteAllUserInvitesFactory({ db }),
-  deleteUserRecord: deleteUserRecordFactory({ db })
+  deleteUserRecord: deleteUserRecordFactory({ db }),
+  emitEvent: getEventBus().emit
 })
 const getUserRole = getUserRoleFactory({ db })
 const changeUserRole = changeUserRoleFactory({
@@ -226,7 +223,7 @@ export = {
       const user = await getUserByEmail({ email: args.userConfirmation.email })
       if (!user) return false
 
-      await deleteUser(user.id)
+      await deleteUser(user.id, context.userId)
       return true
     },
 
@@ -243,17 +240,7 @@ export = {
       await throwForNotHavingServerRole(context, Roles.Server.Guest)
       await validateScopes(context.scopes, Scopes.Profile.Delete)
 
-      await deleteUser(context.userId!)
-
-      await saveActivityFactory({ db })({
-        streamId: null,
-        resourceType: 'user',
-        resourceId: context.userId!,
-        actionType: ActionTypes.User.Delete,
-        userId: context.userId!,
-        info: {},
-        message: 'User deleted'
-      })
+      await deleteUser(context.userId!, context.userId!)
 
       return true
     },

packages/server/modules/core/services/users/management.ts

@@ -1,4 +1,3 @@
-import { addUserUpdatedActivityFactory } from '@/modules/activitystream/services/userActivity'
 import {
   ChangeUserPassword,
   ChangeUserRole,
@@ -56,7 +55,7 @@ export const updateUserAndNotifyFactory =
   (deps: {
     getUser: GetUser
     updateUser: UpdateUser
-    addUserUpdatedActivity: ReturnType<typeof addUserUpdatedActivityFactory>
+    emitEvent: EventBusEmit
   }): UpdateUserAndNotify =>
   async (userId: string, update: UserUpdateInput) => {
     const existingUser = await deps.getUser(userId)
@@ -83,10 +82,13 @@ export const updateUserAndNotifyFactory =
       throw new UserUpdateError("Couldn't update user")
     }
 
-    await deps.addUserUpdatedActivity({
-      oldUser: existingUser,
-      update,
-      updaterId: userId
+    await deps.emitEvent({
+      eventName: UserEvents.Updated,
+      payload: {
+        oldUser: existingUser,
+        update,
+        updaterId: userId
+      }
     })
 
     return newUser
@@ -264,8 +266,9 @@ export const deleteUserFactory =
     getUserDeletableStreams: GetUserDeletableStreams
     deleteAllUserInvites: DeleteAllUserInvites
     deleteUserRecord: DeleteUserRecord
+    emitEvent: EventBusEmit
   }): DeleteUser =>
-  async (id) => {
+  async (id, invokerId) => {
     deps.logger.info('Deleting user ' + id)
     const isLastAdmin = await deps.isLastAdminUser(id)
     if (isLastAdmin) {
@@ -281,7 +284,15 @@ export const deleteUserFactory =
     // THIS REALLY SHOULD BE A REACTION TO THE USER DELETED EVENT EMITTED HER
     await deps.deleteAllUserInvites(id)
 
-    return await deps.deleteUserRecord(id)
+    const deleted = await deps.deleteUserRecord(id)
+    if (deleted) {
+      await deps.emitEvent({
+        eventName: UserEvents.Deleted,
+        payload: { targetUserId: id, invokerUserId: invokerId || id }
+      })
+    }
+
+    return deleted
   }
 
 export const changeUserRoleFactory =

packages/server/modules/core/tests/users.spec.ts

@@ -100,7 +100,6 @@ import {
 } from '@/modules/core/services/users/management'
 import { validateAndCreateUserEmailFactory } from '@/modules/core/services/userEmails'
 import { finalizeInvitedServerRegistrationFactory } from '@/modules/serverinvites/services/processing'
-import { addUserUpdatedActivityFactory } from '@/modules/activitystream/services/userActivity'
 import { dbLogger } from '@/logging/logging'
 import {
   storeApiTokenFactory,
@@ -214,9 +213,7 @@ const getUserByEmail = legacyGetUserByEmailFactory({ db })
 const updateUser = updateUserAndNotifyFactory({
   getUser: getUserFactory({ db }),
   updateUser: updateUserFactory({ db }),
-  addUserUpdatedActivity: addUserUpdatedActivityFactory({
-    saveActivity: saveActivityFactory({ db })
-  })
+  emitEvent: getEventBus().emit
 })
 const updateUserPassword = changePasswordFactory({
   getUser: getUserFactory({ db }),
@@ -231,7 +228,8 @@ const deleteUser = deleteUserFactory({
   isLastAdminUser: isLastAdminUserFactory({ db }),
   getUserDeletableStreams: getUserDeletableStreamsFactory({ db }),
   deleteAllUserInvites: deleteAllUserInvitesFactory({ db }),
-  deleteUserRecord: deleteUserRecordFactory({ db })
+  deleteUserRecord: deleteUserRecordFactory({ db }),
+  emitEvent: getEventBus().emit
 })
 const changeUserRole = changeUserRoleFactory({
   getServerInfo,

packages/server/modules/core/tests/usersAdmin.spec.ts

@@ -1,10 +1,8 @@
-const expect = require('chai').expect
-const assert = require('assert')
-
-const { beforeEachContext } = require('@/test/hooks')
-const { Roles } = require('@speckle/shared')
-const cryptoRandomString = require('crypto-random-string')
-const {
+import assert from 'assert'
+import { beforeEachContext } from '@/test/hooks'
+import { ensureError, Roles } from '@speckle/shared'
+import cryptoRandomString from 'crypto-random-string'
+import {
   legacyGetPaginatedUsersFactory,
   legacyGetPaginatedUsersCountFactory,
   getUserFactory,
@@ -15,44 +13,37 @@ const {
   deleteUserRecordFactory,
   getUserRoleFactory,
   updateUserServerRoleFactory
-} = require('@/modules/core/repositories/users')
-const { db } = require('@/db/knex')
-const {
+} from '@/modules/core/repositories/users'
+import { db } from '@/db/knex'
+import {
   findEmailFactory,
   createUserEmailFactory,
   ensureNoPrimaryEmailForUserFactory
-} = require('@/modules/core/repositories/userEmails')
-const {
-  requestNewEmailVerificationFactory
-} = require('@/modules/emails/services/verification/request')
-const {
-  deleteOldAndInsertNewVerificationFactory
-} = require('@/modules/emails/repositories')
-const { renderEmail } = require('@/modules/emails/services/emailRendering')
-const { sendEmail } = require('@/modules/emails/services/sending')
-const {
+} from '@/modules/core/repositories/userEmails'
+import { requestNewEmailVerificationFactory } from '@/modules/emails/services/verification/request'
+import { deleteOldAndInsertNewVerificationFactory } from '@/modules/emails/repositories'
+import { renderEmail } from '@/modules/emails/services/emailRendering'
+import { sendEmail } from '@/modules/emails/services/sending'
+import {
   createUserFactory,
   deleteUserFactory,
   changeUserRoleFactory
-} = require('@/modules/core/services/users/management')
-const {
-  validateAndCreateUserEmailFactory
-} = require('@/modules/core/services/userEmails')
-const {
-  finalizeInvitedServerRegistrationFactory
-} = require('@/modules/serverinvites/services/processing')
-const {
+} from '@/modules/core/services/users/management'
+import { validateAndCreateUserEmailFactory } from '@/modules/core/services/userEmails'
+import { finalizeInvitedServerRegistrationFactory } from '@/modules/serverinvites/services/processing'
+import {
   deleteServerOnlyInvitesFactory,
   updateAllInviteTargetsFactory,
   deleteAllUserInvitesFactory
-} = require('@/modules/serverinvites/repositories/serverInvites')
-const {
+} from '@/modules/serverinvites/repositories/serverInvites'
+import {
   deleteStreamFactory,
   getUserDeletableStreamsFactory
-} = require('@/modules/core/repositories/streams')
-const { dbLogger } = require('@/logging/logging')
-const { getServerInfoFactory } = require('@/modules/core/repositories/server')
-const { getEventBus } = require('@/modules/shared/services/eventBus')
+} from '@/modules/core/repositories/streams'
+import { dbLogger } from '@/logging/logging'
+import { getServerInfoFactory } from '@/modules/core/repositories/server'
+import { getEventBus } from '@/modules/shared/services/eventBus'
+import { expect } from 'chai'
 
 const getUsers = legacyGetPaginatedUsersFactory({ db })
 const countUsers = legacyGetPaginatedUsersCountFactory({ db })
@@ -91,7 +82,8 @@ const deleteUser = deleteUserFactory({
   isLastAdminUser: isLastAdminUserFactory({ db }),
   getUserDeletableStreams: getUserDeletableStreamsFactory({ db }),
   deleteAllUserInvites: deleteAllUserInvitesFactory({ db }),
-  deleteUserRecord: deleteUserRecordFactory({ db })
+  deleteUserRecord: deleteUserRecordFactory({ db }),
+  emitEvent: getEventBus().emit
 })
 const getUserRole = getUserRoleFactory({ db })
 const buildChangeUserRole = (guestModeEnabled = false) =>
@@ -106,7 +98,8 @@ describe('User admin @user-services', () => {
   const myTestActor = {
     name: 'Gergo Jedlicska',
     email: 'gergo@jedlicska.com',
-    password: 'sn3aky-1337-b1m'
+    password: 'sn3aky-1337-b1m',
+    id: ''
   }
 
   before(async () => {
@@ -143,7 +136,7 @@ describe('User admin @user-services', () => {
 
   it('Get users query limit is sanitized to upper limit', async () => {
     const userInputs = Array(250)
-      .fill()
+      .fill(undefined)
       .map((v, i) => createNewDroid(i))
 
     expect(await countUsers()).to.equal(1)
@@ -178,7 +171,7 @@ describe('User admin @user-services', () => {
         await changeUserRole({ userId: myTestActor.id, role })
         assert.fail('This should have failed')
       } catch (err) {
-        expect(err.message).to.equal(`Invalid role specified: ${role}`)
+        expect(ensureError(err).message).to.equal(`Invalid role specified: ${role}`)
       }
     })
     it('throws if guest role not enabled, but trying to change user role to guest', async () => {
@@ -187,7 +180,7 @@ describe('User admin @user-services', () => {
         await changeUserRole({ userId: myTestActor.id, role })
         assert.fail('This should have failed')
       } catch (err) {
-        expect(err.message).to.equal('Guest role is not enabled')
+        expect(ensureError(err).message).to.equal('Guest role is not enabled')
       }
     })
     it('modifies role', async () => {
@@ -218,7 +211,7 @@ describe('User admin @user-services', () => {
         await changeUserRole({ userId: myTestActor.id, role: Roles.Server.User })
         assert.fail('This should have failed')
       } catch (err) {
-        expect(err.message).to.equal(
+        expect(ensureError(err).message).to.equal(
           'Cannot remove the last admin role from the server'
         )
       }
@@ -226,7 +219,7 @@ describe('User admin @user-services', () => {
   })
 })
 
-const createNewDroid = (number) => {
+const createNewDroid = (number: string | number) => {
   return {
     name: `${number}`,
     email: `${number}@droidarmy.com`,

packages/server/modules/shared/services/eventBus.ts

@@ -212,6 +212,7 @@ export function initializeEventBus() {
 export type EventBus = ReturnType<typeof initializeEventBus>
 export type EventBusPayloads = EventTypes
 export type EventBusEmit = EventBus['emit']
+export type EventBusListen = EventBus['listen']
 export type EmitArg = Parameters<EventBusEmit>[0]
 
 let eventBus: EventBus