From e36ad5b0b01aca802e835b82e1fbb79a0d40e659 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Thu, 21 Feb 2019 10:20:49 +0000 Subject: [PATCH] [FIX] core: handling of o2m record deletion in SSF The SSF would properly mark its own records as deleted, but it would not properly handle deletion requests coming from an onchange, and it would not necessarily convert DELETE_ALL commands (5) into the proper sequence of individual (2)s matching existing records. closes odoo/odoo#31431 --- odoo/addons/test_new_api/tests/test_ui.py | 7 ++++ .../ir.model.access.csv | 2 ++ odoo/addons/test_testing_utilities/menu.xml | 17 ++++++++-- odoo/addons/test_testing_utilities/models.py | 22 ++++++++++++ .../tests/test_form_impl.py | 34 +++++++++++++++++++ odoo/tests/common.py | 10 ++++-- 6 files changed, 87 insertions(+), 5 deletions(-) diff --git a/odoo/addons/test_new_api/tests/test_ui.py b/odoo/addons/test_new_api/tests/test_ui.py index 7291e2b7c3e..5afc2876f4f 100644 --- a/odoo/addons/test_new_api/tests/test_ui.py +++ b/odoo/addons/test_new_api/tests/test_ui.py @@ -5,6 +5,13 @@ import odoo.tests class TestUi(odoo.tests.HttpCase): def test_01_admin_widget_x2many(self): + # FIXME: breaks if too many children of base.menu_tests + + # This tour turns out to be quite sensible to the number of items in + # the base.menu_tests: it's specifically sequenced to be lower (after) + # the default, but doesn't account for the fact that it could + # "fall off" into the "o_extra_menu_items" section if the window is + # too small or there are too many items preceding it in the tests menu self.phantom_js("/web#action=test_new_api.action_discussions", "odoo.__DEBUG__.services['web_tour.tour'].run('widget_x2many', 100)", "odoo.__DEBUG__.services['web_tour.tour'].tours.widget_x2many.ready", diff --git a/odoo/addons/test_testing_utilities/ir.model.access.csv b/odoo/addons/test_testing_utilities/ir.model.access.csv index 697a7334581..c069b568ad4 100644 --- a/odoo/addons/test_testing_utilities/ir.model.access.csv +++ b/odoo/addons/test_testing_utilities/ir.model.access.csv @@ -14,3 +14,5 @@ access_test_testing_utilities_sub2,access_test_testing_utilities_sub2,model_test access_test_testing_utilities_sub3,access_test_testing_utilities_sub3,model_test_testing_utilities_sub3,,1,0,0,0 access_test_testing_utilities_onchange_parent,access_test_testing_utilities_onchange_parent,model_test_testing_utilities_onchange_parent,,1,0,0,0 access_test_testing_utilities_onchange_line,access_test_testing_utilities_onchange_line,model_test_testing_utilities_onchange_line,,1,0,0,0 +access_model_test_testing_utilities_onchange_count,access_model_test_testing_utilities_onchange_count,model_test_testing_utilities_onchange_count,,1,1,1,1 +access_test_testing_utilities_onchange_count_sub,access_test_testing_utilities_onchange_count_sub,model_test_testing_utilities_onchange_count_sub,,1,1,1,1 diff --git a/odoo/addons/test_testing_utilities/menu.xml b/odoo/addons/test_testing_utilities/menu.xml index 3ca18ef2a50..fa5f4f37163 100644 --- a/odoo/addons/test_testing_utilities/menu.xml +++ b/odoo/addons/test_testing_utilities/menu.xml @@ -1,4 +1,6 @@ + + Test o2m onchange thing ir.actions.act_window @@ -6,7 +8,7 @@ tree,form - @@ -17,10 +19,21 @@ tree,form - + + test_change_number_of_lines + ir.actions.act_window + test_testing_utilities.onchange_count + tree,form + + + + Parent External Form 1 diff --git a/odoo/addons/test_testing_utilities/models.py b/odoo/addons/test_testing_utilities/models.py index ffb27cd08d8..dee6dff08fb 100644 --- a/odoo/addons/test_testing_utilities/models.py +++ b/odoo/addons/test_testing_utilities/models.py @@ -211,3 +211,25 @@ class M2OOnchangeLine(models.Model): @api.onchange('dummy') def _onchange_flag(self): self.flag = True + +class O2MChangeCount(models.Model): + _name = 'test_testing_utilities.onchange_count' + _description = _name + + count = fields.Integer() + line_ids = fields.One2many('test_testing_utilities.onchange_count_sub', 'parent') + + @api.onchange('count') + def _onchange_count(self): + Sub = self.env['test_testing_utilities.onchange_count_sub'] + recs = Sub + for i in range(self.count): + recs |= Sub.new({'name': str(i)}) + self.line_ids = recs + +class O2MChangeSub(models.Model): + _name = 'test_testing_utilities.onchange_count_sub' + _description = _name + + parent = fields.Many2one('test_testing_utilities.onchange_count') + name = fields.Char() 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 344ae62f4d2..c13dbd73a60 100644 --- a/odoo/addons/test_testing_utilities/tests/test_form_impl.py +++ b/odoo/addons/test_testing_utilities/tests/test_form_impl.py @@ -432,6 +432,40 @@ class TestO2M(TransactionCase): with f.line_ids.edit(index=0) as new_line: self.assertTrue(new_line.flag) + def test_o2m_remove(self): + def commands(): + return [c[0] for c in f._values['line_ids']] + f = Form(self.env['test_testing_utilities.onchange_count']) + + self.assertEqual(f.count, 0) + self.assertEqual(len(f.line_ids), 0) + + f.count = 5 + self.assertEqual(f.count, 5) + self.assertEqual(len(f.line_ids), 5) + + f.count = 2 + self.assertEqual(f.count, 2) + self.assertEqual(len(f.line_ids), 2) + + f.count = 4 + + r = f.save() + + previous = r.line_ids + self.assertEqual(len(previous), 4) + + with Form(r) as f: + f.count = 2 + self.assertEqual(commands(), [0, 0, 2, 2, 2, 2], "Should contain 2 creations and 4 deletions") + self.assertEqual(len(r.line_ids), 2) + + with Form(r) as f: + f.line_ids.remove(0) + self.assertEqual(commands(), [2, 1]) + f.count = 1 + self.assertEqual(commands(), [0, 2, 2], "should contain 1 '0' command and 2 deletions") + self.assertEqual(len(r.line_ids), 1) class TestEdition(TransactionCase): """ These use the context manager form as we don't need the record diff --git a/odoo/tests/common.py b/odoo/tests/common.py index 447c6ace2f0..dd808aaec30 100644 --- a/odoo/tests/common.py +++ b/odoo/tests/common.py @@ -1435,12 +1435,12 @@ class Form(object): return [] v = [] - c = {t[1]: t[2] for t in current if t[0] == 1} if current else {} + c = {t[1]: t[2] for t in current if t[0] in (1, 2)} if current else {} # which view should this be??? subfields = descr['views']['edition']['fields'] for command in value: - # TODO: get existing sub-values so we can pass them along? if command[0] in (0, 1): + c.pop(command[1], None) # remove record from currents v.append((command[0], command[1], { k: self._cleanup_onchange( subfields[k], v, None @@ -1448,10 +1448,14 @@ class Form(object): for k, v in command[2].items() if k in subfields })) + elif command[0] == 2: + v.append((2, command[1], False)) elif command[0] == 4: - v.append((1, command[1], c.get(command[1], {}))) + v.append((1, command[1], c.pop(command[1], {}))) elif command[0] == 5: v = [] + # explicitly mark all non-relinked (or modified) records as deleted + for id_ in c: v.append((2, id_, False)) return v elif descr['type'] == 'many2many': # onchange result is a bunch of commands, normalize to single 6