Merge lp:~openerp-dev/openobject-server/trunk-wmsimplement-seduce-fp into lp:openobject-server

Proposed by Josse Colpaert (OpenERP)
Status: Needs review
Proposed branch: lp:~openerp-dev/openobject-server/trunk-wmsimplement-seduce-fp
Merge into: lp:openobject-server
Diff against target: 12 lines (+0/-2)
1 file modified
openerp/osv/orm.py (+0/-2)
To merge this branch: bzr merge lp:~openerp-dev/openobject-server/trunk-wmsimplement-seduce-fp
Reviewer Review Type Date Requested Status
Raphael Collet (OpenERP) (community) Disapprove
Vo Minh Thu Pending
Review via email: mp+175057@code.launchpad.net

Description of the change

We want to make it possible to put expressions in the order parameter of a search. That way the results will be sorted by the boolean result of the expression.

By inheriting check_qorder and return True, it will be possible

To post a comment you must log in.
Revision history for this message
Raphael Collet (OpenERP) (rco-openerp) wrote :

Don't know what to think about it.
I understand it allows you to do something that was not foreseen,
but we lose an error message :-(

review: Abstain
4922. By Fabien (Open ERP)

[MERGE] Trunk server

4923. By Cedric Snauwaert (OpenERP)

[MERGE]merge with latest trunk

4924. By Cedric Snauwaert (OpenERP)

[MERGE]merge with latest server trunk

4925. By Josse Colpaert (OpenERP)

[MERGE] Merge from trunk

Revision history for this message
qdp (OpenERP) (qdp) wrote :

actually, we don't really loose the error message because it may be raised later on, when psql will try to order the query.

moreover, this error message was preventing to make queries that are valid like:

=> select id, name, country_id from res_partner order by country_id is not null desc; (i want first to see my partners that have a country_id set)

So?

Revision history for this message
qdp (OpenERP) (qdp) wrote :

on the other hand, if we accept this MP, we need to review the docstring of the method as well.

So i wouldn't merge it as it is currently, but on the principle i don't see any reason to reject it.

Revision history for this message
Raphael Collet (OpenERP) (rco-openerp) wrote :

Psql will indeed raise an error message.
Okay, fine by me.

review: Approve
Revision history for this message
Raphael Collet (OpenERP) (rco-openerp) wrote :

Note that you can inject some SQL from there:

    # client code; current user can read some model
    model.search(..., order="sequence; delete from res_users;")

Ha, you're screwed :-/

review: Disapprove
4926. By Quentin (OpenERP) <email address hidden>

[MERGE] merge with main trunk

4927. By Quentin (OpenERP) <email address hidden>

[MERGE] merged with trunk

Unmerged revisions

4927. By Quentin (OpenERP) <email address hidden>

[MERGE] merged with trunk

4926. By Quentin (OpenERP) <email address hidden>

[MERGE] merge with main trunk

4925. By Josse Colpaert (OpenERP)

[MERGE] Merge from trunk

4924. By Cedric Snauwaert (OpenERP)

[MERGE]merge with latest server trunk

4923. By Cedric Snauwaert (OpenERP)

[MERGE]merge with latest trunk

4922. By Fabien (Open ERP)

[MERGE] Trunk server

4921. By Josse Colpaert (OpenERP)

[IMP] When qorder is overridden and will return True it will be possible to put order by reservation_id <> move_id in quant search (order on expr besides fields)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'openerp/osv/orm.py'
2--- openerp/osv/orm.py 2013-09-30 12:59:46 +0000
3+++ openerp/osv/orm.py 2013-10-16 10:15:46 +0000
4@@ -4884,8 +4884,6 @@
5 inner_clause = self._generate_m2o_order_by(order_field, query)
6 else:
7 continue # ignore non-readable or "non-joinable" fields
8- else:
9- raise ValueError( _("Sorting field %s not found on model %s") %( order_field, self._name))
10 if inner_clause:
11 if isinstance(inner_clause, list):
12 for clause in inner_clause: