Merge lp:~camptocamp/ocb-addons/ocb-7.0-fix_1302630_document_search_order_by-rde into lp:ocb-addons

[Alexandre Fayolle - camptocamp]

[FIX] document: preserve order in search()

port of https://code.launchpad.net/~camptocamp/openobject-addons/7.0_fix-document-search-order-by_rde/+merge/214483 to OCB

[Leonardo Pistone (lepistone)]

same as https://code.launchpad.net/~camptocamp/openobject-addons/7.0_fix-document-search-order-by_rde/+merge/214483

review: Approve (code review)

[Lionel Sausin - Initiatives/Numérigraphe (ls-initiatives)]

- "id" is not a great variable name, python has id() already.
- wouldn't it be the same to write "result = list(ids)" at line 21?

review: Needs Fixing

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

> - wouldn't it be the same to write "result = list(ids)" at line 21?

`list(ids)` wouldn't keep the order of ids.

[Lionel Sausin - Initiatives/Numérigraphe (ls-initiatives)]

I see, line 21 it deserves a comment then.

[Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c)]

LGTM

review: Approve (code review, no test)

[Holger Brunn (Therp) (hbrunn)]

review: Approve (code review)

[Yannick Vaucher @ Camptocamp (yvaucher-c2c)]

LGTM

review: Approve (code review, no test)

[Holger Brunn (Therp) (hbrunn)]

Development for 7.0 has moved to github on https://github.com/OCA/ocb - please move your merge proposal there if it is still valid.

(I close and reject this in order to have a cleaner overview for 6.1 MPs which indeed have to be done on launchpad)

review: Disapprove

Unmerged revisions

10077. By Romain Deheele - Camptocamp on 2014-04-07

[FIX] document: search method overload should take into account order by clause

=== modified file 'document/document.py'
--- document/document.py	2014-03-10 08:54:20 +0000
+++ document/document.py	2014-04-07 08:42:22 +0000
@@ -89,6 +89,11 @@
         if not ids:
             return 0 if count else []
 
+        # Work with a set, as list.remove() is prohibitive for large lists of documents
+        # (takes 20+ seconds on a db with 100k docs during search_count()!)
+        orig_ids = ids
+        ids = set(ids)
+
         # Filter out documents that are in directories that the user is not allowed to read.
         # Must use pure SQL to avoid access rules exceptions (we want to remove the records,
         # not fail), and the records have been filtered in parent's search() anyway.
@@ -108,7 +113,8 @@
         for parent_id in visible_parent_ids:
             ids.extend(parents[parent_id])
 
-        return len(ids) if count else ids
+        result = [id for id in orig_ids if id in ids]
+        return len(result) if count else result
 
     def copy(self, cr, uid, id, default=None, context=None):
         if not default: