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.

Sign up for GitHub

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

[16.0][FIX] sale_stock_release_channel_partner_by_date*: fix computation of release channel #955

Open
wants to merge 2 commits into
base: 16.0
Choose a base branch
from
+123 −11
Open

[16.0][FIX] sale_stock_release_channel_partner_by_date*: fix computation of release channel #955

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c2196c5
sale_stock_release_channel_partner_by_date_delivery: fix computation …
sebalix Nov 29, 2024
be46bf4
fixup! sale_stock_release_channel_partner_by_date_delivery: fix compu…
sebalix Nov 29, 2024
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
3 changes: 2 additions & 1 deletion sale_stock_release_channel_partner_by_date/models/sale_order.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ def _compute_release_channel_id(self):
if not rec._check_release_channel_partner_date_requirements():
continue
channel_date = rec.release_channel_partner_date_id
rec.release_channel_id = channel_date.release_channel_id
if channel_date:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you can check here that the warehouse is compatible with the currently selected channel? Like you did for the carrier.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes indeed. But I have still one open questions about this (you can check the test where there are FIXME in this PR):

If we have configured a channel X (with a carrier) and Y (no carrier), and have a specific channel date set for X.

Currently if we do not define a carrier on the SO, the computed release channel wil be taken from the configured specific channel (as it's the only one that exists). So the SO will show the release channel X.

Then we validate the SO, and we trigger the channel assignment on the delivery: it is now assigned to the release channel Y (of course, there was no carrier defined, I guess it fallbacks on Y because of that).

IMO this is not what the user will expect.

How should we handle SO without carrier (same question for SO without warehouse) regarding release channels?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think you should not include False for warehouse/carrier in _get_release_channel_partner_date_domain. The warehouse should be set by a default value. For the carrier, it is set by delivery_auto_refresh. So if one of them is empty, then we should not show a channel that has a restriction that is not respected

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree (I don't remember why we searched on carrier_id=False in the initial implementation, we did that together and on purpose 🤔 ).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the warehouse, while we could state it is always set on the SO (I could not find some without WH in customers DBs), it is however not mandatory on the release channel. We should consider these release channels as elligible, and thus keep our check on warehouse_id=False in the domain.

rec.release_channel_id = channel_date.release_channel_id

@api.depends(
"state",
Expand Down
24 changes: 18 additions & 6 deletions sale_stock_release_channel_partner_by_date_delivery/models/sale_order.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,23 @@ def _get_release_channel_id_depends(self):

def _get_release_channel_partner_date_domain(self):
domain = super()._get_release_channel_partner_date_domain()
if domain and self.carrier_id:
carrier_domain = [
"|",
("release_channel_id.carrier_ids", "in", self.carrier_id.ids),
("release_channel_id.carrier_ids", "=", False),
]
if domain:
if self.carrier_id:
carrier_domain = [
("release_channel_id.carrier_ids", "in", self.carrier_id.ids),
]
else:
carrier_domain = [
("release_channel_id.carrier_ids", "=", False),
]
domain = expression.AND([domain, carrier_domain])
return domain

def _compute_release_channel_id(self):
# pylint: disable=missing-return
super()._compute_release_channel_id()
for rec in self:
# Selected release channel and carrier have to be compatible
if rec.release_channel_id.carrier_ids and rec.carrier_id:
if rec.carrier_id not in rec.release_channel_id.carrier_ids:
rec.release_channel_id = False
107 changes: 103 additions & 4 deletions sale_stock_release_channel_partner_by_date_delivery/tests/test_sale_release_channel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl)

from odoo import fields
from odoo.tests.common import Form

from odoo.addons.sale_stock_release_channel_partner_by_date.tests.common import (
SaleReleaseChannelCase,
Expand All @@ -22,9 +23,18 @@ def setUpClass(cls):
}
)
cls.carrier_channel.action_wake_up()
cls.carrier2_channel = cls.default_channel.copy(
{
"name": "Test with carrier2",
"sequence": 10,
"carrier_ids": [(6, 0, cls.carrier2.ids)],
}
)
cls.carrier2_channel.action_wake_up()

def test_sale_order_with_wrong_carrier(self):
def test_sale_order_without_carrier_with_channel_date(self):
delivery_date = fields.Datetime.now()
# Create one specific channel not matching the SO regarding the carrier
channel_date_model = self.env["stock.release.channel.partner.date"]
channel_date_model.create(
{
Expand All @@ -33,9 +43,8 @@ def test_sale_order_with_wrong_carrier(self):
"date": delivery_date.date(),
}
)
# With wrong carrier set: order doesn't detect the specific channel for carrier
order = self._create_sale_order(date=delivery_date)
order.carrier_id = self.carrier2
self.assertFalse(order.carrier_id)
self.assertFalse(order.release_channel_id)
order.action_confirm()
self.assertFalse(order.release_channel_id)
Expand All @@ -46,7 +55,7 @@ def test_sale_order_with_wrong_carrier(self):
self.env["stock.release.channel"].assign_release_channel(picking_out)
self.assertEqual(picking_out.release_channel_id, self.default_channel)

def test_sale_order_with_carrier(self):
def test_sale_order_with_carrier_with_channel_date(self):
delivery_date = fields.Datetime.now()
channel_date_model = self.env["stock.release.channel.partner.date"]
channel_date = channel_date_model.create(
Expand All @@ -69,3 +78,93 @@ def test_sale_order_with_carrier(self):
# Then delivery gets the default channel
self.env["stock.release.channel"].assign_release_channel(picking_out)
self.assertEqual(picking_out.release_channel_id, self.carrier_channel)

def test_sale_order_with_carrier_without_channel_date(self):
delivery_date = fields.Datetime.now()
# With carrier set: order doesn't detect any specific channel
order = self._create_sale_order(date=delivery_date)
order.carrier_id = self.carrier
self.assertFalse(order.release_channel_id)
self.assertFalse(order._get_release_channel_partner_date())
# The user is able to set a release channel on the form
with Form(order) as form:
form.release_channel_id = self.carrier_channel
order.action_confirm()
self.assertEqual(order.release_channel_id, self.carrier_channel)
# A specific channel has been created at order confirmation
self.assertTrue(order._get_release_channel_partner_date())
picking_out = order.picking_ids
self.assertFalse(picking_out.release_channel_id)
# Then delivery then gets the channel defined on the SO
self.env["stock.release.channel"].assign_release_channel(picking_out)
self.assertEqual(picking_out.release_channel_id, self.carrier_channel)

def test_sale_order_with_incompatible_channel_and_carrier_1(self):
# Case 1: a release channel having a specific channel configured
# is detected, but incompatible with the selected carrier afterwards.
# => unset the detected release channel
delivery_date = fields.Datetime.now()
channel_date_model = self.env["stock.release.channel.partner.date"]
channel_date_model.create(
{
"partner_id": self.customer.id,
"release_channel_id": self.carrier_channel.id,
"date": delivery_date.date(),
}
)
order = self._create_sale_order(
date=delivery_date, channel=self.carrier_channel
)
self.assertEqual(order.release_channel_id, self.carrier_channel)
# => select a carrier
order.carrier_id = self.carrier2
self.assertFalse(order.release_channel_id)
# Confirm the order and check release channel on delivery
order.action_confirm()
self.assertFalse(order.release_channel_id)
self.assertFalse(order._get_release_channel_partner_date())
picking_out = order.picking_ids
self.assertFalse(picking_out.release_channel_id)
# Then delivery gets the default channel
self.env["stock.release.channel"].assign_release_channel(picking_out)
self.assertEqual(picking_out.release_channel_id, self.default_channel)

def test_sale_order_with_incompatible_channel_and_carrier_2(self):
# Case 2: a release channel having a specific channel configured
# is detected, but incompatible with the selected carrier afterwards.
# However another specific channel is compatible with the selected carrier.
# => update the selected release channel to be compatible with the
# selected carrier
delivery_date = fields.Datetime.now()
channel_date_model = self.env["stock.release.channel.partner.date"]
channel_date_model.create(
{
"partner_id": self.customer.id,
"release_channel_id": self.carrier_channel.id,
"date": delivery_date.date(),
}
)
order = self._create_sale_order(
date=delivery_date, channel=self.carrier_channel
)
self.assertEqual(order.release_channel_id, self.carrier_channel)
channel_date_model = self.env["stock.release.channel.partner.date"]
channel_date = channel_date_model.create(
{
"partner_id": self.customer.id,
"release_channel_id": self.carrier2_channel.id,
"date": delivery_date.date(),
}
)
# => select a carrier
order.carrier_id = self.carrier2
self.assertEqual(order.release_channel_id, self.carrier2_channel)
# Confirm the order and check release channel on delivery
order.action_confirm()
self.assertEqual(order.release_channel_id, self.carrier2_channel)
self.assertEqual(order._get_release_channel_partner_date(), channel_date)
picking_out = order.picking_ids
self.assertFalse(picking_out.release_channel_id)
# Then delivery gets the selected channel
self.env["stock.release.channel"].assign_release_channel(picking_out)
self.assertEqual(picking_out.release_channel_id, self.carrier2_channel)
Loading