more testing and queue work

src/services/backgroundJobService.test.ts

@@ -1,23 +1,23 @@
 // src/services/backgroundJobService.test.ts
-import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
 
 // Mock the 'node-cron' module
 const mockCronSchedule = vi.fn();
 vi.mock('node-cron', () => ({ default: { schedule: mockCronSchedule } }));
 
 import { BackgroundJobService, startBackgroundJobs } from './backgroundJobService';
+import type { Queue } from 'bullmq';
 import { WatchedItemDeal } from '../types';
 import { AdminUserView } from './db/admin.db';
 
 describe('Background Job Service', () => {
   // Create mock dependencies that will be injected into the service
   const mockDbService = {
-    getAllUsers: vi.fn(),
-    getBestSalePricesForUser: vi.fn(),
     createBulkNotifications: vi.fn(),
+    getBestSalePricesForAllUsers: vi.fn(),
   };
-  const mockEmailService = {
-    sendDealNotificationEmail: vi.fn(),
+  const mockEmailQueue = {
+    add: vi.fn(),
   };
   const mockLogger = {
     info: vi.fn(),
@@ -30,75 +30,78 @@ describe('Background Job Service', () => {
     vi.clearAllMocks();
   });
 
-  describe('runDailyDealCheck', () => {
-    const mockUsers: AdminUserView[] = [
-      { user_id: 'user-1', email: 'user1@test.com', full_name: 'User One', role: 'user', created_at: '', avatar_url: null },
-      { user_id: 'user-2', email: 'user2@test.com', full_name: 'User Two', role: 'user', created_at: '', avatar_url: null },
-    ];
+  afterEach(() => {
+    vi.useRealTimers();
+  });
 
-    const mockDeals: WatchedItemDeal[] = [
-      { master_item_id: 1, item_name: 'Apples', best_price_in_cents: 199, store_name: 'Green Grocer', flyer_id: 101, valid_to: '2024-10-20' },
+  describe('runDailyDealCheck', () => {
+    // Mock data representing the result of the new single database query
+    const mockDealsForAllUsers = [
+      // Deals for user-1
+      { user_id: 'user-1', email: 'user1@test.com', full_name: 'User One', master_item_id: 1, item_name: 'Apples', best_price_in_cents: 199, store_name: 'Green Grocer', flyer_id: 101, valid_to: '2024-10-20' },
+      // Deals for user-2
+      { user_id: 'user-2', email: 'user2@test.com', full_name: 'User Two', master_item_id: 2, item_name: 'Milk', best_price_in_cents: 450, store_name: 'Dairy Farm', flyer_id: 102, valid_to: '2024-10-21' },
+      { user_id: 'user-2', email: 'user2@test.com', full_name: 'User Two', master_item_id: 3, item_name: 'Bread', best_price_in_cents: 250, store_name: 'Bakery', flyer_id: 103, valid_to: '2024-10-22' },
     ];
 
     // Instantiate the service with mock dependencies for each test run
-    const service = new BackgroundJobService(mockDbService, mockEmailService, mockLogger);
+    const service = new BackgroundJobService(mockDbService as any, mockEmailQueue as unknown as Queue<any>, mockLogger);
 
-    it('should do nothing if no users are found', async () => {
-      mockDbService.getAllUsers.mockResolvedValue([]);
+    it('should do nothing if no deals are found for any user', async () => {
+      mockDbService.getBestSalePricesForAllUsers.mockResolvedValue([]);
 
       await service.runDailyDealCheck();
 
       expect(mockLogger.info).toHaveBeenCalledWith('[BackgroundJob] Starting daily deal check for all users...');
-      expect(mockLogger.info).toHaveBeenCalledWith('[BackgroundJob] No users found. Skipping deal check.');
-      expect(mockDbService.getBestSalePricesForUser).not.toHaveBeenCalled();
-      expect(mockEmailService.sendDealNotificationEmail).not.toHaveBeenCalled();
+      expect(mockLogger.info).toHaveBeenCalledWith('[BackgroundJob] No deals found for any watched items. Skipping.');
+      expect(mockDbService.getBestSalePricesForAllUsers).toHaveBeenCalledTimes(1);
+      expect(mockEmailQueue.add).not.toHaveBeenCalled();
       expect(mockDbService.createBulkNotifications).not.toHaveBeenCalled();
     });
 
-    it('should process users but not send notifications if no deals are found', async () => {
-      mockDbService.getAllUsers.mockResolvedValue([mockUsers[0]]);
-      mockDbService.getBestSalePricesForUser.mockResolvedValue([]);
+    it('should create notifications and enqueue emails when deals are found', async () => {
+      mockDbService.getBestSalePricesForAllUsers.mockResolvedValue(mockDealsForAllUsers);
 
       await service.runDailyDealCheck();
 
-      expect(mockDbService.getBestSalePricesForUser).toHaveBeenCalledWith('user-1');
-      expect(mockEmailService.sendDealNotificationEmail).not.toHaveBeenCalled();
-      expect(mockDbService.createBulkNotifications).not.toHaveBeenCalled();
-      expect(mockLogger.info).toHaveBeenCalledWith('[BackgroundJob] Daily deal check completed successfully.');
-    });
+      // Check that it fetched all deals once
+      expect(mockDbService.getBestSalePricesForAllUsers).toHaveBeenCalledTimes(1);
 
-    it('should create notifications and send emails when deals are found', async () => {
-      mockDbService.getAllUsers.mockResolvedValue(mockUsers);
-      mockDbService.getBestSalePricesForUser.mockResolvedValue(mockDeals);
-
-      await service.runDailyDealCheck();
-
-      // Check that it processed both users
-      expect(mockDbService.getBestSalePricesForUser).toHaveBeenCalledTimes(2);
-      expect(mockDbService.getBestSalePricesForUser).toHaveBeenCalledWith('user-1');
-      expect(mockDbService.getBestSalePricesForUser).toHaveBeenCalledWith('user-2');
-
-      // Check that emails were sent for both users
-      expect(mockEmailService.sendDealNotificationEmail).toHaveBeenCalledTimes(2);
-      expect(mockEmailService.sendDealNotificationEmail).toHaveBeenCalledWith('user1@test.com', 'User One', mockDeals);
+      // Check that email jobs were enqueued for both users
+      expect(mockEmailQueue.add).toHaveBeenCalledTimes(2);
+      const firstEmailJob = mockEmailQueue.add.mock.calls[0];
+      expect(firstEmailJob[1].to).toBe('user1@test.com');
+      const secondEmailJob = mockEmailQueue.add.mock.calls[1];
+      expect(secondEmailJob[1].to).toBe('user2@test.com');
+      // Check the content for the user with multiple deals
+      expect(secondEmailJob[1].html).toContain('Milk');
+      expect(secondEmailJob[1].html).toContain('Bread');
 
       // Check that in-app notifications were created for both users
       expect(mockDbService.createBulkNotifications).toHaveBeenCalledTimes(1);
       const notificationPayload = mockDbService.createBulkNotifications.mock.calls[0][0];
       expect(notificationPayload).toHaveLength(2);
-      expect(notificationPayload[0]).toEqual({
-        user_id: 'user-1',
-        content: 'You have 1 new deal(s) on your watched items!',
+      // Use expect.arrayContaining to be order-agnostic.
+      expect(notificationPayload).toEqual(expect.arrayContaining([
+        {
+          user_id: 'user-1',
+          content: 'You have 1 new deal(s) on your watched items!',
+          link_url: '/dashboard/deals',
+        },
+        {
+        user_id: 'user-2',
+        content: 'You have 2 new deal(s) on your watched items!',
         link_url: '/dashboard/deals',
-      });
+        }
+      ]));
     });
 
     it('should handle and log errors for individual users without stopping the process', async () => {
-      mockDbService.getAllUsers.mockResolvedValue(mockUsers);
-      // First user fails, second succeeds
-      mockDbService.getBestSalePricesForUser
-        .mockRejectedValueOnce(new Error('User 1 DB Error'))
-        .mockResolvedValueOnce(mockDeals);
+      mockDbService.getBestSalePricesForAllUsers.mockResolvedValue(mockDealsForAllUsers);
+      // Simulate the email queue failing for the first user but succeeding for the second
+      mockEmailQueue.add
+        .mockRejectedValueOnce(new Error('Email queue is down'))
+        .mockResolvedValueOnce({ id: 'job-2' });
 
       await service.runDailyDealCheck();
 
@@ -109,17 +112,16 @@ describe('Background Job Service', () => {
       );
 
       // Check that it still processed user 2 successfully
-      expect(mockEmailService.sendDealNotificationEmail).toHaveBeenCalledTimes(1);
-      expect(mockEmailService.sendDealNotificationEmail).toHaveBeenCalledWith('user2@test.com', 'User Two', mockDeals);
+      // The email queue add should be attempted for both users.
+      expect(mockEmailQueue.add).toHaveBeenCalledTimes(2);
       expect(mockDbService.createBulkNotifications).toHaveBeenCalledTimes(1);
       expect(mockDbService.createBulkNotifications.mock.calls[0][0]).toHaveLength(1); // Only one notification created
+      expect(mockDbService.createBulkNotifications.mock.calls[0][0][0].user_id).toBe('user-2');
     });
 
-    it('should log a critical error if getAllUsers fails', async () => {
-      mockDbService.getAllUsers.mockRejectedValue(new Error('Critical DB Failure'));
-
-      await service.runDailyDealCheck();
-
+    it('should log a critical error if getBestSalePricesForAllUsers fails', async () => {
+      mockDbService.getBestSalePricesForAllUsers.mockRejectedValue(new Error('Critical DB Failure'));
+      await expect(service.runDailyDealCheck()).rejects.toThrow('Critical DB Failure');
       expect(mockLogger.error).toHaveBeenCalledWith(
         '[BackgroundJob] A critical error occurred during the daily deal check:',
         expect.any(Object)
@@ -128,19 +130,66 @@ describe('Background Job Service', () => {
   });
 
   describe('startBackgroundJobs', () => {
-    it('should schedule the cron job with the correct schedule and function', () => {
-      startBackgroundJobs();
+    const mockBackgroundJobService = {
+      runDailyDealCheck: vi.fn(),
+    } as unknown as BackgroundJobService;
 
-      // Expect at least one job to be scheduled
-      expect(mockCronSchedule).toHaveBeenCalled();
-      
-      // Check specifically for the daily deal check job
+    const mockAnalyticsQueue = {
+      add: vi.fn(),
+    } as unknown as Queue;
+
+    beforeEach(() => {
+      mockCronSchedule.mockClear();
+      vi.mocked(mockBackgroundJobService.runDailyDealCheck).mockClear();
+      vi.mocked(mockAnalyticsQueue.add).mockClear();
+    });
+
+    it('should schedule two cron jobs with the correct schedules', () => {
+      startBackgroundJobs(mockBackgroundJobService, mockAnalyticsQueue);
+
+      expect(mockCronSchedule).toHaveBeenCalledTimes(2);
       expect(mockCronSchedule).toHaveBeenCalledWith('0 2 * * *', expect.any(Function));
-      
-      // We can't directly test the bound function instance easily, but we can check the logger
-      // which is called from within startBackgroundJobs.
-      // The mockLogger from the `runDailyDealCheck` scope won't be called here.
-      // This test now primarily verifies that cron.schedule is called correctly.
+      expect(mockCronSchedule).toHaveBeenCalledWith('0 3 * * *', expect.any(Function));
+    });
+
+    it('should call runDailyDealCheck when the first cron job function is executed', async () => {
+      startBackgroundJobs(mockBackgroundJobService, mockAnalyticsQueue);
+
+      // Get the callback function for the first cron job
+      const dailyDealCheckCallback = mockCronSchedule.mock.calls[0][1];
+      await dailyDealCheckCallback();
+
+      expect(mockBackgroundJobService.runDailyDealCheck).toHaveBeenCalledTimes(1);
+    });
+
+    it('should prevent runDailyDealCheck from running if it is already in progress', async () => {
+      // Use fake timers to control promise resolution
+      vi.useFakeTimers();
+
+      // Make the first call hang indefinitely
+      vi.mocked(mockBackgroundJobService.runDailyDealCheck).mockReturnValue(new Promise(() => {}));
+
+      startBackgroundJobs(mockBackgroundJobService, mockAnalyticsQueue);
+      const dailyDealCheckCallback = mockCronSchedule.mock.calls[0][1];
+
+      // Trigger the job once, it will hang
+      const firstCall = dailyDealCheckCallback();
+      // Trigger it a second time immediately
+      const secondCall = dailyDealCheckCallback();
+
+      await Promise.all([firstCall, secondCall]);
+
+      // The service method should only have been called once
+      expect(mockBackgroundJobService.runDailyDealCheck).toHaveBeenCalledTimes(1);
+    });
+
+    it('should enqueue an analytics job when the second cron job function is executed', async () => {
+      startBackgroundJobs(mockBackgroundJobService, mockAnalyticsQueue);
+
+      const analyticsJobCallback = mockCronSchedule.mock.calls[1][1];
+      await analyticsJobCallback();
+
+      expect(mockAnalyticsQueue.add).toHaveBeenCalledWith('generate-daily-report', expect.any(Object), expect.any(Object));
     });
   });
 });