[18.0][FIX] sale_order_type: preserve manual payment term when changing inv…#4273
[18.0][FIX] sale_order_type: preserve manual payment term when changing inv…#4273lef-adhoc wants to merge 1 commit into
Conversation
99cb973 to
3bd8e78
Compare
d6989a8 to
6d7ad4e
Compare
…oice journal When a sale type has no payment_term_id and the user sets a manual term on the sale order, changing the invoice journal triggered a recompute chain (journal_id → company_id → sale_type_id) that caused super() to silently reset invoice_payment_term_id to the partner's default. Skip the super() call for records where neither the partner nor the sale type changed and the sale type carries no term, so the chosen term is kept. Adds a regression test covering this scenario.
6d7ad4e to
efd0cc2
Compare
|
Hi @pedrobaeza, what do you think about this? |
|
EDIT: Seeing that it's on moves, not orders. Trying again. |
|
@pedrobaeza Here's a video showing it running on Runboat |
|
But that's not because of |
|
@pedrobaeza If you delete the _compute_invoice_payment_term_id method from the sale_order_type, you’ll see that the payment term isn’t updated when you update the journal. You can test this in Odoo’s Runbot. |
| for move in self: | ||
| if move.sale_type_id.payment_term_id: | ||
| move.invoice_payment_term_id = move.sale_type_id.payment_term_id | ||
| else: | ||
| previous_term = previous.get(move) | ||
| if previous_term: | ||
| move.invoice_payment_term_id = previous_term |
There was a problem hiding this comment.
For me, the code should be:
| for move in self: | |
| if move.sale_type_id.payment_term_id: | |
| move.invoice_payment_term_id = move.sale_type_id.payment_term_id | |
| else: | |
| previous_term = previous.get(move) | |
| if previous_term: | |
| move.invoice_payment_term_id = previous_term | |
| for move in self: | |
| if not move.invoice_payment_term_id and move.sale_type_id.payment_term_id: | |
| move.invoice_payment_term_id = move.sale_type_id.payment_term_id |
There was a problem hiding this comment.
I think this wouldn't solve the issue that motivated the PR.
In this case, after super(), invoice_payment_term_id is already filled with the partner payment term, so your condition would not apply and the manual value would still be lost.
It would also change the current behavior of the module, because sale_order_type would stop overriding the computed payment term and would only act as a fallback when the field is empty.
nicolascol
left a comment
There was a problem hiding this comment.
LGTM
https://drive.google.com/file/d/1wDO7Syi-YXBKWLedPjgACvHcyWrq94Up/view
Could you watch the video?
@pedrobaeza

…oice journal
When a sale type has no payment_term_id and the user sets a manual term on the sale order, changing the invoice journal triggered a recompute chain (journal_id → company_id → sale_type_id) that caused super() to silently reset invoice_payment_term_id to the partner's default.
Skip the super() call for records where neither the partner nor the sale type changed and the sale type carries no term, so the chosen term is kept. Adds a regression test covering this scenario.