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

[ADD] l10n_it_fatturapa_import_zip #3587

Merged

Conversation

SirAionTech
Copy link
Contributor

@SirAionTech SirAionTech commented Sep 19, 2023

Sostituisce #3526.
Implementa #3510 per 14.0.

@SirAionTech SirAionTech marked this pull request as ready for review September 19, 2023 08:05
@tafaRU tafaRU mentioned this pull request Sep 19, 2023
74 tasks
Copy link
Contributor

@micheledic micheledic left a comment

Choose a reason for hiding this comment

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

Fatto un po' di prove e sembra OK!

return result

def _get_invoice_partner_id(self, fatt):
if self._is_import_attachment_out():
Copy link
Contributor

Choose a reason for hiding this comment

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

Qui bypassi la funzione getCedPrest che si occupa di caricare l'indirizzo completo del partner, per cui viene creato solo con la P.I. se non presente.

Si potrebbe recuperare la funzione getCessComm che era usata nella versione 12.0 di questo modulo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eh sì viene creato un partner proprio con il minimo indispensabile: nome, cognome, codice fiscale, partita IVA, nazione e poco altro.
Secondo me è già un buon inizio e si potrebbe iniziare a usare già così, però capisco che sia migliorabile.

Non esiste la versione 12.0 di questo modulo, mi puoi dare un riferimento al codice di cui scrivevi?
Ovviamente puoi anche proporre la modifica al branch di questa PR.

@SirAionTech
Copy link
Contributor Author

Aggiornamento (tardivo): la PR equivalente per 16.0 (#3584) è stata mergiata

@matteoopenf
Copy link
Contributor

Aggiornamento (tardivo): la PR equivalente per 16.0 (#3584) è stata mergiata

se la pr omologa e' stata mergiata possiamo valutare il merge di questa?

Copy link
Contributor

@matteoopenf matteoopenf left a comment

Choose a reason for hiding this comment

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

ho provato il modulo funzionalmente il modulo e funziona perfettamente, approvazione funzionale

@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). 🤖

@andreampiovesana
Copy link
Contributor

@sergiocorato ok? merge?

@matteoopenf
Copy link
Contributor

@sergiocorato per caso anche per te e' ok?

@matteoopenf
Copy link
Contributor

@sergiocorato remind

@matteoopenf
Copy link
Contributor

per caso si riesce a mergiare?

@andreampiovesana
Copy link
Contributor

merge?

Borruso and others added 7 commits December 6, 2023 16:50
Also allow to create withholding taxes in other tests
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
Sometimes the file that is being read still hasn't been written completely so it is not recognized as a zip file during parsing and raises exception "BadZipfile: File is not a zip file".
@SirAionTech SirAionTech force-pushed the 14.0-mig-l10n_it_fatturapa_import_zip branch from f7ab6fe to 04e0677 Compare December 6, 2023 15:50
@SirAionTech SirAionTech marked this pull request as draft December 6, 2023 15:54
The signature of fatturapa.attachment.out.get_invoice_obj must be the same of fatturapa.attachment.in.get_invoice_obj because the same method is called on both objects
@SirAionTech SirAionTech marked this pull request as ready for review December 6, 2023 16:55
@SirAionTech
Copy link
Contributor Author

Ho fatto rebase e aggiunto qualche modifica dovuta a cambiamenti nelle dipendenze/linting.

@OCA/local-italy-maintainers si può mergiare prima che cambino altre cose?
Mi sembra ci siano abbastanza revisioni, @sergiocorato cosa ne pensi?

Copy link
Contributor

@sergiocorato sergiocorato left a comment

Choose a reason for hiding this comment

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

Eventuali miglioramenti in seguito.

@sergiocorato
Copy link
Contributor

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 14.0-ocabot-merge-pr-3587-by-sergiocorato-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 62f3da4 into OCA:14.0 Dec 13, 2023
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at ae3fcaa. 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