[FIX] models: fix *2many multi-onchange bug

This fixes the case where the lines of a one2many field are modified several
times by onchange methods: instead of retrieving the most recent updates, we
merge them with former updates.

This solution was written as an improvement of a proposal made by Alexis
Delattre and Sébastien Beau as #11620.

openerp/addons/test_new_api/ir.model.access.csv

@@ -2,6 +2,8 @@
 access_category,test_new_api_category,test_new_api.model_test_new_api_category,,1,1,1,1
 access_discussion,test_new_api_discussion,test_new_api.model_test_new_api_discussion,,1,1,1,1
 access_message,test_new_api_message,test_new_api.model_test_new_api_message,,1,1,1,1
+access_multi,test_new_api_multi,test_new_api.model_test_new_api_multi,,1,1,1,1
+access_multi_line,test_new_api_multi_line,test_new_api.model_test_new_api_multi_line,,1,1,1,1
 access_mixed,test_new_api_mixed,test_new_api.model_test_new_api_mixed,,1,1,1,1
 access_test_function_noinfiniterecursion,access_test_function_noinfiniterecursion,model_test_old_api_function_noinfiniterecursion,,1,1,1,1
 access_test_function_counter,access_test_function_counter,model_test_old_api_function_counter,,1,1,1,1

openerp/addons/test_new_api/models.py

@@ -222,6 +222,35 @@ class Message(models.Model):
         return [('author.partner_id', operator, value)]
 
 
+class Multi(models.Model):
+    """ Model for testing multiple onchange methods in cascade that modify a
+        one2many field several times.
+    """
+    _name = 'test_new_api.multi'
+
+    name = fields.Char(related='partner.name', readonly=True)
+    partner = fields.Many2one('res.partner')
+    lines = fields.One2many('test_new_api.multi.line', 'multi')
+
+    @api.onchange('name')
+    def _onchange_name(self):
+        for line in self.lines:
+            line.name = self.name
+
+    @api.onchange('partner')
+    def _onchange_partner(self):
+        for line in self.lines:
+            line.partner = self.partner
+
+
+class MultiLine(models.Model):
+    _name = 'test_new_api.multi.line'
+
+    multi = fields.Many2one('test_new_api.multi', ondelete='cascade')
+    name = fields.Char()
+    partner = fields.Many2one('res.partner')
+
+
 class MixedModel(models.Model):
     _name = 'test_new_api.mixed'
 

openerp/addons/test_new_api/tests/test_onchange.py

@@ -149,6 +149,45 @@ class TestOnChange(common.TransactionCase):
             }),
         ])
 
+    def test_onchange_one2many_multi(self):
+        """ test the effect of multiple onchange methods on one2many fields """
+        partner = self.env.ref('base.res_partner_1')
+        multi = self.env['test_new_api.multi'].create({'partner': partner.id})
+        line = multi.lines.create({'multi': multi.id})
+
+        field_onchange = multi._onchange_spec()
+        self.assertEqual(field_onchange, {
+            'name': '1',
+            'partner': '1',
+            'lines': None,
+            'lines.name': None,
+            'lines.partner': None,
+        })
+
+        values = multi._convert_to_write({key: multi[key] for key in ('name', 'partner', 'lines')})
+        self.assertEqual(values, {
+            'name': partner.name,
+            'partner': partner.id,
+            'lines': [(6, 0, [line.id])],
+        })
+
+        # modify 'partner'
+        #   -> set 'partner' on all lines
+        #   -> recompute 'name'
+        #       -> set 'name' on all lines
+        partner = self.env.ref('base.res_partner_2')
+        values['partner'] = partner.id
+        values['lines'].append((0, 0, {'name': False, 'partner': False}))
+        self.env.invalidate_all()
+        result = multi.onchange(values, 'partner', field_onchange)
+        self.assertEqual(result['value'], {
+            'name': partner.name,
+            'lines': [
+                (1, line.id, {'name': partner.name, 'partner': partner.id}),
+                (0, 0, {'name': partner.name, 'partner': partner.id}),
+            ],
+        })
+
     def test_onchange_specific(self):
         """ test the effect of field-specific onchange method """
         discussion = self.env.ref('test_new_api.discussion_0')

openerp/addons/test_new_api/views.xml

@@ -20,6 +20,13 @@
         </record>
         <menuitem id="menu_messages" action="action_messages" parent="menu_sub" sequence="20"/>
 
+        <record id="action_multi" model="ir.actions.act_window">
+            <field name="name">Multi</field>
+            <field name="res_model">test_new_api.multi</field>
+            <field name="view_mode">tree,form</field>
+        </record>
+        <menuitem id="menu_multi" action="action_multi" parent="menu_sub"/>
+
         <menuitem id="menu_config" name="Configuration" parent="menu_main" sequence="20"/>
 
         <record id="action_categories" model="ir.actions.act_window">
@@ -125,5 +132,39 @@
                 </form>
             </field>
         </record>
+
+        <!-- Multi tree view -->
+        <record id="multi_tree" model="ir.ui.view">
+            <field name="name">multi tree view</field>
+            <field name="model">test_new_api.multi</field>
+            <field name="arch" type="xml">
+                <tree string="Multi">
+                    <field name="name"/>
+                </tree>
+            </field>
+        </record>
+
+        <!-- Multi form view -->
+        <record id="multi_form" model="ir.ui.view">
+            <field name="name">multi form view</field>
+            <field name="model">test_new_api.multi</field>
+            <field name="arch" type="xml">
+                <form string="Multi" version="7.0">
+                    <sheet>
+                        <group>
+                            <field name="name"/>
+                            <field name="partner"/>
+                        </group>
+                        <label for="lines"/>
+                        <field name="lines">
+                            <tree string="Lines" editable="1">
+                                <field name="name"/>
+                                <field name="partner"/>
+                            </tree>
+                        </field>
+                    </sheet>
+                </form>
+            </field>
+        </record>
     </data>
 </openerp>

openerp/models.py

@@ -5944,6 +5944,21 @@ class BaseModel(object):
 
         result = {'value': {}}
 
+        # special case for merging commands from *2many fields
+        # TODO: do not forward-port this in 9.0
+        def merge_commands(commands1, commands2):
+            # retrieve updates from commands1
+            updates = {cmd[1]: cmd[2] for cmd in commands1 if cmd[0] == 1}
+            # enrich commands2 with updates from commands1
+            commands = []
+            for cmd in commands2:
+                if cmd[0] == 1 and cmd[1] in updates:
+                    cmd = (1, cmd[1], dict(updates[cmd[1]], **cmd[2]))
+                elif cmd[0] == 4 and cmd[1] in updates:
+                    cmd = (1, cmd[1], updates[cmd[1]])
+                commands.append(cmd)
+            return commands
+
         # process names in order (or the keys of values if no name given)
         while todo:
             name = todo.pop(0)
@@ -5966,9 +5981,10 @@ class BaseModel(object):
                     newval = record[name]
                     if field.type in ('one2many', 'many2many'):
                         if newval != oldval or newval._is_dirty():
-                            # put new value in result
-                            result['value'][name] = field.convert_to_write(
-                                newval, record._origin, subfields.get(name),
+                            # merge new value into result
+                            result['value'][name] = merge_commands(
+                                result['value'].get(name, []),
+                                field.convert_to_write(newval, record._origin, subfields.get(name)),
                             )
                             todo.append(name)
                         else: