From 2709db0ee7c2444f920933b41da2ae19952a18ae Mon Sep 17 00:00:00 2001 From: Tomas Martin Date: Wed, 23 Sep 2026 18:09:41 -0300 Subject: [PATCH 1/3] feat: [TESIS-107] settle the response convention and wrap the last bare list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The API answered in four shapes because every card picked its own and nobody wrote the rule down. The frontend already carried the symptom as a comment: "index wraps in { data }, but show and update return the object bare". A note like that is the tell that no rule exists. The rule is now one line, in ADR-015: a collection travels wrapped, a single resource travels bare, an error is always `{ "error": ... }`. Only `integrations#index` had to change — it returned an array at the root. That one mattered beyond consistency: an array at the root has nowhere to put `meta`, so that endpoint could never start paginating without breaking whoever reads it. TESIS-108 needs every collection to have that room. Wrapping single resources too was the tidier rule, and it is written down in the ADR as the alternative with the reason it was not taken: ten call sites across four frontier files in the frontend, two of which TESIS-58 and TESIS-61 are editing in live branches right now. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp --- .../api/v1/integrations_controller.rb | 12 +++- ...R-015-convencion-de-respuesta-de-la-api.md | 71 +++++++++++++++++++ docs/guidelines/architecture.md | 16 +++++ spec/requests/api/v1/api_contract_spec.rb | 20 +++--- spec/requests/api/v1/integrations_spec.rb | 18 ++++- 5 files changed, 123 insertions(+), 14 deletions(-) create mode 100644 docs/adr/ADR-015-convencion-de-respuesta-de-la-api.md diff --git a/app/controllers/api/v1/integrations_controller.rb b/app/controllers/api/v1/integrations_controller.rb index 8b84bd49..292245e1 100644 --- a/app/controllers/api/v1/integrations_controller.rb +++ b/app/controllers/api/v1/integrations_controller.rb @@ -5,11 +5,17 @@ module V1 class IntegrationsController < ApplicationController skip_after_action :verify_authorized, :verify_policy_scoped + # Envuelto en `data` como el resto de las colecciones (ADR-015). Era el + # único listado que devolvía un array pelado, y un array en la raíz no + # deja lugar para agregarle `meta` el día que pagine sin romper a quien + # lo consume. def index integrations = current_company.company_integrations.index_by(&:service_id) - render json: IntegrationStatusSerializer.render( - Service.order(:id), integrations_by_service_id: integrations - ) + render json: { + data: IntegrationStatusSerializer.render_as_hash( + Service.order(:id), integrations_by_service_id: integrations + ) + } end def update diff --git a/docs/adr/ADR-015-convencion-de-respuesta-de-la-api.md b/docs/adr/ADR-015-convencion-de-respuesta-de-la-api.md new file mode 100644 index 00000000..d3803853 --- /dev/null +++ b/docs/adr/ADR-015-convencion-de-respuesta-de-la-api.md @@ -0,0 +1,71 @@ +# ADR-015: Convención de respuesta de la API + +**Fecha:** 2026-09-23 +**Estado:** Aceptado + +--- + +## Contexto + +La API creció una card por vez y cada una eligió cómo devolver su respuesta. Nadie escribió la regla, así que no había una: había cuatro formas conviviendo. + +| Forma | Endpoints | +| --------------------------- | -------------------------------------------------------------------- | +| `{ "data": [...], "meta" }` | `GET /products`, `/orders`, `/shipments`, `/failed-events` | +| `{ "data": [...] }` | `/warehouses`, `/stock-transfers`, `/products/:id/mappings`, `/quotes` | +| `[...]` — array pelado | `GET /integrations` | +| objeto pelado | todos los `show`, `create` y `update` | + +El costo no lo paga el backend, lo paga el consumidor. El frontend ya lo tenía anotado como trampa en `src/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 por endpoint. + +Mientras hubo una sola pantalla consumiendo la API la inconsistencia era barata. Con el panel, el catálogo, órdenes, envíos y reportes leyendo de acá, cada pantalla nueva tiene que redescubrir de qué forma contesta cada endpoint. + +## Decisión + +**Una colección viaja envuelta. Un recurso solo viaja pelado.** + +``` +GET /api/v1/products → { "data": [ {...}, {...} ], "meta": { page, per_page, total } } +GET /api/v1/warehouses → { "data": [ {...}, {...} ] } +GET /api/v1/integrations → { "data": [ {...}, {...} ] } + +GET /api/v1/products/:id → { "id": 1, "sku": "...", ... } +POST /api/v1/products → { "id": 1, "sku": "...", ... } +PUT /api/v1/products/:id → { "id": 1, "sku": "...", ... } + +cualquier error → { "error": "..." } +``` + +`meta` aparece sólo si el listado pagina, y es siempre `page`, `per_page` y `total`, contando el scope **ya filtrado**. + +El único endpoint que hubo que cambiar fue `integrations#index`, que devolvía un array en la raíz. + +## Alternativas consideradas + +**Envolver también los recursos solos** (`{ "data": { ... } }` en `show`, `create` y `update`). Es la regla más simple de enunciar —una sola, sin excepciones— y deja lugar para agregarle `meta` a un recurso individual el día que haga falta. + +Se descartó por lo que costaba **ahora**, no por lo que vale: 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. Cambiar el contrato debajo de dos PRs abiertos, para ganar uniformidad en endpoints que hoy nadie confunde, no se paga. + +La puerta queda abierta: pasar de esta convención a la otra es aditivo del lado del backend —envolver lo que hoy va pelado— y el día que se haga, este ADR se reemplaza en vez de discutirse de nuevo. + +**Dejar la inconsistencia documentada** en vez de corregirla. Se descartó porque un array en la raíz no es sólo una forma distinta: no admite agregarle `meta` sin romper a quien lo consume. Si `integrations` pagina algún día —y TESIS-108 lo evalúa— habría que romperlo igual, con más consumidores encima. + +## Consecuencias + +**A favor** + +- La regla se enuncia en una línea y no tiene excepciones que justificar. +- Ninguna colección queda con un array en la raíz, así que cualquiera puede empezar a paginar sin romper su contrato. Es la precondición de TESIS-108. +- El comentario-trampa del frontend se borra: lo que explicaba ya no pasa. +- `spec/requests/api/v1/api_contract_spec.rb` (TESIS-90) fija las tres formas, así que un endpoint nuevo que invente una cuarta rompe la suite. + +**En contra** + +- El consumidor sigue teniendo que saber si lo que pidió es una colección o un recurso. No es gratis, pero es una distinción que ya existe en la URL: `/products` contra `/products/:id`. +- `GET /api/v1/integrations` cambia de forma. Es un cambio que rompe, y va con su lado del frontend en el mismo momento. + +## Referencias + +- TESIS-107 — la card que pedía definir la convención +- TESIS-108 — paginación consistente, que necesita que ninguna colección esté pelada +- TESIS-90 — `api_contract_spec.rb`, donde la regla queda fijada de forma ejecutable diff --git a/docs/guidelines/architecture.md b/docs/guidelines/architecture.md index cddf1550..644497bb 100644 --- a/docs/guidelines/architecture.md +++ b/docs/guidelines/architecture.md @@ -133,6 +133,22 @@ Serializer (Blueprinter) render json: ... ``` +### 4.0 Forma de la respuesta + +Una sola regla, fijada en [ADR-015](../adr/ADR-015-convencion-de-respuesta-de-la-api.md): **una colección viaja envuelta, un recurso solo viaja pelado.** + +``` +GET /api/v1/products → { "data": [ ... ], "meta": { page, per_page, total } } +GET /api/v1/warehouses → { "data": [ ... ] } +GET /api/v1/products/:id → { "id": 1, "sku": "...", ... } +POST /api/v1/products → { "id": 1, "sku": "...", ... } +cualquier error → { "error": "..." } +``` + +`meta` aparece sólo si el listado pagina, y cuenta el scope **ya filtrado**, no la tabla entera. + +Ninguna colección devuelve un array en la raíz: un array pelado no admite agregarle `meta` sin romper a quien lo consume. `spec/requests/api/v1/api_contract_spec.rb` fija las tres formas, así que un endpoint nuevo que invente una cuarta rompe la suite. + ### 4.1 Flujo de un webhook entrante Los eventos de las plataformas externas no siguen el flujo de arriba: no hay JWT, no hay usuario y el proveedor no espera detrás de la lógica de negocio. El gateway persiste y suelta la conexión; el procesamiento corre en un worker (ver [ADR-010](../adr/ADR-010-ingesta-de-ordenes-de-webhooks.md)). diff --git a/spec/requests/api/v1/api_contract_spec.rb b/spec/requests/api/v1/api_contract_spec.rb index e032d83c..17838ce5 100644 --- a/spec/requests/api/v1/api_contract_spec.rb +++ b/spec/requests/api/v1/api_contract_spec.rb @@ -104,13 +104,13 @@ def shipment # ───────────────────────────────────────────────────────── forma del sobre # - # Hoy la API responde con CUATRO formas distintas. No es una decisión: cada - # card eligió la suya y nadie la escribió. TESIS-107 las unifica. + # La regla la fija ADR-015 (TESIS-107) y es una sola: **una colección viaja + # envuelta en `data`** —más `meta` si pagina— y **un recurso solo viaja + # pelado**. Los errores, siempre `{ "error": "..." }`. # - # Se fijan igual, y a propósito: mientras la inconsistencia exista, el front - # tiene que saber cuál le toca a cada endpoint, y este archivo es el único - # lugar donde eso está dicho. Cuando entre TESIS-107, estos cuatro ejemplos - # son la lista de lo que hay que cambiar. + # Antes eran cuatro formas distintas, porque cada card eligió la suya y nadie + # la escribió. Estos ejemplos son lo que impide que vuelva a pasar: agregar + # un endpoint con otra forma tiene que romper acá. describe 'the shape of the envelope, endpoint by endpoint' do it 'wraps a paginated collection in data plus meta', :aggregate_failures do product @@ -136,11 +136,13 @@ def shipment expect(response.parsed_body.keys).to include('sku') end - # El único endpoint que devuelve un array pelado, sin objeto que lo envuelva. - it 'returns a bare array for the integrations listing' do + # Era el único que devolvía un array pelado. Un array en la raíz no admite + # `meta` sin romper a quien lo consume, así que ninguna colección puede + # quedar así. + it 'wraps the integrations listing too, with no exception' do get '/api/v1/integrations', headers: headers - expect(response.parsed_body).to be_an(Array) + expect(response.parsed_body.keys).to eq(['data']) end # Los errores sí son consistentes en toda la API, y conviene que siga así. diff --git a/spec/requests/api/v1/integrations_spec.rb b/spec/requests/api/v1/integrations_spec.rb index 0bd1ae20..7dc1c7cc 100644 --- a/spec/requests/api/v1/integrations_spec.rb +++ b/spec/requests/api/v1/integrations_spec.rb @@ -11,12 +11,26 @@ uri: 'https://api.mercadolibre.com', http_method: 'GET') end + # El listado viaja envuelto en `data`, como todas las colecciones (ADR-015). + def listed + response.parsed_body['data'] + end + describe 'GET /api/v1/integrations' do it 'returns 401 without a token' do get '/api/v1/integrations' expect(response).to have_http_status(:unauthorized) end + # Era el único listado que contestaba un array pelado. Un array en la raíz + # no deja lugar para `meta` sin romper a quien lo consume, y obligaba al + # front a recordar que éste es la excepción (ADR-015, TESIS-107). + it 'wraps the collection in data, like every other listing' do + get '/api/v1/integrations', headers: headers + + expect(response.parsed_body.keys).to eq(['data']) + end + context 'when the company has the service configured' do before do CompanyIntegration.create!(company: company, service: service, @@ -25,7 +39,7 @@ end it 'marks the service as configured and active', :aggregate_failures do - row = response.parsed_body.find { |r| r['service_id'] == service.id } + row = listed.find { |r| r['service_id'] == service.id } expect(row['configured']).to be(true) expect(row['is_active']).to be(true) end @@ -45,7 +59,7 @@ let(:other_company) { Company.create!(name: 'Tenant B', tax_id: '30-22222222-2') } it 'shows the service as not configured for the current tenant', :aggregate_failures do - row = response.parsed_body.find { |r| r['service_id'] == service.id } + row = listed.find { |r| r['service_id'] == service.id } expect(row['configured']).to be(false) expect(row['is_active']).to be(false) end From ad7db50f5464469d629129a351a5e346ec6bc7fe Mon Sep 17 00:00:00 2001 From: Tomas Martin Date: Thu, 24 Sep 2026 21:44:12 -0300 Subject: [PATCH 2/3] fix: [TESIS-107] make the registration errors follow the convention MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The review found the fourth shape the ADR says no longer exists: `auth/register` answered both of its failures as `{ errors: [...] }` — plural, array — with a comment claiming it did so to spare the frontend two formats. It is a single `error` string now, and the contract spec covers both paths, which is why nothing caught it before. The ADR also said three things that were not true: that `api_contract_spec.rb` breaks on any new shape (it only covers the endpoints it lists), that integrations was the only endpoint to change, and that two files were being edited in live branches — TESIS-58 and TESIS-61 merged today. The rule now also states that `error` may carry extra data, as the 409 of optimistic locking does with `current_version`. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp --- .../api/v1/auth/registrations_controller.rb | 12 +++++++---- ...R-015-convencion-de-respuesta-de-la-api.md | 13 +++++++++--- spec/requests/api/v1/api_contract_spec.rb | 20 +++++++++++++++++++ 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/app/controllers/api/v1/auth/registrations_controller.rb b/app/controllers/api/v1/auth/registrations_controller.rb index f28c290e..7a4085ac 100644 --- a/app/controllers/api/v1/auth/registrations_controller.rb +++ b/app/controllers/api/v1/auth/registrations_controller.rb @@ -16,7 +16,11 @@ def create user = ::Auth::RegisterUser.new(params: user_params, company: company).call render json: UserSerializer.render(user), status: :created rescue ActiveRecord::RecordInvalid => e - render json: { errors: e.record.errors.full_messages }, status: :unprocessable_content + # `error` en singular y con un string, como el resto de la API + # (ADR-015). Los mensajes se unen en una oración: el consumidor de un + # alta que falla muestra el motivo, no arma una lista. + render json: { error: e.record.errors.full_messages.to_sentence }, + status: :unprocessable_content end private @@ -29,10 +33,10 @@ def user_params end # Mismo cuerpo para slug ausente, inexistente e inactivo: la respuesta no - # dice si el tenant existe. Se usa el shape `errors` que ya devuelve el - # 422 de validación, para que el frontend no distinga dos formatos. + # dice si el tenant existe. Misma forma que el 422 de validación de acá + # arriba y que la de toda la API, así el frontend no distingue formatos. def render_unknown_tenant - render json: { errors: ['Unable to complete registration'] }, + render json: { error: 'Unable to complete registration' }, status: :unprocessable_content end end diff --git a/docs/adr/ADR-015-convencion-de-respuesta-de-la-api.md b/docs/adr/ADR-015-convencion-de-respuesta-de-la-api.md index d3803853..2ed02428 100644 --- a/docs/adr/ADR-015-convencion-de-respuesta-de-la-api.md +++ b/docs/adr/ADR-015-convencion-de-respuesta-de-la-api.md @@ -36,15 +36,22 @@ PUT /api/v1/products/:id → { "id": 1, "sku": "...", ... } cualquier error → { "error": "..." } ``` +`error` es una sola clave y un solo string, y **siempre está**. Puede venir acompañado de datos para recuperarse: el 409 del locking optimista agrega `current_version`, que es lo que el frontend necesita para reintentar. Lo que no se admite es otra clave en su lugar —`errors` en plural, un array, un objeto por campo—, porque entonces el consumidor tiene que probar dos formas. + +``` +``` + `meta` aparece sólo si el listado pagina, y es siempre `page`, `per_page` y `total`, contando el scope **ya filtrado**. -El único endpoint que hubo que cambiar fue `integrations#index`, que devolvía un array en la raíz. +Hubo que cambiar dos endpoints. `integrations#index`, que devolvía un array en la raíz, y `auth/register`, que respondía sus dos errores como `{ "errors": [...] }` —plural y array—. El registro no lo detectó la primera pasada porque el spec de contrato no lo cubría; ahora sí. ## Alternativas consideradas **Envolver también los recursos solos** (`{ "data": { ... } }` en `show`, `create` y `update`). Es la regla más simple de enunciar —una sola, sin excepciones— y deja lugar para agregarle `meta` a un recurso individual el día que haga falta. -Se descartó por lo que costaba **ahora**, no por lo que vale: 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. Cambiar el contrato debajo de dos PRs abiertos, para ganar uniformidad en endpoints que hoy nadie confunde, no se paga. +Se descartó por lo que costaba **ahora**, no por lo que vale: son diez lugares del frontend, repartidos en cuatro archivos de frontera, y cada uno hay que cambiarlo y volver a probarlo para ganar uniformidad en endpoints que hoy nadie confunde. La URL ya separa los dos casos —`/products` contra `/products/:id`—, así que lo que se compra con esos diez cambios es que la regla se enuncie sin la segunda mitad. + +_(La primera versión de este ADR agregaba que dos de esos archivos estaban siendo editados en las ramas vivas de TESIS-58 y TESIS-61. Las dos se mergearon el 24/09, así que ese motivo ya no corre; el costo de los diez lugares, sí.)_ La puerta queda abierta: pasar de esta convención a la otra es aditivo del lado del backend —envolver lo que hoy va pelado— y el día que se haga, este ADR se reemplaza en vez de discutirse de nuevo. @@ -57,7 +64,7 @@ La puerta queda abierta: pasar de esta convención a la otra es aditivo del lado - La regla se enuncia en una línea y no tiene excepciones que justificar. - Ninguna colección queda con un array en la raíz, así que cualquiera puede empezar a paginar sin romper su contrato. Es la precondición de TESIS-108. - El comentario-trampa del frontend se borra: lo que explicaba ya no pasa. -- `spec/requests/api/v1/api_contract_spec.rb` (TESIS-90) fija las tres formas, así que un endpoint nuevo que invente una cuarta rompe la suite. +- `spec/requests/api/v1/api_contract_spec.rb` (TESIS-90) fija las tres formas **sobre los endpoints que enumera**, hoy incluido el registro. La lista está escrita a mano: un endpoint nuevo con otra forma no rompe nada hasta que se lo agrega ahí. Recorrer todas las rutas sería otra card; mientras tanto, sumar el endpoint al spec es parte de agregarlo. **En contra** diff --git a/spec/requests/api/v1/api_contract_spec.rb b/spec/requests/api/v1/api_contract_spec.rb index 17838ce5..8a7f86ca 100644 --- a/spec/requests/api/v1/api_contract_spec.rb +++ b/spec/requests/api/v1/api_contract_spec.rb @@ -151,6 +151,26 @@ def shipment expect(response.parsed_body.keys).to eq(['error']) end + + # El registro respondía `errors` en plural y con un array: era la cuarta + # forma que este ADR vino a sacar, y no la veía nadie porque el endpoint no + # estaba acá. Los dos caminos que fallan, fijados. + it 'reports a failed registration with the same single error key' do + post '/api/v1/auth/register', + params: { email: 'no-es-un-mail', password: '123' }, + headers: { 'X-Tenant-Slug' => company.slug } + + expect(response.parsed_body.keys).to eq(['error']) + end + + it 'reports an unknown tenant with the same single error key', :aggregate_failures do + post '/api/v1/auth/register', + params: { email: 'nuevo@example.com', password: 'password123' }, + headers: { 'X-Tenant-Slug' => 'no-existe' } + + expect(response.parsed_body.keys).to eq(['error']) + expect(response.parsed_body['error']).to be_a(String) + end end # ──────────────────────────────────────────────── los campos que el front lee From 479afb6931e0836d9ed7d00fcf52fbcc4963e94e Mon Sep 17 00:00:00 2001 From: Tomas Martin Date: Thu, 24 Sep 2026 21:47:42 -0300 Subject: [PATCH 3/3] docs: [TESIS-107] drop the empty code fence left in the ADR Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012xAtddb53LRNVLuyTXyELp --- docs/adr/ADR-015-convencion-de-respuesta-de-la-api.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/docs/adr/ADR-015-convencion-de-respuesta-de-la-api.md b/docs/adr/ADR-015-convencion-de-respuesta-de-la-api.md index 2ed02428..b333a1ef 100644 --- a/docs/adr/ADR-015-convencion-de-respuesta-de-la-api.md +++ b/docs/adr/ADR-015-convencion-de-respuesta-de-la-api.md @@ -38,9 +38,6 @@ cualquier error → { "error": "..." } `error` es una sola clave y un solo string, y **siempre está**. Puede venir acompañado de datos para recuperarse: el 409 del locking optimista agrega `current_version`, que es lo que el frontend necesita para reintentar. Lo que no se admite es otra clave en su lugar —`errors` en plural, un array, un objeto por campo—, porque entonces el consumidor tiene que probar dos formas. -``` -``` - `meta` aparece sólo si el listado pagina, y es siempre `page`, `per_page` y `total`, contando el scope **ya filtrado**. Hubo que cambiar dos endpoints. `integrations#index`, que devolvía un array en la raíz, y `auth/register`, que respondía sus dos errores como `{ "errors": [...] }` —plural y array—. El registro no lo detectó la primera pasada porque el spec de contrato no lo cubría; ahora sí.