[15.0][IMP] account_financial_report: Add grouping by analytical account in Trial balance

[victoralmau]

Changes done:

• Change t-esc and t-raw to t-out fromTrial Balance report
• Refactor from Trial Balance
• Add grouping by analytical account in Trial balance

Please @pedrobaeza can you review it?

@Tecnativa TT48969

[victoralmau]

Ping @pedrobaeza

[pedrobaeza]

On a test on runboat, all the groups by analytic accounting are putting the "Undistributed loss/gain results" with value 0 (account 129 in Spanish CoA, 9999999 in the sample CoA), and even more as I checked the option "Hide accounts at 0" (but it should be independent - it mustn't never appear).

[victoralmau]

Changes done. Complete refactoring and added the option of grouping by analytical account in an extra commit.

[victoralmau]

Finally all changes are done and tested in a customer use case.

[pedrobaeza]

The refactoring is not successful, as the idea is to reduce the number or methods and arguments, and I still see a lot of both. When I told you #1151 (comment), it was to remove all the parameters except one. And I think most of the domains can be shared and reduced, as it's the same over and over...

[pedrobaeza]

Can you use fixed amounts? These tests are wicked this way.

[victoralmau]

Fixed amounts? This was (and is) trying to pull the corresponding report line filtering by some data (account and partner).

[victoralmau]

The refactoring is not successful, as the idea is to reduce the number or methods and arguments, and I still see a lot of both. When I told you #1151 (comment), it was to remove all the parameters except one. And I think most of the domains can be shared and reduced, as it's the same over and over...

This refactoring is not enough? If you prefer i can delete the report completely and do it again, or change it as much as you want with many more hours, but the problem will always be the same, there is NOT enough data (neither in the tests nor creating them manually) to test that all the use cases work correctly and nothing is broken, so you always have to test these changes in "real environments", the same happens with other reports, it is absolutely frustrating.

[victoralmau]

New approach done (3rd) with a clear objective: not to change the existing behavior, only to add a new one.

Changes done (and the reason):

• Added grouped_by field in wizard to group by analytical account.
• Fields are added to the groupby when you want to group and queries are done to get the data by analytical accounts.
• Set the grouped data in total_amount (to use these data later).
• The data currently used in the report: trial_balance and total_amount have a fixed structure on account that we do not want to change (we want to keep the existing operation so far), set trial_balance_grouped to obtain the data grouped by analytical account and account + total_amount_grouped (to represent the sum of all totals). They are also used in the XLS export.
• A different way has been set in template when you want to group to show the data.
• It has been set in the template to add analytic_account_id to the domain when clicking on a data if we are grouping.

[pedrobaeza]

Instead of having to add this extra domain everywhere, can't we add the extra bits in the already present domain variable?

[victoralmau]

I have changed to add aml_domain_extra only where it is really needed.

Just for clarification, you want to then add all this code:

<t
    t-set="aml_domain_extra"
    t-if="grouped_item['id'] > 0"
    t-value="[('analytic_account_id', '=', grouped_item['id'])]"
/>
<t
    t-set="aml_domain_extra"
    t-else=""
    t-value="[('analytic_account_id', '=', False)]"
/>

in the 7 places where aml_domain_extra is currently added (in this case I have set global variable precisely to reduce code and do the same thing that aml_domain_common does for example).

[acysos]

Hello,

Functionally it is correct. I have tested it.

Greetings

[victoralmau]

Ping @pedrobaeza

[pedrobaeza]

/ocabot merge major

[OCA-git-bot]

This PR looks fantastic, let's merge it!
Prepared branch 15.0-ocabot-merge-pr-1151-by-pedrobaeza-bump-major, awaiting test results.

[OCA-git-bot]

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