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][FIX] l10n_it_fatturapa_out: Configurazione in azienda per numero massimo fatture per fattura elettronica #2606

Open
wants to merge 2 commits into
base: 14.0
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions l10n_it_fatturapa_out/models/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,26 @@ def _compute_fatturapa_state(self):

def preventive_checks(self):
for invoice in self:
if invoice.state != "posted":
raise UserError(
_("Impossible to generate XML: invoice not posted: %s")
% invoice.name
)

if not invoice.is_sale_document():
raise UserError(
_("Impossible to generate XML: not a customer invoice: %s")
% invoice.name
)

if not invoice.fiscal_document_type_id:
Copy link
Member

Choose a reason for hiding this comment

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

Sto guardando le specifiche in https://www.agenziaentrate.gov.it/portale/web/guest/specifiche-tecniche-versione-1.7 perché il link nel README a https://www.fatturapa.gov.it/export/fatturazione/it/normativa/f-2.htm risponde 404.
Per il nodo TipoDocumento vedo che c'è anche un vincolo di lunghezza a 4 caratteri mentre in

code = fields.Char(string="Code", size=5, required=True)
il campo è al massimo lungo 5, sai mica come mai?
Visto che con queste modifiche viene aggiunto un controllo dedicato a questo campo, forse sarebbe anche da controllare che la lunghezza sia corretta, che ne pensi?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Direi che va modificato dall'altra parte, in l10n_it_fiscal_document_type. Io il controllo lo potrei mettere, ma se la size diventa 4 di là non serve.

Copy link
Member

Choose a reason for hiding this comment

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

Direi che va modificato dall'altra parte, in l10n_it_fiscal_document_type. Io il controllo lo potrei mettere, ma se la size diventa 4 di là non serve.

Potrebbe essere che esistano dei document type che possono avere lunghezza 5, ma quelli che si possono inserire in una FE devono invece avere lunghezza 4? Anzi, per la FE vedo nel XSD può avere solo certi valori TD01, ..., TD27

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Direi che va modificato dall'altra parte, in l10n_it_fiscal_document_type. Io il controllo lo potrei mettere, ma se la size diventa 4 di là non serve.

Potrebbe essere che esistano dei document type che possono avere lunghezza 5, ma quelli che si possono inserire in una FE devono invece avere lunghezza 4? Anzi, per la FE vedo nel XSD può avere solo certi valori TD01, ..., TD27

Dovrebbe essere un campo che serve solo a fatturapa, c'è una tabella che sono i valori dell'XML: https://github.com/OCA/l10n-italy/blob/fd80fdc7d4aeef613e7aa193674cd62cf0bd4f7b/l10n_it_fiscal_document_type/data/fiscal.document.type.csv

Non ho idea del perché sia size=5

Copy link
Member

Choose a reason for hiding this comment

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

Se stiamo controllando i dati della fattura in modo che poi la generazione del XML non esploda, allora andrebbe controllato che il codice del fiscal_document_type_id rientri tra quelli consentiti nel XSD del XML.

Controllare che almeno sia valorizzato il campo fiscal_document_type_id è comunque un miglioramento rispetto al codice esistente perché il nodo TipoDocumento è obbligatorio, quindi per me può andare bene anche così.

raise UserError(
_(
"Invoice %s fiscal document type must be set",
invoice.name,
)
)

if (
invoice.invoice_payment_term_id
and invoice.invoice_payment_term_id.fatturapa_pt_id.code is False
Expand All @@ -81,6 +95,28 @@ def preventive_checks(self):
)
)

if not invoice.partner_id.city:
Copy link
Member

Choose a reason for hiding this comment

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

Sto guardando le specifiche in https://www.agenziaentrate.gov.it/portale/web/guest/specifiche-tecniche-versione-1.7 perché il link nel README a https://www.fatturapa.gov.it/export/fatturazione/it/normativa/f-2.htm risponde 404.
Da quel che ho capito (penso che qui tu stia risolvendo #2563 (comment)), il nodo obbligatorio è Comune, ed è obbligatorio ogni volta che compare nel XML.

Il template però prende il campo city non solo del partner della fattura:

<t t-call="l10n_it_fatturapa_out.account_invoice_it_FatturaPA_sede">
<t t-set="partner_id" t-value="partner_id" />
</t>

ma anche del partner della company:
<t t-call="l10n_it_fatturapa_out.account_invoice_it_FatturaPA_sede">
<t t-set="partner_id" t-value="company_id.partner_id" />
</t>

e altri, tipo:
<t t-call="l10n_it_fatturapa_out.account_invoice_it_FatturaPA_sede">
<t
t-set="partner_id"
t-value="company_id.fatturapa_stabile_organizzazione"
/>
</t>

quindi se vogliamo fare controlli preventivi per non far fallire la generazione del XML, ci sarebbe da controllare il campo city di tutti i partner coinvolti, che ne pensi?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fatto.

Copy link
Member

Choose a reason for hiding this comment

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

👍🏻

raise UserError(_("Invoice %s partner city must be set", invoice.name))

if not invoice.company_id.partner_id.city:
raise UserError(
_(
"Invoice %s city in our company's partner must be set",
invoice.name,
)
)

if (
invoice.company_id.fatturapa_stabile_organizzazione
and not invoice.company_id.fatturapa_stabile_organizzazione.city
):
raise UserError(
_(
"Invoice %s city in our company's Stable Organization must be set",
invoice.name,
)
)

if not all(
aml.tax_ids for aml in invoice.invoice_line_ids if aml.product_id
):
Expand Down
4 changes: 3 additions & 1 deletion l10n_it_fatturapa_out/models/partner.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ class ResPartner(models.Model):
max_invoice_in_xml = fields.Integer(
string="Max Invoice # in XML",
default=lambda self: self.env.company.max_invoice_in_xml,
help="Maximum number of invoices to group in a single " "XML file. 0=Unlimited",
help="Maximum number of invoices to group in a single XML file."
"0=Unlimited. If this is 0, then the number configured "
"in the account settings is considered.",
)

@api.constrains("max_invoice_in_xml")
Expand Down
11 changes: 9 additions & 2 deletions l10n_it_fatturapa_out/tests/fatturapa_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,16 @@ def set_sequences(
seq_date = inv_seq._create_date_range_seq(dt)
seq_date.number_next_actual = invoice_number

def run_wizard(self, invoice_id):
def run_wizard(self, invoice_ids):
"""
Execute the export wizard on the invoices having ID `invoice_ids`.
:param invoice_ids: integer or list of integers
:return: result of export wizard
"""
if not isinstance(invoice_ids, list):
invoice_ids = [invoice_ids]
wizard = self.wizard_model.create({})
return wizard.with_context({"active_ids": [invoice_id]}).exportFatturaPA()
return wizard.with_context({"active_ids": invoice_ids}).exportFatturaPA()

def set_e_invoice_file_id(self, e_invoice, file_name):
# We need this because file name is random and we can't predict it
Expand Down
5 changes: 5 additions & 0 deletions l10n_it_fatturapa_out/tests/test_fatturapa_out_noteline.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ def test_1_note_taxes(self):
line_form.name = "just a note"
line_form.account_id = self.env["account.account"]
invoice = move_form.save()
invoice.action_post()

res = self.run_wizard(invoice.id)
self.assertTrue(res)
Expand Down Expand Up @@ -85,6 +86,7 @@ def test_2_note_taxes(self):
line_form.name = "just a note"
line_form.account_id = self.env["account.account"]
invoice = move_form.save()
invoice.action_post()

res = self.run_wizard(invoice.id)
self.assertTrue(res)
Expand Down Expand Up @@ -131,6 +133,7 @@ def test_3_note_taxes(self):
line_form.name = "just a note"
line_form.account_id = self.env["account.account"]
invoice = move_form.save()
invoice.action_post()

res = self.run_wizard(invoice.id)
self.assertTrue(res)
Expand Down Expand Up @@ -184,6 +187,7 @@ def test_4_note_taxes(self):
with move_form.invoice_line_ids.edit(0) as line_form:
line_form.price_unit = 0.0

invoice.action_post()
res = self.run_wizard(invoice.id)
self.assertTrue(res)

Expand Down Expand Up @@ -221,6 +225,7 @@ def test_5_note_tax_kind_id(self):
line_form.name = "just a note"
line_form.account_id = self.env["account.account"]
invoice = move_form.save()
invoice.action_post()

res = self.run_wizard(invoice.id)
self.assertTrue(res)
Expand Down
166 changes: 166 additions & 0 deletions l10n_it_fatturapa_out/tests/test_fatturapa_xml_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -960,6 +960,7 @@ def test_multicompany_fail(self):
company_form = Form(self.env["res.company"].sudo(True))
company_form.name = "YourCompany 2"
company_form.vat = "IT07973780013"
company_form.city = "Roma"
company_form.fatturapa_fiscal_position_id = (
self.env.company.fatturapa_fiscal_position_id
)
Expand Down Expand Up @@ -1199,3 +1200,168 @@ def test_18_xml_export(self):
# XML doc to be validated
xml_content = base64.decodebytes(attachment.datas)
self.check_content(xml_content, "IT06363391001_00018.xml")

def test_max_invoice_in_xml(self):
invoice1_form = Form(
self.env["account.move"].with_context({"default_move_type": "out_invoice"})
)
invoice1_form.partner_id = self.res_partner_fatturapa_0
with invoice1_form.line_ids.new() as line_form:
line_form.product_id = self.product_product_10
line_form.account_id = self.a_sale
line_form.tax_ids.clear()
line_form.tax_ids.add(self.tax_22)
invoice1 = invoice1_form.save()
invoice2 = invoice1.copy()
invoice3 = invoice1.copy()

invoice1.action_post()
invoice2.action_post()
invoice3.action_post()
invoices = invoice1 | invoice2 | invoice3

# partner limited, company limited (expect 3 xml attachments)
self.res_partner_fatturapa_0.max_invoice_in_xml = 1
self.env.company.max_invoice_in_xml = 2
res = self.run_wizard(invoices.ids)
attachments = self.attach_model.search(res["domain"])
self.assertEqual(len(attachments), 3)
attachments.unlink()

# partner limited, company unlimited (expect 3 xml attachments)
self.res_partner_fatturapa_0.max_invoice_in_xml = 1
self.env.company.max_invoice_in_xml = 0
res = self.run_wizard(invoices.ids)
attachments = self.attach_model.search(res["domain"])
self.assertEqual(len(attachments), 3)
attachments.unlink()

# partner unlimited, company limited (expect 2 xml attachments)
self.res_partner_fatturapa_0.max_invoice_in_xml = 0
self.env.company.max_invoice_in_xml = 2
res = self.run_wizard(invoices.ids)
attachments = self.attach_model.search(res["domain"])
self.assertEqual(len(attachments), 2)
attachments.unlink()

# partner unlimited, company unlimited (expect 1 xml attachment)
self.res_partner_fatturapa_0.max_invoice_in_xml = 0
self.env.company.max_invoice_in_xml = 0
res = self.run_wizard(invoices.ids)
attachments = self.attach_model.browse(res["res_id"])
self.assertEqual(len(attachments), 1)

def test_preventive_checks(self):
invoice_form = Form(
self.env["account.move"].with_context({"default_move_type": "in_invoice"})
)
invoice_form.partner_id = self.res_partner_fatturapa_0
invoice_form.invoice_date = fields.Date.today()
with invoice_form.line_ids.new() as line_form:
line_form.product_id = self.product_product_10
line_form.account_id = self.a_sale
line_form.tax_ids.clear()
line_form.tax_ids.add(self.tax_22)
invoice = invoice_form.save()

with self.assertRaises(UserError) as ue:
self.run_wizard(invoice.id)
self.assertIn(invoice.name, ue.exception.args[0])
self.assertIn("invoice not posted", ue.exception.args[0])

invoice.action_post()

with self.assertRaises(UserError) as ue:
self.run_wizard(invoice.id)
Copy link
Member

Choose a reason for hiding this comment

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

La fattura non dovrebbe essere confermata prima di generare il file XML?
Strano che il wizard di export permetta di esportare fatture non confermate

Copy link
Contributor Author

@TheMule71 TheMule71 Jan 28, 2022

Choose a reason for hiding this comment

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

Ho aggiunto anche quel controllo.

Copy link
Member

Choose a reason for hiding this comment

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

👍🏻

self.assertIn(invoice.name, ue.exception.args[0])
Copy link
Member

Choose a reason for hiding this comment

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

Puoi chiarire quale dev'essere l'errore sollevato qui e negli assert successivi?
Devono essere errori per i nuovi check sul campo city o fiscal_document, o forse si vuole verificare che salti uno dei check già esistenti?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adesso faccio il controllo anche sul testo del messaggio (che è autoesplicativo). Dovrebbe cambiare con la localizzazione ma i test in github credo possiamo assumere siano in inglese.

Copy link
Member

Choose a reason for hiding this comment

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

👍🏻

self.assertIn("not a customer invoice", ue.exception.args[0])

# everything's fine with this invoice
invoice_form = Form(
self.env["account.move"].with_context({"default_move_type": "out_invoice"})
)
invoice_form.partner_id = self.res_partner_fatturapa_0
invoice_form.invoice_payment_term_id = self.account_payment_term
with invoice_form.line_ids.new() as line_form:
line_form.product_id = self.product_product_10
line_form.account_id = self.a_sale
line_form.tax_ids.clear()
line_form.tax_ids.add(self.tax_22)
invoice = invoice_form.save()
invoice.action_post()
res = self.run_wizard(invoice.ids)
attachments = self.attach_model.browse(res["res_id"])
self.assertEqual(len(attachments), 1)

# change/remove something and see if preventive_checks() catches it
invoice = invoice.copy()
fiscal_document_type_id = invoice.fiscal_document_type_id
invoice.fiscal_document_type_id = False
invoice.action_post()
with self.assertRaises(UserError) as ue:
self.run_wizard(invoice.id)
self.assertIn(invoice.name, ue.exception.args[0])
self.assertIn("fiscal document type must be set", ue.exception.args[0])
invoice.fiscal_document_type_id = fiscal_document_type_id

invoice = invoice.copy()
pt_id = invoice.invoice_payment_term_id.fatturapa_pt_id
invoice.invoice_payment_term_id.fatturapa_pt_id = False
invoice.action_post()
with self.assertRaises(UserError) as ue:
self.run_wizard(invoice.id)
self.assertIn(invoice.name, ue.exception.args[0])
self.assertIn(
"fiscal payment term must be set for the selected payment term",
ue.exception.args[0],
)
invoice.invoice_payment_term_id.fatturapa_pt_id = pt_id

invoice = invoice.copy()
pm_id = invoice.invoice_payment_term_id.fatturapa_pm_id
invoice.invoice_payment_term_id.fatturapa_pm_id = False
invoice.action_post()
with self.assertRaises(UserError) as ue:
self.run_wizard(invoice.id)
self.assertIn(invoice.name, ue.exception.args[0])
self.assertIn(
"fiscal payment method must be set for the selected payment term",
ue.exception.args[0],
)
invoice.invoice_payment_term_id.fatturapa_pm_id = pm_id

invoice = invoice.copy()
city = invoice.partner_id.city
invoice.partner_id.city = False
invoice.action_post()
with self.assertRaises(UserError) as ue:
self.run_wizard(invoice.id)
self.assertIn(invoice.name, ue.exception.args[0])
self.assertIn("city must be set", ue.exception.args[0])
invoice.partner_id.city = city

invoice = invoice.copy()
city = invoice.company_id.partner_id.city
invoice.company_id.partner_id.city = False
invoice.action_post()
with self.assertRaises(UserError) as ue:
self.run_wizard(invoice.id)
self.assertIn(invoice.name, ue.exception.args[0])
self.assertIn("city in our company's partner must be set", ue.exception.args[0])
invoice.company_id.partner_id.city = city

invoice = invoice.copy()
invoice.company_id.fatturapa_stabile_organizzazione = (
invoice.company_id.partner_id.copy()
)
city = invoice.company_id.fatturapa_stabile_organizzazione.city
invoice.company_id.fatturapa_stabile_organizzazione.city = False
invoice.action_post()
with self.assertRaises(UserError) as ue:
self.run_wizard(invoice.id)
self.assertIn(invoice.name, ue.exception.args[0])
self.assertIn(
"city in our company's Stable Organization must be set",
ue.exception.args[0],
)
invoice.company_id.fatturapa_stabile_organizzazione.city = city
Copy link
Member

Choose a reason for hiding this comment

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

Si potrebbe aggiungere anche un'esportazione che finalmente va a buon fine (giusto per dare soddisfazione alla CI 😄) una volta che abbiamo fatto tutte le configurazioni corrette

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Beh per quello ci sono gli altri test, che lo fanno ripetutamente. Tuttavia, ha senso, una volta fatti i controlli per le fatture sbagliate in partenza (non confermate o passive), verificare che stiamo partendo da una fattura corretta, per poi modificare una cosa alla volta e vedere se preventive_check() fa il suo lavoro, punto per punto.

9 changes: 5 additions & 4 deletions l10n_it_fatturapa_out/wizard/wizard_export_fatturapa.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,11 @@ def split_list(my_list, size):
res[invoice.partner_id] = []
res[invoice.partner_id].append(invoice.id)
for partner_id in res.keys():
if partner_id.max_invoice_in_xml:
res[partner_id] = list(
split_list(res[partner_id], partner_id.max_invoice_in_xml)
)
max_invoice_in_xml = (
partner_id.max_invoice_in_xml or self.env.company.max_invoice_in_xml
)
if max_invoice_in_xml:
res[partner_id] = list(split_list(res[partner_id], max_invoice_in_xml))
else:
res[partner_id] = [res[partner_id]]
# The returned dictionary contains a plain res.partner object as key
Expand Down
Loading