refactor: improve type safety by making 'next' optional in async route handlers and updating type imports
All checks were successful
Deploy to Web Server flyer-crawler.projectium.com / deploy (push) Successful in 4m27s
All checks were successful
Deploy to Web Server flyer-crawler.projectium.com / deploy (push) Successful in 4m27s
This commit is contained in:
@@ -7,7 +7,6 @@ import multer from 'multer';
|
||||
import * as db from '../services/db';
|
||||
import { logger } from '../services/logger.server';
|
||||
import { UserProfile } from '../types';
|
||||
import { AsyncRequestHandler } from '../types/express';
|
||||
import { clearGeocodeCache } from '../services/geocodingService.server';
|
||||
|
||||
// --- Bull Board (Job Queue UI) Imports ---
|
||||
@@ -244,7 +243,7 @@ router.post('/trigger/analytics-report', async (req, res, next: NextFunction) =>
|
||||
* POST /api/admin/flyers/:flyerId/cleanup - Enqueue a job to clean up a flyer's files.
|
||||
* This is triggered by an admin after they have verified the flyer processing was successful.
|
||||
*/
|
||||
router.post('/flyers/:flyerId/cleanup', async (req, res, next: NextFunction) => {
|
||||
router.post('/flyers/:flyerId/cleanup', async (req, res) => {
|
||||
const adminUser = req.user as UserProfile;
|
||||
const flyerId = parseInt(req.params.flyerId, 10);
|
||||
|
||||
|
||||
@@ -4,7 +4,6 @@ import supertest from 'supertest';
|
||||
import express, { type Request, type Response, type NextFunction } from 'express';
|
||||
import path from 'node:path';
|
||||
import aiRouter from './ai';
|
||||
import * as aiService from '../services/aiService.server';
|
||||
import { UserProfile } from '../types';
|
||||
import * as db from '../services/db';
|
||||
|
||||
|
||||
@@ -356,7 +356,7 @@ router.post('/search-web', passport.authenticate('jwt', { session: false }), asy
|
||||
}
|
||||
});
|
||||
|
||||
router.post('/plan-trip', passport.authenticate('jwt', { session: false }), async (req, res, next: NextFunction) => {
|
||||
router.post('/plan-trip', passport.authenticate('jwt', { session: false }), async (req, res) => {
|
||||
// try {
|
||||
// const { items, store, userLocation } = req.body;
|
||||
// logger.info(`Server-side trip planning requested for user.`);
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
// src/routes/auth.test.ts
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import supertest from 'supertest';
|
||||
import express from 'express';
|
||||
import express, { Request, Response, NextFunction } from 'express';
|
||||
import cookieParser from 'cookie-parser';
|
||||
import * as bcrypt from 'bcrypt';
|
||||
import authRouter from './auth';
|
||||
@@ -33,10 +33,13 @@ vi.mock('bcrypt', async (importOriginal) => {
|
||||
return { ...actual, compare: vi.fn() };
|
||||
});
|
||||
|
||||
// Define a type for the custom passport callback to avoid `any`.
|
||||
type PassportCallback = (error: Error | null, user: Express.User | false, info?: { message: string }) => void;
|
||||
|
||||
// Mock Passport middleware
|
||||
vi.mock('./passport', () => ({
|
||||
default: {
|
||||
authenticate: (strategy: string, options: any, callback: any) => (req: any, res: any, next: any) => {
|
||||
authenticate: (strategy: string, options: Record<string, unknown>, callback: PassportCallback) => (req: Request) => {
|
||||
// Logic to simulate passport authentication outcome based on test input
|
||||
if (req.body.password === 'wrong_password') {
|
||||
// Simulate incorrect credentials
|
||||
@@ -53,7 +56,7 @@ vi.mock('./passport', () => ({
|
||||
|
||||
// Default success case
|
||||
const user = { user_id: 'user-123', email: req.body.email };
|
||||
callback(null, user, null);
|
||||
callback(null, user, undefined);
|
||||
},
|
||||
},
|
||||
}));
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
// src/routes/passport.test.ts
|
||||
import { describe, it, expect, vi, beforeEach, type Mocked } from 'vitest';
|
||||
import { Request, Response, NextFunction } from 'express';
|
||||
import { Profile } from '../types';
|
||||
|
||||
// Define a type for the JWT verify callback function for type safety.
|
||||
type VerifyCallback = (payload: { user_id: string }, done: (error: Error | null, user?: object | false) => void) => Promise<void>;
|
||||
@@ -65,7 +66,11 @@ describe('Passport Configuration', () => {
|
||||
it('should call done(null, userProfile) on successful authentication', async () => {
|
||||
// Arrange
|
||||
const jwtPayload = { user_id: 'user-123' };
|
||||
const mockProfile: any = { user_id: 'user-123', role: 'user' };
|
||||
const mockProfile: Profile = {
|
||||
user_id: 'user-123',
|
||||
role: 'user',
|
||||
points: 100, // Add missing required property
|
||||
};
|
||||
vi.mocked(mockedDb.findUserProfileById).mockResolvedValue(mockProfile);
|
||||
const done = vi.fn();
|
||||
|
||||
|
||||
@@ -5,6 +5,7 @@ import express from 'express';
|
||||
import publicRouter from './public'; // Import the router we want to test
|
||||
import * as db from '../services/db';
|
||||
import * as fs from 'fs/promises';
|
||||
import { Flyer } from '../types';
|
||||
|
||||
// 1. Mock the Service Layer directly.
|
||||
// This decouples the route tests from the SQL implementation details.
|
||||
@@ -114,9 +115,9 @@ describe('Public Routes (/api)', () => {
|
||||
|
||||
describe('GET /flyers', () => {
|
||||
it('should return a list of flyers on success', async () => {
|
||||
const mockFlyers: any[] = [
|
||||
{ flyer_id: 1, file_name: 'flyer_a.jpg', image_url: '/a.jpg', created_at: new Date().toISOString() },
|
||||
{ flyer_id: 2, file_name: 'flyer_b.jpg', image_url: '/b.jpg', created_at: new Date().toISOString() },
|
||||
const mockFlyers: Flyer[] = [
|
||||
{ flyer_id: 1, file_name: 'flyer_a.jpg', image_url: '/a.jpg', created_at: new Date().toISOString(), item_count: 10 },
|
||||
{ flyer_id: 2, file_name: 'flyer_b.jpg', image_url: '/b.jpg', created_at: new Date().toISOString(), item_count: 20 },
|
||||
];
|
||||
// Mock the service function
|
||||
vi.mocked(db.getFlyers).mockResolvedValue(mockFlyers);
|
||||
|
||||
@@ -3,7 +3,6 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import supertest from 'supertest';
|
||||
import express from 'express';
|
||||
import type { ExecException, ChildProcess } from 'child_process';
|
||||
import { ExecOptions } from 'child_process';
|
||||
import systemRouter from './system';
|
||||
|
||||
// Define a type for the exec callback to avoid using `any`.
|
||||
@@ -54,7 +53,7 @@ describe('System Routes (/api/system)', () => {
|
||||
// The `exec` callback receives (error, stdout, stderr). For success, error is null.
|
||||
// We must match the overloaded signature of `exec`. The second argument can be options or the callback.
|
||||
// By using `...args: any[]`, we create a generic mock that can handle all overloads.
|
||||
vi.mocked(exec).mockImplementation((...args: any[]) => {
|
||||
vi.mocked(exec).mockImplementation((...args: [string, ...unknown[]]) => {
|
||||
// The callback is always the last function argument.
|
||||
const callback = args.find(arg => typeof arg === 'function') as ExecCallback;
|
||||
// For this test, we simulate success by calling the callback with no error.
|
||||
@@ -77,7 +76,7 @@ describe('System Routes (/api/system)', () => {
|
||||
const pm2StoppedOutput = `
|
||||
│ status │ stopped │
|
||||
`;
|
||||
vi.mocked(exec).mockImplementation((...args: any[]) => {
|
||||
vi.mocked(exec).mockImplementation((...args: [string, ...unknown[]]) => {
|
||||
const callback = args.find(arg => typeof arg === 'function') as ExecCallback;
|
||||
callback(null, pm2StoppedOutput, '');
|
||||
return {} as ChildProcess;
|
||||
@@ -94,7 +93,7 @@ describe('System Routes (/api/system)', () => {
|
||||
it('should return success: false when pm2 process does not exist', async () => {
|
||||
// Arrange: Simulate the error and stdout when a process is not found.
|
||||
const processNotFoundOutput = "[PM2][ERROR] Process or Namespace flyer-crawler-api doesn't exist";
|
||||
vi.mocked(exec).mockImplementation((...args: any[]) => {
|
||||
vi.mocked(exec).mockImplementation((...args: [string, ...unknown[]]) => {
|
||||
const callback = args.find(arg => typeof arg === 'function') as ExecCallback;
|
||||
callback(new Error('Command failed') as ExecException, processNotFoundOutput, '');
|
||||
return {} as ChildProcess;
|
||||
@@ -110,7 +109,7 @@ describe('System Routes (/api/system)', () => {
|
||||
|
||||
it('should return 500 on a generic exec error', async () => {
|
||||
// Arrange: Simulate a generic failure of the `exec` command.
|
||||
vi.mocked(exec).mockImplementation((...args: any[]) => {
|
||||
vi.mocked(exec).mockImplementation((...args: [string, ...unknown[]]) => {
|
||||
const callback = args.find(arg => typeof arg === 'function') as ExecCallback;
|
||||
callback(new Error('Generic exec error') as ExecException, '', 'Some stderr output');
|
||||
return {} as ChildProcess;
|
||||
@@ -128,7 +127,7 @@ describe('System Routes (/api/system)', () => {
|
||||
// Arrange: Simulate a scenario where the command writes to stderr but doesn't
|
||||
// produce a formal error object for the callback's first argument.
|
||||
const stderrMessage = 'A non-fatal warning or configuration issue.';
|
||||
vi.mocked(exec).mockImplementation((...args: any[]) => {
|
||||
vi.mocked(exec).mockImplementation((...args: [string, ...unknown[]]) => {
|
||||
const callback = args.find(arg => typeof arg === 'function') as ExecCallback;
|
||||
callback(null, '', stderrMessage);
|
||||
return {} as ChildProcess;
|
||||
|
||||
@@ -7,7 +7,6 @@ import { http, HttpResponse } from 'msw';
|
||||
vi.unmock('./aiApiClient');
|
||||
|
||||
import * as aiApiClient from './aiApiClient';
|
||||
import { FlyerItem } from '../types';
|
||||
|
||||
// 1. Mock logger to keep output clean
|
||||
vi.mock('./logger', () => ({
|
||||
@@ -40,7 +39,7 @@ const server = setupServer(
|
||||
if (contentType?.includes('application/json')) {
|
||||
try {
|
||||
body = await request.json() as Record<string, unknown>;
|
||||
} catch (err) { /* ignore parse error */ }
|
||||
} catch { /* ignore parse error */ }
|
||||
} else if (contentType?.includes('multipart/form-data')) {
|
||||
try {
|
||||
// This is the key part. We read the formData from the request.
|
||||
@@ -53,7 +52,7 @@ const server = setupServer(
|
||||
}
|
||||
formDataBody._isFormData = true;
|
||||
body = formDataBody;
|
||||
} catch (err) { /* ignore parse error */ }
|
||||
} catch { /* ignore parse error */ }
|
||||
}
|
||||
|
||||
requestSpy({
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
* It communicates with the application's own backend endpoints, which then securely
|
||||
* call the Google AI services. This ensures no API keys are exposed on the client.
|
||||
*/
|
||||
import type { FlyerItem, MasterGroceryItem, Store } from "../types";
|
||||
import type { FlyerItem } from "../types";
|
||||
import { logger } from "./logger";
|
||||
import { apiFetchWithAuth } from './apiClient';
|
||||
|
||||
|
||||
@@ -274,7 +274,7 @@ export const extractTextFromImageArea = async (
|
||||
* @param userLocation The user's current geographic coordinates.
|
||||
* @returns A text response with trip planning advice and a list of map sources.
|
||||
*/
|
||||
export const planTripWithMaps = async (items: FlyerItem[], store: { name: string } | undefined, userLocation: GeolocationCoordinates): Promise<{text: string; sources: { uri: string; title: string; }[]}> => {
|
||||
export const planTripWithMaps = async (): Promise<{text: string; sources: { uri: string; title: string; }[]}> => {
|
||||
// const topItems = items.slice(0, 5).map(i => i.item).join(', ');
|
||||
// const storeName = store?.name || 'the grocery store';
|
||||
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
// src/services/db/connection.test.ts
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import { getPool, checkTablesExist, getPoolStatus } from './connection';
|
||||
import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest';
|
||||
|
||||
// Mock the logger
|
||||
vi.mock('../logger', () => ({
|
||||
@@ -71,8 +70,8 @@ describe('DB Connection Service', () => {
|
||||
const pool = getPool();
|
||||
|
||||
// Mock the query response on the *instance* returned by getPool
|
||||
// We cast to any to avoid strict typing issues with the mock injection
|
||||
(pool.query as any) = vi.fn().mockResolvedValue({ rows: [{ table_name: 'users' }, { table_name: 'flyers' }] });
|
||||
// Use `as Mock` for type-safe mocking.
|
||||
(pool.query as Mock).mockResolvedValue({ rows: [{ table_name: 'users' }, { table_name: 'flyers' }] });
|
||||
|
||||
const tableNames = ['users', 'flyers'];
|
||||
const missingTables = await checkTablesExist(tableNames);
|
||||
@@ -85,7 +84,7 @@ describe('DB Connection Service', () => {
|
||||
const { getPool, checkTablesExist } = await import('./connection');
|
||||
const pool = getPool();
|
||||
|
||||
(pool.query as any) = vi.fn().mockResolvedValue({ rows: [{ table_name: 'users' }] });
|
||||
(pool.query as Mock).mockResolvedValue({ rows: [{ table_name: 'users' }] });
|
||||
|
||||
const tableNames = ['users', 'flyers', 'products'];
|
||||
const missingTables = await checkTablesExist(tableNames);
|
||||
|
||||
@@ -51,7 +51,7 @@ describe('User DB Service', () => {
|
||||
totalCount: 0,
|
||||
idleCount: 0,
|
||||
waitingCount: 0,
|
||||
} as any;
|
||||
} as unknown as Pool;
|
||||
});
|
||||
|
||||
vi.clearAllMocks();
|
||||
|
||||
20
src/services/express.d.ts
vendored
20
src/services/express.d.ts
vendored
@@ -1,20 +0,0 @@
|
||||
// src/services/express.d.ts
|
||||
import { Request, Response, NextFunction, RequestHandler } from 'express';
|
||||
|
||||
/**
|
||||
* Defines a more accurate type for asynchronous Express route handlers.
|
||||
* Standard `RequestHandler` requires `next` to be present, which is often unused
|
||||
* in modern async/await handlers, leading to linting errors. This type makes `next` optional.
|
||||
*
|
||||
* It also allows for more specific typing of the request body, query, and params.
|
||||
*/
|
||||
export type AsyncRequestHandler<
|
||||
P = Record<string, string>,
|
||||
ResBody = any,
|
||||
ReqBody = any,
|
||||
ReqQuery = qs.ParsedQs
|
||||
> = (
|
||||
req: Request<P, ResBody, ReqBody, ReqQuery>,
|
||||
res: Response<ResBody>,
|
||||
next?: NextFunction
|
||||
) => Promise<void | unknown>;
|
||||
6
src/types/express.d.ts
vendored
6
src/types/express.d.ts
vendored
@@ -1,5 +1,5 @@
|
||||
// src/types/express.d.ts
|
||||
import { Request, Response, NextFunction, RequestHandler } from 'express';
|
||||
import { Request, Response, NextFunction } from 'express';
|
||||
import * as qs from 'qs';
|
||||
|
||||
/**
|
||||
@@ -11,8 +11,8 @@ import * as qs from 'qs';
|
||||
*/
|
||||
export type AsyncRequestHandler<
|
||||
P = Record<string, string>,
|
||||
ResBody = any,
|
||||
ReqBody = any,
|
||||
ResBody = unknown,
|
||||
ReqBody = unknown,
|
||||
ReqQuery = qs.ParsedQs
|
||||
> = (
|
||||
req: Request<P, ResBody, ReqBody, ReqQuery>,
|
||||
|
||||
Reference in New Issue
Block a user