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

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Stefan Rijnhart (Therp)
Stefan Rijnhart (Therp) has proposed merging lp:~openerp-community/openobject-server/stefan-therp_lp891592-6.1 into lp:openobject-server.

Requested reviews:
  OpenERP Core Team (openerp)
Related bugs:
  Bug #891592 in OpenERP Server: "Context not passed to _constraints functions"
  https://bugs.launchpad.net/openobject-server/+bug/891592

For more details, see:
https://code.launchpad.net/~openerp-community/openobject-server/stefan-therp_lp891592-6.1/+merge/82529

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

--
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.

=== 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__'):


_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp
Reply | Threaded
Open this post in threaded view
|

Re: [Openerp-community] [Merge] lp:~openerp-community/openobject-server/stefan-therp_lp891592-6.1 into lp:openobject-server

Vo Minh Thu (OpenERP)
This was already proposed and rejected. See https://code.launchpad.net/~openerp-groupes/openobject-server/trunk-context-constraints/+merge/62285
--
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     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp
Reply | Threaded
Open this post in threaded view
|

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

Vo Minh Thu (OpenERP)
In reply to this post by Stefan Rijnhart (Therp)
The proposal to merge lp:~openerp-community/openobject-server/stefan-therp_lp891592-6.1 into lp:openobject-server has been updated.

    Status: Needs review => Rejected

For more details, see:
https://code.launchpad.net/~openerp-community/openobject-server/stefan-therp_lp891592-6.1/+merge/82529
--
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     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp
Reply | Threaded
Open this post in threaded view
|

Re: [Openerp-community] [Merge] lp:~openerp-community/openobject-server/stefan-therp_lp891592-6.1 into lp:openobject-server

Olivier Dony (OpenERP)
In reply to this post by Vo Minh Thu (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).

--
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     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp
Reply | Threaded
Open this post in threaded view
|

Re: [Openerp-community] [Merge] lp:~openerp-community/openobject-server/stefan-therp_lp891592-6.1 into lp:openobject-server

Raphael Valyi
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) <[hidden email]>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     : [hidden email]
> Unsubscribe : https://launchpad.net/~openerp-community
> More help   : https://help.launchpad.net/ListHelp
>

--
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     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp
Reply | Threaded
Open this post in threaded view
|

Re: [Openerp-community] [Merge] lp:~openerp-community/openobject-server/stefan-therp_lp891592-6.1 into lp:openobject-server

Olivier Dony (OpenERP)
On 11/17/2011 04:08 PM, RaphaĆ«l Valyi - http://www.akretion.com wrote:
> 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

Those are hooks, and for now we would rather stay away from them as much
as possible. I believe we can reach much better code quality using a
truly clean and extensible API than adding hooks everywhere.
Cleaning up the API and making smaller and more extensible methods makes
the code simpler and easier to read. Adding hooks does the opposite (IMO).

Ultimately, it's probably a matter of taste, though.


> 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.

Don't see why it should.


> 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.

Reading/Writing a o2m field via a parent model calls the
fields.one2many.set/get method which delegate to the CRUD methods of the
child model.

Overriding the CRUD methods of sale.order.line will thus give the
desired/expected effect.

The API would be really crappy otherwise, indeed ;-)

_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp