From 08b0784e76408778c0f2ed82ff4ad93cd7bf77f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 9 Aug 2013 16:47:52 +0200 Subject: [PATCH] [IMP] hr_holidays: - added a workflow transition from refuse to draft, to allow resetting a refused request - resetting is now based on a can_reset field, taht is based on whether it is my own request or I am an hr manager - added an access right on crm_meeting that was preventing hr officers to validate some requests - improved form view to show reset to draft button accordingly - added tests that helped trigerred the various bugs and improvements bzr revid: tde@openerp.com-20130809144752-o21pjbc56o0t8fym --- addons/hr_holidays/hr_holidays.py | 27 ++- addons/hr_holidays/hr_holidays_view.xml | 11 +- addons/hr_holidays/hr_holidays_workflow.xml | 27 ++- .../hr_holidays/security/ir.model.access.csv | 3 +- addons/hr_holidays/tests/__init__.py | 28 +++ addons/hr_holidays/tests/common.py | 98 +++++++++ .../hr_holidays/tests/test_holidays_flow.py | 207 ++++++++++++++++++ 7 files changed, 379 insertions(+), 22 deletions(-) create mode 100644 addons/hr_holidays/tests/__init__.py create mode 100644 addons/hr_holidays/tests/common.py create mode 100644 addons/hr_holidays/tests/test_holidays_flow.py diff --git a/addons/hr_holidays/hr_holidays.py b/addons/hr_holidays/hr_holidays.py index b75d2538e4e..008c711a1f0 100644 --- a/addons/hr_holidays/hr_holidays.py +++ b/addons/hr_holidays/hr_holidays.py @@ -129,6 +129,19 @@ class hr_holidays(osv.osv): result[hol.id] = hol.number_of_days_temp return result + def _get_can_reset(self, cr, uid, ids, name, arg, context=None): + """User can reset a leave request if it is its own leave request or if + he is an Hr Manager. """ + user = self.pool['res.users'].browse(cr, uid, uid, context=context) + group_hr_manager_id = self.pool.get('ir.model.data').get_object_reference(cr, uid, 'base', 'group_hr_manager')[1] + if group_hr_manager_id in [g.id for g in user.groups_id]: + return dict.fromkeys(ids, True) + result = dict.fromkeys(ids, False) + for holiday in self.browse(cr, uid, ids, context=context): + if holiday.employee_id and holiday.employee_id.user_id and holiday.employee_id.user_id.id == uid: + result[holiday.id] = True + return result + def _check_date(self, cr, uid, ids): for holiday in self.browse(cr, uid, ids): holiday_ids = self.search(cr, uid, [('date_from', '<=', holiday.date_to), ('date_to', '>=', holiday.date_from), ('employee_id', '=', holiday.employee_id.id), ('id', '<>', holiday.id)]) @@ -162,6 +175,9 @@ class hr_holidays(osv.osv): 'holiday_type': fields.selection([('employee','By Employee'),('category','By Employee Tag')], 'Allocation Mode', readonly=True, states={'draft':[('readonly',False)], 'confirm':[('readonly',False)]}, help='By Employee: Allocation/Request for individual Employee, By Employee Tag: Allocation/Request for group of employees in category', required=True), 'manager_id2': fields.many2one('hr.employee', 'Second Approval', readonly=True, help='This area is automaticly filled by the user who validate the leave with second level (If Leave type need second validation)'), 'double_validation': fields.related('holiday_status_id', 'double_validation', type='boolean', relation='hr.holidays.status', string='Apply Double Validation'), + 'can_reset': fields.function( + _get_can_reset, + type='boolean'), } _defaults = { 'employee_id': _employee_get, @@ -297,14 +313,9 @@ class hr_holidays(osv.osv): if context is None: context = {} context = dict(context, mail_create_nolog=True) - return super(hr_holidays, self).create(cr, uid, values, context=context) - - def write(self, cr, uid, ids, vals, context=None): - check_fnct = self.pool.get('hr.holidays.status').check_access_rights - for holiday in self.browse(cr, uid, ids, context=context): - if holiday.state in ('validate','validate1') and not check_fnct(cr, uid, 'write', raise_exception=False): - raise osv.except_osv(_('Warning!'),_('You cannot modify a leave request that has been approved. Contact a human resource manager.')) - return super(hr_holidays, self).write(cr, uid, ids, vals, context=context) + hol_id = super(hr_holidays, self).create(cr, uid, values, context=context) + self.check_holidays(cr, uid, [hol_id], context=context) + return hol_id def holidays_reset(self, cr, uid, ids, context=None): self.write(cr, uid, ids, { diff --git a/addons/hr_holidays/hr_holidays_view.xml b/addons/hr_holidays/hr_holidays_view.xml index 9a91a2198ef..e212ba7b2c8 100644 --- a/addons/hr_holidays/hr_holidays_view.xml +++ b/addons/hr_holidays/hr_holidays_view.xml @@ -53,12 +53,14 @@ 1
+
-
@@ -99,11 +101,14 @@ hr.holidays +
+
diff --git a/addons/hr_holidays/hr_holidays_workflow.xml b/addons/hr_holidays/hr_holidays_workflow.xml index 7d626c16ed3..14cbc48138a 100644 --- a/addons/hr_holidays/hr_holidays_workflow.xml +++ b/addons/hr_holidays/hr_holidays_workflow.xml @@ -3,13 +3,15 @@ @@ -49,11 +51,9 @@ OR - refuse - True function holidays_refuse() @@ -66,16 +66,16 @@ confirm - True - + can_reset + - + reset - True - + can_reset + @@ -118,7 +118,6 @@ - @@ -135,5 +134,13 @@ + + + + reset + can_reset + + + diff --git a/addons/hr_holidays/security/ir.model.access.csv b/addons/hr_holidays/security/ir.model.access.csv index 0375a17ac53..8a2594bb7d7 100644 --- a/addons/hr_holidays/security/ir.model.access.csv +++ b/addons/hr_holidays/security/ir.model.access.csv @@ -1,8 +1,9 @@ id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink -access_hr_holydays_status_user,hr.holidays.status user,model_hr_holidays_status,base.group_hr_user,1,1,1,1 access_hr_holidays_user,hr.holidays.user,model_hr_holidays,base.group_hr_user,1,1,1,1 access_hr_holidays_employee,hr.holidays.employee,model_hr_holidays,base.group_user,1,1,1,1 access_hr_holydays_status_employee,hr.holidays.status employee,model_hr_holidays_status,base.group_user,1,0,0,0 +access_hr_holydays_status_manager,hr.holidays.status manager,model_hr_holidays_status,base.group_hr_manager,1,1,1,1 access_hr_holidays_remain_user,hr.holidays.ramain.user,model_hr_holidays_remaining_leaves_user,base.group_hr_user,1,1,1,1 access_resource_calendar_leaves_user,resource_calendar_leaves_user,resource.model_resource_calendar_leaves,base.group_hr_user,1,1,1,1 +access_crm_meeting_hr_user,crm.meeting.hr.user,base_calendar.model_crm_meeting,base.group_hr_user,1,1,1,1 access_crm_meeting_type_manager,crm.meeting.type.manager,base_calendar.model_crm_meeting_type,base.group_hr_manager,1,1,1,1 diff --git a/addons/hr_holidays/tests/__init__.py b/addons/hr_holidays/tests/__init__.py new file mode 100644 index 00000000000..f70a6277163 --- /dev/null +++ b/addons/hr_holidays/tests/__init__.py @@ -0,0 +1,28 @@ +# -*- coding: utf-8 -*- +############################################################################## +# +# OpenERP, Open Source Business Applications +# Copyright (c) 2013-TODAY OpenERP S.A. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . +# +############################################################################## + +from openerp.addons.hr_holidays.tests import test_holidays_flow + +checks = [ + test_holidays_flow, +] + +# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4: diff --git a/addons/hr_holidays/tests/common.py b/addons/hr_holidays/tests/common.py new file mode 100644 index 00000000000..2124a25ac39 --- /dev/null +++ b/addons/hr_holidays/tests/common.py @@ -0,0 +1,98 @@ +# -*- coding: utf-8 -*- +############################################################################## +# +# OpenERP, Open Source Business Applications +# Copyright (c) 2013-TODAY OpenERP S.A. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . +# +############################################################################## + +from openerp.tests import common + + +class TestHrHolidaysBase(common.TransactionCase): + + def setUp(self): + super(TestHrHolidaysBase, self).setUp() + cr, uid = self.cr, self.uid + + # Usefull models + self.hr_employee = self.registry('hr.employee') + self.hr_holidays = self.registry('hr.holidays') + self.hr_holidays_status = self.registry('hr.holidays.status') + self.res_users = self.registry('res.users') + self.res_partner = self.registry('res.partner') + + # Find Employee group + group_employee_ref = self.registry('ir.model.data').get_object_reference(cr, uid, 'base', 'group_user') + self.group_employee_id = group_employee_ref and group_employee_ref[1] or False + + # Find Hr User group + group_hr_user_ref = self.registry('ir.model.data').get_object_reference(cr, uid, 'base', 'group_hr_user') + self.group_hr_user_ref_id = group_hr_user_ref and group_hr_user_ref[1] or False + + # Find Hr Manager group + group_hr_manager_ref = self.registry('ir.model.data').get_object_reference(cr, uid, 'base', 'group_hr_manager') + self.group_hr_manager_ref_id = group_hr_manager_ref and group_hr_manager_ref[1] or False + + # Test partners to use through the various tests + self.hr_partner_id = self.res_partner.create(cr, uid, { + 'name': 'Gertrude AgrolaitPartner', + 'email': 'gertrude.partner@agrolait.com', + }) + self.email_partner_id = self.res_partner.create(cr, uid, { + 'name': 'Patrick Ratatouille', + 'email': 'patrick.ratatouille@agrolait.com', + }) + + # Test users to use through the various tests + self.user_hruser_id = self.res_users.create(cr, uid, { + 'name': 'Armande HrUser', + 'login': 'Armande', + 'alias_name': 'armande', + 'email': 'armande.hruser@example.com', + 'groups_id': [(6, 0, [self.group_employee_id, self.group_hr_user_ref_id])] + }, {'no_reset_password': True}) + self.user_hrmanager_id = self.res_users.create(cr, uid, { + 'name': 'Bastien HrManager', + 'login': 'bastien', + 'alias_name': 'bastien', + 'email': 'bastien.hrmanager@example.com', + 'groups_id': [(6, 0, [self.group_employee_id, self.group_hr_manager_ref_id])] + }, {'no_reset_password': True}) + self.user_none_id = self.res_users.create(cr, uid, { + 'name': 'Charlie Avotbonkeur', + 'login': 'charlie', + 'alias_name': 'charlie', + 'email': 'charlie.noone@example.com', + 'groups_id': [(6, 0, [])] + }, {'no_reset_password': True}) + self.user_employee_id = self.res_users.create(cr, uid, { + 'name': 'David Employee', + 'login': 'david', + 'alias_name': 'david', + 'email': 'david.employee@example.com', + 'groups_id': [(6, 0, [self.group_employee_id])] + }, {'no_reset_password': True}) + + # Hr Data + self.employee_emp_id = self.hr_employee.create(cr, uid, { + 'name': 'David Employee', + 'user_id': self.user_employee_id, + }) + self.employee_hruser_id = self.hr_employee.create(cr, uid, { + 'name': 'Armande HrUser', + 'user_id': self.user_hruser_id, + }) diff --git a/addons/hr_holidays/tests/test_holidays_flow.py b/addons/hr_holidays/tests/test_holidays_flow.py new file mode 100644 index 00000000000..caa6e14f2db --- /dev/null +++ b/addons/hr_holidays/tests/test_holidays_flow.py @@ -0,0 +1,207 @@ +# -*- coding: utf-8 -*- +############################################################################## +# +# OpenERP, Open Source Business Applications +# Copyright (c) 2013-TODAY OpenERP S.A. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . +# +############################################################################## + +from datetime import datetime +from dateutil.relativedelta import relativedelta + +from openerp.addons.hr_holidays.tests.common import TestHrHolidaysBase +from openerp.osv.orm import except_orm +from openerp.tools import mute_logger + + +class TestHolidaysFlow(TestHrHolidaysBase): + + @mute_logger('openerp.addons.base.ir.ir_model', 'openerp.osv.orm') + def test_00_leave_request_flow(self): + """ Testing leave request flow """ + cr, uid = self.cr, self.uid + + def _check_holidays_status(holiday_status, ml, lt, rl, vrl): + self.assertEqual(holiday_status.max_leaves, ml, + 'hr_holidays: wrong type days computation') + self.assertEqual(holiday_status.leaves_taken, lt, + 'hr_holidays: wrong type days computation') + self.assertEqual(holiday_status.remaining_leaves, rl, + 'hr_holidays: wrong type days computation') + self.assertEqual(holiday_status.virtual_remaining_leaves, vrl, + 'hr_holidays: wrong type days computation') + + # HrUser creates some holiday statuses -> crash because only HrManagers should do this + with self.assertRaises(except_orm): + self.holidays_status_dummy = self.hr_holidays_status.create(cr, self.user_hruser_id, { + 'name': 'UserCheats', + 'limit': True, + }) + + # HrManager creates some holiday statuses + self.holidays_status_0 = self.hr_holidays_status.create(cr, self.user_hrmanager_id, { + 'name': 'WithMeetingType', + 'limit': True, + 'categ_id': self.registry('crm.meeting.type').create(cr, self.user_hrmanager_id, {'name': 'NotLimitedMeetingType'}), + }) + self.holidays_status_1 = self.hr_holidays_status.create(cr, self.user_hrmanager_id, { + 'name': 'NotLimited', + 'limit': True, + }) + self.holidays_status_2 = self.hr_holidays_status.create(cr, self.user_hrmanager_id, { + 'name': 'Limited', + 'limit': False, + 'double_validation': True, + }) + + # -------------------------------------------------- + # Case1: unlimited type of leave request + # -------------------------------------------------- + + # Employee creates a leave request for another employee -> should crash + with self.assertRaises(except_orm): + self.hr_holidays.create(cr, self.user_employee_id, { + 'name': 'Hol10', + 'employee_id': self.employee_hruser_id, + 'holiday_status_id': self.holidays_status_1, + 'date_from': (datetime.today() - relativedelta(days=1)), + 'date_to': datetime.today(), + 'number_of_days_temp': 1, + }) + + # Employee creates a leave request in a no-limit category + hol1_id = self.hr_holidays.create(cr, self.user_employee_id, { + 'name': 'Hol11', + 'employee_id': self.employee_emp_id, + 'holiday_status_id': self.holidays_status_1, + 'date_from': (datetime.today() - relativedelta(days=1)), + 'date_to': datetime.today(), + 'number_of_days_temp': 1, + }) + hol1 = self.hr_holidays.browse(cr, self.user_hruser_id, hol1_id) + self.assertEqual(hol1.state, 'confirm', 'hr_holidays: newly created leave request should be in confirm state') + + # Employee validates its leave request -> should not work + self.hr_holidays.signal_validate(cr, self.user_employee_id, [hol1_id]) + hol1.refresh() + self.assertEqual(hol1.state, 'confirm', 'hr_holidays: employee should not be able to validate its own leave request') + + # HrUser validates the employee leave request + self.hr_holidays.signal_validate(cr, self.user_hrmanager_id, [hol1_id]) + hol1.refresh() + self.assertEqual(hol1.state, 'validate', 'hr_holidays: validates leave request should be in validate state') + + # -------------------------------------------------- + # Case2: limited type of leave request + # -------------------------------------------------- + + # Employee creates a new leave request at the same time -> crash, avoid interlapping + with self.assertRaises(except_orm): + self.hr_holidays.create(cr, self.user_employee_id, { + 'name': 'Hol21', + 'employee_id': self.employee_emp_id, + 'holiday_status_id': self.holidays_status_1, + 'date_from': (datetime.today() - relativedelta(days=1)).strftime('%Y-%m-%d %H:%M'), + 'date_to': datetime.today(), + 'number_of_days_temp': 1, + }) + + # Employee creates a leave request in a limited category -> crash, not enough days left + with self.assertRaises(except_orm): + self.hr_holidays.create(cr, self.user_employee_id, { + 'name': 'Hol22', + 'employee_id': self.employee_emp_id, + 'holiday_status_id': self.holidays_status_2, + 'date_from': (datetime.today() + relativedelta(days=0)).strftime('%Y-%m-%d %H:%M'), + 'date_to': (datetime.today() + relativedelta(days=1)), + 'number_of_days_temp': 1, + }) + + # Clean transaction + self.hr_holidays.unlink(cr, uid, self.hr_holidays.search(cr, uid, [('name', 'in', ['Hol21', 'Hol22'])])) + + # HrUser allocates some leaves to the employee + aloc1_id = self.hr_holidays.create(cr, self.user_hruser_id, { + 'name': 'Days for limited category', + 'employee_id': self.employee_emp_id, + 'holiday_status_id': self.holidays_status_2, + 'type': 'add', + 'number_of_days_temp': 2, + }) + # HrUser validates the allocation request + self.hr_holidays.signal_validate(cr, self.user_hruser_id, [aloc1_id]) + self.hr_holidays.signal_second_validate(cr, self.user_hruser_id, [aloc1_id]) + # Checks Employee has effectively some days left + hol_status_2 = self.hr_holidays_status.browse(cr, self.user_employee_id, self.holidays_status_2) + _check_holidays_status(hol_status_2, 2.0, 0.0, 2.0, 2.0) + + # Employee creates a leave request in the limited category, now that he has some days left + hol2_id = self.hr_holidays.create(cr, self.user_employee_id, { + 'name': 'Hol22', + 'employee_id': self.employee_emp_id, + 'holiday_status_id': self.holidays_status_2, + 'date_from': (datetime.today() + relativedelta(days=2)).strftime('%Y-%m-%d %H:%M'), + 'date_to': (datetime.today() + relativedelta(days=3)), + 'number_of_days_temp': 1, + }) + hol2 = self.hr_holidays.browse(cr, self.user_hruser_id, hol2_id) + # Check left days: - 1 virtual remaining day + hol_status_2.refresh() + _check_holidays_status(hol_status_2, 2.0, 0.0, 2.0, 1.0) + + # HrUser validates the first step + self.hr_holidays.signal_validate(cr, self.user_hruser_id, [hol2_id]) + hol2.refresh() + self.assertEqual(hol2.state, 'validate1', + 'hr_holidays: first validation should lead to validate1 state') + + # HrUser validates the second step + self.hr_holidays.signal_second_validate(cr, self.user_hruser_id, [hol2_id]) + hol2.refresh() + self.assertEqual(hol2.state, 'validate', + 'hr_holidays: second validation should lead to validate state') + # Check left days: - 1 day taken + hol_status_2.refresh() + _check_holidays_status(hol_status_2, 2.0, 1.0, 1.0, 1.0) + + # HrManager finds an error: he refuses the leave request + self.hr_holidays.signal_refuse(cr, self.user_hrmanager_id, [hol2_id]) + hol2.refresh() + self.assertEqual(hol2.state, 'refuse', + 'hr_holidays: refuse should lead to refuse state') + # Check left days: 2 days left again + hol_status_2.refresh() + _check_holidays_status(hol_status_2, 2.0, 0.0, 2.0, 2.0) + + # Annoyed, HrUser tries to fix its error and tries to reset the leave request -> does not work, only HrManager + self.hr_holidays.signal_reset(cr, self.user_hruser_id, [hol2_id]) + self.assertEqual(hol2.state, 'refuse', + 'hr_holidays: hr_user should not be able to reset a refused leave request') + + # HrManager resets the request + self.hr_holidays.signal_reset(cr, self.user_hrmanager_id, [hol2_id]) + hol2.refresh() + self.assertEqual(hol2.state, 'draft', + 'hr_holidays: resetting should lead to draft state') + + # HrManager changes the date and put too much days -> crash when confirming + self.hr_holidays.write(cr, self.user_hrmanager_id, [hol2_id], { + 'date_from': (datetime.today() + relativedelta(days=4)).strftime('%Y-%m-%d %H:%M'), + 'date_to': (datetime.today() + relativedelta(days=7)), + 'number_of_days_temp': 4, + }) + with self.assertRaises(except_orm): + self.hr_holidays.signal_confirm(cr, self.user_hrmanager_id, [hol2_id])