From 515b9a78e66049f12a09b9150c0e77ccb925de02 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Voet=20=28ryv=29?= <ryv@odoo.com>
Date: Fri, 13 Nov 2020 15:02:28 +0000
Subject: [PATCH] [REF] stock,mrp: make picking_type_id compute store in
 stock.move

In order to have always a picking_type_id on a stock move (and
consistent with his parent model), make it compute store.
Then clean the call of picking_type_id without change the behavior
(/!\ `move.picking_id.picking_type_id != move.picking_type_id` because
of mrp). Fix some test where the location(_dest) was unset in moves.

Also:
- Clean the mess in the create of `stock.picking` about that
and add a onchange to keep location(_dest) consistent with moves.
- The move `description_picking` is set when the `picking_type_id`
change, not only at the create method.

task-2373638
---
 addons/mrp/models/mrp_production.py     |  2 -
 addons/mrp/models/stock_move.py         |  7 +++
 addons/mrp/tests/test_procurement.py    |  2 +
 addons/stock/models/stock_move.py       | 57 ++++++++++++-------------
 addons/stock/models/stock_move_line.py  |  2 +-
 addons/stock/models/stock_picking.py    | 27 +++++-------
 addons/stock/models/stock_rule.py       |  6 +--
 addons/stock/tests/test_proc_rule.py    |  2 +
 addons/stock/views/stock_move_views.xml |  1 +
 9 files changed, 52 insertions(+), 54 deletions(-)

diff --git a/addons/mrp/models/mrp_production.py b/addons/mrp/models/mrp_production.py
index 235ea0ee6426..b5bfb6b2e40f 100644
--- a/addons/mrp/models/mrp_production.py
+++ b/addons/mrp/models/mrp_production.py
@@ -651,8 +651,6 @@ class MrpProduction(models.Model):
             location.check_access_rule('read')
         except (AttributeError, AccessError):
             location = self.env['stock.warehouse'].search([('company_id', '=', self.env.company.id)], limit=1).lot_stock_id
-        self.move_raw_ids.update({'picking_type_id': self.picking_type_id})
-        self.move_finished_ids.update({'picking_type_id': self.picking_type_id})
         self.location_src_id = self.picking_type_id.default_location_src_id.id or location.id
         self.location_dest_id = self.picking_type_id.default_location_dest_id.id or location.id
 
diff --git a/addons/mrp/models/stock_move.py b/addons/mrp/models/stock_move.py
index c2a61b97090c..75150cae3a0c 100644
--- a/addons/mrp/models/stock_move.py
+++ b/addons/mrp/models/stock_move.py
@@ -115,6 +115,13 @@ class StockMove(models.Model):
         for move in self:
             move.priority = move.raw_material_production_id.priority or move.priority or '0'
 
+    @api.depends('raw_material_production_id.picking_type_id', 'production_id.picking_type_id')
+    def _compute_picking_type_id(self):
+        super()._compute_picking_type_id()
+        for move in self:
+            if move.raw_material_production_id or move.production_id:
+                move.picking_type_id = (move.raw_material_production_id or move.production_id).picking_type_id
+
     @api.depends('raw_material_production_id.lot_producing_id')
     def _compute_order_finished_lot_ids(self):
         for move in self:
diff --git a/addons/mrp/tests/test_procurement.py b/addons/mrp/tests/test_procurement.py
index 297adfcf4550..0932da42624f 100644
--- a/addons/mrp/tests/test_procurement.py
+++ b/addons/mrp/tests/test_procurement.py
@@ -453,6 +453,8 @@ class TestProcurement(TestMrpCommon):
                 'product_uom': product_1.uom_id.id,
                 'product_uom_qty': 10.00,
                 'procure_method': 'make_to_stock',
+                'location_id': self.warehouse.lot_stock_id.id,
+                'location_dest_id': self.ref('stock.stock_location_customers'),
             })],
         })
         pick_output.action_confirm()  # should trigger orderpoint to create and confirm 1st MO
diff --git a/addons/stock/models/stock_move.py b/addons/stock/models/stock_move.py
index dfeaa578e1ef..0699dde62a4d 100644
--- a/addons/stock/models/stock_move.py
+++ b/addons/stock/models/stock_move.py
@@ -132,7 +132,7 @@ class StockMove(models.Model):
         'Propagate cancel and split', default=True,
         help='If checked, when this move is cancelled, cancel the linked move too')
     delay_alert_date = fields.Datetime('Delay Alert Date', help='Process at this date to be on time', compute="_compute_delay_alert_date", store=True)
-    picking_type_id = fields.Many2one('stock.picking.type', 'Operation Type', check_company=True)
+    picking_type_id = fields.Many2one('stock.picking.type', 'Operation Type', compute='_compute_picking_type_id', store=True, check_company=True)
     inventory_id = fields.Many2one('stock.inventory', 'Inventory', check_company=True)
     move_line_ids = fields.One2many('stock.move.line', 'move_id')
     move_line_nosuggest_ids = fields.One2many('stock.move.line', 'move_id', domain=['|', ('product_qty', '=', 0.0), ('qty_done', '!=', 0.0)])
@@ -156,10 +156,10 @@ class StockMove(models.Model):
     warehouse_id = fields.Many2one('stock.warehouse', 'Warehouse', help="Technical field depicting the warehouse to consider for the route selection on the next procurement (if any).")
     has_tracking = fields.Selection(related='product_id.tracking', string='Product with Tracking')
     quantity_done = fields.Float('Quantity Done', compute='_quantity_done_compute', digits='Product Unit of Measure', inverse='_quantity_done_set')
-    show_operations = fields.Boolean(related='picking_id.picking_type_id.show_operations', readonly=False)
+    show_operations = fields.Boolean(related='picking_id.picking_type_id.show_operations')
+    picking_code = fields.Selection(related='picking_id.picking_type_id.code', readonly=True)
     show_details_visible = fields.Boolean('Details Visible', compute='_compute_show_details_visible')
     show_reserved_availability = fields.Boolean('From Supplier', compute='_compute_show_reserved_availability')
-    picking_code = fields.Selection(related='picking_id.picking_type_id.code', readonly=True)
     product_type = fields.Selection(related='product_id.type', readonly=True)
     additional = fields.Boolean("Whether the move was added after the picking's confirmation", default=False)
     is_locked = fields.Boolean(compute='_compute_is_locked', readonly=True)
@@ -200,6 +200,12 @@ class StockMove(models.Model):
         for move in self:
             move.priority = move.picking_id.priority or '0'
 
+    @api.depends('picking_id.picking_type_id')
+    def _compute_picking_type_id(self):
+        for move in self:
+            if move.picking_id:
+                move.picking_type_id = move.picking_id.picking_type_id
+
     @api.depends('picking_id.is_locked')
     def _compute_is_locked(self):
         for move in self:
@@ -225,10 +231,10 @@ class StockMove(models.Model):
             elif len(move.move_line_ids) > 1:
                 move.show_details_visible = True
             else:
-                move.show_details_visible = (((consignment_enabled and move.picking_id.picking_type_id.code != 'incoming') or
+                move.show_details_visible = (((consignment_enabled and move.picking_code != 'incoming') or
                                              show_details_visible or move.has_tracking != 'none') and
                                              move._show_details_in_draft() and
-                                             move.picking_id.picking_type_id.show_operations is False)
+                                             move.show_operations is False)
 
     def _compute_show_reserved_availability(self):
         """ This field is only of use in an attrs in the picking view, in order to hide the
@@ -300,7 +306,7 @@ class StockMove(models.Model):
             else:
                 move.delay_alert_date = False
 
-    @api.depends('move_line_ids.qty_done', 'move_line_ids.product_uom_id', 'move_line_nosuggest_ids.qty_done', 'picking_type_id')
+    @api.depends('move_line_ids.qty_done', 'move_line_ids.product_uom_id', 'move_line_nosuggest_ids.qty_done', 'picking_type_id.show_reserved')
     def _quantity_done_compute(self):
         """ This field represents the sum of the move lines `qty_done`. It allows the user to know
         if there is still work to do.
@@ -400,7 +406,7 @@ class StockMove(models.Model):
                 total_availability = self.env['stock.quant']._get_available_quantity(move.product_id, move.location_id) if move.product_id else 0.0
                 move.availability = min(move.product_qty, total_availability)
 
-    @api.depends('product_id', 'picking_type_id', 'picking_id', 'reserved_availability', 'priority', 'state', 'product_uom_qty', 'location_id')
+    @api.depends('product_id', 'picking_type_id', 'reserved_availability', 'priority', 'state', 'product_uom_qty', 'location_id')
     def _compute_forecast_information(self):
         """ Compute forecasted information of the related product by warehouse."""
         self.forecast_availability = False
@@ -415,11 +421,10 @@ class StockMove(models.Model):
 
         outgoing_unreserved_moves_per_warehouse = defaultdict(lambda: self.env['stock.move'])
         for move in product_moves:
-            picking_type = move.picking_type_id or move.picking_id.picking_type_id
             is_unreserved = move.state in ('waiting', 'confirmed', 'partially_available')
-            if picking_type.code in self._consuming_picking_types() and is_unreserved:
+            if move.picking_type_id.code in self._consuming_picking_types() and is_unreserved:
                 outgoing_unreserved_moves_per_warehouse[warehouse_by_location[move.location_id]] |= move
-            elif picking_type.code in self._consuming_picking_types():
+            elif move.picking_type_id.code in self._consuming_picking_types():
                 move.forecast_availability = move.reserved_availability
 
         for warehouse, moves in outgoing_unreserved_moves_per_warehouse.items():
@@ -497,17 +502,13 @@ class StockMove(models.Model):
                     move_lines_commands.append((0, 0, move_line_vals))
             move.write({'move_line_ids': move_lines_commands})
 
-    @api.depends('picking_id', 'picking_type_id', 'date')
+    @api.depends('picking_type_id', 'date')
     def _compute_reservation_date(self):
         for move in self:
-            if move.state in ['draft', 'confirmed', 'waiting', 'partially_available']:
-                picking_type_id = False
-                if move.picking_type_id:
-                    picking_type_id = move.picking_type_id
-                elif move.picking_id:
-                    picking_type_id = move.picking_id.picking_type_id
-                if picking_type_id and picking_type_id.reservation_method == 'by_date':
-                    move.reservation_date = fields.Date.to_date(move.date) - timedelta(days=move.picking_type_id.reservation_days_before)
+            if move.state not in ['draft', 'confirmed', 'waiting', 'partially_available']:
+                continue
+            if move.picking_type_id and move.picking_type_id.reservation_method == 'by_date':
+                move.reservation_date = fields.Date.to_date(move.date) - timedelta(days=move.picking_type_id.reservation_days_before)
 
     @api.constrains('product_uom')
     def _check_uom(self):
@@ -614,13 +615,11 @@ class StockMove(models.Model):
         """
         self.ensure_one()
 
-        picking_type_id = self.picking_type_id or self.picking_id.picking_type_id
-
         # If "show suggestions" is not checked on the picking type, we have to filter out the
         # reserved move lines. We do this by displaying `move_line_nosuggest_ids`. We use
         # different views to display one field or another so that the webclient doesn't have to
         # fetch both.
-        if picking_type_id.show_reserved:
+        if self.picking_type_id.show_reserved:
             view = self.env.ref('stock.view_stock_move_operations')
         else:
             view = self.env.ref('stock.view_stock_move_nosuggest_operations')
@@ -637,8 +636,8 @@ class StockMove(models.Model):
             'context': dict(
                 self.env.context,
                 show_owner=self.picking_type_id.code != 'incoming',
-                show_lots_m2o=self.has_tracking != 'none' and (picking_type_id.use_existing_lots or self.state == 'done' or self.origin_returned_move_id.id),  # able to create lots, whatever the value of ` use_create_lots`.
-                show_lots_text=self.has_tracking != 'none' and picking_type_id.use_create_lots and not picking_type_id.use_existing_lots and self.state != 'done' and not self.origin_returned_move_id.id,
+                show_lots_m2o=self.has_tracking != 'none' and (self.picking_type_id.use_existing_lots or self.state == 'done' or self.origin_returned_move_id.id),  # able to create lots, whatever the value of ` use_create_lots`.
+                show_lots_text=self.has_tracking != 'none' and self.picking_type_id.use_create_lots and not self.picking_type_id.use_existing_lots and self.state != 'done' and not self.origin_returned_move_id.id,
                 show_source_location=self.picking_type_id.code != 'incoming',
                 show_destination_location=self.picking_type_id.code != 'outgoing',
                 show_package=not self.location_id.usage == 'supplier',
@@ -897,7 +896,7 @@ class StockMove(models.Model):
                 'warning': {'title': _('Warning'), 'message': _('Existing Serial numbers (%s). Please correct the serial numbers encoded.') % ','.join(used_lots.lot_id.mapped('display_name'))}
             }
 
-    @api.onchange('move_line_ids', 'move_line_nosuggest_ids')
+    @api.onchange('move_line_ids', 'move_line_nosuggest_ids', 'picking_type_id')
     def onchange_move_line_ids(self):
         if not self.picking_type_id.use_create_lots:
             # This onchange manages the creation of multiple lot name. We don't
@@ -1141,7 +1140,6 @@ class StockMove(models.Model):
                        and move.state == 'confirmed'
                        and (move._should_bypass_reservation()
                             or move.picking_type_id.reservation_method == 'at_confirm'
-                            or move.picking_id.picking_type_id.reservation_method == 'at_confirm'
                             or (move.reservation_date and move.reservation_date <= fields.Date.today())))\
              ._action_assign()
         if new_push_moves:
@@ -1169,7 +1167,7 @@ class StockMove(models.Model):
             'move_dest_ids': self,
             'group_id': group_id,
             'route_ids': self.route_ids,
-            'warehouse_id': self.warehouse_id or self.picking_id.picking_type_id.warehouse_id or self.picking_type_id.warehouse_id,
+            'warehouse_id': self.warehouse_id or self.picking_type_id.warehouse_id,
             'priority': self.priority,
             'orderpoint_id': self.orderpoint_id,
         }
@@ -1800,8 +1798,7 @@ class StockMove(models.Model):
         for move in self:
             domains.append([('product_id', '=', move.product_id.id), ('location_id', '=', move.location_dest_id.id)])
         static_domain = [('state', 'in', ['confirmed', 'partially_available']), ('procure_method', '=', 'make_to_stock')]
-        reservation_domain = ['|', '|', ('picking_type_id.reservation_method', '=', 'at_confirm'),
-                              ('picking_id.picking_type_id.reservation_method', '=', 'at_confirm'),
-                              ('reservation_date', '<=', fields.Date.today())]
+        reservation_domain = ['|', ('picking_type_id.reservation_method', '=', 'at_confirm'),
+                                   ('reservation_date', '<=', fields.Date.today())]
         moves_to_reserve = self.env['stock.move'].search(expression.AND([static_domain, expression.OR(domains), reservation_domain]))
         moves_to_reserve._action_assign()
diff --git a/addons/stock/models/stock_move_line.py b/addons/stock/models/stock_move_line.py
index 1b64256df4f3..4ebbccecb2da 100644
--- a/addons/stock/models/stock_move_line.py
+++ b/addons/stock/models/stock_move_line.py
@@ -59,6 +59,7 @@ class StockMoveLine(models.Model):
     picking_code = fields.Selection(related='picking_id.picking_type_id.code', readonly=True)
     picking_type_use_create_lots = fields.Boolean(related='picking_id.picking_type_id.use_create_lots', readonly=True)
     picking_type_use_existing_lots = fields.Boolean(related='picking_id.picking_type_id.use_existing_lots', readonly=True)
+    picking_type_entire_packs = fields.Boolean(related='picking_id.picking_type_id.show_entire_packs', readonly=True)
     state = fields.Selection(related='move_id.state', store=True, related_sudo=False)
     is_initial_demand_editable = fields.Boolean(related='move_id.is_initial_demand_editable', readonly=False)
     is_locked = fields.Boolean(related='move_id.is_locked', default=True, readonly=True)
@@ -67,7 +68,6 @@ class StockMoveLine(models.Model):
     reference = fields.Char(related='move_id.reference', store=True, related_sudo=False, readonly=False)
     tracking = fields.Selection(related='product_id.tracking', readonly=True)
     origin = fields.Char(related='move_id.origin', string='Source')
-    picking_type_entire_packs = fields.Boolean(related='picking_id.picking_type_id.show_entire_packs', readonly=True)
     description_picking = fields.Text(string="Description picking")
 
     @api.depends('picking_id.picking_type_id', 'product_id.tracking')
diff --git a/addons/stock/models/stock_picking.py b/addons/stock/models/stock_picking.py
index 783523594b9b..139fa870cdb8 100644
--- a/addons/stock/models/stock_picking.py
+++ b/addons/stock/models/stock_picking.py
@@ -595,9 +595,11 @@ class Picking(models.Model):
             self.location_id = location_id
             self.location_dest_id = location_dest_id
             (self.move_lines | self.move_ids_without_package).update({
-                "picking_type_id": self.picking_type_id,
+                "picking_type_id": self.picking_type_id,  # The compute store doesn't work in case of One2many inverse (move_ids_without_package)
                 "company_id": self.company_id,
             })
+            for move in (self.move_lines | self.move_ids_without_package):
+                move.description_picking = move.product_id._get_description(move.picking_type_id)
 
         if self.partner_id and self.partner_id.picking_warn:
             if self.partner_id.picking_warn == 'no-message' and self.partner_id.parent_id:
@@ -614,6 +616,13 @@ class Picking(models.Model):
                     'message': partner.picking_warn_msg
                 }}
 
+    @api.onchange('location_id', 'location_dest_id')
+    def _onchange_locations(self):
+        (self.move_lines | self.move_ids_without_package).update({
+            "location_id": self.location_id,
+            "location_dest_id": self.location_dest_id
+        })
+
     @api.model
     def create(self, vals):
         defaults = self.default_get(['name', 'picking_type_id'])
@@ -622,22 +631,6 @@ class Picking(models.Model):
             if picking_type.sequence_id:
                 vals['name'] = picking_type.sequence_id.next_by_id()
 
-        # As the on_change in one2many list is WIP, we will overwrite the locations on the stock moves here
-        # As it is a create the format will be a list of (0, 0, dict)
-        moves = vals.get('move_lines', []) + vals.get('move_ids_without_package', [])
-        if moves and vals.get('location_id') and vals.get('location_dest_id'):
-            for move in moves:
-                if len(move) == 3 and move[0] == 0:
-                    move[2]['location_id'] = vals['location_id']
-                    move[2]['location_dest_id'] = vals['location_dest_id']
-                    # When creating a new picking, a move can have no `company_id` (create before
-                    # picking type was defined) or a different `company_id` (the picking type was
-                    # changed for an another company picking type after the move was created).
-                    # So, we define the `company_id` in one of these cases.
-                    picking_type = self.env['stock.picking.type'].browse(vals['picking_type_id'])
-                    if 'picking_type_id' not in move[2] or move[2]['picking_type_id'] != picking_type.id:
-                        move[2]['picking_type_id'] = picking_type.id
-                        move[2]['company_id'] = picking_type.company_id.id
         # make sure to write `schedule_date` *after* the `stock.move` creation in
         # order to get a determinist execution of `_set_scheduled_date`
         scheduled_date = vals.pop('scheduled_date', False)
diff --git a/addons/stock/models/stock_rule.py b/addons/stock/models/stock_rule.py
index f9e2d273d91d..4491185142b9 100644
--- a/addons/stock/models/stock_rule.py
+++ b/addons/stock/models/stock_rule.py
@@ -489,10 +489,8 @@ class ProcurementGroup(models.Model):
             ('state', 'in', ['confirmed', 'partially_available']),
             ('product_uom_qty', '!=', 0.0)
         ]
-        # TODO: make picking_type_id/reservation method one value on stock.move
-        moves_domain = expression.AND([moves_domain, ['|', '|', ('picking_type_id.reservation_method', '=', 'at_confirm'),
-                                                                ('picking_id.picking_type_id.reservation_method', '=', 'at_confirm'),
-                                                                ('reservation_date', '<=', fields.Date.today())]])
+        moves_domain = expression.AND([moves_domain, ['|', ('picking_type_id.reservation_method', '=', 'at_confirm'),
+                                                           ('reservation_date', '<=', fields.Date.today())]])
         if company_id:
             moves_domain = expression.AND([[('company_id', '=', company_id)], moves_domain])
         return moves_domain
diff --git a/addons/stock/tests/test_proc_rule.py b/addons/stock/tests/test_proc_rule.py
index 85d52d2e2885..e6435dcaa4bc 100644
--- a/addons/stock/tests/test_proc_rule.py
+++ b/addons/stock/tests/test_proc_rule.py
@@ -52,6 +52,8 @@ class TestProcRule(TransactionCase):
                 'product_uom': product.uom_id.id,
                 'product_uom_qty': 10.00,
                 'procure_method': 'make_to_order',
+                'location_id': self.ref('stock.stock_location_output'),
+                'location_dest_id': self.ref('stock.stock_location_customers'),
             })],
         }
         pick_output = self.env['stock.picking'].create(vals)
diff --git a/addons/stock/views/stock_move_views.xml b/addons/stock/views/stock_move_views.xml
index 1a40df4f6888..b3d53c08c9a7 100644
--- a/addons/stock/views/stock_move_views.xml
+++ b/addons/stock/views/stock_move_views.xml
@@ -135,6 +135,7 @@
                     <field name="location_id" invisible="1"/>
                     <field name="location_dest_id" invisible="1"/>
                     <field name="picking_id" invisible="1"/>
+                    <field name="picking_type_id" invisible="1"/>
                     <field name="is_locked" invisible="1"/>
                     <field name="picking_type_entire_packs" invisible="1"/>
                     <field name="display_assign_serial" invisible="1"/>
-- 
GitLab