From 49d2e7b9c47cdb37c3d8e1ec6f65ea04140bfc3f Mon Sep 17 00:00:00 2001 From: Jacques-Etienne Baudoux Date: Tue, 16 Apr 2024 20:07:30 +0200 Subject: [PATCH 01/11] delivery_auto_refresh: docstring Don't modify standard method docstring --- delivery_auto_refresh/models/sale_order.py | 10 ++++------ delivery_auto_refresh/models/stock_picking.py | 8 ++++---- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/delivery_auto_refresh/models/sale_order.py b/delivery_auto_refresh/models/sale_order.py index ecb0a14a58..573236fe99 100644 --- a/delivery_auto_refresh/models/sale_order.py +++ b/delivery_auto_refresh/models/sale_order.py @@ -74,13 +74,13 @@ def _auto_refresh_delivery(self): @api.model def create(self, vals): - """Create or refresh delivery line on create.""" + # Create or refresh delivery line on create order = super().create(vals) order._auto_refresh_delivery() return order def write(self, vals): - """Create or refresh delivery line after saving.""" + # Create or refresh delivery line after saving res = super().write(vals) if self._get_param_auto_add_delivery_line() and not self.env.context.get( "auto_refresh_delivery" @@ -93,8 +93,7 @@ def write(self, vals): return res def _create_delivery_line(self, carrier, price_unit): - """Allow users to keep discounts to delivery lines. Unit price will - be recomputed anyway""" + # Allow users to keep discounts to delivery lines. Unit price will be recomputed anyway sol = super()._create_delivery_line(carrier, price_unit) discount = self.env.context.get("delivery_discount") if discount and sol: @@ -145,8 +144,7 @@ class SaleOrderLine(models.Model): _inherit = "sale.order.line" def _get_protected_fields(self): - """Avoid locked orders validation error when voiding the - delivery line""" + # Avoid locked orders validation error when voiding the delivery line fields = super()._get_protected_fields() if self.env.context.get("delivery_auto_refresh_override_locked"): return [x for x in fields if x not in ["product_uom_qty", "price_unit"]] diff --git a/delivery_auto_refresh/models/stock_picking.py b/delivery_auto_refresh/models/stock_picking.py index 0b58e91b2b..beadc553d3 100644 --- a/delivery_auto_refresh/models/stock_picking.py +++ b/delivery_auto_refresh/models/stock_picking.py @@ -8,10 +8,10 @@ class StockPicking(models.Model): _inherit = "stock.picking" def _add_delivery_cost_to_so(self): - """Update delivery price in SO (no matter the type of carrier invoicing policy) - and in picking from picking data if indicated so. Carriers based on rules - doesn't refresh with real picking data, only with SO ones. - """ + # Update delivery price in SO (no matter the type of carrier invoicing + # policy) and in picking from picking data if indicated so. Carriers + # based on rules doesn't refresh with real picking data, only with SO + # ones. res = super()._add_delivery_cost_to_so() refresh_after_picking = ( self.env["ir.config_parameter"] From 8a44424afe5d7b0a6bd3226cd14a57178b3306fa Mon Sep 17 00:00:00 2001 From: Jacques-Etienne Baudoux Date: Tue, 16 Apr 2024 20:10:41 +0200 Subject: [PATCH 02/11] delivery_auto_refresh: fix create in batch --- delivery_auto_refresh/models/sale_order.py | 12 +++++++----- delivery_auto_refresh/readme/CONTRIBUTORS.rst | 2 ++ 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/delivery_auto_refresh/models/sale_order.py b/delivery_auto_refresh/models/sale_order.py index 573236fe99..e0f5a2420c 100644 --- a/delivery_auto_refresh/models/sale_order.py +++ b/delivery_auto_refresh/models/sale_order.py @@ -1,5 +1,6 @@ # Copyright 2018 Tecnativa - Pedro M. Baeza # Copyright 2021 Tecnativa - Carlos Roca +# Copyright 2024 Jacques-Etienne Baudoux (BCIM) # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl). from odoo import api, fields, models @@ -72,12 +73,13 @@ def _auto_refresh_delivery(self): {"recompute_delivery_price": False} ) - @api.model - def create(self, vals): + @api.model_create_multi + def create(self, vals_list): # Create or refresh delivery line on create - order = super().create(vals) - order._auto_refresh_delivery() - return order + orders = super().create(vals_list) + for order in orders: + order._auto_refresh_delivery() + return orders def write(self, vals): # Create or refresh delivery line after saving diff --git a/delivery_auto_refresh/readme/CONTRIBUTORS.rst b/delivery_auto_refresh/readme/CONTRIBUTORS.rst index ef2018bbe0..d830360747 100644 --- a/delivery_auto_refresh/readme/CONTRIBUTORS.rst +++ b/delivery_auto_refresh/readme/CONTRIBUTORS.rst @@ -7,3 +7,5 @@ * Camptocamp : * Maksym Yankin * Simone Orsi + +* Jacques-Etienne Baudoux (BCIM) From 63fbcb1ae08b2935456ad3c93af13bf0da6d452c Mon Sep 17 00:00:00 2001 From: Jacques-Etienne Baudoux Date: Tue, 16 Apr 2024 20:31:37 +0200 Subject: [PATCH 03/11] delivery_auto_refresh: fix write & discount Limit change of write to calling the auto refresh feature Prevent to call auto refresh multiple times Don't pass discount variable in context --- delivery_auto_refresh/models/sale_order.py | 42 +++++++++++----------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/delivery_auto_refresh/models/sale_order.py b/delivery_auto_refresh/models/sale_order.py index e0f5a2420c..d02b7b5554 100644 --- a/delivery_auto_refresh/models/sale_order.py +++ b/delivery_auto_refresh/models/sale_order.py @@ -57,18 +57,32 @@ def _get_param_auto_add_delivery_line(self): .get_param("delivery_auto_refresh.auto_add_delivery_line") ) + def _get_delivery_discount(self): + self.ensure_one() + delivery_lines = self.order_line.filtered("is_delivery") + return delivery_lines[-1:].discount + def _auto_refresh_delivery(self): self.ensure_one() + if self.env.context.get("auto_refresh_delivery"): + return + if not self._get_param_auto_add_delivery_line(): + return + + delivery_discount = self._get_delivery_discount() + # Make sure that if you have removed the carrier, the line is gone if self.state in {"draft", "sent"}: # Context added to avoid the recursive calls and save the new # value of carrier_id self.with_context(auto_refresh_delivery=True)._remove_delivery_line() - if self._get_param_auto_add_delivery_line() and self.carrier_id: + if self.carrier_id: if self.state in {"draft", "sent"}: price_unit = self.carrier_id.rate_shipment(self)["price"] if not self.is_all_service: - self._create_delivery_line(self.carrier_id, price_unit) + sol = self._create_delivery_line(self.carrier_id, price_unit) + if delivery_discount and sol: + sol.discount = delivery_discount self.with_context(auto_refresh_delivery=True).write( {"recompute_delivery_price": False} ) @@ -82,26 +96,14 @@ def create(self, vals_list): return orders def write(self, vals): - # Create or refresh delivery line after saving - res = super().write(vals) - if self._get_param_auto_add_delivery_line() and not self.env.context.get( - "auto_refresh_delivery" - ): - for order in self: - delivery_line = order.order_line.filtered("is_delivery") - order.with_context( - delivery_discount=delivery_line[-1:].discount, - )._auto_refresh_delivery() + # Prevent to refresh delivery in the call to super + res = super(SaleOrder, self.with_context(auto_refresh_delivery=True)).write( + vals + ) + for order in self: + order._auto_refresh_delivery() return res - def _create_delivery_line(self, carrier, price_unit): - # Allow users to keep discounts to delivery lines. Unit price will be recomputed anyway - sol = super()._create_delivery_line(carrier, price_unit) - discount = self.env.context.get("delivery_discount") - if discount and sol: - sol.discount = discount - return sol - def set_delivery_line(self, carrier, amount): if self._get_param_auto_add_delivery_line() and self.state in {"draft", "sent"}: self.carrier_id = carrier.id From bff30d4ac94a353e13045ecc2335d1477a6049bb Mon Sep 17 00:00:00 2001 From: Jacques-Etienne Baudoux Date: Tue, 16 Apr 2024 20:35:53 +0200 Subject: [PATCH 04/11] delivery_auto_refresh: iterate over list, not set --- delivery_auto_refresh/models/sale_order.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/delivery_auto_refresh/models/sale_order.py b/delivery_auto_refresh/models/sale_order.py index d02b7b5554..33abdc921f 100644 --- a/delivery_auto_refresh/models/sale_order.py +++ b/delivery_auto_refresh/models/sale_order.py @@ -72,12 +72,12 @@ def _auto_refresh_delivery(self): delivery_discount = self._get_delivery_discount() # Make sure that if you have removed the carrier, the line is gone - if self.state in {"draft", "sent"}: + if self.state in ("draft", "sent"): # Context added to avoid the recursive calls and save the new # value of carrier_id self.with_context(auto_refresh_delivery=True)._remove_delivery_line() if self.carrier_id: - if self.state in {"draft", "sent"}: + if self.state in ("draft", "sent"): price_unit = self.carrier_id.rate_shipment(self)["price"] if not self.is_all_service: sol = self._create_delivery_line(self.carrier_id, price_unit) @@ -105,7 +105,7 @@ def write(self, vals): return res def set_delivery_line(self, carrier, amount): - if self._get_param_auto_add_delivery_line() and self.state in {"draft", "sent"}: + if self._get_param_auto_add_delivery_line() and self.state in ("draft", "sent"): self.carrier_id = carrier.id else: return super().set_delivery_line(carrier, amount) @@ -133,7 +133,7 @@ def _is_delivery_line_voidable(self): # nothing to be done either. If there are more than one delivery lines # we won't be doing anything as well. if ( - self.state not in {"done", "sale"} + self.state not in ("done", "sale") or self.invoice_ids or not self.order_line.filtered("is_delivery") or len(self.order_line.filtered("is_delivery")) > 1 From 4e803c70b6c1bbb6e3b0f9dbd4909bebf82bc9fa Mon Sep 17 00:00:00 2001 From: Jacques-Etienne Baudoux Date: Tue, 16 Apr 2024 22:06:02 +0200 Subject: [PATCH 05/11] delivery_auto_refresh: refresh the minimum Create or delete line only if necessary Update existing line if necessary --- delivery_auto_refresh/models/sale_order.py | 90 +++++++++++++++------- delivery_auto_refresh/readme/ROADMAP.rst | 2 - 2 files changed, 61 insertions(+), 31 deletions(-) diff --git a/delivery_auto_refresh/models/sale_order.py b/delivery_auto_refresh/models/sale_order.py index 33abdc921f..cc79346bf4 100644 --- a/delivery_auto_refresh/models/sale_order.py +++ b/delivery_auto_refresh/models/sale_order.py @@ -57,10 +57,37 @@ def _get_param_auto_add_delivery_line(self): .get_param("delivery_auto_refresh.auto_add_delivery_line") ) - def _get_delivery_discount(self): - self.ensure_one() - delivery_lines = self.order_line.filtered("is_delivery") - return delivery_lines[-1:].discount + def _update_delivery_line(self, delivery_line, price_unit): + """Update the existing delivery line""" + values = self._prepare_delivery_line_vals(self.carrier_id, price_unit) + new_vals = {} + for f, val in values.items(): + field_def = delivery_line._fields.get(f) + if isinstance(field_def, (fields.One2many, fields.Many2many)): + # Tax is set with a SET command + clear = update = False + for cmd in val: + if cmd[0] == fields.Command.SET: + if delivery_line[f].ids != cmd[2]: + update = True + else: + clear = True + if clear: + new_vals[f] = [fields.Command.CLEAR] + val + elif update: + new_vals[f] = val + elif isinstance(field_def, fields.Many2one): + if delivery_line[f].id != val: + new_vals[f] = val + elif f == "sequence": + # sequence is last sequence + 1. As the delivery line already + # exists, substract 1 + if delivery_line[f] != val - 1: + new_vals[f] = val + elif delivery_line[f] != val: + new_vals[f] = val + if new_vals: + delivery_line.write(new_vals) def _auto_refresh_delivery(self): self.ensure_one() @@ -68,29 +95,40 @@ def _auto_refresh_delivery(self): return if not self._get_param_auto_add_delivery_line(): return + if self.state not in ("draft", "sent"): + return - delivery_discount = self._get_delivery_discount() - - # Make sure that if you have removed the carrier, the line is gone - if self.state in ("draft", "sent"): - # Context added to avoid the recursive calls and save the new - # value of carrier_id - self.with_context(auto_refresh_delivery=True)._remove_delivery_line() - if self.carrier_id: - if self.state in ("draft", "sent"): - price_unit = self.carrier_id.rate_shipment(self)["price"] - if not self.is_all_service: - sol = self._create_delivery_line(self.carrier_id, price_unit) - if delivery_discount and sol: - sol.discount = delivery_discount - self.with_context(auto_refresh_delivery=True).write( - {"recompute_delivery_price": False} - ) + self = self.with_context(auto_refresh_delivery=True) + + if not self.carrier_id: + self._set_delivery_carrier() + + if not self.carrier_id or self.is_all_service: + self._remove_delivery_line() + else: + price_unit = self.carrier_id.rate_shipment(self)["price"] + delivery_lines = self.order_line.filtered("is_delivery") + if not delivery_lines: + self._create_delivery_line(self.carrier_id, price_unit) + elif len(delivery_lines) > 1: + delivery_discount = delivery_lines[-1:].discount + self._remove_delivery_line() + sol = self._create_delivery_line(self.carrier_id, price_unit) + if delivery_discount and sol: + sol.discount = delivery_discount + else: + self._update_delivery_line(delivery_lines[0], price_unit) + if self.recompute_delivery_price: + self.recompute_delivery_price = False @api.model_create_multi def create(self, vals_list): - # Create or refresh delivery line on create - orders = super().create(vals_list) + # Prevent to refresh delivery in the call to super + orders = ( + super(SaleOrder, self.with_context(auto_refresh_delivery=True)) + .create(vals_list) + .with_context(auto_refresh_delivery=False) + ) for order in orders: order._auto_refresh_delivery() return orders @@ -110,12 +148,6 @@ def set_delivery_line(self, carrier, amount): else: return super().set_delivery_line(carrier, amount) - def _remove_delivery_line(self): - current_carrier = self.carrier_id - res = super()._remove_delivery_line() - self.carrier_id = current_carrier - return res - def _is_delivery_line_voidable(self): """If the picking is returned before being invoiced, like when the picking is delivered but immediately return because the customer refused the order, diff --git a/delivery_auto_refresh/readme/ROADMAP.rst b/delivery_auto_refresh/readme/ROADMAP.rst index 7a7d2a7098..5d14317254 100644 --- a/delivery_auto_refresh/readme/ROADMAP.rst +++ b/delivery_auto_refresh/readme/ROADMAP.rst @@ -1,8 +1,6 @@ * After confirming the sales order, the price of the delivery line (if exists) will be only updated after the picking is transferred, but not when you might modify the order lines. -* There can be some duplicate delivery unset/set for assuring that the refresh - is done on all cases. * On multiple deliveries, second and successive pickings update the delivery price, but you can't invoice the new delivery price. * This is only working from user interface, as there's no way of making From b638f3cda0a4525f1874539c62f0e5b826d8e278 Mon Sep 17 00:00:00 2001 From: Jacques-Etienne Baudoux Date: Tue, 16 Apr 2024 23:01:23 +0200 Subject: [PATCH 06/11] delivery_auto_refresh: set carrier --- delivery_auto_refresh/__manifest__.py | 4 +- .../migrations/16.0.2.0.0/post-migration.py | 16 +++++ .../models/res_config_settings.py | 8 +-- delivery_auto_refresh/models/sale_order.py | 29 ++------- delivery_auto_refresh/readme/CONFIGURE.rst | 20 +++--- .../tests/test_delivery_auto_refresh.py | 16 ----- .../views/res_config_settings_views.xml | 64 ++++++++----------- 7 files changed, 58 insertions(+), 99 deletions(-) create mode 100644 delivery_auto_refresh/migrations/16.0.2.0.0/post-migration.py diff --git a/delivery_auto_refresh/__manifest__.py b/delivery_auto_refresh/__manifest__.py index b5184c9b79..258ef10056 100644 --- a/delivery_auto_refresh/__manifest__.py +++ b/delivery_auto_refresh/__manifest__.py @@ -4,13 +4,13 @@ { "name": "Auto-refresh delivery", "summary": "Auto-refresh delivery price in sales orders", - "version": "16.0.1.0.1", + "version": "16.0.2.0.0", "category": "Delivery", "website": "https://github.com/OCA/delivery-carrier", "author": "Tecnativa, Odoo Community Association (OCA)", "license": "AGPL-3", "application": False, "installable": True, - "depends": ["delivery"], + "depends": ["delivery", "sale_order_carrier_auto_assign"], "data": ["views/sale_order_views.xml", "views/res_config_settings_views.xml"], } diff --git a/delivery_auto_refresh/migrations/16.0.2.0.0/post-migration.py b/delivery_auto_refresh/migrations/16.0.2.0.0/post-migration.py new file mode 100644 index 0000000000..e06e317083 --- /dev/null +++ b/delivery_auto_refresh/migrations/16.0.2.0.0/post-migration.py @@ -0,0 +1,16 @@ +# Copyright 2024 Jacques-Etienne Baudoux (BCIM) +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). + +from odoo import SUPERUSER_ID, api + + +def _migrate_setting_to_company(env): + if env["ir.config_parameter"].get_param( + "delivery_auto_refresh.set_default_carrier" + ): + env["res.company"].search([]).carrier_auto_assign_on_create = True + + +def migrate(cr, version): + env = api.Environment(cr, SUPERUSER_ID, {}) + _migrate_setting_to_company(env) diff --git a/delivery_auto_refresh/models/res_config_settings.py b/delivery_auto_refresh/models/res_config_settings.py index 665b471fc8..fd73d5e5de 100644 --- a/delivery_auto_refresh/models/res_config_settings.py +++ b/delivery_auto_refresh/models/res_config_settings.py @@ -7,16 +7,12 @@ class ResConfigSettings(models.TransientModel): _inherit = "res.config.settings" - set_default_carrier = fields.Boolean( - "Set Default Carrier Automatically", - config_parameter="delivery_auto_refresh.set_default_carrier", - ) auto_add_delivery_line = fields.Boolean( - "Add Delivery Line Automatically", + "Refresh shipping cost line automatically", config_parameter="delivery_auto_refresh.auto_add_delivery_line", ) refresh_after_picking = fields.Boolean( - "Refresh After Picking Automatically", + "Refresh after picking automatically", config_parameter="delivery_auto_refresh.refresh_after_picking", ) auto_void_delivery_line = fields.Boolean( diff --git a/delivery_auto_refresh/models/sale_order.py b/delivery_auto_refresh/models/sale_order.py index cc79346bf4..10b789d8b5 100644 --- a/delivery_auto_refresh/models/sale_order.py +++ b/delivery_auto_refresh/models/sale_order.py @@ -9,36 +9,12 @@ class SaleOrder(models.Model): _inherit = "sale.order" + # Migration note: This field is not used anymore and can be dropped in later versions available_carrier_ids = fields.Many2many( comodel_name="delivery.carrier", compute="_compute_available_carrier_ids", ) - @api.onchange("partner_id", "partner_shipping_id") - def _onchange_partner_id(self): - res = None - if hasattr(super(), "_onchange_partner_id"): - res = super()._onchange_partner_id() - set_default_carrier = ( - self.env["ir.config_parameter"] - .sudo() - .get_param("delivery_auto_refresh.set_default_carrier") - ) - if set_default_carrier: - for order in self: - action = order.action_open_delivery_wizard() - carrier_id = self.env["delivery.carrier"].browse( - action["context"]["default_carrier_id"] - ) - # If the carrier isn't allowed for the current shipping address, we wont - # default to it. In that case we'd try to fallback to the former carrier. - order.carrier_id = fields.first( - (carrier_id | order.carrier_id).filtered( - lambda x: x in order.available_carrier_ids._origin - ) - ) - return res - @api.depends("partner_shipping_id") def _compute_available_carrier_ids(self): """We want to apply the same carriers filter in the header as in the wizard""" @@ -46,6 +22,8 @@ def _compute_available_carrier_ids(self): wizard = self.env["choose.delivery.carrier"].new({"order_id": sale.id}) sale.available_carrier_ids = wizard.available_carrier_ids._origin + # End migration note + def _get_param_auto_add_delivery_line(self): # When we have the context 'website_id' it means that we are doing the order from # e-commerce. So we don't want to add the delivery line automatically. @@ -144,6 +122,7 @@ def write(self, vals): def set_delivery_line(self, carrier, amount): if self._get_param_auto_add_delivery_line() and self.state in ("draft", "sent"): + # This will trigger an _auto_refresh_delivery in write self.carrier_id = carrier.id else: return super().set_delivery_line(carrier, amount) diff --git a/delivery_auto_refresh/readme/CONFIGURE.rst b/delivery_auto_refresh/readme/CONFIGURE.rst index 7aec743805..e7e1058df8 100644 --- a/delivery_auto_refresh/readme/CONFIGURE.rst +++ b/delivery_auto_refresh/readme/CONFIGURE.rst @@ -1,11 +1,9 @@ -* Go to *Sales > Settings > Shipping*. -* Locate the setting "Set Default Carrier Automatically" and activate it - if you want to have default carrier computed automatically. -* Locate the setting "Add Delivery Line Automatically" and activate it - if you want to add automatically the - delivery line on save. -* Locate the setting "Refresh After Picking Automatically" and activate it - if you want to refresh delivery price after transferring. -* Locate the setting "Void delivery lines automatically" and activate it - if you want to void the delivery line values (price, units ordered, units delivered) - in the sale order when the delivered picking is returned to refund prior to be invoiced. +Go to *Settings > Sales > Shipping*: +* Enable "Refresh shipping cost line automatically" if you want to add automatically the + delivery line on save and refresh the cost. This will also set the shipping method. +* Enable "Refresh After Picking Automatically" if you want to refresh delivery + price after delivering based on what has been delivered. +* Enable "Void delivery lines automatically" if you want to void the delivery + line values (price, units ordered, units delivered) in the sale order when + the delivery is returned to refund prior to be invoiced. + diff --git a/delivery_auto_refresh/tests/test_delivery_auto_refresh.py b/delivery_auto_refresh/tests/test_delivery_auto_refresh.py index 26492f6462..6d5dbc68c1 100644 --- a/delivery_auto_refresh/tests/test_delivery_auto_refresh.py +++ b/delivery_auto_refresh/tests/test_delivery_auto_refresh.py @@ -78,7 +78,6 @@ def setUpClass(cls): cls.refresh_after_picking = "delivery_auto_refresh.refresh_after_picking" cls.auto_void_delivery_line = "delivery_auto_refresh.auto_void_delivery_line" cls.settings = cls.env["res.config.settings"].create({}) - cls.settings.set_default_carrier = True cls.settings.execute() order_form = Form(cls.env["sale.order"]) order_form.partner_id = cls.partner @@ -197,21 +196,6 @@ def test_no_auto_refresh_picking(self): line_delivery = self.order.order_line.filtered("is_delivery") self.assertEqual(line_delivery.price_unit, 60) - def test_compute_carrier_id(self): - order_form_1 = Form(self.env["sale.order"]) - order_form_1.partner_id = self.partner - self.assertEqual(order_form_1.carrier_id, self.carrier_1) - partner_without_carrier = self.env["res.partner"].create( - { - "name": "Test partner without carrier", - "property_delivery_carrier_id": False, - } - ) - no_carrier = self.env["delivery.carrier"] - order_form_2 = Form(self.env["sale.order"]) - order_form_2.partner_id = partner_without_carrier - self.assertEqual(order_form_2.carrier_id, no_carrier) - def _confirm_sale_order(self, order): sale_form = Form(order) # Force the delivery line creation diff --git a/delivery_auto_refresh/views/res_config_settings_views.xml b/delivery_auto_refresh/views/res_config_settings_views.xml index e5751c4630..2cc2fa5695 100644 --- a/delivery_auto_refresh/views/res_config_settings_views.xml +++ b/delivery_auto_refresh/views/res_config_settings_views.xml @@ -4,51 +4,37 @@ res.config.settings.view.form.inherit.sale res.config.settings - + - -
-
- -
-
-
+ +
+
-
-
- -
-
-