[FIX] web: correctly edit Many2ManyCheckboxes with 100+ values

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#67373

X-original-commit: fa324ef071952d749f635f878bee0fa462cfa011
Signed-off-by: Géry Debongnie (ged) <ged@openerp.com>
Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>

3 files changed, 70 insertions, 4 deletions

diff --git a/addons/web/static/src/js/fields/relational_fields.js b/addons/web/static/src/js/fields/relational_fields.js
index 9efd76985cc..0ad11dba91a 100644
--- a/addons/web/static/src/js/fields/relational_fields.js
+++ b/addons/web/static/src/js/fields/relational_fields.js
@@ -2762,9 +2762,20 @@ var FieldMany2ManyCheckBoxes = AbstractField.extend({
      * @private
      */
     _onChange: function () {
+        // 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 1c0b3a6d33b..7091cff0a49 100644
--- a/addons/web/static/tests/fields/relational_fields_tests.js
+++ b/addons/web/static/tests/fields/relational_fields_tests.js
@@ -2472,6 +2472,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: '<form><field name="timmy" widget="many2many_checkboxes"/></form>',
+            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 be6ea99102b..9f3c4747add 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