From 10e0ac377cc235d81819eaa2a4ec2b6a60fffd70 Mon Sep 17 00:00:00 2001
From: Mathieu Walravens <wama@odoo.com>
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#136024

X-original-commit: 30de83a541557e80ed455af2d42eed43de2b84e3
Signed-off-by: William Henrotin (whe) <whe@odoo.com>
---
 addons/stock_account/models/stock_move.py     |   6 +-
 .../tests/test_stockvaluationlayer.py         | 115 ++++++++++++++++++
 2 files changed, 119 insertions(+), 2 deletions(-)

diff --git a/addons/stock_account/models/stock_move.py b/addons/stock_account/models/stock_move.py
index d089431e19ed..429b2afb3595 100644
--- a/addons/stock_account/models/stock_move.py
+++ b/addons/stock_account/models/stock_move.py
@@ -579,11 +579,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)._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)._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)._prepare_account_move_vals(acc_dest, acc_valuation, journal_id, qty, description, svl_id, cost))
+                    am_vals.append(self.with_company(self.company_id)._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 441d0e619bb6..4c08846613e7 100644
--- a/addons/stock_account/tests/test_stockvaluationlayer.py
+++ b/addons/stock_account/tests/test_stockvaluationlayer.py
@@ -1140,6 +1140,35 @@ class TestAngloSaxonAccounting(AccountTestInvoicingCommon):
 
         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_lines[0].move_line_ids[0].qty_done = quantity_to_return
+        return_pick._action_done()
+        return return_pick.move_lines
+
     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
@@ -1177,3 +1206,89 @@ class TestAngloSaxonAccounting(AccountTestInvoicingCommon):
         self.assertEqual(len(anglo_lines), 2)
         self.assertEqual(abs(anglo_lines[0].balance), 10)
         self.assertEqual(abs(anglo_lines[1].balance), 10)
+
+    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_lines[0].move_line_ids[0].qty_done = 2
+        return_pick._action_done()
+        move2 = return_pick.move_lines
+
+        # 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},
+        ])
-- 
GitLab