Merge lp:~savoirfairelinux-openerp/ocb-server/translate_warnings_1297525-trunk into lp:ocb-server

[Sandy Carter (http://www.savoirfairelinux.com)]

When searching a frame for context, the _() looks for self.localcontext but misses self.context.
The integration of this patch allows for web warnings (such as except_orm) to be translated by _()

[Holger Brunn (Therp) (hbrunn)]

review: Approve (code review)

[lp:~savoirfairelinux-openerp/ocb-server/translate_warnings_1297525-trunk]

5296. By Mohammed Shekha(Open ERP) on 2014-04-06

[FIX]Base, Tools Translation: Fixed the issue of translation, warnings, except_orm and which is raised through OpenERPSession class send method or raised xmlrpclib.Fault in xmlrpc_handle_exception_legacy will also not be translatanble, also there is no way through which translation terms which are used outside any module like used in tools and service will not be registered in pot when translation is exported, we need to add those path explicitly.

5297. By El Hadji Dem (http://www.savoirfairelinux.com) on 2014-04-06

[FIX] Fixed instance of ValidateError not being translated

[Sandy Carter (http://www.savoirfairelinux.com) (sandy-carter)]

Two commits have been added:
r5296 from OpenERP's support
r5297 from our team for a case I missed of error message translation

[Stefan Rijnhart (Opener) (stefan-opener)]

Thanks for the proposal!

Strings like 'AccessError' and 'ValidateError' are not proper English, so they cannot be translated. Either we accept the current strings as fixed codes (in lieu of a proper exception type) and they should not be passed to gettext, or we should replace them with translatable strings such as Access Error and Validation Error.

review: Needs Fixing

[Sandy Carter (http://www.savoirfairelinux.com) (sandy-carter)]

@Stefan they give a satisfactory message when translated (no update to .po needed)

I don't want to change too much from the original code, but if you feel it's really needed, I can replace them with your suggestions.

review: Needs Information

[Stefan Rijnhart (Opener) (stefan-opener)]

Yes, I would like you to take the opportunity to improve this as well. I would prefer to see proper English here, but I am not sure if its value is checked on any occasion in case the exception is caught (in lieu of a proper exception subclassing, I can imagine).

[lp:~savoirfairelinux-openerp/ocb-server/translate_warnings_1297525-trunk]

5298. By Sandy Carter (http://www.savoirfairelinux.com) on 2014-04-30

[IMP] Use english translatable string in error popups

[Sandy Carter (http://www.savoirfairelinux.com) (sandy-carter)]

Done.

Please make sure this doesn't have funny side effects and translates properly.

[Stefan Rijnhart (Opener) (stefan-opener)]

OK, here's a reason not to change the labels and keep them untranslatable:

    hr_attendance/test/attendance_process.yml: assert e[0]=='ValidateError', e

Apparently, these labels are used in tests.

[Sandy Carter (http://www.savoirfairelinux.com) (sandy-carter)]

The tests fail anyway in any other language, especially in yml tests
Should I fix that assert too.

[Stefan Rijnhart (Opener) (stefan-opener)]

Well, if that is possible. What kind of solution were you thinking of to fix the test?

[Sandy Carter (http://www.savoirfairelinux.com) (sandy-carter)]

Basically, if I change the labels, I would change the message in the tests.
Ideally, an assertRaises should be used there, but I have no idea if yml tests support that.

[Stefan Rijnhart (Opener) (stefan-opener)]

Can you be sure that if a translator on Launchpad changed the translation of this term, it does not make the tests fail?

[Stefan Rijnhart (Opener) (stefan-opener)]

The en_US translation I mean.

[Sandy Carter (http://www.savoirfairelinux.com) (sandy-carter)]

It would make the tests fail, but this problem is widespread in OE.
For example test_20_message_post (openerp.addons.mail.tests.test_mail_features.test_mail) fails when a translator changes the value. It just doesn't fail in en_US because the values haven't changed. The same is not true for other locales.

A solution to this would be to use _() in the assert. I am not sure if that's possible in yml.

If we're talking about changing en_US why not revert my last commit and translate "ValidateError" as "Validate Error"?

[Stefan Rijnhart (Opener) (stefan-opener)]

I guess it's hard for us to decide without input from the upstream developers. I really don't want to break ocb tests, and to prevent that it would need a coordinated merge between the two projects. So I would therefore still favour having 'ValidateError' to be restored and untranslatable for ocb, but maybe change it to a translatable 'Validation Error' in proposals on upstream trunk server and the tests in addons?

[Sandy Carter (http://www.savoirfairelinux.com) (sandy-carter)]

I have already made some MPs for upstream, but the code is pretty different.

[Stefan Rijnhart (Opener) (stefan-opener)]

Not sure what you mean there. Do you think my suggestions are not a good idea?

[Sandy Carter (http://www.savoirfairelinux.com) (sandy-carter)]

Sorry. I didn't explain very well.
I have MPs for OCB, 7.0 and trunk for this bug.
OE trunk doesn't have both MP (server and web) because it too much, therefore web doesn't have an MP.
As for server, commits up to r5297 ([FIX] Fixed instance of ValidateError not being translated) have been proposed.

[Stefan Rijnhart (Opener) (stefan-opener)]

I think we need a reviewer with a fresh look on this.

review: Abstain

[Holger Brunn (Therp) (hbrunn)]

Development for 7.0 has moved to github on https://github.com/OCA/ocb - please move your merge proposal there if it is still valid.

(I close and reject this in order to have a cleaner overview for 6.1 MPs which indeed have to be done on launchpad)

review: Disapprove

Unmerged revisions

5298. By Sandy Carter (http://www.savoirfairelinux.com) on 2014-04-30

[IMP] Use english translatable string in error popups

5297. By El Hadji Dem (http://www.savoirfairelinux.com) on 2014-04-06

[FIX] Fixed instance of ValidateError not being translated

5296. By Mohammed Shekha(Open ERP) on 2014-04-06

[FIX]Base, Tools Translation: Fixed the issue of translation, warnings, except_orm and which is raised through OpenERPSession class send method or raised xmlrpclib.Fault in xmlrpc_handle_exception_legacy will also not be translatanble, also there is no way through which translation terms which are used outside any module like used in tools and service will not be registered in pot when translation is exported, we need to add those path explicitly.

5295. By Sandy Carter (http://www.savoirfairelinux.com) on 2014-03-25

Also look for context in self

=== modified file 'openerp/osv/orm.py'
--- openerp/osv/orm.py	2014-04-07 10:57:40 +0000
+++ openerp/osv/orm.py	2014-04-30 15:28:06 +0000
@@ -1559,7 +1559,7 @@
                 )
                 self._invalids.update(fields)
         if error_msgs:
-            raise except_orm('ValidateError', '\n'.join(error_msgs))
+            raise except_orm(_('Validate Error'), '\n'.join(error_msgs))
         else:
             self._invalids.clear()
 

=== modified file 'openerp/service/wsgi_server.py'
--- openerp/service/wsgi_server.py	2013-11-25 10:38:42 +0000
+++ openerp/service/wsgi_server.py	2014-04-30 15:28:06 +0000
@@ -44,6 +44,7 @@
 import openerp
 import openerp.modules
 import openerp.tools.config as config
+from openerp.tools.translate import _
 import websrv_lib
 
 _logger = logging.getLogger(__name__)
@@ -138,17 +139,18 @@
     return response
 
 def xmlrpc_handle_exception_legacy(e):
+    code_string = u"%s -- %s\n\n%s"
     if isinstance(e, openerp.osv.osv.except_osv):
-        fault = xmlrpclib.Fault('warning -- ' + e.name + '\n\n' + e.value, '')
+        fault = xmlrpclib.Fault(code_string % (_("warning"), e.name, e.value), '')
         response = xmlrpclib.dumps(fault, allow_none=False, encoding=None)
     elif isinstance(e, openerp.exceptions.Warning):
-        fault = xmlrpclib.Fault('warning -- Warning\n\n' + str(e), '')
+        fault = xmlrpclib.Fault(code_string % (_("warning"), _("Warning"), e), '')
         response = xmlrpclib.dumps(fault, allow_none=False, encoding=None)
     elif isinstance(e, openerp.exceptions.AccessError):
-        fault = xmlrpclib.Fault('warning -- AccessError\n\n' + str(e), '')
+        fault = xmlrpclib.Fault(code_string % (_("warning"), _("Access Error"), e), '')
         response = xmlrpclib.dumps(fault, allow_none=False, encoding=None)
     elif isinstance(e, openerp.exceptions.AccessDenied):
-        fault = xmlrpclib.Fault('AccessDenied', str(e))
+        fault = xmlrpclib.Fault(_('Access Denied'), str(e))
         response = xmlrpclib.dumps(fault, allow_none=False, encoding=None)
     elif isinstance(e, openerp.exceptions.DeferredException):
         info = e.traceback

=== modified file 'openerp/tools/translate.py'
--- openerp/tools/translate.py	2014-04-11 16:37:46 +0000
+++ openerp/tools/translate.py	2014-04-30 15:28:06 +0000
@@ -196,7 +196,7 @@
             lang = ctx.get('lang')
         s = frame.f_locals.get('self', {})
         if not lang:
-            c = getattr(s, 'localcontext', None)
+            c = getattr(s, 'localcontext', None) or getattr(s, 'context', None)
             if c:
                 lang = c.get('lang')
         if not lang:
@@ -863,7 +863,7 @@
         path_list = [root_path,] + apaths
 
     # Also scan these non-addon paths
-    for bin_path in ['osv', 'report' ]:
+    for bin_path in ['osv', 'report', 'tools', 'service' ]:
         path_list.append(os.path.join(config.config['root_path'], bin_path))
 
     _logger.debug("Scanning modules at paths: ", path_list)