fix: [TESIS-133] answer 400 instead of 500 to a malformed body

[TomasMartin2004]

Ticket de Jira

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

---

Descripción

params.require(:x) devuelve lo que haya bajo la clave. Si eso es un String, el permit de la línea siguiente levanta NoMethodError y la API responde 500 a un body como {"product": "x"}. En TESIS-82 se arregló sólo en WarehousesController pasando a params.expect; el mismo patrón seguía vivo en productos, transferencias de stock, mapeos y órdenes.

Dos formas de resolverlo, según lo que tenga el body:

• Transferencias y mapeos pasan a params.expect. No tienen partes anidadas que se armen a mano, así que alcanza.
• Productos y órdenes conservan permit, detrás de un guard nuevo, body_of, en ApplicationController. expect considera faltante un filtrado que queda vacío, y un PUT que sólo manda stocks o items es un request válido: esas dos partes se recorren línea por línea, cada una con sus propios 422 y su mensaje, así que nunca entran en la lista de permit y el filtrado vuelve vacío. body_of hace el mismo chequeo de forma sin filtrar. (Esto es lo que se desvía del alcance escrito en la card, que pedía expect en los cuatro; el comportamiento que pide es el mismo.)

El envoltorio order respondía 422 con un RecordNotSaved escrito a mano. Su forma es contrato y no negocio, así que ahora es 400 como el resto.

ActionController::ParameterMissing se rescata una sola vez en ApplicationController, para que el 400 viaje como {"error": "..."} (ADR-015) y no con el cuerpo propio de Rails. Los tres controllers que lo declaraban por su cuenta —órdenes, envíos y cotizaciones— sacan su copia.

Un company_id en el body se sigue descartando en silencio: expect filtra las claves que no permite igual que permit. Los specs de TESIS-97 lo siguen probando y pasan.

• products, stock_transfers, product_mappings y orders dejan de responder 500 a un envoltorio que no es un objeto.
• body_of en ApplicationController + rescate único de ParameterMissing.
• No queda ningún rubocop:disable de Rails/StrongParametersExpect en app/.
• 14 specs de request nuevos (String y Array por endpoint de alta y de modificación).
• El ejemplo de CLAUDE.md se actualiza al patrón nuevo.

---

Evidencia visual

N/A

---

Cómo probar

1. bundle exec rspec — 1558 ejemplos, 0 fallas.
2. Con sesión iniciada, POST /api/v1/products con {"product": "Teclado"} → 400 {"error": "param is missing or the value is empty or invalid: product"}. En master es 500.
3. Lo mismo con {"product": ["..."]}, y con stock_transfer, product_mapping y order (alta y PUT /orders/:id).
4. PUT /api/v1/products/:id con {"product": {"stocks": [...]}} —sin ningún campo del producto— sigue respondiendo 200: es el caso que descarta usar expect acá.
5. POST /api/v1/products con company_id en el body sigue creando el producto en la empresa del token.

---

Impacto y consideraciones

¿Introduce breaking changes?
Sí, en los códigos de error, no en el camino feliz. Un body mal formado pasa de 500 a 400, que es lo que la card pide. El envoltorio order que no es un objeto pasa de 422 a 400: si el frontend distinguía por status, ahí cambia. Los 422 de las líneas (items, stocks) y del negocio quedan igual.

¿Requiere nuevas variables de entorno?
No

¿Afecta la arquitectura o genera un nuevo patrón?
Sí, menor: body_of es el lugar donde se valida la forma de un envoltorio que después se recorre a mano. No amerita ADR nuevo; el contrato del cuerpo de error ya está en ADR-015.

🤖 Generated with Claude Code

https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp

[Sanntinat]

Revisión — TESIS-133 (PR #93) · Responder 400 y no 500 a un body mal formado

Revisión de TESIS-133-migrate-strong-params-to-expect (899dd7c), contra origin/master de proyecto-api.

Check	Resultado
CI (GitHub Actions)	lint, scan_ruby, test y validate-pr-title en verde
Rama vs master	Al día. Mergea limpio
Suite local (sobre #93 + #94 mergeados)	1567 ejemplos · 0 fallas · cobertura 99,89 % línea / 99,81 % rama
RuboCop / Brakeman	281 archivos sin ofensas · 0 advertencias
Conflictos con otros PRs	Con #94 (TESIS-136), ninguno: tocan archivos distintos, y la suite de arriba es la de los dos juntos

✅ La desviación de la card está bien fundada, y la comprobé

La card pedía params.expect en los cuatro controllers, y el PR lo usa sólo en transferencias y mapeos. Para productos y órdenes usa un guard, body_of. Verifiqué el motivo con un spec descartable contra la rama:

• ActionController::Parameters.new(product: { stocks: [...] }).expect(product: %i[sku name]) levanta ParameterMissing. expect toma como faltante un filtrado que queda vacío.
• PUT /products/:id con {"product": {"stocks": [...]}} y ningún campo del producto responde 200 con body_of. Con expect habría sido 400.

El chequeo que evitaba el 500 (que el envoltorio sea un objeto) es el mismo en los dos caminos, así que el comportamiento que pide la card se cumple igual.

✅ Lo que dice el cuerpo del PR, verificado contra la rama

• POST /products con {"product": "Teclado"} → 400 {"error": "param is missing or the value is empty or invalid: product"}, con la forma de ADR-015. Con {"product": {}}, también 400.
• expect descarta en silencio un company_id que no lista (lo probé directo sobre Parameters), y los specs de TESIS-97 pasan.
• No queda ningún rubocop:disable de Rails/StrongParametersExpect en app/. El único require que sobrevive es el de credentials, que la card deja afuera.
• Las dos iteraciones por línea (items en órdenes, stocks en productos) ya chequeaban respond_to?(:permit) antes de permit, así que el mismo 500 no reaparece un nivel más abajo.

✅ El rescate único de ParameterMissing arregla de paso un endpoint que el PR no nombra

ShipmentQuotesController levanta ParameterMissing cuando falta el depósito de origen, pero nunca lo había rescatado. En master, ese 400 salía con el cuerpo propio de Rails. Con el rescate en ApplicationController, POST /orders/:id/quotes sin origen ahora responde {"error": "... origin_warehouse_id"}, como el resto de la API (lo comprobé en la rama).

✅ El cambio de contrato del envoltorio order no afecta al front

El envoltorio order que no es un objeto pasa de 422 a 400. Busqué en proyecto-web y ningún código distingue un 422 en órdenes: los status con tratamiento propio son 404, 409 y 412. Además, el front nunca manda un order que no sea un objeto.

✅ Los specs tienen dientes

Rompí dos cosas a la vez: saqué el chequeo is_a?(ActionController::Parameters) de body_of y devolví transferencias a require + permit. Fallaron 10 ejemplos, exactamente los nuevos de productos (alta y modificación), órdenes (alta y modificación) y transferencias, y ningún spec anterior. Los de mapeos no los rompí, porque es el mismo expect que transferencias.

🟡 El ejemplo de CLAUDE.md contradice al controller que nombra

El PR actualiza el ejemplo de la sección «Controller» a params.expect(product: %i[sku name stock]), con el comentario de TESIS-133. Pero el archivo del ejemplo es app/controllers/api/v1/products_controller.rb, justo el controller que este PR no pasa a expect, porque romper el PUT que sólo manda stocks es lo que se quiso evitar.

CLAUDE.md es lo que leen primero tanto las personas como el agente. Así como está, invita a «terminar la migración» de productos y deshacer la decisión del PR. Alcanza con una línea en el comentario del ejemplo: cuando el body tiene partes que se recorren a mano (como stocks o items), el envoltorio se valida con body_of y se hace permit sobre eso. No bloquea el merge.

⚪ Menor

El cuerpo del PR dice «14 specs de request nuevos». En el diff hay 13 it nuevos y uno reemplazado (el rejects when order is not an object que esperaba 422). No cambia nada; lo anoto para que la cuenta coincida.

Los criterios de la card

• Para cada endpoint de alta y modificación de los cuatro recursos, un String o un Array responde 400 y no 500, con su spec de request. Mapeos y transferencias sólo tienen alta (only: %i[index create destroy] e %i[index create]).
• Un company_id en el body se sigue ignorando.
• No queda ningún rubocop:disable de Rails/StrongParametersExpect en app/.
• RuboCop y RSpec en verde.

Veredicto

APPROVE.

La desviación del alcance está justificada y probada, el rescate único mejora un endpoint más de los que declara, y cada caso nuevo tiene un test que falla si se lo saca. Lo de CLAUDE.md conviene corregirlo, pero puede ir en este PR o después.

[TomasMartin2004]

Corregido lo de CLAUDE.md en 8a8d4a1, buen ojo: el ejemplo nombraba products_controller.rb, que es justo el que NO pasa a expect, y tal como estaba invitaba a «terminar la migración» y deshacer el motivo por el que quedó afuera. El comentario ahora dice cuándo el envoltorio va por body_of en vez de expect.

Lo de los «14 specs» es cierto: son 13 nuevos y uno reemplazado. Queda acá y no lo edito en el cuerpo para no pisar lo que ya revisaste.

Gracias por verificar la semántica del filtrado vacío por tu cuenta en vez de creerme, y por encontrar el 400 de ShipmentQuotesController que el PR arregla sin nombrarlo — ese no lo tenía visto.