From 5c5de9d9c12e009f2f3b3ca4a57adb8c89acf811 Mon Sep 17 00:00:00 2001
From: Jared Kipe <jared@hibou.io>
Date: Tue, 6 Oct 2020 07:56:04 -0700
Subject: [PATCH] [IMP] hr_payroll_overtime: detect recursion and prevent the
 simplest type in the form view

---
 hr_payroll_overtime/models/hr_payslip.py      | 17 ++++++--
 hr_payroll_overtime/tests/test_overtime.py    | 39 +++++++++++++++++++
 .../views/hr_work_entry_views.xml             |  8 +++-
 3 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/hr_payroll_overtime/models/hr_payslip.py b/hr_payroll_overtime/models/hr_payslip.py
index 3b50cd7a..4e482bf9 100644
--- a/hr_payroll_overtime/models/hr_payslip.py
+++ b/hr_payroll_overtime/models/hr_payslip.py
@@ -1,5 +1,6 @@
 from collections import defaultdict
 from odoo import models
+from odoo.exceptions import UserError
 
 
 class HRPayslip(models.Model):
@@ -30,10 +31,15 @@ class HRPayslip(models.Model):
         day_hours = defaultdict(float)
         week_hours = defaultdict(float)
         iso_days = set()
-        for iso_date, entries in work_data:
-            iso_date = _adjust_week(iso_date)
-            for work_type, hours, _ in entries:
-                self._aggregate_overtime_add_work_type_hours(work_type, hours, iso_date, result, iso_days, day_hours, week_hours)
+        try:
+            for iso_date, entries in work_data:
+                iso_date = _adjust_week(iso_date)
+                for work_type, hours, _ in entries:
+                    self._aggregate_overtime_add_work_type_hours(work_type, hours, iso_date, result, iso_days, day_hours, week_hours)
+        except RecursionError:
+            raise UserError('RecursionError raised.  Ensure you have not overtime loops, you should have an '
+                            'end work type that does not have any "overtime" version, and would be considered '
+                            'the "highest overtime" work type and rate.')
 
         return result
 
@@ -75,6 +81,9 @@ class HRPayslip(models.Model):
                 # we need to save this because it won't be set once it reenter, we won't know what the original
                 # overtime multiplier was
                 working_aggregation[work_type.overtime_work_type_id][2] = work_type.overtime_type_id.multiplier
+                if work_type == work_type.overtime_work_type_id:
+                    # trivial infinite recursion
+                    raise UserError('Work type %s (id %s) must not have itself as its next overtime type.' % (work_type.name, work_type.id))
                 self._aggregate_overtime_add_work_type_hours(work_type.overtime_work_type_id, ot_hours, iso_date,
                                                              working_aggregation, iso_days, day_hours, week_hours)
         else:
diff --git a/hr_payroll_overtime/tests/test_overtime.py b/hr_payroll_overtime/tests/test_overtime.py
index 808c073e..6c5cf9c3 100644
--- a/hr_payroll_overtime/tests/test_overtime.py
+++ b/hr_payroll_overtime/tests/test_overtime.py
@@ -1,4 +1,5 @@
 from odoo.tests import common
+from odoo.exceptions import UserError
 
 
 class TestOvertime(common.TransactionCase):
@@ -321,3 +322,41 @@ class TestOvertime(common.TransactionCase):
         self.assertTrue(self.work_type_overtime2 in result_data)
         self.assertEqual(result_data[self.work_type_overtime2][0], 0)
         self.assertEqual(result_data[self.work_type_overtime2][1], 2.0)
+
+    def test_13_recursive_infinite_trivial(self):
+        # recursive should will use a second overtime, but not this time!
+        self.overtime_rules.hours_per_day = 8.0
+        self.overtime_rules.multiplier_per_day = 1.5
+        self.work_type.overtime_type_id = self.overtime_rules
+        # overtime goes to itself
+        self.work_type.overtime_work_type_id = self.work_type
+
+        work_data = [
+            ((2020, 24, 2), [
+                # 2hr overtime
+                (self.work_type, 4.0, None),
+                (self.work_type, 6.0, None),
+            ]),
+        ]
+
+        with self.assertRaises(UserError):
+            result_data = self.payslip.aggregate_overtime(work_data)
+
+    def test_14_recursive_infinite_loop(self):
+        # recursive will use a second overtime, but not this time!
+        self.overtime_rules.hours_per_day = 8.0
+        self.overtime_rules.multiplier_per_day = 1.5
+        self.work_type_overtime.overtime_type_id = self.overtime_rules
+        # overtime goes back to worktype
+        self.work_type_overtime.overtime_work_type_id = self.work_type
+
+        work_data = [
+            ((2020, 24, 2), [
+                # 2hr overtime
+                (self.work_type, 4.0, None),
+                (self.work_type, 6.0, None),
+            ]),
+        ]
+
+        with self.assertRaises(UserError):
+            result_data = self.payslip.aggregate_overtime(work_data)
diff --git a/hr_payroll_overtime/views/hr_work_entry_views.xml b/hr_payroll_overtime/views/hr_work_entry_views.xml
index e543a70e..cbdb58f3 100644
--- a/hr_payroll_overtime/views/hr_work_entry_views.xml
+++ b/hr_payroll_overtime/views/hr_work_entry_views.xml
@@ -9,8 +9,12 @@
         <field name="arch" type="xml">
             <xpath expr="//group[@name='main_group']" position="after">
                 <group name="overtime_group">
-                    <field name="overtime_work_type_id" attrs="{'required': [('overtime_type_id', '!=', False)]}"/>
-                    <field name="overtime_type_id" attrs="{'required': [('overtime_work_type_id', '!=', False)]}"/>
+                    <field name="overtime_work_type_id"
+                           domain="[('id', '!=', id)]"
+                           attrs="{'required': [('overtime_type_id', '!=', False)]}" />
+                    <field name="overtime_type_id"
+                           domain="[('id', '!=', id)]"
+                           attrs="{'required': [('overtime_work_type_id', '!=', False)]}" />
                 </group>
             </xpath>
         </field>