Merge lp:~numerigraphe/openobject-server/trunk-partner-category-short-name into lp:openobject-server

[Numérigraphe]

This branch was recently merged to allow other modules to get the category's short name.
Unfortunatly this introduces a recursion bug, so please let me resubmit this branch with a fix for this last issue.
Lionel Sausin.

[Olivier Dony (Odoo) (odo-openerp)]

Hi Lionel,

I agree with the idea of making the name_get of Partner Categories configurable with the context, pretty much like what we already do with Partner Addresses.

However, in order to be consistent I'd suggest using a similar convention for the context keys to what we have for addresses:
    {'partner_category_display': 'short'}

Also, wouldn't it be better to delegate the name_get to super() as soon as the short format is enabled, rather than making the current code more complex?

Finally, as for turning this on the default partner action, I'd suggest keeping this in extra modules where it's used, because this would have no visible effect for the standard addons (category m2m and list display the full category name anyway), so it makes little sense, so it could be removed during a future cleanup anyway.

Instead of asking you to fix the above I did it on the fly while merging, I hope you won't mind ;-) I also adapted the docstring to use our new preferred convention for documenting context options.
Your idea has been preserved and you should be able to use it as you planned in extra modules - and it's credited to you of course.

Thanks for your excellent contributions, as usual!

review: Approve (tech details to be fixed while merging)

[Numérigraphe (numerigraphe)]

Many thanks for your review and improvements.
Lionel Sausin.

[Olivier Dony (Odoo) (odo-openerp)]

Hi Lionel,

Sorry for the late reply, would you mind sharing an example or a traceback of the recursion, and how you triggered it?
On first look the call to super().name_get() should never cause a recursion, unless super().name_get() [in]directly calls read() on the 'complete_name' field. And this is not the case, because _rec_name is the default 'name', not 'complete_name'. Or does this occur only if you install some specific module?

Hope I haven't missed anything obvious...

Thanks!

review: Needs Information

[Numérigraphe (numerigraphe)]

Dear Olivier,
Thanks for your attention.
I don't think I made anything special...
In the module l10n_fr_naf_ape from lp:openerp-nomenclatures, I used the context in the field "APE" on the partner form:
    <field name="ape_id" context="{'partner_category_display': 'short'}" domain="[('parent_id', 'child_of', 'NACE')]"/>

When I press F2 on this field I get this traceback:
"Environment Information :
System : Linux-3.0.0-14-generic-x86_64-with-Ubuntu-11.10-oneiric
OS Name : posix
Distributor ID: Ubuntu
Description: Ubuntu 11.10
Release: 11.10
Codename: oneiric
Operating System Release : 3.0.0-14-generic
Operating System Version : #23-Ubuntu SMP Mon Nov 21 20:28:43 UTC 2011
Operating System Architecture : 64bit
Operating System Locale : fr_FR.UTF-8
Python Version : 2.7.2+
OpenERP-Client Version : 6.1beta
Last revision No. & ID :2029 launchpad_translations_on_behalf_of_openerp-20120109044959-tc9h2xlv1507htn3
Traceback (most recent call last):
  File "/home/ls/Sources/openerp/trunk/server/openerp/service/netrpc_server.py", line 62, in run
    result = netsvc.dispatch_rpc(msg[0], msg[1], msg[2:])
  File "/home/ls/Sources/openerp/trunk/server/openerp/netsvc.py", line 325, in dispatch_rpc
    result = ExportService.getService(service_name).dispatch(method, params)
  File "/home/ls/Sources/openerp/trunk/server/openerp/service/web_services.py", line 580, in dispatch
    res = fn(db, uid, *params)
  File "/home/ls/Sources/openerp/trunk/server/openerp/osv/osv.py", line 120, in wrapper
    return f(self, dbname, *args, **kwargs)
  File "/home/ls/Sources/openerp/trunk/server/openerp/osv/osv.py", line 175, in execute
    res = self.execute_cr(cr, uid, obj, method, *args, **kw)
  File "/home/ls/Sources/openerp/trunk/server/openerp/osv/osv.py", line 163, in execute_cr
    return getattr(object, method)(cr, uid, *args, **kw)
  File "/home/ls/Sources/openerp/trunk/server/openerp/addons/base/res/res_partner.py", line 72, in name_search
    ids = self.search(cr, uid, args, limit=limit, context=context)
  File "/home/ls/Sources/openerp/trunk/server/openerp/osv/orm.py", line 2230, in search
    return self._search(cr, user, args, offset=offset, limit=limit, order=order, context=context, count=count)
  File "/home/ls/Sources/openerp/trunk/server/openerp/osv/orm.py", line 4533, in _search
    query = self._where_calc(cr, user, args, context=context)
  File "/home/ls/Sources/openerp/trunk/server/openerp/osv/orm.py", line 4382, in _where_calc
    e = expression.expression(cr, user, domain, self, context)
  File "/home/ls/Sources/openerp/trunk/server/openerp/osv/expression.py", line 358, in __init__
    self.parse(cr, uid, distribute_not(normalize(exp)), table, context)
  File "/home/ls/Sources/openerp/trunk/server/openerp/osv/expression.py", line 570, in parse
    ids2 = to_ids(right, field_obj)
  File "/home/ls/Sources/openerp/trunk/server/openerp/osv/expression.py", line 403, in to_ids
    for n in names])
  File "/home/ls/Sources/openerp/trunk/server/openerp/addons/base/res/res_partner.py", line 73, in name_search
    return self.name_get(cr, uid, ids, context)
  File "/home/ls/Sources/openerp/trunk/server/openerp/addons/base/res/res_partner.py", line 52, in name_...

[Olivier Dony (Odoo) (odo-openerp)]

On 01/11/2012 06:47 PM, Numérigraphe wrote:
> File "/home/ls/Sources/openerp/trunk/server/openerp/osv/expression.py", line 403, in to_ids
> for n in names])
> File "/home/ls/Sources/openerp/trunk/server/openerp/addons/base/res/res_partner.py", line 73, in name_search
> return self.name_get(cr, uid, ids, context)
> File "/home/ls/Sources/openerp/trunk/server/openerp/addons/base/res/res_partner.py", line 52, in name_get
> return super(res_partner_category, self).name_get(cr, uid, ids, context=context)
> File "/home/ls/Sources/openerp/trunk/server/openerp/addons/base/res/res_partner.py", line 52, in name_get
> return super(res_partner_category, self).name_get(cr, uid, ids, context=context)
> (...repeated many times...)

This is really fishy, because if you actually trace through these calls
you'll see that super(res_partner_category, self) actually resolves to
``self`` -- that's not supposed to happen!

In fact, the reason for this strange behavior is the presence of a
second class with an identical name at the bottom of the file. This
second class '_inherit's the first one, so when Python is done loading
the module, the name "res_partner_category" refers to the last one
instead. Later the super() call dynamically looks for the parent of
"res_partner_category", i.e. the parent of its child: self!

The second class was originally created due to the circular reference
(partner->category->partner). Such cycles were not supported using a
single declaration in 6.0 and earlier. But of course they should have
used different Python class names.
As of 6.1 the framework loads the models in two passes to allow circular
references, so the easiest and cleanest way to fix this is to merge the
two classes in one, which is what I did at r.3958 rev-id
<email address hidden>.

If someone wanted to fix the same thing in 6.0, it would be as simple as
renaming the second class to avoid the name conflict.

Thanks for your feedback and help in solving this tricky issue!

[Numérigraphe (numerigraphe)]

Dear Olivier,
Thanks for your work on this, it works with the change you suggest.
Lionel Sausin.

Unmerged revisions

3881. By Numerigraphe - Lionel Sausin <email address hidden> on 2012-01-04

[FIX] Stop recursion when using partner category name with context['partner_category_display']='short'

3880. By Numerigraphe - Lionel Sausin <email address hidden> on 2012-01-04

[MERGE] upstream updates, including this branch with improvements

=== modified file 'openerp/addons/base/res/res_partner.py'
--- openerp/addons/base/res/res_partner.py	2012-01-04 10:14:44 +0000
+++ openerp/addons/base/res/res_partner.py	2012-01-04 11:21:23 +0000
@@ -49,7 +49,10 @@
         if context is None:
             context = {}
         if context.get('partner_category_display') == 'short':
-            return super(res_partner_category, self).name_get(cr, uid, ids, context=context)
+            # Remove the key from the context to stop recursion 
+            stripped_context = context.copy()
+            del stripped_context['partner_category_display']
+            return super(res_partner_category, self).name_get(cr, uid, ids, context=stripped_context)
         reads = self.read(cr, uid, ids, ['name','parent_id'], context=context)
         res = []
         for record in reads: