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

[MERGE] hr_payroll: Refactor payroll engine + Move to enterprise #31806

Merged
merged 27 commits into from Mar 19, 2019

Conversation

tivisse
Copy link
Contributor

@tivisse tivisse commented Mar 13, 2019

hr_payroll: Remove parent_id hierarchy + Add structures categorization

1/ Remove the parent-children relation on salary structures

Currently it is difficult to see what rules are applied on salary structures.

Most of the structures are inheriting 3 rules (Basic, Gross, Net) from the
'Base for new structures', and those rules are not shown. Same issue if a
structure has another one as parent (as in the French localization).

This bring more annoying issues:

  • Since the saas-12.1, we can already set different accounts for different
    companies on the salary rules for which we want to post journal entries.
    But for the parent rules, like the 'NET' amount, it's impossible to post
    the journal entries on different account for different salary structures.
    (eg: for CP200 employees and CP200 workers). As the rules are following
    a M2M relation on the structure, we have to remove the parent_id and to
    recreate all the rules in the final structure.
  • If we are not at ease with the M2M relation (which is the case for most
    of the end users), we will simply modify the rule on the parent structure.
    This was already done on the French localization on which we modified the
    code, sequence and the name. This could have some desastrous consequences
    as this will break the order on which the rules are computed for other
    structures for the sequence, or break other integration with the payroll
    as we also modify the code, for example the salary configurator.

As this parent-children relation on structure could cause several issues
difficult to spot and as this is quite difficult to correctly visualize the
structure composition, it is better to remove it and define all the rules
inside the structure itself. This improves also the code readability as we
are not trying to recursively retrieve the rules and find a proper sequence
inside them.

2/ Make the rules on the salary structure a o2m instead of a m2m

The issues described just above indicates that it would be better to remove
the M2M relation between salary structures and salary rules, and make it
a O2M relation instead. That way, no more confusion and misconfiguration of
the rules fields (code, sequence, accounts, ...).

3/ Remove the parent-children relation on rules

A salary rule can have a parent salary rule. This mechanism, which is
not obvious at all for an end user allows to specify a parent rules.
If the condition is met for the parent rule (eg: the employee has at least
1 child), all the children rules could be applied (eg: Deduction of 90 euros
if the number of children is between 1 and 1).

Visually it doesn't make a lot of sense, as it displays one line with an
amount of 0 for the parent, and one line with the real amount for each
applied child. On the other hand, no child rule is linked to the structure
and an end user has to click on the parent rule, to see the children and
understand that something mystic and hidden is existing.

This is equivalent to define as many classic rules as we have children, or
even better, define 1 rule that will compute the real amount directly,
as the number of children can be accessed from the localdict when computing
the salary line.

This commit also removed the relation.

4/ Clean some brols in the code (technical)

The 3 first points of the spec allow us to clean a little the code readability
and complexity.

5/ Adapt all the localizations for these changes

All the localizations are impacted by those changes. What is done in this commit
is mainly:

  • l10n_be_hr_payroll: Make 3 separated files for the 3 existing structures (the
    fourth one, belgian worker, is not correct and is removed). For each
    structure, define explicitely the rules (in the correct order by sequence).
    This multiplies the total number of rules as some of them were shared (like
    the withholding tax rule for example), but all the code that is supposed to be
    modified each year/quarter/whenever the government decides to modify the law
    has been moved into a python file, and is available the computation context.
    This will also allow us to update the computation rules without having to
    update the module.
  • l10n_fr_hr_payroll: There were 3 rules following this scheme:
                              Base for new structure
                                        |
                                 Basic structure
                                        |
                              ----------------------
                              |                    |
                           Cadre                Non-Cadre

All the rules parent rules have been duplicated into the 2 remaining
structures, cadre and non-cadre. There were a lot of parent-children relations
between the rules that have been adapted to the new model.

  • l10n_in_hr_payroll: There was a lot of rules in data, and a structure in demo
    data, which was using 4 or 5 of these rules. As the rules doesn't seem to make
    a lot of sense altogether, everything was moved in demo data.

6/ Introduce a new Salary Structure Type model

Currently we have a model hr.contract.type with a M2O on the contracts. This has
never been used in 9 years and is removed in this commit. On the other hand, we
would like to define a new model hr.payroll.structure.type (eg: CP200 Employee),
which will line all the salary structures the Belgian localization could bring
(Classic salary, double holidays, 13th month, ...). This field is defined on the
structure, and is displayed on the contract (as the same place than the contract
type we just removed).

On this model we could also define some fields like:

  • The default pay period: Selection field, default = monthly. This is applied on
    the contract when selecting the structure.
  • The Working Schedule, m2o. This is applied on the contract when selecting the
    structure

For example, on the 'commission paritaire' for teachers, we could set the
default pay period to 'Every 15 days' and the working schedule to '19 hours /
week'. Every contract under this structure would take those values.

Migration

For a migration point of view:

  • All the rules that had a parent_rule_id should be set to
    appears_on_payslip=False. The column parent_rule_id can be dropped afterward.
  • All the rules that are coming from a parent structure or higher should be
    duplicated and the field 'struct_id' should be set on the current structure.
    The column parent_id can be dropped afterward.
  • All the rules that were defined on the structure should have the field
    'struct_id' defined on them.
  • The rules that are not linked to a structure should be unlinked.
  • The column type_id on the contract could be dropped, no need to keep the
    values too.
  • A default structure type is defined in data ('Employee'). This could be set to
    all the existing structure for the newly created 'type_id' field.

TaskID: 1942832

hr_payroll: rename hr.benefit into hr.work.entry

It makes more sense to name an entry in the employee schedule like that.

TaskID: 1941598

hr_holidays: Group team leader imply group user

Currently, group Team Leader does not imply group user.
However a team leader is always a user and should be member of
the corresponding group.

hr_holidays: Allow Team leader to approve

Currently, a Team leader can't approve a leave of an employee
in his team if the leave type requires an allocation.

The reason is the ir.rule for base.group_user restricting allocation
read access to only your own allocation
([('employee_id.user_id', '=', user.id)]). There are no ir.rule for group
Team leader. Therefore, a team leader can't read allocations for his team
members.

Fix: add an ir.rule to allow a team leader to read allocations
of employees in his team.

  • add associated tests

hr_payroll: Fix contract access to approve leave

When a hr.leave is approved, a resource.calendar.leave
is created in the employee's calendar and in the
contract's calendar if it's different.
However, a user in group_holiday_user but not in
group_hr_contract_manager does not have access rights
to read a contract.
This leads to an AccessError

Fix: use sudo to read the contract.

hr_contract: group_hr_contract_manager implies group_user

A user with only the contract access rights should be able to create
a contract.

l10n_be_hr_payroll_fleet: Fix access rights issues

If a user is member of hr_contract.group_hr_contract_manager
but not member of fleet.fleet_group_manager some access issues arise
The issues are related to the car_id field of hr.contract
because the user has no access rights for fleet.vehicle.

Fix to solve the issue:
Make all car related field of contracts available
to fleet manager only.

If the contract manager want to be able to manage cars on
contracts, he must manually be granted fleet manager access.
This is a requirement if the contract manager want to use the
company car feature.

hr_payroll: Allow a contract manager to read salary structures

An employee with only the contract user group should be able to create
a contract. It implies to be able to read a salary structure and
a salary structure type.

hr_payroll: Add leave type One2many on benefit type

Purpose

Currently, linking a benefit type and a leave type is done through
the leave type form view.
This is anoying because it requires to first create the benfit type,
then go to the leave type and link it.
Moreover, linking is the role of a payroll manager which should be
able to do it from the Payroll app.

Specification

Add a One2many field leave_type_ids on hr.benefit.type to
allow to link a leave type when creating a benefit type.

commit a9f97a2
Author: Lucas Lefèvre lul@odoo.com
Date: Mon Mar 4 16:06:33 2019 +0100

hr_payroll: Make payroll groups imply holiday groups

Purpose

Currently, groups hr_holidays.group_hr_holidays_[user|manager]
and hr_payroll.group_hr_payroll_[user|manager] are independant.

A member of payroll groups is not able to approve/refuse a leave.

This is a problem as they should be able to approve/refuse leaves before generating payslips.
This is the point of hr.benefits: a global way of managing
attendances and leaves before generating payslips.

Specification

Payroll groups should imply the corresponding
holiday group.

  1. User
    hr_payroll.group_hr_payroll_user
    implies
    hr_holidays.group_hr_holidays_user

  2. Manager
    hr_payroll.group_hr_payroll_manager
    implies
    hr_holidays.group_hr_holidays_manager

hr_contract: Allow contract managers to manage employee

Contract managers should be able to create/manager employees.
This make sense when a contract manager hires someone.
He first creates the employee, then his contract.

This also allows contract managers to access private addresses
by transitivity.
This is also intended to allow a contract manager to set
a driver (private partner) on the employee's company car
or to access km_home_work for transport reimbursment
(l10n_be_hr_payroll).

hr_payroll: Improve benefits conflict performance

This commit changes the way benefits overlaps are detected.
Currently, all benefits are read from db and the
overlap detection is done in Python.

After this commit, use a single sql query to
only retreive overlapping benefits.
The query takes advantage of the postgresql
range type tsrange and the intersection operator

Performance improvements:
Tests with 500 employees and benefits generated for
15 months (~300000 records in the table)
Conflict detection over 1 month (~20000 records)
=> ~5X speedup (from more than 2s to less than 0.5s)

hr_holidays: Fix validation string typo

hr_holidays: Test access rights post install

Some module can inherit leave models and override some methods.
The overridden method could access some models which require other
access level.
In this case, the access issues would not be found by tests because
other modules are not yet installed.

e.g.
Recently, the _create_resource_leave was overridden in hr_payroll and
is reading the employee's contract. A HR user approving a leave
would raise an access error because he doesn't have contract rights.
If the tests would have been executed post install, this bug would
have been detected.

project_timesheet_holidays: Fix leave validation rights issues

Currently, when a leave is validated, an account.analytic.line
is automatically created. However, the approver may not have
the access rights to do so.

This commit fixes the issues by using sudo to generate the line.

hr_holiday: Don't check access rights on allocation if no employee

If there are no employee set on the allocation request and if the user
is a leave officer, don't check the access rights as it would lead to
a traceback.

hr_payroll: enable color edition on hr benefit types

TaskID: 1947281

hr_payroll: extract common behaviour from custom calendar view

In order to implement the same features both in the calendar and in the
gantt view, we need to extract the behaviours that will be common into a
mixin.

TaskID: 1947281

base: enable usage of js_class on the gantt view

TaskID: 1947281

hr_payroll: Use the structure type on contract instead of structure

For usability purposes, it's better to set a structure type
on a contract (eg: CP200: Belgian Employee) instead of having
to choose between 'Belgian Salary', 'Double holidays', ...

hr_holidays: Don't check approval rights in onchange

When this method is called in the context of an onchange
self.ids is an empty list.

This is a problem when calling holiday.check_access_rule('write')
because it does not find any valid holidays and thus raises an AccessError.

hr_holidays: Skip validation rights if no validation

If validation type is set to no_validation, checking
if a target state is achievalbe should bypass
normal checks for the user's own leaves.
Otherwise, ir.rule raises an AccessError as the user
doesn't normally have the rights for leave in
state validate.

l10n_be_hr_payroll: Manage employee departure

Compute notice duration and if the notice duration is not respected,
a report about termination fees is generated.
At the end of the notice duration, we can generate a certificate
about time off not taken and an other about time off for the next
year.

TaskID: 1914556

hr_payroll: replace contribution register by partner_id

We replace contribution register model by a many2one on res.partner
directly on the payslip.line

Task 1928607

hr_payroll: add multicompany rule for payslip lines

TaskID: 1928607

hr_payroll: Use date_to_str on work entries generation

Purpose

This aim to avoid sending a datetime that will be converted into UTC
and that way have a wrong date set on the payslip.

hr_payroll: Move to enterprise

For more information about this decision, please read
https://www.odoo.com/fr_FR/blog/notre-blog-5/post/odoo-community-enterprise-532

@robodoo robodoo added the CI 🤖 Robodoo has seen passing statuses label Mar 13, 2019
@C3POdoo C3POdoo added the RD research & development, internal work label Mar 13, 2019
@tivisse tivisse force-pushed the master-payroll-structure-yti branch from c1ecb07 to 815d9b1 Compare March 13, 2019 10:44
@robodoo robodoo removed the CI 🤖 Robodoo has seen passing statuses label Mar 13, 2019
@tivisse tivisse force-pushed the master-payroll-structure-yti branch 3 times, most recently from ff0ae4d to 43c5eec Compare March 13, 2019 13:17
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels Mar 13, 2019
@tivisse tivisse force-pushed the master-payroll-structure-yti branch from 9fab907 to f30bf7e Compare March 14, 2019 11:43
@robodoo robodoo removed the CI 🤖 Robodoo has seen passing statuses label Mar 14, 2019
@tivisse tivisse force-pushed the master-payroll-structure-yti branch from f30bf7e to 1e017a5 Compare March 14, 2019 11:49
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels Mar 14, 2019
@@ -486,6 +486,8 @@ def action_refuse(self):
def _check_approval_update(self, state):
""" Check if target state is achievable. """
current_employee = self.env['hr.employee'].search([('user_id', '=', self.env.uid)], limit=1)
if not current_employee:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I understand this anymore. Shouldn't it be:

if not self.mapped('employee_id'):
    return

to check if there is not employee set on the allocation?

@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels Mar 15, 2019
@tivisse tivisse force-pushed the master-payroll-structure-yti branch from ef832ae to 03989ba Compare March 18, 2019 16:33
@robodoo robodoo added the CI 🤖 Robodoo has seen passing statuses label Mar 18, 2019
tivisse and others added 4 commits March 19, 2019 09:24
…ization

1/ Remove the parent-children relation on salary structures

Currently it is difficult to see what rules are applied on salary structures.

Most of the structures are inheriting 3 rules (Basic, Gross, Net) from the
'Base for new structures', and those rules are not shown. Same issue if a
structure has another one as parent (as in the French localization).

This bring more annoying issues:

- Since the saas-12.1, we can already set different accounts for different
  companies on the salary rules for which we want to post journal entries.
  But for the parent rules, like the 'NET' amount, it's impossible to post
  the journal entries on different account for different salary structures.
  (eg: for CP200 employees and CP200 workers). As the rules are following
  a M2M relation on the structure, we have to remove the parent_id and to
  recreate all the rules in the final structure.
- If we are not at ease with the M2M relation (which is the case for most
  of the end users), we will simply modify the rule on the parent structure.
  This was already done on the French localization on which we modified the
  code, sequence and the name. This could have some desastrous consequences
  as this will break the order on which the rules are computed for other
  structures for the sequence, or break other integration with the payroll
  as we also modify the code, for example the salary configurator.

As this parent-children relation on structure could cause several issues
difficult to spot and as this is quite difficult to correctly visualize the
structure composition, it is better to remove it and define all the rules
inside the structure itself. This improves also the code readability as we
are not trying to recursively retrieve the rules and find a proper sequence
inside them.

2/ Make the rules on the salary structure a o2m instead of a m2m

The issues described just above indicates that it would be better to remove
the M2M relation between salary structures and salary rules, and make it
a O2M relation instead. That way, no more confusion and misconfiguration of
the rules fields (code, sequence, accounts, ...).

3/ Remove the parent-children relation on rules

A salary rule can have a parent salary rule. This mechanism, which is
not obvious at all for an end user allows to specify a parent rules.
If the condition is met for the parent rule (eg: the employee has at least
1 child), all the children rules could be applied (eg: Deduction of 90 euros
if the number of children is between 1 and 1).

Visually it doesn't make a lot of sense, as it displays one line with an
amount of 0 for the parent, and one line with the real amount for each
applied child. On the other hand, no child rule is linked to the structure
and an end user has to click on the parent rule, to see the children and
understand that something mystic and hidden is existing.

This is equivalent to define as many classic rules as we have children, or
even better, define 1 rule that will compute the real amount directly,
as the number of children can be accessed from the localdict when computing
the salary line.

This commit also removed the relation.

4/ Clean some brols in the code (technical)

The 3 first points of the spec allow us to clean a little the code readability
and complexity.

5/ Adapt all the localizations for these changes

All the localizations are impacted by those changes. What is done in this commit
is mainly:

- l10n_be_hr_payroll: Make 3 separated files for the 3 existing structures (the
  fourth one, belgian worker, is not correct and is removed). For each
  structure, define explicitely the rules (in the correct order by sequence).
  This multiplies the total number of rules as some of them were shared (like
  the withholding tax rule for example), but all the code that is supposed to be
  modified each year/quarter/whenever the government decides to modify the law
  has been moved into a python file, and is available the computation context.
  This will also allow us to update the computation rules without having to
  update the module.
- l10n_fr_hr_payroll: There were 3 rules following this scheme:
                              Base for new structure
                                        |
                                 Basic structure
                                        |
                              ----------------------
                              |                    |
                           Cadre                Non-Cadre
  All the rules parent rules have been duplicated into the 2 remaining
  structures, cadre and non-cadre. There were a lot of parent-children relations
  between the rules that have been adapted to the new model.
- l10n_in_hr_payroll: There was a lot of rules in data, and a structure in demo
  data, which was using 4 or 5 of these rules. As the rules doesn't seem to make
  a lot of sense altogether, everything was moved in demo data.

6/ Introduce a new Salary Structure Type model

Currently we have a model hr.contract.type with a M2O on the contracts. This has
never been used in 9 years and is removed in this commit. On the other hand, we
would like to define a new model hr.payroll.structure.type (eg: CP200 Employee),
which will line all the salary structures the Belgian localization could bring
(Classic salary, double holidays, 13th month, ...). This field is defined on the
structure, and is displayed on the contract (as the same place than the contract
type we just removed).

On this model we could also define some fields like:
- The default pay period: Selection field, default = monthly. This is applied on
  the contract when selecting the structure.
- The Working Schedule, m2o. This is applied on the contract when selecting the
  structure

For example, on the 'commission paritaire' for teachers, we could set the
default pay period to 'Every 15 days' and the working schedule to '19 hours /
week'. Every contract under this structure would take those values.

Migration
=========

For a migration point of view:
- All the rules that had a parent_rule_id should be set to
  appears_on_payslip=False. The column parent_rule_id can be dropped afterward.
- All the rules that are coming from a parent structure or higher should be
  duplicated and the field 'struct_id' should be set on the current structure.
  The column parent_id can be dropped afterward.
- All the rules that were defined on the structure should have the field
  'struct_id' defined on them.
- The rules that are not linked to a structure should be unlinked.
- The column type_id on the contract could be dropped, no need to keep the
  values too.
- A default structure type is defined in data ('Employee'). This could be set to
  all the existing structure for the newly created 'type_id' field.

TaskID: 1942832
It makes more sense to name an entry in the employee schedule like that.

TaskID: 1941598
Currently, group Team Leader does not imply group user.
However a team leader is always a user and should be member of
the corresponding group.
Currently, a Team leader can't approve a leave of an employee
in his team if the leave type requires an allocation.

The reason is the `ir.rule` for `base.group_user` restricting allocation
read access to only your own allocation (`[('employee_id.user_id', '=', user.id)]`)
There are no `ir.rule` for group Team leader.
Therefore, a team leader can't read allocations for his team members.

Fix: add an `ir.rule` to allow a team leader to read allocations
of employees in his team.

+ add associated tests
LucasLefevre and others added 12 commits March 19, 2019 09:35
Currently, when a leave is validated, an `account.analytic.line`
is automatically created. However, the approver may not have
the access rights to do so.

This commit fixes the issues by using sudo to generate the line.
If there are no employee set on the allocation request and if the user
is a leave officer, don't check the access rights as it would lead to
a traceback.
In order to implement the same features both in the calendar and in the
gantt view, we need to extract the behaviours that will be common into a
mixin.

TaskID: 1947281
…ture

For usability purposes, it's better to set a structure type
on a contract (eg: CP200: Belgian Employee) instead of having
to choose between 'Belgian Salary', 'Double holidays', ...
When this method is called in the context of an onchange
`self.ids` is an empty list.

This is a problem when calling `holiday.check_access_rule('write')`
because it does not find any valid holidays and thus raises an AccessError.
If validation type is set to `no_validation`, checking
if a target state is achievalbe should bypass
normal checks for the user's own leaves.
Otherwise, ir.rule raises an AccessError as the user
doesn't normally have the rights for leave in
state `validate`.
Compute notice duration and if the notice duration is not respected,
a report about termination fees is generated.
At the end of the notice duration, we can generate a certificate
about time off not taken and an other about time off for the next
year.

TaskID: 1914556
We replace contribution register model by a many2one on res.partner
directly on the payslip.line

Task 1928607
Purpose
=======

This aim to avoid sending a datetime that will be converted into UTC
and that way have a wrong date set on the payslip.
@tivisse tivisse changed the title Remove parent_id on structures, make rules a O2M [MERGE] hr_payroll: Refactor payroll engine + Move to enterprise Mar 19, 2019
@tivisse tivisse force-pushed the master-payroll-structure-yti branch from 03989ba to 7794472 Compare March 19, 2019 09:54
@robodoo robodoo removed the CI 🤖 Robodoo has seen passing statuses label Mar 19, 2019
@tivisse
Copy link
Contributor Author

tivisse commented Mar 19, 2019

@robodoo r+ merge

@robodoo
Copy link
Contributor

robodoo commented Mar 19, 2019

Merge method set to merge directly, using the PR as merge commit message

@robodoo robodoo added the CI 🤖 Robodoo has seen passing statuses label Mar 19, 2019
@robodoo
Copy link
Contributor

robodoo commented Mar 19, 2019

Linked pull request(s) odoo/enterprise#3823 not ready. Linked PRs are not staged until all of them are ready.

robodoo added a commit that referenced this pull request Mar 19, 2019
hr_payroll: Remove parent_id hierarchy + Add structures categorization
----------------------------------------------------------------------

1/ Remove the parent-children relation on salary structures

Currently it is difficult to see what rules are applied on salary structures.

Most of the structures are inheriting 3 rules (Basic, Gross, Net) from the
'Base for new structures', and those rules are not shown. Same issue if a
structure has another one as parent (as in the French localization).

This bring more annoying issues:

- Since the saas-12.1, we can already set different accounts for different
  companies on the salary rules for which we want to post journal entries.
  But for the parent rules, like the 'NET' amount, it's impossible to post
  the journal entries on different account for different salary structures.
  (eg: for CP200 employees and CP200 workers). As the rules are following
  a M2M relation on the structure, we have to remove the parent_id and to
  recreate all the rules in the final structure.
- If we are not at ease with the M2M relation (which is the case for most
  of the end users), we will simply modify the rule on the parent structure.
  This was already done on the French localization on which we modified the
  code, sequence and the name. This could have some desastrous consequences
  as this will break the order on which the rules are computed for other
  structures for the sequence, or break other integration with the payroll
  as we also modify the code, for example the salary configurator.

As this parent-children relation on structure could cause several issues
difficult to spot and as this is quite difficult to correctly visualize the
structure composition, it is better to remove it and define all the rules
inside the structure itself. This improves also the code readability as we
are not trying to recursively retrieve the rules and find a proper sequence
inside them.

2/ Make the rules on the salary structure a o2m instead of a m2m

The issues described just above indicates that it would be better to remove
the M2M relation between salary structures and salary rules, and make it
a O2M relation instead. That way, no more confusion and misconfiguration of
the rules fields (code, sequence, accounts, ...).

3/ Remove the parent-children relation on rules

A salary rule can have a parent salary rule. This mechanism, which is
not obvious at all for an end user allows to specify a parent rules.
If the condition is met for the parent rule (eg: the employee has at least
1 child), all the children rules could be applied (eg: Deduction of 90 euros
if the number of children is between 1 and 1).

Visually it doesn't make a lot of sense, as it displays one line with an
amount of 0 for the parent, and one line with the real amount for each
applied child. On the other hand, no child rule is linked to the structure
and an end user has to click on the parent rule, to see the children and
understand that something mystic and hidden is existing.

This is equivalent to define as many classic rules as we have children, or
even better, define 1 rule that will compute the real amount directly,
as the number of children can be accessed from the localdict when computing
the salary line.

This commit also removed the relation.

4/ Clean some brols in the code (technical)

The 3 first points of the spec allow us to clean a little the code readability
and complexity.

5/ Adapt all the localizations for these changes

All the localizations are impacted by those changes. What is done in this commit
is mainly:

- l10n_be_hr_payroll: Make 3 separated files for the 3 existing structures (the
  fourth one, belgian worker, is not correct and is removed). For each
  structure, define explicitely the rules (in the correct order by sequence).
  This multiplies the total number of rules as some of them were shared (like
  the withholding tax rule for example), but all the code that is supposed to be
  modified each year/quarter/whenever the government decides to modify the law
  has been moved into a python file, and is available the computation context.
  This will also allow us to update the computation rules without having to
  update the module.
- l10n_fr_hr_payroll: There were 3 rules following this scheme:
```
                              Base for new structure
                                        |
                                 Basic structure
                                        |
                              ----------------------
                              |                    |
                           Cadre                Non-Cadre
```
All the rules parent rules have been duplicated into the 2 remaining
  structures, cadre and non-cadre. There were a lot of parent-children relations
  between the rules that have been adapted to the new model.
- l10n_in_hr_payroll: There was a lot of rules in data, and a structure in demo
  data, which was using 4 or 5 of these rules. As the rules doesn't seem to make
  a lot of sense altogether, everything was moved in demo data.

6/ Introduce a new Salary Structure Type model

Currently we have a model hr.contract.type with a M2O on the contracts. This has
never been used in 9 years and is removed in this commit. On the other hand, we
would like to define a new model hr.payroll.structure.type (eg: CP200 Employee),
which will line all the salary structures the Belgian localization could bring
(Classic salary, double holidays, 13th month, ...). This field is defined on the
structure, and is displayed on the contract (as the same place than the contract
type we just removed).

On this model we could also define some fields like:
- The default pay period: Selection field, default = monthly. This is applied on
  the contract when selecting the structure.
- The Working Schedule, m2o. This is applied on the contract when selecting the
  structure

For example, on the 'commission paritaire' for teachers, we could set the
default pay period to 'Every 15 days' and the working schedule to '19 hours /
week'. Every contract under this structure would take those values.

Migration
=========

For a migration point of view:
- All the rules that had a parent_rule_id should be set to
  appears_on_payslip=False. The column parent_rule_id can be dropped afterward.
- All the rules that are coming from a parent structure or higher should be
  duplicated and the field 'struct_id' should be set on the current structure.
  The column parent_id can be dropped afterward.
- All the rules that were defined on the structure should have the field
  'struct_id' defined on them.
- The rules that are not linked to a structure should be unlinked.
- The column type_id on the contract could be dropped, no need to keep the
  values too.
- A default structure type is defined in data ('Employee'). This could be set to
  all the existing structure for the newly created 'type_id' field.

TaskID: 1942832

hr_payroll: rename hr.benefit into hr.work.entry
------------------------------------------------

It makes more sense to name an entry in the employee schedule like that.

TaskID: 1941598

hr_holidays: Group team leader imply group user
-----------------------------------------------

Currently, group Team Leader does not imply group user.
However a team leader is always a user and should be member of
the corresponding group.

hr_holidays: Allow Team leader to approve
-----------------------------------------

Currently, a Team leader can't approve a leave of an employee
in his team if the leave type requires an allocation.

The reason is the `ir.rule` for `base.group_user` restricting allocation
read access to only your own allocation
(`[('employee_id.user_id', '=', user.id)]`). There are no `ir.rule` for group
Team leader. Therefore, a team leader can't read allocations for his team
members.

Fix: add an `ir.rule` to allow a team leader to read allocations
of employees in his team.

+ add associated tests

hr_payroll: Fix contract access to approve leave
------------------------------------------------

When a `hr.leave` is approved, a `resource.calendar.leave`
is created in the employee's calendar and in the
contract's calendar if it's different.
However, a user in `group_holiday_user` but not in
`group_hr_contract_manager` does not have access rights
to read a contract.
This leads to an `AccessError`

Fix: use `sudo` to read the contract.

hr_contract: group_hr_contract_manager implies group_user
---------------------------------------------------------

A user with only the contract access rights should be able to create
a contract.

l10n_be_hr_payroll_fleet: Fix access rights issues
--------------------------------------------------

If a user is member of `hr_contract.group_hr_contract_manager`
but not member of `fleet.fleet_group_manager` some access issues arise
The issues are related to the `car_id` field of `hr.contract`
because the user has no access rights for `fleet.vehicle`.

Fix to solve the issue:
Make all car related field of contracts available
to fleet manager only.

If the contract manager want to be able to manage cars on
contracts, he must manually be granted fleet manager access.
This is a requirement if the contract manager want to use the
company car feature.

hr_payroll: Allow a contract manager to read salary structures
--------------------------------------------------------------

An employee with only the contract user group should be able to create
a contract. It implies to be able to read a salary structure and
a salary structure type.

hr_payroll: Add leave type One2many on benefit type
---------------------------------------------------

Purpose
=======

Currently, linking a benefit type and a leave type is done through
the leave type form view.
This is anoying because it requires to first create the benfit type,
then go to the leave type and link it.
Moreover, linking is the role of a payroll manager which should be
able to do it from the Payroll app.

Specification
=============

Add a One2many field `leave_type_ids` on `hr.benefit.type` to
allow to link a leave type when creating a benefit type.

commit a9f97a2
Author: Lucas Lefèvre <lul@odoo.com>
Date:   Mon Mar 4 16:06:33 2019 +0100

hr_payroll: Make payroll groups imply holiday groups
----------------------------------------------------

Purpose
=======

Currently, groups `hr_holidays.group_hr_holidays_[user|manager]`
and `hr_payroll.group_hr_payroll_[user|manager]` are independant.

A member of payroll groups is not able to approve/refuse a leave.

This is a problem as they should be able to approve/refuse leaves before generating payslips.
This is the  point of `hr.benefits`: a global way of managing
attendances and leaves before generating payslips.

Specification
=============

Payroll groups should imply the corresponding
holiday group.

1. User
`hr_payroll.group_hr_payroll_user`
implies
`hr_holidays.group_hr_holidays_user`

2. Manager
`hr_payroll.group_hr_payroll_manager`
implies
`hr_holidays.group_hr_holidays_manager`

hr_contract: Allow contract managers to manage employee
-------------------------------------------------------

Contract managers should be able to create/manager employees.
This make sense when a contract manager hires someone.
He first creates the employee, then his contract.

This also allows contract managers to access private addresses
by transitivity.
This is also intended to allow a contract manager to set
a driver (private  partner) on the employee's company car
or to access `km_home_work` for transport reimbursment
(l10n_be_hr_payroll).

hr_payroll: Improve benefits conflict performance
-------------------------------------------------

This commit changes the way benefits overlaps are detected.
Currently, all benefits are read from db and the
overlap detection is done in Python.

After this commit, use a single sql query to
only retreive overlapping benefits.
The query takes advantage of the postgresql
range type `tsrange` and the intersection operator

Performance improvements:
Tests with 500 employees and benefits generated for
15 months (~300000 records in the table)
Conflict detection over 1 month (~20000 records)
=> ~5X speedup (from more than 2s to less than 0.5s)

hr_holidays: Fix validation string typo
---------------------------------------

hr_holidays: Test access rights post install
--------------------------------------------

Some module can inherit leave models and override some methods.
The overridden method could access some models which require other
access level.
In this case, the access issues would not be found by tests because
other modules are not yet installed.

e.g.
Recently, the `_create_resource_leave` was overridden in hr_payroll and
is reading the employee's contract. A HR user approving a leave
would raise an access error because he doesn't have contract rights.
If the tests would have been executed post install, this bug would
have been detected.

project_timesheet_holidays: Fix leave validation rights issues
--------------------------------------------------------------

Currently, when a leave is validated, an `account.analytic.line`
is automatically created. However, the approver may not have
the access rights to do so.

This commit fixes the issues by using sudo to generate the line.

hr_holiday: Don't check access rights on allocation if no employee
------------------------------------------------------------------

If there are no employee set on the allocation request and if the user
is a leave officer, don't check the access rights as it would lead to
a traceback.

hr_payroll: enable color edition on hr benefit types
----------------------------------------------------

TaskID: 1947281

hr_payroll: extract common behaviour from custom calendar view
--------------------------------------------------------------

In order to implement the same features both in the calendar and in the
gantt view, we need to extract the behaviours that will be common into a
mixin.

TaskID: 1947281

base: enable usage of js_class on the gantt view
------------------------------------------------

TaskID: 1947281

hr_payroll: Use the structure type on contract instead of structure
-------------------------------------------------------------------

For usability purposes, it's better to set a structure type
on a contract (eg: CP200: Belgian Employee) instead of having
to choose between 'Belgian Salary', 'Double holidays', ...

hr_holidays: Don't check approval rights in onchange
----------------------------------------------------

When this method is called in the context of an onchange
`self.ids` is an empty list.

This is a problem when calling `holiday.check_access_rule('write')`
because it does not find any valid holidays and thus raises an AccessError.

hr_holidays: Skip validation rights if no validation
----------------------------------------------------

If validation type is set to `no_validation`, checking
if a target state is achievalbe should bypass
normal checks for the user's own leaves.
Otherwise, ir.rule raises an AccessError as the user
doesn't normally have the rights for leave in
state `validate`.

l10n_be_hr_payroll: Manage employee departure
---------------------------------------------

Compute notice duration and if the notice duration is not respected,
a report about termination fees is generated.
At the end of the notice duration, we can generate a certificate
about time off not taken and an other about time off for the next
year.

TaskID: 1914556

hr_payroll: replace contribution register by partner_id
-------------------------------------------------------

We replace contribution register model by a many2one on res.partner
directly on the payslip.line

Task 1928607

hr_payroll: add multicompany rule for payslip lines
---------------------------------------------------

TaskID: 1928607

hr_payroll: Use date_to_str on work entries generation
------------------------------------------------------

Purpose
=======

This aim to avoid sending a datetime that will be converted into UTC
and that way have a wrong date set on the payslip.

hr_payroll: Move to enterprise
------------------------------

For more information about this decision, please read
https://www.odoo.com/fr_FR/blog/notre-blog-5/post/odoo-community-enterprise-532

closes #31806

Signed-off-by: Yannick Tivisse (yti) <yti@odoo.com>
@robodoo
Copy link
Contributor

robodoo commented Mar 19, 2019

Merged, thanks!

@robodoo robodoo closed this Mar 19, 2019
@robodoo robodoo merged commit 7794472 into odoo:master Mar 19, 2019
@tivisse tivisse deleted the master-payroll-structure-yti branch March 19, 2019 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI 🤖 Robodoo has seen passing statuses RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants