Skip to content

[18.0][FIX] account_financial_report: journal ledger move_line_ids_taxes_data #1418

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.

Sign up for GitHub

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

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

[18.0][FIX] account_financial_report: journal ledger move_line_ids_taxes_data #1418

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 8 additions & 2 deletions account_financial_report/report/journal_ledger.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ def _get_move_lines(self, move_ids, wizard, journal_ids):
)
move_line_ids_taxes_data = {}
if move_lines:
lang = self.env.context.get("lang", "en_US")
# Get the taxes ids for the move lines
query_taxes_params = self._get_query_taxes_params(move_lines)
query_taxes = self._get_query_taxes()
Expand All @@ -217,8 +218,12 @@ def _get_move_lines(self, move_ids, wizard, journal_ids):
if move_line_id not in move_line_ids_taxes_data.keys():
move_line_ids_taxes_data[move_line_id] = {}
move_line_ids_taxes_data[move_line_id][account_tax_id] = {
"name": tax_name,
"description": tax_description,
"name": tax_name.get(lang) or tax_name.get("en_US"),
Copy link
Member

@flotho flotho Dec 19, 2025

Choose a reason for hiding this comment

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

Hi,
Thanks for this usefull PR. Code could be optimized. You defined L206 the lang variable. Its value is either en_US or another one, never None AFAICS. Though, you don't need anymore to switch case on lang in L221 and L223 Could you change this ?
Regards

Hi, thanks for your review.

The thing is you don't necessarily have a translation for your current language. If you look in DB, even though a language is installed, translatable fields will have no other key than "en_US" until they are actually translated once.

Thanks for the answer.
I thought I've understood what you want to do.
What I'm proposing is to change this :
"name": tax_name.get(lang) or tax_name.get("en_US"),
By
"name": tax_name.get(lang),
You can do this because you defined a lang variable with a default value that will be en_US. So the test you made is redundant regarding the possible values in the lang variable.
Am I missing something?
regards

Copy link
Contributor Author

@qgroulard qgroulard Dec 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Let's say I have French and English installed on my DB and my current language is French.

  • lang = "fr_FR"
  • tax_name might be: {"en_US": "Tax 21%"}

Hence, tax_name.get(lang) will return False, which will eventually end up as an empty column in the report.

What I have implemented is a fallback on English translation. This is similar to what the ORM does, if you open the form view of the tax the name won't be empty if you have no French translation, the English name will be displayed instead.

"description": (
tax_description.get(lang) or tax_description.get("en_US")
if tax_description
else False
),
}
Move_Lines = {}
auto_sequence = len(move_ids)
Expand Down Expand Up @@ -325,6 +330,7 @@ def _get_report_values(self, docids, data):
partner_ids_data = move_lines[3]
currency_ids_data = move_lines[4]
tax_line_ids_data = move_lines[5]
move_line_ids_taxes_data = move_lines[6]
for move_data in moves_data:
move_id = move_data["move_id"]
move_data["report_move_lines"] = []
Expand Down