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][MIG] l10n_it_financial_statement_eu: Migration to 16.0 #3599

Merged

Conversation

TennyMkt
Copy link
Contributor

@TennyMkt TennyMkt commented Sep 21, 2023

Modulo per la creazione del bilancio UE.

Nella 14.0 il modulo si chiamava l10n_it_account_balance_eu
Nel da 14 a 16 è stato modificato anche il metodo di assegnazione iniziale delle voci di bilancio ai conti: nella 14.0 veniva utilizzato il file data/account_balance_eu_reclassification.xml mentre nella 16.0 è stata creata una post_init_hook migliore sia dal punto di vista del funzionamento che della leggibilità del sorgente.

L'esportazione XBRL funziona solo se viene fatto il merge di questa PR: OCA/reporting-engine#791

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

@TheMule71
Copy link
Contributor

/ocabot migration l10n_it_financial_statement_eu

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Sep 22, 2023
@OCA-git-bot OCA-git-bot mentioned this pull request Sep 22, 2023
79 tasks
Copy link
Contributor

@SirAionTech SirAionTech 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 solo fatto revisione della storia dei commit e del codice.

Puoi modificare la storia dei commit come descritto in https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests#mergesquash-the-commits-generated-by-bots-or-weblate?

Mancano dei commit presenti in 14.0, ad esempio 657ccdf, è possibile includerli?

Comment on lines 118 to 110
def account_balance_eu_debit_association(
self, acc_code, account_balance_eu_id, force_update=False
def financial_statement_eu_account_association(
self,
acc_code,
debit_fse_id,
credit_fse_id,
force_update,
):
Copy link
Contributor

Choose a reason for hiding this comment

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

Questa modifica non è necessaria per la migrazione dalla 14.0 alla 16.0, immagino sia per il

è stato modificato anche il metodo di assegnazione iniziale

che hai citato in descrizione.
È possibile separare le nuove funzionalità in un commit dedicato e creare una issue come descritto in https://github.com/OCA/l10n-italy/wiki/Team-di-sviluppo#apertura-issue per tracciare la modifica nelle altre versioni?

Già non è sempre banale per un revisore controllare la fedeltà di una migrazione dove cambia il nome del modulo, dei model ecc., se ci sono anche altre modifiche mischiate nello stesso commit poi è ancora peggio.

Copy link
Contributor

Choose a reason for hiding this comment

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

?

_name = "account.balance.eu"
_description = "Account Balance EU line"
class FinancialStatementEU(models.Model):
_name = "financial.statement.eu"
Copy link
Contributor

Choose a reason for hiding this comment

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

Questa modifica, come la rinomina del modulo e le tante altre modifiche alla nomenclatura, richiede una migrazione dei dati, altrimenti migrando un DB dalla 14.0 alla 16.0 i dati non vengono mantenuti.

È possibile aggiungere la migrazione dei dati?

Puoi guardare ad esempio come è stata fatta in 1194fb2, in particolare la cartella migrations e il pre_init_hook nel manifest.
Nota che lo schema seguito è descritto più ad alto livello in #3574 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

/** @odoo-module **/
import {ReportAction} from "@web/webclient/actions/reports/report_action";
import {patch} from "web.utils";
// Import { patch } from '@web/core/utils/patch';
Copy link
Contributor

Choose a reason for hiding this comment

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

Questo si può rimuovere?

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 10 to 21
<!-- <button-->
<!-- t-on-click="on_click_xlsx"-->
<!-- type="button"-->
<!-- class="btn btn-secondary"-->
<!-- title="Export XLSX"-->
<!-- >Export XLSX</button>-->
<!-- <button-->
<!-- t-on-click="on_click_xlsx"-->
<!-- type="button"-->
<!-- class="btn btn-secondary"-->
<!-- title="Export XLSX"-->
<!-- >Export XBRL</button>-->
Copy link
Contributor

Choose a reason for hiding this comment

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

Questo si può rimuovere?

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 9 to 12
from odoo import api, SUPERUSER_ID


def _l10n_it_financial_statement_eu_post_init(cr, registry):
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.

👍

Copy link
Contributor

@primes2h primes2h left a comment

Choose a reason for hiding this comment

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

Giusto un appunto al volo.
Come indicato qui, statement va con la 's' finale (vedi anche il modulo l10n_it_financial_statements_report).

Oltre al nome (l10n_it_financial_statements_eu) vanno modificate anche tutte le altre occorrenze del termine.

statement → statements

@danall59
Copy link

danall59 commented Oct 5, 2023

Giusto un appunto al volo. Come indicato qui, statement va con la 's' finale (vedi anche il modulo l10n_it_financial_statements_report).

Oltre al nome (l10n_it_financial_statements_eu) vanno modificate anche tutte le altre occorrenze del termine.

statement → statements

Come da chat intercorse sull’argomento del cambio nome di questo modulo (versione 14), è stato scelto di usare il singolare “l10n_it_financial_statement_eu” in quanto il termine plurale “financial_statements” si riferisce al fatto che nella preparazione e presentazione del bilancio annuale sono coinvolti vari tipi di documenti quali Bilancio, Nota Integrativa, Relazione sulla gestione ecc. che esulano dalle finalità del modulo e possibilità di Odoo.
Il singolare “financial_statement” è sembrato quindi più adatto ad un modulo che si occupa solo della generazione del Bilancio UE

@primes2h
Copy link
Contributor

primes2h commented Oct 5, 2023

Giusto un appunto al volo. Come indicato qui, statement va con la 's' finale (vedi anche il modulo l10n_it_financial_statements_report).
Oltre al nome (l10n_it_financial_statements_eu) vanno modificate anche tutte le altre occorrenze del termine.
statement → statements

Come da chat intercorse sull’argomento del cambio nome di questo modulo (versione 14), è stato scelto di usare il singolare “l10n_it_financial_statement_eu” in quanto il termine plurale “financial_statements” si riferisce al fatto che nella preparazione e presentazione del bilancio annuale sono coinvolti vari tipi di documenti quali Bilancio, Nota Integrativa, Relazione sulla gestione ecc. che esulano dalle finalità del modulo e possibilità di Odoo. Il singolare “financial_statement” è sembrato quindi più adatto ad un modulo che si occupa solo della generazione del Bilancio UE

Il termine financial statement in genere si riferisce a una dichiarazione generica, quindi potrebbe essere fuorviante IMO.

"Financial statements" è anche il termine usato nei documenti ufficiali EU, che viene tradotto con "bilancio".

Vedi anche
https://www.traduzionechiara.it/terminologia-finanziaria-balance-sheet-o-financial-statements/

@primes2h
Copy link
Contributor

primes2h commented Oct 5, 2023

Nel frattempo avevo già preparato questa PR, attendiamo anche altri commenti.

mktsrl#7

@primes2h
Copy link
Contributor

primes2h commented Oct 5, 2023

si riferisce al fatto che nella preparazione e presentazione del bilancio annuale sono coinvolti vari tipi di documenti quali Bilancio, Nota Integrativa, Relazione sulla gestione ecc. che esulano dalle finalità del modulo e possibilità di Odoo.

Di questi documenti però solo il conto economico e lo stato patrimoniale sono fondamentali e imprescidibili, gli altri sono opzionali.

https://learn.marsdd.com/article/financial-statements-the-four-components/

Se proprio vuoi chiarire che il modulo non contiene i documenti opzionali basterebbe aggiungere una nota nel README.

@danall59
Copy link

danall59 commented Oct 5, 2023

In realtà per la presentazione in CCIAA del Bilancio Annuale sono obbligatori almeno il Bilancio e la Nota Integrativa (entrambi vanno dentro al file XBRL).
Nella pagina che hai linkato c'è scritto
Financial statements are a useful tool in analyzing your company’s financial position and performance. They are comprised of four main components,
Come puoi notare la "s" del plurale è coerente con il fatto che parla di 4 strumenti e inoltre nel seguito usa il termine "Income statement" al singolare per parlare del Conto Economico.
Non bisogna confondere il termine "Bilancio UE" inteso come report con lo stesso termine usato nel contesto "predispongo il Bilancio annuale" inteso come insieme di adempimenti di fine anno per la presentazione in Camera di Commercio.

Detto questo mi taccio e attendiamo che altri si esprimano definitivamente su questo argomento per poi procedere in una direzione o nell'altra.

@andreampiovesana
Copy link
Contributor

@primes2h fai PR alla PR?

@eLBati
Copy link
Member

eLBati commented Nov 17, 2023

In considerazione di #3599 (comment) e della chiamata di stamattina (https://discord.com/channels/753902328494424064/753931375001731072/1174998805150715904), terrei il nome l10n_it_financial_statement_eu e chiederei quindi a @TennyMkt di procedere con gli script di upgrade e valutare gli altri punti aperti. Grazie

Copy link
Contributor

@SirAionTech SirAionTech left a comment

Choose a reason for hiding this comment

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

Oltre a #3599 (review), segnalo anche mktsrl#8.

@TennyMkt
Copy link
Contributor Author

TennyMkt commented Feb 1, 2024

PR aggiornata con gli script di migrazione e altre migliorie.

Aperta anche PR per aggiornare apriori.py di OpenUpgrade: OCA/OpenUpgrade#4290

@francesco-ooops
Copy link
Contributor

Oltre a #3599 (review), segnalo anche mktsrl#8.

@TennyMkt questa è da mergiare o è stata inclusa?

@TennyMkt
Copy link
Contributor Author

Oltre a #3599 (review), segnalo anche mktsrl#8.

@TennyMkt questa è da mergiare o è stata inclusa?

è da mergiare

Copy link
Member

@eLBati eLBati left a comment

Choose a reason for hiding this comment

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

Ciao @TennyMkt , segnalo che , testando il modulo insieme a account_financial_report, otteniamo questo:

ledger

Facendo un pochino di debug js , ho notato che

    _get_xlsx_name() {
        return "Financial statement EU";
    },

viene eseguito anche per i report di account_financial_report.

A voi capita?

Grazie

@TennyMkt
Copy link
Contributor Author

Ciao @TennyMkt , segnalo che , testando il modulo insieme a account_financial_report, otteniamo questo:

ledger ledger

Facendo un pochino di debug js , ho notato che

    _get_xlsx_name() {
        return "Financial statement EU";
    },

viene eseguito anche per i report di account_financial_report.

A voi capita?

Grazie

La funzione _get_xlsx_name() aveva un nome troppo generico e influenzava il comportamento di altri moduli. Siccome era stata impostata solo per eventualmente personalizzare il nome del file esportato ma al momento ritornava una costante, è stata eliminata. Se in futuro servirà parametrizzare il nome del file, verrà ri-creata la funzione con un nome più specifico.

Contestualmente sono emersi altri aggiornamenti da applicare dovuti all'aggiornamento della pre-commit

@TennyMkt TennyMkt force-pushed the 16.0-mig-l10n_it_financial_statement_eu branch from 428282f to 5234b3f Compare May 10, 2024 08:30
@eLBati
Copy link
Member

eLBati commented Jul 3, 2024

@TennyMkt Puoi fare squash di qualche commit? Ad es. il commit Fix non è molto chiaro.

@mrcast mi sai dire se questo modulo è usato in produzione da qualche cliente v16?

Grazie

@TennyMkt TennyMkt force-pushed the 16.0-mig-l10n_it_financial_statement_eu branch from 5234b3f to 3e6122b Compare July 4, 2024 09:40
@TennyMkt
Copy link
Contributor Author

TennyMkt commented Jul 4, 2024

@TennyMkt Puoi fare squash di qualche commit? Ad es. il commit Fix non è molto chiaro.

Squash delle ultime 2 commit fatto e migliorato messaggio ultima commit

Copy link

@danall59 danall59 left a comment

Choose a reason for hiding this comment

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

E' pronta per la merge!

@eLBati
Copy link
Member

eLBati commented Jul 5, 2024

/ocabot rebase

@OCA-git-bot
Copy link
Contributor

Congratulations, PR rebased to 16.0.

@OCA-git-bot OCA-git-bot force-pushed the 16.0-mig-l10n_it_financial_statement_eu branch from 3e6122b to 53dc5dd Compare July 5, 2024 08:04
oca-ci and others added 3 commits July 5, 2024 10:12
[UPD] README.rst

[UPD] README.rst
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: l10n-italy-14.0/l10n-italy-14.0-l10n_it_account_balance_eu
Translate-URL: https://translation.odoo-community.org/projects/l10n-italy-14-0/l10n-italy-14-0-l10n_it_account_balance_eu/
@eLBati eLBati force-pushed the 16.0-mig-l10n_it_financial_statement_eu branch from 53dc5dd to d3f7e0d Compare July 5, 2024 08:13
TennyMkt and others added 2 commits July 5, 2024 10:14
migration script

[FIX] l10n_it_financial_statement: tolta _get_xlsx_name e altre migliorie
Otherwise, when opening a line from the list view:
OwlError: An error occured in the owl lifecycle (see this Error's "cause" property)
    at handleError (http://oca-l10n-italy-16-0-pr3599-6f592ebc11da.runboat.odoo-community.org/web/assets/444-826ea0b/web.assets_common.min.js:1472:101)
    at owl.App.handleError (http://oca-l10n-italy-16-0-pr3599-6f592ebc11da.runboat.odoo-community.org/web/assets/444-826ea0b/web.assets_common.min.js:2100:29)
    at ComponentNode.initiateRender (http://oca-l10n-italy-16-0-pr3599-6f592ebc11da.runboat.odoo-community.org/web/assets/444-826ea0b/web.assets_common.min.js:1562:19)

Caused by: TypeError: Cannot read properties of undefined (reading 'type')
    at compareRecords (http://oca-l10n-italy-16-0-pr3599-6f592ebc11da.runboat.odoo-community.org/web/assets/445-43c5a33/web.assets_backend.min.js:8267:246)
    at stableCompare (http://oca-l10n-italy-16-0-pr3599-6f592ebc11da.runboat.odoo-community.org/web/assets/444-826ea0b/web.assets_common.min.js:6091:145)
    at Array.sort (<anonymous>)
    at Object.stableSort (http://oca-l10n-italy-16-0-pr3599-6f592ebc11da.runboat.odoo-community.org/web/assets/444-826ea0b/web.assets_common.min.js:6091:102)
    at Class._sortList (http://oca-l10n-italy-16-0-pr3599-6f592ebc11da.runboat.odoo-community.org/web/assets/445-43c5a33/web.assets_backend.min.js:8270:54)
    at http://oca-l10n-italy-16-0-pr3599-6f592ebc11da.runboat.odoo-community.org/web/assets/445-43c5a33/web.assets_backend.min.js:8237:113
    at async Promise.all (index 0)
    at async Promise.all (index 0)
    at async Class.load (http://oca-l10n-italy-16-0-pr3599-6f592ebc11da.runboat.odoo-community.org/web/assets/445-43c5a33/web.assets_backend.min.js:7927:244)
    at async Record.load (http://oca-l10n-italy-16-0-pr3599-6f592ebc11da.runboat.odoo-community.org/web/assets/445-43c5a33/web.assets_backend.min.js:2347:77)
@eLBati eLBati force-pushed the 16.0-mig-l10n_it_financial_statement_eu branch from d3f7e0d to afdf1f9 Compare July 5, 2024 08:14
@eLBati
Copy link
Member

eLBati commented Jul 5, 2024

come deciso in chiamata:

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

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

OCA-git-bot added a commit that referenced this pull request Jul 5, 2024
Signed-off-by eLBati
@OCA-git-bot
Copy link
Contributor

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

OCA-git-bot added a commit that referenced this pull request Jul 5, 2024
Signed-off-by eLBati
@OCA-git-bot
Copy link
Contributor

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

OCA-git-bot added a commit that referenced this pull request Jul 5, 2024
Signed-off-by eLBati
@OCA-git-bot
Copy link
Contributor

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

@OCA-git-bot OCA-git-bot merged commit 892985f into OCA:16.0 Jul 5, 2024
5 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at e4e5192. 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.