From c46ed8af92ea9b7b42e9519166d87f41c052e8db Mon Sep 17 00:00:00 2001 From: Mathieu Walravens Date: Wed, 23 Aug 2023 12:59:13 +0000 Subject: [PATCH] [FIX] stock_account: correct accounts for dropship return Before this commit: When returning a dropshipping, the valuation layers created do not have the correct accounts on it: - Valuation -> Input for the first SVL - Output -> Valuation for the second SVL After this commit: For a dropshipped move, valuation layers have the following chain of accounts: - Input -> Valuation for the first SVL - Valuation -> Output for the second SVL Therefore, the return should have it reversed: - Output -> Valuation for the first SVL - Valuation -> Input for the second SVL Steps to reproduce: 1. Create a dropship product with automated inventory valuation 2. Create a Sales Order, go on the PO and confirm it 3. Set quantities and validate dropshipping 4. Return delivered product (dropship return) opw-3391174 closes odoo/odoo#137497 X-original-commit: 3e2421982c229af44aa0f46dfd91bcff3fd729a0 Signed-off-by: William Henrotin (whe) Signed-off-by: Walravens Mathieu (wama) --- addons/stock_account/models/stock_move.py | 6 +- .../tests/test_stockvaluationlayer.py | 147 ++++++++++++++++++ 2 files changed, 151 insertions(+), 2 deletions(-) diff --git a/addons/stock_account/models/stock_move.py b/addons/stock_account/models/stock_move.py index 624a532353d..decd7f00e5c 100644 --- a/addons/stock_account/models/stock_move.py +++ b/addons/stock_account/models/stock_move.py @@ -576,11 +576,13 @@ class StockMove(models.Model): cost = -1 * cost am_vals.append(self.with_company(self.company_id)._prepare_account_move_vals(acc_valuation, acc_dest, journal_id, qty, description, svl_id, cost)) elif self._is_dropshipped_returned(): - if cost > 0: + if cost > 0 and self.location_dest_id._should_be_valued(): am_vals.append(self.with_company(self.company_id).with_context(is_returned=True)._prepare_account_move_vals(acc_valuation, acc_src, journal_id, qty, description, svl_id, cost)) + elif cost > 0: + am_vals.append(self.with_company(self.company_id).with_context(is_returned=True)._prepare_account_move_vals(acc_dest, acc_valuation, journal_id, qty, description, svl_id, cost)) else: cost = -1 * cost - am_vals.append(self.with_company(self.company_id).with_context(is_returned=True)._prepare_account_move_vals(acc_dest, acc_valuation, journal_id, qty, description, svl_id, cost)) + am_vals.append(self.with_company(self.company_id).with_context(is_returned=True)._prepare_account_move_vals(acc_valuation, acc_src, journal_id, qty, description, svl_id, cost)) return am_vals diff --git a/addons/stock_account/tests/test_stockvaluationlayer.py b/addons/stock_account/tests/test_stockvaluationlayer.py index aba0bc80ab9..fede3a692a7 100644 --- a/addons/stock_account/tests/test_stockvaluationlayer.py +++ b/addons/stock_account/tests/test_stockvaluationlayer.py @@ -1137,6 +1137,67 @@ class TestAngloSaxonAccounting(AccountTestInvoicingCommon, TestStockValuationCom 'property_stock_journal': cls.company_data['default_journal_misc'].id, }) + def _make_in_move(self, product, quantity, unit_cost=None, create_picking=False, loc_dest=None, pick_type=None): + """ Helper to create and validate a receipt move. + """ + unit_cost = unit_cost or product.standard_price + loc_dest = loc_dest or self.stock_location + pick_type = pick_type or self.picking_type_in + in_move = self.env['stock.move'].create({ + 'name': 'in %s units @ %s per unit' % (str(quantity), str(unit_cost)), + 'product_id': product.id, + 'location_id': self.supplier_location.id, + 'location_dest_id': loc_dest.id, + 'product_uom': self.uom_unit.id, + 'product_uom_qty': quantity, + 'price_unit': unit_cost, + 'picking_type_id': pick_type.id, + }) + + if create_picking: + picking = self.env['stock.picking'].create({ + 'picking_type_id': in_move.picking_type_id.id, + 'location_id': in_move.location_id.id, + 'location_dest_id': in_move.location_dest_id.id, + }) + in_move.write({'picking_id': picking.id}) + + in_move._action_confirm() + in_move._action_assign() + in_move.move_line_ids.qty_done = quantity + in_move._action_done() + + return in_move.with_context(svl=True) + + def _make_dropship_move(self, product, quantity, unit_cost=None): + dropshipped = self.env['stock.move'].create({ + 'name': 'dropship %s units' % str(quantity), + 'product_id': product.id, + 'location_id': self.supplier_location.id, + 'location_dest_id': self.customer_location.id, + 'product_uom': self.uom_unit.id, + 'product_uom_qty': quantity, + 'picking_type_id': self.picking_type_out.id, + }) + if unit_cost: + dropshipped.price_unit = unit_cost + dropshipped._action_confirm() + dropshipped._action_assign() + dropshipped.move_line_ids.qty_done = quantity + dropshipped._action_done() + return dropshipped + + def _make_return(self, move, quantity_to_return): + stock_return_picking = Form(self.env['stock.return.picking']\ + .with_context(active_ids=[move.picking_id.id], active_id=move.picking_id.id, active_model='stock.picking')) + stock_return_picking = stock_return_picking.save() + stock_return_picking.product_return_moves.quantity = quantity_to_return + stock_return_picking_action = stock_return_picking.create_returns() + return_pick = self.env['stock.picking'].browse(stock_return_picking_action['res_id']) + return_pick.move_ids[0].move_line_ids[0].qty_done = quantity_to_return + return_pick._action_done() + return return_pick.move_ids + def test_avco_and_credit_note(self): """ When reversing an invoice that contains some anglo-saxo AML, the new anglo-saxo AML should have the same value @@ -1201,3 +1262,89 @@ class TestAngloSaxonAccounting(AccountTestInvoicingCommon, TestStockValuationCom self.assertEqual(valuation_line.debit, 0) self.assertEqual(stock_out_line.credit, 0) self.assertEqual(stock_out_line.debit, -100) + + def test_dropship_return_accounts_1(self): + """ + When returning a dropshipped move, make sure the correct accounts are used + """ + # pylint: disable=bad-whitespace + self.product1.categ_id.property_cost_method = 'fifo' + + move1 = self._make_dropship_move(self.product1, 2, unit_cost=10) + move2 = self._make_return(move1, 2) + + # First: Input -> Valuation + # Second: Valuation -> Output + origin_svls = move1.stock_valuation_layer_ids.sorted('quantity', reverse=True) + # First: Output -> Valuation + # Second: Valuation -> Input + return_svls = move2.stock_valuation_layer_ids.sorted('quantity', reverse=True) + self.assertEqual(len(origin_svls), 2) + self.assertEqual(len(return_svls), 2) + + acc_in, acc_out, acc_valuation = self.stock_input_account, self.stock_output_account, self.stock_valuation_account + + # Dropshipping should be: Input -> Output + self.assertRecordValues(origin_svls[0].account_move_id.line_ids, [ + {'account_id': acc_in.id, 'debit': 0, 'credit': 20}, + {'account_id': acc_valuation.id, 'debit': 20, 'credit': 0}, + ]) + self.assertRecordValues(origin_svls[1].account_move_id.line_ids, [ + {'account_id': acc_valuation.id, 'debit': 0, 'credit': 20}, + {'account_id': acc_out.id, 'debit': 20, 'credit': 0}, + ]) + # Return should be: Output -> Input + self.assertRecordValues(return_svls[0].account_move_id.line_ids, [ + {'account_id': acc_out.id, 'debit': 0, 'credit': 20}, + {'account_id': acc_valuation.id, 'debit': 20, 'credit': 0}, + ]) + self.assertRecordValues(return_svls[1].account_move_id.line_ids, [ + {'account_id': acc_valuation.id, 'debit': 0, 'credit': 20}, + {'account_id': acc_in.id, 'debit': 20, 'credit': 0}, + ]) + + def test_dropship_return_accounts_2(self): + """ + When returning a dropshipped move, make sure the correct accounts are used + """ + # pylint: disable=bad-whitespace + self.product1.categ_id.property_cost_method = 'fifo' + + move1 = self._make_dropship_move(self.product1, 2, unit_cost=10) + + # return to WH/Stock + stock_return_picking = Form(self.env['stock.return.picking']\ + .with_context(active_ids=[move1.picking_id.id], active_id=move1.picking_id.id, active_model='stock.picking')) + stock_return_picking = stock_return_picking.save() + stock_return_picking.product_return_moves.quantity = 2 + stock_return_picking.location_id = self.stock_location + stock_return_picking_action = stock_return_picking.create_returns() + return_pick = self.env['stock.picking'].browse(stock_return_picking_action['res_id']) + return_pick.move_ids[0].move_line_ids[0].qty_done = 2 + return_pick._action_done() + move2 = return_pick.move_ids + + # First: Input -> Valuation + # Second: Valuation -> Output + origin_svls = move1.stock_valuation_layer_ids.sorted('quantity', reverse=True) + # Only one: Output -> Valuation + return_svl = move2.stock_valuation_layer_ids + self.assertEqual(len(origin_svls), 2) + self.assertEqual(len(return_svl), 1) + + acc_in, acc_out, acc_valuation = self.stock_input_account, self.stock_output_account, self.stock_valuation_account + + # Dropshipping should be: Input -> Output + self.assertRecordValues(origin_svls[0].account_move_id.line_ids, [ + {'account_id': acc_in.id, 'debit': 0, 'credit': 20}, + {'account_id': acc_valuation.id, 'debit': 20, 'credit': 0}, + ]) + self.assertRecordValues(origin_svls[1].account_move_id.line_ids, [ + {'account_id': acc_valuation.id, 'debit': 0, 'credit': 20}, + {'account_id': acc_out.id, 'debit': 20, 'credit': 0}, + ]) + # Return should be: Output -> Valuation + self.assertRecordValues(return_svl.account_move_id.line_ids, [ + {'account_id': acc_out.id, 'debit': 0, 'credit': 20}, + {'account_id': acc_valuation.id, 'debit': 20, 'credit': 0}, + ])