From 6cd17d575c7d970286ec3dfedcfacf01be5f7592 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Fri, 17 Nov 2017 13:17:16 +0100
Subject: [PATCH] [FIX] web: properly refetch x2manys in some cases

Consider the following situation: a form view with a one2many which
contains sub x2many fields (for example a m2m_tags). Suppose that the
one2many has more than 40 records.  Before this commit, trying to delete
one of these records caused a crash.

The reason is that when we delete a record, we need to reload the
missing records in the one2many field.  We actually do that with a
_readUngroupedList.  However, the _readUngroupedList method did not
process x2many fields, so the information stored in the datapoints was
'raw', and the rest of the web client expected it to be a datapoint.

To reproduce the issue in the testing environment, i had to use
onchanges and a readonly attrs, to reach a situation where a crash could
occur.  This shows that, while the issue is critical, it was not totally
trivial to be impacted by it.
---
 .../static/src/js/views/basic/basic_model.js  |  4 +-
 .../tests/fields/relational_fields_tests.js   | 46 +++++++++++++++++++
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/addons/web/static/src/js/views/basic/basic_model.js b/addons/web/static/src/js/views/basic/basic_model.js
index fcd9785b9a5..ed18be034af 100644
--- a/addons/web/static/src/js/views/basic/basic_model.js
+++ b/addons/web/static/src/js/views/basic/basic_model.js
@@ -1573,7 +1573,9 @@ var BasicModel = AbstractModel.extend({
         return $.when.apply($, defs).then(function () {
             // ensure to fetch up to 'limit' records (may be useful if records of
             // the current page have been removed)
-            return self._readUngroupedList(list);
+            return self._readUngroupedList(list).then(function () {
+                return self._fetchX2ManysBatched(list);
+            });
         });
     },
     /**
diff --git a/addons/web/static/tests/fields/relational_fields_tests.js b/addons/web/static/tests/fields/relational_fields_tests.js
index b0b966a01c1..eed533fb6fe 100644
--- a/addons/web/static/tests/fields/relational_fields_tests.js
+++ b/addons/web/static/tests/fields/relational_fields_tests.js
@@ -5084,6 +5084,52 @@ QUnit.module('relational_fields', {
         form.destroy();
     });
 
+    QUnit.test('many2manytag in one2many, onchange, some modifiers, and more than one page', function (assert) {
+        assert.expect(9);
+
+        this.data.partner.records[0].turtles = [1,2,3];
+
+        this.data.partner.onchanges.turtles = function () {};
+
+        var form = createView({
+            View: FormView,
+            model: 'partner',
+            data: this.data,
+            arch: '<form string="Partners">' +
+                    '<field name="turtles">' +
+                        '<tree editable="top" limit="2">' +
+                            '<field name="turtle_foo"/>' +
+                            '<field name="partner_ids" widget="many2many_tags" attrs="{\'readonly\': [(\'turtle_foo\', \'=\', \'a\')]}"/>' +
+                        '</tree>' +
+                    '</field>' +
+                '</form>',
+            res_id: 1,
+            viewOptions: {mode: 'edit'},
+            mockRPC: function (route, args) {
+                assert.step(args.method);
+                return this._super.apply(this, arguments);
+            },
+        });
+        assert.strictEqual(form.$('.o_data_row').length, 2,
+            'there should be only 2 rows displayed');
+        form.$('.o_list_record_delete').click();
+        form.$('.o_list_record_delete').click();
+
+        assert.strictEqual(form.$('.o_data_row').length, 1,
+            'there should be just one remaining row');
+
+        assert.verifySteps([
+            "read",  // initial read on partner
+            "read",  // initial read on turtle
+            "read",  // batched read on partner (field partner_ids)
+            "read",  // after first delete, read on turtle (to fetch 3rd record)
+            "onchange",  // after first delete, onchange on field turtles
+            "onchange"   // onchange after second delete
+        ]);
+
+        form.destroy();
+    });
+
     QUnit.test('load view for x2many in one2many', function (assert) {
         assert.expect(2);