Skip to content

Conversation

@tishmen
Copy link

@tishmen tishmen commented Oct 16, 2025

Scope:

  • payroll only

Depends on:

  • None

Summary:

  • Migrate to Odoo 19.
  • Follows OCA 19.0 migration tasks (version bump, security field rename, tz API).

Pre-commit:

  • Ran locally before opening; auto-fixes committed.

Tests:

  • Local install example: ./odoo/odoo-bin -c odoo.conf -d odoo_payroll_dev -i payroll --log-level=info --stop-after-init
  • OCA CI will validate on upstream infra.

davejames and others added 30 commits October 16, 2025 18:30
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
@tishmen
Copy link
Author

tishmen commented Oct 16, 2025

  • Translation best practice in Odoo is to keep placeholders inside the translatable string and interpolate after translation. That pattern looks like self.env._("… %(name)s …") % {"name": value}.
  • pylint-odoo ports a logging “not‑lazy” rule to translations and flags this interpolation as W8301, but there is no “lazy‑args” form for _(), so the warning is a stylistic false positive for Odoo’s i18n pattern. To remain translator‑friendly and still pass the hook, I added precise, per‑line disables only on the exact interpolation lines (# pylint: disable=translation-not-lazy).
  • Ruff’s UP031 prefers .format/f‑strings over % formatting. For translatable strings, the percent style with named placeholders is a better fit for Odoo’s extraction and translation workflows. I therefore added per‑line # noqa: UP031 where interpolation occurs after translation to avoid fighting the formatter and to stay consistent with OCA modules.

@tishmen tishmen marked this pull request as ready for review October 17, 2025 00:09
@tishmen
Copy link
Author

tishmen commented Oct 17, 2025

  • Odoo 19 drops the separate hr_contract module; “contracts” are now timeline versions inside core HR via the hr.version model.
    hr.employee _inherits hr.version (current version in employee.version_id), with multiple versions capturing effective changes via date_version + contract_date_start/contract_date_end.
  • I replaced hr.contract usages with hr.version: fields like date_start/date_end → date_version/contract_date_start/contract_date_end, and selection via employee._get_contracts(date_start, date_end), considering only overlapping versions.
  • Views/security were adjusted to target hr.version (contract templates), while hr.contract.type remains in hr for employment type metadata.
  • To preserve payroll features, I extend hr.version (not hr.contract) to carry payroll-specific fields (e.g., struct_id, schedule_pay) and keep existing flows working on the new model.

@tishmen
Copy link
Author

tishmen commented Oct 17, 2025

@kaynnan @WesleyOliveira98 @marcelsavegnago @CristianoMafraJunior pre-commit and tests are green; please test on RunBoat and review/approve.

Copy link
Member

@CristianoMafraJunior CristianoMafraJunior left a comment

Choose a reason for hiding this comment

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

The commit message should be only: [MIG] payroll: Migration to 19.0
970eaf7

@tishmen tishmen force-pushed the 19.0-mig-payroll branch 2 times, most recently from d4fd93d to 0695c91 Compare October 19, 2025 09:49
@tishmen
Copy link
Author

tishmen commented Oct 19, 2025

The commit message should be only: [MIG] payroll: Migration to 19.0 970eaf7

I have changed the commit message to the one specified, thank you.

Copy link
Member

@dreispt dreispt left a comment

Choose a reason for hiding this comment

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

Thank you, this looks like solid and brave work.
I made a few minor comments.
The one that could be a bug is the tz handling.

@realsaiko
Copy link

Demo data installation fails:
./odoo/odoo-bin -c odoo.conf -d odoo_payroll_dev -i payroll --with-demo --log-level=info --stop-after-init
It still has a reference to the "hr.contract" model that was removed in Odoo v19.0

- Drop hr.contract demo records (Odoo 19)

- Use calendar/employee timezone for leave computation

- Replace translation % formatting with env._ kwargs

- Remove redundant payslip unlink override
@tishmen
Copy link
Author

tishmen commented Dec 21, 2025

Thanks for the detailed review. I’ve pushed updates addressing your points: removed the redundant unlink() override, fixed leave-day timezone handling to use the contract/calendar timezone, and refactored the translation strings to use env._(..., **kwargs) so we can drop the noqa/pylint suppressions. Please take another look when you have a moment.

@tishmen
Copy link
Author

tishmen commented Dec 21, 2025

@realsaiko The demo data still referenced hr.contract, which no longer exists in Odoo 19. I’ve updated the demo XML to remove those hr.contract records and instead set the contract/version fields on the employee records, so payroll --with-demo should install cleanly now. If you can retry the install and confirm, that’d be very helpful.

@realsaiko
Copy link

@tishmen great, now the demo installation works without errors

@hieulucky111
Copy link

I don't see where we can set the Salary Structure on the Employee Version (Contract)
image

@hieulucky111
Copy link

The rest seems ok for me. Functional Test OK

- Add struct_id to the Employee/Payroll contract overview
- Copy struct_id from contract templates (whitelist)

Functional
@tishmen
Copy link
Author

tishmen commented Dec 28, 2025

image

@hieulucky111 In Odoo 19 the “contract” is an hr.version edited through the Employee form (the “New Contract” button creates a new version and reloads the employee with context['version_id']), so adding struct_id only on the contract template form
wasn’t enough. I added Salary Structure (struct_id) to the Employee → Payroll → Contract Overview and included it in the template whitelist so “Load a Template” can set it too. Verified on runboat oca-payroll-19-0-pr227-af64a2c7fb96: the field now renders right after “Pay Category”.

Copy link

@hieulucky111 hieulucky111 left a comment

Choose a reason for hiding this comment

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

Functional Test Ok

@dalonsod
Copy link
Contributor

dalonsod commented Jan 5, 2026

Hello @tishmen could cherry-pick #248 ?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.