[16.0][REF] hr_timesheet_begin_end: More extensible, use compute instead of onchange

.copier-answers.yml

@@ -1,5 +1,5 @@
 # Do NOT update manually; changes here will be overwritten by Copier
-_commit: v1.21.1
+_commit: '1.23'
 _src_path: gh:oca/oca-addons-repo-template
 ci: GitHub
 convert_readme_fragments_to_markdown: false
@@ -17,6 +17,7 @@ org_name: Odoo Community Association (OCA)
 org_slug: OCA
 rebel_module_groups:
 - sale_timesheet_rounded
+- hr_timesheet_begin_end
 repo_description: 'TODO: add repo description.'
 repo_name: timesheet
 repo_slug: timesheet

.github/workflows/test.yml

@@ -37,18 +37,25 @@ jobs:
         include:
           - container: ghcr.io/oca/oca-ci/py3.10-odoo16.0:latest
             include: "sale_timesheet_rounded"
-            makepot: "true"
             name: test with Odoo
           - container: ghcr.io/oca/oca-ci/py3.10-ocb16.0:latest
             include: "sale_timesheet_rounded"
             name: test with OCB
+            makepot: "true"
           - container: ghcr.io/oca/oca-ci/py3.10-odoo16.0:latest
-            exclude: "sale_timesheet_rounded"
+            include: "hr_timesheet_begin_end"
+            name: test with Odoo
+          - container: ghcr.io/oca/oca-ci/py3.10-ocb16.0:latest
+            include: "hr_timesheet_begin_end"
+            name: test with OCB
             makepot: "true"
+          - container: ghcr.io/oca/oca-ci/py3.10-odoo16.0:latest
+            exclude: "sale_timesheet_rounded,hr_timesheet_begin_end"
             name: test with Odoo
           - container: ghcr.io/oca/oca-ci/py3.10-ocb16.0:latest
-            exclude: "sale_timesheet_rounded"
+            exclude: "sale_timesheet_rounded,hr_timesheet_begin_end"
             name: test with OCB
+            makepot: "true"
     services:
       postgres:
         image: postgres:12.0
@@ -79,9 +86,5 @@ jobs:
         with:
           token: ${{ secrets.CODECOV_TOKEN }}
       - name: Update .pot files
-        run:
-          oca_export_and_push_pot https://x-access-token:${{ secrets.GIT_PUSH_TOKEN
-          }}@github.com/${{ github.repository }}
-        if:
-          ${{ matrix.makepot == 'true' && github.event_name == 'push' &&
-          github.repository_owner == 'OCA' }}
+        run: oca_export_and_push_pot https://x-access-token:${{ secrets.GIT_PUSH_TOKEN }}@github.com/${{ github.repository }}
+        if: ${{ matrix.makepot == 'true' && github.event_name == 'push' && github.repository_owner == 'OCA' }}

.gitignore

@@ -25,6 +25,15 @@ var/
 *.egg
 *.eggs
 
+# Debian packages
+*.deb
+
+# Redhat packages
+*.rpm
+
+# MacOS packages
+*.dmg
+
 # Installer logs
 pip-log.txt
 pip-delete-this-directory.txt

hr_timesheet_begin_end/models/account_analytic_line.py

@@ -1,5 +1,6 @@
 # Copyright 2015 Camptocamp SA - Guewen Baconnier
 # Copyright 2017 Tecnativa, S.L. - Luis M. Ontalba
+# Copyright 2024 Coop IT Easy SC - Carmen Bianca BAKKER
 # License AGPL-3 - See http://www.gnu.org/licenses/agpl-3.0.html
 
 from datetime import timedelta
@@ -15,16 +16,34 @@
     time_start = fields.Float(string="Begin Hour")
     time_stop = fields.Float(string="End Hour")
 
-    @api.constrains("time_start", "time_stop", "unit_amount")
-    def _check_time_start_stop(self):
-        for line in self:
-            value_to_html = self.env["ir.qweb.field.float_time"].value_to_html
-            start = timedelta(hours=line.time_start)
-            stop = timedelta(hours=line.time_stop)
-            if stop < start:
-                value_to_html(line.time_start, None)
-                value_to_html(line.time_stop, None)
+    # Override to be a computed field.
+    unit_amount = fields.Float(
+        compute="_compute_unit_amount",
+        store=True,
+        readonly=False,
+        # This default 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 this never
+        # happens, and it is instead set to default value 0. Subsequently the
+        # constraint _validate_unit_amount_equal_to_time_diff kicks in and
+        # raises an exception.
+        #
+        # By setting the default to None, the computation is correctly
+        # triggered. If nothing is computed, None falls back to 0.
+        default=None,
+    )
+
+    @api.depends("time_start", "time_stop", "project_id")
+    def _compute_unit_amount(self):
+        # Do not compute/adjust the unit_amount of non-timesheets.
+        lines = self.filtered(lambda line: line.project_id)
+        for line in lines:
+            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
+        for line in self:
+            if line.time_stop < line.time_start:
                 raise exceptions.ValidationError(
                     _(
                         "The beginning hour (%(html_start)s) must "
@@ -35,7 +54,11 @@
                         "html_stop": value_to_html(line.time_stop, None),
                     }
                 )
-            hours = (stop - start).seconds / 3600
+
+    def _validate_unit_amount_equal_to_time_diff(self):
+        value_to_html = self.env["ir.qweb.field.float_time"].value_to_html
+        for line in self:
+            hours = line.unit_amount_from_start_stop()
             rounding = self.env.ref("uom.product_uom_hour").rounding
             if hours and float_compare(
                 hours, line.unit_amount, precision_rounding=rounding
@@ -50,16 +73,21 @@
                         "html_hours": value_to_html(hours, None),
                     }
                 )
-            # check if lines overlap
-            others = self.search(
-                [
-                    ("id", "!=", line.id),
-                    ("employee_id", "=", line.employee_id.id),
-                    ("date", "=", line.date),
-                    ("time_start", "<", line.time_stop),
-                    ("time_stop", ">", line.time_start),
-                ]
-            )
+
+    def _overlap_domain(self):
+        self.ensure_one()
+        return [
+            ("id", "!=", self.id),
+            ("employee_id", "=", self.employee_id.id),
+            ("date", "=", self.date),
+            ("time_start", "<", self.time_stop),
+            ("time_stop", ">", self.time_start),
+        ]
+
+    def _validate_no_overlap(self):
+        value_to_html = self.env["ir.qweb.field.float_time"].value_to_html
+        for line in self:
+            others = self.search(line._overlap_domain())
             if others:
                 message = _("Lines can't overlap:\n")
                 message += "\n".join(
@@ -74,13 +102,28 @@
                 )
                 raise exceptions.ValidationError(message)
 
-    @api.onchange("time_start", "time_stop")
-    def onchange_hours_start_stop(self):
-        start = timedelta(hours=self.time_start)
-        stop = timedelta(hours=self.time_stop)
+    @api.constrains("time_start", "time_stop", "unit_amount")
+    def _check_time_start_stop(self):
+        lines = self.filtered(lambda line: line.project_id)
+        lines._validate_start_before_stop()
+        lines._validate_unit_amount_equal_to_time_diff()
+        lines._validate_no_overlap()
+
+    @api.model
+    def _hours_from_start_stop(self, time_start, time_stop):
+        start = timedelta(hours=time_start)
+        stop = timedelta(hours=time_stop)
         if stop < start:
-            return
-        self.unit_amount = (stop - start).seconds / 3600
+            # Invalid case, but return something sensible.
+            return 0
+        return (stop - start).seconds / 3600
+
+    def unit_amount_from_start_stop(self):
+        self.ensure_one()
+        # Don't handle non-timesheet lines.
+        if not self.project_id:
+            return 0
+        return self._hours_from_start_stop(self.time_start, self.time_stop)
 
     def merge_timesheets(self):  # pragma: no cover
         """This method is needed in case hr_timesheet_sheet is installed"""

hr_timesheet_begin_end/readme/newsfragments/692.bugfix.1.rst

@@ -0,0 +1 @@
+Fixed the test to use timesheet lines instead of bare analytic lines.

hr_timesheet_begin_end/readme/newsfragments/692.feature.1.rst

@@ -0,0 +1 @@
+Refactored the module to be more extensible.

hr_timesheet_begin_end/readme/newsfragments/692.feature.2.rst

@@ -0,0 +1 @@
+Changed ``unit_amount`` into a computed (stored, writeable) field.

hr_timesheet_begin_end/tests/test_timesheet_begin_end.py

@@ -9,31 +9,51 @@ class TestBeginEnd(common.TransactionCase):
     def setUp(self):
         super(TestBeginEnd, self).setUp()
         self.timesheet_line_model = self.env["account.analytic.line"]
-        self.analytic = self.env.ref("analytic.analytic_administratif")
-        self.user = self.env.ref("base.user_root")
+        self.project = self.env.ref("project.project_project_1")
+        self.employee = self.env.ref("hr.employee_qdp")
         self.base_line = {
             "name": "test",
             "date": fields.Date.today(),
             "time_start": 10.0,
             "time_stop": 12.0,
-            "user_id": self.user.id,
             "unit_amount": 2.0,
-            "account_id": self.analytic.id,
-            "amount": -60.0,
+            "project_id": self.project.id,
+            "employee_id": self.employee.id,
         }
 
-    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):