From 1318be22a44de1b67ccdc7065f5cafe8c170d6a5 Mon Sep 17 00:00:00 2001 From: Jorge Pinna Puissant Date: Thu, 6 Jul 2023 08:07:53 +0000 Subject: [PATCH] [FIX] web: a modified field must always be sent to the server Before this commit, when a field of type one2many or many2many, was added and afterward deleted, on the onchange or write call to the server the field wasn't added as a change (because the add and delete command cancel each other). This can cause issues because when adding the x2many field, the onchange on the server can have side effects that we want to remove when deleting the x2many. Now, we follow the following rule : If a field has been modified, its value must always be sent to the server for onchange and write. This commit also avoid to call update on the parent record if the chil record didn't have changes. Task 3179751 Part-of: odoo/odoo#114024 --- .../src/model/relational_model/record.js | 4 +--- .../src/model/relational_model/static_list.js | 2 ++ .../web/static/tests/helpers/mock_server.js | 10 ++++++++-- addons/web/static/tests/mock_server_tests.js | 6 +++--- .../views/fields/many2many_field_tests.js | 2 +- .../views/fields/one2many_field_tests.js | 20 +++++++++++++------ 6 files changed, 29 insertions(+), 15 deletions(-) diff --git a/addons/web/static/src/model/relational_model/record.js b/addons/web/static/src/model/relational_model/record.js index 48fe6b01478..ce22e47798b 100644 --- a/addons/web/static/src/model/relational_model/record.js +++ b/addons/web/static/src/model/relational_model/record.js @@ -481,9 +481,7 @@ export class Record extends DataPoint { } if (field.type === "one2many" || field.type === "many2many") { const commands = value._getCommands({ withReadonly }); - if (commands.length) { - result[fieldName] = commands; - } + result[fieldName] = commands; } else { result[fieldName] = this._formatServerValue(field.type, value); } diff --git a/addons/web/static/src/model/relational_model/static_list.js b/addons/web/static/src/model/relational_model/static_list.js index ad68f56d551..3d6b4b0d694 100644 --- a/addons/web/static/src/model/relational_model/static_list.js +++ b/addons/web/static/src/model/relational_model/static_list.js @@ -388,6 +388,8 @@ export class StaticList extends DataPoint { if (!this._currentIds.includes(record.isNew ? record._virtualId : record.resId)) { // new record created, not yet in the list await this._addRecord(record); + } else if (!record.dirty) { + return; } await this._onUpdate(); if (this.orderBy.length) { diff --git a/addons/web/static/tests/helpers/mock_server.js b/addons/web/static/tests/helpers/mock_server.js index d9757748a12..83d96f69a32 100644 --- a/addons/web/static/tests/helpers/mock_server.js +++ b/addons/web/static/tests/helpers/mock_server.js @@ -2167,7 +2167,11 @@ export class MockServer { } this.writeRecord( comodelName, - { [inverseFieldName]: inverseFieldNewValue }, + { + [inverseFieldName]: inverseFieldNewValue.length + ? inverseFieldNewValue + : false, + }, removedRecordId ); } @@ -2382,7 +2386,9 @@ export class MockServer { } if (["one2many", "many2many"].includes(field.type)) { let ids = record[fieldName] ? record[fieldName].slice() : []; - if (Array.isArray(value)) { + // if a field has been modified, its value must always be sent to the server for onchange and write. + // take into account that the value can be a empty list of commands. + if (Array.isArray(value) && value.length) { if ( value.reduce((hasOnlyInt, val) => hasOnlyInt && Number.isInteger(val), true) ) { diff --git a/addons/web/static/tests/mock_server_tests.js b/addons/web/static/tests/mock_server_tests.js index d2e6d850668..57fd9cabcf8 100644 --- a/addons/web/static/tests/mock_server_tests.js +++ b/addons/web/static/tests/mock_server_tests.js @@ -69,7 +69,7 @@ QUnit.module("MockServer", (hooks) => { partner_id: 2, partner_ids: [1], select: "new", - partner_ref: "partner,2" + partner_ref: "partner,2", }, { foo: 17, @@ -1343,8 +1343,8 @@ QUnit.module("MockServer", (hooks) => { }, ]; const mockServer = new MockServer(data); - mockServer.mockWrite("foo", [[2], { many2many_field: [] }]); - assert.deepEqual(mockServer.models.bar.records[0].many2many_field, []); + mockServer.mockWrite("foo", [[2], { many2many_field: [] }]); // save nothing + assert.deepEqual(mockServer.models.bar.records[0].many2many_field, [2]); }); QUnit.test("many2one update should update inverse field", async function (assert) { diff --git a/addons/web/static/tests/views/fields/many2many_field_tests.js b/addons/web/static/tests/views/fields/many2many_field_tests.js index 795d91393d6..c4b53ab8da2 100644 --- a/addons/web/static/tests/views/fields/many2many_field_tests.js +++ b/addons/web/static/tests/views/fields/many2many_field_tests.js @@ -1538,7 +1538,7 @@ QUnit.module("Fields", (hooks) => { await click(target.querySelector(".modal-body input[type=checkbox]")); await click(target.querySelector(".modal .modal-footer .btn-primary")); - assert.verifySteps(["write", "web_read", "onchange2"]); + assert.verifySteps(["write", "web_read"]); // there is nothing left to save -> should not do a 'write' RPC await clickSave(target); diff --git a/addons/web/static/tests/views/fields/one2many_field_tests.js b/addons/web/static/tests/views/fields/one2many_field_tests.js index a8bf7015eb6..45768ceb390 100644 --- a/addons/web/static/tests/views/fields/one2many_field_tests.js +++ b/addons/web/static/tests/views/fields/one2many_field_tests.js @@ -563,7 +563,7 @@ QUnit.module("Fields", (hooks) => { if (route === "/web/dataset/call_kw/partner/write") { assert.deepEqual( args.args[1].p[0][2], - { foo: "ff", qux: 99 }, + { foo: "ff", qux: 99, turtles: [] }, "The right values should be written" ); } @@ -3809,7 +3809,14 @@ QUnit.module("Fields", (hooks) => { await clickSave(target); assert.containsNone(target, "tr.o_data_row"); - assert.verifySteps(["get_views", "web_read", "onchange2", "onchange2"]); + assert.verifySteps([ + "get_views", + "web_read", + "onchange2", + "onchange2", + "write", + "web_read", + ]); }); QUnit.test("editable one2many list, adding line when only one page", async function (assert) { @@ -6046,7 +6053,7 @@ QUnit.module("Fields", (hooks) => { rec.turtles[0][1], { product_id: [1, "xenomorphe"], - partner_ids: rec.turtles[0][2].partner_ids + partner_ids: rec.turtles[0][2].partner_ids.length ? [ [3, 1], [4, 2], @@ -9619,8 +9626,8 @@ QUnit.module("Fields", (hooks) => { assert.strictEqual( numUserOnchange, - 2, - "there should 2 and only 2 onchange from closing the partner modal" + 1, + "there should 1 and only 1 onchange from closing the partner modal" ); await click(target.querySelector(".o_data_row .o_data_cell")); @@ -10902,6 +10909,7 @@ QUnit.module("Fields", (hooks) => { async mockRPC(route, args) { if (step === 3 && args.method === "onchange2" && args.model === "partner") { assert.deepEqual(args.args[1].turtles[0][2], { + o2m: [], turtle_bar: false, }); } @@ -12470,7 +12478,7 @@ QUnit.module("Fields", (hooks) => { assert.containsNone(target, ".o_kanban_record:not(.o_kanban_ghost)"); await clickSave(target); - assert.verifySteps([]); + assert.verifySteps(["write"]); }); QUnit.test("toggle boolean in o2m with the formView in edition", async function (assert) {