Skip to main content

Notas técnicas y consideraciones

Hallazgos observados al revisar el código. No son incidencias abiertas: son puntos a tener presentes antes de tocar el proyecto.

Control de acceso​

  • El proxy comprueba solo el primer rol. proxy.ts llama a hasAccess(roles[0], pathname), mientras que el sidebar (sidebar-links.tsx) recorre todos los roles del usuario. Un usuario con ["ATC", "ADMIN"] vería en el menú los enlaces de administración (porque el sidebar los resuelve por unión de roles) pero el proxy lo redirigiría a /returns al entrar, ya que evalúa únicamente ATC. Conviene unificar ambos criterios.
  • Los roles se leen del ID token sin verificar la firma. getUserRoles hace Buffer.from(idToken.split(".")[1], "base64") y parsea el payload. El token viene de una sesión ya validada por el SDK de Auth0, así que en la práctica es seguro, pero es un patrón que invita a reutilizarse en contextos donde no lo sería.
  • /api/token expone el access token de robo-api al navegador y no tiene ningún consumidor en el código. Cualquier sesión autenticada puede pedirlo con un GET, lo que amplía el impacto de un XSS: pasa de "sesión con cookie httpOnly" a "bearer token de robo-api en manos de JavaScript". Si no se usa, lo razonable es borrar la ruta.
  • El rol CUSTOMER no tiene entrada en ROLE_ROUTES. Un usuario cuyo único rol sea CUSTOMER pasa la comprobación de sesión pero no tiene acceso a ninguna sección; acaba en /unauthorized o redirigido a /returns sin poder ver nada. Es coherente con el diseño, pero conviene saberlo al diagnosticar accesos.

Seguridad y datos personales​

  • PDF_SIGNATURE_SECRET compartido con el portal del cliente. Es una dependencia acoplada entre dos despliegues independientes: rotarla en uno rompe los enlaces del otro. Ver Integraciones.
  • PDF_SIGNATURE_SECRET sin validación. Si falta, crypto.createHmac("sha256", "") no falla: se firman URLs con secreto vacío y todo parece funcionar (el mismo patrón que en el portal del cliente). lib/marketingCloud.tsx sí valida sus variables al arrancar.
  • PII en los logs. /api/trigger-journey hace un console.log del objeto completo antes de enviarlo, incluyendo ContactKey y el EmailAddress del cliente. Esos logs acaban en los logs del pod.
  • Datos de pedido en localStorage. El alta manual persiste refundProcess (pedido, email, artículos) en localStorage, que sobrevive al cierre de la pestaña. En un equipo compartido de tienda queda hasta completar o reiniciar el alta. El portal del cliente usa sessionStorage para lo equivalente.
  • HTML de terceros renderizado sin sanear. Las descripciones e instrucciones de los métodos se editan aquí con un WYSIWYG que permite HTML crudo y se renderizan con dangerouslySetInnerHTML en los dos portales. Es un vector de XSS auto-infligido: quien administre métodos puede inyectar HTML arbitrario en el portal público.
  • El dominio returns.hawkersco.com está hardcodeado en pdf-button.tsx y en lib/journeys.ts, así que un back-office de desarrollo genera enlaces a producción.

Funcionalidad incompleta o inalcanzable​

  • El journey REFUNDED no se dispara nunca. Existen generateJourneyDataForRefund, la sobrecarga de prepareJourneyPayload y la variable MC_EVENT_KEY_REFUNDED, pero useTriggerJourney solo se invoca con "CREATED" y "APPROVED". El email de reembolso está preparado y sin conectar.
  • Borrar una devolución no está expuesto. actions/returns/delete.ts y /api/returns/delete existen, y la columna de acciones pasa deleteUrl, pero no pasa showDelete, así que la opción no se renderiza. Sí funciona para métodos y motivos, que sí lo pasan.
  • El submenú de estados de Actions es UI muerta. El bloque showStatus pinta tres opciones (Reembolsado, Aprobado, Cancelado) sin ningún onClick, y el único sitio que pasa la prop lo hace con showStatus={false}.
  • DialogDeleteMethod se usa también para borrar motivos. El nombre y su ubicación (components/methods/) no reflejan que es el diálogo de borrado genérico del Actions compartido.

Reglas de negocio codificadas​

  • SKU de gastos de envío hardcodeado. itemIsShippingCost() compara contra "S00233" en lib/utils.ts, igual que en el portal del cliente. Aparece en el cálculo del precio final, en la matriz de editabilidad, en el alta manual, en los payloads de journey y en el PDF.
  • Almacén de Grecia hardcodeado. GREECE_COUNTRY_CODE = "GR" y GREECE_WAREHOUSE_ID = 61: los pedidos griegos se asignan siempre al almacén 61, ignorando el del pedido. Si ese almacén cambia de id, hay que tocar código.
  • Identificadores de estado numéricos en el código. calculateFinalPrice excluye las líneas con id_return_line_status !== 5 (CANCELLED) y getValidItemsToSendApprovalMail filtra por === 4 (APPROVED), en lugar de resolverlos por nombre contra el catálogo como hace el resto del proyecto. Si esos ids cambiaran en robo-api, los cálculos fallarían en silencio.
  • formatCurrency admite hasta 3 decimales por defecto (maximumFractionDigits: 3), lo que puede producir importes con tres decimales en tablas y CSV.

Patrones frágiles​

  • getFilteredLineStatuses llama a un hook sin ser un hook. La función (en lib/return-utils.ts) hace useContext(RoleContext) en su interior pese a no llamarse use*. Funciona porque se invoca de forma incondicional durante el render de ReturnContent, pero infringe las reglas de los hooks: llamarla dentro de una condición o de un bucle rompería el render.
  • Inconsistencia en los nombres del filtro de fechas. parseFilters emite date_from/date_to cuando hay rango completo y dateFrom/dateTo (camelCase) cuando falta una de las dos, con undefined interpolado en la URL. La segunda rama es casi con seguridad un residuo: robo-api solo entiende una de las dos convenciones.
  • La exportación a CSV no tiene tope. El bucle while (!last) recorre el listado completo en páginas de 100. Sin filtros, una exportación puede tardar mucho y llegar a agotar el tiempo de la petición; el catch genérico devolvería un 500 con "Error al exportar CSV" sin distinguir la causa.
  • Doble guardado en el journey de aprobación. Enviar el email de aprobación implica un PUT extra para marcar is_mail_approval_sent. Si ese segundo guardado falla, el email ya se envió y el flag no queda escrito, por lo que en la siguiente edición se reenviaría.
  • editReturn(data: any) es la única acción sin tipar, precisamente la del objeto más complejo del dominio.

Calidad y proceso​

  • No hay tests en el repositorio (ni unitarios ni E2E). El markup expone atributos data-test-id en los puntos clave, pensados para automatización externa; conviene mantenerlos al refactorizar.
  • @typescript-eslint/no-unused-vars está desactivado en eslint.config.mjs, así que imports y variables muertas no se detectan.
  • El pipeline no ejecuta lint ni tests. Un error de TypeScript sí detiene el build de la imagen, pero nada más.
  • next.config.ts conserva comentados los bloques eslint.ignoreDuringBuilds y typescript.ignoreBuildErrors. Están desactivados, que es lo correcto; se dejan como recordatorio de que no deben activarse.
  • La imagen no usa output: "standalone", así que la etapa runner copia node_modules completo. Activarlo reduciría bastante el tamaño de la imagen.
  • npm ci --legacy-peer-deps es necesario para instalar: hay conflictos de peer dependencies sin resolver en el árbol actual.
  • El README.md es el de create-next-app, sin adaptar: habla de Vercel y de localhost:3000 cuando el proyecto corre en el puerto 80 y se despliega en GKE.
  • No hay .env.example. Las 19 variables hay que deducirlas del código. .env.local está correctamente ignorado por git.
  • El despliegue tiene corte de servicio: el pipeline borra el Deployment y espera a que mueran los pods (estrategia Recreate, 1 réplica).
  • La UI está en español fijo. constants/dictionary.ts es un diccionario plano sin framework de i18n; internacionalizar el back-office implicaría reescribir esa capa. No confundirlo con el contenido que administra, que sí es multiidioma.