From fa31c471d23f9f6b50f07035f869c4e45c6751d3 Mon Sep 17 00:00:00 2001
From: Ernesto Tejeda <ernesto.tejeda@tecnativa.com>
Date: Fri, 30 Oct 2020 22:14:52 -0400
Subject: [PATCH] [FIX] rma: RMA quantities are not quantities on hand

---
 rma/__manifest__.py           |  1 +
 rma/data/stock_data.xml       |  9 ++++++++
 rma/hooks.py                  |  9 ++++----
 rma/models/stock_warehouse.py | 42 +++++++++++++++++++++++------------
 rma/tests/test_rma.py         |  7 ++++++
 5 files changed, 49 insertions(+), 19 deletions(-)
 create mode 100644 rma/data/stock_data.xml

diff --git a/rma/__manifest__.py b/rma/__manifest__.py
index fe9c9af8..3f4ac329 100644
--- a/rma/__manifest__.py
+++ b/rma/__manifest__.py
@@ -19,6 +19,7 @@
         "report/report.xml",
         "data/mail_data.xml",
         "data/rma_operation_data.xml",
+        "data/stock_data.xml",
         "security/rma_security.xml",
         "security/ir.model.access.csv",
         "wizard/stock_picking_return_views.xml",
diff --git a/rma/data/stock_data.xml b/rma/data/stock_data.xml
new file mode 100644
index 00000000..6b0812e6
--- /dev/null
+++ b/rma/data/stock_data.xml
@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="utf-8"?>
+<data noupdate="1">
+    <record id="stock_location_rma" model="stock.location">
+        <field name="name">RMA</field>
+        <field name="location_id" ref="stock.stock_location_locations"/>
+        <field name="usage">view</field>
+        <field name="company_id"></field>
+    </record>
+</data>
diff --git a/rma/hooks.py b/rma/hooks.py
index f5e24d84..96b97331 100644
--- a/rma/hooks.py
+++ b/rma/hooks.py
@@ -22,11 +22,10 @@ def post_init_hook(cr, registry):
 
     def create_rma_locations(warehouse):
         stock_location = env['stock.location']
-        location_vals = warehouse._get_locations_values({})
-        for field_name, values in location_vals.items():
-            if field_name == 'rma_loc_id' and not warehouse.rma_loc_id:
-                warehouse.rma_loc_id = stock_location.with_context(
-                    active_test=False).create(values).id
+        if not warehouse.rma_loc_id:
+            rma_location_vals = warehouse._get_rma_location_values()
+            warehouse.rma_loc_id = stock_location.with_context(
+                active_test=False).create(rma_location_vals).id
 
     def create_rma_picking_types(whs):
         ir_sequence_sudo = env['ir.sequence'].sudo()
diff --git a/rma/models/stock_warehouse.py b/rma/models/stock_warehouse.py
index cf2281c1..a591b4cd 100644
--- a/rma/models/stock_warehouse.py
+++ b/rma/models/stock_warehouse.py
@@ -1,7 +1,7 @@
 # Copyright 2020 Tecnativa - Ernesto Tejeda
 # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl).
 
-from odoo import fields, models, _
+from odoo import api, fields, models, _
 
 
 class StockWarehouse(models.Model):
@@ -27,19 +27,33 @@ class StockWarehouse(models.Model):
         string='RMA Location',
     )
 
-    def _get_locations_values(self, vals):
-        values = super()._get_locations_values(vals)
-        values.update({
-            'rma_loc_id': {
-                'name': 'RMA',
-                'active': True,
-                'return_location': True,
-                'usage': 'internal',
-                'company_id': vals.get('company_id', self.company_id.id),
-                'location_id': self.view_location_id.id,
-            },
-        })
-        return values
+    @api.model_create_multi
+    def create(self, vals_list):
+        """ To create an RMA location and link it with a new warehouse,
+        this method is overridden instead of '_get_locations_values'
+        method because the locations that are created with the
+        values ​​returned by that method are forced to be children
+        of view_location_id, and we don't want that.
+        """
+        res = super().create(vals_list)
+        stock_location = self.env['stock.location']
+        for record in res:
+            rma_location_vals = record._get_rma_location_values()
+            record.rma_loc_id = stock_location.create(rma_location_vals).id
+        return res
+
+    def _get_rma_location_values(self):
+        """ this method is intended to be used by 'create' method
+        to create a new RMA location to be linked to a new warehouse.
+        """
+        return {
+            'name': self.view_location_id.name,
+            'active': True,
+            'return_location': True,
+            'usage': 'internal',
+            'company_id': self.company_id.id,
+            'location_id': self.env.ref("rma.stock_location_rma").id,
+        }
 
     def _get_sequence_values(self):
         values = super()._get_sequence_values()
diff --git a/rma/tests/test_rma.py b/rma/tests/test_rma.py
index b6310ece..106f639d 100644
--- a/rma/tests/test_rma.py
+++ b/rma/tests/test_rma.py
@@ -656,3 +656,10 @@ class TestRma(SavepointCase):
         })
         self.assertFalse(warehouse.rma_in_type_id.use_create_lots)
         self.assertTrue(warehouse.rma_in_type_id.use_existing_lots)
+
+    def test_quantities_on_hand(self):
+        rma = self._create_rma(self.partner, self.product, 10, self.rma_loc)
+        rma.action_confirm()
+        rma.reception_move_id.quantity_done = 10
+        rma.reception_move_id.picking_id.action_done()
+        self.assertEqual(rma.product_id.qty_available, 0)