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.

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

[14.0][IMP]l10n_it_delivery_note: split move lines based on dn #4324

Merged
merged 2 commits into from
Sep 10, 2024

Conversation

PicchiSeba
Copy link
Contributor

@PicchiSeba PicchiSeba commented Aug 20, 2024

Con questa PR, quando si creano fatture per un SO i cui prodotti sono suddivisi tra più DDT, le righe verranno organizzate in modo da indicare a quale DDT fanno riferimento i gruppi di prodotti

Esempio:

Screenshot from 2024-08-20 15-57-38

@OCA-git-bot
Copy link
Contributor

Hi @MarcoCalcagni, @aleuffre, @renda-dev,
some modules you are maintaining are being modified, check this out!

Copy link
Contributor

@francesco-ooops francesco-ooops left a comment

Choose a reason for hiding this comment

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

STEPS TO REPRODUCE

Creo SO1:
Customizable desk qty 2
Storage Box qty 1

Confermo

Valido picking1:
Customizable desk qty 1
Storage Box qty 1

Creo backorder
Creo e valido DDT1

Valido picking2:
Customizable desk qty 1

Creo e valido DDT2

Fatturo SO1

CURRENT BEHAVIOR

I riferimenti in fattura sono corretti, ma nell'ordine risulta che solo uno dei due "Customizable desks" sia stato effettivamente fatturato

image

@francesco-ooops
Copy link
Contributor

francesco-ooops commented Aug 27, 2024

Caso limite ma da gestire

STEPS TO REPRODUCE

Creo e confermo SO1:
Customizable desk qty 2

Valido picking1:
Customizable desk qty 1

Creo backorder
Creo e valido DDT1

Valido picking2:
Customizable desk qty 1

Creo DDT2 ma lo lascio in bozza

Creo e confermo SO2:
Customizable desk qty 2

Valido picking3:
Customizable desk qty 1

Creo backorder
Creo e valido DDT3

Valido picking4:
Customizable desk qty 1

Non creo un nuovo DDT ma ne seleziono uno esistente (DDT2 che è rimasto in bozza)
Valido DDT2

Ordini da fatturare > fatturo SO1 e SO2

CURRENT BEHAVIOR

Nella fattura ho le seguenti righe:

DDT1
Customizable desk qty 2
(errato: DDT1 ha qty 1)

DDT2
Customizable desk qty 1
Customizable desk qty 1
(corretto)

DDT3
Customizable desk qty 1
(corretto)

REQUIRED BEHAVIOR

Non fatturare più del consegnato

@PicchiSeba PicchiSeba force-pushed the 14.0-draft-split-move-lines-dn branch 2 times, most recently from 42b279e to c97d8b5 Compare August 27, 2024 15:13
Copy link
Contributor

@francesco-ooops francesco-ooops left a comment

Choose a reason for hiding this comment

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

Funzionalmente mi sembra ok

@odooNextev volete fare qualche prova anche voi per verificare che raggiunga l'obiettivo di #3979 ?

@PicchiSeba PicchiSeba force-pushed the 14.0-draft-split-move-lines-dn branch from c97d8b5 to cab4e16 Compare August 28, 2024 13:52
@PicchiSeba PicchiSeba force-pushed the 14.0-draft-split-move-lines-dn branch from cab4e16 to 8373df6 Compare August 28, 2024 15:02
@odooNextev
Copy link
Contributor

@PicchiSeba non hai previsto un flag per decidere se usare questo comportamento o quello vecchio?
Quando ne abbiamo parlato per la prima volta qualche tempo fa alcuni ci dicevano che a loro andava bene com'era.

@PicchiSeba
Copy link
Contributor Author

No, devo essermelo perso. Lo aggiungo adesso

@odooNextev
Copy link
Contributor

No, devo essermelo perso. Lo aggiungo adesso

Ok, sto provando a trovare le differenze negli approcci delle 2 PR, poi ti dico se anche nella 16 si riesce a raggruppare ed aggiungere le righe solamente con il metodo _assign_delivery_notes_invoices

@francesco-ooops
Copy link
Contributor

@PicchiSeba non hai previsto un flag per decidere se usare questo comportamento o quello vecchio? Quando ne abbiamo parlato per la prima volta qualche tempo fa alcuni ci dicevano che a loro andava bene com'era.

@odooNextev provo a ricostruire questa sanguinolenta questione dato che allo stato attuale la 14 e la 16 hanno due comportamenti diversi:

Cercando di risolvere/implementare #3242 (la issue menziona "la fatturazione di più DDT originati dalla stessa riga ordine di vendita risulta in più righe prima e dopo la riga della fattura", ma non ho capito come riprodurre il "dopo la riga della fattura")

è stata mergiata #3240
e questo è stato un errore mio per aver fatto una review funzionale non troppo approfondita, a mia discolpa le casistiche sono davvero tante e i test evidentemente inadeguati

il merge ha generato la issue #4309

Che è stata risolta da #4313

Poi ci siamo accorti che la issue era cambiata in #4342

Dunque abbiamo provato a portare a compimento il comportamento previsto nella prima issue linkata con un complesso refactoring in #4315

Ma questo approccio si è rivelato troppo invasivo e lo abbiamo lasciato perdere decidendo di modificare lo standard nel comportamento indicato in #3979 con questa PR

Ora non sono solito a cambiare i comportamenti standard di un modulo, ma in questo caso ha senso per vari motivi:

  1. non si può perdere tutto sto tempo dietro un modulo su cui chiunque mette le mani prova disgusto
  2. il modulo già prevede una "versione semplificata" (attualmente rotta e di cui sconsigliare rigorosamente l'uso) e una "versione avanzata" che ha aumentato la complessità e le casistiche da considerare, l'aggiunta di un nuovo flag con diversi comportamenti aumenterebbe nuovamente la complessità
  3. perchè i clienti stessi ci hanno confermato che è molto più comodo avere le righe fatture raggruppate per DDT, es:

DDT1
Prodotto 1, qty 1
Prodotto 2, qty 1

DDT2
Prodotto 1, qty1
Prodotto 2, qty1

piuttosto che raggruppate per prodotto:

DDT1
DDT2
Prodotto 1, qty 2

DDT1
DDT2
Prodotto 2, qty 2

Dunque tutto sto pippone per dire che da maintainer del modulo adotterei 1 riga ddt = 1 riga fattura come standard e se il raggruppamento per prodotto è qualcosa di desiderato si può discutere di un refactoring che riduca sensibilmente la complessità di questo modulo

@sergiocorato
Copy link
Contributor

Non mi è mai capitata la richiesta di fatture raggruppate per prodotto (sarebbe molto difficile da controllare rispetto ai DDT consegnati) per cui concordo che le righe vadano raggruppate per DDT (un utente prende il mano il DDT e le controlla a video o come vuole, spuntado le righe).

Non credo sia coperto il caso di una fatturazione di una parte dei DDT quando sulla stessa riga ci sono più DDT fatturabili, sto controllando.

@francesco-ooops
Copy link
Contributor

@sergiocorato se mi dici a grandi linee i passi per riprodurre faccio anche io dei test 👍

@sergiocorato
Copy link
Contributor

sergiocorato commented Aug 30, 2024

@sergiocorato se mi dici a grandi linee i passi per riprodurre faccio anche io dei test 👍

Credo di aver fatto così:

  1. creare un ordine di vendita con 2 righe con lo stesso prodotto con quantità 10
  2. consegnare con 1 DDT 2 pz della prima riga
  3. fatturare il DDT
  4. consegnare con 1 DDT 3 pz della prima riga
  5. consegnare con 1 DDT 4 pz della prima riga
  6. consegnare con 1 DDT 4 pz della seconda riga
  7. fattura i DDT alle righe 4 e 6. Resta quindi dare fatturare il DDT alla riga 5.

Non risulta nella descrizione della fattura il DDT alla riga 4.

@francesco-ooops
Copy link
Contributor

@sergiocorato questo con la funzionalità DN avanzati giusto?

per i punti 3 e 7 intendi fatturare direttamente dalla form del DN, corretto?

@sergiocorato
Copy link
Contributor

@sergiocorato questo con la funzionalità DN avanzati giusto?

per i punti 3 e 7 intendi fatturare direttamente dalla form del DN, corretto?

Esatto

@sergiocorato
Copy link
Contributor

Beh, dalla lista dei DDT nel secondo caso.

@sergiocorato
Copy link
Contributor

Ho proposto PyTech-SRL#2 , vedo se aggiungere un test.

@francesco-ooops
Copy link
Contributor

francesco-ooops commented Aug 30, 2024

Io quello che ho riscontrato su runboat è il seguente:

creare un ordine di vendita con 2 righe con lo stesso prodotto con quantità 10
consegnare con 1 DDT 2 pz della prima riga
fatturare il DDT
consegnare con 1 DDT 3 pz della prima riga
consegnare con 1 DDT 4 pz della prima riga
consegnare con 1 DDT 4 pz della seconda riga
fattura i DDT alle righe 4

Traceback (most recent call last):
File "/opt/odoo/odoo/http.py", line 652, in _handle_exception
return super(JsonRequest, self)._handle_exception(exception)
File "/opt/odoo/odoo/http.py", line 317, in _handle_exception
raise exception.with_traceback(None) from new_cause
IndexError: tuple index out of range

@sergiocorato ti risulta?

@PicchiSeba
Copy link
Contributor Author

@francesco-ooops ho riprodotto la procedura e non ho trovato errori. Che passaggi hai fatto di preciso?

@sergiocorato
Copy link
Contributor

@francesco-ooops No

@francesco-ooops
Copy link
Contributor

@PicchiSeba @sergiocorato l'errore è piuttosto complesso da riprodurre e non so se questa è la versione più "asciutta", ma sono riuscito solo con questo flusso: https://www.loom.com/share/02be10572e564b7dbd2fe0ae6606c85b

SO:

Riga 1: Prodotto 1, 10
Riga 2: Prodotto 1, 10

DDT1:

Prodotto 1, 2 (da riga 1)

Fatturo

DDT2:

Prodotto 1, 1 (da riga 2)

Valido

DDT 3:

Prodotto 1, 2 (da riga 1)
Prodotto 1, 1 (da riga 2)

Valido

Fatturo DDT2: errore tupla

@PicchiSeba
Copy link
Contributor Author

@francesco-ooops ho riprovato il tuo flusso con anche le modifiche aggiunte da @sergiocorato e quel caso lì è gestito correttamente.

@francesco-ooops
Copy link
Contributor

francesco-ooops commented Sep 9, 2024

Ho proposto PyTech-SRL#2 , vedo se aggiungere un test.

Raga mi viene da piangere, seguendo questi passi

  1. creare un ordine di vendita con 2 righe con lo stesso prodotto con quantità 10
  2. consegnare con 1 DDT 2 pz della prima riga
  3. fatturare il DDT
  4. consegnare con 1 DDT 3 pz della prima riga
  5. consegnare con 1 DDT 4 pz della prima riga
  6. consegnare con 1 DDT 4 pz della seconda riga
  7. fattura i DDT alle righe 4 e 6. Resta quindi dare fatturare il DDT alla riga 5.
  8. Non risulta nella descrizione della fattura il DDT alla riga 4.

mo' i ddt si fatturano da soli VIDEO

Al minuto 0.52 si vede che il DDT al punto 4 è solo validato e non fatturato
Al minuto 1.28, dopo la validazione del DDT al punto 5, il DDT del punto 4 risulta fatturato

@sergiocorato riesci ad aggiungere un test a stretto giro e a verificare?

@francesco-ooops
Copy link
Contributor

@sergiocorato aggiornato, avevo sbagliato link

Copy link
Contributor

@francesco-ooops francesco-ooops left a comment

Choose a reason for hiding this comment

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

al momento non funziona

@sergiocorato
Copy link
Contributor

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 14.0-ocabot-merge-pr-4324-by-sergiocorato-bump-minor, awaiting test results.

@francesco-ooops
Copy link
Contributor

Dopo il merge facciamo il revert (ho un concorso di colpa per aver lasciato la review positiva dopo aver riscontrato problemi)

@OCA-git-bot OCA-git-bot merged commit 2fa8983 into OCA:14.0 Sep 10, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at fd61381. Thanks a lot for contributing to OCA. ❤️

@sergiocorato
Copy link
Contributor

Mi dispiace, al momento non ho tempo di seguire questa PR (mergiata per errore!!!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants