Fix scheduled events inbox notifications (#8964)

* fix scheduled events inbox notifications

Signed-off-by: Nikolay Chunosov <Chunosov.N@gmail.com>

* fix format

Signed-off-by: Nikolay Chunosov <Chunosov.N@gmail.com>

* fix tests

Signed-off-by: Nikolay Chunosov <Chunosov.N@gmail.com>

* fix format

Signed-off-by: Nikolay Chunosov <Chunosov.N@gmail.com>

---------

Signed-off-by: Nikolay Chunosov <Chunosov.N@gmail.com>

services/calendar/pod-calendar-mailer/src/__tests__/handlers.test.ts

@@ -14,7 +14,7 @@
 //
 
 import { Event } from '@hcengineering/calendar'
-import { PersonId, Ref, WorkspaceUuid } from '@hcengineering/core'
+import { MeasureContext, PersonId, Ref, WorkspaceUuid } from '@hcengineering/core'
 import { Meeting, Room } from '@hcengineering/love'
 import { MeetingNotificationType } from '../notification'
 import { eventCreated, eventUpdated, eventDeleted, eventMixin } from '../handlers'
@@ -24,6 +24,10 @@ const ws = 'workspace-id' as WorkspaceUuid
 const meetingHost = 'meeting-host' as PersonId
 const meetingGuest = 'meeting-guest' as PersonId
 const room = 'room-id' as Ref<Room>
+const ctx = {
+  error: jest.fn(),
+  info: jest.fn()
+} as unknown as MeasureContext
 
 function eventFor (user: PersonId, props?: Partial<Event> | Partial<Meeting>): Event {
   return {
@@ -73,7 +77,7 @@ describe('queue message handlers', () => {
     test('there should not be notification when host creates event for himself', async () => {
       const event = eventFor(meetingHost)
 
-      await eventCreated(ws, { event, modifiedBy: meetingHost })
+      await eventCreated(ctx, ws, { event, modifiedBy: meetingHost })
 
       expect(createNotificationSpy).not.toHaveBeenCalled()
     })
@@ -81,7 +85,7 @@ describe('queue message handlers', () => {
     test('there should not be notification when host creates event for guest', async () => {
       const event = eventFor(meetingGuest)
 
-      await eventCreated(ws, { event, modifiedBy: meetingHost })
+      await eventCreated(ctx, ws, { event, modifiedBy: meetingHost })
 
       expect(createNotificationSpy).not.toHaveBeenCalled()
     })
@@ -91,9 +95,9 @@ describe('queue message handlers', () => {
       // A new event which is already a meeting is created for the new participant
       const event = eventFor(meetingGuest, { room })
 
-      await eventCreated(ws, { event, modifiedBy: meetingHost })
+      await eventCreated(ctx, ws, { event, modifiedBy: meetingHost })
 
-      expect(createNotificationSpy).toHaveBeenCalledWith(ws, MeetingNotificationType.Scheduled, event, meetingHost)
+      expect(createNotificationSpy).toHaveBeenCalledWith(ctx, ws, MeetingNotificationType.Scheduled, event, meetingHost)
     })
 
     test('there should not be notification when host creates meeting for guest in the past', async () => {
@@ -101,7 +105,7 @@ describe('queue message handlers', () => {
       // A new event which is already a meeting is created for the new participant
       const event = eventFor(meetingGuest, { room, date: pastDate() })
 
-      await eventCreated(ws, { event, modifiedBy: meetingHost })
+      await eventCreated(ctx, ws, { event, modifiedBy: meetingHost })
 
       expect(createNotificationSpy).not.toHaveBeenCalled()
     })
@@ -111,7 +115,7 @@ describe('queue message handlers', () => {
     test('there should be notification when host updates event for himself', async () => {
       const event = eventFor(meetingHost)
 
-      await eventUpdated(ws, { event, modifiedBy: meetingHost, changes: { date: Date.now() } })
+      await eventUpdated(ctx, ws, { event, modifiedBy: meetingHost, changes: { date: Date.now() } })
 
       expect(createNotificationSpy).not.toHaveBeenCalled()
     })
@@ -119,7 +123,7 @@ describe('queue message handlers', () => {
     test('there should not be notification when host updates event for guest', async () => {
       const event = eventFor(meetingGuest)
 
-      await eventUpdated(ws, { event, modifiedBy: meetingHost, changes: { date: Date.now() } })
+      await eventUpdated(ctx, ws, { event, modifiedBy: meetingHost, changes: { date: Date.now() } })
 
       expect(createNotificationSpy).not.toHaveBeenCalled()
     })
@@ -127,15 +131,21 @@ describe('queue message handlers', () => {
     test('there should be notification when host updates meeting for guest', async () => {
       const event = eventFor(meetingGuest, { room })
 
-      await eventUpdated(ws, { event, modifiedBy: meetingHost, changes: { date: Date.now() } })
+      await eventUpdated(ctx, ws, { event, modifiedBy: meetingHost, changes: { date: Date.now() } })
 
-      expect(createNotificationSpy).toHaveBeenCalledWith(ws, MeetingNotificationType.Rescheduled, event, meetingHost)
+      expect(createNotificationSpy).toHaveBeenCalledWith(
+        ctx,
+        ws,
+        MeetingNotificationType.Rescheduled,
+        event,
+        meetingHost
+      )
     })
 
     test('there should be notification when host updates meeting for guest in the past', async () => {
       const event = eventFor(meetingGuest, { room, date: pastDate() })
 
-      await eventUpdated(ws, { event, modifiedBy: meetingHost, changes: { date: Date.now() } })
+      await eventUpdated(ctx, ws, { event, modifiedBy: meetingHost, changes: { date: Date.now() } })
 
       expect(createNotificationSpy).not.toHaveBeenCalled()
     })
@@ -143,7 +153,7 @@ describe('queue message handlers', () => {
     test('there should not be notification when host updates meeting for himself', async () => {
       const event = eventFor(meetingHost, { room })
 
-      await eventUpdated(ws, { event, modifiedBy: meetingHost, changes: { date: Date.now() } })
+      await eventUpdated(ctx, ws, { event, modifiedBy: meetingHost, changes: { date: Date.now() } })
 
       expect(createNotificationSpy).not.toHaveBeenCalled()
     })
@@ -153,7 +163,7 @@ describe('queue message handlers', () => {
     test('there should not be notification when host deletes event for himself', async () => {
       const event = eventFor(meetingHost)
 
-      await eventDeleted(ws, { event, modifiedBy: meetingHost })
+      await eventDeleted(ctx, ws, { event, modifiedBy: meetingHost })
 
       expect(createNotificationSpy).not.toHaveBeenCalled()
     })
@@ -161,7 +171,7 @@ describe('queue message handlers', () => {
     test('there should not be notification when host deletes event for guest', async () => {
       const event = eventFor(meetingGuest)
 
-      await eventDeleted(ws, { event, modifiedBy: meetingHost })
+      await eventDeleted(ctx, ws, { event, modifiedBy: meetingHost })
 
       expect(createNotificationSpy).not.toHaveBeenCalled()
     })
@@ -169,15 +179,15 @@ describe('queue message handlers', () => {
     test('there should be notification when host deletes meeting for guest', async () => {
       const event = eventFor(meetingGuest, { room })
 
-      await eventDeleted(ws, { event, modifiedBy: meetingHost })
+      await eventDeleted(ctx, ws, { event, modifiedBy: meetingHost })
 
-      expect(createNotificationSpy).toHaveBeenCalledWith(ws, MeetingNotificationType.Canceled, event, meetingHost)
+      expect(createNotificationSpy).toHaveBeenCalledWith(ctx, ws, MeetingNotificationType.Canceled, event, meetingHost)
     })
 
     test('there should not be notification when host deletes meeting for guest in the past', async () => {
       const event = eventFor(meetingGuest, { room, date: pastDate() })
 
-      await eventDeleted(ws, { event, modifiedBy: meetingHost })
+      await eventDeleted(ctx, ws, { event, modifiedBy: meetingHost })
 
       expect(createNotificationSpy).not.toHaveBeenCalled()
     })
@@ -185,7 +195,7 @@ describe('queue message handlers', () => {
     test('there should not be notification when host deletes meeting for himself', async () => {
       const event = eventFor(meetingHost, { room })
 
-      await eventDeleted(ws, { event, modifiedBy: meetingHost })
+      await eventDeleted(ctx, ws, { event, modifiedBy: meetingHost })
 
       expect(createNotificationSpy).not.toHaveBeenCalled()
     })
@@ -195,27 +205,27 @@ describe('queue message handlers', () => {
     test('there should not be notification when event not created before mixin', async () => {
       const event = eventFor(meetingGuest)
 
-      await eventMixin(ws, { event, modifiedBy: meetingHost, changes: { room } })
+      await eventMixin(ctx, ws, { event, modifiedBy: meetingHost, changes: { room } })
 
       expect(createNotificationSpy).not.toHaveBeenCalled()
     })
 
     test('there should be notification when host create meeting for guest', async () => {
       const event0 = eventFor(meetingGuest)
-      await eventCreated(ws, { event: event0, modifiedBy: meetingHost })
+      await eventCreated(ctx, ws, { event: event0, modifiedBy: meetingHost })
 
       const event = eventFor(meetingGuest, { room })
-      await eventMixin(ws, { event, modifiedBy: meetingHost, changes: { room } })
+      await eventMixin(ctx, ws, { event, modifiedBy: meetingHost, changes: { room } })
 
-      expect(createNotificationSpy).toHaveBeenCalledWith(ws, MeetingNotificationType.Scheduled, event, meetingHost)
+      expect(createNotificationSpy).toHaveBeenCalledWith(ctx, ws, MeetingNotificationType.Scheduled, event, meetingHost)
     })
 
     test('there should not be notification when host created meeting for himself', async () => {
       const event0 = eventFor(meetingHost)
-      await eventCreated(ws, { event: event0, modifiedBy: meetingHost })
+      await eventCreated(ctx, ws, { event: event0, modifiedBy: meetingHost })
 
       const event = eventFor(meetingHost, { room })
-      await eventMixin(ws, { event, modifiedBy: meetingHost, changes: { room } })
+      await eventMixin(ctx, ws, { event, modifiedBy: meetingHost, changes: { room } })
 
       expect(createNotificationSpy).not.toHaveBeenCalled()
     })

services/calendar/pod-calendar-mailer/src/handlers.ts

@@ -13,7 +13,7 @@
 // limitations under the License.
 //
 import { Event } from '@hcengineering/calendar'
-import { Data, WorkspaceUuid } from '@hcengineering/core'
+import { Data, MeasureContext, WorkspaceUuid } from '@hcengineering/core'
 import { createNotification, MeetingNotificationType } from './notification'
 import { type EventCUDMessage } from './types'
 import { isMeeting } from './utils'
@@ -36,81 +36,101 @@ function addRecentEvent (eventId: string): void {
 }
 
 export async function eventCreated (
+  ctx: MeasureContext,
   workspaceUuid: WorkspaceUuid,
   message: Omit<EventCUDMessage, 'action'>
-): Promise<void> {
+): Promise<string | undefined> {
   const { event, modifiedBy } = message
 
   // TODO: move emailing logic from huly-schedule here
 
-  if (event.date <= Date.now()) return
+  if (event.date <= Date.now()) {
+    return 'Event is in the past'
+  }
+  if (modifiedBy === event.user) {
+    return 'Event modified by the user'
+  }
 
-  if (modifiedBy !== event.user) {
+  if (await isMeeting(workspaceUuid, event)) {
-    if (await isMeeting(workspaceUuid, event)) {
+    // This happens when the host adds a new participant to the existing meeting
-      // This happens when the host adds a new participant to the existing meeting
+    // A new event which is already a meeting is created for the new participant
-      // A new event which is already a meeting is created for the new participant
+    await createNotification(ctx, workspaceUuid, MeetingNotificationType.Scheduled, event, modifiedBy)
-      await createNotification(workspaceUuid, MeetingNotificationType.Scheduled, event, modifiedBy)
+  } else {
-    } else {
+    // Don't create notifications for reguar events, only for meetings
-      // Don't create notifications for reguar events, only for meetings
+    // But the event will be marked as a meeting in a separate call
-      // But the event will be marked as a meeting in a separate call
+    // immediately after creation, in the "mixin" message
-      // immediately after creation, in the "mixin" message
+    addRecentEvent(event._id)
-      addRecentEvent(event._id)
-    }
   }
 }
 
 export async function eventUpdated (
+  ctx: MeasureContext,
   workspaceUuid: WorkspaceUuid,
   message: Omit<EventCUDMessage, 'action'>
-): Promise<void> {
+): Promise<string | undefined> {
   const { event, modifiedBy } = message
 
   // TODO: if the event was created via huly-schedule, we need to send an email
 
-  if (event.date <= Date.now()) return
+  if (event.date <= Date.now()) {
-
+    return 'Event is in the past'
-  if (modifiedBy !== event.user) {
-    if (await isMeeting(workspaceUuid, event)) {
-      const changes = message.changes as Partial<Data<Event>>
-      if (changes.date !== undefined) {
-        await createNotification(workspaceUuid, MeetingNotificationType.Rescheduled, event, modifiedBy)
-      }
-    }
   }
+  if (modifiedBy === event.user) {
+    return 'Event modified by the user'
+  }
+  if (!(await isMeeting(workspaceUuid, event))) {
+    return 'Event is not a meeting'
+  }
+
+  const changes = message.changes as Partial<Data<Event>>
+  if (changes.date === undefined) {
+    return 'Event date not changed'
+  }
+
+  await createNotification(ctx, workspaceUuid, MeetingNotificationType.Rescheduled, event, modifiedBy)
 }
 
 export async function eventDeleted (
+  ctx: MeasureContext,
   workspaceUuid: WorkspaceUuid,
   message: Omit<EventCUDMessage, 'action'>
-): Promise<void> {
+): Promise<string | undefined> {
   const { event, modifiedBy } = message
 
   // TODO: if the event was created via huly-schedule, we need to send an email
 
-  if (event.date <= Date.now()) return
+  if (event.date <= Date.now()) {
-
+    return 'Event is in the past'
-  if (modifiedBy !== event.user) {
-    if (await isMeeting(workspaceUuid, event)) {
-      await createNotification(workspaceUuid, MeetingNotificationType.Canceled, event, modifiedBy)
-    }
   }
+  if (modifiedBy === event.user) {
+    return 'Event modified by the user'
+  }
+  if (!(await isMeeting(workspaceUuid, event))) {
+    return 'Event is not a meeting'
+  }
+
+  await createNotification(ctx, workspaceUuid, MeetingNotificationType.Canceled, event, modifiedBy)
 }
 
 export async function eventMixin (
+  ctx: MeasureContext,
   workspaceUuid: WorkspaceUuid,
   message: Omit<EventCUDMessage, 'action'>
-): Promise<void> {
+): Promise<string | undefined> {
   const { event, modifiedBy } = message
 
   // TODO: move emailing logic from huly-schedule here
 
-  if (modifiedBy !== event.user) {
+  if (modifiedBy === event.user) {
-    if (recentlyCreatedEvents.has(event._id)) {
+    return 'Event modified by the user'
-      recentlyCreatedEvents.delete(event._id)
-
-      if (await isMeeting(workspaceUuid, event)) {
-        await createNotification(workspaceUuid, MeetingNotificationType.Scheduled, event, modifiedBy)
-      }
-    }
   }
+  if (!recentlyCreatedEvents.has(event._id)) {
+    return 'Event not found in recent events'
+  }
+  if (!(await isMeeting(workspaceUuid, event))) {
+    return 'Event is not a meeting'
+  }
+
+  recentlyCreatedEvents.delete(event._id)
+  await createNotification(ctx, workspaceUuid, MeetingNotificationType.Scheduled, event, modifiedBy)
 }

services/calendar/pod-calendar-mailer/src/index.ts

@@ -55,23 +55,34 @@ async function main (): Promise<void> {
         const ws = message.id as WorkspaceUuid
         const records = message.value
         for (const record of records) {
+          ctx.info('Processing event', {
+            ws,
+            action: record.action,
+            eventId: record.event.eventId,
+            objectId: record.event._id,
+            modifiedBy: record.modifiedBy
+          })
           try {
+            let skipReason
             switch (record.action) {
               case 'create':
-                await eventCreated(ws, record)
+                skipReason = await eventCreated(ctx, ws, record)
                 break
               case 'update':
-                await eventUpdated(ws, record)
+                skipReason = await eventUpdated(ctx, ws, record)
                 break
               case 'delete':
-                await eventDeleted(ws, record)
+                skipReason = await eventDeleted(ctx, ws, record)
                 break
               case 'mixin':
-                await eventMixin(ws, record)
+                skipReason = await eventMixin(ctx, ws, record)
                 break
             }
+            if (skipReason !== undefined) {
+              ctx.info('Notification skipped', { reason: skipReason, objectId: record.event._id })
+            }
           } catch (error) {
-            ctx.error('Error processing message', { error, ws, record })
+            ctx.error('Error processing event', { error, ws, record })
           }
         }
       }

services/calendar/pod-calendar-mailer/src/notification.ts

@@ -14,7 +14,7 @@
 //
 import calendar, { Event } from '@hcengineering/calendar'
 import contact from '@hcengineering/contact'
-import { AccountUuid, Doc, PersonId, Ref, Space, WorkspaceUuid } from '@hcengineering/core'
+import { AccountUuid, Doc, MeasureContext, PersonId, Ref, Space, WorkspaceUuid } from '@hcengineering/core'
 import notification from '@hcengineering/notification'
 import { IntlString } from '@hcengineering/platform'
 import { getClient } from './utils'
@@ -32,6 +32,7 @@ const notificationMessages: Record<MeetingNotificationType, IntlString> = {
 }
 
 export async function createNotification (
+  ctx: MeasureContext,
   workspaceUuid: WorkspaceUuid,
   type: MeetingNotificationType,
   forEvent: Event,
@@ -102,4 +103,11 @@ export async function createNotification (
     archived: false,
     docNotifyContext: docNotifyContextId
   })
+
+  ctx.info('Notification created', {
+    personUuid,
+    eventId: forEvent.eventId,
+    objectId: forEvent._id,
+    spaceId: space._id
+  })
 }

services/calendar/pod-calendar-mailer/src/utils.ts

@@ -15,7 +15,7 @@
 import { getClient as getAccountClient, AccountClient } from '@hcengineering/account-client'
 import { createRestTxOperations } from '@hcengineering/api-client'
 import { Event } from '@hcengineering/calendar'
-import { Hierarchy, PersonId, systemAccountUuid, TxOperations, WorkspaceUuid } from '@hcengineering/core'
+import core, { Hierarchy, PersonId, systemAccountUuid, TxOperations, WorkspaceUuid } from '@hcengineering/core'
 import love from '@hcengineering/love'
 import { generateToken } from '@hcengineering/server-token'
 import config from './config'
@@ -27,7 +27,7 @@ export async function getClient (
   const token = generateToken(systemAccountUuid, workspaceUuid)
   let accountClient = getAccountClient(config.accountsUrl, token)
 
-  if (socialId !== undefined) {
+  if (socialId !== undefined && socialId !== core.account.System) {
     const personUuid = await accountClient.findPersonBySocialId(socialId, true)
     if (personUuid === undefined) {
       throw new Error('Global person not found')