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.

_______________________________________________
Mailing list: https://launchpad.net/~openerp-dev-gtk
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~openerp-dev-gtk
More help   : https://help.launchpad.net/ListHelp

Reply via email to