Merge lp:~jfb-tempo-consulting/unifield-server/us-3808 into lp:~julie-w/unifield-server/US-3808

Proposed by jftempo on 2018-02-02
Status: Needs review
Proposed branch: lp:~jfb-tempo-consulting/unifield-server/us-3808
Merge into: lp:~julie-w/unifield-server/US-3808
Diff against target: 63 lines (+11/-20)
1 file modified
bin/addons/msf_partner/partner.py (+11/-20)
To merge this branch: bzr merge lp:~jfb-tempo-consulting/unifield-server/us-3808
Reviewer Review Type Date Requested Status
Julie Nuguet 2018-02-02 Pending
Review via email: mp+337076@code.launchpad.net
To post a comment you must log in.
jftempo (jfb-tempo-consulting) wrote :

Julie,

Here is a proposal to change the code:
  - test condition after write / creation so we don't have to play with vals['address'][0][2]
  - do not change the context (active_test) as it can be used in the following code.

Do you agree ?

jftempo (jfb-tempo-consulting) wrote :

Last point: the check is done on creation (as requested in the ticket), but also on modification.
It can result in an error msg if a process tries to write something in a duplicated partner (for example Action "Reconciliation: Go to Next Partner" from JI form view).

Unmerged revisions

4701. By jftempo on 2018-02-02

US-3808 [IMP] Do not change context + test condition after creation

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bin/addons/msf_partner/partner.py'
2--- bin/addons/msf_partner/partner.py 2018-01-26 08:50:13 +0000
3+++ bin/addons/msf_partner/partner.py 2018-02-02 14:43:05 +0000
4@@ -594,7 +594,7 @@
5 +[_('%s (Journal Item)') % (aml['move_id'] and aml['move_id'][1] or '') for aml in aml_obj.read(cr, uid, aml_ids, ['move_id'])]
6 )
7
8- def check_partner_unicity(self, cr, uid, vals, partner_id=None, context=None):
9+ def check_partner_unicity(self, cr, uid, partner_id, context=None):
10 """
11 If the partner name is already used, check that the city is not empty AND not used by another partner with the
12 same name. Checks are case insensitive, done with active and inactive partners, and NOT done at synchro time.
13@@ -602,21 +602,10 @@
14 if context is None:
15 context = {}
16 if not context.get('sync_update_execution'):
17- old_partner = None
18- partner_name = city = ''
19- if partner_id:
20- old_partner = self.browse(cr, uid, partner_id, fields_to_fetch=['name', 'city'], context=context)
21- if 'name' in vals:
22- partner_name = vals.get('name', '')
23- elif old_partner:
24- partner_name = old_partner.name
25- if 'address' in vals:
26- city = len(vals['address']) == 1 and len(vals['address'][0]) == 3 and vals['address'][0][2].get('city') or ''
27- elif old_partner:
28- city = old_partner.city or ''
29- partner_domain = partner_id and [('id', '!=', partner_id)] or []
30- partner_domain.append(('name', '=ilike', partner_name))
31- context.update({'active_test': False})
32+ old_partner = self.browse(cr, uid, partner_id, fields_to_fetch=['name', 'city'], context=context)
33+ partner_name = old_partner.name
34+ city = old_partner.city or ''
35+ partner_domain = [('id', '!=', partner_id), ('name', '=ilike', partner_name), ('active', 'in', ['t', 'f'])]
36 duplicate_partner_ids = self.search(cr, uid, partner_domain, order='NO_ORDER', context=context)
37 if duplicate_partner_ids:
38 if not city or self.search_exist(cr, uid, [('id', 'in', duplicate_partner_ids), ('city', '=ilike', city)], context=context):
39@@ -660,9 +649,10 @@
40
41 if vals.get('name'):
42 vals['name'] = vals['name'].strip()
43+ ret = super(res_partner, self).write(cr, uid, ids, vals, context=context)
44 for partner_id in ids:
45- self.check_partner_unicity(cr, uid, vals, partner_id=partner_id, context=context)
46- return super(res_partner, self).write(cr, uid, ids, vals, context=context)
47+ self.check_partner_unicity(cr, uid, partner_id=partner_id, context=context)
48+ return ret
49
50 def create(self, cr, uid, vals, context=None):
51 if context is None:
52@@ -691,8 +681,9 @@
53
54 if vals.get('name'):
55 vals['name'] = vals['name'].strip()
56- self.check_partner_unicity(cr, uid, vals, context=context)
57- return super(res_partner, self).create(cr, uid, vals, context=context)
58+ new_id = super(res_partner, self).create(cr, uid, vals, context=context)
59+ self.check_partner_unicity(cr, uid, partner_id=new_id, context=context)
60+ return new_id
61
62
63 def copy_data(self, cr, uid, id, default=None, context=None):

Subscribers

People subscribed via source and target branches

to all changes: