From 1212f36233c23f81f089162fd391bc053fe6f020 Mon Sep 17 00:00:00 2001
From: Carmen Bianca BAKKER <carmen@coopiteasy.be>
Date: Mon, 17 Jun 2024 17:21:49 +0200
Subject: [PATCH] [REF] hr_timesheet_begin_end: Refactor unit_amount to be a
 compute field

Instead of using onchange, which is less convenient in Odoo 16.

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
---
 .../models/account_analytic_line.py           | 42 +++++++++++++++++--
 .../tests/test_timesheet_begin_end.py         | 37 ++++++++++++----
 2 files changed, 68 insertions(+), 11 deletions(-)

diff --git a/hr_timesheet_begin_end/models/account_analytic_line.py b/hr_timesheet_begin_end/models/account_analytic_line.py
index 543b79a822..6187cbed0d 100644
--- a/hr_timesheet_begin_end/models/account_analytic_line.py
+++ b/hr_timesheet_begin_end/models/account_analytic_line.py
@@ -16,9 +16,45 @@ class AccountAnalyticLine(models.Model):
     time_start = fields.Float(string="Begin Hour")
     time_stop = fields.Float(string="End Hour")
 
-    @api.onchange("time_start", "time_stop")
-    def onchange_hours_start_stop(self):
-        self.unit_amount = self.unit_amount_from_start_stop()
+    # Override to be a computed field.
+    unit_amount = fields.Float(
+        compute="_compute_unit_amount",
+        store=True,
+        readonly=False,
+    )
+
+    @api.model
+    def _add_unit_amount_to_vals_list(self, vals_list):
+        for vals in vals_list:
+            if not vals.get("unit_amount") and (time_start := vals.get("time_start")):
+                # This doesn't use unit_amount_from_start_stop, because that
+                # requires a record to already exist. Modules which modify the
+                # behaviour of unit_amount_from_start_stop should also modify
+                # this method.
+                vals["unit_amount"] = self._hours_from_start_stop(
+                    time_start, vals.get("time_stop", 0)
+                )
+
+    @api.model_create_multi
+    def create(self, vals_list):
+        # This is a workaround for a bizarre situation: if a line is created
+        # with a time range but WITHOUT defining unit_amount, then you would
+        # expect unit_amount to be computed from the range. But for some reason,
+        # this never happens, and it is instead set to 0. Subsequently the
+        # constraint _validate_unit_amount_equal_to_time_diff kicks in and
+        # raises an exception.
+        #
+        # As a workaround, we add unit_amount with the correct value if it isn't
+        # in the vals_list. A better workaround might be to magically detect
+        # that unit_amount is not yet correctly computed in the constraint, and
+        # to skip the constraint if so.
+        self._add_unit_amount_to_vals_list(vals_list)
+        return super().create(vals_list)
+
+    @api.depends("time_start", "time_stop")
+    def _compute_unit_amount(self):
+        for line in self:
+            line.unit_amount = line.unit_amount_from_start_stop()
 
     def _validate_start_before_stop(self):
         value_to_html = self.env["ir.qweb.field.float_time"].value_to_html
diff --git a/hr_timesheet_begin_end/tests/test_timesheet_begin_end.py b/hr_timesheet_begin_end/tests/test_timesheet_begin_end.py
index a70b9b73b5..b5ffd56dba 100644
--- a/hr_timesheet_begin_end/tests/test_timesheet_begin_end.py
+++ b/hr_timesheet_begin_end/tests/test_timesheet_begin_end.py
@@ -22,18 +22,39 @@ def setUp(self):
             "amount": -60.0,
         }
 
-    def test_onchange(self):
-        line = self.timesheet_line_model.new(
-            {"name": "test", "time_start": 10.0, "time_stop": 12.0}
-        )
-        line.onchange_hours_start_stop()
-        self.assertEqual(line.unit_amount, 2)
+    def test_compute_unit_amount(self):
+        line = self.base_line.copy()
+        del line["unit_amount"]
+        line_record = self.timesheet_line_model.create(line)
+        self.assertEqual(line_record.unit_amount, 2)
+        line_record.time_stop = 14.0
+        self.assertEqual(line_record.unit_amount, 4)
+
+    def test_compute_unit_amount_no_compute_if_no_times(self):
+        line = self.base_line.copy()
+        del line["time_start"]
+        del line["time_stop"]
+        line_record = self.timesheet_line_model.create(line)
+        self.assertEqual(line_record.unit_amount, 2.0)
+        line_record.unit_amount = 3.0
+        self.assertEqual(line_record.unit_amount, 3.0)
+
+    def test_compute_unit_amount_to_zero(self):
+        line = self.base_line.copy()
+        del line["unit_amount"]
+        line_record = self.timesheet_line_model.create(line)
+        self.assertEqual(line_record.unit_amount, 2)
+        line_record.write({"time_start": 0, "time_stop": 0})
+        self.assertEqual(line_record.unit_amount, 0)
 
-    def test_onchange_no_update(self):
+    def test_compute_unit_amount_to_zero_no_record(self):
+        # Cannot create/save this model because it breaks a constraint, so using
+        # .new().
         line = self.timesheet_line_model.new(
             {"name": "test", "time_start": 13.0, "time_stop": 12.0}
         )
-        line.onchange_hours_start_stop()
+        self.assertEqual(line.unit_amount, 0)
+        line.time_stop = 10.0
         self.assertEqual(line.unit_amount, 0)
 
     def test_check_begin_before_end(self):