-
-
Notifications
You must be signed in to change notification settings - Fork 305
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
[10.0] add dichiarazione d'intento #789
Conversation
invoice.date_invoice) | ||
if dichiarazioni: | ||
invoice.fiscal_position_id = \ | ||
dichiarazioni.fiscal_position_id.id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
E' corretto che si possano creare più di una dichiarazione d'intento per Tipo - Partner - Intervallo di date ?
In questo caso c'è un errore :
File "/usr/local/lib/python3.5/dist-packages/odoo/addons/l10n_it_dichiarazione_intento/models/account_invoice.py", line 28, in _set_fiscal_position
dichiarazioni.fiscal_position_id.id
File "/usr/lib/python3/dist-packages/odoo/fields.py", line 942, in get
record.ensure_one()
File "/usr/lib/python3/dist-packages/odoo/models.py", line 4393, in ensure_one
raise ValueError("Expected singleton: %s" % self)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potrebbe essere mi sa, visto che ci potrebbero essere diverse vendite con diverse dichiarazioni. (curiosità: stai usando la 10.0 su python 3.5?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(curiosità: stai usando la 10.0 su python 3.5?)
no, stavo migrando il modulo sulla 11, ma l'errore è lo stesso sulla 10
Propongo nel caso in cui ci siano più di una posizione fiscale, di lasciare all'utente la scelta (con il campo vuoto ma required). Altrimenti impostiamo direttamente l'unica posizione fiscale presente. |
Abbiamo fatto alcune verifiche e riguardato il codice. Provo a fare un resoconto. Altro discorso è invece quello relativo ad un cliente con più dichiarazioni, con periodo di validità uguale. |
record.display_name = display_name | ||
|
||
@api.multi | ||
@api.depends('line_ids', 'line_ids.amount') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
La funziona _compute_amounts calcola il valore di avaible_amount (visibile nella tree view)
Dovrebbe dipendere anche dal campo limit_amount
@labaggio Quello di cui parli è una scelta che fu fatta al momento dello sviluppo del modulo. SI pensava, infatti, di avvisare il cliente di aver superato il plafond e lasciare a lui la libertà o di cambiare l'iva sui dettagli della fattura fino a rientrare nel plafond o di permettergli di splittare la fattura in due facendo in modo che la fattura sorgente finisse nel primo plafond ed eventualmente la seconda fosse slegata dalla dichiarazione o fatta sottostare ad una dichiarazione nuova. Se credete che sia giusto sviluppare la funzionalità indicata non trovo obiezioni. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Grazie della PR!
Solo code review
.. contents:: | ||
:local: | ||
|
||
Configuration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Puoi rimuovere i capitoli vuoti?
|
||
**English** | ||
|
||
This module allows you to manage dichiarazioni d'intento. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Qui chiedo soccorso a @primes2h per i termini inglesi giusti.
Una volta trovati, potremmo avere anche la cartella del modulo in inglese?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Qui chiedo soccorso a @primes2h per i termini inglesi giusti.
Il termine corretto qui dovrebbe essere "declaration(s) of intent".
Vedi ad es. https://www.corintax.com/vat-exemption-scheme-for-usual-exporters-in-italy.html
Una volta trovati, potremmo avere anche la cartella del modulo in inglese?
Direi di si. 👍
'license': 'AGPL-3', | ||
'author': 'Francesco Apruzzese, Odoo Community Association (OCA), ' | ||
'Sergio Corato', | ||
'website': 'https://odoo-community.org/', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Qui ci andrebbe il link al repository o il link al modulo (vedi https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#modules)
state = 'valid' | ||
record.state = state | ||
|
||
def get_valid(self, type=None, partner_id=False, date=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type
oscura la funzione https://docs.python.org/2.7/library/functions.html#type, potresti usare un altro nome per la variabile?
# ----- If partner hasn't dichiarazioni, do nothing | ||
if not dichiarazioni: | ||
continue | ||
sign = 1 if invoice.type.endswith('_invoice') else -1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Qui e negli altri punti dove viene fatto questo controllo, meglio controllare che il tipo sia esattamente quello che ti aspetti, come viene fatto in https://github.com/odoo/odoo/blob/bb712ad7017db9c6477ceb40633e2460238b3317/addons/account/models/account_invoice.py#L1215
continue | ||
dichiarazione.line_ids = [(0, 0, { | ||
'taxes_ids': [(6, 0, [tax.id, ])], | ||
'move_line_ids': [(6, 0, [l.id for l in lines])], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'move_line_ids': [(6, 0, [l.id for l in lines])], | |
'move_line_ids': [(6, 0, lines.ids)], |
Questo può tornare utile in diversi punti del modulo
elif invoice_type.startswith('in_'): | ||
product_taxes = line.product_id.supplier_taxes_id | ||
else: | ||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vuoi davvero uscire o sarebbe meglio continue
?
if dichiarazione.available_amount < 0: | ||
raise UserError(_( | ||
'Limit passed for dichiarazione %s.\n' | ||
'Excess value: %s%s' % ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Solo la stringa va tradotta
'Excess value: %s%s' % ( | |
'Excess value: %s%s') % ( |
|
||
@api.multi | ||
@api.depends('number', 'partner_document_number') | ||
def _compute_clean_display_name(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Come mai non usi i metodi standard di display_name
?
Inoltre non c'è bisogno di ridefinire il campo
Per noi la funzionalità attuale va bene. Per quanto riguarda invece il selezionare la posizione fiscale sono d'accordo, cosi come suggerito in call, di prendere la prima disponibile. |
@CiroBoxHub noi abbiamo creato una PR che gestisce lo split tra più dichiarazioni |
@sergiocorato Com'è lo stato di questo modulo? Non l'ho ancora provato |
@CiroBoxHub Dove hai la PR per lo split su più dichiarazioni? |
Ah! forse è stata già mergiata in questa PR? |
guardando i messaggi la PR dello split è già mergiata in questa. |
non l'ho più seguito poi, non ce l'ho in produzione |
@sergiocorato Puoi fare il rebase? |
Inoltre se è funzionante proporrei di mergiarla così iniziamo a fare testing più ad ampio raggio sino a stabilizzarlo |
Il testing per renderla più stabile dovrebbe essere fatto prima di mergiarla... |
[FIX] flake8 [FIX] flake8 [FIX] readme and test [FIX] newline [FIX] remove print statement
Co-Authored-By: Simone Rubino <simone.rubino@agilebg.com>
… dichiarazioni (#6)
ehm, si prima test e verifiche, poi merge :D altrimenti ci 🗡️ |
@sergiocorato @labaggio ... ho fatto l'assunzione che dato che c'è una PR ... probabilmente c'è già qualcuno che la usa .. ma ... vabbbbene! Non lo dico più 💃 ! |
Ci sono i punti aperti delle varie revisioni. Se @sergiocorato non può farsene carico e qualcun altro può, ben venga. Inoltre travis è rosso |
No description provided.