From cd6d54d7bf79074d6c7f58029bbe088f7dcceea6 Mon Sep 17 00:00:00 2001
From: Aaron Bohy <aab@odoo.com>
Date: Tue, 20 Jun 2023 08:09:25 +0000
Subject: [PATCH] [FIX] web: unselect row inside x2many dialog

Have an x2many with an x2many editable list inside the form view
containing at least a row. Open a record, click discard. Open it
again, and click on a row of the x2many to switch it to edit mode.
Before this commit, the row could never be switched back to
readonly. This was the list renderer didn't correctly compute its
active element (it must wait for a micro tick for the mounted hook
of the dialog, its ancestor, to be called).

After fixing this issue, a second one arise: the row can now be
unselected, but it disappears if it is a new record. This is
because we lost the dirty flag when clicking on discard. This is
fixed by storing the dirty state alongside changes in the
savepoint.

Part-of: odoo/odoo#114024
---
 .../static/src/model/relational_model/record.js   | 15 ++++++++-------
 addons/web/static/src/views/list/list_renderer.js |  5 ++++-
 .../tests/views/fields/one2many_field_tests.js    |  8 ++++++++
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/addons/web/static/src/model/relational_model/record.js b/addons/web/static/src/model/relational_model/record.js
index f7ae6685cc9..7133309c35d 100644
--- a/addons/web/static/src/model/relational_model/record.js
+++ b/addons/web/static/src/model/relational_model/record.js
@@ -37,7 +37,6 @@ export class Record extends DataPoint {
         this._unsetRequiredFields = markRaw(new Set());
         this._closeInvalidFieldsNotification = () => {};
 
-        let savePoint;
         const missingFields = this.fieldNames.filter((fieldName) => !(fieldName in data));
         const vals = this._parseServerValues({
             ...this._getDefaultValues(missingFields),
@@ -46,13 +45,14 @@ export class Record extends DataPoint {
         if (this.resId) {
             this._values = markRaw(vals);
             this._changes = markRaw({});
-            savePoint = {};
         } else {
             this._values = markRaw({});
             this._changes = markRaw(vals);
-            savePoint = { ...this._changes };
         }
-        this._savePoint = markRaw(savePoint);
+        this._savePoint = markRaw({
+            dirty: false,
+            changes: { ...this._changes },
+        });
         this.data = { ...this._values, ...this._changes };
 
         const parentRecord = this._parentRecord;
@@ -258,7 +258,8 @@ export class Record extends DataPoint {
     // -------------------------------------------------------------------------
 
     _addSavePoint() {
-        Object.assign(this._savePoint, this._changes);
+        this._savePoint.dirty = this.dirty;
+        Object.assign(this._savePoint.changes, this._changes);
         for (const fieldName in this._changes) {
             if (["one2many", "many2many"].includes(this.fields[fieldName].type)) {
                 this._changes[fieldName]._addSavePoint();
@@ -400,13 +401,13 @@ export class Record extends DataPoint {
     }
 
     _discard() {
-        this.dirty = false;
         for (const fieldName in this._changes) {
             if (["one2many", "many2many"].includes(this.fields[fieldName].type)) {
                 this._changes[fieldName]._discard();
             }
         }
-        this._changes = { ...this._savePoint };
+        this.dirty = this._savePoint.dirty;
+        this._changes = { ...this._savePoint.changes };
         this.data = { ...this._values, ...this._changes };
         this._setEvalContext();
         this._invalidFields.clear();
diff --git a/addons/web/static/src/views/list/list_renderer.js b/addons/web/static/src/views/list/list_renderer.js
index b4e1ee2cd78..df2af077a5e 100644
--- a/addons/web/static/src/views/list/list_renderer.js
+++ b/addons/web/static/src/views/list/list_renderer.js
@@ -110,7 +110,10 @@ export class ListRenderer extends Component {
 
         this.cellToFocus = null;
         this.activeRowId = null;
-        onMounted(() => {
+        onMounted(async () => {
+            // Due to the way elements are mounted in the DOM by Owl (bottom-to-top),
+            // we need to wait the next micro task tick to set the activeElement.
+            await Promise.resolve();
             this.activeElement = this.uiService.activeElement;
         });
         onWillPatch(() => {
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 6da415e82e6..d7ee4d7e313 100644
--- a/addons/web/static/tests/views/fields/one2many_field_tests.js
+++ b/addons/web/static/tests/views/fields/one2many_field_tests.js
@@ -11323,6 +11323,14 @@ QUnit.module("Fields", (hooks) => {
             "second partner turtle is Michelangelo"
         );
         await clickDiscard(target.querySelector(".modal"));
+
+        // re-open, edit michelangelo row, click out -> row still there, in readonly
+        await click(target.querySelectorAll(".o_data_row")[1].querySelector(".o_data_cell"));
+        await click(target.querySelector(".modal .o_data_row .o_data_cell"));
+        assert.containsOnce(target, ".modal .o_selected_row");
+        await click(target.querySelector(".modal"));
+        assert.containsOnce(target, ".modal .o_data_row");
+        assert.strictEqual(target.querySelector(".modal .o_data_cell").innerText, "Michelangelo");
     });
 
     QUnit.test("quickly switch between pages in one2many list", async function (assert) {