Merge lp:~openerp-community/openobject-server/stefan-therp_lp891592-6.1 into lp:openobject-server

[Stefan Rijnhart (Opener)]

This branch passes the context to the _constraints functions on the model.

[Vo Minh Thu (thu)]

This was already proposed and rejected. See https://code.launchpad.net/~openerp-groupes/openobject-server/trunk-context-constraints/+merge/62285

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

On 11/17/2011 02:21 PM, Vo Minh Thu (OpenERP) wrote:
> This was already proposed and rejected.

To elaborate a little bit, this was considered dangerous because:
- most existing constraints do not take a context parameter (logically)
- but most importantly, allowing constraints to be context-dependent
seems wrong, and would encourage bad design practices

It should also not be needed for translation purposes, since the error
message is already translated by the ORM (or can be a callable that
*will* receive the context).

[Raphaël Valyi - http://www.akretion.com (rvalyi)]

I understand why you reject that raw constraints don't use context.
Now, it would be good if OpenERP would have the equivalent of something
like Rails activemodels lifecycle methods such as:

   - validate
   - before_validate
   - after_validate

( see http://api.rubyonrails.org/classes/ActiveRecord/Callbacks.html we
would need some, no necessarily all)

Indeed, today, it's really cumbersome to find a place where we are sure to
pass before saving and object and where we can access the edition context.
We all agree here the edition context can matters don't we?
We are sure (are we?) to pass in the contraints system indeed but as
you highlighted here no context is passed (and we can understand that at
this level).

Then a immediate idea is to override write and create of the object. being
edited.
But this prove to be extremely ugly and crappy in practice. Here is why:
you should override both create and write, eventually calling into the same
validation method, but still.

But there is more: imagine you override the create/write of a sale order
line.
Actually, if you edit a sale order (at least in the GTK client) and create
some order lines inline, then create and write will not be called at the
order line level!!
Only create or write from the sale order will be called.

It means that you should actually ALSO override create and write of the
sale order object!
That's now 4 methods to be overriden o do the same context sensitive check!
And if that object belongs to more objects through some one2many nesting,
well the list keep increasing.

So I want to draw your attention about this. I had to do extremely
ugly/brittle workarounds for some customers on 6.0 because of that. In any
case if you compare to something like Rails this kind of things make the
OpenERP framework extremely slow and frustrating to catch up (translate:
your are not going to meet mass success with OpenERP is this stage of
maturity), so I encourage to do try doing something about that. And I agree
that this need to be at a different level than _constraints which is
reasonable to declare as only data dependent.

Regards.

On Thu, Nov 17, 2011 at 12:31 PM, Olivier Dony (OpenERP) <email address hidden>wrote:

> On 11/17/2011 02:21 PM, Vo Minh Thu (OpenERP) wrote:
> > This was already proposed and rejected.
>
> To elaborate a little bit, this was considered dangerous because:
> - most existing constraints do not take a context parameter (logically)
> - but most importantly, allowing constraints to be context-dependent
> seems wrong, and would encourage bad design practices
>
> It should also not be needed for translation purposes, since the error
> message is already translated by the ORM (or can be a callable that
> *will* receive the context).
>
> --
>
> https://code.launchpad.net/~openerp-community/openobject-server/stefan-therp_lp891592-6.1/+merge/82529
> Your team OpenERP Community is subscribed to branch
> lp:~openerp-community/openobject-server/stefan-therp_lp891592-6.1.
>
> _______________________________________________
> Mailing list: https://launchpad.net/~openerp-community
> Post to : <email address hidden>
> Unsubscribe : https://launchpad.net/~opener...

Unmerged revisions

3810. By Stefan Rijnhart (Opener) on 2011-11-17

[FIX] pass context to _constraint functions

=== modified file 'openerp/osv/orm.py'
--- openerp/osv/orm.py	2011-11-16 11:38:58 +0000
+++ openerp/osv/orm.py	2011-11-17 13:09:19 +0000
@@ -1411,7 +1411,7 @@
         error_msgs = []
         for constraint in self._constraints:
             fun, msg, fields = constraint
-            if not fun(self, cr, uid, ids):
+            if not fun(self, cr, uid, ids, context):
                 # Check presence of __call__ directly instead of using
                 # callable() because it will be deprecated as of Python 3.0
                 if hasattr(msg, '__call__'):