From 999ed04c40ec7a8465dc959b75cc6617cb4e6bf3 Mon Sep 17 00:00:00 2001 From: Denis Ledoux Date: Fri, 14 Nov 2014 14:24:11 +0100 Subject: [PATCH 1/4] [FIX] api: avoid to return all fields *2many in onchanges When an onchange returns a change in a 2many field line (a '1' tuple, update), avoid to return all fields of the *2many field but only the altered field. Otherwise, the web client regard all the fields of the 2many as dirty, and try to write on all fields (even if the value is the same, thus) opw-615062 --- openerp/api.py | 4 ++-- openerp/fields.py | 5 +++-- openerp/models.py | 13 +++++-------- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/openerp/api.py b/openerp/api.py index da9e62f1549..d13bcd57567 100644 --- a/openerp/api.py +++ b/openerp/api.py @@ -2,7 +2,7 @@ ############################################################################## # # OpenERP, Open Source Management Solution -# Copyright (C) 2013 OpenERP (). +# Copyright (C) 2013-2014 OpenERP (). # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU Affero General Public License as @@ -717,7 +717,7 @@ class Environment(object): self.cache = defaultdict(dict) # {field: {id: value, ...}, ...} self.prefetch = defaultdict(set) # {model_name: set(id), ...} self.computed = defaultdict(set) # {field: set(id), ...} - self.dirty = set() # set(record) + self.dirty = defaultdict(set) # {record: set(field_name), ...} self.all = envs envs.add(self) return self diff --git a/openerp/fields.py b/openerp/fields.py index 43b63f84b13..69869db6e4e 100644 --- a/openerp/fields.py +++ b/openerp/fields.py @@ -791,7 +791,7 @@ class Field(object): if env.in_onchange: for invf in self.inverse_fields: invf._update(value, record) - record._dirty = True + record._set_dirty_by(self.name) # determine more dependent fields, and invalidate them if self.relational: @@ -1578,7 +1578,8 @@ class _RelationalMulti(_Relational): # add new and existing records for record in value: if not record.id or record._dirty: - values = dict((k, v) for k, v in record._cache.iteritems() if k in fnames) + dirty_fields = record.env.dirty[record] + values = dict((k, v) for k, v in record._cache.iteritems() if k in fnames and k in dirty_fields) values = record._convert_to_write(values) if not record.id: result.append((0, 0, values)) diff --git a/openerp/models.py b/openerp/models.py index df46fce4d5d..d44ced403a4 100644 --- a/openerp/models.py +++ b/openerp/models.py @@ -5320,15 +5320,12 @@ class BaseModel(object): def _dirty(self): """ Return whether any record in `self` is dirty. """ dirty = self.env.dirty - return any(record in dirty for record in self) + return any(bool(dirty.get(record)) for record in self) - @_dirty.setter - def _dirty(self, value): - """ Mark the records in `self` as dirty. """ - if value: - map(self.env.dirty.add, self) - else: - map(self.env.dirty.discard, self) + def _set_dirty_by(self, field_name): + dirty = self.env.dirty + for record in self: + dirty[record].add(field_name) # # "Dunder" methods From bc8c7596a5df8c67ed1a087dda93af01977a8c0a Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Mon, 24 Nov 2014 15:31:18 +0100 Subject: [PATCH 2/4] [IMP] models: rework the API that deals with dirty fields on records --- openerp/fields.py | 7 +++---- openerp/models.py | 17 +++++++++++------ 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/openerp/fields.py b/openerp/fields.py index 69869db6e4e..d820659d0f6 100644 --- a/openerp/fields.py +++ b/openerp/fields.py @@ -791,7 +791,7 @@ class Field(object): if env.in_onchange: for invf in self.inverse_fields: invf._update(value, record) - record._set_dirty_by(self.name) + record._set_dirty(self.name) # determine more dependent fields, and invalidate them if self.relational: @@ -1577,9 +1577,8 @@ class _RelationalMulti(_Relational): # add new and existing records for record in value: - if not record.id or record._dirty: - dirty_fields = record.env.dirty[record] - values = dict((k, v) for k, v in record._cache.iteritems() if k in fnames and k in dirty_fields) + if not record.id or record._is_dirty(): + values = {k: record._cache[k] for k in record._get_dirty() if k in fnames} values = record._convert_to_write(values) if not record.id: result.append((0, 0, values)) diff --git a/openerp/models.py b/openerp/models.py index d44ced403a4..36c29e3ee0c 100644 --- a/openerp/models.py +++ b/openerp/models.py @@ -5313,16 +5313,21 @@ class BaseModel(object): return record # - # Dirty flag, to mark records modified (in draft mode) + # Dirty flags, to mark record fields modified (in draft mode) # - @property - def _dirty(self): + def _is_dirty(self): """ Return whether any record in `self` is dirty. """ dirty = self.env.dirty - return any(bool(dirty.get(record)) for record in self) + return any(record in dirty for record in self) - def _set_dirty_by(self, field_name): + def _get_dirty(self): + """ Return the list of field names for which `self` is dirty. """ + dirty = self.env.dirty + return list(dirty.get(self, ())) + + def _set_dirty(self, field_name): + """ Mark the records in `self` as dirty for the given `field_name`. """ dirty = self.env.dirty for record in self: dirty[record].add(field_name) @@ -5727,7 +5732,7 @@ class BaseModel(object): field = self._fields[name] newval = record[name] if field.type in ('one2many', 'many2many'): - if newval != oldval or newval._dirty: + 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), From 5ae3215f2184bd50cc9bd995d4d4e4d30d71e1a6 Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Mon, 24 Nov 2014 17:22:47 +0100 Subject: [PATCH 3/4] [FIX] test_new_api: fix/simplify the result of the onchange on one2many fields --- openerp/addons/test_new_api/tests/test_onchange.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/openerp/addons/test_new_api/tests/test_onchange.py b/openerp/addons/test_new_api/tests/test_onchange.py index da28bdf0c1b..709e07f5551 100644 --- a/openerp/addons/test_new_api/tests/test_onchange.py +++ b/openerp/addons/test_new_api/tests/test_onchange.py @@ -140,14 +140,10 @@ class TestOnChange(common.TransactionCase): self.assertItemsEqual(result['value']['messages'], [ (0, 0, { 'name': "[%s] %s" % ("Foo", USER.name), - 'body': BODY, - 'author': USER.id, - 'size': len(BODY), }), (1, message.id, { 'name': "[%s] %s" % ("Foo", USER.name), - 'body': BODY, - 'author': USER.id, + # Note: size is computed because it was not provided beforehand 'size': len(BODY), }), ]) From ebdbd9f8b74547d81fcfef36fb1595760daaaa3b Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Tue, 25 Nov 2014 11:25:50 +0100 Subject: [PATCH 4/4] [FIX] fields: in *2many convert_to_write(), return all fields for new records and dirty fields for existing records --- openerp/addons/test_new_api/tests/test_onchange.py | 3 +++ openerp/fields.py | 11 ++++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/openerp/addons/test_new_api/tests/test_onchange.py b/openerp/addons/test_new_api/tests/test_onchange.py index 709e07f5551..e9b7e1f83aa 100644 --- a/openerp/addons/test_new_api/tests/test_onchange.py +++ b/openerp/addons/test_new_api/tests/test_onchange.py @@ -140,6 +140,9 @@ class TestOnChange(common.TransactionCase): self.assertItemsEqual(result['value']['messages'], [ (0, 0, { 'name': "[%s] %s" % ("Foo", USER.name), + 'body': BODY, + 'author': USER.id, + 'size': len(BODY), }), (1, message.id, { 'name': "[%s] %s" % ("Foo", USER.name), diff --git a/openerp/fields.py b/openerp/fields.py index d820659d0f6..3d08eb0e464 100644 --- a/openerp/fields.py +++ b/openerp/fields.py @@ -1577,13 +1577,14 @@ class _RelationalMulti(_Relational): # add new and existing records for record in value: - if not record.id or record._is_dirty(): + if not record.id: + values = {k: v for k, v in record._cache.iteritems() if k in fnames} + values = record._convert_to_write(values) + result.append((0, 0, values)) + elif record._is_dirty(): values = {k: record._cache[k] for k in record._get_dirty() if k in fnames} values = record._convert_to_write(values) - if not record.id: - result.append((0, 0, values)) - else: - result.append((1, record.id, values)) + result.append((1, record.id, values)) else: add_existing(record.id)