Skip to content

[16.0][IMP] l10n_it_financial_statements_report: add company header #3661

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

Conversation

odooNextev
Copy link
Contributor

Attualmente i report PDF dei bilanci di questo modulo non hanno riferimenti all'azienda a cui si riferiscono i dati ed in ambito multiazienda è un problema.

Per questo motivo ho aggiunto l'intestazione standard al template.

#3660

@stenext
Copy link

stenext commented Oct 11, 2023

Testando questa PR mi sono accorto che bisognerebbe aggiungere nel readme di includere manualmente l'utente nel gruppo "Show Full Accounting Features" per visualizzare la voce di menu per stampare il report.
@SirAionTech che vedo hai fatto la migrazione alla 16.0 (nella 14.0 non penso ci fosse questo problema) cosa ne pensi?
Lo posso aggiungere in questa PR senza aprirne una apposita per una riga?

@SirAionTech
Copy link
Contributor

Testando questa PR mi sono accorto che bisognerebbe aggiungere nel readme di includere manualmente l'utente nel gruppo "Show Full Accounting Features" per visualizzare la voce di menu per stampare il report.
@SirAionTech che vedo hai fatto la migrazione alla 16.0 (nella 14.0 non penso ci fosse questo problema) cosa ne pensi?

Da quello che mi ricordo della migrazione, credo che il comportamento fosse lo stesso in 14.0 quindi sarebbe il caso di seguire https://github.com/OCA/l10n-italy/wiki/Team-di-sviluppo#apertura-issue.

Lo posso aggiungere in questa PR senza aprirne una apposita per una riga?

Se non vuoi aprire un'altra PR è sufficiente metterlo in un commit separato, perché non mi sembra una modifica che ha a che fare con le modifiche di questa PR.
Altrimenti se fosse tutto in un unico commit e dovessimo farne revert, ci perderemmo anche la modifica al README.

@odooNextev
Copy link
Contributor Author

Testando questa PR mi sono accorto che bisognerebbe aggiungere nel readme di includere manualmente l'utente nel gruppo "Show Full Accounting Features" per visualizzare la voce di menu per stampare il report.
@SirAionTech che vedo hai fatto la migrazione alla 16.0 (nella 14.0 non penso ci fosse questo problema) cosa ne pensi?

Da quello che mi ricordo della migrazione, credo che il comportamento fosse lo stesso in 14.0 quindi sarebbe il caso di seguire https://github.com/OCA/l10n-italy/wiki/Team-di-sviluppo#apertura-issue.

Lo posso aggiungere in questa PR senza aprirne una apposita per una riga?

Se non vuoi aprire un'altra PR è sufficiente metterlo in un commit separato, perché non mi sembra una modifica che ha a che fare con le modifiche di questa PR. Altrimenti se fosse tutto in un unico commit e dovessimo farne revert, ci perderemmo anche la modifica al README.

Ho aggiunto delle operazioni preliminari post installazione e la traduzione della sezione usage nel readme 45407d7

@odooNextev odooNextev force-pushed the 16.0-imp-l10n_it_financial_statements_report branch from 45407d7 to 9d0b745 Compare October 12, 2023 13:54
t-call="l10n_it_financial_statements_report.financial_statements_report_base"
/>
<t t-set="company" t-value="o.company_id" />
<t t-call="web.external_layout">

Choose a reason for hiding this comment

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

Ciao, perchè richiami:
<t t-call="web.external_layout">
in precedenza non era presente, ti serve per qualcosa in particolare?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ciao,
di default in questo report non viene stampato alcun riferimento all'azienda a cui appartiene il bilancio ed in ambito multiazienda non è accettabile.
La maniera più veloce per risolvere questo problema è aggiungere l'header classico web.external_layout

Copy link
Contributor

Choose a reason for hiding this comment

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

Anche io sono un po' perplesso da questa modifica:

  • Mi sembra strano aggiungere un layout esterno (web.external_layout), che quindi è per documenti che vanno distribuiti esternamente, quando il layout precedente era interno (account_financial_report.internal_layout); ma credo che questo documento alla fine vada distribuito esternamente quindi magari la nomenclatura era già sbagliata prima.
  • Lo stesso problema esiste anche per tutti gli altri report OCA giusto? Allora non dovrebbe essere modificato il layout account_financial_report.internal_layout?

Ma queste non sono cose secondo me bloccanti.

Il problema è che applicando due layout viene aggiunto due volte il numero di pagina: nel runboat della 14.0 (che ha queste stesse modifiche da #3656) ho ottenuto Financial Statements Report HTML.pdf che ha 2 pagine: entrambe con il numero di pagina 1/1.
Questo perché il numero di pagina viene aggiunto sia in https://github.com/odoo/odoo/blob/9be75bb17063c6b0207aa82144e9f44b6e49dc45/addons/web/views/report_templates.xml#L472 (è uno dei possibili layout esterni) che in https://github.com/OCA/account-financial-reporting/blob/e8942d398c64827ec1eed883997efbd6196c1c5c/account_financial_report/report/templates/layouts.xml#L21.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Effettivamente come segnalavi venivano duplicate le pagine perchè sia external_layout che account_financial_report.internal_layout aggiungevano parti di template comuni.

Non so quanto sia pulita la soluzione che ho appena pubblicata, ma è l'unica che ho trovato per avere lo stile corretto e non avere duplicazioni.

Copy link
Member

Choose a reason for hiding this comment

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

Ciao, di default in questo report non viene stampato alcun riferimento all'azienda a cui appartiene il bilancio ed in ambito multiazienda non è accettabile. La maniera più veloce per risolvere questo problema è aggiungere l'header classico web.external_layout

Ma quindi questo problema lo avrebbero tutti i report di account_financial_report ? A partire dal bilancio di verifica?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sì, tutti e 6

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

@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!
Puoi verificare quanto ho scritto sotto?

t-call="l10n_it_financial_statements_report.financial_statements_report_base"
/>
<t t-set="company" t-value="o.company_id" />
<t t-call="web.external_layout">
Copy link
Contributor

Choose a reason for hiding this comment

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

Anche io sono un po' perplesso da questa modifica:

  • Mi sembra strano aggiungere un layout esterno (web.external_layout), che quindi è per documenti che vanno distribuiti esternamente, quando il layout precedente era interno (account_financial_report.internal_layout); ma credo che questo documento alla fine vada distribuito esternamente quindi magari la nomenclatura era già sbagliata prima.
  • Lo stesso problema esiste anche per tutti gli altri report OCA giusto? Allora non dovrebbe essere modificato il layout account_financial_report.internal_layout?

Ma queste non sono cose secondo me bloccanti.

Il problema è che applicando due layout viene aggiunto due volte il numero di pagina: nel runboat della 14.0 (che ha queste stesse modifiche da #3656) ho ottenuto Financial Statements Report HTML.pdf che ha 2 pagine: entrambe con il numero di pagina 1/1.
Questo perché il numero di pagina viene aggiunto sia in https://github.com/odoo/odoo/blob/9be75bb17063c6b0207aa82144e9f44b6e49dc45/addons/web/views/report_templates.xml#L472 (è uno dei possibili layout esterni) che in https://github.com/OCA/account-financial-reporting/blob/e8942d398c64827ec1eed883997efbd6196c1c5c/account_financial_report/report/templates/layouts.xml#L21.

@francesco-ooops
Copy link
Contributor

@odooNextev puoi rilanciare il runboat?

@stenext stenext force-pushed the 16.0-imp-l10n_it_financial_statements_report branch from 9d0b745 to 57c906c Compare January 11, 2024 08:12
@odooNextev
Copy link
Contributor Author

@odooNextev puoi rilanciare il runboat?

Fatto

Copy link

github-actions bot commented Jun 9, 2024

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Jun 9, 2024
@github-actions github-actions bot closed this Aug 11, 2024
@francesco-ooops
Copy link
Contributor

/ocabot rebase

@OCA-git-bot
Copy link
Contributor

@francesco-ooops The rebase process failed, because command git rebase origin/16.0 failed with output:

Rebasing (1/1)
CONFLICT (modify/delete): l10n_it_financial_statements_report/readme/USAGE.rst deleted in HEAD and modified in 57c906c85 ([IMP] l10n_it_financial_statements_report: add company header).  Version 57c906c85 ([IMP] l10n_it_financial_statements_report: add company header) of l10n_it_financial_statements_report/readme/USAGE.rst left in tree.
error: could not apply 57c906c85... [IMP] l10n_it_financial_statements_report: add company header
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 57c906c85... [IMP] l10n_it_financial_statements_report: add company header

@francesco-ooops
Copy link
Contributor

@odooNextev puoi fare rebase?

@francesco-ooops francesco-ooops removed the stale PR/Issue without recent activity, it'll be soon closed automatically. label Sep 16, 2024
@SirAionTech SirAionTech added the needs fixing Has conflicts or is failing mandatory CI checks label Dec 11, 2024
@stenext stenext force-pushed the 16.0-imp-l10n_it_financial_statements_report branch 2 times, most recently from d9056a1 to 58d784e Compare March 20, 2025 16:42
@odooNextev odooNextev requested a review from SirAionTech March 20, 2025 16:46
@odooNextev
Copy link
Contributor Author

@francesco-ooops manca una review di codice e poi si può mergiare.
E' abbastanza fondamentale perchè non ho usato proprio un metodo pulitissimo per avere il risultato sperato, ma mi sembra quasi impossibile utilizzare totalmente l'ereditarietà senza che si raddoppino le pagine.

@francesco-ooops
Copy link
Contributor

@OCA/local-italy-maintainers potete dare un'occhiata?

t-call="l10n_it_financial_statements_report.financial_statements_report_base"
/>
<t t-set="company" t-value="o.company_id" />
<t t-call="web.external_layout">
Copy link
Member

Choose a reason for hiding this comment

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

Ciao, di default in questo report non viene stampato alcun riferimento all'azienda a cui appartiene il bilancio ed in ambito multiazienda non è accettabile. La maniera più veloce per risolvere questo problema è aggiungere l'header classico web.external_layout

Ma quindi questo problema lo avrebbero tutti i report di account_financial_report ? A partire dal bilancio di verifica?

@eLBati
Copy link
Member

eLBati commented Mar 28, 2025

In chiamata abbiamo valutato che , anche se si tratta di una stampa interna, i dati dell'azienda è bene averli

@eLBati
Copy link
Member

eLBati commented Mar 28, 2025

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 16.0-ocabot-merge-pr-3661-by-eLBati-bump-patch, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Mar 28, 2025
Signed-off-by eLBati
@OCA-git-bot
Copy link
Contributor

@eLBati The merge process could not be finalized, because command oca-gen-addon-readme --if-source-changed --org-name OCA --repo-name l10n-italy --branch 16.0 --addons-dir /tmp/tmpjjc57dk2 --commit failed with output:

Both .md and .rst found for USAGE. Please remove one of /tmp/tmpjjc57dk2/l10n_it_financial_statements_report/readme/USAGE.rst or /tmp/tmpjjc57dk2/l10n_it_financial_statements_report/readme/USAGE.md.

@francesco-ooops
Copy link
Contributor

@odooNextev puoi modificare il file USAGE.md e rimuovere l'rst?

@stenext stenext force-pushed the 16.0-imp-l10n_it_financial_statements_report branch 2 times, most recently from da8de94 to 14f95d4 Compare March 28, 2025 11:33
@odooNextev
Copy link
Contributor Author

@odooNextev puoi modificare il file USAGE.md e rimuovere l'rst?

Fatto

@francesco-ooops
Copy link
Contributor

@odooNextev mmm no ora non c'è più la modifica

@stenext stenext force-pushed the 16.0-imp-l10n_it_financial_statements_report branch from 14f95d4 to 2dae549 Compare March 28, 2025 13:18
@odooNextev
Copy link
Contributor Author

@odooNextev mmm no ora non c'è più la modifica

Giusto, ho corretto adesso

@eLBati
Copy link
Member

eLBati commented Apr 4, 2025

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-3661-by-eLBati-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 20ecb43 into OCA:16.0 Apr 4, 2025
6 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at b437efc. 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
Labels
16.0 merged 🎉 needs fixing Has conflicts or is failing mandatory CI checks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

l10n_it_financial_statements_report: mancano i dati dell'azienda nei report
8 participants