Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion mis_builder/models/kpimatrix.py
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,12 @@ def _load_account_names(self):
def _get_account_name(self, account):
result = f"{account.code} {account.name}"

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.

image

if self._multi_company:
result = f"{result} [{account.company_id.name}]"
company_names = ", ".join(
[c.name for c in account.company_ids[:3]]
) # Limit to first 3 companies
if len(account.company_ids) > 3:
company_names += ", ..."
result = f"{result} [{company_names}]"
Copy link
Member

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.

Copy link
Contributor

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?

Copy link
Member

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.

Copy link
Contributor

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.

Copy link
Member

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.

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:

image

return result

def get_account_name(self, account_id):
Expand Down
1 change: 1 addition & 0 deletions mis_builder/tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from . import test_aggregate
from . import test_data_sources
from . import test_kpi_data
from . import test_kpimatrix_account_names
from . import test_mis_report_instance
from . import test_mis_safe_eval
from . import test_period_dates
Expand Down
126 changes: 126 additions & 0 deletions mis_builder/tests/test_kpimatrix_account_names.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
# Copyright 2025 Geraldo Lopez
# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl.html).

from unittest.mock import MagicMock, Mock

from odoo.tests import common

from ..models.kpimatrix import KpiMatrix


class TestKPIMatrixAccountNames(common.TransactionCase):
"""Unit tests for KpiMatrix._get_account_name() method enhancements"""

@classmethod
def setUpClass(cls):
super().setUpClass()

# Create a proper mock environment for KpiMatrix
mock_env = MagicMock()

# Mock the required models and services
mock_lang_model = Mock()
mock_lang_model._lang_get.return_value = Mock()
mock_env.__getitem__.side_effect = lambda key: {
"res.lang": mock_lang_model,
"mis.report.style": Mock(),
"account.account": Mock(),
}.get(key, Mock())

# Mock user with language
mock_env.user.lang = "en_US"

# Create KpiMatrix with mock environment
cls.kpi_matrix = KpiMatrix(mock_env, multi_company=True)

def _create_mock_account(
self,
code="100",
name="Test Account",
company_ids=None,
):
"""Helper to create mock account objects"""
account = Mock()
account.code = code
account.name = name

account.company_ids = company_ids or []

return account

def _create_mock_company(self, name):
"""Helper to create mock company objects"""
company = Mock()
company.name = name
return company

def test_get_account_name_single_company_mode(self):
"""Test account name without company info in single company mode"""
# Create a separate KpiMatrix for single company mode
mock_env = MagicMock()
mock_lang_model = Mock()
mock_lang_model._lang_get.return_value = Mock()
mock_env.__getitem__.side_effect = lambda key: {
"res.lang": mock_lang_model,
"mis.report.style": Mock(),
"account.account": Mock(),
}.get(key, Mock())
mock_env.user.lang = "en_US"

single_company_matrix = KpiMatrix(mock_env, multi_company=False)

account = self._create_mock_account(code="100", name="Cash")

result = single_company_matrix._get_account_name(account)

self.assertEqual(result, "100 Cash")

def test_get_account_name_with_company_ids_single(self):
"""Test account name with company_ids field containing one company"""
company = self._create_mock_company("Company B")
account = self._create_mock_account(
code="200",
name="Bank",
company_ids=[company],
)

result = self.kpi_matrix._get_account_name(account)
self.assertEqual(result, "200 Bank [Company B]")

def test_get_account_name_with_company_ids_multiple(self):
"""Test account name with company_ids field containing multiple
companies (≤3)"""
companies = [
self._create_mock_company("Company A"),
self._create_mock_company("Company B"),
self._create_mock_company("Company C"),
]

account = self._create_mock_account(
code="300",
name="Receivables",
company_ids=companies,
)

result = self.kpi_matrix._get_account_name(account)
self.assertEqual(result, "300 Receivables [Company A, Company B, Company C]")

def test_get_account_name_with_company_ids_many(self):
"""Test account name with company_ids field containing >3 companies
(should truncate)"""
companies = [
self._create_mock_company("Company A"),
self._create_mock_company("Company B"),
self._create_mock_company("Company C"),
self._create_mock_company("Company D"),
self._create_mock_company("Company E"),
]

account = self._create_mock_account(
code="400",
name="Payables",
company_ids=companies,
)

result = self.kpi_matrix._get_account_name(account)
self.assertEqual(result, "400 Payables [Company A, Company B, Company C, ...]")
Loading