From f052624c97b7d80b983b4afc4136a2557dc2a5c2 Mon Sep 17 00:00:00 2001 From: "Tiffany Chang (tic)" Date: Thu, 2 Dec 2021 13:59:01 +0000 Subject: [PATCH] [FIX] mrp: correctly calculate _set_duration in WO There were some discrepencies when adding to/removing from WO Real Duration: - When adding time that is brings the total Duration over the expected duration, the time should be split between the "productive" and "performance" (i.e. reduced speed) times. This is already happening when using start/pause buttons, so now it is consistent. - Use 'id' instead of "Start Date" to determine which time_ids to reduce/remove since adding a large enough "real duration" value compared to previous time_id(s) could result in an earlier "Start Date" for a newer time_id (i.e. results in which one is adjusted/ deleted = confusing). Using "End Date" was considered, but same non-sequential issue can occur during time split from point above. - Correctly determine if the "productivity" should be 'productive' or not (i.e. 'performance') based on total WO duration (not duration of newly added time_id) Refinement of Part 4 of Task: 2667151 closes odoo/odoo#80319 Signed-off-by: Arnold Moyaux --- addons/mrp/models/mrp_workorder.py | 29 +++++++++++++++--------- addons/mrp/tests/test_order.py | 36 ++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 10 deletions(-) diff --git a/addons/mrp/models/mrp_workorder.py b/addons/mrp/models/mrp_workorder.py index dc4b506429c..149279ec24a 100644 --- a/addons/mrp/models/mrp_workorder.py +++ b/addons/mrp/models/mrp_workorder.py @@ -312,23 +312,32 @@ class MrpWorkorder(models.Model): return minutes * 60 + seconds for order in self: - old_order_duation = sum(order.time_ids.mapped('duration')) + old_order_duration = sum(order.time_ids.mapped('duration')) new_order_duration = order.duration - if new_order_duration == old_order_duation: + if new_order_duration == old_order_duration: continue - delta_duration = new_order_duration - old_order_duation + delta_duration = new_order_duration - old_order_duration if delta_duration > 0: - date_start = datetime.now() - timedelta(seconds=_float_duration_to_second(delta_duration)) - self.env['mrp.workcenter.productivity'].create( - order._prepare_timeline_vals(delta_duration, date_start, datetime.now()) - ) + enddate = datetime.now() + date_start = enddate - timedelta(seconds=_float_duration_to_second(delta_duration)) + if order.duration_expected >= new_order_duration or old_order_duration >= order.duration_expected: + # either only productive or only performance (i.e. reduced speed) time respectively + self.env['mrp.workcenter.productivity'].create( + order._prepare_timeline_vals(new_order_duration, date_start, enddate) + ) + else: + # split between productive and performance (i.e. reduced speed) times + maxdate = fields.Datetime.from_string(enddate) - relativedelta(minutes=new_order_duration - order.duration_expected) + self.env['mrp.workcenter.productivity'].create([ + order._prepare_timeline_vals(order.duration_expected, date_start, maxdate), + order._prepare_timeline_vals(new_order_duration, maxdate, enddate) + ]) else: duration_to_remove = abs(delta_duration) - timelines = order.time_ids.sorted(lambda t: t.date_start) timelines_to_unlink = self.env['mrp.workcenter.productivity'] - for timeline in timelines: + for timeline in order.time_ids.sorted(): if duration_to_remove <= 0.0: break if timeline.duration <= duration_to_remove: @@ -758,7 +767,7 @@ class MrpWorkorder(models.Model): def _prepare_timeline_vals(self, duration, date_start, date_end=False): # Need a loss in case of the real time exceeding the expected - if not self.duration_expected or duration < self.duration_expected: + if not self.duration_expected or duration <= self.duration_expected: loss_id = self.env['mrp.workcenter.productivity.loss'].search([('loss_type', '=', 'productive')], limit=1) if not len(loss_id): raise UserError(_("You need to define at least one productivity loss in the category 'Productivity'. Create one from the Manufacturing app, menu: Configuration / Productivity Losses.")) diff --git a/addons/mrp/tests/test_order.py b/addons/mrp/tests/test_order.py index ff6f90d38b0..48d51a81c27 100644 --- a/addons/mrp/tests/test_order.py +++ b/addons/mrp/tests/test_order.py @@ -2368,3 +2368,39 @@ class TestMrpOrder(TestMrpCommon): production_form.product_qty = 5 production = production_form.save() self.assertEqual(production.workorder_ids[0].duration_expected, 30.0, "Produce 5 with capacity 2, expected is 10mn for each run -> 30mn") + + def test_workorder_set_duration(self): + """ Test that manually setting duration correctly creates/updates time_ids """ + mo = Form(self.env['mrp.production']) + mo.bom_id = self.bom_4 + mo = mo.save() + mo.action_confirm() + + workorder = mo.workorder_ids[0] + expected_duration = workorder.duration_expected + real_duration_under_expected = expected_duration / 2 # first value + real_duration_increased_above_expected = 2 * expected_duration # to create next 2 values + real_duration_decreased = expected_duration * .75 # reduced amount that overlaps the last 2 values + + # update real duration to amount under expected duration + workorder.duration = real_duration_under_expected + self.assertEqual(len(workorder.time_ids), 1, "A time tracking value should have been created") + self.assertEqual(workorder.time_ids[0].loss_type, 'productive', "Total duration < Expected duration => should be productive time") + self.assertEqual(workorder.time_ids[0].duration, real_duration_under_expected, "Incorrect duration for time tracking value") + + # update real duration to amount above expected duration + workorder.duration = real_duration_increased_above_expected + self.assertEqual(len(workorder.time_ids), 3, "Two more time tracking values should have been created") + self.assertEqual(workorder.time_ids[1].loss_type, 'productive', "Duration amount added under the expected duration should be productive time") + self.assertEqual(workorder.time_ids[2].loss_type, 'performance', "Duration amount added above expected duration should be performance (i.e. reduced) time") + self.assertEqual(workorder.time_ids[1].duration, expected_duration - real_duration_under_expected, "Added (productive) time should be expected duration - already existing duration") + self.assertEqual(workorder.time_ids[2].duration, real_duration_increased_above_expected - expected_duration, "Added (reduced) time should be total duration - expected duration") + + # update real duration to amount below expected duration + workorder.duration = real_duration_decreased + # reducing time reverses the time_id order... so we reverse the check + self.assertEqual(len(workorder.time_ids), 2, "One time tracking values should have been deleted") + self.assertEqual(workorder.time_ids[1].loss_type, 'productive', "Original time tracking should be unchanged") + self.assertEqual(workorder.time_ids[1].duration, real_duration_under_expected, "Original time tracking should be unchanged") + self.assertEqual(workorder.time_ids[0].loss_type, 'productive', "Remaining time tracking should be productive") + self.assertEqual(workorder.time_ids[0].duration, real_duration_decreased - real_duration_under_expected, "Time tracking duration should have been reduced to reflect new shorter duration")