Merge lp:~therp-nl/ocb-server/6.1-lp933496 into lp:ocb-server/6.1

[Holger Brunn (Therp)]

This is a backport of the fix for translation weirdnesses in 6.1. They are bound to become worse over time, so I think we really need this for 6.1. With 7.0, I didn't run into sort like problems yet, which is why I don't propose the same thing for 7.0.

[Pedro Manuel Baeza (pedro.baeza)]

Hi, Holger, thank you very much for the fix. I have tried it, but the issues I was experimenting are already there. You can see here an screenshot:

http://img27.imageshack.us/img27/5065/tem5.png

I'm not sure if the problem is the same, but the beginning of the problem start more or less at the same time of the 7.0 debut.

Sorry for not being more helpful, but I don't know very well translation engine and it's very difficult to me to trace the problem.

Regards.

[Holger Brunn (Therp) (hbrunn)]

Did you do an --update=all after applying the patch? It changes the way translations are loaded, so without the update, nothing happens.

[Pedro Manuel Baeza (pedro.baeza)]

Yeah, I did it also on a fresh DB, so maybe the problem is another. I will investigate further when I have a little time, but do you have any tip about what is happenning?

Regards.

[Lionel Sausin - Initiatives/Numérigraphe (ls-initiatives)]

Has there been any progress on this issue?

review: Needs Information

[Alberto Garcia (Factor Libre) (agarcia-flibre)]

After merge there are termns isnt translated like menu purchases. I try with spanish

[Pedro Manuel Baeza (pedro.baeza)]

Digging deeply in the problem, I could test that the patch is working correctly. The problem comes from another place: pot files are not correct on official repository, so there's no valid translation source (neither .po nor .pot files). More info on the related bug report.

I give then my approval to this MP, but I'm going to notify the problem (maybe via another bug), and let Olivier know about it.

Regards.

review: Approve (code review and test)

[Lionel Sausin - Initiatives/Numérigraphe (ls-initiatives)]

The patch can be further improved by using a TransientModel instead of a Model for testing. In v7.0 this fixes a warning about missing permissions. See http://bazaar.launchpad.net/~numerigraphe-team/ocb-server/7.0-po-targets-933496-vmt/revision/4798
I don't see any other problem than this detail, so I approve.

review: Approve

[Lionel Sausin - Initiatives/Numérigraphe (ls-initiatives)]

Same patch proposed for lp:ocb-server/7.0: https://code.launchpad.net/~numerigraphe-team/ocb-server/7.0-po-targets-933496-vmt/+merge/209895

[Pedro Manuel Baeza (pedro.baeza)]

I proceed with the merge making the change that Lionel has proposed (change to orm.TransientModel). In the unlikely case that OpenERP merge the patch in the upstream, we will have a conflict, but it's very easy to resolve.

Regards.

=== added directory 'openerp/tests/addons/test_translation_import'
=== added file 'openerp/tests/addons/test_translation_import/__init__.py'
--- openerp/tests/addons/test_translation_import/__init__.py	1970-01-01 00:00:00 +0000
+++ openerp/tests/addons/test_translation_import/__init__.py	2013-10-29 08:06:06 +0000
@@ -0,0 +1,3 @@
+# -*- coding: utf-8 -*-
+import models
+# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:

=== added file 'openerp/tests/addons/test_translation_import/__openerp__.py'
--- openerp/tests/addons/test_translation_import/__openerp__.py	1970-01-01 00:00:00 +0000
+++ openerp/tests/addons/test_translation_import/__openerp__.py	2013-10-29 08:06:06 +0000
@@ -0,0 +1,16 @@
+# -*- coding: utf-8 -*-
+{
+    'name': 'test-translation-import',
+    'version': '0.1',
+    'category': 'Tests',
+    'description': """A module to test translation import.""",
+    'author': 'OpenERP SA',
+    'maintainer': 'OpenERP SA',
+    'website': 'http://www.openerp.com',
+    'depends': ['base'],
+    'data': ['view.xml'],
+    'test': ['tests.yml'],
+    'installable': True,
+    'auto_install': False,
+}
+# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:

=== added directory 'openerp/tests/addons/test_translation_import/i18n'
=== added file 'openerp/tests/addons/test_translation_import/i18n/fr.po'
--- openerp/tests/addons/test_translation_import/i18n/fr.po	1970-01-01 00:00:00 +0000
+++ openerp/tests/addons/test_translation_import/i18n/fr.po	2013-10-29 08:06:06 +0000
@@ -0,0 +1,52 @@
+# This is a test PO file, not a true french translation.
+# See the POT file for further information.
+msgid ""
+msgstr ""
+"Project-Id-Version: OpenERP Server 6.1\n"
+"Report-Msgid-Bugs-To: \n"
+"POT-Creation-Date: 2012-10-17 12:36+0000\n"
+"PO-Revision-Date: 2012-10-17 12:36+0000\n"
+"Last-Translator: <>\n"
+"Language-Team: \n"
+"MIME-Version: 1.0\n"
+"Content-Type: text/plain; charset=UTF-8\n"
+"Content-Transfer-Encoding: \n"
+"Plural-Forms: \n"
+
+# Note: there is normally an additional line:
+#     #: code:addons/test_translation_import/models.py:17
+# This line is present in the POT and removed here to test the translation
+# import behavior.
+#. module: test_translation_import
+#: field:test.translation.import,name:0
+#, python-format
+msgid "1XBUO5PUYH2RYZSA1FTLRYS8SPCNU1UYXMEYMM25ASV7JC2KTJZQESZYRV9L8CGB"
+msgstr "1XBUO5PUYH2RYZSA1FTLRYS8SPCNU1UYXMEYMM25ASV7JC2KTJZQESZYRV9L8CGB in french"
+
+#. module: test_translation_import
+#: code:addons/test_translation_import/models.py:14
+#, python-format
+msgid "Ijkl"
+msgstr "Ijkl in french"
+
+#. module: test_translation_import
+#: model:ir.model,name:test_translation_import.model_test_translation_import
+msgid "test.translation.import"
+msgstr "test.translation.import in french"
+
+#. module: test_translation_import
+#: help:test.translation.import,name:0
+msgid "Efgh"
+msgstr "Efgh in french"
+
+#. module: test_translation_import
+#: model:ir.actions.act_window,name:test_translation_import.action_test_translation_import
+#: model:ir.ui.menu,name:test_translation_import.menu_test_translation_import
+msgid "Test translation import"
+msgstr "Test translation import in french"
+
+#. module: test_translation_import
+#: model:ir.ui.menu,name:test_translation_import.menu_test_translation
+msgid "Test translation"
+msgstr "Test translation in french"
+

=== added file 'openerp/tests/addons/test_translation_import/i18n/test_translation_import.pot'
--- openerp/tests/addons/test_translation_import/i18n/test_translation_import.pot	1970-01-01 00:00:00 +0000
+++ openerp/tests/addons/test_translation_import/i18n/test_translation_import.pot	2013-10-29 08:06:06 +0000
@@ -0,0 +1,58 @@
+# This is a test POT file, not a true template. It is manually maintained
+# to test the import translation behavior of OpenERP.
+#
+# In particular, the
+# `1XBUO5PUYH2RYZSA1FTLRYS8SPCNU1UYXMEYMM25ASV7JC2KTJZQESZYRV9L8CGB` source is
+# given with two targets (the #: comments): `code` and `field`. The code one is
+# removed in the fr.po file. Still, the import should generate a database entry
+# for the `code` one. I.e. the targets defined in the POT must be added to the
+# targets defined in the PO file.  This was done to fix a bug, as reported by
+# lp:933496.
+#
+msgid ""
+msgstr ""
+"Project-Id-Version: OpenERP Server 6.1\n"
+"Report-Msgid-Bugs-To: \n"
+"POT-Creation-Date: 2012-10-17 12:36+0000\n"
+"PO-Revision-Date: 2012-10-17 12:36+0000\n"
+"Last-Translator: <>\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: test_translation_import
+#: code:addons/test_translation_import/models.py:17
+#: field:test.translation.import,name:0
+#, python-format
+msgid "1XBUO5PUYH2RYZSA1FTLRYS8SPCNU1UYXMEYMM25ASV7JC2KTJZQESZYRV9L8CGB"
+msgstr ""
+
+#. module: test_translation_import
+#: code:addons/test_translation_import/models.py:14
+#, python-format
+msgid "Ijkl"
+msgstr ""
+
+#. module: test_translation_import
+#: model:ir.model,name:test_translation_import.model_test_translation_import
+msgid "test.translation.import"
+msgstr ""
+
+#. module: test_translation_import
+#: help:test.translation.import,name:0
+msgid "Efgh"
+msgstr ""
+
+#. module: test_translation_import
+#: model:ir.actions.act_window,name:test_translation_import.action_test_translation_import
+#: model:ir.ui.menu,name:test_translation_import.menu_test_translation_import
+msgid "Test translation import"
+msgstr ""
+
+#. module: test_translation_import
+#: model:ir.ui.menu,name:test_translation_import.menu_test_translation
+msgid "Test translation"
+msgstr ""
+

=== added file 'openerp/tests/addons/test_translation_import/models.py'
--- openerp/tests/addons/test_translation_import/models.py	1970-01-01 00:00:00 +0000
+++ openerp/tests/addons/test_translation_import/models.py	2013-10-29 08:06:06 +0000
@@ -0,0 +1,21 @@
+# -*- coding: utf-8 -*-
+import openerp
+from openerp.tools.translate import _
+
+class m(openerp.osv.osv.Model):
+    """ A model to provide source strings.
+    """
+    _name = 'test.translation.import'
+
+    _columns = {
+        'name': openerp.osv.fields.char(
+            '1XBUO5PUYH2RYZSA1FTLRYS8SPCNU1UYXMEYMM25ASV7JC2KTJZQESZYRV9L8CGB',
+            size=32, help='Efgh'),
+    }
+
+    _('Ijkl')
+
+    # With the name label above, this source string should be generated twice.
+    _('1XBUO5PUYH2RYZSA1FTLRYS8SPCNU1UYXMEYMM25ASV7JC2KTJZQESZYRV9L8CGB')
+
+# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:

=== added directory 'openerp/tests/addons/test_translation_import/tests'
=== added file 'openerp/tests/addons/test_translation_import/tests.yml'
--- openerp/tests/addons/test_translation_import/tests.yml	1970-01-01 00:00:00 +0000
+++ openerp/tests/addons/test_translation_import/tests.yml	2013-10-29 08:06:06 +0000
@@ -0,0 +1,13 @@
+-
+    Load the french translation.
+-
+    !python {model: ir.translation }: |
+        import openerp
+        openerp.tools.trans_load(cr, 'test_translation_import/i18n/fr.po', 'fr_FR', verbose=False)
+-
+    Assert we have loaded the correct number of entries for the given source string.
+-
+    !python {model: ir.translation }: |
+        ids = self.search(cr, uid,
+            [('src', '=', '1XBUO5PUYH2RYZSA1FTLRYS8SPCNU1UYXMEYMM25ASV7JC2KTJZQESZYRV9L8CGB')])
+        assert len(ids) == 2, "2 entries are expected, got %s instead." % len(ids)

=== added file 'openerp/tests/addons/test_translation_import/tests/__init__.py'
--- openerp/tests/addons/test_translation_import/tests/__init__.py	1970-01-01 00:00:00 +0000
+++ openerp/tests/addons/test_translation_import/tests/__init__.py	2013-10-29 08:06:06 +0000
@@ -0,0 +1,10 @@
+# -*- coding: utf-8 -*-
+import unittest2
+
+import test_term_count
+
+suite = [
+    test_term_count
+    ]
+
+# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:

=== added file 'openerp/tests/addons/test_translation_import/tests/test_term_count.py'
--- openerp/tests/addons/test_translation_import/tests/test_term_count.py	1970-01-01 00:00:00 +0000
+++ openerp/tests/addons/test_translation_import/tests/test_term_count.py	2013-10-29 08:06:06 +0000
@@ -0,0 +1,17 @@
+# -*- coding: utf-8 -*-
+
+import openerp
+from openerp.tests import common
+
+class TestTermCount(common.TransactionCase):
+
+    def test_count_term(self):
+        """
+        Just make sure we have as many translation entries as we wanted.
+        """
+        openerp.tools.trans_load(self.cr, 'test_translation_import/i18n/fr.po', 'fr_FR', verbose=False)
+        ids = self.registry('ir.translation').search(self.cr, self.uid,
+            [('src', '=', '1XBUO5PUYH2RYZSA1FTLRYS8SPCNU1UYXMEYMM25ASV7JC2KTJZQESZYRV9L8CGB')])
+        self.assertEqual(len(ids), 2)
+
+# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:

=== added file 'openerp/tests/addons/test_translation_import/view.xml'
--- openerp/tests/addons/test_translation_import/view.xml	1970-01-01 00:00:00 +0000
+++ openerp/tests/addons/test_translation_import/view.xml	2013-10-29 08:06:06 +0000
@@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="utf-8"?>
+<openerp>
+    <data>
+
+        <record id="action_test_translation_import" model="ir.actions.act_window">
+            <field name="name">Test translation import</field>
+            <field name="type">ir.actions.act_window</field>
+            <field name="res_model">test.translation.import</field>
+            <field name="view_type">form</field>
+            <field name="view_mode">tree,form</field>
+            <field name="target">current</field>
+        </record>
+
+        <menuitem icon="STOCK_PREFERENCES" id="base.menu_tests" name="Tests"/>
+
+        <menuitem id="menu_test_translation" parent="base.menu_tests" name="Test translation"/>
+
+        <menuitem id="menu_test_translation_import"
+            name="Test translation import"
+            action="action_test_translation_import"
+            parent="menu_test_translation"/>
+    </data>
+</openerp>

=== modified file 'openerp/tools/translate.py'
--- openerp/tools/translate.py	2012-10-08 11:09:46 +0000
+++ openerp/tools/translate.py	2013-10-29 08:06:06 +0000
@@ -308,6 +308,10 @@
                 if line.startswith('#~ '):
                     break
                 if line.startswith('#:'):
+                    # Process the `reference` comments. Each line can specify
+                    # multiple targets (e.g. model, view, code, selection,
+                    # ...). For each target, we will return an additional
+                    # entry.
                     for lpart in line[2:].strip().split(' '):
                         trans_info = lpart.strip().split(':',2)
                         if trans_info and len(trans_info) == 2:
@@ -356,6 +360,9 @@
                 line = self.lines.pop(0).strip()
 
             if tmp_tnrs and not fuzzy:
+                # Use the first target for the current entry (returned at the
+                # end of this next() call), and keep the others to generate
+                # additional entries (returned the next next() calls).
                 type, name, res_id = tmp_tnrs.pop(0)
                 for t, n, r in tmp_tnrs:
                     self.tnrs.append((t, n, r, source, trad))
@@ -897,6 +904,10 @@
             # lets create the language with locale information
             lang_obj.load_lang(cr, 1, lang=lang, lang_name=lang_name)
 
+        # Parse also the POT: it will possibly provide additional targets.
+        # (Because the POT comments are correct on Launchpad but not the
+        # PO comments due to a Launchpad limitation.)
+        pot_reader = []
 
         # now, the serious things: we read the language file
         fileobj.seek(0)
@@ -909,19 +920,42 @@
         elif fileformat == 'po':
             reader = TinyPoFile(fileobj)
             f = ['type', 'name', 'res_id', 'src', 'value']
+
+            # Make a reade for the POT file and be somewhat defensive for the
+            # stable branch.
+            if fileobj.name.endswith('.po'):
+                try:
+                    # Normally the path looks like /path/to/xxx/i18n/lang.po
+                    # and we try to find the corresponding
+                    # /path/to/xxx/i18n/xxx.pot file.
+                    head, tail = os.path.split(fileobj.name)
+                    head2, tail2 = os.path.split(head)
+                    head3, tail3 = os.path.split(head2)
+                    pot_handle = misc.file_open(os.path.join(head3, tail3, 'i18n', tail3 + '.pot'))
+                    pot_reader = TinyPoFile(pot_handle)
+                except:
+                    pass
+
         else:
             _logger.error('Bad file format: %s', fileformat)
             raise Exception(_('Bad file format'))
 
+        # Read the POT `reference` comments, and keep them indexed by source
+        # string.
+        pot_targets = {}
+        for type, name, res_id, src, _ in pot_reader:
+            if type is not None:
+                pot_targets.setdefault(src, {'value': None, 'targets': []})
+                pot_targets[src]['targets'].append((type, name, res_id))
+
         # read the rest of the file
-        line = 1
         irt_cursor = trans_obj._get_import_cursor(cr, uid, context=context)
 
-        for row in reader:
-            line += 1
+        def process_row(row):
+            """Process a single PO (or POT) entry."""
             # skip empty rows and rows where the translation field (=last fiefd) is empty
             #if (not row) or (not row[-1]):
-            #    continue
+            #    return
 
             # dictionary which holds values for this line of the csv file
             # {'lang': ..., 'type': ..., 'name': ..., 'res_id': ...,
@@ -933,9 +967,17 @@
                     continue
                 dic[f[i]] = row[i]
 
+            # Get the `reference` comments from the POT.
+            src = row[3]
+            if pot_reader and src in pot_targets:
+                pot_targets[src]['targets'] = filter(lambda x: x != row[:3], pot_targets[src]['targets'])
+                pot_targets[src]['value'] = row[4]
+                if not pot_targets[src]['targets']:
+                    del pot_targets[src]
+
             # This would skip terms that fail to specify a res_id
             if not dic.get('res_id', False):
-                continue
+                return
 
             res_id = dic.pop('res_id')
             if res_id and isinstance(res_id, (int, long)) \
@@ -961,7 +1003,23 @@
 
             irt_cursor.push(dic)
 
+        # First process the entries from the PO file (doing so also fill/remove
+        # the entries from the POT file).
+        for row in reader:
+            process_row(row)
+
+        # Then process the entries implied by the POT file (which is more
+        # correct w.r.t. the targets) if some of them remain.
+        pot_rows = []
+        for src in pot_targets:
+            value = pot_targets[src]['value']
+            for type, name, res_id in pot_targets[src]['targets']:
+                pot_rows.append((type, name, res_id, src, value))
+        for row in pot_rows:
+            process_row(row)
+
         irt_cursor.finish()
+
         if verbose:
             _logger.info("translation file loaded succesfully")
     except IOError: