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

[ADD] l10n_it_account_balance_eu #3064

Merged
merged 1 commit into from
Sep 1, 2023

Conversation

TennyMkt
Copy link
Contributor

Nuovo modulo MKT per la generazione del bilancio riclassificato secondo lo schema del bilancio UE

@TennyMkt TennyMkt force-pushed the 14.0-l10n_it_account_balance_eu branch from 3e579ba to 0520846 Compare November 24, 2022 17:08
@andreampiovesana
Copy link
Contributor

andreampiovesana commented Nov 30, 2022

serve aggiugere nelle dipendenze l10n_it_rea + l10n_it_fiscalcode

@TennyMkt TennyMkt force-pushed the 14.0-l10n_it_account_balance_eu branch 2 times, most recently from 65b1a6b to 9b87d71 Compare December 15, 2022 09:03
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.

not useful comments

def _default_date_to(self):
return date(date.today().year - 1, 12, 31)

# CAMPI DA CHIEDERE
Copy link
Contributor

Choose a reason for hiding this comment

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

serve?

</field>
</record>

<!-- Azione che apre il wizard di calcolo del bilancio -->
Copy link
Contributor

Choose a reason for hiding this comment

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

serve commento?

<field name="target">new</field>
</record>

<!-- Oggetto menu che apre il wizard per il calcolo del bilancio UE -->
Copy link
Contributor

Choose a reason for hiding this comment

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

idem

Copy link

@stefano-ooops stefano-ooops left a comment

Choose a reason for hiding this comment

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

Testato sembra andare tutto bene, importante sarà il collegamento con il piano dei conti

@francesco-ooops
Copy link
Contributor

@andreampiovesana puoi aggiornare la review?

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.

go

Copy link
Contributor

@SirTakobi SirTakobi 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!
Lo sviluppo è sicuramente interessante, oltre ai commenti qui sotto ti segnalo un paio di altre cose:

  • puoi tenere solo i commit significativi?
    I commit di merge sarebbero da eliminare e ci sono diversi commit con lo stesso messaggio.
    Per l'aggiunta di un modulo di solito è sufficiente un unico commit, in questo caso con messaggio tipo
    [ADD] l10n_it_account_balance_eu
  • puoi aggiungere un test?
    In generale non è obbligatorio, ma in questo caso ci sono centinaia di righe di codice non coperte.
    Senza test, ad ogni modifica bisognerebbe eseguire manualmente tutti i flussi per essere certi di non aver rotto nulla.

Copy link
Contributor

Choose a reason for hiding this comment

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

Questo serve?
A occhio mi pare serva ad aprire dei record al click su un link, ma non ci sono link nel report creato da questo modulo.

Copy link
Contributor

Choose a reason for hiding this comment

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

In effetti non serviva, file eliminato

Copy link
Contributor

Choose a reason for hiding this comment

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

👍


var QWeb = core.qweb;

const AFRReportAction = ReportAction.extend({
Copy link
Contributor

Choose a reason for hiding this comment

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

Cosa significa AFRReportAction?

Copy link
Contributor

Choose a reason for hiding this comment

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

Modificato in: BalEUReportAction

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Installando il modulo in un DB vuoto non ho le associazioni, riporto i passi che ho fatto:

  1. Creo un DB vuoto
  2. Installo il modulo l10n_it_account_balance_eu

Osservato:
I conti del piano dei conti italiano (l10n_it si è installato automaticamente) non sono mappati con le linee del Bilancio EU.

Atteso:
I conti del piano dei conti italiano (l10n_it si è installato automaticamente) sono mappati con le linee del Bilancio EU.

Osservazioni:
Aggiornando nuovamente il modulo l10n_it_account_balance_eu, le associazioni vengono fatte.
Penso succeda perché l10n_it viene installato dopo il modulo, puoi verificare?

Copy link
Contributor

Choose a reason for hiding this comment

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

risolto aggiungendo l10n_it nelle depends

Copy link
Contributor Author

Choose a reason for hiding this comment

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

l10n_it nelle depends creava altri problemi. Quindi al momento in fase di installazione associa i conti con le voci di bilancio solo se la contabilità era già presente (la stragrande maggioranza dei casi), è stato aggiornato il README per indicarlo.

Copy link
Contributor

Choose a reason for hiding this comment

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

Si potrebbe fare un modulo autoinstallante dipendente da questo e da l10n_it che fa l'associazione

Copy link
Contributor

Choose a reason for hiding this comment

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

l10n_it nelle depends creava altri problemi. Quindi al momento in fase di installazione associa i conti con le voci di bilancio solo se la contabilità era già presente (la stragrande maggioranza dei casi), è stato aggiornato il README per indicarlo.

👍

data = {
"form_data": self.read()[0],
"balance_ue_lines": balance_ue_lines_report_data,
"warnings": self.log_warnings.split("\n"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Nelle prove che ho fatto, non ho warnings ma viene comunque visualizzato il box rosso:
image

Credo sia perché ''.split("\n") = [''], puoi verificare?

Copy link
Contributor

Choose a reason for hiding this comment

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

sistemato, adesso lo split viene fatto solo se ci sono warning

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

)
else:
account_balance_eu_amount_rounded = account_balance_eu_amount
self.balance_ue_lines[item.code] = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Ti descrivo il problema che ho trovato in questo punto:

  1. Avvio Odoo
  2. Genero il report: verifico che prima di questa riga, self.balance_ue_lines = {}
  3. Ri-genero il report

Atteso:
prima di questa riga, self.balance_ue_lines = {}

Osservato:
prima di questa riga, self.balance_ue_lines conserva i valori del report generato al passo 2

Osservazioni:
Credo che succeda perché self.balance_ue_lines è una variabile di classe.
Questo può portare a dei problemi perché se alcune chiavi del dizionario self.balance_ue_lines non vengono aggiornate, allora verranno mostrati i valori dell'ultimo report generato.
Puoi verificare?

Copy link
Contributor

Choose a reason for hiding this comment

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

balance_ue_lines trasformata in una variabile locale di cal_balance_ue_data

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Comment on lines 7 to 11
def my_round(val, precision):
# il round arrotonda il 5 al pari più vicino (1.5 -> 2 e 2.5 -> 2)
# aggiungo un infinitesimo per farlo arrotondare sempre per eccesso
return round(val + 1e-10, precision)
Copy link
Contributor

Choose a reason for hiding this comment

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

È possibile avere un nome più significativo per questa funzione?
Giusto per info: se ho ben capito il comportamento atteso, si può ottenere anche con math.ceil:

>>> import math
>>> precision = 0
>>> math.ceil(1.5 * (10**precision)) / (10**precision)
2.0
>>> math.ceil(2.5 * (10**precision)) / (10**precision)
3.0

così da non ricorrere al hack di aggiungere un infinitesimo.

Un'ultima nota: il codice e i commenti, per quanto possibile, andrebbero scritti in inglese:

Use English variable names and write comments in English.

(da https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#33idioms)

Copy link
Contributor

Choose a reason for hiding this comment

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

sostituita con la:

def round_half_up(val, precision):
    multiplier = 10**precision
    return math.floor(val * multiplier + 0.5) / multiplier

Copy link
Contributor

Choose a reason for hiding this comment

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

io userei currency_id.round() se disponibile oppure odoo.tools.float_round()

from odoo import tools
>>> tools.float_round(1.5,precision_rounding=1)
2.0
>>> tools.float_round(2.5,precision_rounding=1)
3.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

aggiornato usando la odoo.tools.float_round()

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

!= self.balance_ue_lines["PP"]["rounded_amount"]
):
self.state = "UNBALANCED"
self.log_warnings = (
Copy link
Contributor

Choose a reason for hiding this comment

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

È possibile rendere traducibile questo messaggio e quelli sotto?

Copy link
Contributor

Choose a reason for hiding this comment

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

tradotti tutti i messaggi di warning

Copy link
Contributor

Choose a reason for hiding this comment

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

👍


def generate_xlsx_report(self, workbook, data, record_data):
balance_data = data["form_data"]
sheet = workbook.add_worksheet("Bilancio")
Copy link
Contributor

Choose a reason for hiding this comment

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

È possibile rendere traducibile questa stringa e quelle sotto?

Copy link
Contributor

Choose a reason for hiding this comment

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

tradotte tutte le stringhe dell'XLSX

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

balance_form_data = data["form_data"]
i_year = "i_" + str(balance_form_data["year"])
d_year = "d_" + str(balance_form_data["year"])
xbrl = """<?xml version = "1.0" encoding = "UTF-8"?>
Copy link
Contributor

Choose a reason for hiding this comment

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

Creare un file XML da una stringa non è proprio comodo: un'altra opzione è usare una tecnica simile a

template_values = self.get_template_values()
content = env.ref(
"l10n_it_fatturapa_out.account_invoice_it_FatturaPA_export"
)._render(template_values)

Copy link
Contributor

Choose a reason for hiding this comment

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

Grazie per il suggerimento. Però, per il momento, preferiamo lasciarlo come stringa, se sarà necessario metterci mano valuteremo se utilizzare il template

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

with all the entries of the most recent taxonomies by making available the
following features:

* associate the items in the Chart of Accounts with the items in the EU Budget (with
Copy link
Contributor

Choose a reason for hiding this comment

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

Questo elenco viene visualizzato così
image
non credo che le frasi mezze in grassetto siano volute, è possibile visualizzarlo come un normale elenco?

Copy link
Contributor

Choose a reason for hiding this comment

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

corretto

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@SirTakobi
Copy link
Contributor

Provando il modulo con @eLBati abbiamo notato che questi conti non sono mappati:

  • 280100 bilancio di apertura
  • 280200 bilancio di chiusura
  • 311200 premi su vendite
  • 311210 sopravvenienze attive straordinarie
  • 391501 interessi attivi v/clienti
  • 391502 interessi attivi bancari
  • 391503 interessi attivi postali
  • 392037 proventi finanziari diversi
  • 398100 Utili differenza di cassa
  • 483120 interessi passivi v/fornitori
  • 483307 interessi passivi bancari
  • 483412 sconti passivi bancari
  • 483903 interessi passivi su mutui
  • 490200 oneri finanziari diversi
  • 498100 Perdite differenza di cassa
  • 499200 stato patrimoniale
  • 910100 conto di risultato economico
  • 999999 Utili/Perdite non distribuiti

Come mai?
Credo siano conti standard, ad esempio 280100 viene da https://github.com/odoo/odoo/blob/536560bbf55d1d9a6f5bd8d8f145f7bb13569ae4/addons/l10n_it/data/account.account.template.csv#L86

@TennyMkt TennyMkt force-pushed the 14.0-l10n_it_account_balance_eu branch from 026c5ff to d97c468 Compare April 13, 2023 07:33
@TennyMkt TennyMkt changed the title [14.0] Proposta PR per modulo l10n_it_account_balance_eu [ADD] l10n_it_account_balance_eu Apr 13, 2023
@TennyMkt TennyMkt force-pushed the 14.0-l10n_it_account_balance_eu branch 2 times, most recently from fa9632f to 89e498d Compare April 13, 2023 09:55
@mktsrl
Copy link
Contributor

mktsrl commented Apr 13, 2023

Provando il modulo con @eLBati abbiamo notato che questi conti non sono mappati:

  • 280100 bilancio di apertura
  • 280200 bilancio di chiusura
  • 311200 premi su vendite
  • 311210 sopravvenienze attive straordinarie
  • 391501 interessi attivi v/clienti
  • 391502 interessi attivi bancari
  • 391503 interessi attivi postali
  • 392037 proventi finanziari diversi
  • 398100 Utili differenza di cassa
  • 483120 interessi passivi v/fornitori
  • 483307 interessi passivi bancari
  • 483412 sconti passivi bancari
  • 483903 interessi passivi su mutui
  • 490200 oneri finanziari diversi
  • 498100 Perdite differenza di cassa
  • 499200 stato patrimoniale
  • 910100 conto di risultato economico
  • 999999 Utili/Perdite non distribuiti

Come mai? Credo siano conti standard, ad esempio 280100 viene da https://github.com/odoo/odoo/blob/536560bbf55d1d9a6f5bd8d8f145f7bb13569ae4/addons/l10n_it/data/account.account.template.csv#L86

I conti
910100 conto di risultato economico
999999 Utili/Perdite non distribuiti
280100 bilancio di apertura
280200 bilancio di chiusura
sono (credo) conti tecnici transitori che al termine della Chiusura e Riapertura di Esercizio dovrebbero essere a zero e pertanto non andare ad influenzare il Bilancio UE.
Tendenzialmente se questi conti contengono un valore diverso da zero vuol dire che qualcosa non funziona nella Bilancio.
Per nostra comodità noi abbiamo spostato 280100 e 280200 come conti 9XXXXX
Abbiamo aggiunto anche una voce (con codice: --) per associare i conti che devono essere ignorati ai fini del bilancio UE ma viene segnalato se il saldo è diverso da 0.

Per quanto riguarda i conti standard del Piano dei Conti OCA/l10n_italy noi ci siamo basati su una nuova azienda creata vuota (senza dati demo) con tutti i moduli della contabilità italiana (senza il modulo POS) circa 3 anni fa con una installazione ODOO 12 CE.

Anche nella nostra “azienda con pdc standard” è presente il conto
311200 premi su vendite
e nella prossima PUSH che caricheremo è già stata prevista la riclassifica in
E.A511 5) Altri Ricavi e Proventi / Altri

Invece i conti
311210 sopravvenienze attive straordinarie = 710200 (stiamo ancora verificando con in nostro commercialista, dovrebbe essere E.A511 5) Altri Ricavi e proventi / Altri )

391501 interessi attivi v/clienti                                = 511000    -> [E.C244] Proventi e Oneri Finanziari / Altri Proventi Finanziari / proventi diversi dai precedenti / Altri   
391502 interessi attivi bancari                                 = 511500    -> [E.C244] Proventi e Oneri Finanziari / Altri Proventi Finanziari / proventi diversi dai precedenti / Altri
391503 interessi attivi postali                                  = 511600    -> [E.C215] Proventi e Oneri Finanziari / Altri Proventi Finanziari / da crediti iscritti nelle immobilizzazioni / Altri
392037 proventi finanziari diversi                            = 514000    -> [E.C23] Proventi e Oneri Finanziari / Altri Proventi Finanziari / da titoli iscritti nell'attivo circolante che non costituiscono partecipazioni
398100 Utili differenza di cassa                               (non presente)
483120 interessi passivi v/fornitori                          = 520100    -> [E.C35] Proventi e Oneri Finanziari / Interessi ed altri Oneri Finanziari / Altri
483307 interessi passivi bancari                              = 520200    -> [E.C35] Proventi e Oneri Finanziari / Interessi ed altri Oneri Finanziari / Altri
483412 sconti passivi bancari                                  = 520300    -> [E.C35] Proventi e Oneri Finanziari / Interessi ed altri Oneri Finanziari / Altri
483903 interessi passivi su mutui                            = 521000    -> [E.C35] Proventi e Oneri Finanziari / Interessi ed altri Oneri Finanziari / Altri  
490200 oneri finanziari diversi                                 = 524000     -> [E.C35] Proventi e Oneri Finanziari / Interessi ed altri Oneri Finanziari / Altri  
498100 Perdite differenza di cassa                          (non presente)
499200 stato patrimoniale                                       (non presente e non ha senso il nome “stato patrimoniale”)

non sono presenti nel nostro PdC “standard” e di fianco ad ognuno ho messo il codice presente nel nostro (se esiste un conto con nome simile)
Dato che questi codici qui sopra non esistono nel PdC standard in nostro possesso, è possibile aggiungerli alla tabella delle riclassificazioni automatiche che vengono effettuate in fase di installazione del modulo.
Fate le vostre verifiche e fateci sapere.

@TennyMkt TennyMkt force-pushed the 14.0-l10n_it_account_balance_eu branch 3 times, most recently from 25a9302 to 76fe731 Compare April 17, 2023 14:38
@TennyMkt TennyMkt force-pushed the 14.0-l10n_it_account_balance_eu branch 3 times, most recently from 7e5e2a0 to 4563b4a Compare April 28, 2023 09:49
@TennyMkt TennyMkt mentioned this pull request Apr 28, 2023
@TennyMkt TennyMkt force-pushed the 14.0-l10n_it_account_balance_eu branch 2 times, most recently from 0225f98 to 23f49b7 Compare May 2, 2023 12:45
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
test funzionale fatto

Copy link
Contributor

@dcorio dcorio left a comment

Choose a reason for hiding this comment

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

Testato funzionalmente in produzione su più clienti reali. LGTM

@eLBati
Copy link
Member

eLBati commented May 19, 2023

@SirTakobi a proposito di #3064 (review) per me l'unica cosa che sicuramente resta da fare è l'unione dei commit. I test al momento non sono necessari, essendo alpha

Comment on lines 39 to 40
string="ASSETS (Patrimonial)"
name="Attivo"
Copy link
Contributor

Choose a reason for hiding this comment

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

Non credo che "Patrimonial" qui sia il termine corretto.
Se ti riferisci a "Stato patrimoniale" è "Balance Sheet", se ti riferisci a "Patrimonio netto" è "Equity".

"Attivo" → "Assets"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fatto

Comment on lines 44 to 45
string="LIABILITIES (Patrimonial)"
name="Passivo"
Copy link
Contributor

Choose a reason for hiding this comment

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

Vedi sopra.

"Passivo" → "Liabilities"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fatto

balance_state = "UNBALANCED"
log_warnings = log_warnings + (
_(
"NON-SQUARE Balance: {:s} (Assets) - {:s} (Liabilities) = {:s}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Partendo dal significato che è stato dato in italiano, presente nella traduzione, direi che è più "Unbalanced financial statements: ...".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fatto come suggerito

@primes2h
Copy link
Contributor

primes2h commented Jul 6, 2023

@TennyMkt potete rinominare il commit [ADD] l10n_it_account_balance_eu ?
Credo sia da rigenerare il .po

* commit rinominata

* file .po e .pot sistemati per unire le definizioni doppie

Mi sembra che il .pot non sia stato rigenerato correttamente, contiene ancora qualche stringa vecchia.

@primes2h
Copy link
Contributor

primes2h commented Jul 6, 2023

Non sono convinto della scelta di rinviare la correzione (anche se comprendo che è onerosa) perché un brutto nome può compromettere la diffusione e l'utilizzo del modulo.

Su questo hai perfettamente ragione. In più se non viene fatto ora verrebbe solo passato l'onere delle correzioni a chi verrà dopo.

Tra l'altro non si tratterebbe solo di modificare il nome del modulo, ma di rivedere anche i nomi dei file/modelli/viste/campi ecc... lasciando balance solo dove ci si riferisce effettivamente al termine corretto (es. il "saldo").

Inoltre lo stesso lavoro andrebbe fatto anche sul modulo l10n_it_account_balance (rif. PR #3343).

Copy link
Contributor

@SirTakobi SirTakobi left a comment

Choose a reason for hiding this comment

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

Grazie mille delle modifiche!
Ti segnalo qui sotto un problema che ho trovato e qualche altro commento.

Grazie anche dei chiarimenti in #3064 (comment), purtroppo non sapevo come si usasse in genere il report di questa PR.
Potresti allora chiarire magari nel help del campo hide_acc_amount_0 a cosa serve? Da quanto ho capito del commento linkato, pensavo non fosse usato e invece è coinvolto in qualche logica in https://github.com/OCA/l10n-italy/pull/3064/files#diff-2816a2182a0f425151451d3a9891a831581ca4a6d74a8b4ab7625e98676942f2R265:

if (not hide_acc_amount_0) or (acc_amount != 0):
    account_list.append(
        {
            "code": item.get("code"),
            "desc": item.get("name"),
            "amount": acc_amount,
        }
    )


{
"name": "ITA - Bilancio UE con XBRL",
"version": "14.0.1.0.6",
Copy link
Contributor

Choose a reason for hiding this comment

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

La versione di un nuovo modulo di norma è 14.0.1.0.0, forse se ne è anche già parlato in questa PR e non sto trovando il commento.
Non credo comunque sia una modifica necessaria per arrivare al merge

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fatto

Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

Comment on lines 454 to 466
format_decimal(
balance_ue_lines["PA"]["rounded_amount"],
format="#,##0.00",
locale="it_IT",
),
Copy link
Contributor

Choose a reason for hiding this comment

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

Il primo parametro di formatLang è un Environment mentre in diversi punti viene passato come primo parametro il valore da convertire, questo causa il seguente errore:

[...]
File "/path/to/l10n-italy/l10n_it_account_balance_eu/models/account_balance_eu.py", line 460, in cal_balance_ue_data
formatLang(balance_ue_lines["PA"]["rounded_amount"], 2),
File "/path/to/odoo/odoo/tools/misc.py", line 1310, in formatLang
lang_obj = get_lang(env)
File "/path/to/odoo/odoo/tools/misc.py", line 1271, in get_lang
langs = [code for code, _ in env['res.lang'].get_installed()]
TypeError: 'float' object is not subscriptable

Puoi verificare?

Inoltre, invece di scolpire le 2 cifre decimali penso sarebbe meglio ricavarle dalla currency: anche in questo formatLang ti aiuta con il parametro currency_obj, vedi https://github.com/odoo/odoo/blob/bc188d62b3d003d56309f35a06bb2f694a55492e/odoo/tools/misc.py#L1342

Comment on lines 98 to 99
self.date_from = self.date_range_id.date_start
self.date_to = self.date_range_id.date_end
Copy link
Contributor

Choose a reason for hiding this comment

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

Quando date_range_id è vuoto, questo onchange svuota i campi date_from e date_to che avrebbero invece i loro default aggiunti dalle ultime modifiche.
Non so se è voluto, ma ti consiglio di modificare le date solo quando è stato selezionato un Date Range:

Suggested change
self.date_from = self.date_range_id.date_start
self.date_to = self.date_range_id.date_end
date_range = self.date_range_id
if date_range:
self.date_from = date_range.date_start
self.date_to = date_range.date_end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fatto

Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

(calc_type == "non_assoc")
or (
(calc_type == "d")
and ((acc_amount >= 0) or (not acc_credit_id))
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@primes2h
Copy link
Contributor

primes2h commented Jul 7, 2023

Non sono convinto della scelta di rinviare la correzione (anche se comprendo che è onerosa) perché un brutto nome può compromettere la diffusione e l'utilizzo del modulo.

Su questo hai perfettamente ragione. In più se non viene fatto ora verrebbe solo passato l'onere delle correzioni a chi verrà dopo.

Tra l'altro non si tratterebbe solo di modificare il nome del modulo, ma di rivedere anche i nomi dei file/modelli/viste/campi ecc... lasciando balance solo dove ci si riferisce effettivamente al termine corretto (es. il "saldo").

Inoltre lo stesso lavoro andrebbe fatto anche sul modulo l10n_it_account_balance (rif. PR #3343).

@OCA/local-italy-developers
Rif. #3343 (comment)

@primes2h
Copy link
Contributor

Non sono convinto della scelta di rinviare la correzione (anche se comprendo che è onerosa) perché un brutto nome può compromettere la diffusione e l'utilizzo del modulo.

Su questo hai perfettamente ragione. In più se non viene fatto ora verrebbe solo passato l'onere delle correzioni a chi verrà dopo.
Tra l'altro non si tratterebbe solo di modificare il nome del modulo, ma di rivedere anche i nomi dei file/modelli/viste/campi ecc... lasciando balance solo dove ci si riferisce effettivamente al termine corretto (es. il "saldo").
Inoltre lo stesso lavoro andrebbe fatto anche sul modulo l10n_it_account_balance (rif. PR #3343).

@OCA/local-italy-developers Rif. #3343 (comment)

Ehm, intendevo @OCA/local-italy-maintainers

@TennyMkt TennyMkt force-pushed the 14.0-l10n_it_account_balance_eu branch from c7882e5 to cb50247 Compare July 11, 2023 08:59
@TennyMkt
Copy link
Contributor Author

Grazie mille delle modifiche! Ti segnalo qui sotto un problema che ho trovato e qualche altro commento.

Grazie anche dei chiarimenti in #3064 (comment), purtroppo non sapevo come si usasse in genere il report di questa PR. Potresti allora chiarire magari nel help del campo hide_acc_amount_0 a cosa serve? Da quanto ho capito del commento linkato, pensavo non fosse usato e invece è coinvolto in qualche logica in https://github.com/OCA/l10n-italy/pull/3064/files#diff-2816a2182a0f425151451d3a9891a831581ca4a6d74a8b4ab7625e98676942f2R265:

if (not hide_acc_amount_0) or (acc_amount != 0):
    account_list.append(
        {
            "code": item.get("code"),
            "desc": item.get("name"),
            "amount": acc_amount,
        }
    )
  • aggiunto help del campo hide_acc_amount_0
  • sistemate chiamate della formatLang

@TennyMkt
Copy link
Contributor Author

@TennyMkt potete rinominare il commit [ADD] l10n_it_account_balance_eu ?
Credo sia da rigenerare il .po

* commit rinominata

* file .po e .pot sistemati per unire le definizioni doppie

Mi sembra che il .pot non sia stato rigenerato correttamente, contiene ancora qualche stringa vecchia.

rigenerato e aggiornato .pot

hide_acc_amount_0 = form_data["hide_acc_amount_0"]
ignore_closing_move = form_data["ignore_closing_move"]
if ignore_closing_move:
if not self.env["account.move"].fields_get("closing_type"):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if not self.env["account.move"].fields_get("closing_type"):
if not self.env["account.move"].fields_get(allfields=["closing_type"]):

Il parametro allfields di fields_get assegnato qui è una lista di nomi di campi (rif. https://github.com/odoo/odoo/blob/86935640fca0b8a6a7a75bfaae7371177011d982/odoo/models.py#L2872)

Ho notato solo ora questo campo ignore_closing_move, però mi sembra avere qualche effetto solo quando esiste il campo account.move.closing_type (credo venga da https://github.com/OCA/account-closing/blob/437fb52ef26e9cd489e7d602c40f06aa7f875743/account_fiscal_year_closing/models/account_move.py#L29 che non è tra le dipendenze), è possibile aggiungere una nota nel README o nel help del campo riguardo questa particolarità?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aggiunta spiegazione di tutte le opzioni del wizard nel USAGE
In pratica ignora i movimenti di chiusura (che rendono il saldo del conto a 0) generati dal modulo 'account_fiscal_year_closing', se il modulo non è istallato non viene fatto il controllo.
Viene controllato se esiste il campo anzichè mettere il modulo nelle dipendenza perchè anche se viene installato e non utilizzato è come se non ci fosse.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

else:
rounded_amount += balance_ue_lines[child.code]["rounded_amount"]
total_amount += balance_ue_lines[child.code]["total_amount"]
balance_ue_lines[code]["rounded_amount"] = tools.float_round(rounded_amount, 2)
Copy link
Contributor

@SirTakobi SirTakobi Jul 24, 2023

Choose a reason for hiding this comment

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

Come mai qui non si usa round_bal_val per arrotondare all'unità o ai due decimali?

Non so se è questo il responsabile ma ho notato che scegliendo l'arrotondamento all'unità nel HTML/PDF ho
image
dove si vedono i due decimali, mentre nel XLSX ho
image
dove i decimali non si vedono, è possibile correggere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Il saldo dei conti viene sempre stampato con 2 decimali, era errata l'esportazione in XLSX che arrotondava all'unità di euro se veniva scelta come opzione per le linee di bilancio. Ho corretto l'esportazione XLSX e spiegato nel README

Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻


{
"name": "ITA - Bilancio UE con XBRL",
"version": "14.0.1.0.6",
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

Comment on lines 454 to 466
format_decimal(
balance_ue_lines["PA"]["rounded_amount"],
format="#,##0.00",
locale="it_IT",
),
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

Comment on lines 98 to 99
self.date_from = self.date_range_id.date_start
self.date_to = self.date_range_id.date_end
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

* associate the items in the Chart of Accounts with the items in the EU Budget (with automatic pre-association during module installation)
* process the accounting records of a given period (for example
last year) in order to obtain the accounting balance sheet in EU format, carrying out the appropriate congruence checks, association of all the accounts moved in the period and balancing of the balance sheet. The reports also show the details of the accounts associated with a certain item of the EU balance
* PDF printout of the EU Balance
Copy link
Contributor

Choose a reason for hiding this comment

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

Non ho confrontato il significato in italiano/inglese, ho solo notato che nell'inglese manca un punto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mancava il punto della visualizzazione HTML nell'inglese. Con l'occasione ho corretto dal readme in inglese tutte le occorrenze di "balance"

Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

@TennyMkt TennyMkt force-pushed the 14.0-l10n_it_account_balance_eu branch from cb50247 to 4feffc5 Compare July 26, 2023 13:44
Copy link
Contributor

@SirTakobi SirTakobi 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!
Ho fatto revisione del codice e provato qualche stampa, per me è ok.

Comment on lines 1 to 3
**Italiano**
Il termine "Balance" è errato per indicare in inglese il bilancio.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
**Italiano**
Il termine "Balance" è errato per indicare in inglese il bilancio.
**Italiano**
Il termine "Balance" è errato per indicare in inglese il bilancio.

Altrimenti viene visualizzato sulla stessa riga:
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sistemato

Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

Comment on lines 8 to 11
**English**
The term "Balance" is incorrect to indicate the balance sheet in English.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
**English**
The term "Balance" is incorrect to indicate the balance sheet in English.
**English**
The term "Balance" is incorrect to indicate the balance sheet in English.

Altrimenti viene visualizzato sulla stessa riga:
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sistemato

Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

hide_acc_amount_0 = form_data["hide_acc_amount_0"]
ignore_closing_move = form_data["ignore_closing_move"]
if ignore_closing_move:
if not self.env["account.move"].fields_get("closing_type"):
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

else:
rounded_amount += balance_ue_lines[child.code]["rounded_amount"]
total_amount += balance_ue_lines[child.code]["total_amount"]
balance_ue_lines[code]["rounded_amount"] = tools.float_round(rounded_amount, 2)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

* associate the items in the Chart of Accounts with the items in the EU Budget (with automatic pre-association during module installation)
* process the accounting records of a given period (for example
last year) in order to obtain the accounting balance sheet in EU format, carrying out the appropriate congruence checks, association of all the accounts moved in the period and balancing of the balance sheet. The reports also show the details of the accounts associated with a certain item of the EU balance
* PDF printout of the EU Balance
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

@TennyMkt TennyMkt force-pushed the 14.0-l10n_it_account_balance_eu branch from 4feffc5 to c18eb36 Compare August 1, 2023 07:11
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

@scigghia scigghia left a comment

Choose a reason for hiding this comment

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

👍

@andreampiovesana
Copy link
Contributor

merge?

@francesco-ooops
Copy link
Contributor

@primes2h puoi aggiornare la review?

@eLBati
Copy link
Member

eLBati commented Sep 1, 2023

Considero le richieste di @primes2h soddisfatte e procedo

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 14.0-ocabot-merge-pr-3064-by-eLBati-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Sep 1, 2023
Signed-off-by eLBati
@OCA-git-bot
Copy link
Contributor

It looks like something changed on 14.0 in the meantime.
Let me try again (no action is required from you).
Prepared branch 14.0-ocabot-merge-pr-3064-by-eLBati-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 191a6f4 into OCA:14.0 Sep 1, 2023
4 of 6 checks passed
@OCA-git-bot
Copy link
Contributor

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

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

Successfully merging this pull request may close these issues.