Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] hr_timesheet: avoid setting an archived employee to a timesheet in the form view #94308

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions addons/hr_timesheet/i18n/hr_timesheet.pot
Original file line number Diff line number Diff line change
Expand Up @@ -1137,6 +1137,12 @@ msgstr ""
msgid "Timesheets can be logged on this task."
msgstr ""

#. module: hr_timesheet
#: code:addons/hr_timesheet/models/hr_timesheet.py:0
#, python-format
msgid "Timesheets must be created with an active employee."
msgstr ""

#. module: hr_timesheet
#: model:digest.tip,name:hr_timesheet.digest_tip_hr_timesheet_0
msgid "Tip: Record your Timesheets faster"
Expand Down Expand Up @@ -1282,6 +1288,12 @@ msgid ""
"to timesheet on the project."
msgstr ""

#. module: hr_timesheet
#: code:addons/hr_timesheet/models/hr_timesheet.py:0
#, python-format
msgid "You cannot set an archived employee to the existing timesheets."
msgstr ""

#. module: hr_timesheet
#: code:addons/hr_timesheet/models/project.py:0
#, python-format
Expand Down
12 changes: 10 additions & 2 deletions addons/hr_timesheet/models/hr_timesheet.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,12 @@ def create(self, vals_list):

# Although this make a second loop on the vals, we need to wait the preprocess as it could change the company_id in the vals
# TODO To be refactored in master
employees = self.env['hr.employee'].sudo().with_context(active_test=False).search([('user_id', 'in', user_ids)])
employees = self.env['hr.employee'].sudo().search([('user_id', 'in', user_ids)])
employee_for_user_company = defaultdict(dict)
for employee in employees:
employee_for_user_company[employee.user_id.id][employee.company_id.id] = employee.id

employee_ids = set()
for vals in vals_list:
# compute employee only for timesheet lines, makes no sense for other lines
if not vals.get('employee_id') and vals.get('project_id'):
Expand All @@ -133,7 +134,10 @@ def create(self, vals_list):
continue
company_id = list(employee_for_company)[0] if len(employee_for_company) == 1 else vals.get('company_id', self.env.company.id)
vals['employee_id'] = employee_for_company.get(company_id, False)

elif vals.get('employee_id'):
employee_ids.add(vals['employee_id'])
if any(not emp.active for emp in self.env['hr.employee'].browse(list(employee_ids))):
raise UserError(_('Timesheets must be created with an active employee.'))
lines = super(AccountAnalyticLine, self).create(vals_list)
for line, values in zip(lines, vals_list):
if line.project_id: # applied only for timesheet
Expand All @@ -146,6 +150,10 @@ def write(self, values):
raise AccessError(_("You cannot access timesheets that are not yours."))

values = self._timesheet_preprocess(values)
if values.get('employee_id'):
employee = self.env['hr.employee'].browse(values['employee_id'])
if not employee.active:
raise UserError(_('You cannot set an archived employee to the existing timesheets.'))
if 'name' in values and not values.get('name'):
values['name'] = '/'
result = super(AccountAnalyticLine, self).write(values)
Expand Down
19 changes: 19 additions & 0 deletions addons/hr_timesheet/tests/test_timesheet.py
Original file line number Diff line number Diff line change
Expand Up @@ -419,3 +419,22 @@ def test_ensure_product_uom_set_in_timesheet(self):
timesheet1.unit_amount + timesheet2.unit_amount,
'The total timesheet time of this project should be equal to 4.'
)
def test_create_timesheet_with_archived_employee(self):
''' the timesheet can be created or edited only with an active employee
'''
Comment on lines +423 to +424
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
''' the timesheet can be created or edited only with an active employee
'''
""" test cannot create/update timesheet with archived employee """

self.empl_employee2.active = False
batch_vals = {
'project_id': self.project_customer.id,
'task_id': self.task1.id,
'name': 'archived employee timesheet',
'unit_amount': 3,
'employee_id': self.empl_employee2.id
}

self.assertRaises(UserError, self.env['account.analytic.line'].create, batch_vals)

batch_vals["employee_id"] = self.empl_employee.id
timesheet = self.env['account.analytic.line'].create(batch_vals)

with self.assertRaises(UserError):
timesheet.employee_id = self.empl_employee2
3 changes: 2 additions & 1 deletion addons/hr_timesheet/views/hr_timesheet_views.xml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
<attribute name="invisible">0</attribute>
<attribute name="required">1</attribute>
<attribute name="widget">many2one_avatar_employee</attribute>
<attribute name="context">{'active_test': True}</attribute>
</xpath>
</field>
</record>
Expand Down Expand Up @@ -151,7 +152,7 @@
<field name="priority">10</field>
<field name="arch" type="xml">
<xpath expr="//field[@name='date']" position="before">
<field name="employee_id" required="1"/>
<field name="employee_id" required="1" context="{'active_test': True}"/>
<field name="user_id" invisible="1"/>
</xpath>
</field>
Expand Down
6 changes: 3 additions & 3 deletions addons/hr_timesheet/views/project_views.xml
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@
<tree editable="bottom" string="Timesheet Activities" default_order="date">
<field name="date"/>
<field name="user_id" invisible="1"/>
<field name="employee_id" required="1" widget="many2one_avatar_employee"/>
<field name="employee_id" required="1" widget="many2one_avatar_employee" context="{'active_test': True}"/>
<field name="name" required="0"/>
<field name="unit_amount" widget="timesheet_uom" decoration-danger="unit_amount &gt; 24"/>
<field name="project_id" invisible="1"/>
Expand All @@ -122,7 +122,7 @@
<kanban class="o_kanban_mobile">
<field name="date"/>
<field name="user_id"/>
<field name="employee_id" widget="many2one_avatar_employee"/>
<field name="employee_id" widget="many2one_avatar_employee" context="{'active_test': True}"/>
<field name="name"/>
<field name="unit_amount" decoration-danger="unit_amount &gt; 24"/>
<field name="project_id"/>
Expand Down Expand Up @@ -157,7 +157,7 @@
<group>
<field name="date"/>
<field name="user_id" invisible="1"/>
<field name="employee_id" required="1" widget="many2one_avatar_employee"/>
<field name="employee_id" required="1" widget="many2one_avatar_employee" context="{'active_test': True}"/>
<field name="name" required="0"/>
<field name="unit_amount" string="Duration" widget="float_time" decoration-danger="unit_amount &gt; 24"/>
<field name="project_id" invisible="1"/>
Expand Down