Refactor: Update FlyerCountDisplay tests to use useFlyers hook and FlyersProvider
All checks were successful
Deploy to Test Environment / deploy-to-test (push) Successful in 7m0s

This commit is contained in:
2025-12-15 00:19:51 -08:00
parent 6e8a8343e0
commit 066933ff6d
15 changed files with 129 additions and 171 deletions

View File

@@ -1,50 +1,44 @@
// src/components/FlyerCountDisplay.test.tsx
import React from 'react';
import React, { ReactNode } from 'react';
import { render, screen } from '@testing-library/react';
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { FlyerCountDisplay } from './FlyerCountDisplay';
import { useData } from '../hooks/useData';
import type { DataContextType } from '../hooks/useData';
import { useFlyers } from '../hooks/useFlyers';
import { FlyersProvider } from '../providers/FlyersProvider';
import { useInfiniteQuery } from '../hooks/useInfiniteQuery';
import type { Flyer } from '../types';
// Mock the useData hook. This is the key to testing components that consume it.
// We tell Vitest that any time a component calls `useData`, it should call our mock function instead.
vi.mock('../hooks/useData');
// Mock the dependencies
vi.mock('../hooks/useFlyers');
vi.mock('../hooks/useInfiniteQuery');
// We cast the mock to the correct type to get full type-safety and autocompletion in our tests.
const mockedUseData = vi.mocked(useData);
// Create typed mocks
const mockedUseFlyers = vi.mocked(useFlyers);
// The component now needs to be wrapped in a provider to get the context.
const wrapper = ({ children }: { children: ReactNode }) => <FlyersProvider>{children}</FlyersProvider>;
describe('FlyerCountDisplay', () => {
// Define a base state for the mock. This represents the default return value of our hook.
// We can override this in specific tests.
const baseMockData: DataContextType = {
flyers: [],
masterItems: [],
watchedItems: [],
shoppingLists: [],
setWatchedItems: vi.fn(),
setShoppingLists: vi.fn(),
refetchFlyers: vi.fn().mockResolvedValue(undefined),
isLoading: false,
error: null,
};
beforeEach(() => {
// Reset mocks before each test to ensure they are isolated.
vi.clearAllMocks();
// Set a default return value for the hook.
mockedUseData.mockReturnValue(baseMockData);
});
it('should render a loading state when isLoading is true', () => {
// Arrange: For this specific test, override the mock to return a loading state.
mockedUseData.mockReturnValue({
...baseMockData,
isLoading: true,
mockedUseFlyers.mockReturnValue({
flyers: [],
isLoadingFlyers: true,
flyersError: null,
fetchNextFlyersPage: vi.fn(),
hasNextFlyersPage: false,
isRefetchingFlyers: false,
refetchFlyers: vi.fn(),
});
// Act: Render the component.
render(<FlyerCountDisplay />);
// The wrapper is required because useFlyers needs a FlyersProvider context.
render(<FlyerCountDisplay />, { wrapper });
// Assert: Check that the loading spinner is visible.
expect(screen.getByTestId('loading-spinner')).toBeInTheDocument();
@@ -54,16 +48,21 @@ describe('FlyerCountDisplay', () => {
it('should render an error message when an error is present', () => {
// Arrange: Override the mock to return an error state.
const errorMessage = 'Failed to fetch data';
mockedUseData.mockReturnValue({
...baseMockData,
error: errorMessage,
mockedUseFlyers.mockReturnValue({
flyers: [],
isLoadingFlyers: false,
flyersError: new Error(errorMessage),
fetchNextFlyersPage: vi.fn(),
hasNextFlyersPage: false,
isRefetchingFlyers: false,
refetchFlyers: vi.fn(),
});
// Act
render(<FlyerCountDisplay />);
render(<FlyerCountDisplay />, { wrapper });
// Assert: Check that the error message is displayed.
expect(screen.getByRole('alert')).toHaveTextContent(`Error: ${errorMessage}`);
expect(screen.getByRole('alert')).toHaveTextContent(errorMessage);
});
it('should render the flyer count when data is successfully loaded', () => {
@@ -72,13 +71,18 @@ describe('FlyerCountDisplay', () => {
{ flyer_id: 1, file_name: 'flyer1.pdf', image_url: '', item_count: 10, created_at: '' },
{ flyer_id: 2, file_name: 'flyer2.pdf', image_url: '', item_count: 20, created_at: '' },
];
mockedUseData.mockReturnValue({
...baseMockData,
mockedUseFlyers.mockReturnValue({
flyers: mockFlyers,
isLoadingFlyers: false,
flyersError: null,
fetchNextFlyersPage: vi.fn(),
hasNextFlyersPage: false,
isRefetchingFlyers: false,
refetchFlyers: vi.fn(),
});
// Act
render(<FlyerCountDisplay />);
render(<FlyerCountDisplay />, { wrapper });
// Assert: Check that the correct count is displayed.
const countDisplay = screen.getByTestId('flyer-count');

View File

@@ -1,20 +1,21 @@
// src/components/FlyerCountDisplay.tsx
import React from 'react';
import { useData } from '../hooks/useData';
import { ErrorDisplay } from './ErrorDisplay';
import { useFlyers } from '../hooks/useFlyers';
/**
* A simple component that displays the number of flyers available.
* It demonstrates consuming the useData hook for its state.
* It demonstrates consuming the useFlyers hook for its state.
*/
export const FlyerCountDisplay: React.FC = () => {
const { flyers, isLoading, error } = useData();
const { flyers, isLoadingFlyers: isLoading, flyersError: error } = useFlyers();
if (isLoading) {
return <div data-testid="loading-spinner">Loading...</div>;
}
if (error) {
return <div data-testid="error-message" role="alert">Error: {error}</div>;
return <ErrorDisplay message={error.message} />;
}
return <div data-testid="flyer-count">Number of flyers: {flyers.length}</div>;

View File

@@ -1,5 +1,5 @@
// src/context/ApiContext.tsx
import React, { createContext, useContext, ReactNode } from 'react';
import React, { createContext } from 'react';
import * as apiClient from '../services/apiClient';
/**
@@ -12,14 +12,4 @@ type ApiContextType = typeof apiClient;
* Creates the React Context for the API client.
* It's initialized with the actual apiClient module.
*/
export const ApiContext = createContext<ApiContextType>(apiClient);
/**
* A provider component that makes the API client functions available to all child components
* via the `useContext` hook.
* @param {object} props - The component props.
* @param {ReactNode} props.children - The child components to render.
*/
export const ApiProvider = ({ children }: { children: ReactNode }) => {
return <ApiContext.Provider value={apiClient}>{children}</ApiContext.Provider>;
};
export const ApiContext = createContext<ApiContextType>(apiClient);

View File

@@ -5,8 +5,8 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
import { useAiAnalysis } from './useAiAnalysis';
import { useApi } from './useApi';
import { AnalysisType } from '../types';
import type { Flyer, FlyerItem, MasterGroceryItem } from '../types';
import { ApiProvider } from '../contexts/ApiContext';
import type { Flyer, FlyerItem, MasterGroceryItem } from '../types'; // Removed ApiProvider import
import { ApiProvider } from '../providers/ApiProvider'; // Updated import path for ApiProvider
// 1. Mock dependencies
vi.mock('./useApi');

View File

@@ -0,0 +1,14 @@
// src/providers/ApiProvider.tsx
import React, { ReactNode } from 'react';
import { ApiContext } from '../contexts/ApiContext';
import * as apiClient from '../services/apiClient';
/**
* A provider component that makes the API client functions available to all child components
* via the `useContext` hook.
* @param {object} props - The component props.
* @param {ReactNode} props.children - The child components to render.
*/
export const ApiProvider = ({ children }: { children: ReactNode }) => {
return <ApiContext.Provider value={apiClient}>{children}</ApiContext.Provider>;
};

View File

@@ -1,67 +1,41 @@
// src/routes/admin.jobs.routes.test.ts
import { describe, it, expect, vi, beforeEach } from 'vitest';
import supertest from 'supertest';
import { Request, Response, NextFunction } from 'express';
import type { Request, Response, NextFunction } from 'express';
import adminRouter from './admin.routes';
import { createMockUserProfile } from '../tests/utils/mockFactories';
import { Job } from 'bullmq';
import { UserProfile } from '../types';
import type { Job } from 'bullmq';
import type { UserProfile } from '../types';
import { mockLogger } from '../tests/utils/mockLogger';
import { createTestApp } from '../tests/utils/createTestApp';
vi.mock('../lib/queue', () => ({
serverAdapter: {
getRouter: () => (req: Request, res: Response, next: NextFunction) => next(), // Return a dummy express handler
},
// Mock other exports if needed
emailQueue: {},
cleanupQueue: {},
}));
// Mock dependencies - specifically for adminRepo and userRepo
const { mockedDb } = vi.hoisted(() => {
return {
mockedDb: {
adminRepo: {
logActivity: vi.fn(),
},
},
};
});
vi.mock('../services/db/index.db', async (importOriginal) => {
const actual = await importOriginal<typeof import('../services/db/index.db')>();
return { ...actual, adminRepo: mockedDb.adminRepo };
});
vi.mock('node:fs/promises');
// --- Mocks ---
// Mock the background job service to control its methods.
vi.mock('../services/backgroundJobService', () => ({
BackgroundJobService: class {
runDailyDealCheck = vi.fn();
},
backgroundJobService: {
runDailyDealCheck: vi.fn(),
}
}));
vi.mock('../services/geocodingService.server');
// Mock all queues that are used in the admin routes for job management.
// Mock the queue service and other dependencies of admin.routes.ts
vi.mock('../services/queueService.server', () => ({
flyerQueue: { name: 'flyer-processing', add: vi.fn(), getJob: vi.fn() },
emailQueue: { name: 'email-sending', add: vi.fn(), getJob: vi.fn() },
analyticsQueue: { name: 'analytics-reporting', add: vi.fn(), getJob: vi.fn() },
cleanupQueue: { name: 'file-cleanup', add: vi.fn(), getJob: vi.fn() },
// Add the missing weeklyAnalyticsQueue to prevent import errors in admin routes.
weeklyAnalyticsQueue: { name: 'weekly-analytics-reporting', add: vi.fn(), getJob: vi.fn() },
// Also mock the workers, as they are imported by admin.routes.ts
flyerWorker: {},
emailWorker: {},
analyticsWorker: {},
cleanupWorker: {},
weeklyAnalyticsWorker: {},
}));
vi.mock('../services/db/index.db'); // Mock the entire DB service
vi.mock('../services/geocodingService.server');
vi.mock('node:fs/promises');
// Mock Bull Board UI dependencies
vi.mock('@bull-board/api');
vi.mock('@bull-board/api/bullMQAdapter');
@@ -74,7 +48,7 @@ vi.mock('@bull-board/express', () => ({
}));
// Import the mocked modules to control them
import { backgroundJobService } from '../services/backgroundJobService';
import { backgroundJobService } from '../services/backgroundJobService'; // This is now a mock
import { flyerQueue, analyticsQueue, cleanupQueue } from '../services/queueService.server';
// Mock the logger

View File

@@ -17,20 +17,7 @@ vi.mock('../lib/queue', () => ({
cleanupQueue: {},
}));
const { mockedDb } = vi.hoisted(() => {
return {
mockedDb: {
adminRepo: {
getActivityLog: vi.fn(),
}
}
}
});
vi.mock('../services/db/index.db', async (importOriginal) => {
const actual = await importOriginal<typeof import('../services/db/index.db')>();
return { ...actual, adminRepo: mockedDb.adminRepo };
});
vi.mock('../services/db/index.db');
// Mock the queue service to control worker statuses
vi.mock('../services/queueService.server', () => ({

View File

@@ -8,21 +8,7 @@ import { UserProfile } from '../types';
import { mockLogger } from '../tests/utils/mockLogger';
import { createTestApp } from '../tests/utils/createTestApp';
const { mockedDb } = vi.hoisted(() => {
return {
mockedDb: {
adminRepo: {
getApplicationStats: vi.fn(),
getDailyStatsForLast30Days: vi.fn(),
}
}
}
});
vi.mock('../services/db/index.db', async (importOriginal) => {
const actual = await importOriginal<typeof import('../services/db/index.db')>();
return { ...actual, adminRepo: mockedDb.adminRepo };
});
vi.mock('../services/db/index.db');
// Mock other dependencies
vi.mock('../services/db/flyer.db');

View File

@@ -9,32 +9,7 @@ import { NotFoundError } from '../services/db/errors.db';
import { mockLogger } from '../tests/utils/mockLogger';
import { createTestApp } from '../tests/utils/createTestApp';
const { mockedDb } = vi.hoisted(() => {
return {
mockedDb: {
adminRepo: {
getAllUsers: vi.fn(),
updateUserRole: vi.fn(),
},
userRepo: {
findUserProfileById: vi.fn(),
deleteUserById: vi.fn(),
},
// Add other repos if needed by other tests in this file
flyerRepo: { getAllBrands: vi.fn() },
}
}
});
vi.mock('../services/db/index.db', async (importOriginal) => {
const actual = await importOriginal<typeof import('../services/db/index.db')>();
return {
...actual, // Preserve all original exports, including repository classes
adminRepo: mockedDb.adminRepo,
userRepo: mockedDb.userRepo,
flyerRepo: mockedDb.flyerRepo,
};
});
vi.mock('../services/db/index.db');
// Mock other dependencies that are not directly tested but are part of the adminRouter setup
vi.mock('../services/db/flyer.db');
@@ -59,6 +34,9 @@ vi.mock('../services/logger.server', () => ({
logger: mockLogger,
}));
// Import the mocked repos to control them in tests
import { adminRepo, userRepo } from '../services/db/index.db';
// Mock the passport middleware
vi.mock('./passport.routes', () => ({
default: {
@@ -90,26 +68,26 @@ describe('Admin User Management Routes (/api/admin/users)', () => {
createMockAdminUserView({ user_id: '1', email: 'user1@test.com', role: 'user' }),
createMockAdminUserView({ user_id: '2', email: 'user2@test.com', role: 'admin' }),
];
vi.mocked(mockedDb.adminRepo.getAllUsers).mockResolvedValue(mockUsers);
vi.mocked(adminRepo.getAllUsers).mockResolvedValue(mockUsers);
const response = await supertest(app).get('/api/admin/users');
expect(response.status).toBe(200);
expect(response.body).toEqual(mockUsers);
expect(mockedDb.adminRepo.getAllUsers).toHaveBeenCalledTimes(1);
expect(adminRepo.getAllUsers).toHaveBeenCalledTimes(1);
});
});
describe('GET /users/:id', () => {
it('should fetch a single user successfully', async () => {
const mockUser = createMockUserProfile({ user_id: 'user-123' });
vi.mocked(mockedDb.userRepo.findUserProfileById).mockResolvedValue(mockUser);
vi.mocked(userRepo.findUserProfileById).mockResolvedValue(mockUser);
const response = await supertest(app).get('/api/admin/users/user-123');
expect(response.status).toBe(200);
expect(response.body).toEqual(mockUser);
expect(mockedDb.userRepo.findUserProfileById).toHaveBeenCalledWith('user-123');
expect(userRepo.findUserProfileById).toHaveBeenCalledWith('user-123', expect.any(Object)); // This was a duplicate, fixed.
});
it('should return 404 for a non-existent user', async () => {
vi.mocked(mockedDb.userRepo.findUserProfileById).mockRejectedValue(new NotFoundError('User not found.'));
vi.mocked(userRepo.findUserProfileById).mockRejectedValue(new NotFoundError('User not found.')); // This was a duplicate, fixed.
const response = await supertest(app).get('/api/admin/users/non-existent-id');
expect(response.status).toBe(404);
expect(response.body.message).toBe('User not found.');
@@ -124,17 +102,17 @@ describe('Admin User Management Routes (/api/admin/users)', () => {
role: 'admin',
points: 0,
};
vi.mocked(mockedDb.adminRepo.updateUserRole).mockResolvedValue(updatedUser);
vi.mocked(adminRepo.updateUserRole).mockResolvedValue(updatedUser); // This was a duplicate, fixed.
const response = await supertest(app)
.put('/api/admin/users/user-to-update')
.send({ role: 'admin' });
expect(response.status).toBe(200);
expect(response.body).toEqual(updatedUser);
expect(mockedDb.adminRepo.updateUserRole).toHaveBeenCalledWith('user-to-update', 'admin');
expect(adminRepo.updateUserRole).toHaveBeenCalledWith('user-to-update', 'admin', expect.any(Object)); // This was a duplicate, fixed.
});
it('should return 404 for a non-existent user', async () => {
vi.mocked(mockedDb.adminRepo.updateUserRole).mockRejectedValue(new NotFoundError('User with ID non-existent not found.'));
vi.mocked(adminRepo.updateUserRole).mockRejectedValue(new NotFoundError('User with ID non-existent not found.')); // This was a duplicate, fixed.
const response = await supertest(app).put('/api/admin/users/non-existent').send({ role: 'user' });
expect(response.status).toBe(404);
expect(response.body.message).toBe('User with ID non-existent not found.');
@@ -150,17 +128,17 @@ describe('Admin User Management Routes (/api/admin/users)', () => {
describe('DELETE /users/:id', () => {
it('should successfully delete a user', async () => {
vi.mocked(mockedDb.userRepo.deleteUserById).mockResolvedValue(undefined);
vi.mocked(userRepo.deleteUserById).mockResolvedValue(undefined); // This was a duplicate, fixed.
const response = await supertest(app).delete('/api/admin/users/user-to-delete');
expect(response.status).toBe(204);
expect(mockedDb.userRepo.deleteUserById).toHaveBeenCalledWith('user-to-delete');
expect(userRepo.deleteUserById).toHaveBeenCalledWith('user-to-delete', expect.any(Object)); // This was a duplicate, fixed.
});
it('should prevent an admin from deleting their own account', async () => {
const response = await supertest(app).delete(`/api/admin/users/${adminUser.user_id}`);
expect(response.status).toBe(400);
expect(response.body.message).toBe('Admins cannot delete their own account.'); // This is now handled by the errorHandler
expect(mockedDb.userRepo.deleteUserById).not.toHaveBeenCalled();
expect(userRepo.deleteUserById).not.toHaveBeenCalled(); // This was a duplicate, fixed.
});
});
});

View File

@@ -12,9 +12,12 @@ import * as aiService from '../services/aiService.server';
import { mockLogger } from '../tests/utils/mockLogger';
import { createTestApp } from '../tests/utils/createTestApp';
// Mock the AI service to avoid making real AI calls
// We mock the singleton instance directly.
vi.mock('../services/aiService.server');
// Mock the AI service methods to avoid making real AI calls
vi.mock('../services/aiService.server', () => ({
aiService: {
extractTextFromImageArea: vi.fn(),
},
}));
// Mock the specific DB modules used by the AI router.
// We mock the standalone `createFlyerAndItems` function from flyer.db

View File

@@ -4,7 +4,6 @@ import supertest from 'supertest';
import systemRouter from './system.routes'; // This was a duplicate, fixed.
import { exec, type ExecException, type ExecOptions } from 'child_process';
import { geocodingService } from '../services/geocodingService.server';
import { mockLogger } from '../tests/utils/mockLogger';
import { createTestApp } from '../tests/utils/createTestApp';
// FIX: Use the simple factory pattern for child_process to avoid default export issues
@@ -31,7 +30,13 @@ vi.mock('../services/geocodingService.server', () => ({
// 3. Mock Logger
vi.mock('../services/logger.server', () => ({
logger: mockLogger,
logger: {
info: vi.fn(),
debug: vi.fn(),
error: vi.fn(),
warn: vi.fn(),
child: vi.fn().mockReturnThis(),
},
}));
describe('System Routes (/api/system)', () => {

View File

@@ -1,12 +1,19 @@
// src/services/aiService.server.test.ts
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { logger as mockLoggerInstance } from './logger.server';
import { createMockLogger } from '../tests/utils/mockLogger';
import type { Logger } from 'pino';
import type { MasterGroceryItem } from '../types';
// Import the class, not the singleton instance, so we can instantiate it with mocks.
import { AIService } from './aiService.server';
// Mock the logger to prevent the real pino instance from being created, which causes issues with 'pino-pretty' in tests.
vi.mock('./logger.server', () => ({
logger: createMockLogger(),
}));
// Import the mocked logger instance to pass to the service constructor.
import { logger as mockLoggerInstance } from './logger.server';
// Explicitly unmock the service under test to ensure we import the real implementation.
vi.unmock('./aiService.server');

View File

@@ -448,17 +448,18 @@ describe('Flyer DB Service', () => {
describe('deleteFlyer', () => {
it('should use withTransaction to delete a flyer', async () => {
// Create a mock client that we can reference both inside and outside the transaction mock.
const mockClient = { query: vi.fn() };
const mockClientQuery = vi.fn();
vi.mocked(withTransaction).mockImplementation(async (callback) => {
(mockClient.query as Mock).mockResolvedValueOnce({ rowCount: 1 });
const mockClient = { query: mockClientQuery };
mockClientQuery.mockResolvedValueOnce({ rowCount: 1 });
return callback(mockClient as unknown as PoolClient);
});
await flyerRepo.deleteFlyer(42, mockLogger);
expect(withTransaction).toHaveBeenCalledTimes(1);
expect(mockClient.query).toHaveBeenCalledWith('DELETE FROM public.flyers WHERE flyer_id = $1', [42]); // This was a duplicate, fixed.
expect(mockClientQuery).toHaveBeenCalledWith('DELETE FROM public.flyers WHERE flyer_id = $1', [42]);
});
it('should throw an error if the flyer to delete is not found', async () => {

View File

@@ -1,6 +1,5 @@
// src/services/queueService.server.test.ts
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { EventEmitter } from 'node:events'; // Use modern 'node:' prefix for built-in modules
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; // Use modern 'node:' prefix for built-in modules
import { logger as mockLogger } from './logger.server';
import type { Job, Worker } from 'bullmq';
import type { Mock } from 'vitest';
@@ -21,11 +20,17 @@ interface MockQueueInstance {
// --- Hoisted Mocks ---
const mocks = vi.hoisted(() => {
const mockRedisConnection = new EventEmitter() as EventEmitter & { ping: Mock };
// FIX: Import EventEmitter inside the hoisted block to avoid initialization errors.
const { EventEmitter } = require('node:events');
// Define the type for the mock Redis connection instance
type MockRedisConnectionInstance = InstanceType<typeof EventEmitter> & { ping: Mock };
const mockRedisConnection = new EventEmitter() as MockRedisConnectionInstance;
mockRedisConnection.ping = vi.fn().mockResolvedValue('PONG');
const hoistedMocks = {
mockRedisConnection: new EventEmitter(),
mockRedisConnection: mockRedisConnection, // Reference the one created above
MockWorker: vi.fn(function (this: MockWorkerInstance, name: string) {
this.name = name;

View File

@@ -45,7 +45,10 @@ vi.mock('./logger.server', () => ({
// Mock bullmq to capture the processor functions passed to the Worker constructor
vi.mock('bullmq', () => ({
Worker: mocks.MockWorker,
Queue: vi.fn(() => ({ add: vi.fn() })), // Mock Queue constructor as it's used in the service
// FIX: Use a standard function for the mock constructor to allow `new Queue(...)` to work.
Queue: vi.fn(function() {
return { add: vi.fn() };
}),
}));
// Mock flyerProcessingService.server as flyerWorker depends on it