[15.0][MIG] payroll #81

mtelahun · 2022-10-13T11:43:18Z

Fixes: #19
cc: #78

mtelahun · 2022-10-13T11:58:52Z

It looks like there's a build problem. Probably a stale config somewhere. Can someone take a look please?
@nimarosa @pedrobaeza

mtelahun · 2022-10-13T11:59:48Z

I mean the build failures. I'll take care of the pre-commit failures.

mtelahun · 2022-10-13T12:04:41Z

@nimarosa @pedrobaeza Sorry. It was my own fault for not checking the migration went properly.

nimarosa · 2022-10-13T12:20:19Z

@mtelahun i think contract views changed a bit in v15.0 that's why build error is about a xpath not found.

nimarosa · 2022-10-13T12:25:14Z

/ocabot migration payroll

nimarosa · 2022-10-13T12:57:44Z

@mtelahun Please before finishing this do a rebase with 15.0 version since i pushed updated dotfiles to have v15.0 up to date.

norlinhenrik · 2022-10-13T21:15:14Z

Can we replace hr.rule.parameter & hr.rule.parameter.value with a dependency on base_time_parameter?

nimarosa · 2022-10-13T21:59:02Z

@norlinhenrik Hello Henrik, I think adding a dependency to a server-tools module will be bad for payroll.

Most people don't download all oca repos in his instances so it will end up with an dependency error when trying to install payroll and not having the other module. I think it's good to maintain the less dependencies required here.

Also, and this one is more personal, the way I see it the user who uses rule parameters expect to have the view inside payroll module. Doing this with an inherited module will be a little tricky because we need to filter all views by domain and add again the module field in base_time_parameter and think of a way to integrate it in payroll the way it's done now.

In my instances I use time parameter to store specific payroll data that has to be easily changed by the user and also have some custom implementations around this model and no time to migrate them now to a new behavior or module.

If you have a good way of doing this by leaving the current functionality intact you can propose a PR on the 14.0 branch and it will be migrated to 15.

I'm not a fan of introducing big changes in migrations because it causes confusion and errors and will requiere migration scripts, that's why I wanted to do the big changes in 14.0 so we make modules maintenance more easy and straightforward.

Hope you understand and feel free to suggest a PR if you can do it without affecting current functionality.

mtelahun · 2022-10-14T07:24:03Z

@nimarosa All is green now. Sorry for the noise. This was the first migration I did in a long time.

During my migration I noticed that Odoo has removed the code field from hr.leave.type. So, I have reverted the payroll module to using the name instead.
Maybe, starting from 15.0 onward we need a new model to map hr.leave.type to a code in worked days. What do you think?

mtelahun · 2022-10-14T07:29:11Z

Can we replace hr.rule.parameter & hr.rule.parameter.value with a dependency on base_time_parameter?

I think this is a separate issue and belongs in a different PR

nimarosa · 2022-10-14T09:52:45Z

During my migration I noticed that Odoo has removed the code field from hr.leave.type. So, I have reverted the payroll module to using the name instead.

Maybe, starting from 15.0 onward we need a new model to map hr.leave.type to a code in worked days. What do you think?

Hello @mtelahun. I hate when odoo changes things like this, but I think is has to be because enterprise payroll module didn't use that code because enterprise module is different (our module is better un my opinion).

What do you think about inheriting hr.leave.type here in the payroll module and adding this field again? That way we can maintain current functionality that is key for the module, at least for me. I think using name in salary rules will generate problems.

If you think that adding this field to leave type will fix it we can include it in this PR.

nimarosa · 2022-10-14T09:59:36Z

@mtelahun also noticed you removed all string="" of a lot of fields. Why is that?

mtelahun · 2022-10-14T12:04:51Z

Yes, that's a better solution. I'll do that.

…

On 14/10/2022 12:52, Nicolas Rodriguez Sande wrote: During my migration I noticed that Odoo has removed the |code| field from hr.leave.type. So, I have reverted the payroll module to using the |name| instead. Maybe, starting from 15.0 onward we need a new model to map hr.leave.type to a code in worked days. What do you think? Hello @mtelahun <https://github.com/mtelahun>. I hate when odoo changes things like this, but I think is has to be because enterprise payroll module didn't use that code because enterprise module is different (our module is better un my opinion). What do you think about inheriting hr.leave.type here in the payroll module and adding this field again? That way we can maintain current functionality that is key for the module, at least for me. I think using name in salary rules will generate problems. If you think that adding this field to leave type will fix it we can include it in this PR. — Reply to this email directly, view it on GitHub <#81 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB4YOHGXVHIW4Y7XH3RXG4TWDEUPTANCNFSM6AAAAAAREGB4YU>. You are receiving this because you were mentioned.Message ID: ***@***.***>

mtelahun · 2022-10-14T12:08:29Z

pre-commit errors: - string attribute on `<tree>` view is no longer allowed - in model creation string attribute on a field that is the same as the default string provided by the ORM are no longer allowed

…

On 14/10/2022 12:59, Nicolas Rodriguez Sande wrote: @mtelahun <https://github.com/mtelahun> also noticed you removed all string="" of a lot of fields. Why is that? — Reply to this email directly, view it on GitHub <#81 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB4YOHBLGVQPY4GRGJ4X37TWDEVJFANCNFSM6AAAAAAREGB4YU>. You are receiving this because you were mentioned.Message ID: ***@***.***>

nimarosa · 2022-10-14T12:28:24Z

@mtelahun yes but you are removing string in model creation not on tree view. Correct me maybe I didn't understand

mtelahun · 2022-10-14T12:38:42Z

Please re-read my reply. There are two places where the `string=` attribute raises errors. 1. in `<tree>` view 2. in model creation when the `string=` attribute is the same as the default one provided by the ORM

…

On 14/10/2022 15:28, Nicolas Rodriguez Sande wrote: @mtelahun <https://github.com/mtelahun> yes but you are removing string in model creation not on tree view. Correct me maybe I didn't understand — Reply to this email directly, view it on GitHub <#81 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB4YOHHRVG6D64O6WXBI7E3WDFGXHANCNFSM6AAAAAAREGB4YU>. You are receiving this because you were mentioned.Message ID: ***@***.***>

…egory

Currently translated at 100.0% (359 of 359 strings) Translation: payroll-14.0/payroll-14.0-payroll Translate-URL: https://translation.odoo-community.org/projects/payroll-14-0/payroll-14-0-payroll/es_AR/

mtelahun · 2022-11-10T14:01:58Z

@nimarosa @apps2grow Please review and let's merge. This incorporates the latest changes.

nimarosa · 2022-11-10T17:01:25Z

@mtelahun for me it's okay. Just done a code review and commit history looks okay.

Did you tested it yet? Like functional.

norlinhenrik

I successfully computed a payslip for Marc Demo.

OCA-git-bot · 2022-11-10T21:00:02Z

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

nimarosa · 2022-11-10T22:31:47Z

Okay let's go.

I have a few PRs that I might add to 14.0 but we can migrate them later.

/ocabot merge nobump

OCA-git-bot · 2022-11-10T22:32:21Z

This PR looks fantastic, let's merge it!
Prepared branch 15.0-ocabot-merge-pr-81-by-nimarosa-bump-nobump, awaiting test results.

OCA-git-bot · 2022-11-10T22:37:09Z

@nimarosa The merge process could not be finalized, because command git push origin 15.0-ocabot-merge-pr-81-by-nimarosa-bump-nobump:15.0 failed with output:

To https://github.com/OCA/payroll
 ! [rejected]        15.0-ocabot-merge-pr-81-by-nimarosa-bump-nobump -> 15.0 (fetch first)
error: failed to push some refs to 'https://***@github.com/OCA/payroll'
hint: Updates were rejected because the remote contains work that you do
hint: not have locally. This is usually caused by another repository pushing
hint: to the same ref. You may want to first integrate the remote changes
hint: (e.g., 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.

OCA-git-bot · 2022-11-10T22:37:13Z

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

mtelahun · 2022-11-11T07:32:59Z

@nimarosa Yes, I tested it. Thanks for merging. Mike.

…

On 11/11/2022 01:31, Nicolas Rodriguez Sande wrote: Okay let's go. I have a few PRs that I might add to 14.0 but we can migrate them later. /ocabot merge nobump — Reply to this email directly, view it on GitHub <#81 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB4YOHCMPD2LPCSOLNZUJR3WHVZV7ANCNFSM6AAAAAAREGB4YU>. You are receiving this because you were mentioned.Message ID: ***@***.***>

mtelahun force-pushed the 15.0-mig-payroll branch from b48ecfa to 6405d3e Compare October 13, 2022 12:13

nimarosa added this to the 15.0 milestone Oct 13, 2022

nimarosa added the migration label Oct 13, 2022

OCA-git-bot mentioned this pull request Oct 13, 2022

Migration to version 15.0 #19

Open

6 tasks

OCA deleted a comment from oca-clabot Oct 13, 2022

nimarosa assigned mtelahun Oct 13, 2022

mtelahun force-pushed the 15.0-mig-payroll branch from 6405d3e to c236735 Compare October 13, 2022 12:46

mtelahun force-pushed the 15.0-mig-payroll branch 4 times, most recently from 358748c to 29f9870 Compare October 14, 2022 07:12

mtelahun force-pushed the 15.0-mig-payroll branch from 29f9870 to aaac623 Compare October 14, 2022 07:26

mtelahun force-pushed the 15.0-mig-payroll branch from aaac623 to a6d58db Compare October 14, 2022 08:42

mtelahun mentioned this pull request Oct 14, 2022

[15.0][MIG] Migrate payroll_account #84

Merged

norlinhenrik and others added 10 commits November 10, 2022 16:52

[IMP] payroll: settings descriptions, show_details_by_salary_rule_cat…

d7ec081

…egory

[IMP] payroll: required fields, removed details_by_salary_rule_category

25aa131

[IMP] payroll: hide_invisible_lines (who don't appear on payslip)

c2f082e

[IMP] payroll: New setting: Require code and category

295fe56

[IMP] payroll: test rule code in payslip line

1554991

[IMP] payroll: removed required=False, improved text descriptions

3c52973

[IMP] payroll: test rule and category with and without code

4feb0fe

Translated using Weblate (Spanish (Argentina))

d98d547

Currently translated at 100.0% (359 of 359 strings) Translation: payroll-14.0/payroll-14.0-payroll Translate-URL: https://translation.odoo-community.org/projects/payroll-14-0/payroll-14-0-payroll/es_AR/

[IMP] payroll: black, isort, prettier

9984698

[MIG] payroll: Migration to 15.0

b3970c1

mtelahun force-pushed the 15.0-mig-payroll branch from 996c6b0 to b3970c1 Compare November 10, 2022 13:52

nimarosa requested review from a user and norlinhenrik November 10, 2022 17:02

nimarosa approved these changes Nov 10, 2022

View reviewed changes

norlinhenrik approved these changes Nov 10, 2022

View reviewed changes

OCA-git-bot added approved ready to merge labels Nov 10, 2022

OCA-git-bot added the bot is merging ⏳ label Nov 10, 2022

OCA-git-bot merged commit fd97f54 into OCA:15.0 Nov 10, 2022

OCA-git-bot removed the bot is merging ⏳ label Nov 10, 2022

OCA-git-bot added the merged 🎉 label Nov 10, 2022

mtelahun deleted the 15.0-mig-payroll branch November 12, 2022 08:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[15.0][MIG] payroll #81

[15.0][MIG] payroll #81

mtelahun commented Oct 13, 2022

mtelahun commented Oct 13, 2022

mtelahun commented Oct 13, 2022

mtelahun commented Oct 13, 2022

nimarosa commented Oct 13, 2022

nimarosa commented Oct 13, 2022

nimarosa commented Oct 13, 2022

norlinhenrik commented Oct 13, 2022

nimarosa commented Oct 13, 2022

mtelahun commented Oct 14, 2022

mtelahun commented Oct 14, 2022

nimarosa commented Oct 14, 2022

nimarosa commented Oct 14, 2022 •

edited

Loading

mtelahun commented Oct 14, 2022 via email

mtelahun commented Oct 14, 2022 via email

nimarosa commented Oct 14, 2022

mtelahun commented Oct 14, 2022 via email

mtelahun commented Nov 10, 2022

nimarosa commented Nov 10, 2022

norlinhenrik left a comment

OCA-git-bot commented Nov 10, 2022

nimarosa commented Nov 10, 2022

OCA-git-bot commented Nov 10, 2022

OCA-git-bot commented Nov 10, 2022

OCA-git-bot commented Nov 10, 2022

mtelahun commented Nov 11, 2022 via email

[15.0][MIG] payroll #81

[15.0][MIG] payroll #81

Conversation

mtelahun commented Oct 13, 2022

mtelahun commented Oct 13, 2022

mtelahun commented Oct 13, 2022

mtelahun commented Oct 13, 2022

nimarosa commented Oct 13, 2022

nimarosa commented Oct 13, 2022

nimarosa commented Oct 13, 2022

norlinhenrik commented Oct 13, 2022

nimarosa commented Oct 13, 2022

mtelahun commented Oct 14, 2022

mtelahun commented Oct 14, 2022

nimarosa commented Oct 14, 2022

nimarosa commented Oct 14, 2022 • edited Loading

mtelahun commented Oct 14, 2022 via email

mtelahun commented Oct 14, 2022 via email

nimarosa commented Oct 14, 2022

mtelahun commented Oct 14, 2022 via email

mtelahun commented Nov 10, 2022

nimarosa commented Nov 10, 2022

norlinhenrik left a comment

Choose a reason for hiding this comment

OCA-git-bot commented Nov 10, 2022

nimarosa commented Nov 10, 2022

OCA-git-bot commented Nov 10, 2022

OCA-git-bot commented Nov 10, 2022

OCA-git-bot commented Nov 10, 2022

mtelahun commented Nov 11, 2022 via email

nimarosa commented Oct 14, 2022 •

edited

Loading