[16.0][FIX] sale_stock_release_channel_partner_by_date*: fix computation of release channel #955
Conversation
…of release channel
sebalix
changed the title
| @@ -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: | |||
There was a problem hiding this comment.
Maybe you can check here that the warehouse is compatible with the currently selected channel? Like you did for the carrier.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
I agree (I don't remember why we searched on carrier_id=False in the initial implementation, we did that together and on purpose 🤔 ).
There was a problem hiding this comment.
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.
| order.carrier_id = self.carrier2 | ||
| self.assertFalse(order.release_channel_id) | ||
| self.assertFalse(order.carrier_id) | ||
| self.assertEqual(order.release_channel_id, self.carrier_channel) |
There was a problem hiding this comment.
This looks wrong to me. If the channel has a carrier restriction and the order has no carrier, you cannot end-up with the channel auto assigned to the SO
…tation of release channel
Aims to supersede #954