[18.0][OU-FIX] account: correct payment state for migrated payments#5767
[18.0][OU-FIX] account: correct payment state for migrated payments#5767akashr355 wants to merge 6 commits into
Conversation
[MIG] account: recompute payment states post-migration During the 18.0 migration, `fill_account_payment` maps all <=17 payments to 'in_process' because a payment's own journal entry defaults to `payment_state = 'not_paid'`. Consequently, fully settled payments fail to migrate to the 'paid' state. This introduces a post-migration script to re-trigger `_compute_state()` for in-process payments. This ensures fully reconciled payments are promoted to 'paid', accurately reflecting their true status.
pedrobaeza
left a comment
There was a problem hiding this comment.
This is not happening in my migrated instances, so something on your side (maybe a module like account_in_payment installed) is different, so you should fix it in that part, not here.
|
|
Please fix pre-commit. |
Test added please review. |
pre-commit fixed. |
hbrunn
left a comment
There was a problem hiding this comment.
thanks for the tests.
do we need to cover this branch too?
@pedrobaeza I think this is indeed wrong, it's just hard to decipher the generated bullshit. Note it's about the state field of account.payment, not payment state of invoices
| FROM account_payment_method_line apml | ||
| WHERE apml.journal_id = ap.journal_id | ||
| AND apml.payment_account_id IS NOT NULL | ||
| ) |
There was a problem hiding this comment.
here the other possibilities from https://github.com/odoo/odoo/blob/18.0/addons/account/models/account_payment.py#L268-L272 are missing
There was a problem hiding this comment.
Fixed — I dropped the SQL enumeration of liquidity accounts entirely. Calling _compute_state uses _seek_for_lines, which covers all of _get_valid_liquidity_accounts(): default_account_id, payment_method_line_id.payment_account_id, the journal's inbound/outbound method-line accounts, and outstanding_account_id.
| ) | ||
| UPDATE account_payment ap | ||
| SET state = 'paid', | ||
| is_matched = TRUE |
There was a problem hiding this comment.
this field exists in v17 already, so shouldn't be touched
There was a problem hiding this comment.
Removed — I no longer set is_matched. _compute_state only re-queues it for the payments it promotes via the reconciled-invoices branch, exactly as core does, so the migrated value is otherwise left untouched.
Yes — and it's covered now. I reworked the fix to call account.payment._compute_state() directly instead of re-implementing it in SQL, so the reconciled-invoices/bills-all-paid branch (L482-487) is handled by core itself. fill_account_payment writes state with raw SQL, so the stored compute never runs; this just re-triggers it (batched). Added a test for that branch too: test_payment_state_outstanding_fully_paid (reconcilable outstanding account + fully-paid invoice → promoted to paid), alongside not fully paid (partial → stays in_process). |
Agreed the SQL was hard to follow — that's why I switched to calling the core compute. Nothing can drift from Odoo now, and it's readable. Thanks for the review. |
|
it's not the sql that's the problem, but the wall of text in the comments. please revert to your previous version, just add the extra accounts in the subquery, and do a second one for invoices/bills. and rewrite the comments in a more succinct non-grating-obviously-generated way |
[MIG] account: recompute payment states post-migration
During the 18.0 migration,
fill_account_paymentmaps all <=17 payments to 'in_process' because a payment's own journal entry defaults topayment_state = 'not_paid'. Consequently, fully settled payments fail to migrate to the 'paid' state.This introduces a post-migration script to re-trigger
_compute_state()for in-process payments. This ensures fully reconciled payments are promoted to 'paid', accurately reflecting their true status.