From d7ec92f349efebf48efddf1b4d66e4b33f8bcffa Mon Sep 17 00:00:00 2001 From: "Jurgen (jugj)" Date: Fri, 21 Apr 2023 13:47:17 +0000 Subject: [PATCH] [FIX] hr_holidays : Remove automatic notification if no responsible_ids is set Currently in hr_leaves_type, if no responsible_ids are set, all members of group_hr_holidays_user would get notified, the new behavior changes that to : If no responsible_ids are set, no one is notified. Some unit tests had to be adapted to this new flow, since they were sometimes getting created with no responsible_id assigned, leading to no one being notified. task-3284318 closes odoo/odoo#125738 X-original-commit: fb1a6a8f0072398c57508b59724fcbde9a34b124 Signed-off-by: Kevin Baptiste --- addons/hr_holidays/i18n/hr_holidays.pot | 6 ++--- addons/hr_holidays/models/hr_leave.py | 24 +++++++++---------- .../hr_holidays/models/hr_leave_allocation.py | 24 +++++++++---------- addons/hr_holidays/models/hr_leave_type.py | 4 ++-- .../tests/test_accrual_allocations.py | 4 ++-- .../hr_holidays/tests/test_holidays_flow.py | 1 + .../hr_holidays/tests/test_leave_requests.py | 3 ++- addons/hr_holidays/tests/test_res_partner.py | 1 + .../hr_holidays/views/hr_leave_type_views.xml | 6 +++-- 9 files changed, 38 insertions(+), 35 deletions(-) diff --git a/addons/hr_holidays/i18n/hr_holidays.pot b/addons/hr_holidays/i18n/hr_holidays.pot index e42ede6fc57..15a8a589583 100644 --- a/addons/hr_holidays/i18n/hr_holidays.pot +++ b/addons/hr_holidays/i18n/hr_holidays.pot @@ -1360,8 +1360,8 @@ msgstr "" #. module: hr_holidays #: model:ir.model.fields,help:hr_holidays.field_hr_leave_type__responsible_ids msgid "" -"Choose the Time Off Officer who will be notified to approve allocation or " -"Time Off request" +"Choose the Time Off Officers who will be notified to approve allocation or " +"Time Off Request. If empty, nobody will be notified" msgstr "" #. module: hr_holidays @@ -3018,7 +3018,7 @@ msgstr "" #. module: hr_holidays #: model:ir.model.fields,field_description:hr_holidays.field_hr_leave_type__responsible_ids -msgid "Responsible Time Off Officer" +msgid "Notified Time Off Officer" msgstr "" #. module: hr_holidays diff --git a/addons/hr_holidays/models/hr_leave.py b/addons/hr_holidays/models/hr_leave.py index b63cdb3386a..2918b7ba57a 100644 --- a/addons/hr_holidays/models/hr_leave.py +++ b/addons/hr_holidays/models/hr_leave.py @@ -1588,8 +1588,7 @@ class HolidaysRequest(models.Model): elif self.validation_type == 'hr' or (self.validation_type == 'both' and self.state == 'validate1'): if self.holiday_status_id.responsible_ids: responsible = self.holiday_status_id.responsible_ids - else: - responsible = self.env.ref('hr_holidays.group_hr_holidays_user').users.filtered(lambda u: self.holiday_status_id.company_id in u.company_ids) + return responsible def activity_update(self): @@ -1604,16 +1603,17 @@ class HolidaysRequest(models.Model): if holiday.state == 'draft': to_clean |= holiday elif holiday.state == 'confirm': - user_ids = holiday.sudo()._get_responsible_for_approval().ids or self.env.user.ids - for user_id in user_ids: - activity_vals.append({ - 'activity_type_id': self.env.ref('hr_holidays.mail_act_leave_approval').id, - 'automated': True, - 'note': note, - 'user_id': user_id, - 'res_id': holiday.id, - 'res_model_id': self.env.ref('hr_holidays.model_hr_leave').id, - }) + if holiday.holiday_status_id.responsible_ids: + user_ids = holiday.sudo()._get_responsible_for_approval().ids or self.env.user.ids + for user_id in user_ids: + activity_vals.append({ + 'activity_type_id': self.env.ref('hr_holidays.mail_act_leave_approval').id, + 'automated': True, + 'note': note, + 'user_id': user_id, + 'res_id': holiday.id, + 'res_model_id': self.env.ref('hr_holidays.model_hr_leave').id, + }) elif holiday.state == 'validate': to_do |= holiday elif holiday.state == 'refuse': diff --git a/addons/hr_holidays/models/hr_leave_allocation.py b/addons/hr_holidays/models/hr_leave_allocation.py index 6ff5b946e02..693582195c4 100644 --- a/addons/hr_holidays/models/hr_leave_allocation.py +++ b/addons/hr_holidays/models/hr_leave_allocation.py @@ -757,9 +757,6 @@ class HolidaysAllocation(models.Model): if self.validation_type == 'officer' or self.validation_type == 'set': if self.holiday_status_id.responsible_ids: responsible = self.holiday_status_id.responsible_ids - else: - responsible = self.env.ref('hr_holidays.group_hr_holidays_user').users.filtered(lambda u: self.holiday_status_id.company_id in u.company_ids) - return responsible def activity_update(self): @@ -776,16 +773,17 @@ class HolidaysAllocation(models.Model): if allocation.state == 'draft': to_clean |= allocation elif allocation.state == 'confirm': - user_ids = allocation.sudo()._get_responsible_for_approval().ids or self.env.user.ids - for user_id in user_ids: - activity_vals.append({ - 'activity_type_id': self.env.ref('hr_holidays.mail_act_leave_allocation_approval').id, - 'automated': True, - 'note': note, - 'user_id': user_id, - 'res_id': allocation.id, - 'res_model_id': self.env.ref('hr_holidays.model_hr_leave_allocation').id, - }) + if allocation.holiday_status_id.responsible_ids: + user_ids = allocation.sudo()._get_responsible_for_approval().ids + for user_id in user_ids: + activity_vals.append({ + 'activity_type_id': self.env.ref('hr_holidays.mail_act_leave_allocation_approval').id, + 'automated': True, + 'note': note, + 'user_id': user_id, + 'res_id': allocation.id, + 'res_model_id': self.env.ref('hr_holidays.model_hr_leave_allocation').id, + }) elif allocation.state == 'validate': to_do |= allocation elif allocation.state == 'refuse': diff --git a/addons/hr_holidays/models/hr_leave_type.py b/addons/hr_holidays/models/hr_leave_type.py index 57f93ca8d66..7148eb5a3f2 100644 --- a/addons/hr_holidays/models/hr_leave_type.py +++ b/addons/hr_holidays/models/hr_leave_type.py @@ -59,12 +59,12 @@ class HolidaysType(models.Model): compute='_compute_group_days_leave', string='Group Time Off') company_id = fields.Many2one('res.company', string='Company', default=lambda self: self.env.company) responsible_ids = fields.Many2many( - 'res.users', 'hr_leave_type_res_users_rel', 'hr_leave_type_id', 'res_users_id', string='Responsible Time Off Officer', + 'res.users', 'hr_leave_type_res_users_rel', 'hr_leave_type_id', 'res_users_id', string='Notified Time Off Officer', domain=lambda self: [('groups_id', 'in', self.env.ref('hr_holidays.group_hr_holidays_user').id), ('share', '=', False), ('company_ids', 'in', self.env.company.id)], auto_join=True, - help="Choose the Time Off Officer who will be notified to approve allocation or Time Off request") + help="Choose the Time Off Officers who will be notified to approve allocation or Time Off Request. If empty, nobody will be notified") leave_validation_type = fields.Selection([ ('no_validation', 'No Validation'), ('hr', 'By Time Off Officer'), diff --git a/addons/hr_holidays/tests/test_accrual_allocations.py b/addons/hr_holidays/tests/test_accrual_allocations.py index 45c7adf7164..c27c421d30f 100644 --- a/addons/hr_holidays/tests/test_accrual_allocations.py +++ b/addons/hr_holidays/tests/test_accrual_allocations.py @@ -805,7 +805,7 @@ class TestAccrualAllocations(TestHrHolidaysCommon): # The second level could give 6 days but since the first level was already giving # 3 days, the second level gives 3 days to reach the second level's limit. # The third level gives 1 day since it only counts for one iteration. - self.assertEqual(allocation.number_of_days, 7) + self.assertAlmostEqual(allocation.number_of_days, 7, 2) def test_accrual_lost_previous_days(self): # Test that when an allocation with two levels is made and that the first level has it's action @@ -876,7 +876,7 @@ class TestAccrualAllocations(TestHrHolidaysCommon): allocation.action_validate() with freeze_time('2022-4-1'): allocation._update_accrual() - self.assertEqual(allocation.number_of_days, 3, "Invalid number of days") + self.assertAlmostEqual(allocation.number_of_days, 3, 2, "Invalid number of days") def test_accrual_maximum_leaves(self): accrual_plan = self.env['hr.leave.accrual.plan'].with_context(tracking_disable=True).create({ diff --git a/addons/hr_holidays/tests/test_holidays_flow.py b/addons/hr_holidays/tests/test_holidays_flow.py index e625f036da3..9c8112db67f 100644 --- a/addons/hr_holidays/tests/test_holidays_flow.py +++ b/addons/hr_holidays/tests/test_holidays_flow.py @@ -134,6 +134,7 @@ class TestHolidaysFlow(TestHrHolidaysCommon): 'employee_requests': 'no', 'allocation_validation_type': 'officer', 'leave_validation_type': 'both', + 'responsible_ids': [Command.link(self.env.ref('base.user_admin').id)] }) HolidaysEmployeeGroup = Requests.with_user(self.user_employee_id) diff --git a/addons/hr_holidays/tests/test_leave_requests.py b/addons/hr_holidays/tests/test_leave_requests.py index d482bc1613a..d9a77f8df9d 100644 --- a/addons/hr_holidays/tests/test_leave_requests.py +++ b/addons/hr_holidays/tests/test_leave_requests.py @@ -7,7 +7,7 @@ from dateutil.relativedelta import relativedelta from freezegun import freeze_time from pytz import timezone, UTC -from odoo import fields +from odoo import fields, Command from odoo.exceptions import ValidationError from odoo.tools import mute_logger from odoo.tests.common import Form @@ -1069,6 +1069,7 @@ class TestLeaveRequests(TestHrHolidaysCommon): 'name': leave_validation_type.capitalize(), 'leave_validation_type': leave_validation_type, 'requires_allocation': 'no', + 'responsible_ids': [Command.link(self.env.ref('base.user_admin').id)], }) current_leave = self.env['hr.leave'].with_user(self.user_employee_id).create({ 'name': 'Holiday Request', diff --git a/addons/hr_holidays/tests/test_res_partner.py b/addons/hr_holidays/tests/test_res_partner.py index 66f2a4d87c6..3b87e623a7e 100644 --- a/addons/hr_holidays/tests/test_res_partner.py +++ b/addons/hr_holidays/tests/test_res_partner.py @@ -39,6 +39,7 @@ class TestPartner(TransactionCase): 'requires_allocation': 'no', 'name': 'Legal Leaves', 'time_type': 'leave', + 'responsible_ids': cls.users.ids }) cls.leaves = cls.env['hr.leave'].create([{ 'date_from': cls.today + relativedelta(days=-2), diff --git a/addons/hr_holidays/views/hr_leave_type_views.xml b/addons/hr_holidays/views/hr_leave_type_views.xml index d2fdd917206..fa3ce8ab773 100644 --- a/addons/hr_holidays/views/hr_leave_type_views.xml +++ b/addons/hr_holidays/views/hr_leave_type_views.xml @@ -76,7 +76,7 @@ - + @@ -114,13 +114,15 @@ hr.leave.type.normal.tree hr.leave.type - + +