Code review comment for lp:~openerp-dev/openobject-server/7.0-opw-591308-jam

Hello,

I agree with the patch, we indeed need to make sure that the company of the partner from which each user inherits is the same company as the company of the user.

This is a bit strange because the user may change their company from their preferences at any time, and this will also propagate the change to the partner record, but there's no easy way to prevent that. Changing the record rules like suggested in comment #68 of bug 1073087 would help but it would not fix 100% cases, for example there are cases where the partner company is used indirectly (e.g. for mail messages etc.)

I have a few functional and technical remarks about the patch:

Functionally
------------
- We should allow setting the company_id of the partner to False in order to make the partner shared between all companies (the company_id field is not required on res.partner). So res_users.write() must only write to the partner if `user.partner_id.company_id and user.partner_id.company_id != new_company_id`. That way we allow keeping partner.company_id to False.
- The synchronization must be managed carefully from the partner side as well. The write() method of res.partner must only allow to set the company_id of a partner if it is the same as the company of all users that inherit from this partner (this is to allow the code from res_users to write to the partner!) or if setting the company_id to False (this is compatible with any user company).
- This code does not take care of fixing all existing records which have a problem. We should also provide SQL queries to execute on customer databases to fix the existing records:

  -- To show the partners with this issue
  SELECT p.id AS partner_id, p.name, p.company_id AS partner_company, u.company_id AS user_company
  FROM res_partner p JOIN res_users u ON (u.partner_id = p.id)
  WHERE p.company_id IS NOT NULL AND p.company_id != u.company_id;

  -- To fix the partners
  UPDATE res_partner p SET company_id = u.company_id
  FROM res_users u
  WHERE u.partner_id = p.id AND p.company_id IS NOT NULL;

Technically
-----------
- l.11-13, 25-28: simplify code: using a browse record you can make the code much simpler, and there is really no need to call _get_company:
    user = self.browse(cr, uid, user_id, context=context)
    if user.partner_id.company_id: # if partner is global we keep it that way
        user.partner_id.write({'company_id': user.company_id.id})

Thanks!