diff --git a/odoo/addons/test_testing_utilities/ir.model.access.csv b/odoo/addons/test_testing_utilities/ir.model.access.csv index 1cb9a9f542a..2e7786fdcc4 100644 --- a/odoo/addons/test_testing_utilities/ir.model.access.csv +++ b/odoo/addons/test_testing_utilities/ir.model.access.csv @@ -20,3 +20,5 @@ access_test_testing_utilities_onchange_count_sub,access_test_testing_utilities_o access_o2m_readonly_subfield_parent,access_o2m_readonly_subfield_parent,model_o2m_readonly_subfield_parent,,1,1,1,1 access_o2m_readonly_subfield_child,access_o2m_readonly_subfield_child,model_o2m_readonly_subfield_child,,1,1,1,1 access_test_testing_utilities_req_bool,access_test_testing_utilities_req_bool,model_test_testing_utilities_req_bool,,1,0,0,0 +access_o2m_changes_children,access_o2m_changes_children,model_o2m_changes_children,base.group_user,1,0,0,0 +access_o2m_changes_children_lines,access_o2m_changes_children_lines,model_o2m_changes_children_lines,base.group_user,1,0,0,0 diff --git a/odoo/addons/test_testing_utilities/models.py b/odoo/addons/test_testing_utilities/models.py index 769a6662317..cf2b4f0e8c6 100644 --- a/odoo/addons/test_testing_utilities/models.py +++ b/odoo/addons/test_testing_utilities/models.py @@ -279,3 +279,23 @@ class ReqBool(models.Model): _name = _description = 'test_testing_utilities.req_bool' f_bool = fields.Boolean(required=True) + +class O2MChangesChildren(models.Model): + _name = _description = 'o2m_changes_children' + + name = fields.Char() + v = fields.Integer() + line_ids = fields.One2many('o2m_changes_children.lines', 'parent_id') + + @api.onchange('v') + def _onchange_v(self): + for record in self: + for line in record.line_ids: + line.v = record.v + +class O2MChangesChildrenLines(models.Model): + _name = _description = 'o2m_changes_children.lines' + + parent_id = fields.Many2one('o2m_changes_children') + v = fields.Integer() + vv = fields.Integer() diff --git a/odoo/addons/test_testing_utilities/tests/test_form_impl.py b/odoo/addons/test_testing_utilities/tests/test_form_impl.py index 3c1d53bd6da..cc2620f9e46 100644 --- a/odoo/addons/test_testing_utilities/tests/test_form_impl.py +++ b/odoo/addons/test_testing_utilities/tests/test_form_impl.py @@ -534,6 +534,44 @@ class TestO2M(TransactionCase): a | b | c ) + def test_o2m_onchange_change_saved(self): + """ If an onchange updates o2m values (in existing sub-records of an + existing record), those updated values should be saved, both if the + sub-records were touched by the user and not (check that one maybe) + """ + # create record: line created before v is updated should reflect it, + # line created after should not + f = Form(self.env['o2m_changes_children']) + with f.line_ids.new() as line: + line.v = 1 + line.vv = 5 + f.v = 5 + with f.line_ids.new() as line: + ... + r = f.save() + self.assertEqual(r.v, 5) + self.assertEqual(r.mapped('line_ids.vv'), [5, 0]) + self.assertEqual(r.line_ids[0].v, 5, "onchange should have updated the existing lines") + self.assertEqual(r.line_ids[1].v, 0, "onchange should not impact new line") + + # update record: onchange then touch the lines + with Form(r) as f: + f.v = 6 + with f.line_ids.edit(0) as line: + line.vv = 1 + with f.line_ids.edit(1) as line: + line.vv = 2 + self.assertEqual(r.v, 6) + self.assertEqual(r.mapped('line_ids.vv'), [1, 2]) + self.assertEqual(r.mapped('line_ids.v'), [6, 6], "onchange should have updated vs") + + # update record: onchange then don't even touch the lines + with Form(r) as f: + f.v = 7 + self.assertEqual(r.v, 7) + self.assertEqual(r.mapped('line_ids.vv'), [1, 2]) + self.assertEqual(r.mapped('line_ids.v'), [7, 7]) + class TestEdition(TransactionCase): """ These use the context manager form as we don't need the record post-save (we already have it) and it's easier to see what bits act on diff --git a/odoo/tests/common.py b/odoo/tests/common.py index 462e6111485..cbde36ac6d6 100644 --- a/odoo/tests/common.py +++ b/odoo/tests/common.py @@ -1489,8 +1489,12 @@ class Form(object): fields = self._view['fields'] def cleanup(k, v): if fields[k]['type'] == 'one2many': - # o2m default gets a (6) at the start, makes no sense - return [c for c in v if c[0] != 6] + return [ + # use None as "empty" value for UPDATE instead of {} + (1, c[1], None) if c[0] == 1 and not c[2] else c + for c in v + if c[0] != 6 # o2m default gets a (6) at the start, nonsensical + ] elif fields[k]['type'] == 'datetime' and isinstance(v, datetime): return odoo.fields.Datetime.to_string(v) elif fields[k]['type'] == 'date' and isinstance(v, date): @@ -1614,7 +1618,8 @@ class Form(object): # * the environment's context (?) # * a few magic values record_id = self._values.get('id') or False - ctx = dict(self._values) + + ctx = dict(self._values_to_save(all_fields=True)) ctx.update(self._env.context) ctx.update( id=record_id, @@ -1670,36 +1675,37 @@ class Form(object): r.write(values) else: r = self._model.create(values) + self._values.update( + record_to_values(self._view['fields'], r) + ) + self._changed.clear() self._model.flush() self._model.invalidate_cache() - [data] = r.read(list(self._view['fields'])) - # FIXME: process relational fields - # alternative: iterate on record & read from it directly? pb: would - # provide recordsets for relational fields which may or may not be - # what we ultimately want, but we've got to re-process them anyway so...? - self._values.update(data) - self._changed.clear() return r - def _values_to_save(self): + def _values_to_save(self, all_fields=False): """ Validates values and returns only fields modified since load/save + + :param bool all_fields: if False (the default), checks for required + fields and only save fields which are changed + and not readonly """ values = {} fields = self._view['fields'] for f in fields: descr = fields[f] v = self._values[f] - if self._get_modifier(f, 'required') and not (descr['type'] == 'boolean' or self._get_modifier(f, 'invisible')): + # note: maybe `invisible` should not skip `required` if model attribute + if not all_fields and self._get_modifier(f, 'required') and not (descr['type'] == 'boolean' or self._get_modifier(f, 'invisible')): assert v is not False, "{} is a required field".format(f) - - # skip unmodified fields - if f not in self._changed: + # skip unmodified fields unless all_fields (also always ignore id) + if f == 'id' or not (all_fields or f in self._changed): continue if self._get_modifier(f, 'readonly'): node = _get_node(self._view, f) - if not node.get('force_save'): + if not (all_fields or node.get('force_save')): continue if descr['type'] == 'one2many': @@ -1716,10 +1722,14 @@ class Form(object): for (c, rid, vs) in oldvals: if c in (0, 1): - items = list(getattr(vs, 'changed_items', vs.items)()) + vs = vs or {} + if all_fields: + items = list(vs.items()) + else: + items = list(getattr(vs, 'changed_items', vs.items)()) fields_ = view['fields'] missing = fields_.keys() - vs.keys() - if missing: + if missing: # FIXME: maaaybe this should be done at the start? Model = self._env[descr['relation']] if c == 0: vs.update(dict.fromkeys(missing, False)) @@ -1732,11 +1742,12 @@ class Form(object): {k: v for k, v in fields_.items() if k not in vs}, Model.browse(rid) )) - vs.setdefault('id', False) - vs['•parent•'] = self._values + context = dict(vs) + context.setdefault('id', False) + context['•parent•'] = self._values vs = { k: v for k, v in items - if nodes[k].get('force_save') or not self._get_modifier(k, 'readonly', modmap=modifiers, vals=vs) + if all_fields or nodes[k].get('force_save') or not self._get_modifier(k, 'readonly', modmap=modifiers, vals=context) } v.append((c, rid, vs)) @@ -1759,7 +1770,7 @@ class Form(object): self._model.flush() self._model.invalidate_cache() if result.get('warning'): - _logger.getChild('onchange').warn("%(title)s %(message)s" % result.get('warning')) + _logger.getChild('onchange').warning("%(title)s %(message)s" % result.get('warning')) values = result.get('value', {}) # mark onchange output as changed self._changed.update(values.keys()) @@ -1801,27 +1812,48 @@ class Form(object): if not descr['views']: return [] + if current is None: + current = [] v = [] - c = {t[1] for t in current if t[0] in (1, 2)} if current else set() + c = {t[1] for t in current if t[0] in (1, 2)} + current_values = {c[1]: c[2] for c in current if c[0] == 1} # which view should this be??? subfields = descr['views']['edition']['fields'] # TODO: simplistic, unlikely to work if e.g. there's a 5 inbetween other commands for command in value: - if command[0] in (0, 1): - c.discard(command[1]) - v.append((command[0], command[1], { - k: self._cleanup_onchange( - subfields[k], v, None - ) + if command[0] == 0: + v.append((0, 0, { + k: self._cleanup_onchange(subfields[k], v, None) for k, v in command[2].items() if k in subfields })) + elif command[0] == 1: + record_id = command[1] + c.discard(record_id) + stored = current_values.get(record_id) + if stored is None: + record = self._env[descr['relation']].browse(record_id) + stored = UpdateDict(record_to_values(subfields, record)) + + updates = ( + (k, self._cleanup_onchange(subfields[k], v, None)) + for k, v in command[2].items() + if k in subfields + ) + for field, value in updates: + # if there are values from the onchange which differ + # from current values, update & mark field as changed + if stored.get(field, value) != value: + stored._changed.add(field) + stored[field] = value + + v.append((1, record_id, stored)) elif command[0] == 2: c.discard(command[1]) v.append((2, command[1], False)) elif command[0] == 4: c.discard(command[1]) - v.append((1, command[1], {})) + v.append((1, command[1], None)) elif command[0] == 5: v = [] # explicitly mark all non-relinked (or modified) records as deleted @@ -1870,7 +1902,10 @@ class O2MForm(Form): if index is None: self._init_from_defaults(m) else: - self._values.update(proxy._records[index]) + vals = proxy._records[index] + self._values.update(vals) + if hasattr(vals, '_changed'): + self._changed.update(vals._changed) def _get_modifier(self, field, modifier, default=False, modmap=None, vals=None): if vals is None: @@ -1882,8 +1917,8 @@ class O2MForm(Form): values = super(O2MForm, self)._onchange_values() # computed o2m may not have a relation_field(?) descr = self._proxy._descr - if 'relation_field' in descr: - values[descr['relation_field']] = self._proxy._parent._values + if 'relation_field' in descr: # note: should be fine because not recursive + values[descr['relation_field']] = self._proxy._parent._onchange_values() return values def save(self): @@ -1898,7 +1933,9 @@ class O2MForm(Form): if c == 0: vs.update(values) elif c == 1: - vs = UpdateDict(vs) + if vs is None: + vs = UpdateDict() + assert isinstance(vs, UpdateDict), type(vs) vs.update(values) commands[index] = (1, id_, vs) else: @@ -1907,12 +1944,14 @@ class O2MForm(Form): # FIXME: should be called when performing on change => value needs to be serialised into parent every time? proxy._parent._perform_onchange([proxy._field]) - def _values_to_save(self): + def _values_to_save(self, all_fields=False): """ Validates values and returns only fields modified since load/save """ values = UpdateDict(self._values) values._changed.update(self._changed) + if all_fields: + return values for f in self._view['fields']: if self._get_modifier(f, 'required') and not (self._get_modifier(f, 'column_invisible') or self._get_modifier(f, 'invisible')): @@ -1924,6 +1963,8 @@ class UpdateDict(dict): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self._changed = set() + if args and isinstance(args[0], UpdateDict): + self._changed.update(args[0]._changed) def changed_items(self): return ( @@ -1957,11 +1998,11 @@ class O2MProxy(X2MProxy): if command == 0: self._records.append(values) elif command == 1: - # read based on view info - r = model.browse(rid) - record = record_to_values(fields, r) - record.update(values) - self._records.append(record) + if values is None: + # read based on view info + r = model.browse(rid) + values = UpdateDict(record_to_values(fields, r)) + self._records.append(values) elif command == 2: pass else: @@ -2129,7 +2170,7 @@ def record_to_values(fields, record): assert v._name == descr['relation'] v = [(6, 0, v.ids)] elif descr['type'] == 'one2many': - v = [(1, r.id, {}) for r in v] + v = [(1, r.id, None) for r in v] elif descr['type'] == 'datetime' and isinstance(v, datetime): v = odoo.fields.Datetime.to_string(v) elif descr['type'] == 'date' and isinstance(v, date):