Skip to content

fix: [TESIS-999020] tell sales apart by channel, not by company, on webhook ingestion - #108

Draft
LauAubert wants to merge 3 commits into
masterfrom
TESIS-999020-webhook-idempotency-per-channel
Draft

LauAubert wants to merge 3 commits into
masterfrom
TESIS-999020-webhook-idempotency-per-channel

Conversation

@LauAubert

@LauAubert LauAubert commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

Ticket de Jira

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

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


Descripción

La ingesta de webhooks buscaba la venta duplicada con Order.find_by(external_order_id:) —acotado sólo por tenant— y el respaldo era un índice único (company_id, external_order_id). Pero cada canal numera sus ventas por su lado: si una empresa tiene Mercado Libre y Tiendanube y el segundo manda un id que el primero ya usó, la venta se tomaba por duplicada. El log quedaba processed, no se creaba la orden, no se descontaba stock y no quedaba nada en la DLQ: la venta se perdía sin ninguna señal.

Repro en master: ML ingiere el id 5001 (2 unidades) y después TN ingiere el 5001 (3 unidades) → los dos logs processed, una sola orden, stock 18 (se descontaron sólo 2), FailedEvent.count == 0.

Decisiones que conviene mirar:

La misma venta es el mismo id en el mismo canal. La búsqueda (already_registered), la validación del modelo y el índice único pasan a company_integration_id + external_order_id. La integración ya implica la empresa (cada una es de una sola), así que el aislamiento entre tenants no cambia.

El NULL de la integración no deja huecos. Las órdenes manuales no pueden llevar id externo (ORDER_FIELDS no lo permite), y todas las que entran por webhook traen su integración. No hace falta NULLS NOT DISTINCT.

La migración no puede fallar con datos existentes. Todo lo que era único por empresa también lo es por integración.

schema.rb: para no migrar la base de desarrollo compartida, tomé el volcado real de la base de test después de migrar y apliqué sólo las dos líneas de este cambio (versión e índice). El resto del volcado difería en el formato de los CHECK por la versión local de Postgres.

  • Migración ScopeOrderExternalIdToIntegration (timestamp real, sin duplicados contra master).
  • Orders::ProcessWebhookOrder#already_registered y ORDERS_UNIQUE_INDEX apuntan al índice nuevo; el rescate de la carrera entre dos workers usa la misma búsqueda.
  • Order valida la unicidad del id externo por integración.
  • Actualiza los pasos 2 y 3 de la ingesta en ADR-010.
  • Specs: el mismo id en otro canal crea su orden y descuenta stock; la unicidad del modelo dentro de un canal, entre canales de la misma empresa y entre empresas.

Evidencia visual

N/A


Cómo probar

Precondición: bin/rails db:migrate.

  1. Mandar por el webhook de Mercado Libre de Norte una venta con id 5001 → se crea la orden.
  2. Mandar la misma venta otra vez por ML → no se duplica (idempotencia intacta).
  3. Mandar por el webhook de Tiendanube de Norte una venta con id 5001 → se crea otra orden, de TN, y descuenta su stock. En master se descarta en silencio.

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

Dientes: con la búsqueda por empresa, fallan los 2 specs nuevos de la ingesta.


Impacto y consideraciones

¿Introduce breaking changes?
No para los clientes. Cambia un índice único (migración reversible).

¿Requiere nuevas variables de entorno?
No

¿Afecta la arquitectura o genera un nuevo patrón?
Ajusta la clave de idempotencia de ADR-010 (de empresa a canal); el ADR queda actualizado en este PR.

Conflictos esperables: con proyecto-api#104 (misma clase, otra zona) y con cualquier rama que agregue migraciones (la línea de versión de schema.rb).

🤖 Generated with Claude Code

LauAubert and others added 3 commits October 2, 2026 02:01
…ebhook ingestion

The webhook ingestion looked the duplicate sale up by external_order_id
within the company, backed by a unique index on (company_id,
external_order_id). Two channels of the same company number their sales on
their own, so when a second channel sent an id the first had already used,
the sale was taken for a duplicate: the log ended processed with no order, no
units taken and nothing in the dead letter queue. The sale was lost without
a signal.

The same sale is now the same id in the same channel: the lookup, the model
validation and the unique index are scoped to company_integration_id, which
already implies the company. Manual orders cannot carry an external id, so
the null integration leaves nothing uncovered.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@TomasMartin2004

Copy link
Copy Markdown
Contributor

Revisado. Lo veo bien para implementar.

El diagnóstico es correcto y es un problema de diseño, no un descuido: cada canal numera sus ventas por su lado, así que external_order_id sin el canal no identifica nada. Con dos integraciones en una empresa, una venta de la segunda con un id que la primera ya usó se tomaba por duplicada y se perdía sin ninguna señal: log processed, sin orden, sin stock descontado y sin nada en la DLQ. Es la peor forma de fallar que hay en este sistema. Toca RF-20 (idempotencia), que es alcance comprometido.

Lo que verifiqué

  • company_integration_id ya implica la empresa —cada integración pertenece a una sola—, así que el índice nuevo no afloja el aislamiento por tenant. Era lo primero que había que chequear al sacar company_id del índice.
  • Las órdenes manuales no tienen external_order_id (el alta no lo permite), así que los NULL de company_integration_id no dejan nada sin cubrir.
  • already_registered filtra por la integración del propio log, no por la del payload. Correcto: el canal lo determina el gateway, no el cuerpo del mensaje.
  • El rescate de RecordNotUnique sigue verificando la violación por nombre de índice, y el nombre se actualizó en los dos lados (ORDERS_UNIQUE_INDEX y la migración). Si eso se hubiera desincronizado, un duplicado real habría escalado a la DLQ como fallo. Está bien.
  • ADR-010 queda actualizado en los puntos 2 y 3 de las barreras de idempotencia.
  • Los specs de modelo cubren los tres casos: mismo canal (rechaza), dos canales de la misma empresa (acepta) y dos empresas (acepta).

Antes de mergear

  1. CONFLICTING contra master, y acá el conflicto es más molesto que en los otros: el PR toca db/schema.rb, y TESIS-138 (Shopify) metió migraciones después. Al rebasar hay que regenerar schema.rb desde la base, no resolver el conflicto a mano.
  2. La migración hay que correrla en la VPS cuando se despliegue. Es un cambio de índice, no de datos: no hay riesgo de que falle por filas existentes, porque el índice nuevo es más permisivo que el que saca.
  3. Toca process_webhook_order.rb, igual que fix: [TESIS-999016] keep the stock of a sale that arrives already cancelled #104. Definí un orden entre los dos.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants