[14.0] [FIX] Ricevere fatture elettroniche in base64 via PEC #2915

[odooNextev]

Risolve #2915 per la v14.0.

[OCA-git-bot]

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

[SirTakobi]

Grazie della PR!
Ti stavo giusto chiedendo di mantenere l'authorship ma vedo che l'hai appena fatto 😄.

Ho fatto revisione del codice e secondo me va bene.

Il messaggio del commit dovrebbe essere in inglese e menzionare il modulo che modifica per seguire https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#commit-message.
Per modificarlo puoi fare cherry-pick del commit della 12.0 2be5317

[TheMule71]

Nota per il forward port alla 16.0. Questa PR dipende dalla #3118.

[SirTakobi]

Non avevo notato che questa PR modifica due moduli in un unico commit, anche questo non si potrebbe fare secondo https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#71commit-message, riesci spezzarli come erano in #2916 ?

[odooNextev]

Non avevo notato che questa PR modifica due moduli in un unico commit, anche questo non si potrebbe fare secondo https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#71commit-message, riesci spezzarli come erano in #2916 ?

@TheMule71 stamattina diceva (e abbiamo condiviso anche io e @tafaRU) che tocca sì 2 moduli, ma la piccola modifica della docstring di un metodo di l10n_it_sdi_channel sarebbe inutile senza l'altra a l10n_it_fatturapa_pec, è quindi corretto fare un singolo commit.
Spero di aver spiegato bene il suo concetto.

[SirTakobi]

Non avevo notato che questa PR modifica due moduli in un unico commit, anche questo non si potrebbe fare secondo https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#71commit-message, riesci spezzarli come erano in #2916 ?

@TheMule71 stamattina diceva (e abbiamo condiviso anche io e @tafaRU) che tocca sì 2 moduli, ma la piccola modifica della docstring di un metodo di l10n_it_sdi_channel sarebbe inutile senza l'altra a l10n_it_fatturapa_pec, è quindi corretto fare un singolo commit. Spero di aver spiegato bene il suo concetto.

Capito, secondo me invece vanno separati come avevo fatto in origine quindi la mia review rimane così, ovviamente si può mergiare lo stesso se arrivano altre approvazioni.
Se vuoi lasciare così però per favore mettiti come co-autore del commit :)

[TheMule71]

Per me non c'è ragione per separare commit per moduli come principio generale. Come principio generale, si separano patch che non c'entrano una con l'altra.

A volte, il fatto che siano in moduli diversi è un buon indicatore del fatto che non c'entrino una con l'altra. A volte no. Oggi facevo l'esempio di un commit che cambia la signature di un metodo, modifica non solo il modulo in questione ma anche tutti i chiamanti. Separare quel commit per modulo è sbagliato in quanto crea dei commit "rotti", che non funzionano.

In questo caso, trattandosi di un commento, è abbastanza irrilevante. Il mio scopo è quello di non rompere l'uso di git bisect per tracciare bachi. Ogni commit deve essere 'buono' nel senso di funzionante come codice. Anche perché al 99.99% il bug che sto tracciando è altrove.

Comunque a livello di logica anche in questo caso, se uno atterrasse sul commit intermedio tipo 2be531798c2e1333f175287891ad6527ee72aaa9 e si mettesse ad esplorare i sorgenti, si troverebbe a leggere un commento sbagliato nel file l10n_it_sdi_channel/models/sdi.py nel senso che file_name_content_dict già non mappa più al contento codificato base64.

Le modifiche correlate vanno tenute nello stesso commit, le modifiche indipendenti andrebbero separate.

[SirTakobi]

Per me non c'è ragione per separare commit per moduli come principio generale.

Però così, alla migrazione ci troverremmo lo stesso messaggio di commit presente per diversi moduli e non più allineato con il contenuto del commit migrato.

Se pensi che vada gestito in modo diverso dalle linee guida in https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#71commit-message puoi proporre lì delle modifiche.

[TheMule71]

Non è necessario. Quanto propongo è perfettamente conforme alle linee guida. Lo scopo del separare i commit è delineato esplicitamente:
"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."

Nell'es. che ho fatto, un cambio di API, non ha alcun senso fare un revert per un modulo separatamente, perché lo romperebbe. Se si torna alla vecchia API, lo si fa ovunque, non avrebbe senso avere un solo modulo che chiama un metodo con la vecchia API non funzionante.

Lo stesso vale per il nostro caso. Non avrebbe senso riportare indietro il commento allo stato precedende senza disfare anche l'altra parte del commit. O si decodifica base64 oppure no. Le modifiche, anche se riguardano due moduli diversi, sono un'unità indivisibile.

[odooNextev]

@tafaRU si può mergiare?
E' da mesi ormai che ho il modulo installato dal cliente con questa versione patchata

[sergiocorato]

LGTM

[sergiocorato]

/ocabot merge patch

[OCA-git-bot]

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 14.0-ocabot-merge-pr-3500-by-sergiocorato-bump-patch, awaiting test results.

[OCA-git-bot]

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

[mde-scopea]

Hi Guys, thanks for the job.
But since this commit, it's not possible to update/clone the repo on a Windows operating system.

The filename POSTA CERTIFICATA: Invio File 7339338 (base64).txt is incompatible for a NTFS filesystem.

Is it necessary to have ':' in the filename ?
Possible to rename it w/o ':' ?

Thanks.

[matteoopenf]

Hi Guys, thanks for the job. But since this commit, it's not possible to update/clone the repo on a Windows operating system.

The filename POSTA CERTIFICATA: Invio File 7339338 (base64).txt is incompatible for a NTFS filesystem.

Is it necessary to have ':' in the filename ? Possible to rename it w/o ':' ?

Thanks.

.txt is not a valid xml you should have a .xml file

[francesco-ooops]

@odooNextev

[SirAionTech]

Hi Guys, thanks for the job. But since this commit, it's not possible to update/clone the repo on a Windows operating system.

The filename POSTA CERTIFICATA: Invio File 7339338 (base64).txt is incompatible for a NTFS filesystem.

Is it necessary to have ':' in the filename ? Possible to rename it w/o ':' ?

Thanks.

I think it is possible to rename it, something similar has already been proposed for 12.0 in #3112.