-
-
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
[16.0][IMP]l10n_it_delivery_note: create invoice from delivery note #4184
base: 16.0
Are you sure you want to change the base?
[16.0][IMP]l10n_it_delivery_note: create invoice from delivery note #4184
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.
LGTM
@odooNextev questa PR è linkata ad una PR linkata ad un'altra PR, riusciresti ad aprire una nuova issue per spiegare in breve cosa andiamo a risolvere con questa? Grazie mille :) |
8bccb10
to
f7120b3
Compare
Vd #4188 |
@francesco-ooops ora c'è la issue |
f4f7178
to
785e9c0
Compare
@francesco-ooops @Borruso Ora i test passano. Riuscireste a fare una review? Grazie mille |
@odooNextev scusa mi son perso nella catena di issues e PR, mi daresti due indicazioni rapide per testare funzionalmente? |
Per testare dovresti generare una fattura da più ddt, dovresti ottenere le righe raggruppate per ogni delivery note. Nei settings trovi una checkbox dedicata "Group Delivery note invoices by quantity" che viene utilizzata per gestire questo caso oppure mantenere il metodo attuale. |
@odooNextev puoi aggiungere il funzionamento alla documentazione? |
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 mille per la PR
Molto ìnteressante
@@ -49,3 +49,6 @@ def _default_virtual_locations_root(self): | |||
related="company_id.display_delivery_method_dn_report", | |||
readonly=False, | |||
) | |||
delivery_note_group_by_quantity = fields.Boolean( | |||
string="Group Delivery note invoices by quantity" |
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.
dovresti mettere il related con la company come fatto per gli altri campi
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
8ace12c
to
1986d00
Compare
1986d00
to
85cfce8
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.
LGTM
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.
@odooNextev puoi aggiungere il funzionamento alla documentazione?
13e63d9
to
7bf8843
Compare
è sufficiente? |
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.
OK in uso presso cliente
puoi aggiornare l'Approve ? |
La documentazione è ok, al momento non riesco a fare test funzionale |
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
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.
A parte la documentazione con un file in italiano e uno in inglese ma vabè 😅 (non dipende certo da voi)
Funzionalmente mi sembra lineare anche se potrei essermi perso qualche casistica, mergerei
in ogni caso credo che il funzionamento senza il flag attivato sia da fixare:
1 ordine con 1 riga > 2 DDT > 1 fattura al momento risulta così nel runboat 16 (mentre nella 14 c'è il bug segnalato in #4309 )
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.
@odooNextev come dicevo funzionale ok, tuttavia a quasi quattro anni di distanza dal merge di questo modulo stiamo ancora lottando per avere il riferimento corretto del DDT in fattura (decisamente non una funzionalità secondaria e vari clienti ce l'hanno fatto notare con fastidio).
Mi sembra evidente che su questo tema non ci sia stata una analisi delle casistiche da gestire per poter aggiungere dei test che evitino regressioni, sarebbe cosa molto utile aggiungerle in questa PR, che ne dite?
Nel mentre stiamo facendo lo stesso per assicurare che funzioni il riferimento DDT in fattura anche senza questo flag attivo, appena fatto riportiamo tutto nella v16 👍
@francesco-ooops ok allora aggiungo dei test per la parte che abbiamo introdotto |
@odooNextev in realtà ho riscontrato una modifica al funzionamento standard in questa PR, passi per riprodurre:
SO, product 1, qty 2 Prima della PR: Dopo la PR |
7bf8843
to
d157c8f
Compare
@francesco-ooops ho aggiunto un test abbastanza corposo.
Stai testando nel runboat? Hai ragione sullo spostamento delle righe delle note, devo sistemare. |
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.
@odooNextev steps to reproduce:
Abilito flag "Group invoice lines by Delivery Note"
Creo SO001 con prodotto 1, qty 2, valido
Spedisco prodotto 1, qty 1 (creo backorder) > creo e valido DDT001 > creo fattura INV001 da ordine
Spedisco prodotto 1, qty 1 > creo e valido DDT002 > creo fattura INV002 da ordine
Current behavior
La seconda fattura INV002 non ha riferimento al DDT ma ha due volte la riga prodotto (?)
Required behavior
INV001 ha riferimento a DDT001, INV002 ha riferimento a DDT002
@MarcoCalcagni non so se è così sicuro tenerlo in ambiente di produzione al momento
d157c8f
to
811645b
Compare
@stenext pingami quando serve che testo :) |
Sì, devo correggere un paio di cose perciò ti avviso quando ho fatto, non fare affidamento sui push |
for line in invoiceable_lines.mapped("delivery_note_line_ids"): | ||
if not down_payment_section_added and line.sale_line_id.is_downpayment: | ||
# Create a dedicated section for the down payments | ||
# (put at the end of the invoiceable_lines) | ||
invoice_line_vals.append( | ||
Command.create( | ||
order._prepare_down_payment_section_line( | ||
sequence=invoice_item_sequence | ||
) | ||
), | ||
) | ||
down_payment_section_added = True | ||
invoice_item_sequence += 10 | ||
invoice_line_vals.append( | ||
Command.create( | ||
line._prepare_ddt_invoice_line(sequence=invoice_item_sequence) | ||
), | ||
) | ||
invoice_item_sequence += 10 | ||
|
||
return invoice_ids | ||
invoice_vals["invoice_line_ids"] += invoice_line_vals | ||
invoice_vals_list.append(invoice_vals) | ||
|
||
if not invoice_vals_list and self._context.get( | ||
"raise_if_nothing_to_invoice", True | ||
): | ||
raise UserError(self._nothing_to_invoice_error_message()) | ||
|
||
# 2) Manage 'grouped' parameter: group by (partner_id, currency_id). | ||
if not grouped: | ||
new_invoice_vals_list = [] | ||
invoice_grouping_keys = self._get_invoice_grouping_keys() | ||
invoice_vals_list = sorted( | ||
invoice_vals_list, | ||
key=lambda x: [ | ||
x.get(grouping_key) for grouping_key in invoice_grouping_keys | ||
], | ||
) | ||
for _grouping_keys, invoices in groupby( | ||
invoice_vals_list, | ||
key=lambda x: [ | ||
x.get(grouping_key) for grouping_key in invoice_grouping_keys | ||
], | ||
): | ||
origins = set() | ||
payment_refs = set() | ||
refs = set() | ||
ref_invoice_vals = None | ||
for invoice_vals in invoices: | ||
if not ref_invoice_vals: | ||
ref_invoice_vals = invoice_vals | ||
else: | ||
ref_invoice_vals["invoice_line_ids"] += invoice_vals[ | ||
"invoice_line_ids" | ||
] | ||
origins.add(invoice_vals["invoice_origin"]) | ||
payment_refs.add(invoice_vals["payment_reference"]) | ||
refs.add(invoice_vals["ref"]) | ||
ref_invoice_vals.update( | ||
{ | ||
"ref": ", ".join(refs)[:2000], | ||
"invoice_origin": ", ".join(origins), | ||
"payment_reference": len(payment_refs) == 1 | ||
and payment_refs.pop() | ||
or False, | ||
} | ||
) | ||
new_invoice_vals_list.append(ref_invoice_vals) | ||
invoice_vals_list = new_invoice_vals_list | ||
if len(invoice_vals_list) < len(self): | ||
SaleOrderLine = self.env["sale.order.line"] | ||
for invoice in invoice_vals_list: | ||
sequence = 1 | ||
for line in invoice["invoice_line_ids"]: | ||
line[2]["sequence"] = SaleOrderLine._get_invoice_line_sequence( | ||
new=sequence, old=line[2]["sequence"] | ||
) | ||
sequence += 1 | ||
|
||
moves = ( | ||
self.env["account.move"] | ||
.sudo() | ||
.with_context(default_move_type="out_invoice") | ||
.create(invoice_vals_list) | ||
) | ||
if final: | ||
moves.sudo().filtered( | ||
lambda m: m.amount_total < 0 | ||
).action_switch_invoice_into_refund_credit_note() | ||
for move in moves: | ||
move.message_post_with_view( | ||
"mail.message_origin_link", | ||
values={"self": move, "origin": move.line_ids.sale_line_ids.order_id}, | ||
subtype_id=self.env["ir.model.data"]._xmlid_to_res_id("mail.mt_note"), | ||
) | ||
# return moves | ||
self._assign_delivery_notes_invoices(moves.ids) | ||
self._generate_delivery_note_lines(moves.ids) | ||
return moves |
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.
issue: questa implementazione (che riprende, con piccole modifiche, il metodo per come scritto nel modulo sale
) non chiama mai super()
, rompendo tutti gli altri moduli che personalizzano _create_invoices()
, tra cui i moduli core sale_timesheet
e repair
e svariati moduli OCA
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 andare bene una soluzione del genere?
Chiamare la super()
nel caso in cui ci sia attivata l'opzione è quasi impossibile perchè andrebbe fatta a priori per poi distruggere tutto quello che ha fatto per sostituire le righe con il nuovo raggruppamento.
L'unica cosa è che diventerebbe impossibile richiamare il metodo originale nel caso il flag non fosse attivo ed ho dovuto riportare il metodo originale e non mi piace molto...
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.
Con un monkeypatching di questo tipo, credo non bisognerebbe fare un nuovo model ma solo definire la funzione (fuori dalla classe), e poi andarla a sostituire alla funzione originale come hai fatto in fondo.
A mali estremi, potrebbe funzionare. Di certo la funzione originale è troppo lunga per andarsi a inserire in maniera efficace.
Abbiamo provato un'implementazione alternativa per la stessa idea di fondo (1 riga DDT = 1 riga fattura) che speriamo essere più compatibile qui #4324 (Non è ancora pronta, ci sono test da aggiungere e edge cases da limare)
Al momento non è possibile creare una fattura direttamente da ddt come nelle precedenti versione. (#3552 )
Creazione fattura con più righe per ogni riga nei vari ddt:(#3979 )