From fbffdc71b23887e960409968b6f0e9579aa69507 Mon Sep 17 00:00:00 2001
From: Ernesto Tejeda <ernesto.tejeda@tecnativa.com>
Date: Wed, 19 Aug 2020 00:19:25 -0400
Subject: [PATCH] [FIX+IMP] rma, rma_sale: fix bugs and add improvements
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Fix thrown error when trying to download a picking from the portal.
- Add the hook method to prepare RMA values ​​from the return pick wizard.
- Add the access rule for portal users.
- Show the portal 'Request RMAs' button on the sales page only to users
related to the sales order.
---
 rma/controllers/main.py             | 16 ++++++++--------
 rma/data/mail_data.xml              |  2 +-
 rma/i18n/es.po                      |  2 +-
 rma/models/rma.py                   |  6 ++++++
 rma/models/stock_move.py            | 26 ++++++++++++++++++++++++++
 rma/models/stock_warehouse.py       |  2 +-
 rma/security/rma_security.xml       | 11 +++++++++--
 rma/views/rma_portal_templates.xml  |  4 ++--
 rma/views/rma_views.xml             |  2 +-
 rma/wizard/stock_picking_return.py  | 27 ++++-----------------------
 rma_sale/controllers/sale_portal.py |  3 +++
 rma_sale/models/__init__.py         |  1 +
 rma_sale/models/sale.py             |  1 -
 rma_sale/models/stock_move.py       | 13 +++++++++++++
 14 files changed, 76 insertions(+), 40 deletions(-)
 create mode 100644 rma_sale/models/stock_move.py

diff --git a/rma/controllers/main.py b/rma/controllers/main.py
index f53942f1..aeb6d1c8 100644
--- a/rma/controllers/main.py
+++ b/rma/controllers/main.py
@@ -102,13 +102,13 @@ class PortalRma(CustomerPortal):
         values = self._rma_get_page_view_values(rma_sudo, access_token, **kw)
         return request.render("rma.portal_rma_page", values)
 
-    @http.route(['/my/rma/picking/pdf/<int:picking_id>'],
+    @http.route(['/my/rma/picking/pdf/<int:rma_id>/<int:picking_id>'],
                 type='http', auth="public", website=True)
-    def portal_my_rma_picking_report(self, picking_id, access_token=None,
-                                     **kw):
+    def portal_my_rma_picking_report(self, rma_id, picking_id,
+                                     access_token=None, **kw):
         try:
-            picking_sudo = self._stock_picking_check_access(
-                picking_id, access_token=access_token)
+            picking_sudo = self._picking_check_access(
+                rma_id, picking_id, access_token=access_token)
         except exceptions.AccessError:
             return request.redirect('/my')
         report_sudo = request.env.ref('stock.action_report_delivery').sudo()
@@ -119,14 +119,14 @@ class PortalRma(CustomerPortal):
         ]
         return request.make_response(pdf, headers=pdfhttpheaders)
 
-    def _stock_picking_check_access(self, picking_id, access_token=None):
+    def _picking_check_access(self, rma_id, picking_id, access_token=None):
+        rma = request.env['rma'].browse([rma_id])
         picking = request.env['stock.picking'].browse([picking_id])
         picking_sudo = picking.sudo()
         try:
             picking.check_access_rights('read')
             picking.check_access_rule('read')
         except exceptions.AccessError:
-            if not access_token or not consteq(
-                    picking_sudo.sale_id.access_token, access_token):
+            if not access_token or not consteq(rma.access_token, access_token):
                 raise
         return picking_sudo
diff --git a/rma/data/mail_data.xml b/rma/data/mail_data.xml
index 207747bc..7c118726 100644
--- a/rma/data/mail_data.xml
+++ b/rma/data/mail_data.xml
@@ -5,7 +5,7 @@
         <field name="name">Draft RMA</field>
         <field name="res_model">rma</field>
         <field name="default" eval="False"/>
-        <field name="description">New RMA in draft state</field>
+        <field name="description">RMA in draft state</field>
     </record>
     <!-- rma_team-related subtypes for messaging / Chatter -->
     <record id="mt_rma_team_rma_draft" model="mail.message.subtype">
diff --git a/rma/i18n/es.po b/rma/i18n/es.po
index 5df1ef08..edcfa9d2 100644
--- a/rma/i18n/es.po
+++ b/rma/i18n/es.po
@@ -1355,7 +1355,7 @@ msgstr ""
 #: model:ir.model.fields,field_description:rma.field_rma__state
 #: model_terms:ir.ui.view,arch_db:rma.rma_view_search
 msgid "State"
-msgstr "Provincia"
+msgstr "Estado"
 
 #. module: rma
 #: code:addons/rma/controllers/main.py:40
diff --git a/rma/models/rma.py b/rma/models/rma.py
index 8a9ddcd8..ab95a420 100644
--- a/rma/models/rma.py
+++ b/rma/models/rma.py
@@ -1080,6 +1080,12 @@ class Rma(models.Model):
         }
 
     # Mail business methods
+    def _track_subtype(self, init_values):
+        self.ensure_one()
+        if 'state' in init_values and self.state == 'draft':
+            return 'rma.mt_rma_draft'
+        return super()._track_subtype(init_values)
+
     def message_new(self, msg_dict, custom_values=None):
         """Extract the needed values from an incoming rma emails data-set
         to be used to create an RMA.
diff --git a/rma/models/stock_move.py b/rma/models/stock_move.py
index 8ab511ce..c20e64c6 100644
--- a/rma/models/stock_move.py
+++ b/rma/models/stock_move.py
@@ -88,6 +88,32 @@ class StockMove(models.Model):
         res['rma_id'] = self.rma_id.id
         return res
 
+    def _prepare_return_rma_vals(self, original_picking):
+        """ hook method for preparing an RMA from the 'return picking wizard'.
+        """
+        self.ensure_one()
+        partner = original_picking.partner_id
+        if hasattr(original_picking, 'sale_id') and original_picking.sale_id:
+            partner_invoice_id = original_picking.sale_id.partner_invoice_id.id
+        else:
+            partner_invoice_id = partner.address_get(
+                ['invoice']).get('invoice', False),
+        return {
+            'user_id': self.env.user.id,
+            'partner_id': partner.id,
+            'partner_invoice_id': partner_invoice_id,
+            'origin': original_picking.name,
+            'picking_id': original_picking.id,
+            'move_id': self.origin_returned_move_id.id,
+            'product_id': self.origin_returned_move_id.product_id.id,
+            'product_uom_qty': self.product_uom_qty,
+            'product_uom': self.product_uom.id,
+            'reception_move_id': self.id,
+            'company_id': self.company_id.id,
+            'location_id': self.location_dest_id.id,
+            'state': 'confirmed',
+        }
+
 
 class StockRule(models.Model):
     _inherit = 'stock.rule'
diff --git a/rma/models/stock_warehouse.py b/rma/models/stock_warehouse.py
index 7c0716d2..5896accd 100644
--- a/rma/models/stock_warehouse.py
+++ b/rma/models/stock_warehouse.py
@@ -62,7 +62,7 @@ class StockWarehouse(models.Model):
             sequence_data = warehouse._get_sequence_values()
             warehouse.rma_in_type_id.sequence_id.write(
                 sequence_data['rma_in_type_id'])
-            warehouse.rma_in_type_id.sequence_id.write(
+            warehouse.rma_out_type_id.sequence_id.write(
                 sequence_data['rma_out_type_id'])
 
     def _get_picking_type_create_values(self, max_sequence):
diff --git a/rma/security/rma_security.xml b/rma/security/rma_security.xml
index 5a8037c9..6604005f 100644
--- a/rma/security/rma_security.xml
+++ b/rma/security/rma_security.xml
@@ -30,16 +30,23 @@
     <!-- Record Rules -->
     <record id="rma_rule_user_own" model="ir.rule">
         <field name="name">Personal RMAs</field>
-        <field ref="model_rma" name="model_id"/>
+        <field name="model_id" ref="model_rma"/>
         <field name="domain_force">['|',('user_id','=',user.id),('user_id','=',False)]</field>
         <field name="groups" eval="[(4, ref('rma_group_user_own'))]"/>
     </record>
     <record id="rma_rule_user_all" model="ir.rule">
         <field name="name">All RMAs</field>
-        <field ref="model_rma" name="model_id"/>
+        <field name="model_id" ref="model_rma"/>
         <field name="domain_force">[(1,'=',1)]</field>
         <field name="groups" eval="[(4, ref('rma_group_user_all'))]"/>
     </record>
+    <!-- RMA model rules for portal users -->
+    <record id="rma_rule_portal" model="ir.rule">
+        <field name="name">RMA portal users</field>
+        <field name="model_id" ref="rma.model_rma"/>
+        <field name="domain_force">[('message_partner_ids', 'child_of', [user.partner_id.commercial_partner_id.id])]</field>
+        <field name="groups" eval="[(4, ref('base.group_portal'))]"/>
+    </record>
     <!-- Multi-Company Rules -->
     <record id="rma_rule_multi_company" model="ir.rule">
         <field name="name">RMA multi-company</field>
diff --git a/rma/views/rma_portal_templates.xml b/rma/views/rma_portal_templates.xml
index e4ae704e..9219c044 100644
--- a/rma/views/rma_portal_templates.xml
+++ b/rma/views/rma_portal_templates.xml
@@ -191,7 +191,7 @@
                     <section t-if="rma.reception_move_id" id="reception_section" style="page-break-inside: auto;" class="mt32">
                         <strong class="d-block mb-1">Reception</strong>
                         <t t-set="picking" t-value="rma.reception_move_id.picking_id"/>
-                        <t t-set="report_url" t-value="'/my/rma/picking/pdf/%s?%s' % (picking.id, keep_query())"/>
+                        <t t-set="report_url" t-value="'/my/rma/picking/pdf/%s/%s?%s' % (rma.id, picking.id, keep_query())"/>
                         <a class="list-group-item list-group-item-action d-flex flex-wrap align-items-center justify-content-between py-2 px-3" t-att-href="report_url">
                             <div>
                                 <i class="fa fa-truck mr-1" role="img" aria-label="Download" title="Download"/>
@@ -230,7 +230,7 @@
                         <strong class="d-block mb-1">Delivery</strong>
                         <ul class="list-group mb-4">
                             <t t-foreach="rma.delivery_move_ids.mapped('picking_id')" t-as="picking">
-                                <t t-set="report_url" t-value="'/my/rma/picking/pdf/%s?%s' % (picking.id, keep_query())"/>
+                                <t t-set="report_url" t-value="'/my/rma/picking/pdf/%s/%s?%s' % (rma.id, picking.id, keep_query())"/>
                                 <a class="list-group-item list-group-item-action d-flex flex-wrap align-items-center justify-content-between py-2 px-3" t-att-href="report_url">
                                     <div>
                                         <i class="fa fa-truck mr-1" role="img" aria-label="Download" title="Download"/>
diff --git a/rma/views/rma_views.xml b/rma/views/rma_views.xml
index c86d14c2..463a29a3 100644
--- a/rma/views/rma_views.xml
+++ b/rma/views/rma_views.xml
@@ -164,7 +164,7 @@
                             <field name="picking_id"
                                    options="{'no_create': True}"/>
                             <field name="move_id"
-                                   attrs="{'required': [('picking_id', '!=', False)], 'readonly': [('picking_id', '=', False)]}"
+                                   attrs="{'required': [('picking_id', '!=', False)], 'readonly': ['|', ('picking_id', '=', False), ('state', '!=', 'draft')]}"
                                    options="{'no_create': True}"
                                    force_save="1"/>
                             <field name="product_id"
diff --git a/rma/wizard/stock_picking_return.py b/rma/wizard/stock_picking_return.py
index 0608c341..a9791370 100644
--- a/rma/wizard/stock_picking_return.py
+++ b/rma/wizard/stock_picking_return.py
@@ -51,33 +51,14 @@ class ReturnPicking(models.TransientModel):
             # picking and change the default picking type to rma picking type
             self_with_context = self.with_context(set_rma_picking_type=True)
             res = super(ReturnPicking, self_with_context).create_returns()
-            partner = self.picking_id.partner_id
-            if not partner:
+            if not self.picking_id.partner_id:
                 raise ValidationError(_(
                     "You must specify the 'Customer' in the "
                     "'Stock Picking' from which RMAs will be created"))
-            picking = self.picking_id
             returned_picking = self.env['stock.picking'].browse(res['res_id'])
-            if hasattr(picking, 'sale_id') and picking.sale_id:
-                partner_invoice_id = picking.sale_id.partner_invoice_id.id
-            else:
-                partner_invoice_id = partner.address_get(
-                    ['invoice']).get('invoice', False),
-            for move in returned_picking.move_lines:
-                self.env['rma'].create({
-                    'partner_id': partner.id,
-                    'partner_invoice_id': partner_invoice_id,
-                    'origin': picking.name,
-                    'picking_id': picking.id,
-                    'move_id': move.origin_returned_move_id.id,
-                    'product_id': move.origin_returned_move_id.product_id.id,
-                    'product_uom_qty': move.product_uom_qty,
-                    'product_uom': move.product_uom.id,
-                    'reception_move_id': move.id,
-                    'company_id': move.company_id.id,
-                    'location_id': move.location_dest_id.id,
-                    'state': 'confirmed',
-                })
+            vals_list = [move._prepare_return_rma_vals(self.picking_id)
+                         for move in returned_picking.move_lines]
+            self.env['rma'].create(vals_list)
             return res
         else:
             return super().create_returns()
diff --git a/rma_sale/controllers/sale_portal.py b/rma_sale/controllers/sale_portal.py
index 51dd8a7b..f7b32330 100644
--- a/rma_sale/controllers/sale_portal.py
+++ b/rma_sale/controllers/sale_portal.py
@@ -34,6 +34,9 @@ class CustomerPortal(CustomerPortal):
         rma = wizard.sudo().create_rma(from_portal=True)
         for rec in rma:
             rec.origin += _(' (Portal)')
+        # Add the user as follower of the created RMAs so they can
+        # later view them.
+        rma.message_subscribe([request.env.user.partner_id.id])
         if len(rma) == 0:
             route = order_sudo.get_portal_url()
         else:
diff --git a/rma_sale/models/__init__.py b/rma_sale/models/__init__.py
index d8e54bcb..0090dd37 100644
--- a/rma_sale/models/__init__.py
+++ b/rma_sale/models/__init__.py
@@ -2,3 +2,4 @@
 
 from . import rma
 from . import sale
+from . import stock_move
diff --git a/rma_sale/models/sale.py b/rma_sale/models/sale.py
index 25cd223a..a9a9208d 100644
--- a/rma_sale/models/sale.py
+++ b/rma_sale/models/sale.py
@@ -77,7 +77,6 @@ class SaleOrder(models.Model):
     def get_portal_delivery_rma_data(self):
         self.ensure_one()
         data = []
-
         rma_product = self.rma_ids.mapped('product_id')
         for line in self.order_line.filtered(
                 lambda r: r.product_id not in rma_product):
diff --git a/rma_sale/models/stock_move.py b/rma_sale/models/stock_move.py
new file mode 100644
index 00000000..382fcb8e
--- /dev/null
+++ b/rma_sale/models/stock_move.py
@@ -0,0 +1,13 @@
+# Copyright 2020 Tecnativa - Ernesto Tejeda
+# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl).
+
+from odoo import models
+
+
+class StockMove(models.Model):
+    _inherit = "stock.move"
+
+    def _prepare_return_rma_vals(self, original_picking):
+        res = super()._prepare_return_rma_vals(original_picking)
+        res.update(order_id=original_picking.sale_id.id)
+        return res