Merge lp:~openerp-dev/openobject-addons/6.0-opw_bug-734191-ach into lp:openobject-addons/6.0

Proposed by Anup(SerpentCS)
Status: Needs review
Proposed branch: lp:~openerp-dev/openobject-addons/6.0-opw_bug-734191-ach
Merge into: lp:openobject-addons/6.0
Diff against target: 49 lines (+22/-3)
1 file modified
product/product.py (+22/-3)
To merge this branch: bzr merge lp:~openerp-dev/openobject-addons/6.0-opw_bug-734191-ach
Reviewer Review Type Date Requested Status
Jacques-Etienne Baudoux (community) Disapprove
Olivier Dony (Odoo) Needs Information
Jay Vora (Serpent Consulting Services) (community) Approve
Stephane Wirtel (OpenERP) Pending
Review via email: mp+56116@code.launchpad.net

Description of the change

Hello,

   The issue has been fixed by this solution. Due to context the German was being copied in the orignal data or as an english translation. It's fixed.
   The translations were not also being copied properly and that was also the reason for not having the correct results in different languages.

Thanks.

To post a comment you must log in.
4503. By Jay Vora (Serpent Consulting Services)

[FIX] Stock : Removed the buttons which could confuse the users (Ref : Case 4970)

4504. By Jay Vora (Serpent Consulting Services)

[MERGE] Merged Dhruti's branch for the bugfix of lp:739172

4505. By Jay Vora (Serpent Consulting Services)

[FIX] Backported the fix from trunk for the bug lp:740047

4506. By Jay Vora (Serpent Consulting Services)

[FIX] product : Copying product now reflecting with proper translations

Revision history for this message
Jay Vora (Serpent Consulting Services) (jayvora) wrote :

Anup,

Thanks for the merge, this looks good.Let us wait for odo's approval as this belongs to server as well.

Thanks again.

review: Approve
Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

This looks good but I don't think it works properly when the product_template.id and product_product.id are different.

This is a situation that happens very often, as soon as you use product variants, or even without that (it used to be that deleting a product would not delete the template, even if there was only one).

You can simulate this easily: insert an additional template or product manually in the database, and see if this still works.

I think this might even need to be fixed in the orm's copy_translation method, because it does not seem to take it into account.

Minor other things:
- why the funky indentation in copy_translation()? ;-)
- as a good practice, do not alter the original context, please use a copy, otherwise you might create side-effects when you modify/remove the language, for example if the RPC call does something else afterwards.

review: Needs Information
Revision history for this message
Jay Vora (Serpent Consulting Services) (jayvora) wrote :

Olivier,

Thanks for the review.

Please don't get confused with the 'name' of ir_translation record.

Here is a clear distinction:
trans_ids = trans_obj.search(cr, uid, [('res_id','=',new_id), ('name','=','product.template,name')])

-- The new_id will be the ID of the table product_product only(disregard to whatever is the ID of product_template).
-- Name of the record will be 'product_template,name' because we have a convention that this name be designed as 'model which has this field in, field name'. Here the translatable field 'name' resides into the columns of product.template.

This would work very normal when we dont overide copy() for product_product().
But, in case of translation, its if we have overridden copy(),we should override copy_translations() too.

Correct me if I missed anything.

I am investigating more.

Revision history for this message
Jacques-Etienne Baudoux (jbaudoux) wrote :

I don't think it will work. If you have 'variant' in the context (line 639), it's calling only create and never calling copy_translations.

The minor other things Olivier told you are still remaining:
- why the funky indentation in copy_translation()? ;-)
- as a good practice, do not alter the original context, please use a copy, otherwise you might create side-effects when you modify/remove the language, for example if the RPC call does something else afterwards.
630 context_without_lang = context.copy()
631 if 'lang' in context_without_lang:
632 del context_without_lang['lang']
633 product = self.read(cr, uid, id, ['name'], context=context_without_lang)

651 return super(product_product, self).copy(cr, uid, id, default=default,
652 context=context)

review: Disapprove

Unmerged revisions

4506. By Jay Vora (Serpent Consulting Services)

[FIX] product : Copying product now reflecting with proper translations

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'product/product.py'
2--- product/product.py 2011-03-10 09:52:25 +0000
3+++ product/product.py 2011-04-04 13:48:55 +0000
4@@ -607,15 +607,34 @@
5
6 return res
7
8+ def copy_translations(self, cr, uid, old_id, new_id, context=None):
9+ # Copying valid values for the ir_translation records
10+ if context is None:
11+ context = {}
12+ super(product_product,self).copy_translations(cr, uid, old_id, new_id, context=context)
13+ trans_obj = self.pool.get('ir.translation')
14+ trans_ids = trans_obj.search(cr, uid, [('res_id','=',new_id), ('name','=','product.template,name')])
15+ trans_records = trans_obj.read(cr, uid, trans_ids, ['src','value','lang'])
16+ for trans_rec in trans_records:
17+ context['lang'] = trans_rec['lang']
18+ trans_rec['src'] += ' (copy)'
19+ trans_rec['value'] += _(' (copy)')
20+ tr_id = trans_rec['id']
21+ del trans_rec['id']
22+ trans_obj.write(cr, uid, [tr_id], trans_rec)
23+
24 def copy(self, cr, uid, id, default=None, context=None):
25 if context is None:
26 context={}
27-
28+ #In order to fetch the name of the record stored in DB, we need to remove the contextual langauge.
29+ context_with_lang = context.copy()
30+ if 'lang' in context:
31+ del context['lang']
32 product = self.read(cr, uid, id, ['name'], context=context)
33 if not default:
34 default = {}
35 default = default.copy()
36- default['name'] = product['name'] + _(' (copy)')
37+ default['name'] = product['name'] + ' (copy)'
38
39 if context.get('variant',False):
40 fields = ['product_tmpl_id', 'active', 'variants', 'default_code',
41@@ -630,7 +649,7 @@
42 return self.create(cr, uid, data)
43 else:
44 return super(product_product, self).copy(cr, uid, id, default=default,
45- context=context)
46+ context=context_with_lang)
47 product_product()
48
49 class product_packaging(osv.osv):