[19.0][FIX] hr_expense_payment: derive payment links from reconciliation (rework for OCA#355 review) - #5
Closed
dnplkndll wants to merge 2 commits into
Closed
Conversation
hr.expense.sheet was removed in 19.0; retarget the payment register to hr.expense.
Address review on OCA#355 (pilarvargas-tecnativa): - Do not redefine account.payment.expense_ids: 19.0 core already owns that field (related to move_id.expense_ids, i.e. company-paid expenses) and hangs logic off it (_compute_outstanding_account_id, the write() guard, action_open_expense). The module field is renamed to reconciled_expense_ids. - Replace the stored Many2many + wizard context threading + post_init hook with fields computed from the reconciliation, using core's account.move._get_reconciled_payments()/_get_reconciled_amls() helpers (the reconciled_bill_ids pattern). The link can never go out of sync with the standard relationship, includes partially reconciled payments, and works however the payment was registered. - compute_sudo on both fields: employees can read their expenses without accounting access, so the reconciliation walk needs sudo. - No migration script needed anymore: nothing is stored, so 18.0 payment_expense_sheet_rel data is superseded by the reconciliation itself on upgraded databases. - Tests cover: core field not overridden, back-link on register payment, partial payment, grouped multi-expense payment, unreconcile dissolving the link, searchability of both computed fields, employee (non-accounting) read access, and company-paid expenses staying core-only.
dnplkndll
force-pushed
the
19.0-mig-hr_expense_payment-rework
branch
from
July 23, 2026 23:02
8f9add1 to
225c1c7
Compare
dnplkndll
force-pushed
the
19.0-mig-hr_expense_payment
branch
from
July 23, 2026 23:10
093ce9c to
3fa1c43
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rework of the
hr_expense_payment19.0 MIG answering pilarvargas-tecnativa's review on OCA/hr-expense#355. Do not merge — after approval the commit gets folded into the[MIG]commit on19.0-mig-hr_expense_paymentand force-pushed there.What the review flagged
account.payment.expense_ids, which 19.0 core already owns (related tomove_id.expense_ids) — the independent M2M shadows the core field and can desync.payment_expense_sheet_reldata (post_init_hook only runs on install).full_reconcile_idonly — misses partially reconciled payments.The rework
account.payment.reconciled_expense_ids(renamed) +hr.expense.payment_idsare now computed from the reconciliation via core'saccount.move._get_reconciled_payments()/_get_reconciled_amls()— thereconciled_bill_idspattern. Non-stored +search.matched_debit_ids/matched_credit_ids).action_paycontext threading, andhooks.pydeleted — reconciliation already captures every registration path, including payments registered move-side or reconciled manually.hr_expense_cancel(only consumer, readsexpense.payment_idsbefore unreconciling) keeps working unchanged.Prior art
account.payment.reconciled_invoice_ids/reconciled_bill_ids; helpers_get_reconciled_amls/_get_reconciled_payments(partial-inclusive).l10n_br_account_due_list.payment_move_line_ids— non-stored M2M computed from partials); account-analytic#579 & l10n-brazil#4750 (drop module field duplicated by core); sale-workflow#3013 (sale_delivery_statecore-collision refactor).Test conditions covered (from the review + prior-art threads)
core
expense_idsnot overridden · back-link on Register Payment · partial payment · second payment for the remainder · grouped multi-expense payment · unreconcile dissolves the link (no stale rows) · both fields searchable · employee (non-accounting) read access —compute_sudo, red→green proven · company-paid stays core-only (payment.expense_ids), module fields empty.Local run (stock
odoo:19.0+ fresh DB, demo on): 8/8 tests pass. Removingcompute_sudomakes the employee-access test fail with the exact AccessError (red→green verified).