From 6a83ace2b13e1f59edb05ecb679270c2202affc1 Mon Sep 17 00:00:00 2001 From: Victor Feyens Date: Tue, 27 Feb 2024 13:19:47 +0100 Subject: [PATCH] [FIX] web: x2many field with empty changes In some conditions, a x2many field can be considered as modified by the webclient when in fact there is no change in 'meaningful' stored fields. In this case, on save, an empty list of magic commands will be sent to the server, potentially triggering unexpected recomputations. Steps to reproduce: * install sale_stock & sale_management * create a new storable product * create a new SO * create a new line with this product * Confirm the SO * Set or Update the SO delivery date (Other info tab) * Save -> In the chatter, you will notice an useless tracking value being printed for the SO Total, with identical values before and after update. Cause Since the server received an empty list of commands for the `order_line` field, this triggered a recomputation of the total amounts of the SO, even though there were no effective changes in the lines. Solution Do not send empty command lists for x2m fields. This will avoid unexpected recomputation and also improve performance since the fields were recomputed for 'nothing' closes odoo/odoo#155549 Signed-off-by: Aaron Bohy (aab) Co-authored-by: Aaron Bohy --- .../src/model/relational_model/record.js | 3 + .../views/fields/one2many_field_tests.js | 153 +++++++++++++++++- 2 files changed, 154 insertions(+), 2 deletions(-) diff --git a/addons/web/static/src/model/relational_model/record.js b/addons/web/static/src/model/relational_model/record.js index bcde8abd128..dc9284d448d 100644 --- a/addons/web/static/src/model/relational_model/record.js +++ b/addons/web/static/src/model/relational_model/record.js @@ -602,6 +602,9 @@ export class Record extends DataPoint { } if (field.type === "one2many" || field.type === "many2many") { const commands = value._getCommands({ withReadonly }); + if (!this.isNew && !commands.length && !withReadonly) { + continue; + } result[fieldName] = commands; } else { result[fieldName] = this._formatServerValue(field.type, value); 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 d500f6311d2..119f3ff9cea 100644 --- a/addons/web/static/tests/views/fields/one2many_field_tests.js +++ b/addons/web/static/tests/views/fields/one2many_field_tests.js @@ -4443,7 +4443,7 @@ QUnit.module("Fields", (hooks) => { await clickSave(target); assert.containsNone(target, "tr.o_data_row"); - assert.verifySteps(["get_views", "web_read", "onchange", "onchange", "web_save"]); + assert.verifySteps(["get_views", "web_read", "onchange", "onchange"]); }); QUnit.test("discard O2M field with close button", async function (assert) { @@ -13213,7 +13213,7 @@ QUnit.module("Fields", (hooks) => { assert.containsNone(target, ".o_kanban_record:not(.o_kanban_ghost)"); await clickSave(target); - assert.verifySteps(["web_save"]); + assert.verifySteps([]); }); QUnit.test("toggle boolean in o2m with the formView in edition", async function (assert) { @@ -14509,4 +14509,153 @@ QUnit.module("Fields", (hooks) => { assert.containsN(target, ".o_kanban_record:not(.o_kanban_ghost)", 2); } ); + + QUnit.test( + "onchange on x2many returning an update command with only readonly fields", + async function (assert) { + serverData.models.partner.records[0].turtles = [2]; + serverData.models.turtle.fields.display_name.readonly = true; + serverData.models.partner.onchanges = { + bar: (obj) => { + obj.turtles = [[1, 2, { display_name: "onchange name" }]]; + }, + }; + + await makeView({ + type: "form", + resModel: "partner", + serverData, + arch: ` +
+ + + + + `, + mockRPC(route, args) { + assert.step(args.method); + if (args.method === "web_save") { + assert.deepEqual(args.args[1], { bar: false }); // should not contain turtles + } + }, + resId: 1, + }); + + assert.containsOnce(target, ".o_field_widget[name=turtles] .o_data_row"); + assert.strictEqual(target.querySelector(".o_data_cell").innerText, "donatello"); + + await click(target.querySelector(".o_field_widget[name=bar] input")); + assert.strictEqual(target.querySelector(".o_data_cell").innerText, "onchange name"); + + await clickSave(target); + assert.strictEqual(target.querySelector(".o_data_cell").innerText, "donatello"); + assert.verifySteps(["get_views", "web_read", "onchange", "web_save"]); + } + ); + + QUnit.test( + "onchange on x2many returning a create command with only readonly fields", + async function (assert) { + serverData.models.turtle.fields.display_name.readonly = true; + serverData.models.partner.onchanges = { + bar: (obj) => { + obj.turtles = [[0, false, { display_name: "onchange name" }]]; + }, + }; + + await makeView({ + type: "form", + resModel: "partner", + serverData, + arch: ` +
+ + + + + `, + resId: 1, + mockRPC(route, args) { + assert.step(args.method); + if (args.method === "web_save") { + assert.deepEqual(args.args[1], { + bar: false, + turtles: [[0, args.args[1].turtles[0][1], {}]], + }); + } + }, + }); + + assert.containsOnce(target, ".o_field_widget[name=turtles] .o_data_row"); + assert.strictEqual(target.querySelector(".o_data_cell").innerText, "donatello"); + + await click(target.querySelector(".o_field_widget[name=bar] input")); + assert.containsN(target, ".o_field_widget[name=turtles] .o_data_row", 2); + assert.strictEqual( + target.querySelectorAll(".o_data_cell")[1].innerText, + "onchange name" + ); + + await clickSave(target); + assert.containsN(target, ".o_field_widget[name=turtles] .o_data_row", 2); + assert.verifySteps(["get_views", "web_read", "onchange", "web_save"]); + } + ); + + QUnit.test( + "onchange on x2many add and delete x2m record, returning to initial state", + async function (assert) { + serverData.models.turtle.fields.display_name.readonly = true; + serverData.models.partner.onchanges = { + turtles: function () {}, + }; + + let onchangeCount = 0; + await makeView({ + type: "form", + resModel: "partner", + serverData, + arch: ` +
+ + + + + +
`, + resId: 1, + mockRPC(route, args) { + assert.step(args.method); + if (args.method === "onchange") { + if (onchangeCount === 1) { + // partner turtles onchange for the new x2m record + assert.strictEqual(args.model, 'partner'); + assert.deepEqual(Object.keys(args.args[1]), ["turtles"]); + assert.strictEqual(args.args[1].turtles[0][0], 0); + assert.deepEqual(args.args[2], ['turtles']); + } else if (onchangeCount === 2) { + // x2m record removed, empty list of commands expected + assert.strictEqual(args.model, 'partner'); + assert.deepEqual(Object.keys(args.args[1]), ["turtles"]); + assert.deepEqual(args.args[1].turtles, []); + assert.deepEqual(args.args[2], ['turtles']); + } + onchangeCount++; + } + }, + }); + + assert.containsOnce(target, ".o_field_widget[name=turtles] .o_data_row"); + assert.strictEqual(target.querySelector(".o_data_cell").innerText, "donatello"); + + await addRow(target); + assert.containsN(target, ".o_field_widget[name=turtles] .o_data_row", 2); + await removeRow(target, 1); + assert.containsN(target, ".o_field_widget[name=turtles] .o_data_row", 1); + + await clickSave(target); + assert.containsN(target, ".o_field_widget[name=turtles] .o_data_row", 1); + assert.verifySteps(["get_views", "web_read", "onchange", "onchange", "onchange"]); + } + ); });