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

[17.0][MIG] payroll: Migration to 17.0 #142

Merged
merged 147 commits into from
May 16, 2024

Conversation

hapolinario
Copy link

@hapolinario hapolinario commented Feb 2, 2024

Migration payroll module to 17.0.
Supersedes #138

davejames and others added 30 commits December 21, 2023 07:27
this commit renames the module from hr_payroll to payroll. This is to avoid conflicting with the hr_payroll module included in Odoo Enterprise
Also removes links to localisations which have not yet been migrated. These can be re-added when the localisations are ported.
author nicolasrsande <[email protected]> 1643135061 -0300
committer nicolasrsande <[email protected]> 1643668459 -0300

fix typo in leave calculation

default for worked days should not compute leaves, because leaves are calculated separately

we should not add data because it interfers with custom localization payroll modules

fix pre-commit

add demo data so the test can be executed

leaves should be computed in negative value to help creating salary rules

Add mantainer key

remove mantainer in payroll_account

14.0-minor-fixes

fix typo in leave calculation

we should not add data because it interfers with custom localization payroll modules

fix pre-commit

add demo data so the test can be executed

leaves should be computed in negative value to help creating salary rules

Add mantainer key

remove mantainer in payroll_account

14.0-payroll-minor-fixes
accept suggestion use repr() on the string

change indentation of error
Currently translated at 93.6% (266 of 284 strings)

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

Translation: payroll-14.0/payroll-14.0-payroll
Translate-URL: https://translation.odoo-community.org/projects/payroll-14-0/payroll-14-0-payroll/ca/
Currently translated at 86.9% (247 of 284 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/
Currently translated at 100.0% (284 of 284 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/
Currently translated at 100.0% (284 of 284 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/
Currently translated at 100.0% (284 of 284 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/
Currently translated at 78.1% (222 of 284 strings)

Translation: payroll-14.0/payroll-14.0-payroll
Translate-URL: https://translation.odoo-community.org/projects/payroll-14-0/payroll-14-0-payroll/it/
Currently translated at 100.0% (290 of 290 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/
The unittest for the payroll module is broken in several ways:
    1. It doesn't actually test anything in the payslip, just the states
       when various buttons are simulated
    2. The payslip is created programatically, so no payslip values are loaded
    3. The employee's contract is in "draft" state so even if it tried to
       load any values it wouldn't work because the contract isn't "open"
    4. It modifies a salary input line, but doesn't verify it worked. It
       doesn't work because of the above mentioned reasons.

To ensure a minimal amount of testing:
    o Make sure the employee's contract is in "open" state
    o Use the "Form" testing object when creating a payslip to ensure
      the onchange_employee() is run
    o Actually test the payroll calculations are done correctly
…salary rule

A side effect of the way the payslip line quantity and rate are set when
amount_select is set to "code" causes them to never evaluate to zero. When they
evaluate to 0 the side effect causes them to be set to 1.0 and 100.0, respectively.

    "result_qty" in localdict and localdict["result_qty"] or 1.0
    "result_rate" in localdict and localdict["result_rate"] or 100.0
weblate and others added 4 commits December 21, 2023 07:27
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: payroll-16.0/payroll-16.0-payroll
Translate-URL: https://translation.odoo-community.org/projects/payroll-16-0/payroll-16-0-payroll/
Currently translated at 57.2% (205 of 358 strings)

Translation: payroll-16.0/payroll-16.0-payroll
Translate-URL: https://translation.odoo-community.org/projects/payroll-16-0/payroll-16-0-payroll/it/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: payroll-16.0/payroll-16.0-payroll
Translate-URL: https://translation.odoo-community.org/projects/payroll-16-0/payroll-16-0-payroll/
@nimarosa
Copy link
Contributor

Will review this PR next week. Please is there are other functional or code reviewers will be great to make the process fast.

@peluko00
Copy link

@hapolinario squash commits please

@hussain
Copy link

hussain commented Mar 5, 2024

I will do review this week.

@hussain
Copy link

hussain commented Mar 7, 2024

Functional OK with minor view changes.

@hapolinario
Please squash last commits of same description once you're done.

image

<group name="period_group">
<label for="date_start" string="Period" />
<div>
<field
Copy link

Choose a reason for hiding this comment

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

You are using widget="daterange" so there is no need to put both start & end date separately. Please remove one of them as they show duplicate on screen.

image

</field>
</page>
</notebook>
<group string="Notes">
Copy link

Choose a reason for hiding this comment

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

The note field is somehow very narrow and not going full page width.

image

groups="payroll.group_payroll_manager"
>
<block title="Accounting" id="payroll_accountant">
<setting>
Copy link

Choose a reason for hiding this comment

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

There are duplicate text in configuration.

image

<setting> block can be replaced with:

<setting id="" string="Payroll Entries" help="Post payslips in accounting">
	<field name="module_payroll_account" />
</setting>

Please apply same to all other <settings> blocks.

</group>
<newline />
<group>
<separator string="Description" />
Copy link

Choose a reason for hiding this comment

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

The description box is very narrow, should be full page width probably.

image

@OCA-git-bot
Copy link
Contributor

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). 🤖

@dreispt
Copy link
Member

dreispt commented May 16, 2024

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 17.0-ocabot-merge-pr-142-by-dreispt-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 8f8f86a into OCA:17.0 May 16, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

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

Copy link

@celm1990 celm1990 left a comment

Choose a reason for hiding this comment

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

My comment a little late: I think it is not a good idea to skip E501. It's better to split the lines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.