Code Review — NutriPaw PR #47

Resumen de la revisión

Este PR introduce el endpoint /api/v1/plans/weight-loss para generar planes calóricos personalizados en NutriPaw. La estructura general es correcta y el modelo Sequelize está bien definido, pero se identificaron 2 hallazgos críticos: ausencia de validación de entrada (permite inyección de datos maliciosos y desbordamiento aritmético) y una consulta SQL construida con interpolación de cadenas que expone el sistema a SQL injection. Adicionalmente, hay un problema de eficiencia por consultas N+1 en el servicio y varios puntos de mantenibilidad. Se recomienda no hacer merge hasta resolver los hallazgos críticos y de severidad alta.

Critical

High

Medium

Low

Archivos modificados

📄

src/routes/plans.ts

Añadió la ruta POST /weight-loss con validación inline incompleta y manejo de errores genérico.

⚙️

src/services/planCalculator.ts

Implementó la lógica de cálculo de calorías con interpolación SQL insegura y bucle de inserción de registros N+1.

🗃️

src/models/Plan.ts

Definió el modelo Sequelize Plan con las columnas necesarias; falta índice en pet_id para consultas frecuentes.

🧪

tests/plans.test.ts

Añadió test del happy path; no cubre casos de error ni entradas inválidas.

Hallazgos por severidad

Critical src/routes/plans.ts Línea 34

Sin validación de entrada — desbordamiento aritmético y datos inválidos.
Los campos current_weight, target_weight y weeks se pasan directamente al servicio sin validar tipo ni rango. Un valor weeks=0 produce una división por cero en el cálculo calórico; valores negativos generan planes incoherentes que podrían dañar al animal.

Sugerencia

- const { pet_id, current_weight, target_weight, weeks } = req.body;
+ import { z } from 'zod';
+ const PlanSchema = z.object({
+   pet_id: z.string().uuid(),
+   current_weight: z.number().positive().max(200),
+   target_weight: z.number().positive().max(200),
+   weeks: z.number().int().min(1).max(52),
+ });
+ const parsed = PlanSchema.safeParse(req.body);
+ if (!parsed.success) return res.status(400).json({ error: parsed.error.flatten() });
+ const { pet_id, current_weight, target_weight, weeks } = parsed.data;
        

Critical src/services/planCalculator.ts Línea 61

SQL Injection por interpolación directa de cadenas.
La consulta de historial del paciente se construye con template literals usando pet_id sin escapar. Aunque pet_id pasa por Sequelize en otros lugares, aquí se usa sequelize.query() con interpolación directa, lo que permite SQL injection clásica.

Sugerencia

- const history = await sequelize.query(
-   `SELECT * FROM weight_records WHERE pet_id = '${pet_id}'`
- );
+ const history = await sequelize.query(
+   'SELECT * FROM weight_records WHERE pet_id = :petId',
+   { replacements: { petId: pet_id }, type: QueryTypes.SELECT }
+ );
        

High src/services/planCalculator.ts Línea 88–102

Consultas N+1 — inserción de registros diarios en bucle.
El bucle for (let day = 1; day <= weeks * 7; day++) ejecuta un INSERT por cada día del plan. Para un plan de 12 semanas genera 84 queries individuales. Debe reemplazarse por bulkCreate.

Sugerencia

- for (let day = 1; day <= weeks * 7; day++) {
-   await DailyRecord.create({ plan_id: plan.id, day, calories });
- }
+ const records = Array.from({ length: weeks * 7 }, (_, i) => ({
+   plan_id: plan.id,
+   day: i + 1,
+   calories,
+ }));
+ await DailyRecord.bulkCreate(records);
        

High src/routes/plans.ts Línea 52

Error genérico expone stack trace en producción.
El bloque catch devuelve res.status(500).json({ error: e.message, stack: e.stack }), exponiendo detalles internos del servidor al cliente. El stack trace es útil en desarrollo pero no debe llegar a producción.

Sugerencia

- res.status(500).json({ error: e.message, stack: e.stack });
+ logger.error('weight-loss plan creation failed', { error: e });
+ res.status(500).json({ error: 'Internal server error' });
        

Medium src/models/Plan.ts Línea 18

Falta índice en pet_id — degradación de rendimiento en consultas frecuentes.
El modelo no define un índice sobre pet_id, que es la columna más consultada. Sin índice, cada lookup escanea la tabla completa (full table scan) y el rendimiento se degradará a medida que crezca el dataset.

Sugerencia

  pet_id: {
    type: DataTypes.UUID,
-   allowNull: false,
+   allowNull: false,
+   index: true,   // o define en indexes: [{ fields: ['pet_id'] }] a nivel de modelo
  },
        

Medium src/services/planCalculator.ts Línea 24

Número mágico en la fórmula calórica.
La fórmula usa el literal 7700 (kcal por kg de grasa) sin explicación. Si los requisitos nutricionales cambian, no hay forma de rastrearlo. Extraer como constante nombrada con comentario JSDoc.

Sugerencia

+ /** Kilocalorías aproximadas por kilogramo de tejido adiposo (estándar WSAVA 2023) */
+ const KCAL_PER_KG_FAT = 7700;
 
- const totalKcalDeficit = (current_weight - target_weight) * 7700;
+ const totalKcalDeficit = (current_weight - target_weight) * KCAL_PER_KG_FAT;
        

Medium tests/plans.test.ts Línea 1–45

Cobertura de tests insuficiente — solo happy path.
Los tests actuales verifican únicamente el caso de éxito. No hay tests para: weeks=0, current_weight < target_weight, pet_id inexistente, ni cuerpo de request vacío. Estos son los casos que más frecuentemente causan regresiones en producción.

Low src/routes/plans.ts Línea 12

Nombre de función demasiado genérico.
La función del handler se llama handlePlan. Con múltiples tipos de planes en el futuro, el nombre será ambiguo. Renombrar a createWeightLossPlan para mayor claridad.

Low src/services/planCalculator.ts Línea 5

Import no utilizado: lodash.
Se importa import _ from 'lodash' pero no se usa en el archivo. Eliminar para reducir el bundle y evitar dependencias innecesarias.

Low src/models/Plan.ts Línea 8

Falta timestamps: true en el modelo.
El modelo no especifica explícitamente timestamps. Si la configuración global de Sequelize los desactiva, el modelo no tendrá createdAt/updatedAt, lo que dificulta auditorías. Declararlo explícitamente.

Low src/routes/plans.ts Línea 3

Typo en comentario JSDoc.
El comentario dice "Calcuates the daily caloric deficit" (falta la 'l' en "calculates"). Corregir a "Calculates the daily caloric deficit".

Recomendaciones clave

Bloquear merge hasta resolver los 2 hallazgos críticos: añadir validación de entrada con Zod y reemplazar la interpolación SQL por parámetros nombrados de Sequelize.

Reemplazar bucle de inserciones por bulkCreate para evitar la degradación de rendimiento en planes de múltiples semanas (hallazgo alto).

Sanitizar el manejo de errores: nunca exponer stack traces al cliente. Usar un logger central y devolver mensajes genéricos en producción.

Ampliar los tests con casos de borde: entradas inválidas, divisiones por cero, y pet_id no existente. El happy path solo verifica el flujo óptimo.

Añadir índice en pet_id del modelo Plan y declarar explícitamente los timestamps para consistencia del esquema.

feat: add pet weight-loss plan endpoint

Resumen de la revisión

Hallazgos por severidad

Recomendaciones clave