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

[FIX] mrp, sale_mrp: _run_pull creates many pickings #162947

Conversation

pno-odoo
Copy link
Contributor

The steps to reproduce:

  • Go to a warehouse. Under the technical information tab, change the field sam_loc_id from "WH/Post-Production" to "WH/Stock".
  • Create a sales order with 2 different products and confirm it.
  • 2 pickings will be created with different group_id MO/XXXX instead of a single picking.

After this commit, we only check the sam_loc_id if we are in 3 steps manufacturing.

OPW-3871886


I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@robodoo
Copy link
Contributor

robodoo commented Apr 23, 2024

@C3POdoo C3POdoo requested a review from a team April 23, 2024 08:26
@C3POdoo C3POdoo added the OE the report is linked to a support ticket (opw-...) label Apr 23, 2024
Copy link
Contributor

@clesgow clesgow left a comment

Choose a reason for hiding this comment

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

Thanks for your fix, can you add a test as well? 🙂

Comment on lines 94 to 99
if rule.picking_type_id == warehouse_id.sam_type_id or (
warehouse_id.manufacture_steps == 'pbm_sam'
and warehouse_id.sam_loc_id
and warehouse_id.sam_loc_id.parent_path in rule.location_src_id.parent_path
):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if rule.picking_type_id == warehouse_id.sam_type_id or (
warehouse_id.manufacture_steps == 'pbm_sam'
and warehouse_id.sam_loc_id
and warehouse_id.sam_loc_id.parent_path in rule.location_src_id.parent_path
):
if warehouse_id.manufacture_steps != 'pbm_sam':
continue
if rule.picking_type_id == warehouse_id.sam_type_id or (warehouse_id.sam_loc_id and warehouse_id.sam_loc_id.parent_path in rule.location_src_id.parent_path):

Since the whole point of this override is to handle the pbm_sam case, I don't think we should do any of the following if the option is disabled.

@pno-odoo pno-odoo force-pushed the 15.0-mrp-run_pull_generate_multiple_pickings-pno branch from 1e1da7b to 2bcfc08 Compare April 25, 2024 13:55
@pno-odoo pno-odoo changed the title [FIX] mrp: _run_pull creates many pickings [FIX] mrp, sale_mrp: _run_pull creates many pickings Apr 26, 2024
@clesgow
Copy link
Contributor

clesgow commented Apr 26, 2024

Have you tried running your test locally? And doesn't it looks a bit... out of place? 👀

Even with the required fixes on it, the test passes even without your fix as well.

@pno-odoo pno-odoo force-pushed the 15.0-mrp-run_pull_generate_multiple_pickings-pno branch from 2bcfc08 to 4589c8a Compare April 26, 2024 11:06
@pno-odoo
Copy link
Contributor Author

Oops, my bad. It should be alright now.
Thank you!

@clesgow
Copy link
Contributor

clesgow commented Apr 26, 2024

I think you missed that part:

Even with the required fixes on it, the test passes even without your fix as well.

@pno-odoo
Copy link
Contributor Author

I think you missed that part:

Even with the required fixes on it, the test passes even without your fix as well.

I double-checked and without the fix, the test fails

image

@clesgow
Copy link
Contributor

clesgow commented Apr 26, 2024

Turns out I was dumb. 🥲
Apologies for the confusion.

@pno-odoo pno-odoo force-pushed the 15.0-mrp-run_pull_generate_multiple_pickings-pno branch from 4589c8a to e8882b3 Compare April 26, 2024 14:10
Comment on lines 94 to 96
if warehouse_id.manufacture_steps != 'pbm_sam':
continue
Copy link
Contributor

Choose a reason for hiding this comment

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

After some more thinking and discussion, this is sadly not enough. While this effectively solves your case, it still leaves other doors open if a rule unrelated to manufacture ends up having a location child of sam_loc_id.

I'd propose something along those lines instead:

Suggested change
if warehouse_id.manufacture_steps != 'pbm_sam':
continue
manu_rule = rule.route_id.rule_ids.filtered(lambda r: r.action == 'manufacture' and r.warehouse_id == warehouse_id)
if warehouse_id.manufacture_steps != 'pbm_sam' or not manu_rule:
continue

Basically move the manu_rule search above (and remove the useless code that checked if it was found or not), to avoid triggering this code if we're not in a manufacture context at all. Hopefully this will be enough.

What do you think?

The steps to reproduce:
- Go to a warehouse. Under the technical information tab, change the field sam_loc_id from "WH/Post-Production" to "WH/Stock".
- Create a sales order with 2 different products and confirm it.
- 2 pickings will be created with different group_id MO/XXXX instead of a single picking.

After this commit we only check the sam_loc_id if we are in 3 steps manufacturing.

OPW-3871886
@pno-odoo pno-odoo force-pushed the 15.0-mrp-run_pull_generate_multiple_pickings-pno branch from e8882b3 to 21ee104 Compare April 29, 2024 08:33
@clesgow
Copy link
Contributor

clesgow commented Apr 30, 2024

Thank you!
@robodoo r+

robodoo pushed a commit that referenced this pull request Apr 30, 2024
The steps to reproduce:
- Go to a warehouse. Under the technical information tab, change the field sam_loc_id from "WH/Post-Production" to "WH/Stock".
- Create a sales order with 2 different products and confirm it.
- 2 pickings will be created with different group_id MO/XXXX instead of a single picking.

After this commit we only check the sam_loc_id if we are in 3 steps manufacturing.

OPW-3871886

closes #162947

Signed-off-by: Quentin Wolfs (quwo) <quwo@odoo.com>
@robodoo robodoo closed this Apr 30, 2024
willylohws pushed a commit to willylohws/odoo that referenced this pull request May 1, 2024
The steps to reproduce:
- Go to a warehouse. Under the technical information tab, change the field sam_loc_id from "WH/Post-Production" to "WH/Stock".
- Create a sales order with 2 different products and confirm it.
- 2 pickings will be created with different group_id MO/XXXX instead of a single picking.

After this commit we only check the sam_loc_id if we are in 3 steps manufacturing.

OPW-3871886

closes odoo#162947

Signed-off-by: Quentin Wolfs (quwo) <quwo@odoo.com>
@fw-bot
Copy link
Contributor

fw-bot commented May 4, 2024

@pno-odoo @clesgow this pull request has forward-port PRs awaiting action (not merged or closed):

1 similar comment
@fw-bot
Copy link
Contributor

fw-bot commented May 5, 2024

@pno-odoo @clesgow this pull request has forward-port PRs awaiting action (not merged or closed):

@fw-bot
Copy link
Contributor

fw-bot commented May 6, 2024

@pno-odoo @clesgow this pull request has forward-port PRs awaiting action (not merged or closed):

@fw-bot
Copy link
Contributor

fw-bot commented May 7, 2024

@pno-odoo @clesgow this pull request has forward-port PRs awaiting action (not merged or closed):

@fw-bot fw-bot deleted the 15.0-mrp-run_pull_generate_multiple_pickings-pno branch May 14, 2024 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OE the report is linked to a support ticket (opw-...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants