Merge lp:~camptocamp/openobject-addons/trunk_webkit_improvment_rc into lp:openobject-addons

[Nicolas Bessi - Camptocamp]

[FIX] RC2 fix of image embeeding, possibilit to have tmp file name in same milisecond corrected, imporvement of multi header support when cusomizing report, fix of translation getter retunr scr if no translation available

[Olivier Dony (Odoo) (odo-openerp)]

Hello Nicolas,

A few remarks:
- adding try/except blocks without specifying the exception class is a no-go, you need to at least specify "except Exception" if you don't know a more precise class to catch. Also, couldn't it be a single try/except block?

I haven't checked, but do you know if the "Exception" type that you are throwing is properly reported to the user, or is it simply displayed in the log? Don't you want to use an except_osv one?
I know I had to fix several things during the initial review to get exceptions reported to the user correctly.

- As for the question about 0 margins, OpenERP does not yet support distinguishing 0 from NULL values for integers. However I wonder if your margins fields should not be fields.char instead, because the wkhtmltopdf params are strings indeed, as you can pass "10px" or "10.5cm", etc. This would give you more flexibility, wouldn't it? Note: You need to be careful however because I think wkhtmltopdf only supports the same unit of measure for all margins, and the default ones are "10mm". So when specifying a unit it needs to be set for all margins and it must be the same.

I'm okay with the latter change for RC2, as it is needed to properly fix the margins.

What do you think?

review: Needs Fixing

[Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount)]

Hello,

For the try catch block around the render method I strictly followed the mako doc. That why there is not one unique try except block but one around each render call. Mako does not provide a render exception type (or I missed it in doc). The exception returned is munch more easy to understand. But I agree with the osv_except point and the general Exception type catch point (I have to test if it works).

For the margin I prefer to keep float because setting margin in different unit may crash wkhtmltopdf, and it prevent wrong entries. But why not provide a select field with supported unit and set mm as default.

Many thanks

Nicolas

Le 17 nov. 2010 à 16:53, "Olivier Dony (OpenERP)" <email address hidden> a écrit :

> Review: Needs Fixing
> Hello Nicolas,
>
> A few remarks:
> - adding try/except blocks without specifying the exception class is a no-go, you need to at least specify "except Exception" if you don't know a more precise class to catch. Also, couldn't it be a single try/except block?
>
> I haven't checked, but do you know if the "Exception" type that you are throwing is properly reported to the user, or is it simply displayed in the log? Don't you want to use an except_osv one?
> I know I had to fix several things during the initial review to get exceptions reported to the user correctly.
>
> - As for the question about 0 margins, OpenERP does not yet support distinguishing 0 from NULL values for integers. However I wonder if your margins fields should not be fields.char instead, because the wkhtmltopdf params are strings indeed, as you can pass "10px" or "10.5cm", etc. This would give you more flexibility, wouldn't it? Note: You need to be careful however because I think wkhtmltopdf only supports the same unit of measure for all margins, and the default ones are "10mm". So when specifying a unit it needs to be set for all margins and it must be the same.
>
> I'm okay with the latter change for RC2, as it is needed to properly fix the margins.
>
> What do you think?
> --
> https://code.launchpad.net/~c2c/openobject-addons/trunk_webkit_improvment_rc/+merge/40847
> Your team Camptocamp is subscribed to branch lp:~c2c/openobject-addons/trunk_webkit_improvment_rc.

=== modified file 'report_webkit/report_helper.py'
--- report_webkit/report_helper.py	2010-10-14 13:38:14 +0000
+++ report_webkit/report_helper.py	2010-11-25 16:51:17 +0000
@@ -51,7 +51,7 @@
             height = 'width="%spx"'%(height)
         else :
             height = ' '
-        toreturn = '<img %s %s src="data:image/%s;base64,%s">'%(
+        toreturn = '<img %s %s src="data:image/%s;base64,%s" />'%(
             width,
             height,
             type, 

=== modified file 'report_webkit/webkit_report.py'
--- report_webkit/webkit_report.py	2010-10-26 13:22:54 +0000
+++ report_webkit/webkit_report.py	2010-11-25 16:51:17 +0000
@@ -35,7 +35,7 @@
 import tempfile
 import time
 from mako.template import Template
-
+from mako import exceptions
 import netsvc
 import pooler
 from report_helper import WebKitHelper
@@ -84,8 +84,10 @@
                             _('path to Wkhtmltopdf is not absolute'),
                             'for path %s'%(path)
                             )
-    def generate_pdf(self, comm_path, report_xml, header, footer, html_list):
+    def generate_pdf(self, comm_path, report_xml, header, footer, html_list, webkit_header=False):
         """Call webkit in order to generate pdf"""
+        if not webkit_header:
+            webkit_header = report_xml.webkit_header
         tmp_dir = tempfile.gettempdir()
         out = report_xml.name+str(time.time())+'.pdf'
         out = os.path.join(tmp_dir, out.replace(' ',''))
@@ -120,20 +122,22 @@
             file_to_del.append(foot_file.name)
             command.append("--footer-html '%s'"%(foot_file.name))
             
-        if report_xml.webkit_header.margin_top :
-            command.append('--margin-top %s'%(report_xml.webkit_header.margin_top))
-        if report_xml.webkit_header.margin_bottom :
-            command.append('--margin-bottom %s'%(report_xml.webkit_header.margin_bottom))
-        if report_xml.webkit_header.margin_left :
-            command.append('--margin-left %s'%(report_xml.webkit_header.margin_left))
-        if report_xml.webkit_header.margin_right :
-            command.append('--margin-right %s'%(report_xml.webkit_header.margin_right))
-        if report_xml.webkit_header.orientation :
-            command.append("--orientation '%s'"%(report_xml.webkit_header.orientation))
-        if report_xml.webkit_header.format :
-            command.append(" --page-size '%s'"%(report_xml.webkit_header.format))
+        if webkit_header.margin_top :
+            command.append('--margin-top %s'%(webkit_header.margin_top))
+        if webkit_header.margin_bottom :
+            command.append('--margin-bottom %s'%(webkit_header.margin_bottom))
+        if webkit_header.margin_left :
+            command.append('--margin-left %s'%(webkit_header.margin_left))
+        if webkit_header.margin_right :
+            command.append('--margin-right %s'%(webkit_header.margin_right))
+        if webkit_header.orientation :
+            command.append("--orientation '%s'"%(webkit_header.orientation))
+        if webkit_header.format :
+            command.append(" --page-size '%s'"%(webkit_header.format))
+        count = 0
         for html in html_list :
-            html_file = file(os.path.join(tmp_dir, str(time.time()) + '.body.html'), 'w')
+            html_file = file(os.path.join(tmp_dir, str(time.time()) + str(count) +'.body.html'), 'w')
+            count += 1
             html_file.write(html)
             html_file.close()
             file_to_del.append(html_file.name)
@@ -168,6 +172,8 @@
         """Translate String."""
         ir_translation = self.pool.get('ir.translation')
         res = ir_translation._get_source(self.parser_instance.cr, self.parser_instance.uid, self.name, 'report', self.localcontext.get('lang', 'en_US'), src)
+        if not res :
+            return src
         return res 
  
     def formatLang(self, value, digits=None, date=False, date_time=False, grouping=True, monetary=False):
@@ -272,45 +278,64 @@
         #default_filters=['unicode', 'entity'] can be used to set global filter
         body_mako_tpl = Template(template ,input_encoding='utf-8')
         helper = WebKitHelper(cursor, uid, report_xml.id, context)
-        html = body_mako_tpl.render(     helper=helper, 
-                                         css=css,
-                                         _=self.translate_call,
-                                         **self.parser_instance.localcontext
-                                         )
+        try :
+            html = body_mako_tpl.render(     helper=helper, 
+                                             css=css,
+                                             _=self.translate_call,
+                                             **self.parser_instance.localcontext
+                                        )
+        except Exception, e:
+            msg = exceptions.text_error_template().render()
+            netsvc.Logger().notifyChannel('Webkit render', netsvc.LOG_ERROR, msg)
+            raise except_osv(_('Webkit render'), msg)
         head_mako_tpl = Template(header, input_encoding='utf-8')
-        head = head_mako_tpl.render(
-                                    company=company, 
-                                    time=time, 
-                                    helper=helper, 
-                                    css=css,
-                                    formatLang=self.formatLang,
-                                    setLang=self.setLang,
-                                    _=self.translate_call,
-                                    _debug=False
-                                )
+        try :
+            head = head_mako_tpl.render(
+                                        company=company, 
+                                        time=time, 
+                                        helper=helper, 
+                                        css=css,
+                                        formatLang=self.formatLang,
+                                        setLang=self.setLang,
+                                        _=self.translate_call,
+                                        _debug=False
+                                    )
+        except Exception, e:
+            raise except_osv(_('Webkit render'),
+                exceptions.text_error_template().render())
         foot = False
         if footer :
             foot_mako_tpl = Template(footer ,input_encoding='utf-8')
-            foot = foot_mako_tpl.render(
-                                        company=company, 
-                                        time=time, 
-                                        helper=helper, 
-                                        css=css, 
-                                        formatLang=self.formatLang,
-                                        setLang=self.setLang,
-                                        _=self.translate_call,
-                                        )
+            try :
+                foot = foot_mako_tpl.render(
+                                            company=company, 
+                                            time=time, 
+                                            helper=helper, 
+                                            css=css, 
+                                            formatLang=self.formatLang,
+                                            setLang=self.setLang,
+                                            _=self.translate_call,
+                                            )
+            except:
+                msg = exceptions.text_error_template().render()
+                netsvc.Logger().notifyChannel('Webkit render', netsvc.LOG_ERROR, msg)
+                raise except_osv(_('Webkit render'), msg)
         if report_xml.webkit_debug :
-            deb = head_mako_tpl.render(
-                                        company=company, 
-                                        time=time, 
-                                        helper=helper, 
-                                        css=css, 
-                                        _debug=html,
-                                        formatLang=self.formatLang,
-                                        setLang=self.setLang,
-                                        _=self.translate_call,
-                                        )
+            try :
+                deb = head_mako_tpl.render(
+                                            company=company, 
+                                            time=time, 
+                                            helper=helper, 
+                                            css=css, 
+                                            _debug=html,
+                                            formatLang=self.formatLang,
+                                            setLang=self.setLang,
+                                            _=self.translate_call,
+                                            )
+            except Exception, e:
+                msg = exceptions.text_error_template().render()
+                netsvc.Logger().notifyChannel('Webkit render', netsvc.LOG_ERROR, msg)
+                raise except_osv(_('Webkit render'), msg)
             return (deb, 'html')
         bin = self.get_lib(cursor, uid, company.id)
         pdf = self.generate_pdf(bin, report_xml, head, foot, [html])