diff --git a/odoo/addons/test_new_api/tests/test_onchange.py b/odoo/addons/test_new_api/tests/test_onchange.py index 03cad7f4009f4..16144c266fc1d 100644 --- a/odoo/addons/test_new_api/tests/test_onchange.py +++ b/odoo/addons/test_new_api/tests/test_onchange.py @@ -108,12 +108,12 @@ def test_onchange_many2one(self): def test_onchange_one2many(self): """ test the effect of onchange() on one2many fields """ - BODY = "What a beautiful day!" USER = self.env.user # create an independent message - message = self.Message.create({'body': BODY}) - self.assertEqual(message.name, "[%s] %s" % ('', USER.name)) + message1 = self.Message.create({'body': "ABC"}) + message2 = self.Message.create({'body': "ABC"}) + self.assertEqual(message1.name, "[%s] %s" % ('', USER.name)) field_onchange = self.Discussion._onchange_spec() self.assertEqual(field_onchange.get('name'), '1') @@ -130,12 +130,14 @@ def test_onchange_one2many(self): 'moderator': False, 'participants': [], 'messages': [ - (4, message.id), + (4, message1.id), + (4, message2.id), + (1, message2.id, {'body': "XYZ"}), (0, 0, { 'name': "[%s] %s" % ('', USER.name), - 'body': BODY, + 'body': "ABC", 'author': USER.id, - 'size': len(BODY), + 'size': 3, 'important': False, }), ], @@ -143,20 +145,20 @@ def test_onchange_one2many(self): self.env.cache.invalidate() result = self.Discussion.onchange(values, 'name', field_onchange) self.assertIn('messages', result['value']) - self.assertItemsEqual(result['value']['messages'], [ + self.assertEqual(result['value']['messages'], [ (5,), - (1, message.id, { + (1, message1.id, { 'name': "[%s] %s" % ("Foo", USER.name), - 'body': message.body, - 'author': message.author.name_get()[0], - 'size': message.size, - 'important': message.important, + }), + (1, message2.id, { + 'name': "[%s] %s" % ("Foo", USER.name), + 'body': "XYZ", # this must be sent back }), (0, 0, { 'name': "[%s] %s" % ("Foo", USER.name), - 'body': BODY, + 'body': "ABC", 'author': USER.name_get()[0], - 'size': len(BODY), + 'size': 3, 'important': False, }), ]) @@ -167,7 +169,8 @@ def test_onchange_one2many(self): result = self.Discussion.with_context(generate_dummy_message=True).onchange(values, 'name', one_level_fields) self.assertEqual(result['value']['messages'], [ (5,), - (4, message.id), + (4, message1.id), + (4, message2.id), (0, 0, {}), (0, 0, {}), ]) @@ -260,8 +263,7 @@ def test_onchange_one2many_multi(self): 'lines': [ (5,), (1, line1.id, {'name': partner2.name, - 'partner': (partner2.id, partner2.name), - 'tags': [(5,)]}), + 'partner': (partner2.id, partner2.name)}), (0, 0, {'name': partner2.name, 'partner': (partner2.id, partner2.name), 'tags': [(5,)]}), @@ -280,18 +282,30 @@ def test_onchange_one2many_multi(self): self.env.cache.invalidate() result = multi.onchange(values, 'partner', field_onchange) - self.assertEqual(result['value'], { + expected_value = { 'name': partner2.name, 'lines': [ (5,), (1, line1.id, {'name': partner2.name, - 'partner': (partner2.id, partner2.name), - 'tags': [(5,)]}), + 'partner': (partner2.id, partner2.name)}), (0, 0, {'name': partner2.name, 'partner': (partner2.id, partner2.name), 'tags': [(5,), (0, 0, {'name': 'Tag'})]}), ], - }) + } + self.assertEqual(result['value'], expected_value) + + # ensure ID is not returned when asked and a many2many record is set to be created + self.env.cache.invalidate() + + result = multi.onchange(values, 'partner', dict(field_onchange, **{'lines.tags.id': None})) + self.assertEqual(result['value'], expected_value) + + # ensure inverse of one2many field is not returned + self.env.cache.invalidate() + + result = multi.onchange(values, 'partner', dict(field_onchange, **{'lines.multi': None})) + self.assertEqual(result['value'], expected_value) def test_onchange_specific(self): """ test the effect of field-specific onchange method """ @@ -323,8 +337,7 @@ def test_onchange_specific(self): self.assertIn('participants', result['value']) self.assertItemsEqual( result['value']['participants'], - [(5,)] + [(1, user.id, {'display_name': user.display_name}) - for user in discussion.participants + demo], + [(5,)] + [(4, user.id) for user in discussion.participants + demo], ) def test_onchange_default(self): @@ -360,6 +373,8 @@ def test_onchange_one2many_value(self): self.assertEqual(len(discussion.messages), 3) messages = [(4, msg.id) for msg in discussion.messages] messages[0] = (1, messages[0][1], {'body': 'test onchange'}) + lines = ["%s:%s" % (m.name, m.body) for m in discussion.messages] + lines[0] = "%s:%s" % (discussion.messages[0].name, 'test onchange') values = { 'name': discussion.name, 'moderator': demo.id, @@ -370,8 +385,7 @@ def test_onchange_one2many_value(self): } result = discussion.onchange(values, 'messages', field_onchange) self.assertIn('message_concat', result['value']) - self.assertEqual(result['value']['message_concat'], - "\n".join(["%s:%s" % (m.name, m.body) for m in discussion.messages])) + self.assertEqual(result['value']['message_concat'], "\n".join(lines)) 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""" @@ -417,28 +431,15 @@ def test_onchange_one2many_with_domain_on_related_field(self): '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], } + self.env.cache.invalidate() result = discussion.onchange(values, 'name', field_onchange) - # When one2many domain contains non-computed field, things are ok - self.assertEqual(result['value']['important_messages'], - [(5,)] + [(4, msg.id) for msg in discussion.important_messages]) - - # But here with commit 5676d81, we get value of: [(2, email.id)] self.assertEqual( result['value']['important_emails'], - [(5,), - (1, email.id, { - 'name': u'[Foo Bar] %s' % USER.name, - 'body': email.body, - 'author': USER.name_get()[0], - 'important': True, - 'email_to': demo.email, - 'size': email.size, - })] + [(5,), (1, email.id, {'name': u'[Foo Bar] %s' % USER.name})], ) def test_onchange_related(self): diff --git a/odoo/models.py b/odoo/models.py index 30a7529f9366f..dc1a4b4f427a6 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -5044,31 +5044,98 @@ def onchange(self, values, field_name, field_onchange): if not all(name in self._fields for name in names): return {} - # filter out keys in field_onchange that do not refer to actual fields - dotnames = [] - for dotname in field_onchange: - try: - model = self.browse() - for name in dotname.split('.'): - model = model[name] - dotnames.append(dotname) - except Exception: - pass + def PrefixTree(model, dotnames): + """ Return a prefix tree for sequences of field names. """ + if not dotnames: + return {} + # group dotnames by prefix + suffixes = defaultdict(list) + for dotname in dotnames: + # name, *names = dotname.split('.', 1) + names = dotname.split('.', 1) + name = names.pop(0) + suffixes[name].extend(names) + # fill in prefix tree in fields order + tree = OrderedDict() + for name, field in model._fields.items(): + if name in suffixes: + tree[name] = subtree = PrefixTree(model[name], suffixes[name]) + if subtree and field.type == 'one2many': + subtree.pop(field.inverse_name, None) + return tree + + class Snapshot(dict): + """ A dict with the values of a record, following a prefix tree. """ + __slots__ = () + + def __init__(self, record, tree): + # put record in dict to include it when comparing snapshots + super(Snapshot, self).__init__({'': record, '': tree}) + for name, subnames in tree.items(): + # x2many fields are serialized as a list of line snapshots + self[name] = ( + [Snapshot(line, subnames) for line in record[name]] + if subnames else record[name] + ) + + def diff(self, other): + """ Return the values in ``self`` that differ from ``other``. + Requires record cache invalidation for correct output! + """ + record = self[''] + result = {} + for name, subnames in self[''].items(): + if (name == 'id') or (other.get(name) == self[name]): + continue + if not subnames: + field = record._fields[name] + result[name] = field.convert_to_onchange(self[name], record, {}) + else: + # x2many fields: serialize value as commands + result[name] = commands = [(5,)] + for line_snapshot in self[name]: + line = line_snapshot[''] + if not line.id: + # new line: send diff from scratch + line_diff = line_snapshot.diff({}) + commands.append((0, line.id.ref or 0, line_diff)) + else: + # existing line: send diff from database + # (requires a clean record cache!) + line_diff = line_snapshot.diff(Snapshot(line, subnames)) + if line_diff: + commands.append((1, line.id, line_diff)) + else: + commands.append((4, line.id)) + return result + + nametree = PrefixTree(self.browse(), field_onchange) + + # prefetch x2many lines without data (for the initial snapshot) + for name, subnames in nametree.items(): + if subnames and values.get(name): + # retrieve all ids in commands, and read the expected fields + line_ids = [] + for cmd in values[name]: + if cmd[0] in (1, 4): + line_ids.append(cmd[1]) + elif cmd[0] == 6: + line_ids.extend(cmd[2]) + lines = self.browse()[name].browse(line_ids) + lines.read(list(subnames), load='_classic_write') # create a new record with values, and attach ``self`` to it with env.do_in_onchange(): record = self.new(values) - values = {name: record[name] for name in record._cache} # attach ``self`` with a different context (for cache consistency) record._origin = self.with_context(__onchange=True) - # load fields on secondary records, to avoid false changes + # make a snapshot based on the initial values of record with env.do_in_onchange(): - for dotname in dotnames: - record.mapped(dotname) + snapshot0 = snapshot1 = Snapshot(record, nametree) # determine which field(s) should be triggered an onchange - todo = list(names) or list(values) + todo = list(names or nametree) done = set() # dummy assignment: trigger invalidations on the record @@ -5084,7 +5151,6 @@ def onchange(self, values, field_name, field_onchange): record[name] = value result = {} - dirty = set() # process names in order (or the keys of values if no name given) while todo: @@ -5098,34 +5164,17 @@ def onchange(self, values, field_name, field_onchange): if field_onchange.get(name): record._onchange_eval(name, field_onchange[name], result) - # force re-evaluation of function fields on secondary records - for dotname in dotnames: - record.mapped(dotname) + # make a snapshot (this forces evaluation of computed fields) + snapshot1 = Snapshot(record, nametree) # determine which fields have been modified - for name, oldval in values.items(): - field = self._fields[name] - newval = record[name] - if newval != oldval or ( - field.type in ('one2many', 'many2many') and newval._is_dirty() - ): + for name in nametree: + if snapshot1[name] != snapshot0[name]: todo.append(name) - dirty.add(name) - - # determine subfields for field.convert_to_onchange() below - Tree = lambda: defaultdict(Tree) - subnames = Tree() - for dotname in dotnames: - subtree = subnames - for name in dotname.split('.'): - subtree = subtree[name] - # collect values from dirty fields - with env.do_in_onchange(): - result['value'] = { - name: self._fields[name].convert_to_onchange(record[name], record, subnames[name]) - for name in dirty - } + # determine values that have changed by comparing snapshots + self.invalidate_cache() + result['value'] = snapshot1.diff(snapshot0) return result