lots more tests !

src/services/db/admin.db.test.ts

@@ -1,6 +1,7 @@
 // src/services/db/admin.db.test.ts
 import { describe, it, expect, vi, beforeEach } from 'vitest';
 import { mockPoolInstance } from '../../tests/setup/tests-setup-unit';
+import { ForeignKeyConstraintError } from './errors.db';
 import { AdminRepository } from './admin.db';
 import type { SuggestedCorrection, AdminUserView, User } from '../../types';
 
@@ -23,6 +24,11 @@ describe('Admin DB Service', () => {
   beforeEach(() => {
     // Reset the global mock's call history before each test.
     vi.clearAllMocks();
+
+    // For transactional methods, we mock the client returned by `connect()`
+    const mockClient = { ...mockPoolInstance, release: vi.fn() };
+    vi.mocked(mockPoolInstance.connect).mockResolvedValue(mockClient as any);
+
     // Instantiate the repository with the mock pool for each test
     adminRepo = new AdminRepository(mockPoolInstance as any);
   });
@@ -253,6 +259,94 @@ describe('Admin DB Service', () => {
     });
   });
 
+  describe('resolveUnmatchedFlyerItem', () => {
+    it('should execute a transaction to resolve an unmatched item', async () => {
+      const mockClient = { query: vi.fn(), release: vi.fn() };
+      vi.mocked(mockPoolInstance.connect).mockResolvedValue(mockClient as any);
+
+      // Mock the sequence of calls within the transaction
+      mockClient.query
+        .mockResolvedValueOnce({ rows: [] }) // BEGIN
+        .mockResolvedValueOnce({ rows: [{ flyer_item_id: 55 }] }) // SELECT flyer_item_id
+        .mockResolvedValueOnce({ rowCount: 1 }) // UPDATE flyer_items
+        .mockResolvedValueOnce({ rowCount: 1 }) // UPDATE unmatched_flyer_items
+        .mockResolvedValueOnce({ rows: [] }); // COMMIT
+
+      await adminRepo.resolveUnmatchedFlyerItem(1, 101);
+
+      expect(mockClient.query).toHaveBeenCalledWith('BEGIN');
+      expect(mockClient.query).toHaveBeenCalledWith(expect.stringContaining('SELECT flyer_item_id FROM public.unmatched_flyer_items'), [1]);
+      expect(mockClient.query).toHaveBeenCalledWith(expect.stringContaining('UPDATE public.flyer_items'), [101, 55]);
+      expect(mockClient.query).toHaveBeenCalledWith(expect.stringContaining("UPDATE public.unmatched_flyer_items SET status = 'resolved'"), [1]);
+      expect(mockClient.query).toHaveBeenCalledWith('COMMIT');
+      expect(mockClient.release).toHaveBeenCalled();
+    });
+
+    it('should throw an error if the unmatched item is not found', async () => {
+      const mockClient = { query: vi.fn(), release: vi.fn() };
+      vi.mocked(mockPoolInstance.connect).mockResolvedValue(mockClient as any);
+
+      // Mock the SELECT to find no item
+      mockClient.query
+        .mockResolvedValueOnce({ rows: [] }) // BEGIN
+        .mockResolvedValueOnce({ rowCount: 0, rows: [] }); // SELECT finds nothing
+
+      await expect(adminRepo.resolveUnmatchedFlyerItem(999, 101)).rejects.toThrow('Failed to resolve unmatched flyer item.');
+      expect(mockClient.query).toHaveBeenCalledWith('ROLLBACK');
+      expect(mockClient.release).toHaveBeenCalled();
+    });
+
+    it('should rollback transaction on generic error', async () => {
+      const mockClient = { query: vi.fn(), release: vi.fn() };
+      vi.mocked(mockPoolInstance.connect).mockResolvedValue(mockClient as any);
+      const dbError = new Error('DB Error');
+
+      // Mock the second UPDATE to fail
+      mockClient.query
+        .mockResolvedValueOnce({ rows: [] }) // BEGIN
+        .mockResolvedValueOnce({ rows: [{ flyer_item_id: 55 }] }) // SELECT flyer_item_id
+        .mockRejectedValueOnce(dbError); // UPDATE flyer_items fails
+
+      await expect(adminRepo.resolveUnmatchedFlyerItem(1, 101)).rejects.toThrow('Failed to resolve unmatched flyer item.');
+      expect(mockClient.query).toHaveBeenCalledWith('ROLLBACK');
+      expect(mockClient.release).toHaveBeenCalled();
+    });
+  });
+
+  describe('ignoreUnmatchedFlyerItem', () => {
+    it('should update the status of an unmatched item to "ignored"', async () => {
+      mockPoolInstance.query.mockResolvedValue({ rowCount: 1 });
+      await adminRepo.ignoreUnmatchedFlyerItem(1);
+      expect(mockPoolInstance.query).toHaveBeenCalledWith("UPDATE public.unmatched_flyer_items SET status = 'ignored' WHERE unmatched_flyer_item_id = $1", [1]);
+    });
+
+    it('should throw an error if the database query fails', async () => {
+      const dbError = new Error('DB Error');
+      mockPoolInstance.query.mockRejectedValue(dbError);
+      await expect(adminRepo.ignoreUnmatchedFlyerItem(1)).rejects.toThrow('Failed to ignore unmatched flyer item.');
+    });
+  });
+
+  describe('resetFailedLoginAttempts', () => {
+    it('should execute an UPDATE query to reset failed attempts', async () => {
+      mockPoolInstance.query.mockResolvedValue({ rows: [] });
+      await adminRepo.resetFailedLoginAttempts('user-123', '127.0.0.1');
+
+      expect(mockPoolInstance.query).toHaveBeenCalledWith(
+        expect.stringContaining('UPDATE public.users'),
+        ['user-123', '127.0.0.1']
+      );
+    });
+
+    it('should not throw an error if the database query fails (non-critical)', async () => {
+      const dbError = new Error('DB Error');
+      mockPoolInstance.query.mockRejectedValue(dbError);
+      await expect(adminRepo.resetFailedLoginAttempts('user-123', '127.0.0.1')).resolves.toBeUndefined();
+      const { logger } = await import('../logger.server');
+      expect(logger.error).toHaveBeenCalledWith(expect.stringContaining('resetFailedLoginAttempts'), expect.any(Object));
+    });
+  });
+
   describe('incrementFailedLoginAttempts', () => {
     it('should execute an UPDATE query to increment failed attempts', async () => {
       mockPoolInstance.query.mockResolvedValue({ rows: [] });
@@ -351,4 +445,13 @@ describe('Admin DB Service', () => {
       await expect(adminRepo.updateUserRole('1', 'admin')).rejects.toThrow('DB Error');
     });
   });
+
+  it('should throw ForeignKeyConstraintError if the user does not exist on update', async () => {
+    const dbError = new Error('violates foreign key constraint');
+    (dbError as any).code = '23503';
+    mockPoolInstance.query.mockRejectedValue(dbError);
+
+    await expect(adminRepo.updateUserRole('non-existent-user', 'admin')).rejects.toThrow(ForeignKeyConstraintError);
+    await expect(adminRepo.updateUserRole('non-existent-user', 'admin')).rejects.toThrow('The specified user does not exist.');
+  });
 });

src/services/db/flyer.db.test.ts

@@ -8,7 +8,7 @@ vi.unmock('./flyer.db');
 
 import { FlyerRepository, createFlyerAndItems } from './flyer.db';
 import { UniqueConstraintError, ForeignKeyConstraintError } from './errors.db';
-import type { FlyerInsert, FlyerItemInsert, Brand, Flyer, FlyerItem } from '../../types';
+import type { FlyerInsert, FlyerItemInsert, Brand, Flyer, FlyerItem, FlyerDbInsert } from '../../types';
 
 // Mock dependencies
 vi.mock('../logger.server', () => ({
@@ -30,14 +30,64 @@ describe('Flyer DB Service', () => {
     flyerRepo = new FlyerRepository(mockPoolInstance as any);
   });
 
+  describe('findOrCreateStore', () => {
+    it('should find an existing store and return its ID', async () => {
+      mockPoolInstance.query.mockResolvedValue({ rows: [{ store_id: 1 }] });
+      const result = await flyerRepo.findOrCreateStore('Existing Store');
+      expect(result).toBe(1);
+      expect(mockPoolInstance.query).toHaveBeenCalledWith('SELECT store_id FROM public.stores WHERE name = $1', ['Existing Store']);
+    });
+
+    it('should create a new store if it does not exist', async () => {
+      mockPoolInstance.query
+        .mockResolvedValueOnce({ rows: [] }) // First SELECT finds nothing
+        .mockResolvedValueOnce({ rows: [{ store_id: 2 }] }); // INSERT returns new ID
+      const result = await flyerRepo.findOrCreateStore('New Store');
+      expect(result).toBe(2);
+      expect(mockPoolInstance.query).toHaveBeenCalledWith('INSERT INTO public.stores (name) VALUES ($1) RETURNING store_id', ['New Store']);
+    });
+
+    it('should handle race condition where store is created between SELECT and INSERT', async () => {
+      const uniqueConstraintError = new Error('duplicate key value violates unique constraint');
+      (uniqueConstraintError as any).code = '23505';
+
+      mockPoolInstance.query
+        .mockResolvedValueOnce({ rows: [] }) // First SELECT finds nothing
+        .mockRejectedValueOnce(uniqueConstraintError) // INSERT fails due to race condition
+        .mockResolvedValueOnce({ rows: [{ store_id: 3 }] }); // Second SELECT finds the store
+
+      const result = await flyerRepo.findOrCreateStore('Racy Store');
+      expect(result).toBe(3);
+      expect(mockPoolInstance.query).toHaveBeenCalledTimes(3);
+    });
+
+    it('should throw an error if the database query fails', async () => {
+      const dbError = new Error('DB Error');
+      mockPoolInstance.query.mockRejectedValue(dbError);
+      await expect(flyerRepo.findOrCreateStore('Any Store')).rejects.toThrow('Failed to find or create store in database.');
+    });
+
+    it('should throw an error if race condition recovery fails', async () => {
+      const uniqueConstraintError = new Error('duplicate key value violates unique constraint');
+      (uniqueConstraintError as any).code = '23505';
+
+      mockPoolInstance.query
+        .mockResolvedValueOnce({ rows: [] }) // First SELECT
+        .mockRejectedValueOnce(uniqueConstraintError) // INSERT fails
+        .mockRejectedValueOnce(new Error('Second select fails')); // Recovery SELECT fails
+
+      await expect(flyerRepo.findOrCreateStore('Racy Store')).rejects.toThrow('Failed to find or create store in database.');
+    });
+  });
+
   describe('insertFlyer', () => {
     it('should execute an INSERT query and return the new flyer', async () => {
-      const flyerData: FlyerInsert = {
+      const flyerData: FlyerDbInsert = {
         file_name: 'test.jpg',
         image_url: '/images/test.jpg',
         icon_url: '/images/icons/test.jpg',
         checksum: 'checksum123',
-        store_name: 'Test Store',
+        store_id: 1,
         valid_from: '2024-01-01',
         valid_to: '2024-01-07',
         store_address: '123 Test St',
@@ -58,7 +108,7 @@ describe('Flyer DB Service', () => {
           '/images/test.jpg',
           '/images/icons/test.jpg',
           'checksum123',
-          'Test Store',
+          1,
           '2024-01-01',
           '2024-01-07',
           '123 Test St',
@@ -69,7 +119,7 @@ describe('Flyer DB Service', () => {
     });
 
     it('should throw UniqueConstraintError on duplicate checksum', async () => {
-      const flyerData: FlyerInsert = { checksum: 'duplicate-checksum' } as FlyerInsert;
+      const flyerData: FlyerDbInsert = { checksum: 'duplicate-checksum' } as FlyerDbInsert;
       const dbError = new Error('duplicate key value violates unique constraint "flyers_checksum_key"');
       (dbError as any).code = '23505';
       mockPoolInstance.query.mockRejectedValue(dbError);
@@ -79,7 +129,7 @@ describe('Flyer DB Service', () => {
     });
 
     it('should throw a generic error if the database query fails', async () => {
-      const flyerData: FlyerInsert = { checksum: 'fail-checksum' } as FlyerInsert;
+      const flyerData: FlyerDbInsert = { checksum: 'fail-checksum' } as FlyerDbInsert;
       mockPoolInstance.query.mockRejectedValue(new Error('DB Connection Error'));
       await expect(flyerRepo.insertFlyer(flyerData)).rejects.toThrow('Failed to insert flyer into database.');
     });
@@ -164,7 +214,7 @@ describe('Flyer DB Service', () => {
       // in the returned object structure (e.g., new columns added to the DB).
       // This ensures the core data is correct without being overly brittle.
       expect(result).toEqual({
-        flyer: expect.objectContaining(mockFlyer),
+        flyer: mockFlyer,
         items: expect.arrayContaining([
           expect.objectContaining(mockItems[0])
         ])
@@ -177,8 +227,8 @@ describe('Flyer DB Service', () => {
       expect(mockClient.query).not.toHaveBeenCalledWith('ROLLBACK');
 
       // Verify the individual functions were called with the client
-      expect(mockClient.query).toHaveBeenCalledWith(expect.stringContaining('INSERT INTO flyers'), expect.any(Array));
-      expect(mockClient.query).toHaveBeenCalledWith(expect.stringContaining('INSERT INTO flyer_items'), expect.any(Array));
+      expect(mockClient.query).toHaveBeenCalledWith(expect.stringContaining('SELECT store_id FROM public.stores'), expect.any(Array));
+      expect(mockClient.query).toHaveBeenCalledWith(expect.stringContaining('INSERT INTO flyers'), expect.any(Array)); // This is now the 2nd call
     });
 
     it('should ROLLBACK the transaction if an error occurs', async () => {
@@ -196,7 +246,7 @@ describe('Flyer DB Service', () => {
       // Mock insertFlyer to succeed, but insertFlyerItems to fail
       mockClient.query
         .mockResolvedValueOnce({ rows: [] }) // BEGIN
-        .mockResolvedValueOnce({ rows: [createMockFlyer()] }) // insertFlyer
+        .mockResolvedValueOnce({ rows: [{ store_id: 1 }] }) // findOrCreateStore
         .mockRejectedValueOnce(dbError); // insertFlyerItems fails
 
       // The transactional function re-throws the original error from the failed step.
@@ -322,12 +372,6 @@ describe('Flyer DB Service', () => {
       expect(result).toEqual([]);
     });
 
-    it('should handle an empty array of flyer IDs without querying the database', async () => {
-      // The implementation should short-circuit and return [] without a query.
-      const result = await flyerRepo.getFlyerItemsForFlyers([]);
-      expect(result).toEqual([]);
-    });
-
     it('should throw an error if the database query fails', async () => {
       const dbError = new Error('DB Error');
       mockPoolInstance.query.mockRejectedValue(dbError);
@@ -426,5 +470,19 @@ describe('Flyer DB Service', () => {
       await expect(flyerRepo.deleteFlyer(999)).rejects.toThrow('Failed to delete flyer.');
       expect(mockClient.query).toHaveBeenCalledWith('ROLLBACK');
     });
+
+    it('should rollback transaction on generic error', async () => {
+      const mockClient = { query: vi.fn(), release: vi.fn() };
+      vi.mocked(mockPoolInstance.connect).mockResolvedValue(mockClient as any);
+      const dbError = new Error('DB Error');
+      // Mock BEGIN to succeed, but DELETE to fail
+      mockClient.query
+        .mockResolvedValueOnce({ rows: [] }) // BEGIN
+        .mockRejectedValueOnce(dbError); // DELETE fails
+
+      await expect(flyerRepo.deleteFlyer(42)).rejects.toThrow('Failed to delete flyer.');
+      expect(mockClient.query).toHaveBeenCalledWith('ROLLBACK');
+      expect(mockClient.release).toHaveBeenCalled();
+    });
   });
 });

src/services/db/notification.db.test.ts

@@ -44,6 +44,13 @@ describe('Notification DB Service', () => {
       expect(result).toEqual(mockNotifications);
     });
 
+    it('should return an empty array if the user has no notifications', async () => {
+      mockPoolInstance.query.mockResolvedValue({ rows: [] });
+      const result = await notificationRepo.getNotificationsForUser('user-456', 10, 0);
+      expect(result).toEqual([]);
+      expect(mockPoolInstance.query).toHaveBeenCalledWith(expect.any(String), ['user-456', 10, 0]);
+    });
+
     it('should throw an error if the database query fails', async () => {
       const dbError = new Error('DB Error');
       mockPoolInstance.query.mockRejectedValue(dbError);
@@ -60,6 +67,18 @@ describe('Notification DB Service', () => {
       expect(result).toEqual(mockNotification);
     });
 
+    it('should insert a notification with a linkUrl', async () => {
+      const mockNotification: Notification = { notification_id: 2, user_id: 'user-123', content: 'Test with link', link_url: '/some/link', is_read: false, created_at: '' };
+      mockPoolInstance.query.mockResolvedValue({ rows: [mockNotification] });
+
+      const result = await notificationRepo.createNotification('user-123', 'Test with link', '/some/link');
+      expect(mockPoolInstance.query).toHaveBeenCalledWith(
+        expect.stringContaining('INSERT INTO public.notifications'),
+        ['user-123', 'Test with link', '/some/link']
+      );
+      expect(result).toEqual(mockNotification);
+    });
+
     it('should throw ForeignKeyConstraintError if user does not exist', async () => {
       const dbError = new Error('violates foreign key constraint');
       (dbError as any).code = '23503';
@@ -113,7 +132,7 @@ describe('Notification DB Service', () => {
       const mockNotification: Notification = { notification_id: 123, user_id: 'abc', content: 'msg', is_read: true, created_at: '' };
       mockPoolInstance.query.mockResolvedValue({ rows: [mockNotification], rowCount: 1 });
 
-      const result = await notificationRepo.markNotificationAsRead(123, 'user-abc');
+      const result = await notificationRepo.markNotificationAsRead(123, 'abc');
       expect(result).toEqual(mockNotification);
     });
 
@@ -121,14 +140,24 @@ describe('Notification DB Service', () => {
       // FIX: Ensure rowCount is 0
       mockPoolInstance.query.mockResolvedValue({ rows: [], rowCount: 0 });
       
-      await expect(notificationRepo.markNotificationAsRead(999, 'user-abc'))
+      await expect(notificationRepo.markNotificationAsRead(999, 'abc'))
         .rejects.toThrow('Notification not found or user does not have permission.');
     });
 
+    it('should re-throw the specific "not found" error if it occurs', async () => {
+      // This tests the `if (error instanceof Error && error.message.startsWith('Notification not found'))` line
+      const notFoundError = new Error('Notification not found or user does not have permission.');
+      mockPoolInstance.query.mockImplementation(() => {
+        throw notFoundError;
+      });
+
+      await expect(notificationRepo.markNotificationAsRead(999, 'user-abc')).rejects.toThrow(notFoundError);
+    });
+
     it('should throw a generic error if the database query fails', async () => {
       const dbError = new Error('DB Error');
       mockPoolInstance.query.mockRejectedValue(dbError);
-      await expect(notificationRepo.markNotificationAsRead(123, 'user-abc')).rejects.toThrow('Failed to mark notification as read.');
+      await expect(notificationRepo.markNotificationAsRead(123, 'abc')).rejects.toThrow('Failed to mark notification as read.');
     });
   });
 
@@ -151,4 +180,28 @@ describe('Notification DB Service', () => {
       await expect(notificationRepo.markAllNotificationsAsRead('user-xyz')).rejects.toThrow('Failed to mark notifications as read.');
     });
   });
+
+  describe('deleteOldNotifications', () => {
+    it('should execute a DELETE query and return the number of deleted rows', async () => {
+      mockPoolInstance.query.mockResolvedValue({ rowCount: 5 });
+      const result = await notificationRepo.deleteOldNotifications(30);
+      expect(mockPoolInstance.query).toHaveBeenCalledWith(
+        `DELETE FROM public.notifications WHERE created_at < NOW() - ($1 * interval '1 day')`,
+        [30]
+      );
+      expect(result).toBe(5);
+    });
+
+    it('should return 0 if rowCount is null or undefined', async () => {
+      mockPoolInstance.query.mockResolvedValue({ rowCount: null });
+      const result = await notificationRepo.deleteOldNotifications(30);
+      expect(result).toBe(0);
+    });
+
+    it('should throw an error if the database query fails', async () => {
+      const dbError = new Error('DB Error');
+      mockPoolInstance.query.mockRejectedValue(dbError);
+      await expect(notificationRepo.deleteOldNotifications(30)).rejects.toThrow('Failed to delete old notifications.');
+    });
+  });
 });