Refactor: Introduce requiredString helper for consistent validation across routes and services
All checks were successful
Deploy to Test Environment / deploy-to-test (push) Successful in 8m21s

This commit is contained in:
2025-12-15 19:19:23 -08:00
parent d5f185ad99
commit 08340a4cf2
16 changed files with 81 additions and 48 deletions

View File

@@ -1,6 +1,6 @@
// src/features/flyer/AnalysisPanel.test.tsx
import React from 'react';
import { render, screen, fireEvent, waitFor } from '@testing-library/react';
import { render, screen, fireEvent } from '@testing-library/react';
import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest';
import { AnalysisPanel } from './AnalysisPanel';
import { useFlyerItems } from '../../hooks/useFlyerItems';

View File

@@ -102,14 +102,14 @@ export const useAiAnalysis = ({ flyerItems, selectedFlyer, watchedItems }: UseAi
// Check for Geolocation error specifically or by code (1 = PERMISSION_DENIED)
if (
(typeof e === 'object' && e !== null && 'code' in e && (e as any).code === 1) ||
(typeof e === 'object' && e !== null && 'code' in e && (e as { code: unknown }).code === 1) ||
(typeof GeolocationPositionError !== 'undefined' && e instanceof GeolocationPositionError && e.code === GeolocationPositionError.PERMISSION_DENIED)
) {
message = "Geolocation permission denied.";
} else if (e instanceof Error) {
message = e.message;
} else if (typeof e === 'object' && e !== null && 'message' in e) {
message = (e as any).message;
message = String((e as { message: unknown }).message);
} else if (typeof e === 'string') {
message = e;
}
@@ -128,7 +128,10 @@ export const useAiAnalysis = ({ flyerItems, selectedFlyer, watchedItems }: UseAi
const generateImage = useCallback(async () => {
const mealPlanText = results[AnalysisType.DEEP_DIVE];
if (!mealPlanText) return;
if (!mealPlanText) {
logger.warn('generateImage called but no meal plan text available in results');
return;
}
setInternalError(null);
try {
await generateImageApi(mealPlanText);
@@ -138,7 +141,7 @@ export const useAiAnalysis = ({ flyerItems, selectedFlyer, watchedItems }: UseAi
if (e instanceof Error) {
message = e.message;
} else if (typeof e === 'object' && e !== null && 'message' in e) {
message = (e as any).message;
message = String((e as { message: unknown }).message);
} else if (typeof e === 'string') {
message = e;
}

View File

@@ -94,7 +94,8 @@ describe('useAuth Hook and AuthProvider', () => {
localStorageMock.setItem('authToken', 'valid-token');
mockedApiClient.getAuthenticatedUserProfile.mockResolvedValue({
ok: true,
json: async () => mockProfile,
status: 200,
json: () => Promise.resolve(mockProfile),
} as unknown as Response);
const { result } = renderHook(() => useAuth(), { wrapper });
@@ -133,7 +134,8 @@ describe('useAuth Hook and AuthProvider', () => {
it('sets token, fetches profile, and updates state on successful login', async () => {
mockedApiClient.getAuthenticatedUserProfile.mockResolvedValue({
ok: true,
json: async () => mockProfile,
status: 200,
json: () => Promise.resolve(mockProfile),
} as unknown as Response);
const { result } = renderHook(() => useAuth(), { wrapper });
@@ -184,7 +186,8 @@ describe('useAuth Hook and AuthProvider', () => {
localStorageMock.setItem('authToken', 'valid-token');
mockedApiClient.getAuthenticatedUserProfile.mockResolvedValue({
ok: true,
json: async () => mockProfile,
status: 200,
json: () => Promise.resolve(mockProfile),
} as unknown as Response);
const { result } = renderHook(() => useAuth(), { wrapper });
@@ -209,7 +212,8 @@ describe('useAuth Hook and AuthProvider', () => {
localStorageMock.setItem('authToken', 'valid-token');
mockedApiClient.getAuthenticatedUserProfile.mockResolvedValue({
ok: true,
json: async () => mockProfile,
status: 200,
json: () => Promise.resolve(mockProfile),
} as unknown as Response);
const { result } = renderHook(() => useAuth(), { wrapper });

View File

@@ -156,7 +156,7 @@ describe('useUserData Hook and UserDataProvider', () => {
// Define the sequence: 1st call (Watched) -> Error, 2nd call (Shopping) -> Success
// We want this to persist for multiple renders.
mockedUseApiOnMount.mockImplementation((fn) => {
mockedUseApiOnMount.mockImplementation((_fn) => {
// We can't easily distinguish based on 'fn' arg without inspecting it,
// but we know the order is Watched then Shopping in the provider.
// A simple toggle approach works if strict order is maintained.

View File

@@ -49,9 +49,9 @@ describe('SystemCheck', () => {
// Helper to set GEMINI_API_KEY for specific tests.
const setGeminiApiKey = (value: string | undefined) => {
if (value === undefined) {
delete (import.meta.env as any).GEMINI_API_KEY;
delete (import.meta.env as Record<string, string | undefined>).GEMINI_API_KEY;
} else {
(import.meta.env as any).GEMINI_API_KEY = value;
(import.meta.env as Record<string, string | undefined>).GEMINI_API_KEY = value;
}
};

View File

@@ -23,6 +23,9 @@ import { backgroundJobService } from '../services/backgroundJobService';
import { flyerQueue, emailQueue, analyticsQueue, cleanupQueue, weeklyAnalyticsQueue, flyerWorker, emailWorker, analyticsWorker, cleanupWorker, weeklyAnalyticsWorker } from '../services/queueService.server'; // Import your queues
import { getSimpleWeekAndYear } from '../utils/dateUtils';
// Helper for consistent required string validation (handles missing/null/empty)
const requiredString = (message: string) => z.preprocess((val) => val ?? '', z.string().min(1, message));
/**
* A factory for creating a Zod schema that validates a UUID in the request parameters.
* @param key The name of the parameter key (e.g., 'userId').
@@ -41,7 +44,7 @@ const numericIdParamSchema = (key: string, message = `Invalid ID for parameter '
const updateCorrectionSchema = numericIdParamSchema('id').extend({
body: z.object({
suggested_value: z.string().min(1, 'A new suggested_value is required.'),
suggested_value: requiredString('A new suggested_value is required.'),
}),
});
@@ -73,7 +76,7 @@ const activityLogSchema = z.object({
const jobRetrySchema = z.object({
params: z.object({
queueName: z.enum(['flyer-processing', 'email-sending', 'analytics-reporting', 'file-cleanup', 'weekly-analytics-reporting']),
jobId: z.string().min(1, 'A valid Job ID is required.'),
jobId: requiredString('A valid Job ID is required.'),
}),
});

View File

@@ -15,7 +15,6 @@ import { logger } from '../services/logger.server';
import { UserProfile, ExtractedCoreData } from '../types';
import { flyerQueue } from '../services/queueService.server';
import { validateRequest } from '../middleware/validation.middleware';
import { NotFoundError } from '../services/db/errors.db';
const router = Router();
@@ -27,18 +26,18 @@ interface FlyerProcessPayload extends Partial<ExtractedCoreData> {
}
// --- Zod Schemas for AI Routes (as per ADR-003) ---
// Helper for consistent required string validation (handles missing/null/empty)
const requiredString = (message: string) => z.preprocess((val) => val ?? '', z.string().min(1, message));
const uploadAndProcessSchema = z.object({
body: z.object({
checksum: z.string().refine(val => val && val.length > 0, {
message: 'File checksum is required.',
}),
checksum: requiredString('File checksum is required.'),
}),
});
const jobIdParamSchema = z.object({
params: z.object({
jobId: z.string().min(1, 'A valid Job ID is required.'),
jobId: requiredString('A valid Job ID is required.'),
}),
});
@@ -51,9 +50,7 @@ const errMsg = (e: unknown) => {
const rescanAreaSchema = z.object({
body: z.object({
cropArea: z.string().refine(val => val && val.length > 0, {
message: 'cropArea must be a valid JSON string.',
}).transform((val, ctx) => {
cropArea: requiredString('cropArea must be a valid JSON string.').transform((val, ctx) => {
try { return JSON.parse(val); }
catch (err) {
// Log the actual parsing error for better debugging if invalid JSON is sent.
@@ -61,9 +58,7 @@ const rescanAreaSchema = z.object({
ctx.addIssue({ code: z.ZodIssueCode.custom, message: 'cropArea must be a valid JSON string.' }); return z.NEVER;
}
}),
extractionType: z.string().refine(val => val && val.length > 0, {
message: 'extractionType is required.',
}),
extractionType: requiredString('extractionType is required.'),
}),
});
@@ -91,15 +86,15 @@ const planTripSchema = z.object({
});
const generateImageSchema = z.object({
body: z.object({ prompt: z.string().min(1) }),
body: z.object({ prompt: requiredString('A prompt is required.') }),
});
const generateSpeechSchema = z.object({
body: z.object({ text: z.string().min(1) }),
body: z.object({ text: requiredString('Text is required.') }),
});
const searchWebSchema = z.object({
body: z.object({ query: z.string().min(1, 'A search query is required.') }),
body: z.object({ query: requiredString('A search query is required.') }),
});
// --- Multer Configuration for File Uploads ---

View File

@@ -1,12 +1,11 @@
// src/routes/auth.routes.test.ts
import { describe, it, expect, vi, beforeEach } from 'vitest';
import supertest from 'supertest';
import { Request, Response, NextFunction, RequestHandler } from 'express';
import { Request, Response, NextFunction } from 'express';
import cookieParser from 'cookie-parser';
import * as bcrypt from 'bcrypt';
import { createMockUserProfile, createMockUserWithPasswordHash } from '../tests/utils/mockFactories';
import { mockLogger } from '../tests/utils/mockLogger';
import { createTestApp } from '../tests/utils/createTestApp';
// --- FIX: Hoist passport mocks to be available for vi.mock ---
const passportMocks = vi.hoisted(() => {
@@ -199,7 +198,10 @@ describe('Auth Routes (/api/auth)', () => {
expect(response.status).toBe(400);
// The validation middleware returns errors in an array.
// We check if any of the error messages contain the expected text.
const errorMessages = response.body.errors?.map((e: any) => e.message).join(' ');
interface ZodError {
message: string;
}
const errorMessages = response.body.errors?.map((e: ZodError) => e.message).join(' ');
expect(errorMessages).toMatch(/Password is too weak/i);
});
@@ -468,11 +470,12 @@ describe('Auth Routes (/api/auth)', () => {
});
it('should return 403 if refresh token is invalid', async () => {
vi.mocked(db.userRepo.findUserByRefreshToken).mockRejectedValue(new Error('Invalid or expired refresh token.'));
// Mock finding no user for this token, which should trigger the 403 logic
vi.mocked(db.userRepo.findUserByRefreshToken).mockResolvedValue(undefined as any);
const response = await supertest(app)
.post('/api/auth/refresh-token')
.set('Cookie', 'refreshToken=invalid-token'); // This was a duplicate, fixed.
.set('Cookie', 'refreshToken=invalid-token');
expect(response.status).toBe(403);
});

View File

@@ -37,6 +37,9 @@ const validatePasswordStrength = (password: string): { isValid: boolean; feedbac
return { isValid: true };
};
// Helper for consistent required string validation (handles missing/null/empty)
const requiredString = (message: string) => z.preprocess((val) => val ?? '', z.string().min(1, message));
// Conditionally disable rate limiting for the test environment
const isTestEnv = process.env.NODE_ENV === 'test';
@@ -79,7 +82,7 @@ const forgotPasswordSchema = z.object({
const resetPasswordSchema = z.object({
body: z.object({
token: z.string().min(1, 'Token is required.'),
token: requiredString('Token is required.'),
newPassword: z.string().min(8, 'Password must be at least 8 characters long.').superRefine((password, ctx) => {
const strength = validatePasswordStrength(password);
if (!strength.isValid) ctx.addIssue({ code: 'custom', message: strength.feedback });

View File

@@ -8,6 +8,9 @@ import { validateRequest } from '../middleware/validation.middleware';
const router = express.Router();
// Helper for consistent required string validation (handles missing/null/empty)
const requiredString = (message: string) => z.preprocess((val) => val ?? '', z.string().min(1, message));
// --- Zod Schemas for Budget Routes (as per ADR-003) ---
const budgetIdParamSchema = z.object({
@@ -18,7 +21,7 @@ const budgetIdParamSchema = z.object({
const createBudgetSchema = z.object({
body: z.object({
name: z.string().min(1, 'Budget name is required.'),
name: requiredString('Budget name is required.'),
amount_cents: z.number().int().positive('Amount must be a positive integer.'),
period: z.enum(['weekly', 'monthly']),
start_date: z.string().date('Start date must be a valid date in YYYY-MM-DD format.'),

View File

@@ -11,6 +11,9 @@ import { validateRequest } from '../middleware/validation.middleware';
const router = express.Router();
const adminGamificationRouter = express.Router(); // Create a new router for admin-only routes.
// Helper for consistent required string validation (handles missing/null/empty)
const requiredString = (message: string) => z.preprocess((val) => val ?? '', z.string().min(1, message));
// --- Zod Schemas for Gamification Routes (as per ADR-003) ---
const leaderboardSchema = z.object({
@@ -21,8 +24,8 @@ const leaderboardSchema = z.object({
const awardAchievementSchema = z.object({
body: z.object({
userId: z.string().min(1, 'userId is required.'),
achievementName: z.string().min(1, 'achievementName is required.'),
userId: requiredString('userId is required.'),
achievementName: requiredString('achievementName is required.'),
}),
});
@@ -46,7 +49,6 @@ router.get('/', async (req, res, next: NextFunction) => {
* GET /api/achievements/leaderboard - Get the top users by points.
* This is a public endpoint.
*/
type LeaderboardRequest = z.infer<typeof leaderboardSchema>;
router.get('/leaderboard', validateRequest(leaderboardSchema), async (req, res, next: NextFunction): Promise<void> => {
// Apply ADR-003 pattern for type safety.
// Explicitly coerce query params to ensure numbers are passed to the repo,

View File

@@ -6,6 +6,9 @@ import { validateRequest } from '../middleware/validation.middleware';
const router = Router();
// Helper for consistent required string validation (handles missing/null/empty)
const requiredString = (message: string) => z.preprocess((val) => val ?? '', z.string().min(1, message));
// --- Zod Schemas for Recipe Routes (as per ADR-003) ---
const bySalePercentageSchema = z.object({
@@ -22,8 +25,8 @@ const bySaleIngredientsSchema = z.object({
const byIngredientAndTagSchema = z.object({
query: z.object({
ingredient: z.string().min(1, 'Query parameter "ingredient" is required.'),
tag: z.string().min(1, 'Query parameter "tag" is required.'),
ingredient: requiredString('Query parameter "ingredient" is required.'),
tag: requiredString('Query parameter "tag" is required.'),
}),
});

View File

@@ -8,9 +8,12 @@ import { validateRequest } from '../middleware/validation.middleware';
const router = Router();
// Helper for consistent required string validation (handles missing/null/empty)
const requiredString = (message: string) => z.preprocess((val) => val ?? '', z.string().min(1, message));
const geocodeSchema = z.object({
body: z.object({
address: z.string().min(1, 'An address string is required.'),
address: requiredString('An address string is required.'),
}),
});

View File

@@ -33,6 +33,9 @@ const validatePasswordStrength = (password: string): { isValid: boolean; feedbac
return { isValid: true };
};
// Helper for consistent required string validation (handles missing/null/empty)
const requiredString = (message: string) => z.preprocess((val) => val ?? '', z.string().min(1, message));
// --- Zod Schemas for User Routes (as per ADR-003) ---
const numericIdParam = (key: string) => z.object({
@@ -54,17 +57,17 @@ const updatePasswordSchema = z.object({
});
const deleteAccountSchema = z.object({
body: z.object({ password: z.string().min(1, "Field 'password' is required.") }),
body: z.object({ password: requiredString("Field 'password' is required.") }),
});
const addWatchedItemSchema = z.object({
body: z.object({
itemName: z.string().min(1, "Field 'itemName' is required."),
category: z.string().min(1, "Field 'category' is required."),
itemName: requiredString("Field 'itemName' is required."),
category: requiredString("Field 'category' is required."),
}),
});
const createShoppingListSchema = z.object({ body: z.object({ name: z.string().min(1, "Field 'name' is required.") }) });
const createShoppingListSchema = z.object({ body: z.object({ name: requiredString("Field 'name' is required.") }) });
// Apply the JWT authentication middleware to all routes in this file.
const notificationQuerySchema = z.object({
@@ -417,7 +420,7 @@ router.delete('/shopping-lists/:listId', validateRequest(shoppingListIdSchema),
const addShoppingListItemSchema = shoppingListIdSchema.extend({
body: z.object({
masterItemId: z.number().int().positive().optional(),
customItemName: z.string().min(1).optional(),
customItemName: requiredString('customItemName required?'),
}).refine(data => data.masterItemId || data.customItemName, { message: 'Either masterItemId or customItemName must be provided.' }),
});
type AddShoppingListItemRequest = z.infer<typeof addShoppingListItemSchema>;

View File

@@ -12,6 +12,9 @@ import { z } from 'zod';
import { pRateLimit } from 'p-ratelimit';
import type { FlyerItem, MasterGroceryItem, ExtractedFlyerItem } from '../types';
// Helper for consistent required string validation (handles missing/null/empty)
const requiredString = (message: string) => z.preprocess((val) => val ?? '', z.string().min(1, message));
// --- Zod Schemas for AI Response Validation (exported for the transformer) ---
const ExtractedFlyerItemSchema = z.object({
item: z.string(),
@@ -23,7 +26,7 @@ const ExtractedFlyerItemSchema = z.object({
});
export const AiFlyerDataSchema = z.object({
store_name: z.string().min(1, { message: "Store name cannot be empty" }),
store_name: requiredString( "Store name cannot be empty" ),
valid_from: z.string().nullable(),
valid_to: z.string().nullable(),
store_address: z.string().nullable(),

View File

@@ -11,6 +11,11 @@ import { PdfConversionError, AiDataValidationError } from './processingErrors';
import { FlyerDataTransformer } from './flyerDataTransformer';
import { logger as globalLogger } from './logger.server';
import type { Logger } from 'pino';
// Helper for consistent required string validation (handles missing/null/empty)
const requiredString = (message: string) => z.preprocess((val) => val ?? '', z.string().min(1, message));
// --- Start: Interfaces for Dependency Injection ---
export interface IFileSystem {
@@ -56,7 +61,7 @@ const ExtractedFlyerItemSchema = z.object({
});
export const AiFlyerDataSchema = z.object({
store_name: z.string().min(1, { message: "Store name cannot be empty" }),
store_name: requiredString( "Store name cannot be empty" ),
valid_from: z.string().nullable(),
valid_to: z.string().nullable(),
store_address: z.string().nullable(),