fix schedduler. update test cases.

backend/src/schedulers/__mocks__/create-job.ts

@@ -0,0 +1,20 @@
+/** This mock returns a fake CronJob class. Running start() will immediate
+ *  execute the callback instead of waiting for specified time */
+const mockCreateJob = jest.fn(
+  (cronTime, callback, mutex, { title, message }) => {
+    return {
+      start: jest.fn(async () => {
+        console.log(`Mock run`);
+        try {
+          await callback(); // Simulate the callback execution
+        } catch (e) {
+          console.error(`Mock error`);
+        } finally {
+          console.log(`Mock end run`);
+        }
+      }),
+    };
+  },
+);
+
+export { mockCreateJob as createJob };

backend/src/schedulers/delete-announcements-scheduler.spec.ts

@@ -0,0 +1,29 @@
+import waitFor from 'wait-for-expect';
+import deleteAnnouncementsJob from './delete-announcements-scheduler';
+import { announcementService } from '../v1/services/announcements-service';
+
+jest.mock('../config', () => ({
+  config: {
+    get: (key: string) => {
+      const settings = {
+        'server:deleteAnnouncementsCronTime': '121212121',
+      };
+
+      return settings[key];
+    },
+  },
+}));
+
+jest.mock('./create-job');
+jest.mock('../v1/services/announcements-service');
+
+describe('delete-announcements-scheduler', () => {
+  it('should run the function', async () => {
+    deleteAnnouncementsJob.start();
+    await waitFor(async () => {
+      expect(
+        announcementService.deleteAnnouncementsSchedule,
+      ).toHaveBeenCalled();
+    });
+  });
+});

backend/src/schedulers/delete-announcements-scheduler.ts

@@ -1,8 +1,8 @@
 import { config } from '../config';
-import { schedulerService } from '../v1/services/scheduler-service';
 import { logger as log } from '../logger';
 import advisoryLock from 'advisory-lock';
 import { createJob } from './create-job';
+import { announcementService } from '../v1/services/announcements-service';
 
 const SCHEDULER_NAME = 'DeleteAnnouncements';
 const mutex = advisoryLock(config.get('server:databaseUrl'))(
@@ -14,7 +14,7 @@ export default createJob(
   crontime,
   async () => {
     log.info(`Starting scheduled job '${SCHEDULER_NAME}'.`);
-    await schedulerService.deleteAnnouncementSchedule();
+    await announcementService.deleteAnnouncementsSchedule();
     log.info(`Completed scheduled job '${SCHEDULER_NAME}'.`);
   },
   mutex,

backend/src/schedulers/run.all.spec.ts

@@ -45,6 +45,15 @@ jest.mock('./email-expiring-announcements-scheduler', () => ({
   },
 }));
 
+const mockDeleteAnnouncementsScheduler = jest.fn();
+jest.mock('./delete-announcements-scheduler', () => ({
+  __esModule: true,
+
+  default: {
+    start: () => mockDeleteAnnouncementsScheduler(),
+  },
+}));
+
 describe('run.all', () => {
   it('should start all jobs', async () => {
     run();
@@ -53,5 +62,6 @@ describe('run.all', () => {
     expect(mockStartReportLock).toHaveBeenCalled();
     expect(mockExpireAnnouncementsLock).toHaveBeenCalled();
     expect(mockEmailExpiringAnnouncementsScheduler).toHaveBeenCalled();
+    expect(mockDeleteAnnouncementsScheduler).toHaveBeenCalled();
   });
 });

backend/src/schedulers/run.all.ts

@@ -4,6 +4,7 @@ import deleteUserErrorsJob from './delete-user-errors-scheduler';
 import lockReportsJob from './lock-reports-scheduler';
 import expireAnnouncementsJob from './expire-announcements-scheduler';
 import emailExpiringAnnouncementsJob from './email-expiring-announcements-scheduler';
+import deleteAnnouncementsJob from './delete-announcements-scheduler';
 
 export const run = () => {
   try {
@@ -12,6 +13,7 @@ export const run = () => {
     lockReportsJob?.start();
     expireAnnouncementsJob?.start();
     emailExpiringAnnouncementsJob?.start();
+    deleteAnnouncementsJob?.start();
   } catch (error) {
     /* istanbul ignore next */
     logger.error(error);

backend/src/v1/services/announcements-service.spec.ts

@@ -977,23 +977,23 @@ describe('AnnouncementsService', () => {
 
       mockS3ApiDeleteFiles.mockResolvedValue(new Set(['file1', 'file2'])); // files deleted
 
-      mockDeleteManyHistory.mockResolvedValue({});
-      mockDeleteManyResource.mockResolvedValue({});
       mockDeleteManyResourceHistory.mockResolvedValue({});
+      mockDeleteManyResource.mockResolvedValue({});
+      mockDeleteManyHistory.mockResolvedValue({});
       mockDeleteMany.mockResolvedValue({});
 
       await announcementService.deleteAnnouncementsSchedule();
 
       expect(mockS3ApiDeleteFiles).toHaveBeenCalledWith(['file1', 'file2']);
 
       // Two files, so each of these are called twice
-      expect(mockDeleteManyHistory).toHaveBeenCalledTimes(2);
-      expect(mockDeleteManyResource).toHaveBeenCalledTimes(2);
       expect(mockDeleteManyResourceHistory).toHaveBeenCalledTimes(2);
+      expect(mockDeleteManyResource).toHaveBeenCalledTimes(2);
+      expect(mockDeleteManyHistory).toHaveBeenCalledTimes(2);
       expect(mockDeleteMany).toHaveBeenCalledTimes(2);
     });
 
-    it('should log an error if announcement deletion fails', async () => {
+    it("shouldn't delete from the database when s3 fails to delete files", async () => {
       mockFindMany.mockResolvedValueOnce([
         { announcement_id: 1, title: 'Announcement 1' },
         { announcement_id: 2, title: 'Announcement 2' },
@@ -1008,13 +1008,54 @@ describe('AnnouncementsService', () => {
       await announcementService.deleteAnnouncementsSchedule();
 
       // Even though there are two files, only one of them was deleted
-      expect(mockDeleteManyHistory).toHaveBeenCalledTimes(1);
-      expect(mockDeleteManyResource).toHaveBeenCalledTimes(1);
       expect(mockDeleteManyResourceHistory).toHaveBeenCalledTimes(1);
+      expect(mockDeleteManyResource).toHaveBeenCalledTimes(1);
+      expect(mockDeleteManyHistory).toHaveBeenCalledTimes(1);
       expect(mockDeleteMany).toHaveBeenCalledTimes(1);
       expect(mockDeleteMany).toHaveBeenCalledWith({
         where: { announcement_id: 2 },
       });
     });
+
+    it("shouldn't do anything if there's nothing to delete", async () => {
+      //test that no announcements were found
+      mockFindMany.mockResolvedValueOnce([]);
+      await announcementService.deleteAnnouncementsSchedule();
+      expect(mockFindManyResource).toHaveBeenCalledTimes(0); //should return before this function is called
+    });
+
+    it("shouldn't do anything if nothing is safe to delete", async () => {
+      //test that if the resources that were found couldn't be deleted from s3, then nothing happens
+      mockFindMany.mockResolvedValueOnce([
+        { announcement_id: 1, title: 'Announcement 1' },
+        { announcement_id: 2, title: 'Announcement 2' },
+      ]);
+      mockFindManyResource.mockResolvedValueOnce([
+        { announcement_id: 1, attachment_file_id: 'file1' },
+        { announcement_id: 2, attachment_file_id: 'file2' },
+      ]);
+      mockS3ApiDeleteFiles.mockResolvedValue(new Set([])); // didn't delete anything
+
+      await announcementService.deleteAnnouncementsSchedule();
+      expect(mockDeleteManyHistory).toHaveBeenCalledTimes(0); //should return before this function is called
+    });
+
+    it('should log if database failed to delete', async () => {
+      //test that if the resources that were found couldn't be deleted from s3, then nothing happens
+      mockFindMany.mockResolvedValueOnce([
+        { announcement_id: 1, title: 'Announcement 1' },
+        { announcement_id: 2, title: 'Announcement 2' },
+      ]);
+      mockFindManyResource.mockResolvedValueOnce([
+        { announcement_id: 1, attachment_file_id: 'file1' },
+        { announcement_id: 2, attachment_file_id: 'file2' },
+      ]);
+      mockS3ApiDeleteFiles.mockResolvedValue(new Set(['file1'])); // didn't delete anything
+      mockDeleteManyResourceHistory.mockRejectedValue(new Error('err'));
+
+      await announcementService.deleteAnnouncementsSchedule();
+      expect(mockDeleteManyResourceHistory).toHaveBeenCalledTimes(1);
+      expect(mockDeleteManyHistory).toHaveBeenCalledTimes(0); //should error before this function is called
+    });
   });
 });