Revert "[FIX] models: make onchange return a smaller diff"

This reverts commit 2054b59285 which
already has been fixed multiple times [0].

However this change does not correctly handle successive onchange calls on
ony2many fields which alter existing rows.

i.e., in saas~11.4, an onchange has been added on move lines [1] in
order to create and/or update lines containing taxes amounts.
However, with diff-only onchange, a second call to onchange will discard
changes made by a first one as nothing change between the 2 calls.

Scenario: On an existing account.move, add a new credit line with a tax set.
The first onchange will update the tax line with new amount. Add a
second line (with debit amount balanced). The onchange will not change
the tax line, resulting as it value being reset to initial value. As a
result, the move is not balanced and cannot be saved.

[0] cd2ccc4110, 5e58ac16c3, 07023137cd
[1] d49ab568a2
This commit is contained in:
Christophe Simonis
2018-07-31 19:36:30 +02:00
parent 96de6756ab
commit 8c81eaabe6
2 changed files with 67 additions and 132 deletions
+38 -26
View File
@@ -139,10 +139,14 @@ class TestOnChange(common.TransactionCase):
self.env.cache.invalidate()
result = self.Discussion.onchange(values, 'name', field_onchange)
self.assertIn('messages', result['value'])
self.assertEqual(result['value']['messages'], [
self.assertItemsEqual(result['value']['messages'], [
(5,),
(1, message.id, {
'name': "[%s] %s" % ("Foo", USER.name),
'body': message.body,
'author': message.author.name_get()[0],
'size': message.size,
'important': message.important,
}),
(0, 0, {
'name': "[%s] %s" % ("Foo", USER.name),
@@ -252,7 +256,8 @@ class TestOnChange(common.TransactionCase):
'lines': [
(5,),
(1, line1.id, {'name': partner2.name,
'partner': (partner2.id, partner2.name)}),
'partner': (partner2.id, partner2.name),
'tags': [(5,)]}),
(0, 0, {'name': partner2.name,
'partner': (partner2.id, partner2.name),
'tags': [(5,)]}),
@@ -260,17 +265,6 @@ class TestOnChange(common.TransactionCase):
})
# do it again, but this time with a new tag on the second line
expected_new_tag_result = {
'name': partner2.name,
'lines': [
(5,),
(1, line1.id, {'name': partner2.name,
'partner': (partner2.id, partner2.name)}),
(0, 0, {'name': partner2.name,
'partner': (partner2.id, partner2.name),
'tags': [(5,), (0, 0, {'name': 'Tag'})]}),
],
}
values = {
'name': partner1.name,
'partner': partner2.id, # this one just changed
@@ -282,13 +276,18 @@ class TestOnChange(common.TransactionCase):
self.env.cache.invalidate()
result = multi.onchange(values, 'partner', field_onchange)
self.assertEqual(result['value'], expected_new_tag_result)
# 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_new_tag_result)
self.assertEqual(result['value'], {
'name': partner2.name,
'lines': [
(5,),
(1, line1.id, {'name': partner2.name,
'partner': (partner2.id, partner2.name),
'tags': [(5,)]}),
(0, 0, {'name': partner2.name,
'partner': (partner2.id, partner2.name),
'tags': [(5,), (0, 0, {'name': 'Tag'})]}),
],
})
def test_onchange_specific(self):
""" test the effect of field-specific onchange method """
@@ -320,7 +319,8 @@ class TestOnChange(common.TransactionCase):
self.assertIn('participants', result['value'])
self.assertItemsEqual(
result['value']['participants'],
[(5,)] + [(4, user.id) for user in discussion.participants + demo],
[(5,)] + [(1, user.id, {'display_name': user.display_name})
for user in discussion.participants + demo],
)
def test_onchange_default(self):
@@ -356,8 +356,6 @@ class TestOnChange(common.TransactionCase):
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,
@@ -368,7 +366,8 @@ class TestOnChange(common.TransactionCase):
}
result = discussion.onchange(values, 'messages', field_onchange)
self.assertIn('message_concat', result['value'])
self.assertEqual(result['value']['message_concat'], "\n".join(lines))
self.assertEqual(result['value']['message_concat'],
"\n".join(["%s:%s" % (m.name, m.body) for m in 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"""
@@ -414,15 +413,28 @@ class TestOnChange(common.TransactionCase):
'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})],
[(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,
})]
)
def test_onchange_related(self):
+29 -106
View File
@@ -5040,114 +5040,28 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
if not all(name in self._fields for name in names):
return {}
class PrefixTree(OrderedDict):
""" A prefix tree for sequences of field names. The tree is a
dictionary that associates each given field name to its
corresponding subtree (in fields order)::
# tree corresponding to dotnames
# ['name', 'line_ids.product_id', 'line_ids.tags_ids.name']
{
'name': {},
'line_ids': {
'product_id': {},
'tags_ids': {
'name': {},
},
},
}
"""
def __init__(self, model, dotnames):
super(PrefixTree, self).__init__()
if not dotnames:
return
# group dotnames by prefix
suffixes = defaultdict(list)
for dotname in dotnames:
names = dotname.split('.', 1)
name_suffixes = suffixes[names[0]]
if len(names) > 1:
name_suffixes.append(names[1])
# fill in self in fields order
for name in model._fields:
if name in suffixes:
self[name] = PrefixTree(model[name], suffixes[name])
def dotnames(self):
""" Iterate over the sequences of field names. """
for name, subnames in self.items():
yield name
for dotname in subnames.dotnames():
yield "%s.%s" % (name, dotname)
nametree = PrefixTree(self.browse(), field_onchange)
dotnames = list(nametree.dotnames())
def snapshot(record, tree=nametree):
""" Return a dict with the values of record, following nametree. """
vals = {}
for name, subnames in tree.items():
if subnames:
# x2many fields as {line: snapshot(line), ...}
vals[name] = OrderedDict(
(line, snapshot(line, subnames))
for line in record[name]
)
else:
vals[name] = record[name]
return vals
def diff(record, old, new, tree=nametree):
""" Return the values that differ between snapshots.
The snapshot ``old`` may be empty (for new records).
"""
result = {}
for name, subnames in tree.items():
if name == 'id':
continue
if old and old[name] == new[name]:
continue
field = record._fields[name]
if not subnames:
result[name] = field.convert_to_onchange(new[name], record, {})
continue
# x2many fields: serialize value as commands
result[name] = commands = [(5,)]
old_val = old.get(name) or {}
for line, vals in new[name].items():
vals0 = (old_val.get(line) or snapshot(line, subnames)) if line.id else {}
line_diff = diff(line, vals0, vals, subnames)
if not line.id:
commands.append((0, line.id.ref or 0, line_diff))
elif line_diff:
commands.append((1, line.id, line_diff))
else:
commands.append((4, line.id))
return result
# 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')
# 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
# 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 nametree}
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)
# make a snapshot based on the initial values of record
# load fields on secondary records, to avoid false changes
with env.do_in_onchange():
before = snapshot(record)
for dotname in dotnames:
record.mapped(dotname)
# determine which field(s) should be triggered an onchange
todo = list(names) or list(values)
@@ -5166,6 +5080,7 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
record[name] = value
result = {}
dirty = set()
# process names in order (or the keys of values if no name given)
while todo:
@@ -5191,14 +5106,22 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
field.type in ('one2many', 'many2many') and newval._is_dirty()
):
todo.append(name)
dirty.add(name)
# make a snapshot based on the final values of record
# 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():
after = snapshot(record)
# determine values that have changed by comparing snapshots
self.invalidate_cache()
result['value'] = diff(record, before, after)
result['value'] = {
name: self._fields[name].convert_to_onchange(record[name], record, subnames[name])
for name in dirty
}
return result