T3206-Correct-existing-translation-data by jordyBSK · Pull Request #2098 · CompassionCH/compassion-modules

jordyBSK · 2026-06-10T09:34:54Z

refactor: count only completed translations in stats

Update nb_translated_letters to filter on translation_status == "done"
Replace translate_date usage with translate_done for yearly stats
Add precise @api.depends on translation_status and translate_done
Ensure counters reflect only completed translations

- Update nb_translated_letters to filter on translation_status == "done" - Replace translate_date usage with translate_done for yearly stats - Add precise @api.depends on translation_status and translate_done - Ensure counters reflect only completed translations

gemini-code-assist

Code Review

This pull request updates the computed fields tracking translated letters to filter by status and date. The reviewer suggests optimizing these computations by replacing the in-memory Python filtering on the One2many field with database-level aggregations using read_group to prevent performance degradation and high memory usage.

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.

gemini-code-assist · 2026-06-10T09:36:14Z

+    @api.depends("translated_letter_ids.translation_status")
    def _compute_nb_translated_letters(self):
        for translator in self:
-            translator.nb_translated_letters = len(translator.translated_letter_ids)
+            translator.nb_translated_letters = len(
+                translator.translated_letter_ids.filtered(
+                    lambda it: it.translation_status == "done"
+                )
+            )


Using filtered on a One2many field (translated_letter_ids) loads all associated correspondence records into memory. For translators with a large history of letters, this can cause significant performance degradation and high memory usage. Since this is a stored computed field, it will be recomputed frequently. Using read_group allows Odoo to perform a single optimized SQL aggregation query, which is much faster and avoids loading any records into memory.

@api.depends("translated_letter_ids.translation_status") def _compute_nb_translated_letters(self): groups = self.env["correspondence"].read_group( [("new_translator_id", "in", self.ids), ("translation_status", "=", "done")], ["new_translator_id"], ["new_translator_id"] ) mapped_data = { g["new_translator_id"][0]: g["new_translator_id_count"] for g in groups if g["new_translator_id"] } for translator in self: translator.nb_translated_letters = mapped_data.get(translator.id, 0)

gemini-code-assist · 2026-06-10T09:36:14Z

+    @api.depends("translated_letter_ids.translation_status", "translated_letter_ids.translate_done")
    def _compute_nb_translated_letters_this_year(self):
+        current_year = fields.Datetime.now().year
        for translator in self:
            translator.nb_translated_letters_this_year = len(
                translator.translated_letter_ids.filtered(
-                    lambda it: it.translate_date
-                    and it.translate_date.year == fields.Datetime.now().year
+                    lambda it: it.translation_status == "done" and
+                               it.translate_done and
+                               it.translate_done.year == current_year
                )
            )


Filtering the One2many field in Python to count letters for the current year loads all correspondence records into memory. We can optimize this by using read_group with a date range filter on translate_done directly in the database query, which avoids loading records and executes a highly efficient database-level aggregation.

@api.depends("translated_letter_ids.translation_status", "translated_letter_ids.translate_done") def _compute_nb_translated_letters_this_year(self): current_year = fields.Datetime.now().year groups = self.env["correspondence"].read_group( [ ("new_translator_id", "in", self.ids), ("translation_status", "=", "done"), ("translate_done", ">=", f"{current_year}-01-01 00:00:00"), ("translate_done", "<=", f"{current_year}-12-31 23:59:59"), ], ["new_translator_id"], ["new_translator_id"] ) mapped_data = { g["new_translator_id"][0]: g["new_translator_id_count"] for g in groups if g["new_translator_id"] } for translator in self: translator.nb_translated_letters_this_year = mapped_data.get(translator.id, 0)

gemini-code-assist · 2026-06-10T09:36:14Z

+    @api.depends("translated_letter_ids.translation_status", "translated_letter_ids.translate_done")
    def _compute_nb_translated_letters_last_year(self):
+        last_year = fields.Datetime.now().year - 1
        for translator in self:
            translator.nb_translated_letters_last_year = len(
                translator.translated_letter_ids.filtered(
-                    lambda it: it.translate_date
-                    and it.translate_date.year == fields.Datetime.now().year - 1
+                    lambda it: it.translation_status == "done" and
+                               it.translate_done and
+                               it.translate_done.year == last_year
                )
            )


Similar to the current year computation, filtering the One2many field in Python to count letters for the last year loads all correspondence records into memory. Using read_group with a date range filter on translate_done directly in the database query avoids loading records and executes a highly efficient database-level aggregation.

Suggested change

@api.depends("translated_letter_ids.translation_status", "translated_letter_ids.translate_done")

def _compute_nb_translated_letters_last_year(self):

last_year = fields.Datetime.now().year - 1

for translator in self:

translator.nb_translated_letters_last_year = len(

translator.translated_letter_ids.filtered(

lambda it: it.translate_date

and it.translate_date.year == fields.Datetime.now().year - 1

lambda it: it.translation_status == "done" and

it.translate_done and

it.translate_done.year == last_year

)

)

@api.depends("translated_letter_ids.translation_status", "translated_letter_ids.translate_done")

def _compute_nb_translated_letters_last_year(self):

last_year = fields.Datetime.now().year - 1

groups = self.env["correspondence"].read_group(

[

("new_translator_id", "in", self.ids),

("translation_status", "=", "done"),

("translate_done", ">=", f"{last_year}-01-01 00:00:00"),

("translate_done", "<=", f"{last_year}-12-31 23:59:59"),

],

["new_translator_id"],

["new_translator_id"]

)

mapped_data = {

g["new_translator_id"][0]: g["new_translator_id_count"]

for g in groups

if g["new_translator_id"]

}

for translator in self:

translator.nb_translated_letters_last_year = mapped_data.get(translator.id, 0)

greptile-apps · 2026-06-10T09:38:07Z

Confidence Score: 3/5

The translation statistics logic is narrowed correctly for normal writes, but the migration path can leave stored counters stale after upgrade.

The changed compute logic counts completed translations and uses completion dates as intended during ordinary record updates.
The migration queues recomputation for existing stored counters but does not drain the queued work before returning, so upgraded databases may keep inflated values until a later write triggers recomputation.
The affected behavior is limited to existing persisted statistics during upgrade, not the regular compute rules for newly changed records.

sbc_translation/migrations/18.0.1.1.0/post-migration.py

T-Rex Logs

What T-Rex did

Compared pre-change and post-change counters for the fixture and confirmed the post-change totals align with the expected 4 total, 2 this-year, and 1 last-year; also verified that writes to translation_status and translate_done are covered by the declared @api.depends fields.
Inspected the queued-counters logs to verify migration behavior: before-change counters were stored as 3 total, 2 this-year, 1 last-year with queued_recompute_items of 3, and after recomputing the totals they became 2 total, 1 this-year, 1 last-year; after-change draining leaves queued_recompute_items at 0 with corrected stored values.

_{Ran code and verified through T-Rex}

Comments Outside Diff (1)

General comment

Queued Counters Stay Stale
- Bug
  - The post-migration hook queues recomputation of stored translation.user statistic fields but only calls flush_model(). The executed harness shows that after migrate() returns, stale stored values remain visible even though recompute items were queued. Dashboard/statistics consumers reading these stored fields can therefore continue to see inflated pre-change counters until another operation drains or triggers recomputation.
- Cause
  - env.add_to_compute() only schedules stored-field recomputation. translators.flush_model([...]) flushes pending cached/database writes for those fields but does not drain the recompute queue, so the queued compute work is not applied during the migration.
- Fix
  - After queuing the fields, explicitly drain Odoo's recompute queue for the affected records/fields using the appropriate Odoo 18 API, then flush/commit as needed. For example, replace the final flush_model()-only step with the Odoo-version-appropriate recompute drain such as _recompute_recordset()/environment recompute processing for translation.user, followed by flushing the recomputed fields.
_{Ran code and verified through T-Rex}

_{Reviews (5): Last reviewed commit: "refactor: refactor the code" | Re-trigger Greptile}

NoeBerdoz

Thanks for your work on this! The refactor direction looks great, however, there is a key issue to fix before we can merge your proposal.

You changed the compute method, but not the existing data in the database:
Because these fields have store=True, Odoo will not automatically recompute the old rows during an upgrade. It will just keep reading the old, incorrect values. To fix this and meet the ticket requirements, we need a migration script to backfill the data and force a recompute for existing records.

Let me know if you need help setting up the migration script!

NoeBerdoz · 2026-06-15T14:04:29Z

+    @api.depends(
+        "translated_letter_ids.translation_status",
+        "translated_letter_ids.translate_done",
+    )
+    @api.depends(
+        "translated_letter_ids.translation_status",
+        "translated_letter_ids.translate_done",
+    )


Looks like a code duplication to remove

Suggested change

@api.depends(

"translated_letter_ids.translation_status",

"translated_letter_ids.translate_done",

)

@api.depends(

"translated_letter_ids.translation_status",

"translated_letter_ids.translate_done",

)

@api.depends(

"translated_letter_ids.translation_status",

"translated_letter_ids.translate_done",

)

…nslated by the translators

greptile-apps · 2026-06-25T14:36:37Z

+    translators.flush_model([
+        "nb_translated_letters",
+        "nb_translated_letters_this_year",
+        "nb_translated_letters_last_year"
+    ])


Queued Counters Stay Stale

When this migration runs, env.add_to_compute() only queues these stored fields for recomputation; flush_model() flushes cached values but does not drain the recompute queue. Existing translation.user rows can keep their old inflated counters after upgrade, so the dashboard API continues to return the pre-change totals until another write happens to trigger a recompute.

gemini-code-assist Bot reviewed Jun 10, 2026

View reviewed changes

jordyBSK added 2 commits June 10, 2026 11:47

fix: fix the file translation_user.py with the pre-commit

db9499d

refactor: added changes recommended by gemini code

1c46838

greptile-apps Bot reviewed Jun 12, 2026

View reviewed changes

Comment thread sbc_translation/models/translation_user.py

Comment thread sbc_translation/models/translation_user.py

refactor: fix the file with pre-commit

ccbaa04

NoeBerdoz requested changes Jun 15, 2026

View reviewed changes

jordyBSK added 2 commits June 25, 2026 16:20

feat: Added a migration file to recalculate the number of letters tra…

ac0c91a

…nslated by the translators

refactor: refactor the code

098d5e7

greptile-apps Bot reviewed Jun 25, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

T3206-Correct-existing-translation-data#2098

T3206-Correct-existing-translation-data#2098
jordyBSK wants to merge 6 commits into
18.0from
T3206-Correct-existing-translation-data

jordyBSK commented Jun 10, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Jun 10, 2026

Uh oh!

gemini-code-assist Bot Jun 10, 2026

Uh oh!

gemini-code-assist Bot Jun 10, 2026

Uh oh!

greptile-apps Bot commented Jun 10, 2026 •

edited

Loading

T-Rex Logs

Comments Outside Diff (1)

Uh oh!

Uh oh!

Uh oh!

NoeBerdoz left a comment

Uh oh!

NoeBerdoz Jun 15, 2026

Uh oh!

Uh oh!

greptile-apps Bot Jun 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

jordyBSK commented Jun 10, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 3/5

T-Rex Logs

Comments Outside Diff (1)

Uh oh!

Uh oh!

Uh oh!

NoeBerdoz left a comment

Choose a reason for hiding this comment

Uh oh!

NoeBerdoz Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

greptile-apps Bot commented Jun 10, 2026 •

edited

Loading