Skip to content

fix: [TESIS-999016] keep the stock of a sale that arrives already cancelled - #104

Draft
LauAubert wants to merge 2 commits into
masterfrom
TESIS-999016-webhook-cancelled-order-keeps-stock
Draft

LauAubert wants to merge 2 commits into
masterfrom
TESIS-999016-webhook-cancelled-order-keeps-stock

Conversation

@LauAubert

Copy link
Copy Markdown
Member

Ticket de Jira

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

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


Descripción

Orders::ProcessWebhookOrder#status acepta cualquier estado de Order::STATUSES, incluido cancelled, y create_order descontaba stock por cada línea sin mirarlo. Una venta cuya primera notificación ya llega cancelada —en Mercado Libre es normal: un pago rechazado— quedaba registrada como cancelada con sus unidades descontadas para siempre: una orden cancelada no se puede modificar (UpdateOrder, 409) ni volver a cancelar, así que nada las devolvía. Y el callback de Stock publicaba el número rebajado en todos los canales.

Repro en master: plantilla con response_value_mapper {'cancelado' => 'cancelled'}, stock en 20, webhook {"id":"100","status":"cancelado","items":[{"id":"P1","qty":5}]} → log processed, orden cancelled, stock en 15.

Decisiones que conviene mirar:

Se registra, sin stock. Descartarla también resolvía el stock, pero se perdería el rastro de que la venta existió (y la idempotencia: si el canal la reenvía, se la reconoce como duplicada). Se crea la orden cancelada con sus líneas, y las líneas no se llevan unidades.

Líneas sin depósito. Sin descuento, el picking de DeductStock no elige depósito, así que la línea queda con warehouse_id: nil, igual que las anteriores a TESIS-126. Como la orden ya está cancelada, ninguna edición ni cancelación va a intentar devolverle unidades.

Mapeo de productos, igual que antes. Una venta cancelada con un producto sin mapear sigue fallando y yendo a la DLQ: la orden necesita saber qué producto es cada línea. No se cambió ese criterio en esta card.

  • ProcessWebhookOrder#register_item toma unidades a través de take_units, que no descuenta para una orden cancelled.
  • Specs: la venta cancelada se registra, no toca el stock, deja el log processed y sus líneas sin depósito; control negativo con una venta pagada que sí descuenta.

Evidencia visual

N/A


Cómo probar

  1. Con una plantilla de canal que traduzca un estado externo a cancelled, mandar un webhook de una venta nueva con ese estado.
  2. La orden aparece como cancelled y el stock de sus productos no cambia. En master baja.
  3. Mandar la misma venta como pagada (otro id) → descuenta como siempre.

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

Dientes: sin la guarda de take_units, fallan 2 de los specs nuevos.


Impacto y consideraciones

¿Introduce breaking changes?
No. Corrige el stock de un caso que hoy lo deja mal.

¿Requiere nuevas variables de entorno?
No

¿Afecta la arquitectura o genera un nuevo patrón?
No

Fuera de alcance: una venta que entró pagada y después se cancela en el canal sigue sin devolver stock (ADR-013: «la cancelación sigue sin camino» para webhooks); proyecto-api#102 agrega la cancelación manual desde la API.

Conflicto esperable con TESIS-138-shopify-integration: esa rama toca order_attributes en el mismo archivo; este cambio está en register_item, otra zona.

🤖 Generated with Claude Code

LauAubert and others added 2 commits October 2, 2026 01:49
…celled

The order ingestion accepts any known status, cancelled included, and took
the units of every line regardless. A sale whose first notification already
comes cancelled (a rejected payment in Mercado Libre) left its units deducted
for good: a cancelled order cannot be modified nor cancelled again, so
nothing ever gave them back, and the stock callbacks published the lower
quantity to every channel.

The sale is still recorded, so it leaves a trace, but its lines take no
units and therefore record no warehouse, like the lines older than
TESIS-126. Paid and pending sales deduct as before.

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 caso no es raro: en Mercado Libre un pago rechazado hace que la primera notificación de la venta llegue ya cancelada. Hoy esa venta se registra cancelada y se lleva el stock igual, y como una orden cancelada no se puede editar (UpdateOrder responde 409) ni volver a cancelar, esas unidades no vuelven nunca. Pega justo en el problema que el proyecto dice resolver: inventario subestimado y canales publicando menos unidades de las que hay.

Lo que verifiqué

  • take_units devuelve nil para una orden cancelada, así que la línea queda sin depósito. Está bien y está explicado: no se descontó de ningún lado, no hay depósito que registrar, y como la orden ya está cancelada nada va a intentar devolverle unidades más adelante. Es el mismo estado que las líneas anteriores a TESIS-126.
  • La venta se sigue registrando con sus líneas y su cantidad, así que queda el rastro de que existió. Eso importa: no entrar la venta sería el otro error.
  • El log queda processed y no en la DLQ, que es lo correcto: no falló nada.
  • ADR-010 queda actualizado con la fila nueva en la tabla de qué corta y qué no. Que el ADR se mueva junto con el código es lo que hace que esa tabla siga sirviendo.
  • El spec when the sale arrives paid es la contraprueba: confirma que la venta normal sigue descontando. Sin eso, el PR podría estar apagando el descuento de más y pasando en verde.

Lo que queda afuera

Esto cubre la venta que nace cancelada. La que entra pending o paid y después se cancela sigue sin devolver stock: eso es #102, que no mergearía por otras razones. Vale dejarlo escrito en la card para que no quede la impresión de que el agujero de la cancelación quedó cerrado entero.

Antes de mergear

El PR está CONFLICTING contra master (lo dejó así el merge de TESIS-138) y toca process_webhook_order.rb, el mismo archivo que #108. Elegí un orden entre los dos y rebasá el segundo sobre el primero.

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