-
-
Notifications
You must be signed in to change notification settings - Fork 347
[18.0][FIX] mis_builder: account.account has company_ids in Odoo 18 #730
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
base: 18.0
Are you sure you want to change the base?
[18.0][FIX] mis_builder: account.account has company_ids in Odoo 18 #730
Conversation
|
Hi @sbidoul, |
b465065 to
d9ad820
Compare
| ) # Limit to first 3 companies | ||
| if len(account.company_ids) > 3: | ||
| company_names += ", ..." | ||
| result = f"{result} [{company_names}]" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @StefanRijnhart ! This should fix the bug indeed.
Functionally though I'm not sure.
I'd say, we at least need to show only companies that are in the report query_company_ids.
That should be relatively easy to do by passing this list of companies when constructing the KpiMatrix, next to or in place of the multi_company flag.
But, I wonder if it makes sense functionally to show, say, debit/credit by account with multiple companies on the same line.
Thinking even further I wonder if it is technically possible to have a KpiMatrixRow for an account detail that concern multiple companies, or if we rather always have one row per (account, company). In that case, the company must be a property of the Row.
I'll try to do a few tests to better (re)understand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @sbidoul, have you been able to do the tests you mentioned?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not yet. In the meantime any functional feedback of users testing this PR in various circumstances is welcome.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recently I have used this PR to compare customer data between v15 and v18 in multi-company MIS reports, and the results seem good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, thanks. I'll need to find answers to my questions above, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello, thank you very much for your work.
I see a problem in how Odoo does things in this version, which is that the company to be represented must be selected in the context, otherwise account.code returns False and that is what is displayed on the screen, so it would never enter in if len(account.company_ids) > 3:
I upload screenshot with my local whit this changes:
|
Hello @StefanRijnhart, can do a rebase to test this change? |
Limit display to 3 companies plus ellipsis.
Fixes
```
File "/home/odoo/mis-builder/mis_builder/models/kpimatrix.py", line 478, in _get_account_name
result = f"{result} [{account.company_id.name}]"
^^^^^^^^^^^^^^^^^^
AttributeError: 'account.account' object has no attribute 'company_id'
```
Co-authored-by: Stefan Rijnhart <[email protected]>
d9ad820 to
c1bb8bf
Compare
|
@Xino61122 done. |
| ) # Limit to first 3 companies | ||
| if len(account.company_ids) > 3: | ||
| company_names += ", ..." | ||
| result = f"{result} [{company_names}]" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello, thank you very much for your work.
I see a problem in how Odoo does things in this version, which is that the company to be represented must be selected in the context, otherwise account.code returns False and that is what is displayed on the screen, so it would never enter in if len(account.company_ids) > 3:
I upload screenshot with my local whit this changes:
| self._account_names = {a.id: self._get_account_name(a) for a in accounts} | ||
|
|
||
| def _get_account_name(self, account): | ||
| result = f"{account.code} {account.name}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To ensure that False is not entered as the code, we can add the following to the code
if not account.code:
account = account.with_company(account.company_ids[0])
This way, it takes the company code from the current account. The problem is that companies with the same accounting account still do not appear on the same line, but it is a possible solution.
This is how it would look with the change.
|
Closing in favour of #756 |
Adapted from #711, but observing that in Odoo 18, there is no
company_idfield anymore, and thecompany_idsfield is required, thus removing several test cases.Limit display to 3 companies plus ellipsis.
Fixes