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

[MIG] l10n_it_sepa_bonifici -> l10n_it_sct_cbi #4189

Merged
merged 8 commits into from
Sep 9, 2024

Conversation

SirAionTech
Copy link
Contributor

@SirAionTech SirAionTech commented Jun 5, 2024

Migrazione da https://github.com/OCA/l10n-italy/tree/ce2daa663db1e2e009dc85021fac2f6379238f43/l10n_it_sepa_bonifici (8.0).

Il modulo è stato in buona parte riscritto come conseguenza dei cambiamenti nella dipendenza account_banking_pain_base, ad esempio OCA/bank-payment@574c258.

Ho inglesizzato il nome del modulo, non ho fatto script di migrazione perché credo che nessuno voglia fare una migrazione da 8.0 a 16.0; se vuole farlo, i dati di questo modulo sono il minore dei suoi problemi.

Per le traduzioni ho fatto un semplice find/replace ho tradotto come mi sembrava più sensato, lascerei il compito il più possibile ai nostri ottimi traduttori 😇.

Il modulo migrato rispetta i nuovi schema in vigore dal 18/03/2024 1.
È possibile verificare la validità dei file generati su https://www.cbiservice.com/correttoresepa.php, selezionando l'opzione: SCT - CBIBdyPaymentRequest 00.04.01 (new).

Fatemi sapere cosa ne pensate 😄

Footnotes

  1. https://www.cbi-org.eu/Home/Servizio-CBI-dal-18-marzo-2024-saranno-in-produzione-le-nuove-release-SEPA-v-2019

@SirAionTech SirAionTech force-pushed the 16.0-mig-l10n_it_sepa_bonifici branch 3 times, most recently from baa2ffd to ef9ddeb Compare June 5, 2024 10:48
@SirAionTech

This comment was marked as resolved.

@SirAionTech SirAionTech marked this pull request as ready for review June 5, 2024 13:08
@SirAionTech SirAionTech force-pushed the 16.0-mig-l10n_it_sepa_bonifici branch from 5efff31 to 05c86f6 Compare June 5, 2024 15:55
@SirAionTech
Copy link
Contributor Author

/ocabot migration l10n_it_sepa_bonifici

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Jun 14, 2024
@OCA-git-bot OCA-git-bot mentioned this pull request Jun 14, 2024
79 tasks
@francesco-ooops
Copy link
Contributor

@SirAionTech rilancia pure

@SirAionTech
Copy link
Contributor Author

@SirAionTech rilancia pure

Cosa? Il comando

/ocabot migration l10n_it_sepa_bonifici

era andato bene

alessandrocamilli and others added 7 commits June 17, 2024 13:19
Added Cross Border payment

Fix country data

Removed commented lines

New ensure_one to secure one export at a time.

tag image:: replaced by figure::

travis warning

travis warning (2)

travis warning (3)

Pass test_flake8

test_flake8 (2)

Changes for test_pylint

improved syntax
l10n_it_sepa_bonifici: remove unused file
Currently translated at 11.4% (4 of 35 strings)

Translation: l10n-italy-8.0/l10n-italy-8.0-l10n_it_sepa_bonifici
Translate-URL: https://translation.odoo-community.org/projects/l10n-italy-8-0/l10n-italy-8-0-l10n_it_sepa_bonifici/it/
@SirAionTech SirAionTech force-pushed the 16.0-mig-l10n_it_sepa_bonifici branch from 05c86f6 to 2e23817 Compare June 17, 2024 11:19
@SirAionTech
Copy link
Contributor Author

Fatto rebase dopo il merge di #4193.

@francesco-ooops
Copy link
Contributor

@matteoopenf dato che è il porting di un modulo creato da openforce, potresti fare review? Grazie! :)

Copy link
Contributor

@aleuffre aleuffre left a comment

Choose a reason for hiding this comment

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

Code review, LGTM.

Excellent work, very clean and readable.

l10n_it_sct_cbi/models/account_payment_order.py Outdated Show resolved Hide resolved
@SirAionTech SirAionTech force-pushed the 16.0-mig-l10n_it_sepa_bonifici branch 2 times, most recently from c07d810 to 09c750b Compare June 25, 2024 14:36
@MaurizioConte
Copy link

image
Test Pagamento a Fornitore quando si genera il file si riceve l'errore in slide

@SirAionTech
Copy link
Contributor Author

image Test Pagamento a Fornitore quando si genera il file si riceve l'errore in slide

Grazie della revisione!
Puoi riportare i passi che hai fatto per ottenere l'errore? Oppure un video se ti viene più comodo.
Nel log dovresti poi trovare il contenuto del file prodotto, puoi riportare anche quello?

Copy link
Contributor

@andreampiovesana andreampiovesana left a comment

Choose a reason for hiding this comment

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

merge please

@SirAionTech SirAionTech marked this pull request as draft July 8, 2024 07:12
Use new standards
Added namespace to pain descendants
IBAN is mandatory
Initiating party is mandatory
FinInstd/Othr is not allowed
ReqdExctnDt has new structure
BICFI in PaymentRequest instead of BIC
Manage multiple priorities
Added test
Updated README
Updated translations
@SirAionTech SirAionTech force-pushed the 16.0-mig-l10n_it_sepa_bonifici branch from 2a22cd9 to f22405a Compare July 9, 2024 09:32
@SirAionTech
Copy link
Contributor Author

Ho portato in bozza perché abbiamo trovato un problema in un particolare caso d'uso: se l'ordine di pagamento raggruppava i pagamenti in diversi gruppi, allora la generazione del file falliva con errore:

Element '{urn:CBI:xsd:CBIPaymentRequest.00.04.01}PmtInf': This element is not expected.

Maggiori dettagli sono nel nuovo test aggiunto in https://github.com/OCA/l10n-italy/compare/2a22cd9d74680551e7d25adf52eac6d0ee61ad3f..f22405a446ead2ec0e4510e7ba237dcb73b5e8ff, insieme alla sua correzione.

@SirAionTech SirAionTech marked this pull request as ready for review July 9, 2024 10:00
Copy link

@MaurizioPellegrinet MaurizioPellegrinet left a comment

Choose a reason for hiding this comment

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

Un gran bel lavoro, test funzionale: OK

@SirAionTech
Copy link
Contributor Author

SirAionTech commented Aug 23, 2024

requested review from @andreampiovesana, @aleuffre and @MaurizioConte

🙏 reminder

@MaurizioConte
Copy link

Fatto test su runboat e quando si conferma il pagamento viene indicato FILE GENERATO ma negli allegati il file non è presente.

@SirAionTech
Copy link
Contributor Author

Fatto test su runboat e quando si conferma il pagamento viene indicato FILE GENERATO ma negli allegati il file non è presente.

In runboat vedo che il metodo di pagamento è Manuale
image
sarebbe invece da usare il metodo di pagamento Richiesta di pagamento SEPA Credit Transfer con CBI
image
come indicato nel README del modulo (le immagini si vedranno solo dopo il merge)

@MaurizioConte
Copy link

TEST RUNBOAT con verifica file generato su sito CBI - Tutto positivo

@SirAionTech
Copy link
Contributor Author

TEST RUNBOAT con verifica file generato su sito CBI - Tutto positivo

Grazie, potresti mica aggiungere una revisione?
Se non sai come fare trovi delle istruzioni in https://www.odoo-italia.org/documentazione/14.0/sviluppo/review.html#revisione-funzionale.

Copy link

@MaurizioConte MaurizioConte left a comment

Choose a reason for hiding this comment

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

I test hanno dato esito positivo

@francesco-ooops
Copy link
Contributor

@OCA/local-italy-maintainers che dite si può mergiare? era già stata approvata precedentemente

Copy link
Contributor

@aleuffre aleuffre left a comment

Choose a reason for hiding this comment

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

Code review, LGTM

@francesco-ooops
Copy link
Contributor

@SirAionTech go?

@eLBati
Copy link
Member

eLBati commented Sep 9, 2024

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-4189-by-eLBati-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 8e0f46c into OCA:16.0 Sep 9, 2024
6 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

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

@francesco-ooops
Copy link
Contributor

@eLBati @SirAionTech qualche idea perchè nella history del modulo mergiato non ci sono i commit originali?
https://github.com/OCA/l10n-italy/commits/16.0/l10n_it_sct_cbi

@SirAionTech
Copy link
Contributor Author

@eLBati @SirAionTech qualche idea perchè nella history del modulo mergiato non ci sono i commit originali? https://github.com/OCA/l10n-italy/commits/16.0/l10n_it_sct_cbi

Perché ha cambiato nome, i commit precedenti questa migrazione sono sul modulo l10n_it_sepa_bonifici.

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.