Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion app/poros/orders/replace_order_lines.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
24 changes: 20 additions & 4 deletions app/poros/products/concerns/warehouse_validation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
17 changes: 17 additions & 0 deletions spec/requests/api/v1/order_updates_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
42 changes: 42 additions & 0 deletions spec/requests/api/v1/products_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
Loading