Skip to content
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

[16][FIX] account_financial_report : make aged report configuration working with multi-company #1246

Open
wants to merge 1 commit into
base: 16.0
Choose a base branch
from

Conversation

florian-dacosta
Copy link
Contributor

@florian-dacosta florian-dacosta commented Oct 30, 2024

The field default_age_partner_config_id configuration (in res.config.settings) does not work in multi-company environment.
Indeed, the default_XXX config create a global (for all companies) ir.default, meaning, it is the same setting for all companies.
If you change this configuration for company A, and then go to the settings of company B, you can see it has been update too.

The issue here is that the account.age.report.configuration model has a company_id field, always filled. And there are multi-company security rule.
So if the account.age.report.configuration belongs to company A, and you try to generate a aged balance report for company B, with this configuration, it will fail because of multi-company access issues.

I changed this config field to use instead the specific config settings (overriding the get_values/set_values). This way, this configuration becomes company dependent instead of beeing global and it solves the multi-company issue when generating aged balance report.

review welcome @metaminux @emiliesoutiras @Kev-Roche

I will forward port it to 17 when merged

@pedrobaeza pedrobaeza added this to the 16.0 milestone Oct 30, 2024
@pedrobaeza
Copy link
Member

AFAIK, defaults can be applied by company, and that was the initial intention. If that's not working, then it's better to simply fix it to load/store the default for the current company.

@florian-dacosta
Copy link
Contributor Author

AFAIK, defaults can be applied by company, and that was the initial intention.

As far as I have tested, none of the default_XXX fields from res.config.wizard are applied by company.
The ir.default model allow to have a default by company, but the mechanism from res.config.settings that creates these ir.default does not manage the company.
Looking here https://github.com/odoo/odoo/blob/16.0/odoo/addons/base/models/res_config.py#L544
We see, there are no company, so it is always generating the default globally.
You can easily test with native default config like "Invoicing Policy" (sale => configuration => settings) This one is a selection field and is applied globally (for all companies).

Also, looking to the description of the feature by Odoo, it seems to be wanted : https://github.com/odoo/odoo/blob/16.0/odoo/addons/base/models/res_config.py#L340 (or maybe I misinterpret the "global" meaning of this description)

it's better to simply fix it to load/store the default for the current company.

I am opened to change the way to fix this issue, but I don't understand exactly what you mean. I can't really fix/change the way the default_xxx are managed in res.config.settings Odoo's framework in this module that is why I am going for an other way to do it. I don't see a simpler way.

@pedrobaeza
Copy link
Member

What I suggest is to override get_values (https://github.com/odoo/odoo/blob/21d4f951b6ca4722fe75a30c0dea106fb393aace/odoo/addons/base/models/res_config.py#L461) and set_values (https://github.com/odoo/odoo/blob/21d4f951b6ca4722fe75a30c0dea106fb393aace/odoo/addons/base/models/res_config.py#L525) to read/store the ir.default company-aware. You have plenty of examples in Odoo itself.

@florian-dacosta
Copy link
Contributor Author

Well, sure, I can do it this way, I don't get how it is simpler though...

@pedrobaeza
Copy link
Member

The idea is for not overloading the res.company model, and also for not requiring specific code for loading that default value on the wizard.

@florian-dacosta florian-dacosta force-pushed the 16-fix-multi-company-aged-config branch from 5853bec to 798e5b0 Compare October 31, 2024 10:20
@florian-dacosta
Copy link
Contributor Author

Here it is @pedrobaeza

@florian-dacosta florian-dacosta force-pushed the 16-fix-multi-company-aged-config branch from 798e5b0 to 997959f Compare October 31, 2024 10:22
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants