[FIX] mail: onAdd/onDelete hooks on many without inverse

The opportunity is taken to fix an issue where removing a record from
assign would not update the inverse.

closes odoo/odoo#152442

Signed-off-by: Alexandre Kühn (aku) <aku@odoo.com>

addons/mail/static/src/core/common/record.js

@@ -669,22 +669,31 @@ class RecordList extends Array {
         return Record.MAKE_UPDATE(function recordListAssign() {
             /** @type {Record[]|Set<Record>|RecordList<Record|any[]>} */
             const collection = Record.isRecord(data) ? [data] : data;
-            // l1 and collection could be same record list,
+            // data and collection could be same record list,
             // save before clear to not push mutated recordlist that is empty
             const vals = [...collection];
-            /** @type {R[]} */
-            const oldRecordsProxy = recordList._proxyInternal.slice.call(recordList._proxy);
-            for (const oldRecordProxy of oldRecordsProxy) {
-                toRaw(oldRecordProxy)._raw.__uses__.delete(recordList);
-            }
-            const recordsProxy = vals.map((val) =>
+            const oldRecords = recordList._proxyInternal.slice
+                .call(recordList._proxy)
+                .map((recordProxy) => toRaw(recordProxy)._raw);
+            const newRecords = vals.map((val) =>
                 recordList._insert(val, function recordListAssignInsert(record) {
-                    record.__uses__.add(recordList);
+                    if (record.notIn(oldRecords)) {
+                        record.__uses__.add(recordList);
+                        Record.ADD_QUEUE(recordList.field, "onAdd", record);
+                    }
                 })
             );
-            recordList._proxy.data = recordsProxy.map(
-                (recordProxy) => toRaw(recordProxy)._raw.localId
-            );
+            const inverse = recordList.fieldDefinition.inverse;
+            for (const oldRecord of oldRecords) {
+                if (oldRecord.notIn(newRecords)) {
+                    oldRecord.__uses__.delete(recordList);
+                    Record.ADD_QUEUE(recordList.field, "onDelete", oldRecord);
+                    if (inverse) {
+                        oldRecord._fields.get(inverse).value.delete(recordList.owner);
+                    }
+                }
+            }
+            recordList._proxy.data = newRecords.map((newRecord) => newRecord.localId);
         });
     }
     /** @param {R[]} records */

addons/mail/static/tests/core/record_tests.js

@@ -4,6 +4,7 @@ import { BaseStore, Record, makeStore, modelRegistry } from "@mail/core/common/r
 
 import { registry } from "@web/core/registry";
 import { clearRegistryWithCleanup, makeTestEnv } from "@web/../tests/helpers/mock_env";
+import { assertSteps, step } from "@web/../tests/utils";
 import { markup, reactive, toRaw } from "@odoo/owl";
 
 const serviceRegistry = registry.category("services");
@@ -615,3 +616,77 @@ QUnit.test("store updates can be observed", async (assert) => {
     rawStore.Model.store.abc = 3;
     assert.verifySteps(["abc:3"], "observable from Model.store");
 });
+
+QUnit.test("onAdd/onDelete hooks on one without inverse", async () => {
+    (class Thread extends Record {
+        static id = "name";
+    }).register();
+    (class Member extends Record {
+        static id = "name";
+        name;
+        thread = Record.one("Thread", {
+            onAdd: (thread) => step(`thread.onAdd(${thread.name})`),
+            onDelete: (thread) => step(`thread.onDelete(${thread.name})`),
+        });
+    }).register();
+    const store = await start();
+    const general = store.Thread.insert("General");
+    const john = store.Member.insert("John");
+    await assertSteps([]);
+    john.thread = general;
+    await assertSteps(["thread.onAdd(General)"]);
+    john.thread = general;
+    await assertSteps([]);
+    john.thread = undefined;
+    await assertSteps(["thread.onDelete(General)"]);
+});
+
+QUnit.test("onAdd/onDelete hooks on many without inverse", async () => {
+    (class Thread extends Record {
+        static id = "name";
+        name;
+        members = Record.many("Member", {
+            onAdd: (member) => step(`members.onAdd(${member.name})`),
+            onDelete: (member) => step(`members.onDelete(${member.name})`),
+        });
+    }).register();
+    (class Member extends Record {
+        static id = "name";
+    }).register();
+    const store = await start();
+    const general = store.Thread.insert("General");
+    const jane = store.Member.insert("Jane");
+    const john = store.Member.insert("John");
+    await assertSteps([]);
+    general.members = jane;
+    await assertSteps(["members.onAdd(Jane)"]);
+    general.members = jane;
+    await assertSteps([]);
+    general.members = [["ADD", john]];
+    await assertSteps(["members.onAdd(John)"]);
+    general.members = undefined;
+    await assertSteps(["members.onDelete(John)", "members.onDelete(Jane)"]);
+});
+
+QUnit.test("record list assign should update inverse fields", async (assert) => {
+    (class Thread extends Record {
+        static id = "name";
+        name;
+        members = Record.many("Member", { inverse: "thread" });
+    }).register();
+    (class Member extends Record {
+        static id = "name";
+        thread = Record.one("Thread", { inverse: "members" });
+    }).register();
+    const store = await start();
+    const general = store.Thread.insert("General");
+    const jane = store.Member.insert("Jane");
+    general.members = jane; // direct assignation of value goes through assign()
+    assert.ok(jane.thread.eq(general));
+    general.members = []; // writing empty array specifically goes through assign()
+    assert.notOk(jane.thread);
+    jane.thread = general;
+    assert.ok(jane.in(general.members));
+    jane.thread = [];
+    assert.ok(jane.notIn(general.members));
+});