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

[16.0] [IMP] l10n_it_riba: aggiungere esposizione/rischio e migliorare la gestione degli insoluti #3984

Open
wants to merge 5 commits into
base: 16.0
Choose a base branch
from

Conversation

odooNextev
Copy link
Contributor

@odooNextev odooNextev commented Feb 22, 2024

Ammontare scoperto/esposizione

Per risolvere questa issue #3983 abbiamo assegnato all'account.move.line del pagamento che viene creata alla conferma della riba.slip.line la data effettiva della scadenza della riba.slip.line e non quella della riba.slip.

image

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:

image

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").

image

image

image

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.

@francesco-ooops francesco-ooops linked an issue Feb 22, 2024 that may be closed by this pull request
2 tasks
@odooNextev odooNextev changed the title [16.0] [IMP] l10n_it_riba: add exposition amount field [16.0] [IMP] l10n_it_riba: aggiungere esposizione/rischio e migliorare la gestione degli insoluti Feb 23, 2024
Copy link

@MaurizioPellegrinet MaurizioPellegrinet left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@MarcoCalcagni MarcoCalcagni left a comment

Choose a reason for hiding this comment

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

LGTM

Go !!

@odooNextev
Copy link
Contributor Author

@OCA/local-italy-maintainers abbiamo un paio di review funzionali positive, c'è qualcuno che può farne una tecnica?

riba_credited_ids = fields.One2many(
"riba.slip", "credit_move_id", "Credited RiBa Slips", readonly=True
)

exposition = fields.Float(
Copy link
Member

@tafaRU tafaRU Mar 8, 2024

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

buona idea

Copy link
Contributor

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

domani dovrei riuscire

Copy link
Contributor Author

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

Copy link
Member

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?

Copy link
Contributor Author

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

@tafaRU va bene?

riba_credited_ids = fields.One2many(
"riba.slip", "credit_move_id", "Credited RiBa Slips", readonly=True
)

exposition = fields.Float(
Copy link
Contributor

@primes2h primes2h Mar 28, 2024

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.

Copy link
Contributor Author

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

Copy link
Contributor

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".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@primes2h ho fatto

Copy link
Contributor

Choose a reason for hiding this comment

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

@primes2h ho fatto

Grazie 👍

Copy link
Contributor

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

Copy link

@MaurizioPellegrinet MaurizioPellegrinet left a 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:

  1. 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

  2. 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

@odooNextev
Copy link
Contributor Author

Riscontro un'anomalia e a tal proposito allego 2 video:

  1. 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
  2. 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.

@stenext stenext force-pushed the 16.0-imp-l10n_it_riba-expo branch 2 times, most recently from 243518f to 4e6cd9d Compare April 12, 2024 10:31
@odooNextev
Copy link
Contributor Author

odooNextev commented Apr 12, 2024

E' stato aggiornato qualcosa nel pre-commit?
Prima di oggi non ricevevo questo messaggio ed in locale non fallisce ancora adesso:
l10n_it_riba/models/account.py:69:46: B023 Function definition does not bind loop variable today``

In alcuni thread ho trovato che dovrei fare qualcosa del genere:

lambda line, today=today: line.riba

Così passa, ma non ne capisco il senso

@SirAionTech
Copy link
Contributor

E' stato aggiornato qualcosa nel pre-commit?
Prima di oggi non ricevevo questo messaggio ed in locale non fallisce ancora adesso:
l10n_it_riba/models/account.py:69:46: B023 Function definition does not bind loop variable today``

Non credo, ho dovuto fare un bel po' di correzioni per quell'errore già in #4059.

@odooNextev
Copy link
Contributor Author

odooNextev commented Apr 19, 2024

Riscontro un'anomalia e a tal proposito allego 2 video:

  1. 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
  2. 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

Abbiamo provato a risolvere il problema con questo commit: e1f5850
Il senso è "se si salta il wizard per marcare come insoluta una riga della distinta, significa che non voglio creare ulteriori righe in contabilità" perciò abbiamo pensato di annullare e poi eliminare la registrazione relativa alla riga che viene creata all'accredito della distinta (acceptance_move_id).
In questa maniera la fattura torna "parzialmente pagata" (se ci sono altri pagamenti) o "non pagata" ed è possibile riemettere la riba (togliendo i filtri dalla lista "emissione".

@MaurizioPellegrinet fammi sapere cosa ne pensi.

@MaurizioPellegrinet
Copy link

Riscontro un'anomalia e a tal proposito allego 2 video:

  1. 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
  2. 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

Abbiamo provato a risolvere il problema con questo commit: e1f5850 Il senso è "se si salta il wizard per marcare come insoluta una riga della distinta, significa che non voglio creare ulteriori righe in contabilità" perciò abbiamo pensato di annullare e poi eliminare la registrazione relativa alla riga che viene creata all'accredito della distinta (acceptance_move_id). In questa maniera la fattura torna "parzialmente pagata" (se ci sono altri pagamenti) o "non pagata" ed è possibile riemettere la riba (togliendo i filtri dalla lista "emissione".

@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.
In questo modo contabilmente ho traccia dei vari passaggi e riesco a far chiudere i conti transitori degli effetti

Copy link
Contributor

@andreampiovesana andreampiovesana left a 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?

@odooNextev
Copy link
Contributor Author

@tafaRU puoi aggiornare la review?

Copy link

@MaurizioPellegrinet MaurizioPellegrinet left a comment

Choose a reason for hiding this comment

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

LGTM

@andreampiovesana
Copy link
Contributor

merge?

@primes2h
Copy link
Contributor

@odooNextev riesci a fare un rebase per risolvere i confilitti?

@stenext stenext force-pushed the 16.0-imp-l10n_it_riba-expo branch from e1f5850 to c6a2f10 Compare May 10, 2024 08:13
Borruso added a commit to DinamicheAziendali/l10n-italy that referenced this pull request Jul 23, 2024
@MaurizioConte
Copy link

MaurizioConte commented Jul 23, 2024

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.

@francesco-ooops
Copy link
Contributor

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.

@odooNextev questa funzionalità rientra nello scopo della PR a tua opinione?

@MaurizioConte
Copy link

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.

@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.

@odooNextev
Copy link
Contributor Author

@MaurizioConte @francesco-ooops appena riesco a guardarci provo a risolvere

@odooNextev
Copy link
Contributor Author

@MaurizioConte secondo noi il problema è la differenza nella gestione del SBF da parte delle diverse banche.
Quello che noi ed i nostri clienti usiamo prevede questo flusso:

  1. emissione fattura con registrazione DA crediti v/cliente A ricavo
  2. emissione riba DA conto portafoglio A crediti v/cliente
  3. accredito riba DA conto credito riba A conto portafoglio
  4. incasso riba
  • se la riba viene pagata:
    a) DA banca A conto credito riba
  • se la riba è insoluta:
    a) DA crediti v/cliente A banca (nel caso in cui la banca alla scadenza accrediti tutta la distinta e poi storni l'insoluto)
    b) DA *diversi A conto credito riba (nel caso in cui la banca alla scadenza accrediti l'importo al netto dell'insoluto)
    *DA crediti v/cliente per l'importo dell'insoluto
    *DA banca per l'importo incassato

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.

@TheMule71
Copy link
Contributor

E' stato aggiornato qualcosa nel pre-commit? Prima di oggi non ricevevo questo messaggio ed in locale non fallisce ancora adesso: l10n_it_riba/models/account.py:69:46: B023 Function definition does not bind loop variable today``

In alcuni thread ho trovato che dovrei fare qualcosa del genere:

lambda line, today=today: line.riba

Così passa, ma non ne capisco il senso

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 .filtered. La mapped deve effettivamente eseguire la filtered.

E la variabile in questione today non cambia all'interno del loop. In effetti, andrebbe spostata fuori dal for.

Ma se la funzione venisse eseguita dopo (fuori da loop), e se il valore di today dipendesse dalla variabile di loop, la condizione line.date_maturity > today userebbe un solo valore di today, quello che ha all'uscita del loop.

Per fare un es. pratico, se la condizione recitasse line.date_maturity > invoice.date e se la filtered fosse "lazy" e non venisse usata immediatemente ma fuori dal loop, vedesti il problema manifestarsi. Tutte le lambda (una per ogni fattura) userebbero come valore di invoice non quello quando la lambda è stata creata (la fattura corrente nel loop) ma l'ultimo valore alla fine del loop (l'ultima fattura).

In questo caso, visto che tanto nel loop non cambia, io proverei semplicemente a spostare fuori dal loop today = ....

Dimostrazione del problema:

functions = []

# creo 5 funzioni senza argomenti, che ritornano il valore corrente di i
for i in range(5):
    functions.append(lambda: i)

# in realtà no: ritornano tutte il valore di i alla fine del loop
print("i =",i)
for f in functions:
    print("f =", f())

# nelle funzioni non viene salvato il valore di i, ma solo dove andare a prenderlo al momento dell'esecuzione
i = 4
f = 4
f = 4
f = 4
f = 4
f = 4

@odooNextev
Copy link
Contributor Author

@TheMule71 chiarissimo, grazie
Al prossimo push faccio quella piccola correzione

@odooNextev
Copy link
Contributor Author

@MaurizioConte ho aggiunto un'impostazione per definire se si utilizza SBF con incasso immediato o con incasso a maturazione valuta.

image

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.

@stenext stenext force-pushed the 16.0-imp-l10n_it_riba-expo branch 2 times, most recently from c44fc94 to 528938b Compare July 26, 2024 14:47
@odooNextev
Copy link
Contributor Author

@MaurizioPellegrinet ho aggiunto la possibilità di modificare la data di registrazione di una registrazione (account.move) già confermata.
Ho creato un wizard che si apre dalle registrazioni:

image

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.

@MaurizioPellegrinet
Copy link

@odooNextev grazie mille, sembra funzionare. Lunedì provo altri test più approfonditi per verificare che non ci siano altre implicazioni. Superrrrr!

Copy link

@MaurizioPellegrinet MaurizioPellegrinet left a comment

Choose a reason for hiding this comment

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

Revisipne funzionale: OK

Copy link
Member

@tafaRU tafaRU left a 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()
Copy link
Member

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?

@odooNextev
Copy link
Contributor Author

  • tutte le nuove funzionalità, non solo la prima che hai aggiunto, andrebbero descritte nel README

Ok

  • alcuni parti di codice riportano il copyright di altri autori ma non vedo l'autorship del commit: è voluto?

Immagino intendi questo: https://github.com/OCA/l10n-italy/pull/3984/files#diff-14f17eafd7915abe5ba350f6628d6b1b7850500fff1816f21c532091cfaa22d5L16
Non so perchè mi riporti questa differenza, devo controllare gli spazi

  • la history dei commit la vuoi tenere così?

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.

@tafaRU
Copy link
Member

tafaRU commented Aug 8, 2024

  • alcuni parti di codice riportano il copyright di altri autori ma non vedo l'autorship del commit: è voluto?

Immagino intendi questo: https://github.com/OCA/l10n-italy/pull/3984/files#diff-14f17eafd7915abe5ba350f6628d6b1b7850500fff1816f21c532091cfaa22d5L16 Non so perchè mi riporti questa differenza, devo controllare gli spazi

Mi riferisco a questi due commit:

In [1] aggiungi a4227cd#diff-04a8a365a32c3d6bceb153a18902bea732d9146b2328388fda2901a10f3b3d3bR147 come mai?
In [2] aggiungi 75d93fd#diff-40a5fed0388ac924cc0dfaa472e3c8873a1186f102c57eb7a267585c48d931f8R3 è corretto?

@odooNextev
Copy link
Contributor Author

  • alcuni parti di codice riportano il copyright di altri autori ma non vedo l'autorship del commit: è voluto?

Immagino intendi questo: https://github.com/OCA/l10n-italy/pull/3984/files#diff-14f17eafd7915abe5ba350f6628d6b1b7850500fff1816f21c532091cfaa22d5L16 Non so perchè mi riporti questa differenza, devo controllare gli spazi

Mi riferisco a questi due commit:

In [1] aggiungi a4227cd#diff-04a8a365a32c3d6bceb153a18902bea732d9146b2328388fda2901a10f3b3d3bR147 come mai? In [2] aggiungi 75d93fd#diff-40a5fed0388ac924cc0dfaa472e3c8873a1186f102c57eb7a267585c48d931f8R3 è corretto?

Penso sia dovuto ad un rebase con fixup...
Devo guardarci meglio

@stenext stenext force-pushed the 16.0-imp-l10n_it_riba-expo branch 3 times, most recently from 2e87feb to e1b616c Compare August 9, 2024 14:01
@odooNextev
Copy link
Contributor Author

@tafaRU dovrei aver fatto tutto quello che c'era da fare:

  • aggiungere descrizioni in usage e configuration
  • togliere copyright
  • aggiungere test con entrambe le configurazioni di sbf

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

Successfully merging this pull request may close these issues.

[IMP] l10n_it_riba: refactoring