Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MIG][IMP] delivery_multi_destination: Introduce 'based on destination' delivery type #887

Open
wants to merge 1 commit into
base: 16.0
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions delivery_multi_destination/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl).

from . import models
from . import wizards
2 changes: 1 addition & 1 deletion delivery_multi_destination/__manifest__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

{
"name": "Multiple destinations for the same delivery method",
"version": "16.0.1.0.0",
"version": "16.0.2.0.0",
"category": "Delivery",
"website": "https://github.com/OCA/delivery-carrier",
"author": "Tecnativa, Odoo Community Association (OCA)",
Expand Down
1 change: 1 addition & 0 deletions delivery_multi_destination/demo/delivery_carrier_demo.xml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
<field name="name">International Carrier Inc.</field>
<field name="sequence">4</field>
<field name="destination_type">multi</field>
<field name="delivery_type">base_on_destination</field>
<field name="product_id" ref="product_product_delivery_carrier_multi" />
</record>
<record id="product_product_delivery_carrier_multi_child_1" model="product.product">
Expand Down
15 changes: 15 additions & 0 deletions delivery_multi_destination/migrations/16.0.2.0.0/pre-migration.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Copyright 2022 Coop IT Easy SC
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl)
from openupgradelib import openupgrade


@openupgrade.migrate()
def migrate(env, version):
openupgrade.logged_query(
env.cr,
"""
UPDATE delivery_carrier
SET delivery_type = 'base_on_destination'
WHERE destination_type = 'multi'
""",
)
36 changes: 33 additions & 3 deletions delivery_multi_destination/models/delivery_carrier.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,40 @@ class DeliveryCarrier(models.Model):
ondelete="cascade",
)
destination_type = fields.Selection(
selection=[("one", "One destination"), ("multi", "Multiple destinations")],
default="one",
required=True,
selection=[
("one", "One destination"),
("multi", "Multiple destinations"),
],
compute="_compute_destination_type",
inverse="_inverse_destination_type",
store=True,
)
delivery_type = fields.Selection(
selection_add=[("base_on_destination", "Based on Destination")],
ondelete={"base_on_destination": "set default"},
)

@api.depends("delivery_type")
def _compute_destination_type(self):
for carrier in self:
if carrier.delivery_type == "base_on_destination":
carrier.destination_type = "multi"
else:
carrier.destination_type = "one"

def _inverse_destination_type(self):
for carrier in self:
# Switch to multi
if carrier.destination_type == "multi":
carrier.delivery_type = "base_on_destination"
# Switch away from multi -> we know that destination_type is
# non-multi. However, in a hypothetical scenario where we switch
# from one non-multi destination_type to another, we don't want to
# forcibly reset delivery_type to 'fixed' each time, so we check
# whether delivery_type is invalid for a non-multi destination_type
# before we forcibly reset to 'fixed'.
elif carrier.delivery_type == "base_on_destination":
carrier.delivery_type = "fixed"

@api.onchange("destination_type", "child_ids")
def _onchange_destination_type(self):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Introduced 'based on destination' delivery type. This is the delivery type used
by all multi-destination carriers instead of 'fixed'. Consequently, destination
type has been turned into a computed field that checks if the delivery type is
'based on destination' or not.
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ def setUpClass(cls):
{
"name": "Test carrier single",
"destination_type": "one",
"fixed_price": 100,
"child_ids": False,
}
)
Expand All @@ -84,8 +85,7 @@ def _create_carrier(self, childs):
carrier_form = Form(self.env["delivery.carrier"])
carrier_form.name = "Test carrier multi"
carrier_form.product_id = self.product
carrier_form.delivery_type = "fixed"
carrier_form.fixed_price = 100
carrier_form.delivery_type = "base_on_destination"
# this needs to be done in this order
carrier_form.destination_type = "multi"
for child_item in childs:
Expand Down Expand Up @@ -136,6 +136,20 @@ def test_delivery_multi_destination(self):
sale_order_line = order.order_line.filtered("is_delivery")
self.assertAlmostEqual(sale_order_line.price_unit, 150, 2)

def test_compute(self):
self.carrier_multi.delivery_type = "fixed"
self.assertEqual(self.carrier_multi.destination_type, "one")
self.carrier_multi.delivery_type = "base_on_destination"
self.assertEqual(self.carrier_multi.destination_type, "multi")

def test_inverse(self):
self.carrier_multi.destination_type = "one"
self.assertEqual(self.carrier_multi.destination_type, "one")
self.assertEqual(self.carrier_multi.delivery_type, "fixed")
self.carrier_multi.destination_type = "multi"
self.assertEqual(self.carrier_multi.destination_type, "multi")
self.assertEqual(self.carrier_multi.delivery_type, "base_on_destination")

def test_search(self):
carriers = self.env["delivery.carrier"].search([])
children_carrier = self.carrier_multi.with_context(
Expand Down
5 changes: 5 additions & 0 deletions delivery_multi_destination/wizards/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# SPDX-FileCopyrightText: 2024 Coop IT Easy SC
#
# SPDX-License-Identifier: AGPL-3.0-or-later

from . import choose_delivery_carrier
18 changes: 18 additions & 0 deletions delivery_multi_destination/wizards/choose_delivery_carrier.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# SPDX-FileCopyrightText: 2024 Coop IT Easy SC
#
# SPDX-License-Identifier: AGPL-3.0-or-later

from odoo import api, models


class ChooseDeliveryCarrier(models.TransientModel):
_inherit = "choose.delivery.carrier"

@api.onchange("carrier_id")
def _onchange_carrier_id(self):
result = super()._onchange_carrier_id()
if self.delivery_type == "base_on_destination":
vals = self._get_shipment_rate()
if vals.get("error_message"):
return {"error": vals["error_message"]}

Check warning on line 17 in delivery_multi_destination/wizards/choose_delivery_carrier.py

View check run for this annotation

Codecov / codecov/patch

delivery_multi_destination/wizards/choose_delivery_carrier.py#L17

Added line #L17 was not covered by tests
return result
Loading