diff --git a/app/poros/orders/replace_order_lines.rb b/app/poros/orders/replace_order_lines.rb index 7e34fb0c..bb6bd4db 100644 --- a/app/poros/orders/replace_order_lines.rb +++ b/app/poros/orders/replace_order_lines.rb @@ -115,8 +115,10 @@ def validate_lines_without_warehouse! # Warehouse es CompanyScoped, pero fuera de un request Current puede estar # en nil y el scope no aplica: el company_id va explícito, como en el alta. + # Enteros antes de comparar: `["1", 1]` son el mismo depósito y, crudos, + # contaban como dos y daban un 422 falso de «no pertenece a esta empresa». def validate_new_warehouses! - ids = new_items.pluck(:warehouse_id).uniq + ids = new_items.map { |item| item[:warehouse_id].to_s.to_i }.uniq return if Warehouse.where(id: ids, company_id: @order.company_id).count == ids.size raise ActiveRecord::RecordNotSaved, 'One or more warehouses do not belong to this company' diff --git a/app/poros/products/concerns/warehouse_validation.rb b/app/poros/products/concerns/warehouse_validation.rb index 82815c8f..48252090 100644 --- a/app/poros/products/concerns/warehouse_validation.rb +++ b/app/poros/products/concerns/warehouse_validation.rb @@ -14,15 +14,31 @@ module WarehouseValidation # revierte todo). Warehouse.where(id:) ya filtra por empresa vía el # default_scope de CompanyScoped, así que no hace falta repetir company_id. def validate_warehouses_belong_to_company! - # rubocop:disable-next Rails/Pluck - warehouse_ids = @stocks_params.map { |s| s[:warehouse_id] }.uniq - owned = Warehouse.where(id: warehouse_ids).pluck(:id) - return if owned.sort == warehouse_ids.sort + warehouse_ids = normalized_warehouse_ids! + return if Warehouse.where(id: warehouse_ids).count == warehouse_ids.size # Mensaje genérico a propósito: no exponer IDs de depósitos de otro tenant. raise ActiveRecord::RecordNotSaved, 'One or more warehouses do not belong to this company' end + + # Los ids del request como enteros únicos. Antes se comparaban crudos: una + # fila sin `warehouse_id` hacía reventar el `sort` (nil contra Integer, un + # 500), y ids que llegaban como texto ("5") no coincidían con los de la base + # y daban un 422 falso de «no pertenece a esta empresa». + def normalized_warehouse_ids! + @stocks_params.map.with_index { |stock, index| warehouse_id_of!(stock, index) }.uniq + end + + def warehouse_id_of!(stock, index) + value = stock[:warehouse_id] + id = value if value.is_a?(Integer) + id ||= value.to_i if value.is_a?(String) && value.match?(/\A\d+\z/) + return id if id&.positive? + + raise ActiveRecord::RecordNotSaved, + "stocks[#{index}]: warehouse_id must be a positive integer" + end end end end diff --git a/spec/requests/api/v1/order_updates_spec.rb b/spec/requests/api/v1/order_updates_spec.rb index 089ad883..23c97da4 100644 --- a/spec/requests/api/v1/order_updates_spec.rb +++ b/spec/requests/api/v1/order_updates_spec.rb @@ -283,4 +283,21 @@ def foreign_order expect(order.reload.customer_name).to eq('Juan Pérez') end end + + # Hallazgo de auditoría (TESIS-89): el mismo depósito en texto y en número + # contaba como dos y daba un 422 falso. + def stocked_second_product + Product.create!(company: company, sku: 'CEL-2', name: 'Funda').tap do |second| + Stock.create!(product: second, warehouse: warehouse, quantity: 5) + end + end + + it 'accepts new lines that name the same warehouse as text and as a number' do + second = stocked_second_product + put_order(items: [{ id: line.id, quantity: 4 }, + { product_id: second.id, warehouse_id: warehouse.id.to_s, quantity: 1, unit_price: 5 }, + { product_id: product.id, warehouse_id: warehouse.id, quantity: 1, unit_price: 100 }]) + + expect(response).to have_http_status(:ok) + end end diff --git a/spec/requests/api/v1/products_spec.rb b/spec/requests/api/v1/products_spec.rb index 439bebbd..09eebe8d 100644 --- a/spec/requests/api/v1/products_spec.rb +++ b/spec/requests/api/v1/products_spec.rb @@ -977,6 +977,48 @@ def put_stock(quantity) end end + # Hallazgo de auditoría (TESIS-89): las filas de `stocks` se comparaban crudas + # contra los ids de la base. Una fila sin depósito hacía reventar el `sort` + # (500), y un id en texto daba un 422 falso de «no pertenece a la empresa». + describe 'the warehouse of each stock row' do + let(:product) { Product.create!(company: company, sku: 'ROW-001', name: 'Filas') } + let(:central) do + Warehouse.create!(company: company, name: 'Central', zip_code: '1900', address: 'Calle 1') + end + + def put_stocks(stocks) + put "/api/v1/products/#{product.id}", + params: { product: { name: 'Filas', stocks: stocks } }, headers: headers, as: :json + end + + it 'answers 422 and names the row that has no warehouse', :aggregate_failures do + put_stocks([{ warehouse_id: central.id, quantity: 1 }, { quantity: 3 }]) + + expect(response).to have_http_status(:unprocessable_content) + expect(response.parsed_body['error']).to eq('stocks[1]: warehouse_id must be a positive integer') + end + + it 'accepts a warehouse id that comes as text', :aggregate_failures do + put_stocks([{ warehouse_id: central.id.to_s, quantity: 4 }]) + + expect(response).to have_http_status(:ok) + expect(Stock.find_by(product: product, warehouse: central).quantity).to eq(4) + end + + it 'still refuses a warehouse of another company' do + put_stocks([{ warehouse_id: other_warehouse.id, quantity: 1 }]) + + expect(response.parsed_body['error']).to eq('One or more warehouses do not belong to this company') + end + + it 'refuses the same row on creation too' do + post '/api/v1/products', params: { product: { sku: 'ROW-002', name: 'Nuevo', stocks: [{ quantity: 1 }] } }, + headers: headers, as: :json + + expect(response).to have_http_status(:unprocessable_content) + end + end + describe 'PATCH /api/v1/products/:id' do let!(:product) do Product.create!(company: company, sku: 'PROD-001', name: 'Original')