[5.0] Regression in base_contact: revno 2568 breaks res.partner.job search (2010-02-05)

[Borja López Soilán (NeoPolus)]

The revno 2568 (<email address hidden>, see http://bazaar.launchpad.net/~openerp/openobject-addons/5.0/revision/2568) breaks the search method of res.partner.job.

(Note: This bug is present on the 5.0.7 release)

On revno 2551, the search method of the res.partner.job object was extended on base_contact to allow searching by contact name/first_name and introduced one bug (if order is 'sequence_contact' or 'sequence_partner' it breaks). Later the revno 2568 removed one line from that method breaking it a bit more.

The search method looks like this:

*****************
def search(self, cr, user, args, offset=0, limit=None, order=None, context=None, count=False):
        job_ids = []
        for arg in args:
            if arg[0] == 'address_id':
                self._order = 'sequence_partner'
            elif arg[0] == 'contact_id':
                self._order = 'sequence_contact'
                                     <<<<<<<<<<<<< MISSING elif arg[0] == 'name': !!!!!!!!! >>>>>>>>>>>>>><
                contact_obj = self.pool.get('res.partner.contact')
                search_arg = ['|', ('first_name', 'ilike', arg[2]), ('name', 'ilike', arg[2])]
                contact_ids = contact_obj.search(cr, user, search_arg, offset=offset, limit=limit, order=order, context=context, count=count)
                contacts = contact_obj.browse(cr, user, contact_ids, context=context)
                for contact in contacts:
                    job_ids.extend([item.id for item in contact.job_ids])

        res = super(res_partner_job,self).search(cr, user, args, offset=offset, limit=limit, order=order, context=context, count=count)
        if job_ids:
            res = list(set(res + job_ids))
*****************

Without the "elif arg[0] == 'name':" line, the search on the contacts will be invalid, as it will be searching for an id (a contact id) on the name/first name fields!

But also, if the specified order field is on res_partner_job, but not on res_partner_contact (like 'sequence_contact'), the search will fail too (no field sequence_contact on res_parner_contact table) cause the contact search uses the same order fields of the parent (job) search!

[Stephane Wirtel (OpenERP) (stephane-openerp)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I think that the code is wrong because the 'name' field of res.partner.job is a res.partner :/

We can override the search and use a key in the context or replace the name_search.

To check

- --
Stephane Wirtel - "As OpenERP is OpenSource, please feel free to contribute."
Quality/Release Manager
Technical Project Manager
Tiny SPRL
40, Chaussee de Namur
B-1367 Grand-Rosiere
* Tel: [phone]
* Fax: [phone]
* Web: http://www.openerp.com
* Planet: http://www.openerp.com/planet/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkuNSzsACgkQnqh8s27uQNW4hACgqo7klkfwNTPWDksgFp8+2sgI
8BMAn3tHVZyeLCFyWcByPIVZbU7NbiNV
=BCtP
-----END PGP SIGNATURE-----

[Borja López Soilán (NeoPolus) (borjals)]

Stephane, they where trying to extend the search so you could search by the contact name or first name.

But yes, that also meant that you can't longer search res.partner.job on some res.partner (cause the field name was used to store the partner id! That means that it will search on the contact's name with the partner id).

I propose this:
  - Revert the search method to the old version (revert changesets 2551 and 2568).
  - Add, to res.partner.job, two related or function fields, with store (so we can search in SQL), to the contact name and last name.
               'contact_name': fields.related('contact_id', 'name', type='char', size=30, string='Last Name'),
               'contact_first_name': fields.related('contact_id', 'first_name', type='char', size=30, string='First Name'),

And I repeat, in the released 5.0.7 version the res.parner.job search is severely broken:
  - From revno 2551 to revno 2567 you can't search by partner id properly (it searches on contact name with the partner id).
  - Since revno 2568 you can't search by contact_id (gives error).

[Anup(SerpentCS) (anup-serpent)]

Hello Borja,

    The code seems to be working absolutely perfect as normally it does on the revision 2568. As you are talking about searching by partner_id it searches perfectly. As whenever a search is made by partner_id it directly calls the super class method using the same args without changing them. In the previous revisions it was giving the partner associated with the contact but now it searches by the partner_id itself.

   In case of searching by contact_id the contact_id is a merged string of 3 different fields i.e. title name and first name. In the previous revisions it was not possible to search the contact by its 'name' (Last Name). So that is solved by the revision 2568.

   I also checked by searching the fields by sequence_contact and sequence_partner and they are also working as required.

   I would like to request you to check it once again and if you still face the problems let us notify about it. I am closing the bug as it has been checked various times and we failed to meet to the error.

   kindly reopen the bug if still it appears or feel free to ask more.

Thank you

[Omar (Pexego) (omar7r)]

Hi,

I reopen the bug because, in my opinion, res.partner.contact should have a name_search method defined and the behavior will be the same, but with improvements because, if the args in search method were:

[('contact_id', 'in', [2560, 2561, 2562, 2563, 2564, 2565, 2566, 2567, 2568, 2569, 2570, 2571, 2572, 2573, 2574, 2575, 2576, 2577, 2578, 2579, 2580, 2581, 2582, 2583, 2584, 2585, 2586, 2587, 2588, 2589, 2590, 2591, 2592, 2593, 20536, 2366, 2367, 2368, 2369, 2370, 2371, 2372, 2373, 2374, 2375, 2376, 2377, 2378, 2379, 2380, 2381, 2382, 2383, 2384, 2385, 2386, 2387, 2388, 2389, 2390, 2391,...])]

The result query was:

SELECT res_partner_contact.id FROM "res_partner_contact" WHERE (res_partner_contact.active = E'True') AND ((res_partner_contact.first_name ilike E'%[2560, 2561, 2562, 2563, 2564, 2565, 2566, 2567, 2568, 2569, 2570, 2571, 2572, 2573, 2574, 2575, 2576, 2577, 2578, 2579, 2580, 2581, 2582, 2583, 2584, 2585, 2586, 2587, 2588, 2589, 2590, 2591, 2592, 2593, 20536, 2366, 2367, 2368, 2369, 2370, 2371, 2372, 2373, 2374, 2375, 2376, 2377, 2378, 2379, 2380, 2381, 2382, 2383, 2384, 2385, 2386, 2387, 2388, 2389, 2390, 2391,..]%') OR (res_partner_contact.name ilike E'%[2560, 2561, 2562, 2563, 2564, 2565, 2566, 2567, 2568, 2569, 2570, 2571, 2572, 2573, 2574, 2575, 2576, 2577, 2578, 2579, 2580, 2581, 2582, 2583, 2584, 2585, 2586, 2587, 2588, 2589, 2590, 2591, 2592, 2593, 20536, 2366, 2367, 2368, 2369, 2370, 2371, 2372, 2373, 2374, 2375, 2376, 2377, 2378, 2379, 2380, 2381, 2382, 2383, 2384, 2385, 2386, 2387, 2388, 2389, 2390, 2391,...]%')) ORDER BY id;

This query will be unnecessary, because I search for contacts by id, then the search method does a second query with super search and it does a set of two results.

Second query was:

select res_partner_job.id from "res_partner_job" where (res_partner_job.contact_id in (2560, 2561, 2562, 2563, 2564, 2565, 2566, 2567, 2568, 2569, 2570, 2571, 2572, 2573, 2574, 2575, 2576, 2577, 2578, 2579, 2580, 2581, 2582, 2583, 2584, 2585, 2586, 2587, 2588, 2589, 2590, 2591, 2592, 2593, 20536, 2366, 2367, 2368, 2369, 2370, 2371, 2372, 2373, 2374, 2375, 2376, 2377, 2378, 2379, 2380, 2381, 2382, 2383, 2384, 2385, 2386, 2387, 2388, 2389, 2390, 2391,...)) order by sequence_contact

It is very slow. I analyze it with pgfouine only the first query. See the result:

SELECT res_partner_contact.id FROM "res_partner_contact" WHERE (res_partner_contact.active = E'') AND ((res_partner_contact.first_name ilike E'') OR (res_partner_contact.name ilike E'')) ORDER BY id;

Av. duration (s): 0.89
Times executed: 50
Total duration: 44.3s

Very Slow.

I think that the search method should be erased because with a name_search in contact, it is very fast because don't do this behavior the name_search should be like this:

def name_search(self, cr, uid, name='', args=None, operator='ilike', context=None, limit=None):
        args = args or []
        if name:
            ids = []
            ids.extend(self.search(cr, uid, ['|',('name', operator, name),('first_name', operator, name)] + args, limit=limit, context=context or {}))
        else:
            ids = self.search(cr, uid, args, limit=lim...

[Anup(SerpentCS) (anup-serpent)]

Hello Omar,

    It has been fixed by revision 2862 <email address hidden>

Thanks a lot for pointing towards the hidden issue. We really appreciate that.

[Borja López Soilán (NeoPolus) (borjals)]

Thanks to you Anup :)