[18.0][IMP] hr_expense_sequence: propagate number to journal entries - #377
Open
JordiBForgeFlow wants to merge 3 commits into
Open
[18.0][IMP] hr_expense_sequence: propagate number to journal entries#377JordiBForgeFlow wants to merge 3 commits into
JordiBForgeFlow wants to merge 3 commits into
Conversation
Set the expense report number as the reference of the accounting documents it generates, so the accounts payable can be reconciled by expense report number: - Reports paid by the employee: the number is set on the vendor bill's `ref` and `payment_reference`. - Reports paid by the company: the number is set on each payment's `memo` and on the `ref` of its journal entry. The memo is needed as its inverse writes back on the move reference. `account.move.line.ref` is a stored related field on `account.move.ref`, so the number ends up on every journal item, searchable from the reconciliation widget and the aged payable report.
`hr_expense_invoice` bypasses the creation of the journal entries on approval for employee-paid reports, so they only exist after `action_sheet_move_post`. Asserting right after the approval made the tests fail whenever that module is installed alongside, as on the CI.
… reference Setting the report number as the whole reference discarded the one the accounting documents already carried. That is a real loss for reports paid by the company, which generate one journal entry per expense line: every entry of a report ended up with the same reference, so the flight could no longer be told from the hotel in the accounts payable. The number is now a prefix, `EX0001 - Client visit`. Searching by number still works, since `ref` is a trigram-indexed field matched with ilike, and sorting journal items by reference now groups them by report. `ref` and `payment_reference` are set to the same value on purpose: core builds the payable line label out of both and concatenates them when they differ, which would have produced `EX0001 - Client visit - EX0001`.
Member
Author
|
The error testing OCB seems to be unrelated https://github.com/OCA/hr-expense/actions/runs/30613090613/job/91100087880?pr=377#step:2:173 |
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
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.
Problem
hr_expense_sequencenumbers expense reports (EX0001), but that number stopsat
hr.expense.sheet. The accounting documents the report generates arereferenced by free text instead:
ref = sheet.name(the report summary);ref = expense.name.So there is no way to tie a payable line back to the expense report it came
from, and reconciling the accounts payable by report number is impossible.
What this does
The report number is prefixed onto the reference of every journal entry the
report generates:
Client visitEX0001 - Client visitFlightEX0001 - Flightaccount.move.line.refis a stored, trigram-indexed related field onaccount.move.ref, so the number lands on every journal item — searchable,groupable and sortable from the reconciliation widget, the Journal Items list
and the Aged Payable report.
A prefix rather than a replacement, for two reasons:
reference would give every entry of a report the same value and you could no
longer tell the flight from the hotel;
each line still says what it was.
Searching by number is unaffected — the search view matches
refwithilikeon a trigram index.
Implementation
Two hooks, both taken after
super()so the value being prefixed is whatevercore (or another module) decided:
hr.expense.sheet._prepare_bills_vals()— employee-paid vendor bill;hr.expense._prepare_payments_vals()— company-paid payment entries._prepare_move_vals()is not enough on its own: both callers setrefexplicitly after spreading its result, so it would be overwritten.
Two non-obvious points, both commented in the code:
refandpayment_referenceare set to the same value. Core builds tpayable line's label from both and concatenates them when they differ
(
account_move_line.py:_compute_name), so prefixing onlyrefwould haveproduced
EX0001 - Client visit - EX0001on the AP line.memois set too.account.payment._inverse_memowritesonto
account.move.ref, and payments are created after the moves in_do_create_moves, so setting the move reference alone is silently undoneReports without a number (
/) are left completely untouched. No new depende—
hr_expensealready depends onaccount. Version bumped to18.0.1.1.0.Tests
tests/test_hr_expense_sequence_account_move.py, 6 tests: employee-paid billpayments, a manually forced number, no number, and an empty reference (no
dangling separator).
Run green both standalone and with
hr_expense_invoiceinstalled — that modbypasses move creation on approval for employee-paid reports, so the tests
assert after
action_sheet_move_post().Not covered
reference;
assign_old_sequencesnumbers the existing reports but does notrewrite their posted entries.
_do_reverse_movesforcesref: False. They auto-reconcile against the original, so this does not banything, but the number is not on them.
Find attached a video that shows the behaviour:
hr_expense_sequence_move_ref.mp4