[MIG] l10n_it_fatturapa_import_zip #3511

SirTakobi · 2023-07-31T14:04:49Z

Implementa #3510 per 16.0.

Migrazione fatta a partire da #3526.

Ho incluso delle modifiche alla gestione delle aziende nei test di l10n_it_fatturapa_in, se necessario posso spostarle in una PR dedicata.

The test's user like real users is logged in only one company at a time. That company is implicitly used in `search`es thanks to multi-company record rules.

Also allow to create withholding taxes in other tests

…mportare in massa e-fatture in/out

Do not assume there is a `/tmp` directory or that path separator is `/` so that this can also work in other FileSystems than Linux's

Override exposed methods instead of duplicating Reuse common tests data

andreampiovesana · 2023-07-31T14:13:58Z

nel nome pr puoi aggiungere [16.0] ?

SirTakobi · 2023-07-31T14:44:51Z

Sì ma preferirei lasciarlo così perché il branch è già scritto subito sotto il nome della PR ed è filtrabile con `base:16.0` nella lista delle PR.

…

On Mon, Jul 31, 2023 at 4:14 PM Andrea Piovesana ***@***.***> wrote: nel nome pr puoi aggiungere [16.0] ? — Reply to this email directly, view it on GitHub <#3511 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AYHLQYADWIYXIGA3TJHLTELXS64TBANCNFSM6AAAAAA26J6254> . You are receiving this because you authored the thread.Message ID: ***@***.***>

eLBati · 2023-08-28T13:39:53Z

Su runboat, importando il file
IT01234567890_FPR03.zip
ottengo

  File "/mnt/data/odoo-addons-dir/l10n_it_fatturapa_import_zip/models/attachment.py", line 134, in action_import
    _extract_zip_file(tmp_dir, self.datas)
  File "/mnt/data/odoo-addons-dir/l10n_it_fatturapa_import_zip/models/attachment.py", line 19, in _extract_zip_file
    with zipfile.ZipFile(tmp_file.name, mode="r") as zip_ref:
  File "/usr/lib/python3.10/zipfile.py", line 1269, in __init__
    self._RealGetContents()
  File "/usr/lib/python3.10/zipfile.py", line 1336, in _RealGetContents
    raise BadZipFile("File is not a zip file")
zipfile.BadZipFile: File is not a zip file

Probabilmente un problema relativo a come ho creato il file

micheledic · 2023-09-18T10:35:17Z

secondo me(correggetemi se sbaglio) dovrebbero essere due PR separate, una relativa a fatturapa_in e l'altro che implementa import_zip considerando che l'improvement può anche esistere senza fatturapa_zip

matteoopenf · 2023-09-18T10:44:44Z

secondo me(correggetemi se sbaglio) dovrebbero essere due PR separate, una relativa a fatturapa_in e l'altro che implementa import_zip considerando che l'improvement può anche esistere senza fatturapa_zip

secondo me ha poco senso scindere le due cose e si allungo i tempi di merge

micheledic · 2023-09-18T10:44:09Z

l10n_it_fatturapa_import_zip/wizards/wizard_import_fatturapa.py

+    def _is_import_attachment_out(self):
+        model = self._get_selected_model()
+        return model == ATTACHMENT_OUT_MODEL_NAME
+
+    def _check_attachment(self, attachment):
+        if self._is_import_attachment_out():
+            if attachment.out_invoice_ids:
+                raise UserError(
+                    _("File %s is linked to invoices yet.", attachment.name)
+                )
+            result = True
+        else:
+            result = super()._check_attachment(attachment)


Secondo me questo codice non dovrebbe essere relativo a import_zip considerando che se si triggera il wizard fatturapa_in con active_model = fatturapa.attachment.out va in errore al call di _check_attachment (che richiede in_invoice_ids)

inoltre secondo me il wizard, visto che può funzionare sia IN che OUT, dovrebbe stare su l10n_it_fatturapa e i moduli _in e _out dovrebbero mettere logiche specifiche in ed out

ma è un refactoring abbastanza complesso, sicuramente da gestire in una PR a parte

Inoltre alla riga 1911
partner_id = self.getCedPrest(cedentePrestatore)

Bisognerebbe prevedere che in caso e sia una fattura in uscita , bisogna prendere il cessionarioCommittente che sarebbe l'effettivo cliente anzicchè il cedente prestatore che è la company che ha emesso la fattura
ad es nel caso di attachment.out

cessionarioCommittente = ( fatt.FatturaElettronicaHeader.CessionarioCommittente ) partner_id = self.getPartnerBase(cessionarioCommittente.DatiAnagrafici)

Grazie della revisione!

Secondo me questo codice non dovrebbe essere relativo a import_zip considerando che se si triggera il wizard fatturapa_in con active_model = fatturapa.attachment.out va in errore al call di _check_attachment (che richiede in_invoice_ids)

Puoi riportare quali passi hai fatto per ottenere l'errore? Ho provato sia a caricare uno zip che a importare una fattura elettronica con il wizard ma non ho ottenuto errori.

inoltre secondo me il wizard, visto che può funzionare sia IN che OUT, dovrebbe stare su l10n_it_fatturapa e i moduli _in e _out dovrebbero mettere logiche specifiche in ed out

ma è un refactoring abbastanza complesso, sicuramente da gestire in una PR a parte

Era una delle possibilità, alla fine è stata scelta l'implementazione attuale.
Si può fare dopo, oppure puoi proporre un'altra PR con la tua implementazione.

Inoltre alla riga 1911 partner_id = self.getCedPrest(cedentePrestatore)

Bisognerebbe prevedere che in caso e sia una fattura in uscita , bisogna prendere il cessionarioCommittente che sarebbe l'effettivo cliente anzicchè il cedente prestatore che è la company che ha emesso la fattura ad es nel caso di attachment.out

cessionarioCommittente = ( fatt.FatturaElettronicaHeader.CessionarioCommittente ) partner_id = self.getPartnerBase(cessionarioCommittente.DatiAnagrafici)

Puoi riportare quali passi hai fatto per ottenere l'errore e il comportamento attuale/atteso?

Grazie della revisione!

Secondo me questo codice non dovrebbe essere relativo a import_zip considerando che se si triggera il wizard fatturapa_in con active_model = fatturapa.attachment.out va in errore al call di _check_attachment (che richiede in_invoice_ids)

Puoi riportare quali passi hai fatto per ottenere l'errore? Ho provato sia a caricare uno zip che a importare una fattura elettronica con il wizard ma non ho ottenuto errori.

Con fatturapa_import_zip disinstallato prova a caricare un file OUT , va in errore perchè _check_attachment di fatturapa_in prova ad accedere sempre al campo in_invoice_ids

inoltre secondo me il wizard, visto che può funzionare sia IN che OUT, dovrebbe stare su l10n_it_fatturapa e i moduli _in e _out dovrebbero mettere logiche specifiche in ed out
ma è un refactoring abbastanza complesso, sicuramente da gestire in una PR a parte

Era una delle possibilità, alla fine è stata scelta l'implementazione attuale. Si può fare dopo, oppure puoi proporre un'altra PR con la tua implementazione.

Inoltre alla riga 1911 partner_id = self.getCedPrest(cedentePrestatore)
Bisognerebbe prevedere che in caso e sia una fattura in uscita , bisogna prendere il cessionarioCommittente che sarebbe l'effettivo cliente anzicchè il cedente prestatore che è la company che ha emesso la fattura ad es nel caso di attachment.out

cessionarioCommittente = ( fatt.FatturaElettronicaHeader.CessionarioCommittente ) partner_id = self.getPartnerBase(cessionarioCommittente.DatiAnagrafici)

Puoi riportare quali passi hai fatto per ottenere l'errore e il comportamento attuale/atteso?

Ho provato a caricare un file fattura cliente , la fattura che crea ha come cliente la company in cui ti trovi perchè nel caso di fatture cliente , i due campi sono invertiti (visto che la company di odoo è colei che emette fattura e il cliente è il cessionario )

Grazie della revisione!

Secondo me questo codice non dovrebbe essere relativo a import_zip considerando che se si triggera il wizard fatturapa_in con active_model = fatturapa.attachment.out va in errore al call di _check_attachment (che richiede in_invoice_ids)

Puoi riportare quali passi hai fatto per ottenere l'errore? Ho provato sia a caricare uno zip che a importare una fattura elettronica con il wizard ma non ho ottenuto errori.

Con fatturapa_import_zip disinstallato prova a caricare un file OUT , va in errore perchè _check_attachment di fatturapa_in prova ad accedere sempre al campo in_invoice_ids

Mi sembra normale che l10n_it_fatturapa_in non possa importare un file OUT, pensi che invece dovrebbe poterlo importare?
Il campo in_invoice_ids è definito in l10n_it_fatturapa_in quindi non vedo quale sia il problema, puoi riportare lo stack dell'errore? Riesci a riprodurlo in runboat?

inoltre secondo me il wizard, visto che può funzionare sia IN che OUT, dovrebbe stare su l10n_it_fatturapa e i moduli _in e _out dovrebbero mettere logiche specifiche in ed out
ma è un refactoring abbastanza complesso, sicuramente da gestire in una PR a parte

Era una delle possibilità, alla fine è stata scelta l'implementazione attuale. Si può fare dopo, oppure puoi proporre un'altra PR con la tua implementazione.

Inoltre alla riga 1911 partner_id = self.getCedPrest(cedentePrestatore)
Bisognerebbe prevedere che in caso e sia una fattura in uscita , bisogna prendere il cessionarioCommittente che sarebbe l'effettivo cliente anzicchè il cedente prestatore che è la company che ha emesso la fattura ad es nel caso di attachment.out

cessionarioCommittente = ( fatt.FatturaElettronicaHeader.CessionarioCommittente ) partner_id = self.getPartnerBase(cessionarioCommittente.DatiAnagrafici)

Puoi riportare quali passi hai fatto per ottenere l'errore e il comportamento attuale/atteso?

Ho provato a caricare un file fattura cliente , la fattura che crea ha come cliente la company in cui ti trovi perchè nel caso di fatture cliente , i due campi sono invertiti (visto che la company di odoo è colei che emette fattura e il cliente è il cessionario )

Capito, grazie del chiarimento, appena riesco ci guardo.
Nella 14.0 (#3526) invece il comportamento ti risulta essere corretto?

Grazie della revisione!

Secondo me questo codice non dovrebbe essere relativo a import_zip considerando che se si triggera il wizard fatturapa_in con active_model = fatturapa.attachment.out va in errore al call di _check_attachment (che richiede in_invoice_ids)

Puoi riportare quali passi hai fatto per ottenere l'errore? Ho provato sia a caricare uno zip che a importare una fattura elettronica con il wizard ma non ho ottenuto errori.

Con fatturapa_import_zip disinstallato prova a caricare un file OUT , va in errore perchè _check_attachment di fatturapa_in prova ad accedere sempre al campo in_invoice_ids

Mi sembra normale che l10n_it_fatturapa_in non possa importare un file OUT, pensi che invece dovrebbe poterlo importare? Il campo in_invoice_ids è definito in l10n_it_fatturapa_in quindi non vedo quale sia il problema, puoi riportare lo stack dell'errore? Riesci a riprodurlo in runboat?

Esatto logicamente potrebbe non esser corretto importare con fatturapa_in un file OUT (e per questo secondo me dovrebbe essere il wizard dovrebbe essere in l10n_it_fatturapa, ma come dicevamo semmai questo refactoring sarebbe da fare in separata PR) ma vedevo che in import_zip c'erano dei metodi d'appoggio utili anche al di fuori dell'import_zip ad es

def _is_import_attachment_out(self): model = self._get_selected_model() return model == ATTACHMENT_OUT_MODEL_NAME def _check_attachment(self, attachment): if self._is_import_attachment_out(): if attachment.out_invoice_ids: raise UserError( _("File %s is linked to invoices yet.", attachment.name) ) result = True else: result = super()._check_attachment(attachment) return result

che a questo punto potrebbero essere "industrializzati" ovvero definiti nei wizard fatturapa_in e fatturapa_out così che possa essere possibile importare un fatturapa.out sfruttando lo stesso wizard .

Intendo ad esempio definire queste sotto-funzioni su fatturapa_in e farne l'override su fatturapa_out così che anche installando solamente fatturapa_in e fatturapa_out , si possono importare .xml col wizard senza import_zip installato.

Dimenticavo, bisogna mettere un
tmp_file.flush()
alla riga 19 di attachment.py di fatturapa_import_zip sennò non si riesce ad importare zip e riporta l'errore riportato da @eLBati

Potresti committare almeno questo fix e provvedo a caricare un zip su runboat con la problematica?

inoltre secondo me il wizard, visto che può funzionare sia IN che OUT, dovrebbe stare su l10n_it_fatturapa e i moduli _in e _out dovrebbero mettere logiche specifiche in ed out
ma è un refactoring abbastanza complesso, sicuramente da gestire in una PR a parte

Era una delle possibilità, alla fine è stata scelta l'implementazione attuale. Si può fare dopo, oppure puoi proporre un'altra PR con la tua implementazione.

Inoltre alla riga 1911 partner_id = self.getCedPrest(cedentePrestatore)
Bisognerebbe prevedere che in caso e sia una fattura in uscita , bisogna prendere il cessionarioCommittente che sarebbe l'effettivo cliente anzicchè il cedente prestatore che è la company che ha emesso la fattura ad es nel caso di attachment.out

cessionarioCommittente = ( fatt.FatturaElettronicaHeader.CessionarioCommittente ) partner_id = self.getPartnerBase(cessionarioCommittente.DatiAnagrafici)

Puoi riportare quali passi hai fatto per ottenere l'errore e il comportamento attuale/atteso?

Ho provato a caricare un file fattura cliente , la fattura che crea ha come cliente la company in cui ti trovi perchè nel caso di fatture cliente , i due campi sono invertiti (visto che la company di odoo è colei che emette fattura e il cliente è il cessionario )

Capito, grazie del chiarimento, appena riesco ci guardo. Nella 14.0 (#3526) invece il comportamento ti risulta essere corretto?

Scusami sta iniziando a dare anche su #3526 lo stesso problema in modo randomico

File "/usr/lib/python3.8/zipfile.py", line 1269, in __init__ self._RealGetContents() File "/usr/lib/python3.8/zipfile.py", line 1336, in _RealGetContents raise BadZipFile("File is not a zip file")

Risolvibile anche sulla 14.0 con un flush.
Non so se a questo punto possa dipendere dala versione di tempfile visto che è un problema recente e randomico. Sembra che delle volte non flusha il contenuto del file e quindi zipfile non riesce ad aprirlo.

Potresti committare il .flush() su entrambe le PR e rifaccio un giro di test?

Ho provato a caricare un file fattura cliente , la fattura che crea ha come cliente la company in cui ti trovi perchè nel caso di fatture cliente , i due campi sono invertiti (visto che la company di odoo è colei che emette fattura e il cliente è il cessionario )

Questa PR è di un account disattivato quindi ho creato #3584 per correggere la tua segnalazione, puoi verificare lì se ora il comportamento è come ti aspetti?

SirAionTech · 2023-09-18T15:33:53Z

Da chiudere, sostituita da #3584

SirTakobi and others added 7 commits July 31, 2023 15:48

[COV] l10n_it_fatturapa_in: Company management

d5c82db

The test's user like real users is logged in only one company at a time. That company is implicitly used in `search`es thanks to multi-company record rules.

[REF] l10n_it_fatturapa_in: Allow overrides for importing invoices

53d0422

Also allow to create withholding taxes in other tests

[ADD] l10n_it_fatturapa_import_zip: aggiunto modulo che permette di i…

ff439a9

…mportare in massa e-fatture in/out

[IMP] l10n_it_fatturapa_import_zip: FS-Agnostic file management

9d24594

Do not assume there is a `/tmp` directory or that path separator is `/` so that this can also work in other FileSystems than Linux's

[REF] l10n_it_fatturapa_import_zip: Invoices creation

6c18811

Override exposed methods instead of duplicating Reuse common tests data

[IMP] l10n_it_fatturapa_import_zip: pre-commit execution

d1558d3

[MIG] l10n_it_fatturapa_import_zip: Migration to 16.0

6abdb02

SirTakobi marked this pull request as ready for review July 31, 2023 15:27

micheledic reviewed Sep 18, 2023

View reviewed changes

SirAionTech mentioned this pull request Sep 18, 2023

[MIG] l10n_it_fatturapa_import_zip #3584

Merged

tafaRU closed this Sep 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MIG] l10n_it_fatturapa_import_zip #3511

[MIG] l10n_it_fatturapa_import_zip #3511

SirTakobi commented Jul 31, 2023 •

edited

Loading

andreampiovesana commented Jul 31, 2023

SirTakobi commented Jul 31, 2023 via email

eLBati commented Aug 28, 2023 •

edited

Loading

micheledic commented Sep 18, 2023

matteoopenf commented Sep 18, 2023

micheledic Sep 18, 2023

micheledic Sep 18, 2023

micheledic Sep 18, 2023

SirAionTech Sep 18, 2023

micheledic Sep 18, 2023 •

edited

Loading

SirAionTech Sep 18, 2023

micheledic Sep 18, 2023 •

edited

Loading

SirAionTech Sep 18, 2023

SirAionTech commented Sep 18, 2023

[MIG] l10n_it_fatturapa_import_zip #3511

[MIG] l10n_it_fatturapa_import_zip #3511

Conversation

SirTakobi commented Jul 31, 2023 • edited Loading

andreampiovesana commented Jul 31, 2023

SirTakobi commented Jul 31, 2023 via email

eLBati commented Aug 28, 2023 • edited Loading

micheledic commented Sep 18, 2023

matteoopenf commented Sep 18, 2023

micheledic Sep 18, 2023

Choose a reason for hiding this comment

micheledic Sep 18, 2023

Choose a reason for hiding this comment

micheledic Sep 18, 2023

Choose a reason for hiding this comment

SirAionTech Sep 18, 2023

Choose a reason for hiding this comment

micheledic Sep 18, 2023 • edited Loading

Choose a reason for hiding this comment

SirAionTech Sep 18, 2023

Choose a reason for hiding this comment

micheledic Sep 18, 2023 • edited Loading

Choose a reason for hiding this comment

SirAionTech Sep 18, 2023

Choose a reason for hiding this comment

SirAionTech commented Sep 18, 2023

SirTakobi commented Jul 31, 2023 •

edited

Loading

eLBati commented Aug 28, 2023 •

edited

Loading

micheledic Sep 18, 2023 •

edited

Loading

micheledic Sep 18, 2023 •

edited

Loading