Skip to content

[16.0][FIX] stock_release_channel_partner_by_date: properly detect the exception #999

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

Conversation

jbaudoux
Copy link
Contributor

@jbaudoux jbaudoux commented Mar 27, 2025

Fixing compatibility with:

cc @sebalix @santostelmo

@OCA-git-bot
Copy link
Contributor

Hi @mt-software-de, @sebalix,
some modules you are maintaining are being modified, check this out!

@jbaudoux jbaudoux force-pushed the 16.0-imp-stock_release_channel_partner_by_date-enforce_specific_channel_partner_date branch 2 times, most recently from c6be5de to 69488de Compare March 27, 2025 17:53
@jbaudoux jbaudoux added work in progress no stale Use this label to prevent the automated stale action from closing this PR/Issue. labels Mar 31, 2025
Copy link
Contributor

@sebalix sebalix left a comment

Choose a reason for hiding this comment

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

LG overall but still one question

@@ -111,16 +111,22 @@ def _get_release_channel_possible_candidate_domain_channel(self):
return [
("is_manual_assignment", "=", False),
("state", "!=", "asleep"),
"|",
("picking_type_ids", "=", False),
("picking_type_ids", "in", self.picking_type_id.ids),
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it expected to use picking data in _get_release_channel_possible_candidate_domain_channel domain hook? I thought we designed it for basic criteria selection of the channel (like the ones already defined) without relying on current picking. For that we have _get_release_channel_possible_candidate_domain_picking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need a domain to find out that the exception is valid for that warehouse & carrier but not picking type as it could be recomputed by the warehouse flow module.
I'll add docstring to those methods.

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 added the docstrings

jbaudoux and others added 4 commits May 14, 2025 14:09
When searching a channel for a picking, if the channel has a warehouse
then it must match otherwise the picking type must match
Ensure warehouse is set on test channels
Fix compatilibity with stock_release_channel_partner_by_date.
Domain must apply on channel so that we don't restrict partner specific
channels.
@jbaudoux jbaudoux force-pushed the 16.0-imp-stock_release_channel_partner_by_date-enforce_specific_channel_partner_date branch from 67d53ea to 7551f74 Compare May 14, 2025 12:12
@jbaudoux jbaudoux force-pushed the 16.0-imp-stock_release_channel_partner_by_date-enforce_specific_channel_partner_date branch from 323f018 to 4cbd205 Compare May 14, 2025 15:22
@sebalix sebalix added this to the 16.0 milestone May 14, 2025
@jbaudoux
Copy link
Contributor Author

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-999-by-jbaudoux-bump-patch, awaiting test results.

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@OCA-git-bot OCA-git-bot merged commit c34e19f into OCA:16.0 May 14, 2025
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 7c59d37. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved merged 🎉 no stale Use this label to prevent the automated stale action from closing this PR/Issue. ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants