From ba03b6c6977b0fa21c954844f021ed2cc619171f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Theys?= Date: Mon, 3 Aug 2020 13:06:01 +0000 Subject: [PATCH] [FIX] mail: save JS records per model and use exists if possible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This allows get to be simplified to only handle local id. Part of task-2308312 closes odoo/odoo#55346 X-original-commit: fa013d1d29e64706a08499d0f564327766410f8e Related: odoo/enterprise#12159 Signed-off-by: Alexandre Kühn (aku) --- .../static/src/components/dialog/dialog.js | 8 ++- .../messaging_menu/messaging_menu.js | 2 +- .../notification_list/notification_list.js | 4 +- addons/mail/static/src/model/model_field.js | 6 +- addons/mail/static/src/model/model_manager.js | 65 ++++++++++--------- .../static/src/models/composer/composer.js | 2 +- addons/mail/static/src/models/model/model.js | 10 +-- 7 files changed, 50 insertions(+), 47 deletions(-) diff --git a/addons/mail/static/src/components/dialog/dialog.js b/addons/mail/static/src/components/dialog/dialog.js index dc7ddf88e0a..ae6aace8e03 100644 --- a/addons/mail/static/src/components/dialog/dialog.js +++ b/addons/mail/static/src/components/dialog/dialog.js @@ -70,12 +70,16 @@ class Dialog extends Component { * @param {MouseEvent} ev */ _onClickGlobal(ev) { - if (this._componentRef.el.contains(ev.target)) { + if (this._componentRef.el && this._componentRef.el.contains(ev.target)) { return; } // TODO: this should be child logic (will crash if child doesn't have isCloseable!!) // task-2092965 - if (this._componentRef.comp.isCloseable && !this._componentRef.comp.isCloseable()) { + if ( + this._componentRef.comp && + this._componentRef.comp.isCloseable && + !this._componentRef.comp.isCloseable() + ) { return; } this.dialog.close(); diff --git a/addons/mail/static/src/components/messaging_menu/messaging_menu.js b/addons/mail/static/src/components/messaging_menu/messaging_menu.js index b4647d698cc..d705517c8a3 100644 --- a/addons/mail/static/src/components/messaging_menu/messaging_menu.js +++ b/addons/mail/static/src/components/messaging_menu/messaging_menu.js @@ -247,7 +247,7 @@ class MessagingMenu extends Component { */ _onSelectThread(ev) { ev.stopPropagation(); - this.env.models['mail.thread'].get(ev.detail.thread).open(); + ev.detail.thread.open(); if (!this.env.messaging.device.isMobile) { this.messagingMenu.close(); } diff --git a/addons/mail/static/src/components/notification_list/notification_list.js b/addons/mail/static/src/components/notification_list/notification_list.js index 5f714afc6d6..08bcbca4b10 100644 --- a/addons/mail/static/src/components/notification_list/notification_list.js +++ b/addons/mail/static/src/components/notification_list/notification_list.js @@ -53,8 +53,8 @@ class NotificationList extends Component { */ async _loadPreviews() { const threads = this.notifications - .filter(notification => notification.thread) - .map(notification => this.env.models['mail.thread'].get(notification.thread)); + .filter(notification => notification.thread && notification.thread.exists()) + .map(notification => notification.thread); this.env.models['mail.thread'].loadPreviews(threads); } diff --git a/addons/mail/static/src/model/model_field.js b/addons/mail/static/src/model/model_field.js index 54b7881b7fe..4a003d9f24f 100644 --- a/addons/mail/static/src/model/model_field.js +++ b/addons/mail/static/src/model/model_field.js @@ -601,8 +601,7 @@ class ModelField { * @param {string|string[]|mail.model|mail.model[]} newValue */ _setRelationLink(record, newValue) { - const Model = record.constructor; - if (!Model.get(record)) { + if (!record.exists()) { // current record may be deleted due to causality return; } @@ -770,8 +769,7 @@ class ModelField { * @param {string|string[]|mail.model|mail.model[]|null} newValue */ _setRelationUnlink(record, newValue) { - const Model = record.constructor; - if (!Model.get(record)) { + if (!record.exists()) { // current record may be deleted due to causality return; } diff --git a/addons/mail/static/src/model/model_manager.js b/addons/mail/static/src/model/model_manager.js index 732e3923fb2..3b1d52a36e1 100644 --- a/addons/mail/static/src/model/model_manager.js +++ b/addons/mail/static/src/model/model_manager.js @@ -50,10 +50,6 @@ class ModelManager { * processing computes. */ this._isInUpdateCycle = false; - /** - * Contains all records. key is local id, while value is the record. - */ - this._records = {}; /** * Fields flagged to call compute during an update cycle. * For instance, when a field with dependents got update, dependent @@ -97,8 +93,7 @@ class ModelManager { * @returns {mail.model[]} records matching criteria. */ all(Model, filterFunc) { - const allRecords = Object.values(this._records) - .filter(e => e instanceof Model); + const allRecords = Object.values(Model.__records); if (filterFunc) { return allRecords.filter(filterFunc); } @@ -156,7 +151,7 @@ class ModelManager { // Contains revNumber of record for checking record update in useStore. record.__state = 0; - this._records[record.localId] = record; + Model.__records[record.localId] = record; record.init(); // Make default values of its fields for newly created record. @@ -210,7 +205,7 @@ class ModelManager { delete(record) { this._updateCycle(() => { const Model = record.constructor; - if (!this.get(Model, record)) { + if (!record.exists()) { // Record has already been deleted. // (e.g. unlinking one of its reverse relation was causal) return; @@ -238,7 +233,7 @@ class ModelManager { data[relation.fieldName] = [['unlink-all']]; } record.update(data); - delete this._records[record.localId]; + delete Model.__records[record.localId]; }); } @@ -247,8 +242,10 @@ class ModelManager { */ deleteAll() { this._updateCycle(() => { - for (const record of Object.values(this._records)) { - record.delete(); + for (const Model of Object.values(this.env.models)) { + for (const record of Object.values(Model.__records)) { + record.delete(); + } } }); } @@ -261,7 +258,7 @@ class ModelManager { * @returns {boolean} */ exists(Model, record) { - return this._records[record.localId] ? true : false; + return Model.__records[record.localId] ? true : false; } /** @@ -279,29 +276,30 @@ class ModelManager { /** * This method returns the record of provided model that matches provided - * record/local id. Useful to convert a local id to a record, and also to - * determine whether the record is still "alive" (i.e. not deleted). Note - * that even if there's a record in the system having provided local id, if - * the resulting record is not an instance of this model, this getter + * local id. Useful to convert a local id to a record. + * Note that even if there's a record in the system having provided local + * id, if the resulting record is not an instance of this model, this getter * assumes the record does not exist. * * @param {mail.model} Model class - * @param {string|mail.model|undefined} recordOrLocalId + * @param {string} localId * @returns {mail.model|undefined} record, if exists */ - get(Model, recordOrLocalId) { - if (recordOrLocalId === undefined) { - return undefined; - } - const record = this._records[ - recordOrLocalId instanceof this.env.models['mail.model'] - ? recordOrLocalId.localId - : recordOrLocalId - ]; - if (!(record instanceof Model)) { + get(Model, localId) { + if (!localId) { return; } - return record; + const record = Model.__records[localId]; + if (record) { + return record; + } + // support for inherited models (eg. relation targeting `mail.model`) + for (const Model of Object.values(this.env.models)) { + const record = Model.__records[localId]; + if (record) { + return record; + } + } } /** @@ -676,6 +674,10 @@ class ModelManager { // Make environment accessible from Model. const Model = generatable.factory(Models); Model.env = this.env; + /** + * Contains all records. key is local id, while value is the record. + */ + Model.__records = {}; for (const patch of generatable.patches) { switch (patch.type) { case 'class': @@ -689,7 +691,7 @@ class ModelManager { break; } } - if (!Model.hasOwnProperty('modelName')) { + if (!Object.prototype.hasOwnProperty.call(Model, 'modelName')) { throw new Error(`Missing static property "modelName" on Model class "${Model.name}".`); } if (generatedNames.includes(Model.modelName)) { @@ -761,7 +763,7 @@ class ModelManager { * 1. Prepare fields. */ for (const Model of Object.values(Models)) { - if (!Model.hasOwnProperty('fields')) { + if (!Object.prototype.hasOwnProperty.call(Model, 'fields')) { Model.fields = {}; } Model.inverseRelations = []; @@ -922,8 +924,7 @@ class ModelManager { while (this._toUpdateAfters.size > 0) { for (const [record, previous] of this._toUpdateAfters) { this._toUpdateAfters.delete(record); - const RecordToUpdateModel = record.constructor; - if (this.get(RecordToUpdateModel, record)) { + if (record.exists()) { record._updateAfter(previous); } } diff --git a/addons/mail/static/src/models/composer/composer.js b/addons/mail/static/src/models/composer/composer.js index 78e1ad4e505..91d53a6e6b0 100644 --- a/addons/mail/static/src/models/composer/composer.js +++ b/addons/mail/static/src/models/composer/composer.js @@ -109,7 +109,7 @@ function factory(dependencies) { }; const options = { on_close: () => { - if (!this.constructor.get(this)) { + if (!this.exists()) { return; } this._reset(); diff --git a/addons/mail/static/src/models/model/model.js b/addons/mail/static/src/models/model/model.js index 10b17d6795e..c7e4e4f2641 100644 --- a/addons/mail/static/src/models/model/model.js +++ b/addons/mail/static/src/models/model/model.js @@ -88,11 +88,11 @@ function factory() { * assumes the record does not exist. * * @static - * @param {string|mail.model|undefined} recordOrLocalId + * @param {string} localId * @returns {mail.model|undefined} */ - static get(recordOrLocalId) { - return this.env.modelManager.get(this, recordOrLocalId); + static get(localId) { + return this.env.modelManager.get(this, localId); } /** @@ -123,13 +123,13 @@ function factory() { async async(func) { return new Promise((resolve, reject) => { Promise.resolve(func()).then(result => { - if (this.constructor.get(this)) { + if (this.exists()) { resolve(result); } else { reject(new RecordDeletedError(this.localId)); } }).catch(error => { - if (this.constructor.get(this)) { + if (this.exists()) { reject(error); } else { reject(new RecordDeletedError(this.localId));