Repository navigation
Om 97 - #67
Om 97 #67Lianyang1234 wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughSe migraron módulos de DTOs de JavaScript a TypeScript, reemplazando validaciones en el DTO de solicitud con lógica de servicio. La validación de rango de precios se movió a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/modules/commerce/commerces/store.service.js (1)
877-886: Validación duplicada entre DTO y servicio.Esta validación de
price_min > price_maxya existe en el DTO (líneas 73-82 defilter-store-products.dto.ts) y se ejecuta vía middleware antes de llegar al controlador. Sin embargo, esta duplicación provee defensa en profundidad para llamadas directas al servicio (como en los tests unitarios), por lo que es aceptable mantenerla.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/commerce/commerces/store.service.js` around lines 877 - 886, Keep the existing price_min > price_max validation in the service (the if block that checks normalizedMinPrice and normalizedMaxPrice) to preserve defense-in-depth for direct service calls/tests; add a concise comment above that block explaining it duplicates the DTO validation (so future readers know it's intentional), and ensure the thrown error still returns status: 400 with the same message and uses normalizedMinPrice/normalizedMaxPrice as the checked variables.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/modules/commerce/commerces/store.routes.js`:
- Line 14: The import of FilterStoreProductsDTO using a .ts extension in
store.routes.js will cause a runtime module resolution error; fix by either
changing the import to reference the compiled .js output of
FilterStoreProductsDTO, update your start/dev scripts to run via tsx (so Node
can load .ts at runtime), or ensure tsc builds the project before start and keep
the import pointing to the compiled .js file; locate the import statement for
FilterStoreProductsDTO in store.routes.js and apply one of these three fixes so
the module can be resolved at runtime.
In `@src/modules/global/dtos/commerce/filter-store-products.dto.ts`:
- Around line 53-71: Los validators `sortBy` y `sortOrder` usan la forma
inválida `{ error: "..." }` en z.enum; actualiza ambos para usar la API correcta
de Zod 4 — por ejemplo reemplaza `z.enum([...], { error: "mensaje" })` por
`z.enum([...], "mensaje")` (o alternadamente `{ error: (issues) => "mensaje"
}`), manteniendo la envoltura con `z.preprocess(emptyToUndefined, ...)`, las
llamadas a `z.enum` y las propiedades `.optional().default(...)` intactas para
`sortBy` y `sortOrder`.
---
Nitpick comments:
In `@src/modules/commerce/commerces/store.service.js`:
- Around line 877-886: Keep the existing price_min > price_max validation in the
service (the if block that checks normalizedMinPrice and normalizedMaxPrice) to
preserve defense-in-depth for direct service calls/tests; add a concise comment
above that block explaining it duplicates the DTO validation (so future readers
know it's intentional), and ensure the thrown error still returns status: 400
with the same message and uses normalizedMinPrice/normalizedMaxPrice as the
checked variables.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 77b939da-d457-4877-94ea-481b29fc1bf8
📒 Files selected for processing (8)
src/modules/commerce/commerces/store.controller.jssrc/modules/commerce/commerces/store.routes.jssrc/modules/commerce/commerces/store.service.jssrc/modules/global/dtos/commerce/filter-store-products.dto.jssrc/modules/global/dtos/commerce/filter-store-products.dto.tssrc/modules/global/dtos/commerce/filter-store-products.response.jssrc/modules/global/dtos/commerce/filter-store-products.response.tstests/e2e/stores.test.js
💤 Files with no reviewable changes (2)
- src/modules/global/dtos/commerce/filter-store-products.response.js
- src/modules/global/dtos/commerce/filter-store-products.dto.js
| import { parsePagination } from "../../../middlewares/pagination.middleware.js"; | ||
| import { validate } from "../../../middlewares/validate.middleware.js"; | ||
| import { FilterStoreProductsDTO } from "../../global/dtos/commerce/filter-store-products.dto.js"; | ||
| import { FilterStoreProductsDTO } from "../../global/dtos/commerce/filter-store-products.dto.ts"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verificar configuración de TypeScript y scripts de build
cat tsconfig.json 2>/dev/null || echo "No tsconfig.json found"
cat package.json | jq '.scripts' 2>/dev/nullRepository: CrisNAC/BackendMarketplace
Length of output: 530
🏁 Script executed:
# Check if the target .ts file exists
fd -t f "filter-store-products.dto.ts"Repository: CrisNAC/BackendMarketplace
Length of output: 131
🏁 Script executed:
# Check the content of src/server.js to see how it's bootstrapped
head -30 src/server.jsRepository: CrisNAC/BackendMarketplace
Length of output: 279
🏁 Script executed:
# Check if tsx or ts-node are installed as dependencies
jq '.dependencies, .devDependencies' package.json | grep -E "(tsx|ts-node|esbuild|webpack)" || echo "No tsx/ts-node found"Repository: CrisNAC/BackendMarketplace
Length of output: 89
🏁 Script executed:
# Check the actual store.routes.js file to understand the context better
cat src/modules/commerce/commerces/store.routes.js | head -20Repository: CrisNAC/BackendMarketplace
Length of output: 650
🏁 Script executed:
# Check if app.js and app.ts both exist
ls -la src/app.* 2>/dev/null || echo "No app files found"Repository: CrisNAC/BackendMarketplace
Length of output: 129
🏁 Script executed:
# Check if there's any build setup or if files are pre-compiled
ls -la src/ | head -20Repository: CrisNAC/BackendMarketplace
Length of output: 754
🏁 Script executed:
# Check if package.json has a prepare script or if build is automatically run
jq '.scripts | to_entries | .[]' package.jsonRepository: CrisNAC/BackendMarketplace
Length of output: 366
🏁 Script executed:
# Verify how the actual imports work - check if .ts is being transpiled in build
cat package.json | jq '.build, .type'Repository: CrisNAC/BackendMarketplace
Length of output: 83
Importación de archivo .ts desde archivo .js causará error de runtime.
La importación directa con extensión .ts desde un archivo JavaScript no funcionará. Node.js no puede resolver extensiones .ts en runtime con los scripts actuales (node sin tsx). Aunque tsx está instalado como dependencia, los scripts de start y dev usan node y nodemon directamente, no tsx.
Opciones para resolver:
- Cambiar el import a:
import { FilterStoreProductsDTO } from "../../global/dtos/commerce/filter-store-products.dto.js";(si el archivo está compilado a .js) - O usar
tsxen los scripts:"start": "tsx src/server.js"y"dev": "nodemon --exec tsx src/server.js" - O asegurar que el build con
tscse ejecute antes destart(agregar step previo)
Este cambio causará un error de módulo no encontrado cuando la app intente iniciarse.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/commerce/commerces/store.routes.js` at line 14, The import of
FilterStoreProductsDTO using a .ts extension in store.routes.js will cause a
runtime module resolution error; fix by either changing the import to reference
the compiled .js output of FilterStoreProductsDTO, update your start/dev scripts
to run via tsx (so Node can load .ts at runtime), or ensure tsc builds the
project before start and keep the import pointing to the compiled .js file;
locate the import statement for FilterStoreProductsDTO in store.routes.js and
apply one of these three fixes so the module can be resolved at runtime.
| sortBy: z.preprocess( | ||
| emptyToUndefined, | ||
| z | ||
| .enum(["created_at", "price", "name"], { | ||
| error: "sortBy debe ser created_at, price o name" | ||
| }) | ||
| .optional() | ||
| .default("created_at") | ||
| ), | ||
|
|
||
| sortOrder: z.preprocess( | ||
| emptyToUndefined, | ||
| z | ||
| .enum(["asc", "desc"], { | ||
| error: "sortOrder debe ser asc o desc" | ||
| }) | ||
| .optional() | ||
| .default("desc") | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Zod 4 z.enum error customization syntax and parameters
💡 Result:
In Zod 4, error customization for z.enum uses the unified error parameter, replacing the Zod 3 errorMap. The error param accepts either a static string (applied to all errors from the schema) or a function that receives an issue object (iss) and returns a string or {message: string}. Syntax: - Static string: z.enum(["a", "b"], "Must be a or b!") - Function (error map): z.enum(["a", "b"], { error: (iss) => "Custom message" }) The function receives iss with properties like code (e.g., "invalid_type", "invalid_value" for enum), input, expected. Return undefined to fall back to default. Example for z.enum: const StatusSchema = z.enum(["draft", "published"], { error: (issue) => { if (issue.code === "invalid_value") { return "Status must be 'draft' or 'published'"; } return "Invalid status"; } }); This applies to all issues from the z.enum schema (invalid type, invalid enum value). For refinements on the enum, use separate error params. Zod 3 used errorMap: (issue, ctx) => ({message: string}); Zod 4 simplifies to error: (issue) => string | {message: string} | undefined, dropping ctx and specific params like invalid_type_error. Precedence: schema-level > per-parse > global > locale.
Citations:
- 1: https://zod.dev/error-customization
- 2: https://v4.zod.dev/error-customization
- 3: https://v4.zod.dev/v4?id=simplified-error-customization
- 4: https://v4.zod.dev/v4/changelog?id=improves-enum-support
- 5: https://v4.zod.dev/error-customization?id=global-error-customization
Corregir la sintaxis de personalización de errores en z.enum() para Zod 4.
La sintaxis { error: "mensaje" } no es válida en Zod 4. Debe ser o bien una cadena de texto como segundo argumento: z.enum([...], "mensaje"), o una función dentro del objeto: { error: (iss) => "mensaje" }. Esto aplica a los campos sortBy (línea 57) y sortOrder (línea 67).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/global/dtos/commerce/filter-store-products.dto.ts` around lines
53 - 71, Los validators `sortBy` y `sortOrder` usan la forma inválida `{ error:
"..." }` en z.enum; actualiza ambos para usar la API correcta de Zod 4 — por
ejemplo reemplaza `z.enum([...], { error: "mensaje" })` por `z.enum([...],
"mensaje")` (o alternadamente `{ error: (issues) => "mensaje" }`), manteniendo
la envoltura con `z.preprocess(emptyToUndefined, ...)`, las llamadas a `z.enum`
y las propiedades `.optional().default(...)` intactas para `sortBy` y
`sortOrder`.
1 participant
Cambio JS a TS
Summary by CodeRabbit
Correcciones de Errores
Refactorización
Pruebas