Skip to content

fix(spp_programs): locale-aware total_amount_in_words (#236) - #300

Open
Tarekchehahde wants to merge 1 commit into
OpenSPP:19.0from
Tarekchehahde:fix/236-total-amount-in-words-locale
Open

fix(spp_programs): locale-aware total_amount_in_words (#236)#300
Tarekchehahde wants to merge 1 commit into
OpenSPP:19.0from
Tarekchehahde:fix/236-total-amount-in-words-locale

Conversation

@Tarekchehahde

Copy link
Copy Markdown
Contributor

Summary

  • Use env.context['lang'] or env.user.lang (with en fallback) for num2words in _compute_total_amount_in_words.
  • Extend test_total_amount_in_words to assert French output under fr_FR context.

Test plan

  • ./spp test spp_programstest_total_amount_in_words
  • Manual: open a cycle form in fr_FR, confirm amount-in-words renders in French

Fixes #236

Made with Cursor

…P#236)

Derive num2words language from env context or user lang instead of
hardcoding English; fall back to en for unsupported locales.

Co-authored-by: Cursor <cursoragent@cursor.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces localization support for converting total amounts into words using the num2words library, falling back to English if the requested language is not supported, and adds a corresponding test case for French. The reviewer suggested improving the language resolution logic to try the full language code (preserving regional dialects like pt_BR) before falling back to the base language code and finally English.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +263 to 268
lang_code = (self.env.context.get("lang") or self.env.user.lang or "en_US").split("_")[0]
try:
amount_in_words = num2words(record.total_amount, lang=lang_code).title()
except NotImplementedError:
amount_in_words = num2words(record.total_amount, lang="en").title()
record.total_amount_in_words = f"{amount_in_words} {record.currency_id.name}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

By immediately splitting the language code on _ (e.g., converting pt_BR to pt), regional dialects supported by num2words (such as Brazilian Portuguese pt_BR or British English en_GB) are lost, falling back to the generic base language.

A more robust approach is to attempt translation using the full language code first, then fall back to the base language code (split by _), and finally fall back to English (en) if neither is supported.

Suggested change
lang_code = (self.env.context.get("lang") or self.env.user.lang or "en_US").split("_")[0]
try:
amount_in_words = num2words(record.total_amount, lang=lang_code).title()
except NotImplementedError:
amount_in_words = num2words(record.total_amount, lang="en").title()
record.total_amount_in_words = f"{amount_in_words} {record.currency_id.name}"
lang_code = self.env.context.get("lang") or self.env.user.lang or "en_US"
for lang in (lang_code, lang_code.split("_")[0], "en"):
try:
amount_in_words = num2words(record.total_amount, lang=lang).title()
break
except NotImplementedError:
continue
record.total_amount_in_words = f"{amount_in_words} {record.currency_id.name}"

@gonzalesedwin1123 gonzalesedwin1123 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for this — the root-cause analysis in #236 is spot-on, and you patched the right file despite the issue linking to cycle_base.py (which turns out to be dead code — it's never imported in models/__init__.py; the live model is in cycle.py). The fallback chain and the NotImplementedError guard are both reasonable. Two must-fixes before this can merge, plus a few suggestions.

Must-fix 1: add @api.depends_context("lang")

total_amount_in_words is a non-stored computed field. Odoo caches non-stored computes per record per transaction, and the cache only varies by context keys declared via @api.depends_context. Without it, the first language computed in a transaction wins: read the field in English, then read cycle.with_context(lang="fr_FR").total_amount_in_words in the same transaction, and you get the cached English string back with no recompute. That hits exactly the flows #236 cares about — e.g. the QWeb summary report (report/summary_report.xml renders doc.total_amount_in_words) when rendered under a partner's language after the record was already read in another language.

@api.depends("total_amount", "currency_id")
@api.depends_context("lang")
def _compute_total_amount_in_words(self):

The current test doesn't catch this because it calls the private _compute_total_amount_in_words() directly, which force-overwrites the cache — it validates the compute body, not the field's behavior. Please restructure the test to read the field naturally: read cycle.total_amount_in_words (English) first, then read cycle.with_context(lang="fr_FR").total_amount_in_words on the same record in the same transaction. That ordering fails without the decorator and passes with it, so it pins the actual bug.

Must-fix 2: version bump + HISTORY entry

spp_programs is a released module (currently 19.0.2.2.1 on 19.0). Please bump the micro version in __manifest__.py (to whatever is next when this merges — 19.0.2.2.2 as of today) and add an entry to spp_programs/readme/HISTORY.md.

Suggestions (non-blocking)

  1. Drop the .split("_")[0]num2words already tries the full lang code first and then falls back to the first 2 letters before raising NotImplementedError. Passing the full locale is simpler and preserves the region-specific converters num2words ships (fr_CH, fr_BE, es_CO, …). Your existing try/except NotImplementedError already covers truly unsupported codes.
  2. Hoist the lang lookup out of the for record in self loop — it's the same value every iteration.
  3. Add a fallback test with an unsupported locale (e.g. with_context(lang="xx_XX")) asserting the English output, so the except branch is covered.

CI note

No workflows ever ran on this PR (fork PR awaiting first-contributor approval at open time, and no pushes since). Your next push should trigger CI automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(spp_programs): spp.cycle.total_amount_in_words renders amount in English regardless of user/record language

2 participants