-
Notifications
You must be signed in to change notification settings - Fork 26k
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
base: 18.0
Are you sure you want to change the base?
Conversation
2e30584
to
84c540b
Compare
There was a problem hiding this 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. 🙂
addons/mrp/models/mrp_production.py
Outdated
'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) |
There was a problem hiding this comment.
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.
addons/mrp/models/mrp_workorder.py
Outdated
@@ -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' |
There was a problem hiding this comment.
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
.
4df7292
to
ed7c87d
Compare
addons/mrp/tests/test_order.py
Outdated
(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}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(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. 🙂
addons/mrp/tests/test_order.py
Outdated
'type': 'normal', | ||
'bom_line_ids': [] | ||
}) | ||
self.bom_depends.operation_ids[1].blocked_by_operation_ids = [(4, self.bom_depends.operation_ids[0].id)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)] |
addons/mrp/tests/test_order.py
Outdated
self.product_depends = self.env['product.product'].create({ | ||
'name': 'depends', | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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? 🤔
addons/mrp/tests/test_order.py
Outdated
|
||
production_form = Form(production) | ||
with production_form.workorder_ids.edit(1) as wo: | ||
wo.duration = 15 # in 15 minutes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wo.duration = 15 # in 15 minutes | |
wo.duration = 15 |
I think that's obvious enough 😄
addons/mrp/tests/test_order.py
Outdated
'name': 'depends', | ||
}) | ||
|
||
self.bom_depends = self.env['mrp.bom'].create({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
ed7c87d
to
e039107
Compare
Steps to recreate the issue:
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
andmax
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