refs #5036 refactor transaction and fixing ticket logs

loopback/common/models/loggable.js

@@ -7,190 +7,154 @@ module.exports = function(Self) {
     };
 
     Self.observe('before save', async function(ctx) {
-        const loopBackContext = LoopBackContext.getCurrentContext();
         const models = ctx.Model.app.models;
         const definition = ctx.Model.definition;
         const Model = models[definition.name];
-        const settings = definition.settings;
         let opts = ctx.options;
         if (!opts) opts = ctx.options = {};
 
-        // Check for transactions
+        // await txBegin(ctx);
-        if (opts.txLevel)
+
-            opts.txLevel++;
+        // try {
-        else if (!opts.transaction) {
+        // await grabUserLog(ctx, 'login');
-            opts.txLevel = 1;
+
-            opts.transaction = await Model.beginTransaction({});
+        let oldInstance;
+        let newInstance;
+        if (ctx.data) {
+            const changes = pick(ctx.currentInstance, Object.keys(ctx.data));
+            newInstance = ctx.data;
+            oldInstance = changes;
+
+            if (ctx.where && !ctx.currentInstance) {
+                const fields = Object.keys(ctx.data);
+
+                ctx.oldInstances = await Model.find({
+                    where: ctx.where,
+                    fields: fields
+                }, opts);
+            }
         }
 
-        try {
+        // Get changes from created instance
-            // Check if grabUser is active
+        if (ctx.isNewInstance)
-            if (loopBackContext && settings.log && settings.log.grabUser) {
+            newInstance = ctx.instance.__data;
-                const userId = loopBackContext.active.accessToken.userId;
-                const user = await models.Account.findById(userId, {fields: ['name']}, opts);
-                await Model.rawSql(`CALL account.myUser_loginWithName(?)`, [user.name], opts);
-            }
 
-            let oldInstance;
+        ctx.hookState.oldInstance = oldInstance;
-            let newInstance;
+        ctx.hookState.newInstance = newInstance;
-            if (ctx.data) {
+        // } catch (e) {
-                const changes = pick(ctx.currentInstance, Object.keys(ctx.data));
+        //     await txEnd(ctx, true);
-                newInstance = ctx.data;
+        // }
-                oldInstance = changes;
-
-                if (ctx.where && !ctx.currentInstance) {
-                    const fields = Object.keys(ctx.data);
-
-                    ctx.oldInstances = await Model.find({
-                        where: ctx.where,
-                        fields: fields
-                    }, opts);
-                }
-            }
-
-            // Get changes from created instance
-            if (ctx.isNewInstance)
-                newInstance = ctx.instance.__data;
-
-            ctx.hookState.oldInstance = oldInstance;
-            ctx.hookState.newInstance = newInstance;
-        } catch (e) {
-            await txEnd(ctx, true);
-        }
     });
 
-    async function txEnd(ctx, undoChanges) {
-        const opts = ctx.options || {};
-        if (opts.txLevel) {
-            opts.txLevel--;
-            if (opts.txLevel === 0) {
-                const tx = opts.transaction;
-                delete opts.txLevel;
-                delete opts.transaction;
-
-                if (undoChanges)
-                    await tx.rollback();
-                else
-                    await tx.commit();
-            }
-        }
-    }
-
     Self.observe('after save', async function(ctx) {
         const loopBackContext = LoopBackContext.getCurrentContext();
-        await logInModel(ctx, loopBackContext);
-    });
-
-    async function logInModel(ctx, loopBackContext) {
         const models = ctx.Model.app.models;
         const definition = ctx.Model.definition;
-        const Model = models[definition.name];
         const settings = ctx.Model.definition.settings;
         const relations = ctx.Model.relations;
 
         let opts = ctx.options;
         if (!opts) opts = ctx.options = {};
 
-        try {
+        // try {
-            let primaryKey;
+        let primaryKey;
-            for (let property in definition.properties) {
+        for (let property in definition.properties) {
-                if (definition.properties[property].id) {
+            if (definition.properties[property].id) {
-                    primaryKey = property;
+                primaryKey = property;
-                    break;
+                break;
-                }
             }
-
-            if (!primaryKey) throw new Error('Primary key not found');
-            let originId;
-
-            // RELATIONS LOG
-            let changedModelId;
-
-            if (ctx.instance && !settings.log.relation) {
-                originId = ctx.instance.id;
-                changedModelId = ctx.instance.id;
-            } else if (settings.log.relation) {
-                primaryKey = relations[settings.log.relation].keyFrom;
-
-                if (ctx.where && ctx.where[primaryKey])
-                    originId = ctx.where[primaryKey];
-                else if (ctx.instance) {
-                    originId = ctx.instance[primaryKey];
-                    changedModelId = ctx.instance.id;
-                }
-            } else {
-                originId = ctx.currentInstance.id;
-                changedModelId = ctx.currentInstance.id;
-            }
-
-            // Sets the changedModelValue to save and the instances changed in case its an updateAll
-            let showField = settings.log.showField;
-            let where;
-            if (showField && (!ctx.instance || !ctx.instance[showField]) && ctx.where) {
-                changedModelId = [];
-                where = [];
-                let changedInstances = await models[definition.name].find({
-                    where: ctx.where,
-                    fields: ['id', showField, primaryKey]
-                }, opts);
-
-                changedInstances.forEach(element => {
-                    where.push(element[showField]);
-                    changedModelId.push(element.id);
-                    originId = element[primaryKey];
-                });
-            } else if (ctx.hookState.oldInstance)
-                where = ctx.instance[showField];
-
-            // Set oldInstance, newInstance, userFk and action
-            let oldInstance = {};
-            if (ctx.hookState.oldInstance)
-                Object.assign(oldInstance, ctx.hookState.oldInstance);
-
-            let newInstance = {};
-            if (ctx.hookState.newInstance)
-                Object.assign(newInstance, ctx.hookState.newInstance);
-
-            let userFk;
-            if (loopBackContext)
-                userFk = loopBackContext.active.accessToken.userId;
-
-            const action = setActionType(ctx);
-
-            removeUnloggable(definition, oldInstance);
-            removeUnloggable(definition, newInstance);
-
-            oldInstance = await fkToValue(oldInstance, ctx);
-            newInstance = await fkToValue(newInstance, ctx);
-
-            // Prevent log with no new changes
-            const hasNewChanges = Object.keys(newInstance).length;
-            if (!hasNewChanges) return;
-
-            let logRecord = {
-                originFk: originId,
-                userFk: userFk,
-                action: action,
-                changedModel: definition.name,
-                changedModelId: changedModelId, // Model property with an different data type will throw a NaN error
-                changedModelValue: where,
-                oldInstance: oldInstance,
-                newInstance: newInstance
-            };
-
-            const logsToSave = setLogsToSave(where, changedModelId, logRecord, ctx);
-            const logModel = settings.log.model;
-            await models[logModel].create(logsToSave, opts);
-
-            // Check if grabUser is active
-            if (settings.log && settings.log.grabUser) await Model.rawSql(`CALL account.myUser_logout()`, null, opts);
-        } catch (e) {
-            await txEnd(ctx, true);
-            throw e;
         }
 
-        await txEnd(ctx);
+        if (!primaryKey) throw new Error('Primary key not found');
-    }
+        let originId;
+
+        // RELATIONS LOG
+        let changedModelId;
+
+        if (ctx.instance && !settings.log.relation) {
+            originId = ctx.instance.id;
+            changedModelId = ctx.instance.id;
+        } else if (settings.log.relation) {
+            primaryKey = relations[settings.log.relation].keyFrom;
+
+            if (ctx.where && ctx.where[primaryKey])
+                originId = ctx.where[primaryKey];
+            else if (ctx.instance) {
+                originId = ctx.instance[primaryKey];
+                changedModelId = ctx.instance.id;
+            }
+        } else {
+            originId = ctx.currentInstance.id;
+            changedModelId = ctx.currentInstance.id;
+        }
+
+        // Sets the changedModelValue to save and the instances changed in case its an updateAll
+        let showField = settings.log.showField;
+        let where;
+        if (showField && (!ctx.instance || !ctx.instance[showField]) && ctx.where) {
+            changedModelId = [];
+            where = [];
+            let changedInstances = await models[definition.name].find({
+                where: ctx.where,
+                fields: ['id', showField, primaryKey]
+            }, opts);
+
+            changedInstances.forEach(element => {
+                where.push(element[showField]);
+                changedModelId.push(element.id);
+                originId = element[primaryKey];
+            });
+        } else if (ctx.hookState.oldInstance)
+            where = ctx.instance[showField];
+
+        // Set oldInstance, newInstance, userFk and action
+        let oldInstance = {};
+        if (ctx.hookState.oldInstance)
+            Object.assign(oldInstance, ctx.hookState.oldInstance);
+
+        let newInstance = {};
+        if (ctx.hookState.newInstance)
+            Object.assign(newInstance, ctx.hookState.newInstance);
+
+        let userFk;
+        if (loopBackContext)
+            userFk = loopBackContext.active.accessToken.userId;
+
+        const action = setActionType(ctx);
+
+        removeUnloggable(definition, oldInstance);
+        removeUnloggable(definition, newInstance);
+
+        oldInstance = await fkToValue(oldInstance, ctx);
+        newInstance = await fkToValue(newInstance, ctx);
+
+        // Prevent log with no new changes
+        const hasNewChanges = Object.keys(newInstance).length;
+        if (!hasNewChanges) return;
+
+        let logRecord = {
+            originFk: originId,
+            userFk: userFk,
+            action: action,
+            changedModel: definition.name,
+            changedModelId: changedModelId, // Model property with an different data type will throw a NaN error
+            changedModelValue: where,
+            oldInstance: oldInstance,
+            newInstance: newInstance
+        };
+
+        const logsToSave = setLogsToSave(where, changedModelId, logRecord, ctx);
+        const logModel = settings.log.model;
+        await models[logModel].create(logsToSave, opts);
+
+        //     await grabUserLog(ctx, 'logout');
+        // } catch (e) {
+        //     await txEnd(ctx, true);
+        //     throw e;
+        // }
+
+        // await txEnd(ctx);
+    });
 
     Self.observe('before delete', async function(ctx) {
         const models = ctx.Model.app.models;
@@ -199,6 +163,11 @@ module.exports = function(Self) {
         let opts = ctx.options;
         if (!opts) opts = ctx.options = {};
 
+        // await txBegin(ctx);
+
+        // try {
+        //     await grabUserLog(ctx, 'login');
+
         if (ctx.where) {
             const affectedModel = definition.name;
             let deletedInstances = await models[affectedModel].find({
@@ -220,43 +189,52 @@ module.exports = function(Self) {
                 ctx.hookState.oldInstance = arrangedDeletedInstances;
             }
         }
+        // } catch (e) {
+        //     await txEnd(ctx, true);
+        // }
     });
 
     Self.observe('after delete', async function(ctx) {
         const loopBackContext = LoopBackContext.getCurrentContext();
-        if (ctx.hookState.oldInstance) logDeletedInstances(ctx, loopBackContext);
-    });
-
-    async function logDeletedInstances(ctx, loopBackContext) {
         const models = ctx.Model.app.models;
         const definition = ctx.Model.definition;
         const settings = definition.settings;
         let opts = ctx.options;
         if (!opts) opts = ctx.options = {};
 
-        ctx.hookState.oldInstance.forEach(async instance => {
+        // try {
-            let userFk;
+        if (ctx.hookState.oldInstance) {
-            if (loopBackContext)
+            ctx.hookState.oldInstance.forEach(async instance => {
-                userFk = loopBackContext.active.accessToken.userId;
+                let userFk;
+                if (loopBackContext)
+                    userFk = loopBackContext.active.accessToken.userId;
 
-            const changedModelValue = settings.log.changedModelValue;
+                const changedModelValue = settings.log.changedModelValue;
-            const logRecord = {
+                const logRecord = {
-                originFk: instance.originFk,
+                    originFk: instance.originFk,
-                userFk: userFk,
+                    userFk: userFk,
-                action: 'delete',
+                    action: 'delete',
-                changedModel: definition.name,
+                    changedModel: definition.name,
-                changedModelId: instance.id,
+                    changedModelId: instance.id,
-                changedModelValue: instance[changedModelValue],
+                    changedModelValue: instance[changedModelValue],
-                oldInstance: instance,
+                    oldInstance: instance,
-                newInstance: {}
+                    newInstance: {}
-            };
+                };
 
-            delete instance.originFk;
+                delete instance.originFk;
 
-            const logModel = settings.log.model;
+                const logModel = settings.log.model;
-            await models[logModel].create(logRecord, opts);
+                await models[logModel].create(logRecord, opts);
-        });
+            });
-    }
+        }
+        //     await grabUserLog(ctx, 'logout');
+        // } catch (e) {
+        //     await txEnd(ctx, true);
+        //     throw e;
+        // }
+
+        // await txEnd(ctx);
+    });
 
     // Get log values from a foreign key
     async function fkToValue(instance, ctx) {
@@ -381,4 +359,69 @@ module.exports = function(Self) {
 
         return 'delete';
     }
+
+    /**
+     * The functions txBegin and txEnd are used to transactionate the loggable.
+     * When a new transaction is created, they add a txLevel because in some cases
+     * the transactions are performed in a recursive way.
+     *
+     * The function grabUserLog check if the option grabUser is active in the Model and
+     * login or logout the user in the database depending on the action.
+     *
+     * Now they are commented out because the way to handle the errors
+     * that occur in the database that leave opened transactions has not been found.
+     * (https://redmine.verdnatura.es/issues/5036)
+    **/
+
+    // async function txBegin(ctx) {
+    //     const opts = ctx.options || {};
+    //     const models = ctx.Model.app.models;
+    //     const definition = ctx.Model.definition;
+    //     const Model = models[definition.name];
+
+    //     if (opts.txLevel)
+    //         opts.txLevel++;
+    //     else if (!opts.transaction) {
+    //         opts.txLevel = 1;
+    //         opts.transaction = await Model.beginTransaction({});
+    //     }
+    // }
+
+    // async function txEnd(ctx, undoChanges) {
+    //     const opts = ctx.options || {};
+
+    //     if (opts.txLevel) {
+    //         opts.txLevel--;
+    //         if (opts.txLevel === 0) {
+    //             const tx = opts.transaction;
+    //             delete opts.txLevel;
+    //             delete opts.transaction;
+
+    //             if (undoChanges)
+    //                 await tx.rollback();
+    //             else
+    //                 await tx.commit();
+    //         }
+    //     }
+    // }
+
+    // async function grabUserLog(ctx, logAction) {
+    //     const opts = ctx.options || {};
+    //     const models = ctx.Model.app.models;
+    //     const definition = ctx.Model.definition;
+    //     const settings = definition.settings;
+    //     const Model = models[definition.name];
+    //     const hasGrabUser = settings.log && settings.log.grabUser;
+
+    //     if (logAction === 'login' && hasGrabUser) {
+    //         const loopBackContext = LoopBackContext.getCurrentContext();
+
+    //         if (loopBackContext) {
+    //             const userId = loopBackContext.active.accessToken.userId;
+    //             const user = await models.Account.findById(userId, {fields: ['name']}, opts);
+    //             await Model.rawSql(`CALL account.myUser_loginWithName(?)`, [user.name], opts);
+    //         }
+    //     } else if (logAction === 'logout' && hasGrabUser)
+    //         await Model.rawSql(`CALL account.myUser_logout()`, null, opts);
+    // }
 };

loopback/util/log.js

@@ -91,7 +91,11 @@ exports.getChanges = (original, changes) => {
         const isPrivate = firstChar == '$';
         if (isPrivate) return;
 
-        if (changes[property] != original[property]) {
+        const hasChanges = original[property] instanceof Date ?
+            changes[property]?.getTime() != original[property]?.getTime() :
+            changes[property] != original[property];
+
+        if (hasChanges) {
             newChanges[property] = changes[property];
 
             if (original[property] != undefined)

modules/ticket/back/methods/ticket/componentUpdate.js

@@ -160,18 +160,29 @@ module.exports = Self => {
                     'shipped',
                     'landed',
                     'isDeleted',
-                    'routeFk'
+                    'routeFk',
+                    'nickname'
                 ],
                 include: [
                     {
                         relation: 'client',
                         scope: {
                             fields: 'salesPersonFk'
-                        }
+                        },
-                    }]
+                        include: [
+                            {
+                                relation: 'address',
+                                scope: {
+                                    fields: 'nickname'
+                                }
+                            }
+                        ]
+                    },
+                ]
             }, myOptions);
 
             args.routeFk = null;
+            if (args.isWithoutNegatives === false) delete args.isWithoutNegatives;
             const updatedTicket = Object.assign({}, args);
             delete updatedTicket.ctx;
             delete updatedTicket.option;