From 91d69578495c078a985e7eb4b2deb1caf117e126 Mon Sep 17 00:00:00 2001 From: Simon Lejeune Date: Thu, 10 Aug 2017 16:24:10 +0200 Subject: [PATCH] [FIX] stock_account: fifo and negative stock rename `_get_candidates_out_move` to `_get_fifo_candidates_out_move` and `_get_candidates_move` to `_get_fifo_candidates_in_move` as it's more explicit that they're only used in FIFO costing. Fix `_update_future_cumulated_value` to filter only on internal moves and to remove the value added in the past (self is always an out move to which we added some value) and rename into `update_fifo_future_cumulated_value`. In `action_done`, rename `qty_taken_for_candidate` to `qty_taken_for_candidate` as it's the qty we take to compensate the candidate and not a quantity we take on the candidate (in this case, some old out moves that we could not valuate in time because the stock was negative). We fix the write on the value and cumulated value to decrease the qty_taken_for_candidate*move.price_unit, because an out move has always a negative value and we actually substract the value of `move` which is always an in move and has always a positive value. We fix the purchase test where negative out moves were valued positively and add `test_fifo_negative_1` that tests the sending of goods we do not have and the receipt encoded afterwards. --- addons/purchase/test/fifo_price.yml | 4 +- addons/stock_account/models/product.py | 10 ++- addons/stock_account/models/stock.py | 44 ++++++---- .../tests/test_stockvaluation.py | 80 +++++++++++++++++++ 4 files changed, 117 insertions(+), 21 deletions(-) diff --git a/addons/purchase/test/fifo_price.yml b/addons/purchase/test/fifo_price.yml index a8ad4e24240..598e41844e8 100644 --- a/addons/purchase/test/fifo_price.yml +++ b/addons/purchase/test/fifo_price.yml @@ -343,7 +343,7 @@ self.env['stock.immediate.transfer'].create({'pick_id': picking.id}).process() original_out_move = self.env['stock.picking'].browse(ref('outgoing_fifo_shipment_neg')).move_lines[0] assert original_out_move.remaining_qty == 50.0, 'On the out move of 100, 50 still needs to be matched' - assert original_out_move.value == 2500, 'Value of the move should be 2500' + assert original_out_move.value == -2500, 'Value of the move should be -2500' - Receive purchase order with 600 kg FIFO Ice Cream at 80 euro/kg - @@ -368,6 +368,6 @@ picking = self.picking_ids[0] self.env['stock.immediate.transfer'].create({'pick_id': picking.id}).process() original_out_move = self.env['stock.picking'].browse(ref('outgoing_fifo_shipment_neg')).move_lines[0] - assert original_out_move.value == 6500.0, 'First original out move should have 6500 as value' + assert original_out_move.value == -6500.0, 'First original out move should have -6500 as value' assert original_out_move.product_id.stock_value == 12000.0, 'Stock Value should be 12000' assert original_out_move.product_id.qty_available == 150.0, 'Qty available should be 150' \ No newline at end of file diff --git a/addons/stock_account/models/product.py b/addons/stock_account/models/product.py index bbb6f28a0cc..f7ba6e6f805 100644 --- a/addons/stock_account/models/product.py +++ b/addons/stock_account/models/product.py @@ -176,13 +176,17 @@ class ProductProduct(models.Model): return 0.0 return latest.cumulated_value - def _get_candidates_out_move(self): + def _get_fifo_candidates_out_move(self): + """ Find OUT moves that were not valued in time because of negative stock. + """ self.ensure_one() domain = [('product_id', '=', self.id), ('remaining_qty', '>', 0.0)] + self.env['stock.move']._get_out_base_domain() candidates = self.env['stock.move'].search(domain, order='date, id') return candidates - def _get_candidates_move(self): + def _get_fifo_candidates_in_move(self): + """ Find IN moves that can be used to value OUT moves. + """ self.ensure_one() domain = [('product_id', '=', self.id), ('remaining_qty', '>', 0.0)] + self.env['stock.move']._get_in_base_domain() candidates = self.env['stock.move'].search(domain, order='date, id') @@ -205,7 +209,7 @@ class ProductProduct(models.Model): elif product.cost_method == 'average': product.stock_value = product._get_latest_cumulated_value() elif product.cost_method == 'fifo': #Could also do same as for average, but it would lead to more rounding errors - moves = product._get_candidates_move() + moves = product._get_fifo_candidates_in_move() value = 0 for move in moves: value += move.remaining_qty * move.price_unit diff --git a/addons/stock_account/models/stock.py b/addons/stock_account/models/stock.py index d1c0197c07e..f314446d777 100644 --- a/addons/stock_account/models/stock.py +++ b/addons/stock_account/models/stock.py @@ -92,13 +92,23 @@ class StockMove(models.Model): return move.price_unit or self.product_id.standard_price return self.product_id.standard_price - def _update_future_cumulated_value(self, value): + def _update_fifo_future_cumulated_value(self, value): + """ This method is intended to be called on an OUT move that was not valued in time because + of negative stock with `value` argument representing its new value. + """ self.ensure_one() - moves = self.search([('state', '=', 'done'), - ('date', '>', self.date), - ('product_id', '=', self.product_id.id)]) + domain = self._get_all_domain() + domain += [ + '|', + ('date', '>', self.date), + '&', + ('date', '=', self.date), + ('id', '>', self.id) + ] + moves = self.search(domain, order='date, id') for move in moves: - move.value += value + # As `self` is always an out move, we decrease its value on the future moves. + move.cumulated_value -= value @ api.model def _get_in_base_domain(self, company_id=False): @@ -223,20 +233,22 @@ class StockMove(models.Model): 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': - # If you find an out with qty_remaining (because of negative stock), you can change it over there - candidates_out = move.product_id._get_candidates_out_move() + # 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. + candidates_out = move.product_id._get_fifo_candidates_out_move() qty_to_take = move.product_qty for candidate in candidates_out: if candidate.remaining_qty < qty_to_take: - qty_taken_on_candidate = candidate.remaining_qty + qty_taken_for_candidate = candidate.remaining_qty else: - qty_taken_on_candidate = qty_to_take - candidate.remaining_qty -= qty_taken_on_candidate - move.remaining_qty -= qty_taken_on_candidate - qty_to_take -= qty_taken_on_candidate - candidate.value += move.price_unit * qty_taken_on_candidate - candidate.cumulated_value += move.price_unit * qty_taken_on_candidate - candidate._update_future_cumulated_value(move.price_unit * qty_taken_on_candidate) + qty_taken_for_candidate = qty_to_take + candidate.remaining_qty -= qty_taken_for_candidate + move.remaining_qty -= qty_taken_for_candidate + qty_to_take -= qty_taken_for_candidate + candidate.value -= move.price_unit * qty_taken_for_candidate + candidate.cumulated_value -= move.price_unit * qty_taken_for_candidate + candidate._update_fifo_future_cumulated_value(move.price_unit * qty_taken_for_candidate) candidate.price_unit = candidate.value / candidate.product_qty move.last_done_qty = move.product_id.qty_available else: @@ -246,7 +258,7 @@ class StockMove(models.Model): if move.product_id.cost_method == 'fifo': qty_to_take = move.product_qty tmp_value = 0 - candidates = move.product_id._get_candidates_move() + candidates = move.product_id._get_fifo_candidates_in_move() last_candidate = False for candidate in candidates: if candidate.remaining_qty <= qty_to_take: diff --git a/addons/stock_account/tests/test_stockvaluation.py b/addons/stock_account/tests/test_stockvaluation.py index 3328b1f3d96..965801c6eed 100644 --- a/addons/stock_account/tests/test_stockvaluation.py +++ b/addons/stock_account/tests/test_stockvaluation.py @@ -462,3 +462,83 @@ class TestStockValuation(TransactionCase): self.assertEqual(move5.cumulated_value, 795.9) # fuck you, rounding # self.assertEqual(move5.cumulated_value, 796) + def test_fifo_negative_1(self): + self.product1.product_tmpl_id.cost_method = 'fifo' + move1 = self.env['stock.move'].create({ + 'name': '50 out', + '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': 50.0, + 'price_unit': 0, + 'move_line_ids': [(0, 0, { + 'product_id': self.product1.id, + 'location_id': self.stock_location.id, + 'location_dest_id': self.customer_location.id, + 'product_uom_id': self.uom_unit.id, + 'qty_done': 50.0, + })] + }) + move1.action_confirm() + move1.action_done() + + self.assertEqual(move1.value, 0.0) + self.assertEqual(move1.cumulated_value, 0.0) + # normally unused in out moves, but as it moved negative stock we mark it + self.assertEqual(move1.remaining_qty, 50.0) + + move2 = self.env['stock.move'].create({ + 'name': '40 in @15', + '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': 40.0, + 'price_unit': 15.0, + 'move_line_ids': [(0, 0, { + 'product_id': self.product1.id, + 'location_id': self.supplier_location.id, + 'location_dest_id': self.stock_location.id, + 'product_uom_id': self.uom_unit.id, + 'qty_done': 40.0, + })] + }) + move2.action_confirm() + move2.with_context(debug=True).action_done() + + self.assertEqual(move1.value, -600.0) + self.assertEqual(move1.cumulated_value, -600.0) + self.assertEqual(move1.remaining_qty, 10.0) + self.assertEqual(move2.value, 600.0) + self.assertEqual(move2.remaining_qty, 0.0) + self.assertEqual(move2.cumulated_value, 0.0) + + move3 = self.env['stock.move'].create({ + 'name': '20 in @25', + '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': 20.0, + 'price_unit': 25.0, + 'move_line_ids': [(0, 0, { + 'product_id': self.product1.id, + 'location_id': self.supplier_location.id, + 'location_dest_id': self.stock_location.id, + 'product_uom_id': self.uom_unit.id, + 'qty_done': 20.0 + })] + }) + move3.action_confirm() + move3.action_done() + + self.assertEqual(move1.value, -850.0) + self.assertEqual(move1.cumulated_value, -850.0) + self.assertEqual(move1.remaining_qty, 0.0) + self.assertEqual(move2.value, 600.0) + self.assertEqual(move2.remaining_qty, 0.0) + self.assertEqual(move2.cumulated_value, -250.0) + self.assertEqual(move3.value, 500.0) + self.assertEqual(move3.remaining_qty, 10.0) + self.assertEqual(move3.cumulated_value, 250.0)