[16.0][FIX] hr_expense_invoice: Incorrect data and check when creating vendor bill from expense #249
Conversation
|
Hello @pedrobaeza , can you reset de runboat so we can test it? Thank you! |
pedrobaeza
force-pushed
the
16.0-fix-hr_expense_invoice-create_bill
branch
from
c072e51 to
3794eac
Compare
|
Re-pushed |
There was a problem hiding this comment.
Hello @pedrobaeza , I've came across a new issue with the flow you suggested.
If in the bill I add a Vendor or a reference the validation error show up again.
https://www.loom.com/share/989658911ea94a788dc820adee39b5dd?sid=1b33c2b6-16d8-4ded-8613-e7d680d25761
There was a problem hiding this comment.
Hello, upon reviewing the PR, I don't see that it follows the correct flow. If the expense is X, the total amount of the invoice should also be X. Therefore, if we add a tax to the invoice, the price must change so that the total remains X.
When I perform these steps, a warning appears and does not allow us to confirm the invoice because it is based on the taxable base and not on the total. This is incorrect.
You can see the flow in the following video: https://www.loom.com/share/9c620effe6174653b380aa7a09d403ae?sid=631e7f66-ca85-42e8-8d54-d4cbb7450a37
|
@pedrobaeza can I continue this PR in a separate PR using your code (and mentioning you)? |
|
We can merge this one and you add an extra PR on top of it. These fixes are welcome although not completed. |
There was a problem hiding this comment.
I've spotted an strange behaviour.
If you perform a write when you are changing the supplier and the lines (always with a match on the total amounts) at the same time (modify all the fields you need to change and click save), the move.amount_total value is the old one.
The key to know when to allow Odoo perform it's changes or checks is to add the context checking.
With this small modifications works perfect.
| # Check if the amount of the invoice linked to an invoice is different | ||
| # Done here in the write instead of a Python constraint as the computed field | ||
| # amount_total is not yet updated on that moment | ||
| res = super().write(vals) |
There was a problem hiding this comment.
| res = super().write(vals) | |
| res = super().write(vals) | |
| if self.env.context.get("skip_account_move_synchronization"): | |
| return res |
There was a problem hiding this comment.
@pedrobaeza can you address it?
@loida-vm @edlopen please approve this PR in order to allow me to continue doing things
…or bill from expense Two fixes: 1. The "amount_total" field now reflects the total of the invoice including taxes, so we have to use "untaxed_amount". 2. If changing any data in the linked vendor bill, the computed fields used for the comparison are not yet recomputed, so let's do the check on a later stage (after the write).
pedrobaeza
force-pushed
the
16.0-fix-hr_expense_invoice-create_bill
branch
from
3794eac to
8b8aeb3
Compare
|
/ocabot merge patch |
|
What a great day to merge this nice PR. Let's do it! |
|
Congratulations, your PR was merged at f787c47. Thanks a lot for contributing to OCA. ❤️ |
There was a problem hiding this comment.
I’m presenting a video here doing the functional check.
As you can see, and as my colleague @Shide mentioned, the value move.amount_total it "remembers" is the old one. Therefore, if the case is like the one I’m presenting (the second) in the video, it still doesn't allow me to confirm the invoice.
https://www.loom.com/share/23ba25ca6c664f42af38a3ae70b09b20?sid=145650c5-cc3c-4e07-bd78-73a03646ca3b
|
It was this morning, after the change |
|
Well, then the patch suggested by @Shide seems to be not working in that case. |
|
@loida-vm I'll fix your second case soon. |
|
Ok, thank you very much. Aside from that case, the PR works correctly. |
|
Will we have this resolved in version 17? Thanks |
Two fixes:
Fixes #245
@Tecnativa