diff --git a/app/controllers/api/v1/auth/registrations_controller.rb b/app/controllers/api/v1/auth/registrations_controller.rb index de1e95e..5b33242 100644 --- a/app/controllers/api/v1/auth/registrations_controller.rb +++ b/app/controllers/api/v1/auth/registrations_controller.rb @@ -19,7 +19,11 @@ def create ::Auth::RegisterUser.new(params: user_params, company: company).call render_request_received 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 @@ -40,10 +44,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/app/controllers/api/v1/integrations_controller.rb b/app/controllers/api/v1/integrations_controller.rb index 790b6c5..f679bd6 100644 --- a/app/controllers/api/v1/integrations_controller.rb +++ b/app/controllers/api/v1/integrations_controller.rb @@ -9,11 +9,17 @@ class IntegrationsController < ApplicationController # modificación sí: ver CompanyIntegrationPolicy. skip_after_action :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 0000000..b333a1e --- /dev/null +++ b/docs/adr/ADR-015-convencion-de-respuesta-de-la-api.md @@ -0,0 +1,75 @@ +# 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": "..." } +``` + +`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í. + +## 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, 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. + +**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 **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** + +- 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 b5a9119..faba770 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 e032d83..8a7f86c 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í. @@ -149,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 diff --git a/spec/requests/api/v1/integrations_spec.rb b/spec/requests/api/v1/integrations_spec.rb index e5bb5cb..3305f01 100644 --- a/spec/requests/api/v1/integrations_spec.rb +++ b/spec/requests/api/v1/integrations_spec.rb @@ -13,12 +13,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, @@ -27,7 +41,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 @@ -47,7 +61,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