test: [TESIS-93] cover the high-risk paths and measure the coverage

.gitignore

@@ -32,3 +32,5 @@
 # Ignore key files for decrypting credentials and more.
 /config/*.key
 
+# Reporte de cobertura de SimpleCov (TESIS-93).
+/coverage/

Gemfile

@@ -60,6 +60,11 @@ end
 group :test do
   # Stubs de HTTP para testear el adaptador de integraciones sin red real.
   gem 'webmock'
+
+  # Mide qué líneas ejecuta la suite (TESIS-93). require: false porque lo
+  # arranca spec_helper antes de cargar la aplicación: lo que se carga antes
+  # que SimpleCov no se mide.
+  gem 'simplecov', require: false
 end
 
 gem 'devise', '~> 5.0'

Gemfile.lock

@@ -363,6 +363,7 @@ GEM
       rubocop (~> 1.86, >= 1.86.2)
     ruby-progressbar (1.13.0)
     securerandom (0.4.1)
+    simplecov (1.3.0)
     solid_cable (4.0.2)
       actioncable (>= 7.2)
       activejob (>= 7.2)
@@ -457,6 +458,7 @@ DEPENDENCIES
   rubocop-rails
   rubocop-rails-omakase
   rubocop-rspec
+  simplecov
   solid_cable
   solid_cache
   solid_queue
@@ -591,6 +593,7 @@ CHECKSUMS
   rubocop-rspec (3.10.2) sha256=0b3e2ecc592cd10ecbf0095bb58d1e357905276e069643523cc19eb7495f65e2
   ruby-progressbar (1.13.0) sha256=80fc9c47a9b640d6834e0dc7b3c94c9df37f08cb072b7761e4a71e22cff29b33
   securerandom (0.4.1) sha256=cc5193d414a4341b6e225f0cb4446aceca8e50d5e1888743fac16987638ea0b1
+  simplecov (1.3.0) sha256=d9886307863c1ead47657dcbed869bfffd82318808b09014d8f0ff77dcfe7492
   solid_cable (4.0.2) sha256=084636a67679ad00d23088b33c84047e614bcf41ee559db24b414d83cdc42d03
   solid_cache (1.0.10) sha256=bc05a2fb3ac78a6f43cbb5946679cf9db67dd30d22939ededc385cb93e120d41
   solid_queue (1.7.0) sha256=6566b70b801d1c317c81bba7bcdd5677c019afac584a30374b4164002ca356d3

app/poros/shipments/poll_tracking_status.rb

@@ -51,8 +51,13 @@ def tracking_service
 
     # Se revalida al ejecutar, no al encolar: entre el barrido y este job el
     # envío pudo entregarse (por otro ciclo, o a mano desde el panel).
+    # `order(:id)` no es cosmético: en la consulta masiva los números viajan
+    # concatenados en la URI, así que sin un orden fijo el mismo lote produce
+    # URLs distintas entre corridas. Postgres puede devolver las filas en
+    # cualquier orden, y eso ya hacía fallar de forma intermitente al spec que
+    # fija la URL del lote (TESIS-93).
     def shipments
-      @shipments ||= @integration.shipments.in_flight.where(id: @shipment_ids).to_a
+      @shipments ||= @integration.shipments.in_flight.where(id: @shipment_ids).order(:id).to_a
     end
 
     # Pares [envío, movimiento traducido] a registrar.

lefthook.yml

@@ -17,7 +17,10 @@ pre-push:
   parallel: true
   commands:
     rspec:
-      run: bundle exec rspec
+      # COVERAGE_FLOOR: acá se corre la suite entera, así que el piso de
+      # cobertura de spec_helper.rb tiene sentido. En una corrida parcial no se
+      # evalúa (ver el comentario ahí).
+      run: COVERAGE_FLOOR=1 bundle exec rspec
     branch-name:
       run: |
         branch=$(git rev-parse --abbrev-ref HEAD)

spec/avo/filters_spec.rb

@@ -0,0 +1,108 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+# Los filtros del panel de administración (TESIS-93).
+#
+# Son tres clases de tres líneas y ninguna tenía un ejemplo: si un filtro
+# ignorara el valor elegido, el panel mostraría la tabla entera y el
+# administrador no tendría cómo notarlo —una lista más larga de lo que pidió no
+# parece un error—.
+#
+# Se prueban por `apply`, que es el único método que Avo les llama, con la
+# consulta sin scopear: el aislamiento por empresa es de la API, y el backoffice
+# ve a propósito todas las empresas.
+RSpec.describe 'Avo filters', type: :model do
+  let(:company) { Company.create!(name: 'Norte', tax_id: '30-11111111-1') }
+  let(:other_company) { Company.create!(name: 'Sur', tax_id: '30-22222222-2') }
+  let(:couriers) { {} }
+
+  def order_for(a_company, status: 'pending')
+    Current.set(company_id: a_company.id) do
+      Order.create!(company: a_company, customer_name: 'Juan', status: status)
+    end
+  end
+
+  # Una sola integración por empresa: un envío por orden, y dos integraciones de
+  # la misma empresa pedirían dos couriers distintos.
+  def courier_of(a_company)
+    couriers[a_company.id] ||= Current.set(company_id: a_company.id) do
+      courier_integration(company: a_company, name: "Andreani #{a_company.id}")
+    end
+  end
+
+  def shipment_for(a_company, status: 'pending')
+    courier = courier_of(a_company)
+
+    Current.set(company_id: a_company.id) do
+      order = Order.create!(company: a_company, customer_name: 'Juan', status: 'paid')
+      Shipment.create!(company: a_company, order: order, company_integration: courier,
+                       status: status)
+    end
+  end
+
+  describe Avo::Filters::CompanyFilter do
+    subject(:filter) { described_class.new }
+
+    it 'keeps only the rows of the chosen company' do
+      mine = order_for(company)
+      order_for(other_company)
+
+      expect(filter.apply(nil, Order.unscoped, company.id).pluck(:id)).to eq([mine.id])
+    end
+
+    it 'leaves the query untouched when nothing is chosen' do
+      order_for(company)
+      order_for(other_company)
+
+      expect(filter.apply(nil, Order.unscoped, '').count).to eq(2)
+    end
+
+    it 'offers one option per company, by name', :aggregate_failures do
+      company
+      other_company
+
+      expect(filter.options.values).to include('Norte', 'Sur')
+    end
+  end
+
+  describe Avo::Filters::OrderStatusFilter do
+    subject(:filter) { described_class.new }
+
+    it 'keeps only the orders in the chosen status' do
+      paid = order_for(company, status: 'paid')
+      order_for(company, status: 'pending')
+
+      expect(filter.apply(nil, Order.unscoped, 'paid').pluck(:id)).to eq([paid.id])
+    end
+
+    it 'leaves the query untouched when nothing is chosen' do
+      order_for(company, status: 'paid')
+      order_for(company, status: 'pending')
+
+      expect(filter.apply(nil, Order.unscoped, nil).count).to eq(2)
+    end
+
+    it 'offers exactly the statuses an order can be in' do
+      expect(filter.options.keys).to match_array(Order::STATUSES)
+    end
+  end
+
+  describe Avo::Filters::ShipmentStatusFilter do
+    subject(:filter) { described_class.new }
+
+    it 'keeps only the shipments in the chosen status' do
+      in_transit = shipment_for(company, status: 'in_transit')
+      shipment_for(company, status: 'pending')
+
+      expect(filter.apply(nil, Shipment.unscoped, 'in_transit').pluck(:id)).to eq([in_transit.id])
+    end
+
+    it 'leaves the query untouched when nothing is chosen' do
+      shipment_for(company, status: 'in_transit')
+      shipment_for(company, status: 'pending')
+
+      expect(filter.apply(nil, Shipment.unscoped, '').count).to eq(2)
+    end
+  end
+end

spec/jobs/shipments/scan_pull_tracking_job_spec.rb

@@ -104,6 +104,24 @@ def run = described_class.new.perform
       .with(other.company_integration_id, [other.id], company_b.id)
   end
 
+  # Una integración con plantilla de seguimiento y sin nada en vuelo: el barrido
+  # la encuentra y tiene que salir sin encolar. Sin este ejemplo, dividir por la
+  # cantidad de grupos con la lista vacía —una división por cero— no la veía
+  # nadie (TESIS-93).
+  it 'enqueues nothing for a courier with no shipment in flight' do
+    integration_for(company_a)
+
+    expect { run }.not_to have_enqueued_job(Shipments::PollTrackingJob)
+  end
+
+  it 'still sweeps the couriers that do have shipments in flight' do
+    integration_for(company_a)
+    other = shipment_for(integration_for(company_b), 'CB-1')
+
+    expect { run }.to have_enqueued_job(Shipments::PollTrackingJob)
+      .with(other.company_integration_id, [other.id], company_b.id)
+  end
+
   it 'ignores couriers without a tracking template' do
     shipment_for(integration_for(company_a, service: courier_service('Andreani')), 'AND-1')
     expect { run }.not_to have_enqueued_job(Shipments::PollTrackingJob)

spec/models/stock_transfer_spec.rb

@@ -45,6 +45,40 @@ def other_company
   # `validate: true` en el enum existe para que un estado desconocido invalide
   # el registro en lugar de explotar en el asignador (mismo criterio que
   # WebhookLog y FailedEvent).
+  # Las dos validaciones cruzadas arrancan con un guard: sin depósitos, o sin
+  # empresa, no hay nada que comparar y la que tiene que hablar es la
+  # presencia. Sin estos ejemplos el guard no lo ejercitaba nadie, y quitarlo
+  # —que convierte la falta de un depósito en un error confuso sobre el otro—
+  # no rompía la suite (TESIS-93).
+  context 'when a warehouse is missing' do
+    before { transfer.origin_warehouse = nil }
+
+    it 'is invalid' do
+      expect(transfer).not_to be_valid
+    end
+
+    it 'complains about the missing warehouse and not about the two being equal' do
+      transfer.valid?
+
+      expect(transfer.errors[:destination_warehouse]).to be_empty
+    end
+  end
+
+  context 'when the transfer has no company' do
+    before { transfer.company = nil }
+
+    it 'is invalid' do
+      expect(transfer).not_to be_valid
+    end
+
+    it 'does not blame the product for belonging elsewhere', :aggregate_failures do
+      transfer.valid?
+
+      expect(transfer.errors[:product]).to be_empty
+      expect(transfer.errors[:origin_warehouse]).to be_empty
+    end
+  end
+
   it 'rejects an unknown status without raising on assignment' do
     transfer.status = 'lost'
     expect(transfer).not_to be_valid

spec/models/webhook_log_spec.rb

@@ -40,6 +40,17 @@
   # Sin esta validación, un Current heredado haría que assign_current_company
   # pisara el company_id explícito y el log terminara en el tenant equivocado
   # en silencio. Acá falla fuerte en vez de escribir mal.
+  # El guard de la validación cruzada: sin integración no hay con qué comparar,
+  # y quien tiene que hablar es la presencia de la asociación. Nadie lo
+  # ejercitaba (TESIS-93).
+  it 'does not blame the integration when there is none', :aggregate_failures do
+    log = described_class.new(company: company)
+
+    log.valid?
+
+    expect(log.errors[:company_integration]).not_to include('must belong to the same company')
+  end
+
   it 'refuses to write under a leaked tenant instead of doing it silently' do
     integration
     Current.company_id = Company.create!(name: 'Intruso', tax_id: '30-88888888-8').id

spec/policies/application_policy_spec.rb

@@ -0,0 +1,89 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+# La base de la que heredan todas las policies (TESIS-93).
+#
+# Lo que se prueba acá no es una regla de negocio sino una postura: **nada está
+# permitido salvo que una policy concreta lo permita**. Cada uno de estos
+# defaults es lo que va a contestar una acción nueva cuya policy todavía no se
+# escribió, o un recurso al que alguien le agregó un endpoint y se olvidó del
+# permiso. Que respondan `false` es la diferencia entre un permiso faltante que
+# se nota y uno que se filtra.
+#
+# Ninguna policy real ejercita estos métodos —todas los pisan—, así que sin
+# estos ejemplos el comportamiento por defecto no lo verifica nadie.
+RSpec.describe ApplicationPolicy do
+  subject(:policy) { described_class.new(user, record) }
+
+  let(:company) { Company.create!(name: 'Acme', tax_id: '20-12345678-9') }
+  let(:user) { User.create!(email: 'a@acme.com', password: 'pass123', company: company) }
+  let(:record) { Product.new(company: company, sku: 'SKU-1', name: 'Widget') }
+
+  describe 'the default answer to every action' do
+    it 'refuses to list', :aggregate_failures do
+      expect(policy.index?).to be(false)
+      expect(policy.show?).to be(false)
+    end
+
+    it 'refuses to write', :aggregate_failures do
+      expect(policy.create?).to be(false)
+      expect(policy.update?).to be(false)
+      expect(policy.destroy?).to be(false)
+    end
+
+    # Con un usuario presente y un registro de su propia empresa: la negativa no
+    # depende de que falte el usuario, es el default.
+    it 'refuses even to an authenticated user over a record of their own company' do
+      expect(policy.index?).to be(false)
+    end
+  end
+
+  # `new?` y `edit?` no tienen regla propia: son las vistas de `create?` y
+  # `update?`. Si alguien redefine `create?` y se olvida de `new?`, esto fija
+  # que no hacía falta acordarse.
+  describe 'the aliases of the write actions' do
+    it 'answers new? with create?' do
+      expect(policy.new?).to eq(policy.create?)
+    end
+
+    it 'answers edit? with update?' do
+      expect(policy.edit?).to eq(policy.update?)
+    end
+
+    context 'when a subclass allows creating and updating' do
+      let(:permissive) do
+        Class.new(described_class) do
+          def create? = true
+          def update? = true
+        end
+      end
+
+      it 'follows the subclass instead of the default', :aggregate_failures do
+        subclass_policy = permissive.new(user, record)
+
+        expect(subclass_policy.new?).to be(true)
+        expect(subclass_policy.edit?).to be(true)
+      end
+    end
+  end
+
+  describe ApplicationPolicy::Scope do
+    # Un Scope que no define `resolve` no devuelve "todo": explota. Es el otro
+    # lado de la misma postura — una policy a medio escribir falla a la vista y
+    # no filtra la tabla entera del tenant equivocado.
+    it 'refuses to resolve a scope that did not define it' do
+      scope = described_class.new(user, Product.all)
+
+      expect { scope.resolve }.to raise_error(NoMethodError, /must define #resolve/)
+    end
+
+    it 'names the class that has to define it' do
+      incomplete = Class.new(described_class)
+      stub_const('IncompleteScope', incomplete)
+
+      expect { incomplete.new(user, Product.all).resolve }
+        .to raise_error(NoMethodError, /IncompleteScope/)
+    end
+  end
+end

spec/policies/product_mapping_policy_spec.rb

@@ -38,6 +38,18 @@ def foreign_product
     expect(policy).to be_destroy
   end
 
+  # `record.product` puede venir en nil: CompanyScoped filtra también las
+  # asociaciones, así que un mapping de otro tenant llega sin producto. El `&.`
+  # es lo que evita el NoMethodError, y su rama nil no la ejercitaba nadie
+  # (TESIS-93).
+  context 'when the mapped product is out of the scope of the tenant' do
+    before { allow(mapping).to receive(:product).and_return(nil) }
+
+    it 'denies destroying it instead of raising' do
+      expect(policy).not_to be_destroy
+    end
+  end
+
   context 'when the mapped product belongs to another company' do
     let(:mapping) { mapping_for(foreign_product, 'Shopify') }