[FIX] mail: save JS records per model and use exists if possible

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) <aku@odoo.com>
This commit is contained in:
Sébastien Theys
2020-08-03 15:49:04 +00:00
parent fe029223a5
commit ba03b6c697
7 changed files with 50 additions and 47 deletions
@@ -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();
@@ -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();
}
@@ -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);
}
+2 -4
View File
@@ -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;
}
+33 -32
View File
@@ -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);
}
}
@@ -109,7 +109,7 @@ function factory(dependencies) {
};
const options = {
on_close: () => {
if (!this.constructor.get(this)) {
if (!this.exists()) {
return;
}
this._reset();
+5 -5
View File
@@ -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));