[14.0][MIG] l10n_it_fatturapa_out_triple_discount #3681
Conversation
|
@LorenzoC0 che dici di #3269 (comment) ? edit: o meglio, di OCA/account-invoicing#1442 |
| 'name': 'ITA - Fattura elettronica - Integrazione sconto triplo', | ||
| "summary": "Modulo ponte tra emissione " | ||
| "fatture elettroniche e sconto triplo", | ||
| "version": "14.0.1.0.1", |
There was a problem hiding this comment.
non dovrebbe essere 14.0.1.0.0
eLBati
force-pushed
the
14.0-mig-l10n_it_fatturapa_out_triple_discount
branch
3 times, most recently
from
8806037
to
bac3b8c
Compare
|
@LorenzoC0 Puoi fixare pre-commit e fare un rebase? |
|
/ocabot rebase |
|
Congratulations, PR rebased to 14.0. |
OCA-git-bot
force-pushed
the
14.0-mig-l10n_it_fatturapa_out_triple_discount
branch
from
bac3b8c
to
eae434a
Compare
eLBati
force-pushed
the
14.0-mig-l10n_it_fatturapa_out_triple_discount
branch
from
eae434a
to
20ee6ce
Compare
eLBati
force-pushed
the
14.0-mig-l10n_it_fatturapa_out_triple_discount
branch
2 times, most recently
from
2e7aab9
to
a66568c
Compare
|
Buongiorno, ma questo modulo è stato inglobato in un altro? |
no, e' a se stante e forse ora siamo arrivati alla migrazione "finale" quella buona dopo diversi tentativi |
|
@LorenzoC0 questi sono stati riabilitati? #1588 |
|
|
@LorenzoC0 @eLBati non penso sia possibile mergiare questa PR senza test robusti, potete aggiungerli? |
There was a problem hiding this comment.
Grazie della PR!
Come già scritto in descrizione, mancano i test automatici: sono d'accordo con #3681 (comment) che in questo caso siano necessari per il merge.
Questo modulo (principalmente a causa delle sue dipendenze) introduce diversi problemi, i test automatici aiutano a mantenere la situazione controllata senza il bisogno di fare test manuali.
Ho da poco fatto la migrazione a 16.0 in #3720, dove tra le altre cose ho riattivato i test automatici, magari può tornare utile.
| if discount_field == "discount2": | ||
| price_unit = line.price_unit * (1 - line.discount / 100) | ||
| return price_unit * line[discount_field] / 100 | ||
| elif discount_field == "discount3": | ||
| price_unit = line.price_unit * (1 - line.discount / 100) | ||
| price_unit = price_unit * (1 - line.discount2 / 100) | ||
| return price_unit * line[discount_field] / 100 | ||
| return line.price_unit * line[discount_field] / 100 |
There was a problem hiding this comment.
Questo codice serve solo nel caso in cui sia installato account_invoice_triple_discount che non è una dipendenza del modulo l10n_it_fatturapa_out, si può spostare in l10n_it_fatturapa_out_triple_discount?
| <ScontoMaggiorazione t-if="line.discount2 != 0 or importo2 != 0"> | ||
| <!-- [2.2.1.10] --> | ||
| <t t-if="importo"> | ||
| <Tipo t-if="importo2 > 0">SC</Tipo> | ||
| <Tipo t-if="importo2 <= 0">MG</Tipo> | ||
| <Importo t-esc="format_numbers(abs(importo2))" /> | ||
| </t> | ||
| <t t-else=""> | ||
| <Tipo t-if="line.discount2 > 0">SC</Tipo> | ||
| <Tipo t-if="line.discount2 <= 0">MG</Tipo> | ||
| <Percentuale t-esc="format_numbers_two(abs(line.discount2))" /> | ||
| </t> | ||
| </ScontoMaggiorazione> | ||
| <ScontoMaggiorazione t-if="line.discount3 != 0 or importo3 != 0"> | ||
| <!-- [2.2.1.10] --> | ||
| <t t-if="importo"> | ||
| <Tipo t-if="importo3 > 0">SC</Tipo> | ||
| <Tipo t-if="importo3 <= 0">MG</Tipo> | ||
| <Importo t-esc="format_numbers(abs(importo3))" /> | ||
| </t> | ||
| <t t-else=""> | ||
| <Tipo t-if="line.discount3 > 0">SC</Tipo> | ||
| <Tipo t-if="line.discount3 <= 0">MG</Tipo> | ||
| <Percentuale t-esc="format_numbers_two(abs(line.discount3))" /> | ||
| </t> | ||
| </ScontoMaggiorazione> |
There was a problem hiding this comment.
È possibile evitare di ripetere lo stesso markup più volte?
Penso sia anche meglio spostare la logica più possibile lato Python perché credo sia più facile da modificare con override e simili
There was a problem hiding this comment.
È possibile ripristinare i test automatici del modulo?
Sono stati disattivati per un vecchio problema e non è detto che sia ancora valido
There was a problem hiding this comment.
Seguendo la procedura descritta in https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-16.0#technical-method-to-migrate-a-module-from-150-to-160-branch, questo commit andrebbe prima del commit di migrazione, è possibile spostarlo?
Altrimenti nel commit di migrazione ci sono modifiche utili solo a far felice pre-commit mentre dovrebbero esserci solo modifiche dovute alla migrazione del modulo
| 'name': 'ITA - Fattura elettronica - Integrazione sconto triplo', | ||
| "summary": "Modulo ponte tra emissione " | ||
| "fatture elettroniche e sconto triplo", | ||
| "version": "12.0.2.0.1", | ||
|
|
||
| "name": "ITA - Fattura elettronica - Integrazione sconto triplo", | ||
| "summary": "Modulo ponte tra emissione " "fatture elettroniche e sconto triplo", |
There was a problem hiding this comment.
Queste modifiche non sono necessarie per la migrazione in sé ma solo per pre-commit, è possibile spostarle nel commit dedicato?
There was a problem hiding this comment.
A quanto ho capito, questo commit è per correggere un errore del modulo account_invoice_triple_discount, che non è nemmeno dipendenza di l10n_it_fatturapa_in, penso sia più corretto correggere invece il modulo account_invoice_triple_discount.
There was a problem hiding this comment.
Non mi risultano errori in account_invoice_triple_discount.
È invece corretto considerare 100.00000000000014 uguale a 100
There was a problem hiding this comment.
Non mi risultano errori in
account_invoice_triple_discount. È invece corretto considerare 100.00000000000014 uguale a 100
Allora non ho capito qualcosa, come mai queste modifiche sono necessarie solo quando account_invoice_triple_discount è presente?
Ho trovato questo tipo di errore nella versione 16.0 di account_invoice_triple_discount e l'ho corretto in OCA/account-invoicing#1606, è possibile che ci sia un problema analogo anche nella versione 14.0.
There was a problem hiding this comment.
Ho portato la OCA/account-invoicing#1606 a v14 con ec3059c490a071e0289bdfdee3bfd49b41444b3a ma non risolve il problema su l10n_it_fatturapa_in:
2023-11-21 16:32:41,749 612 ERROR demo_1 odoo.addons.l10n_it_fatturapa_in.tests.test_import_fatturapa_xml: FAIL: TestFatturaPAXMLValidation.test_33_xml_import
Traceback (most recent call last):
File "/opt/l10n-italy/l10n_it_fatturapa_in/tests/test_import_fatturapa_xml.py", line 664, in test_33_xml_import
self.assertEqual(round(invoice.amount_total, 2), 24.4)
AssertionError: 15.6 != 24.4
There was a problem hiding this comment.
Sì certo nel DB ci possono essere tanti decimali, e ci possono finire se scrivi direttamente sul DB o se manometti la precisione mentre usi l'ORM, ma se usi l'ORM normalmente (record.field = value) i valori vengono arrotondati prima di essere scritti in https://github.com/odoo/odoo/blob/92e95a1f4f8ea914b257e03200f537d8a4bb5e3c/odoo/fields.py#L1350.
I moduli del triplo sconto aumentano la precisione perché (in breve) applicare diverse percentuali in successione può avere necessità di usare più decimali durante il calcolo altrimenti viene sbagliato.
Il valore scritto alla fine del calcolo però viene arrotondato con la precisione originale del campo, infatti in https://github.com/OCA/account-invoicing/blob/830c0f0b74ec0d80b34ed72011a23dcb81f6b29f/account_invoice_triple_discount/models/account_move_line.py#L80 (16.0) la precisione originale viene ripristinata prima di scrivere il valore.
Nonostante il ripristino della precisione però veniva comunque scritto un valore con più decimali, motivo per cui ho fatto OCA/account-invoicing#1606.
Ma poi non ho ancora capito come mai il codice del modulo l10n_fatturapa_in dovrebbe porsi questo tipo di problemi quando non dipende nemmeno da account_invoice_triple_discount.
Vuol dire che dovremmo sempre usare AlmostEqual invece di Equal perché magari prima o poi arriverà un modulo che salva valori con un'altra precisione rispetto a quella precisione dei campi?
There was a problem hiding this comment.
In realtà anche se arrotondati, per un problema noto dei float, si rischia di trovarsi con molte cifre decimali anche se ne sono impostate 2, questo perché non tutti i numeri in virgola mobile possono essere rappresentati correttamete in un sistema binario.
Il caso più conosciuto è la somma di 0.2 + 0.1, per logica è 0.3 per qualsiasi PC / linguaggio è 0.30000000000004
Un errore simile ci sta capitando con delle fatture RC, per cui l'importo della fattura differisce da quello della e-invoice poiché il controllo se c'è differenza tra i due totali, differisce per il numero di cifre decimali
There was a problem hiding this comment.
Vuol dire che dovremmo sempre usare
AlmostEqualinvece diEqualperché magari prima o poi arriverà un modulo che salva valori con un'altra precisione rispetto a quella precisione dei campi?
Beh per i campi float che possono contenere il risultato di un calcolo sì, direi che andrebbe sempre usato AlmostEqual.
Infatti odoo confronta float e monetary utilizzando la precisione impostata a sistema: https://github.com/odoo/odoo/blob/2578c309b970f0f6a8b0bf5ba8711b0562e23940/odoo/tests/common.py#L489
Quindi odoo tendenzialmente usa una precisione ancora inferiore di quella di AlmostEqual
price_unit in particolare non veniva controllato con AlmostEqual perchè non era il risultato di un calcolo, ma di un input; mentre con account_invoice_triple_discount viene ricalcolato se non sbaglio.
There was a problem hiding this comment.
In realtà anche se arrotondati, per un problema noto dei float, si rischia di trovarsi con molte cifre decimali anche se ne sono impostate 2, questo perché non tutti i numeri in virgola mobile possono essere rappresentati correttamete in un sistema binario.
Vuol dire che dovremmo sempre usare
AlmostEqualinvece diEqualperché magari prima o poi arriverà un modulo che salva valori con un'altra precisione rispetto a quella precisione dei campi?Beh per i campi float che possono contenere il risultato di un calcolo sì, direi che andrebbe sempre usato
AlmostEqual.
Capito, quindi questo commit non ha a che fare con triple discount, ma tutt'a un tratto vogliamo usare AlmostEqual per confrontare i float; il fatto che sia lo stesso momento che è apparso il triple discount è una coincidenza evidentemente 😄.
A me continua a non convincere, le mie ragioni le ho spiegate sopra.
Perché allora non vengono modificati tutti gli Equal in AlmostEqual?
price_unitin particolare non veniva controllato conAlmostEqualperchè non era il risultato di un calcolo, ma di un input; mentre conaccount_invoice_triple_discountviene ricalcolato se non sbaglio.
Sì, ma come ho scritto in #3681 (comment) il valore salvato dovrebbe risultare arrotondato correttamente.
There was a problem hiding this comment.
il fatto che sia lo stesso momento che è apparso il triple discount è una coincidenza evidentemente 😄
Perchè mai?
There was a problem hiding this comment.
In questo commit ci sono anche modifiche a un altro modulo, è possibile avere commit che modificano un solo modulo? Vedi https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#commit-message, in particolare:
Avoid commits which simultaneously impact lots of modules. Try to split into different commits where impacted modules are different. This is helpful if we need to revert changes on a module separately.
| <Importo t-esc="format_numbers(abs(importo2))" /> | ||
| </t> | ||
| <t t-else=""> | ||
| <Tipo t-if="line.discount2 > 0">SC</Tipo> | ||
| <Tipo t-if="line.discount2 <= 0">MG</Tipo> | ||
| <Percentuale t-esc="format_numbers_two(abs(line.discount2))" /> |
There was a problem hiding this comment.
Il modulo nella 12.0 aggiungeva solo il nodo Percentuale, è possibile mantenere il vecchio comportamento nella migrazione?
La modifica al comportamento esistente andrebbe almeno in un commit diverso, o meglio ancora in una PR diversa, e secondo https://github.com/OCA/l10n-italy/wiki/Team-di-sviluppo#apertura-issue ci vorrebbe anche una issue dedicata.
Anche perché lo sconto triplo potrebbe portare un importo scontato diverso a seconda di come vengono applicati gli sconti, ad esempio additivo o moltiplicativo o chissà cos'altro ci si può inventare con gli override di https://github.com/OCA/account-invoicing/blob/bab944e76aa71c3f300695fc90a4656a9055da4e/account_invoice_triple_discount/models/account_move_line.py#L97
…API and to new file names
it fixes: - removes xs:dateTime if bogus and not mandatory i.e. 0001-01-01T00:00:00.000+02:00 where python raises OverflowError - removes timezone from xs:date to make pyxb able to compare with 1-1-1970, it also removes the need of patching pyxb - removes space only strings if not mandatory, else replace with a dash breaking change: modules needs to import binding.fatturapa instead of bindings.fatturapa_v_1_2, this would be asl useful for new specs
…e now rounded to the nearest even result instead of away from zero". See https://docs.python.org/3/whatsnew/3.0.html#builtins Examples: >>> '%.2f' % 75.845 '75.84' >>> '%.2f' % 75.855 '75.86' >>> round(2.5) 2 >>> round(3.5) 4
…scount: modifica del type di Importo in ScontoMaggiorazioneType (OCA#1875) * [IMP] l10n_it_fatturapa_out: change precision_rounding of ScontoMaggiorazioneType according to e-invoicing specs 1.6 * [IMP] l10n_it_fatturapa_out_triple_discount: change precision_rounding of ScontoMaggiorazioneType according to e-invoicing specs 1.6
Currently translated at 100.0% (1 of 1 strings) Translation: l10n-italy-12.0/l10n-italy-12.0-l10n_it_fatturapa_out_triple_discount Translate-URL: https://translation.odoo-community.org/projects/l10n-italy-12-0/l10n-italy-12-0-l10n_it_fatturapa_out_triple_discount/it/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: l10n-italy-12.0/l10n-italy-12.0-l10n_it_fatturapa_out_triple_discount Translate-URL: https://translation.odoo-community.org/projects/l10n-italy-12-0/l10n-italy-12-0-l10n_it_fatturapa_out_triple_discount/
eLBati
force-pushed
the
14.0-mig-l10n_it_fatturapa_out_triple_discount
branch
from
a66568c
to
8dd0e03
Compare
|
Chiudo in favore di #3743 |
Prendendo spunto dalla vecchia PR ho sistemato un po' la logica della creazione dell'XML. Dai test manuali funziona tutto quanto, sarebbe necessario creare dei nuovi test automatizzati.
Dipende da OCA/account-invoicing#1592