Merge lp:~openerp-community/openobject-server/fix-1094212-multicompany-res_partner into lp:openobject-server/7.0

[Niels Huylebroeck]

The problem:

The result of cr.execute is bad (why hasn't everyone at openerp been brainwashed yet?) because it will contain results not appropriate for the current user (because of un-applied record rules)

The solution:

After we fetched the result cr.execute I now force the search to be executed (unconditionally) which is no problem even if "args" was not passed (see beginning of function where it is set to [] if it was None). Doing this search it will pass all the ids we have found so far but will also apply the required record rules, thus filtering out the unreadable partner ids in the process.

[Fabien (Open ERP) (fp-tinyerp)]

I guess a clean solution would be to completly replace the cr.execute by a self.search, instead of doing two searches. It's also important for performance issues.

review: Needs Fixing

[Niels Huylebroeck (red15)]

I agree but the current state of affair was that it didn't work for
multicompany setup AND there were already 2 search queries most of the time
anyway (plenty of arguments being passed).

In fact by removing that if statement you could reason I saved time on the
python side, because that check was (almost) redundant anyway.

[Cloves Almeida (cjalmeida)]

A solution to give an order of magnitude performance speed would be:

a) create a "name_with_company" stored function field on res_partner to avoid the join;

b) create an expression index[1] on "lower(name_with_company)";

c) change the query to use ('lower(%s) like lower(name_with_company)') for ("name","ilike") name_search (about 99% of the case)

d) keep the self.search to apply the record rules - the IN on the database side is a cheap query.

[1] http://www.postgresql.org/docs/9.1/static/indexes-expressional.html

[Stefan Rijnhart (Opener) (stefan-opener)]

This MP is set to merged but it is not really. http://bazaar.launchpad.net/~openerp/openobject-server/7.0/revision/4786

[Stefan Rijnhart (Opener) (stefan-opener)]

Pointing to revision 4786 because that is what the MP said that it was merged at before I changed its status. Looks like someone merged the branch but then made manual changes. The effect is that if you try to merge the branch now, bzr says 'Nothing to do'. But the faulty line is still there: http://bazaar.launchpad.net/~openerp/openobject-server/7.0/view/4786/openerp/addons/base/res/res_partner.py#L466

[Niels Huylebroeck (red15)]

Yes, it seem Fabien himself has merged this branch but forgot to revert it before working on something else (the document management apparently)

This branch from Fabien was then merged by Thibault into the v7.0 branch.

http://imgur.com/kOGTTIM

[lp:~openerp-community/openobject-server/fix-1094212-multicompany-res_partner]

4762. By Niels Huylebroeck on 2013-02-25

[FIX] base: allow searching in multicompany environment on res_partner.

[Niels Huylebroeck (red15)]

Re-pushed with a new unique id to allow real merging this time.

=== modified file 'openerp/addons/base/res/res_partner.py'
--- openerp/addons/base/res/res_partner.py	2013-01-14 17:43:55 +0000
+++ openerp/addons/base/res/res_partner.py	2013-02-25 14:00:33 +0000
@@ -463,8 +463,7 @@
                              OR partner.name || ' (' || COALESCE(company.name,'') || ')'
                           ''' + operator + ' %(name)s ' + limit_str, query_args)
             ids = map(lambda x: x[0], cr.fetchall())
-            if args:
-                ids = self.search(cr, uid, [('id', 'in', ids)] + args, limit=limit, context=context)
+            ids = self.search(cr, uid, [('id', 'in', ids)] + args, limit=limit, context=context)
             if ids:
                 return self.name_get(cr, uid, ids, context)
         return super(res_partner,self).name_search(cr, uid, name, args, operator=operator, context=context, limit=limit)