From afff55e80913cfc38030fcc8b397c1a2d6aa1ff2 Mon Sep 17 00:00:00 2001 From: Simon Lejeune Date: Thu, 10 Aug 2017 18:45:16 +0200 Subject: [PATCH] [FIX] stock_account: average and negative stock Do not inline the average price calculation in `action_done`, as there's a computed field that handle correctly the negative stock (does not trigger division by 0). Also, complete this compute method by also looking on transit locations and implement two fallbacks if the quantity in stock is null: return the latest cost, if not found return the standard price. Also, make sure that `remaining_qty` is only set on fifo moves. --- addons/stock_account/models/product.py | 18 +++- addons/stock_account/models/stock.py | 15 +-- .../tests/test_stockvaluation.py | 98 ++++++++++++++++++- 3 files changed, 114 insertions(+), 17 deletions(-) diff --git a/addons/stock_account/models/product.py b/addons/stock_account/models/product.py index b3ecdc62dc7..20c39f6f367 100644 --- a/addons/stock_account/models/product.py +++ b/addons/stock_account/models/product.py @@ -3,7 +3,7 @@ from odoo import api, fields, models, tools, _ from odoo.exceptions import UserError -from odoo.tools import float_is_zero +from odoo.tools import float_is_zero, pycompat from odoo.addons import decimal_precision as dp @@ -183,14 +183,22 @@ class ProductProduct(models.Model): return candidates @api.multi + @api.depends('stock_move_ids.product_qty', 'stock_move_ids.state') def _compute_average_price(self): - for product in self: + """ Compute the average price of a product.product according to the latest cumulated value + and the quantity that is in stock (for all the company owned locations, i.e. including the + warehouses transit ones). If nothing is in stick, we fallback first on the latest average + price on an out move used for this product then on the standard price. + """ + for p_set, product in pycompat.izip(self, self.with_context(company_owned=True)): if product.qty_available > 0: last_cumulated_value = product._get_latest_cumulated_value() - product.average_price = last_cumulated_value / product.qty_available + p_set.average_price = last_cumulated_value / product.qty_available else: - product.average_price = 0 - + domain = [('product_id', '=', product.id), ('last_done_qty', '>', 0.0)] + self.env['stock.move']._get_all_base_domain() + move = self.env['stock.move'].search(domain, order='date desc, id desc', limit=1) + p_set.average_price = move and move.cumulated_value / move.last_done_qty or product.standard_price + @api.multi def _compute_stock_value(self): for product in self: diff --git a/addons/stock_account/models/stock.py b/addons/stock_account/models/stock.py index f314446d777..10da8ee47e7 100644 --- a/addons/stock_account/models/stock.py +++ b/addons/stock_account/models/stock.py @@ -218,11 +218,11 @@ class StockMove(models.Model): @api.multi def action_done(self): - qty_available = {} - for move in self: - #Should write move.price_unit here maybe, certainly on incoming - if move.product_id.cost_method == 'average': - qty_available[move.product_id.id] = move.product_id.with_context(company_owned=True).qty_available + average_price = {} + for move in self.filtered(lambda m: m.product_id.cost_method == 'average'): + # For the average costing method, the value depends of the quantity in stock at the + # moment of the transfer, so fetch the value before calling super. + average_price[move.product_id.id] = move.product_id.average_price res = super(StockMove, self).action_done() for move in res: if move._is_in(): @@ -231,8 +231,9 @@ class StockMove(models.Model): move.price_unit = move._get_price_unit() move.value = move.price_unit * move.product_qty move.cumulated_value = move.product_id._get_latest_cumulated_value(exclude_move=move) + move.value - move.remaining_qty = move.product_qty if move.product_id.cost_method == 'fifo': + move.remaining_qty = move.product_qty + # If there are some OUT moves that we could not value in time, find them # and value them now with this new IN move. After that, update the cumulated # value along the moves done after the newly valued move. @@ -281,7 +282,7 @@ class StockMove(models.Model): move.last_done_qty = move.product_id.qty_available elif move.product_id.cost_method == 'average': curr_rounding = move.company_id.currency_id.rounding - avg_price_unit = float_round(move.product_id._get_latest_cumulated_value(exclude_move=move) / qty_available[move.product_id.id], precision_rounding=curr_rounding) + avg_price_unit = average_price[move.product_id.id] move.value = float_round(-avg_price_unit * move.product_qty, precision_rounding=curr_rounding) move.remaining_qty = 0 move.cumulated_value = move.product_id._get_latest_cumulated_value(exclude_move=move) + move.value diff --git a/addons/stock_account/tests/test_stockvaluation.py b/addons/stock_account/tests/test_stockvaluation.py index 965801c6eed..d5178fd7ca9 100644 --- a/addons/stock_account/tests/test_stockvaluation.py +++ b/addons/stock_account/tests/test_stockvaluation.py @@ -386,8 +386,7 @@ class TestStockValuation(TransactionCase): self.assertEqual(move1.value, 900.0) self.assertEqual(move1.cumulated_value, 900.0) - - self.assertEqual(move1.remaining_qty, 60.0) + self.assertEqual(move1.remaining_qty, 0.0) # unusedin average move # Purchase 140 units @ 15.50 per unit move2 = self.env['stock.move'].create({ @@ -407,7 +406,7 @@ class TestStockValuation(TransactionCase): self.assertEqual(move2.value, 2170.0) self.assertEqual(move2.cumulated_value, 3070.0) - # Sale 190 units @ 15.00 per unit + # Sale 190 units @ 15.35 per unit move3 = self.env['stock.move'].create({ 'name': 'Sale 190 units @ 19.00 per unit', 'location_id': self.stock_location.id, @@ -456,7 +455,7 @@ class TestStockValuation(TransactionCase): move5.action_confirm() move5.action_assign() move5.move_line_ids.qty_done = 30.0 - move5.action_done() + move5.with_context(debug=True).action_done() self.assertEqual(move5.value, -477.6) self.assertEqual(move5.cumulated_value, 795.9) # fuck you, rounding @@ -505,7 +504,7 @@ class TestStockValuation(TransactionCase): })] }) move2.action_confirm() - move2.with_context(debug=True).action_done() + move2.action_done() self.assertEqual(move1.value, -600.0) self.assertEqual(move1.cumulated_value, -600.0) @@ -542,3 +541,92 @@ class TestStockValuation(TransactionCase): self.assertEqual(move3.value, 500.0) self.assertEqual(move3.remaining_qty, 10.0) self.assertEqual(move3.cumulated_value, 250.0) + + def test_average_negative_1(self): + """ Send goods that you don't have in stock and never received any unit. + """ + self.product1.product_tmpl_id.cost_method = 'average' + + # set a standard price + self.product1.standard_price = 99 + + # send 10 units that we do not have + self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product1, self.stock_location), 0) + move1 = self.env['stock.move'].create({ + 'name': 'test_average_negative_1', + 'location_id': self.stock_location.id, + 'location_dest_id': self.customer_location.id, + 'product_id': self.product1.id, + 'product_uom': self.uom_unit.id, + 'product_uom_qty': 10.0, + }) + move1.action_confirm() + move1.force_assign() + move1.quantity_done = 10.0 + move1.action_done() + self.assertEqual(move1.value, -990.0) # as no move out were done for this product, fallback on the standard price + self.assertEqual(move1.cumulated_value, -990.0) + self.assertEqual(move1.remaining_qty, 0.0) # unused in average move + + def test_average_negative_2(self): + """ Send goods that you don't have in stock but received and send some units before. + """ + self.product1.product_tmpl_id.cost_method = 'average' + + # set a standard price + self.product1.standard_price = 99 + + # Receives 10 produts at 10 + move1 = self.env['stock.move'].create({ + 'name': '68 units @ 15.00 per unit', + 'location_id': self.supplier_location.id, + 'location_dest_id': self.stock_location.id, + 'product_id': self.product1.id, + 'product_uom': self.uom_unit.id, + 'product_uom_qty': 10.0, + 'price_unit': 10, + }) + move1.action_confirm() + move1.action_assign() + move1.move_line_ids.qty_done = 10.0 + move1.action_done() + + self.assertEqual(move1.value, 100.0) + self.assertEqual(move1.cumulated_value, 100.0) + self.assertEqual(move1.remaining_qty, 0.0) # unused in average move + + # send 10 products + move2 = self.env['stock.move'].create({ + 'name': 'Sale 94 units @ 19.00 per unit', + 'location_id': self.stock_location.id, + 'location_dest_id': self.customer_location.id, + 'product_id': self.product1.id, + 'product_uom': self.uom_unit.id, + 'product_uom_qty': 10.0, + }) + move2.action_confirm() + move2.action_assign() + move2.move_line_ids.qty_done = 10.0 + move2.action_done() + + self.assertEqual(move2.value, -100.0) + self.assertEqual(move2.cumulated_value, 0.0) + self.assertEqual(move2.remaining_qty, 0.0) # unused in average move + + # send 10 products again + move3 = self.env['stock.move'].create({ + 'name': 'Sale 94 units @ 19.00 per unit', + 'location_id': self.stock_location.id, + 'location_dest_id': self.customer_location.id, + 'product_id': self.product1.id, + 'product_uom': self.uom_unit.id, + 'product_uom_qty': 10.0, + }) + move3.action_confirm() + move3.force_assign() + move3.quantity_done = 10.0 + move3.action_done() + + self.assertEqual(move3.value, -100.0) # as no move out were done for this product, fallback on latest cost + self.assertEqual(move3.cumulated_value, -100.0) + self.assertEqual(move3.remaining_qty, 0.0) # unused in average move