feat: [TESIS-107] settle the response convention and wrap the last bare list

[TomasMartin2004]

Ticket de Jira

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

---

Descripción

Fija la convención de respuesta de la API y corrige el único endpoint que no la cumplía.

La regla, en una línea: una colección viaja envuelta en data —más meta si pagina—, un recurso solo viaja pelado, y un error es siempre { "error": "..." }. Queda escrita en ADR-015 y en docs/guidelines/architecture.md §4.0.

Por qué importaba. La API respondía con cuatro formas porque cada card eligió la suya y nadie la escribió. El costo no lo pagaba el backend: el frontend ya lo tenía anotado como trampa en features/inventory/api.ts — «index envuelve en { data: [...] }, pero show y update devuelven el objeto pelado». Un comentario así es la señal de que la regla no existe; si existiera, no haría falta recordarla endpoint por endpoint.

Qué cambia en el código: un solo endpoint. GET /integrations devolvía un array en la raíz. Ahora envuelve.

Ese no era un caso de simple inconsistencia: un array en la raíz no admite agregarle meta sin romper a quien lo consume. Si algún día ese listado pagina —y TESIS-108 lo evalúa— habría que romperlo igual, con más consumidores encima. Cerrarlo ahora es la precondición de esa card.

⚖️ La decisión que pedía la card

La card planteaba dos opciones y recomendaba la A (envolver también los recursos solos). Se tomó la B, y el ADR deja escrito por qué, con el número que lo justifica:

Son diez lugares del frontend, en cuatro archivos de frontera, y dos de esos archivos son los que TESIS-58 y TESIS-61 están editando en ramas vivas —las dos ya en conflicto—. Cambiar el contrato debajo de dos PRs abiertos, para ganar uniformidad en endpoints que hoy nadie confunde, no se paga.

La puerta queda abierta y el ADR lo dice: pasar de esta convención a la A es aditivo del lado del backend, y el día que se haga este ADR se reemplaza en vez de discutirse de nuevo.

🔗 Va con su lado del frontend

proyectoFinalFRLP/proyecto-web#50, que lee el listado desde el envelope y borra los tres comentarios que describían la inconsistencia. Es un cambio que rompe: mergear primero éste y después el del front, o el panel se queda sin integraciones el rato que pase entre uno y otro.

🛠️ Cambios

• integrations#index envuelve en { data: ... } (render_as_hash en vez de render)
• ADR-015 nuevo, con la regla, las alternativas y sus motivos
• docs/guidelines/architecture.md §4.0 — la regla donde se busca el flujo de un request
• integrations_spec.rb: un ejemplo que fija el envelope, y los dos que leían el array pelado pasan por un helper
• api_contract_spec.rb (TESIS-90): el ejemplo del array pelado pasa a exigir el envelope, y la cabecera deja de describir cuatro formas para enunciar la regla

🧪 Cómo probarlo

Precondición: sesión iniciada con un usuario del tenant norte.

1. GET /api/v1/integrations → { "data": [ ... ] }. Antes: [ ... ].
2. GET /api/v1/warehouses y GET /api/v1/products → sin cambios.
3. GET /api/v1/products/:id → sigue pelado, que es la regla para un recurso solo.
4. Cualquier error → { "error": "..." }.

Verificación: bundle exec rspec (1252 ejemplos, 0 fallas) y bundle exec rubocop (256 archivos, sin ofensas).

---

Impacto y consideraciones

¿Introduce breaking changes?
Sí, uno: GET /api/v1/integrations cambia de forma. Va con su PR del front en el mismo momento; el orden está arriba.

¿Requiere nuevas variables de entorno?
No.

¿Afecta la arquitectura o genera un nuevo patrón?
Sí: fija una convención que hasta ahora no estaba escrita. ADR-015.

---

🤖 Generated with Claude Code

https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp

[Sanntinat]

Revisión — TESIS-107 (PR #86) · Convención de respuesta de la API

Revisión contra origin/master de proyecto-api, sobre TESIS-107-response-envelope (2709db0), con la card TESIS-107 (sin comentarios de alcance), su lado del front (proyecto-web#50) y la descripción del PR al lado.

Check	Resultado
Suite completa (corrida local, sobre la rama)	1252 examples, 0 failures en 2 de 3 corridas. La otra: 1 failure intermitente en poll_tracking_status_spec.rb:211, que no es de este PR (ver abajo)
RuboCop · Brakeman (local, sobre el merge de los 4 PRs de la API)	266 archivos, sin ofensas · 0 warnings
CI (GitHub Actions)	scan_ruby · lint · test · validate-pr-title: success
Rama vs master	Al día (0 commits atrás)
Merge con los otros PRs abiertos de la API (#85, #87, #88)	Sin conflictos; los cuatro juntos: 1403 examples, 0 failures

✅ El cambio de código es correcto y es el único que hacía falta

Revisé cada render json: de app/controllers. Salvo integrations#index, todas las colecciones ya iban en { data }: productos, depósitos, mapeos, eventos fallidos, envíos, órdenes, transferencias, cotizaciones, provincias y categorías. Todos los Serializer.render(...) restantes son de un recurso solo. Así que la afirmación de que «el único endpoint que hubo que cambiar fue integrations#index» es cierta, y el cambio (render_as_hash dentro de { data: }) es el mínimo.

✅ La decisión B está bien tomada y bien escrita

Ir contra la recomendación de la card es legítimo si el motivo queda escrito, y el ADR lo escribe: alternativas, costo, y el camino de vuelta (envolver también los recursos es aditivo del lado del backend). Coincido con que /products contra /products/:id ya separa colección de recurso en la URL.

🟡 La regla dice «cualquier error → { "error": "..." }», y hay un endpoint que no la cumple

POST /api/v1/auth/register responde los errores con otra clave. Lo verifiqué con una sonda de request spec:

POST /auth/register (datos inválidos)     -> 422 {"errors":["Email is invalid","Password is too short (minimum is 6 characters)"]}
POST /auth/register (tenant desconocido)  -> 422 {"errors":["Unable to complete registration"]}

Es errors en plural y con un array, en registrations_controller.rb:19 y :35. El segundo hasta tiene un comentario que explica por qué usa esa forma: «para que el frontend no distinga dos formatos». Justo lo contrario de lo que fija el ADR.

El primer criterio de la card es «una sola regla, sin excepciones sin justificar», y el ADR escribe la regla de los errores en términos absolutos. Hay dos salidas, y cualquiera sirve:

• Alinearlo ({ error: e.record.errors.full_messages.to_sentence }) y agregar el endpoint al bloque de errores de api_contract_spec.rb. El front no usa este endpoint (lo busqué en src/), así que no rompe a nadie.
• Escribir la excepción en el ADR, con su motivo.

Además, OptimisticLocking responde el 409 con { error, current_version }. Eso es una extensión y no otra forma, pero la regla lo tendría que admitir («error siempre está; puede venir acompañado de datos para recuperarse»).

🟡 El spec de contrato no hace lo que el ADR dice que hace

El ADR y architecture.md §4.0 afirman: «api_contract_spec.rb fija las tres formas, así que un endpoint nuevo que invente una cuarta rompe la suite». No es así. El bloque the shape of the envelope, endpoint by endpoint prueba cinco endpoints puntuales: productos, depósitos, un producto, integraciones y un 404. Un endpoint nuevo con otra forma no pasa por ninguno de ellos, así que la suite sigue verde. El registro de arriba es la prueba: hoy responde una cuarta forma y nada falla.

No pido un spec que recorra todas las rutas (sería otra card). Pido que el texto diga lo que el spec protege: «fija la forma de estos endpoints; uno nuevo tiene que sumarse acá».

🟡 El motivo principal del ADR venció hoy

El argumento para descartar la opción A es que dos de los archivos afectados «son los que TESIS-58 y TESIS-61 están editando en ramas vivas». Las dos se mergearon hoy (web#47 y web#48). La decisión se puede sostener igual, por el costo de tocar diez lugares del front y porque la URL ya distingue colección de recurso, pero el ADR queda apoyado en un hecho que ya no es cierto. Conviene reescribir ese párrafo con el motivo que dura. Si al releerlo la A ahora conviene, es el momento: todavía no entró nada.

Aparte: un spec intermitente que no es de este PR

En una de tres corridas falló Shipments::PollTrackingStatus … logs the element that matches no shipment of the query (poll_tracking_status_spec.rb:211). Este PR no toca esa zona. Es el spec intermitente que api#88 (TESIS-93) cierra con un order(:id), y los números están en esa revisión.

⚠️ Orden de merge

Es un cambio que rompe: este PR y web#50 tienen que entrar juntos, éste primero. #87 (TESIS-108) está apilado encima.

Los criterios de la card

• [~] La convención está escrita y es una sola regla, sin excepciones sin justificar. Está escrita, pero el registro es una excepción sin justificar.
• [~] Todos los endpoints de api/v1 la cumplen, integrations incluido. Todas las colecciones sí. Los errores del registro, no.
• El frontend consume la forma nueva y no quedan comentarios describiendo inconsistencias (en web#50).
• Hay specs que fallan si alguien rompe el envelope de los endpoints cubiertos. No de uno nuevo (ver arriba).

Veredicto

REQUEST CHANGES.

El código está bien y la decisión está bien argumentada. Lo que pido es que el ADR, que es el entregable principal de la card, diga la verdad sobre el estado de la API: alinear o documentar el registro, corregir lo que se afirma del spec de contrato y reescribir el motivo que venció. Son cambios chicos, casi todos de texto.

[TomasMartin2004]

Respuesta — TESIS-107 (review de Santiago, 24/sep)

Commits en TESIS-107-response-envelope: ad7db50 y 479afb6. Los tres puntos eran del entregable principal de la card —el ADR— y los tres estaban mal. Resueltos.

🟡 El registro respondía la cuarta forma

Reproducido: {"errors":["Email is invalid", ...]}, plural y array, en los dos caminos que fallan. Y con un comentario que decía que era para que el front no distinguiera dos formatos, que es exactamente lo contrario de lo que terminaba haciendo.

Elegí alinearlo, no documentar la excepción: el front no usa el endpoint (lo verifiqué en src/), así que no rompe a nadie, y una excepción en el ADR del día uno envejece mal.

render json: { error: e.record.errors.full_messages.to_sentence }, status: :unprocessable_content

Y lo metí en api_contract_spec.rb, con los dos caminos: datos inválidos y tenant desconocido. Era el agujero que hizo que nadie lo viera.

También quedó escrito lo del 409: error siempre está y puede venir acompañado —current_version en el locking optimista—; lo que no se admite es otra clave en su lugar.

🟡 El spec de contrato no hacía lo que yo decía que hacía

Cierto, y era la afirmación más peligrosa del ADR, porque daba por cubierto algo que no lo estaba —el propio registro era la prueba—. Ahora dice lo que protege:

fija las tres formas sobre los endpoints que enumera. La lista está escrita a mano: un endpoint nuevo con otra forma no rompe nada hasta que se lo agrega ahí.

🟡 El motivo que venció

Tenías razón: dos de los tres argumentos para descartar la opción A se mergearon ayer. Lo releí con eso en mente y la decisión se sostiene, pero por lo que queda: diez lugares del front en cuatro archivos de frontera, y la URL ya separa colección de recurso. Reescribí el párrafo con ese motivo y dejé una nota de que el otro ya no corre —el ADR no debería esconder que su argumento cambió—.

Sobre el spec intermitente

Es el mismo que arregla #88 con el order(:id). A mí me falló dos veces durante TESIS-93, en ejemplos distintos; a vos, una de tres. Va por ese PR.

Verificación: CI=true rspec → 1254 ejemplos, 0 fallas; RuboCop limpio en 256 archivos.

Orden de merge: sigue siendo este + web#50 juntos, éste primero; después #87, que ya trae este merge en su base; después #85.

🤖 Generated with Claude Code

https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp

[Sanntinat]

Re-revisión — TESIS-107 (PR #86) · Convención de respuesta de la API

Segunda vuelta sobre TESIS-107-response-envelope (479afb6), contra origin/master de proyecto-api. La primera revisión pidió cambios por tres cosas del ADR-015: el registro respondía otra forma de error, el spec de contrato no cubría lo que el ADR decía y uno de los motivos había vencido.

Check	Resultado
CI (GitHub Actions)	lint · scan_ruby · test · validate-pr-title: success
Rama vs master	2 commits atrás (TESIS-82 y TESIS-129). Mergea limpio
Suite sobre el merge con master (local)	1343 examples, 0 failures · RuboCop 269 archivos sin ofensas
Conflictos con otros PRs	Con #87, en el ADR-015: #87 mergeó esta rama un minuto antes de 479afb6 (ver abajo)

🟡 → ✅ El registro ya responde como el resto

Los dos caminos que fallan responden { "error": "..." }: los datos inválidos (los mensajes unidos en una oración) y el tenant desconocido. Alinearlo en lugar de documentar la excepción es la decisión correcta, y quedó escrito por qué se pudo: el front no consume el endpoint.

El merge con master no rompe nada. TESIS-82 reescribió el registro (ahora responde 202 con { status: 'pending_approval' } y un límite de intentos). Lo revisé sobre el resultado del merge: el 202 es un objeto, el 429 de AttemptLimit es { error } y los dos errores de este PR quedan intactos. Busqué en los controllers de api/ cualquier render json: de error sin la clave error y no quedó ninguno.

Los ejemplos nuevos del contrato tienen dientes. Volví los dos render del registro a errors: [...]:

api_contract_spec.rb + auth_spec.rb -> 59 examples, 2 failures
  reports a failed registration with the same single error key
  reports an unknown tenant with the same single error key

🟡 → ✅ El spec de contrato dice lo que cubre

«Fija las tres formas sobre los endpoints que enumera», con la aclaración de que la lista es a mano y de que sumar un endpoint nuevo ahí es parte de agregarlo. Es exactamente lo que faltaba, sin prometer de más.

🟡 → ✅ El motivo que venció

El párrafo se sostiene ahora con lo que sigue vigente (diez lugares del front, y la URL ya separa colección de recurso). Además deja una nota de que el otro motivo dejó de correr. Me parece la forma honesta de hacerlo.

⚠️ Para #87: le falta el último commit de esta rama

#87 trae esta rama mergeada en 8594b2a, pero de un momento anterior a 479afb6. Si entra este PR y después #87, choca el ADR-015 en el párrafo de meta. Lo resolví quedándome con la versión de #87, que es la que agrega la tabla de excepciones, y la suite pasa. Con volver a mergear esta rama en #87 antes de su turno, desaparece.

Los criterios de la card

• Hay una convención escrita y no tiene excepciones sin nombrar.
• Ninguna colección devuelve un array en la raíz.
• Hay specs que fallan si alguien rompe el envelope de los endpoints cubiertos, registro incluido. Verificado.

Veredicto

APPROVE.

Los tres puntos están resueltos, y el del registro con un spec que lo protege. Sigue valiendo el orden que anunciaste: éste junto con web#50, éste primero.