From bd113d0709f11097b1a035fe9498b5cc90b24d13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Wed, 20 Sep 2017 15:15:11 +0200 Subject: [PATCH] [FIX] web: properly destroy field widgets in some cases Before this commit, the web client did not properly destroy field widgets when updating a one2many after an onchange. This is rarely an issue, because in general, there are no other field widgets besides the row currently in edition. However, if we specify explicitely a widget, or if we use a custom widget, it may be a problem. --- .../js/views/list/list_editable_renderer.js | 28 ++++++++++++++----- .../tests/fields/relational_fields_tests.js | 24 ++++++++++++++-- 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/addons/web/static/src/js/views/list/list_editable_renderer.js b/addons/web/static/src/js/views/list/list_editable_renderer.js index 31d8154f3c1..89aa3d865dd 100644 --- a/addons/web/static/src/js/views/list/list_editable_renderer.js +++ b/addons/web/static/src/js/views/list/list_editable_renderer.js @@ -127,7 +127,7 @@ ListRenderer.include({ */ confirmUpdate: function (state, id, fields, ev) { var self = this; - var oldRowIndex = _.findIndex(this.state.data, {id: id}); + var oldData = this.state.data; this.state = state; return this.confirmChange(state, id, fields, ev).then(function () { // If no record with 'id' can be found in the state, the @@ -137,9 +137,15 @@ ListRenderer.include({ if (!record) { return; } + var oldRowIndex = _.findIndex(oldData, {id: id}); var $row = self.$('.o_data_row:nth(' + oldRowIndex + ')'); $row.nextAll('.o_data_row').remove(); $row.prevAll().remove(); + _.each(oldData, function (rec) { + if (rec.id !== id) { + self._destroyFieldWidgets(rec.id); + } + }); var newRowIndex = _.findIndex(state.data, {id: id}); _.each(state.data, function (record, index) { if (index === newRowIndex) { @@ -186,7 +192,6 @@ ListRenderer.include({ * @param {string} recordID */ removeLine: function (state, recordID) { - var self = this; var rowIndex = _.findIndex(this.state.data, {id: recordID}); this.state = state; if (rowIndex === -1) { @@ -204,11 +209,7 @@ ListRenderer.include({ $row.replaceWith(this._renderEmptyRow()); } - // destroy the removed row's widgets - var widgetsToDestroy = this.allFieldWidgets[recordID].slice(); - _.each(widgetsToDestroy, function (widget) { - self._destroyFieldWidget(recordID, widget); - }); + this._destroyFieldWidgets(recordID); }, /** * Updates the already rendered row associated to the given recordID so that @@ -354,6 +355,19 @@ ListRenderer.include({ // Private //-------------------------------------------------------------------------- + /** + * Destroy all field widgets corresponding to a record. Useful when we are + * removing a useless row. + * + * @param {string} recordID + */ + _destroyFieldWidgets: function (recordID) { + if (recordID in this.allFieldWidgets) { + var widgetsToDestroy = this.allFieldWidgets[recordID].slice(); + _.each(widgetsToDestroy, this._destroyFieldWidget.bind(this, recordID)); + delete this.allFieldWidgets[recordID]; + } + }, /** * Returns the current number of columns. The editable renderer may add a * trash icon on the right of a record, so we need to take this into account diff --git a/addons/web/static/tests/fields/relational_fields_tests.js b/addons/web/static/tests/fields/relational_fields_tests.js index 31b42d7d4f1..c2159854238 100644 --- a/addons/web/static/tests/fields/relational_fields_tests.js +++ b/addons/web/static/tests/fields/relational_fields_tests.js @@ -1,6 +1,7 @@ odoo.define('web.relational_fields_tests', function (require) { "use strict"; +var AbstractField = require('web.AbstractField'); var BasicModel = require('web.BasicModel'); var concurrency = require('web.concurrency'); var FormView = require('web.FormView'); @@ -3537,7 +3538,19 @@ QUnit.module('relational_fields', { }); QUnit.test('one2many with CREATE onchanges correctly refreshed', function (assert) { - assert.expect(4); + assert.expect(5); + + var delta = 0; + var oldInit = AbstractField.prototype.init; + var oldDestroy = AbstractField.prototype.destroy; + AbstractField.prototype.init = function () { + delta++; + oldInit.apply(this, arguments); + }; + AbstractField.prototype.destroy = function () { + delta--; + oldDestroy.apply(this, arguments); + }; var deactiveOnchange = true; @@ -3584,7 +3597,7 @@ QUnit.module('relational_fields', { '' + '' + '' + - '' + + '' + '' + '' + '' + @@ -3596,7 +3609,7 @@ QUnit.module('relational_fields', { }); assert.strictEqual(form.$('.o_data_row').length, 0, - "o2m shouldnn't contain any row"); + "o2m shouldn't contain any row"); form.$('.o_field_x2many_list_row_add a').click(); // trigger the first onchange @@ -3615,12 +3628,17 @@ QUnit.module('relational_fields', { assert.strictEqual(form.$('.o_data_row').text(), "first20second-20", "should correctly refresh the records"); + assert.strictEqual(form.$('.o_field_widget').length, delta, + "all (non visible) field widgets should have been destroyed"); + form.$buttons.find('.o_form_button_save').click(); assert.strictEqual(form.$('.o_data_row').text(), "first20second-20", "should correctly refresh the records after save"); form.destroy(); + AbstractField.prototype.init = oldInit; + AbstractField.prototype.destroy = oldDestroy; }); QUnit.test('one2many list (editable): readonly domain is evaluated', function (assert) {