-
-
Notifications
You must be signed in to change notification settings - Fork 301
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_riba: aggiungere esposizione/rischio e migliorare la gestione degli insoluti #3984
base: 16.0
Are you sure you want to change the base?
Conversation
a6dbfc3
to
23aded2
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.
LGTM
Go !!
@OCA/local-italy-maintainers abbiamo un paio di review funzionali positive, c'è qualcuno che può farne una tecnica? |
l10n_it_riba/models/account.py
Outdated
riba_credited_ids = fields.One2many( | ||
"riba.slip", "credit_move_id", "Credited RiBa Slips", readonly=True | ||
) | ||
|
||
exposition = fields.Float( |
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.
Trattandosi di un nuovo campo documenterei la sua funzionalità sul README. Inoltre potrebbe aver senso renderlo store
in modo da ordinare la tree
per il nuovo campo. Cosa ne pensi?
Ultima cosa: documenterei anche lato codice la nuova funzionalità ad esempio aggiungendo l'attributo help
al campo.
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.
buona idea
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 riesci a fare le modifiche?
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.
domani dovrei riuscire
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.
@tafaRU ho fatto quanto chiedevi
@andreampiovesana più tardi cerco di aggiungere quello che mi hai chiesto
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 @odooNextev
mancherebbe ancora questo punto 😉
Trattandosi di un nuovo campo documenterei la sua funzionalità sul README
Hai modo di farlo?
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 aggiunto un messaggio nello USAGE, non so se va bene.
In ogni caso, precommit mi ha generato e modificato i file .rst dagli .md, è corretto o bisogna lasciarlo fare a Github?
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.
@tafaRU va bene?
l10n_it_riba/models/account.py
Outdated
riba_credited_ids = fields.One2many( | ||
"riba.slip", "credit_move_id", "Credited RiBa Slips", readonly=True | ||
) | ||
|
||
exposition = fields.Float( |
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 un appunto al volo, exposition
in inglese indica esposizione nel senso di mostra (es. evento pubblico come una fiera), non ha a che fare con l'ambito contabile.
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' vero, infatti in chiamata un venerdì avevo chiesto che termine era più appropriato per il campo, ma era passato questo.
Sì potrebbe chiamare "risk" o "overdraft"
Dimmi tu
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' vero, infatti in chiamata un venerdì avevo chiesto che termine era più appropriato per il campo, ma era passato questo. Sì potrebbe chiamare "risk" o "overdraft" Dimmi tu
Un "overdraft" si verifica quando viene prelevata una somma superiore a quella presente in un conto.
In questo caso mi pare di capire che il termine si riferisca alle fatture non pagate ma "non" ancora scadute, giusto?
In tal caso potrebbe andar bene un semplice "open 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.
@primes2h ho fatto
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.
@primes2h ho fatto
Grazie 👍
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.
@primes2h ho fatto
@odooNextev Ah, visto ora.
Andrebbe modificato anche il messaggio di commit 1aa2e40
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.
Riscontro un'anomalia e a tal proposito allego 2 video:
-
riba insoluta errata --> quando registro l'insoluto con "salta e conferma insoluto" non evidenzia l'insoluto, nè riapre la posizione pertanto non posso riemettere la riba a saldo
-
riba insoluta corretta --> quando registro l'insoluto con "crea" tutto procede correttamente e posso riemettere la riba sulla fattura.
Sarebbe inoltre interessante prendere in esame l'idea di emettere riba scaglionate a fronte di un unico insoluto
Video.zip
Grazie della segnalazione, provo a cercare l'errore. |
243518f
to
4e6cd9d
Compare
E' stato aggiornato qualcosa nel pre-commit? In alcuni thread ho trovato che dovrei fare qualcosa del genere:
Così passa, ma non ne capisco il senso |
4e6cd9d
to
dde3ba4
Compare
Non credo, ho dovuto fare un bel po' di correzioni per quell'errore già in #4059. |
Abbiamo provato a risolvere il problema con questo commit: e1f5850 @MaurizioPellegrinet fammi sapere cosa ne pensi. |
Ciao, secondo me la soluzione migliore rimane quella di fare i vari passaggi esclusivamente con il tasto "crea", sia nell'operazione di accredito che in quella di pagamento o insoluto. |
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
@primes2h puoi verificare se adesso va bene?
@tafaRU puoi aggiornare la review? |
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
merge? |
@odooNextev riesci a fare un rebase per risolvere i confilitti? |
e1f5850
to
c6a2f10
Compare
Buonasera, dai test che ho fatto anche io manca un passo ovvero, nel momento in cui viene eliminata la riconciliazione tra fattura e riba il movimento che era legato alla fattura deve essere riconciliato con l'insoluto in questo modo il giro è completo e soprattutto poi si può mettere quella RIBA in un altra distinta laddove necessario. Da ulteriori verifiche per la quadratura dei conti contabili utilizzati la registrazione dell'insoluto non sistema il valore insoluto dai conti portafogli riba e riba all'incasso. Prima di questa PR quando si indicava una RIBA come insoluta veniva generata una registrazione contabile che utilizzava 4 conti contabili e quella deve continuare ad esistere. |
@odooNextev questa funzionalità rientra nello scopo della PR a tua opinione? |
Lo scopo della PR dovrebbe essere di togliere la riconciliazione della riba alla fattura e riconciliare quella riba con l'insoluto ma le altre registrazioni devono restare. |
@MaurizioConte @francesco-ooops appena riesco a guardarci provo a risolvere |
@MaurizioConte secondo noi il problema è la differenza nella gestione del SBF da parte delle diverse banche.
Probabilmente il vostro flusso è differente, può capitare infatti che l'accredito riba corrisponda all'effettivo movimento del conto corrente che nel nostro caso avviene solo all'incasso e quindi per noi l'insoluto non tocca i conti di portafogli e di credito riba. |
Il senso è questo - cerco di spegarlo come posso -: quando una funzione python viene definita, non vengono salvati i valori delle variabili non locali, ma i riferimenti (puntatori in C, reference in perl, ecc. chiamali come vuoi) all'enviroment esterno. I loro valori vengono presi al momento dell'esecuzione della funzione. In questo caso, non è un problema perché la lambda viene eseguita subito alla riga successiva dopo la E la variabile in questione Ma se la funzione venisse eseguita dopo (fuori da loop), e se il valore di Per fare un es. pratico, se la condizione recitasse In questo caso, visto che tanto nel loop non cambia, io proverei semplicemente a spostare fuori dal loop Dimostrazione del problema:
|
@TheMule71 chiarissimo, grazie |
@MaurizioConte ho aggiunto un'impostazione per definire se si utilizza SBF con incasso immediato o con incasso a maturazione valuta. In questa maniera si può tenere il "vecchio" comportamento negli insoluti che può andare bene a voi, mentre selezionando "At currency maturation" può andare bene a noi. |
c44fc94
to
528938b
Compare
@MaurizioPellegrinet ho aggiunto la possibilità di modificare la data di registrazione di una registrazione ( E' ancora embrionale perchè bisognerà capire che nome dargli e soprattutto cercare di limitarlo alle riba per evitare usi impropri, ma sembra funzionare. Fammi sapere se trovi qualche problema. |
@odooNextev grazie mille, sembra funzionare. Lunedì provo altri test più approfonditi per verificare che non ci siano altre implicazioni. Superrrrr! |
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.
Revisipne 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! Nel complesso, da un punto di vista tecnico, mi sembra 👍
Ci sono però alcuni punti da verificare / dubbi da chiarire:
- tutte le nuove funzionalità, non solo la prima che hai aggiunto, andrebbero descritte nel README
- alcuni parti di codice riportano il copyright di altri autori ma non vedo l'autorship del commit: è voluto?
- la history dei commit la vuoi tenere così?
Grazie.
@@ -405,11 +405,6 @@ def test_past_due_riba(self): | |||
# self.assertTrue( | |||
# bank_past_due_line.id in [l.id for l in move_lines_for_rec]) | |||
|
|||
riba_list.line_ids[0].past_due_move_id.line_ids.remove_move_reconcile() |
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 hai eliminato queste righe?
Ok
Immagino intendi questo: https://github.com/OCA/l10n-italy/pull/3984/files#diff-14f17eafd7915abe5ba350f6628d6b1b7850500fff1816f21c532091cfaa22d5L16
Dipende dal dettaglio che vogliamo avere, li ho lasciati separati perchè sono abbastanza indipendenti così se qualcuno volesse lavorarci e disattivare alcune funzionalità può farlo. |
Mi riferisco a questi due commit: In [1] aggiungi a4227cd#diff-04a8a365a32c3d6bceb153a18902bea732d9146b2328388fda2901a10f3b3d3bR147 come mai? |
Penso sia dovuto ad un rebase con fixup... |
2e87feb
to
e1b616c
Compare
e1b616c
to
5803821
Compare
@tafaRU dovrei aver fatto tutto quello che c'era da fare:
|
Ammontare scoperto/esposizione
Per risolvere questa issue #3983 abbiamo assegnato all'
account.move.line
del pagamento che viene creata alla conferma dellariba.slip.line
la data effettiva della scadenza dellariba.slip.line
e non quella dellariba.slip
.Nel modello
account.move
abbiamo aggiunto un campo che calcola lo scoperto/esposizione della fattura, ovvero quello che attualmente si suppone solamente che sia pagato, ma effettivamente no come indicato nella vista qui sotto:Insoluti
Per risolvere questa issue #3983 abbiamo eliminato le 2 righe di registrazione sezionale con nome "Bills" e conto effects_account_id in credito e quella con nome "RiBa" e conto riba_bank_account_id in debito e quando si marca come insoluta una riba.slip.line abbiamo annullato la riconciliazione con la account.move.line del pagamento correlata.
Facendo ciò la fattura tornerebbe "non pagata" o "parzialmente pagata" (oltre ad avere il flag "past due").
Ho dovuto rimuovere una parte dei test con questo commit: af22ff9
Suppongo che
riba_list.line_ids[0].past_due_move_id.line_ids
non si riferisca più alle 2 righe di registrazione contabile "Bills" e "RiBa" che non vengono più create e quindi è inutile fare dei check, però l'ho messo in un commitseparato per conferma.