From a48382124f84bd64ea741a2c5e50dd540f42416c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Krier?= Date: Sun, 12 Apr 2020 23:18:15 +0200 Subject: [PATCH] Support explicit delete and remove for saving and on_change xxx2Many issue4140 review291281002 --- CHANGELOG | 1 + doc/ref/models/fields.rst | 23 +++++-- trytond/model/fields/many2many.py | 10 +++ trytond/model/fields/one2many.py | 10 +++ trytond/model/model.py | 3 +- trytond/model/modelstorage.py | 32 +++++++-- trytond/model/modelview.py | 36 +++++++--- trytond/tests/modelstorage.py | 25 +++++++ trytond/tests/test_modelstorage.py | 107 +++++++++++++++++++++++++++++ trytond/tests/test_modelview.py | 45 ++++++++++-- 10 files changed, 266 insertions(+), 26 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index dd649638d1..5df672d4c8 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,4 @@ +* Support explicit delete and remove for saving and on_change xxx2Many * Add export_data_domain to ModelStorage * Add route to export CSV data * Add test on target of relation fields diff --git a/doc/ref/models/fields.rst b/doc/ref/models/fields.rst index 8dd857c909..1dc2bcf36f 100644 --- a/doc/ref/models/fields.rst +++ b/doc/ref/models/fields.rst @@ -757,6 +757,12 @@ This field accepts as written value a list of tuples like this: Same as :attr:`Many2One.search_context` +Instance methods: + +.. method:: One2Many.remove(instance, records) + + Remove the target records from the instance instead of deleting them. + Many2Many --------- @@ -815,6 +821,15 @@ This field accepts as written value a list of tuples like the :class:`One2Many`. Same as :attr:`One2Many.filter` +.. attribute:: Many2Many.search_order + + Same as :attr:`Many2One.search_order` + +.. attribute:: Many2Many.search_context + + Same as :attr:`Many2One.search_context` + + Instance methods: .. method:: Many2Many.get_relation() @@ -825,13 +840,9 @@ Instance methods: Return the target :class:`~trytond.model.Model`. -.. attribute:: Many2Many.search_order +.. method:: Many2Many.delete(instance, records): - Same as :attr:`Many2One.search_order` - -.. attribute:: Many2Many.search_context - - Same as :attr:`Many2One.search_context` + Delete the target records from the instance instead of removing them. One2One ------- diff --git a/trytond/model/fields/many2many.py b/trytond/model/fields/many2many.py index a8ea780908..48351be36f 100644 --- a/trytond/model/fields/many2many.py +++ b/trytond/model/fields/many2many.py @@ -1,5 +1,6 @@ # This file is part of Tryton. The COPYRIGHT file at the top level of # this repository contains the full copyright notices and license terms. +from collections import defaultdict from itertools import chain from sql import Cast, Literal, Null @@ -286,6 +287,15 @@ class Many2Many(Field): records = instanciate_values(Target, value) super(Many2Many, self).__set__(inst, records) + def delete(self, inst, records): + records = set(records) + if inst._deleted is None: + inst._deleted = defaultdict(set) + inst._deleted[self.name].update(map(int, records)) + setattr( + inst, self.name, + [r for r in getattr(inst, self.name) if r not in records]) + def convert_domain_tree(self, domain, tables): Target = self.get_target() table, _ = tables[None] diff --git a/trytond/model/fields/one2many.py b/trytond/model/fields/one2many.py index a760eb11d7..d0155b0a4d 100644 --- a/trytond/model/fields/one2many.py +++ b/trytond/model/fields/one2many.py @@ -1,5 +1,6 @@ # This file is part of Tryton. The COPYRIGHT file at the top level of # this repository contains the full copyright notices and license terms. +from collections import defaultdict from itertools import chain from sql import Cast, Literal from sql.functions import Substring, Position @@ -260,6 +261,15 @@ class One2Many(Field): records = instanciate_values(Target, value, **extra) super(One2Many, self).__set__(inst, records) + def remove(self, inst, records): + records = set(records) + if inst._removed is None: + inst._removed = defaultdict(set) + inst._removed[self.name].update(map(int, records)) + setattr( + inst, self.name, + [r for r in getattr(inst, self.name) if r not in records]) + def convert_domain(self, domain, tables, Model): from ..modelsql import convert_from pool = Pool() diff --git a/trytond/model/model.py b/trytond/model/model.py index c4ab96fc70..66aedd749e 100644 --- a/trytond/model/model.py +++ b/trytond/model/model.py @@ -29,7 +29,7 @@ class Model(URLMixin, PoolBase, metaclass=ModelMeta): """ Define a model in Tryton. """ - __slots__ = ('_id', '_values', '_init_values') + __slots__ = ('_id', '_values', '_init_values', '_removed', '_deleted') _rec_name = 'name' id = fields.Integer(lazy_gettext('ir.msg_ID'), readonly=True) @@ -246,6 +246,7 @@ class Model(URLMixin, PoolBase, metaclass=ModelMeta): if id is not None: id = int(id) self._id = id + self._deleted = self._removed = None if kwargs: self._values = {} parent_values = defaultdict(dict) diff --git a/trytond/model/modelstorage.py b/trytond/model/modelstorage.py index 9b19a3a663..126c45e86a 100644 --- a/trytond/model/modelstorage.py +++ b/trytond/model/modelstorage.py @@ -1608,11 +1608,11 @@ class ModelStorage(Model): if self.id is not None and self.id >= 0: _values, self._values = self._values, None try: - to_remove = [t.id for t in getattr(self, fname)] + previous = [t.id for t in getattr(self, fname)] finally: self._values = _values else: - to_remove = [] + previous = [] to_add = [] to_create = [] to_write = [] @@ -1629,8 +1629,8 @@ class ModelStorage(Model): if target.id is None or target.id < 0: to_create.append(target._save_values) else: - if target.id in to_remove: - to_remove.remove(target.id) + if target.id in previous: + previous.remove(target.id) else: to_add.append(target.id) target_values = target._save_values @@ -1641,8 +1641,26 @@ class ModelStorage(Model): if t_values: target._values = t_values value = [] - if to_remove: - value.append(('remove', to_remove)) + if previous: + to_delete, to_remove = [], [] + deleted = removed = None + if self._deleted: + deleted = self._deleted[fname] + if self._removed: + removed = self._removed[fname] + for id_ in previous: + if deleted and id_ in deleted: + to_delete.append(id_) + elif removed and id_ in removed: + to_remove.append(id_) + elif field._type == 'one2many': + to_delete.append(id_) + else: + to_remove.append(id_) + if to_delete: + value.append(('delete', to_delete)) + if to_remove: + value.append(('remove', to_remove)) if to_add: value.append(('add', to_add)) if to_create: @@ -1698,6 +1716,8 @@ class ModelStorage(Model): raise for record in to_create + to_write: record._init_values = None + record._deleted = None + record._removed = None records = latter diff --git a/trytond/model/modelview.py b/trytond/model/modelview.py index 9e7855f874..d8e0df2011 100644 --- a/trytond/model/modelview.py +++ b/trytond/model/modelview.py @@ -793,11 +793,12 @@ class ModelView(Model): value = str(value) else: value = value.id - elif field._type == 'one2many': + elif field._type in ['one2many', 'many2many']: targets = value - init_targets = list(init_values.get(fname, targets)) + init_targets = list(init_values.get( + fname, targets if field._type == 'one2many' else [])) value = collections.defaultdict(list) - value['remove'] = [t.id for t in init_targets if t.id] + previous = [t.id for t in init_targets if t.id] for i, target in enumerate(targets): if (field._type == 'one2many' and field.field @@ -808,8 +809,8 @@ class ModelView(Model): else: t_values = None try: - if target.id in value['remove']: - value['remove'].remove(target.id) + if target.id in previous: + previous.remove(target.id) if isinstance(target, ModelView): target_changed = target._changed_values if target_changed: @@ -827,16 +828,33 @@ class ModelView(Model): target._init_values = target_init_values else: added_values = target._default_values + added_values['id'] = target.id value['add'].append((i, added_values)) finally: if t_values: target._values = t_values - if not value['remove']: - del value['remove'] + if previous: + to_delete, to_remove = [], [] + deleted = removed = None + if self._deleted: + deleted = self._deleted[fname] + if self._removed: + removed = self._removed[fname] + for id_ in previous: + if deleted and id_ in deleted: + to_delete.append(id_) + elif removed and id_ in removed: + to_remove.append(id_) + elif field._type == 'one2many': + to_delete.append(id_) + else: + to_remove.append(id_) + if to_delete: + value['delete'] = to_delete + if to_remove: + value['remove'] = to_remove if not value: continue value = dict(value) - elif field._type == 'many2many': - value = [r.id for r in value] changed[fname] = value return changed diff --git a/trytond/tests/modelstorage.py b/trytond/tests/modelstorage.py index edc81353cc..e8275463a4 100644 --- a/trytond/tests/modelstorage.py +++ b/trytond/tests/modelstorage.py @@ -18,6 +18,28 @@ class ModelStorageRequired(ModelSQL): name = fields.Char('Name', required=True) +class ModelStorageSave2Many(ModelSQL): + "Model Storage to test save with xxx2many" + __name__ = 'test.modelstorage.save2many' + targets = fields.One2Many( + 'test.modelstorage.save2many.target', 'parent', "Targets") + m2m_targets = fields.Many2Many( + 'test.modelstorage.save2many.relation', 'parent', 'target', "Targets") + + +class ModelStorageSave2ManyTarget(ModelSQL): + "Model Storage to test save with xxx2many" + __name__ = 'test.modelstorage.save2many.target' + parent = fields.Many2One('test.modelstorage.save2many', "Parent") + + +class ModelStorageSave2ManyRelation(ModelSQL): + "Model Storage to test save with xxx2many" + __name__ = 'test.modelstorage.save2many.relation' + parent = fields.Many2One('test.modelstorage.save2many', "Parent") + target = fields.Many2One('test.modelstorage.save2many.target', "Target") + + class ModelStorageContext(ModelSQL): 'Model Storage to test Context' __name__ = 'test.modelstorage.context' @@ -94,6 +116,9 @@ def register(module): Pool.register( ModelStorage, ModelStorageRequired, + ModelStorageSave2Many, + ModelStorageSave2ManyTarget, + ModelStorageSave2ManyRelation, ModelStorageContext, ModelStoragePYSONDomain, ModelStorageRelationDomain, diff --git a/trytond/tests/test_modelstorage.py b/trytond/tests/test_modelstorage.py index 5800993f62..b0696fd1d8 100644 --- a/trytond/tests/test_modelstorage.py +++ b/trytond/tests/test_modelstorage.py @@ -141,6 +141,113 @@ class ModelStorageTestCase(unittest.TestCase): self.assertEqual(foo.name, 'foo') self.assertIsNone(bar.name) + @with_transaction() + def test_save_one2many_create(self): + "Test save one2many create" + pool = Pool() + ModelStorage = pool.get('test.modelstorage.save2many') + Target = pool.get('test.modelstorage.save2many.target') + + record = ModelStorage() + record.targets = [Target()] + record.save() + + self.assertTrue(record.id) + self.assertEqual(len(record.targets), 1) + + @with_transaction() + def test_save_one2many_add(self): + "Test save one2many add" + pool = Pool() + ModelStorage = pool.get('test.modelstorage.save2many') + Target = pool.get('test.modelstorage.save2many.target') + + target = Target() + target.save() + record = ModelStorage() + record.targets = [target] + record.save() + + self.assertTrue(record.id) + self.assertEqual(len(record.targets), 1) + self.assertEqual(len(Target.search([])), 1) + + @with_transaction() + def test_save_one2many_delete(self): + "Test save one2many delete" + pool = Pool() + ModelStorage = pool.get('test.modelstorage.save2many') + Target = pool.get('test.modelstorage.save2many.target') + + record, = ModelStorage.create([{'targets': [('create', [{}])]}]) + record.targets = [] + record.save() + + self.assertEqual(len(record.targets), 0) + self.assertEqual(Target.search([], count=True), 0) + + @with_transaction() + def test_save_one2many_remove(self): + "Test save one2many remove" + pool = Pool() + ModelStorage = pool.get('test.modelstorage.save2many') + Target = pool.get('test.modelstorage.save2many.target') + + record, = ModelStorage.create([{'targets': [('create', [{}])]}]) + ModelStorage.targets.remove(record, record.targets) + record.save() + + self.assertEqual(len(record.targets), 0) + self.assertEqual(Target.search([], count=True), 1) + + @with_transaction() + def test_save_many2many_add(self): + "Test save many2many add" + pool = Pool() + ModelStorage = pool.get('test.modelstorage.save2many') + Target = pool.get('test.modelstorage.save2many.target') + + target = Target() + target.save() + record = ModelStorage() + record.m2m_targets = [target] + record.save() + + self.assertTrue(record.id) + self.assertEqual(len(record.m2m_targets), 1) + self.assertEqual(len(Target.search([])), 1) + + @with_transaction() + def test_save_many2many_delete(self): + "Test save many2many delete" + pool = Pool() + ModelStorage = pool.get('test.modelstorage.save2many') + Target = pool.get('test.modelstorage.save2many.target') + + record, = ModelStorage.create([{'m2m_targets': [('create', [{}])]}]) + ModelStorage.m2m_targets.delete(record, record.m2m_targets) + record.save() + + self.assertEqual(len(record.targets), 0) + self.assertEqual(Target.search([], count=True), 0) + + @with_transaction() + def test_save_many2many_remove(self): + "Test save one2many remove" + pool = Pool() + ModelStorage = pool.get('test.modelstorage.save2many') + Target = pool.get('test.modelstorage.save2many.target') + + target = Target() + target.save() + record, = ModelStorage.create([ + {'m2m_targets': [('add', [target.id])]}]) + record.m2m_targets = [] + record.save() + + self.assertEqual(len(record.m2m_targets), 0) + self.assertEqual(Target.search([], count=True), 1) + @with_transaction(context={'_check_access': True}) def test_model_translations(self): 'Test any user can translate fields and duplicate its records' diff --git a/trytond/tests/test_modelview.py b/trytond/tests/test_modelview.py index 56e3ab53b7..1865bd8f87 100644 --- a/trytond/tests/test_modelview.py +++ b/trytond/tests/test_modelview.py @@ -40,7 +40,7 @@ class ModelView(unittest.TestCase): 'ref_target': 'test.modelview.changed_values.target,2', 'targets': { 'add': [ - (0, {'name': 'bar'}), + (0, {'id': None, 'name': 'bar'}), ], }, }) @@ -59,9 +59,12 @@ class ModelView(unittest.TestCase): self.assertEqual(record._changed_values, { 'targets': { 'update': [{'id': 1, 'name': 'bar'}], - 'remove': [2], + 'delete': [2], + }, + 'm2m_targets': { + 'remove': [5, 6, 7], + 'add': [(0, {'id': 9}), (1, {'id': 10})], }, - 'm2m_targets': [9, 10], }) # change only one2many record @@ -89,6 +92,40 @@ class ModelView(unittest.TestCase): }, }) + @with_transaction() + def test_changed_values_removed(self): + "Test removed" + pool = Pool() + Model = pool.get('test.modelview.changed_values') + Target = pool.get('test.modelview.changed_values.target') + + target = Target(1) + record = Model(targets=[target]) + Model.targets.remove(record, [target]) + + self.assertEqual(record._changed_values, { + 'targets': { + 'remove': [1], + }, + }) + + @with_transaction() + def test_changed_values_deleted(self): + "Test deleted" + pool = Pool() + Model = pool.get('test.modelview.changed_values') + Target = pool.get('test.modelview.changed_values.target') + + target = Target(1) + record = Model(m2m_targets=[target]) + Model.m2m_targets.delete(record, [target]) + + self.assertEqual(record._changed_values, { + 'm2m_targets': { + 'delete': [1], + }, + }) + @with_transaction() def test_changed_values_rec_name(self): "Test rec_name of ModelStorage is added" @@ -123,7 +160,7 @@ class ModelView(unittest.TestCase): self.assertEqual(record._changed_values, { 'name': "Record", 'targets': { - 'add': [(0, {'name': "Target"})], + 'add': [(0, {'id': None, 'name': "Target"})], }, })