From 21099f3eaf7b01a4f605c49daea529210b599bf0 Mon Sep 17 00:00:00 2001 From: Nicolas Martinelli Date: Thu, 24 May 2018 13:02:28 +0200 Subject: [PATCH] [FIX] stock, stock_account: valuation with owner Issue 1 ------- - Set a product with: Inventory Valuation: Manual Costing Method: Average Cost (or Standard Price) - Receive 1 unit @ 10, set an owner In the 'Inventory Valuation' report, the product is valued @ 10 while no accounting entry was created. The system simply multiplies the available quantity by the cost price, without taking into account the ownership of the quants. Issue 2 ------- - Set a product with: Inventory Valuation: Automated Costing Method: Average Cost (or Standard Price) - Receive 1 unit @ 10, set an owner In the 'Inventory Valuation' report, the product is valued @ 10 while no accounting entry was created. Once again, the system simply multiplies the available quantity by the cost price, without taking into account the AML. Solution -------- A slight refactoring of the `_compute_stock_value` method is necessary to make consistent the the FIFO computation and the Standard / Average method: - in case of manual valuation, the quantity on stock moves are taken into account - in case of perpetual valuation, the quantity on AMLs is taken into account The method `_compute_quantities_dict` needs a minor change so a `False` value can be handled correctly. The FIFO method is slightly changed: - the quantity at date takes into account the owner - in case of perpetual valuation, AMLs are used instead of stock moves Bonus point: an error was discovered in `_create_account_move_line` thanks to the test `test_change_cost_method_1`. The forced quantity should follow the same sign logic than the product quantity. opw-1849535 --- addons/stock/models/product.py | 6 +- addons/stock_account/models/product.py | 63 ++++---- addons/stock_account/models/stock.py | 3 +- .../tests/test_stockvaluation.py | 152 +++++++++++++++++- 4 files changed, 190 insertions(+), 34 deletions(-) diff --git a/addons/stock/models/product.py b/addons/stock/models/product.py index 97b437b4c5a..fef75714961 100644 --- a/addons/stock/models/product.py +++ b/addons/stock/models/product.py @@ -98,13 +98,13 @@ class Product(models.Model): domain_move_in = [('product_id', 'in', self.ids)] + domain_move_in_loc domain_move_out = [('product_id', 'in', self.ids)] + domain_move_out_loc - if lot_id: + if lot_id is not None: domain_quant += [('lot_id', '=', lot_id)] - if owner_id: + if owner_id is not None: domain_quant += [('owner_id', '=', owner_id)] domain_move_in += [('restrict_partner_id', '=', owner_id)] domain_move_out += [('restrict_partner_id', '=', owner_id)] - if package_id: + if package_id is not None: domain_quant += [('package_id', '=', package_id)] if dates_in_the_past: domain_move_in_done = list(domain_move_in) diff --git a/addons/stock_account/models/product.py b/addons/stock_account/models/product.py index 10832aa8bde..2c028bd4fc2 100644 --- a/addons/stock_account/models/product.py +++ b/addons/stock_account/models/product.py @@ -198,39 +198,45 @@ class ProductProduct(models.Model): fifo_automated_values[(row[0], row[1])] = (row[2], row[3], list(row[4])) for product in self: - if product.cost_method in ['standard', 'average']: - qty_available = product.with_context(company_owned=True).qty_available - price_used = product.standard_price - if to_date: - price_used = product.get_history_price( - self.env.user.company_id.id, - date=to_date, - ) - product.stock_value = price_used * qty_available - product.qty_at_date = qty_available - elif product.cost_method == 'fifo': - if to_date: - if product.product_tmpl_id.valuation == 'manual_periodic': - domain = [('product_id', '=', product.id), ('date', '<=', to_date)] + StockMove._get_all_base_domain() - moves = StockMove.search(domain) + if to_date: + price_used = product.get_history_price( + self.env.user.company_id.id, + date=to_date, + ) if product.cost_method in ['standard', 'average'] else 0.0 + if product.product_tmpl_id.valuation == 'manual_periodic': + domain = [('product_id', '=', product.id), ('date', '<=', to_date)] + StockMove._get_all_base_domain() + moves = StockMove.search(domain) + product.qty_at_date = product.with_context(company_owned=True, owner_id=False).qty_available + if product.cost_method == 'fifo': product.stock_value = sum(moves.mapped('value')) - product.qty_at_date = product.with_context(company_owned=True).qty_available product.stock_fifo_manual_move_ids = StockMove.browse(moves.ids) - elif product.product_tmpl_id.valuation == 'real_time': - valuation_account_id = product.categ_id.property_stock_valuation_account_id.id - value, quantity, aml_ids = fifo_automated_values.get((product.id, valuation_account_id)) or (0, 0, []) + elif product.cost_method in ['standard', 'average']: + product.stock_value = product.qty_at_date * price_used + elif product.product_tmpl_id.valuation == 'real_time': + valuation_account_id = product.categ_id.property_stock_valuation_account_id.id + value, quantity, aml_ids = fifo_automated_values.get((product.id, valuation_account_id)) or (0, 0, []) + product.qty_at_date = quantity + if product.cost_method == 'fifo': product.stock_value = value - product.qty_at_date = quantity product.stock_fifo_real_time_aml_ids = self.env['account.move.line'].browse(aml_ids) - else: - product.stock_value, moves = product._sum_remaining_values() - product.qty_at_date = product.with_context(company_owned=True).qty_available - if product.product_tmpl_id.valuation == 'manual_periodic': - product.stock_fifo_manual_move_ids = moves - elif product.product_tmpl_id.valuation == 'real_time': - valuation_account_id = product.categ_id.property_stock_valuation_account_id.id - value, quantity, aml_ids = fifo_automated_values.get((product.id, valuation_account_id)) or (0, 0, []) + elif product.cost_method in ['standard', 'average']: + product.stock_value = quantity * price_used + else: + if product.product_tmpl_id.valuation == 'manual_periodic': + product.qty_at_date = product.with_context(company_owned=True, owner_id=False).qty_available + if product.cost_method == 'fifo': + product.stock_value, product.stock_fifo_manual_move_ids = product._sum_remaining_values() + elif product.cost_method in ['standard', 'average']: + product.stock_value = product.qty_at_date * product.standard_price + elif product.product_tmpl_id.valuation == 'real_time': + valuation_account_id = product.categ_id.property_stock_valuation_account_id.id + value, quantity, aml_ids = fifo_automated_values.get((product.id, valuation_account_id)) or (0, 0, []) + product.qty_at_date = quantity + if product.cost_method == 'fifo': + product.stock_value = value product.stock_fifo_real_time_aml_ids = self.env['account.move.line'].browse(aml_ids) + elif product.cost_method in ['standard', 'average']: + product.stock_value = quantity * product.standard_price def action_valuation_at_date_details(self): """ Returns an action with either a list view of all the valued stock moves of `self` if the @@ -401,4 +407,3 @@ class ProductCategory(models.Model): 'message': _("Changing your cost method is an important change that will impact your inventory valuation. Are you sure you want to make that change?"), } } - diff --git a/addons/stock_account/models/stock.py b/addons/stock_account/models/stock.py index 2a7cb101c4e..af38ff50027 100644 --- a/addons/stock_account/models/stock.py +++ b/addons/stock_account/models/stock.py @@ -581,7 +581,8 @@ class StockMove(models.Model): def _create_account_move_line(self, credit_account_id, debit_account_id, journal_id): self.ensure_one() AccountMove = self.env['account.move'] - quantity = self.env.context.get('forced_quantity', self.product_qty if self._is_in() else -1 * self.product_qty) + quantity = self.env.context.get('forced_quantity', self.product_qty) + quantity = quantity if self._is_in() else -1 * quantity # Make an informative `ref` on the created account move to differentiate between classic # movements, vacuum and edition of past moves. diff --git a/addons/stock_account/tests/test_stockvaluation.py b/addons/stock_account/tests/test_stockvaluation.py index 0450d66e9a1..ce88777d13d 100644 --- a/addons/stock_account/tests/test_stockvaluation.py +++ b/addons/stock_account/tests/test_stockvaluation.py @@ -2046,6 +2046,8 @@ class TestStockValuation(TransactionCase): self.assertEqual(len(move1.account_move_ids), 1) + self.assertAlmostEqual(self.product1.qty_available, 10.0) + self.assertAlmostEqual(self.product1.qty_at_date, 10.0) self.assertEqual(self.product1.stock_value, 100) # --------------------------------------------------------------------- @@ -2088,6 +2090,8 @@ class TestStockValuation(TransactionCase): self.assertEqual(len(move2.account_move_ids), 1) + self.assertAlmostEqual(self.product1.qty_available, 20.0) + self.assertAlmostEqual(self.product1.qty_at_date, 20.0) self.assertEqual(self.product1.stock_value, 220) # --------------------------------------------------------------------- @@ -2132,6 +2136,8 @@ class TestStockValuation(TransactionCase): self.assertEqual(len(move3.account_move_ids), 1) + self.assertAlmostEqual(self.product1.qty_available, 12.0) + self.assertAlmostEqual(self.product1.qty_at_date, 12.0) self.assertEqual(self.product1.stock_value, 140) # --------------------------------------------------------------------- @@ -2165,6 +2171,7 @@ class TestStockValuation(TransactionCase): # Ending # --------------------------------------------------------------------- self.assertEqual(self.product1.qty_available, 6) + self.assertAlmostEqual(self.product1.qty_at_date, 6.0) self.assertEqual(self.product1.stock_value, 72) self.assertEqual(sum(self._get_stock_input_move_lines().mapped('debit')), 0) self.assertEqual(sum(self._get_stock_input_move_lines().mapped('credit')), 220) @@ -2520,8 +2527,33 @@ class TestStockValuation(TransactionCase): move2.move_line_ids.qty_done = 1.0 move2._action_done() + self.assertAlmostEqual(self.product1.qty_available, 2.0) + self.assertAlmostEqual(self.product1.qty_at_date, 2.0) self.assertAlmostEqual(self.product1.standard_price, 7.5) + def test_average_perpetual_5(self): + ''' Set owner on incoming move => no valuation ''' + self.product1.product_tmpl_id.cost_method = 'average' + + move1 = self.env['stock.move'].create({ + 'name': 'Receive 1 unit at 10', + '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': 1.0, + 'price_unit': 10, + }) + move1._action_confirm() + move1._action_assign() + move1.move_line_ids.qty_done = 1.0 + move1.move_line_ids.owner_id = self.owner1.id + move1._action_done() + + self.assertAlmostEqual(self.product1.qty_available, 1.0) + self.assertAlmostEqual(self.product1.qty_at_date, 0.0) + self.assertAlmostEqual(self.product1.stock_value, 0.0) + def test_average_negative_1(self): """ Test edit in the past. Receive 10, send 20, edit the second move to only send 10. """ @@ -2810,6 +2842,77 @@ class TestStockValuation(TransactionCase): self.assertEqual(move7.value, 100.0) self.assertEqual(self.product1.standard_price, 10) + def test_average_manual_1(self): + ''' Set owner on incoming move => no valuation ''' + self.product1.product_tmpl_id.cost_method = 'average' + self.product1.product_tmpl_id.valuation = 'manual_periodic' + + move1 = self.env['stock.move'].create({ + 'name': 'Receive 1 unit at 10', + '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': 1.0, + 'price_unit': 10, + }) + move1._action_confirm() + move1._action_assign() + move1.move_line_ids.qty_done = 1.0 + move1.move_line_ids.owner_id = self.owner1.id + move1._action_done() + + self.assertAlmostEqual(self.product1.qty_available, 1.0) + self.assertAlmostEqual(self.product1.qty_at_date, 0.0) + self.assertAlmostEqual(self.product1.stock_value, 0.0) + + def test_standard_perpetual_1(self): + ''' Set owner on incoming move => no valuation ''' + self.product1.product_tmpl_id.cost_method = 'standard' + + move1 = self.env['stock.move'].create({ + 'name': 'Receive 1 unit at 10', + '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': 1.0, + 'price_unit': 10, + }) + move1._action_confirm() + move1._action_assign() + move1.move_line_ids.qty_done = 1.0 + move1.move_line_ids.owner_id = self.owner1.id + move1._action_done() + + self.assertAlmostEqual(self.product1.qty_available, 1.0) + self.assertAlmostEqual(self.product1.qty_at_date, 0.0) + self.assertAlmostEqual(self.product1.stock_value, 0.0) + + def test_standard_manual_1(self): + ''' Set owner on incoming move => no valuation ''' + self.product1.product_tmpl_id.cost_method = 'standard' + self.product1.product_tmpl_id.valuation = 'manual_periodic' + + move1 = self.env['stock.move'].create({ + 'name': 'Receive 1 unit at 10', + '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': 1.0, + 'price_unit': 10, + }) + move1._action_confirm() + move1._action_assign() + move1.move_line_ids.qty_done = 1.0 + move1.move_line_ids.owner_id = self.owner1.id + move1._action_done() + + self.assertAlmostEqual(self.product1.qty_available, 1.0) + self.assertAlmostEqual(self.product1.qty_at_date, 0.0) + self.assertAlmostEqual(self.product1.stock_value, 0.0) + def test_change_cost_method_1(self): """ Change the cost method from FIFO to AVCO. """ @@ -2862,6 +2965,8 @@ class TestStockValuation(TransactionCase): move3.move_line_ids.qty_done = 1.0 move3._action_done() + self.assertAlmostEqual(self.product1.qty_available, 19) + self.assertAlmostEqual(self.product1.qty_at_date, 19) self.assertEqual(self.product1.stock_value, 240) # --------------------------------------------------------------------- @@ -2930,6 +3035,8 @@ class TestStockValuation(TransactionCase): move3.move_line_ids.qty_done = 1.0 move3._action_done() + self.assertAlmostEqual(self.product1.qty_available, 19) + self.assertAlmostEqual(self.product1.qty_at_date, 19) self.assertEqual(self.product1.stock_value, 240) # --------------------------------------------------------------------- @@ -3003,6 +3110,8 @@ class TestStockValuation(TransactionCase): self.assertEqual(move1.value, 10) self.assertEqual(move1.remaining_value, 10) self.assertEqual(move1.remaining_qty, 1) + self.assertAlmostEqual(self.product1.qty_available, 0.0) + self.assertAlmostEqual(self.product1.qty_at_date, 2.0) self.assertEqual(self.product1.stock_value, 10) self.assertTrue(len(move1.account_move_ids), 1) @@ -3152,8 +3261,10 @@ class TestStockValuation(TransactionCase): move1.move_line_ids.qty_done = 10 move1._action_done() move1.date = date2 + move1.account_move_ids.write({'date': date2}) self.assertEqual(self.product1.qty_available, 10) + self.assertAlmostEqual(self.product1.qty_at_date, 10.0) self.assertEqual(self.product1.stock_value, 100) # receive 20 @@ -3170,8 +3281,10 @@ class TestStockValuation(TransactionCase): move2.move_line_ids.qty_done = 20 move2._action_done() move2.date = date3 + move2.account_move_ids.write({'date': date3}) self.assertEqual(self.product1.qty_available, 30) + self.assertAlmostEqual(self.product1.qty_at_date, 30.0) self.assertEqual(self.product1.stock_value, 300) # send 15 @@ -3188,8 +3301,10 @@ class TestStockValuation(TransactionCase): move3.move_line_ids.qty_done = 15 move3._action_done() move3.date = date4 + move3.account_move_ids.write({'date': date4}) self.assertEqual(self.product1.qty_available, 15) + self.assertAlmostEqual(self.product1.qty_at_date, 15.0) self.assertEqual(self.product1.stock_value, 150) # set the standard price to 5 @@ -3197,6 +3312,7 @@ class TestStockValuation(TransactionCase): self.env['product.price.history'].search([('product_id', '=', self.product1.id)], order='datetime desc, id DESC', limit=1).datetime = date5 self.assertEqual(self.product1.qty_available, 15) + self.assertAlmostEqual(self.product1.qty_at_date, 15.0) self.assertEqual(self.product1.stock_value, 75) # send 20 @@ -3213,8 +3329,10 @@ class TestStockValuation(TransactionCase): move4.move_line_ids.qty_done = 20 move4._action_done() move4.date = date6 + move4.account_move_ids.write({'date': date6}) self.assertEqual(self.product1.qty_available, -5) + self.assertAlmostEqual(self.product1.qty_at_date, -5.0) self.assertEqual(self.product1.stock_value, -25) # set the standard price to 7.5 @@ -3235,11 +3353,13 @@ class TestStockValuation(TransactionCase): move5.move_line_ids.qty_done = 100 move5._action_done() move5.date = date8 + move5.account_move_ids.write({'date': date8}) self.assertEqual(self.product1.qty_available, 95) + self.assertAlmostEqual(self.product1.qty_at_date, 95.0) self.assertEqual(self.product1.stock_value, 712.5) - # Quantity at date + # Quantity available at date self.assertEqual(self.product1.with_context(to_date=Date.to_string(date1)).qty_available, 0) self.assertEqual(self.product1.with_context(to_date=Date.to_string(date2)).qty_available, 10) self.assertEqual(self.product1.with_context(to_date=Date.to_string(date3)).qty_available, 30) @@ -3257,12 +3377,25 @@ class TestStockValuation(TransactionCase): self.assertEqual(self.product1.with_context(to_date=Date.to_string(date6)).stock_value, -25) self.assertEqual(self.product1.with_context(to_date=Date.to_string(date8)).stock_value, 712.5) + # Quantity at date + self.assertAlmostEqual(self.product1.with_context(to_date=Date.to_string(date1)).qty_at_date, 0.0) + self.assertAlmostEqual(self.product1.with_context(to_date=Date.to_string(date2)).qty_at_date, 10.0) + self.assertAlmostEqual(self.product1.with_context(to_date=Date.to_string(date3)).qty_at_date, 30.0) + self.assertAlmostEqual(self.product1.with_context(to_date=Date.to_string(date4)).qty_at_date, 15.0) + self.assertAlmostEqual(self.product1.with_context(to_date=Date.to_string(date5)).qty_at_date, 15.0) + self.assertAlmostEqual(self.product1.with_context(to_date=Date.to_string(date6)).qty_at_date, -5.0) + self.assertAlmostEqual(self.product1.with_context(to_date=Date.to_string(date7)).qty_at_date, -5.0) + self.assertAlmostEqual(self.product1.with_context(to_date=Date.to_string(date8)).qty_at_date, 95.0) + # edit the done quantity of move1, decrease it self.assertEqual(self.product1.with_context(to_date=Date.to_string(date2)).qty_available, 10) + self.assertAlmostEqual(self.product1.with_context(to_date=Date.to_string(date2)).qty_at_date, 10.0) move1.quantity_done = 5 + move1.account_move_ids.write({'date': date2}) # the quantity at date will reflect the change directly self.assertEqual(self.product1.with_context(to_date=Date.to_string(date2)).qty_available, 5) + self.assertAlmostEqual(self.product1.with_context(to_date=Date.to_string(date2)).qty_at_date, 5.0) # as when we decrease a quantity on a recreipt, we consider it as a out move with the price # of today, the value will be decrease of 100 - (5*7.5) @@ -3274,12 +3407,17 @@ class TestStockValuation(TransactionCase): # edit move 4, send 15 instead of 20 # we now have +5 + 20 - 15 -20 = -10 * a standard price of 5 + self.assertAlmostEqual(self.product1.with_context(to_date=Date.to_string(date6)).qty_available, -10.0) + self.assertAlmostEqual(self.product1.with_context(to_date=Date.to_string(date6)).qty_at_date, -10.0) self.assertEqual(self.product1.with_context(to_date=Date.to_string(date6)).stock_value, -50) move4.quantity_done = 15 + move4.account_move_ids.write({'date': date6}) # -(20*5) + (5*7.5) self.assertEqual(move4.value, -62.5) # we now have +5 + 20 - 15 -15 = -5 * a standard price of 5 + self.assertAlmostEqual(self.product1.with_context(to_date=Date.to_string(date6)).qty_available, -5.0) + self.assertAlmostEqual(self.product1.with_context(to_date=Date.to_string(date6)).qty_at_date, -5.0) self.assertEqual(self.product1.with_context(to_date=Date.to_string(date6)).stock_value, -25) def test_at_date_fifo_1(self): @@ -3315,6 +3453,7 @@ class TestStockValuation(TransactionCase): move1.account_move_ids.write({'date': date1}) self.assertEqual(self.product1.qty_available, 10) + self.assertAlmostEqual(self.product1.qty_at_date, 10.0) self.assertEqual(self.product1.stock_value, 100) # receive 10@12 @@ -3335,6 +3474,7 @@ class TestStockValuation(TransactionCase): move2.account_move_ids.write({'date': date2}) self.assertEqual(self.product1.qty_available, 20) + self.assertAlmostEqual(self.product1.qty_at_date, 20) self.assertEqual(self.product1.stock_value, 220) # send 15 @@ -3354,6 +3494,7 @@ class TestStockValuation(TransactionCase): move3.account_move_ids.write({'date': date3}) self.assertEqual(self.product1.qty_available, 5) + self.assertAlmostEqual(self.product1.qty_at_date, 5.0) self.assertEqual(self.product1.stock_value, 60) # send 20 @@ -3373,6 +3514,7 @@ class TestStockValuation(TransactionCase): move4.account_move_ids.write({'date': date4}) self.assertEqual(self.product1.qty_available, -15) + self.assertAlmostEqual(self.product1.qty_at_date, -15.0) self.assertEqual(self.product1.stock_value, -180) # receive 100@15 @@ -3393,6 +3535,7 @@ class TestStockValuation(TransactionCase): move5.account_move_ids.write({'date': date5}) self.assertEqual(self.product1.qty_available, 85) + self.assertAlmostEqual(self.product1.qty_at_date, 85.0) self.assertEqual(self.product1.stock_value, 1320) # run the vacuum to compensate the negative stock move @@ -3400,6 +3543,7 @@ class TestStockValuation(TransactionCase): move4.account_move_ids[0].write({'date': date6}) self.assertEqual(self.product1.qty_available, 85) + self.assertAlmostEqual(self.product1.qty_at_date, 85.0) self.assertEqual(self.product1.stock_value, 1275) # Edit the quantity done of move1, increase it. @@ -3472,6 +3616,7 @@ class TestStockValuation(TransactionCase): move1.account_move_ids.write({'date': date1}) self.assertEqual(self.product1.qty_available, 10) + self.assertAlmostEqual(self.product1.qty_at_date, 10.0) self.assertEqual(self.product1.stock_value, 100) # receive 10@15 @@ -3492,6 +3637,7 @@ class TestStockValuation(TransactionCase): move2.account_move_ids.write({'date': date2}) self.assertEqual(self.product1.qty_available, 20) + self.assertAlmostEqual(self.product1.qty_at_date, 20.0) self.assertEqual(self.product1.stock_value, 250) # send 30 @@ -3511,6 +3657,7 @@ class TestStockValuation(TransactionCase): move3.account_move_ids.write({'date': date3}) self.assertEqual(self.product1.qty_available, -10) + self.assertAlmostEqual(self.product1.qty_at_date, -10.0) self.assertEqual(self.product1.stock_value, -150) # receive 10@20 @@ -3531,6 +3678,7 @@ class TestStockValuation(TransactionCase): move4.account_move_ids.write({'date': date4}) self.assertEqual(self.product1.qty_available, 0) + self.assertAlmostEqual(self.product1.qty_at_date, 0.0) self.assertEqual(self.product1.stock_value, 50) # receive 10@10 @@ -3551,6 +3699,7 @@ class TestStockValuation(TransactionCase): move5.account_move_ids.write({'date': date5}) self.assertEqual(self.product1.qty_available, 10) + self.assertAlmostEqual(self.product1.qty_at_date, 10.0) self.assertEqual(self.product1.stock_value, 150) # run the vacuum to compensate the negative stock move @@ -3558,6 +3707,7 @@ class TestStockValuation(TransactionCase): move3.account_move_ids[0].write({'date': date6}) self.assertEqual(self.product1.qty_available, 10) + self.assertAlmostEqual(self.product1.qty_at_date, 10.0) self.assertEqual(self.product1.stock_value, 100) # ---------------------------------------------------------------------