Content-Length: 382799 | pFad | https://github.com/odoo/odoo/pull/187017

66 [FIX] mrp: setting workorder duration traceback by Dotni · Pull Request #187017 · odoo/odoo · GitHub
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: setting workorder duration traceback #187017

Open
wants to merge 1 commit into
base: 18.0
Choose a base branch
from

Conversation

Dotni
Copy link
Contributor

@Dotni Dotni commented Nov 13, 2024

Steps to recreate the issue:

  • Create a MO with the SEC-ASSEM bill of materials and confirm it
  • Unblock the Drill 1 workcenter
  • Start working on the packing work order
  • Modify the duration on the long time assembly work order
  • Save

Current behavior before PR:
Traceback

Desired behavior after PR is merged:
No traceback

When a work order depends on a another workorder, setting the duration on the depending work order will trigger a call to _plan_workorder, but all workorders don't necessarily have a leave_id as their depending on a non-finished work order.
The min and max functions can't work with False values, triggering the traceback.

task-id: 4282910


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

@robodoo
Copy link
Contributor

robodoo commented Nov 13, 2024

Pull request status dashboard

@Dotni Dotni force-pushed the 18.0-mrp-duration_traceback-marm branch 2 times, most recently from 2e30584 to 84c540b Compare November 13, 2024 08:45
@C3POdoo C3POdoo added the RD research & development, internal work label Nov 13, 2024
@clesgow clesgow marked this pull request as ready for review December 20, 2024 14:06
@C3POdoo C3POdoo requested review from a team and clesgow and removed request for a team December 20, 2024 14:07
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.

A little test would be nice to avoid this issue in the future. 🙂

Comment on lines 1550 to 1551
'date_start': min(leave_id.date_from for leave_id in workorders.leave_id),
'date_finished': max(leave_id.date_to for leave_id in workorders.leave_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather have the filtered() above updated to exclude workorders without leave_id instead.
Otherwise, if you somehow end up with all workorders that have no leave_id, then you'll try to write False on date_start, which is a field.

@@ -344,8 +344,6 @@ def _float_duration_to_second(duration):
delta_duration = new_order_duration - old_order_duration

if delta_duration > 0:
if order.state not in ('progress', 'done'):
order.state = 'progress'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you remove that? Seems unrelated to the bug. 🤔
Also, since it will add a time sheet for the added time, there's little difference between doing that or starting then pausing a workorder. Yet in the first case the wo would still be ready and in the other case it would be progress.

@Dotni Dotni force-pushed the 18.0-mrp-duration_traceback-marm branch 2 times, most recently from 4df7292 to ed7c87d Compare January 10, 2025 10:52
Comment on lines 4940 to 4941
(0, 0, {'name': 'Cutting Machine', 'workcenter_id': self.workcenter_1.id, 'time_cycle': 12, 'sequence': 1}),
(0, 0, {'name': 'Weld Machine', 'workcenter_id': self.workcenter_1.id, 'time_cycle': 18, 'sequence': 2}),
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
(0, 0, {'name': 'Cutting Machine', 'workcenter_id': self.workcenter_1.id, 'time_cycle': 12, 'sequence': 1}),
(0, 0, {'name': 'Weld Machine', 'workcenter_id': self.workcenter_1.id, 'time_cycle': 18, 'sequence': 2}),
Command.create({'name': 'Cutting Machine', 'workcenter_id': self.workcenter_1.id, 'time_cycle': 12, 'sequence': 1}),
Command.create({'name': 'Weld Machine', 'workcenter_id': self.workcenter_1.id, 'time_cycle': 18, 'sequence': 2}),

Commands are a bit easier to read. 🙂

'type': 'normal',
'bom_line_ids': []
})
self.bom_depends.operation_ids[1].blocked_by_operation_ids = [(4, self.bom_depends.operation_ids[0].id)]
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
self.bom_depends.operation_ids[1].blocked_by_operation_ids = [(4, self.bom_depends.operation_ids[0].id)]
self.bom_depends.operation_ids[1].blocked_by_operation_ids = [Command.link(self.bom_depends.operation_ids[0].id)]

Comment on lines 4927 to 4929
self.product_depends = self.env['product.product'].create({
'name': 'depends',
})
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
self.product_depends = self.env['product.product'].create({
'name': 'depends',
})
self.product_depends = self.env['product.product'].create({
'name': 'depends',
})

That product is never used? 🤔


production_form = Form(production)
with production_form.workorder_ids.edit(1) as wo:
wo.duration = 15 # in 15 minutes
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
wo.duration = 15 # in 15 minutes
wo.duration = 15

I think that's obvious enough 😄

'name': 'depends',
})

self.bom_depends = self.env['mrp.bom'].create({
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
self.bom_depends = self.env['mrp.bom'].create({
bom_depends = self.env['mrp.bom'].create({

Even more, rather than re-creating self.bom_3, why don't you just use it directly, after adding the allow_operation_dependencies and operation link that you need?
That would shorten the test and avoid creating unnecessary records. 🙂

Steps to recreate the issue:

- Create a MO with the SEC-ASSEM bill of materials and confirm it
- Unblock the Drill 1 workcenter
- Start working on the packing work order
- Modify the duration on the long time assembly work order
- Save

Current behavior before PR:
Traceback

Desired behavior after PR is merged:
No traceback

When a work order depends on a another workorder, setting the duration on
the depending work order will trigger a call to `_plan_workorder`,
but all workorders don't necessarily have a leave_id as their depending
on a non-finished work order.
The `min` and `max` functions can't work with False values, triggering the traceback.

task-id: 4282910
@Dotni Dotni force-pushed the 18.0-mrp-duration_traceback-marm branch from ed7c87d to e039107 Compare January 16, 2025 07:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: https://github.com/odoo/odoo/pull/187017

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy