From 3faa9ab0a5658d154a336feeb13236a79eaf0ffa Mon Sep 17 00:00:00 2001
From: Aaron Bohy <aab@odoo.com>
Date: Mon, 11 Sep 2023 15:38:18 +0200
Subject: [PATCH] [FIX] web: resequence x2many ordered by id but id not in view

Have a form view with an x2many list with handle field and a
default_order containing id (e.g. sequence,id), but id not being
defined in the list view. Before this commit, on an existing
record, it was impossible to resequence records of the x2many.

The reason is that after resequencing (drag&drop), the model tried
to sort the records, and since the "id" field wasn't in the view,
but was in the default_order, records were reloaded and the changes
(the new sequences) were lost.

The "id" case is special because the field doesn't need to be set
in the view, it is always known by the model. However, it is only
available in record.data if it is defined in the view. This commit
treats the "id" field individually in the sort algorithm.

Bug reported in the new relational model feedback pad.

closes odoo/odoo#135045

Signed-off-by: Francois Georis (fge) <fge@odoo.com>
---
 .../src/model/relational_model/static_list.js |  8 +++-
 .../views/fields/one2many_field_tests.js      | 46 +++++++++++++++++++
 2 files changed, 52 insertions(+), 2 deletions(-)

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 96acaf6369f..4469adfbeaf 100644
--- a/addons/web/static/src/model/relational_model/static_list.js
+++ b/addons/web/static/src/model/relational_model/static_list.js
@@ -19,8 +19,11 @@ function compareFieldValues(v1, v2, fieldType) {
 
 function compareRecords(r1, r2, orderBy, fields) {
     const { name, asc } = orderBy[0];
-    const v1 = asc ? r1.data[name] : r2.data[name];
-    const v2 = asc ? r2.data[name] : r1.data[name];
+    function getValue(record, fieldName) {
+        return fieldName === "id" ? record.resId : record.data[fieldName];
+    }
+    const v1 = asc ? getValue(r1, name) : getValue(r2, name);
+    const v2 = asc ? getValue(r2, name) : getValue(r1, name);
     if (compareFieldValues(v1, v2, fields[name].type)) {
         return -1;
     }
@@ -783,6 +786,7 @@ export class StaticList extends DataPoint {
                 return true;
             }
             // record has already been loaded -> check if we already read all orderBy fields
+            fieldNames = fieldNames.filter((fieldName) => fieldName !== "id");
             return intersection(fieldNames, record.fieldNames).length !== fieldNames.length;
         });
     }
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 7c18bdd939e..cc2e44bd2b9 100644
--- a/addons/web/static/tests/views/fields/one2many_field_tests.js
+++ b/addons/web/static/tests/views/fields/one2many_field_tests.js
@@ -14125,4 +14125,50 @@ QUnit.module("Fields", (hooks) => {
             await click(target.querySelector(".o-autocomplete--dropdown-menu li a"));
         }
     );
+
+    QUnit.test("one2many with default_order on id, but id not in view", async function (assert) {
+        serverData.models.partner.records[0].turtles = [1, 2, 3];
+        await makeView({
+            type: "form",
+            resModel: "partner",
+            serverData,
+            arch: `
+                <form>
+                    <field name="turtles">
+                        <tree editable="top" default_order="turtle_int,id">
+                            <field name="turtle_int" widget="handle"/>
+                            <field name="turtle_foo"/>
+                        </tree>
+                    </field>
+                </form>`,
+            mockRPC(route, args) {
+                assert.step(args.method);
+                if (args.method === "web_save") {
+                    assert.deepEqual(args.args[1].turtles, [
+                        [1, 3, { turtle_int: 0 }],
+                        [1, 1, { turtle_int: 1 }],
+                        [1, 2, { turtle_int: 2 }],
+                    ]);
+                }
+            },
+            resId: 1,
+        });
+
+        assert.deepEqual(getNodesTextContent(target.querySelectorAll(".o_data_cell.o_list_char")), [
+            "yop",
+            "blip",
+            "kawa",
+        ]);
+
+        // drag the third record to top of the list
+        await dragAndDrop("tbody tr:nth-child(3) .o_handle_cell", "tbody tr", "top");
+        await clickSave(target);
+
+        assert.deepEqual(getNodesTextContent(target.querySelectorAll(".o_data_cell.o_list_char")), [
+            "kawa",
+            "yop",
+            "blip",
+        ]);
+        assert.verifySteps(["get_views", "web_read", "web_save"]);
+    });
 });