Skip to content

[10.0] Improvement to l10n_it_fiscalcode - #278

Closed
Giuliano69 wants to merge 59 commits into
OCA:10.0from
Giuliano69:fc-01
Closed

[10.0] Improvement to l10n_it_fiscalcode #278
Giuliano69 wants to merge 59 commits into
OCA:10.0from
Giuliano69:fc-01

Conversation

@Giuliano69

Copy link
Copy Markdown

Hi
I modified it_fiscalcode module.
Changed :
-the check_fiscalcode algorithm, to consider different Italian fiscalcode length depending if partner is an Italian person or an Italian company. Removed field "individual" using instead "old" is_company field. Prefixed underscore "_" in name for OCA compatibility
-the button for the fiscalcode calculation based on personal data, is now visible only if the partner is a person and not if it is a company (in this case VAT code is the fiscalcode for any company, firm, business corporation...)

PS
I also have PROBLEM adding onchange() method on the fiscalcode field for further functionalities.
The method seems not to be called (nor in normal run, nor in debug mode..) .
May you please test if you get the same behaviour ?

I found a similar claim on
http://stackoverflow.com/questions/32326483/odoo-8-onchange-on-inherited-models
but classic inheritance should work for the onchange() method....

@oca-clabot

Copy link
Copy Markdown

Hey @Giuliano69, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/website.cla
Here is a list of the users:

Appreciation of efforts,
OCA CLAbot

Comment thread l10n_it_fiscalcode/model/res_partner.py Outdated
# Copyright 2014 Associazione Odoo Italia (<http://www.odoo-italia.org>)
# Copyright 2016 Andrea Gallina (Apulia Software)
# Copyright 2016 Giuliano Lotta
# version 0.1- changed method check_fiscalcode() to consider multiple situation

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The history of the module is in the commits. We don't need of a description of every version.

Comment thread l10n_it_fiscalcode/model/res_partner.py Outdated
# - https://github.com/OCA/maintainer-tools/blob/master/CONTRIBUTING.md#oca-guidelines
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).

# 1: imports of python lib

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review your comments

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

ciao Fabrizio,
revisioni effettuate

Comment thread l10n_it_fiscalcode/model/res_partner.py Outdated
# Constraints and onchanges
@api.multi
def check_fiscalcode(self):
def _check_fiscalcode(self):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are you sure that this functin isn't inherit by other modules? If you change function name, every module after this will crash.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

ciao Fabrizio,
la funzione è un contrain (ora in api.constrain) richiamato internamento nella fase di scrittura record.
Non viene invocato "esternamente". Modifica sicura.

 Changes to be committed:
	+ modified:   l10n_it_fiscalcode/model/res_partner.py
        -Removed _constrain field and function, as they have been deprecated
        in Odoo V8.
        -Modified check funcion to work with new @api.
        -Iserted different messages to explain error found in fiscalcode

	+ modified:   l10n_it_fiscalcode/tests/test_fiscalcode.py
        - removed link to  base.res.partner_2 (Agrolait) to the test the module
        (may be not present in each installation or may be a business company
        without applicable fiscalcode)
        Created a temporary partner (is a person) to test the fiscalcode.
     Changes to be committed:
        + modified:   l10n_it_fiscalcode/model/res_partner.py
            -Removed _constrain field and function, as they have been deprecated
            in Odoo V8.
            -Modified constrain check funcion to work with new @api.
            -Different messages to explain error found in fiscalcode

        + modified:   l10n_it_fiscalcode/tests/test_fiscalcode.py
            - removed link to  base.res.partner_2 (Agrolait) to the test the module
            (may be not present in each installation or may be a business company
            without applicable fiscalcode)
            Created a temporary partner (is a person) to test the fiscalcode.
@Giuliano69

Copy link
Copy Markdown
Author

Hi Fabrizio,
I removed also the _constrain field, as it has been deprecated from V8.
Ported method to new @api.constrains. The method is invoked internally for fiscalcode validation before writing the record to dbase.
At present, it should be invoked only from the res.partner model (user action) and the wizard (fiscalcode filed filling)

I'm getting some strange error from travis about the test unit files, that I cannot understand...

@Giuliano69 Giuliano69 changed the title Fc 01 [10.0] Improvement to l10n_it_fiscalcode Nov 5, 2016
Comment thread .travis.yml Outdated
- python-lxml # because pip installation is slow
- python-simplejson
- python-serial
- python-yaml

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Questo file non dovrebbe essere modificato

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Grazie della segnalazione, rimetto come in origine.

Mi chiedevo come mai il file travis del progetto italiano non è uguale al template dei quality tools:
https://github.com/OCA/maintainer-quality-tools/blob/master/sample_files/.travis.yml

Comment thread l10n_it_fiscalcode/model/res_partner.py Outdated
# fiscalcode empty. Nothing o check..
elif partner.is_company:
# fiscalcode not empty and partner is a company
if partner.country_id.name == u"Italia":

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

res.country.name è un campo traducibile. Userei il campo code

Comment thread l10n_it_fiscalcode/model/res_partner.py Outdated
"The fiscal code doesn't seem to be correct.", ["fiscalcode"])
]
if not is_fc_ok:
raise ValidationError(msg)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Il messaggio deve essere traducibile.

Vedi
https://github.com/OCA/maintainer-tools/blob/master/CONTRIBUTING.md
e in particolare
from openerp.tools.translate import _

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Scusa Lorenzo,
forse ho interpretato male le istruzioni per le traduzioni odoo (sono nuovo....) , ma i messaggi di errore e di help non dovrebbero venir tradotti automaticamente dal sistema senza usara _() ?

Oppure intendi che devo metterla in unicode ?

Leggevo su
https://www.odoo.com/documentation/10.0/reference/translations.html
Odoo automatically exports translatable strings from "data"-type content:
help/error messages of _constraints and _sql_constraints are exported

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Modifiche effettuate.
Verifiche e test travis a buon fine.

@eLBati

eLBati commented Nov 7, 2016

Copy link
Copy Markdown
Member

@Giuliano69 grazie della PR.

Controlla gli errorei sollevati da lint:

l10n_it_fiscalcode/model/res_partner.py:10:1: E302 expected 2 blank lines, found 1
l10n_it_fiscalcode/model/res_partner.py:22:17: E225 missing whitespace around operator
l10n_it_fiscalcode/model/res_partner.py:23:12: E225 missing whitespace around operator
l10n_it_fiscalcode/model/res_partner.py:25:38: E701 multiple statements on one line (colon)
l10n_it_fiscalcode/model/res_partner.py:26:17: E116 unexpected indentation (comment)
l10n_it_fiscalcode/model/res_partner.py:31:40: W291 trailing whitespace
l10n_it_fiscalcode/model/res_partner.py:33:26: E111 indentation is not a multiple of four
l10n_it_fiscalcode/model/res_partner.py:34:26: E111 indentation is not a multiple of four
l10n_it_fiscalcode/model/res_partner.py:34:31: E225 missing whitespace around operator
l10n_it_fiscalcode/model/res_partner.py:34:80: E501 line too long (119 > 79 characters)
l10n_it_fiscalcode/model/res_partner.py:38:26: E225 missing whitespace around operator
l10n_it_fiscalcode/model/res_partner.py:40:63: W291 trailing whitespace
l10n_it_fiscalcode/model/res_partner.py:41:80: E501 line too long (90 > 79 characters)
l10n_it_fiscalcode/model/res_partner.py:45:24: E225 missing whitespace around operator
l10n_it_fiscalcode/model/res_partner.py:46:80: E501 line too long (105 > 79 characters)
l10n_it_fiscalcode/model/res_partner.py:47:80: E501 line too long (103 > 79 characters)
l10n_it_fiscalcode/tests/test_fiscalcode.py:11:80: E501 line too long (99 > 79 characters)
l10n_it_fiscalcode/tests/test_fiscalcode.py:15:80: E501 line too long (84 > 79 characters)
l10n_it_fiscalcode/tests/test_fiscalcode.py:15:83: E202 whitespace before ')'
l10n_it_fiscalcode/tests/test_fiscalcode.py:19:22: E999 SyntaxError: invalid syntax

I problemi di tests/test_fiscalcode.py derivano da un errore di sintassi, segnalato anche da lint

@Giuliano69

Copy link
Copy Markdown
Author

Volevo proporre una fase di brainstorming con contributi di tutti, sia per una versione ancora migliore che per il backporting in corso.

FUNZIONALITÀ attese

  • PERSONE fisiche private. Richiede un codice fiscale sulla base del quale poter emettere fattura al posto della PIVA (obbligo dal 2013). E’ utile aver un supporto al calcolo del CF, ma non deve imporlo, a causa delle omocodie.
  • SOCIETA’ private /associazioni//enti pubblici. Sebbene spesso uguale alla PIVA, può essere diverso sia per gli enti pubblici che per le aziende costituite prima del dicembre 2001. In ogni caso,, è un campo numerico con lunghezza 13(PIVA=ITnn). Non serve avere un pulsante per il calcolo del CF perché totalmente diverso.
  • IMPRESA INDIVIDUALE ( persone fisiche titolari di … cc 2082 -2083)
    Civilmente e fiscalmente non esiste un soggetto giuridico azienda diverso dalla persona fisica (può usare il conto corrente di casa per l’azienda e presenta una unica dichiarazione redditi);
    Una soluzione immediata potrebbe essere inserire nel campo “name” il contenuto della “ditta” [Gianna Bella-Pulizie scale], ma codificare come “person” il partner.
    La soluzione attuale, invece, definisce un nuovo campo individual (che a questo punto dovrebbe essere presente solo nel caso di partner aziende...)
    Il campo individual per quale ALTRO scopo/modulo è utilizzato nella localizzazione italiana ? Attualmente mi viene in mente solo il caso di voler codificare eventuali altri collaboratori della impresa individuale…

Algoritmo CONSTRAIN campo codicefiscale

Allo stato attuale una verifica sulla completa correttezza del codice ficale non è possibile. Tuttavia a seconda dei casi è possibile “filtrare” alcuni casi principali:

  • PERSONE fisiche Italiane e straniere; sono possibili omocodie su territorio nazionale. Rischioso bloccare l’inserimento di un codice apparentemente sbagliato ma “corretto”. La LUNGHEZZA è tuttavia sempre di 16 caratteri ed è alfanumerico.
  • SOCIETA’ private /associazioni//enti pubblici ITALIANI : sia che siano pubbliche o private, il codice fiscale è lungo quanto la PIVA. In ogni caso,, è comunque numerico con lunghezza PIVA (13=ITnn). Nessun codice fiscale ammesso per società estere.

@gigidn

gigidn commented Nov 7, 2016

Copy link
Copy Markdown

@Giuliano69 ti ho gia' risposto ... ma repetita iuvant. Il codice fiscale che qui stiam discutento è qualcosa di semplicemente italiano, quindi solo numeri (niente IT) per aziende/enti/quel_che_ti_pare mentre per le persone fisiche e ditte individuali segue le regole del "codice fiscale".

Generalmente, ma non necessariamente, il codice fiscale di un'azienda coincide con la partita iva. Nel caso di cambio di sede legale i due valori diventano diversi seppur i caratteri di controllo coincidono (originariamente era una PI).

L'attuale modulo (v8) del codice fiscale permette il calcolo su tabelle proprie di codici comuni, questo rende lungo il processo di aggiornamento ed oltretutto questi dati non si capisce a quando son aggiornate etc etc ... sarebbe un bene dividere il modulo in una parte solo CF e l'altra calcolo che andrebbe installata separatamente.

Non capisco che fastidio ti dia il flag individual ... questo è strutturale del modulo CF e se valorizzato dice al sistema che la verifica del CF va fatta come se fosse un individuo e non un'azienda. mi sembra un modo pulito a differenza di quel che suggerisci te che diventerebbe un casino.
Se guardi la logica che ho proposto questo campo ha effetto se e solo se il partner è un'azienda (ignorato in caso di individui) con la logica di cui sopra ... non vedo altro modo piu' pulito di far questo.

@Giuliano69

Copy link
Copy Markdown
Author

Ciao gigi.
Forse non ho espresso bene il modello proposto per il codice fiscale. Sintetizzo

Se TOGLIAMO is_individual

  • La persona fisica (privata o imprenditore) avrebbero entrambi una codifica come “person”, ed un codice fiscale “normale” a 16 char
  • Le aziende (private o pubbliche) avrebbero tutte una codifica “company”, ed un codice fiscale a 11 num (se non vogliamo IT)
  • la verifica della lunghezza del codice fiscale è banale
  • non è possibile dare dipendenti all’imprenditore individuale

Se METTIAMO is_individual,

  • dalla persona privata dobbiamo togliere PIVA (non è più applicabile se gli imprenditori finiscono in company)
  • nella azienda avremo CF con lunghezze diverse, e dobbiamo discriminare la congruità del CF a seconda del nuovo campo
  • il supporto al calcolo codice fiscale va disabilitato per le aziende/società “normali”
    -possiamo attribuire dipendenti all’imprenditore individuale

La prima soluzione è più semplice, e non aggiunge ulteriori campi.
La seconda soluzione aggiunge un ulteriore campo.

Se il campo è stato sempre presente nella V8, e gli utenti ci sono ormai abituati, lo rimette ed implemento le varie modifiche nel caso di persona/azienda dette sopra....
Il vero problema sarabbe stato se is_individual era usato da altri modulit....

                removed python libraries not present in original
                branch
	modified:   l10n_it_fiscalcode/i18n/it.po
                translated new terms in Italian
	modified:   l10n_it_fiscalcode/model/res_partner.py
                Reinroduced field (_is_sole_trader) to idemtify
                when a company is a sole trader.
                Modified check method to consider lenght and alphanum
                when checking the fiscalcode in different situation
	modified:   l10n_it_fiscalcode/tests/test_fiscalcode.py
	modified:   l10n_it_fiscalcode/view/fiscalcode_view.xml
                field is_soletrader dinamically get visible when
                partner is a company
	modified:   l10n_it_fiscalcode/wizard/compute_fc.py
                Only literal strings can be marked for exports,
                not expressions or variables.
	modified:   l10n_it_fiscalcode/wizard/compute_fc_view.xml
@Giuliano69

Copy link
Copy Markdown
Author

Nuova versione: reintrodotto il campo per impresa individuale nel caso di partner azienda; gestione dinamica del campo stesso e del pulsante calcolo CF, a seconda che il contatto sia azienda o meno, e che sia impresa individuale o meno.
Modifiche al codice di verifica fiscalcode; Traduzione nuovi termini.

Per la validazione del codice fiscale "sicura" con l'attuale libreria "codicefiscale", ho aperto una domanda agli siluppatori:
ema/pycodicefiscale#3
attualmente le omocodie NON sono supportate e il codice darebbe errore (falso negativo) se capitasse una omocodia. Mi segnalano invece che la diversa libraria
https://arthurdejong.org/python-stdnum/ sembrerebbe più aggiornata (?!) e capace di validare anche i codici ficali con omocodie......

DOMANDE

Travis si inchioda sempre lamentandosi che non può spedire mail (u"Unable to send email, please configure the sender's email address.", None)... si può aggirare ?

Esiste un modo per cambiare dinamicamente colore/stile ad un campo per segnalare che un valore (fiscalcode) richiede attenzione ?
Un qualcosa tipo gli attrs {} con isvisible, readonly, required, che cambiano a seconda di una condizione...
Vedevo qualcosa fatto da AgileBG, per la 6.1 solamente
http://planet.agilebg.com/2012/09/openerp-custom-fields-colors-and-styles/
Niente di attuale ?

…check (omocodie)

	modified:   l10n_it_fiscalcode/i18n/it.po (translation update)
	modified:   l10n_it_fiscalcode/model/res_partner.py (fiscal code validation against omocodie)
	modified:   l10n_it_fiscalcode/tests/test_fiscalcode.py (changed defaut use to admin)
…de.py:21:13: E124 closing bracket does not match visual indentation
	modified:   l10n_it_fiscalcode/tests/test_fiscalcode.py - changed user reference in setup, and gived him an email
…ode from fiscalcode check in wizard, to validation of constrains in partner record creation
…in filter; added test for empty fiscalcode field
… command for Italian country id, and added test for no fiscalcode in partner
@Giuliano69

Copy link
Copy Markdown
Author

Copertura test codice raggiunta anche con codecov.
Change requests implementate.
Tutti i test superati.

@Giuliano69

Copy link
Copy Markdown
Author

Chiedo un aiuto per le prossime volte...
Ho perso molto tempo con Travis
in quanto in locale non riesco a convincere Odoo ad eseguire i test.
Ho provato anche a dare ogni parametro via riga di comando per ridurre le variabili, ma senza successo.
Qualche idea da cosa può dipendere ?
https://odoo-italia.org/index.php/kunena/20-python/13610-test-unit-not-running-odoo-10?start=6

@eLBati eLBati left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@Giuliano69 grazie mille per il contributo.
Ti ho lasciato qualche commento.

Relativamente ai problemi con i test, questo https://www.odoo.com/forum/how-to/developers-13/how-to-run-tests-526 non aiuta?

Infine, riesci a fare squash di un po' di commits in modo da unirli secondo la loro logica?

Grazie

'Changing partner type from company to '
'person will remove the "Sole Trader" selection '
'in the Accounting tab.\nFiscal code may need '
'to be changed accordingly.')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Visualizzerei il messaggio solo se is_soletrader fosse True, per avvisare del cambiamento

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Implementato la condizione su is_soletrader e la traduzione in italiano.

Comment thread l10n_it_fiscalcode/wizard/compute_fc.py Outdated
msg = _(
'Insert fiscal code %s is different'
' from the computed one ( %s ).\nIf you want to use'
' the computed one, please remove first the insert'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

--> inserted o anche provided

@Giuliano69

Copy link
Copy Markdown
Author

Ciao Lorenzo,
ho inserito le modifiche richieste.
Ho AGGIUNTO anche una verifica sul codice fiscale inserito, rispetto ad un codice fiscale identico già in uso (verifica non bloccante) e la possibilità di ricerca anche sul campo codice fiscale.

Ho un PROBLEMA:
a me su travis si conclude senza nessun tipo di errore, qui invece
dice che tutti i check sono falliti (???)
si lamenta del file di traduzione in confitto con non so cosa .

Mi dai qualche indizio ? :-)

@eLBati

eLBati commented Dec 15, 2016

Copy link
Copy Markdown
Member

@Giuliano69 Vedo che travis è verde. Mentre mancherebbe ancora la copertura dei test.
Ti ho risposto velocemente sul forum.
Vuoi provare a descrivere qua, con riferimenti al codice, il problema che dicevi?

@Giuliano69

Copy link
Copy Markdown
Author

Ciao Lorenzo,
il modulo di test evidenza un bug dell'ORM (come da screenshot in fase di debugging).
La classe Transactioncase dovrebbe fare un rollback dei nuovi record a test finito, ma nell'eseguire un metodo onchange su un record appena creato nel TransactionCase, questo risulta già cancellato.
Da qui due problemi:

  • -un eventuale test che controlli il record fallisce
  • -non si può coprire tutto il codice come vorrebbe codecov, perchè se il record non esiste non viene eseguita una certa parte di codice.

Tutti i test tranne codecov sono superati.
Cosa serve per terminare il modulo ?

@OpenCode

Copy link
Copy Markdown
Contributor

@Giuliano69

TransactionCase fa girare i metodi in transazioni differenti. Pertanto i metodi sono isolati. Nel tuo caso, se non ho capito male, ti serve SingleTransactionCase

https://www.odoo.com/documentation/8.0/reference/testing.html#openerp.tests.common.TransactionCase

eLBati and others added 3 commits January 11, 2017 16:48
… functionalities

	modified:   l10n_it_fiscalcode/i18n/it.po - added new terms
	modified:   l10n_it_fiscalcode/model/res_partner.py - modified onchange() to check for same fiscalcode
	modified:   l10n_it_fiscalcode/tests/test_partner.py - modified test unit for same fiscalcode testcase
@oca-clabot

Copy link
Copy Markdown

Hey @Giuliano69,
We acknowledge that the following users have signed our Contributor License Agreement:

Appreciation of efforts,
OCA CLAbot

@eLBati eLBati left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Grazie @Giuliano69 , ti ho lasciato dei commenti, fammi sapere cosa ne pensi.

Poi faccio un test manuale completo e direi che possiamo andare avanti

* Python module to handle/check standardized numbers and codes `python-stdnum`_.
* Python library for Italian fiscal code creation - `codicefiscale`_.

.. _python-stdnum: https://pypi.python.org/pypi/python-stdnum/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ciao @Giuliano69 , non è necessaria un'altra libreria: codicefiscale permette di verificare la validità del CF , vedi https://github.com/ema/pycodicefiscale

Comment thread .travis.yml
after_success:
- travis_after_tests_success
- coveralls
- codecov

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Queste 2 righe non dovrebbero essere aggiunte

""" Each test method is run independently and the database transaction
is rolled back after each.
"""
def setUp(self):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Questo metodo può essere rimosso

@FrancoTampieri

Copy link
Copy Markdown

@eLBati @Giuliano69 @OpenCode I see that PR is in stand-by from a while, can I help you to finalize it?

@eLBati

eLBati commented Aug 28, 2018

Copy link
Copy Markdown
Member

Chiudo per mancanza di risposte. Se qualcuno è interessato può ovviamente riaprirla

@eLBati eLBati closed this Aug 28, 2018
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.

6 participants