[17.0][FIX] account_payment_purchase: copy the fields to supplier invoices. #1326
Conversation
Tisho99
force-pushed
the
17.0-imp-account_payment_purchase
branch
from
8a6bf67 to
c91c941
Compare
|
Hi @victoralmau This PR overrides a function you created last month (#1303), but it respects the account_payment_partner compute and your test. Maybe you are interested A review would also be appreciated Thank you! |
Tisho99
force-pushed
the
17.0-imp-account_payment_purchase
branch
from
c91c941 to
74103b9
Compare
Tisho99
force-pushed
the
17.0-imp-account_payment_purchase
branch
from
74103b9 to
c0e5006
Compare
|
@Tisho99 You commented that this PR is fixing a functionallity of the module. Souldn't it be a FIX, instead of an IMP, then? |
Tisho99
changed the title
|
@Tisho99 What about the commit message? |
Tisho99
force-pushed
the
17.0-imp-account_payment_purchase
branch
from
c0e5006 to
d1aa866
Compare
Updated |
pedrobaeza
changed the title
| else: | ||
| order.payment_mode_id = False |
There was a problem hiding this comment.
This part should go out. The payment mode should remain to the existing one.
There was a problem hiding this comment.
Okay, the module already sets it to False, but i think you are right. i'll change it
| res = super()._prepare_invoice() | ||
| vals = {"payment_mode_id": self.payment_mode_id.id} | ||
| if self.payment_mode_id.payment_method_id.bank_account_required: | ||
| vals["partner_bank_id"] = self.supplier_partner_bank_id.id | ||
| else: | ||
| res.pop("partner_bank_id") | ||
| res.update(vals) | ||
| return res |
There was a problem hiding this comment.
Please avoid the use of an intermediate dictionary:
| res = super()._prepare_invoice() | |
| vals = {"payment_mode_id": self.payment_mode_id.id} | |
| if self.payment_mode_id.payment_method_id.bank_account_required: | |
| vals["partner_bank_id"] = self.supplier_partner_bank_id.id | |
| else: | |
| res.pop("partner_bank_id") | |
| res.update(vals) | |
| return res | |
| res = super()._prepare_invoice() | |
| res["payment_mode_id"]: self.payment_mode_id.id | |
| res.pop("partner_bank_id", False) | |
| if self.payment_mode_id.payment_method_id.bank_account_required: | |
| res["partner_bank_id"] = self.supplier_partner_bank_id.id | |
| return res |
But respecting the constaints
Tisho99
force-pushed
the
17.0-imp-account_payment_purchase
branch
from
d1aa866 to
1a75664
Compare
|
Hi @pedrobaeza I applied your requested changes Thank you for the review! |
|
What a great day to merge this nice PR. Let's do it! |
|
Congratulations, your PR was merged at 316a732. Thanks a lot for contributing to OCA. ❤️ |
5 participants
The account_payment_purchase says that the new fields (Bank Account and Payment Mode) are copied from partner to purchase order, but they are not.
This PR fixes it, but respecting the constrains of the compute of account_payment_partner, the payment mode has to require a bank account to copy the bank account. (Related with #1303)
I have added this constraint to the compute of the bank_account_id of the purchase, but it can be manually edited.
I have also adapted the _onchange_purchase_auto_complete to use the new function.
T-6102