[FIX] account: invoice document layout IBAN #163565
Conversation
There was a problem hiding this comment.
LGTM Thanks for the fix !
@william-andre ready for your review 😄 (not tested by a po yet but should be good)
| @@ -66,7 +66,7 @@ def _inverse_account_number(self): | |||
| elif record.account_number: | |||
| record.partner_id.bank_ids = [ | |||
| Command.create({ | |||
| 'account_number': record.account_number, | |||
| 'acc_number': record.account_number, | |||
There was a problem hiding this comment.
The if branch will change the account number of all the banks if there are multiple
Why don't we do anything if there is not qr_code? That seems like a very hidden behavior.
If somehow the field becomes shown without a qr_code, I would expect the behavior to be the same.
There was a problem hiding this comment.
Indeed maybe it should only change the account number of the first one 🤔, maybe we can just do record.partner_id.bank_ids[0].acc_number in the line 65.
When coming from an invoice the configure layout has an extra step which is the possibility to change the setting for the qr_code. When selected and only when selected the acc_number field appears and we have the possibility to change it. It was a spec of @chklop, maybe he has some more insight on this ?
There was a problem hiding this comment.
We should not change all current account numbers by the one provided indeed.
The Bank account number field is not useful here if we don't activate QR-code, so it's logical it's not displayed nor used if the "activate QR codes" is not ticked on that wizard. The two are indeed linked.
There was a problem hiding this comment.
We won't change anything if there is no number set (already managed by the code)
And if there is a number set just find it weird to have tho read qr_code in the inverse function of account_number when the computation field doesn't depend on it: it's a code smell.
There was a problem hiding this comment.
Thanks for the review. I changed to only change the first account number
Edit: And removed the if condition on the QR code
hupo-odoo
force-pushed
the
master-invoice_layout_qrcode_fix-hupo
branch
from
fdd271a
to
6be55d3
Compare
There was a problem hiding this comment.
Ok, not changing the qr_code because it is only a minor thing and not related to this bug...
@robodoo r+
Step to reproduce: - Select a company that has not yet sent invoice (create one if needed) - Create an invoice and press "Send and Print" - => The wizard to configure your first invoice pops up - Press the button "QR Code" - Enter a random IBAN - ==> Traceback stating that "Invalid field 'account_number' on model 'res.partner.bank'" Issue introduced in this commit: odoo@45d924b task-id: 3893654
hupo-odoo
force-pushed
the
master-invoice_layout_qrcode_fix-hupo
branch
from
6be55d3
to
cb9a363
Compare
Sorry, I pushed at the same time of r+ and removed the if condition on the qr_code. Hope it's ok |
|
@robodoo r+ No worries 😊 |
Step to reproduce: - Select a company that has not yet sent invoice (create one if needed) - Create an invoice and press "Send and Print" - => The wizard to configure your first invoice pops up - Press the button "QR Code" - Enter a random IBAN - ==> Traceback stating that "Invalid field 'account_number' on model 'res.partner.bank'" Issue introduced in this commit: 45d924b closes #163565 Task-id: 3893654 Signed-off-by: William André (wan) <wan@odoo.com>
Step to reproduce:
Issue introduced in this commit: 45d924b
task-id: 3893654
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr