Merge lp:~openerp-dev/openobject-server/trunk-bug-712254-ysa into lp:openobject-server

Proposed by Yogesh (SerpentCS)
Status: Rejected
Rejected by: Vo Minh Thu
Proposed branch: lp:~openerp-dev/openobject-server/trunk-bug-712254-ysa
Merge into: lp:openobject-server
Diff against target: 65 lines (+15/-10)
1 file modified
openerp/osv/fields.py (+15/-10)
To merge this branch: bzr merge lp:~openerp-dev/openobject-server/trunk-bug-712254-ysa
Reviewer Review Type Date Requested Status
Yogesh (SerpentCS) (community) Needs Resubmitting
Naresh(OpenERP) Pending
Review via email: mp+65941@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Vo Minh Thu (thu) wrote :

- You are computing the company_id with

  company_id = obj.pool.get('res.users').browse(cr, uid, uid, context).company_id.id

but later in _fnct_write, it is computed with

  cid = company._company_default_get(cr, uid, obj._name, def_id, context=context)

Did you choose the former for a specific reason?

- You have made twice the same modification. Please refactor the code when you see duplicated code.

3481. By Yogesh (SerpentCS)

[FIX+IMP] improvement in fnct_write and fnct_read method of property class.

Revision history for this message
Yogesh (SerpentCS) (yogesh-serpentcs) wrote :

Hello sir,

I have improve the code. Please check it.

Thanks,

review: Needs Resubmitting
Revision history for this message
Vo Minh Thu (thu) wrote :

Actually, the call to _company_default_get shouldn't be there (it is unrelated to the property field). We also want to make the read/write function be much simpler, with the main work being done in ir.properties.

qdp will work on it (next week I think) as part of some larger rework, so I will reject this merge proposal.

Part of the reason it needs a rework is that at creation time, the company is chosen (wrongly) by _company_default_get. Later when we want to read the property, we have no idea of the company that was chosen and cannot decide which property is the right one.

Unmerged revisions

3481. By Yogesh (SerpentCS)

[FIX+IMP] improvement in fnct_write and fnct_read method of property class.

3480. By Yogesh (SerpentCS)

[FIX] add domain comapny_id = current user comapny in fnct_read and _get_by_id method in fields.property class.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'openerp/osv/fields.py'
2--- openerp/osv/fields.py 2011-06-24 14:01:03 +0000
3+++ openerp/osv/fields.py 2011-06-27 10:13:32 +0000
4@@ -1026,11 +1026,11 @@
5 replaces[value._name][value.id] = True
6 return default_value, replaces
7
8- def _get_by_id(self, obj, cr, uid, prop_name, ids, context=None):
9+ def _get_by_id(self, obj, cr, uid, prop_name, ids, company_id=False, context=None):
10 prop = obj.pool.get('ir.property')
11 vids = [obj._name + ',' + str(oid) for oid in ids]
12
13- domain = [('fields_id.model', '=', obj._name), ('fields_id.name','in',prop_name)]
14+ domain = [('fields_id.model', '=', obj._name), ('fields_id.name','in',prop_name), ('company_id','=', company_id)]
15 #domain = prop._get_domain(cr, uid, prop_name, obj._name, context)
16 if vids:
17 domain = [('res_id', 'in', vids)] + domain
18@@ -1040,18 +1040,16 @@
19 def _fnct_write(self, obj, cr, uid, id, prop_name, id_val, obj_dest, context=None):
20 if context is None:
21 context = {}
22-
23- nids = self._get_by_id(obj, cr, uid, [prop_name], [id], context)
24+ def_id = self._field_get(cr, uid, obj._name, prop_name)
25+ company_id = obj.pool.get('res.company')._company_default_get(cr, uid, obj._name,
26+ def_id, context=context)
27+ nids = self._get_by_id(obj, cr, uid, [prop_name], [id], company_id, context)
28 if nids:
29 cr.execute('DELETE FROM ir_property WHERE id IN %s', (tuple(nids),))
30
31 default_val = self._get_default(obj, cr, uid, prop_name, context)
32
33 if id_val is not default_val:
34- def_id = self._field_get(cr, uid, obj._name, prop_name)
35- company = obj.pool.get('res.company')
36- cid = company._company_default_get(cr, uid, obj._name, def_id,
37- context=context)
38 propdef = obj.pool.get('ir.model.fields').browse(cr, uid, def_id,
39 context=context)
40 prop = obj.pool.get('ir.property')
41@@ -1059,7 +1057,7 @@
42 'name': propdef.name,
43 'value': id_val,
44 'res_id': obj._name+','+str(id),
45- 'company_id': cid,
46+ 'company_id': company_id,
47 'fields_id': def_id,
48 'type': self._type,
49 }, context=context)
50@@ -1068,7 +1066,14 @@
51
52 def _fnct_read(self, obj, cr, uid, ids, prop_name, obj_dest, context=None):
53 properties = obj.pool.get('ir.property')
54- domain = [('fields_id.model', '=', obj._name), ('fields_id.name','in',prop_name)]
55+ company_obj = obj.pool.get('res.company')
56+ company_id = []
57+ for prop in prop_name:
58+ def_id = self._field_get(cr, uid, obj._name, prop)
59+ company_id.append(company_obj._company_default_get(cr, uid, obj._name,
60+ def_id, context=context))
61+ domain = [('fields_id.model', '=', obj._name), ('fields_id.name','in',prop_name),
62+ ('company_id','in', company_id)]
63 domain += [('res_id','in', [obj._name + ',' + str(oid) for oid in ids])]
64 nids = properties.search(cr, uid, domain, context=context)
65 default_val,replaces = self._get_defaults(obj, cr, uid, prop_name, context)