← Back to team overview

openerp-dev-web team mailing list archive

Re: [Merge] lp:~openerp-dev/openobject-server/ysa-trunk-server into lp:openobject-server

 

Review: Disapprove
Hello,

I disagree with these 2 fixes:

- for bug 695960, you changed the behavior of toxml() in some cases, introducing an inconsistency (it will now return a unicode value if val was unicode, otherwise a utf-8 string). This is not correct, and could cause regressions or strange bugs. Moreover, you replaced val.encode() with ustr(val), but ustr(val) is useless when val is unicode, because there is nothing to do and ustr will directly return val.
You need to fix the report code or completely change toxml to always return unicode values + change the comments + double-check that all addons calling toxml() will still work with a unicode value (some may be passing that value to a function that only accepts utf-8 strings...)

- for bug 698023, the problem is not that there is an error message with the constraint violation, which is normal, but the fact that the product category cannot be deleted even after removing all the products in it. Hiding the traceback of exception will not solve the bug, and will make other constraint violation issues harder to troubleshoot. This really needs to be fixed in the product module, for example in the unlink method of product.product, to unlink() the product.template if there is no other product.product remaining for this template. This way when they are all gone the category can be deleted.

Thanks!
-- 
https://code.launchpad.net/~openerp-dev/openobject-server/ysa-trunk-server/+merge/45351
Your team OpenERP R&D Team is subscribed to branch lp:~openerp-dev/openobject-server/ysa-trunk-server.



References