Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

14.0 mig l10n it fatturapa out wt #2386

Closed

Conversation

CiroBoxHub
Copy link
Contributor

@CiroBoxHub CiroBoxHub commented Aug 6, 2021

Descrizione del problema o della funzionalità:

Porting del modulo l10n_it_fatturapa_out_wt per l'inserimento nel xml della fattura dei fati ritenuta

Condizioni:
E' necessario che la PR #2388 venga mergiata per il corretto funzionamento di questa

--
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

@OCA-git-bot
Copy link
Contributor

Hi @CiroBoxHub! Thank you very much for this contribution. As the addon you are improving does not have a declared maintainer, I take the opportunity to mention that you can consider adopting it. To do so, please read the maintainer role description, and, if interested, create a pull request to add your GitHub login to the maintainers key of the addon manifest.

@CiroBoxHub CiroBoxHub marked this pull request as draft August 6, 2021 08:01
@CiroBoxHub CiroBoxHub mentioned this pull request Aug 6, 2021
74 tasks
TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Aug 6, 2021
@francescapenso
Copy link

Solo in caso di note di credito non viene creata la stanza e tutte quelle al suo interno.
IN questa stanza è indicato anche il netto a pagare.
Per le ft normali (receivable) invece la crea correttamente

TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Aug 20, 2021
@CiroBoxHub
Copy link
Contributor Author

E' necessario che la PR #2388 venga mergiata per il corretto funzionamento di questa

TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Sep 3, 2021
TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Sep 17, 2021
TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Oct 1, 2021
TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Oct 8, 2021
@SimoRubi
Copy link
Member

E' necessario che la PR #2388 venga mergiata per il corretto funzionamento di questa

@CiroBoxHub la #2388 è stata mergiata, puoi fare rebase?

@tafaRU
Copy link
Member

tafaRU commented Oct 12, 2021

/ocabot rebase

@OCA-git-bot
Copy link
Contributor

Congratulations, PR rebased to 14.0.

@OCA-git-bot OCA-git-bot force-pushed the 14.0-mig-l10n_it_fatturapa_out_wt branch from 883ce31 to 2fe8c0a Compare October 12, 2021 15:21
@CiroBoxHub CiroBoxHub marked this pull request as ready for review October 12, 2021 18:11
Copy link
Member

@SimoRubi SimoRubi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Puoi correggere Travis?
Falliscono proprio i test di questo modulo:

2021-10-12 17:01:19,637 7090 ERROR openerp_test odoo.addons.l10n_it_fatturapa_out_wt.tests.test_fatturapa_wt: FAIL: TestInvoiceWT.test_e_invoice_wt_enas_1
Traceback (most recent call last):
File "/home/travis/build/OCA/l10n-italy/setup/l10n_it_fatturapa_out_wt/odoo/addons/l10n_it_fatturapa_out_wt/tests/test_fatturapa_wt.py", line 286, in test_e_invoice_wt_enas_1
module_name="l10n_it_fatturapa_out_wt",
File "/home/travis/build/OCA/l10n-italy/setup/l10n_it_fatturapa_out/odoo/addons/l10n_it_fatturapa_out/tests/fatturapa_common.py", line 198, in check_content
self.assertEqual(etree.tostring(test_fatt), etree.tostring(xml))
AssertionError: b'<ns[2754 chars]ogo><DatiRiepilogo><AliquotaIVA>0.00</Aliquota[505 chars]ica>' != b'<ns[2754 chars]ogo></DatiBeniServizi><DatiPagamento><Condizio[282 chars]ica>'
2021-10-12 17:01:19,638 7090 INFO openerp_test odoo.addons.l10n_it_fatturapa_out_wt.tests.test_fatturapa_wt: Starting TestInvoiceWT.test_e_invoice_wt_enas_2 ...
2021-10-12 17:01:20,931 7090 INFO openerp_test odoo.addons.l10n_it_fatturapa_out_wt.tests.test_fatturapa_wt: ======================================================================
2021-10-12 17:01:20,931 7090 ERROR openerp_test odoo.addons.l10n_it_fatturapa_out_wt.tests.test_fatturapa_wt: FAIL: TestInvoiceWT.test_e_invoice_wt_enas_2
Traceback (most recent call last):
File "/home/travis/build/OCA/l10n-italy/setup/l10n_it_fatturapa_out_wt/odoo/addons/l10n_it_fatturapa_out_wt/tests/test_fatturapa_wt.py", line 345, in test_e_invoice_wt_enas_2
module_name="l10n_it_fatturapa_out_wt",
File "/home/travis/build/OCA/l10n-italy/setup/l10n_it_fatturapa_out/odoo/addons/l10n_it_fatturapa_out/tests/fatturapa_common.py", line 198, in check_content
self.assertEqual(etree.tostring(test_fatt), etree.tostring(xml))
AssertionError: b'<ns[2732 chars]o>10.83</ImponibileImporto><Imposta>0.00</Impo[418 chars]ica>' != b'<ns[2732 chars]o>10.00</ImponibileImporto><Imposta>0.00</Impo[418 chars]ica>'

(da https://app.travis-ci.com/github/OCA/l10n-italy/jobs/542735700#L2184-L2200)

TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Oct 15, 2021
@tafaRU
Copy link
Member

tafaRU commented Oct 22, 2021

@CiroBoxHub pre-commit fallisce: puoi controllare?

@CiroBoxHub
Copy link
Contributor Author

@SimoRubi @TheMule71 ho dovuto riscrivere la funzione perché due test fallivano per dei dati che non venivano scritti

@CiroBoxHub
Copy link
Contributor Author

@CiroBoxHub pre-commit fallisce: puoi controllare?

Ciao @tafaRU dovrei aver sistemato

Copy link
Member

@SimoRubi SimoRubi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grazie della PR!
Puoi ridurre un po' il numero di commit?
Non credo siano tutti essenziali, ti puoi mettere d'accordo con @TheMule71 per gestire il fixup e farlo diventare un commit normale se non lo volete squashare per l'authorship

@@ -1,28 +0,0 @@
# Translation of Odoo Server.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Come mai non c'è più la traduzione in italiano?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

SdI expects a summary for every tax mentioned in the invoice,
even those with price_total == 0.
"""
out_computed = {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Qui viene in pratica riscritto il metodo presente in super, è possibile evitarlo e riutilizzare il codice esistente?
Forse ne puoi parlare con @TheMule71 che ha scritto il metodo in super e potete mettere a punto un refactoring.

Copy link
Contributor Author

@CiroBoxHub CiroBoxHub Oct 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SimoRubi cosa intendi per "presente in super"? ho dovuto scrivere un nuovo metodo riportando il codice della funzione originale perché la super di una funzione innestata non viene eseguito in nessun modo. Neanche l'override funziona. Questo mi permette di poter aggiungere il mio codice in coda a quello già eseguito senza perdere i dati già presente

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cosa intendi per "presente in super"?

Intendo che questo stesso codice è già presente in

def get_all_taxes(record):
quindi copiarlo con qualche modifica non è secondo me la cosa migliore perché se viene aggiornato/corretto il codice linkato, tale modifica dovrà essere riportata a mano qui.

Ok che il super di una funzione innestata non esiste ma forse si potrebbe recuperare la funzione originale dal risultato di get_template_values e aggiungere le modifiche che ti interessano, ad esempio:

def get_template_values(self):
    template_values = super().get_template_values()
    [...]
    def get_all_taxes_extend(record):
        original_res = template_values['all_taxes'][record.id](record)  # tipo super()
        # Aggiungi quello che ti serve

Il modo migliore però secondo me sarebbe rendere queste funzioni più facilmente raggiungibili/modificabili dai moduli dipendenti, come scrivevo ti potresti coordinare con @TheMule71 che le ha scritte in origine per fare un refactoring.

)
index += 1
return res
def _get_efattura_class(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Questo non mi pare venga utilizzato, si può rimuovere?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

# tested 2021/08/06 @TheMule71

# XXX - a company named "YourCompany" alread exists
# we move it out of the way but we should do better here
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Come mai non si può modificare la company esistente?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Come mai non si può modificare la company esistente?

Perché non è quella giusta (intesa come id). La company che usiamo è ereditata da AccountTestInvoicingCommon(), e si chiama (prima della nostre modifiche) 'company_1_data'. Tutto il setup avviene usando quella come company, per es. self.sales_journal.company_id punta a quella.

La vecchia 'Your Company' è quella dei dati demo, che manca comunque di tutti i dati per riuscire a fatturare XML.

Quindi l'alternativa è

  • o si usa 'Your Company' dei dati demo, la si modifica per i dati di fatturapa, e poi si rifà il tutto il setup (conti, tasse, ecc) usando quella,
  • o si usa il setup di AccountTestInvoicingCommon() e lo si estende per la fatturarazione elettronica, col solo problema di settare il nome a 'Your Company', questo per non dover cambiare tutti gli XML dei test. Per poter cambiare nome occorre rinominare quella omonima della demo.

Io ho preferito la seconda, anche se come ho detto spesso, la vera soluzione sarebbe di concentrare il setup in l10n_it_fatturapa e creare un framework di test estendendo quello standard di AccountTestInvoicingCommon() che poi tutti gli altri moduli l10n_fatturapa* possano usare. Ma il refactoring dell'infrastrutta di test è stato spostato successivamente alla fase di migrazione. Al momento ciascun modulo prende company_1_data e se la adatta in modo che generi gli XML esattamente così com'erano nella 12.

Poi per carità in fase di refactoring se decidiamo che usare AccountTestInvoicingCommon() è più scomodo che altro, possiamo farci la nostra classe non derivata.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Come detto stamattina in chiamata, rimandiamo la scrittura di test decenti a dopo #2507

@@ -1,28 +0,0 @@
# Translation of Odoo Server.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

# tested 2021/08/06 @TheMule71

# XXX - a company named "YourCompany" alread exists
# we move it out of the way but we should do better here
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Come detto stamattina in chiamata, rimandiamo la scrittura di test decenti a dopo #2507

SdI expects a summary for every tax mentioned in the invoice,
even those with price_total == 0.
"""
out_computed = {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cosa intendi per "presente in super"?

Intendo che questo stesso codice è già presente in

def get_all_taxes(record):
quindi copiarlo con qualche modifica non è secondo me la cosa migliore perché se viene aggiornato/corretto il codice linkato, tale modifica dovrà essere riportata a mano qui.

Ok che il super di una funzione innestata non esiste ma forse si potrebbe recuperare la funzione originale dal risultato di get_template_values e aggiungere le modifiche che ti interessano, ad esempio:

def get_template_values(self):
    template_values = super().get_template_values()
    [...]
    def get_all_taxes_extend(record):
        original_res = template_values['all_taxes'][record.id](record)  # tipo super()
        # Aggiungi quello che ti serve

Il modo migliore però secondo me sarebbe rendere queste funzioni più facilmente raggiungibili/modificabili dai moduli dipendenti, come scrivevo ti potresti coordinare con @TheMule71 che le ha scritte in origine per fare un refactoring.

)
index += 1
return res
def _get_efattura_class(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

TheMule71 added a commit to TheMule71/l10n-italy that referenced this pull request Jan 21, 2022
TheMule71 added a commit to TheMule71/l10n-italy that referenced this pull request Jan 21, 2022
Copy link
Contributor

@andreampiovesana andreampiovesana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

go

Lara Baggio and others added 19 commits May 12, 2022 14:45
* [IMP] extend wt_types selection with new code and add tax_id for daticassa

* [IMP] use causale_pagamento for all withholding tax type

* [IMP] improve wizard_export_fatturapa with new 1.6 xml tack specific
…CA#1878)

* [12.0][FIX] fatturapa withholding tax invoice received - e-invoice 1.6

* [FIX] l10n_it_fatturapa_out_wt pep8 fixes

This reverts commit 421ef9e51634383263638e9b52ea4038829f62ec.

* [IMP] migration script

* [FIX] l10n_it_fatturapa_out_wt: Fixed ordering in DatiRitenuta generation, according to XML test file.
Otherwise:
2020-09-17 07:34:14,789 7808 ERROR openerp_test odoo.addons.l10n_it_fatturapa_out_wt.tests.test_fatturapa_wt: ` AssertionError: b'<ns[1677 chars]a>RT02</TipoRitenuta><ImportoRitenuta>2.00</Im[1155 chars]ica>' != b'<ns[1677 chars]a>RT04</TipoRitenuta><ImportoRitenuta>0.83</Im[1155 chars]ica>'

Co-authored-by: SimoRubi <[email protected]>
Currently translated at 100.0% (2 of 2 strings)

Translation: l10n-italy-12.0/l10n-italy-12.0-l10n_it_fatturapa_out_wt
Translate-URL: https://translation.odoo-community.org/projects/l10n-italy-12-0/l10n-italy-12-0-l10n_it_fatturapa_out_wt/it/
@TheMule71
Copy link
Contributor

Dipende da #2787.

TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Jun 29, 2022
TheMule71 added a commit to odoo-italia/l10n-italy that referenced this pull request Jun 29, 2022
@tafaRU
Copy link
Member

tafaRU commented Jul 22, 2022

@TheMule71 la si può chiudere a favore di #2848?

@TheMule71
Copy link
Contributor

Si

@TheMule71 TheMule71 closed this Jul 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.