[FIX] fields: during an onchange(), do not invalidate *2many fields because of their domain

Our usage of domain on fields One2many seems to trigger an obscure behaviour on
onchange.

With the following (simplified) config:

    Message(models.Model):
        _name = 'test_new_api.message'
        important = fields.Boolean('Important')

    Discussion(models.Model):
        _name = 'test_new_api.discussion'
        name = fields.Char('Name')
        important_emails = fields.One2Many('test_new_api.emailmessage', 'discussion',
                                           domain=[('important', '=', True)])

    Email(models.Model):
        _name = 'test_new_api.emailmessage'
        _inherits = {'test_new_api.message': 'message'}

        discussion = fields.Many2one('test_new_api.discussion', 'Discussion')
        message = fields.Many2one('test_new_api.message', 'Message')

Steps:
- We change 'name' on discussion, triggers an `onchange()` call
- we ends up filling cache on virtual record (on secondary fields, we calling
  record.mapped('important_emails.important'))
- we get a cache miss ('important' field not provided, only 'important_emails' ids,
  i.e with no change on existing records)
- we fill the cache, this mark 'important' field as modified
- because of commit 5676d812 and because 'important' is that case is a related (i.e
  computed) field we triggers cache recomputation
- as there is no way to recompute 'important_emails' for virtual record (no real
  ID) we ends up with empty 'important_emails' generating removal of existing records.

=> Finally changing any value for 'test_new_api.discussion' that trigger an onchange
will always reset 'important_emails' to empty

Fixed by Raphael Collet <rco@odoo.com>, and test by Xavier Alt <xal@odoo.com>.

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

@@ -2,6 +2,7 @@
 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_emailmessage,test_new_api_emailmessage,test_new_api.model_test_new_api_emailmessage,,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

openerp/addons/test_new_api/models.py

@@ -145,6 +145,9 @@ class Discussion(models.Model):
     message_changes = fields.Integer(string='Message changes')
     important_messages = fields.One2many('test_new_api.message', 'discussion',
                                          domain=[('important', '=', True)])
+    emails = fields.One2many('test_new_api.emailmessage', 'discussion')
+    important_emails = fields.One2many('test_new_api.emailmessage', 'discussion',
+                                       domain=[('important', '=', True)])
 
     @api.onchange('moderator')
     def _onchange_moderator(self):
@@ -225,6 +228,14 @@ class Message(models.Model):
         return [('author.partner_id', operator, value)]
 
 
+class EmailMessage(models.Model):
+    _name = 'test_new_api.emailmessage'
+    _inherits = {'test_new_api.message': 'message'}
+
+    message = fields.Many2one('test_new_api.message', 'Message',
+                              required=True, ondelete='cascade')
+    email_to = fields.Char('To')
+
 class Multi(models.Model):
     """ Model for testing multiple onchange methods in cascade that modify a
         one2many field several times.

openerp/addons/test_new_api/tests/test_onchange.py

@@ -8,6 +8,7 @@ class TestOnChange(common.TransactionCase):
         super(TestOnChange, self).setUp()
         self.Discussion = self.env['test_new_api.discussion']
         self.Message = self.env['test_new_api.message']
+        self.EmailMessage = self.env['test_new_api.emailmessage']
 
     def test_default_get(self):
         """ checking values returned by default_get() """
@@ -237,3 +238,60 @@ class TestOnChange(common.TransactionCase):
         result = discussion.onchange(values, 'messages', field_onchange)
         self.assertIn('message_changes', result['value'])
         self.assertEqual(result['value']['message_changes'], len(discussion.messages))
+
+    def test_onchange_one2many_with_domain_on_related_field(self):
+        """ test the value of the one2many field when defined with a domain on a related field"""
+        discussion = self.env.ref('test_new_api.discussion_0')
+        demo = self.env.ref('base.user_demo')
+
+        # mimic UI behaviour, so we get subfields
+        # (we need at least subfield: 'important_emails.important')
+        view_info = self.Discussion.fields_view_get(
+            view_id=self.env.ref('test_new_api.discussion_form').id,
+            view_type='form')
+        field_onchange = self.Discussion._onchange_spec(view_info=view_info)
+        self.assertEqual(field_onchange.get('messages'), '1')
+
+        BODY = "What a beautiful day!"
+        USER = self.env.user
+
+        # create standalone email
+        email = self.EmailMessage.create({
+            'discussion': discussion.id,
+            'name': "[%s] %s" % ('', USER.name),
+            'body': BODY,
+            'author': USER.id,
+            'important': False,
+        })
+
+        # check if server-side cache is working correctly
+        self.env.invalidate_all()
+        self.assertIn(email, discussion.emails)
+        self.assertNotIn(email, discussion.important_emails)
+        email.important = True
+        self.assertIn(email, discussion.important_emails)
+
+        # check that when trigger an onchange, we don't reset important emails
+        # (force `invalidate_all` as but appear in onchange only when we get a
+        # cache miss)
+        self.env.invalidate_all()
+        self.assertEqual(len(discussion.messages), 4)
+        values = {
+            'name': "Foo Bar",
+            'moderator': demo.id,
+            'categories': [(4, cat.id) for cat in discussion.categories],
+            'messages': [(4, msg.id) for msg in discussion.messages],
+            'participants': [(4, usr.id) for usr in discussion.participants],
+            'message_changes': 0,
+            'important_messages': [(4, msg.id) for msg in discussion.important_messages],
+            'important_emails': [(4, eml.id) for eml in discussion.important_emails],
+        }
+        result = discussion.onchange(values, 'name', field_onchange)
+
+        # When one2many domain contains non-computed field, things are ok
+        self.assertEqual(result['value']['important_messages'],
+            [(1, email.message.id, {'name': u'[Foo Bar] %s' % USER.name})])
+
+        # But here with commit 5676d81, we get value of: [(2, email.id)]
+        self.assertEqual(result['value']['important_emails'],
+            [(1, email.id, {'name': u'[Foo Bar] %s' % USER.name})])

openerp/addons/test_new_api/views.xml

@@ -54,12 +54,35 @@
                                     <tree name="Messages">
                                         <field name="name"/>
                                         <field name="body"/>
+                                        <field name="important"/>
                                     </tree>
                                     <form string="Message" version="7.0">
                                         <group>
                                             <field name="name"/>
                                             <field name="author"/>
                                             <field name="size"/>
+                                            <field name="important"/>
+                                        </group>
+                                        <label for="body"/>
+                                        <field name="body"/>
+                                    </form>
+                                </field>
+                            </page>
+                            <page string="Emails">
+                                <field name="important_emails">
+                                    <tree name="Important Messages">
+                                        <field name="name"/>
+                                        <field name="body"/>
+                                        <field name="important"/>
+                                        <field name="email_to"/>
+                                    </tree>
+                                    <form string="Important Message" version="7.0">
+                                        <group>
+                                            <field name="name"/>
+                                            <field name="author"/>
+                                            <field name="size"/>
+                                            <field name="important"/>
+                                            <field name="email_to"/>
                                         </group>
                                         <label for="body"/>
                                         <field name="body"/>

openerp/fields.py

@@ -998,6 +998,13 @@ class Field(object):
         # ``records``, except fields currently being computed
         spec = []
         for field, path in self._triggers:
+            if not field.compute:
+                # Note: do not invalidate non-computed fields. Such fields may
+                # require invalidation in general (like *2many fields with
+                # domains) but should not be invalidated in this case, because
+                # we would simply lose their values during an onchange!
+                continue
+
             target = env[field.model_name]
             computed = target.browse(env.computed[field])
             if path == 'id':