From fab14759ebb6a45b0ebf52cf1016e9603048f5a8 Mon Sep 17 00:00:00 2001 From: LauAubert Date: Fri, 2 Oct 2026 02:01:01 -0300 Subject: [PATCH 1/3] fix: [TESIS-999020] tell sales apart by channel, not by company, on webhook 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 --- app/models/order.rb | 4 +- app/poros/orders/process_webhook_order.rb | 17 +++++-- ..._scope_order_external_id_to_integration.rb | 18 +++++++ db/schema.rb | 4 +- spec/models/order_spec.rb | 47 ++++++++++++++----- .../orders/process_webhook_order_spec.rb | 32 +++++++++++++ 6 files changed, 102 insertions(+), 20 deletions(-) create mode 100644 db/migrate/20261002045858_scope_order_external_id_to_integration.rb diff --git a/app/models/order.rb b/app/models/order.rb index 4a65a8cb..8e9d15e1 100644 --- a/app/models/order.rb +++ b/app/models/order.rb @@ -35,7 +35,9 @@ class Order < ApplicationRecord validates :customer_name, presence: true validates :status, presence: true, inclusion: { in: STATUSES } - validates :external_order_id, uniqueness: { scope: :company_id }, allow_nil: true + # Único por canal y no por empresa: dos canales de la misma empresa pueden + # usar el mismo id. Es el índice `index_orders_on_integration_and_external_order_id`. + validates :external_order_id, uniqueness: { scope: :company_integration_id }, allow_nil: true # allow_nil: las órdenes anteriores a TESIS-114 que no tienen líneas no tienen # con qué calcularlo, y la orden vive un instante sin total dentro de la # transacción que la crea. diff --git a/app/poros/orders/process_webhook_order.rb b/app/poros/orders/process_webhook_order.rb index 7fa31cf0..7f86b00b 100644 --- a/app/poros/orders/process_webhook_order.rb +++ b/app/poros/orders/process_webhook_order.rb @@ -20,7 +20,7 @@ class UnmappedProductError < StandardError; end MISSING_ORDER_ID = 'the payload does not carry an external order id' MISSING_ITEMS = 'the payload does not carry any order item' UNREADABLE_ITEMS = 'the template could not read %d of the order items in the payload' - ORDERS_UNIQUE_INDEX = 'index_orders_on_company_id_and_external_order_id' + ORDERS_UNIQUE_INDEX = 'index_orders_on_integration_and_external_order_id' def initialize(webhook_log:) super() @@ -44,7 +44,7 @@ def call def ingest validate_payload! - duplicate = Order.find_by(external_order_id: external_order_id) + duplicate = already_registered return duplicate if duplicate items = resolve_items @@ -58,10 +58,19 @@ def ingest # log quedaría en `processed` sin ninguna orden creada. raise unless e.message.include?(ORDERS_UNIQUE_INDEX) - # Dos workers con el mismo evento: el índice único (company_id, + # Dos workers con el mismo evento: el índice único (company_integration_id, # external_order_id) deja pasar a uno solo. El que perdió la carrera no # tiene nada que hacer, la venta ya está registrada. - Order.find_by(external_order_id: external_order_id) + already_registered + end + + # La misma venta es el mismo id **en el mismo canal**. Antes se buscaba por + # empresa: si dos canales de una empresa usaban el mismo id (cada uno numera + # por su lado), la segunda venta se tomaba por duplicada, el log quedaba + # `processed` sin orden, sin stock descontado y sin nada en la DLQ. + def already_registered + Order.find_by(company_integration_id: @log.company_integration_id, + external_order_id: external_order_id) end def create_order(items) diff --git a/db/migrate/20261002045858_scope_order_external_id_to_integration.rb b/db/migrate/20261002045858_scope_order_external_id_to_integration.rb new file mode 100644 index 00000000..5aaf38dd --- /dev/null +++ b/db/migrate/20261002045858_scope_order_external_id_to_integration.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +# La venta externa se identifica por el canal que la mandó, no por la empresa: +# dos canales de la misma empresa pueden usar el mismo id (Mercado Libre y +# Tiendanube numeran cada uno por su lado). Con el índice por empresa, la +# segunda venta se tomaba por duplicada y se perdía sin dejar error. +# +# `company_integration_id` ya implica la empresa: cada integración es de una +# sola. Las órdenes manuales no tienen id externo (el alta no lo permite), así +# que el NULL de la integración no deja nada sin cubrir. +class ScopeOrderExternalIdToIntegration < ActiveRecord::Migration[8.1] + def change + remove_index :orders, %i[company_id external_order_id], unique: true, + name: 'index_orders_on_company_id_and_external_order_id' + add_index :orders, %i[company_integration_id external_order_id], + unique: true, name: 'index_orders_on_integration_and_external_order_id' + end +end diff --git a/db/schema.rb b/db/schema.rb index a88bf772..eecc8d53 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_09_25_120000) do +ActiveRecord::Schema[8.1].define(version: 2026_10_02_045858) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" @@ -110,7 +110,7 @@ t.string "status", default: "pending", null: false t.decimal "total_amount", precision: 10, scale: 2 t.datetime "updated_at", null: false - t.index ["company_id", "external_order_id"], name: "index_orders_on_company_id_and_external_order_id", unique: true + t.index ["company_integration_id", "external_order_id"], name: "index_orders_on_integration_and_external_order_id", unique: true t.index ["company_id"], name: "index_orders_on_company_id" t.index ["company_integration_id"], name: "index_orders_on_company_integration_id" t.check_constraint "status::text = ANY (ARRAY['pending'::character varying, 'paid'::character varying, 'cancelled'::character varying]::text[])", name: "orders_status_check" diff --git a/spec/models/order_spec.rb b/spec/models/order_spec.rb index 5ef07839..616cb3a4 100644 --- a/spec/models/order_spec.rb +++ b/spec/models/order_spec.rb @@ -76,20 +76,41 @@ expect(another).to be_valid end - it 'enforces external_order_id uniqueness scoped to company' do - order.external_order_id = 'ML-123' - order.save! - duplicate = described_class.new(company: company, customer_name: 'Otro', - external_order_id: 'ML-123') - expect(duplicate).not_to be_valid - end + # Única por canal: la misma venta es el mismo id en la misma integración. + describe 'the external order id' do + def channel(owner, name) + CompanyIntegration.create!( + company: owner, + service: Service.create!(service_name: name, type: 'ecommerce', + uri: "https://#{name.downcase}.test", http_method: 'GET') + ) + end - it 'allows the same external_order_id across different companies' do - order.external_order_id = 'ML-123' - order.save! - other = described_class.new(company: other_company, customer_name: 'Otro', - external_order_id: 'ML-123') - expect(other).to be_valid + def sale(owner, integration, id = 'ML-123') + described_class.new(company: owner, customer_name: 'Cliente', company_integration: integration, + external_order_id: id) + end + + it 'is unique within the same channel' do + ml = channel(company, 'ML') + sale(company, ml).save! + + expect(sale(company, ml)).not_to be_valid + end + + # Cada canal numera por su lado: antes la segunda venta se tomaba por + # duplicada y se perdía sin dejar error. + it 'repeats freely between two channels of the same company' do + sale(company, channel(company, 'ML')).save! + + expect(sale(company, channel(company, 'TN'))).to be_valid + end + + it 'repeats freely between companies' do + sale(company, channel(company, 'ML')).save! + + expect(sale(other_company, channel(other_company, 'ML2'))).to be_valid + end end it 'rejects a company_integration from another company', :aggregate_failures do diff --git a/spec/poros/orders/process_webhook_order_spec.rb b/spec/poros/orders/process_webhook_order_spec.rb index 4576afa7..fb3895c1 100644 --- a/spec/poros/orders/process_webhook_order_spec.rb +++ b/spec/poros/orders/process_webhook_order_spec.rb @@ -197,6 +197,38 @@ def create_log(body = payload) end end + # Hallazgo de auditoría (TESIS-89): la idempotencia buscaba el id externo en + # toda la empresa. La venta de otro canal con el mismo id se tomaba por + # duplicada: log `processed`, sin orden, sin stock descontado, sin DLQ. + context 'when another channel of the company already used the same external id' do + let(:other_channel) { CompanyIntegration.create!(company: company, service: create_service) } + + before do + publish('SKU-1', 'MLA-1', stock: 20) + ProductMapping.create!(product: Product.find_by(sku: 'SKU-1'), company_integration: other_channel, + external_product_id: 'TN-1') + described_class.new(webhook_log: create_log(order_payload(items: [line('MLA-1', 2, 10)]))).call + end + + def sale_from_other_channel + WebhookLog.create!(company_id: company.id, company_integration: other_channel, + payload: order_payload(items: [line('TN-1', 3, 10)])) + end + + it 'registers it as a sale of its own', :aggregate_failures do + order = described_class.new(webhook_log: sale_from_other_channel).call + + expect(order.company_integration).to eq(other_channel) + expect(Order.where(external_order_id: 'ML-1001').count).to eq(2) + end + + it 'takes its units from the stock' do + described_class.new(webhook_log: sale_from_other_channel).call + + expect(stock_of('SKU-1')).to eq(15) + end + end + context 'when the log was already processed' do before { log.update!(status: :processed) } From 01bf9164914b051fed9861d35b639798ffe09e0b Mon Sep 17 00:00:00 2001 From: LauAubert Date: Fri, 2 Oct 2026 02:01:42 -0300 Subject: [PATCH 2/3] docs: [TESIS-999020] key the webhook idempotency by channel in ADR-010 --- Gemfile.lock | 8 ++++++++ docs/adr/ADR-010-ingesta-de-ordenes-de-webhooks.md | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 549eb76a..697e1f07 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -219,6 +219,8 @@ GEM nio4r (2.7.5) nokogiri (1.19.4-aarch64-linux-gnu) racc (~> 1.4) + nokogiri (1.19.4-arm64-darwin) + racc (~> 1.4) nokogiri (1.19.4-x64-mingw-ucrt) racc (~> 1.4) nokogiri (1.19.4-x86_64-linux-gnu) @@ -234,6 +236,7 @@ GEM ast (~> 2.4.1) racc pg (1.6.3-aarch64-linux) + pg (1.6.3-arm64-darwin) pg (1.6.3-x64-mingw-ucrt) pg (1.6.3-x86_64-linux) pp (0.6.4) @@ -390,6 +393,7 @@ GEM thor (1.5.0) thruster (0.1.26) thruster (0.1.26-aarch64-linux) + thruster (0.1.26-arm64-darwin) thruster (0.1.26-x86_64-linux) timeout (0.6.1) tsort (0.2.0) @@ -431,6 +435,7 @@ GEM PLATFORMS aarch64-linux + arm64-darwin-25 x64-mingw-ucrt x86_64-linux @@ -543,6 +548,7 @@ CHECKSUMS net-ssh (7.3.3) sha256=831def58b2c51dcef66ec00d29397d4f210de89c19fe78f95873ca30f386e86a nio4r (2.7.5) sha256=6c90168e48fb5f8e768419c93abb94ba2b892a1d0602cb06eef16d8b7df1dca1 nokogiri (1.19.4-aarch64-linux-gnu) sha256=1269fb644a6de405057a53dd5c762b1209b43ca7424f839454d3dbc677c31a8f + nokogiri (1.19.4-arm64-darwin) nokogiri (1.19.4-x64-mingw-ucrt) sha256=051da97b8eccfdb5444fed40246a35e10d7298b9efe759b4cd25455ea04c587e nokogiri (1.19.4-x86_64-linux-gnu) sha256=379fae440b28915e3f19d752ce2dcf8465ed2b2fbefd2a7ca0dd497bc981a06a orm_adapter (0.5.0) sha256=aa5d0be5d540cbb46d3a93e88061f4ece6a25f6e97d6a47122beb84fe595e9b9 @@ -551,6 +557,7 @@ CHECKSUMS parallel (2.1.0) sha256=b35258865c2e31134c5ecb708beaaf6772adf9d5efae28e93e99260877b09356 parser (3.3.12.0) sha256=21a6d7f755d5a24dfbdc6e6b772e4e879a52e7631a88bc5a3a134606052c9828 pg (1.6.3-aarch64-linux) sha256=0698ad563e02383c27510b76bf7d4cd2de19cd1d16a5013f375dd473e4be72ea + pg (1.6.3-arm64-darwin) pg (1.6.3-x64-mingw-ucrt) sha256=cdff974cbde6935e07b8f5cc3c2af7d320bc5263bde06acaae08302194ebf5e1 pg (1.6.3-x86_64-linux) sha256=5d9e188c8f7a0295d162b7b88a768d8452a899977d44f3274d1946d67920ae8d pp (0.6.4) sha256=dfcb0fce700c41456265922884f9fe195d7fbb0674a3578e6c0f69588e82b570 @@ -601,6 +608,7 @@ CHECKSUMS thor (1.5.0) sha256=e3a9e55fe857e44859ce104a84675ab6e8cd59c650a49106a05f55f136425e73 thruster (0.1.26) sha256=6e45e807086b29d51404841bd1ad493b67cd95892fd65dc5afcdd32e82e94ce8 thruster (0.1.26-aarch64-linux) sha256=2171cb34928c0250830008f535c4ab2ee57846cc3f5d3e96c3475f7b3de7a541 + thruster (0.1.26-arm64-darwin) thruster (0.1.26-x86_64-linux) sha256=3117a6ee430663f845a0457699fe9a05232dcc6c396e2cc83504de5a223c60e8 timeout (0.6.1) sha256=78f57368a7e7bbadec56971f78a3f5ecbcfb59b7fcbb0a3ed6ddc08a5094accb tsort (0.2.0) sha256=9650a793f6859a43b6641671278f79cfead60ac714148aabe4e3f0060480089f diff --git a/docs/adr/ADR-010-ingesta-de-ordenes-de-webhooks.md b/docs/adr/ADR-010-ingesta-de-ordenes-de-webhooks.md index 55796040..a623b4f1 100644 --- a/docs/adr/ADR-010-ingesta-de-ordenes-de-webhooks.md +++ b/docs/adr/ADR-010-ingesta-de-ordenes-de-webhooks.md @@ -124,8 +124,8 @@ La excepción **no se propaga** desde el job: si subiera, Active Job reintentar Las plataformas reenvían webhooks y Solid Queue garantiza *at-least-once*: el mismo evento puede llegar a procesarse más de una vez, y descontar el stock dos veces por una sola venta es el peor error posible acá. Tres barreras, en orden: 1. Un log ya `processed` no se vuelve a procesar. -2. Si ya existe una `Order` con ese `external_order_id`, se marca el log como procesado y no se crea nada. -3. Si dos workers corren a la vez, el índice único `(company_id, external_order_id)` deja pasar a uno solo; el que pierde la carrera captura el `RecordNotUnique` y termina como duplicado. El rescate verifica que la violación sea **la de ese índice** por nombre: cubre toda la transacción, y un índice único que aparezca más adelante en `order_items` o en `stocks` es un fallo real que tiene que llegar a la DLQ, no un duplicado ya registrado. +2. Si ya existe una `Order` con ese `external_order_id` **en la misma integración**, se marca el log como procesado y no se crea nada. La clave es el canal y no la empresa: cada canal numera sus ventas por su lado, y con la clave por empresa la venta de un segundo canal con un id repetido se descartaba en silencio (TESIS-999020). +3. Si dos workers corren a la vez, el índice único `(company_integration_id, external_order_id)` deja pasar a uno solo; el que pierde la carrera captura el `RecordNotUnique` y termina como duplicado. El rescate verifica que la violación sea **la de ese índice** por nombre: cubre toda la transacción, y un índice único que aparezca más adelante en `order_items` o en `stocks` es un fallo real que tiene que llegar a la DLQ, no un duplicado ya registrado. ### Ruteo en el gateway From fbb438a6c532d9e8f619eec019fddd1782f8b55a Mon Sep 17 00:00:00 2001 From: LauAubert Date: Fri, 2 Oct 2026 02:02:14 -0300 Subject: [PATCH 3/3] chore: [TESIS-999020] restore the Gemfile.lock of master Co-Authored-By: Claude Opus 5.5 --- Gemfile.lock | 8 -------- 1 file changed, 8 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 697e1f07..549eb76a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -219,8 +219,6 @@ GEM nio4r (2.7.5) nokogiri (1.19.4-aarch64-linux-gnu) racc (~> 1.4) - nokogiri (1.19.4-arm64-darwin) - racc (~> 1.4) nokogiri (1.19.4-x64-mingw-ucrt) racc (~> 1.4) nokogiri (1.19.4-x86_64-linux-gnu) @@ -236,7 +234,6 @@ GEM ast (~> 2.4.1) racc pg (1.6.3-aarch64-linux) - pg (1.6.3-arm64-darwin) pg (1.6.3-x64-mingw-ucrt) pg (1.6.3-x86_64-linux) pp (0.6.4) @@ -393,7 +390,6 @@ GEM thor (1.5.0) thruster (0.1.26) thruster (0.1.26-aarch64-linux) - thruster (0.1.26-arm64-darwin) thruster (0.1.26-x86_64-linux) timeout (0.6.1) tsort (0.2.0) @@ -435,7 +431,6 @@ GEM PLATFORMS aarch64-linux - arm64-darwin-25 x64-mingw-ucrt x86_64-linux @@ -548,7 +543,6 @@ CHECKSUMS net-ssh (7.3.3) sha256=831def58b2c51dcef66ec00d29397d4f210de89c19fe78f95873ca30f386e86a nio4r (2.7.5) sha256=6c90168e48fb5f8e768419c93abb94ba2b892a1d0602cb06eef16d8b7df1dca1 nokogiri (1.19.4-aarch64-linux-gnu) sha256=1269fb644a6de405057a53dd5c762b1209b43ca7424f839454d3dbc677c31a8f - nokogiri (1.19.4-arm64-darwin) nokogiri (1.19.4-x64-mingw-ucrt) sha256=051da97b8eccfdb5444fed40246a35e10d7298b9efe759b4cd25455ea04c587e nokogiri (1.19.4-x86_64-linux-gnu) sha256=379fae440b28915e3f19d752ce2dcf8465ed2b2fbefd2a7ca0dd497bc981a06a orm_adapter (0.5.0) sha256=aa5d0be5d540cbb46d3a93e88061f4ece6a25f6e97d6a47122beb84fe595e9b9 @@ -557,7 +551,6 @@ CHECKSUMS parallel (2.1.0) sha256=b35258865c2e31134c5ecb708beaaf6772adf9d5efae28e93e99260877b09356 parser (3.3.12.0) sha256=21a6d7f755d5a24dfbdc6e6b772e4e879a52e7631a88bc5a3a134606052c9828 pg (1.6.3-aarch64-linux) sha256=0698ad563e02383c27510b76bf7d4cd2de19cd1d16a5013f375dd473e4be72ea - pg (1.6.3-arm64-darwin) pg (1.6.3-x64-mingw-ucrt) sha256=cdff974cbde6935e07b8f5cc3c2af7d320bc5263bde06acaae08302194ebf5e1 pg (1.6.3-x86_64-linux) sha256=5d9e188c8f7a0295d162b7b88a768d8452a899977d44f3274d1946d67920ae8d pp (0.6.4) sha256=dfcb0fce700c41456265922884f9fe195d7fbb0674a3578e6c0f69588e82b570 @@ -608,7 +601,6 @@ CHECKSUMS thor (1.5.0) sha256=e3a9e55fe857e44859ce104a84675ab6e8cd59c650a49106a05f55f136425e73 thruster (0.1.26) sha256=6e45e807086b29d51404841bd1ad493b67cd95892fd65dc5afcdd32e82e94ce8 thruster (0.1.26-aarch64-linux) sha256=2171cb34928c0250830008f535c4ab2ee57846cc3f5d3e96c3475f7b3de7a541 - thruster (0.1.26-arm64-darwin) thruster (0.1.26-x86_64-linux) sha256=3117a6ee430663f845a0457699fe9a05232dcc6c396e2cc83504de5a223c60e8 timeout (0.6.1) sha256=78f57368a7e7bbadec56971f78a3f5ecbcfb59b7fcbb0a3ed6ddc08a5094accb tsort (0.2.0) sha256=9650a793f6859a43b6641671278f79cfead60ac714148aabe4e3f0060480089f