From 469dd4cad9e612c1db21658658d8eaa8fa265e7e Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Thu, 4 Mar 2021 15:11:38 +0000 Subject: [PATCH] [FIX] web: correctly edit Many2ManyCheckboxes with 100+ values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Many2ManyCheckboxes widget displays all values that could be in the many2many relation, with a checkbox indicating whether each value is in the relation or not. It is designed to be set on fields where the comodel contains a few records (typically, we don't want to see dozens of checkboxes in the form view). This widget shouldn't be used on many2manys with a large comodel, as we have better tools to handle them (like a tree view). We deal with extreme cases (when the widget is, by mistake, set on a field where the comodel is huge) by using the name_search limit of 100: at most 100 checkboxes are displayed. Before this commit, this extreme situation wasn't correctly handled. If there were in the relation records that weren't displayed (because they weren't inside the 100 limit), then, editing the value by (un)selecting a checkbox would automatically remove all non displayed values from the relation. This commit ensures that we keep in the relation all values that aren't displayed. Issue spotted when working on opw~2439041 closes odoo/odoo#67400 X-original-commit: 9e9d3aa78c42ad4ffca3b56a28382ed84078cde3 Signed-off-by: Géry Debongnie (ged) Signed-off-by: Aaron Bohy (aab) --- .../static/src/js/fields/relational_fields.js | 11 ++++ .../tests/fields/relational_fields_tests.js | 56 +++++++++++++++++++ .../web/static/tests/helpers/mock_server.js | 7 +-- doc/reference/javascript_reference.rst | 7 ++- 4 files changed, 75 insertions(+), 6 deletions(-) diff --git a/addons/web/static/src/js/fields/relational_fields.js b/addons/web/static/src/js/fields/relational_fields.js index 5a9b847bb8e..fac5d69d2c1 100644 --- a/addons/web/static/src/js/fields/relational_fields.js +++ b/addons/web/static/src/js/fields/relational_fields.js @@ -2914,9 +2914,20 @@ var FieldMany2ManyCheckBoxes = AbstractField.extend({ */ _onChange: function () { if (this.mode !== 'readonly') { + // Get the list of selected ids var ids = _.map(this.$('input:checked'), function (input) { return $(input).data("record-id"); }); + // The number of displayed checkboxes is limited to 100 (name_search + // limit, server-side), to prevent extreme cases where thousands of + // records are fetched/displayed. If not all values are displayed, it may + // happen that some values that are in the relation aren't available in the + // widget. In this case, when the user (un)selects a value, we don't + // want to remove those non displayed values from the relation. For that + // reason, we manually add those values to the list of ids. + const displayedIds = this.m2mValues.map(v => v[0]); + const idsInRelation = this.value.res_ids; + ids = ids.concat(idsInRelation.filter(a => !displayedIds.includes(a))); this._setValue({ operation: 'REPLACE_WITH', ids: ids, diff --git a/addons/web/static/tests/fields/relational_fields_tests.js b/addons/web/static/tests/fields/relational_fields_tests.js index 880a6e95409..32f46fdca59 100644 --- a/addons/web/static/tests/fields/relational_fields_tests.js +++ b/addons/web/static/tests/fields/relational_fields_tests.js @@ -2584,6 +2584,62 @@ QUnit.module('relational_fields', { form.destroy(); }); + QUnit.test('widget many2many_checkboxes with 100+ values', async function (assert) { + // The many2many_checkboxes widget limits the displayed values to 100 (this is the + // server-side name_search limit). This test encodes a scenario where there are more than + // 100 records in the co-model, and all values in the many2many relationship aren't + // displayed in the widget (due to the limit). If the user (un)selects a checkbox, we don't + // want to remove all values that aren't displayed from the relation. + assert.expect(5); + + const records = []; + for (let id = 1; id < 150; id++) { + records.push({ + id, + display_name: `type ${id}`, + color: id % 7, + }); + } + this.data.partner_type.records = records; + this.data.partner.records[0].timmy = records.map((r) => r.id); + + const form = await createView({ + View: FormView, + model: 'partner', + data: this.data, + arch: '
', + res_id: 1, + async mockRPC(route, args) { + if (args.method === 'write') { + const expectedIds = records.map((r) => r.id); + expectedIds.shift(); + assert.deepEqual(args.args[1].timmy, [[6, false, expectedIds]]); + } + const result = await this._super(...arguments); + if (args.method === 'name_search') { + assert.strictEqual(result.length, 100, + "sanity check: name_search automatically sets the limit to 100"); + } + return result; + }, + viewOptions: { + mode: 'edit', + }, + }); + + assert.containsN(form, '.o_field_widget[name=timmy] input[type=checkbox]', 100, + "should only display 100 checkboxes"); + assert.ok(form.$('.o_field_widget[name=timmy] input[type=checkbox]:first').is(':checked')); + + // toggle the first value + await testUtils.dom.click(form.$('.o_field_widget[name=timmy] input[type=checkbox]:first')); + assert.notOk(form.$('.o_field_widget[name=timmy] input[type=checkbox]:first').is(':checked')); + + await testUtils.form.clickSave(form); + + form.destroy(); + }); + QUnit.module('FieldMany2ManyBinaryMultiFiles'); QUnit.test('widget many2many_binary', async function (assert) { diff --git a/addons/web/static/tests/helpers/mock_server.js b/addons/web/static/tests/helpers/mock_server.js index 7857c2933aa..1a6f2d80b4c 100644 --- a/addons/web/static/tests/helpers/mock_server.js +++ b/addons/web/static/tests/helpers/mock_server.js @@ -1253,10 +1253,12 @@ var MockServer = Class.extend({ * @param {string} args[0] * @param {Array} args[1], search domain * @param {Object} _kwargs + * @param {number} [_kwargs.limit=100] server-side default limit * @returns {Array[]} a list of [id, display_name] */ _mockNameSearch: function (model, args, _kwargs) { var str = args && typeof args[0] === 'string' ? args[0] : _kwargs.name; + const limit = _kwargs.limit || 100; var domain = (args && args[1]) || _kwargs.args || []; var records = this._getRecords(model, domain); if (str.length) { @@ -1267,10 +1269,7 @@ var MockServer = Class.extend({ var result = _.map(records, function (record) { return [record.id, record.display_name]; }); - if (_kwargs.limit) { - return result.slice(0, _kwargs.limit); - } - return result; + return result.slice(0, limit); }, /** * Simulate an 'onchange' rpc diff --git a/doc/reference/javascript_reference.rst b/doc/reference/javascript_reference.rst index e6d0ad9b56f..7f33d6c8d97 100644 --- a/doc/reference/javascript_reference.rst +++ b/doc/reference/javascript_reference.rst @@ -2469,8 +2469,11 @@ kanban.many2many_tags (KanbanFieldMany2ManyTags) - Supported field types: *many2many* many2many_checkboxes (FieldMany2ManyCheckBoxes) - This field displays a list of checkboxes and allow the user to select a - subset of the choices. + This field displays a list of checkboxes and allows the user to select a + subset of the choices. Note that the number of displayed values is limited to + 100. This limit isn't customizable. It simply allows to handle extreme cases + where this widget is wrongly set on a field with a huge comodel. In those + cases, a list view is more adequate as it allows pagination and filtering. - Supported field types: *many2many*