diff --git a/addons/delivery/i18n/delivery.pot b/addons/delivery/i18n/delivery.pot index a66aedc9388..777f0b4668e 100644 --- a/addons/delivery/i18n/delivery.pot +++ b/addons/delivery/i18n/delivery.pot @@ -354,8 +354,8 @@ msgstr "" #. module: delivery #: model:ir.model.fields,help:delivery.field_delivery_carrier__invoice_policy msgid "" -"Estimated Cost: the customer will be invoiced the estimated cost of the shipping.\n" -"Real Cost: the customer will be invoiced the real cost of the shipping, the cost of theshipping will be updated on the SO after the delivery." +"Estimated Cost: the customer will be invoiced the estimated cost of the " +"shipping." msgstr "" #. module: delivery diff --git a/addons/stock/i18n/stock.pot b/addons/stock/i18n/stock.pot index e837b78a557..afa549260e1 100644 --- a/addons/stock/i18n/stock.pot +++ b/addons/stock/i18n/stock.pot @@ -4585,6 +4585,11 @@ msgstr "" msgid "Lot Label Format to auto-print" msgstr "" +#. module: stock +#: model:ir.model,name:stock.model_report_stock_label_lot_template_view +msgid "Lot Label Report" +msgstr "" + #. module: stock #: model:ir.model.fields,field_description:stock.field_product_product__lot_properties_definition msgid "Lot Properties" @@ -10200,6 +10205,15 @@ msgid "" " or split the same package into two location." msgstr "" +#. module: stock +#. odoo-python +#: code:addons/stock/models/stock_picking.py:0 +#, python-format +msgid "" +"You cannot pack products into the same package when they are from different " +"transfers with different operation types." +msgstr "" + #. module: stock #. odoo-python #: code:addons/stock/models/stock_move.py:0 diff --git a/addons/stock/models/stock_picking.py b/addons/stock/models/stock_picking.py index 54384f0a302..3f3601dfffa 100644 --- a/addons/stock/models/stock_picking.py +++ b/addons/stock/models/stock_picking.py @@ -1499,7 +1499,11 @@ class Picking(models.Model): return action return package_id - def _package_move_lines(self): + def _package_move_lines(self, batch_pack=False): + # in theory, the picking_type should always be the same (i.e. for batch transfers), + # but customizations may bypass it and cause unexpected behavior so we avoid allowing those situations + if len(self.picking_type_id) > 1: + raise UserError(_("You cannot pack products into the same package when they are from different transfers with different operation types.")) quantity_move_line_ids = self.move_line_ids.filtered( lambda ml: float_compare(ml.quantity, 0.0, precision_rounding=ml.product_uom_id.rounding) > 0 and diff --git a/addons/stock/tests/test_packing.py b/addons/stock/tests/test_packing.py index 254a90b90c6..a323cd701db 100644 --- a/addons/stock/tests/test_packing.py +++ b/addons/stock/tests/test_packing.py @@ -1511,6 +1511,69 @@ class TestPacking(TestPackingCommon): self.assertEqual(quantB.quantity, 4, "All 4 units of product B should be in location B") self.assertEqual(quantB.package_id.id, pack.id, "Product B should still be in the initial package.") + def test_expected_package_move_lines(self): + """ Test direct calling of `_package_move_lines` since it doesn't handle all multi-record cases + It's unlikely this situations will occur, but in case it is for customizations/future features, + ensure that we don't have unexpected behavior """ + + self.env['stock.quant']._update_available_quantity(self.productA, self.stock_location, 20.0) + + internal_picking_1 = self.env['stock.picking'].create({ + 'picking_type_id': self.warehouse.int_type_id.id, + 'location_id': self.stock_location.id, + 'location_dest_id': self.stock_location.id, + }) + self.env['stock.move'].create({ + 'name': self.productA.name, + 'product_id': self.productA.id, + 'product_uom_qty': 5, + 'quantity': 5, + 'product_uom': self.productA.uom_id.id, + 'picking_id': internal_picking_1.id, + 'location_id': self.stock_location.id, + 'location_dest_id': self.stock_location.id, + }) + + internal_picking_2 = self.env['stock.picking'].create({ + 'picking_type_id': self.warehouse.int_type_id.id, + 'location_id': self.stock_location.id, + 'location_dest_id': self.stock_location.id, + }) + self.env['stock.move'].create({ + 'name': self.productA.name, + 'product_id': self.productA.id, + 'product_uom_qty': 5, + 'quantity': 5, + 'product_uom': self.productA.uom_id.id, + 'picking_id': internal_picking_2.id, + 'location_id': self.stock_location.id, + 'location_dest_id': self.stock_location.id, + }) + + in_picking_1 = self.env['stock.picking'].create({ + 'picking_type_id': self.warehouse.in_type_id.id, + 'location_id': self.stock_location.id, + 'location_dest_id': self.stock_location.id, + 'state': 'draft', + }) + self.env['stock.move'].create({ + 'name': self.productA.name, + 'product_id': self.productA.id, + 'product_uom_qty': 5, + 'quantity': 5, + 'product_uom': self.productA.uom_id.id, + 'picking_id': in_picking_1.id, + 'location_id': self.stock_location.id, + 'location_dest_id': self.stock_location.id, + }) + + # can't mix operation types + with self.assertRaises(UserError): + move_lines_to_pack = (internal_picking_1 | in_picking_1)._package_move_lines() + + move_lines_to_pack = (internal_picking_1 | internal_picking_2)._package_move_lines() + self.assertEqual(len(move_lines_to_pack), 2, "all move lines in pickings should have been selected to pack") + def test_package_selection(self): """ Test that the package selection is correct when using the least_package_strategy: diff --git a/addons/stock_delivery/i18n/stock_delivery.pot b/addons/stock_delivery/i18n/stock_delivery.pot index 6b42816cfa1..597ad82431b 100644 --- a/addons/stock_delivery/i18n/stock_delivery.pot +++ b/addons/stock_delivery/i18n/stock_delivery.pot @@ -559,6 +559,16 @@ msgstr "" msgid "Weight:" msgstr "" +#. module: stock_delivery +#. odoo-python +#: code:addons/stock_delivery/models/stock_picking.py:0 +#, python-format +msgid "" +"You cannot pack products into the same package when they have different " +"carriers (i.e. check that all of their transfers have a carrier assigned and" +" are using the same carrier)." +msgstr "" + #. module: stock_delivery #: model_terms:ir.ui.view,arch_db:stock_delivery.delivery_tracking_url_warning_form msgid "You have multiple tracker links, they are available in the chatter." diff --git a/addons/stock_delivery/models/stock_picking.py b/addons/stock_delivery/models/stock_picking.py index 8574f7c379d..10a47fe31e9 100644 --- a/addons/stock_delivery/models/stock_picking.py +++ b/addons/stock_delivery/models/stock_picking.py @@ -138,12 +138,15 @@ class StockPicking(models.Model): def _pre_put_in_pack_hook(self, move_line_ids): res = super(StockPicking, self)._pre_put_in_pack_hook(move_line_ids) if not res: - if self.carrier_id: - return self._set_delivery_package_type() + if move_line_ids.carrier_id: + if len(move_line_ids.carrier_id) > 1 or any(not ml.carrier_id for ml in move_line_ids): + # avoid (duplicate) costs for products + raise UserError(_("You cannot pack products into the same package when they have different carriers (i.e. check that all of their transfers have a carrier assigned and are using the same carrier).")) + return self._set_delivery_package_type(batch_pack=len(move_line_ids.picking_id) > 1) else: return res - def _set_delivery_package_type(self): + def _set_delivery_package_type(self, batch_pack=False): """ This method returns an action allowing to set the package type and the shipping weight on the stock.quant.package. """ @@ -152,7 +155,8 @@ class StockPicking(models.Model): context = dict( self.env.context, current_package_carrier_type=self.carrier_id.delivery_type, - default_picking_id=self.id + default_picking_id=self.id, + batch_pack=batch_pack, ) # As we pass the `delivery_type` ('fixed' or 'base_on_rule' by default) in a key who # correspond to the `package_carrier_type` ('none' to default), we make a conversion. diff --git a/addons/stock_delivery/tests/test_packing_delivery.py b/addons/stock_delivery/tests/test_packing_delivery.py index 42fcfb67c26..f0f19aaf649 100644 --- a/addons/stock_delivery/tests/test_packing_delivery.py +++ b/addons/stock_delivery/tests/test_packing_delivery.py @@ -1,9 +1,11 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from odoo.addons.stock.tests.test_packing import TestPackingCommon +from odoo.exceptions import UserError from odoo.tests import Form from unittest.mock import patch + class TestPacking(TestPackingCommon): @classmethod @@ -141,3 +143,56 @@ class TestPacking(TestPackingCommon): for p in so.picking_ids: self.assertEqual(p.carrier_tracking_ref, "666") + + def test_batch_picking_delivery(self): + """ + Check that when Put in Pack is called for batch pickings (i.e. faked with multi-record action + calling to avoid extra batch+delivery module for just a test) then: + - Same delivery carrier = works + - Different delivery carriers = UserError + """ + self.env['stock.quant']._update_available_quantity(self.productA, self.stock_location, 4.0) + another_test_carrier_product = self.env['product.product'].create({ + 'name': 'Another test carrier product', + 'type': 'service', + }) + test_carrier = self.env['delivery.carrier'].create({ + 'name': 'Another Test carrier', + 'delivery_type': 'fixed', + 'product_id': another_test_carrier_product.id, + }) + + delivery_1 = self.env['stock.picking'].create({ + 'partner_id': self.env['res.partner'].create({'name': 'A partner'}).id, + 'picking_type_id': self.warehouse.out_type_id.id, + 'location_id': self.stock_location.id, + 'location_dest_id': self.customer_location.id, + 'carrier_id': self.test_carrier.id + }) + ml_1 = self.env['stock.move.line'].create({ + 'product_id': self.productA.id, + 'product_uom_id': self.productA.uom_id.id, + 'picking_id': delivery_1.id, + 'quantity': 1, + 'location_id': self.stock_location.id, + 'location_dest_id': self.customer_location.id + }) + + # Test that differing carrier + put in pack = UserError + delivery_2 = delivery_1.copy({'carrier_id': test_carrier.id}) + ml_1.copy({'picking_id': delivery_2.id, 'quantity': 1}) + # recreate the `action_put_in_pack`` steps so we don't have to add test to new module for batch pickings + # to use batch version of method (which bypass the ensure_one() check in the stock_picking action) + move_lines_to_pack = (delivery_1 | delivery_2)._package_move_lines() + self.assertEqual(len(move_lines_to_pack), 2, 'There should be move lines that can be "put in pack"') + with self.assertRaises(UserError): + delivery_1._pre_put_in_pack_hook(move_lines_to_pack) + + # Test that same carrier + put in pack = OK! + delivery_2.carrier_id = delivery_1.carrier_id + move_lines_to_pack = (delivery_1 | delivery_2)._package_move_lines() + self.assertEqual(len(move_lines_to_pack), 2, 'There should be move lines that can be "put in pack"') + delivery_1._pre_put_in_pack_hook(move_lines_to_pack) + package = delivery_1._put_in_pack(move_lines_to_pack) + self.assertEqual(delivery_1.move_line_ids.result_package_id, package, 'Delivery 1 moves should have been put in package.') + self.assertEqual(delivery_2.move_line_ids.result_package_id, package, 'Delivery 2 moves should have been put in package.') diff --git a/addons/stock_delivery/wizard/choose_delivery_package.py b/addons/stock_delivery/wizard/choose_delivery_package.py index 346eb691f18..f5b735fedab 100644 --- a/addons/stock_delivery/wizard/choose_delivery_package.py +++ b/addons/stock_delivery/wizard/choose_delivery_package.py @@ -44,8 +44,7 @@ class ChooseDeliveryPackage(models.TransientModel): return {'warning': warning_mess} def action_put_in_pack(self): - move_line_ids = self.picking_id._package_move_lines() - + move_line_ids = self.picking_id._package_move_lines(batch_pack=self.env.context.get("batch_pack")) delivery_package = self.picking_id._put_in_pack(move_line_ids) # write shipping weight and package type on 'stock_quant_package' if needed if self.delivery_package_type_id: diff --git a/addons/stock_picking_batch/models/stock_picking.py b/addons/stock_picking_batch/models/stock_picking.py index a2b2af07b60..83f478f6acc 100644 --- a/addons/stock_picking_batch/models/stock_picking.py +++ b/addons/stock_picking_batch/models/stock_picking.py @@ -232,6 +232,11 @@ class StockPicking(models.Model): return domain + def _package_move_lines(self, batch_pack=False): + if batch_pack: + return super(StockPicking, self.batch_id.picking_ids if self.batch_id else self)._package_move_lines(batch_pack) + return super()._package_move_lines(batch_pack) + def assign_batch_user(self, user_id): if not user_id: return @@ -241,9 +246,6 @@ class StockPicking(models.Model): log_message = _('Assigned to %s Responsible', pick.batch_id._get_html_link()) pick.message_post(body=log_message) - def _package_move_lines(self): - return super(StockPicking, self.batch_id.picking_ids if self.batch_id else self)._package_move_lines() - def action_view_batch(self): self.ensure_one() return { diff --git a/addons/stock_picking_batch/models/stock_picking_batch.py b/addons/stock_picking_batch/models/stock_picking_batch.py index 4090581ec58..afbb7c55b74 100644 --- a/addons/stock_picking_batch/models/stock_picking_batch.py +++ b/addons/stock_picking_batch/models/stock_picking_batch.py @@ -236,7 +236,7 @@ class StockPickingBatch(models.Model): """ self.ensure_one() if self.state not in ('done', 'cancel'): - move_line_ids = self.picking_ids[0]._package_move_lines() + move_line_ids = self.picking_ids[0]._package_move_lines(batch_pack=True) if move_line_ids: res = move_line_ids.picking_id[0]._pre_put_in_pack_hook(move_line_ids) if res: diff --git a/addons/stock_picking_batch/tests/test_batch_picking.py b/addons/stock_picking_batch/tests/test_batch_picking.py index 272b0de3644..2e870c3c691 100644 --- a/addons/stock_picking_batch/tests/test_batch_picking.py +++ b/addons/stock_picking_batch/tests/test_batch_picking.py @@ -319,6 +319,7 @@ class TestBatchPicking(TransactionCase): self.assertEqual(self.picking_client_1.state, 'done', 'Picking 1 should be done') self.assertEqual(self.picking_client_1.move_ids.product_uom_qty, 5, 'initial demand should be 5 after picking split') self.assertFalse(self.picking_client_2.batch_id) + def test_put_in_pack(self): self.env['stock.quant']._update_available_quantity(self.productA, self.stock_location, 10.0) self.env['stock.quant']._update_available_quantity(self.productB, self.stock_location, 10.0) @@ -357,6 +358,20 @@ class TestBatchPicking(TransactionCase): # final package location should be correctly set based on wizard self.assertEqual(package.location_id.id, self.customer_location.id) + def test_put_in_pack_within_single_picking(self): + """ Test that when `action_put_in_pack` is called on a picking that is also in a batch, + only that picking's moves are put in the pack """ + + self.env['stock.quant']._update_available_quantity(self.productA, self.stock_location, 10.0) + self.env['stock.quant']._update_available_quantity(self.productB, self.stock_location, 10.0) + + self.batch.action_confirm() + self.batch.action_assign() + self.batch.move_line_ids.quantity = 5 + package = self.picking_client_1.action_put_in_pack() + self.assertEqual(self.picking_client_1.move_line_ids.result_package_id, package) + self.assertFalse(self.picking_client_2.move_line_ids.result_package_id, "Other picking in batch shouldn't have been put in a package") + def test_auto_batch(self): """ Test a simple auto-batch scenario with new picking type to avoid conflicts with existing picking types. The pickings look like this: