[14.0] IMP l10n_account_it: add format_vat_it to res_partner #2314
Conversation
TheMule71
changed the title
TheMule71
force-pushed
the
14.0-imp-add-format_vat_it
branch
from
8d64d42 to
5136fa7
Compare
|
Segnalo una certa sovrapposizione con odoo/odoo#71920 |
eLBati
changed the title
TheMule71
force-pushed
the
14.0-imp-add-format_vat_it
branch
from
5136fa7 to
ea2f2c3
Compare
Fatto. Le casistiche non è che siano molte, rimuove gli spazi, mette maiuscole le prime due lettere. |
| class ResPartner(models.Model): | ||
| _inherit = "res.partner" | ||
|
|
||
| def format_vat_it(self, vat): |
There was a problem hiding this comment.
Questo metodo viene chiamato da base_vat in https://github.com/odoo/odoo/blob/0268986b60477c7fb46d93f45f3355f546f0155c/addons/base_vat/models/res_partner.py#L545 quindi potresti aggiungere la dipendenza da base_vat?
Attualmente viene comunque chiamato dai test (è coperto) solo perché altri moduli dipendono da base_vat quindi viene comunque installato nell'ambiente di Travis.
Puoi verificarlo in locale installando solo questo modulo: questo metodo non viene mai chiamato.
There was a problem hiding this comment.
Non so se ho ben capito.
base_vat chiama questo modulo, non viceversa. Sarebbe una dipendenza se questo modulo chiamasse funzioni di base_vat.
Ora, la funzionalità del test non è disponibile se manca base_vat. Non so se è possibile specificare una dipendenza che valga solo per i test, o se a livello di test abbia senso controllare che ci sia base_vat e sono in quel caso eseguire il test.
There was a problem hiding this comment.
In generale una situazione del genere si risolverebbe con un modulo ponte ma in questo caso mi pare eccessivo, quindi lasciamo pure così 👍🏻
| @@ -128,3 +128,11 @@ def test_vat_22_50(self): | |||
| self.assertEqual(tax.balance, -22) | |||
| self.assertEqual(tax.deductible_balance, -11) | |||
| self.assertEqual(tax.undeductible_balance, -11) | |||
|
|
|||
| def test_format_vat_it(self): | |||
| partner_id = self.env.ref("base.res_partner_12") | |||
There was a problem hiding this comment.
Pare che questo test passi anche senza il metodo che hai aggiunto a res.partner, puoi verificare?
Se aggiungi un test per una modifica, il test dovrebbe fallire senza la tua modifica e passare con essa.
There was a problem hiding this comment.
Ero abbastanza sicuro che fallisse senza, anche perché mi ero scordato di mettere from . import res_partner in models/__init__.py e ho litigato 5 minuti col debugger prima di capire perché la funzione venisse ignorata. E il test falliva. Non mi pare di aver modificato altro dopo. Domani controllo.
There was a problem hiding this comment.
Stamattina in chiamata abbiamo verificato che questo test passa anche senza il nuovo metodo che hai aggiunto a res.partner, vedi tu se chiudere tutto o aggiungere solo il test
| _inherit = "res.partner" | ||
|
|
||
| def format_vat_it(self, vat): | ||
| return vat and re.sub(r"\W+", "", vat).upper() or False |
There was a problem hiding this comment.
Da come viene chiamato questo metodo in base_vat (https://github.com/odoo/odoo/blob/0268986b60477c7fb46d93f45f3355f546f0155c/addons/base_vat/models/res_partner.py#L545), pare che qui ci debbano essere solo cifre.
Potresti quindi modificare l'espressione regolare in modo da rimuovere tutto ciò che non è una cifra?
Attualmente inserendo la P.IVA IT1234567s0017, questa non viene ripulita ma viene sollevato l'errore
Questo perché l'espressione regolare in pratica rimuove solo tutto ciò che non è alfanumerico lasciando i caratteri alfabetici.
Essendo fuori dallo scope di questa PR (rimuovere solo gli spazi), questo non lo valuterei come bloccante per il merge ma solo un nice to have.
There was a problem hiding this comment.
Trovo corretta l'eccezione. Rimuovere gli spazi (che la 99% vuol dire lo spazio tra IT e il numero) è una cosa di formattazione del campo: "IT 12345670017" e "IT12345670017" sono la stessa piva formattata in due modi diversi.
Se l'utente inserisce un errore di battitura, trovo giusto segnalarlo e non inserire un euristica per "correggere" l'input per es. rimuovendo le lettere.
|
Messa in draft perché apparentemente non serve, core lo fa già... |
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
github-actions
bot
added
the
stale
Aggiunge formattazione per IT con rimozione spazi.
--
Confermo di aver firmato il CLA https://odoo-community.org/page/cla e di aver letto le linee guida su https://odoo-community.org/page/contributing