[14.0] mig l10n_it_account_balance_report by TonyMasciI · Pull Request #3169 · OCA/l10n-italy

TonyMasciI · 2023-02-10T11:58:34Z

Storia PR:

Prima: PR chiusa per errore in fase di rebase

Seconda: PR aperta in seguenza alla prima, ma mancano i commit di migrazione

stefano-ooops · 2023-02-10T13:55:07Z

Come nella prima PR errore ancora presente
i test li ho fatti andando a stampare conto economico e stato patrimoniale dai movimenti presenti nel runboat, ti allego la stampa del mastrino che viene riportato in maniera errata nello stato patrimoniale

francesco-ooops · 2023-02-13T11:58:08Z

@TonyMasciI facci sapere quando si può testare nuovamente

francesco-ooops · 2023-02-21T08:58:29Z

Come nella prima PR errore ancora presente i test li ho fatti andando a stampare conto economico e stato patrimoniale dai movimenti presenti nel runboat, ti allego la stampa del mastrino che viene riportato in maniera errata nello stato patrimoniale

@TonyMasciI riesci a verificare questo? grazie!

TonyMasciI · 2023-02-21T10:31:43Z

Come nella prima PR errore ancora presente i test li ho fatti andando a stampare conto economico e stato patrimoniale dai movimenti presenti nel runboat, ti allego la stampa del mastrino che viene riportato in maniera errata nello stato patrimoniale

@TonyMasciI riesci a verificare questo? grazie!

Ciao @francesco-ooops Il Mastrino che stai testando non è il menù che è presente in questo modulo, questo menù è presente in questo Repo, è vero che questo modulo l10n_it_account_balance_report dipende da account_financial_report, ma deve essere testato quest'altro menù:

Ti direi di testare il menù nel Repo, e nel caso di aprire un issue.

francesco-ooops · 2023-02-21T11:19:55Z

@TonyMasciI forse abbiamo capito dove sta l'inghippo, puoi mettere la PR in "open" così possiamo testarla internamente? grazie

francesco-ooops · 2023-03-02T14:27:47Z

@SirTakobi visto che lavoravi su #3199 , ti posso chiedere di verificare se per te è tutto ok in questa migrazione? :)

SirTakobi · 2023-03-03T09:48:42Z

@SirTakobi visto che lavoravi su #3199 , ti posso chiedere di verificare se per te è tutto ok in questa migrazione? :)

Ok quando ho tempo la guardo

francesco-ooops · 2023-03-17T11:32:14Z

@TonyMasciI questo doc è l'esito del test di @stefano-ooops su un db pulito della 12 e della 14, puoi darci un feedback?

francesco-ooops · 2023-03-22T16:23:45Z

@TonyMasciI gentile reminder

TonyMasciI · 2023-03-24T14:18:31Z

@TonyMasciI questo doc è l'esito del test di @stefano-ooops su un db pulito della 12 e della 14, puoi darci un feedback?

Rilasciata una correzione al problema che hai riscontrato. Fammi sapere!

stefano-ooops · 2023-03-24T14:38:13Z

@TonyMasciI questo doc è l'esito del test di @stefano-ooops su un db pulito della 12 e della 14, puoi darci un feedback?

Rilasciata una correzione al problema che hai riscontrato. Fammi sapere!

testato su runbot ora funziona

francesco-ooops · 2023-03-24T14:41:09Z

@SirTakobi se vuoi dare un occhio finalizziamo 🤞

stefano-ooops

OK FUNZIONA

SirTakobi

Grazie della PR!
Puoi schiacciare i commit dei bot come indicato in https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests#mergesquash-the-commits-generated-by-bots-or-weblate?

Visto che sono state aggiunte centinaia di righe di codice, potresti aggiungere almeno un test?

SirTakobi · 2023-03-24T16:12:24Z

-        Incomes = "Income", "Other Income"
+        Incomes = "Current Year Earnings", "Income", "Other Income"
        -->
+        <record model="account.account.type" id="account.data_unaffected_earnings">


Come mai è necessario modificare questo tipo di conto per la migrazione?
Insieme alla modifica poco sotto, sembra essere un rollback di #2832

SirTakobi · 2023-03-24T16:15:02Z

-            ("incomes", "Incomes"),
-            ("liabilities", "Liabilities"),
-        ],
+        # [


Se non servono penso sia meglio eliminarli

SirTakobi · 2023-03-24T16:26:14Z

+                        <t t-if="o.hide_account_at_0">
+                            <t t-set="style" t-value="'font-size: 14px;'" />
+                        </t>
+                        <t t-if="o.show_hierarchy">


Come mai show_hierarchy è preso da o mentre più sotto limit_hierarchy_level direttamente dalla variabili disponibili?
Se non c'è un motivo specifico, si può uniformare il comportamento?

SirTakobi · 2023-03-24T16:43:31Z

+            <div class="act_as_cell left">
+                <span>
+                    <a
+                        t-att-data-active-id="line.account_id.id"


Suggested change

t-att-data-active-id="line.account_id.id"

t-att-data-active-id="active_id"

SirTakobi · 2023-03-24T16:48:54Z

                    <t
                        t-set="domain"
-                        t-value="[('account_id', 'in', line.compute_account_ids.ids),
+                        t-value="[('account_id', 'in', line.group_id.account_ids.ids),


Come mai questa modifica? Guardando come veniva calcolato compute_account_ids non mi sembra equivalente, ma forse mi sono perso qualcosa nelle logiche della migrazione di account.

SirTakobi · 2023-03-27T08:29:53Z

+        Report, the only data we need is the ending_balance field.
+        """
+
+        def is_removable(d):


Questo cambia il comportamento anche del report originale, è voluto?
Mi pare che l'unica cosa diversa sia questa funzione innestata, quindi prima di fare un override completo proverei almeno a suggerire nel modulo account_financial_report di estrarre questa funzione (magari in un metodo model), e implementare lì la nuova logica ma solo per il nostro report.

SirTakobi · 2023-03-27T09:27:28Z


-    @api.multi
-    def compute_data_for_report(self):
+    def clear_data(self, tb_data):


Questo metodo è dedicato all'inversione di segno dei totali di conti di un certo tipo, non ho trovato nulla del genere prima della migrazione, come mai è stato necessario aggiungerlo?
Prima della migrazione, i segni del totale venivano ribaltati semplicemente se negativi:

l10n-italy/l10n_it_account_balance_report/report/account_balance_report.py

Lines 163 to 167 in 7fc7eac

total_balance = 0

if float_compare(total_credit, total_debit, digits) == 1:

total_balance = total_credit - total_debit

elif float_compare(total_credit, total_debit, digits) == -1:

total_balance = total_debit - total_credit

Aggiunto il metodo clear_data per aggiustare i segni dei totali dei conti, dato che le informazioni arrivano sballate dal modello trial_banace del modulo account_financial_report, questa modifica è stata apportata per aveere dei conti sotto 0 come nel caso d'uso del documento.

SirTakobi · 2023-03-27T10:04:18Z

        [("profit_loss", "Profit & Loss"), ("balance_sheet", "Balance Sheet")],
    )
+
+    company_id = fields.Many2one(


Ho capito che tutte queste colonne vengono aggiunte perché non sono più recuperabili dal vecchio trial_balance_id, ma come mai è necessario aggiungerle? Non si può invece seguire la logica che viene utilizzata nel Trial Balance?
Ad esempio invece di dichiarare il campo company_id (che non viene più definito come campo in nessun report di account_financial_report), si potrebbe utilizzare il valore tb_data['company_id'].

In questo modo credo che le modifiche della migrazione sarebbero molto più ridotte, ma forse mi sfugge qualche problema in questo approccio?
In pratica, mi pare si stia cercando di ricreare la vecchia logica dei report account_financial_report invece di adattarsi alla nuova logica, e questo implica la creazione di tutti questi campi e le centinaia di righe di codice aggiunte nella migrazione.
Se possibile (come scrivevo, magari mi sfugge qualcosa per cui tutto ciò è necessario), cercherei di mantenere le modifiche di migrazione minimali.

per i campi company_id | date_from | date_to sono stato costretto a lasciarli perché servono per la generazione del report.

Ho provato a seguire la logica utilizzata dal Trial Balance in #3343 e non serve aggiungere nuovi campi, fammi sapere cosa ne pensi

francesco-ooops · 2023-03-30T08:38:29Z

@TonyMasciI riesci a sistemare a stretto giro?

francesco-ooops · 2023-04-06T08:04:49Z

@TonyMasciI ping :)

francesco-ooops · 2023-04-14T07:57:14Z

@TonyMasciI pong

francesco-ooops · 2023-04-19T09:25:21Z

@TonyMasciI pensi di riuscire a metterci le mani a breve?

Altrimenti siamo disponibili a prenderla in carico, basta farci sapere :)

francesco-ooops · 2023-04-26T07:53:38Z

@TonyMasciI se gentilmente mi dai conferma che non hai aggiornamenti pronti da aggiungere, questa la prendiamo in carico noi

TonyMasciI · 2023-04-26T07:57:45Z

@TonyMasciI se gentilmente mi dai conferma che non hai aggiornamenti pronti da aggiungere, questa la prendiamo in carico noi

Ciao, controllo e ti faccio sapere

francesco-ooops · 2023-04-27T07:43:24Z

@TonyMasciI buongiorno, riesci a darci un feedback in giornata? vorremmo arrivare in fondo a questa PR

grazie

TonyMasciI · 2023-04-27T09:35:00Z

@TonyMasciI buongiorno, riesci a darci un feedback in giornata? vorremmo arrivare in fondo a questa PR

grazie

@francesco-ooops vorrei aggiungere anche dei TEST, ma non credo che riesco in giornata, forse domani pomeriggio o settimana prossima.

francesco-ooops · 2023-04-27T10:21:36Z

@SirTakobi puoi verificare gli ultimi aggiornamenti?

Currently translated at 92.9% (119 of 128 strings) Translation: l10n-italy-12.0/l10n-italy-12.0-l10n_it_account_balance_report Translate-URL: https://translation.odoo-community.org/projects/l10n-italy-12-0/l10n-italy-12-0-l10n_it_account_balance_report/it/

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: l10n-italy-12.0/l10n-italy-12.0-l10n_it_account_balance_report Translate-URL: https://translation.odoo-community.org/projects/l10n-italy-12-0/l10n-italy-12-0-l10n_it_account_balance_report/

Currently translated at 95.3% (122 of 128 strings) Translation: l10n-italy-12.0/l10n-italy-12.0-l10n_it_account_balance_report Translate-URL: https://translation.odoo-community.org/projects/l10n-italy-12-0/l10n-italy-12-0-l10n_it_account_balance_report/it/

Usando il modulo l10n_it_account_balance_report, il cron di Pulizia dati interni diventa molto lento e in alcuni casi va in blocco. L'autovacuum della tabella report_trial_balance diventa molto lenta per via delle cascade presenti nel modulo. Inserendo gli indici nei campi collegati il problema si risolve, come già realizzato nel repo OCA/account-financial-reporting#750.

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: l10n-italy-12.0/l10n-italy-12.0-l10n_it_account_balance_report Translate-URL: https://translation.odoo-community.org/projects/l10n-italy-12-0/l10n-italy-12-0-l10n_it_account_balance_report/

stefano-ooops

testato ok

francesco-ooops · 2023-05-19T13:39:28Z

@SirTakobi dovremmo esserci, riesci a fare review?

SirTakobi · 2023-05-19T14:01:52Z

@SirTakobi dovremmo esserci, riesci a fare review?

Ne abbiamo parlato stamattina in https://discord.gg/yesf5HJ, e ieri anche internamente, per il momento non riesco ad aggiornare la review

eLBati

Ci stiamo lavorando per provare a sistemare alcune cose. Pubblicheremo nei prossimi giorni

SirTakobi · 2023-05-18T12:23:05Z

Puoi schiacciare i commit dei bot come indicato in https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests#mergesquash-the-commits-generated-by-bots-or-weblate?

(da #3169 (review))

❔

SirTakobi · 2023-05-18T12:25:51Z

Mancano diversi commit, a partire da 6e7b901

SirTakobi · 2023-05-18T12:30:30Z

-    "maintainers": ["SilvioGregorini"],
-    "website": "https://github.com/OCA/l10n-italy"
-    "/tree/12.0/l10n_it_account_balance_report",
+    "website": "https://github.com/OCA/l10n-italy" "/l10n_it_account_balance_report",


Il link "https://github.com/OCA/l10n-italy/l10n_it_account_balance_report" non è corretto, dovrebbe essere "https://github.com/OCA/l10n-italy/tree/14.0/l10n_it_account_balance_report" oppure "https://github.com/OCA/l10n-italy"

SirTakobi · 2023-05-18T12:32:11Z

        </record>

        <!--
-        Liabilities = "Current Year Earnings", "Payable", "Credit Card", "Prepayments",


Questa modifica penso sia da rimuovere

SirTakobi · 2023-05-18T12:35:37Z

-        Incomes = "Income", "Other Income"
+        Incomes = "Current Year Earnings", "Income", "Other Income"
        -->
+        <record model="account.account.type" id="account.data_unaffected_earnings">


SirTakobi · 2023-05-18T12:35:47Z

-            ("incomes", "Incomes"),
-            ("liabilities", "Liabilities"),
-        ],
+        # [


SirTakobi · 2023-05-18T13:08:33Z

+            <div class="act_as_cell left">
+                <span>
+                    <a
+                        t-att-data-active-id="line.account_id.id"


SirTakobi · 2023-05-18T13:09:28Z

                    <t
                        t-set="domain"
-                        t-value="[('account_id', 'in', line.compute_account_ids.ids),
+                        t-value="[('account_id', 'in', line.group_id.account_ids.ids),


SirTakobi · 2023-05-18T13:10:04Z

+        Report, the only data we need is the ending_balance field.
+        """
+
+        def is_removable(d):


SirTakobi · 2023-05-18T13:26:21Z

    )

+    # Override of `trial.balance.report.wizard` to set True as default value
+    show_hierarchy = fields.Boolean(default=True)


Questo non mi sembra faccia parte della migrazione ma è un nuovo comportamento, che modifica anche il comportamento del wizard da cui eredita; puoi metterlo in un commit o una PR dedicata?

SirTakobi · 2023-06-27T11:34:29Z

        [("profit_loss", "Profit & Loss"), ("balance_sheet", "Balance Sheet")],
    )
+
+    company_id = fields.Many2one(


Ho provato a seguire la logica utilizzata dal Trial Balance in #3343 e non serve aggiungere nuovi campi, fammi sapere cosa ne pensi

TonyMasciI · 2023-07-06T07:04:45Z

Chiusa in favore della PR: #3343

TonyMasciI mentioned this pull request Feb 10, 2023

14.0 mig l10n it account balance report #3130

Closed

TonyMasciI force-pushed the 14.0-mig-l10_it_account_balance_report branch from 6f6af42 to 9038783 Compare February 10, 2023 13:24

TonyMasciI force-pushed the 14.0-mig-l10_it_account_balance_report branch from 9038783 to d6c2fce Compare February 10, 2023 13:58

TonyMasciI marked this pull request as ready for review February 21, 2023 11:25

eLBati changed the title ~~14.0 mig l10 it account balance report~~ [14.0] mig l10n_it_account_balance_report Mar 22, 2023

TonyMasciI force-pushed the 14.0-mig-l10_it_account_balance_report branch from 6d9122d to 05cc242 Compare March 24, 2023 14:17

stefano-ooops approved these changes Mar 24, 2023

View reviewed changes

SirTakobi suggested changes Mar 27, 2023

View reviewed changes

TonyMasciI force-pushed the 14.0-mig-l10_it_account_balance_report branch from 5b5b484 to 6271728 Compare April 27, 2023 09:04

OCA-git-bot and others added 18 commits May 19, 2023 12:02

l10n_it_account_balance_report 12.0.1.0.2

8c8be9a

[UPD] account_balance_report: use numerical values in .xlsx reports

7a04024

[UPD] Update l10n_it_account_balance_report.pot

e6f1bbf

l10n_it_account_balance_report 12.0.1.0.3

58725bb

[UPD] Update l10n_it_account_balance_report.pot

7b60d98

l10n_it_account_balance_report 12.0.1.0.4

ab2aa2e

[FIX] l10n_it_account_balance_report: Consider previous years' earnings

aa036ec

[UPD] Update l10n_it_account_balance_report.pot

86e75b7

Reformatted after template update

aa881c5

Manually fix manifest website key

acde92b

[IMP] l10_it_account_balance_report: black, isort, prettier

5654db3

[MIG] l10n_it_account_balance_report to 14.0

f79338e

TonyMasciI force-pushed the 14.0-mig-l10_it_account_balance_report branch from d3449fd to f79338e Compare May 19, 2023 10:03

stefano-ooops approved these changes May 19, 2023

View reviewed changes

GSLabIt approved these changes May 22, 2023

View reviewed changes

tafaRU mentioned this pull request May 24, 2023

Migration to version 14.0 #1905

Open

76 tasks

eLBati requested changes May 25, 2023

View reviewed changes

SirTakobi mentioned this pull request May 30, 2023

[14.0] [MIG] l10n_it_account_balance_report > l10n_it_financial_statements_report #3343

Merged

6 tasks

SirTakobi reviewed Jun 27, 2023

View reviewed changes

SirTakobi suggested changes Jun 27, 2023

View reviewed changes

TonyMasciI closed this Jul 6, 2023

	t-att-data-active-id="line.account_id.id"
	t-att-data-active-id="active_id"

	total_balance = 0
	if float_compare(total_credit, total_debit, digits) == 1:
	total_balance = total_credit - total_debit
	elif float_compare(total_credit, total_debit, digits) == -1:
	total_balance = total_debit - total_credit

Uh oh!

Conversation

TonyMasciI commented Feb 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stefano-ooops commented Feb 10, 2023

Uh oh!

francesco-ooops commented Feb 13, 2023

Uh oh!

francesco-ooops commented Feb 21, 2023

Uh oh!

TonyMasciI commented Feb 21, 2023

Uh oh!

francesco-ooops commented Feb 21, 2023

Uh oh!

francesco-ooops commented Mar 2, 2023

Uh oh!

SirTakobi commented Mar 3, 2023

Uh oh!

francesco-ooops commented Mar 17, 2023

Uh oh!

francesco-ooops commented Mar 22, 2023

Uh oh!

TonyMasciI commented Mar 24, 2023

Uh oh!

stefano-ooops commented Mar 24, 2023

Uh oh!

francesco-ooops commented Mar 24, 2023

Uh oh!

stefano-ooops left a comment

Choose a reason for hiding this comment

Uh oh!

SirTakobi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

francesco-ooops commented Mar 30, 2023

Uh oh!

francesco-ooops commented Apr 6, 2023

Uh oh!

francesco-ooops commented Apr 14, 2023

Uh oh!

francesco-ooops commented Apr 19, 2023

Uh oh!

francesco-ooops commented Apr 26, 2023

Uh oh!

TonyMasciI commented Apr 26, 2023

Uh oh!

francesco-ooops commented Apr 27, 2023

TonyMasciI commented Feb 10, 2023 •

edited

Loading

TonyMasciI commented Apr 27, 2023 •

edited

Loading