fix: [TESIS-153] validate the warehouse of each product stock row

[LauAubert]

Ticket de Jira

https://proyectofinalfrlp.atlassian.net/browse/TESIS-153

ID provisorio: se reemplaza por la clave real al cargar la card en Jira. Card: cards/018.md. Sale de una auditoría de código (TESIS-89).

---

Descripción

Products::Concerns::WarehouseValidation comparaba los warehouse_id de stocks[] crudos contra los de la base (owned.sort == warehouse_ids.sort). Dos fallas salían de ahí:

• 500: una fila sin warehouse_id dejaba [id, nil], y el sort levantaba ArgumentError (comparison of Integer with nil failed), que nadie rescata. Le pasaba a POST y a PUT /api/v1/products.
• 422 falso: si los ids llegaban como texto ("5"), [5] == ["5"] daba falso y la API respondía «One or more warehouses do not belong to this company» por un depósito que sí era de la empresa.

Orders::ReplaceOrderLines#validate_new_warehouses! tenía el mismo 422 falso cuando dos líneas nuevas nombraban el mismo depósito como "1" y como 1 (contaban como dos).

• WarehouseValidation normaliza cada id a entero (acepta número o dígitos) y responde 422 nombrando la fila cuando falta o no es válido: stocks[1]: warehouse_id must be a positive integer.
• La pertenencia se verifica por cantidad sobre los ids únicos (Warehouse.where(id: ids).count == ids.size); el scope de CompanyScoped sigue acotando al tenant y el mensaje para un depósito ajeno sigue siendo el genérico.
• ReplaceOrderLines#validate_new_warehouses! compara enteros.
• Specs de request: fila sin depósito (422 con el mensaje), id en texto (200 y stock guardado), depósito de otro tenant (mismo mensaje genérico), alta con fila sin depósito, y la edición de orden con el mismo depósito en texto y en número.

---

Evidencia visual

N/A

---

Cómo probar

1. PUT /api/v1/products/:id con { product: { name: 'X', stocks: [{ warehouse_id: W, quantity: 1 }, { quantity: 3 }] } } → 422 stocks[1]: warehouse_id must be a positive integer. En master, 500.
2. Con stocks: [{ warehouse_id: "W", quantity: 4 }] (texto) → 200 y el stock queda en 4. En master, 422.
3. Con un depósito de otra empresa → 422 «One or more warehouses do not belong to this company», como siempre.

Verificación: bundle exec rspec (1572 ejemplos, 0 fallas, cobertura 99.89% de línea), rubocop y brakeman limpios.

Dientes: con la versión de master del concern, fallan 2 de los 4 specs nuevos de productos.

---

Impacto y consideraciones

¿Introduce breaking changes?
No. Un request que antes fallaba con 500 o con un 422 equivocado ahora responde lo correcto.

¿Requiere nuevas variables de entorno?
No

¿Afecta la arquitectura o genera un nuevo patrón?
No. Mismo criterio de positive_integer que ya usaba ReplaceOrderLines.

🤖 Generated with Claude Code

[TomasMartin2004]

Revisado. Lo veo bien para implementar.

Las dos fallas son reales y las dos se alcanzan desde la UI:

• una fila de stocks sin warehouse_id dejaba [id, nil] y el sort levantaba ArgumentError, o sea 500 en POST y en PUT /api/v1/products;
• un id que llega como texto ("5") no coincidía con el entero de la base y la API respondía un 422 de «One or more warehouses do not belong to this company» que era mentira. Ese es el peor de los dos: acusa a un depósito propio de ser de otra empresa.

Lo que verifiqué

• El cambio de owned.sort == warehouse_ids.sort a Warehouse.where(id: ids).count == ids.size es equivalente y además no trae ids a memoria para después ordenarlos.
• warehouse_id_of! acepta Integer y String de sólo dígitos, y rechaza todo lo demás nombrando la fila (stocks[1]: warehouse_id must be a positive integer). Nombrarla está bien: es un dato del request del propio usuario, no filtra nada de otro tenant.
• En ReplaceOrderLines#validate_new_warehouses!, item[:warehouse_id].to_s.to_i convierte nil en 0, que no existe y cae igual en el 422. Es el mismo resultado que antes (Warehouse.where(id: [nil]) tampoco encontraba nada), y de todas formas validate_lines_without_warehouse! corre antes para las líneas que mueven stock. No cambia comportamiento.
• El spec cubre que un depósito de otra empresa sigue dando el 422 genérico. Era lo importante: al normalizar los ids no había que aflojar la validación de tenant, y no se aflojó.

Antes de mergear

scan_ruby en rojo es la corrida vieja del 02/10 (falla igual en toda la tanda y master está verde). Rebasá y vuelve a correr.

[TomasMartin2004]

Revisión del diff completo. El código es idéntico al que leí cuando los PRs estaban en draft —ningún commit nuevo—, así que lo que sigue es el veredicto formal.

✅ Aprobado

Las dos fallas son reales y las dos se alcanzan desde la UI:

• una fila de stocks sin warehouse_id dejaba [id, nil] y el sort levantaba ArgumentError: 500 en POST y en PUT /api/v1/products;
• un id que llega como texto ("5") no coincidía con el entero de la base y la API respondía un 422 de «One or more warehouses do not belong to this company» que era mentira. Ese es el peor de los dos: acusa a un depósito propio de ser de otra empresa.

Lo que verifiqué

• owned.sort == warehouse_ids.sort → Warehouse.where(id: ids).count == ids.size es equivalente y además no trae ids a memoria para ordenarlos.
• warehouse_id_of! acepta Integer y String de sólo dígitos y rechaza el resto nombrando la fila. Nombrarla está bien: es un dato del request del propio usuario, no filtra nada de otro tenant.
• En ReplaceOrderLines, item[:warehouse_id].to_s.to_i convierte nil en 0, que no existe y cae igual en el 422 — el mismo resultado que antes. Y validate_lines_without_warehouse! corre antes para las líneas que mueven stock. No cambia comportamiento.
• El spec cubre que un depósito de otra empresa sigue dando el 422 genérico. Era lo importante: al normalizar los ids no había que aflojar la validación de tenant, y no se aflojó.