feat(server): wait 60 seconds before sending email on new album item

Album update jobs will now wait a minute to send. If a new image is added while that job is pending, the old job will be cancelled, and a new one will be enqueued for a minute.

This is to prevent a flood of notifications by dragging in images directly to the album, which adds them to the album one at a time.

Album updates now include a list of users to email, which is generally everybody except the updater. If somebody else updates the album within that minute, both people will get an album update email in a minute, as they both added images and the other should be notified.

server/src/interfaces/event.interface.ts

@@ -14,7 +14,7 @@ type EmitEventMap = {
   'config.validate': [{ newConfig: SystemConfig; oldConfig: SystemConfig }];
 
   // album events
-  'album.update': [{ id: string; deliverTo: string[] }];
+  'album.update': [{ id: string; recipientIds: string[] }];
   'album.invite': [{ id: string; userId: string }];
 
   // tag events

server/src/interfaces/job.interface.ts

@@ -188,7 +188,7 @@ export interface INotifyAlbumInviteJob extends IEntityJob {
 }
 
 export interface INotifyAlbumUpdateJob extends IEntityJob, IDelayedJob {
-  deliverTo: string[];
+  recipientIds: string[];
 }
 
 export interface JobCounts {

server/src/repositories/job.repository.ts

@@ -283,8 +283,7 @@ export class JobRepository implements IJobRepository {
       return;
     }
     const queueName = JOBS_TO_QUEUE[item.name]
-    const existingJob = (await this.getQueue(queueName).getJobs('delayed'))
-      .find(job => job.id === data.id);
+    const existingJob = (await this.getQueue(queueName).getJob(data.id));
     if (existingJob) {
       try {
         await existingJob.remove();

server/src/services/album.service.spec.ts

@@ -574,7 +574,7 @@ describe(AlbumService.name, () => {
       expect(albumMock.addAssetIds).toHaveBeenCalledWith('album-123', ['asset-1', 'asset-2', 'asset-3']);
       expect(eventMock.emit).toHaveBeenCalledWith('album.update', {
         id: 'album-123',
-        deliverTo: []
+        recipientIds: []
       });
     });
 
@@ -618,7 +618,7 @@ describe(AlbumService.name, () => {
       expect(albumMock.addAssetIds).toHaveBeenCalledWith('album-123', ['asset-1', 'asset-2', 'asset-3']);
       expect(eventMock.emit).toHaveBeenCalledWith('album.update', {
         id: 'album-123',
-        deliverTo: ['admin_id']
+        recipientIds: ['admin_id']
       });
     });
 

server/src/services/album.service.ts

@@ -195,7 +195,7 @@ export class AlbumService {
       const allUsersExceptUs = [...album.albumUsers.map(au => au.user.id), album.owner.id]
         .filter(userId => userId !== auth.user.id);
 
-      await this.eventRepository.emit('album.update', { id, deliverTo: allUsersExceptUs });
+      await this.eventRepository.emit('album.update', { id, recipientIds: allUsersExceptUs });
     }
 
     return results;

server/src/services/notification.service.spec.ts

@@ -24,6 +24,7 @@ import { newSystemMetadataRepositoryMock } from 'test/repositories/system-metada
 import { newUserRepositoryMock } from 'test/repositories/user.repository.mock';
 import { Mocked } from 'vitest';
 import { any } from 'joi';
+import mock from 'mock-fs';
 
 const configs = {
   smtpDisabled: Object.freeze<SystemConfig>({
@@ -149,10 +150,10 @@ describe(NotificationService.name, () => {
 
   describe('onAlbumUpdateEvent', () => {
     it('should queue notify album update event', async () => {
-      await sut.onAlbumUpdate({ id: 'album', deliverTo: '42' });
+      await sut.onAlbumUpdate({ id: 'album', recipientIds: ['42'] });
       expect(jobMock.replaceQueuedOrInsertJob).toHaveBeenCalledWith({
         name: JobName.NOTIFY_ALBUM_UPDATE,
-        data: { id: 'album', deliverTo: '42', delay: 60_000 },
+        data: { id: 'album', recipientIds: ['42'], delay: 60_000 },
       }, expect.anything());
     });
   });
@@ -423,14 +424,14 @@ describe(NotificationService.name, () => {
 
   describe('handleAlbumUpdate', () => {
     it('should skip if album could not be found', async () => {
-      await expect(sut.handleAlbumUpdate({ id: '', senderId: '' })).resolves.toBe(JobStatus.SKIPPED);
+      await expect(sut.handleAlbumUpdate({ id: '', recipientIds: ['1'] })).resolves.toBe(JobStatus.SKIPPED);
       expect(userMock.get).not.toHaveBeenCalled();
     });
 
     it('should skip if owner could not be found', async () => {
       albumMock.getById.mockResolvedValue(albumStub.emptyWithValidThumbnail);
 
-      await expect(sut.handleAlbumUpdate({ id: '', senderId: '' })).resolves.toBe(JobStatus.SKIPPED);
+      await expect(sut.handleAlbumUpdate({ id: '', senderId: ['1'] })).resolves.toBe(JobStatus.SKIPPED);
       expect(systemMock.get).not.toHaveBeenCalled();
     });
 
@@ -442,7 +443,7 @@ describe(NotificationService.name, () => {
       userMock.get.mockResolvedValueOnce(userStub.user1);
       notificationMock.renderEmail.mockResolvedValue({ html: '', text: '' });
 
-      await sut.handleAlbumUpdate({ id: '', deliverTo: [userStub.user1.id] });
+      await sut.handleAlbumUpdate({ id: '', recipientIds: [userStub.user1.id] });
       expect(userMock.get).toHaveBeenCalledWith(userStub.user1.id, { withDeleted: false });
       expect(notificationMock.renderEmail).not.toHaveBeenCalled();
     });
@@ -465,7 +466,7 @@ describe(NotificationService.name, () => {
       });
       notificationMock.renderEmail.mockResolvedValue({ html: '', text: '' });
 
-      await sut.handleAlbumUpdate({ id: '', deliverTo: [userStub.user1.id] });
+      await sut.handleAlbumUpdate({ id: '', recipientIds: [userStub.user1.id] });
       expect(userMock.get).toHaveBeenCalledWith(userStub.user1.id, { withDeleted: false });
       expect(notificationMock.renderEmail).not.toHaveBeenCalled();
     });
@@ -488,7 +489,7 @@ describe(NotificationService.name, () => {
       });
       notificationMock.renderEmail.mockResolvedValue({ html: '', text: '' });
 
-      await sut.handleAlbumUpdate({ id: '', deliverTo: [userStub.user1.id] });
+      await sut.handleAlbumUpdate({ id: '', recipientIds: [userStub.user1.id] });
       expect(userMock.get).toHaveBeenCalledWith(userStub.user1.id, { withDeleted: false });
       expect(notificationMock.renderEmail).not.toHaveBeenCalled();
     });
@@ -501,7 +502,7 @@ describe(NotificationService.name, () => {
       userMock.get.mockResolvedValue(userStub.user1);
       notificationMock.renderEmail.mockResolvedValue({ html: '', text: '' });
 
-      await sut.handleAlbumUpdate({ id: '', deliverTo: [userStub.user1.id] });
+      await sut.handleAlbumUpdate({ id: '', recipientIds: [userStub.user1.id] });
       expect(userMock.get).toHaveBeenCalledWith(userStub.user1.id, { withDeleted: false });
       expect(notificationMock.renderEmail).toHaveBeenCalled();
       expect(jobMock.queue).toHaveBeenCalled();

server/src/services/notification.service.ts

@@ -66,19 +66,19 @@ export class NotificationService {
   }
 
   @OnEmit({ event: 'album.update' })
-  async onAlbumUpdate({ id, deliverTo }: ArgOf<'album.update'>) {
+  async onAlbumUpdate({ id, recipientIds }: ArgOf<'album.update'>) {
     // if deliverTo is empty, album likely only has one user part of it, don't queue notification if so
-    if (!deliverTo) {
+    if (!recipientIds) {
       return;
     }
     const job: JobItem = {
       name: JobName.NOTIFY_ALBUM_UPDATE,
-      data: { id, deliverTo, delay: NotificationService.albumUpdateEmailDelayMs },
+      data: { id, recipientIds, delay: NotificationService.albumUpdateEmailDelayMs },
     };
     await this.jobRepository.replaceQueuedOrInsertJob(job, (fromExistingJob, fromEnqueueingJob) => {
-      for (let string of fromExistingJob.deliverTo) {
-        if (!fromEnqueueingJob.deliverTo.includes(string)) {
-          fromEnqueueingJob.deliverTo.push(string);
+      for (let string of fromExistingJob.recipientIds) {
+        if (!fromEnqueueingJob.recipientIds.includes(string)) {
+          fromEnqueueingJob.recipientIds.push(string);
         }
       }
       return fromEnqueueingJob;
@@ -198,7 +198,7 @@ export class NotificationService {
     return JobStatus.SUCCESS;
   }
 
-  async handleAlbumUpdate({ id, deliverTo }: INotifyAlbumUpdateJob) {
+  async handleAlbumUpdate({ id, recipientIds }: INotifyAlbumUpdateJob) {
     const album = await this.albumRepository.getById(id, { withAssets: false });
 
     if (!album) {
@@ -210,7 +210,7 @@ export class NotificationService {
       return JobStatus.SKIPPED;
     }
 
-    const recipients = [...album.albumUsers.map((user) => user.user), owner].filter((user) => deliverTo.includes(user.id));
+    const recipients = [...album.albumUsers.map((user) => user.user), owner].filter((user) => recipientIds.includes(user.id));
     const attachment = await this.getAlbumThumbnailAttachment(album);
 
     const { server } = await this.configCore.getConfig({ withCache: false });