-
-
Notifications
You must be signed in to change notification settings - Fork 320
[16.0] [FIX] l10n_it_split_payment: when editing invoice lines, move lines are not correctly computed #4233
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
Conversation
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.
Test funzionale OK
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!
Puoi modificare il messaggio del commit in modo che segua le linee guida in https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#71commit-message?
In particolare:
please check if the commit message is cut with ellipsis
Puoi aggiungere un test? Così si evitano regressioni.
return res | ||
for move in self: | ||
if move.split_payment: | ||
split_payment_total = 0 |
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.
Questo viene dichiarato qui e sommato poco sotto, ma non trovo dove viene usato: a cosa serve?
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.
Rimosso
write_off_line_vals = line._build_writeoff_line() | ||
split_payment_total += write_off_line_vals["debit"] | ||
line_sp = fields.first( | ||
line.move_id.line_ids.filtered( |
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.
line.move_id.line_ids.filtered( | |
move.line_ids.filtered( |
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.
Fatto
float_compare( | ||
line_sp.price_unit, | ||
write_off_line_vals["price_unit"], | ||
precision_rounding=line.move_id.currency_id.rounding, |
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.
precision_rounding=line.move_id.currency_id.rounding, | |
precision_rounding=move.currency_id.rounding, |
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.
Fatto
): | ||
line_sp.write(write_off_line_vals) | ||
else: | ||
if line.move_id.amount_sp: |
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.
if line.move_id.amount_sp: | |
if move.amount_sp: |
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.
Fatto
if line.display_type == "tax" and not line.is_split_payment: | ||
write_off_line_vals = line._build_writeoff_line() | ||
split_payment_total += write_off_line_vals["debit"] | ||
line_sp = fields.first( |
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.
Per ogni riga cerca tra tutte le righe della move
e troverà sempre la stessa line_sp
, non sarebbe meglio salvarla? Così almeno evitiamo di rifare la ricerca e il filtered
che tra l'altro ora è anche su un campo calcolato.
Se poi riuscissimo anche a fare la scrittura fuori dal loop non sarebbe male.
In pratica, ora che siamo nella move
si potrebbe riscrivere un po' questo loop per evitare di rifare le stesse operazioni per ogni riga: ad esempio ottenere in un colpo solo l'importo da mettere nel writeoff
da tutte le righe che non sono is_split_payment
, visto che _build_writeoff_line
prende quasi tutti i dati dalla move
; cosa ne pensi?
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.
Ho tirato fuori line_sp
ma non mi è chiara l'altra proposta
if line.move_id.amount_sp: | ||
line.move_id.with_context( | ||
skip_split_payment_computation=True | ||
).line_ids = [(0, 0, write_off_line_vals)] |
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.
Rispetto al codice rimosso sotto non c'è più _sync_dynamic_lines
, sai mica come mai non serve più?
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.
Dovrebbe essere lui a scrivere le move lines indesiderate
|
||
|
||
class AccountMoveLine(models.Model): | ||
_inherit = "account.move.line" | ||
|
||
is_split_payment = fields.Boolean() | ||
is_split_payment = fields.Boolean(compute="_compute_is_split_payment") |
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.
Si potrebbe rendere storato e dipendente da account_id
, che ne dici?
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.
Fatto
if line.move_id.amount_sp: | ||
line.move_id.with_context( | ||
skip_split_payment_computation=True | ||
).line_ids = [(0, 0, write_off_line_vals)] |
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.
).line_ids = [(0, 0, write_off_line_vals)] | |
).line_ids = [Command.create(write_off_line_vals)] |
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.
Fatto
f6b5fe7
to
0e6a669
Compare
Fatto
Al momento non riesco |
c34606a
to
1b72988
Compare
1b72988
to
5cabb0a
Compare
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.
Funzionale OK
per caso verrà mergiata a breve? |
@matteoopenf stando alla review, mancava l'aggiunta di un test se @eLBati non riesce potreste aggiungerlo voi |
/ocabot rebase |
When editing invoice lines, move lines are not correctly computed
Congratulations, PR rebased to 16.0. |
5cabb0a
to
f003442
Compare
@francesco-ooops col codice della #4233, continuo ad avere il problema. Ho fatto un test, aggiungendo al context check_move_validity=False, quando la write di account.move chiama la super e richiamando _check_balanced prima della return. ![]() |
Buongiorno, ho testato la PR con le modifiche suggerite anche da @masimassimo relativamente al problema segnalato nella #4432 ma il risultato è il medesimo. L'errore di creazione fattura per scritture sbilanciate rimane. |
Ho anche verificato che il problema ora si presenta anche senza la presenza del DDT |
@CiroBoxHub se vuoi possiamo vedere di fare qualche test insieme |
Ciao @masimassimo ho fatto un test a partire dalla runbot della PR è ho il problema anche li http://oca-l10n-italy-16-0-pr4233-f003442ef656.runboat.odoo-community.org/web?debug=1#cids=1&menu_id=160&action=274&model=account.move&view_type=form&id=16 questa la fattura creata che non fa salvare. C'è qualcosa di configurato male? |
Ciao @masimassimo, il secondo screen del messaggio fa riferimento a una fattura o una nota di credito? Perché per un nostro cliente ci siamo accorti che generando la nota di credito dalla fattura si perde il giroconto dello split payment; da una veloce analisi sembra che il metodo _sync_dynamic_line a riga 2107 identifichi la riga dello split payment come da rimuovere, andando poi ad accorpare l'iva al totale della fattura |
@patrickt-oforce è una fattura |
@CiroBoxHub lo screenshot col codice che ho riportato nel precedente commento, è diverso da quello della PR. |
Ciao @masimassimo si infatti l'ho riportato manualmente nel mio ambiente ma continuo ad avere il problema |
qualcuno ha poi trovato qualche soluzione? |
quindi a te con il fix nello screen sta funzionando in produzione? |
@masimassimo abbiamo aperto questa che comprende il tuo fix e ora sembra funzionare tutto correttamente. |
Quindi chiudo in favore di #4630 |
#4231