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

[10.0] FE 1.6 #1986

Merged
merged 9 commits into from
Dec 28, 2020
Merged

[10.0] FE 1.6 #1986

merged 9 commits into from
Dec 28, 2020

Conversation

SimoRubi
Copy link
Member

Fattura elettronica 1.6 per v10.

Modifiche fatte a partire da https://github.com/LevelPrime/l10n-italy/tree/10.0-l10n_it_fatturapa-1.6-wt che a sua volta include quanto presente in #1899.

cc @CiroBoxHub @stevech091

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

robyf70 and others added 4 commits December 17, 2020 14:40
[10.0][IMP] l10n_it_fiscal_payment_term: Updated payment terms

[10.0][IMP] l10n_it_fatturapa_out: CodiceValore is now String35LatinExtType (no encoding required) (#14)

[10.0][IMP] l10n it account tax kind fatturapa 1.6

[10.0][IMP] importo bollo non più obbligatorio e-invoice 1.6

[10.0][IMP] aggiunti fiscal document types fatturapa 1.6

[10.0][IMP] l10n_it_causali_pagamento: modify casuale.pagamento records according to e-invoicing specs 1.6

[10.0][12.0][IMP] l10n_it_fatturapa_out and l10n_it_fatturapa_out_triple_discount: modifica del type di Importo in ScontoMaggiorazioneType

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

[10.0][l10n_it_fatturapa_in] Arrotonda a 2 decimali gli arrotondamenti delle fatture di acquisto

[10.0][l10n_it_fatturapa_in] Arrotonda a 2 decimali gli arrotondamenti delle fatture di acquisto

[10.0][IMP][WIP] Withholding tax fattura elettonica v1.6
Set default tax during e-invoice import to user company's supplier tax

Align exported XML files for tests with product precision for e-invoice

This reverts commit 83b745d.

Check original strings, not translations

Create correct type of wt

Fix amount check, considering rounding account

Consider rounding during amount computation

Add rounding account.move.line

10.00000000 is not valid for Percentuale

Fix apostrophe for payment reasons

Fix flake8

Add demo taxes for withholding tax tests

Attachment content is not in bytes

Dedicated precision for PrezzoUnitario generation
@OCA-git-bot
Copy link
Contributor

Hi @sergiocorato, @eLBati,
some modules you are maintaining are being modified, check this out!

@CiroBoxHub
Copy link
Contributor

👍

Copy link
Contributor

@scigghia scigghia left a comment

Choose a reason for hiding this comment

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

👍 @CiroBoxHub @SimoRubi @robyf70
bravi ragazzi

Copy link

@marco-marchiori marco-marchiori left a comment

Choose a reason for hiding this comment

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

functional review
testata con import di

  • fattura professionista
  • fattura agente
  • natura 3.4
  • tipo TD24
  • sistema emittente
  • export ft natura 3.4
    Tutti i documenti vengono correttamente importati

Nota1 (possibile miglioramento) il TD da documento non viene riportato in altri dati fattura
Nota2 ritenuta enasarco aggiunta manualmente

Copy link
Member

@tafaRU tafaRU left a comment

Choose a reason for hiding this comment

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

Nella versione 12.0 all'interno del modulo l10n_it_fatturapa_in è presente la cartella migrations:
image
qui invece no.

Verificare quali sono gli script da includere e in quali PR sono state aggiunte nella 12.0.
L'ultimo in ordine di tempo è stato aggiunto da: #1914

@CiroBoxHub
Copy link
Contributor

Nella versione 12.0 all'interno del modulo l10n_it_fatturapa_in è presente la cartella migrations:
image
qui invece no.

Verificare quali sono gli script da includere e in quali PR sono state aggiunte nella 12.0.
L'ultimo in ordine di tempo è stato aggiunto da: #1914

@tafaRU posso fare il cherry pick di questo? Secondo te è sufficiente?
e15f365#diff-58177127198cd6b674eb96da8b4009c2ae27c01966d990115dc8d0f756e74fc8

@tafaRU
Copy link
Member

tafaRU commented Dec 18, 2020

Secondo te è sufficiente?

@CiroBoxHub non ho verificato, infatti scrivevo:

Verificare quali sono gli script da includere

So però che ci sta lavorando @SimoRubi attenderei pertanto una sua risposta prima di procedere.

@SimoRubi
Copy link
Member Author

SimoRubi commented Dec 18, 2020

@tafaRU delle 3 migrazioni presenti in v12:

  • 12.0.2.0.0: l'ho provata in locale con un nostro cliente e ottengo fatture con un warning tipo:
    E-bill contains ImportoRitenuta 0.0 but created invoice has got 260.0
    
    Questo perché le righe delle ritenute vengono generate, ma l'importo della ritenuta proveniente dalla FE importata non viene migrato nei loro nuovi campi amount.
    Attualmente non sto trovando dove in v10 tale importo veniva salvato (in v12 pare fosse ftpa_withholding_amount che però non esiste più), anzi pare che in fase di import venisse salvato solo il tipo (rif.
    invoice_data['ftpa_withholding_type'] = Withholding.TipoRitenuta
    ) ma non l'importo.
    Un'idea che ho è copiare l'importo di account.invoice.withholding_tax_amount dentro una riga delle ritenute create, ma non credo sia del tutto corretto.
  • 12.0.1.18.3: popola il campo invoices_date che non esiste, quindi direi che non serve
  • 12.0.1.15.0: allinea il campo ftpa_withholding_amount con la ritenuta nella fattura Odoo, direi che non serve perché verrà già popolato correttamente dalla migrazione in 12.0.2.0.0

In tutto ciò potrei essermi perso qualche concetto fondamentale sulle ritenute perché le conosco un po' così, se qualcuno ha idee su come andrebbe fatta la migrazione scriva pure qui :)
Forse @sergiocorato che facesti la migrazione in v12 hai qualche idea?

EDIT: ho pushato qui sotto quella che è la mia idea, mi farete sapere voi se ha senso o meno.
Nel DB dove ho provato, non c'è più nessun warning.

Copy link

@stevech091 stevech091 left a comment

Choose a reason for hiding this comment

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

Test funzionali ok anche con migrazione dati ritenute d'acconto.

@SimoRubi SimoRubi requested a review from tafaRU December 21, 2020 16:54
@tafaRU
Copy link
Member

tafaRU commented Dec 22, 2020

@marco-marchiori potresti aggiornare la tua review dopo le ultime modifiche apportate?

@tafaRU
Copy link
Member

tafaRU commented Dec 23, 2020

  • Attualmente non sto trovando dove in v10 tale importo veniva salvato (in v12 pare fosse ftpa_withholding_amount che però non esiste più), anzi pare che in fase di import venisse salvato solo il tipo (rif.
    invoice_data['ftpa_withholding_type'] = Withholding.TipoRitenuta

    ) ma non l'importo.

@SimoRubi confermo la tua analisi: nella 12.0 veniva gestito anche ImportoRitenuta come si evince da https://github.com/OCA/l10n-italy/pull/1873/files#diff-a5effb15a448aa412fb06bb6e1b06cc44f9dc2653f43f339942d28d6ec5a9af3L1223-L1224

Un'idea che ho è copiare l'importo di account.invoice.withholding_tax_amount dentro una riga delle ritenute create, ma non credo sia del tutto corretto.

Ti riferisci a 2bc9c68#diff-16188e939fe351db410e56d1a5b9388829d66b327467a2ab47e1a17677521163L13-R13 giusto?
L'alternativa è non migrare affatto il contenuto di quel campo ma dal momento che @stevech091 ha approvato la modifica assumerei che tale soluzione sia accettabile 👍

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

Copy link
Contributor

@robyf70 robyf70 left a comment

Choose a reason for hiding this comment

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

LGTM

@tafaRU
Copy link
Member

tafaRU commented Dec 23, 2020

Attendo la review aggiornata da parte di @marco-marchiori prima di mergiare così come richiesto in #1986 (comment).

Copy link

@marco-marchiori marco-marchiori left a comment

Choose a reason for hiding this comment

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

Testati gli ultimi cambiamenti. Nessun errore
Fattura avvocato: importata, validata
Fattura agente: importata, validata NB non mi riporta le due ritenute, anche se le rileva
Fattura natura N3.4 importata, validata
Fattura con "sistemaemittente" importata, validata
Fattura con TD24 importata, validata

@tafaRU
Copy link
Member

tafaRU commented Dec 23, 2020

Fattura agente: importata, validata NB non mi riporta le due ritenute, anche se le rileva

@marco-marchiori sulla 12.0 si ha lo stesso comportamento?
É per capire se non ci siamo persi qualche modifica durante il backporting. In caso di risposta affermativa possiamo tenerne traccia aprendo una nuova issue e procedere con il merge della PR corrente.

@marco-marchiori
Copy link

@tafaRU in effetti ho visto ora lo stesso comportamento nella v12 per la stessa fattura, in caso di fattura agente -con due ritenute- non me le riporta ma devo riprenderle a mano

@tafaRU
Copy link
Member

tafaRU commented Dec 28, 2020

Ok @marco-marchiori grazie, puoi aprire una issue in cui descrivi il problema?

@tafaRU
Copy link
Member

tafaRU commented Dec 28, 2020

/ocabot merge major

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 10.0-ocabot-merge-pr-1986-by-tafaRU-bump-major, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit b4ff0e6 into OCA:10.0 Dec 28, 2020
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at d68b616. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants