diff --git a/package.json b/package.json index 2eb5eb56..b9aaa8e8 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "hawk.api", - "version": "1.5.3", + "version": "1.5.0", "main": "index.ts", "license": "BUSL-1.1", "scripts": { diff --git a/src/models/eventsFactory.js b/src/models/eventsFactory.js index fccae66d..e7234566 100644 --- a/src/models/eventsFactory.js +++ b/src/models/eventsFactory.js @@ -889,7 +889,7 @@ class EventsFactory extends Factory { * @param {string|ObjectId} userId - id of the user who is visiting events * @returns {Promise<{ updatedCount: number, updatedEventIds: string[], failedEventIds: string[] }>} */ - async bulkVisitEvent(eventIds, userId) { + async bulkVisitEvents(eventIds, userId) { const { collection, found, diff --git a/src/resolvers/event.js b/src/resolvers/event.js index af1d6f70..dd6f12b6 100644 --- a/src/resolvers/event.js +++ b/src/resolvers/event.js @@ -6,6 +6,7 @@ const { } = require('./helpers/bulkEvents'); const { aiService } = require('../services/ai'); const { UserInputError } = require('apollo-server-express'); +const { ObjectId } = require('mongodb'); /** * See all types and fields here {@see ../typeDefs/event.graphql} @@ -161,7 +162,7 @@ module.exports = { } const factory = getEventsFactory(context, projectId); - const result = await factory.bulkVisitEvent(validEventIds, user.id); + const result = await factory.bulkVisitEvents(validEventIds, user.id); return { ...result, @@ -317,6 +318,10 @@ module.exports = { const factory = getEventsFactory(context, projectId); if (assignee) { + if (!ObjectId.isValid(String(assignee))) { + throw new UserInputError('assignee must be a valid id or null'); + } + const userExists = await factories.usersFactory.findById(assignee); if (!userExists) { diff --git a/src/resolvers/helpers/bulkEvents.js b/src/resolvers/helpers/bulkEvents.js index 4d0238a5..50dedb6f 100644 --- a/src/resolvers/helpers/bulkEvents.js +++ b/src/resolvers/helpers/bulkEvents.js @@ -2,6 +2,8 @@ const sendPersonalNotification = require('../../utils/personalNotifications').de const { UserInputError } = require('apollo-server-express'); const { ObjectId } = require('mongodb'); +const ASSIGNEE_NOTIFICATIONS_CHUNK_SIZE = 25; + /** * Enqueue assignee notifications in background (do not block resolver response) * @@ -26,20 +28,40 @@ function fireAndForgetAssigneeNotifications({ return; } - Promise.allSettled(eventIds.map(eventId => sendPersonalNotification(assigneeData, { - type: 'assignee', - payload: { - assigneeId, - projectId, - whoAssignedId, - eventId, - }, - }))) - .then((results) => { - const failedResults = results.filter(result => result.status === 'rejected'); + Promise.resolve() + .then(async () => { + const failedResults = []; + + for (let i = 0; i < eventIds.length; i += ASSIGNEE_NOTIFICATIONS_CHUNK_SIZE) { + const chunk = eventIds.slice(i, i + ASSIGNEE_NOTIFICATIONS_CHUNK_SIZE); + const results = await Promise.allSettled(chunk.map(eventId => sendPersonalNotification(assigneeData, { + type: 'assignee', + payload: { + assigneeId, + projectId, + whoAssignedId, + eventId, + }, + }))); + + failedResults.push(...results.filter(result => result.status === 'rejected')); + } if (failedResults.length > 0) { - console.error('Failed to enqueue assignee notifications', failedResults); + const failedMessages = failedResults.map((result) => { + const reason = result && result.reason; + + if (reason && typeof reason.message === 'string') { + return reason.message; + } + + return String(reason || 'Unknown error'); + }); + + console.error('Failed to enqueue assignee notifications', { + failedCount: failedResults.length, + errors: failedMessages, + }); } }) .catch((error) => { diff --git a/src/typeDefs/event.ts b/src/typeDefs/event.ts index 849c708a..1d792aff 100644 --- a/src/typeDefs/event.ts +++ b/src/typeDefs/event.ts @@ -487,7 +487,7 @@ type BulkUpdateAssigneeResponse { } """ -Result of bulk toggling event marks (resolve / ignore) +Result of bulk toggling event marks (resolve / ignore / starred) """ type BulkToggleEventMarksResult { """ @@ -602,7 +602,8 @@ extend type Mutation { """ Toggle the same mark on many original events at once (resolved, ignored or starred). - Same toggle semantics as toggleEventMark per event. + Uses bulk semantics: if every selected event already has the mark, clear it for all; + otherwise set it on each selected event that does not have it yet. """ bulkToggleEventMarks( """ diff --git a/test/models/eventsFactory-bulk-visit.test.ts b/test/models/eventsFactory-bulk-visit.test.ts index d5b19feb..7a1b1627 100644 --- a/test/models/eventsFactory-bulk-visit.test.ts +++ b/test/models/eventsFactory-bulk-visit.test.ts @@ -35,7 +35,7 @@ jest.mock('../../src/mongo', () => ({ // eslint-disable-next-line @typescript-eslint/no-var-requires, @typescript-eslint/no-explicit-any -- CJS class const EventsFactory = require('../../src/models/eventsFactory') as any; -describe('EventsFactory.bulkVisitEvent', () => { +describe('EventsFactory.bulkVisitEvents', () => { const projectId = '507f1f77bcf86cd799439011'; beforeEach(() => { @@ -57,7 +57,7 @@ describe('EventsFactory.bulkVisitEvent', () => { }); collectionMock.updateMany.mockResolvedValue({ modifiedCount: 1 }); - const result = await factory.bulkVisitEvent([ a.toString(), b.toString() ], userId.toString()); + const result = await factory.bulkVisitEvents([ a.toString(), b.toString() ], userId.toString()); expect(result.updatedCount).toBe(1); expect(result.updatedEventIds).toEqual([ b.toString() ]); @@ -72,7 +72,7 @@ describe('EventsFactory.bulkVisitEvent', () => { toArray: () => Promise.resolve([]), }); - const result = await factory.bulkVisitEvent([ missing.toString() ], new ObjectId().toString()); + const result = await factory.bulkVisitEvents([ missing.toString() ], new ObjectId().toString()); expect(result.updatedCount).toBe(0); expect(result.updatedEventIds).toEqual([]); diff --git a/test/resolvers/event-bulk-update-assignee.test.ts b/test/resolvers/event-bulk-update-assignee.test.ts index 3ab7ce76..9fcc093d 100644 --- a/test/resolvers/event-bulk-update-assignee.test.ts +++ b/test/resolvers/event-bulk-update-assignee.test.ts @@ -26,16 +26,17 @@ const eventResolvers = require('../../src/resolvers/event') as { }; const bulkUpdateAssignee = jest.fn(); +const ASSIGNEE_ID = '507f1f77bcf86cd799439012'; describe('EventsMutations.bulkUpdateAssignee', () => { const ctx = { user: { id: 'u1' }, factories: { usersFactory: { - findById: jest.fn().mockResolvedValue({ id: 'assignee-1' }), + findById: jest.fn().mockResolvedValue({ id: ASSIGNEE_ID }), dataLoaders: { userById: { - load: jest.fn().mockResolvedValue({ id: 'assignee-1', email: 'a@a.a' }), + load: jest.fn().mockResolvedValue({ id: ASSIGNEE_ID, email: 'a@a.a' }), }, }, }, @@ -44,7 +45,7 @@ describe('EventsMutations.bulkUpdateAssignee', () => { }, workspacesFactory: { findById: jest.fn().mockResolvedValue({ - getMemberInfo: jest.fn().mockResolvedValue({ userId: 'assignee-1' }), + getMemberInfo: jest.fn().mockResolvedValue({ userId: ASSIGNEE_ID }), }), }, }, @@ -66,13 +67,31 @@ describe('EventsMutations.bulkUpdateAssignee', () => { await expect( eventResolvers.EventsMutations.bulkUpdateAssignee( {}, - { input: { projectId: 'p1', eventIds: [], assignee: 'assignee-1' } }, + { input: { projectId: 'p1', eventIds: [], assignee: ASSIGNEE_ID } }, ctx ) ).rejects.toThrow(UserInputError); expect(bulkUpdateAssignee).not.toHaveBeenCalled(); }); + it('should throw when assignee id is invalid', async () => { + await expect( + eventResolvers.EventsMutations.bulkUpdateAssignee( + {}, + { + input: { + projectId: 'p1', + eventIds: [ '507f1f77bcf86cd799439011' ], + assignee: 'not-an-object-id', + }, + }, + ctx + ) + ).rejects.toThrow(UserInputError); + + expect(bulkUpdateAssignee).not.toHaveBeenCalled(); + }); + it('should call factory for bulk assign', async () => { const result = await eventResolvers.EventsMutations.bulkUpdateAssignee( {}, @@ -80,7 +99,7 @@ describe('EventsMutations.bulkUpdateAssignee', () => { input: { projectId: 'p1', eventIds: [ '507f1f77bcf86cd799439011' ], - assignee: 'assignee-1', + assignee: ASSIGNEE_ID, }, }, ctx @@ -89,15 +108,15 @@ describe('EventsMutations.bulkUpdateAssignee', () => { expect(result.updatedCount).toBe(1); expect(bulkUpdateAssignee).toHaveBeenCalledWith( [ '507f1f77bcf86cd799439011' ], - 'assignee-1' + ASSIGNEE_ID ); expect(sendPersonalNotification).toHaveBeenCalledTimes(1); expect(sendPersonalNotification).toHaveBeenCalledWith( - expect.objectContaining({ id: 'assignee-1' }), + expect.objectContaining({ id: ASSIGNEE_ID }), expect.objectContaining({ type: 'assignee', payload: expect.objectContaining({ - assigneeId: 'assignee-1', + assigneeId: ASSIGNEE_ID, projectId: 'p1', whoAssignedId: 'u1', eventId: '507f1f77bcf86cd799439011', @@ -119,7 +138,7 @@ describe('EventsMutations.bulkUpdateAssignee', () => { input: { projectId: 'p1', eventIds: [ '507f1f77bcf86cd799439011', 'invalid-id' ], - assignee: 'assignee-1', + assignee: ASSIGNEE_ID, }, }, ctx @@ -127,7 +146,7 @@ describe('EventsMutations.bulkUpdateAssignee', () => { expect(bulkUpdateAssignee).toHaveBeenCalledWith( [ '507f1f77bcf86cd799439011' ], - 'assignee-1' + ASSIGNEE_ID ); expect(result).toEqual({ updatedCount: 1, @@ -143,7 +162,7 @@ describe('EventsMutations.bulkUpdateAssignee', () => { input: { projectId: 'p1', eventIds: [ 'bad-1', 'bad-2' ], - assignee: 'assignee-1', + assignee: ASSIGNEE_ID, }, }, ctx diff --git a/test/resolvers/event-bulk-visit.test.ts b/test/resolvers/event-bulk-visit.test.ts index 632d4a71..e99ce988 100644 --- a/test/resolvers/event-bulk-visit.test.ts +++ b/test/resolvers/event-bulk-visit.test.ts @@ -17,7 +17,7 @@ const eventResolvers = require('../../src/resolvers/event') as { }; }; -const bulkVisitEvent = jest.fn(); +const bulkVisitEvents = jest.fn(); describe('Mutation.bulkVisitEvents', () => { const ctx = { @@ -26,11 +26,11 @@ describe('Mutation.bulkVisitEvents', () => { beforeEach(() => { jest.clearAllMocks(); - (getEventsFactory as unknown as jest.Mock).mockReturnValue({ bulkVisitEvent }); + (getEventsFactory as unknown as jest.Mock).mockReturnValue({ bulkVisitEvents }); }); it('should call factory with valid ids only and merge invalid ids', async () => { - bulkVisitEvent.mockResolvedValue({ + bulkVisitEvents.mockResolvedValue({ updatedCount: 1, updatedEventIds: [ '507f1f77bcf86cd799439012' ], failedEventIds: [ '507f1f77bcf86cd799439099' ], @@ -42,7 +42,7 @@ describe('Mutation.bulkVisitEvents', () => { ctx ); - expect(bulkVisitEvent).toHaveBeenCalledWith( + expect(bulkVisitEvents).toHaveBeenCalledWith( [ '507f1f77bcf86cd799439012' ], '507f1f77bcf86cd799439011' ); @@ -60,7 +60,7 @@ describe('Mutation.bulkVisitEvents', () => { ctx ); - expect(bulkVisitEvent).not.toHaveBeenCalled(); + expect(bulkVisitEvents).not.toHaveBeenCalled(); expect(result).toEqual({ updatedCount: 0, updatedEventIds: [],