Repository: OCA/odoo-pre-commit-hooks

Issue:
False Positive: "Duplicate PO message definition" for Contextually Different Translations
## Module
hook id: oca-checks-po

## Describe the bug
The Odoo pre-commit hook for .po[t] files incorrectly flags valid translations as duplicates when they have different msgstr values.

## To Reproduce

Create a .po file with multiple translations for the same msgid, each with different msgstr values.

e.g : 
  # Translation 1
  msgid "Submitted"
  msgstr "Soumis"

  # Translation 2
  msgid "Submitted"
  msgstr "Soumise"

**Expected behavior**
The pre-commit hook should not flag different msgstr values for the same msgid as duplicates when they represent valid translations in different contexts.

Discussion:
@luisg123v: I think the described behavior is expected, because Odoo doesn't support providing different translations for the same term. In fact, if the translation file is re-exported, those will be lost and a single one will be included.@moylop260: @Ahmed-Maghraoui-Eezee-It 

Could you export the PO file in order to double-confirm that Luis said, please?@Ahmed-Maghraoui-Eezee-It: @moylop260 @luisg123v 
it's not the same term, the first translation is related to a view in the current custom module, and the second is related to a selection field in a standard module.
The translations are working fine and when I export the po file for each module I get different translations as expected.@moylop260: Could you share the po file and the odoo version, please?

@Ahmed-Maghraoui-Eezee-It: odoo version 18

[fr_BE.po.txt](https://github.com/user-attachments/files/18604663/fr_BE.po.txt)@moylop260: I have created a video with the following steps:
1. Export po file for Spanish for 2 modules (e.g. sale_management, account)
2. Open the po file and split the "active" msgid in 2 msgstr for different modules similar your example

```txt
...

-- ORIGINAL
#. modules: account, sale_management
#: model:ir.model.fields,field_description:account.field_account_account_tag__active
#: model:ir.model.fields,field_description:sale_management.field_sale_order_template__active
#: model:ir.model.fields.selection,name:account.selection__account_lock_exception__state__active
#: model_terms:ir.ui.view,arch_db:account.view_account_tax_search
msgid "Active"
msgstr "Activo"

...
```

```txt
-- EDITED
#. modules: account
#: model:ir.model.fields,field_description:account.field_account_account_tag__active
#: model:ir.model.fields.selection,name:account.selection__account_lock_exception__state__active
#: model_terms:ir.ui.view,arch_db:account.view_account_tax_search
msgid "Active"
msgstr "Activo1"

#. modules: sale_management
#: model:ir.model.fields,field_description:sale_management.field_sale_order_template__active
msgid "Active"
msgstr "Activo2"
```

The diff

```diff
--- original
+++ edited
@@ -1,11 +1,11 @@
-...
-
-#. modules: account, sale_management
+#. modules: account
 #: model:ir.model.fields,field_description:account.field_account_account_tag__active
-#: model:ir.model.fields,field_description:sale_management.field_sale_order_template__active
 #: model:ir.model.fields.selection,name:account.selection__account_lock_exception__state__active
 #: model_terms:ir.ui.view,arch_db:account.view_account_tax_search
 msgid "Active"
-msgstr "Activo"
+msgstr "Activo1"

-...
+#. modules: sale_management
+#: model:ir.model.fields,field_description:sale_management.field_sale_order_template__active
+msgid "Active"
+msgstr "Activo2"
```

3. Re-import the file modified and check the changes were applied
4. Repeat step 1 (re-export in the same way)
5. Check the msgid was grouped and the second term was deleted

The new PO file looks like the original one

```txt
...

#. modules: account, sale_management
#: model:ir.model.fields,field_description:account.field_account_account_tag__active
#: model:ir.model.fields,field_description:sale_management.field_sale_order_template__active
#: model:ir.model.fields.selection,name:account.selection__account_lock_exception__state__active
#: model_terms:ir.ui.view,arch_db:account.view_account_tax_search
msgid "Active"
msgstr "Activo1"

...
```

See the video:
 - [![IMAGE ALT TEXT HERE](https://img.youtube.com/vi/PUkWmg8bPkk/0.jpg)](https://www.youtube.com/watch?v=PUkWmg8bPkk)

Notice this is the purpose of the check, so it is a valid case.

Also, notice you are using a po file for 2 modules, so maybe it should be used in `MODULE/i18n_extra/fr_be.po` instead of `MODULE/i18n/fr_be.po` where you can maintain manually the po file without exporting and we can disable this check for this extra folder (It needs a change)

@luisg123v: @Ahmed-Maghraoui-Eezee-It,

> it's not the same term, the first translation is related to a view in the current custom module, and the second is related to a selection field in a standard module.

I think this is the source of the issue. Your file `i18n/fr_BE.po` is not intended to contain translations from other modules, only for the same module. When you re-export translations, you should only include translations from the module they're contained on. For translating other modules, you should use the `i18n_extra` folder.

Maybe we can disable the check for `i18n_extra` translations, as suggested by @moylop260, but I think you wouldn't need that change, unless you need to translate the same term of several other modules different from the custom one.@moylop260: Please, review:
https://github.com/OCA/odoo-pre-commit-hooks/pull/123

I have added a better message but I think it could be even better

Could you check it, please?@Ahmed-Maghraoui-Eezee-It: Thank you so much @moylop260 @luisg123v 

Turns out adding the translations to the i18n_extra was exactly what I needed.
Sorry for the hassle, but I really appreciate your help!
@moylop260: @Ahmed-Maghraoui-Eezee-It 

Thank you for reporting the issue

IMHO it is a valid case

I only needed to understand it very well in order to know what was the best fix for that

It will be released in v0.1.1


Solution patch:
diff --git a/README.md b/README.md
index 5b71968..6ceb770 100644
--- a/README.md
+++ b/README.md
@@ -418,9 +418,9 @@ options:
 
  * po-duplicate-message-definition
 
-    - https://github.com/OCA/odoo-pre-commit-hooks/blob/v0.1.0/test_repo/broken_module/i18n/es.po#L17 Duplicate PO message definition `Branch` in lines 23, 29
-    - https://github.com/OCA/odoo-pre-commit-hooks/blob/v0.1.0/test_repo/broken_module/i18n/es.po#L35 Duplicate PO message definition `Message id toooooooooooooooooooooooooooo...` in lines 41
-    - https://github.com/OCA/odoo-pre-commit-hooks/blob/v0.1.0/test_repo/broken_module/i18n/es.po#L65 Duplicate PO message definition `One variable {variable1}` in lines 71
+    - https://github.com/OCA/odoo-pre-commit-hooks/blob/v0.1.0/test_repo/broken_module/i18n/es.po#L17 Duplicate PO message definition `Branch` in lines 23, 29. Odoo exports these items by msgid and delete one of them. Use the `i18n_extra` folder instead of `i18n` to ignore this message.
+    - https://github.com/OCA/odoo-pre-commit-hooks/blob/v0.1.0/test_repo/broken_module/i18n/es.po#L35 Duplicate PO message definition `Message id toooooooooooooooooooooooooooo...` in lines 41. Odoo exports these items by msgid and delete one of them. Use the `i18n_extra` folder instead of `i18n` to ignore this message.
+    - https://github.com/OCA/odoo-pre-commit-hooks/blob/v0.1.0/test_repo/broken_module/i18n/es.po#L65 Duplicate PO message definition `One variable {variable1}` in lines 71. Odoo exports these items by msgid and delete one of them. Use the `i18n_extra` folder instead of `i18n` to ignore this message.
 
  * po-duplicate-model-definition
 
diff --git a/src/oca_pre_commit_hooks/checks_odoo_module_po.py b/src/oca_pre_commit_hooks/checks_odoo_module_po.py
index 305ebcb..4c2ec9e 100644
--- a/src/oca_pre_commit_hooks/checks_odoo_module_po.py
+++ b/src/oca_pre_commit_hooks/checks_odoo_module_po.py
@@ -330,8 +330,8 @@ class ChecksOdooModulePO(BaseChecker):
             if entry.obsolete:
                 continue
 
-            # po_duplicate_message_definition
-            if self.is_message_enabled("po-duplicate-message-definition"):
+            # Ignore i18n_extra https://github.com/OCA/odoo-pre-commit-hooks/issues/122
+            if self.is_message_enabled("po-duplicate-message-definition") and self.data_section != "i18n_extra":
                 duplicated[hash(entry.msgid)].append(entry)
 
             if self.is_message_enabled("po-duplicate-model-definition"):
@@ -351,7 +351,9 @@ class ChecksOdooModulePO(BaseChecker):
                     msg_id_short = f"{msg_id_short}..."
                 self.register_error(
                     code="po-duplicate-message-definition",
-                    message=f"Duplicate PO message definition `{msg_id_short}` in lines {duplicated_str}",
+                    message=f"Duplicate PO message definition `{msg_id_short}` in lines {duplicated_str}. "
+                    "Odoo exports these items by msgid and delete one of them. "
+                    "Use the `i18n_extra` folder instead of `i18n` to ignore this message.",
                     filepath=self.filename_short,
                     line=self._get_po_line_number(entries[0]),
                 )


Test patch:
diff --git a/test_repo/broken_module/i18n_extra/es.po b/test_repo/broken_module/i18n_extra/es.po
new file mode 100644
index 0000000..2dece62
--- /dev/null
+++ b/test_repo/broken_module/i18n_extra/es.po
@@ -0,0 +1,25 @@
+#
+msgid ""
+msgstr ""
+"Project-Id-Version: Odoo Server\n"
+"Report-Msgid-Bugs-To: \n"
+"POT-Creation-Date: 1985-04-14 17:12+0000\n"
+"PO-Revision-Date: 1985-04-14 02:03+0000\n"
+"Last-Translator: Moisés López <moylop260@vauxoo.com>\n"
+"Language-Team: \n"
+"MIME-Version: 1.0\n"
+"Content-Type: text/plain; charset=UTF-8\n"
+"Content-Transfer-Encoding: \n"
+"Plural-Forms: \n"
+
+#. module: broken_module
+#: model:ir.model.fields,field_description5:broken_module.field_wizard_description5
+#, python-format
+msgid "'Duplicate PO message definition' ignored for i18n_extra"
+msgstr "'translation' 1"
+
+#. module: broken_module
+#: model:ir.model.fields,field_description10:broken_module.field_wizard_description10
+#, python-format
+msgid "'Duplicate PO message definition' ignored for i18n_extra"
+msgstr "'translation' 2"