Merge lp:~vauxoo/openobject-server/7.0-custom_domain_method into lp:openobject-server/7.0

[Jose Antonio Morales Ponce(vauxoo) - - http://www.vauxoo.com]

If you try to do a custom domain from the field definition(domain=custom_method) is complicated because when the method is called only sent the model object and not sent the cursor and the user for this moment, principal parameters to do everything is why that I modified the method to send diferent values when a domain is sent because they only use a custom domain in mail module and only they need the main object using an anonymous function (lambda)

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

Hello Jose,

Thank you for proposing this improvement to OpenERP!
I'm afraid we'll have to decline it because this was not the intended behavior for callable o2m domains.

Indeed this feature was introduced in v7 (here: [1]) for the only purpose of supporting static domains on inherited o2m fields where the reverse foreign key is not a true foreign key.
And you're right, this is used by the mail.thread model which is abstract and inherited by many models, and whose `message_ids` field has a composite reverse foreign key in the mail.message table: (res_model, res_id). So this is the reason why the `domain` callable will only receive the model as an argument. It should only be used for similar purposes.

We don't want to allow more complex cases because developers will start to abuse them ;-) This is very frequent in the framework, and then the developers are disappointed when that abusive behavior gets broken by a future patch, because it was not supposed to work. For example if a context is provided in a method, developers will always find a way to implement context-sensitive logic even in places where it should not happen, like workflows or _constraints.

In general you should only use framework features in the way they are used by official addons, unless explicitly documented. If you think you need this feature but you are not in the same case as mail.thread, you should think about a different model/design to accomplish what you need. Otherwise you're probably going to meet other problems in the long run, and this patch is not the solution.

I hope you understand this explanation.

In addition, I have a few technical remarks about your patch:
- This is a wishlist API change, so even if backwards-compatible it will never be accepted in a stable version. You should have proposed it for trunk, not 7.0.
- Your implementation accepts callables with different signatures. This is an anti-pattern and OpenERP is already plagued with such horrors, like the read() and browse() methods that return different types based on the IDs/ID passed. This forces *every* piece of code that deals with that API to check the actual signature, adding a lot of useless code everywhere. We would like to avoid this in the future.
- You only patched 1 out of the 3 places where domain callables are managed, so this patch will not properly work. Search fields.py for 'self._domain(' to see all of them.

Thanks!

[1] server 7.0 revno 4333.1.3 revid:<email address hidden>

review: Disapprove

Unmerged revisions

5093. By Jose Antonio Morales Ponce(vauxoo) - - http://www.vauxoo.com on 2013-10-04

[IMP] Send the principal parameters to make an intelligent domain calling a custom method

=== modified file 'openerp/osv/fields.py'
--- openerp/osv/fields.py	2013-06-20 13:10:57 +0000
+++ openerp/osv/fields.py	2013-10-04 02:59:09 +0000
@@ -50,6 +50,7 @@
 from openerp.tools import html_sanitize
 import simplejson
 from openerp import SUPERUSER_ID
+import inspect
 
 _logger = logging.getLogger(__name__)
 
@@ -1525,7 +1526,23 @@
             res['selection'] = field.selection(model, cr, user, context)
     if res['type'] in ('one2many', 'many2many', 'many2one'):
         res['relation'] = field._obj
-        res['domain'] = field._domain(model) if callable(field._domain) else field._domain
+
+        if callable(field._domain):
+            elements = inspect.getargspec(field._domain)                                  
+            elements = elements.args                                                                   
+
+            if len(elements) == 1:
+                res['domain'] = field._domain(model)
+
+            elif len(elements) > 2:
+                res['domain'] = field._domain(model, cr, user)
+
+            else:
+                res['domain'] = field._domain
+            
+        else:
+            res['domain'] = field._domain
+
         res['context'] = field._context
 
     if isinstance(field, one2many):