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

[16.0][IMP] stock_picking_group_by_max_weight: Split if max weight exceed #1451

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

lmignon
Copy link
Contributor

@lmignon lmignon commented Dec 12, 2023

No description provided.

Split moves into different pickings at assignment time if the total weight
of the picking is greater than the maximum weight allowed on the picking type.

Prior to this commit only new moves created after the creation of an open picking
were put into a new picking upon assignment if the total weight of the picking
would exceed the maximum weight allowed on the picking type. This commit improves
this behavior by also splitting existing moves into different pickings after the
moves have been assigned to a picking. IOW, in a post-assignment step, the system
will split picking lines into different pickings when the total weight is exceeded.
@lmignon lmignon force-pushed the 16.0-split-max-weight branch from 43dfd0c to 5a13075 Compare December 12, 2023 14:09
@lmignon lmignon marked this pull request as ready for review December 12, 2023 14:09
Copy link
Contributor

@jbaudoux jbaudoux left a comment

Choose a reason for hiding this comment

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

I don't like the approach. It doesn't look to work properly when moves are added. Post processs is called before action_confirm, so you break the moves merging mechanism of odoo.

You better not split the moves in multiple pickings at this stage.

I would have implemented this at the batch picking creation based on the picking device limit. If the picking weight exceeds the picking device max weight, create a split order (you have the method in shopfloor) containing a subset of the moves within the weight limit.
In your PR here you already have defined the concept of max weight: https://github.com/OCA/wms/pull/534/files#diff-b737ddc38c458efb5b613c33ee65b4cefb4a0fc5b0dc46e93b7712ec34cda478R22
Add the split picking creation as mentioned here : https://github.com/OCA/wms/pull/534/files#r1428768425

Copy link
Contributor

@jbaudoux jbaudoux left a comment

Choose a reason for hiding this comment

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

As the module already exists and this improves it, let's approve this PR.

I still think this should be approached differently...

Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label May 19, 2024
@rousseldenis rousseldenis added this to the 16.0 milestone May 21, 2024
@rousseldenis
Copy link
Contributor

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 16.0-ocabot-merge-pr-1451-by-rousseldenis-bump-minor, awaiting test results.

OCA-git-bot added a commit that referenced this pull request May 21, 2024
Signed-off-by rousseldenis
@OCA-git-bot
Copy link
Contributor

@rousseldenis your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-1451-by-rousseldenis-bump-minor.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@github-actions github-actions bot removed the stale PR/Issue without recent activity, it'll be soon closed automatically. label May 26, 2024
Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Sep 29, 2024
@rousseldenis
Copy link
Contributor

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 16.0-ocabot-merge-pr-1451-by-rousseldenis-bump-minor, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 1cf57d7 into OCA:16.0 Oct 15, 2024
4 of 5 checks passed
@OCA-git-bot
Copy link
Contributor

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

@lmignon lmignon deleted the 16.0-split-max-weight branch October 15, 2024 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved merged 🎉 stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants