Merge lp:~cedric-lebrouster/openobject-server/7.0-bug-1253052-parent-order into lp:openobject-server/7.0

[Cedric Le Brouster(OpenFire)]

Fixes parent_left and parent_right recomputation in write/create methods ( bug lp:1253052 )

[lp:~cedric-lebrouster/openobject-server/7.0-bug-1253052-parent-order]

5190. By Cedric Le Brouster(OpenFire) on 2014-03-07

Add tests for _parent_order

5191. By Cedric Le Brouster(OpenFire) on 2014-06-13

[FIX] orm: fix miscalculation of _parent_left and _parent_right in create() method for some special case.
(i.e. when new object parent's _parent_left value is 0)

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

https://github.com/odoo/odoo/pull/829

Unmerged revisions

5191. By Cedric Le Brouster(OpenFire) on 2014-06-13

[FIX] orm: fix miscalculation of _parent_left and _parent_right in create() method for some special case.
(i.e. when new object parent's _parent_left value is 0)

5190. By Cedric Le Brouster(OpenFire) on 2014-03-07

Add tests for _parent_order

5189. By Cedric Le Brouster(OpenFire) on 2014-01-15

[FIX] orm: wrong parent_left and parent_right recomputation in write/create methods (see bug 1253052)

=== modified file 'openerp/addons/base/test/base_test.yml'
--- openerp/addons/base/test/base_test.yml	2014-04-16 11:22:39 +0000
+++ openerp/addons/base/test/base_test.yml	2014-06-13 10:05:21 +0000
@@ -137,7 +137,19 @@
         assert len(old_struct) == 4, "After duplication, previous record must have old childs records only"
         assert (not set(new_struct).intersection(old_struct)), "After duplication, nodes should not be mixed"
 -
-    6. Restore pool state after the test
+    6. Tests on _parent_order.
+-
+    !python {model: res.partner.category }: |
+        another_root = self.create(cr, uid, {'name':'Root categ'})
+        struct = self.search(cr, uid, [('id','in',(ref('test_categ_root'),another_root))])
+        assert struct[0]==another_root, "Root partner categories should be sorted."
+        struct = self.search(cr, uid, [('id','in',(ref('test_categ_1'),ref('test_categ_2')))])
+        assert struct[0]==ref('test_categ_1'), "Partner categories should be sorted."
+        self.write(cr, uid, ref('test_categ_1'), {'name':'Child 3'})
+        struct = self.search(cr, uid, [('id','in',(ref('test_categ_1'),ref('test_categ_2')))])
+        assert struct[0]==ref('test_categ_2'), "Partner categories should be re-sorted after name update."
+-
+    7. Restore pool state after the test
 -
     !python {model: res.partner.category}: |
         self.pool._init = True
 
=== modified file 'openerp/osv/orm.py'
--- openerp/osv/orm.py	2014-05-19 16:12:41 +0000
+++ openerp/osv/orm.py	2014-06-13 10:05:21 +0000
@@ -4176,22 +4176,31 @@
 
         parents_changed = []
         parent_order = self._parent_order or self._order
-        if self._parent_store and (self._parent_name in vals):
+        if self._parent_store:
             # The parent_left/right computation may take up to
             # 5 seconds. No need to recompute the values if the
-            # parent is the same.
+            # parent and order field are the same.
             # Note: to respect parent_order, nodes must be processed in
             # order, so ``parents_changed`` must be ordered properly.
-            parent_val = vals[self._parent_name]
-            if parent_val:
-                query = "SELECT id FROM %s WHERE id IN %%s AND (%s != %%s OR %s IS NULL) ORDER BY %s" % \
-                                (self._table, self._parent_name, self._parent_name, parent_order)
-                cr.execute(query, (tuple(ids), parent_val))
-            else:
-                query = "SELECT id FROM %s WHERE id IN %%s AND (%s IS NOT NULL) ORDER BY %s" % \
-                                (self._table, self._parent_name, parent_order)
-                cr.execute(query, (tuple(ids),))
-            parents_changed = map(operator.itemgetter(0), cr.fetchall())
+            query = "SELECT id FROM %s WHERE id IN %%s" % (self._table,)
+            query_params = [tuple(ids)]
+            query_clause = ""
+            parent_params = [self._parent_name]
+            for param in parent_order.split(','):
+                parent_params.append(param.strip())
+            for parent_param in parent_params:
+                if parent_param in vals:
+                    parents_changed = True
+                    parent_val = vals[parent_param]
+                    if parent_val:
+                        query_clause += (query_clause and ' OR ' or '') + "%s != %%s OR %s IS NULL" % (parent_param,parent_param)
+                        query_params.append(parent_val)
+                    else:
+                        query_clause += (query_clause and ' OR ' or '') + "%s IS NOT NULL" % (parent_param,)
+            if parents_changed:
+                query += (query_clause and (' AND (' + query_clause + ')') or '') + " ORDER BY %s" % (self._parent_order,)
+                cr.execute(query, tuple(query_params))
+                parents_changed = map(operator.itemgetter(0), cr.fetchall())
 
         upd0 = []
         upd1 = []
@@ -4289,18 +4298,16 @@
             if self.pool._init:
                 self.pool._init_parent[self._name] = True
             else:
-                order = self._parent_order or self._order
-                parent_val = vals[self._parent_name]
-                if parent_val:
-                    clause, params = '%s=%%s' % (self._parent_name,), (parent_val,)
-                else:
-                    clause, params = '%s IS NULL' % (self._parent_name,), ()
-
                 for id in parents_changed:
-                    cr.execute('SELECT parent_left, parent_right FROM %s WHERE id=%%s' % (self._table,), (id,))
-                    pleft, pright = cr.fetchone()
+                    cr.execute('SELECT parent_left, parent_right, %s FROM %s WHERE id=%%s' % (self._parent_name, self._table,), (id,))
+                    pleft, pright, parent_val = cr.fetchone()
                     distance = pright - pleft + 1
 
+                    if parent_val:
+                        clause, params = '%s=%%s' % (self._parent_name,), (parent_val,)
+                    else:
+                        clause, params = '%s IS NULL' % (self._parent_name,), ()
+
                     # Positions of current siblings, to locate proper insertion point;
                     # this can _not_ be fetched outside the loop, as it needs to be refreshed
                     # after each update, in case several nodes are sequentially inserted one
@@ -4520,21 +4527,25 @@
                 self.pool._init_parent[self._name] = True
             else:
                 parent = vals.get(self._parent_name, False)
+                where_clause = ' where '+self._parent_name
                 if parent:
-                    cr.execute('select parent_right from '+self._table+' where '+self._parent_name+'=%s order by '+(self._parent_order or self._order), (parent,))
-                    pleft_old = None
-                    result_p = cr.fetchall()
-                    for (pleft,) in result_p:
-                        if not pleft:
-                            break
-                        pleft_old = pleft
-                    if not pleft_old:
-                        cr.execute('select parent_left from '+self._table+' where id=%s', (parent,))
-                        pleft_old = cr.fetchone()[0]
-                    pleft = pleft_old
-                else:
-                    cr.execute('select max(parent_right) from '+self._table)
-                    pleft = cr.fetchone()[0] or 0
+                    where_clause += '=%s'
+                    where_params = (parent,)
+                else:
+                    where_clause += ' is null'
+                    where_params = tuple()
+                cr.execute('select parent_right from '+self._table + where_clause+' order by '+(self._parent_order or self._order), where_params)
+                pleft_old = None
+                result_p = cr.fetchall()
+                for (pleft,) in result_p:
+                    if not pleft:
+                        break
+                    pleft_old = pleft
+                if not pleft_old and parent:
+                    cr.execute('select parent_left from '+self._table+' where id=%s', (parent,))
+                    pleft = cr.fetchone()[0]
+                else:
+                    pleft = pleft_old or -1
                 cr.execute('update '+self._table+' set parent_left=parent_left+2 where parent_left>%s', (pleft,))
                 cr.execute('update '+self._table+' set parent_right=parent_right+2 where parent_right>%s', (pleft,))
                 cr.execute('update '+self._table+' set parent_left=%s,parent_right=%s where id=%s', (pleft+1, pleft+2, id_new))