From 27c723bd56acaf4a8eca4891f84adf731386bac9 Mon Sep 17 00:00:00 2001 From: Ricardo Garim Date: Tue, 25 Mar 2025 06:59:23 -0300 Subject: [PATCH 1/8] refactor: improves status change schedule --- .../server/configuration/outlookCalendar.ts | 1 + .../server/services/calendar/service.ts | 166 ++++++++++++++++-- .../setupAppointmentStatusChange.ts | 12 +- .../src/types/ICalendarService.ts | 1 + .../src/models/ICalendarEventModel.ts | 3 + packages/models/src/models/CalendarEvent.ts | 53 ++++++ 6 files changed, 219 insertions(+), 17 deletions(-) diff --git a/apps/meteor/ee/server/configuration/outlookCalendar.ts b/apps/meteor/ee/server/configuration/outlookCalendar.ts index 67c8d79450300..b7a60e655e052 100644 --- a/apps/meteor/ee/server/configuration/outlookCalendar.ts +++ b/apps/meteor/ee/server/configuration/outlookCalendar.ts @@ -9,5 +9,6 @@ Meteor.startup(() => addSettings(); await Calendar.setupNextNotification(); + await Calendar.setupNextStatusChange(); }), ); diff --git a/apps/meteor/server/services/calendar/service.ts b/apps/meteor/server/services/calendar/service.ts index 80f6d7aac6a7e..006a7f6c2d6d9 100644 --- a/apps/meteor/server/services/calendar/service.ts +++ b/apps/meteor/server/services/calendar/service.ts @@ -5,12 +5,13 @@ import { UserStatus } from '@rocket.chat/core-typings'; import { cronJobs } from '@rocket.chat/cron'; import { Logger } from '@rocket.chat/logger'; import type { InsertionModel } from '@rocket.chat/model-typings'; -import { CalendarEvent } from '@rocket.chat/models'; +import { CalendarEvent, Users } from '@rocket.chat/models'; import type { UpdateResult, DeleteResult } from 'mongodb'; +import { applyStatusChange } from './statusEvents/applyStatusChange'; import { cancelUpcomingStatusChanges } from './statusEvents/cancelUpcomingStatusChanges'; +import { generateCronJobId } from './statusEvents/generateCronJobId'; import { removeCronJobs } from './statusEvents/removeCronJobs'; -import { setupAppointmentStatusChange } from './statusEvents/setupAppointmentStatusChange'; import { getShiftedTime } from './utils/getShiftedTime'; import { settings } from '../../../app/settings/server'; import { getUserPreference } from '../../../app/utils/server/lib/getUserPreference'; @@ -22,6 +23,11 @@ const defaultMinutesForNotifications = 5; export class CalendarService extends ServiceClassInternal implements ICalendarService { protected name = 'calendar'; + constructor() { + super(); + void this.setupNextStatusChange(); + } + public async create(data: Omit, 'reminderTime' | 'notificationSent'>): Promise { const { uid, startTime, endTime, subject, description, reminderMinutesBeforeStart, meetingUrl, busy } = data; const minutes = reminderMinutesBeforeStart ?? defaultMinutesForNotifications; @@ -43,7 +49,7 @@ export class CalendarService extends ServiceClassInternal implements ICalendarSe const insertResult = await CalendarEvent.insertOne(insertData); await this.setupNextNotification(); if (busy !== false) { - await setupAppointmentStatusChange(insertResult.insertedId, uid, startTime, endTime, UserStatus.BUSY, true); + await this.setupNextStatusChange(); } return insertResult.insertedId; @@ -82,8 +88,9 @@ export class CalendarService extends ServiceClassInternal implements ICalendarSe await this.setupNextNotification(); if (busy !== false) { - await setupAppointmentStatusChange(insertResult.insertedId, uid, startTime, endTime, UserStatus.BUSY, true); + await this.setupNextStatusChange(); } + return insertResult.insertedId; } @@ -91,7 +98,7 @@ export class CalendarService extends ServiceClassInternal implements ICalendarSe if (updateResult.modifiedCount > 0) { await this.setupNextNotification(); if (busy !== false) { - await setupAppointmentStatusChange(event._id, uid, startTime, endTime, UserStatus.BUSY, true); + await this.setupNextStatusChange(); } } @@ -135,16 +142,9 @@ export class CalendarService extends ServiceClassInternal implements ICalendarSe if (startTime || endTime) { await removeCronJobs(eventId, event.uid); - const isBusy = busy !== undefined ? busy : event.busy !== false; if (isBusy) { - const effectiveStartTime = startTime || event.startTime; - const effectiveEndTime = endTime || event.endTime; - - // Only proceed if we have both valid start and end times - if (effectiveStartTime && effectiveEndTime) { - await setupAppointmentStatusChange(eventId, event.uid, effectiveStartTime, effectiveEndTime, UserStatus.BUSY, true); - } + await this.setupNextStatusChange(); } } } @@ -158,15 +158,26 @@ export class CalendarService extends ServiceClassInternal implements ICalendarSe await removeCronJobs(eventId, event.uid); } - return CalendarEvent.deleteOne({ + const result = await CalendarEvent.deleteOne({ _id: eventId, }); + + if (result.deletedCount > 0) { + await this.setupNextStatusChange(); + } + + return result; } public async setupNextNotification(): Promise { return this.doSetupNextNotification(false); } + public async setupNextStatusChange(): Promise { + console.log('setupNextStatusChange'); + return this.doSetupNextStatusChange(); + } + public async cancelUpcomingStatusChanges(uid: IUser['_id'], endTime = new Date()): Promise { return cancelUpcomingStatusChanges(uid, endTime); } @@ -200,6 +211,133 @@ export class CalendarService extends ServiceClassInternal implements ICalendarSe await cronJobs.addAtTimestamp('calendar-reminders', date, async () => this.sendCurrentNotifications(date)); } + private async doSetupNextStatusChange(): Promise { + // This method is called in the following moments: + // 1. When a new busy event is created or imported + // 2. When a busy event is updated (time/busy status changes) + // 3. When a busy event is deleted + // 4. When a status change job executes and completes + // 5. When an event ends and the status is restored + // 6. From Outlook Calendar integration (ee/server/configuration/outlookCalendar.ts) + + const busyStatusEnabled = settings.get('Calendar_BusyStatus_Enabled'); + if (!busyStatusEnabled) { + const chainJobId = 'calendar-next-status-change'; + if (await cronJobs.has(chainJobId)) { + await cronJobs.remove(chainJobId); + } + return; + } + + const now = new Date(); + const startOfNextMinute = new Date(now); + startOfNextMinute.setSeconds(0, 0); + startOfNextMinute.setMinutes(startOfNextMinute.getMinutes() + 1); + + const endOfNextMinute = new Date(startOfNextMinute); + endOfNextMinute.setMinutes(startOfNextMinute.getMinutes() + 1); + + const eventsToScheduleNow = await CalendarEvent.findEventsToScheduleNow(now, endOfNextMinute).toArray(); + const nextFutureEvent = await CalendarEvent.findNextFutureEvent(endOfNextMinute); + + // PART 1: SCHEDULE ALL EVENTS STARTING SOON (NOW THROUGH NEXT MINUTE) + // Schedule all events that need to be scheduled now + for await (const event of eventsToScheduleNow) { + if (!event.endTime) { + continue; + } + + const startCronJobId = generateCronJobId(event._id, event.uid, 'status'); + if (await cronJobs.has(startCronJobId)) { + continue; + } + + await cronJobs.addAtTimestamp(startCronJobId, event.startTime, async () => { + const user = await Users.findOneById(event.uid, { projection: { status: 1 } }); + if (!user) { + return; + } + + const previousStatus = user.status; + await applyStatusChange({ + eventId: event._id, + uid: event.uid, + startTime: event.startTime, + endTime: event.endTime, + status: UserStatus.BUSY, + shouldScheduleRemoval: true, + }); + + const endCronJobId = generateCronJobId(event._id, event.uid, 'status'); + + if (await cronJobs.has(endCronJobId)) { + await cronJobs.remove(endCronJobId); + } + + if (!event.endTime) { + return; + } + + await cronJobs.addAtTimestamp(endCronJobId, event.endTime, async () => { + await applyStatusChange({ + eventId: event._id, + uid: event.uid, + startTime: event.startTime, + endTime: event.endTime, + status: previousStatus, + shouldScheduleRemoval: false, + }); + await this.doSetupNextStatusChange(); + }); + }); + } + + // PART 2: ENSURE IN-PROGRESS EVENTS HAVE SCHEDULED END TIMES + // This handles cases like: + // - Service restarts during an event + // - Events imported while in progress + // - Any missed end-time schedules + const inProgressEvents = await CalendarEvent.findInProgressEvents(now).toArray(); + for await (const event of inProgressEvents) { + if (!event.endTime) { + continue; + } + + const endCronJobId = generateCronJobId(event._id, event.uid, 'status'); + if (!(await cronJobs.has(endCronJobId))) { + const user = await Users.findOneById(event.uid, { projection: { status: 1 } }); + if (!user) { + continue; + } + + await cronJobs.addAtTimestamp(endCronJobId, event.endTime, async () => { + await applyStatusChange({ + eventId: event._id, + uid: event.uid, + startTime: event.startTime, + endTime: event.endTime, + status: user.status === UserStatus.BUSY ? UserStatus.ONLINE : user.status, + shouldScheduleRemoval: false, + }); + await this.doSetupNextStatusChange(); + }); + } + } + + // PART 3: SCHEDULE THE NEXT CHECK + // Set up the chain to the next check + const chainJobId = 'calendar-next-status-change'; + + if (await cronJobs.has(chainJobId)) { + await cronJobs.remove(chainJobId); + } + + const nextCheckTime = nextFutureEvent?.startTime || endOfNextMinute; + await cronJobs.addAtTimestamp(chainJobId, nextCheckTime, async () => { + await this.doSetupNextStatusChange(); + }); + } + private async sendCurrentNotifications(date: Date): Promise { const events = await CalendarEvent.findEventsToNotify(date, 1).toArray(); for await (const event of events) { diff --git a/apps/meteor/server/services/calendar/statusEvents/setupAppointmentStatusChange.ts b/apps/meteor/server/services/calendar/statusEvents/setupAppointmentStatusChange.ts index 907f2a6bf2c66..0b818a23dedf1 100644 --- a/apps/meteor/server/services/calendar/statusEvents/setupAppointmentStatusChange.ts +++ b/apps/meteor/server/services/calendar/statusEvents/setupAppointmentStatusChange.ts @@ -33,7 +33,13 @@ export async function setupAppointmentStatusChange( await cronJobs.remove(cronJobId); } - await cronJobs.addAtTimestamp(cronJobId, scheduledTime, async () => - applyStatusChange({ eventId, uid, startTime, endTime, status, shouldScheduleRemoval }), - ); + await cronJobs.addAtTimestamp(cronJobId, scheduledTime, async () => { + await applyStatusChange({ eventId, uid, startTime, endTime, status, shouldScheduleRemoval }); + + if (!shouldScheduleRemoval) { + if (await cronJobs.has('calendar-next-status-change')) { + await cronJobs.remove('calendar-next-status-change'); + } + } + }); } diff --git a/packages/core-services/src/types/ICalendarService.ts b/packages/core-services/src/types/ICalendarService.ts index f74b63b056b81..a7744ddd7c12d 100644 --- a/packages/core-services/src/types/ICalendarService.ts +++ b/packages/core-services/src/types/ICalendarService.ts @@ -10,5 +10,6 @@ export interface ICalendarService { update(eventId: ICalendarEvent['_id'], data: Partial): Promise; delete(eventId: ICalendarEvent['_id']): Promise; setupNextNotification(): Promise; + setupNextStatusChange(): Promise; cancelUpcomingStatusChanges(uid: IUser['_id'], endTime?: Date): Promise; } diff --git a/packages/model-typings/src/models/ICalendarEventModel.ts b/packages/model-typings/src/models/ICalendarEventModel.ts index 24c4bc30ac0a3..019c12ac59d18 100644 --- a/packages/model-typings/src/models/ICalendarEventModel.ts +++ b/packages/model-typings/src/models/ICalendarEventModel.ts @@ -15,4 +15,7 @@ export interface ICalendarEventModel extends IBaseModel { ): Promise; findOverlappingEvents(eventId: ICalendarEvent['_id'], uid: IUser['_id'], startTime: Date, endTime: Date): FindCursor; findEligibleEventsForCancelation(uid: IUser['_id'], endTime: Date): FindCursor; + findEventsToScheduleNow(now: Date, endTime: Date): FindCursor; + findNextFutureEvent(startTime: Date): Promise; + findInProgressEvents(now: Date): FindCursor; } diff --git a/packages/models/src/models/CalendarEvent.ts b/packages/models/src/models/CalendarEvent.ts index 4de0c4a638e1a..0972846ea1a27 100644 --- a/packages/models/src/models/CalendarEvent.ts +++ b/packages/models/src/models/CalendarEvent.ts @@ -149,4 +149,57 @@ export class CalendarEventRaw extends BaseRaw implements ICalend endTime: { $exists: true, $gte: endTime }, }); } + + public findEventsToScheduleNow(now: Date, endTime: Date): FindCursor { + return this.find( + { + startTime: { $gt: now, $lt: endTime }, + busy: { $ne: false }, + endTime: { $exists: true }, + }, + { + sort: { startTime: 1 }, + projection: { + _id: 1, + uid: 1, + startTime: 1, + endTime: 1, + }, + }, + ); + } + + public async findNextFutureEvent(startTime: Date): Promise { + return this.findOne( + { + startTime: { $gte: startTime }, + busy: { $ne: false }, + endTime: { $exists: true }, + }, + { + sort: { startTime: 1 }, + projection: { + startTime: 1, + }, + }, + ); + } + + public findInProgressEvents(now: Date): FindCursor { + return this.find( + { + startTime: { $lt: now }, + endTime: { $gt: now }, + busy: { $ne: false }, + }, + { + projection: { + _id: 1, + uid: 1, + startTime: 1, + endTime: 1, + }, + }, + ); + } } From d051493c04f4b24bb5a8f490f4ea6dd9763cea32 Mon Sep 17 00:00:00 2001 From: Ricardo Garim Date: Tue, 25 Mar 2025 08:49:14 -0300 Subject: [PATCH 2/8] test: removes unneeded unit tests --- .../server/services/calendar/service.ts | 1 - .../server/services/calendar/service.tests.ts | 438 ++++-------------- 2 files changed, 94 insertions(+), 345 deletions(-) diff --git a/apps/meteor/server/services/calendar/service.ts b/apps/meteor/server/services/calendar/service.ts index 006a7f6c2d6d9..9ddd22dd883df 100644 --- a/apps/meteor/server/services/calendar/service.ts +++ b/apps/meteor/server/services/calendar/service.ts @@ -174,7 +174,6 @@ export class CalendarService extends ServiceClassInternal implements ICalendarSe } public async setupNextStatusChange(): Promise { - console.log('setupNextStatusChange'); return this.doSetupNextStatusChange(); } diff --git a/apps/meteor/tests/unit/server/services/calendar/service.tests.ts b/apps/meteor/tests/unit/server/services/calendar/service.tests.ts index d6fc603089ce9..8ab6836965903 100644 --- a/apps/meteor/tests/unit/server/services/calendar/service.tests.ts +++ b/apps/meteor/tests/unit/server/services/calendar/service.tests.ts @@ -21,12 +21,20 @@ const CalendarEventMock = { findEventsToNotify: sinon.stub(), flagNotificationSent: sinon.stub(), findOneByExternalIdAndUserId: sinon.stub(), + findEventsToScheduleNow: sinon.stub(), + findNextFutureEvent: sinon.stub(), + findInProgressEvents: sinon.stub(), +}; + +const UsersMock = { + findOne: sinon.stub(), }; const statusEventManagerMock = { setupAppointmentStatusChange: sinon.stub().resolves(), removeCronJobs: sinon.stub().resolves(), cancelUpcomingStatusChanges: sinon.stub().resolves(), + applyStatusChange: sinon.stub().resolves(), }; const getUserPreferenceMock = sinon.stub(); @@ -35,10 +43,11 @@ const serviceMocks = { './statusEvents/cancelUpcomingStatusChanges': { cancelUpcomingStatusChanges: statusEventManagerMock.cancelUpcomingStatusChanges }, './statusEvents/removeCronJobs': { removeCronJobs: statusEventManagerMock.removeCronJobs }, './statusEvents/setupAppointmentStatusChange': { setupAppointmentStatusChange: statusEventManagerMock.setupAppointmentStatusChange }, + './statusEvents/applyStatusChange': { applyStatusChange: statusEventManagerMock.applyStatusChange }, '../../../app/settings/server': { settings: settingsMock }, '@rocket.chat/core-services': { api, ServiceClassInternal: class {} }, '@rocket.chat/cron': { cronJobs: cronJobsMock }, - '@rocket.chat/models': { CalendarEvent: CalendarEventMock }, + '@rocket.chat/models': { CalendarEvent: CalendarEventMock, Users: UsersMock }, '../../../app/utils/server/lib/getUserPreference': { getUserPreference: getUserPreferenceMock }, }; @@ -74,8 +83,10 @@ describe('CalendarService', () => { sandbox.stub(proto, 'sendEventNotification').resolves(); sandbox.stub(proto, 'sendCurrentNotifications').resolves(); sandbox.stub(proto, 'doSetupNextNotification').resolves(); + sandbox.stub(proto, 'doSetupNextStatusChange').resolves(); sandbox.stub(service, 'setupNextNotification').resolves(); + sandbox.stub(service, 'setupNextStatusChange').resolves(); } function setupCalendarEventMocks() { @@ -93,6 +104,13 @@ describe('CalendarService', () => { }), flagNotificationSent: sinon.stub().resolves(), findOneByExternalIdAndUserId: sinon.stub().resolves(null), + findEventsToScheduleNow: sinon.stub().returns({ + toArray: sinon.stub().resolves([]), + }), + findNextFutureEvent: sinon.stub().resolves(null), + findInProgressEvents: sinon.stub().returns({ + toArray: sinon.stub().resolves([]), + }), }; Object.assign(CalendarEventMock, freshMocks); @@ -147,34 +165,7 @@ describe('CalendarService', () => { reminderMinutesBeforeStart: 5, notificationSent: false, }); - sinon.assert.calledOnce(statusEventManagerMock.setupAppointmentStatusChange); - }); - - it('should create event without end time if not provided', async () => { - const eventData = { - uid: fakeUserId, - startTime: fakeStartTime, - subject: fakeSubject, - description: fakeDescription, - }; - - await service.create(eventData); - - expect(CalendarEventMock.insertOne.firstCall.args[0]).to.not.have.property('endTime'); - }); - - it('should use default reminder minutes if not provided', async () => { - const eventData = { - uid: fakeUserId, - startTime: fakeStartTime, - subject: fakeSubject, - description: fakeDescription, - }; - - await service.create(eventData); - - const insertedData = CalendarEventMock.insertOne.firstCall.args[0]; - expect(insertedData).to.have.property('reminderMinutesBeforeStart', 5); + sinon.assert.calledOnce(service.setupNextStatusChange); }); }); @@ -190,24 +181,7 @@ describe('CalendarService', () => { await service.import(eventData); sinon.assert.calledOnce(CalendarEventMock.insertOne); - sinon.assert.calledOnce(statusEventManagerMock.setupAppointmentStatusChange); - }); - - it('should create a new event if event with externalId not found', async () => { - const eventData = { - uid: fakeUserId, - startTime: fakeStartTime, - subject: fakeSubject, - description: fakeDescription, - externalId: fakeExternalId, - }; - - CalendarEventMock.findOneByExternalIdAndUserId.resolves(null); - - await service.import(eventData); - - sinon.assert.calledWith(CalendarEventMock.findOneByExternalIdAndUserId, fakeExternalId, fakeUserId); - sinon.assert.calledOnce(CalendarEventMock.insertOne); + sinon.assert.calledOnce(service.setupNextStatusChange); }); it('should update existing event if found by externalId', async () => { @@ -231,58 +205,6 @@ describe('CalendarService', () => { sinon.assert.calledOnce(CalendarEventMock.updateEvent); sinon.assert.notCalled(CalendarEventMock.insertOne); }); - - it('should extract meeting URL from description if not provided', async () => { - const eventData = { - uid: fakeUserId, - startTime: fakeStartTime, - subject: fakeSubject, - description: 'Description with callUrl=https://meet.test/123', - externalId: fakeExternalId, - }; - - const proto = Object.getPrototypeOf(service); - await service.import(eventData); - - sinon.assert.calledWith(proto.parseDescriptionForMeetingUrl as sinon.SinonStub, eventData.description); - }); - }); - - describe('#get', () => { - it('should retrieve a single event by ID', async () => { - const fakeEvent = { - _id: fakeEventId, - uid: fakeUserId, - startTime: fakeStartTime, - subject: fakeSubject, - }; - - CalendarEventMock.findOne.resolves(fakeEvent); - - const result = await service.get(fakeEventId); - - sinon.assert.calledWith(CalendarEventMock.findOne, { _id: fakeEventId }); - expect(result).to.equal(fakeEvent); - }); - }); - - describe('#list', () => { - it('should retrieve events for a user on a specific date', async () => { - const fakeEvents = [ - { _id: 'event1', uid: fakeUserId, startTime: fakeStartTime }, - { _id: 'event2', uid: fakeUserId, startTime: fakeStartTime }, - ]; - - CalendarEventMock.findByUserIdAndDate.returns({ - toArray: sinon.stub().resolves(fakeEvents), - }); - - const fakeDate = new Date('2025-01-01'); - const result = await service.list(fakeUserId, fakeDate); - - sinon.assert.calledWith(CalendarEventMock.findByUserIdAndDate, fakeUserId, fakeDate); - expect(result).to.equal(fakeEvents); - }); }); describe('#update', () => { @@ -307,14 +229,6 @@ describe('CalendarService', () => { sinon.assert.calledWith(CalendarEventMock.updateEvent, fakeEventId, sinon.match.has('subject', 'Updated Subject')); }); - it('should do nothing if event not found', async () => { - CalendarEventMock.findOne.resolves(null); - - await service.update(fakeEventId, { subject: 'New Subject' }); - - sinon.assert.notCalled(CalendarEventMock.updateEvent); - }); - it('should update cron jobs when start/end times change', async () => { const fakeEvent = { _id: fakeEventId, @@ -335,42 +249,7 @@ describe('CalendarService', () => { }); sinon.assert.calledOnce(statusEventManagerMock.removeCronJobs); - sinon.assert.calledOnce(statusEventManagerMock.setupAppointmentStatusChange); - }); - - it('should extract meeting URL from description if not provided', async () => { - const fakeEvent = { - _id: fakeEventId, - uid: fakeUserId, - startTime: fakeStartTime, - subject: fakeSubject, - }; - - CalendarEventMock.findOne.resolves(fakeEvent); - - const proto = Object.getPrototypeOf(service); - - await service.update(fakeEventId, { - description: 'Description with callUrl=https://meet.test/123', - }); - - sinon.assert.called(proto.parseDescriptionForMeetingUrl as sinon.SinonStub); - }); - - it('should setup next notification if event was modified', async () => { - const fakeEvent = { - _id: fakeEventId, - uid: fakeUserId, - startTime: fakeStartTime, - subject: fakeSubject, - }; - - CalendarEventMock.findOne.resolves(fakeEvent); - CalendarEventMock.updateEvent.resolves({ modifiedCount: 1 } as UpdateResult); - - await service.update(fakeEventId, { subject: 'New Subject' }); - - sinon.assert.calledOnce(service.setupNextNotification as sinon.SinonStub); + sinon.assert.calledOnce(service.setupNextStatusChange); }); }); @@ -390,15 +269,6 @@ describe('CalendarService', () => { sinon.assert.calledOnce(statusEventManagerMock.removeCronJobs); sinon.assert.calledWith(CalendarEventMock.deleteOne, { _id: fakeEventId }); }); - - it('should only delete the event if not found', async () => { - CalendarEventMock.findOne.resolves(null); - - await service.delete(fakeEventId); - - sinon.assert.notCalled(statusEventManagerMock.removeCronJobs); - sinon.assert.calledOnce(CalendarEventMock.deleteOne); - }); }); describe('#setupNextNotification', () => { @@ -421,30 +291,7 @@ describe('CalendarService', () => { }); }); - describe('#cancelUpcomingStatusChanges', () => { - it('should delegate to statusEventManager', async () => { - await service.cancelUpcomingStatusChanges(fakeUserId); - - sinon.assert.calledWith(statusEventManagerMock.cancelUpcomingStatusChanges, fakeUserId); - }); - - it('should pass custom end time if provided', async () => { - const customDate = new Date('2025-02-01'); - - await service.cancelUpcomingStatusChanges(fakeUserId, customDate); - - sinon.assert.calledWith(statusEventManagerMock.cancelUpcomingStatusChanges, fakeUserId, customDate); - }); - }); - describe('Private: parseDescriptionForMeetingUrl', () => { - it('should return undefined for empty description', async () => { - await testPrivateMethod(service, 'parseDescriptionForMeetingUrl', async (method) => { - const result = await method(''); - expect(result).to.be.undefined; - }); - }); - it('should extract URL from description with default pattern', async () => { await testPrivateMethod(service, 'parseDescriptionForMeetingUrl', async (method) => { const testDescription = 'Join at https://meet.example.com?callUrl=https://special-meeting.com/123'; @@ -452,139 +299,9 @@ describe('CalendarService', () => { expect(result).to.equal('https://special-meeting.com/123'); }); }); - - it('should return undefined if regex pattern is empty', async () => { - await testPrivateMethod(service, 'parseDescriptionForMeetingUrl', async (method) => { - settingsMock.set('Calendar_MeetingUrl_Regex', ''); - - const result = await method('Test description with no pattern match'); - expect(result).to.be.undefined; - }); - }); - - it('should handle URL decoding', async () => { - await testPrivateMethod(service, 'parseDescriptionForMeetingUrl', async (method) => { - const encodedUrl = 'Join meeting at link with callUrl%3Dhttps%3A%2F%2Fmeeting.example.com%2F123'; - const result = await method(encodedUrl); - expect(result).to.include('https://meeting.example.com/123'); - }); - }); - }); - - describe('Private: findImportedEvent', () => { - it('should call the model method with correct parameters', async () => { - await testPrivateMethod(service, 'findImportedEvent', async (method) => { - await method(fakeExternalId, fakeUserId); - sinon.assert.calledWith(CalendarEventMock.findOneByExternalIdAndUserId, fakeExternalId, fakeUserId); - }); - }); - - it('should return the event when found', async () => { - await testPrivateMethod(service, 'findImportedEvent', async (method) => { - const fakeEvent = { _id: fakeEventId, externalId: fakeExternalId, uid: fakeUserId }; - CalendarEventMock.findOneByExternalIdAndUserId.resolves(fakeEvent); - - const result = await method(fakeExternalId, fakeUserId); - expect(result).to.equal(fakeEvent); - }); - }); - - it('should return null when event not found', async () => { - await testPrivateMethod(service, 'findImportedEvent', async (method) => { - CalendarEventMock.findOneByExternalIdAndUserId.resolves(null); - - const result = await method(fakeExternalId, fakeUserId); - expect(result).to.be.null; - }); - }); - }); - - describe('Private: sendEventNotification', () => { - it('should not send notification if user preference is disabled', async () => { - await testPrivateMethod(service, 'sendEventNotification', async (method) => { - getUserPreferenceMock.resolves(false); - - const fakeEvent = { - _id: fakeEventId, - uid: fakeUserId, - startTime: fakeStartTime, - subject: fakeSubject, - }; - - await method(fakeEvent); - - sinon.assert.calledWith(getUserPreferenceMock, fakeUserId, 'notifyCalendarEvents'); - sinon.assert.notCalled(api.broadcast as sinon.SinonStub); - }); - }); - - it('should send notification with correct event data', async () => { - await testPrivateMethod(service, 'sendEventNotification', async (method) => { - getUserPreferenceMock.resolves(true); - - const fakeEvent = { - _id: fakeEventId, - uid: fakeUserId, - startTime: fakeStartTime, - subject: fakeSubject, - }; - - await method(fakeEvent); - - sinon.assert.calledWith( - api.broadcast as sinon.SinonStub, - 'notify.calendar', - fakeUserId, - sinon.match({ - title: fakeSubject, - payload: { _id: fakeEventId }, - }), - ); - }); - }); - }); - - describe('Private: sendCurrentNotifications', () => { - it('should send notification for all events and flag them as sent', async () => { - await testPrivateMethod(service, 'sendCurrentNotifications', async (method) => { - const proto = Object.getPrototypeOf(service); - (proto.sendEventNotification as sinon.SinonStub).restore(); - sandbox.stub(proto, 'sendEventNotification').resolves(); - - const fakeDate = new Date('2025-01-01T10:00:00Z'); - const fakeEvents = [ - { _id: 'event1', uid: fakeUserId, startTime: fakeStartTime }, - { _id: 'event2', uid: fakeUserId, startTime: fakeStartTime }, - ]; - - CalendarEventMock.findEventsToNotify.returns({ - toArray: sinon.stub().resolves(fakeEvents), - }); - - await method(fakeDate); - - sinon.assert.calledWith(CalendarEventMock.findEventsToNotify, fakeDate, 1); - sinon.assert.calledTwice(proto.sendEventNotification as sinon.SinonStub); - sinon.assert.calledTwice(CalendarEventMock.flagNotificationSent); - sinon.assert.calledWith(CalendarEventMock.flagNotificationSent, 'event1'); - sinon.assert.calledWith(CalendarEventMock.flagNotificationSent, 'event2'); - sinon.assert.calledOnceWithExactly(proto.doSetupNextNotification as sinon.SinonStub, true); - }); - }); }); describe('Private: doSetupNextNotification', () => { - it('should remove calendar-reminders cron job if no events found', async () => { - await testPrivateMethod(service, 'doSetupNextNotification', async (method) => { - CalendarEventMock.findNextNotificationDate.resolves(null); - cronJobsMock.jobNames.add('calendar-reminders'); - - await method(false); - - expect(cronJobsMock.jobNames.has('calendar-reminders')).to.false; - }); - }); - it('should schedule notifications at the next date', async () => { await testPrivateMethod(service, 'doSetupNextNotification', async (method) => { const nextDate = new Date('2025-01-01T10:00:00Z'); @@ -595,56 +312,89 @@ describe('CalendarService', () => { expect(cronJobsMock.jobNames.has('calendar-reminders')).to.true; }); }); + }); - it('should send current notifications if date is in the past', async () => { - await testPrivateMethod(service, 'doSetupNextNotification', async (method) => { - const proto = Object.getPrototypeOf(service); - (proto.sendCurrentNotifications as sinon.SinonStub).restore(); - sandbox.stub(proto, 'sendCurrentNotifications').resolves(); - - const pastDate = new Date(); - pastDate.setMinutes(pastDate.getMinutes() - 10); - CalendarEventMock.findNextNotificationDate.resolves(pastDate); - - await method(false); - - sinon.assert.calledWith(proto.sendCurrentNotifications as sinon.SinonStub, pastDate); - expect(cronJobsMock.jobNames.size).to.equal(0); + describe('Private: doSetupNextStatusChange', () => { + it('should not run when busy status setting is disabled', async () => { + await testPrivateMethod(service, 'doSetupNextStatusChange', async (method) => { + settingsMock.set('Calendar_BusyStatus_Enabled', false); + + const originalHas = cronJobsMock.has; + const originalRemove = cronJobsMock.remove; + const originalAddAtTimestamp = cronJobsMock.addAtTimestamp; + + const hasStub = sinon.stub().resolves(true); + const removeStub = sinon.stub().resolves(); + const addAtTimestampStub = sinon.stub().resolves(); + + cronJobsMock.has = hasStub; + cronJobsMock.remove = removeStub; + cronJobsMock.addAtTimestamp = addAtTimestampStub; + + try { + await method(); + sinon.assert.calledWith(hasStub, 'calendar-next-status-change'); + sinon.assert.calledWith(removeStub, 'calendar-next-status-change'); + sinon.assert.notCalled(addAtTimestampStub); + } finally { + cronJobsMock.has = originalHas; + cronJobsMock.remove = originalRemove; + cronJobsMock.addAtTimestamp = originalAddAtTimestamp; + } }); }); - it('should schedule future notifications even if date is in the past when recursive', async () => { - await testPrivateMethod(service, 'doSetupNextNotification', async (method) => { - const pastDate = new Date(); - pastDate.setMinutes(pastDate.getMinutes() - 10); - CalendarEventMock.findNextNotificationDate.resolves(pastDate); + it('should schedule jobs when busy status setting is enabled', async () => { + await testPrivateMethod(service, 'doSetupNextStatusChange', async (method) => { + settingsMock.set('Calendar_BusyStatus_Enabled', true); - await method(true); + const startOfNextMinute = new Date(); + startOfNextMinute.setSeconds(0, 0); + startOfNextMinute.setMinutes(startOfNextMinute.getMinutes() + 1); - sinon.assert.notCalled(service.sendCurrentNotifications as sinon.SinonStub); - expect(cronJobsMock.jobNames.size).to.equal(1); - }); - }); - }); + const endOfNextMinute = new Date(startOfNextMinute); + endOfNextMinute.setMinutes(startOfNextMinute.getMinutes() + 1); - describe('Overlapping events', () => { - it('should not set up status change if no endTime is provided when updating', async () => { - const fakeEvent = { - _id: fakeEventId, - uid: fakeUserId, - startTime: fakeStartTime, - subject: fakeSubject, - }; - - CalendarEventMock.findOne.resolves(fakeEvent); + const futureEvent = { + _id: 'future123', + uid: fakeUserId, + startTime: endOfNextMinute, + endTime: new Date(endOfNextMinute.getTime() + 3600000), + }; - await service.update(fakeEventId, { - subject: 'New Subject', + CalendarEventMock.findEventsToScheduleNow.returns({ + toArray: sinon.stub().resolves([]), + }); + CalendarEventMock.findNextFutureEvent.resolves(futureEvent); + + const originalHas = cronJobsMock.has; + const originalRemove = cronJobsMock.remove; + const originalAddAtTimestamp = cronJobsMock.addAtTimestamp; + + const hasStub = sinon.stub().resolves(false); + const removeStub = sinon.stub().resolves(); + const addAtTimestampStub = sinon.stub().resolves(); + + cronJobsMock.has = hasStub; + cronJobsMock.remove = removeStub; + cronJobsMock.addAtTimestamp = addAtTimestampStub; + + try { + await method(); + + sinon.assert.calledWith(hasStub, 'calendar-next-status-change'); + sinon.assert.notCalled(removeStub); + sinon.assert.calledWith(addAtTimestampStub, 'calendar-next-status-change', futureEvent.startTime, sinon.match.func); + } finally { + cronJobsMock.has = originalHas; + cronJobsMock.remove = originalRemove; + cronJobsMock.addAtTimestamp = originalAddAtTimestamp; + } }); - - sinon.assert.notCalled(statusEventManagerMock.setupAppointmentStatusChange); }); + }); + describe('Overlapping events', () => { it('should cancel upcoming status changes for a user', async () => { const customDate = new Date('2025-02-01'); From adfdc0d8e309f5f01cadb31df0b3aa98042dd6cb Mon Sep 17 00:00:00 2001 From: Ricardo Garim Date: Wed, 26 Mar 2025 09:46:38 -0300 Subject: [PATCH 3/8] improves scheduling strategy --- .../server/services/calendar/service.ts | 187 ++++++++---------- .../statusEvents/applyStatusChange.ts | 10 +- .../services/calendar/statusEvents/index.ts | 2 - .../server/services/calendar/service.tests.ts | 95 +++++++-- .../statusEvents/applyStatusChange.ts | 5 - .../setupAppointmentStatusChange.ts | 77 -------- packages/core-typings/src/ICalendarEvent.ts | 2 + .../src/models/ICalendarEventModel.ts | 2 + packages/models/src/models/CalendarEvent.ts | 40 +++- 9 files changed, 217 insertions(+), 203 deletions(-) delete mode 100644 apps/meteor/tests/unit/server/services/calendar/statusEvents/setupAppointmentStatusChange.ts diff --git a/apps/meteor/server/services/calendar/service.ts b/apps/meteor/server/services/calendar/service.ts index 9ddd22dd883df..a4e82d94e23d5 100644 --- a/apps/meteor/server/services/calendar/service.ts +++ b/apps/meteor/server/services/calendar/service.ts @@ -10,7 +10,6 @@ import type { UpdateResult, DeleteResult } from 'mongodb'; import { applyStatusChange } from './statusEvents/applyStatusChange'; import { cancelUpcomingStatusChanges } from './statusEvents/cancelUpcomingStatusChanges'; -import { generateCronJobId } from './statusEvents/generateCronJobId'; import { removeCronJobs } from './statusEvents/removeCronJobs'; import { getShiftedTime } from './utils/getShiftedTime'; import { settings } from '../../../app/settings/server'; @@ -23,11 +22,6 @@ const defaultMinutesForNotifications = 5; export class CalendarService extends ServiceClassInternal implements ICalendarService { protected name = 'calendar'; - constructor() { - super(); - void this.setupNextStatusChange(); - } - public async create(data: Omit, 'reminderTime' | 'notificationSent'>): Promise { const { uid, startTime, endTime, subject, description, reminderMinutesBeforeStart, meetingUrl, busy } = data; const minutes = reminderMinutesBeforeStart ?? defaultMinutesForNotifications; @@ -219,122 +213,115 @@ export class CalendarService extends ServiceClassInternal implements ICalendarSe // 5. When an event ends and the status is restored // 6. From Outlook Calendar integration (ee/server/configuration/outlookCalendar.ts) - const busyStatusEnabled = settings.get('Calendar_BusyStatus_Enabled'); - if (!busyStatusEnabled) { - const chainJobId = 'calendar-next-status-change'; - if (await cronJobs.has(chainJobId)) { - await cronJobs.remove(chainJobId); - } - return; + // const busyStatusEnabled = settings.get('Calendar_BusyStatus_Enabled'); + // if (!busyStatusEnabled) { + // const schedulerJobId = 'calendar-status-scheduler'; + // if (await cronJobs.has(schedulerJobId)) { + // await cronJobs.remove(schedulerJobId); + // } + // return; + // } + + const schedulerJobId = 'calendar-status-scheduler'; + if (await cronJobs.has(schedulerJobId)) { + await cronJobs.remove(schedulerJobId); } const now = new Date(); - const startOfNextMinute = new Date(now); - startOfNextMinute.setSeconds(0, 0); - startOfNextMinute.setMinutes(startOfNextMinute.getMinutes() + 1); + const nextStartEvent = await CalendarEvent.findNextFutureEvent(now); + const inProgressEvents = await CalendarEvent.findInProgressEvents(now).toArray(); + const eventsWithEndTime = inProgressEvents.filter((event) => event.endTime && event.busy !== false); + if (eventsWithEndTime.length === 0 && !nextStartEvent) { + return; + } - const endOfNextMinute = new Date(startOfNextMinute); - endOfNextMinute.setMinutes(startOfNextMinute.getMinutes() + 1); + let nextEndTime: Date | null = null; + if (eventsWithEndTime.length > 0) { + nextEndTime = eventsWithEndTime.reduce((earliest, event) => { + return event.endTime!.getTime() < earliest.getTime() ? event.endTime! : earliest; + }, eventsWithEndTime[0].endTime!); + } - const eventsToScheduleNow = await CalendarEvent.findEventsToScheduleNow(now, endOfNextMinute).toArray(); - const nextFutureEvent = await CalendarEvent.findNextFutureEvent(endOfNextMinute); + let nextProcessTime: Date; + if (nextStartEvent && nextEndTime) { + nextProcessTime = nextStartEvent.startTime.getTime() < nextEndTime.getTime() ? nextStartEvent.startTime : nextEndTime; + } else if (nextStartEvent) { + nextProcessTime = nextStartEvent.startTime; + } else { + nextProcessTime = nextEndTime!; + } - // PART 1: SCHEDULE ALL EVENTS STARTING SOON (NOW THROUGH NEXT MINUTE) - // Schedule all events that need to be scheduled now - for await (const event of eventsToScheduleNow) { - if (!event.endTime) { + await cronJobs.addAtTimestamp(schedulerJobId, nextProcessTime, async () => this.processStatusChangesAtTime(nextProcessTime)); + } + + private async processStatusChangesAtTime(time: Date): Promise { + const eventsStartingNow = await CalendarEvent.findEventsStartingNow(time).toArray(); + for await (const event of eventsStartingNow) { + if (event.busy === false) { continue; } + await this.processEventStart(event); + } - const startCronJobId = generateCronJobId(event._id, event.uid, 'status'); - if (await cronJobs.has(startCronJobId)) { + const eventsEndingNow = await CalendarEvent.findEventsEndingNow(time).toArray(); + for await (const event of eventsEndingNow) { + if (event.busy === false) { continue; } + await this.processEventEnd(event); + } - await cronJobs.addAtTimestamp(startCronJobId, event.startTime, async () => { - const user = await Users.findOneById(event.uid, { projection: { status: 1 } }); - if (!user) { - return; - } - - const previousStatus = user.status; - await applyStatusChange({ - eventId: event._id, - uid: event.uid, - startTime: event.startTime, - endTime: event.endTime, - status: UserStatus.BUSY, - shouldScheduleRemoval: true, - }); - - const endCronJobId = generateCronJobId(event._id, event.uid, 'status'); - - if (await cronJobs.has(endCronJobId)) { - await cronJobs.remove(endCronJobId); - } + await this.doSetupNextStatusChange(); + } - if (!event.endTime) { - return; - } + private async processEventStart(event: ICalendarEvent): Promise { + if (!event.endTime) { + return; + } - await cronJobs.addAtTimestamp(endCronJobId, event.endTime, async () => { - await applyStatusChange({ - eventId: event._id, - uid: event.uid, - startTime: event.startTime, - endTime: event.endTime, - status: previousStatus, - shouldScheduleRemoval: false, - }); - await this.doSetupNextStatusChange(); - }); - }); + const user = await Users.findOneById(event.uid, { projection: { status: 1 } }); + if (!user || user.status === UserStatus.OFFLINE) { + return; } - // PART 2: ENSURE IN-PROGRESS EVENTS HAVE SCHEDULED END TIMES - // This handles cases like: - // - Service restarts during an event - // - Events imported while in progress - // - Any missed end-time schedules - const inProgressEvents = await CalendarEvent.findInProgressEvents(now).toArray(); - for await (const event of inProgressEvents) { - if (!event.endTime) { - continue; - } + if (user.status) { + await CalendarEvent.updateEvent(event._id, { previousStatus: user.status }); + } - const endCronJobId = generateCronJobId(event._id, event.uid, 'status'); - if (!(await cronJobs.has(endCronJobId))) { - const user = await Users.findOneById(event.uid, { projection: { status: 1 } }); - if (!user) { - continue; - } + await applyStatusChange({ + eventId: event._id, + uid: event.uid, + startTime: event.startTime, + endTime: event.endTime, + status: UserStatus.BUSY, + }); + } - await cronJobs.addAtTimestamp(endCronJobId, event.endTime, async () => { - await applyStatusChange({ - eventId: event._id, - uid: event.uid, - startTime: event.startTime, - endTime: event.endTime, - status: user.status === UserStatus.BUSY ? UserStatus.ONLINE : user.status, - shouldScheduleRemoval: false, - }); - await this.doSetupNextStatusChange(); - }); - } + private async processEventEnd(event: ICalendarEvent): Promise { + if (!event.endTime) { + return; } - // PART 3: SCHEDULE THE NEXT CHECK - // Set up the chain to the next check - const chainJobId = 'calendar-next-status-change'; - - if (await cronJobs.has(chainJobId)) { - await cronJobs.remove(chainJobId); + const user = await Users.findOneById(event.uid, { projection: { status: 1 } }); + if (!user) { + return; } - const nextCheckTime = nextFutureEvent?.startTime || endOfNextMinute; - await cronJobs.addAtTimestamp(chainJobId, nextCheckTime, async () => { - await this.doSetupNextStatusChange(); - }); + // Only restore status if: + // 1. The current status is BUSY (meaning it was set by our system, not manually changed by user) + // 2. We have a previousStatus stored from before the event started + + if (event.previousStatus && event.previousStatus === user.status) { + await applyStatusChange({ + eventId: event._id, + uid: event.uid, + startTime: event.startTime, + endTime: event.endTime, + status: event.previousStatus, + }); + } else { + logger.debug(`Not restoring status for user ${event.uid}: current=${user.status}, stored=${event.previousStatus}`); + } } private async sendCurrentNotifications(date: Date): Promise { diff --git a/apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts b/apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts index d47fe85237ef0..dc0f6b7494706 100644 --- a/apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts +++ b/apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts @@ -2,8 +2,9 @@ import { api } from '@rocket.chat/core-services'; import { UserStatus } from '@rocket.chat/core-typings'; import type { ICalendarEvent, IUser } from '@rocket.chat/core-typings'; import { Users } from '@rocket.chat/models'; +import { Logger } from '@rocket.chat/logger'; -import { setupAppointmentStatusChange } from './setupAppointmentStatusChange'; +const logger = new Logger('Calendar'); export async function applyStatusChange({ eventId, @@ -11,7 +12,6 @@ export async function applyStatusChange({ startTime, endTime, status, - shouldScheduleRemoval, }: { eventId: ICalendarEvent['_id']; uid: IUser['_id']; @@ -20,6 +20,8 @@ export async function applyStatusChange({ status?: UserStatus; shouldScheduleRemoval?: boolean; }): Promise { + logger.debug(`Applying status change for event ${eventId} at ${startTime} ${endTime ? `to ${endTime}` : ''} to ${status}`); + const user = await Users.findOneById(uid, { projection: { roles: 1, username: 1, name: 1, status: 1 } }); if (!user || user.status === UserStatus.OFFLINE) { return; @@ -40,8 +42,4 @@ export async function applyStatusChange({ }, previousStatus, }); - - if (shouldScheduleRemoval && endTime) { - await setupAppointmentStatusChange(eventId, uid, startTime, endTime, previousStatus, false); - } } diff --git a/apps/meteor/server/services/calendar/statusEvents/index.ts b/apps/meteor/server/services/calendar/statusEvents/index.ts index ff4133ca8bc56..e6eca7f011c7e 100644 --- a/apps/meteor/server/services/calendar/statusEvents/index.ts +++ b/apps/meteor/server/services/calendar/statusEvents/index.ts @@ -3,7 +3,6 @@ import { cancelUpcomingStatusChanges } from './cancelUpcomingStatusChanges'; import { generateCronJobId } from './generateCronJobId'; import { handleOverlappingEvents } from './handleOverlappingEvents'; import { removeCronJobs } from './removeCronJobs'; -import { setupAppointmentStatusChange } from './setupAppointmentStatusChange'; export const statusEventManager = { applyStatusChange, @@ -11,5 +10,4 @@ export const statusEventManager = { generateCronJobId, handleOverlappingEvents, removeCronJobs, - setupAppointmentStatusChange, } as const; diff --git a/apps/meteor/tests/unit/server/services/calendar/service.tests.ts b/apps/meteor/tests/unit/server/services/calendar/service.tests.ts index 8ab6836965903..c795f44b18d21 100644 --- a/apps/meteor/tests/unit/server/services/calendar/service.tests.ts +++ b/apps/meteor/tests/unit/server/services/calendar/service.tests.ts @@ -31,7 +31,6 @@ const UsersMock = { }; const statusEventManagerMock = { - setupAppointmentStatusChange: sinon.stub().resolves(), removeCronJobs: sinon.stub().resolves(), cancelUpcomingStatusChanges: sinon.stub().resolves(), applyStatusChange: sinon.stub().resolves(), @@ -42,7 +41,6 @@ const getUserPreferenceMock = sinon.stub(); const serviceMocks = { './statusEvents/cancelUpcomingStatusChanges': { cancelUpcomingStatusChanges: statusEventManagerMock.cancelUpcomingStatusChanges }, './statusEvents/removeCronJobs': { removeCronJobs: statusEventManagerMock.removeCronJobs }, - './statusEvents/setupAppointmentStatusChange': { setupAppointmentStatusChange: statusEventManagerMock.setupAppointmentStatusChange }, './statusEvents/applyStatusChange': { applyStatusChange: statusEventManagerMock.applyStatusChange }, '../../../app/settings/server': { settings: settingsMock }, '@rocket.chat/core-services': { api, ServiceClassInternal: class {} }, @@ -344,7 +342,7 @@ describe('CalendarService', () => { }); }); - it('should schedule jobs when busy status setting is enabled', async () => { + it('should schedule a single chain job to handle all events when busy status setting is enabled', async () => { await testPrivateMethod(service, 'doSetupNextStatusChange', async (method) => { settingsMock.set('Calendar_BusyStatus_Enabled', true); @@ -355,36 +353,111 @@ describe('CalendarService', () => { const endOfNextMinute = new Date(startOfNextMinute); endOfNextMinute.setMinutes(startOfNextMinute.getMinutes() + 1); + const eventStartingSoon = { + _id: 'soon123', + uid: fakeUserId, + startTime: startOfNextMinute, + endTime: new Date(startOfNextMinute.getTime() + 3600000), // 1 hour later + }; + const futureEvent = { _id: 'future123', uid: fakeUserId, startTime: endOfNextMinute, - endTime: new Date(endOfNextMinute.getTime() + 3600000), + endTime: new Date(endOfNextMinute.getTime() + 3600000), // 1 hour later }; - + CalendarEventMock.findEventsToScheduleNow.returns({ - toArray: sinon.stub().resolves([]), + toArray: sinon.stub().resolves([eventStartingSoon]), }); CalendarEventMock.findNextFutureEvent.resolves(futureEvent); - + const originalHas = cronJobsMock.has; const originalRemove = cronJobsMock.remove; const originalAddAtTimestamp = cronJobsMock.addAtTimestamp; - + const hasStub = sinon.stub().resolves(false); const removeStub = sinon.stub().resolves(); const addAtTimestampStub = sinon.stub().resolves(); - + cronJobsMock.has = hasStub; cronJobsMock.remove = removeStub; cronJobsMock.addAtTimestamp = addAtTimestampStub; - + try { await method(); sinon.assert.calledWith(hasStub, 'calendar-next-status-change'); sinon.assert.notCalled(removeStub); - sinon.assert.calledWith(addAtTimestampStub, 'calendar-next-status-change', futureEvent.startTime, sinon.match.func); + + sinon.assert.calledOnce(addAtTimestampStub); + + sinon.assert.calledWith( + addAtTimestampStub, + 'calendar-next-status-change', + futureEvent.startTime, + sinon.match.func + ); + + sinon.assert.neverCalledWith( + addAtTimestampStub, + sinon.match(/^calendar-status-/), + sinon.match.any, + sinon.match.any + ); + } finally { + cronJobsMock.has = originalHas; + cronJobsMock.remove = originalRemove; + cronJobsMock.addAtTimestamp = originalAddAtTimestamp; + } + }); + }); + + it('should fetch events at execution time rather than scheduling them individually', async () => { + await testPrivateMethod(service, 'doSetupNextStatusChange', async (method) => { + settingsMock.set('Calendar_BusyStatus_Enabled', true); + + const now = new Date(); + const startOfNextMinute = new Date(now); + startOfNextMinute.setSeconds(0, 0); + startOfNextMinute.setMinutes(startOfNextMinute.getMinutes() + 1); + + const endOfNextMinute = new Date(startOfNextMinute); + endOfNextMinute.setMinutes(startOfNextMinute.getMinutes() + 1); + + CalendarEventMock.findEventsToScheduleNow.returns({ + toArray: sinon.stub().resolves([]) + }); + CalendarEventMock.findNextFutureEvent.resolves(null); + + const originalHas = cronJobsMock.has; + const originalRemove = cronJobsMock.remove; + const originalAddAtTimestamp = cronJobsMock.addAtTimestamp; + + const hasStub = sinon.stub().resolves(false); + const removeStub = sinon.stub().resolves(); + const addAtTimestampStub = sinon.stub().resolves(); + + cronJobsMock.has = hasStub; + cronJobsMock.remove = removeStub; + cronJobsMock.addAtTimestamp = addAtTimestampStub; + + try { + await method(); + + sinon.assert.calledWith( + addAtTimestampStub, + 'calendar-next-status-change', + endOfNextMinute, + sinon.match.func + ); + + const callback = addAtTimestampStub.firstCall.args[2]; + const doSetupNextStatusChangeStub = sinon.stub(service, 'doSetupNextStatusChange').resolves(); + await callback(); + + sinon.assert.calledOnce(doSetupNextStatusChangeStub); + doSetupNextStatusChangeStub.restore(); } finally { cronJobsMock.has = originalHas; cronJobsMock.remove = originalRemove; diff --git a/apps/meteor/tests/unit/server/services/calendar/statusEvents/applyStatusChange.ts b/apps/meteor/tests/unit/server/services/calendar/statusEvents/applyStatusChange.ts index 30053e86cc1e7..e0a58279210da 100644 --- a/apps/meteor/tests/unit/server/services/calendar/statusEvents/applyStatusChange.ts +++ b/apps/meteor/tests/unit/server/services/calendar/statusEvents/applyStatusChange.ts @@ -11,10 +11,7 @@ const UsersMock = { updateStatusAndStatusDefault: sinon.stub().resolves(), }; -const setupAppointmentStatusChange = sinon.stub().resolves(); - const { applyStatusChange } = proxyquire.noCallThru().load('../../../../../../server/services/calendar/statusEvents/applyStatusChange', { - './setupAppointmentStatusChange': { setupAppointmentStatusChange }, '@rocket.chat/core-services': { api }, '@rocket.chat/models': { Users: UsersMock, @@ -52,8 +49,6 @@ describe('Calendar.StatusEvents', () => { function setupOtherMocks() { sandbox.stub(api, 'broadcast').resolves(); - - setupAppointmentStatusChange.resetHistory(); } afterEach(() => { diff --git a/apps/meteor/tests/unit/server/services/calendar/statusEvents/setupAppointmentStatusChange.ts b/apps/meteor/tests/unit/server/services/calendar/statusEvents/setupAppointmentStatusChange.ts deleted file mode 100644 index 7df232de9c700..0000000000000 --- a/apps/meteor/tests/unit/server/services/calendar/statusEvents/setupAppointmentStatusChange.ts +++ /dev/null @@ -1,77 +0,0 @@ -import { UserStatus } from '@rocket.chat/core-typings'; -import { expect } from 'chai'; -import { describe, it, beforeEach } from 'mocha'; -import proxyquire from 'proxyquire'; -import sinon from 'sinon'; - -import { MockedCronJobs } from '../mocks/cronJobs'; - -const settingsMock = new Map(); -const cronJobsMock = new MockedCronJobs(); - -const applyStatusChange = sinon.stub(); -const handleOverlappingEvents = sinon.stub(); - -const { setupAppointmentStatusChange } = proxyquire - .noCallThru() - .load('../../../../../../server/services/calendar/statusEvents/setupAppointmentStatusChange', { - './applyStatusChange': { applyStatusChange }, - './handleOverlappingEvents': { handleOverlappingEvents }, - '../../../../app/settings/server': { settings: settingsMock }, - '@rocket.chat/cron': { cronJobs: cronJobsMock }, - }); - -describe('Calendar.StatusEvents', () => { - const fakeEventId = 'eventId123'; - const fakeUserId = 'userId456'; - const fakeStartTime = new Date('2025-01-01T10:00:00Z'); - const fakeEndTime = new Date('2025-01-01T11:00:00Z'); - const statusId = `calendar-presence-status-${fakeEventId}-${fakeUserId}`; - - beforeEach(() => { - cronJobsMock.jobNames.clear(); - applyStatusChange.resetHistory(); - handleOverlappingEvents.resetHistory(); - settingsMock.clear(); - settingsMock.set('Calendar_BusyStatus_Enabled', true); - }); - - describe('#setupAppointmentStatusChange', () => { - it('should do nothing if busy status setting is disabled', async () => { - settingsMock.set('Calendar_BusyStatus_Enabled', false); - - await setupAppointmentStatusChange(fakeEventId, fakeUserId, fakeStartTime, fakeEndTime, undefined, false); - - expect(cronJobsMock.jobNames.size).to.equal(0); - }); - - it('should do nothing if endTime is not provided', async () => { - await setupAppointmentStatusChange(fakeEventId, fakeUserId, fakeStartTime, undefined, undefined, false); - - expect(cronJobsMock.jobNames.size).to.equal(0); - }); - - it('should handle overlapping events when shouldScheduleRemoval=true', async () => { - handleOverlappingEvents.resolves({ shouldProceed: false }); - - await setupAppointmentStatusChange(fakeEventId, fakeUserId, fakeStartTime, fakeEndTime, UserStatus.BUSY, true); - - expect(handleOverlappingEvents.callCount).to.equal(1); - expect(cronJobsMock.jobNames.size).to.equal(0); - }); - - it('should schedule status change at the start time when shouldScheduleRemoval=true', async () => { - handleOverlappingEvents.resolves({ shouldProceed: true }); - - await setupAppointmentStatusChange(fakeEventId, fakeUserId, fakeStartTime, fakeEndTime, UserStatus.BUSY, true); - - expect(cronJobsMock.jobNames.has(statusId)).to.true; - }); - - it('should schedule status change at the end time when shouldScheduleRemoval=false', async () => { - await setupAppointmentStatusChange(fakeEventId, fakeUserId, fakeStartTime, fakeEndTime, UserStatus.BUSY, false); - - expect(cronJobsMock.jobNames.has(statusId)).to.true; - }); - }); -}); diff --git a/packages/core-typings/src/ICalendarEvent.ts b/packages/core-typings/src/ICalendarEvent.ts index fb58bde124c43..a9fb9fea12a9c 100644 --- a/packages/core-typings/src/ICalendarEvent.ts +++ b/packages/core-typings/src/ICalendarEvent.ts @@ -1,5 +1,6 @@ import type { IRocketChatRecord } from './IRocketChatRecord'; import type { IUser } from './IUser'; +import { UserStatus } from './UserStatus'; export interface ICalendarEvent extends IRocketChatRecord { startTime: Date; @@ -17,4 +18,5 @@ export interface ICalendarEvent extends IRocketChatRecord { reminderTime?: Date; busy?: boolean; + previousStatus?: UserStatus; } diff --git a/packages/model-typings/src/models/ICalendarEventModel.ts b/packages/model-typings/src/models/ICalendarEventModel.ts index 019c12ac59d18..92b6ec898ff76 100644 --- a/packages/model-typings/src/models/ICalendarEventModel.ts +++ b/packages/model-typings/src/models/ICalendarEventModel.ts @@ -18,4 +18,6 @@ export interface ICalendarEventModel extends IBaseModel { findEventsToScheduleNow(now: Date, endTime: Date): FindCursor; findNextFutureEvent(startTime: Date): Promise; findInProgressEvents(now: Date): FindCursor; + findEventsStartingNow(now: Date): FindCursor; + findEventsEndingNow(now: Date): FindCursor; } diff --git a/packages/models/src/models/CalendarEvent.ts b/packages/models/src/models/CalendarEvent.ts index 0972846ea1a27..adf8526fea498 100644 --- a/packages/models/src/models/CalendarEvent.ts +++ b/packages/models/src/models/CalendarEvent.ts @@ -50,7 +50,7 @@ export class CalendarEventRaw extends BaseRaw implements ICalend public async updateEvent( eventId: ICalendarEvent['_id'], - { subject, description, startTime, meetingUrl, reminderMinutesBeforeStart, reminderTime }: Partial, + { subject, description, startTime, meetingUrl, reminderMinutesBeforeStart, reminderTime, previousStatus }: Partial, ): Promise { return this.updateOne( { _id: eventId }, @@ -62,6 +62,7 @@ export class CalendarEventRaw extends BaseRaw implements ICalend ...(meetingUrl !== undefined ? { meetingUrl } : {}), ...(reminderMinutesBeforeStart ? { reminderMinutesBeforeStart } : {}), ...(reminderTime ? { reminderTime } : {}), + ...(previousStatus ? { previousStatus } : {}), }, }, ); @@ -153,7 +154,7 @@ export class CalendarEventRaw extends BaseRaw implements ICalend public findEventsToScheduleNow(now: Date, endTime: Date): FindCursor { return this.find( { - startTime: { $gt: now, $lt: endTime }, + startTime: { $gte: now, $lt: endTime }, busy: { $ne: false }, endTime: { $exists: true }, }, @@ -185,6 +186,41 @@ export class CalendarEventRaw extends BaseRaw implements ICalend ); } + public findEventsStartingNow(now: Date): FindCursor { + return this.find({ + startTime: { + $gte: new Date(now.getTime() - 1000), + $lt: new Date(now.getTime() + 1000), + }, + busy: { $ne: false }, + }, { + projection: { + _id: 1, + uid: 1, + startTime: 1, + endTime: 1, + }, + }); + } + + public findEventsEndingNow(now: Date): FindCursor { + return this.find({ + endTime: { + $gte: new Date(now.getTime() - 1000), + $lt: new Date(now.getTime() + 1000), + }, + busy: { $ne: false }, + }, { + projection: { + _id: 1, + uid: 1, + startTime: 1, + endTime: 1, + previousStatus: 1, + }, + }); + } + public findInProgressEvents(now: Date): FindCursor { return this.find( { From fdeb1890a1a7924906eb2e6421e08d4699736f83 Mon Sep 17 00:00:00 2001 From: Ricardo Garim Date: Wed, 26 Mar 2025 10:27:34 -0300 Subject: [PATCH 4/8] eslint fix --- apps/meteor/server/services/calendar/service.ts | 16 ++++++++-------- packages/core-typings/src/ICalendarEvent.ts | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/apps/meteor/server/services/calendar/service.ts b/apps/meteor/server/services/calendar/service.ts index a4e82d94e23d5..f99ee50899b05 100644 --- a/apps/meteor/server/services/calendar/service.ts +++ b/apps/meteor/server/services/calendar/service.ts @@ -213,14 +213,14 @@ export class CalendarService extends ServiceClassInternal implements ICalendarSe // 5. When an event ends and the status is restored // 6. From Outlook Calendar integration (ee/server/configuration/outlookCalendar.ts) - // const busyStatusEnabled = settings.get('Calendar_BusyStatus_Enabled'); - // if (!busyStatusEnabled) { - // const schedulerJobId = 'calendar-status-scheduler'; - // if (await cronJobs.has(schedulerJobId)) { - // await cronJobs.remove(schedulerJobId); - // } - // return; - // } + const busyStatusEnabled = settings.get('Calendar_BusyStatus_Enabled'); + if (!busyStatusEnabled) { + const schedulerJobId = 'calendar-status-scheduler'; + if (await cronJobs.has(schedulerJobId)) { + await cronJobs.remove(schedulerJobId); + } + return; + } const schedulerJobId = 'calendar-status-scheduler'; if (await cronJobs.has(schedulerJobId)) { diff --git a/packages/core-typings/src/ICalendarEvent.ts b/packages/core-typings/src/ICalendarEvent.ts index a9fb9fea12a9c..5088f99cebfca 100644 --- a/packages/core-typings/src/ICalendarEvent.ts +++ b/packages/core-typings/src/ICalendarEvent.ts @@ -1,6 +1,6 @@ import type { IRocketChatRecord } from './IRocketChatRecord'; import type { IUser } from './IUser'; -import { UserStatus } from './UserStatus'; +import type { UserStatus } from './UserStatus'; export interface ICalendarEvent extends IRocketChatRecord { startTime: Date; From 8d5b34475da87d1dd4986a8a1eaaac819e0a4d72 Mon Sep 17 00:00:00 2001 From: Ricardo Garim Date: Wed, 26 Mar 2025 10:59:17 -0300 Subject: [PATCH 5/8] eslint fix --- .../statusEvents/applyStatusChange.ts | 4 +- .../server/services/calendar/service.tests.ts | 57 +++++++----------- packages/models/src/models/CalendarEvent.ts | 60 ++++++++++--------- 3 files changed, 56 insertions(+), 65 deletions(-) diff --git a/apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts b/apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts index dc0f6b7494706..860d5df5e4d3b 100644 --- a/apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts +++ b/apps/meteor/server/services/calendar/statusEvents/applyStatusChange.ts @@ -1,8 +1,8 @@ import { api } from '@rocket.chat/core-services'; import { UserStatus } from '@rocket.chat/core-typings'; import type { ICalendarEvent, IUser } from '@rocket.chat/core-typings'; -import { Users } from '@rocket.chat/models'; import { Logger } from '@rocket.chat/logger'; +import { Users } from '@rocket.chat/models'; const logger = new Logger('Calendar'); @@ -21,7 +21,7 @@ export async function applyStatusChange({ shouldScheduleRemoval?: boolean; }): Promise { logger.debug(`Applying status change for event ${eventId} at ${startTime} ${endTime ? `to ${endTime}` : ''} to ${status}`); - + const user = await Users.findOneById(uid, { projection: { roles: 1, username: 1, name: 1, status: 1 } }); if (!user || user.status === UserStatus.OFFLINE) { return; diff --git a/apps/meteor/tests/unit/server/services/calendar/service.tests.ts b/apps/meteor/tests/unit/server/services/calendar/service.tests.ts index c795f44b18d21..e071b65c763e8 100644 --- a/apps/meteor/tests/unit/server/services/calendar/service.tests.ts +++ b/apps/meteor/tests/unit/server/services/calendar/service.tests.ts @@ -366,45 +366,35 @@ describe('CalendarService', () => { startTime: endOfNextMinute, endTime: new Date(endOfNextMinute.getTime() + 3600000), // 1 hour later }; - + CalendarEventMock.findEventsToScheduleNow.returns({ toArray: sinon.stub().resolves([eventStartingSoon]), }); CalendarEventMock.findNextFutureEvent.resolves(futureEvent); - + const originalHas = cronJobsMock.has; const originalRemove = cronJobsMock.remove; const originalAddAtTimestamp = cronJobsMock.addAtTimestamp; - + const hasStub = sinon.stub().resolves(false); const removeStub = sinon.stub().resolves(); const addAtTimestampStub = sinon.stub().resolves(); - + cronJobsMock.has = hasStub; cronJobsMock.remove = removeStub; cronJobsMock.addAtTimestamp = addAtTimestampStub; - + try { await method(); sinon.assert.calledWith(hasStub, 'calendar-next-status-change'); sinon.assert.notCalled(removeStub); - + sinon.assert.calledOnce(addAtTimestampStub); - - sinon.assert.calledWith( - addAtTimestampStub, - 'calendar-next-status-change', - futureEvent.startTime, - sinon.match.func - ); - - sinon.assert.neverCalledWith( - addAtTimestampStub, - sinon.match(/^calendar-status-/), - sinon.match.any, - sinon.match.any - ); + + sinon.assert.calledWith(addAtTimestampStub, 'calendar-next-status-change', futureEvent.startTime, sinon.match.func); + + sinon.assert.neverCalledWith(addAtTimestampStub, sinon.match(/^calendar-status-/), sinon.match.any, sinon.match.any); } finally { cronJobsMock.has = originalHas; cronJobsMock.remove = originalRemove; @@ -421,41 +411,36 @@ describe('CalendarService', () => { const startOfNextMinute = new Date(now); startOfNextMinute.setSeconds(0, 0); startOfNextMinute.setMinutes(startOfNextMinute.getMinutes() + 1); - + const endOfNextMinute = new Date(startOfNextMinute); endOfNextMinute.setMinutes(startOfNextMinute.getMinutes() + 1); - + CalendarEventMock.findEventsToScheduleNow.returns({ - toArray: sinon.stub().resolves([]) + toArray: sinon.stub().resolves([]), }); CalendarEventMock.findNextFutureEvent.resolves(null); - + const originalHas = cronJobsMock.has; const originalRemove = cronJobsMock.remove; const originalAddAtTimestamp = cronJobsMock.addAtTimestamp; - + const hasStub = sinon.stub().resolves(false); const removeStub = sinon.stub().resolves(); const addAtTimestampStub = sinon.stub().resolves(); - + cronJobsMock.has = hasStub; cronJobsMock.remove = removeStub; cronJobsMock.addAtTimestamp = addAtTimestampStub; - + try { await method(); - - sinon.assert.calledWith( - addAtTimestampStub, - 'calendar-next-status-change', - endOfNextMinute, - sinon.match.func - ); - + + sinon.assert.calledWith(addAtTimestampStub, 'calendar-next-status-change', endOfNextMinute, sinon.match.func); + const callback = addAtTimestampStub.firstCall.args[2]; const doSetupNextStatusChangeStub = sinon.stub(service, 'doSetupNextStatusChange').resolves(); await callback(); - + sinon.assert.calledOnce(doSetupNextStatusChangeStub); doSetupNextStatusChangeStub.restore(); } finally { diff --git a/packages/models/src/models/CalendarEvent.ts b/packages/models/src/models/CalendarEvent.ts index adf8526fea498..a2cb48bfab706 100644 --- a/packages/models/src/models/CalendarEvent.ts +++ b/packages/models/src/models/CalendarEvent.ts @@ -187,38 +187,44 @@ export class CalendarEventRaw extends BaseRaw implements ICalend } public findEventsStartingNow(now: Date): FindCursor { - return this.find({ - startTime: { - $gte: new Date(now.getTime() - 1000), - $lt: new Date(now.getTime() + 1000), - }, - busy: { $ne: false }, - }, { - projection: { - _id: 1, - uid: 1, - startTime: 1, - endTime: 1, + return this.find( + { + startTime: { + $gte: new Date(now.getTime() - 1000), + $lt: new Date(now.getTime() + 1000), + }, + busy: { $ne: false }, }, - }); + { + projection: { + _id: 1, + uid: 1, + startTime: 1, + endTime: 1, + }, + }, + ); } public findEventsEndingNow(now: Date): FindCursor { - return this.find({ - endTime: { - $gte: new Date(now.getTime() - 1000), - $lt: new Date(now.getTime() + 1000), - }, - busy: { $ne: false }, - }, { - projection: { - _id: 1, - uid: 1, - startTime: 1, - endTime: 1, - previousStatus: 1, + return this.find( + { + endTime: { + $gte: new Date(now.getTime() - 1000), + $lt: new Date(now.getTime() + 1000), + }, + busy: { $ne: false }, }, - }); + { + projection: { + _id: 1, + uid: 1, + startTime: 1, + endTime: 1, + previousStatus: 1, + }, + }, + ); } public findInProgressEvents(now: Date): FindCursor { From 4bbe9e58097afff6e507cfaee18d548779adeb90 Mon Sep 17 00:00:00 2001 From: Ricardo Garim Date: Wed, 26 Mar 2025 11:33:03 -0300 Subject: [PATCH 6/8] typecheck fix --- .../calendar/statusEvents/applyStatusChange.ts | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/apps/meteor/tests/unit/server/services/calendar/statusEvents/applyStatusChange.ts b/apps/meteor/tests/unit/server/services/calendar/statusEvents/applyStatusChange.ts index e0a58279210da..65818c9d1f09a 100644 --- a/apps/meteor/tests/unit/server/services/calendar/statusEvents/applyStatusChange.ts +++ b/apps/meteor/tests/unit/server/services/calendar/statusEvents/applyStatusChange.ts @@ -139,26 +139,13 @@ describe('Calendar.StatusEvents', () => { }); it('should schedule status revert when shouldScheduleRemoval=true', async () => { - const previousStatus = UserStatus.ONLINE; - await applyStatusChange({ eventId: fakeEventId, uid: fakeUserId, startTime: fakeStartTime, endTime: fakeEndTime, status: UserStatus.BUSY, - shouldScheduleRemoval: true, }); - - expect(setupAppointmentStatusChange.callCount).to.equal(1); - expect(setupAppointmentStatusChange.firstCall.args).to.deep.equal([ - fakeEventId, - fakeUserId, - fakeStartTime, - fakeEndTime, - previousStatus, - false, - ]); }); }); }); From 003129fc7c76cfcf6cceae40001023f7dbd64161 Mon Sep 17 00:00:00 2001 From: Ricardo Garim Date: Thu, 27 Mar 2025 08:09:22 -0300 Subject: [PATCH 7/8] makes processStatusChangesAtTime looks at current time events --- apps/meteor/server/services/calendar/service.ts | 10 ++++++---- .../model-typings/src/models/ICalendarEventModel.ts | 4 ++-- packages/models/src/models/CalendarEvent.ts | 12 ++++++------ 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/apps/meteor/server/services/calendar/service.ts b/apps/meteor/server/services/calendar/service.ts index f99ee50899b05..376ae689c6dd9 100644 --- a/apps/meteor/server/services/calendar/service.ts +++ b/apps/meteor/server/services/calendar/service.ts @@ -251,11 +251,13 @@ export class CalendarService extends ServiceClassInternal implements ICalendarSe nextProcessTime = nextEndTime!; } - await cronJobs.addAtTimestamp(schedulerJobId, nextProcessTime, async () => this.processStatusChangesAtTime(nextProcessTime)); + await cronJobs.addAtTimestamp(schedulerJobId, nextProcessTime, async () => this.processStatusChangesAtTime()); } - private async processStatusChangesAtTime(time: Date): Promise { - const eventsStartingNow = await CalendarEvent.findEventsStartingNow(time).toArray(); + private async processStatusChangesAtTime(): Promise { + const processTime = new Date(); + + const eventsStartingNow = await CalendarEvent.findEventsStartingNow(processTime, 5000).toArray(); for await (const event of eventsStartingNow) { if (event.busy === false) { continue; @@ -263,7 +265,7 @@ export class CalendarService extends ServiceClassInternal implements ICalendarSe await this.processEventStart(event); } - const eventsEndingNow = await CalendarEvent.findEventsEndingNow(time).toArray(); + const eventsEndingNow = await CalendarEvent.findEventsEndingNow(processTime, 5000).toArray(); for await (const event of eventsEndingNow) { if (event.busy === false) { continue; diff --git a/packages/model-typings/src/models/ICalendarEventModel.ts b/packages/model-typings/src/models/ICalendarEventModel.ts index 92b6ec898ff76..c195c71e97658 100644 --- a/packages/model-typings/src/models/ICalendarEventModel.ts +++ b/packages/model-typings/src/models/ICalendarEventModel.ts @@ -18,6 +18,6 @@ export interface ICalendarEventModel extends IBaseModel { findEventsToScheduleNow(now: Date, endTime: Date): FindCursor; findNextFutureEvent(startTime: Date): Promise; findInProgressEvents(now: Date): FindCursor; - findEventsStartingNow(now: Date): FindCursor; - findEventsEndingNow(now: Date): FindCursor; + findEventsStartingNow(now: Date, offset?: number): FindCursor; + findEventsEndingNow(now: Date, offset?: number): FindCursor; } diff --git a/packages/models/src/models/CalendarEvent.ts b/packages/models/src/models/CalendarEvent.ts index a2cb48bfab706..26debeab7a324 100644 --- a/packages/models/src/models/CalendarEvent.ts +++ b/packages/models/src/models/CalendarEvent.ts @@ -186,12 +186,12 @@ export class CalendarEventRaw extends BaseRaw implements ICalend ); } - public findEventsStartingNow(now: Date): FindCursor { + public findEventsStartingNow(now: Date, offset = 1000): FindCursor { return this.find( { startTime: { - $gte: new Date(now.getTime() - 1000), - $lt: new Date(now.getTime() + 1000), + $gte: new Date(now.getTime() - offset), + $lt: new Date(now.getTime() + offset), }, busy: { $ne: false }, }, @@ -206,12 +206,12 @@ export class CalendarEventRaw extends BaseRaw implements ICalend ); } - public findEventsEndingNow(now: Date): FindCursor { + public findEventsEndingNow(now: Date, offset = 1000): FindCursor { return this.find( { endTime: { - $gte: new Date(now.getTime() - 1000), - $lt: new Date(now.getTime() + 1000), + $gte: new Date(now.getTime() - offset), + $lt: new Date(now.getTime() + offset), }, busy: { $ne: false }, }, From 402de308ea90fda7e54aa4dc7bd34291314ade17 Mon Sep 17 00:00:00 2001 From: Ricardo Garim Date: Fri, 28 Mar 2025 10:16:44 -0300 Subject: [PATCH 8/8] review fixes --- apps/meteor/server/services/calendar/service.ts | 16 ++++++++++------ .../src/models/ICalendarEventModel.ts | 4 ++-- packages/models/src/models/CalendarEvent.ts | 4 ++-- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/apps/meteor/server/services/calendar/service.ts b/apps/meteor/server/services/calendar/service.ts index 376ae689c6dd9..e96956e4f9462 100644 --- a/apps/meteor/server/services/calendar/service.ts +++ b/apps/meteor/server/services/calendar/service.ts @@ -236,10 +236,11 @@ export class CalendarService extends ServiceClassInternal implements ICalendarSe } let nextEndTime: Date | null = null; - if (eventsWithEndTime.length > 0) { + if (eventsWithEndTime.length > 0 && eventsWithEndTime[0].endTime) { nextEndTime = eventsWithEndTime.reduce((earliest, event) => { - return event.endTime!.getTime() < earliest.getTime() ? event.endTime! : earliest; - }, eventsWithEndTime[0].endTime!); + if (!event.endTime) return earliest; + return event.endTime.getTime() < earliest.getTime() ? event.endTime : earliest; + }, eventsWithEndTime[0].endTime); } let nextProcessTime: Date; @@ -247,8 +248,11 @@ export class CalendarService extends ServiceClassInternal implements ICalendarSe nextProcessTime = nextStartEvent.startTime.getTime() < nextEndTime.getTime() ? nextStartEvent.startTime : nextEndTime; } else if (nextStartEvent) { nextProcessTime = nextStartEvent.startTime; + } else if (nextEndTime) { + nextProcessTime = nextEndTime; } else { - nextProcessTime = nextEndTime!; + // This should never happen due to the earlier check, but just in case + return; } await cronJobs.addAtTimestamp(schedulerJobId, nextProcessTime, async () => this.processStatusChangesAtTime()); @@ -257,7 +261,7 @@ export class CalendarService extends ServiceClassInternal implements ICalendarSe private async processStatusChangesAtTime(): Promise { const processTime = new Date(); - const eventsStartingNow = await CalendarEvent.findEventsStartingNow(processTime, 5000).toArray(); + const eventsStartingNow = await CalendarEvent.findEventsStartingNow({ now: processTime, offset: 5000 }).toArray(); for await (const event of eventsStartingNow) { if (event.busy === false) { continue; @@ -265,7 +269,7 @@ export class CalendarService extends ServiceClassInternal implements ICalendarSe await this.processEventStart(event); } - const eventsEndingNow = await CalendarEvent.findEventsEndingNow(processTime, 5000).toArray(); + const eventsEndingNow = await CalendarEvent.findEventsEndingNow({ now: processTime, offset: 5000 }).toArray(); for await (const event of eventsEndingNow) { if (event.busy === false) { continue; diff --git a/packages/model-typings/src/models/ICalendarEventModel.ts b/packages/model-typings/src/models/ICalendarEventModel.ts index c195c71e97658..9aae06964d25a 100644 --- a/packages/model-typings/src/models/ICalendarEventModel.ts +++ b/packages/model-typings/src/models/ICalendarEventModel.ts @@ -18,6 +18,6 @@ export interface ICalendarEventModel extends IBaseModel { findEventsToScheduleNow(now: Date, endTime: Date): FindCursor; findNextFutureEvent(startTime: Date): Promise; findInProgressEvents(now: Date): FindCursor; - findEventsStartingNow(now: Date, offset?: number): FindCursor; - findEventsEndingNow(now: Date, offset?: number): FindCursor; + findEventsStartingNow({ now, offset }: { now: Date; offset?: number }): FindCursor; + findEventsEndingNow({ now, offset }: { now: Date; offset?: number }): FindCursor; } diff --git a/packages/models/src/models/CalendarEvent.ts b/packages/models/src/models/CalendarEvent.ts index 26debeab7a324..afbbf83352fa8 100644 --- a/packages/models/src/models/CalendarEvent.ts +++ b/packages/models/src/models/CalendarEvent.ts @@ -186,7 +186,7 @@ export class CalendarEventRaw extends BaseRaw implements ICalend ); } - public findEventsStartingNow(now: Date, offset = 1000): FindCursor { + public findEventsStartingNow({ now, offset = 1000 }: { now: Date; offset?: number }): FindCursor { return this.find( { startTime: { @@ -206,7 +206,7 @@ export class CalendarEventRaw extends BaseRaw implements ICalend ); } - public findEventsEndingNow(now: Date, offset = 1000): FindCursor { + public findEventsEndingNow({ now, offset = 1000 }: { now: Date; offset?: number }): FindCursor { return this.find( { endTime: {