openerp-community-reviewer team mailing list archive

["Cedric Le Brouster\(OpenFire\)" <cedric.lebrouster@xxxxxxxxxxx>]

Cedric Le Brouster(OpenFire) has proposed merging lp:~cedric-lebrouster/ocb-server/ocb-7.0-bug-1253052-parent-order into lp:ocb-server.

Requested reviews:
  OpenERP Community Backports Team (ocb)
Related bugs:
  Bug #1253052 in OpenERP Server: "parent_left,parent_right are not recomputed when parent_order is changed"
  https://bugs.launchpad.net/openobject-server/+bug/1253052

For more details, see:
https://code.launchpad.net/~cedric-lebrouster/ocb-server/ocb-7.0-bug-1253052-parent-order/+merge/209708

Fixes parent_left and parent_right recomputation in write/create methods ( bug lp:1253052 )
-- 
https://code.launchpad.net/~cedric-lebrouster/ocb-server/ocb-7.0-bug-1253052-parent-order/+merge/209708
Your team OpenERP Community Backports Team is requested to review the proposed merge of lp:~cedric-lebrouster/ocb-server/ocb-7.0-bug-1253052-parent-order into lp:ocb-server.

=== modified file 'openerp/osv/orm.py'
--- openerp/osv/orm.py	2014-03-05 15:18:15 +0000
+++ openerp/osv/orm.py	2014-03-06 14:40:28 +0000
@@ -4168,22 +4168,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 = []
@@ -4281,18 +4290,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
@@ -4512,21 +4519,24 @@
                 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
+                    where_clause += '=%s'
+                    where_params = (parent,)
                 else:
-                    cr.execute('select max(parent_right) from '+self._table)
-                    pleft = cr.fetchone()[0] or 0
+                    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_old = cr.fetchone()[0]
+                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))