[16.0][MIG] account_financial_report: Migration to 16.0 #939

ramiadavid · 2022-10-28T07:37:32Z

I am migrating this module and it requires some changes:

General ledger:

The Analytic account field has been modified by Analytic distribution which is a JSON that allows assign a % in several analytic accounts:
- I modify the report so that it shows this information. If you don't like the appearance both in the report and in the xlsx, tell me what you think is better
- As the new Analytic distribution field is a JSON, I can't find a way to filter the move lines by analytic account in the domain, so I have added a many2many computed field to store the analytic accounts from the JSON to be able to filter. If you know a better way tell me how I can do it
The account.analytic.tag model has been deleted:
- It is removed from the report

This depends on OCA/reporting-engine#654 which is pending to be merged

RodrigoBM · 2022-11-30T11:13:18Z

Hi @ramiadavid ,
Thanks for this pr.

Can you fix red pre-commit and add report_xlsx to test-requirements.txt to fix runboat and tests?

Inspiration can be found there

Thanks!

ramiadavid · 2022-11-30T13:33:08Z

@RodrigoBM Ok, runboad and tests fixed but pre-commit gives an error

An unexpected error has occurred: CalledProcessError: command: ('/usr/bin/git', 'fetch', 'origin', '--tags') return code: 128 expected return code: 0 stdout: (none) stderr: fatal: could not read Username for 'https://gitlab.com/': No such device or address

pedrobaeza · 2022-11-30T13:39:55Z

pre-commit fixed in #957

You can rebase. The temporary PR put for report_xlsx seems to not be the definitive one though.

victoralmau · 2022-11-30T16:49:15Z

Please, cherry-pick #935 and #958 to commit history.

* dynamize general ledger

…ort. This commit includes move lines on the report date by default. These lines shouldn't be considered in the futur.

…#383)

…with hierarchy. Update indentation, remove empty lines from header. Update test. Update pylint. Remove company_id on computing accounts, since account.group is not a company based model, filtering accounts is done on trial balance report. Update account variables. Improve condition in padding on accounts. Add option to print hierarchy based on defined accounts/computed accounts. Add VAT report, hierarchy from tax tags ans taxes. Fix pylint, xlsx report generation header. Update code to select code_prefix or name. Update code to select code_prefix or name. Update code to select code_prefix or name. Fix domain in base amounts in vat report. Change trial balance code_prefix or name. Update trail balance, add tests for vat report. Update pylint, amounts as monetary, many2one option on generation excels. Update pulint. Add VAT Report in readme. Add VAT Report in readme. Update array_agg. Update array_agg. Update array_agg. Add option in VAT Report to be printed on Tax Tags - Tax Groups. Add widget to hierarchy_on on trial balance.

Translated using Weblate (Arabic) Currently translated at 95.7% (177 of 185 strings) Translation: account-financial-reporting-11.0/account-financial-reporting-11.0-account_financial_report Translate-URL: https://translation.odoo-community.org/projects/account-financial-reporting-11-0/account-financial-reporting-11-0-account_financial_report/ar/

Some changes in the databases been made in OCA#403, but no migration is needed.

When this module is installed along with other chart account different from generic one, the number of expected accounts and the computation change (for example, in Spain, the unaffected earnings account is 129000, choking with group with code prefix 1). This commit makes the tests resistent to these changes.

To general ledger

When there are a lot of account.move.line (several millions) and print any of the Qweb reports, that will generate also a lot of transient objects. As these objects are created with an "insert" query, the cleaning normally triggered by the count of the records in transient tables is not done, so only the cleaning based on the age of the records is processed (by default, records older than 1 hours are deleted), but the cron task is only ran one time per day. For large setups this can lead to memory errors at that point. This change prevents the memory error by executing the transient record cleanup for the report models in this module in SQL.

Currently translated at 73.9% (215 of 291 strings) Translation: account-financial-reporting-11.0/account-financial-reporting-11.0-account_financial_report Translate-URL: https://translation.odoo-community.org/projects/account-financial-reporting-11-0/account-financial-reporting-11-0-account_financial_report/de/

Currently translated at 50.2% (146 of 291 strings) Translation: account-financial-reporting-11.0/account-financial-reporting-11.0-account_financial_report Translate-URL: https://translation.odoo-community.org/projects/account-financial-reporting-11-0/account-financial-reporting-11-0-account_financial_report/es/

Currently translated at 96.2% (178 of 185 strings) Translation: account-financial-reporting-11.0/account-financial-reporting-11.0-account_financial_report Translate-URL: https://translation.odoo-community.org/projects/account-financial-reporting-11-0/account-financial-reporting-11-0-account_financial_report/ar/

Currently translated at 100.0% (291 of 291 strings) Translation: account-financial-reporting-11.0/account-financial-reporting-11.0-account_financial_report Translate-URL: https://translation.odoo-community.org/projects/account-financial-reporting-11-0/account-financial-reporting-11-0-account_financial_report/es/

Currently translated at 83.5% (243 of 291 strings) Translation: account-financial-reporting-11.0/account-financial-reporting-11.0-account_financial_report Translate-URL: https://translation.odoo-community.org/projects/account-financial-reporting-11-0/account-financial-reporting-11-0-account_financial_report/nl/

Currently translated at 100.0% (291 of 291 strings) Translation: account-financial-reporting-11.0/account-financial-reporting-11.0-account_financial_report Translate-URL: https://translation.odoo-community.org/projects/account-financial-reporting-11-0/account-financial-reporting-11-0-account_financial_report/es/

ramiadavid · 2022-12-18T08:32:53Z

Please cherry-pick #966 to historial commits

Done

victoralmau

Code review OK

pedrobaeza

The analytic conversion doesn't seem correct to me, as it involves an stored m2m field that is going to provoke a bottleneck both on install and on daily work. Data should be extracted on the fly from the new data model. And tags equivalent is lost.

pedrobaeza · 2022-12-20T08:09:18Z

account_financial_report/models/account_move_line.py



 class AccountMoveLine(models.Model):
    _inherit = "account.move.line"

+    analytic_account_ids = fields.Many2many(
+        "account.analytic.account", compute="_compute_analytic_account_ids", store=True


You must avoid this new stored field that will be triggered on module installation. Imagine if you already have millions of journal items...

Why is this needed?

@pedrobaeza
The analytic_account_id field has been changed to analytic_distribution which is a JSON.
I've created this new field because, in order to filter by Analytical Account, I don't know any other way to do it with a JSON field in a domain. And that created field has to be stored to be able to use it in a domain.

This is used in reports/general_ledger.py#L278

You know some way?

Thinking now, i think that we could use the analytic lines analytic_line_ids field to filter using the domain ('analytic_line_ids.account_id', 'in, 'xxx'). But these lines are not created until the invoice is published, and I don't know if this is a problem for a report

account_financial_report/models/account_move_line.py

account_financial_report/models/ir_actions_report.py

account_financial_report/report/general_ledger.py

pedrobaeza

Thanks for the efforts done, David.

2 things I have found reviewing the final result:

I see good to have a different color for the clickable elements, but the blue is too light and difficult a bit the reading. Can you put a darker blue?
This is outside of the migration work, but I see awkward (and also risky) that an invoicing user is able to see all the financial reports, so I think we should restrict them to "show full account" group. This can be made in a separate commit to be backported to previous versions.

ramiadavid · 2022-12-24T09:48:16Z

Thanks for the efforts done, David.

2 things I have found reviewing the final result:

I see good to have a different color for the clickable elements, but the blue is too light and difficult a bit the reading. Can you put a darker blue?

This is outside of the migration work, but I see awkward (and also risky) that an invoicing user is able to see all the financial reports, so I think we should restrict them to "show full account" group. This can be made in a separate commit to be backported to previous versions.

Hello @pedrobaeza I have not made any changes regarding the colors, it is the default color for the links in the Odoo reports, maybe they have changed it for v16. I assign a darker color by modifying the report.css. tell me how you see it now

Regarding the change of permissions, and in order not to make a mistake, it would only be to delete the first group, correct?

<menuitem parent="account.menu_finance_reports" id="menu_oca_reports" name="OCA accounting reports" groups="account.group_account_manager,account.group_account_user" />

pedrobaeza · 2022-12-24T09:54:46Z

Yes, David, the naming of the groups is terrible, as group_account_user gives more permissions than group_account_manager!

…Show Full Accounting Features'

ramiadavid · 2022-12-24T10:03:27Z

@pedrobaeza access group changed, but I don't know what is the procedure to make a backport to other versions, is it necessary to do a PR to each one of them with that commit? I guess when this PR is merged

It remains to be seen what to do with the analytic_account_ids field

pedrobaeza

Let's keep it for now as is with the analytic thing and others can do further pull requests to make more improvements.

About the backport, you have to cherry-pick the commit in a new branch and make the pull request.

/ocabot merge nobump

OCA-git-bot · 2022-12-24T10:08:20Z

On my way to merge this fine PR!
Prepared branch 16.0-ocabot-merge-pr-939-by-pedrobaeza-bump-nobump, awaiting test results.

OCA-git-bot · 2022-12-24T10:14:56Z

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

rousseldenis · 2023-10-20T08:23:56Z

@pedrobaeza @ramiadavid The introduction of the new field should have at least been set with an init hook as on existing database, it is too bulky ! I will do it.

Moreover, that field IMHO should have been set in another module...

pedrobaeza · 2023-10-20T08:38:21Z

Well, sometimes the side effects can't be anticipated. I didn't realize about it when reviewing.

ramiadavid mentioned this pull request Oct 28, 2022

Migration to version 16.0 #924

Open

7 tasks

ramiadavid force-pushed the 16.0-mig-account_financial_report branch from d8a8874 to 0473404 Compare October 28, 2022 07:47

ramiadavid marked this pull request as ready for review October 28, 2022 07:50

ramiadavid force-pushed the 16.0-mig-account_financial_report branch 2 times, most recently from 39c8677 to 547240c Compare November 30, 2022 13:26

JordiBForgeFlow and others added 21 commits November 30, 2022 18:07

[MIG] account_financial_report_qweb: Migration to 11.0

a595897

* dynamize general ledger

[FIX] account_financial_report_qweb: date condition in open items rep…

a2fb7de

…ort. This commit includes move lines on the report date by default. These lines shouldn't be considered in the futur.

[10.0] AFR Aged partner balance : columns and datas consistency. (OCA…

724b742

…#383)

[ADD] Responsive reports

c94e0b1

fixes the reporting of unaffected earnings account

56d130c

[11.0][IMP] account_financial_report - foreign_currency

c78de4c

Increasing version because of #PR403

50e18c7

Some changes in the databases been made in OCA#403, but no migration is needed.

move font-family from generic tags

d252e7d

[ADD] 11.0 account_financial_report: add filter by journals

db0c9ba

To general ledger

[11][FIX] Replace partner_ids by journal_ids

583a829

[FIX] _transient_clean_rows_older_than() WHERE clause

5b280fc

victoralmau approved these changes Dec 19, 2022

View reviewed changes

pedrobaeza requested changes Dec 20, 2022

View reviewed changes

ramiadavid force-pushed the 16.0-mig-account_financial_report branch from 117d2e5 to 21cc200 Compare December 20, 2022 13:33

pedrobaeza reviewed Dec 23, 2022

View reviewed changes

ramiadavid force-pushed the 16.0-mig-account_financial_report branch from 21cc200 to c47b644 Compare December 24, 2022 09:48

[MIG] account_financial_report: Migration to 16.0

36c5b35

ramiadavid force-pushed the 16.0-mig-account_financial_report branch from c47b644 to 36c5b35 Compare December 24, 2022 09:50

[IMP] account_financial_report: change report access groups to only '…

69d538f

…Show Full Accounting Features'

pedrobaeza approved these changes Dec 24, 2022

View reviewed changes

OCA-git-bot added the approved label Dec 24, 2022

OCA-git-bot added the bot is merging ⏳ label Dec 24, 2022

OCA-git-bot merged commit ca7c4cf into OCA:16.0 Dec 24, 2022

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels Dec 24, 2022

CasVissers-360ERP mentioned this pull request Mar 14, 2023

[MIG] Migrate mis_builder, mis_builder_budget, mis_builder_demo to 16.0 OCA/mis-builder#472

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[16.0][MIG] account_financial_report: Migration to 16.0 #939

[16.0][MIG] account_financial_report: Migration to 16.0 #939

ramiadavid commented Oct 28, 2022

RodrigoBM commented Nov 30, 2022

ramiadavid commented Nov 30, 2022

pedrobaeza commented Nov 30, 2022

victoralmau commented Nov 30, 2022

ramiadavid commented Dec 18, 2022

victoralmau left a comment

pedrobaeza left a comment

pedrobaeza Dec 20, 2022

ramiadavid Dec 20, 2022

pedrobaeza left a comment

ramiadavid commented Dec 24, 2022

pedrobaeza commented Dec 24, 2022

ramiadavid commented Dec 24, 2022

pedrobaeza left a comment

OCA-git-bot commented Dec 24, 2022

OCA-git-bot commented Dec 24, 2022

rousseldenis commented Oct 20, 2023 •

edited

Loading

pedrobaeza commented Oct 20, 2023

[16.0][MIG] account_financial_report: Migration to 16.0 #939

[16.0][MIG] account_financial_report: Migration to 16.0 #939

Conversation

ramiadavid commented Oct 28, 2022

RodrigoBM commented Nov 30, 2022

ramiadavid commented Nov 30, 2022

pedrobaeza commented Nov 30, 2022

victoralmau commented Nov 30, 2022

ramiadavid commented Dec 18, 2022

victoralmau left a comment

Choose a reason for hiding this comment

pedrobaeza left a comment

Choose a reason for hiding this comment

pedrobaeza Dec 20, 2022

Choose a reason for hiding this comment

ramiadavid Dec 20, 2022

Choose a reason for hiding this comment

pedrobaeza left a comment

Choose a reason for hiding this comment

ramiadavid commented Dec 24, 2022

pedrobaeza commented Dec 24, 2022

ramiadavid commented Dec 24, 2022

pedrobaeza left a comment

Choose a reason for hiding this comment

OCA-git-bot commented Dec 24, 2022

OCA-git-bot commented Dec 24, 2022

rousseldenis commented Oct 20, 2023 • edited Loading

pedrobaeza commented Oct 20, 2023

rousseldenis commented Oct 20, 2023 •

edited

Loading