diff --git a/addons/stock_account/models/stock.py b/addons/stock_account/models/stock.py index a128f5a6942421d55c5be2f3be21db4731406d17..284fde84d75dc45794e3ad20000a5ae617f37986 100644 --- a/addons/stock_account/models/stock.py +++ b/addons/stock_account/models/stock.py @@ -71,37 +71,45 @@ class StockMoveLine(models.Model): if move.product_id.valuation == 'real_time' and (move._is_in() or move._is_out()): move.with_context(force_valuation_amount=correction_value)._account_entry_move() return res - + @api.multi def write(self, vals): + """ When editing a done stock.move.line, we impact the valuation. Users may increase or + decrease the `qty_done` field. There are three cost method available: standard, average + and fifo. We implement the logic in a similar way for standard and average: increase + or decrease the original value with the standard or average price of today. In fifo, we + have a different logic wheter the move is incoming or outgoing. If the move is incoming, we + update the value and remaining_value/qty with the unit price of the move. If the move is + outgoing and the user increases qty_done, we call _run_fifo and it'll consume layer(s) in + the stack the same way a new outgoing move would have done. If the move is outoing and the + user decreases qty_done, we either increase the last receipt candidate if one is found or + we decrease the value with the last fifo price. + """ if 'qty_done' in vals: - # We need to update the `value`, `remaining_value` and `remaining_qty` on the linked - # stock move. moves_to_update = {} for move_line in self.filtered(lambda ml: ml.state == 'done' and (ml.move_id._is_in() or ml.move_id._is_out())): moves_to_update[move_line.move_id] = vals['qty_done'] - move_line.qty_done for move_id, qty_difference in moves_to_update.items(): - # more/less units are available, update `remaining_value` and - # `remaining_qty` on the linked stock move. - move_vals = {'remaining_qty': move_id.remaining_qty + qty_difference} - new_remaining_value = 0 + move_vals = {} if move_id.product_id.cost_method in ['standard', 'average']: correction_value = qty_difference * move_id.product_id.standard_price - move_vals['value'] = move_id.value - correction_value - move_vals.pop('remaining_qty') + if move_id._is_in(): + move_vals['value'] = move_id.value + correction_value + elif move_id._is_out(): + move_vals['value'] = move_id.value - correction_value else: - # FIFO handling if move_id._is_in(): correction_value = qty_difference * move_id.price_unit new_remaining_value = move_id.remaining_value + correction_value + move_vals['value'] = move_id.value + correction_value + move_vals['remaining_qty'] = move_id.remaining_qty + qty_difference + move_vals['remaining_value'] = move_id.remaining_value + correction_value elif move_id._is_out() and qty_difference > 0: - # send more, run fifo again correction_value = self.env['stock.move']._run_fifo(move_id, quantity=qty_difference) - new_remaining_value = move_id.remaining_value + correction_value - move_vals.pop('remaining_qty') + # no need to adapt `remaining_qty` and `remaining_value` as `_run_fifo` took care of it + move_vals['value'] = move_id.value - correction_value elif move_id._is_out() and qty_difference < 0: - # fake return, find the last receipt and augment its qties candidates_receipt = self.env['stock.move'].search(move_id._get_in_domain(), order='date, id desc', limit=1) if candidates_receipt: candidates_receipt.write({ @@ -111,11 +119,7 @@ class StockMoveLine(models.Model): correction_value = qty_difference * candidates_receipt.price_unit else: correction_value = qty_difference * move_id.product_id.standard_price - move_vals.pop('remaining_qty') - if move_id._is_out(): - move_vals['remaining_value'] = new_remaining_value if new_remaining_value < 0 else 0 - else: - move_vals['remaining_value'] = new_remaining_value + move_vals['value'] = move_id.value - correction_value move_id.write(move_vals) if move_id.product_id.valuation == 'real_time': @@ -210,13 +214,23 @@ class StockMove(models.Model): @api.model def _run_fifo(self, move, quantity=None): + """ Value `move` according to the FIFO rule, meaning we consume the + oldest receipt first. Candidates receipts are marked consumed or free + thanks to their `remaining_qty` and `remaining_value` fields. + By definition, `move` should be an outgoing stock move. + + :param quantity: quantity to value instead of `move.product_qty` + :returns: valued amount in absolute + """ move.ensure_one() - # Find back incoming stock moves (called candidates here) to value this move. + + # Deal with possible move lines that do not impact the valuation. valued_move_lines = move.move_line_ids.filtered(lambda ml: ml.location_id._should_be_valued() and not ml.location_dest_id._should_be_valued() and not ml.owner_id) valued_quantity = 0 for valued_move_line in valued_move_lines: valued_quantity += valued_move_line.product_uom_id._compute_quantity(valued_move_line.qty_done, move.product_id.uom_id) + # Find back incoming stock moves (called candidates here) to value this move. qty_to_take_on_candidates = quantity or valued_quantity candidates = move.product_id._get_fifo_candidates_in_move() new_standard_price = 0 @@ -259,11 +273,12 @@ class StockMove(models.Model): elif qty_to_take_on_candidates > 0: last_fifo_price = new_standard_price or move.product_id.standard_price negative_stock_value = last_fifo_price * -qty_to_take_on_candidates + tmp_value += abs(negative_stock_value) vals = { 'remaining_qty': move.remaining_qty + -qty_to_take_on_candidates, 'remaining_value': move.remaining_value + negative_stock_value, - 'value': -tmp_value + negative_stock_value, - 'price_unit': (-tmp_value + negative_stock_value) / (move.product_qty or quantity), + 'value': -tmp_value, + 'price_unit': -1 * last_fifo_price, } move.write(vals) return tmp_value @@ -388,12 +403,14 @@ class StockMove(models.Model): if not candidates: continue qty_to_take_on_candidates = abs(move.remaining_qty) + qty_taken_on_candidates = 0 tmp_value = 0 for candidate in candidates: if candidate.remaining_qty <= qty_to_take_on_candidates: qty_taken_on_candidate = candidate.remaining_qty else: qty_taken_on_candidate = qty_to_take_on_candidates + qty_taken_on_candidates += qty_taken_on_candidate value_taken_on_candidate = qty_taken_on_candidate * candidate.price_unit candidate_vals = { @@ -407,27 +424,22 @@ class StockMove(models.Model): if qty_to_take_on_candidates == 0: break - remaining_value_before_vacuum = move.remaining_value - # If `remaining_qty` should be updated to 0, we wipe `remaining_value`. If it was set - # it was only used to infer the correction entry anyway. - new_remaining_qty = -qty_to_take_on_candidates - new_remaining_value = 0 if not new_remaining_qty else move.remaining_value + tmp_value + # When working with `price_unit`, beware that out move are negative. + move_price_unit = move.price_unit if move._is_out() else -1 * move.price_unit + # Get the estimated value we will correct. + remaining_value_before_vacuum = qty_taken_on_candidates * move_price_unit + new_remaining_qty = move.remaining_qty + qty_taken_on_candidates + new_remaining_value = new_remaining_qty * abs(move.price_unit) + + corrected_value = remaining_value_before_vacuum + tmp_value move.write({ 'remaining_value': new_remaining_value, 'remaining_qty': new_remaining_qty, + 'value': move.value - corrected_value, }) if move.product_id.valuation == 'real_time': - # If `move.remaining_value` is negative, it means that we initially valued this move at - # an estimated price *and* posted an entry. `tmp_value` is the real value we took to - # compensate and should always be positive, but if the remaining value is still negative - # we have to take care to not overvalue by decreasing the correction entry by what's - # already been posted. - corrected_value = tmp_value - if remaining_value_before_vacuum < 0: - corrected_value += remaining_value_before_vacuum - # If `corrected_value` is 0, absolutely do *not* call `_account_entry_move`. We # force the amount in the context, but in the case it is 0 it'll create an entry # for the entire cost of the move. This case happens when the candidates moves diff --git a/addons/stock_account/tests/test_stockvaluation.py b/addons/stock_account/tests/test_stockvaluation.py index 29fa3d737617317ceaa28f35cddf14541b07e9a3..5d20a17f2bf2949f7c86f7dc6aa3fb98b116239d 100644 --- a/addons/stock_account/tests/test_stockvaluation.py +++ b/addons/stock_account/tests/test_stockvaluation.py @@ -243,7 +243,7 @@ class TestStockValuation(TransactionCase): self.assertEqual(move1.product_uom_qty, 12.0) self.assertEqual(move1.price_unit, 10.0) self.assertEqual(move1.remaining_qty, 9.0) - self.assertEqual(move1.value, 100.0) + self.assertEqual(move1.value, 120.0) # move 1 is now 10@10 + 2@10 self.assertEqual(move1.remaining_value, 90.0) # account values for move1 @@ -326,7 +326,7 @@ class TestStockValuation(TransactionCase): self.assertEqual(move1.product_uom_qty, 12) self.assertEqual(move1.price_unit, 10.0) self.assertEqual(move1.remaining_qty, 0) - self.assertEqual(move1.value, 100) + self.assertEqual(move1.value, 120) self.assertEqual(move1.remaining_value, 0) self.assertEqual(move2.product_uom_qty, 10.0) self.assertEqual(move2.price_unit, 8.0) @@ -388,7 +388,7 @@ class TestStockValuation(TransactionCase): self.assertEqual(move1.product_uom_qty, 12) self.assertEqual(move1.price_unit, 10.0) self.assertEqual(move1.remaining_qty, 0) - self.assertEqual(move1.value, 100) + self.assertEqual(move1.value, 120) self.assertEqual(move1.remaining_value, 0) self.assertEqual(move2.product_uom_qty, 10.0) self.assertEqual(move2.price_unit, 8.0) @@ -457,7 +457,7 @@ class TestStockValuation(TransactionCase): self.assertEqual(move1.product_uom_qty, 12) self.assertEqual(move1.price_unit, 10.0) self.assertEqual(move1.remaining_qty, 0) - self.assertEqual(move1.value, 100) + self.assertEqual(move1.value, 120) self.assertEqual(move1.remaining_value, 0) self.assertEqual(move2.product_uom_qty, 10.0) self.assertEqual(move2.price_unit, 8.0) @@ -505,7 +505,7 @@ class TestStockValuation(TransactionCase): self.assertEqual(move1.product_uom_qty, 12) self.assertEqual(move1.price_unit, 10.0) self.assertEqual(move1.remaining_qty, 0) - self.assertEqual(move1.value, 100) + self.assertEqual(move1.value, 120) self.assertEqual(move1.remaining_value, 0) self.assertEqual(move2.product_uom_qty, 10.0) self.assertEqual(move2.price_unit, 8.0) @@ -522,7 +522,11 @@ class TestStockValuation(TransactionCase): self.assertEqual(move4.remaining_value, 0) self.assertEqual(move5.product_uom_qty, 20.0) self.assertEqual(move5.remaining_qty, 0.0) - self.assertEqual(move5.value, -160.0) + + # move5 sent 10@8 and 10@estimated price of 8 + # the vacuum compensated the 10@8 by 12@10 + # -(10*8 + 10@12) = -200 + self.assertEqual(move5.value, -200.0) self.assertEqual(move5.remaining_value, 0.0) self.assertEqual(move6.product_uom_qty, 10) self.assertEqual(move6.price_unit, 12.0) @@ -553,7 +557,7 @@ class TestStockValuation(TransactionCase): self.assertEqual(move6.product_uom_qty, 8) self.assertEqual(move6.price_unit, 12) self.assertEqual(move6.remaining_qty, -2) - self.assertEqual(move6.value, 120) + self.assertEqual(move6.value, 96) # move6 is now 8@12 self.assertEqual(move6.remaining_value, -24) # account values for move1 @@ -634,7 +638,7 @@ class TestStockValuation(TransactionCase): self.assertEqual(move1.product_uom_qty, 12) self.assertEqual(move1.price_unit, 10.0) self.assertEqual(move1.remaining_qty, 0) - self.assertEqual(move1.value, 100) + self.assertEqual(move1.value, 120) self.assertEqual(move1.remaining_value, 0) self.assertEqual(move2.product_uom_qty, 10.0) self.assertEqual(move2.price_unit, 8.0) @@ -651,12 +655,12 @@ class TestStockValuation(TransactionCase): self.assertEqual(move4.remaining_value, 0) self.assertEqual(move5.product_uom_qty, 20.0) self.assertEqual(move5.remaining_qty, 0.0) - self.assertEqual(move5.value, -160.0) + self.assertEqual(move5.value, -200.0) self.assertEqual(move5.remaining_value, 0.0) self.assertEqual(move6.product_uom_qty, 8) self.assertEqual(move6.price_unit, 12.0) self.assertEqual(move6.remaining_qty, 0.0) - self.assertEqual(move6.value, 120) + self.assertEqual(move6.value, 90) self.assertEqual(move6.remaining_value, 0) self.assertEqual(move7.product_uom_qty, 4.0) self.assertEqual(move7.price_unit, 15.0) @@ -681,14 +685,28 @@ class TestStockValuation(TransactionCase): # --------------------------------------------------------------------- # Ending # --------------------------------------------------------------------- + # check on remaining_qty self.assertEqual(self.product1.qty_available, 2) + # check on remaining_value self.assertEqual(self.product1.stock_value, 30) + # check on accounting entries self.assertEqual(sum(self._get_stock_input_move_lines().mapped('debit')), 30) self.assertEqual(sum(self._get_stock_input_move_lines().mapped('credit')), 380) self.assertEqual(sum(self._get_stock_valuation_move_lines().mapped('debit')), 380) self.assertEqual(sum(self._get_stock_valuation_move_lines().mapped('credit')), 350) self.assertEqual(sum(self._get_stock_output_move_lines().mapped('debit')), 320) self.assertEqual(sum(self._get_stock_output_move_lines().mapped('credit')), 0) + moves = move1 + move2 + move3 + move4 + move5 + move6 + move7 + # check on value + self.assertEqual(sum(moves.mapped('value')), 30) + # check on product_qty + qty = 0 + for move in moves: + if move._is_in(): + qty += move.product_qty + else: + qty -= move.product_qty + self.assertEqual(qty, 2) def test_fifo_perpetual_2(self): # http://accountingexplained.com/financial/inventories/fifo-method @@ -1224,8 +1242,8 @@ class TestStockValuation(TransactionCase): self.env['stock.move']._run_fifo_vacuum() # stock values for move1 and move2 - self.assertEqual(move1.value, -400.0) - self.assertEqual(move1.remaining_value, 200.0) + self.assertEqual(move1.value, -680.0) # 40@15 + 10@8 + self.assertEqual(move1.remaining_value, -80.0) self.assertEqual(move1.remaining_qty, -10.0) self.assertEqual(move2.value, 600.0) self.assertEqual(move2.remaining_value, 0.0) @@ -1235,14 +1253,13 @@ class TestStockValuation(TransactionCase): valuation_aml = self._get_stock_valuation_move_lines() vacuum1_valuation_aml = valuation_aml[-1] self.assertEqual(vacuum1_valuation_aml.debit, 0) - # 200 was credited more in valuation (we initially credited 400 but the vacuum realized - # that 600 was the right value) - self.assertEqual(vacuum1_valuation_aml.credit, 200) + # 280 was credited more in valuation (we compensated 40 items here, so initially 40 were + # valued at 8 -> 320 in credit but now we actually sent 40@15 = 600, so the difference is + # 280 more credited) + self.assertEqual(vacuum1_valuation_aml.credit, 280) output_aml = self._get_stock_output_move_lines() vacuum1_output_aml = output_aml[-1] - # 200 was debited more in output (we initially debited 400 but the vacuum realized - # that 600 was the right value) - self.assertEqual(vacuum1_output_aml.debit, 200) + self.assertEqual(vacuum1_output_aml.debit, 280) self.assertEqual(vacuum1_output_aml.credit, 0) self.assertTrue(set(move1.mapped('account_move_ids.line_ids').ids) == {move1_valuation_aml.id, move1_output_aml.id, vacuum1_valuation_aml.id, vacuum1_output_aml.id}) @@ -1290,7 +1307,7 @@ class TestStockValuation(TransactionCase): self.env['stock.move']._run_fifo_vacuum() # stock values for move1-3 - self.assertEqual(move1.value, -400.0) + self.assertEqual(move1.value, -850.0) # 40@15 + 10@25 self.assertEqual(move1.remaining_value, 0.0) self.assertEqual(move1.remaining_qty, 0.0) self.assertEqual(move2.value, 600.0) @@ -1304,14 +1321,11 @@ class TestStockValuation(TransactionCase): valuation_aml = self._get_stock_valuation_move_lines() vacuum2_valuation_aml = valuation_aml[-1] self.assertEqual(vacuum2_valuation_aml.debit, 0) - # 250 was credited more in valuation (we initially credited 400+200 but the vacuum realized - # that 850 was the right value) - self.assertEqual(vacuum2_valuation_aml.credit, 250) + # there is still 10@8 to compensate with 10@25 -> 170 to credit more in the valuation account + self.assertEqual(vacuum2_valuation_aml.credit, 170) output_aml = self._get_stock_output_move_lines() vacuum2_output_aml = output_aml[-1] - # 250 was debited more in output (we initially debited 400+200 but the vacuum realized - # that 850 was the right value) - self.assertEqual(vacuum2_output_aml.debit, 250) + self.assertEqual(vacuum2_output_aml.debit, 170) self.assertEqual(vacuum2_output_aml.credit, 0) self.assertTrue(set(move1.mapped('account_move_ids.line_ids').ids) == {move1_valuation_aml.id, move1_output_aml.id, vacuum1_valuation_aml.id, vacuum1_output_aml.id, vacuum2_valuation_aml.id, vacuum2_output_aml.id}) @@ -1655,6 +1669,18 @@ class TestStockValuation(TransactionCase): self.assertEqual(sum(self._get_stock_output_move_lines().mapped('credit')), 0) def test_fifo_add_move_in_done_picking_1(self): + """ The flow is: + + product2 std price = 20 + IN01 10@10 product1 + IN01 10@20 product2 + IN01 correction 10@20 -> 11@20 (product2) + DO01 11 product2 + DO02 1 product2 + DO02 correction 1 -> 2 (negative stock) + IN03 2@30 product2 + vacuum + """ self.product1.product_tmpl_id.cost_method = 'fifo' # --------------------------------------------------------------------- @@ -1725,16 +1751,21 @@ class TestStockValuation(TransactionCase): self.assertEqual(self.product2.qty_available, 10) self.assertEqual(self.product2.stock_value, 200) + self.assertEqual(sum(self._get_stock_valuation_move_lines().mapped('debit')), 300) + self.assertEqual(sum(self._get_stock_valuation_move_lines().mapped('credit')), 0) + # --------------------------------------------------------------------- # Edit the previous stock move, receive 11 # --------------------------------------------------------------------- move2.quantity_done = 11 - self.assertEqual(move2.value, 200.0) + self.assertEqual(move2.value, 220.0) # after correction, the move should be valued at 11@20 self.assertEqual(move2.remaining_qty, 11.0) self.assertEqual(move2.price_unit, 20.0) self.assertEqual(move2.remaining_value, 220.0) + self.assertEqual(sum(self._get_stock_valuation_move_lines().mapped('debit')), 320) + self.assertEqual(sum(self._get_stock_valuation_move_lines().mapped('credit')), 0) # --------------------------------------------------------------------- # Send 11 product 2 # --------------------------------------------------------------------- @@ -1768,9 +1799,13 @@ class TestStockValuation(TransactionCase): self.assertEqual(move3.remaining_qty, 0.0) self.assertEqual(move3.price_unit, -20.0) self.assertEqual(move3.remaining_value, 0.0) + self.assertEqual(self.product2.qty_available, 0) + + self.assertEqual(sum(self._get_stock_valuation_move_lines().mapped('debit')), 320) + self.assertEqual(sum(self._get_stock_valuation_move_lines().mapped('credit')), 220) # --------------------------------------------------------------------- - # Add one move of product 2 + # Add one move of product 2, this'll make some negative stock. # --------------------------------------------------------------------- move4 = self.env['stock.move'].create({ 'picking_id': delivery.id, @@ -1794,12 +1829,18 @@ class TestStockValuation(TransactionCase): self.assertEqual(move4.price_unit, -20.0) self.assertEqual(move4.remaining_value, -20.0) + self.assertEqual(self.product2.qty_available, -1) + + self.assertEqual(sum(self._get_stock_valuation_move_lines().mapped('debit')), 320) + self.assertEqual(sum(self._get_stock_valuation_move_lines().mapped('credit')), 240) + # --------------------------------------------------------------------- # edit the created move, add 1 # --------------------------------------------------------------------- move4.quantity_done = 2 - self.assertEqual(move4.value, -20.0) + self.assertEqual(self.product2.qty_available, -2) + self.assertEqual(move4.value, -40.0) self.assertEqual(move4.remaining_qty, -2.0) self.assertEqual(move4.price_unit, -20.0) self.assertEqual(move4.remaining_value, -40.0) @@ -1823,7 +1864,7 @@ class TestStockValuation(TransactionCase): # --------------------------------------------------------------------- # receive 2 products 2 @ 30 # --------------------------------------------------------------------- - move1 = self.env['stock.move'].create({ + move5 = self.env['stock.move'].create({ 'picking_id': receipt.id, 'name': '10 in', 'location_id': self.supplier_location.id, @@ -1840,8 +1881,11 @@ class TestStockValuation(TransactionCase): 'qty_done': 2.0, })] }) - move1._action_confirm() - move1._action_done() + move5._action_confirm() + move5._action_done() + + self.assertEqual(sum(self._get_stock_valuation_move_lines().mapped('debit')), 380) + self.assertEqual(sum(self._get_stock_valuation_move_lines().mapped('credit')), 260) # --------------------------------------------------------------------- # run vacuum @@ -1858,6 +1902,7 @@ class TestStockValuation(TransactionCase): self.assertEqual(self.product2.qty_available, 0) self.assertEqual(self.product2.stock_value, 0) self.assertEqual(move4.remaining_value, 0) + self.assertEqual(move4.value, -60) # after correction, the move is valued -(2*30) def test_fifo_add_moveline_in_done_move_1(self): self.product1.product_tmpl_id.cost_method = 'fifo' @@ -1921,7 +1966,7 @@ class TestStockValuation(TransactionCase): self.assertEqual(sum(self._get_stock_output_move_lines().mapped('credit')), 0) def test_fifo_edit_done_move1(self): - """ Check that incrementing the done quantity will correctly re-run a fifo lookup. + """ Increase OUT done move while quantities are available. """ self.product1.product_tmpl_id.cost_method = 'fifo' @@ -2059,6 +2104,9 @@ class TestStockValuation(TransactionCase): # --------------------------------------------------------------------- move3.quantity_done = 14 self.assertEqual(move3.product_qty, 14) + # old value: -80 -(8@10) + # new value: -148 => -(10@10 + 4@12) + self.assertEqual(move3.value, -148) # older move self.assertEqual(move1.remaining_value, 0) # before, 20 @@ -2090,6 +2138,8 @@ class TestStockValuation(TransactionCase): self.assertEqual(sum(self._get_stock_output_move_lines().mapped('credit')), 0) def test_fifo_edit_done_move2(self): + """ Decrease, then increase OUT done move while quantities are available. + """ self.product1.product_tmpl_id.cost_method = 'fifo' # --------------------------------------------------------------------- @@ -2152,7 +2202,7 @@ class TestStockValuation(TransactionCase): # --------------------------------------------------------------------- move2.quantity_done = 8 - self.assertEqual(move2.value, -100.0) + self.assertEqual(move2.value, -80.0) # the move actually sent 8@10 self.assertEqual(move2.remaining_qty, 0.0) self.assertEqual(move2.remaining_value, 0.0) @@ -2167,7 +2217,7 @@ class TestStockValuation(TransactionCase): # --------------------------------------------------------------------- move2.with_context(debug=True).quantity_done = 10 - self.assertEqual(move2.value, -100.0) + self.assertEqual(move2.value, -100.0) # the move actually sent 10@10 self.assertEqual(move2.remaining_qty, 0.0) self.assertEqual(move2.remaining_value, 0.0)