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

Proposed by jftempo
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 Pending
Review via email: mp+337076@code.launchpad.net
To post a comment you must log in.
Revision history for this message
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 ?

Revision history for this message
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).

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'bin/addons/msf_partner/partner.py'
--- bin/addons/msf_partner/partner.py 2018-01-26 08:50:13 +0000
+++ bin/addons/msf_partner/partner.py 2018-02-02 14:43:05 +0000
@@ -594,7 +594,7 @@
594 +[_('%s (Journal Item)') % (aml['move_id'] and aml['move_id'][1] or '') for aml in aml_obj.read(cr, uid, aml_ids, ['move_id'])]594 +[_('%s (Journal Item)') % (aml['move_id'] and aml['move_id'][1] or '') for aml in aml_obj.read(cr, uid, aml_ids, ['move_id'])]
595 )595 )
596596
597 def check_partner_unicity(self, cr, uid, vals, partner_id=None, context=None):597 def check_partner_unicity(self, cr, uid, partner_id, context=None):
598 """598 """
599 If the partner name is already used, check that the city is not empty AND not used by another partner with the599 If the partner name is already used, check that the city is not empty AND not used by another partner with the
600 same name. Checks are case insensitive, done with active and inactive partners, and NOT done at synchro time.600 same name. Checks are case insensitive, done with active and inactive partners, and NOT done at synchro time.
@@ -602,21 +602,10 @@
602 if context is None:602 if context is None:
603 context = {}603 context = {}
604 if not context.get('sync_update_execution'):604 if not context.get('sync_update_execution'):
605 old_partner = None605 old_partner = self.browse(cr, uid, partner_id, fields_to_fetch=['name', 'city'], context=context)
606 partner_name = city = ''606 partner_name = old_partner.name
607 if partner_id:607 city = old_partner.city or ''
608 old_partner = self.browse(cr, uid, partner_id, fields_to_fetch=['name', 'city'], context=context)608 partner_domain = [('id', '!=', partner_id), ('name', '=ilike', partner_name), ('active', 'in', ['t', 'f'])]
609 if 'name' in vals:
610 partner_name = vals.get('name', '')
611 elif old_partner:
612 partner_name = old_partner.name
613 if 'address' in vals:
614 city = len(vals['address']) == 1 and len(vals['address'][0]) == 3 and vals['address'][0][2].get('city') or ''
615 elif old_partner:
616 city = old_partner.city or ''
617 partner_domain = partner_id and [('id', '!=', partner_id)] or []
618 partner_domain.append(('name', '=ilike', partner_name))
619 context.update({'active_test': False})
620 duplicate_partner_ids = self.search(cr, uid, partner_domain, order='NO_ORDER', context=context)609 duplicate_partner_ids = self.search(cr, uid, partner_domain, order='NO_ORDER', context=context)
621 if duplicate_partner_ids:610 if duplicate_partner_ids:
622 if not city or self.search_exist(cr, uid, [('id', 'in', duplicate_partner_ids), ('city', '=ilike', city)], context=context):611 if not city or self.search_exist(cr, uid, [('id', 'in', duplicate_partner_ids), ('city', '=ilike', city)], context=context):
@@ -660,9 +649,10 @@
660649
661 if vals.get('name'):650 if vals.get('name'):
662 vals['name'] = vals['name'].strip()651 vals['name'] = vals['name'].strip()
652 ret = super(res_partner, self).write(cr, uid, ids, vals, context=context)
663 for partner_id in ids:653 for partner_id in ids:
664 self.check_partner_unicity(cr, uid, vals, partner_id=partner_id, context=context)654 self.check_partner_unicity(cr, uid, partner_id=partner_id, context=context)
665 return super(res_partner, self).write(cr, uid, ids, vals, context=context)655 return ret
666656
667 def create(self, cr, uid, vals, context=None):657 def create(self, cr, uid, vals, context=None):
668 if context is None:658 if context is None:
@@ -691,8 +681,9 @@
691681
692 if vals.get('name'):682 if vals.get('name'):
693 vals['name'] = vals['name'].strip()683 vals['name'] = vals['name'].strip()
694 self.check_partner_unicity(cr, uid, vals, context=context)684 new_id = super(res_partner, self).create(cr, uid, vals, context=context)
695 return super(res_partner, self).create(cr, uid, vals, context=context)685 self.check_partner_unicity(cr, uid, partner_id=new_id, context=context)
686 return new_id
696687
697688
698 def copy_data(self, cr, uid, id, default=None, context=None):689 def copy_data(self, cr, uid, id, default=None, context=None):

Subscribers

People subscribed via source and target branches

to all changes: