From b896b35dd2c4d35c0c2cb18e97b03b3e818ab441 Mon Sep 17 00:00:00 2001 From: "Victor Piryns (pivi)" Date: Mon, 5 Dec 2022 13:41:16 +0000 Subject: [PATCH] [FIX] {sale,purchase}_stock: propagate package type to stock moves Current behaviour: If you have a confirmed SO, with a `sale.order.line` that has a `product_packaging_id`, and you write a new `product_packaging_id`, the "Delivery Order" has 2 lines, 1 move with the old qty and the old packaging, and another line with the difference of qty and the new packaging. Same behaviour is present on purchase side. Expected behaviour: If you have multiple `stock.move.line` from the same `sale.order.line`, they should be able to merge, when you changed the `product_packaging_id`. Ex: If you edit an SOL from 1 pack of 10 to 1 pack of 20, we should have 1 move line with qty 20 in packs of 20, instead of 2 lines, one with qty 10 in packs of 10, and another line with qty 10 in packs of 20. Same behaviour is expected on purchase side. Steps to reproduce: - Install Sales and Inventory - Activate "Product Packaging" in Settings - Create a new product with 2 types of packaging - PackOf10 with quantity of 10 - PackOf20 with quantity of 20 - Create a SO with a new line that product, quantity 10 - Confirm the SO - Edit the SOL with 1 pack of 20 (`product_uom_qty`=20) - The "Delivery Order" has 2 lines, instead of 1 with the new packaging Reason for the problem: When saving the SO/PO, a new `procurement` is created which will create a new `stock.move.line` with the new packaging. This will prevent the lines to merge correctly, because they have different packaging. Fix: When writing the `product_packaging_id` on a `sale.order.line`/`purchase.order.line`, we directly write the package on the `stock.move.line`, before any `procurements` are created, so the generate move lines can correctly be merged. Affected versions: - 15.0 - saas-15.2 - saas-15.3 - 16.0 - master opw-3002612 closes odoo/odoo#114866 X-original-commit: 87ed4b2f6c9c36202a1a4f3de143f638cf38d047 Signed-off-by: William Henrotin (whe) Signed-off-by: Piryns Victor (pivi) --- addons/purchase_stock/models/purchase.py | 6 +++ .../tests/test_purchase_order.py | 54 +++++++++++++++++++ addons/sale_stock/models/sale_order_line.py | 5 ++ addons/sale_stock/tests/test_sale_stock.py | 38 +++++++++++-- addons/stock/models/stock_move.py | 26 +++++++++ 5 files changed, 124 insertions(+), 5 deletions(-) diff --git a/addons/purchase_stock/models/purchase.py b/addons/purchase_stock/models/purchase.py index 642dc4212dc..2c5876e794d 100644 --- a/addons/purchase_stock/models/purchase.py +++ b/addons/purchase_stock/models/purchase.py @@ -363,6 +363,12 @@ class PurchaseOrderLine(models.Model): new_date = fields.Datetime.to_datetime(values['date_planned']) self.filtered(lambda l: not l.display_type)._update_move_date_deadline(new_date) lines = self.filtered(lambda l: l.order_id.state == 'purchase') + + if 'product_packaging_id' in values: + self.move_ids.filtered( + lambda m: m.state not in ['cancel', 'done'] + ).product_packaging_id = values['product_packaging_id'] + previous_product_qty = {line.id: line.product_uom_qty for line in lines} result = super(PurchaseOrderLine, self).write(values) if 'price_unit' in values: diff --git a/addons/purchase_stock/tests/test_purchase_order.py b/addons/purchase_stock/tests/test_purchase_order.py index 230cd88b2ac..48b59e3c86d 100644 --- a/addons/purchase_stock/tests/test_purchase_order.py +++ b/addons/purchase_stock/tests/test_purchase_order.py @@ -526,6 +526,60 @@ class TestPurchaseOrder(ValuationReconciliationTestCommon): self.assertEqual(po.picking_ids.move_ids.product_uom_qty, 8) + def test_packaging_propagation(self): + """ + Editing the packaging on an purchase.order.line + should propagate to the delivery order, so that + when we are editing the packaging, the lines can be merged + with the new packaging and quantity. + """ + # set the 3 step route + warehouse = self.company_data['default_warehouse'] + warehouse.reception_steps = 'three_steps' + packOf10 = self.env['product.packaging'].create({ + 'name': 'PackOf10', + 'product_id': self.product_a.id, + 'qty': 10 + }) + + packOf20 = self.env['product.packaging'].create({ + 'name': 'PackOf20', + 'product_id': self.product_a.id, + 'qty': 20 + }) + + po = self.env['purchase.order'].create({ + 'partner_id': self.partner_a.id, + 'order_line': [ + (0, 0, { + 'product_id': self.product_a.id, + 'product_uom_qty': 10.0, + 'product_uom': self.product_a.uom_id.id, + 'product_packaging_id': packOf10.id, + })], + }) + po.button_confirm() + # the 3 moves for the 3 steps + step_1 = po.order_line.move_ids + step_2 = step_1.move_dest_ids + step_3 = step_2.move_dest_ids + self.assertEqual(step_1.product_packaging_id, packOf10) + self.assertEqual(step_2.product_packaging_id, packOf10) + self.assertEqual(step_3.product_packaging_id, packOf10) + + po.order_line[0].write({ + 'product_packaging_id': packOf20.id, + 'product_uom_qty': 20 + }) + self.assertEqual(step_1.product_packaging_id, packOf20) + self.assertEqual(step_2.product_packaging_id, packOf20) + self.assertEqual(step_3.product_packaging_id, packOf20) + + po.order_line[0].write({'product_packaging_id': False}) + self.assertFalse(step_1.product_packaging_id) + self.assertFalse(step_2.product_packaging_id) + self.assertFalse(step_3.product_packaging_id) + def test_putaway_strategy_in_backorder(self): stock_location = self.company_data['default_warehouse'].lot_stock_id sub_loc_01 = self.env['stock.location'].create([{ diff --git a/addons/sale_stock/models/sale_order_line.py b/addons/sale_stock/models/sale_order_line.py index 736f06c2058..69fce515c41 100644 --- a/addons/sale_stock/models/sale_order_line.py +++ b/addons/sale_stock/models/sale_order_line.py @@ -176,6 +176,11 @@ class SaleOrderLine(models.Model): if 'product_uom_qty' in values: lines = self.filtered(lambda r: r.state == 'sale' and not r.is_expense) + if 'product_packaging_id' in values: + self.move_ids.filtered( + lambda m: m.state not in ['cancel', 'done'] + ).product_packaging_id = values['product_packaging_id'] + previous_product_uom_qty = {line.id: line.product_uom_qty for line in lines} res = super(SaleOrderLine, self).write(values) if lines: diff --git a/addons/sale_stock/tests/test_sale_stock.py b/addons/sale_stock/tests/test_sale_stock.py index ab76630efad..1d5db1b20ed 100644 --- a/addons/sale_stock/tests/test_sale_stock.py +++ b/addons/sale_stock/tests/test_sale_stock.py @@ -1043,14 +1043,23 @@ class TestSaleStock(TestSaleCommon, ValuationReconciliationTestCommon): """Create a SO with lines using packaging, check the packaging propagate to its move. """ + warehouse = self.company_data['default_warehouse'] + warehouse.delivery_steps = 'pick_pack_ship' product = self.env['product.product'].create({ 'name': 'Product with packaging', 'type': 'product', }) - packaging = self.env['product.packaging'].create({ - 'name': 'box', + packOf10 = self.env['product.packaging'].create({ + 'name': 'PackOf10', 'product_id': product.id, + 'qty': 10 + }) + + packOf20 = self.env['product.packaging'].create({ + 'name': 'PackOf20', + 'product_id': product.id, + 'qty': 20 }) so = self.env['sale.order'].create({ @@ -1058,13 +1067,32 @@ class TestSaleStock(TestSaleCommon, ValuationReconciliationTestCommon): 'order_line': [ (0, 0, { 'product_id': product.id, - 'product_uom_qty': 1.0, + 'product_uom_qty': 10.0, 'product_uom': product.uom_id.id, - 'product_packaging_id': packaging.id, + 'product_packaging_id': packOf10.id, })], }) so.action_confirm() - self.assertEqual(so.order_line.move_ids.product_packaging_id, packaging) + pick = so.order_line.move_ids + pack = pick.move_orig_ids + ship = pack.move_orig_ids + self.assertEqual(pick.product_packaging_id, packOf10) + self.assertEqual(pack.product_packaging_id, packOf10) + self.assertEqual(ship.product_packaging_id, packOf10) + + so.order_line[0].write({ + 'product_packaging_id': packOf20.id, + 'product_uom_qty': 20 + }) + self.assertEqual(so.order_line.move_ids.product_packaging_id, packOf20) + self.assertEqual(pick.product_packaging_id, packOf20) + self.assertEqual(pack.product_packaging_id, packOf20) + self.assertEqual(ship.product_packaging_id, packOf20) + + so.order_line[0].write({'product_packaging_id': False}) + self.assertFalse(pick.product_packaging_id) + self.assertFalse(pack.product_packaging_id) + self.assertFalse(ship.product_packaging_id) def test_15_cancel_delivery(self): """ Suppose the option "Lock Confirmed Sales" enabled and a product with the invoicing diff --git a/addons/stock/models/stock_move.py b/addons/stock/models/stock_move.py index 6b8563f8e14..fe54a445608 100644 --- a/addons/stock/models/stock_move.py +++ b/addons/stock/models/stock_move.py @@ -688,6 +688,9 @@ Please change the quantity done or the rounding precision of your unit of measur receipt_moves_to_reassign |= move_to_unreserve.filtered(lambda m: m.location_id.usage == 'supplier') receipt_moves_to_reassign |= (self - move_to_unreserve).filtered(lambda m: m.location_id.usage == 'supplier' and m.state in ('partially_available', 'assigned')) move_to_recompute_state |= self - move_to_unreserve - receipt_moves_to_reassign + # propagate product_packaging_id changes in the stock move chain + if 'product_packaging_id' in vals: + self._propagate_product_packaging(vals['product_packaging_id']) if 'date_deadline' in vals: self._set_date_deadline(vals.get('date_deadline')) res = super(StockMove, self).write(vals) @@ -697,6 +700,29 @@ Please change the quantity done or the rounding precision of your unit of measur receipt_moves_to_reassign._action_assign() return res + def _propagate_product_packaging(self, product_package_id): + """ + Propagate the product_packaging_id of a move to its destination and origin. + If there is a bifurcation in the chain we do not propagate the package. + """ + already_propagated_ids = self.env.context.get('product_packaging_propagation_ids', set()) | set(self.ids) + self = self.with_context(product_packaging_propagation_ids=already_propagated_ids) + for move in self: + # propagate on destination move + for move_dest in move.move_dest_ids: + if move_dest.id not in already_propagated_ids and \ + move_dest.state not in ['cancel', 'done'] and \ + move_dest.product_packaging_id.id != product_package_id and \ + move_dest.move_orig_ids == move: # checks that you are the only parent move of your destination + move_dest.product_packaging_id = product_package_id + # propagate on origin move + for move_orig in move.move_orig_ids: + if move_orig.id not in already_propagated_ids and \ + move_orig.state not in ['cancel', 'done'] and \ + move_orig.product_packaging_id.id != product_package_id and \ + move_orig.move_dest_ids == move: # checks that you are the only child move of your origin + move_orig.product_packaging_id = product_package_id + def _delay_alert_get_documents(self): """Returns a list of recordset of the documents linked to the stock.move in `self` in order to post the delay alert next activity. These documents are deduplicated. This method is meant