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.

Sign up for GitHub

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

Jump to bottom

[IMP] l10n_it_fiscalcode: update library to check fiscalcode #4511

Open
wants to merge 3 commits into
base: 16.0
Choose a base branch
from
Open

[IMP] l10n_it_fiscalcode: update library to check fiscalcode #4511

wants to merge 3 commits into from

Conversation

patrickt-oforce
Copy link
Contributor

@patrickt-oforce patrickt-oforce commented Dec 16, 2024 •
edited
Loading

Update library to check if a partner fiscal is formally valid, using python-codicefiscale library that can validate a fiscalcode even if this is omocode.

In this PR are included fix to test of l10n_it_fatturapa_in e l10n_it_fatturapa_import_zip, because try to validate the FC in xml, result to an error on check digit, make automated tests failing

@francesco-ooops francesco-ooops mentioned this pull request Dec 16, 2024
Closed
2 tasks
@francesco-ooops francesco-ooops linked an issue Dec 16, 2024 that may be closed by this pull request
[l10n_it_fiscalcode] Cambiare metodo di validazione del codice fiscale #1833
Closed
2 tasks
@patrickt-oforce
Copy link
Contributor Author

@francesco-ooops grazie stavo correggendo anche i dati di test di fatturapa_in e import_zip che hanno un codicefiscale errato per l'intermediario

@patrickt-oforce patrickt-oforce force-pushed the 16.0-IMP-l10n_it_fiscalcode branch 2 times, most recently from aab964f to 9b9e594 Compare December 16, 2024 18:14
@SirAionTech SirAionTech removed a link to an issue Dec 19, 2024
[l10n_it_fiscalcode] Cambiare metodo di validazione del codice fiscale #1833
Closed
2 tasks
@patrickt-oforce patrickt-oforce mentioned this pull request Dec 23, 2024
Open
2 tasks
@patrickt-oforce
Copy link
Contributor Author

@SirAionTech issue aperta e linkata

@francesco-ooops francesco-ooops linked an issue Dec 23, 2024 that may be closed by this pull request
Allow to validate omocode italian fiscal code #4518
Open
2 tasks
sergiocorato
sergiocorato reviewed Jan 20, 2025
View reviewed changes
l10n_it_fiscalcode/wizard/compute_fc.py Outdated
nat_code = self._get_national_code(
f.birth_city.name, f.birth_province.code, f.birth_date
)
if not nat_code:
raise UserError(_("National code is missing"))
c_f = build(

if isinstance(f.birth_date, datetime.date):
Copy link
Contributor

Choose a reason for hiding this comment

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

Il campo birth_date può essere una stringa?

Copy link
Contributor Author

@patrickt-oforce patrickt-oforce Jan 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

La libreria proposta deve ricevere il valore di birth_date come stringa

Copy link
Contributor

Choose a reason for hiding this comment

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

Ma in che caso il campo birth_date = fields.Date("Date of birth", required=True) può contenere una stringa?

Copy link
Contributor Author

@patrickt-oforce patrickt-oforce Jan 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Scusami ma credo che non ci stiamo capendo; la libreria proposta vuole il valore del campo birth_date come stringa, il metodo per calcolare il CF richiede che il valore di birth_date sia datetime o str qui gli arriva come data e lo converto in str

Copy link
Contributor

Choose a reason for hiding this comment

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

Già 😅
Il campo "f.birth_date" come potrebbe contenere una stringa se è definito come fields.Date()?
A che serve "if isinstance(f.birth_date, datetime.date):"?
Trasformalo in stringa direttamente.

Copy link
Contributor Author

@patrickt-oforce patrickt-oforce Feb 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Può contenere una stringa perché nei test viene passato come tale, mentre da UI viene caricato come fields.Date(). Quindi invece che modificare i test ho preferito fare un controllo e una conversione. I test, per quanto possibile non andrebbero modificati, perché se sono corretti sono la garanzia che il nuovo codice non introduce regressioni

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sergiocorato aggiornato i test e rimosso il check nel wizard

@patrickt-oforce patrickt-oforce force-pushed the 16.0-IMP-l10n_it_fiscalcode branch from 9b9e594 to ceb0add Compare January 30, 2025 10:05
@matteoopenf
Copy link
Contributor

@sergiocorato per caso si potrebbe mergiare?

@patrickt-oforce patrickt-oforce force-pushed the 16.0-IMP-l10n_it_fiscalcode branch from ceb0add to 418fb36 Compare February 21, 2025 11:22
@patrickt-oforce patrickt-oforce force-pushed the 16.0-IMP-l10n_it_fiscalcode branch from 418fb36 to 335fd9f Compare February 21, 2025 18:08
@patrickt-oforce patrickt-oforce deleted the 16.0-IMP-l10n_it_fiscalcode branch February 21, 2025 18:13
@patrickt-oforce patrickt-oforce restored the 16.0-IMP-l10n_it_fiscalcode branch February 21, 2025 18:15
@OCA-git-bot
Copy link
Contributor

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

@patrickt-oforce patrickt-oforce force-pushed the 16.0-IMP-l10n_it_fiscalcode branch from 56b7f09 to 20b273c Compare February 21, 2025 18:41
@micheledic
Copy link
Contributor

Si può mergiare ? ci sono spesso casi di omocode e l'altra libreria riesce a riconoscerli senza problemi

@matteoopenf
Copy link
Contributor

@francesco-ooops puoi fare rebase che ora sicuramente passerebbe i test?
Grazie Mille

@francesco-ooops
Copy link
Contributor

/ocabot rebase

@OCA-git-bot
Copy link
Contributor

Congratulations, PR rebased to 16.0.

@OCA-git-bot OCA-git-bot force-pushed the 16.0-IMP-l10n_it_fiscalcode branch from 20b273c to ca0eb47 Compare February 24, 2025 11:52
micheledic
micheledic suggested changes Feb 24, 2025
View reviewed changes
l10n_it_fiscalcode/__manifest__.py Outdated
@@ -7,7 +7,7 @@

{
"name": "ITA - Codice fiscale",
"version": "16.0.1.0.3",
"version": "16.0.1.0.3+PR4511",
Copy link
Contributor

Choose a reason for hiding this comment

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

Da rimuovere

Copy link
Contributor

Choose a reason for hiding this comment

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

@patrickt-oforce

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@micheledic rimosso il numero della PR nel manifest

Copy link
Contributor

Choose a reason for hiding this comment

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

Ho visto il commit, sarebbe da modificare proprio il primo commit su l10n_it_fiscalcode evitando proprio il cambio del version nel primo commit .
Essendo il primo commit, dovrai fare col rebase
prova a seguire questo
https://www.geeksforgeeks.org/how-to-modify-a-specific-commit/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@micheledic ci siamo?

@patrickt-oforce patrickt-oforce force-pushed the 16.0-IMP-l10n_it_fiscalcode branch from ca0eb47 to e691b99 Compare February 25, 2025 18:12
@micheledic
Copy link
Contributor

@patrickt-oforce puoi modificare il commit che introduce "+PR45111" anzicche modificarlo con l'ultimo commit?

@patrickt-oforce patrickt-oforce force-pushed the 16.0-IMP-l10n_it_fiscalcode branch from e691b99 to 6def155 Compare February 26, 2025 08:30
@patrickt-oforce patrickt-oforce force-pushed the 16.0-IMP-l10n_it_fiscalcode branch from 6def155 to 56783c5 Compare February 26, 2025 08:38
@patrickt-oforce
Copy link
Contributor Author

@patrickt-oforce puoi modificare il commit che introduce "+PR45111" anzicche modificarlo con l'ultimo commit?

Dovrei averlo fatto; quando puoi ricontrolla se ok

@micheledic
Copy link
Contributor

@patrickt-oforce puoi modificare il commit che introduce "+PR45111" anzicche modificarlo con l'ultimo commit?

Dovrei averlo fatto; quando puoi ricontrolla se ok

Bisogna fixare anche il codice fiscale nel file
https://github.com/patrickt-oforce/l10n-italy/blob/67402ebb08f1aa4896cbce56ee1326ad6ec947c6/l10n_it_fatturapa_in/tests/data/IT05979361218_003.xml#L56
A quanto pare col vecchio modulo anche dei CF errati li dava come OK

Verificando quel CF effettivamente è errato, quindi è da cambiare il test
image

@micheledic
Copy link
Contributor

Anche su altro file il CF è errato
https://github.com/search?q=repo%3Apatrickt-oforce%2Fl10n-italy%20path%3A%2F%5El10n_it_fatturapa_in%5C%2Ftests%5C%2Fdata%5C%2F%2F%20MRORSS90E25B111T&type=code

@patrickt-oforce
[FIX] l10n_it_fatturapa_in: fix value of intermediary fiscal code
b25ea25 
In file IT05979361218_004.xml fix value of intermiedary fiscal code
@patrickt-oforce
[IMP] l10n_it_fatturapa_import_zip: update value of intermediary fisc…
e9ccfd5 
…al code in invoices of archive file
@patrickt-oforce patrickt-oforce force-pushed the 16.0-IMP-l10n_it_fiscalcode branch from 56783c5 to e9ccfd5 Compare February 26, 2025 09:27
@patrickt-oforce
Copy link
Contributor Author

Anche su altro file il CF è errato https://github.com/search?q=repo%3Apatrickt-oforce%2Fl10n-italy%20path%3A%2F%5El10n_it_fatturapa_in%5C%2Ftests%5C%2Fdata%5C%2F%2F%20MRORSS90E25B111T&type=code

SI era rimasto quello vecchio, corretto

@micheledic
Copy link
Contributor

Per me è OK!

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matteoopenf matteoopenf matteoopenf approved these changes

@micheledic micheledic micheledic approved these changes

@sergiocorato sergiocorato Awaiting requested review from sergiocorato

Assignees
No one assigned
Labels
approved ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow to validate omocode italian fiscal code
7 participants
@patrickt-oforce @matteoopenf @OCA-git-bot @micheledic @francesco-ooops @sergiocorato @SirAionTech