14.0-major-rule-parameters-function

[nimarosa]

Hello, this PR introduces the concept of "Salary Rule Parameters" which is a model that allows the user to create parameters that can be called inside salary rules to get useful values for the calculation for a salary rule.

The concept is that the rule parameters can have "versions" which means that you can have more than one version of the parameter depending of the date in the payslip.

This is useful in localization where you have to get charging params that are not the same depending of the date, and also can be updated in regular basis by the localization developers.

This function adds the whole rule_parameters model and views and also the method payslip.rule_parameter(code) that can be used inside the salary rules to get the correct value and version.

TODO:

1. It will require translation to other languages, i hope the bot updates the translation files for that.
2. We can extend the functionality to allow rule_parameters to be populated by importing actions.

Hope you find this useful for a merge, any doubts you can reach me here.

PD: This PR also adds some UI tweaks in the views of hr_payslip, in order to order and make the views more user-readable. I will be adding more UI tweaks this month in other PRs.

[OCA-git-bot]

Hi @appstogrow,
some modules you are maintaining are being modified, check this out!

[ghost]

This PR is very useful 👍

I would delete the hr_rule_parameter_value_view_form and have <tree editable="bottom"> in the hr_rule_parameter_view_form.

I also would like to let different companies use the same code for different purposes.
PYTHON:
(hr.rule.parameter.value)
company_id = fields.Many2one(related="rule_parameter_id.company_id")
(hr.rule.parameter)
company_id = fields.Many2one('res.company', "Company", required=True, default=lambda self: self.env.company)
[("code", "=", code), ("date_from", "<=", date), ("company_id", "=", self.env.company.id)], limit=1
("_unique", "unique (code, company_id)", "Two rule parameters cannot have the same code."),
VIEWS
After "code" in form & tree views of hr.rule.parameter:
<field name="company_id" required="1" groups="base.group_multi_company" />

What do you think?

[nimarosa]

@appstogrow Great to hear you like the PR.

I think there are great suggestions. Actually I don't know why i used a form and not a tree editable. I will make the changes this afternoon and commit.

[ghost]

This feature of a "parameter" and time-dependent "value" is a generic feature.
Accounting tax rates sometimes change, and a time-dependent tax "value" would be very useful.
Therefore I suggest to make a new module with the parameter/value models, and let both payroll and a tax-module depend on it.
Actually I already made a module called base_field_value.
If we add date_from here, maybe payroll could depend on base_field_value?

The comments below are off topic. If you find them interesting, maybe we should create an issue to discuss them.

My Norwegian payroll localization l10n_no_payroll depends on hr_field_value which depends on base_field_value to have many "fields" without creating them as database columns.
I use it mainly for payroll reporting ("a-melding") to the tax authority.
Previously I also used it a lot in salary rules. A use case is a field for meals which may be used in python this way:
no_of_meals = float(payslip.field_value_ids.filtered(lambda x: x.field_code == "meals").value)

Now most of my salary rules depend on hr_rule_qty_rate_amount. Example python code:
amount = payslip.qty_rate_amount_ids.filtered(lambda x: x.salary_rule_id.id == rule.id).amount
On the Salary rule, select Salary rule input from Payslip.
On the Payslip, register a Salary Rule line with the quantity, rate and amount you want to have on the payslip line after you click Compute Sheet.

[nimarosa]

@appstogrow

base_field_value id a great module. I've used it a time ago.

I think the problem with using that module in payroll will be that you would have to define parameters outside payroll views and sometimes this confuses the user.

In my Argentinian payroll localization this PR is used to define tax rates which might change and also labor union specific parameters that changes all the time like 6 to 10 times a year. It would be more convenient to have this feature directly in the payroll module so we don't have to add external oca dependencies which the user might not want in his system.

It's a great idea to use it for reporting and I hope we can discuss it in the future.

The main idea of this new model is to consume parameters in a easy way, that's why in the payslip the user only has to use the code:
payslip.rule_parameter(CODE) to get the value, and the function uses the payslip date to choose between the values revisions. I also think it's important to have this values in the database because, at least in Argentina, it's very common to need the different dates values to make calculations on some rules, not only the actual one.

Also like you said base_field_value don't have the from_date so I think we will need to change it if we use it.

Finally this PR is a first version of this functionality I plan to add support for more complex situations like using this parameters in categories to allow changing their calculation formula from the ui. Also caching this params in memory in payslip creation to make it most fast (I should be fast right now but caching is sometimes good because there are people that makes 100-200 payslips at once with payslip run)

I think that for this and for the user comprehension makes more sense to have the functionality native in the payroll module rather than in other module with dependencies.

[nimarosa]

@appstogrow also I forgot to say that the way we ship our payroll localization is that we create data xml files that adds all salary rules, categories, registers and parameters, also leave types.

The localization module leaves the salary structures ready for each labor union in my country so the user don't have to enter practically any data in payroll, only in contracts and employees and the localization updates constantly with new values and rates that change in regulations or labor union "prices".

So for that is useful to store this in the database this way we can ship the parameters in data files and don't need to create it manually.

My approach to this module is to make it the more automatic as possible if using a localization (like Argentinian) so the user practically don't need to create a lot of rules and logic, we ship the module with all this updated data (in Argentina updates to salary rules happen all the months). So the user only need to worry for updating the localization module.

That's other reason I think I must be useful to have this values in his own table in db despite the general approach of your other module.

[ghost]

@nimarosa thank you for sharing all of this.
I like payslip.rule_parameter(CODE)
Caching is also a good idea for performance.

I just wonder if hr_rule_parameter.py with the models hr.rule.parameter and hr.rule.parameter.value could be in a separate module. Then I could re-use this logic in accounting. Maybe hr.rule.parameter could have an extra field:
module = fields.Selection([('payroll', 'Payroll'), ('account', 'Accounting')], string='Module')
And maybe the model names could be more generic, e.g. res.parameter and res.parameter.value
All the logic could be in the payroll module.

Would it be more difficult to ship your localization if these two models would belong to separate module?

[nimarosa]

@appstogrow Okay i think is a good idea, but will require creating another general module like you said. In which OCA repo do you think it would fit the separate module?

Then we can depend on the module here and adapt the logic to work with payroll, so the module could be used in another modules that requires time-depending parameter values.

I will have to create the new module and submit it to other OCA repo and then make the changes here. Would you like to help?

Also if you have more suggestions about it we can make a more extendful module for the parameters. Also i think i would call the new module base_time_dependant_parameter or something like that. What do you think?

Also if you could tell me more of the functionality of the accounting module where you use this behavior? Because i'm struggling with the payroll accounting module and thinking rewriting all the logic, i think we can work in that too.

[ghost]

I suggest the server-tools repo.
I suggest "dependent" (US spelling) instead of "dependant" (UK spelling), module name base_time_dependent_parameter.
Both models/hr_rule_parameter.py and views/hr_rule_parameter_views.xml should be in this generic module.
The menuitem could have parent="base.menu_ir_property" (Settings/Technical/Parameters).

class Base(models.AbstractModel):
    _inherit = 'base'

    def get_time_dependent_value(self, code):
        return self.env["base.time.dependent.parameter"]._get_value_from_code(code, self.dict.date_to)

_inherit = 'base' will make the method available to all models.
Then I can use it for taxes right away with no extra development:

1. Configure a parameter 'vat_high_rate'
2. Install account_tax_python module
3. Write tax code like value = company.get_time_dependent_value('vat_high_rate')

I have also been struggling with the payroll_account module. I have rewritten all the logic for hr.payslip method test_payslip_accounting() to set correct analytic account and Norwegian payroll taxes (which I will try to replace with your payroll tax implementation).

[nimarosa]

@appstogrow Just to let you know that i will work in this this weekend. I was a bit busy last one. If you have more suggestions please let me know.

[nimarosa]

Hello @appstogrow could you check my branch of server-tools and let me know what you think? Also if you want you can PR to it if you think we can make more changes to the new module. Any suggestion is welcome.

https://github.com/nimarosa/server-tools/tree/14.0-add-base_time_dependent_parameter/base_time_dependent_parameter

[nimarosa]

Then when we are ready i will make the PR to the OCA server-tools repo and then when they merge it, i will update this PR with the changes to implement this module here in Payroll.

Please don't close the PR because i'm actually using it in production and i'm consuming the merge of this PR currently.

[ghost]

@nimarosa I have made a PR to your branch.

[nimarosa]

@appstogrow PR merged and waiting for finishing some more tweaks. When i successfully merge the new module in server-tooks repo, i will edit this one with the dependency and implementation of the new module for payroll.

[nimarosa]

@appstogrow Will try to work in this PR this weekend. The idea is to first make the PR to the other repo, and then adapt this implementation to use the new module base_time_dependent_parameter.
Any thoughts or ideas that you want to add?

[ghost]

Good, go ahead! 👍 I have nothing to add right now.

[mtelahun]

Hello all,
I can see how this functionality can be useful, but this isn't the only possible solution either. For example, I've solved it in a different way in our repo: https://github.com/trevi-software/trevi-hr. Rather than have this in the payroll module I would prefer to see it in a separate module of its own . That way anyone who wants to use the functionality can install the module and anyone who doesn't need it can leave it uninstalled.

[nimarosa]

@mtelahun Can you share how you solved this in your repo? I can find a module that does the same that this PR.

On the other hand, the idea is to merge this functionality in server-tools so it's available to other modules too, and the payroll will implement that module.

[ghost]

@mtelahun Wow, you have made a lot of payroll modules! Which module has "solved it in a different way"?

[nimarosa]

Hi @appstogrow, did you finally used the time parameters implementation in account? I would like to see it if you have it so I can finish the PR in server-tools and change this implementation. I didn't have much time to finish that so I think I will this week.

[mtelahun]

The payroll_policy_*, hr_benefit* modules are the ones that come to mind. They allow to configure many things related to how attendance, leave, OT, etc should be handled and then payroll_policy_payslip handles the calculations according to the configured policies. The hr_benefit & hr_benefit_payroll is probably more similar to contract advantages feature.

…

On 20/06/2022 13:18, Henrik Norlin wrote: @mtelahun <https://github.com/mtelahun> Wow, you have made a lot of payroll modules! Which module has "solved it in a different way"? — Reply to this email directly, view it on GitHub <#31 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB4YOHFNUDIQJ4HSKUQQQJDVQBANZANCNFSM5SXHNYZQ>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[nimarosa]

@mtelahun Okay i see. This PR introduced time dependent parameters so it's no the same of contract advantages (there is two different PRs).

This function allow to have parameters that change depending of the date of the payslip. So it's useful for saving data that changes depending of the month of the payslip.

[ghost]

Hi @appstogrow, did you finally used the time parameters implementation in account? I would like to see it if you have it so I can finish the PR in server-tools and change this implementation. I didn't have much time to finish that so I think I will this week.

Go ahead, I am happy with the base_time_dependent_parameter.

My desired use case was for account.tax, but it has no awareness of time. So a consistent implementation would have to change every method calling account.tax. I made a patch to pass tax_date from a few methods, and it works. Not sure if I will use it in production.

[nimarosa]

@appstogrow Please check OCA/server-tools#2364 i made another PR because it become quite old and it was a mess to squash commits with new repo changes.

If you have other suggestions you can do it in my branch so we can update the PR.

[nimarosa]

Hello @pedrobaeza. As discussed in OCA/server-tools#2364, i reviewed this PR and it's ready to merge here in payroll.

On the server-tools repo, i will change your suggestions and let you know, so we can get this merged here for payroll and leave the functionality in server-tools for anyone who wants to use it with another module.

[pedrobaeza]

@appstogrow please confirm it's OK

[ghost]

I suggested a couple changes to OCA/server-tools#2364. Let's finish and merge that PR.

@nimarosa Would you like to make a new PR with the changes in hr_payslip_views.xml?

Then we can close this PR without merging.

[nimarosa]

Hello @appstogrow , did you read the comments of @pedrobaeza in the server-tools PR?

He said that he prefer to merge this here to not add new dependencies to payroll.

Anyways I will finish the PR of server tools and get it merged so you and anyone can use the functionality.

But to have that in payroll (which I use in my localization) we have to merge this PR here.

[nimarosa]

I merged the branch to 14.0 and also removed the payslip_views changes i've made, because i'm including them in #47.

As i noted in my last message, @appstogrow i will finish the changes in server-tools repo but also with @pedrobaeza we agreed in merging this changes directly in payroll to not mess with dependencies for the current users of the module.

So the idea is to merge this here and in future versions we can replace the functionality by inheriting the new module in server-tools. Keep posted will be submitting changes there soon.

[nimarosa]

Someone has any input in how to remove all the commits messages that have been added by the merge in my branch? Or it doesn't matter? The files changed are okay but now i have here all the commit messages.

[pedrobaeza]

The problem is that you have merged instead of rebasing. You must rebase your PR over the latest fetched branch.

[nimarosa]

@pedrobaeza Okay it's done

[nimarosa]

@pedrobaeza everything is approved can we merge?

[pedrobaeza]

/ocabot merge major

[OCA-git-bot]

This PR looks fantastic, let's merge it!
Prepared branch 14.0-ocabot-merge-pr-31-by-pedrobaeza-bump-major, awaiting test results.

[OCA-git-bot]

Congratulations, your PR was merged at e85887a. Thanks a lot for contributing to OCA. ❤️

[mtelahun]

@nimarosa The hr_benefit* and payroll_policy_* modules. My goal was to solve this by providing a generic mechanism to allow the user to update policies on their own (without any programmer involvement) as regulatory environment changed or to allow me to just ship updated xml data modules for my clients.

…

On 19/06/2022 22:51, Nicolas Rodriguez Sande wrote: @mtelahun <https://github.com/mtelahun> Can you share how you solved this in your repo? I can find a module that does the same that this PR. On the other hand, the idea is to merge this functionality in server-tools so it's available to other modules too, and the payroll will implement that module. — Reply to this email directly, view it on GitHub <#31 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB4YOHBXUKTIWKKZGI5JZPDVP523FANCNFSM5SXHNYZQ>. You are receiving this because you were mentioned.Message ID: ***@***.***>