From 4006e346f95bd9fe3b0a4b36dc07b71f4094e0f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Theys?= Date: Fri, 2 Feb 2024 12:45:09 +0100 Subject: [PATCH] [FIX] mail: onAdd/onDelete hooks on many without inverse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- addons/mail/static/src/core/common/record.js | 31 +++++--- addons/mail/static/tests/core/record_tests.js | 75 +++++++++++++++++++ 2 files changed, 95 insertions(+), 11 deletions(-) diff --git a/addons/mail/static/src/core/common/record.js b/addons/mail/static/src/core/common/record.js index bbff9ae8df9..c34253ef5ae 100644 --- a/addons/mail/static/src/core/common/record.js +++ b/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|RecordList} */ 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 */ diff --git a/addons/mail/static/tests/core/record_tests.js b/addons/mail/static/tests/core/record_tests.js index e8f97ebedae..5912966ffc8 100644 --- a/addons/mail/static/tests/core/record_tests.js +++ b/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)); +});