account-analytic [16.0][ADD] analytic_mixin_analytic

This module adds a many2many analytic accounts field to the analytic mixin, calculated from the value of the analytic distribution, in an effort to mitigate the difficulties associated with the absence of these fields directly on the model inheriting the mixin.

@qrtl

Jun 16 '23 03:06 AungKoKoLin1997

I think it would be better to have the fields stored

I don't see the reason to make the fields stored at the moment.

On the other hand, It would be great to have it integrated at some point with others modules like this one - https://github.com/OCA/account-financial-reporting/blob/16.0/account_financial_report/models/account_move_line.py - to avoid having several logic in the OCA

Yes, I think so. After this module is merged, we can make adjustment to other modules that want to have analytic accounts by just inheriting this module.

Sep 29 '23 04:09 AungKoKoLin1997

This is really nice. As long as many modules will depend on this one I think it is better to introduce it as LGPL, that way we don't have to modify the license in the modules that inherit from this one.

Sep 29 '23 09:09 AaronHForgeFlow

Yes, let's make it LGPL.

@jdidderen-noviat Do you have some specific inconveniences in mind if we do not make the field stored?

Sep 29 '23 09:09 yostashiro

@jdidderen-noviat Do you have some specific inconveniences in mind if we do not make the field stored?

If I take the example of the account_financial_report:

The analytic_account_ids field is used in search domains (https://github.com/OCA/account-financial-reporting/blob/89144fe180ca526d3382ee7bb159b99e39602c68/account_financial_report/report/general_ledger.py#L278)
In some cases, financial reports search through thousands of accounting entries, and if the values have to be calculated for each record, in my opinion, it's a loss in performance.

Sep 30 '23 18:09 jdidderen-noviat

@jdidderen-noviat Thanks for the pointer. I overlooked a concept used in reporting.

Oct 02 '23 01:10 AungKoKoLin1997

Creating stored fields is challenging in this module. Simply adding the store attribute results in overly long table names, leading to errors. I encountered the following error: odoo.http: Table name 'account_analytic_account_account_analytic_distribution_model_rel' is too long.

To address this, I tried specifying the relation attribute as shown below:

analytic_account_ids = fields.Many2many(
    "account.analytic.account",
    relation="analytic_mixin_account_rel",
    column1="mixin_id",
    column2="account_id",
    compute="_compute_analytic_account_ids",
    string="Analytic Accounts",
    store=True,
    help="Analytic accounts computed from analytic distribution.",
)

However, this introduced another problem. Since several models inherit from analytic.mixin, it results in a conflict, as evidenced by the error: TypeError: Many2many fields account.reconcile.model.line.analytic_account_ids and account.analytic.distribution.model.analytic_account_ids use the same table and columns.

Any suggestions?

Oct 02 '23 04:10 AungKoKoLin1997

A quick workaround I can think of is to shorten the field name to something like analytic_acct_ids which should keep the table name within 63-char limit.

Oct 02 '23 13:10 yostashiro

A quick workaround I can think of is to shorten the field name to something like analytic_acct_ids which should keep the table name within 63-char limit.

@yostashiro Unless I'm mistaken, the field name doesn't influence the generation of the table name for a Many2many relationship in Odoo. The format typically follows the pattern model_1_name_model_2_name_rel.

Oct 02 '23 14:10 AungKoKoLin1997

Oh yes, that's right. Sorry.

Oct 02 '23 14:10 yostashiro

I've chosen to keep non-stored fields in this module for two main reasons:

Technical: Due to the reasons previously mentioned. (We might, however, force stored fields in each inherited model.)
Functionality: To prevent excessive strain on existing records when we use stored fields. For instance, other models depend on analytic.mixin, and we would need to handle all of them using hooks.

Oct 03 '23 02:10 AungKoKoLin1997

@jdidderen-noviat Do you have any suggestions or concern regarding these comments (https://github.com/OCA/account-analytic/pull/565#issuecomment-1742393181, https://github.com/OCA/account-analytic/pull/565#issuecomment-1744047702)

Apr 18 '24 02:04 AungKoKoLin1997

I've chosen to keep non-stored fields in this module for two main reasons:

Technical: Due to the reasons previously mentioned. (We might, however, force stored fields in each inherited model.)

Functionality: To prevent excessive strain on existing records when we use stored fields. For instance, other models depend on analytic.mixin, and we would need to handle all of them using hooks.

I support keeping non-stored fields by default, and force stored field in the inherited model when needed.

Can we merge?

May 28 '24 15:05 norlinhenrik

After merging, please review #630 which depends on this PR.

May 29 '24 15:05 norlinhenrik

@sbidoul Would you like to review?

Jun 10 '24 08:06 norlinhenrik

@OCA/account-analytic-maintainers Could you please review this PR?

Jul 09 '24 09:07 AungKoKoLin1997

What do you think about computing 2 more fields: analytic_plan_ids & analytic_plan_names ? If we can merge this PR, I can open a new PR with this improvement.

Jul 21 '24 18:07 norlinhenrik

On the other hand, It would be great to have it integrated at some point with others modules like this one - https://github.com/OCA/account-financial-reporting/blob/16.0/account_financial_report/models/account_move_line.py - to avoid having several logic in the OCA

I have made #673 as an alternative to this PR, and #1207 for account_financial_report.

Jul 23 '24 08:07 norlinhenrik

[16.0][ADD] analytic_mixin_analytic_account